Michael Niewöhner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35524 )
Change subject: mb/supermicro/x11: remove vendor id config ......................................................................
mb/supermicro/x11: remove vendor id config
The vendor id option is useless here as the SSVID is set to the default value from the PCI Cfg register anyway. Besides that the Kconfig option isn't meant for retrofit ports (see 7e1c83). The right place would be the devicetree.
Signed-off-by: Michael Niewöhner foss@mniewoehner.de Change-Id: If67c679bb342f63096902535734106e4f1651118 --- M src/mainboard/supermicro/x11-lga1151-series/Kconfig 1 file changed, 0 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/35524/1
diff --git a/src/mainboard/supermicro/x11-lga1151-series/Kconfig b/src/mainboard/supermicro/x11-lga1151-series/Kconfig index 28b3495..3aaf1d3 100644 --- a/src/mainboard/supermicro/x11-lga1151-series/Kconfig +++ b/src/mainboard/supermicro/x11-lga1151-series/Kconfig @@ -66,10 +66,6 @@ int default 8
-config SUBSYSTEM_VENDOR_ID - hex - default 0x8086 - config CONSOLE_POST bool default y
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35524 )
Change subject: mb/supermicro/x11: remove vendor id config ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35524/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35524/1//COMMIT_MSG@9 PS1, Line 9: the default : value from the PCI Cfg This seems only true for Intel devices?
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35524 )
Change subject: mb/supermicro/x11: remove vendor id config ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35524/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35524/1//COMMIT_MSG@9 PS1, Line 9: the default : value from the PCI Cfg
This seems only true for Intel devices?
well, we're speaking of an intel device here; do you like this to be said here? "... anyway for Intel devices"?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35524 )
Change subject: mb/supermicro/x11: remove vendor id config ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35524/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35524/1//COMMIT_MSG@9 PS1, Line 9: the default : value from the PCI Cfg
well, we're speaking of an intel device here; do you like this to be said here? "... […]
No, we're speaking of all PCI devices that are on or somebody could plug onto the board, at least AFAICS.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35524 )
Change subject: mb/supermicro/x11: remove vendor id config ......................................................................
Patch Set 1:
(1 comment)
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35524/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35524/1//COMMIT_MSG@9 PS1, Line 9: the default : value from the PCI Cfg
No, we're speaking of all PCI devices that are on or somebody could […]
Sorry, I don't really get what you want me to do...
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35524 )
Change subject: mb/supermicro/x11: remove vendor id config ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35524/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35524/1//COMMIT_MSG@9 PS1, Line 9: the default : value from the PCI Cfg
Sorry, I don't really get what you want me to do...
In one sentence: don't write confusing commit messages.
You could write "Most SSVID registers are filled with 0x8086 (their VID) by default[, anyway]."
I have no idea why that 8086 was there in the Kconfig, btw. It's best to talk to the authors, I don't know if it serves any purpose.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35524 )
Change subject: mb/supermicro/x11: remove vendor id config ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35524/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35524/1//COMMIT_MSG@9 PS1, Line 9: the default : value from the PCI Cfg
In one sentence: don't write confusing commit messages. […]
this was requested in the initial CR but neither did they give an explanation nor did they change anything... Additionally I always add those guys as reviewer but no one (except Patrick) responds in any way...
I will update the commit msg.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35524 )
Change subject: mb/supermicro/x11: remove vendor id config ......................................................................
Patch Set 2: Code-Review+2
Hello Patrick Rudolph, Angel Pons, Philipp Deppenwiese, build bot (Jenkins), Nico Huber, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35524
to look at the new patch set (#3).
Change subject: mb/supermicro/x11: remove unneeded vendor id config ......................................................................
mb/supermicro/x11: remove unneeded vendor id config
The vendor id option set here is useless as most SSVID registers are filled with 0x8086 (their VID) by default, anyway.
Besides that the Kconfig option isn't meant for retrofit ports (see 7e1c83). The right place would be the devicetree.
Signed-off-by: Michael Niewöhner foss@mniewoehner.de Change-Id: If67c679bb342f63096902535734106e4f1651118 --- M src/mainboard/supermicro/x11-lga1151-series/Kconfig 1 file changed, 0 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/35524/3
Hello Patrick Rudolph, Angel Pons, Philipp Deppenwiese, build bot (Jenkins), Nico Huber, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35524
to look at the new patch set (#4).
Change subject: mb/supermicro/x11: remove unneeded vendor id config ......................................................................
mb/supermicro/x11: remove unneeded vendor id config
The vendor id option set here is useless as most SSVID registers are filled with 0x8086 (their VID) by default, anyway.
Besides that the Kconfig option isn't meant for retrofit ports (see 7e1c83). The right place would be the devicetree.
Signed-off-by: Michael Niewöhner foss@mniewoehner.de Change-Id: If67c679bb342f63096902535734106e4f1651118 --- M src/mainboard/supermicro/x11-lga1151-series/Kconfig 1 file changed, 0 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/35524/4
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35524 )
Change subject: mb/supermicro/x11: remove unneeded vendor id config ......................................................................
Patch Set 3: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/35524/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35524/3//COMMIT_MSG@13 PS3, Line 13: 7e1c83). The right place would be the devicetree. Please use:
… ports, cf commit 7e1c83e3 (Add Kconfig options to override Subsystem Vendor and Device ID).
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35524 )
Change subject: mb/supermicro/x11: remove unneeded vendor id config ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35524/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35524/3//COMMIT_MSG@13 PS3, Line 13: 7e1c83). The right place would be the devicetree.
Please use: […]
what is cf?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35524 )
Change subject: mb/supermicro/x11: remove unneeded vendor id config ......................................................................
Patch Set 7: Code-Review+2
(3 comments)
https://review.coreboot.org/c/coreboot/+/35524/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35524/1//COMMIT_MSG@9 PS1, Line 9: the default : value from the PCI Cfg
this was requested in the initial CR but neither did they give an explanation nor did they change an […]
Ack
https://review.coreboot.org/c/coreboot/+/35524/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35524/3//COMMIT_MSG@13 PS3, Line 13: 7e1c83). The right place would be the devicetree.
what is cf?
short for `confer`, actually: cf.
Usually, one specifies at least 7 chars of the commit hash. In any case git would be right, e.g.: git log --format='%h (%s)'
https://review.coreboot.org/c/coreboot/+/35524/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35524/7//COMMIT_MSG@9 PS7, Line 9: The vendor id option set here is useless as most SSVID registers are filled NB. Usual Git commit-message line limit is 72 chars (some stupid exception allows 75 in coreboot, but that's not encouraged).
Hello Patrick Rudolph, Angel Pons, Paul Menzel, Philipp Deppenwiese, build bot (Jenkins), Nico Huber, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35524
to look at the new patch set (#9).
Change subject: mb/supermicro/x11: remove unneeded vendor id config ......................................................................
mb/supermicro/x11: remove unneeded vendor id config
The vendor id option set here is useless as most SSVID registers get filled with 0x8086 (their VID) by default, anyway.
Besides that the Kconfig option isn't meant for retrofit ports (see 7e1c83). The right place would be the devicetree.
Signed-off-by: Michael Niewöhner foss@mniewoehner.de Change-Id: If67c679bb342f63096902535734106e4f1651118 --- M src/mainboard/supermicro/x11-lga1151-series/Kconfig 1 file changed, 0 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/35524/9
Hello Patrick Rudolph, Angel Pons, Paul Menzel, Philipp Deppenwiese, build bot (Jenkins), Nico Huber, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35524
to look at the new patch set (#10).
Change subject: mb/supermicro/x11: remove unneeded vendor id config ......................................................................
mb/supermicro/x11: remove unneeded vendor id config
The vendor id option set here is useless as most SSVID registers get filled with 0x8086 (their VID) by default, anyway.
Besides that the Kconfig option isn't meant for retrofit ports (see 7e1c83). The right place would be the devicetree.
Signed-off-by: Michael Niewöhner foss@mniewoehner.de Change-Id: If67c679bb342f63096902535734106e4f1651118 --- M src/mainboard/supermicro/x11-lga1151-series/Kconfig 1 file changed, 0 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/35524/10
Hello Patrick Rudolph, Angel Pons, Paul Menzel, Philipp Deppenwiese, build bot (Jenkins), Nico Huber, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35524
to look at the new patch set (#11).
Change subject: mb/supermicro/x11: remove unneeded vendor id config ......................................................................
mb/supermicro/x11: remove unneeded vendor id config
The vendor id option set here is useless as most SSVID registers get filled with 0x8086 (their VID) by default, anyway.
Besides that the Kconfig option isn't meant for retrofit ports (see 7e1c83). The right place would be the devicetree.
Signed-off-by: Michael Niewöhner foss@mniewoehner.de Change-Id: If67c679bb342f63096902535734106e4f1651118 --- M src/mainboard/supermicro/x11-lga1151-series/Kconfig 1 file changed, 0 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/35524/11
Hello Patrick Rudolph, Angel Pons, Paul Menzel, Philipp Deppenwiese, build bot (Jenkins), Nico Huber, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35524
to look at the new patch set (#12).
Change subject: mb/supermicro/x11: remove unneeded vendor id config ......................................................................
mb/supermicro/x11: remove unneeded vendor id config
The vendor id option set here is useless as most SSVID registers get filled with 0x8086 (their VID) by default, anyway.
Besides that the Kconfig option isn't meant for retrofit ports (see 7e1c83). The right place would be the devicetree.
Signed-off-by: Michael Niewöhner foss@mniewoehner.de Change-Id: If67c679bb342f63096902535734106e4f1651118 --- M src/mainboard/supermicro/x11-lga1151-series/Kconfig 1 file changed, 0 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/35524/12
Hello Patrick Rudolph, Angel Pons, Paul Menzel, Philipp Deppenwiese, build bot (Jenkins), Nico Huber, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35524
to look at the new patch set (#13).
Change subject: mb/supermicro/x11: remove unneeded vendor id config ......................................................................
mb/supermicro/x11: remove unneeded vendor id config
The vendor id option set here is useless as most SSVID registers get filled with 0x8086 (their VID) by default, anyway.
Besides that the Kconfig option isn't meant for retrofit ports, cf. commit 7e1c83e31bd (Add Kconfig options to override Subsystem Vendor and Device ID). The right place would be the devicetree.
Signed-off-by: Michael Niewöhner foss@mniewoehner.de Change-Id: If67c679bb342f63096902535734106e4f1651118 --- M src/mainboard/supermicro/x11-lga1151-series/Kconfig 1 file changed, 0 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/35524/13
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35524 )
Change subject: mb/supermicro/x11: remove unneeded vendor id config ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35524/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35524/3//COMMIT_MSG@13 PS3, Line 13: 7e1c83). The right place would be the devicetree.
short for `confer`, actually: cf. […]
Done
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35524 )
Change subject: mb/supermicro/x11: remove unneeded vendor id config ......................................................................
Patch Set 13:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35524/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35524/7//COMMIT_MSG@9 PS7, Line 9: The vendor id option set here is useless as most SSVID registers are filled
NB. Usual Git commit-message line limit is 72 chars (some stupid exception […]
\o/ I had col limit set to 72, wtf :D will be corrected
Hello Patrick Rudolph, Angel Pons, Paul Menzel, Philipp Deppenwiese, build bot (Jenkins), Nico Huber, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35524
to look at the new patch set (#14).
Change subject: mb/supermicro/x11-lga1151-series: remove unneeded vendor id config ......................................................................
mb/supermicro/x11-lga1151-series: remove unneeded vendor id config
The vendor id option set here is useless as most SSVID registers get filled with 0x8086 (their VID) by default, anyway.
Besides that the Kconfig option isn't meant for retrofit ports, cf. commit 7e1c83e31bd (Add Kconfig options to override Subsystem Vendor and Device ID). The right place would be the devicetree.
Signed-off-by: Michael Niewöhner foss@mniewoehner.de Change-Id: If67c679bb342f63096902535734106e4f1651118 --- M src/mainboard/supermicro/x11-lga1151-series/Kconfig 1 file changed, 0 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/35524/14
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35524 )
Change subject: mb/supermicro/x11-lga1151-series: remove unneeded vendor id config ......................................................................
Patch Set 14: Code-Review+2
Patrick Rudolph has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35524 )
Change subject: mb/supermicro/x11-lga1151-series: remove unneeded vendor id config ......................................................................
mb/supermicro/x11-lga1151-series: remove unneeded vendor id config
The vendor id option set here is useless as most SSVID registers get filled with 0x8086 (their VID) by default, anyway.
Besides that the Kconfig option isn't meant for retrofit ports, cf. commit 7e1c83e31bd (Add Kconfig options to override Subsystem Vendor and Device ID). The right place would be the devicetree.
Signed-off-by: Michael Niewöhner foss@mniewoehner.de Change-Id: If67c679bb342f63096902535734106e4f1651118 Reviewed-on: https://review.coreboot.org/c/coreboot/+/35524 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Patrick Rudolph siro@das-labor.org --- M src/mainboard/supermicro/x11-lga1151-series/Kconfig 1 file changed, 0 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Rudolph: Looks good to me, approved
diff --git a/src/mainboard/supermicro/x11-lga1151-series/Kconfig b/src/mainboard/supermicro/x11-lga1151-series/Kconfig index 54be4e0..6fc9aed 100644 --- a/src/mainboard/supermicro/x11-lga1151-series/Kconfig +++ b/src/mainboard/supermicro/x11-lga1151-series/Kconfig @@ -64,10 +64,6 @@ int default 8
-config SUBSYSTEM_VENDOR_ID - hex - default 0x8086 - config CONSOLE_POST bool default y
-
Michael Niewöhner (Code Review) -
Nico Huber (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)