Josie Nordrum has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44158 )
Change subject: mb/google/zork: Use dev_nested_path for dmic gpio update ......................................................................
mb/google/zork: Use dev_nested_path for dmic gpio update
Created function update_dmic_gpio to update gpio machine and rewrote to use the new find_dev_nested_path function for consistency.
BUG=None BRANCH=None TEST=None
Change-Id: I96cf207f24c6117d98ff2bf7e6e5cd282489e805 Signed-off-by: Josie Nordrum josienordrum@google.com --- M src/mainboard/google/zork/variants/baseboard/ramstage_common.c 1 file changed, 46 insertions(+), 35 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/44158/1
diff --git a/src/mainboard/google/zork/variants/baseboard/ramstage_common.c b/src/mainboard/google/zork/variants/baseboard/ramstage_common.c index e4c9869..d80f478 100644 --- a/src/mainboard/google/zork/variants/baseboard/ramstage_common.c +++ b/src/mainboard/google/zork/variants/baseboard/ramstage_common.c @@ -13,6 +13,22 @@ extern struct chip_operations drivers_amd_i2s_machine_dev_ops; extern struct chip_operations drivers_i2c_generic_ops;
+static const struct device_path acp_machine_path[] = { + { + .type = DEVICE_PATH_PCI, + .pci.devfn = PCIE_GPP_A_DEVFN + }, + { + .type = DEVICE_PATH_PCI, + .pci.devfn = AUDIO_DEVFN + }, + { + .type = DEVICE_PATH_GENERIC, + .generic.id = 0, + .generic.subid = 0 + } +}; + static const struct device_path rt5682_path[] = { { .type = DEVICE_PATH_PCI, @@ -36,17 +52,17 @@
void update_hp_int_odl(void) { - const struct device *rt5682_dev; - struct drivers_i2c_generic_config *cfg; - struct acpi_gpio *gpio; + const struct device *rt5682_dev; struct + drivers_i2c_generic_config * cfg; struct acpi_gpio *gpio;
if (!variant_uses_codec_gpi()) return;
rt5682_dev = find_dev_nested_path( - pci_root_bus(), rt5682_path, ARRAY_SIZE(rt5682_path)); + pci_root_bus(), rt5682_path, ARRAY_SIZE(rt5682_path)); if (!rt5682_dev) { - printk(BIOS_ERR, "%s: Failed to find audio device\n", __func__); + printk(BIOS_ERR, "%s: Failed to find audio device\n", + __func__); return; }
@@ -61,44 +77,39 @@
}
-void variant_audio_update(void) +void update_dmic_gpio(void) { - const struct device *gpp_a_dev; - const struct device *acp_dev; - struct device *machine_dev = NULL; + const struct device *machine_dev; struct + drivers_amd_i2s_machine_dev_config * cfg; struct acpi_gpio *gpio;
if (variant_uses_v3_schematics()) return;
- gpp_a_dev = pcidev_path_on_root(PCIE_GPP_A_DEVFN); - if (gpp_a_dev == NULL) + machine_dev = find_dev_nested_path( + pci_root_bus(), acp_machine_path, ARRAY_SIZE(acp_machine_path)); + if (!machine_dev) { + printk(BIOS_ERR, "%s: Failed to find ACP machine device\n", + __func__); return; - - acp_dev = pcidev_path_behind(gpp_a_dev->link_list, AUDIO_DEVFN); - if (acp_dev == NULL) - return; - - while ((machine_dev = dev_bus_each_child(acp_dev->link_list, machine_dev)) != NULL) { - struct drivers_amd_i2s_machine_dev_config *cfg; - struct acpi_gpio *gpio; - - if (machine_dev->chip_info == NULL) - continue; - - if (machine_dev->chip_ops != &drivers_amd_i2s_machine_dev_ops) - continue; - - cfg = machine_dev->chip_info; - gpio = &cfg->dmic_select_gpio; - - if (CONFIG(BOARD_GOOGLE_BASEBOARD_TREMBYLE)) - gpio->pins[0] = GPIO_13; - else - gpio->pins[0] = GPIO_6; - - break; }
+ if (machine_dev->chip_ops != &drivers_amd_i2s_machine_dev_ops) { + printk(BIOS_ERR, "%s: Incorrect device found\n", __func__); + return; + } + + cfg = config_of(machine_dev); gpio = &cfg->dmic_select_gpio; + + if (CONFIG(BOARD_GOOGLE_BASEBOARD_TREMBYLE)) + gpio->pins[0] = GPIO_13; + else + gpio->pins[0] = GPIO_6; + +} + +void update_hp_int_odl(void) +{ + update_dmic_gpio(); update_hp_int_odl(); }
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44158 )
Change subject: mb/google/zork: Use dev_nested_path for dmic gpio update ......................................................................
Patch Set 1:
(9 comments)
https://review.coreboot.org/c/coreboot/+/44158/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44158/1//COMMIT_MSG@9 PS1, Line 9: gpio machine DMIC GPIO for ACP machine device
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/ramstage_common.c:
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... PS1, Line 16: acp_machine_path This can live within update_dmic_gpio().
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... PS1, Line 55: struct : drivers_i2c_generic_config * cfg; struct acpi_gpio *gpio; Unrelated change.
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... PS1, Line 65: Unrelated change. Not required.
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... PS1, Line 80: void static void
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... PS1, Line 82: struct Move to next line.
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... PS1, Line 83: struct acpi_gpio *gpio; Move to next line.
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... PS1, Line 83: No space after *
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... PS1, Line 101: gpio = &cfg->dmic_select_gpio; Move to next line.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44158 )
Change subject: mb/google/zork: Use dev_nested_path for dmic gpio update ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/44158/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44158/1//COMMIT_MSG@9 PS1, Line 9: Created Please use present tense.
https://review.coreboot.org/c/coreboot/+/44158/1//COMMIT_MSG@9 PS1, Line 9: rewrote Ditto.
https://review.coreboot.org/c/coreboot/+/44158/1//COMMIT_MSG@10 PS1, Line 10: new find_dev_nested_path function “new” to me implies, it’s added in this commit. But it’s already there.
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/ramstage_common.c:
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... PS1, Line 92: __func__); This should fit in 96 characters.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44158 )
Change subject: mb/google/zork: Use dev_nested_path for dmic gpio update ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/ramstage_common.c:
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... PS1, Line 110: update_hp_int_odl This function name should remain as variant_audio_update().
Hello build bot (Jenkins), Raul Rangel, Furquan Shaikh, Rob Barnes, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44158
to look at the new patch set (#2).
Change subject: mb/google/zork: Use dev_nested_path for dmic gpio update ......................................................................
mb/google/zork: Use dev_nested_path for dmic gpio update
Create function update_dmic_gpio to update DMIC GPIO for ACP machine and use find_dev_nested_path function for consistency.
BUG=None BRANCH=None TEST=None
Change-Id: I96cf207f24c6117d98ff2bf7e6e5cd282489e805 Signed-off-by: Josie Nordrum josienordrum@google.com --- M src/mainboard/google/zork/variants/baseboard/ramstage_common.c 1 file changed, 42 insertions(+), 30 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/44158/2
Josie Nordrum has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44158 )
Change subject: mb/google/zork: Use dev_nested_path for dmic gpio update ......................................................................
Patch Set 3:
(14 comments)
https://review.coreboot.org/c/coreboot/+/44158/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44158/1//COMMIT_MSG@9 PS1, Line 9: rewrote
Ditto.
Done
https://review.coreboot.org/c/coreboot/+/44158/1//COMMIT_MSG@9 PS1, Line 9: gpio machine
DMIC GPIO for ACP machine device
Done
https://review.coreboot.org/c/coreboot/+/44158/1//COMMIT_MSG@9 PS1, Line 9: Created
Please use present tense.
Done
https://review.coreboot.org/c/coreboot/+/44158/1//COMMIT_MSG@10 PS1, Line 10: new find_dev_nested_path function
“new” to me implies, it’s added in this commit. But it’s already there.
Done
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... File src/mainboard/google/zork/variants/baseboard/ramstage_common.c:
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... PS1, Line 16: acp_machine_path
This can live within update_dmic_gpio().
Done
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... PS1, Line 55: struct : drivers_i2c_generic_config * cfg; struct acpi_gpio *gpio;
Unrelated change.
Done
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... PS1, Line 65:
Unrelated change. Not required.
Done
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... PS1, Line 80: void
static void
Done
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... PS1, Line 82: struct
Move to next line.
Done
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... PS1, Line 83: struct acpi_gpio *gpio;
Move to next line.
Done
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... PS1, Line 83:
No space after *
Done
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... PS1, Line 92: __func__);
This should fit in 96 characters.
Done
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... PS1, Line 101: gpio = &cfg->dmic_select_gpio;
Move to next line.
Done
https://review.coreboot.org/c/coreboot/+/44158/1/src/mainboard/google/zork/v... PS1, Line 110: update_hp_int_odl
This function name should remain as variant_audio_update().
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44158 )
Change subject: mb/google/zork: Use dev_nested_path for dmic gpio update ......................................................................
Patch Set 3: Code-Review+2
Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44158 )
Change subject: mb/google/zork: Use dev_nested_path for dmic gpio update ......................................................................
mb/google/zork: Use dev_nested_path for dmic gpio update
Create function update_dmic_gpio to update DMIC GPIO for ACP machine and use find_dev_nested_path function for consistency.
BUG=None BRANCH=None TEST=None
Change-Id: I96cf207f24c6117d98ff2bf7e6e5cd282489e805 Signed-off-by: Josie Nordrum josienordrum@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/44158 Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/zork/variants/baseboard/ramstage_common.c 1 file changed, 42 insertions(+), 30 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/mainboard/google/zork/variants/baseboard/ramstage_common.c b/src/mainboard/google/zork/variants/baseboard/ramstage_common.c index 298837b..8a6fa47 100644 --- a/src/mainboard/google/zork/variants/baseboard/ramstage_common.c +++ b/src/mainboard/google/zork/variants/baseboard/ramstage_common.c @@ -65,44 +65,56 @@
}
-void variant_audio_update(void) +static void update_dmic_gpio(void) { - const struct device *gpp_a_dev; - const struct device *acp_dev; - struct device *machine_dev = NULL; + static const struct device_path acp_machine_path[] = { + { + .type = DEVICE_PATH_PCI, + .pci.devfn = PCIE_GPP_A_DEVFN + }, + { + .type = DEVICE_PATH_PCI, + .pci.devfn = AUDIO_DEVFN + }, + { + .type = DEVICE_PATH_GENERIC, + .generic.id = 0, + .generic.subid = 0 + } + }; + + const struct device *machine_dev; + struct drivers_amd_i2s_machine_dev_config *cfg; + struct acpi_gpio *gpio;
if (variant_uses_v3_schematics()) return;
- gpp_a_dev = pcidev_path_on_root(PCIE_GPP_A_DEVFN); - if (gpp_a_dev == NULL) + machine_dev = find_dev_nested_path( + pci_root_bus(), acp_machine_path, ARRAY_SIZE(acp_machine_path)); + if (!machine_dev) { + printk(BIOS_ERR, "%s: Failed to find ACP machine device\n", __func__); return; - - acp_dev = pcidev_path_behind(gpp_a_dev->link_list, AUDIO_DEVFN); - if (acp_dev == NULL) - return; - - while ((machine_dev = dev_bus_each_child(acp_dev->link_list, machine_dev)) != NULL) { - struct drivers_amd_i2s_machine_dev_config *cfg; - struct acpi_gpio *gpio; - - if (machine_dev->chip_info == NULL) - continue; - - if (machine_dev->chip_ops != &drivers_amd_i2s_machine_dev_ops) - continue; - - cfg = machine_dev->chip_info; - gpio = &cfg->dmic_select_gpio; - - if (CONFIG(BOARD_GOOGLE_BASEBOARD_TREMBYLE)) - gpio->pins[0] = GPIO_13; - else - gpio->pins[0] = GPIO_6; - - break; }
+ if (machine_dev->chip_ops != &drivers_amd_i2s_machine_dev_ops) { + printk(BIOS_ERR, "%s: Incorrect device found\n", __func__); + return; + } + + cfg = config_of(machine_dev); + gpio = &cfg->dmic_select_gpio; + + if (CONFIG(BOARD_GOOGLE_BASEBOARD_TREMBYLE)) + gpio->pins[0] = GPIO_13; + else + gpio->pins[0] = GPIO_6; + +} + +void variant_audio_update(void) +{ + update_dmic_gpio(); update_hp_int_odl(); }
-
Furquan Shaikh (Code Review) -
Josie Nordrum (Code Review)
-
Paul Menzel (Code Review)