[PATCH v3 0/8] Add RZ/G3E SDHI support

[PATCH v3 0/8] Add RZ/G3E SDHI support

[Biju Das]

The SD/MMC block on the RZ/G3E ("R9A09G047") SoC is similar to that
of the RZ/V2H, but the SD0 channel has only dedicated pins, so we must
use SD_STATUS register to control voltage and power enable (internal
regulator).

For SD1 and SD2 channel we can either use gpio regulator or internal
regulator (using SD_STATUS register) for voltage switching.

For SD0, fixed voltage(eMMC) uses fixed regulator and non-fixed voltage
(SD) uses internal regulator.

v2->v3:
 * Collected tags
 * Renamed internal regulator labels vqmmc_sdhi{0..2}->sdhi{0..2}_vqmmc.
 * Updated regulator phandles on SoM/Board dts.
 * Dropped renaming the gpio regulator label vqmmc_sdhi1->vqmmc_sdhi1_gpio.
 * Renamed node sd0emmc->sd0-emmc
 * Renamed sd0-emmc-{ctrl,data,rst}->sd0-{ctrl,data,rst}
 * Moved header file gpio.h from patch#6 to patch#8.
 * Dropped overriding internal regulator name.
 * Dropped #if guard in pinctrl node for SDHI0
 * Renamed the label/node sdhi0_pins: sd0->sdhi0_usd_pins: sd0-usd.
v1->v2:
 * Collected tags.
 * Documented internal regulator as optional property for both RZ/G3E and
   RZ/V2H SoCs.
 * Updated commit description for regulator used in SD0 fixed and
   non-fixed voltage case in patch#3.
 * As the node enabling of internal regulator is controlled through status,
   added a check for device availability.
 * Status of internal regulator is disabled in the SoC .dtsi. Override
   the status in the board DTS when needed.
 * Added support for enabling SDHI internal regulator in RZ/V2H
 * Added missing header file gpio.h
 * Used fixed regulator for eMMC on SD0 and dropped sd0-iovs pins for
   eMMC.
 * Sorted pinctrl nodes for sd2
 * Enabled internal regulator for SD2.
 * Added support for enabling SD on SDHI0
 * Replaced the regulator usd_vdd_3p3v->reg_3p3v.
 * Renamed the gpio-hog node sd1-pwr-en->sd1-pwr-en-hog.
 * Sorted sd1 pin ctrl nodes.

Biju Das (8):
  dt-bindings: mmc: renesas,sdhi: Document RZ/G3E support
  mmc: renesas_sdhi: Arrange local variables in reverse xmas tree order
  mmc: renesas_sdhi: Add support for RZ/G3E SoC
  arm64: dts: renesas: r9a09g047: Add SDHI0-SDHI2 nodes
  arm64: dts: renesas: r9a09g057: Add support for enabling SDHI internal
    regulator
  arm64: dts: renesas: rzg3e-smarc-som: Enable SDHI{0,2}
  arm64: dts: renesas: rzg3e-smarc-som: Add support for enable SD on
    SDHI0
  arm64: dts: renesas: r9a09g047e57-smarc: Enable SDHI1

 .../devicetree/bindings/mmc/renesas,sdhi.yaml |  16 ++
 arch/arm64/boot/dts/renesas/r9a09g047.dtsi    |  60 +++++++
 .../boot/dts/renesas/r9a09g047e57-smarc.dts   |  49 ++++++
 arch/arm64/boot/dts/renesas/r9a09g057.dtsi    |  21 +++
 .../boot/dts/renesas/renesas-smarc2.dtsi      |  18 ++
 .../boot/dts/renesas/rzg3e-smarc-som.dtsi     | 154 ++++++++++++++++++
 drivers/mmc/host/renesas_sdhi.h               |   1 +
 drivers/mmc/host/renesas_sdhi_core.c          | 136 +++++++++++++++-
 drivers/mmc/host/tmio_mmc.h                   |   5 +
 9 files changed, 459 insertions(+), 1 deletion(-)

-- 
2.43.0

[PATCH v3 1/8] dt-bindings: mmc: renesas,sdhi: Document RZ/G3E support

[Biju Das]

The SD/MMC block on the RZ/G3E ("R9A09G047") SoC is similar to that
of the RZ/V2H, but the SD0 channel has only dedicated pins, so we must
use SD_STATUS register to control voltage and power enable (internal
regulator), for non-fixed voltage (SD) MMC interface. However, it is
optional for fixed voltage MMC interface (eMMC).

For SD1 and SD2 channels, we can either use gpio regulator or internal
regulator (using SD_STATUS register) for voltage switching.

Document RZ/G3E SDHI IP support with optional internal regulator for
both RZ/G3E and RZ/V2H SoC.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2->v3:
 * Collected tags.
v1->v2:
 * Dropped tags.
 * Documented internal regulator as optional property for both RZ/G3E and
   RZ/V2H SoCs.
---
 .../devicetree/bindings/mmc/renesas,sdhi.yaml    | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
index af378b9ff3f4..773baa6c2656 100644
--- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
+++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
@@ -68,6 +68,9 @@ properties:
               - renesas,sdhi-r9a08g045 # RZ/G3S
               - renesas,sdhi-r9a09g011 # RZ/V2M
           - const: renesas,rzg2l-sdhi
+      - items:
+          - const: renesas,sdhi-r9a09g047 # RZ/G3E
+          - const: renesas,sdhi-r9a09g057 # RZ/V2H(P)
 
   reg:
     maxItems: 1
@@ -211,6 +214,19 @@ allOf:
         sectioned off to be run by a separate second clock source to allow
         the main core clock to be turned off to save power.
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,sdhi-r9a09g057
+    then:
+      properties:
+        vqmmc-regulator:
+          type: object
+          description: VQMMC SD regulator
+          $ref: /schemas/regulator/regulator.yaml#
+          unevaluatedProperties: false
+
 required:
   - compatible
   - reg
-- 
2.43.0

[PATCH v3 2/8] mmc: renesas_sdhi: Arrange local variables in reverse xmas tree order

[Biju Das]

Arrange local variables in reverse xmas tree for probe().

Reviewed-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v2->v3:
 * No change.
v1->v2:
 * Collected tags.
---
 drivers/mmc/host/renesas_sdhi_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index f73b84bae0c4..6ea651409774 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -910,8 +910,8 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 		       const struct renesas_sdhi_quirks *quirks)
 {
 	struct tmio_mmc_data *mmd = pdev->dev.platform_data;
-	struct tmio_mmc_data *mmc_data;
 	struct renesas_sdhi_dma *dma_priv;
+	struct tmio_mmc_data *mmc_data;
 	struct tmio_mmc_host *host;
 	struct renesas_sdhi *priv;
 	int num_irqs, irq, ret, i;
-- 
2.43.0

Re: [PATCH v3 2/8] mmc: renesas_sdhi: Arrange local variables in reverse xmas tree order

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 226 bytes --]


>  	struct tmio_mmc_data *mmd = pdev->dev.platform_data;
> -	struct tmio_mmc_data *mmc_data;
>  	struct renesas_sdhi_dma *dma_priv;
> +	struct tmio_mmc_data *mmc_data;

I'd prefer to have the same types close to each other.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH v3 2/8] mmc: renesas_sdhi: Arrange local variables in reverse xmas tree order

[Biju Das]

Hi Wolfram,

Thanks for the feedback.

> -----Original Message-----
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Sent: 28 February 2025 09:36
> Subject: Re: [PATCH v3 2/8] mmc: renesas_sdhi: Arrange local variables in reverse xmas tree order
> 
> 
> >  	struct tmio_mmc_data *mmd = pdev->dev.platform_data;
> > -	struct tmio_mmc_data *mmc_data;
> >  	struct renesas_sdhi_dma *dma_priv;
> > +	struct tmio_mmc_data *mmc_data;
> 
> I'd prefer to have the same types close to each other.

OK, will drop this patch.

Cheers,
Biju

[PATCH v3 3/8] mmc: renesas_sdhi: Add support for RZ/G3E SoC

[Biju Das]

The SDHI/eMMC IPs in the RZ/G3E SoC are similar to those in R-Car Gen3.
However, the RZ/G3E SD0 channel has Voltage level control and PWEN pin
support via SD_STATUS register.

internal regulator support is added to control the voltage levels of
the SD pins via sd_iovs/sd_pwen bits in SD_STATUS register by populating
vqmmc-regulator child node.

SD1 and SD2 channels have gpio regulator support and internal regulator
support. Selection of the regulator is based on the regulator phandle.
Similar case for SD0 fixed voltage (eMMC) that uses fixed regulator and
SD0 non-fixed voltage (SD0) that uses internal regulator.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v2->v3:
 * No change.
v1->v2:
 * Updated commit description for regulator used in SD0 fixed and
   non-fixed voltage case.
 * As the node enabling of internal regulator is controlled through status,
   added a check for device availability.
---
 drivers/mmc/host/renesas_sdhi.h      |   1 +
 drivers/mmc/host/renesas_sdhi_core.c | 134 +++++++++++++++++++++++++++
 drivers/mmc/host/tmio_mmc.h          |   5 +
 3 files changed, 140 insertions(+)

diff --git a/drivers/mmc/host/renesas_sdhi.h b/drivers/mmc/host/renesas_sdhi.h
index f12a87442338..291ddb4ad9be 100644
--- a/drivers/mmc/host/renesas_sdhi.h
+++ b/drivers/mmc/host/renesas_sdhi.h
@@ -95,6 +95,7 @@ struct renesas_sdhi {
 
 	struct reset_control *rstc;
 	struct tmio_mmc_host *host;
+	struct regulator_dev *rdev;
 };
 
 #define host_to_priv(host) \
diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 6ea651409774..99700d89aa8c 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -32,6 +32,8 @@
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
 #include <linux/regulator/consumer.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/of_regulator.h>
 #include <linux/reset.h>
 #include <linux/sh_dma.h>
 #include <linux/slab.h>
@@ -581,12 +583,24 @@ static void renesas_sdhi_reset(struct tmio_mmc_host *host, bool preserve)
 
 	if (!preserve) {
 		if (priv->rstc) {
+			u32 sd_status;
+			/*
+			 * HW reset might have toggled the regulator state in
+			 * HW which regulator core might be unaware of so save
+			 * and restore the regulator state during HW reset.
+			 */
+			if (priv->rdev)
+				sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);
+
 			reset_control_reset(priv->rstc);
 			/* Unknown why but without polling reset status, it will hang */
 			read_poll_timeout(reset_control_status, ret, ret == 0, 1, 100,
 					  false, priv->rstc);
 			/* At least SDHI_VER_GEN2_SDR50 needs manual release of reset */
 			sd_ctrl_write16(host, CTL_RESET_SD, 0x0001);
+			if (priv->rdev)
+				sd_ctrl_write32(host, CTL_SD_STATUS, sd_status);
+
 			priv->needs_adjust_hs400 = false;
 			renesas_sdhi_set_clock(host, host->clk_cache);
 
@@ -904,15 +918,113 @@ static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable)
 	renesas_sdhi_sdbuf_width(host, enable ? width : 16);
 }
 
+static const unsigned int renesas_sdhi_vqmmc_voltages[] = {
+	3300000, 1800000
+};
+
+static int renesas_sdhi_regulator_disable(struct regulator_dev *rdev)
+{
+	struct tmio_mmc_host *host = rdev_get_drvdata(rdev);
+	u32 sd_status;
+
+	sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);
+	sd_status &= ~SD_STATUS_PWEN;
+	sd_ctrl_write32(host, CTL_SD_STATUS, sd_status);
+
+	return 0;
+}
+
+static int renesas_sdhi_regulator_enable(struct regulator_dev *rdev)
+{
+	struct tmio_mmc_host *host = rdev_get_drvdata(rdev);
+	u32 sd_status;
+
+	sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);
+	sd_status |= SD_STATUS_PWEN;
+	sd_ctrl_write32(host, CTL_SD_STATUS, sd_status);
+
+	return 0;
+}
+
+static int renesas_sdhi_regulator_is_enabled(struct regulator_dev *rdev)
+{
+	struct tmio_mmc_host *host = rdev_get_drvdata(rdev);
+	u32 sd_status;
+
+	sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);
+
+	return (sd_status & SD_STATUS_PWEN) ? 1 : 0;
+}
+
+static int renesas_sdhi_regulator_get_voltage(struct regulator_dev *rdev)
+{
+	struct tmio_mmc_host *host = rdev_get_drvdata(rdev);
+	u32 sd_status;
+
+	sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);
+
+	return (sd_status & SD_STATUS_IOVS) ? 1800000 : 3300000;
+}
+
+static int renesas_sdhi_regulator_set_voltage(struct regulator_dev *rdev,
+					      int min_uV, int max_uV,
+					      unsigned int *selector)
+{
+	struct tmio_mmc_host *host = rdev_get_drvdata(rdev);
+	u32 sd_status;
+
+	sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);
+	if (min_uV >= 1700000 && max_uV <= 1950000) {
+		sd_status |= SD_STATUS_IOVS;
+		*selector = 1;
+	} else {
+		sd_status &= ~SD_STATUS_IOVS;
+		*selector = 0;
+	}
+	sd_ctrl_write32(host, CTL_SD_STATUS, sd_status);
+
+	return 0;
+}
+
+static int renesas_sdhi_regulator_list_voltage(struct regulator_dev *rdev,
+					       unsigned int selector)
+{
+	if (selector >= ARRAY_SIZE(renesas_sdhi_vqmmc_voltages))
+		return -EINVAL;
+
+	return renesas_sdhi_vqmmc_voltages[selector];
+}
+
+static const struct regulator_ops renesas_sdhi_regulator_voltage_ops = {
+	.enable = renesas_sdhi_regulator_enable,
+	.disable = renesas_sdhi_regulator_disable,
+	.is_enabled = renesas_sdhi_regulator_is_enabled,
+	.list_voltage = renesas_sdhi_regulator_list_voltage,
+	.get_voltage = renesas_sdhi_regulator_get_voltage,
+	.set_voltage = renesas_sdhi_regulator_set_voltage,
+};
+
+static struct regulator_desc renesas_sdhi_vqmmc_regulator = {
+	.of_match	= of_match_ptr("vqmmc-regulator"),
+	.owner		= THIS_MODULE,
+	.type		= REGULATOR_VOLTAGE,
+	.ops		= &renesas_sdhi_regulator_voltage_ops,
+	.volt_table	= renesas_sdhi_vqmmc_voltages,
+	.n_voltages	= ARRAY_SIZE(renesas_sdhi_vqmmc_voltages),
+};
+
 int renesas_sdhi_probe(struct platform_device *pdev,
 		       const struct tmio_mmc_dma_ops *dma_ops,
 		       const struct renesas_sdhi_of_data *of_data,
 		       const struct renesas_sdhi_quirks *quirks)
 {
+	struct regulator_config rcfg = { .dev = &pdev->dev, };
 	struct tmio_mmc_data *mmd = pdev->dev.platform_data;
 	struct renesas_sdhi_dma *dma_priv;
+	struct device *dev = &pdev->dev;
 	struct tmio_mmc_data *mmc_data;
 	struct tmio_mmc_host *host;
+	struct regulator_dev *rdev;
 	struct renesas_sdhi *priv;
 	int num_irqs, irq, ret, i;
 	struct resource *res;
@@ -1053,6 +1165,28 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 	if (ret)
 		goto efree;
 
+	rcfg.of_node = of_get_child_by_name(dev->of_node, "vqmmc-regulator");
+	if (!of_device_is_available(rcfg.of_node)) {
+		of_node_put(rcfg.of_node);
+		rcfg.of_node = NULL;
+	}
+
+	if (rcfg.of_node) {
+		rcfg.driver_data = priv->host;
+
+		renesas_sdhi_vqmmc_regulator.name = "sdhi-vqmmc-regulator";
+		renesas_sdhi_vqmmc_regulator.of_match = of_match_ptr("vqmmc-regulator");
+		renesas_sdhi_vqmmc_regulator.type = REGULATOR_VOLTAGE;
+		renesas_sdhi_vqmmc_regulator.owner = THIS_MODULE;
+		rdev = devm_regulator_register(dev, &renesas_sdhi_vqmmc_regulator, &rcfg);
+		of_node_put(rcfg.of_node);
+		if (IS_ERR(rdev)) {
+			dev_err(dev, "regulator register failed err=%ld", PTR_ERR(rdev));
+			goto efree;
+		}
+		priv->rdev = rdev;
+	}
+
 	ver = sd_ctrl_read16(host, CTL_VERSION);
 	/* GEN2_SDR104 is first known SDHI to use 32bit block count */
 	if (ver < SDHI_VER_GEN2_SDR104 && mmc_data->max_blk_count > U16_MAX)
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index a75755f31d31..5970ca598850 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -44,6 +44,7 @@
 #define CTL_RESET_SD 0xe0
 #define CTL_VERSION 0xe2
 #define CTL_SDIF_MODE 0xe6 /* only known on R-Car 2+ */
+#define CTL_SD_STATUS 0xf2 /* only known on RZ/{G2L,G3E,V2H} */
 
 /* Definitions for values the CTL_STOP_INTERNAL_ACTION register can take */
 #define TMIO_STOP_STP		BIT(0)
@@ -103,6 +104,10 @@
 /* Definitions for values the CTL_SDIF_MODE register can take */
 #define SDIF_MODE_HS400		BIT(0) /* only known on R-Car 2+ */
 
+/* Definitions for values the CTL_SD_STATUS register can take */
+#define SD_STATUS_PWEN		BIT(0) /* only known on RZ/{G3E,V2H} */
+#define SD_STATUS_IOVS		BIT(16) /* only known on RZ/{G3E,V2H} */
+
 /* Define some IRQ masks */
 /* This is the mask used at reset by the chip */
 #define TMIO_MASK_ALL           0x837f031d
-- 
2.43.0

Re: [PATCH v3 3/8] mmc: renesas_sdhi: Add support for RZ/G3E SoC

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 369 bytes --]


> +			/*
> +			 * HW reset might have toggled the regulator state in
> +			 * HW which regulator core might be unaware of so save
> +			 * and restore the regulator state during HW reset.
> +			 */

Since this is a hard reset, can't we just reset the regulator to an initial
state? It seems strange to preserve a value when the 'preserve' flag is
explicitly not set.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH v3 3/8] mmc: renesas_sdhi: Add support for RZ/G3E SoC

[Biju Das]

Hi Wolfram,

Thanks for the feedback.

> -----Original Message-----
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Sent: 27 February 2025 15:16
> Subject: Re: [PATCH v3 3/8] mmc: renesas_sdhi: Add support for RZ/G3E SoC
> 
> 
> > +			/*
> > +			 * HW reset might have toggled the regulator state in
> > +			 * HW which regulator core might be unaware of so save
> > +			 * and restore the regulator state during HW reset.
> > +			 */
> 
> Since this is a hard reset, can't we just reset the regulator to an initial state? It seems strange to
> preserve a value when the 'preserve' flag is explicitly not set.

Assume, this happens after the card is switched to UHS state and 
then the command won't work in UHS state if we bring the regulator
initial state of 3.3V ,

In external regulator case, we don't toggle the regulator to initial state of 3.3V.
That is the reason it is still working even when hard reset is applied in UHS state.

Am I missing anything please let me know? I can debug further.

Cheers,
Biju

RE: [PATCH v3 3/8] mmc: renesas_sdhi: Add support for RZ/G3E SoC

[Biju Das]

Hi Wolfram,

> -----Original Message-----
> From: Biju Das <biju.das.jz@bp.renesas.com>
> Sent: 27 February 2025 15:30
> Subject: RE: [PATCH v3 3/8] mmc: renesas_sdhi: Add support for RZ/G3E SoC
> 
> Hi Wolfram,
> 
> Thanks for the feedback.
> 
> > -----Original Message-----
> > From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > Sent: 27 February 2025 15:16
> > Subject: Re: [PATCH v3 3/8] mmc: renesas_sdhi: Add support for RZ/G3E
> > SoC
> >
> >
> > > +			/*
> > > +			 * HW reset might have toggled the regulator state in
> > > +			 * HW which regulator core might be unaware of so save
> > > +			 * and restore the regulator state during HW reset.
> > > +			 */
> >
> > Since this is a hard reset, can't we just reset the regulator to an
> > initial state? It seems strange to preserve a value when the 'preserve' flag is explicitly not set.
> 
> Assume, this happens after the card is switched to UHS state and then the command won't work in UHS
> state if we bring the regulator initial state of 3.3V ,
> 
> In external regulator case, we don't toggle the regulator to initial state of 3.3V.
> That is the reason it is still working even when hard reset is applied in UHS state.
> 
> Am I missing anything please let me know? I can debug further.

Previously I got an issue, where the card was unable to detect after hot removal and plug.
Now it is working without this change.

Not sure the previous issue related to delay associated with hard reset.
I am doing more testing without this change.

Cheers,
Biju

RE: [PATCH v3 3/8] mmc: renesas_sdhi: Add support for RZ/G3E SoC

[Biju Das]

Hi Wolfram,

> -----Original Message-----
> From: Biju Das
> Sent: 27 February 2025 16:27
> Subject: RE: [PATCH v3 3/8] mmc: renesas_sdhi: Add support for RZ/G3E SoC
> 
> Hi Wolfram,
> 
> > -----Original Message-----
> > From: Biju Das <biju.das.jz@bp.renesas.com>
> > Sent: 27 February 2025 15:30
> > Subject: RE: [PATCH v3 3/8] mmc: renesas_sdhi: Add support for RZ/G3E
> > SoC
> >
> > Hi Wolfram,
> >
> > Thanks for the feedback.
> >
> > > -----Original Message-----
> > > From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > > Sent: 27 February 2025 15:16
> > > Subject: Re: [PATCH v3 3/8] mmc: renesas_sdhi: Add support for
> > > RZ/G3E SoC
> > >
> > >
> > > > +			/*
> > > > +			 * HW reset might have toggled the regulator state in
> > > > +			 * HW which regulator core might be unaware of so save
> > > > +			 * and restore the regulator state during HW reset.
> > > > +			 */
> > >
> > > Since this is a hard reset, can't we just reset the regulator to an
> > > initial state? It seems strange to preserve a value when the 'preserve' flag is explicitly not
> set.
> >
> > Assume, this happens after the card is switched to UHS state and then
> > the command won't work in UHS state if we bring the regulator initial
> > state of 3.3V ,
> >
> > In external regulator case, we don't toggle the regulator to initial state of 3.3V.
> > That is the reason it is still working even when hard reset is applied in UHS state.
> >
> > Am I missing anything please let me know? I can debug further.
> 
> Previously I got an issue, where the card was unable to detect after hot removal and plug.
> Now it is working without this change.
> 
> Not sure the previous issue related to delay associated with hard reset.
> I am doing more testing without this change.

I have added some debug code[1], with SDHI1 using internal regulator as it is easy to do insert/removal of SD cards:

Without the regulator register restore code hot removal/plug:
-------------------------------------------------------------

root@smarc-rzg3e:~# [   31.400277] mmc1: card aaaa removed
[   31.450751] renesas_sdhi_internal_dmac 15c10000.mmc: #####renesas_sdhi_reset 595 sd_status before reset=10000
[   31.460812] renesas_sdhi_internal_dmac 15c10000.mmc: #####renesas_sdhi_reset 607 sd_status after reset=1
[   35.811808] SDHI1-VQMMC: disabling
[   36.526838] renesas_sdhi_internal_dmac 15c10000.mmc: #####renesas_sdhi_reset 595 sd_status before reset=0
[   36.536642] renesas_sdhi_internal_dmac 15c10000.mmc: #####renesas_sdhi_reset 607 sd_status after reset=1
[   39.322682] renesas_sdhi_internal_dmac 15c10000.mmc: #####renesas_sdhi_reset 607 sd_status after reset=1
[   39.361826] mmc1: Skipping voltage switch
[   39.610915] mmc1: new high speed SDHC card at address aaaa
[   39.618237] mmcblk1: mmc1:aaaa SE32G 29.7 GiB
[   39.629668]  mmcblk1: p1

With the regulator register restore code hot removal/plug:
--------------------------------------------------------

root@smarc-rzg3e:~# [   23.886933] mmc1: card aaaa removed
[   23.937416] renesas_sdhi_internal_dmac 15c10000.mmc: #####renesas_sdhi_reset 595 sd_status before reset=10000
[   25.581984] mmc1: new UHS-I speed SDR104 SDHC card at address aaaa
[   25.589989] mmcblk1: mmc1:aaaa SE32G 29.7 GiB
[   25.600507]  mmcblk1: p1

root@smarc-rzg3e:~#


Please share your thoughts.


[1]

+                       dev_err(&host->pdev->dev,"#####%s %d sd_status before reset=%x",__func__,__LINE__, sd_status);

                        reset_control_reset(priv->rstc);
                        /* Unknown why but without polling reset status, it will hang */
                        read_poll_timeout(reset_control_status, ret, ret == 0, 1, 100,
@@ -599,8 +602,13 @@ static void renesas_sdhi_reset(struct tmio_mmc_host *host, bool preserve)
                        /* At least SDHI_VER_GEN2_SDR50 needs manual release of reset */
                        sd_ctrl_write16(host, CTL_RESET_SD, 0x0001);
                        if (priv->rdev)
-                               sd_ctrl_write32(host, CTL_SD_STATUS, sd_status);
+                               sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);
+
+                       dev_err(&host->pdev->dev,"#####%s %d sd_status after reset=%x",__func__,__LINE__, sd_status);



Cheers,
Biju

RE: [PATCH v3 3/8] mmc: renesas_sdhi: Add support for RZ/G3E SoC

[Biju Das]

Hi Wolfram,

> -----Original Message-----
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Sent: 27 February 2025 15:16
> Subject: Re: [PATCH v3 3/8] mmc: renesas_sdhi: Add support for RZ/G3E SoC
> 
> 
> > +			/*
> > +			 * HW reset might have toggled the regulator state in
> > +			 * HW which regulator core might be unaware of so save
> > +			 * and restore the regulator state during HW reset.
> > +			 */
> 
> Since this is a hard reset, can't we just reset the regulator to an initial state? It seems strange to
> preserve a value when the 'preserve' flag is explicitly not set.

For internal regulator case this also works OK. In this case, we don't need to preserve the value.

This avoids changing internal vqmmc regulator from 1.8V to 3.3V during UHS state.
Currently hardware reset causes vqmmc regulator to change from 1.8V to 3.3V.

-       if (!preserve) {
+       if (!preserve && !priv->rdev) {

Please share your thoughts.

Cheers,
Biju

Re: [PATCH v3 3/8] mmc: renesas_sdhi: Add support for RZ/G3E SoC

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 249 bytes --]


> -       if (!preserve) {
> +       if (!preserve && !priv->rdev) {

'!preserve' basically means a hard reset is requested.  We should then
try hard to actually do a hard reset and not only a soft reset. So, this
approach is the wrong path IMHO.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 3/8] mmc: renesas_sdhi: Add support for RZ/G3E SoC

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 1041 bytes --]

On Thu, Feb 06, 2025 at 01:40:27PM +0000, Biju Das wrote:
> The SDHI/eMMC IPs in the RZ/G3E SoC are similar to those in R-Car Gen3.
> However, the RZ/G3E SD0 channel has Voltage level control and PWEN pin
> support via SD_STATUS register.
> 
> internal regulator support is added to control the voltage levels of
> the SD pins via sd_iovs/sd_pwen bits in SD_STATUS register by populating
> vqmmc-regulator child node.
> 
> SD1 and SD2 channels have gpio regulator support and internal regulator
> support. Selection of the regulator is based on the regulator phandle.
> Similar case for SD0 fixed voltage (eMMC) that uses fixed regulator and
> SD0 non-fixed voltage (SD0) that uses internal regulator.

Okay, since I don't see a constant state of the regulator, let's just
restore the original value as you do here. I mean it works.

> +			if (priv->rdev)
> +				sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);

What about introducing sd_ctrl_read32? Or ask the engineers to move
SD_STATUS_IOVS to bit 15 ;)


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH v3 3/8] mmc: renesas_sdhi: Add support for RZ/G3E SoC

[Biju Das]

Hi Wolfram Sang,

Thanks for the feedback.

> -----Original Message-----
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Sent: 28 February 2025 09:42
> Subject: Re: [PATCH v3 3/8] mmc: renesas_sdhi: Add support for RZ/G3E SoC
> 
> On Thu, Feb 06, 2025 at 01:40:27PM +0000, Biju Das wrote:
> > The SDHI/eMMC IPs in the RZ/G3E SoC are similar to those in R-Car Gen3.
> > However, the RZ/G3E SD0 channel has Voltage level control and PWEN pin
> > support via SD_STATUS register.
> >
> > internal regulator support is added to control the voltage levels of
> > the SD pins via sd_iovs/sd_pwen bits in SD_STATUS register by
> > populating vqmmc-regulator child node.
> >
> > SD1 and SD2 channels have gpio regulator support and internal
> > regulator support. Selection of the regulator is based on the regulator phandle.
> > Similar case for SD0 fixed voltage (eMMC) that uses fixed regulator
> > and
> > SD0 non-fixed voltage (SD0) that uses internal regulator.
> 
> Okay, since I don't see a constant state of the regulator, let's just restore the original value as
> you do here. I mean it works.

OK.

> 
> > +			if (priv->rdev)
> > +				sd_ctrl_read32_rep(host, CTL_SD_STATUS, &sd_status, 1);
> 
> What about introducing sd_ctrl_read32?

Agreed, will introduce sd_ctrl_read32().

Cheers,
Biju

[PATCH v3 4/8] arm64: dts: renesas: r9a09g047: Add SDHI0-SDHI2 nodes

[Biju Das]

Add SDHI0-SDHI2 nodes to RZ/G3E ("R9A09G047") SoC DTSI.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v2->v3:
 * Renamed internal regulator labels vqmmc_sdhi{0..2}->sdhi{0..2}_vqmmc.
v1->v2:
 * Status of internal regulator is disabled in the SoC .dtsi. Override
   the status in the board DTS when needed.
---
 arch/arm64/boot/dts/renesas/r9a09g047.dtsi | 60 ++++++++++++++++++++++
 1 file changed, 60 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
index c93aa16d0a6e..fe5eec87f2a5 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
@@ -410,6 +410,66 @@ gic: interrupt-controller@14900000 {
 			interrupt-controller;
 			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>;
 		};
+
+		sdhi0: mmc@15c00000  {
+			compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
+			reg = <0x0 0x15c00000 0 0x10000>;
+			interrupts = <GIC_SPI 735 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 736 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 0xa3>, <&cpg CPG_MOD 0xa5>,
+				 <&cpg CPG_MOD 0xa4>, <&cpg CPG_MOD 0xa6>;
+			clock-names = "core", "clkh", "cd", "aclk";
+			resets = <&cpg 0xa7>;
+			power-domains = <&cpg>;
+			status = "disabled";
+
+			sdhi0_vqmmc: vqmmc-regulator {
+				regulator-name = "SDHI0-VQMMC";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+				status = "disabled";
+			};
+		};
+
+		sdhi1: mmc@15c10000 {
+			compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
+			reg = <0x0 0x15c10000 0 0x10000>;
+			interrupts = <GIC_SPI 737 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 738 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 0xa7>, <&cpg CPG_MOD 0xa9>,
+				 <&cpg CPG_MOD 0xa8>, <&cpg CPG_MOD 0xaa>;
+			clock-names = "core", "clkh", "cd", "aclk";
+			resets = <&cpg 0xa8>;
+			power-domains = <&cpg>;
+			status = "disabled";
+
+			sdhi1_vqmmc: vqmmc-regulator {
+				regulator-name = "SDHI1-VQMMC";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+				status = "disabled";
+			};
+		};
+
+		sdhi2: mmc@15c20000 {
+			compatible = "renesas,sdhi-r9a09g047", "renesas,sdhi-r9a09g057";
+			reg = <0x0 0x15c20000 0 0x10000>;
+			interrupts = <GIC_SPI 739 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 740 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD 0xab>, <&cpg CPG_MOD 0xad>,
+				 <&cpg CPG_MOD 0xac>, <&cpg CPG_MOD 0xae>;
+			clock-names = "core", "clkh", "cd", "aclk";
+			resets = <&cpg 0xa9>;
+			power-domains = <&cpg>;
+			status = "disabled";
+
+			sdhi2_vqmmc: vqmmc-regulator {
+				regulator-name = "SDHI2-VQMMC";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+				status = "disabled";
+			};
+		};
 	};
 
 	timer {
-- 
2.43.0

Re: [PATCH v3 4/8] arm64: dts: renesas: r9a09g047: Add SDHI0-SDHI2 nodes

[Geert Uytterhoeven]

On Thu, 6 Feb 2025 at 14:41, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Add SDHI0-SDHI2 nodes to RZ/G3E ("R9A09G047") SoC DTSI.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v2->v3:
>  * Renamed internal regulator labels vqmmc_sdhi{0..2}->sdhi{0..2}_vqmmc.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Queueing in renesas-devel is postponed, pending acceptance of the DT
bindings by the MMC maintainer.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 4/8] arm64: dts: renesas: r9a09g047: Add SDHI0-SDHI2 nodes

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 352 bytes --]

On Thu, Feb 06, 2025 at 01:40:28PM +0000, Biju Das wrote:
> Add SDHI0-SDHI2 nodes to RZ/G3E ("R9A09G047") SoC DTSI.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

I don't have a datasheet, so I can't check the numbers. The general
structure looks good to me, though:

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 5/8] arm64: dts: renesas: r9a09g057: Add support for enabling SDHI internal regulator

[Biju Das]

Add support for enabling SDHI internal regulator, by overriding the
status on the board DTS, when needed.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 * Updated commit description
 * Renamed internal regulator labels vqmmc_sdhi{0..2}->sdhi{0..2}_vqmmc.
 * Dropped renaming the gpio regulator label vqmmc_sdhi1->vqmmc_sdhi1_gpio.
---
 arch/arm64/boot/dts/renesas/r9a09g057.dtsi | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a09g057.dtsi b/arch/arm64/boot/dts/renesas/r9a09g057.dtsi
index f7a2f8ca864f..bdaab9ae10b8 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g057.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a09g057.dtsi
@@ -602,6 +602,13 @@ sdhi0: mmc@15c00000  {
 			resets = <&cpg 0xa7>;
 			power-domains = <&cpg>;
 			status = "disabled";
+
+			sdhi0_vqmmc: vqmmc-regulator {
+				regulator-name = "SDHI0-VQMMC";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+				status = "disabled";
+			};
 		};
 
 		sdhi1: mmc@15c10000 {
@@ -615,6 +622,13 @@ sdhi1: mmc@15c10000 {
 			resets = <&cpg 0xa8>;
 			power-domains = <&cpg>;
 			status = "disabled";
+
+			sdhi1_vqmmc: vqmmc-regulator {
+				regulator-name = "SDHI1-VQMMC";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+				status = "disabled";
+			};
 		};
 
 		sdhi2: mmc@15c20000 {
@@ -628,6 +642,13 @@ sdhi2: mmc@15c20000 {
 			resets = <&cpg 0xa9>;
 			power-domains = <&cpg>;
 			status = "disabled";
+
+			sdhi2_vqmmc: vqmmc-regulator {
+				regulator-name = "SDHI2-VQMMC";
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+				status = "disabled";
+			};
 		};
 	};
 
-- 
2.43.0

Re: [PATCH v3 5/8] arm64: dts: renesas: r9a09g057: Add support for enabling SDHI internal regulator

[Geert Uytterhoeven]

On Thu, 6 Feb 2025 at 14:41, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Add support for enabling SDHI internal regulator, by overriding the
> status on the board DTS, when needed.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  * Updated commit description
>  * Renamed internal regulator labels vqmmc_sdhi{0..2}->sdhi{0..2}_vqmmc.
>  * Dropped renaming the gpio regulator label vqmmc_sdhi1->vqmmc_sdhi1_gpio.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Queueing in renesas-devel is postponed, pending acceptance of the DT
bindings by the MMC maintainer.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 5/8] arm64: dts: renesas: r9a09g057: Add support for enabling SDHI internal regulator

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 297 bytes --]

On Thu, Feb 06, 2025 at 01:40:29PM +0000, Biju Das wrote:
> Add support for enabling SDHI internal regulator, by overriding the
> status on the board DTS, when needed.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 6/8] arm64: dts: renesas: rzg3e-smarc-som: Enable SDHI{0,2}

[Biju Das]

Enable eMMC on SDHI0 and SD on SDHI2 on RZ/G3E SMARC SoM.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v2->v3:
 * Renamed node sd0emmc->sd0-emmc
 * Renamed sd0-emmc-{ctrl,data,rst}->sd0-{ctrl,data,rst}
 * Dropped header file gpio.h.
 * Dropped overriding internal regulator name.
 * Updated regulator phandle.
v1->v2:
 * Added missing header file gpio.h
 * Used fixed regulator for eMMC on SD0 and dropped sd0-iovs pins for
   eMMC.
 * Sorted pinctrl nodes for sd2
 * Enabled internal regulator for SD2.
---
 .../boot/dts/renesas/rzg3e-smarc-som.dtsi     | 100 ++++++++++++++++++
 1 file changed, 100 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/rzg3e-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg3e-smarc-som.dtsi
index f4ba050beb0d..fcbabe2cb003 100644
--- a/arch/arm64/boot/dts/renesas/rzg3e-smarc-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg3e-smarc-som.dtsi
@@ -8,17 +8,86 @@
 / {
 	compatible = "renesas,rzg3e-smarcm", "renesas,r9a09g047e57", "renesas,r9a09g047";
 
+	aliases {
+		mmc0 = &sdhi0;
+		mmc2 = &sdhi2;
+	};
+
 	memory@48000000 {
 		device_type = "memory";
 		/* First 128MB is reserved for secure area. */
 		reg = <0x0 0x48000000 0x0 0xf8000000>;
 	};
+
+	reg_1p8v: regulator-1p8v {
+		compatible = "regulator-fixed";
+		regulator-name = "fixed-1.8V";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <1800000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
+
+	reg_3p3v: regulator-3p3v {
+		compatible = "regulator-fixed";
+		regulator-name = "fixed-3.3V";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		regulator-boot-on;
+		regulator-always-on;
+	};
 };
 
 &audio_extal_clk {
 	clock-frequency = <48000000>;
 };
 
+&pinctrl {
+	sdhi0_emmc_pins: sd0-emmc {
+		sd0-ctrl {
+			pins = "SD0CLK", "SD0CMD";
+			renesas,output-impedance = <3>;
+		};
+
+		sd0-data {
+			pins = "SD0DAT0", "SD0DAT1", "SD0DAT2", "SD0DAT3",
+			       "SD0DAT4", "SD0DAT5", "SD0DAT6", "SD0DAT7";
+			renesas,output-impedance = <3>;
+		};
+
+		sd0-rst {
+			pins = "SD0RSTN";
+			renesas,output-impedance = <3>;
+		};
+	};
+
+	sdhi2_pins: sd2 {
+		sd2-cd {
+			pinmux = <RZG3E_PORT_PINMUX(K, 0, 1)>; /* SD2CD */
+		};
+
+		sd2-ctrl {
+			pinmux = <RZG3E_PORT_PINMUX(H, 0, 1)>, /* SD2CLK */
+				 <RZG3E_PORT_PINMUX(H, 1, 1)>; /* SD2CMD */
+		};
+
+		sd2-data {
+			pinmux = <RZG3E_PORT_PINMUX(H, 2, 1)>, /* SD2DAT0 */
+				 <RZG3E_PORT_PINMUX(H, 3, 1)>, /* SD2DAT1 */
+				 <RZG3E_PORT_PINMUX(H, 4, 1)>, /* SD2DAT2 */
+				 <RZG3E_PORT_PINMUX(H, 5, 1)>; /* SD2DAT3 */
+		};
+
+		sd2-iovs {
+			pinmux = <RZG3E_PORT_PINMUX(K, 1, 1)>; /* SD2IOVS */
+		};
+
+		sd2-pwen {
+			pinmux = <RZG3E_PORT_PINMUX(K, 2, 1)>; /* SD2PWEN */
+		};
+	};
+};
+
 &qextal_clk {
 	clock-frequency = <24000000>;
 };
@@ -27,6 +96,37 @@ &rtxin_clk {
 	clock-frequency = <32768>;
 };
 
+&sdhi0 {
+	pinctrl-0 = <&sdhi0_emmc_pins>;
+	pinctrl-1 = <&sdhi0_emmc_pins>;
+	pinctrl-names = "default", "state_uhs";
+
+	vmmc-supply = <&reg_3p3v>;
+	vqmmc-supply = <&reg_1p8v>;
+	bus-width = <8>;
+	mmc-hs200-1_8v;
+	non-removable;
+	fixed-emmc-driver-type = <1>;
+	status = "okay";
+};
+
+&sdhi2 {
+	pinctrl-0 = <&sdhi2_pins>;
+	pinctrl-1 = <&sdhi2_pins>;
+	pinctrl-names = "default", "state_uhs";
+
+	vmmc-supply = <&reg_3p3v>;
+	vqmmc-supply = <&sdhi2_vqmmc>;
+	bus-width = <4>;
+	sd-uhs-sdr50;
+	sd-uhs-sdr104;
+	status = "okay";
+};
+
+&sdhi2_vqmmc {
+	status = "okay";
+};
+
 &wdt1 {
 	status = "okay";
 };
-- 
2.43.0

Re: [PATCH v3 6/8] arm64: dts: renesas: rzg3e-smarc-som: Enable SDHI{0,2}

[Geert Uytterhoeven]

On Thu, 6 Feb 2025 at 14:41, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Enable eMMC on SDHI0 and SD on SDHI2 on RZ/G3E SMARC SoM.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v2->v3:
>  * Renamed node sd0emmc->sd0-emmc
>  * Renamed sd0-emmc-{ctrl,data,rst}->sd0-{ctrl,data,rst}
>  * Dropped header file gpio.h.
>  * Dropped overriding internal regulator name.
>  * Updated regulator phandle.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Queueing in renesas-devel is postponed, pending acceptance of the DT
bindings by the MMC maintainer.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v3 6/8] arm64: dts: renesas: rzg3e-smarc-som: Enable SDHI{0,2}

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 307 bytes --]

On Thu, Feb 06, 2025 at 01:40:30PM +0000, Biju Das wrote:
> Enable eMMC on SDHI0 and SD on SDHI2 on RZ/G3E SMARC SoM.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

FYI, I won't review these board enablements. I don't have schematics and
Geert already reviewed them. It should be good.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v3 7/8] arm64: dts: renesas: rzg3e-smarc-som: Add support for enable SD on SDHI0

[Biju Das]

Add support for enabling SD on SDHI0 on RZ/G3E SMARC SoM. It is enabled
by setting the macro SW_SD0_DEV_SEL to 1 in board DTS and setting the
switch SYS.1 to ON position on the SoM.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v2->v3:
 * Dropped #if guard in pinctrl node for SDHI0
 * Renamed the label/node sdhi0_pins: sd0->sdhi0_usd_pins: sd0-usd.
 * Dropped overriding regulator name.
 * Updated regulator phandle.
v2:
 * New patch
---
 .../boot/dts/renesas/r9a09g047e57-smarc.dts   |  3 ++
 .../boot/dts/renesas/rzg3e-smarc-som.dtsi     | 54 +++++++++++++++++++
 2 files changed, 57 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts b/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts
index c063d47e2952..152a00aa354b 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts
+++ b/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts
@@ -7,6 +7,9 @@
 
 /dts-v1/;
 
+/* Switch selection settings */
+#define SW_SD0_DEV_SEL		0
+
 #include <dt-bindings/pinctrl/renesas,r9a09g047-pinctrl.h>
 #include "r9a09g047e57.dtsi"
 #include "rzg3e-smarc-som.dtsi"
diff --git a/arch/arm64/boot/dts/renesas/rzg3e-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg3e-smarc-som.dtsi
index fcbabe2cb003..1966f2ce70b8 100644
--- a/arch/arm64/boot/dts/renesas/rzg3e-smarc-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg3e-smarc-som.dtsi
@@ -5,6 +5,15 @@
  * Copyright (C) 2024 Renesas Electronics Corp.
  */
 
+/*
+ * Please set the switch position SYS.1 on the SoM and the corresponding macro
+ * SW_SD0_DEV_SEL on the board DTS:
+ *
+ * SW_SD0_DEV_SEL:
+ *      0 - SD0 is connected to eMMC (default)
+ *      1 - SD0 is connected to uSD0 card
+ */
+
 / {
 	compatible = "renesas,rzg3e-smarcm", "renesas,r9a09g047e57", "renesas,r9a09g047";
 
@@ -43,6 +52,32 @@ &audio_extal_clk {
 };
 
 &pinctrl {
+	sdhi0_usd_pins: sd0-usd {
+		sd0-cd {
+			pinmux = <RZG3E_PORT_PINMUX(5, 0, 8)>;
+		};
+
+		sd0-ctrl {
+			pins = "SD0CLK", "SD0CMD";
+			renesas,output-impedance = <3>;
+		};
+
+		sd0-data {
+			pins = "SD0DAT0", "SD0DAT1", "SD0DAT2", "SD0DAT3";
+			renesas,output-impedance = <3>;
+		};
+
+		sd0-iovs {
+			pins = "SD0IOVS";
+			renesas,output-impedance = <3>;
+		};
+
+		sd0-pwen {
+			pins = "SD0PWEN";
+			renesas,output-impedance = <3>;
+		};
+	};
+
 	sdhi0_emmc_pins: sd0-emmc {
 		sd0-ctrl {
 			pins = "SD0CLK", "SD0CMD";
@@ -96,6 +131,24 @@ &rtxin_clk {
 	clock-frequency = <32768>;
 };
 
+#if (SW_SD0_DEV_SEL)
+&sdhi0 {
+	pinctrl-0 = <&sdhi0_usd_pins>;
+	pinctrl-1 = <&sdhi0_usd_pins>;
+	pinctrl-names = "default", "state_uhs";
+
+	vmmc-supply = <&reg_3p3v>;
+	vqmmc-supply = <&sdhi0_vqmmc>;
+	bus-width = <4>;
+	sd-uhs-sdr50;
+	sd-uhs-sdr104;
+	status = "okay";
+};
+
+&sdhi0_vqmmc {
+	status = "okay";
+};
+#else
 &sdhi0 {
 	pinctrl-0 = <&sdhi0_emmc_pins>;
 	pinctrl-1 = <&sdhi0_emmc_pins>;
@@ -109,6 +162,7 @@ &sdhi0 {
 	fixed-emmc-driver-type = <1>;
 	status = "okay";
 };
+#endif
 
 &sdhi2 {
 	pinctrl-0 = <&sdhi2_pins>;
-- 
2.43.0

Re: [PATCH v3 7/8] arm64: dts: renesas: rzg3e-smarc-som: Add support for enable SD on SDHI0

[Geert Uytterhoeven]

On Thu, 6 Feb 2025 at 14:41, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Add support for enabling SD on SDHI0 on RZ/G3E SMARC SoM. It is enabled
> by setting the macro SW_SD0_DEV_SEL to 1 in board DTS and setting the
> switch SYS.1 to ON position on the SoM.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v2->v3:
>  * Dropped #if guard in pinctrl node for SDHI0
>  * Renamed the label/node sdhi0_pins: sd0->sdhi0_usd_pins: sd0-usd.
>  * Dropped overriding regulator name.
>  * Updated regulator phandle.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Queueing in renesas-devel is postponed, pending acceptance of the DT
bindings by the MMC maintainer.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v3 8/8] arm64: dts: renesas: r9a09g047e57-smarc: Enable SDHI1

[Biju Das]

Enable SDHI1 on the RZ/G3E SMARC EVK platform using gpio regulator for
voltage switching.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v2->v3:
 * Added header file gpio.h.
v1->v2:
 * Replaced the regulator usd_vdd_3p3v->reg_3p3v.
 * Renamed the gpio-hog node sd1-pwr-en->sd1-pwr-en-hog.
 * Sorted sd1 pin ctrl nodes.
---
 .../boot/dts/renesas/r9a09g047e57-smarc.dts   | 46 +++++++++++++++++++
 .../boot/dts/renesas/renesas-smarc2.dtsi      | 18 ++++++++
 2 files changed, 64 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts b/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts
index 152a00aa354b..5d7983812c70 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts
+++ b/arch/arm64/boot/dts/renesas/r9a09g047e57-smarc.dts
@@ -9,7 +9,9 @@
 
 /* Switch selection settings */
 #define SW_SD0_DEV_SEL		0
+#define SW_SDIO_M2E		0
 
+#include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/pinctrl/renesas,r9a09g047-pinctrl.h>
 #include "r9a09g047e57.dtsi"
 #include "rzg3e-smarc-som.dtsi"
@@ -19,6 +21,16 @@ / {
 	model = "Renesas SMARC EVK version 2 based on r9a09g047e57";
 	compatible = "renesas,smarc2-evk", "renesas,rzg3e-smarcm",
 		     "renesas,r9a09g047e57", "renesas,r9a09g047";
+
+	vqmmc_sd1_pvdd: regulator-vqmmc-sd1-pvdd {
+		compatible = "regulator-gpio";
+		regulator-name = "SD1_PVDD";
+		regulator-min-microvolt = <1800000>;
+		regulator-max-microvolt = <3300000>;
+		gpios = <&pinctrl RZG3E_GPIO(1, 5) GPIO_ACTIVE_HIGH>;
+		gpios-states = <0>;
+		states = <3300000 0>, <1800000 1>;
+	};
 };
 
 &pinctrl {
@@ -26,9 +38,43 @@ scif_pins: scif {
 		pins = "SCIF_TXD", "SCIF_RXD";
 		renesas,output-impedance = <1>;
 	};
+
+	sd1-pwr-en-hog {
+		gpio-hog;
+		gpios = <RZG3E_GPIO(1, 6) GPIO_ACTIVE_HIGH>;
+		output-high;
+		line-name = "sd1_pwr_en";
+	};
+
+	sdhi1_pins: sd1 {
+		sd1-cd {
+			pinmux = <RZG3E_PORT_PINMUX(1, 4, 8)>; /* SD1CD */
+		};
+
+		sd1-ctrl {
+			pinmux = <RZG3E_PORT_PINMUX(G, 0, 1)>, /* SD1CLK */
+				 <RZG3E_PORT_PINMUX(G, 1, 1)>; /* SD1CMD */
+		};
+
+		sd1-data {
+			pinmux = <RZG3E_PORT_PINMUX(G, 2, 1)>, /* SD1DAT0 */
+				 <RZG3E_PORT_PINMUX(G, 3, 1)>, /* SD1DAT1 */
+				 <RZG3E_PORT_PINMUX(G, 4, 1)>, /* SD1DAT2 */
+				 <RZG3E_PORT_PINMUX(G, 5, 1)>; /* SD1DAT3 */
+		};
+	};
 };
 
 &scif0 {
 	pinctrl-0 = <&scif_pins>;
 	pinctrl-names = "default";
 };
+
+&sdhi1 {
+	pinctrl-0 = <&sdhi1_pins>;
+	pinctrl-1 = <&sdhi1_pins>;
+	pinctrl-names = "default", "state_uhs";
+
+	vmmc-supply = <&reg_3p3v>;
+	vqmmc-supply = <&vqmmc_sd1_pvdd>;
+};
diff --git a/arch/arm64/boot/dts/renesas/renesas-smarc2.dtsi b/arch/arm64/boot/dts/renesas/renesas-smarc2.dtsi
index e378d55e6e9b..fd82df8adc1e 100644
--- a/arch/arm64/boot/dts/renesas/renesas-smarc2.dtsi
+++ b/arch/arm64/boot/dts/renesas/renesas-smarc2.dtsi
@@ -5,6 +5,15 @@
  * Copyright (C) 2024 Renesas Electronics Corp.
  */
 
+/*
+ * Please set the switch position SW_OPT_MUX.1 on the carrier board and the
+ * corresponding macro SW_SDIO_M2E on the board DTS:
+ *
+ * SW_SDIO_M2E:
+ *     0 - SMARC SDIO signal is connected to uSD1
+ *     1 - SMARC SDIO signal is connected to M.2 Key E connector
+ */
+
 / {
 	model = "Renesas RZ SMARC Carrier-II Board";
 	compatible = "renesas,smarc2-evk";
@@ -16,9 +25,18 @@ chosen {
 
 	aliases {
 		serial3 = &scif0;
+		mmc1 = &sdhi1;
 	};
 };
 
 &scif0 {
 	status = "okay";
 };
+
+&sdhi1 {
+	bus-width = <4>;
+	sd-uhs-sdr50;
+	sd-uhs-sdr104;
+
+	status = "okay";
+};
-- 
2.43.0

Re: [PATCH v3 8/8] arm64: dts: renesas: r9a09g047e57-smarc: Enable SDHI1

[Geert Uytterhoeven]

On Thu, 6 Feb 2025 at 14:41, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Enable SDHI1 on the RZ/G3E SMARC EVK platform using gpio regulator for
> voltage switching.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v2->v3:
>  * Added header file gpio.h.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Queueing in renesas-devel is postponed, pending acceptance of the DT
bindings by the MMC maintainer.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH v3 0/8] Add RZ/G3E SDHI support

[Biju Das]

Hi Ulf, Wolfram,

> -----Original Message-----
> From: Biju Das <biju.das.jz@bp.renesas.com>
> Sent: 06 February 2025 13:40
> Subject: [PATCH v3 0/8] Add RZ/G3E SDHI support
> 
> The SD/MMC block on the RZ/G3E ("R9A09G047") SoC is similar to that of the RZ/V2H, but the SD0 channel
> has only dedicated pins, so we must use SD_STATUS register to control voltage and power enable
> (internal regulator).
> 
> For SD1 and SD2 channel we can either use gpio regulator or internal regulator (using SD_STATUS
> register) for voltage switching.
> 
> For SD0, fixed voltage(eMMC) uses fixed regulator and non-fixed voltage
> (SD) uses internal regulator.
> 
> v2->v3:
>  * Collected tags
>  * Renamed internal regulator labels vqmmc_sdhi{0..2}->sdhi{0..2}_vqmmc.
>  * Updated regulator phandles on SoM/Board dts.
>  * Dropped renaming the gpio regulator label vqmmc_sdhi1->vqmmc_sdhi1_gpio.
>  * Renamed node sd0emmc->sd0-emmc
>  * Renamed sd0-emmc-{ctrl,data,rst}->sd0-{ctrl,data,rst}
>  * Moved header file gpio.h from patch#6 to patch#8.
>  * Dropped overriding internal regulator name.
>  * Dropped #if guard in pinctrl node for SDHI0
>  * Renamed the label/node sdhi0_pins: sd0->sdhi0_usd_pins: sd0-usd.
> v1->v2:
>  * Collected tags.
>  * Documented internal regulator as optional property for both RZ/G3E and
>    RZ/V2H SoCs.
>  * Updated commit description for regulator used in SD0 fixed and
>    non-fixed voltage case in patch#3.
>  * As the node enabling of internal regulator is controlled through status,
>    added a check for device availability.
>  * Status of internal regulator is disabled in the SoC .dtsi. Override
>    the status in the board DTS when needed.
>  * Added support for enabling SDHI internal regulator in RZ/V2H
>  * Added missing header file gpio.h
>  * Used fixed regulator for eMMC on SD0 and dropped sd0-iovs pins for
>    eMMC.
>  * Sorted pinctrl nodes for sd2
>  * Enabled internal regulator for SD2.
>  * Added support for enabling SD on SDHI0
>  * Replaced the regulator usd_vdd_3p3v->reg_3p3v.
>  * Renamed the gpio-hog node sd1-pwr-en->sd1-pwr-en-hog.
>  * Sorted sd1 pin ctrl nodes.
> 
> Biju Das (8):
>   dt-bindings: mmc: renesas,sdhi: Document RZ/G3E support
>   mmc: renesas_sdhi: Arrange local variables in reverse xmas tree order
>   mmc: renesas_sdhi: Add support for RZ/G3E SoC
>   arm64: dts: renesas: r9a09g047: Add SDHI0-SDHI2 nodes
>   arm64: dts: renesas: r9a09g057: Add support for enabling SDHI internal
>     regulator
>   arm64: dts: renesas: rzg3e-smarc-som: Enable SDHI{0,2}
>   arm64: dts: renesas: rzg3e-smarc-som: Add support for enable SD on
>     SDHI0
>   arm64: dts: renesas: r9a09g047e57-smarc: Enable SDHI1
> 
>  .../devicetree/bindings/mmc/renesas,sdhi.yaml |  16 ++
>  arch/arm64/boot/dts/renesas/r9a09g047.dtsi    |  60 +++++++
>  .../boot/dts/renesas/r9a09g047e57-smarc.dts   |  49 ++++++
>  arch/arm64/boot/dts/renesas/r9a09g057.dtsi    |  21 +++
>  .../boot/dts/renesas/renesas-smarc2.dtsi      |  18 ++
>  .../boot/dts/renesas/rzg3e-smarc-som.dtsi     | 154 ++++++++++++++++++
>  drivers/mmc/host/renesas_sdhi.h               |   1 +
>  drivers/mmc/host/renesas_sdhi_core.c          | 136 +++++++++++++++-
>  drivers/mmc/host/tmio_mmc.h                   |   5 +
>  9 files changed, 459 insertions(+), 1 deletion(-)

Gentle ping.

Cheers,
Biju