[PATCH 0/7] Add RZ/G3E SDHI support

[PATCH 0/7] 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.

Biju Das (7):
  dt-bindings: mmc: renesas,sdhi: Document RZ/G3E support
  clk: renesas: r9a09g047: Add SDHI clocks/resets
  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: rzg3e-smarc-som: Enable SDHI{0,2}
  arm64: dts: renesas: r9a09g047e57-smarc: Enable SDHI1

 .../devicetree/bindings/mmc/renesas,sdhi.yaml |  20 +++
 arch/arm64/boot/dts/renesas/r9a09g047.dtsi    |  57 ++++++++
 .../boot/dts/renesas/r9a09g047e57-smarc.dts   |  65 +++++++++
 .../boot/dts/renesas/renesas-smarc2.dtsi      |   9 ++
 .../boot/dts/renesas/rzg3e-smarc-som.dtsi     |  89 ++++++++++++
 drivers/clk/renesas/r9a09g047-cpg.c           |  31 +++++
 drivers/mmc/host/renesas_sdhi.h               |   1 +
 drivers/mmc/host/renesas_sdhi_core.c          | 131 +++++++++++++++++-
 drivers/mmc/host/tmio_mmc.h                   |   5 +
 9 files changed, 407 insertions(+), 1 deletion(-)

-- 
2.43.0

[PATCH 1/7] 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 SD1 and SD2 channel we can either use gpio regulator or internal
regulator (using SD_STATUS register) for voltage switching.

Document RZ/G3E SDHI IP support.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 .../devicetree/bindings/mmc/renesas,sdhi.yaml | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
index af378b9ff3f4..ef3acf0f58e0 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
@@ -124,6 +127,7 @@ allOf:
         compatible:
           contains:
             enum:
+              - renesas,sdhi-r9a09g047
               - renesas,sdhi-r9a09g057
               - renesas,rzg2l-sdhi
     then:
@@ -211,6 +215,22 @@ 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-r9a09g047
+    then:
+      properties:
+        vqmmc-regulator:
+          type: object
+          description: VQMMC SD regulator
+          $ref: /schemas/regulator/regulator.yaml#
+          unevaluatedProperties: false
+
+      required:
+        - vqmmc-regulator
+
 required:
   - compatible
   - reg
-- 
2.43.0

[PATCH 3/7] mmc: renesas_sdhi: Arrange local variables in reverse xmas tree order

[Biju Das]

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

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 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

[PATCH 4/7] 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.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/mmc/host/renesas_sdhi.h      |   1 +
 drivers/mmc/host/renesas_sdhi_core.c | 129 +++++++++++++++++++++++++++
 drivers/mmc/host/tmio_mmc.h          |   5 ++
 3 files changed, 135 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..bbfa68ca682b 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,23 @@ 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 (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 1/7] dt-bindings: mmc: renesas,sdhi: Document RZ/G3E support

[Tommaso Merciai]

Hi Biju,

Thanks for the patch.

On Sun, Jan 26, 2025 at 01:46:03PM +0000, Biju Das wrote:
> 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.
> 
> Document RZ/G3E SDHI IP support.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  .../devicetree/bindings/mmc/renesas,sdhi.yaml | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> index af378b9ff3f4..ef3acf0f58e0 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
> @@ -124,6 +127,7 @@ allOf:
>          compatible:
>            contains:
>              enum:
> +              - renesas,sdhi-r9a09g047
>                - renesas,sdhi-r9a09g057
>                - renesas,rzg2l-sdhi
>      then:
> @@ -211,6 +215,22 @@ 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-r9a09g047
> +    then:
> +      properties:
> +        vqmmc-regulator:
> +          type: object
> +          description: VQMMC SD regulator
> +          $ref: /schemas/regulator/regulator.yaml#
> +          unevaluatedProperties: false
> +
> +      required:
> +        - vqmmc-regulator
> +
>  required:
>    - compatible
>    - reg
> -- 
> 2.43.0
> 

Reviewed-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>

Re: [PATCH 3/7] mmc: renesas_sdhi: Arrange local variables in reverse xmas tree order

[Tommaso Merciai]

Hi Biju,

Thanks for the patch.

On Sun, Jan 26, 2025 at 01:46:05PM +0000, Biju Das wrote:
> Arrange local variables in reverse xmas tree for probe().
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  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
> 

Reviewed-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>

Thanks & Regards,
Tommaso

Re: [PATCH 4/7] mmc: renesas_sdhi: Add support for RZ/G3E SoC

[Tommaso Merciai]

On Sun, Jan 26, 2025 at 01:46:06PM +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.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  drivers/mmc/host/renesas_sdhi.h      |   1 +
>  drivers/mmc/host/renesas_sdhi_core.c | 129 +++++++++++++++++++++++++++
>  drivers/mmc/host/tmio_mmc.h          |   5 ++
>  3 files changed, 135 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..bbfa68ca682b 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,23 @@ 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 (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
>

Reviewed-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>

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

[Rob Herring]

On Sun, Jan 26, 2025 at 01:46:03PM +0000, Biju Das wrote:
> 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.
> 
> Document RZ/G3E SDHI IP support.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  .../devicetree/bindings/mmc/renesas,sdhi.yaml | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> index af378b9ff3f4..ef3acf0f58e0 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
> @@ -124,6 +127,7 @@ allOf:
>          compatible:
>            contains:
>              enum:
> +              - renesas,sdhi-r9a09g047
>                - renesas,sdhi-r9a09g057
>                - renesas,rzg2l-sdhi
>      then:
> @@ -211,6 +215,22 @@ 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-r9a09g047
> +    then:
> +      properties:
> +        vqmmc-regulator:
> +          type: object
> +          description: VQMMC SD regulator
> +          $ref: /schemas/regulator/regulator.yaml#
> +          unevaluatedProperties: false
> +
> +      required:
> +        - vqmmc-regulator

The driver treats this as optional. If this is required, then is 
renesas,sdhi-r9a09g047 really compatible with renesas,sdhi-r9a09g057?

Rob

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

[Biju Das]

Hi Rob,

Thanks for the feedback.

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: 27 January 2025 19:16
> Subject: Re: [PATCH 1/7] dt-bindings: mmc: renesas,sdhi: Document RZ/G3E support
> 
> On Sun, Jan 26, 2025 at 01:46:03PM +0000, Biju Das wrote:
> > 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.
> >
> > Document RZ/G3E SDHI IP support.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >  .../devicetree/bindings/mmc/renesas,sdhi.yaml | 20
> > +++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > index af378b9ff3f4..ef3acf0f58e0 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
> > @@ -124,6 +127,7 @@ allOf:
> >          compatible:
> >            contains:
> >              enum:
> > +              - renesas,sdhi-r9a09g047
> >                - renesas,sdhi-r9a09g057
> >                - renesas,rzg2l-sdhi
> >      then:
> > @@ -211,6 +215,22 @@ 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-r9a09g047
> > +    then:
> > +      properties:
> > +        vqmmc-regulator:
> > +          type: object
> > +          description: VQMMC SD regulator
> > +          $ref: /schemas/regulator/regulator.yaml#
> > +          unevaluatedProperties: false
> > +
> > +      required:
> > +        - vqmmc-regulator
> 
> The driver treats this as optional. If this is required, then is
> renesas,sdhi-r9a09g047 really compatible with renesas,sdhi-r9a09g057?

Ok, I will make it optional for both renesas,sdhi-r9a09g057 and renesas,sdhi-r9a09g047
in the next version.

Cheers,
Biju

Re: [PATCH 3/7] mmc: renesas_sdhi: Arrange local variables in reverse xmas tree order

[Geert Uytterhoeven]

On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> Arrange local variables in reverse xmas tree for probe().
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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 1/7] dt-bindings: mmc: renesas,sdhi: Document RZ/G3E support

[Geert Uytterhoeven]

Hi Biju,

On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> 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.
> Document RZ/G3E SDHI IP support.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- 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)

OK... but...

>
>    reg:
>      maxItems: 1
> @@ -124,6 +127,7 @@ allOf:
>          compatible:
>            contains:
>              enum:
> +              - renesas,sdhi-r9a09g047
>                - renesas,sdhi-r9a09g057
>                - renesas,rzg2l-sdhi
>      then:
> @@ -211,6 +215,22 @@ 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-r9a09g047
> +    then:
> +      properties:
> +        vqmmc-regulator:
> +          type: object
> +          description: VQMMC SD regulator
> +          $ref: /schemas/regulator/regulator.yaml#
> +          unevaluatedProperties: false
> +
> +      required:
> +        - vqmmc-regulator
> +
>  required:
>    - compatible
>    - reg

Given RZ/V2H can use the internal regulator control, too, I think it
can be optional on both.  Then renesas,sdhi-r9a09g047 can just use
renesas,sdhi-r9a09g057 as a fallback compatible.

Note for the casual reader: as the related pins can be used as GPIOs
on all RZ/V2H SD channels, the initial idea to add support for the
internal regulator control was dropped, and replaced by the simpler
solution of using a gpio-regulator.  Unfortunately that simple option
is not available for SD0 on RZ/G3E.

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 1/7] dt-bindings: mmc: renesas,sdhi: Document RZ/G3E support

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 28 January 2025 11:16
> Subject: Re: [PATCH 1/7] dt-bindings: mmc: renesas,sdhi: Document RZ/G3E support
> 
> Hi Biju,
> 
> On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > 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.
> > Document RZ/G3E SDHI IP support.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- 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)
> 
> OK... but...
> 
> >
> >    reg:
> >      maxItems: 1
> > @@ -124,6 +127,7 @@ allOf:
> >          compatible:
> >            contains:
> >              enum:
> > +              - renesas,sdhi-r9a09g047
> >                - renesas,sdhi-r9a09g057
> >                - renesas,rzg2l-sdhi
> >      then:
> > @@ -211,6 +215,22 @@ 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-r9a09g047
> > +    then:
> > +      properties:
> > +        vqmmc-regulator:
> > +          type: object
> > +          description: VQMMC SD regulator
> > +          $ref: /schemas/regulator/regulator.yaml#
> > +          unevaluatedProperties: false
> > +
> > +      required:
> > +        - vqmmc-regulator
> > +
> >  required:
> >    - compatible
> >    - reg
> 
> Given RZ/V2H can use the internal regulator control, too, I think it can be optional on both.  Then
> renesas,sdhi-r9a09g047 can just use
> renesas,sdhi-r9a09g057 as a fallback compatible.

Agreed, will make internal regulator control. optional for both RZ/G2H and RZ/G3E
and just use renesas,sdhi-r9a09g057 as a fallback compatible for renesas,sdhi-r9a09g047.

Cheers,
Biju

Re: [PATCH 4/7] mmc: renesas_sdhi: Add support for RZ/G3E SoC

[Geert Uytterhoeven]

Hi Biju,

On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> 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.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

Thanks for your patch!

> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c

> @@ -1053,6 +1165,23 @@ 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 this node becomes required on RZ/V2H and RZ/G3E, and controlled
through status, you also need:

    if (!of_device_is_available(rcfg.of_node)) {
            of_node_put(rcfg.of_node);
            rcfg.of_node = NULL;
    }

Or introduce of_get_available_child_by_name()...

> +       if (rcfg.of_node) {
> +               rcfg.driver_data = priv->host;
> +
> +               renesas_sdhi_vqmmc_regulator.name = "sdhi-vqmmc-regulator";

Name conflict in case of multiple instances?

> +               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)

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 4/7] mmc: renesas_sdhi: Add support for RZ/G3E SoC

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: 28 January 2025 13:32
> Subject: Re: [PATCH 4/7] mmc: renesas_sdhi: Add support for RZ/G3E SoC
> 
> Hi Biju,
> 
> On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> 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.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/drivers/mmc/host/renesas_sdhi_core.c
> > +++ b/drivers/mmc/host/renesas_sdhi_core.c
> 
> > @@ -1053,6 +1165,23 @@ 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 this node becomes required on RZ/V2H and RZ/G3E, and controlled through status, you also need:
> 
>     if (!of_device_is_available(rcfg.of_node)) {
>             of_node_put(rcfg.of_node);
>             rcfg.of_node = NULL;
>     }

OK.

> 
> Or introduce of_get_available_child_by_name()...

OK, will send a separate patch for optimizing the above 2 calls.

> 
> > +       if (rcfg.of_node) {
> > +               rcfg.driver_data = priv->host;
> > +
> > +               renesas_sdhi_vqmmc_regulator.name =
> > + "sdhi-vqmmc-regulator";
> 
> Name conflict in case of multiple instances?

This regulator name is Overriden by of_regulator() and it will pick the name from there.
See below. Am I missing anything?

root@smarc-rzg3e:/cip-test-scripts# cat /sys/class/regulator/regulator.*/name
regulator-dummy
fixed-3.3V
fixed-3.3V
SDHI1 VccQ
SDHI0-VQMMC
SDHI2-VQMMC
SDHI1-VQMMC

Cheers,
Biju

RE: [PATCH 4/7] mmc: renesas_sdhi: Add support for RZ/G3E SoC

[Biju Das]

Hi Geert,

 
> > -----Original Message-----
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > Sent: 28 January 2025 13:32
> > Subject: Re: [PATCH 4/7] mmc: renesas_sdhi: Add support for RZ/G3E SoC
> >
> > Hi Biju,
> >
> > On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> 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.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> >
> > Thanks for your patch!
> >
> > > --- a/drivers/mmc/host/renesas_sdhi_core.c
> > > +++ b/drivers/mmc/host/renesas_sdhi_core.c
> >
> > > @@ -1053,6 +1165,23 @@ int renesas_sdhi_probe(struct platform_device *pdev,
> > >         if (ret)
> > >                 goto efree;
> > >
> >
> > > +       if (rcfg.of_node) {
> > > +               rcfg.driver_data = priv->host;
> > > +
> > > +               renesas_sdhi_vqmmc_regulator.name =
> > > + "sdhi-vqmmc-regulator";
> >
> > Name conflict in case of multiple instances?
> 
> This regulator name is Overriden by of_regulator() and it will pick the name from there.
> See below. Am I missing anything?

Just to add, without the name it returns error see [1],

Regulator name from DT node is picked by [2] and [3]. So there won't be conflict for
Multiple instances.

[1] https://elixir.bootlin.com/linux/v6.13-rc7/source/drivers/regulator/core.c#L5597

[2] https://elixir.bootlin.com/linux/v6.13-rc7/source/drivers/regulator/core.c#L5659
[3] https://elixir.bootlin.com/linux/v6.13-rc7/source/drivers/regulator/of_regulator.c#L97

Cheers,
Biju

> 
> root@smarc-rzg3e:/cip-test-scripts# cat /sys/class/regulator/regulator.*/name
> regulator-dummy
> fixed-3.3V
> fixed-3.3V
> SDHI1 VccQ
> SDHI0-VQMMC
> SDHI2-VQMMC
> SDHI1-VQMMC
> 
> Cheers,
> Biju

Re: [PATCH 4/7] mmc: renesas_sdhi: Add support for RZ/G3E SoC

[Geert Uytterhoeven]

Hi Biju,

On Wed, 29 Jan 2025 at 15:45, Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > -----Original Message-----
> > > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > > Sent: 28 January 2025 13:32
> > > Subject: Re: [PATCH 4/7] mmc: renesas_sdhi: Add support for RZ/G3E SoC
> > >
> > > On Sun, 26 Jan 2025 at 14:46, Biju Das <biju.das.jz@bp.renesas.com> 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.
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > >
> > > > --- a/drivers/mmc/host/renesas_sdhi_core.c
> > > > +++ b/drivers/mmc/host/renesas_sdhi_core.c
> > >
> > > > @@ -1053,6 +1165,23 @@ int renesas_sdhi_probe(struct platform_device *pdev,
> > > >         if (ret)
> > > >                 goto efree;
> > > >
> > >
> > > > +       if (rcfg.of_node) {
> > > > +               rcfg.driver_data = priv->host;
> > > > +
> > > > +               renesas_sdhi_vqmmc_regulator.name =
> > > > + "sdhi-vqmmc-regulator";
> > >
> > > Name conflict in case of multiple instances?
> >
> > This regulator name is Overriden by of_regulator() and it will pick the name from there.
> > See below. Am I missing anything?
>
> Just to add, without the name it returns error see [1],

Thanks, I had arrived at the same conclusion.

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