Andrew McRae has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35466 )
Change subject: drivers/wifi/generic.c: Use upper case for ACPI WiFi name ......................................................................
drivers/wifi/generic.c: Use upper case for ACPI WiFi name
The ACPI specification requires upper case letters for the device name. For Intel WiFi names, the name is generated, so ensure that only upper case letters are used.
Change-Id: I803b9bc81804eec7bd5220b9dbc6ddd0bb0ecbcc Signed-off-by: Andrew McRae amcrae@chromium.org --- M src/drivers/wifi/generic.c 1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/35466/1
diff --git a/src/drivers/wifi/generic.c b/src/drivers/wifi/generic.c index b593ffe..bb225b7 100644 --- a/src/drivers/wifi/generic.c +++ b/src/drivers/wifi/generic.c @@ -239,7 +239,8 @@ { static char wifi_acpi_name[WIFI_ACPI_NAME_MAX_LEN];
- snprintf(wifi_acpi_name, sizeof(wifi_acpi_name), "WF%02x", + /* ACPI specification requires uppercase name */ + snprintf(wifi_acpi_name, sizeof(wifi_acpi_name), "WF%02X", (dev_path_encode(dev) & 0xff)); return wifi_acpi_name; }
Andrew McRae has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35466 )
Change subject: drivers/wifi/generic.c: Use upper case for ACPI WiFi name ......................................................................
Patch Set 1:
Thanks for the review!
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35466 )
Change subject: drivers/wifi/generic.c: Use upper case for ACPI WiFi name ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35466/1/src/drivers/wifi/generic.c File src/drivers/wifi/generic.c:
https://review.coreboot.org/c/coreboot/+/35466/1/src/drivers/wifi/generic.c@... PS1, Line 242: ACPI specification I think it is helpful to cite specifically where in the ACPI specification the particular requirement you refer to applies here.
For example, `ACPI 6.3, ASL 19.2.2: Requires that 'Name' be of the form [..].` is less ambiguous, assuming that is part of the grammar you are referring to here, or maybe 19.2.4?
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35466 )
Change subject: drivers/wifi/generic.c: Use upper case for ACPI WiFi name ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35466/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35466/1//COMMIT_MSG@7 PS1, Line 7: Use upper case for ACPI WiFi name 'Uppercase the ASL Name as required by spec'
ACPI is the overall standard however we are talking about the generated ASL here.
https://review.coreboot.org/c/coreboot/+/35466/1//COMMIT_MSG@9 PS1, Line 9: The ACPI specification requires upper case letters for the device name. trim this indent and include the specific part of the spec to which you refer.
Hello Edward O'Callaghan, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35466
to look at the new patch set (#2).
Change subject: drivers/wifi/generic.c: Upper case the AML WiFi name as required by spec ......................................................................
drivers/wifi/generic.c: Upper case the AML WiFi name as required by spec
ACPI specification Version 6.0 (Section 20.2.2 Name Objects Encoding), LeadNameChar and NameChar support 'A'-'Z' only and not 'a'-'z'. For Intel WiFi names, the name is generated, so ensure that only upper case letters are used.
Change-Id: I803b9bc81804eec7bd5220b9dbc6ddd0bb0ecbcc Signed-off-by: Andrew McRae amcrae@chromium.org --- M src/drivers/wifi/generic.c 1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/35466/2
Andrew McRae has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35466 )
Change subject: drivers/wifi/generic.c: Upper case the AML WiFi name as required by spec ......................................................................
Patch Set 2:
Patch Set 1:
(2 comments)
OK done
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35466 )
Change subject: drivers/wifi/generic.c: Upper case the AML WiFi name as required by spec ......................................................................
Patch Set 2:
(2 comments)
Patch Set 2:
Patch Set 1:
(2 comments)
OK done
not quite see other comments ;)
https://review.coreboot.org/c/coreboot/+/35466/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35466/2//COMMIT_MSG@9 PS2, Line 9: ACPI specification Version 6.0 (Section 20.2.2 Name Objects Encoding), Again, remove indent on commit msg body.
Something like;
``` The ACPI 6.3; ASL 20.2.2 (Name Objects Encoding), states that;
LeadNameChar := ‘A’-‘Z’ | ‘_’ NameChar := DigitChar | LeadNameChar
Hence, the Intel WiFi names generated in ASL are required to be upper-cased letters. ```
states very precisely what is happening here.
https://review.coreboot.org/c/coreboot/+/35466/2/src/drivers/wifi/generic.c File src/drivers/wifi/generic.c:
https://review.coreboot.org/c/coreboot/+/35466/2/src/drivers/wifi/generic.c@... PS2, Line 242: /* ACPI specification requires uppercase name */ either
/* ACPI 6.3; ASL 20.2.2 (Name Objects Encoding). */
or,
/* ACPI 6.3, ASL 20.2.2: Requires that 'Name' be of the form `0`-`9` | ‘A’-‘Z’ | ‘_’. */
pick one.
Hello Edward O'Callaghan, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35466
to look at the new patch set (#3).
Change subject: drivers/wifi/generic.c: Upper case the AML WiFi name as required by spec ......................................................................
drivers/wifi/generic.c: Upper case the AML WiFi name as required by spec
ACPI 6.3, ASL 20.2.2 (Name Objects Encoding) states:
LeadNameChar := 'A'-'Z' | '_' NameChar := DigitChar | LeadNameChar
Hence, the Intel WiFi names generated in ASL are required to be upper-cased letters.
Change-Id: I803b9bc81804eec7bd5220b9dbc6ddd0bb0ecbcc Signed-off-by: Andrew McRae amcrae@chromium.org --- M src/drivers/wifi/generic.c 1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/35466/3
Andrew McRae has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35466 )
Change subject: drivers/wifi/generic.c: Upper case the AML WiFi name as required by spec ......................................................................
Patch Set 3:
Patch Set 2:
(2 comments)
Patch Set 2:
Patch Set 1:
(2 comments)
OK done
not quite see other comments ;)
OK done
Hello Edward O'Callaghan, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35466
to look at the new patch set (#4).
Change subject: drivers/wifi/generic.c: Upper case the AML WiFi name as required by spec ......................................................................
drivers/wifi/generic.c: Upper case the AML WiFi name as required by spec
ACPI 6.3, ASL 20.2.2 (Name Objects Encoding) states:
LeadNameChar := 'A'-'Z' | '_' NameChar := DigitChar | LeadNameChar
Hence, the Intel WiFi names generated in ASL are required to be upper-cased letters.
Change-Id: I803b9bc81804eec7bd5220b9dbc6ddd0bb0ecbcc Signed-off-by: Andrew McRae amcrae@chromium.org --- M src/drivers/wifi/generic.c 1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/35466/4
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35466 )
Change subject: drivers/wifi/generic.c: Upper case the AML WiFi name as required by spec ......................................................................
Patch Set 4: Code-Review+2
Andrew McRae has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35466 )
Change subject: drivers/wifi/generic.c: Upper case the AML WiFi name as required by spec ......................................................................
Patch Set 4:
(5 comments)
https://review.coreboot.org/c/coreboot/+/35466/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35466/1//COMMIT_MSG@7 PS1, Line 7: Use upper case for ACPI WiFi name
'Uppercase the ASL Name as required by spec' […]
Ack
https://review.coreboot.org/c/coreboot/+/35466/1//COMMIT_MSG@9 PS1, Line 9: The ACPI specification requires upper case letters for the device name.
trim this indent and include the specific part of the spec to which you refer.
Ack
https://review.coreboot.org/c/coreboot/+/35466/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35466/2//COMMIT_MSG@9 PS2, Line 9: ACPI specification Version 6.0 (Section 20.2.2 Name Objects Encoding),
Again, remove indent on commit msg body. […]
Done
https://review.coreboot.org/c/coreboot/+/35466/1/src/drivers/wifi/generic.c File src/drivers/wifi/generic.c:
https://review.coreboot.org/c/coreboot/+/35466/1/src/drivers/wifi/generic.c@... PS1, Line 242: ACPI specification
I think it is helpful to cite specifically where in the ACPI specification the particular requiremen […]
Done
https://review.coreboot.org/c/coreboot/+/35466/2/src/drivers/wifi/generic.c File src/drivers/wifi/generic.c:
https://review.coreboot.org/c/coreboot/+/35466/2/src/drivers/wifi/generic.c@... PS2, Line 242: /* ACPI specification requires uppercase name */
either […]
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35466 )
Change subject: drivers/wifi/generic.c: Upper case the AML WiFi name as required by spec ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35466/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35466/4//COMMIT_MSG@16 PS4, Line 16: BUG=b:141206986 TEST=
Hello Edward O'Callaghan, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35466
to look at the new patch set (#5).
Change subject: drivers/wifi/generic.c: Upper case the AML WiFi name as required by spec ......................................................................
drivers/wifi/generic.c: Upper case the AML WiFi name as required by spec
ACPI 6.3, ASL 20.2.2 (Name Objects Encoding) states:
LeadNameChar := 'A'-'Z' | '_' NameChar := DigitChar | LeadNameChar
Hence, the Intel WiFi names generated in ASL are required to be upper-cased letters.
BUG=b:141206986 TEST=Reflash and confirmed SSDT table has correct name.
Change-Id: I803b9bc81804eec7bd5220b9dbc6ddd0bb0ecbcc Signed-off-by: Andrew McRae amcrae@chromium.org --- M src/drivers/wifi/generic.c 1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/35466/5
Andrew McRae has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35466 )
Change subject: drivers/wifi/generic.c: Upper case the AML WiFi name as required by spec ......................................................................
Patch Set 5:
(1 comment)
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35466/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35466/4//COMMIT_MSG@16 PS4, Line 16:
BUG=b:141206986 […]
Done
Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35466 )
Change subject: drivers/wifi/generic.c: Upper case the AML WiFi name as required by spec ......................................................................
drivers/wifi/generic.c: Upper case the AML WiFi name as required by spec
ACPI 6.3, ASL 20.2.2 (Name Objects Encoding) states:
LeadNameChar := 'A'-'Z' | '_' NameChar := DigitChar | LeadNameChar
Hence, the Intel WiFi names generated in ASL are required to be upper-cased letters.
BUG=b:141206986 TEST=Reflash and confirmed SSDT table has correct name.
Change-Id: I803b9bc81804eec7bd5220b9dbc6ddd0bb0ecbcc Signed-off-by: Andrew McRae amcrae@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/35466 Reviewed-by: Edward O'Callaghan quasisec@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/wifi/generic.c 1 file changed, 2 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved
diff --git a/src/drivers/wifi/generic.c b/src/drivers/wifi/generic.c index b593ffe..fc88f4a 100644 --- a/src/drivers/wifi/generic.c +++ b/src/drivers/wifi/generic.c @@ -239,7 +239,8 @@ { static char wifi_acpi_name[WIFI_ACPI_NAME_MAX_LEN];
- snprintf(wifi_acpi_name, sizeof(wifi_acpi_name), "WF%02x", + /* ACPI 6.3, ASL 20.2.2: (Name Objects Encoding). */ + snprintf(wifi_acpi_name, sizeof(wifi_acpi_name), "WF%02X", (dev_path_encode(dev) & 0xff)); return wifi_acpi_name; }
-
Andrew McRae (Code Review) -
Edward O'Callaghan (Code Review)
-
Furquan Shaikh (Code Review)