Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/46038 )
Change subject: drivers/wifi/generic: Limit scope of ACPI-related functions to generic.c ......................................................................
drivers/wifi/generic: Limit scope of ACPI-related functions to generic.c
This change limits the scope of `wifi_generic_fill_ssdt()` and `wifi_generic_acpi_name()` to generic.c since they are not used outside of this file anymore. Also, since there is no need to split SSDT generator into two separate functions, `wifi_generic_fill_ssdt_generator()` is dropped and `.acpi_fill_ssdt` directly points to `wifi_generic_fill_ssdt()`.
BUG=b:169802515 BRANCH=zork
Change-Id: I2cbb97f43d2d9f9ed6d3cf8f0a9b13a7f30e922e Signed-off-by: Furquan Shaikh furquan@google.com --- M src/drivers/wifi/generic/chip.h M src/drivers/wifi/generic/generic.c 2 files changed, 4 insertions(+), 31 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/46038/1
diff --git a/src/drivers/wifi/generic/chip.h b/src/drivers/wifi/generic/chip.h index 02ab504..e3b0ba5 100644 --- a/src/drivers/wifi/generic/chip.h +++ b/src/drivers/wifi/generic/chip.h @@ -11,26 +11,4 @@ unsigned int wake; };
-/** - * wifi_generic_fill_ssdt() - Fill ACPI SSDT table for WiFi controller - * @dev: Device structure corresponding to WiFi controller. - * @config: Generic wifi config required to fill ACPI SSDT table. - * - * This function implements common device operation to help fill ACPI SSDT - * table for WiFi controller. - */ -void wifi_generic_fill_ssdt(const struct device *dev, - const struct drivers_wifi_generic_config *config); - -/** - * wifi_generic_acpi_name() - Get ACPI name for WiFi controller - * @dev: Device structure corresponding to WiFi controller. - * - * This function implements common device operation to get the ACPI name for - * WiFi controller. - * - * Return: string representing the ACPI name for WiFi controller. - */ -const char *wifi_generic_acpi_name(const struct device *dev); - #endif /* _GENERIC_WIFI_H_ */ diff --git a/src/drivers/wifi/generic/generic.c b/src/drivers/wifi/generic/generic.c index 97f5bda..48201fc 100644 --- a/src/drivers/wifi/generic/generic.c +++ b/src/drivers/wifi/generic/generic.c @@ -163,11 +163,11 @@ acpigen_pop_len(); }
-void wifi_generic_fill_ssdt(const struct device *dev, - const struct drivers_wifi_generic_config *config) +static void wifi_generic_fill_ssdt(const struct device *dev) { const char *path; u32 address; + const struct drivers_wifi_generic_config *config = dev->chip_info;
if (!dev->enabled) return; @@ -226,7 +226,7 @@ dev->chip_ops ? dev->chip_ops->name : "", dev_path(dev)); }
-const char *wifi_generic_acpi_name(const struct device *dev) +static const char *wifi_generic_acpi_name(const struct device *dev) { static char wifi_acpi_name[WIFI_ACPI_NAME_MAX_LEN];
@@ -235,11 +235,6 @@ (dev_path_encode(dev) & 0xff)); return wifi_acpi_name; } - -static void wifi_generic_fill_ssdt_generator(const struct device *dev) -{ - wifi_generic_fill_ssdt(dev, dev->chip_info); -} #endif
static void wifi_pci_dev_init(struct device *dev) @@ -291,7 +286,7 @@ .ops_pci = &pci_dev_ops_pci, #if CONFIG(HAVE_ACPI_TABLES) .acpi_name = wifi_generic_acpi_name, - .acpi_fill_ssdt = wifi_generic_fill_ssdt_generator, + .acpi_fill_ssdt = wifi_generic_fill_ssdt, #endif #if CONFIG(GENERATE_SMBIOS_TABLES) .get_smbios_data = smbios_write_wifi,
Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46038 )
Change subject: drivers/wifi/generic: Limit scope of ACPI-related functions to generic.c ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/46038/1/src/drivers/wifi/generic/ch... File src/drivers/wifi/generic/chip.h:
https://review.coreboot.org/c/coreboot/+/46038/1/src/drivers/wifi/generic/ch... PS1, Line 17: * @config: Generic wifi config required to fill ACPI SSDT table. Should these comments be moved to generic.c?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46038 )
Change subject: drivers/wifi/generic: Limit scope of ACPI-related functions to generic.c ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/46038/1/src/drivers/wifi/generic/ch... File src/drivers/wifi/generic/chip.h:
https://review.coreboot.org/c/coreboot/+/46038/1/src/drivers/wifi/generic/ch... PS1, Line 17: * @config: Generic wifi config required to fill ACPI SSDT table.
Should these comments be moved to generic. […]
Those are not really required. The names are pretty self-descriptive in my opinion.
Hello build bot (Jenkins), Duncan Laurie, Rob Barnes, Karthik Ramasubramanian,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46038
to look at the new patch set (#3).
Change subject: drivers/wifi/generic: Limit scope of ACPI-related functions to generic.c ......................................................................
drivers/wifi/generic: Limit scope of ACPI-related functions to generic.c
This change limits the scope of `wifi_generic_fill_ssdt()` and `wifi_generic_acpi_name()` to generic.c since they are not used outside of this file anymore. Also, since there is no need to split SSDT generator into two separate functions, `wifi_generic_fill_ssdt_generator()` is dropped and `.acpi_fill_ssdt` directly points to `wifi_generic_fill_ssdt()`.
BUG=b:169802515 BRANCH=zork
Change-Id: I2cbb97f43d2d9f9ed6d3cf8f0a9b13a7f30e922e Signed-off-by: Furquan Shaikh furquan@google.com --- M src/drivers/wifi/generic/chip.h M src/drivers/wifi/generic/generic.c 2 files changed, 4 insertions(+), 31 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/46038/3
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46038 )
Change subject: drivers/wifi/generic: Limit scope of ACPI-related functions to generic.c ......................................................................
Patch Set 3: Code-Review+2
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46038 )
Change subject: drivers/wifi/generic: Limit scope of ACPI-related functions to generic.c ......................................................................
Patch Set 7: Code-Review+2
Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46038 )
Change subject: drivers/wifi/generic: Limit scope of ACPI-related functions to generic.c ......................................................................
drivers/wifi/generic: Limit scope of ACPI-related functions to generic.c
This change limits the scope of `wifi_generic_fill_ssdt()` and `wifi_generic_acpi_name()` to generic.c since they are not used outside of this file anymore. Also, since there is no need to split SSDT generator into two separate functions, `wifi_generic_fill_ssdt_generator()` is dropped and `.acpi_fill_ssdt` directly points to `wifi_generic_fill_ssdt()`.
BUG=b:169802515 BRANCH=zork
Change-Id: I2cbb97f43d2d9f9ed6d3cf8f0a9b13a7f30e922e Signed-off-by: Furquan Shaikh furquan@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/46038 Reviewed-by: Michael Niewöhner foss@mniewoehner.de Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-by: Rob Barnes robbarnes@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/drivers/wifi/generic/chip.h M src/drivers/wifi/generic/generic.c 2 files changed, 4 insertions(+), 31 deletions(-)
Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved Michael Niewöhner: Looks good to me, approved Rob Barnes: Looks good to me, but someone else must approve
diff --git a/src/drivers/wifi/generic/chip.h b/src/drivers/wifi/generic/chip.h index 02ab504..e3b0ba5 100644 --- a/src/drivers/wifi/generic/chip.h +++ b/src/drivers/wifi/generic/chip.h @@ -11,26 +11,4 @@ unsigned int wake; };
-/** - * wifi_generic_fill_ssdt() - Fill ACPI SSDT table for WiFi controller - * @dev: Device structure corresponding to WiFi controller. - * @config: Generic wifi config required to fill ACPI SSDT table. - * - * This function implements common device operation to help fill ACPI SSDT - * table for WiFi controller. - */ -void wifi_generic_fill_ssdt(const struct device *dev, - const struct drivers_wifi_generic_config *config); - -/** - * wifi_generic_acpi_name() - Get ACPI name for WiFi controller - * @dev: Device structure corresponding to WiFi controller. - * - * This function implements common device operation to get the ACPI name for - * WiFi controller. - * - * Return: string representing the ACPI name for WiFi controller. - */ -const char *wifi_generic_acpi_name(const struct device *dev); - #endif /* _GENERIC_WIFI_H_ */ diff --git a/src/drivers/wifi/generic/generic.c b/src/drivers/wifi/generic/generic.c index ba061d0..1b15211 100644 --- a/src/drivers/wifi/generic/generic.c +++ b/src/drivers/wifi/generic/generic.c @@ -163,11 +163,11 @@ acpigen_pop_len(); }
-void wifi_generic_fill_ssdt(const struct device *dev, - const struct drivers_wifi_generic_config *config) +static void wifi_generic_fill_ssdt(const struct device *dev) { const char *path; u32 address; + const struct drivers_wifi_generic_config *config = dev->chip_info;
if (!dev->enabled) return; @@ -226,7 +226,7 @@ dev->chip_ops ? dev->chip_ops->name : "", dev_path(dev)); }
-const char *wifi_generic_acpi_name(const struct device *dev) +static const char *wifi_generic_acpi_name(const struct device *dev) { static char wifi_acpi_name[WIFI_ACPI_NAME_MAX_LEN];
@@ -235,11 +235,6 @@ (dev_path_encode(dev) & 0xff)); return wifi_acpi_name; } - -static void wifi_generic_fill_ssdt_generator(const struct device *dev) -{ - wifi_generic_fill_ssdt(dev, dev->chip_info); -} #endif
static void wifi_pci_dev_init(struct device *dev) @@ -292,7 +287,7 @@ .ops_pci = &pci_dev_ops_pci, #if CONFIG(HAVE_ACPI_TABLES) .acpi_name = wifi_generic_acpi_name, - .acpi_fill_ssdt = wifi_generic_fill_ssdt_generator, + .acpi_fill_ssdt = wifi_generic_fill_ssdt, #endif #if CONFIG(GENERATE_SMBIOS_TABLES) .get_smbios_data = smbios_write_wifi,