Change in coreboot[master]: acpigen: Add function to generate unicode names

[Patrick Rudolph (Code Review)]

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37789 )

Change subject: acpigen: Add function to generate unicode names
......................................................................

acpigen: Add function to generate unicode names

Add function to generate Unicode names and use if for _STR.
The ACPI spec requires _STR to return a buffer containing UTF-16
characters.

Use the introduced function in IPMI driver.

Change-Id: I16992bd449e3a51f6a8875731cd45a9f43de5c8c
Signed-off-by: Patrick Rudolph 
---
M src/arch/x86/acpigen.c
M src/arch/x86/include/arch/acpigen.h
M src/drivers/ipmi/ipmi_kcs_ops.c
3 files changed, 17 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/37789/1

diff --git a/src/arch/x86/acpigen.c b/src/arch/x86/acpigen.c
index cc724a0..15c58bd 100644
--- a/src/arch/x86/acpigen.c
+++ b/src/arch/x86/acpigen.c
@@ -199,6 +199,21 @@
    acpigen_write_string(string);
 }

+void acpigen_write_name_unicode(const char *name, const char *string)
+{
+	const size_t len = strlen(string) + 1;
+	acpigen_write_name(name);
+	acpigen_emit_byte(BUFFER_OP);
+	acpigen_write_len_f();
+	acpigen_write_integer(len);
+	for (size_t i = 0; i < len; i++) {
+		const char c = string[i];
+		/* Simple ASCII to UTF-16 conversion */
+		acpigen_emit_word(c >= 0 ? c : '?');
+	}
+	acpigen_pop_len();
+}
+
 void acpigen_emit_stream(const char *data, int size)
 {
    int i;
diff --git a/src/arch/x86/include/arch/acpigen.h b/src/arch/x86/include/arch/acpigen.h
index 8b8c873..f74c30f 100644
--- a/src/arch/x86/include/arch/acpigen.h
+++ b/src/arch/x86/include/arch/acpigen.h
@@ -310,6 +310,7 @@
 void acpigen_write_qword(uint64_t data);
 void acpigen_write_integer(uint64_t data);
 void acpigen_write_string(const char *string);
+void acpigen_write_name_unicode(const char *name, const char *string);
 void acpigen_write_name(const char *name);
 void acpigen_write_name_zero(const char *name);
 void acpigen_write_name_one(const char *name);
diff --git a/src/drivers/ipmi/ipmi_kcs_ops.c b/src/drivers/ipmi/ipmi_kcs_ops.c
index 5cb8995..e0fa1b0 100644
--- a/src/drivers/ipmi/ipmi_kcs_ops.c
+++ b/src/drivers/ipmi/ipmi_kcs_ops.c
@@ -253,7 +253,7 @@
    acpigen_write_scope(scope);
    acpigen_write_device("SPMI");
    acpigen_write_name_string("_HID", "IPI0001");
-	acpigen_write_name_string("_STR", "IPMI_KCS");
+	acpigen_write_name_unicode("_STR", "IPMI_KCS");
    acpigen_write_name_byte("_UID", dev->command);
    acpigen_write_STA(0xf);
    acpigen_write_name("_CRS");

[Paul Menzel (Code Review)]

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

Change subject: acpigen: Add function to generate unicode names
......................................................................

Patch Set 1:

(2 comments)

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

https://review.coreboot.org/c/coreboot/+/37789/1//COMMIT_MSG@9 
PS1, Line 9: if
it

https://review.coreboot.org/c/coreboot/+/37789/1//COMMIT_MSG@12 
PS1, Line 12: 
Can you comment on the implementation? When are ? characters used as substitute?

[Christian Walter (Code Review)]

Christian Walter has uploaded a new patch set (#3) to the change originally created by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/37789 )

Change subject: acpigen: Add function to generate unicode names
......................................................................

acpigen: Add function to generate unicode names

Add function to generate Unicode names and use if for _STR.
The ACPI spec requires _STR to return a buffer containing UTF-16
characters.

Use the introduced function in IPMI driver.

Change-Id: I16992bd449e3a51f6a8875731cd45a9f43de5c8c
Signed-off-by: Patrick Rudolph 
---
M src/arch/x86/acpigen.c
M src/arch/x86/include/arch/acpigen.h
M src/drivers/ipmi/ipmi_kcs_ops.c
3 files changed, 17 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/37789/3

[Patrick Rudolph (Code Review)]

Hello Morgan Jang, Johnny Lin, David Hendricks, Christian Walter, build bot (Jenkins), Andrey Petrov,

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

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

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

Change subject: acpigen: Add function to generate unicode names
......................................................................

acpigen: Add function to generate unicode names

The ACPI spec 6.3 chapter 6.1.10 states that _STR has to return a buffer
containing UTF-16 characters.

Add function to generate Unicode names and use if for _STR. It will
replace non-ASCII characters with '?'.

Use the introduced function in IPMI driver.

Fixes ACPI warning shown in fwts.

Change-Id: I16992bd449e3a51f6a8875731cd45a9f43de5c8c
Signed-off-by: Patrick Rudolph 
---
M src/arch/x86/acpigen.c
M src/arch/x86/include/arch/acpigen.h
M src/drivers/ipmi/ipmi_kcs_ops.c
3 files changed, 17 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/37789/5

[Patrick Rudolph (Code Review)]

Hello Morgan Jang, Johnny Lin, David Hendricks, Christian Walter, build bot (Jenkins), Andrey Petrov,

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

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

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

Change subject: acpigen: Add function to generate unicode names
......................................................................

acpigen: Add function to generate unicode names

The ACPI spec 6.3 chapter 6.1.10 states that _STR has to return a buffer
containing UTF-16 characters.

Add function to generate Unicode names and use it for _STR. It will
replace non-ASCII characters with '?'.

Use the introduced function in IPMI driver.

Fixes ACPI warning shown in fwts.

Change-Id: I16992bd449e3a51f6a8875731cd45a9f43de5c8c
Signed-off-by: Patrick Rudolph 
---
M src/arch/x86/acpigen.c
M src/arch/x86/include/arch/acpigen.h
M src/drivers/ipmi/ipmi_kcs_ops.c
3 files changed, 17 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/37789/6

[Patrick Rudolph (Code Review)]

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

Change subject: acpigen: Add function to generate unicode names
......................................................................

Patch Set 6:

(2 comments)

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

https://review.coreboot.org/c/coreboot/+/37789/1//COMMIT_MSG@9 
PS1, Line 9: if
> it

Done

https://review.coreboot.org/c/coreboot/+/37789/1//COMMIT_MSG@12 
PS1, Line 12:
> Can you comment on the implementation? When are ? characters used as substitute?

Done

[Paul Menzel (Code Review)]

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

Change subject: acpigen: Add function to generate unicode names
......................................................................

Patch Set 6: Code-Review+1

[Philipp Deppenwiese (Code Review)]

Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37789 )

Change subject: acpigen: Add function to generate unicode names
......................................................................

Patch Set 6: Code-Review+2

[Patrick Rudolph (Code Review)]

Patrick Rudolph has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37789 )

Change subject: acpigen: Add function to generate unicode names
......................................................................

acpigen: Add function to generate unicode names

The ACPI spec 6.3 chapter 6.1.10 states that _STR has to return a buffer
containing UTF-16 characters.

Add function to generate Unicode names and use it for _STR. It will
replace non-ASCII characters with '?'.

Use the introduced function in IPMI driver.

Fixes ACPI warning shown in fwts.

Change-Id: I16992bd449e3a51f6a8875731cd45a9f43de5c8c
Signed-off-by: Patrick Rudolph 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/37789
Reviewed-by: Paul Menzel 
Reviewed-by: Philipp Deppenwiese 
Tested-by: build bot (Jenkins) 
---
M src/arch/x86/acpigen.c
M src/arch/x86/include/arch/acpigen.h
M src/drivers/ipmi/ipmi_kcs_ops.c
3 files changed, 17 insertions(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Paul Menzel: Looks good to me, but someone else must approve
  Philipp Deppenwiese: Looks good to me, approved

diff --git a/src/arch/x86/acpigen.c b/src/arch/x86/acpigen.c
index cc724a0..493131e 100644
--- a/src/arch/x86/acpigen.c
+++ b/src/arch/x86/acpigen.c
@@ -199,6 +199,21 @@
    acpigen_write_string(string);
 }

+void acpigen_write_name_unicode(const char *name, const char *string)
+{
+	const size_t len = strlen(string) + 1;
+	acpigen_write_name(name);
+	acpigen_emit_byte(BUFFER_OP);
+	acpigen_write_len_f();
+	acpigen_write_integer(len);
+	for (size_t i = 0; i < len; i++) {
+		const char c = string[i];
+		/* Simple ASCII to UTF-16 conversion, replace non ASCII characters */
+		acpigen_emit_word(c >= 0 ? c : '?');
+	}
+	acpigen_pop_len();
+}
+
 void acpigen_emit_stream(const char *data, int size)
 {
    int i;
diff --git a/src/arch/x86/include/arch/acpigen.h b/src/arch/x86/include/arch/acpigen.h
index 11fe232..c9a6485 100644
--- a/src/arch/x86/include/arch/acpigen.h
+++ b/src/arch/x86/include/arch/acpigen.h
@@ -309,6 +309,7 @@
 void acpigen_write_qword(uint64_t data);
 void acpigen_write_integer(uint64_t data);
 void acpigen_write_string(const char *string);
+void acpigen_write_name_unicode(const char *name, const char *string);
 void acpigen_write_name(const char *name);
 void acpigen_write_name_zero(const char *name);
 void acpigen_write_name_one(const char *name);
diff --git a/src/drivers/ipmi/ipmi_kcs_ops.c b/src/drivers/ipmi/ipmi_kcs_ops.c
index 5cb8995..e0fa1b0 100644
--- a/src/drivers/ipmi/ipmi_kcs_ops.c
+++ b/src/drivers/ipmi/ipmi_kcs_ops.c
@@ -253,7 +253,7 @@
    acpigen_write_scope(scope);
    acpigen_write_device("SPMI");
    acpigen_write_name_string("_HID", "IPI0001");
-	acpigen_write_name_string("_STR", "IPMI_KCS");
+	acpigen_write_name_unicode("_STR", "IPMI_KCS");
    acpigen_write_name_byte("_UID", dev->command);
    acpigen_write_STA(0xf);
    acpigen_write_name("_CRS");