Change in coreboot[master]: drivers/ipmi: Add uid parameter to struct drivers_ipmi_config
Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40705 )
Change subject: drivers/ipmi: Add uid parameter to struct drivers_ipmi_config ......................................................................
drivers/ipmi: Add uid parameter to struct drivers_ipmi_config
This change adds uid parameter to drivers_ipmi_config that can be used by ipmi_ssdt() to store the uid value to be used by ipmi_write_acpi_tables. This allows to remove the requirement in ipmi_ssdt() to update dev->command. This is being done in preparation to make the struct device * parameter to fill_ssdt as const.
Change-Id: Ieb41771c75aae902191bba5d220796e6c343f8e0 Signed-off-by: Furquan Shaikh furquan@google.com --- M src/drivers/ipmi/chip.h M src/drivers/ipmi/ipmi_kcs_ops.c M src/drivers/net/r8168.c 3 files changed, 5 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40705/1
diff --git a/src/drivers/ipmi/chip.h b/src/drivers/ipmi/chip.h index 4e9641b..ede9af6 100644 --- a/src/drivers/ipmi/chip.h +++ b/src/drivers/ipmi/chip.h @@ -23,6 +23,7 @@ * Will be used if wait_for_bmc is true. */ u16 bmc_boot_timeout; + unsigned int uid; /* Auto-filled by ipmi_ssdt() */ };
#endif /* _IMPI_CHIP_H_ */ diff --git a/src/drivers/ipmi/ipmi_kcs_ops.c b/src/drivers/ipmi/ipmi_kcs_ops.c index 043616a..349abbc 100644 --- a/src/drivers/ipmi/ipmi_kcs_ops.c +++ b/src/drivers/ipmi/ipmi_kcs_ops.c @@ -212,7 +212,7 @@ acpi_create_ipmi(dev, spmi, (ipmi_revision_major << 8) | (ipmi_revision_minor << 4), &addr, IPMI_INTERFACE_KCS, gpe_interrupt, apic_interrupt, - dev->command); + conf->uid);
acpi_add_table(rsdp, spmi);
@@ -236,14 +236,14 @@ conf = dev->chip_info;
/* Use command to pass UID to ipmi_write_acpi_tables */ - dev->command = uid_cnt++; + conf->uid = uid_cnt++;
/* write SPMI device */ acpigen_write_scope(scope); acpigen_write_device("SPMI"); acpigen_write_name_string("_HID", "IPI0001"); acpigen_write_name_unicode("_STR", "IPMI_KCS"); - acpigen_write_name_byte("_UID", dev->command); + acpigen_write_name_byte("_UID", conf->uid); acpigen_write_STA(0xf); acpigen_write_name("_CRS"); acpigen_write_resourcetemplate_header(); diff --git a/src/drivers/net/r8168.c b/src/drivers/net/r8168.c index c547f17..b904586 100644 --- a/src/drivers/net/r8168.c +++ b/src/drivers/net/r8168.c @@ -306,7 +306,7 @@
#if CONFIG(HAVE_ACPI_TABLES) #define R8168_ACPI_HID "R8168" -static void r8168_net_fill_ssdt(struct device *dev) +static void r8168_net_fill_ssdt(const struct device *dev) { struct drivers_net_config *config = dev->chip_info; const char *path = acpi_device_path(dev->bus->dev);
Hello build bot (Jenkins), Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40705
to look at the new patch set (#3).
Change subject: drivers/ipmi: Add uid parameter to struct drivers_ipmi_config ......................................................................
drivers/ipmi: Add uid parameter to struct drivers_ipmi_config
This change adds uid parameter to drivers_ipmi_config that can be used by ipmi_ssdt() to store the uid value to be used by ipmi_write_acpi_tables. This allows to remove the requirement in ipmi_ssdt() to update dev->command. This is being done in preparation to make the struct device * parameter to fill_ssdt as const.
Change-Id: Ieb41771c75aae902191bba5d220796e6c343f8e0 Signed-off-by: Furquan Shaikh furquan@google.com --- M src/drivers/ipmi/chip.h M src/drivers/ipmi/ipmi_kcs_ops.c 2 files changed, 4 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/05/40705/3
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40705 )
Change subject: drivers/ipmi: Add uid parameter to struct drivers_ipmi_config ......................................................................
Patch Set 6: Code-Review+2
Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40705 )
Change subject: drivers/ipmi: Add uid parameter to struct drivers_ipmi_config ......................................................................
drivers/ipmi: Add uid parameter to struct drivers_ipmi_config
This change adds uid parameter to drivers_ipmi_config that can be used by ipmi_ssdt() to store the uid value to be used by ipmi_write_acpi_tables. This allows to remove the requirement in ipmi_ssdt() to update dev->command. This is being done in preparation to make the struct device * parameter to fill_ssdt as const.
Change-Id: Ieb41771c75aae902191bba5d220796e6c343f8e0 Signed-off-by: Furquan Shaikh furquan@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/40705 Reviewed-by: Aaron Durbin adurbin@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/ipmi/chip.h M src/drivers/ipmi/ipmi_kcs_ops.c 2 files changed, 4 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved
diff --git a/src/drivers/ipmi/chip.h b/src/drivers/ipmi/chip.h index 4e9641b..ede9af6 100644 --- a/src/drivers/ipmi/chip.h +++ b/src/drivers/ipmi/chip.h @@ -23,6 +23,7 @@ * Will be used if wait_for_bmc is true. */ u16 bmc_boot_timeout; + unsigned int uid; /* Auto-filled by ipmi_ssdt() */ };
#endif /* _IMPI_CHIP_H_ */ diff --git a/src/drivers/ipmi/ipmi_kcs_ops.c b/src/drivers/ipmi/ipmi_kcs_ops.c index 043616a..349abbc 100644 --- a/src/drivers/ipmi/ipmi_kcs_ops.c +++ b/src/drivers/ipmi/ipmi_kcs_ops.c @@ -212,7 +212,7 @@ acpi_create_ipmi(dev, spmi, (ipmi_revision_major << 8) | (ipmi_revision_minor << 4), &addr, IPMI_INTERFACE_KCS, gpe_interrupt, apic_interrupt, - dev->command); + conf->uid);
acpi_add_table(rsdp, spmi);
@@ -236,14 +236,14 @@ conf = dev->chip_info;
/* Use command to pass UID to ipmi_write_acpi_tables */ - dev->command = uid_cnt++; + conf->uid = uid_cnt++;
/* write SPMI device */ acpigen_write_scope(scope); acpigen_write_device("SPMI"); acpigen_write_name_string("_HID", "IPI0001"); acpigen_write_name_unicode("_STR", "IPMI_KCS"); - acpigen_write_name_byte("_UID", dev->command); + acpigen_write_name_byte("_UID", conf->uid); acpigen_write_STA(0xf); acpigen_write_name("_CRS"); acpigen_write_resourcetemplate_header();
Bill XIE has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40705 )
Change subject: drivers/ipmi: Add uid parameter to struct drivers_ipmi_config ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40705/7/src/drivers/ipmi/ipmi_kcs_o... File src/drivers/ipmi/ipmi_kcs_ops.c:
https://review.coreboot.org/c/coreboot/+/40705/7/src/drivers/ipmi/ipmi_kcs_o... PS7, Line 215: conf->uid); What will happen if (conf == NULL) ? My conpiler complains "potential null pointer dereference [-Werror=null-dereference]" here.
https://review.coreboot.org/c/coreboot/+/40705/7/src/drivers/ipmi/ipmi_kcs_o... PS7, Line 239: conf->uid = uid_cnt++; What will happen if (conf == NULL) ?
-
Aaron Durbin (Code Review) -
Bill XIE (Code Review)
-
Furquan Shaikh (Code Review)