Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42141 )
Change subject: ec/google/wilco: Fix comment about enclosure type ......................................................................
ec/google/wilco: Fix comment about enclosure type
SYSTEM_TYPE_CONVERTIBLE is not valid SMBIOS enclosure type, but selecting it implies SMBIOS_ENCLOSURE_CONVERTIBLE.
Change-Id: Ib658af7b80586428b22f08a738964637e1fbd17a Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/ec/google/wilco/acpi/vbtn.asl 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/42141/1
diff --git a/src/ec/google/wilco/acpi/vbtn.asl b/src/ec/google/wilco/acpi/vbtn.asl index 6227367..bc9dd34 100644 --- a/src/ec/google/wilco/acpi/vbtn.asl +++ b/src/ec/google/wilco/acpi/vbtn.asl @@ -5,7 +5,7 @@ * the Linux kernel at drivers/platform/x86/intel-vbtn.c * * For tablet/laptop and dock/undock events to work the board must - * select SYSTEM_TYPE_CONVERTIBLE for the SMBIOS enclosure type to + * have SMBIOS_ENCLOSURE_CONVERTIBLE for the SMBIOS enclosure type to * indicate the device is a convertible. */
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42141 )
Change subject: ec/google/wilco: Fix comment about enclosure type ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42141/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42141/1//COMMIT_MSG@7 PS1, Line 7: Fix The comment was not wrong per se, as long as selecting SYSTEM_TYPE_CONVERTIBLE resulted in the SMBIOS enclosure type being SMBIOS_ENCLOSURE_CONVERTIBLE. Is that true?
https://review.coreboot.org/c/coreboot/+/42141/1/src/ec/google/wilco/acpi/vb... File src/ec/google/wilco/acpi/vbtn.asl:
https://review.coreboot.org/c/coreboot/+/42141/1/src/ec/google/wilco/acpi/vb... PS1, Line 8: * have SMBIOS_ENCLOSURE_CONVERTIBLE for the SMBIOS enclosure type to IMHO, this makes it too tempting to manually set the SMBIOS enclosure type for this code to work. If we want people to select SYSTEM_TYPE_CONVERTIBLE instead, the original wording would be more appropriate.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42141 )
Change subject: ec/google/wilco: Fix comment about enclosure type ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42141/1/src/ec/google/wilco/acpi/vb... File src/ec/google/wilco/acpi/vbtn.asl:
https://review.coreboot.org/c/coreboot/+/42141/1/src/ec/google/wilco/acpi/vb... PS1, Line 8: * have SMBIOS_ENCLOSURE_CONVERTIBLE for the SMBIOS enclosure type to
IMHO, this makes it too tempting to manually set the SMBIOS enclosure type for this code to work. […]
My proposal is to drop SYSTEM_TYPE_xx Kconfig.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42141 )
Change subject: ec/google/wilco: Fix comment about enclosure type ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42141/1/src/ec/google/wilco/acpi/vb... File src/ec/google/wilco/acpi/vbtn.asl:
https://review.coreboot.org/c/coreboot/+/42141/1/src/ec/google/wilco/acpi/vb... PS1, Line 8: * have SMBIOS_ENCLOSURE_CONVERTIBLE for the SMBIOS enclosure type to
My proposal is to drop SYSTEM_TYPE_xx Kconfig.
Right, that is news to me. Since most others would be in a similar situation, explain in the commit message that the plan is to drop the SYSTEM_TYPE_xx Kconfig options?
I'd also like to know about the rationale behind this decision.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42141 )
Change subject: ec/google/wilco: Fix comment about enclosure type ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42141/1/src/ec/google/wilco/acpi/vb... File src/ec/google/wilco/acpi/vbtn.asl:
https://review.coreboot.org/c/coreboot/+/42141/1/src/ec/google/wilco/acpi/vb... PS1, Line 8: * have SMBIOS_ENCLOSURE_CONVERTIBLE for the SMBIOS enclosure type to
Right, that is news to me. […]
It's not a decision, a proposal. Let's see what comments appear in CB:42142.
Kyösti Mälkki has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/42141 )
Change subject: ec/google/wilco: Fix comment about enclosure type ......................................................................
Abandoned
Kyösti Mälkki has restored this change. ( https://review.coreboot.org/c/coreboot/+/42141 )
Change subject: ec/google/wilco: Fix comment about enclosure type ......................................................................
Restored
Attention is currently required from: Kyösti Mälkki. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42141 )
Change subject: ec/google/wilco: Fix comment about enclosure type ......................................................................
Patch Set 5: Code-Review+1
Attention is currently required from: Kyösti Mälkki. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42141 )
Change subject: ec/google/wilco: Fix comment about enclosure type ......................................................................
Patch Set 5: Code-Review+2
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/42141/comment/a7c1e20c_3111d60f PS1, Line 7: Fix
The comment was not wrong per se, as long as selecting SYSTEM_TYPE_CONVERTIBLE resulted in the SMBIO […]
Ack
File src/ec/google/wilco/acpi/vbtn.asl:
https://review.coreboot.org/c/coreboot/+/42141/comment/9d94aef6_b0741127 PS1, Line 8: * have SMBIOS_ENCLOSURE_CONVERTIBLE for the SMBIOS enclosure type to
It's not a decision, a proposal. Let's see what comments appear in CB:42142.
Ack
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42141 )
Change subject: ec/google/wilco: Fix comment about enclosure type ......................................................................
ec/google/wilco: Fix comment about enclosure type
SYSTEM_TYPE_CONVERTIBLE is not valid SMBIOS enclosure type, but selecting it implies SMBIOS_ENCLOSURE_CONVERTIBLE.
Change-Id: Ib658af7b80586428b22f08a738964637e1fbd17a Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/42141 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Paul Menzel paulepanter@mailbox.org Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/ec/google/wilco/acpi/vbtn.asl 1 file changed, 1 insertion(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Angel Pons: Looks good to me, approved
diff --git a/src/ec/google/wilco/acpi/vbtn.asl b/src/ec/google/wilco/acpi/vbtn.asl index 6227367..bc9dd34 100644 --- a/src/ec/google/wilco/acpi/vbtn.asl +++ b/src/ec/google/wilco/acpi/vbtn.asl @@ -5,7 +5,7 @@ * the Linux kernel at drivers/platform/x86/intel-vbtn.c * * For tablet/laptop and dock/undock events to work the board must - * select SYSTEM_TYPE_CONVERTIBLE for the SMBIOS enclosure type to + * have SMBIOS_ENCLOSURE_CONVERTIBLE for the SMBIOS enclosure type to * indicate the device is a convertible. */
-
Angel Pons (Code Review) -
Kyösti Mälkki (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)