Change in coreboot[master]: drivers/wifi: Check device is of type PCI before checking vendor ID
Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47364 )
Change subject: drivers/wifi: Check device is of type PCI before checking vendor ID ......................................................................
drivers/wifi: Check device is of type PCI before checking vendor ID
CB:46865 ("mb, soc/intel: Reorganize CNVi device entries in devicetree") reorganized the devicetree entries to make the representation of CNVi device consistent with other internal PCI devices. Since a dummy generic device is added for the CNVi device, `emit_sar_acpi_structures()` needs to first check if the device is PCI before checking the vendor ID. This ensures that SAR table generation is skipped only for PCIe devices with non-Intel vendor IDs and not for the dummy generic device.
BUG=b:165105210
Change-Id: I3c8d18538b94ed1072cfcc108552f3a1ac320395 Signed-off-by: Furquan Shaikh furquan@google.com --- M src/drivers/wifi/generic/acpi.c 1 file changed, 5 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/47364/1
diff --git a/src/drivers/wifi/generic/acpi.c b/src/drivers/wifi/generic/acpi.c index 1cc4bd0..cd5af4e 100644 --- a/src/drivers/wifi/generic/acpi.c +++ b/src/drivers/wifi/generic/acpi.c @@ -44,8 +44,11 @@ struct wifi_sar_limits sar_limits; struct wifi_sar_delta_table *wgds;
- /* CBFS SAR and SAR ACPI tables are currently used only by Intel WiFi devices. */ - if (dev->vendor != PCI_VENDOR_ID_INTEL) + /* + * If device type is PCI, ensure that the device has Intel vendor ID. CBFS SAR and SAR + * ACPI tables are currently used only by Intel WiFi devices. + */ + if (dev->path.type == DEVICE_PATH_PCI && dev->vendor != PCI_VENDOR_ID_INTEL) return;
/* Retrieve the sar limits data */
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47364 )
Change subject: drivers/wifi: Check device is of type PCI before checking vendor ID ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/47364/1/src/drivers/wifi/generic/ac... File src/drivers/wifi/generic/acpi.c:
https://review.coreboot.org/c/coreboot/+/47364/1/src/drivers/wifi/generic/ac... PS1, Line 215: if (CONFIG(USE_SAR)) How about move the check, here?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47364 )
Change subject: drivers/wifi: Check device is of type PCI before checking vendor ID ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47364/1/src/drivers/wifi/generic/ac... File src/drivers/wifi/generic/acpi.c:
https://review.coreboot.org/c/coreboot/+/47364/1/src/drivers/wifi/generic/ac... PS1, Line 215: if (CONFIG(USE_SAR))
How about move the check, here?
I don't think it would make much difference. I kept it inside `emit_sar_acpi_structures()` to make the code in this function easier to read. To be honest, I think either ways its fine. Unless you feel very strongly about it, I think we can keep the check in `emit_sar_acpi_structures()`.
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47364 )
Change subject: drivers/wifi: Check device is of type PCI before checking vendor ID ......................................................................
Patch Set 1: Code-Review+2
Dtrain Hsu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47364 )
Change subject: drivers/wifi: Check device is of type PCI before checking vendor ID ......................................................................
Patch Set 1: Code-Review+1
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47364 )
Change subject: drivers/wifi: Check device is of type PCI before checking vendor ID ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47364/1/src/drivers/wifi/generic/ac... File src/drivers/wifi/generic/acpi.c:
https://review.coreboot.org/c/coreboot/+/47364/1/src/drivers/wifi/generic/ac... PS1, Line 215: if (CONFIG(USE_SAR))
I don't think it would make much difference. […]
I'm fine. It's okay.
Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47364 )
Change subject: drivers/wifi: Check device is of type PCI before checking vendor ID ......................................................................
drivers/wifi: Check device is of type PCI before checking vendor ID
CB:46865 ("mb, soc/intel: Reorganize CNVi device entries in devicetree") reorganized the devicetree entries to make the representation of CNVi device consistent with other internal PCI devices. Since a dummy generic device is added for the CNVi device, `emit_sar_acpi_structures()` needs to first check if the device is PCI before checking the vendor ID. This ensures that SAR table generation is skipped only for PCIe devices with non-Intel vendor IDs and not for the dummy generic device.
BUG=b:165105210
Change-Id: I3c8d18538b94ed1072cfcc108552f3a1ac320395 Signed-off-by: Furquan Shaikh furquan@google.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/47364 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: EricR Lai ericr_lai@compal.corp-partner.google.com Reviewed-by: Duncan Laurie dlaurie@chromium.org Reviewed-by: Dtrain Hsu dtrain_hsu@compal.corp-partner.google.com --- M src/drivers/wifi/generic/acpi.c 1 file changed, 5 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Duncan Laurie: Looks good to me, approved EricR Lai: Looks good to me, approved Dtrain Hsu: Looks good to me, but someone else must approve
diff --git a/src/drivers/wifi/generic/acpi.c b/src/drivers/wifi/generic/acpi.c index 1cc4bd0..cd5af4e 100644 --- a/src/drivers/wifi/generic/acpi.c +++ b/src/drivers/wifi/generic/acpi.c @@ -44,8 +44,11 @@ struct wifi_sar_limits sar_limits; struct wifi_sar_delta_table *wgds;
- /* CBFS SAR and SAR ACPI tables are currently used only by Intel WiFi devices. */ - if (dev->vendor != PCI_VENDOR_ID_INTEL) + /* + * If device type is PCI, ensure that the device has Intel vendor ID. CBFS SAR and SAR + * ACPI tables are currently used only by Intel WiFi devices. + */ + if (dev->path.type == DEVICE_PATH_PCI && dev->vendor != PCI_VENDOR_ID_INTEL) return;
/* Retrieve the sar limits data */
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47364 )
Change subject: drivers/wifi: Check device is of type PCI before checking vendor ID ......................................................................
Patch Set 2:
Submitting earlier because it fixes a regression.
-
Dtrain Hsu (Code Review) -
Duncan Laurie (Code Review)
-
EricR Lai (Code Review)
-
Furquan Shaikh (Code Review)