Change in ...coreboot[master]: soc/intel/cannonlake: Add support to disable PCIe WLAN dynamically

[Maulik V Vaghela (Code Review)]

Maulik V Vaghela has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30611

Change subject: soc/intel/cannonlake: Add support to disable PCIe WLAN dynamically
......................................................................

soc/intel/cannonlake: Add support to disable PCIe WLAN dynamically

Ideally we should disable PCIe WLAN if CNVi is enabled and out of reset.
This code adds support to achieve this. SoC code will check if CNVi is
up or not and if is enabled, it'll disable PCIe WLAN.

PCI WLAN device and function are board dependent and mainboard must
implement function which returns PCI_DEVFN for WLAN.

BUG=none
BRANCH=none
TEST=none

Change-Id: Ief1896b3d915018edca136c26f4e834e0c9003ac
Signed-off-by: Maulik V Vaghela 
---
M src/soc/intel/cannonlake/fsp_params.c
1 file changed, 44 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/30611/1

diff --git a/src/soc/intel/cannonlake/fsp_params.c b/src/soc/intel/cannonlake/fsp_params.c
index 8166dea..4225471 100644
--- a/src/soc/intel/cannonlake/fsp_params.c
+++ b/src/soc/intel/cannonlake/fsp_params.c
@@ -17,6 +17,8 @@
 #include <console/console.h>
 #include <device/device.h>
 #include <device/pci.h>
+#include <device/pci_def.h>
+#include <device/pci_ops.h>
 #include <fsp/api.h>
 #include <fsp/util.h>
 #include <intelblocks/xdci.h>
@@ -25,6 +27,42 @@
 #include <soc/ramstage.h>
 #include <string.h>

+static bool is_cnvi_held_in_reset(void)
+{
+	struct device *dev = dev_find_slot(0, PCH_DEVFN_CNVI);
+	uint32_t reg = pci_read_config32(dev, PCI_VENDOR_ID);
+
+	/*
+	 * If vendor/device ID for CNVi reads as 0xffffffff, then it is safe to
+	 * assume that it is being held in reset.
+	 */
+	if (reg == 0xffffffff)
+		return true;
+
+	return false;
+}
+
+/*
+ * Check if CNVi PCI device is released from reset. If yes, then the system is
+ * booting with CNVi module. In this case, the PCIe device for WiFi needs to
+ * be disabled. If CNVi device is held in reset, then disable it.
+ */
+static void wifi_device_update(void)
+{
+	struct device *dev;
+	unsigned int devfn;
+
+	if (is_cnvi_held_in_reset())
+		devfn = PCH_DEVFN_CNVI;
+	else
+		devfn = mainboard_get_wifi_device();
+
+	if (devfn != 0) {
+		dev = dev_find_slot(0, devfn);
+		dev->enabled = 0;
+	}
+}
+
 static void parse_devicetree(FSP_S_CONFIG *params)
 {
    struct device *dev = SA_DEV_ROOT;
@@ -86,6 +124,7 @@
    }

mainboard_silicon_init_params(params);
+	wifi_device_update ();

/* Unlock upper 8 bytes of RTC RAM */
    params->PchLockDownRtcMemoryLock = 0;
@@ -236,3 +275,8 @@
 {
    printk(BIOS_DEBUG, "WEAK: %s/%s called\n", __FILE__, __func__);
 }
+
+__weak uint32_t mainboard_get_wifi_device(void)
+{
+	return 0;
+}

Change in ...coreboot[master]: soc/intel/cannonlake: Add support to disable Cnvi/PCIe WLAN dynamically

[Maulik V Vaghela (Code Review)]

Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30611 )

Change subject: soc/intel/cannonlake: Add support to disable Cnvi/PCIe WLAN dynamically
......................................................................

Patch Set 7:

Hi Furquan/Arthur,

I have checked the bug for which this change was introduced on octopus. In case of octopus, it was creating ssdt entry for CNVi as well as PCIe WLAN and that was the reason, we had to disable device in mainboard.
In case of hatch, PCI resource allocator will disable device itself if CNVi module is not connected and ACPI tables are generated after PCI enumeration process. Since device is disabled, SSDT entry won't be generated and hence we don't need this change here. I have also tested this change on our RVP board and I have also dumped SSDT file and checked for the WIFI entry.
1. No module connected : There is no entry for WIFI in SSDT file.
2. CNVi module connected: Single ASL entry for "WIFI" found with root port 14.3 (_SB.PCI0)
3. PCIe WLAN module connected:Single ASL entry for "WIFI" found with RP9 (_SB.PCI0.RP09)

I will abandon this patch for this reason.
Please let me know your inputs.

Change in ...coreboot[master]: soc/intel/cannonlake: Add support to disable Cnvi/PCIe WLAN dynamically

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30611 )

Change subject: soc/intel/cannonlake: Add support to disable Cnvi/PCIe WLAN dynamically
......................................................................

Patch Set 7:
> Patch Set 7:
>
> Hi Furquan/Arthur,
>
> I have checked the bug for which this change was introduced on octopus. In case of octopus, it was creating ssdt entry for CNVi as well as PCIe WLAN and that was the reason, we had to disable device in mainboard.
> In case of hatch, PCI resource allocator will disable device itself if CNVi module is not connected and ACPI tables are generated after PCI enumeration process. Since device is disabled, SSDT entry won't be generated and hence we don't need this change here. I have also tested this change on our RVP board and I have also dumped SSDT file and checked for the WIFI entry.
> No module connected : There is no entry for WIFI in SSDT file.
> CNVi module connected: Single ASL entry for "WIFI" found with root port 14.3 (_SB.PCI0)
> PCIe WLAN module connected:Single ASL entry for "WIFI" found with RP9 (_SB.PCI0.RP09)
>
> I will abandon this patch for this reason.
> Please let me know your inputs.

Sounds good. But you might still have to configure GPIOs differently within mainboard depending upon the selected configuration.

Change in ...coreboot[master]: soc/intel/cannonlake: Add support to disable Cnvi/PCIe WLAN dynamically

[Maulik V Vaghela (Code Review)]

Maulik V Vaghela has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/30611 )

Change subject: soc/intel/cannonlake: Add support to disable Cnvi/PCIe WLAN dynamically
......................................................................

Abandoned