Change in coreboot[master]: mb/asrock/h110m: remove unused Device4Enable from devtree

[Maxim Polyakov (Code Review)]

Maxim Polyakov has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44300 )

Change subject: mb/asrock/h110m: remove unused Device4Enable from devtree
......................................................................

mb/asrock/h110m: remove unused Device4Enable from devtree

This option has been removed from the parameters structure for intel
skylake CPU.

Change-Id: I9dc6649ad693d18fdc85046ebbcc730a17fed0bf
Signed-off-by: Maxim Polyakov 
---
M src/mainboard/asrock/h110m/devicetree.cb
1 file changed, 1 insertion(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/44300/1

diff --git a/src/mainboard/asrock/h110m/devicetree.cb b/src/mainboard/asrock/h110m/devicetree.cb
index 68ef2a4..f75fdc8 100644
--- a/src/mainboard/asrock/h110m/devicetree.cb
+++ b/src/mainboard/asrock/h110m/devicetree.cb
@@ -152,9 +152,7 @@
    	device pci 02.0 on # Integrated Graphics Device
    		subsystemid 0x1849 0x1912
    	end
-		device pci 04.0 on      # Thermal Subsystem
-			register "Device4Enable" = "1"
-		end
+		device pci 04.0 on  and # Thermal Subsystem
    	device pci 08.0 off end # Gaussian Mixture Model
    	device pci 14.0 on # USB xHCI
    		subsystemid 0x1849 0xa131

[Maxim Polyakov (Code Review)]

Maxim Polyakov has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/44300 )

Change subject: mb/asrock/h110m: remove unused Device4Enable from devtree
......................................................................

mb/asrock/h110m: remove unused Device4Enable from devtree

This option has been removed from the parameters structure for intel
skylake CPU.

Change-Id: I9dc6649ad693d18fdc85046ebbcc730a17fed0bf
Signed-off-by: Maxim Polyakov 
---
M src/mainboard/asrock/h110m/devicetree.cb
1 file changed, 1 insertion(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/44300/2

[Maxim Polyakov (Code Review)]

Hello build bot (Jenkins),

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

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

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

Change subject: mb/asrock/h110m: remove unused Device4Enable from devtree
......................................................................

mb/asrock/h110m: remove unused Device4Enable from devtree

This option has been removed from the parameters structure for Intel
Skylake CPU (commit 9c1c009).

Change-Id: I9dc6649ad693d18fdc85046ebbcc730a17fed0bf
Signed-off-by: Maxim Polyakov 
---
M src/mainboard/asrock/h110m/devicetree.cb
1 file changed, 1 insertion(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/44300/3

[Felix Singer (Code Review)]

Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44300 )

Change subject: mb/asrock/h110m: remove unused Device4Enable from devtree
......................................................................

Patch Set 3:

There is already CB:43079.

[Maxim Polyakov (Code Review)]

Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44300 )

Change subject: mb/asrock/h110m: remove unused Device4Enable from devtree
......................................................................

Patch Set 3:
> Patch Set 3:
>
> There is already CB:43079.

But not for this board

[Felix Singer (Code Review)]

Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44300 )

Change subject: mb/asrock/h110m: remove unused Device4Enable from devtree
......................................................................

Patch Set 3: Code-Review+2

CB:43079 reverts the patch, which is causing the chaos.

But I'm also fine with this one. So I give this a 2+.

[Maxim Polyakov (Code Review)]

Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44300 )

Change subject: mb/asrock/h110m: remove unused Device4Enable from devtree
......................................................................

Patch Set 3:
> Patch Set 3: Code-Review+2
>
> CB:43079 reverts the patch, which is causing the chaos.

There is no need to do revert these changes. Let's just add this for asrock.

[Maxim Polyakov (Code Review)]

Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44300 )

Change subject: mb/asrock/h110m: remove unused Device4Enable from devtree
......................................................................

Patch Set 3:
> Patch Set 3:
> > Patch Set 3: Code-Review+2
> >
> > CB:43079 reverts the patch, which is causing the chaos.
>
> There is no need to do revert these changes. Let's just add this for asrock.

Oh sorry, I didn't see https://review.coreboot.org/c/coreboot/+/44297

[Felix Singer (Code Review)]

Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44300 )

Change subject: mb/asrock/h110m: remove unused Device4Enable from devtree
......................................................................

Patch Set 3:
> Patch Set 3:
> > Patch Set 3:
> > > Patch Set 3: Code-Review+2
> > >
> > > CB:43079 reverts the patch, which is causing the chaos.
> >
> > There is no need to do revert these changes. Let's just add this for asrock.
>
> Oh sorry, I didn't see https://review.coreboot.org/c/coreboot/+/44297

No problem. I abandoned it, because I just want to get any patch merged so that master is fixed.

[Maxim Polyakov (Code Review)]

Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44300 )

Change subject: mb/asrock/h110m: remove unused Device4Enable from devtree
......................................................................

Patch Set 3:
> Patch Set 3:
> > Patch Set 3:
> >
> > There is already CB:43079.
>
> But not for this board

And please ignore this my comment, I'm a little confused here :)

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44300 )

Change subject: mb/asrock/h110m: remove unused Device4Enable from devtree
......................................................................

Patch Set 3: Code-Review+2

[Michael Niewöhner (Code Review)]

Michael Niewöhner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44300 )

Change subject: mb/asrock/h110m: remove unused Device4Enable from devtree
......................................................................

mb/asrock/h110m: remove unused Device4Enable from devtree

This option has been removed from the parameters structure for Intel
Skylake CPU (commit 9c1c009).

Change-Id: I9dc6649ad693d18fdc85046ebbcc730a17fed0bf
Signed-off-by: Maxim Polyakov 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44300
Tested-by: build bot (Jenkins) 
Reviewed-by:  Felix Singer 
Reviewed-by: Michael Niewöhner
---
M src/mainboard/asrock/h110m/devicetree.cb
1 file changed, 1 insertion(+), 3 deletions(-)

Approvals:
  build bot (Jenkins): Verified
   Felix Singer: Looks good to me, approved
  Michael Niewöhner: Looks good to me, approved

diff --git a/src/mainboard/asrock/h110m/devicetree.cb b/src/mainboard/asrock/h110m/devicetree.cb
index 68ef2a4..6de2a63 100644
--- a/src/mainboard/asrock/h110m/devicetree.cb
+++ b/src/mainboard/asrock/h110m/devicetree.cb
@@ -152,9 +152,7 @@
    	device pci 02.0 on # Integrated Graphics Device
    		subsystemid 0x1849 0x1912
    	end
-		device pci 04.0 on      # Thermal Subsystem
-			register "Device4Enable" = "1"
-		end
+		device pci 04.0 on  end # Thermal Subsystem
    	device pci 08.0 off end # Gaussian Mixture Model
    	device pci 14.0 on # USB xHCI
    		subsystemid 0x1849 0xa131

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44300 )

Change subject: mb/asrock/h110m: remove unused Device4Enable from devtree
......................................................................

Patch Set 4:

Landed early to unbreak master.

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44300 )

Change subject: mb/asrock/h110m: remove unused Device4Enable from devtree
......................................................................

Patch Set 4:

Given that we just recently revised our fast track rules after heated discussions came up around fast-tracking changes, I would have appreciated a somewhat different handling here, mostly on two points:

1. 3 CR+2 for fast track merging, this has two
2. A more detailed commit message making it clearer that this commit is about fixing a regression on master and that the change is trivial (in as many words) and how that regression came to be (a devicetree symbol was removed in cb:xxxx that duplicated devicetree behavior and that this board still used. cb:xxxx passed because ... but failed due to merge issues)

Luckily it's _really_ limited impact (a single board's device tree, targetting a symbol that was simply removed), so not much harm done, but please be more careful in the future.

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44300 )

Change subject: mb/asrock/h110m: remove unused Device4Enable from devtree
......................................................................

Patch Set 4:

Addendum: but despite the criticism, thanks for fixing master :-)