Hello Felix Singer, Elyes HAOUAS, build bot (Jenkins), Nico Huber, Martin Roth, Patrick Georgi, Paul Menzel, Duncan Laurie, Stefan Reinauer,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45513
to look at the new patch set (#2).
Change subject: lint: check for misuse of Kconfig SUBSYSTEM_*_ID
......................................................................
lint: check for misuse of Kconfig SUBSYSTEM_*_ID
Check that nobody misuses the Kconfigs SUBSYSTEM_*_ID. They are meant to
be used for overriding the devicetree subsystem ids locally but shall
not be added to a board's Kconfig. Instead, the devicetree option
`subsystemid` should be used.
Add a linter script for this that finds and warns about such misuse.
Also add a note in the Kconfigs' description.
Change-Id: I21c021c718154f1396f795a555af47a76d6efe03
Signed-off-by: Michael Niewöhner <foss(a)mniewoehner.de>
---
M src/device/Kconfig
M util/lint/check_lint_tests
A util/lint/lint-stable-024-kconfig-no-subsystem
3 files changed, 26 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/45513/2
--
To view, visit https://review.coreboot.org/c/coreboot/+/45513
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I21c021c718154f1396f795a555af47a76d6efe03
Gerrit-Change-Number: 45513
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: Elyes HAOUAS
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: newpatchset
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45513 )
Change subject: lint: check for misuse of Kconfig SUBSYSTEM_*_ID
......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45513/1/util/lint/check_lint_tests
File util/lint/check_lint_tests:
https://review.coreboot.org/c/coreboot/+/45513/1/util/lint/check_lint_tests…
PS1, Line 31: lint
use lint-stable instead so that it runs on the builder?
https://review.coreboot.org/c/coreboot/+/45513/1/util/lint/check_lint_tests…
PS1, Line 31: 009
Since this is a new test, the number should be the next number in the lint list - currently 24.
--
To view, visit https://review.coreboot.org/c/coreboot/+/45513
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I21c021c718154f1396f795a555af47a76d6efe03
Gerrit-Change-Number: 45513
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org>
Gerrit-Reviewer: Elyes HAOUAS
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Fri, 18 Sep 2020 14:59:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42134 )
Change subject: src/include: Add PnP/HWM update functions
......................................................................
src/include: Add PnP/HWM update functions
RMW (read/modify/write) ops on PnP devices has never been so simple.
Change-Id: Ica01211af2a9a00aed98880844a836f6b7957b14
Signed-off-by: Angel Pons <th3fanbus(a)gmail.com>
---
M src/include/device/pnp.h
M src/include/device/pnp_ops.h
M src/include/superio/hwm5_conf.h
3 files changed, 51 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/42134/1
diff --git a/src/include/device/pnp.h b/src/include/device/pnp.h
index 800bcc0..158d40e 100644
--- a/src/include/device/pnp.h
+++ b/src/include/device/pnp.h
@@ -133,4 +133,29 @@
outb(value, port + 1);
}
+/*
+ * void pnp_update_index(u16 port, u8 reg, u8 mask, u8 or)
+ * Description:
+ * This routine updates indexed I/O registers. The reg byte is written
+ * to the index register at I/O address = port. The value is then read
+ * from the data register at I/O address = port + 1. This value is ANDed
+ * with the mask value and then ORed with the or value. Finally, the
+ * new value is written to the data register at I/O address = port + 1.
+ *
+ * Parameters:
+ * @param[in] u16 port = The address of the port index register.
+ * @param[in] u8 reg = The offset within the indexed space.
+ * @param[in] u8 mask = The mask value to apply to the data register value.
+ * @param[in] u8 or = The or value to apply to the data register value.
+ */
+static inline void pnp_update_index(u16 port, u8 reg, u8 mask, u8 or)
+{
+ outb(reg, port);
+
+ uint8_t value = inb(port + 1);
+ value &= mask;
+ value |= or;
+ outb(value, port + 1);
+}
+
#endif /* DEVICE_PNP_H */
diff --git a/src/include/device/pnp_ops.h b/src/include/device/pnp_ops.h
index 0cfdd61..783c3ad 100644
--- a/src/include/device/pnp_ops.h
+++ b/src/include/device/pnp_ops.h
@@ -22,6 +22,12 @@
return pnp_read_index(dev >> 8, reg);
}
+static __always_inline void pnp_update_config(
+ pnp_devfn_t dev, uint8_t reg, uint8_t mask, uint8_t or)
+{
+ pnp_update_index(dev >> 8, reg, mask, or);
+}
+
static __always_inline
void pnp_set_logical_device(pnp_devfn_t dev)
{
diff --git a/src/include/superio/hwm5_conf.h b/src/include/superio/hwm5_conf.h
index 661f3ee..5e9a142 100644
--- a/src/include/superio/hwm5_conf.h
+++ b/src/include/superio/hwm5_conf.h
@@ -44,4 +44,24 @@
pnp_write_index(base + 5, reg, value);
}
+/*
+ * void pnp_update_hwm5_index(u16 base, u8 reg, u8 mask, u8 or)
+ * Description:
+ * This routine updates indexed I/O registers. The reg byte is written
+ * to the index register at I/O address = port + 5. The value is then read
+ * from the data register at I/O address = port + 6. This value is ANDed
+ * with the mask value and then ORed with the or value. Finally, the
+ * new value is written to the data register at I/O address = port + 6.
+ *
+ * Parameters:
+ * @param[in] u16 port = The address of the port index register.
+ * @param[in] u8 reg = The offset within the indexed space.
+ * @param[in] u8 mask = The mask value to apply to the data register value.
+ * @param[in] u8 or = The or value to apply to the data register value.
+ */
+static inline void pnp_update_hwm5_index(u16 base, u8 reg, u8 mask, u8 or)
+{
+ pnp_update_index(base + 5, reg, mask, or);
+}
+
#endif /* DEVICE_PNP_HWM5_CONF_H */
--
To view, visit https://review.coreboot.org/c/coreboot/+/42134
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ica01211af2a9a00aed98880844a836f6b7957b14
Gerrit-Change-Number: 42134
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newchange
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45417 )
Change subject: soc/intel: cnl,icl,elh,jsl,tgl: enable C6DRAM
......................................................................
Patch Set 6:
> > > What about adding a Kconfig (default n) instead?
> >
> > How about removing the option entirely? We already have
> > enough unused options ;)
>
> Huh? Remove an option just because some ucode breaks it? This is a power-saving option we definitely should *not* remove entirely.
It was unused, hence we would effectively not remove any functionality.
And yes, it's a power-saving option. That means without some business
backup, it will likely never be really tested. People start testing and
say "it boots". But this option has no effect on booting at all. If
you wanted to test this on a single unit with a single ucode revision,
I'd suggest that you use this unit for a few years without making any
more changes just to see if it is stable ;) If it trips once per year
and somebody would sell 1,000 units, that would mean roughly 3 units
trip per day. So really, we don't have the means to test this properly.
If you can find some documentation about when it is safe to use, it
would be a completely different story, IMHO. But until then, why present
an option that would need a warning in the help text?
--
To view, visit https://review.coreboot.org/c/coreboot/+/45417
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I556dba59bc06c9101bdfdfd6aee00610aac516e3
Gerrit-Change-Number: 45417
Gerrit-PatchSet: 6
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: Alex Levin <levinale(a)google.com>
Gerrit-Reviewer: Andrew McRae <amcrae(a)google.com>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com>
Gerrit-Reviewer: Anil Kumar K <anil.kumar.k(a)intel.com>
Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org>
Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com>
Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Elyes HAOUAS
Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: John Su <john_su(a)compal.corp-partner.google.com>
Gerrit-Reviewer: Jonas Löffelholz <jonas.loeffelholz(a)9elements.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Lijian Zhao <lijian.zhao(a)intel.com>
Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com>
Gerrit-Reviewer: Maulik V Vaghela <maulik.v.vaghela(a)intel.com>
Gerrit-Reviewer: Nathaniel L Desimone <nathaniel.l.desimone(a)intel.com>
Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Fagerburg <pfagerburg(a)google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com>
Gerrit-Reviewer: Shaunak Saha <shaunak.saha(a)intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Thejaswani Putta <thejaswani.putta(a)intel.com>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Tim Chen <tim-chen(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org>
Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Fri, 18 Sep 2020 08:47:41 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45491 )
Change subject: mb/system76/lemp9: drop disabled options from devicetree
......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45491/1//COMMIT_MSG
Commit Message:
https://review.coreboot.org/c/coreboot/+/45491/1//COMMIT_MSG@7
PS1, Line 7: registers
> options
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/45491
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I2a1a91778e83dc49c6dcf2d518cd3591f7ec4cfa
Gerrit-Change-Number: 45491
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Niewöhner <foss(a)mniewoehner.de>
Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Comment-Date: Fri, 18 Sep 2020 08:21:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net>
Gerrit-MessageType: comment