Aaron Durbin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42231 )
Change subject: soc/amd/picasso: initialize ACP device at init() time ......................................................................
soc/amd/picasso: initialize ACP device at init() time
The ACP device sits behind a bridge. Despite the logs indicating the bridge is likely hooked up, there's some unusual behavior of writes not sticking. Aside from the speculation of what's causing the issues the initialization of the device should occur at init() because of these potential dependencies.
BUG=b:155882600
Change-Id: I8fa83d7d1d4f356c56971d4175a2ae6497a92fb8 Signed-off-by: Aaron Durbin adurbin@chromium.org --- M src/soc/amd/picasso/acp.c 1 file changed, 3 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/42231/1
diff --git a/src/soc/amd/picasso/acp.c b/src/soc/amd/picasso/acp.c index cbe5d86..854dd8f 100644 --- a/src/soc/amd/picasso/acp.c +++ b/src/soc/amd/picasso/acp.c @@ -13,15 +13,13 @@ #include <amdblocks/acpimmio.h> #include <commonlib/helpers.h>
-static void enable(struct device *dev) +static void init(struct device *dev) { const struct soc_amd_picasso_config *cfg; const struct device *nb_dev = pcidev_path_on_root(GNB_DEVFN); struct resource *res; uintptr_t bar;
- pci_dev_enable_resources(dev); - /* Set the proper I2S_PIN_CONFIG state */ if (!nb_dev || !nb_dev->chip_info) return; @@ -44,7 +42,8 @@ static struct device_operations acp_ops = { .read_resources = pci_dev_read_resources, .set_resources = pci_dev_set_resources, - .enable_resources = enable, + .enable_resources = pci_dev_enable_resources, + .init = init, .ops_pci = &pci_dev_ops_pci, };
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42231 )
Change subject: soc/amd/picasso: initialize ACP device at init() time ......................................................................
Patch Set 1: Code-Review+2
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42231 )
Change subject: soc/amd/picasso: initialize ACP device at init() time ......................................................................
Patch Set 1: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42231 )
Change subject: soc/amd/picasso: initialize ACP device at init() time ......................................................................
Patch Set 1: Code-Review+2
Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42231 )
Change subject: soc/amd/picasso: initialize ACP device at init() time ......................................................................
soc/amd/picasso: initialize ACP device at init() time
The ACP device sits behind a bridge. Despite the logs indicating the bridge is likely hooked up, there's some unusual behavior of writes not sticking. Aside from the speculation of what's causing the issues the initialization of the device should occur at init() because of these potential dependencies.
BUG=b:155882600
Change-Id: I8fa83d7d1d4f356c56971d4175a2ae6497a92fb8 Signed-off-by: Aaron Durbin adurbin@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/42231 Reviewed-by: Raul Rangel rrangel@chromium.org Reviewed-by: Felix Held felix-coreboot@felixheld.de Reviewed-by: Furquan Shaikh furquan@google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/amd/picasso/acp.c 1 file changed, 3 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved Furquan Shaikh: Looks good to me, approved Raul Rangel: Looks good to me, approved
diff --git a/src/soc/amd/picasso/acp.c b/src/soc/amd/picasso/acp.c index cbe5d86..854dd8f 100644 --- a/src/soc/amd/picasso/acp.c +++ b/src/soc/amd/picasso/acp.c @@ -13,15 +13,13 @@ #include <amdblocks/acpimmio.h> #include <commonlib/helpers.h>
-static void enable(struct device *dev) +static void init(struct device *dev) { const struct soc_amd_picasso_config *cfg; const struct device *nb_dev = pcidev_path_on_root(GNB_DEVFN); struct resource *res; uintptr_t bar;
- pci_dev_enable_resources(dev); - /* Set the proper I2S_PIN_CONFIG state */ if (!nb_dev || !nb_dev->chip_info) return; @@ -44,7 +42,8 @@ static struct device_operations acp_ops = { .read_resources = pci_dev_read_resources, .set_resources = pci_dev_set_resources, - .enable_resources = enable, + .enable_resources = pci_dev_enable_resources, + .init = init, .ops_pci = &pci_dev_ops_pci, };
-
Aaron Durbin (Code Review) -
Felix Held (Code Review)
-
Furquan Shaikh (Code Review)
-
Raul Rangel (Code Review)