Change in coreboot[master]: mb/lenovo/t60: Fix override devicetrees

[Nico Huber (Code Review)]

Hello Peter Lemenkov, Angel Pons, Arthur Heymans,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/43150

to review the following change.

Change subject: mb/lenovo/t60: Fix override devicetrees
......................................................................

mb/lenovo/t60: Fix override devicetrees

When converting to override trees in c1dc2d5e68 (mb/lenovo/t60:
Switch to override tree), some device nodes were missed. These
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 
---
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/50/43150/1

diff --git a/src/mainboard/lenovo/t60/variants/t60/overridetree.cb b/src/mainboard/lenovo/t60/variants/t60/overridetree.cb
index dc6abfb..c58884a 100644
--- a/src/mainboard/lenovo/t60/variants/t60/overridetree.cb
+++ b/src/mainboard/lenovo/t60/variants/t60/overridetree.cb
@@ -29,6 +29,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
@@ -47,6 +48,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 9bac582..00f3f19 100644
--- a/src/mainboard/lenovo/t60/variants/z61t/overridetree.cb
+++ b/src/mainboard/lenovo/t60/variants/z61t/overridetree.cb
@@ -42,6 +42,7 @@
    				# vendor clockgen setup
    				register "regs" = "{ 0x6d, 0xff, 0xff,
    					0x20, 0x41, 0x7f, 0x18, 0x00 }"
+					device i2c 69 on end
    			end
    		end
    	end

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43150 )

Change subject: mb/lenovo/t60: Fix override devicetrees
......................................................................

Patch Set 1: Code-Review+2

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43150 )

Change subject: mb/lenovo/t60: Fix override devicetrees
......................................................................

Patch Set 1: Code-Review+2

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43150 )

Change subject: mb/lenovo/t60: Fix override devicetrees
......................................................................

Patch Set 1: Code-Review+1

(1 comment)

Two fix-ups for regressions in one day. Thank you Nico.

(I am unable to test though.)

https://review.coreboot.org/c/coreboot/+/43150/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/43150/1//COMMIT_MSG@15 
PS1, Line 15: 
Could you also describe the problem, that Linux needed to be booted with `maxcpus=1`?

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43150 )

Change subject: mb/lenovo/t60: Fix override devicetrees
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/43150/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/43150/1//COMMIT_MSG@9 
PS1, Line 9: c1dc2d5e68
If you add the word *commit* before the hash, Gerrit will mark this up as a link.

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43150 )

Change subject: mb/lenovo/t60: Fix override devicetrees
......................................................................

Patch Set 1:

Should this also be fixed in *4.11_branch*?

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43150 )

Change subject: mb/lenovo/t60: Fix override devicetrees
......................................................................

Patch Set 1:
> Patch Set 1:
>
> Should this also be fixed in *4.11_branch*?

There's a cherry-pick button. I wonder what it does.

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43150 )

Change subject: mb/lenovo/t60: Fix override devicetrees
......................................................................

Patch Set 1:
> Patch Set 1:
> > Patch Set 1:
> >
> > Should this also be fixed in *4.11_branch*?
>
> There's a cherry-pick button. I wonder what it does.

Poof! CB:43065 happened.

[Nico Huber (Code Review)]

Hello build bot (Jenkins), Peter Lemenkov, Paul Menzel, Angel Pons, Arthur Heymans, Alexander Couzens, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/43150

to look at the new patch set (#2).

Change subject: mb/lenovo/t60: Fix override devicetrees
......................................................................

mb/lenovo/t60: Fix override devicetrees

When converting to override trees in commit c1dc2d5e68 (mb/lenovo/t60:
Switch to override tree), some device nodes were missed. These 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 clock gen 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 
---
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/50/43150/2

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43150 )

Change subject: mb/lenovo/t60: Fix override devicetrees
......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/43150/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/43150/1//COMMIT_MSG@9 
PS1, Line 9: c1dc2d5e68
> If you add the word *commit* before the hash, Gerrit will mark this up as a link.

Done

https://review.coreboot.org/c/coreboot/+/43150/1//COMMIT_MSG@15 
PS1, Line 15:
> Could you also describe the problem, that Linux needed to be booted with `maxcpus=1`?

Done

[Nico Huber (Code Review)]

Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43150 )

Change subject: mb/lenovo/t60: Fix override devicetrees
......................................................................

mb/lenovo/t60: Fix override devicetrees

When converting to override trees in commit c1dc2d5e68 (mb/lenovo/t60:
Switch to override tree), some device nodes were missed. These 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 clock gen 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 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/43150
Reviewed-by: Arthur Heymans 
Reviewed-by: Angel Pons 
Reviewed-by: Paul Menzel 
Tested-by: build bot (Jenkins) 
---
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
  Paul Menzel: Looks good to me, but someone else must approve
  Arthur Heymans: Looks good to me, approved
  Angel Pons: 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 dc6abfb..c58884a 100644
--- a/src/mainboard/lenovo/t60/variants/t60/overridetree.cb
+++ b/src/mainboard/lenovo/t60/variants/t60/overridetree.cb
@@ -29,6 +29,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
@@ -47,6 +48,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 9bac582..00f3f19 100644
--- a/src/mainboard/lenovo/t60/variants/z61t/overridetree.cb
+++ b/src/mainboard/lenovo/t60/variants/z61t/overridetree.cb
@@ -42,6 +42,7 @@
    				# vendor clockgen setup
    				register "regs" = "{ 0x6d, 0xff, 0xff,
    					0x20, 0x41, 0x7f, 0x18, 0x00 }"
+					device i2c 69 on end
    			end
    		end
    	end