Hello Nico Huber,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/43065
to review the following change.
Change subject: mb/lenovo/t60: Fix override devicetrees ......................................................................
mb/lenovo/t60: Fix override devicetrees
Commit c1dc2d5e68 (mb/lenovo/t60: Switch to override tree) converted these boards to override trees, but some device nodes were missed. Said nodes are essential, as `chip` configuration data is always tied to device nodes. The resulting `static.c` contained multiple copies of the `chip` configuration structs, but the wrong ones were hooked up.
Change-Id: I6c26d633d1860cf9a5415994444e75ae1c2e59ad Signed-off-by: Nico Huber nico.h@gmx.de --- M src/mainboard/lenovo/t60/variants/t60/overridetree.cb M src/mainboard/lenovo/t60/variants/z61t/overridetree.cb 2 files changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/43065/1
diff --git a/src/mainboard/lenovo/t60/variants/t60/overridetree.cb b/src/mainboard/lenovo/t60/variants/t60/overridetree.cb index eee3a4d..551fff3 100644 --- a/src/mainboard/lenovo/t60/variants/t60/overridetree.cb +++ b/src/mainboard/lenovo/t60/variants/t60/overridetree.cb @@ -44,6 +44,7 @@ register "has_bdc_detection" = "1" register "bdc_gpio_num" = "7" register "bdc_gpio_lvl" = "0" + device pnp ff.2 on end end chip superio/nsc/pc87384 device pnp 2e.2 off # Serial Port / IR @@ -62,6 +63,7 @@ register "regs" = "{ 0x2e, 0xf7, 0x3c, 0x20, 0x01, 0x00, 0x1b, 0x01, 0x54, 0xff, 0xff, 0x07 }" + device i2c 69 on end end end end diff --git a/src/mainboard/lenovo/t60/variants/z61t/overridetree.cb b/src/mainboard/lenovo/t60/variants/z61t/overridetree.cb index d29df3b..c372b18 100644 --- a/src/mainboard/lenovo/t60/variants/z61t/overridetree.cb +++ b/src/mainboard/lenovo/t60/variants/z61t/overridetree.cb @@ -57,6 +57,7 @@ # vendor clockgen setup register "regs" = "{ 0x6d, 0xff, 0xff, 0x20, 0x41, 0x7f, 0x18, 0x00 }" + device i2c 69 on end end end end
Hello build bot (Jenkins), Nico Huber, Alexander Couzens, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43065
to look at the new patch set (#2).
Change subject: mb/lenovo/t60: Fix override devicetrees ......................................................................
mb/lenovo/t60: Fix override devicetrees
Commit c1dc2d5e68 (mb/lenovo/t60: Switch to override tree) converted these boards to override trees, but some device nodes were missed. Said nodes are essential, as `chip` configuration data is always tied to device nodes. The resulting `static.c` contained multiple copies of the `chip` configuration structs, but the wrong ones were hooked up.
The therefore missing configuration of the clockgen led to general instability, especially with SMP under Linux (probably due to the attempt to enter lower C states on an idle core). Passing `maxcpus=1` to the Linux kernel served as a workaround.
Change-Id: I6c26d633d1860cf9a5415994444e75ae1c2e59ad Signed-off-by: Nico Huber nico.h@gmx.de --- M src/mainboard/lenovo/t60/variants/t60/overridetree.cb M src/mainboard/lenovo/t60/variants/z61t/overridetree.cb 2 files changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/43065/2
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43065 )
Change subject: mb/lenovo/t60: Fix override devicetrees ......................................................................
Patch Set 3: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/43065/3/src/mainboard/lenovo/t60/va... File src/mainboard/lenovo/t60/variants/t60/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43065/3/src/mainboard/lenovo/t60/va... PS3, Line 59: chip drivers/i2c/ck505 can we drop this from the base devicetree?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43065 )
Change subject: mb/lenovo/t60: Fix override devicetrees ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43065/3/src/mainboard/lenovo/t60/va... File src/mainboard/lenovo/t60/variants/t60/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43065/3/src/mainboard/lenovo/t60/va... PS3, Line 59: chip drivers/i2c/ck505
can we drop this from the base devicetree?
I've no idea. I just cherry-picked this change from master to 4.11_branch
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43065 )
Change subject: mb/lenovo/t60: Fix override devicetrees ......................................................................
Patch Set 3: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/43065/3/src/mainboard/lenovo/t60/va... File src/mainboard/lenovo/t60/variants/t60/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/43065/3/src/mainboard/lenovo/t60/va... PS3, Line 59: chip drivers/i2c/ck505
I've no idea. I just cherry-picked this change from master to 4.11_branch
Oh right, I missed that.
Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43065 )
Change subject: mb/lenovo/t60: Fix override devicetrees ......................................................................
mb/lenovo/t60: Fix override devicetrees
Commit c1dc2d5e68 (mb/lenovo/t60: Switch to override tree) converted these boards to override trees, but some device nodes were missed. Said nodes are essential, as `chip` configuration data is always tied to device nodes. The resulting `static.c` contained multiple copies of the `chip` configuration structs, but the wrong ones were hooked up.
The therefore missing configuration of the clockgen led to general instability, especially with SMP under Linux (probably due to the attempt to enter lower C states on an idle core). Passing `maxcpus=1` to the Linux kernel served as a workaround.
Change-Id: I6c26d633d1860cf9a5415994444e75ae1c2e59ad Signed-off-by: Nico Huber nico.h@gmx.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/43065 Reviewed-by: Arthur Heymans arthur@aheymans.xyz Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/lenovo/t60/variants/t60/overridetree.cb M src/mainboard/lenovo/t60/variants/z61t/overridetree.cb 2 files changed, 3 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved
diff --git a/src/mainboard/lenovo/t60/variants/t60/overridetree.cb b/src/mainboard/lenovo/t60/variants/t60/overridetree.cb index eee3a4d..551fff3 100644 --- a/src/mainboard/lenovo/t60/variants/t60/overridetree.cb +++ b/src/mainboard/lenovo/t60/variants/t60/overridetree.cb @@ -44,6 +44,7 @@ register "has_bdc_detection" = "1" register "bdc_gpio_num" = "7" register "bdc_gpio_lvl" = "0" + device pnp ff.2 on end end chip superio/nsc/pc87384 device pnp 2e.2 off # Serial Port / IR @@ -62,6 +63,7 @@ register "regs" = "{ 0x2e, 0xf7, 0x3c, 0x20, 0x01, 0x00, 0x1b, 0x01, 0x54, 0xff, 0xff, 0x07 }" + device i2c 69 on end end end end diff --git a/src/mainboard/lenovo/t60/variants/z61t/overridetree.cb b/src/mainboard/lenovo/t60/variants/z61t/overridetree.cb index d29df3b..c372b18 100644 --- a/src/mainboard/lenovo/t60/variants/z61t/overridetree.cb +++ b/src/mainboard/lenovo/t60/variants/z61t/overridetree.cb @@ -57,6 +57,7 @@ # vendor clockgen setup register "regs" = "{ 0x6d, 0xff, 0xff, 0x20, 0x41, 0x7f, 0x18, 0x00 }" + device i2c 69 on end end end end
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)