From 93928194c4707a3194a999c96bcc67fe7c7b07f9 Mon Sep 17 00:00:00 2001 From: Reka Norman Date: Fri, 22 Jul 2022 08:01:19 +1000 Subject: drivers/wifi/generic: Revert changes to generate missing SSDT for PCIe wifi This reverts commit 5e6fd360de7fe92f1e8b1b3eb20241809e2a6aff. On nereid, the SSDT entry for the PCIe wifi device is missing, causing wake-on-WLAN not to work since the _PRW is missing. It seems like when commit 5e6fd360de changed the SSDT generation logic for CNVi and PCIe wifi, it broke the PCIe case. `wifi_pcie_ops` are never assigned to any device, so `parent && parent->ops == &wifi_pcie_ops` always returns false, and the `wifi_cnvi_ops` are used even for PCIe devices. Undo the changes in that CL. This allows both the CNVi and PCIe cases to work. That CL was meant to fix an issue with the CNVi _PRW containing garbage, but I can't reproduce this when the change is undone. It was also meant to fix the following error on CNVi devices, but I don't see any errors with this change: [ERROR] NONE missing set_resources BUB=b:233325709 TEST=On both nivviks (CNVi) and nereid (PCIe), check that the SSDT contains the correct wifi device entries (below), including a _PRW containing the correct GPE, and check that wake-on-WLAN works. nivviks: ``` Scope (\_SB.PCI0.CNVW) { Name (_PRW, Package (0x02) // _PRW: Power Resources for Wake { 0x6D, 0x03 }) Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method { } } ``` nereid: ``` Device (\_SB.PCI0.RP01.WF00) { Name (_UID, 0x923ACF1C) // _UID: Unique ID Name (_DDN, "WIFI Device") // _DDN: DOS Device Name Name (_ADR, 0x0000000000000000) // _ADR: Address } Scope (\_SB.PCI0.RP01.WF00) { Name (_PRW, Package (0x02) // _PRW: Power Resources for Wake { 0x23, 0x03 }) Method (_DSM, 4, Serialized) // _DSM: Device-Specific Method { } } ``` Fixes: 5e6fd360de ("drivers/wifi/generic: Fix properties in generic-under-PCI device case") Change-Id: I100c5ee3842997c50444e5ce68d583834ed3a8ad Signed-off-by: Reka Norman Reviewed-on: https://review.coreboot.org/c/coreboot/+/66063 Reviewed-by: Eric Lai Reviewed-by: Kangheui Won Reviewed-by: Tim Wawrzynczak Tested-by: build bot (Jenkins) --- src/drivers/wifi/generic/acpi.c | 32 ++++++++++++++------------------ src/drivers/wifi/generic/generic.c | 13 +++---------- 2 files changed, 17 insertions(+), 28 deletions(-) (limited to 'src/drivers/wifi/generic') diff --git a/src/drivers/wifi/generic/acpi.c b/src/drivers/wifi/generic/acpi.c index 88bcf3a483..ee122ea2d8 100644 --- a/src/drivers/wifi/generic/acpi.c +++ b/src/drivers/wifi/generic/acpi.c @@ -517,21 +517,19 @@ static void wifi_ssdt_write_properties(const struct device *dev, const char *sco /* Scope */ acpigen_write_scope(scope); - if (dev->path.type == DEVICE_PATH_GENERIC) { - if (config) { - /* Wake capabilities */ - acpigen_write_PRW(config->wake, ACPI_S3); - - /* Add _DSD for DmaProperty property. */ - if (config->is_untrusted) { - struct acpi_dp *dsd, *pkg; - - dsd = acpi_dp_new_table("_DSD"); - pkg = acpi_dp_new_table(ACPI_DSD_DMA_PROPERTY_UUID); - acpi_dp_add_integer(pkg, "DmaProperty", 1); - acpi_dp_add_package(dsd, pkg); - acpi_dp_write(dsd); - } + if (config) { + /* Wake capabilities */ + acpigen_write_PRW(config->wake, ACPI_S3); + + /* Add _DSD for DmaProperty property. */ + if (config->is_untrusted) { + struct acpi_dp *dsd, *pkg; + + dsd = acpi_dp_new_table("_DSD"); + pkg = acpi_dp_new_table(ACPI_DSD_DMA_PROPERTY_UUID); + acpi_dp_add_integer(pkg, "DmaProperty", 1); + acpi_dp_add_package(dsd, pkg); + acpi_dp_write(dsd); } } @@ -603,9 +601,7 @@ void wifi_pcie_fill_ssdt(const struct device *dev) return; wifi_ssdt_write_device(dev, path); - const struct device *child = dev->link_list->children; - if (child && child->path.type == DEVICE_PATH_GENERIC) - wifi_ssdt_write_properties(child, path); + wifi_ssdt_write_properties(dev, path); } const char *wifi_pcie_acpi_name(const struct device *dev) diff --git a/src/drivers/wifi/generic/generic.c b/src/drivers/wifi/generic/generic.c index e8e066bc79..62a543032f 100644 --- a/src/drivers/wifi/generic/generic.c +++ b/src/drivers/wifi/generic/generic.c @@ -21,7 +21,6 @@ struct device_operations wifi_pcie_ops = { .enable_resources = pci_dev_enable_resources, .init = wifi_pci_dev_init, .ops_pci = &pci_dev_ops_pci, - .scan_bus = scan_static_bus, #if CONFIG(HAVE_ACPI_TABLES) .acpi_name = wifi_pcie_acpi_name, .acpi_fill_ssdt = wifi_pcie_fill_ssdt, @@ -42,11 +41,6 @@ struct device_operations wifi_cnvi_ops = { #endif }; -struct device_operations wifi_generic_ops = { - .read_resources = noop_read_resources, - .set_resources = noop_set_resources, -}; - static bool is_cnvi(const struct device *dev) { return dev && dev->path.type != DEVICE_PATH_PCI; @@ -66,11 +60,10 @@ bool wifi_generic_cnvi_ddr_rfim_enabled(const struct device *dev) static void wifi_generic_enable(struct device *dev) { #if !DEVTREE_EARLY - const struct device *parent = dev->bus->dev; - if (parent && parent->ops == &wifi_pcie_ops) - dev->ops = &wifi_generic_ops; - else + if (is_cnvi(dev)) dev->ops = &wifi_cnvi_ops; + else + dev->ops = &wifi_pcie_ops; #endif } -- cgit v1.2.3