Change in coreboot[master]: mb/google/volteer: Enable RTD3 for SD card

[Duncan Laurie (Code Review)]

Duncan Laurie has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47377 )

Change subject: mb/google/volteer: Enable RTD3 for SD card
......................................................................

mb/google/volteer: Enable RTD3 for SD card

Enable the PCIe RTD3 driver for the PCIe attached SD card interface
and provide the enable/reset GPIOs.  These GPIOs are common across
all variants so this is implemented in the baseboard devicetree with
an fw_config probe if the device is present.  The RTS5261 device
does not have an enable GPIO so it is disabled in a workaround in
mainboard.c, along with marking the SD-Express device as external.

BUG=b:162289926, b:162289982
TEST=Tested on Delbin platform to ensure the system can enter the
S0i3.2 substate and suspend/resume is stable.
enabling this for the regular Genesys

Change-Id: I40fe05829783c7bce2a2c4c1520a4a7430642e26
Signed-off-by: Duncan Laurie 
---
M src/mainboard/google/volteer/fw_config.c
M src/mainboard/google/volteer/mainboard.c
M src/mainboard/google/volteer/variants/baseboard/devicetree.cb
3 files changed, 47 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/47377/1

diff --git a/src/mainboard/google/volteer/fw_config.c b/src/mainboard/google/volteer/fw_config.c
index 0538e74..13e356a 100644
--- a/src/mainboard/google/volteer/fw_config.c
+++ b/src/mainboard/google/volteer/fw_config.c
@@ -66,6 +66,10 @@
    PAD_NC(GPP_R7, NONE),
 };

+static const struct pad_config sd_gl9755s_pads[] = {
+	PAD_CFG_GPO(GPP_D16, 1, DEEP),
+};
+
 static void fw_config_handle(void *unused)
 {
    if (fw_config_probe(FW_CONFIG(AUDIO, NONE))) {
@@ -93,5 +97,9 @@
    	gpio_configure_pads(dmic_enable_pads, ARRAY_SIZE(dmic_enable_pads));
    	gpio_configure_pads(sndw_disable_pads, ARRAY_SIZE(sndw_disable_pads));
    }
+	if (fw_config_probe(FW_CONFIG(DB_SD, SD_GL9755S))) {
+		printk(BIOS_INFO, "Configure GPIOs for SD GL9755S.\n");
+		gpio_configure_pads(sd_gl9755s_pads, ARRAY_SIZE(sd_gl9755s_pads));
+	}
 }
 BOOT_STATE_INIT_ENTRY(BS_DEV_ENABLE, BS_ON_ENTRY, fw_config_handle, NULL);
diff --git a/src/mainboard/google/volteer/mainboard.c b/src/mainboard/google/volteer/mainboard.c
index 0850e74..b18ef3d 100644
--- a/src/mainboard/google/volteer/mainboard.c
+++ b/src/mainboard/google/volteer/mainboard.c
@@ -16,6 +16,7 @@
 #include <vb2_api.h>

#include "drivers/intel/pmc_mux/conn/chip.h"
+#include "soc/intel/common/block/pcie/rtd3/chip.h"

extern struct chip_operations drivers_intel_pmc_mux_conn_ops;

@@ -138,6 +139,14 @@
    gpio_configure_pads_with_override(base_pads, base_num, override_pads, override_num);
 }

+extern struct chip_operations soc_intel_common_block_pcie_rtd3_ops;
+
+static bool is_rtd3(struct device *dev)
+{
+	return dev->path.type == DEVICE_PATH_GENERIC && dev->path.generic.id == 0 &&
+		dev->chip_ops == &soc_intel_common_block_pcie_rtd3_ops;
+}
+
 void mainboard_silicon_init_params(FSP_S_CONFIG *params)
 {
    bool has_usb4;
@@ -150,6 +159,25 @@
    	memset(params->ITbtPcieRootPortEn, 0,
    	       ARRAY_SIZE(params->ITbtPcieRootPortEn)
    		       * sizeof(*params->ITbtPcieRootPortEn));
+
+	/* Configure SD-Express device with differences from devicetree. */
+	if (fw_config_probe(FW_CONFIG(DB_SD, SD_RTS5261))) {
+		const struct device *rp8 = pcidev_path_on_root(PCH_DEVFN_PCIE8);
+		const struct device *rtd3;
+		struct soc_intel_common_block_pcie_rtd3_config *config;
+
+		if (rp8 && rp8->link_list->children) {
+			rtd3 = dev_find_matching_device_on_bus(rp8->link_list, is_rtd3);
+			if (rtd3) {
+				printk(BIOS_INFO, "Configure SD-Express as External Port\n");
+				config = rtd3->chip_info;
+				config->is_external = 1;
+
+				/* Disable enable GPIO, pin is used for card present. */
+				config->enable_gpio.pin_count = 0;
+			}
+		}
+	}
 }

struct chip_operations mainboard_ops = {
diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb
index 20a3843..d923843 100644
--- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb
+++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb
@@ -444,7 +444,17 @@
    	device ref sata on end
    	device ref pcie_rp1 on end
    	device ref pcie_rp7 on end
-		device ref pcie_rp8 on end
+		device ref pcie_rp8 on
+			probe DB_SD SD_GL9755S
+			probe DB_SD SD_RTS5261
+			chip soc/intel/common/block/pcie/rtd3
+				# Enable GPIO is disabled in mainboard.c for RTS5261
+				register "enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_D16)"
+				register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_H3)"
+				register "srcclk_pin" = "3"
+				device generic 0 on end
+			end
+		end
    	device ref pcie_rp9 on end
    	device ref pcie_rp11 on end
    	device ref uart0 on end

[Duncan Laurie (Code Review)]

Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47377 )

Change subject: mb/google/volteer: Enable RTD3 for SD card
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/47377/1/src/mainboard/google/voltee... 
File src/mainboard/google/volteer/mainboard.c:

https://review.coreboot.org/c/coreboot/+/47377/1/src/mainboard/google/voltee... 
PS1, Line 170: dev_find_matching_device_on_bus
More of the same mess where we want slightly different configuration in devicetree for different fw_config options but don't have a good way to do it...

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47377 )

Change subject: mb/google/volteer: Enable RTD3 for SD card
......................................................................

Patch Set 2: Code-Review+2

[Furquan Shaikh (Code Review)]

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

Change subject: mb/google/volteer: Enable RTD3 for SD card
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/47377/2/src/mainboard/google/voltee... 
File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/47377/2/src/mainboard/google/voltee... 
PS2, Line 451: # Enable GPIO is disabled in mainboard.c for RTS5261
Can this not be handled in devicetree using multiple entries each with its own probe property?

chip soc/intel/common/block/pcie/rtd3
    register "enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_D16)"
    register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_H3)"
    register "srcclk_pin" = "3"
    device generic 0 on
    	probe DB_SD SD_GL9755S
    end
end
chip soc/intel/common/block/pcie/rtd3
    register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_H3)"
    register "srcclk_pin" = "3"
    register "is_external" = "1"
    device generic 1 on
    	probe DB_SD SD_RTS5261
    end
end

[Duncan Laurie (Code Review)]

Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47377 )

Change subject: mb/google/volteer: Enable RTD3 for SD card
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/47377/2/src/mainboard/google/voltee... 
File src/mainboard/google/volteer/variants/baseboard/devicetree.cb:

https://review.coreboot.org/c/coreboot/+/47377/2/src/mainboard/google/voltee... 
PS2, Line 451: # Enable GPIO is disabled in mainboard.c for RTS5261
> Can this not be handled in devicetree using multiple entries each with its own probe property? […]

Yes I had worked up a similar patch this weekend, need to update this.

[Duncan Laurie (Code Review)]

Hello build bot (Jenkins), Tim Wawrzynczak,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/47377

to look at the new patch set (#3).

Change subject: mb/google/volteer: Enable RTD3 for SD card
......................................................................

mb/google/volteer: Enable RTD3 for SD card

Enable the PCIe RTD3 driver for the PCIe attached SD card interface
and provide the enable/reset GPIOs.  These GPIOs are common across
all variants so this is implemented in the baseboard devicetree with
an fw_config probe if the device is present.  The RTS5261 device
does not have an enable GPIO so it is disabled in a workaround in
mainboard.c, along with marking the SD-Express device as external.

BUG=b:162289926, b:162289982
TEST=Tested on Delbin platform to ensure the system can enter the
S0i3.2 substate and suspend/resume is stable.
enabling this for the regular Genesys

Change-Id: I40fe05829783c7bce2a2c4c1520a4a7430642e26
Signed-off-by: Duncan Laurie 
---
M src/mainboard/google/volteer/fw_config.c
M src/mainboard/google/volteer/variants/baseboard/devicetree.cb
2 files changed, 28 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/47377/3

[Furquan Shaikh (Code Review)]

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

Change subject: mb/google/volteer: Enable RTD3 for SD card
......................................................................

Patch Set 3: Code-Review+2

[Duncan Laurie (Code Review)]

Duncan Laurie has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47377 )

Change subject: mb/google/volteer: Enable RTD3 for SD card
......................................................................

mb/google/volteer: Enable RTD3 for SD card

Enable the PCIe RTD3 driver for the PCIe attached SD card interface
and provide the enable/reset GPIOs.  These GPIOs are common across
all variants so this is implemented in the baseboard devicetree with
an fw_config probe if the device is present.  The RTS5261 device
does not have an enable GPIO so it is disabled in a workaround in
mainboard.c, along with marking the SD-Express device as external.

BUG=b:162289926, b:162289982
TEST=Tested on Delbin platform to ensure the system can enter the
S0i3.2 substate and suspend/resume is stable.
enabling this for the regular Genesys

Change-Id: I40fe05829783c7bce2a2c4c1520a4a7430642e26
Signed-off-by: Duncan Laurie 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/47377
Tested-by: build bot (Jenkins) 
Reviewed-by: Furquan Shaikh 
---
M src/mainboard/google/volteer/fw_config.c
M src/mainboard/google/volteer/variants/baseboard/devicetree.cb
2 files changed, 28 insertions(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Furquan Shaikh: Looks good to me, approved

diff --git a/src/mainboard/google/volteer/fw_config.c b/src/mainboard/google/volteer/fw_config.c
index 4067fa5..50e8f93 100644
--- a/src/mainboard/google/volteer/fw_config.c
+++ b/src/mainboard/google/volteer/fw_config.c
@@ -66,6 +66,10 @@
    PAD_NC(GPP_R7, NONE),
 };

+static const struct pad_config sd_gl9755s_pads[] = {
+	PAD_CFG_GPO(GPP_D16, 1, DEEP),
+};
+
 static void fw_config_handle(void *unused)
 {
    if (fw_config_probe(FW_CONFIG(AUDIO, NONE))) {
@@ -94,5 +98,9 @@
    	gpio_configure_pads(dmic_enable_pads, ARRAY_SIZE(dmic_enable_pads));
    	gpio_configure_pads(sndw_disable_pads, ARRAY_SIZE(sndw_disable_pads));
    }
+	if (fw_config_probe(FW_CONFIG(DB_SD, SD_GL9755S))) {
+		printk(BIOS_INFO, "Configure GPIOs for SD GL9755S.\n");
+		gpio_configure_pads(sd_gl9755s_pads, ARRAY_SIZE(sd_gl9755s_pads));
+	}
 }
 BOOT_STATE_INIT_ENTRY(BS_DEV_ENABLE, BS_ON_ENTRY, fw_config_handle, NULL);
diff --git a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb
index 50be736..fa807ba 100644
--- a/src/mainboard/google/volteer/variants/baseboard/devicetree.cb
+++ b/src/mainboard/google/volteer/variants/baseboard/devicetree.cb
@@ -445,7 +445,26 @@
    	device ref sata on end
    	device ref pcie_rp1 on end
    	device ref pcie_rp7 on end
-		device ref pcie_rp8 on end
+		device ref pcie_rp8 on
+			probe DB_SD SD_GL9755S
+			probe DB_SD SD_RTS5261
+			chip soc/intel/common/block/pcie/rtd3
+				register "enable_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_HIGH(GPP_D16)"
+				register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_H3)"
+				register "srcclk_pin" = "3"
+				device generic 0 on
+					probe DB_SD SD_GL9755S
+				end
+			end
+			chip soc/intel/common/block/pcie/rtd3
+				register "reset_gpio" = "ACPI_GPIO_OUTPUT_ACTIVE_LOW(GPP_H3)"
+				register "srcclk_pin" = "3"
+				register "is_external" = "1"
+				device generic 1 on
+					probe DB_SD SD_RTS5261
+				end
+			end
+		end
    	device ref pcie_rp9 on end
    	device ref pcie_rp11 on end
    	device ref uart0 on end