[PATCH v4 0/3] Add SD/MMC support for Renesas RZ/V2H(P) SoC

[PATCH v4 0/3] Add SD/MMC support for Renesas RZ/V2H(P) SoC

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Hi All,

This patch series aims to add SD/MMC support for Renesas RZ/V2H(P) SoC.

Below is the sample usage of using internal regulator:

SoC DTSI node:
sdhi1: mmc@15c10000 {
        compatible = "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 167>,
                        <&cpg CPG_MOD 169>,
                        <&cpg CPG_MOD 168>,
                        <&cpg CPG_MOD 170>;
        clock-names = "core", "clkh", "cd", "aclk";
        resets = <&cpg 168>;
        power-domains = <&cpg>;
        status = "disabled";

        vqmmc_sdhi1: vqmmc-regulator {
                regulator-compatible = "vqmmc-r9a09g057-regulator";
                regulator-name = "sdhi1-vqmmc-regulator";
                regulator-min-microvolt = <1800000>;
                regulator-max-microvolt = <3300000>;
                status = "disabled";
        };
};

* Example of SDHI1 while using internal regulator:
** Board DTS:
&sdhi1 {
        pinctrl-0 = <&sdhi1_pins>;
        pinctrl-1 = <&sdhi1_pins>;
        pinctrl-names = "default", "state_uhs";
        vmmc-supply = <&reg_3p3v>;
        vqmmc-supply = <&vqmmc_sdhi1>;
        bus-width = <4>;
        sd-uhs-sdr50;
        sd-uhs-sdr104;
        status = "okay";
};

&vqmmc_sdhi1 {
	status = "okay";
};


* Example of SDHI1 while using GPIO regulator while internal regulator is present:
** Board DTS:
vccq_sdhi1: regulator-vccq-sdhi1 {
        compatible = "regulator-gpio";
        regulator-name = "SDHI1 VccQ";
        regulator-min-microvolt = <1800000>;
        regulator-max-microvolt = <3300000>;
        gpios = <&pinctrl RZG2L_GPIO(10, 2) GPIO_ACTIVE_HIGH>;
        gpios-states = <0>;
        states = <3300000 0>, <1800000 1>;
};

&sdhi1 {
        pinctrl-0 = <&sdhi1_pins>;
        pinctrl-1 = <&sdhi1_pins>;
        pinctrl-names = "default", "state_uhs";
        vmmc-supply = <&reg_3p3v>;
        vqmmc-supply = <&vccq_sdhi1>;
        bus-width = <4>;
        sd-uhs-sdr50;
        sd-uhs-sdr104;
        status = "okay";
};

v3->V4
- Dropped 'renesas,sdhi-use-internal-regulator' property
- Defined vqmmc-regulator in top level
- For special handling of internal regulator now using of_device_is_available()
- Fixed comments from Claudiu
- Rebased patch on top of https://patchwork.kernel.org/project/linux-renesas-soc/patch/20240626085015.32171-2-wsa+renesas@sang-engineering.com/

v2->v3
- Renamed vqmmc-r9a09g057-regulator object to vqmmc-regulator
- Added regulator-compatible property for vqmmc-regulator
- Added 'renesas,sdhi-use-internal-regulator' DT property
- Included RB tags for patch 2/3
- Moved regulator info to renesas_sdhi_of_data instead of quirks
- Added support to configure the init state of regulator
- Added function pointers to configure regulator
- Added REGULATOR_CHANGE_VOLTAGE mask

v1->v2
- Dropped regulator core API changes
- Updated DT binding
- Now controlling PWEN bit via regultor api

v1: https://patchwork.kernel.org/project/linux-renesas-soc/cover/20240605074936.578687-1-prabhakar.mahadev-lad.rj@bp.renesas.com/

Cheers,
Prabhakar

Lad Prabhakar (3):
  dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P) support
  mmc: tmio: Use MMC core APIs to control the vqmmc regulator
  mmc: renesas_sdhi: Add support for RZ/V2H(P) SoC

 .../devicetree/bindings/mmc/renesas,sdhi.yaml |  26 +++-
 drivers/mmc/host/renesas_sdhi.h               |  13 ++
 drivers/mmc/host/renesas_sdhi_core.c          |  98 ++++++++++++
 drivers/mmc/host/renesas_sdhi_internal_dmac.c | 147 ++++++++++++++++++
 drivers/mmc/host/tmio_mmc.h                   |   5 +
 drivers/mmc/host/tmio_mmc_core.c              |   7 +-
 6 files changed, 291 insertions(+), 5 deletions(-)

-- 
2.34.1

[PATCH v4 1/3] dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P) support

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

The SD/MMC block on the RZ/V2H(P) ("R9A09G057") SoC is similar to that
of the R-Car Gen3, but it has some differences:
- HS400 is not supported.
- It supports the SD_IOVS bit to control the IO voltage level.
- It supports fixed address mode.

To accommodate these differences, a SoC-specific 'renesas,sdhi-r9a09g057'
compatible string is added.

A 'vqmmc-regulator' object is introduced to handle the power enable (PWEN)
and voltage level switching for the SD/MMC.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v3->v4
- Dropped 'renesas,sdhi-use-internal-regulator' property
- Moved 'vqmmc-regulator' to the top level

v2->v3
- Renamed vqmmc-r9a09g057-regulator object to vqmmc-regulator
- Added regulator-compatible property for vqmmc-regulator
- Added 'renesas,sdhi-use-internal-regulator' property

v1->v2
- Moved vqmmc object in the if block
- Updated commit message
---
 .../devicetree/bindings/mmc/renesas,sdhi.yaml | 26 ++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
index 3d0e61e59856..d632b67080bd 100644
--- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
+++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
@@ -18,6 +18,7 @@ properties:
           - renesas,sdhi-r7s9210 # SH-Mobile AG5
           - renesas,sdhi-r8a73a4 # R-Mobile APE6
           - renesas,sdhi-r8a7740 # R-Mobile A1
+          - renesas,sdhi-r9a09g057 # RZ/V2H(P)
           - renesas,sdhi-sh73a0  # R-Mobile APE6
       - items:
           - enum:
@@ -111,6 +112,15 @@ properties:
 
   max-frequency: true
 
+  vqmmc-regulator:
+    type: object
+    description: VQMMC SD regulator
+    $ref: /schemas/regulator/regulator.yaml#
+    unevaluatedProperties: false
+    properties:
+      regulator-compatible:
+        pattern: "^vqmmc-r9a09g057-regulator"
+
 allOf:
   - $ref: mmc-controller.yaml
 
@@ -118,7 +128,9 @@ allOf:
       properties:
         compatible:
           contains:
-            const: renesas,rzg2l-sdhi
+            enum:
+              - renesas,sdhi-r9a09g057
+              - renesas,rzg2l-sdhi
     then:
       properties:
         clocks:
@@ -204,6 +216,18 @@ 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:
+      required:
+        - vqmmc-regulator
+    else:
+      properties:
+        vqmmc-regulator: false
+
 required:
   - compatible
   - reg
-- 
2.34.1

[PATCH v4 2/3] mmc: tmio: Use MMC core APIs to control the vqmmc regulator

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Use the mmc_regulator_enable_vqmmc() and mmc_regulator_disable_vqmmc() APIs
to enable/disable the vqmmc regulator.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> # on RZ/G3S
---
v3->v4
- Collected tested by tag from Claudiu

v2->v3
- Included RB tags

v1->v2
- New patch
---
 drivers/mmc/host/tmio_mmc_core.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 51bd2365795b..9a9e283493b0 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -897,8 +897,8 @@ static void tmio_mmc_power_on(struct tmio_mmc_host *host, unsigned short vdd)
 	 * It seems, VccQ should be switched on after Vcc, this is also what the
 	 * omap_hsmmc.c driver does.
 	 */
-	if (!IS_ERR(mmc->supply.vqmmc) && !ret) {
-		ret = regulator_enable(mmc->supply.vqmmc);
+	if (!ret) {
+		ret = mmc_regulator_enable_vqmmc(mmc);
 		usleep_range(200, 300);
 	}
 
@@ -911,8 +911,7 @@ static void tmio_mmc_power_off(struct tmio_mmc_host *host)
 {
 	struct mmc_host *mmc = host->mmc;
 
-	if (!IS_ERR(mmc->supply.vqmmc))
-		regulator_disable(mmc->supply.vqmmc);
+	mmc_regulator_disable_vqmmc(mmc);
 
 	if (!IS_ERR(mmc->supply.vmmc))
 		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
-- 
2.34.1

[PATCH v4 3/3] mmc: renesas_sdhi: Add support for RZ/V2H(P) SoC

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

The SDHI/eMMC IPs found in the RZ/V2H(P) (a.k.a. r9a09g057) are very
similar to those found in R-Car Gen3. However, they are not identical,
necessitating an SoC-specific compatible string for fine-tuning driver
support.

Key features of the RZ/V2H(P) SDHI/eMMC IPs include:
- Voltage level control via the IOVS bit.
- PWEN pin support via SD_STATUS register.
- Lack of HS400 support.
- Fixed address mode operation.

internal regulator support is added to control the voltage levels of SD
pins via sd_iovs/sd_pwen bits in SD_STATUS register.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> # on RZ/G3S
---
v3->v4
- Dropped using 'renesas,sdhi-use-internal-regulator' property
- Now using of_device_is_available() to check if regulator is available and enabled
- Dropped extra spaces during operations 
- Included tested by tag from Claudiu
- Rebased patch on top of https://patchwork.kernel.org/project/linux-renesas-soc/patch/20240626085015.32171-2-wsa+renesas@sang-engineering.com/

v2->v3
- Moved regulator info to renesas_sdhi_of_data instead of quirks
- Added support to configure the init state of regulator
- Added function pointers to configure regulator
- Added REGULATOR_CHANGE_VOLTAGE mask

v1->v2
- Now controlling PWEN bit get/set_voltage
---
 drivers/mmc/host/renesas_sdhi.h               |  13 ++
 drivers/mmc/host/renesas_sdhi_core.c          |  98 ++++++++++++
 drivers/mmc/host/renesas_sdhi_internal_dmac.c | 147 ++++++++++++++++++
 drivers/mmc/host/tmio_mmc.h                   |   5 +
 4 files changed, 263 insertions(+)

diff --git a/drivers/mmc/host/renesas_sdhi.h b/drivers/mmc/host/renesas_sdhi.h
index f12a87442338..cd509e7142ba 100644
--- a/drivers/mmc/host/renesas_sdhi.h
+++ b/drivers/mmc/host/renesas_sdhi.h
@@ -11,6 +11,8 @@
 
 #include <linux/dmaengine.h>
 #include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
 #include <linux/workqueue.h>
 #include "tmio_mmc.h"
 
@@ -36,6 +38,12 @@ struct renesas_sdhi_of_data {
 	unsigned int max_blk_count;
 	unsigned short max_segs;
 	unsigned long sdhi_flags;
+	struct regulator_desc *rdesc;
+	struct regulator_init_data *reg_init_data;
+	bool regulator_init_state;
+	unsigned int regulator_init_voltage;
+	int (*regulator_force_endis)(struct regulator_dev *rdev, bool enable);
+	int (*regulator_force_voltage)(struct regulator_dev *rdev, unsigned int voltage);
 };
 
 #define SDHI_CALIB_TABLE_MAX 32
@@ -95,6 +103,11 @@ struct renesas_sdhi {
 
 	struct reset_control *rstc;
 	struct tmio_mmc_host *host;
+
+	bool use_internal_regulator;
+	struct regulator_dev *internal_regulator;
+	int (*regulator_force_endis)(struct regulator_dev *rdev, bool enable);
+	int (*regulator_force_voltage)(struct regulator_dev *rdev, unsigned int voltage);
 };
 
 #define host_to_priv(host) \
diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 0fc159b52fa9..6f8e745477b5 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -581,12 +581,50 @@ static void renesas_sdhi_reset(struct tmio_mmc_host *host, bool preserve)
 
 	if (!preserve) {
 		if (priv->rstc) {
+			bool regulator_enabled;
+
+			/* to restore back the internal regulator after reset make a copy of it */
+			if (priv->use_internal_regulator)
+				regulator_enabled = regulator_is_enabled(host->mmc->supply.vqmmc);
+
 			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->use_internal_regulator) {
+				int voltage;
+
+				/*
+				 * HW reset might have toggled the regulator state in HW
+				 * which regulator core might be unaware of so restore
+				 * back the regulator state in HW bypassing the regulator
+				 * core.
+				 */
+				if (regulator_enabled !=
+				    regulator_is_enabled(host->mmc->supply.vqmmc)) {
+					ret = priv->regulator_force_endis(priv->internal_regulator,
+									  regulator_enabled);
+					if (ret)
+						dev_err(&host->pdev->dev,
+							"Failed to %s internal regulator\n",
+							regulator_enabled ? "enable" : "disable");
+				}
+
+				/* restore back voltage on vqmmc supply */
+				voltage = regulator_get_voltage(host->mmc->supply.vqmmc);
+				if (voltage != host->mmc->ios.signal_voltage) {
+					voltage = host->mmc->ios.signal_voltage ==
+						  MMC_SIGNAL_VOLTAGE_330 ? 3300000 : 1800000;
+					ret = regulator_set_voltage(host->mmc->supply.vqmmc,
+								    voltage, voltage);
+					if (ret)
+						dev_err(&host->pdev->dev,
+							"Failed to set voltage on internal regulator\n");
+				}
+			}
+
 			priv->needs_adjust_hs400 = false;
 			renesas_sdhi_set_clock(host, host->clk_cache);
 
@@ -904,12 +942,36 @@ static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable)
 	renesas_sdhi_sdbuf_width(host, enable ? width : 16);
 }
 
+static int renesas_sdhi_internal_dmac_register_regulator(struct platform_device *pdev,
+							 const struct renesas_sdhi_of_data *of_data)
+{
+	struct tmio_mmc_host *host = platform_get_drvdata(pdev);
+	struct renesas_sdhi *priv = host_to_priv(host);
+	struct regulator_config rcfg = {
+		.dev = &pdev->dev,
+		.driver_data = host,
+		.init_data = of_data->reg_init_data,
+	};
+	const char *devname;
+
+	devname = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-vqmmc-regulator",
+				 dev_name(&pdev->dev));
+	if (!devname)
+		return -ENOMEM;
+
+	of_data->rdesc->name = devname;
+	priv->internal_regulator = devm_regulator_register(&pdev->dev, of_data->rdesc, &rcfg);
+
+	return PTR_ERR_OR_ZERO(priv->internal_regulator);
+}
+
 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 tmio_mmc_data *mmd = pdev->dev.platform_data;
+	struct device_node *internal_regulator;
 	struct tmio_mmc_data *mmc_data;
 	struct renesas_sdhi_dma *dma_priv;
 	struct tmio_mmc_host *host;
@@ -972,6 +1034,14 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 
 	priv->host = host;
 
+	if (pdev->dev.of_node) {
+		internal_regulator = of_get_child_by_name(pdev->dev.of_node, "vqmmc-regulator");
+		if (internal_regulator) {
+			priv->use_internal_regulator = of_device_is_available(internal_regulator);
+			of_node_put(internal_regulator);
+		}
+	}
+
 	if (of_data) {
 		mmc_data->flags |= of_data->tmio_flags;
 		mmc_data->ocr_mask = of_data->tmio_ocr_mask;
@@ -982,6 +1052,18 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 		mmc_data->max_segs = of_data->max_segs;
 		dma_priv->dma_buswidth = of_data->dma_buswidth;
 		host->bus_shift = of_data->bus_shift;
+		if (priv->use_internal_regulator) {
+			if (!of_data->regulator_force_endis)
+				return dev_err_probe(&pdev->dev, -EINVAL,
+					"missing function pointer to force regulator enable/disable");
+			priv->regulator_force_endis =
+				of_data->regulator_force_endis;
+			if (!of_data->regulator_force_voltage)
+				return dev_err_probe(&pdev->dev, -EINVAL,
+					"missing function pointer to force regulator voltage");
+			priv->regulator_force_voltage =
+				of_data->regulator_force_voltage;
+		}
 		/* Fallback for old DTs */
 		if (!priv->clkh && of_data->sdhi_flags & SDHI_FLAG_NEED_CLKH_FALLBACK)
 			priv->clkh = clk_get_parent(clk_get_parent(priv->clk));
@@ -1053,6 +1135,22 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 	if (ret)
 		goto efree;
 
+	if (priv->use_internal_regulator && of_data) {
+		ret = renesas_sdhi_internal_dmac_register_regulator(pdev, of_data);
+		if (ret)
+			goto efree;
+
+		/* Set the default init state for regulator in the HW */
+		ret = priv->regulator_force_endis(priv->internal_regulator,
+						  of_data->regulator_init_state);
+		if (ret)
+			goto efree;
+		ret = priv->regulator_force_voltage(priv->internal_regulator,
+						    of_data->regulator_init_voltage);
+		if (ret)
+			goto efree;
+	}
+
 	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/renesas_sdhi_internal_dmac.c b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
index d4b66daeda66..991e832821af 100644
--- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
@@ -89,6 +89,147 @@ static struct renesas_sdhi_scc rcar_gen3_scc_taps[] = {
 	},
 };
 
+static const unsigned int r9a09g057_vqmmc_voltages[] = {
+	1800000, 3300000,
+};
+
+static int r9a09g057_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 r9a09g057_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 r9a09g057_regulator_force_endis(struct regulator_dev *rdev, bool enable)
+{
+	if (enable)
+		return r9a09g057_regulator_enable(rdev);
+
+	return r9a09g057_regulator_disable(rdev);
+}
+
+static int r9a09g057_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);
+}
+
+static int r9a09g057_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);
+	if (sd_status & SD_STATUS_IOVS)
+		return 1800000;
+
+	return 3300000;
+}
+
+static int r9a09g057_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 = 0;
+	} else {
+		sd_status &= ~SD_STATUS_IOVS;
+		*selector = 1;
+	}
+	sd_ctrl_write32(host, CTL_SD_STATUS, sd_status);
+
+	return 0;
+}
+
+static int r9a09g057_regulator_force_voltage(struct regulator_dev *rdev,
+					     unsigned int voltage)
+{
+	unsigned int selector = 0;
+
+	return r9a09g057_regulator_set_voltage(rdev, voltage, voltage, &selector);
+}
+
+static int r9a09g057_regulator_list_voltage(struct regulator_dev *rdev,
+					    unsigned int selector)
+{
+	if (selector >= ARRAY_SIZE(r9a09g057_vqmmc_voltages))
+		return -EINVAL;
+
+	return r9a09g057_vqmmc_voltages[selector];
+}
+
+static struct regulator_init_data r9a09g057_regulator_init_data = {
+	.constraints = {
+		.valid_ops_mask = REGULATOR_CHANGE_STATUS | REGULATOR_CHANGE_VOLTAGE,
+	},
+};
+
+static const struct regulator_ops r9a09g057_regulator_voltage_ops = {
+	.enable = r9a09g057_regulator_enable,
+	.disable = r9a09g057_regulator_disable,
+	.is_enabled = r9a09g057_regulator_is_enabled,
+	.list_voltage = r9a09g057_regulator_list_voltage,
+	.get_voltage = r9a09g057_regulator_get_voltage,
+	.set_voltage = r9a09g057_regulator_set_voltage,
+	.map_voltage = regulator_map_voltage_ascend,
+};
+
+static struct regulator_desc r9a09g057_vqmmc_regulator = {
+	.of_match	= of_match_ptr("vqmmc-r9a09g057-regulator"),
+	.owner		= THIS_MODULE,
+	.type		= REGULATOR_VOLTAGE,
+	.ops		= &r9a09g057_regulator_voltage_ops,
+	.volt_table	= r9a09g057_vqmmc_voltages,
+	.n_voltages	= ARRAY_SIZE(r9a09g057_vqmmc_voltages),
+};
+
+static const struct renesas_sdhi_of_data of_data_r9a09g057 = {
+	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
+			  TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2,
+	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
+			  MMC_CAP_CMD23 | MMC_CAP_WAIT_WHILE_BUSY,
+	.capabilities2	= MMC_CAP2_NO_WRITE_PROTECT | MMC_CAP2_MERGE_CAPABLE,
+	.bus_shift	= 2,
+	.scc_offset	= 0x1000,
+	.taps		= rcar_gen3_scc_taps,
+	.taps_num	= ARRAY_SIZE(rcar_gen3_scc_taps),
+	/* DMAC can handle 32bit blk count but only 1 segment */
+	.max_blk_count	= UINT_MAX / TMIO_MAX_BLK_SIZE,
+	.max_segs	= 1,
+	.sdhi_flags	= SDHI_FLAG_NEED_CLKH_FALLBACK,
+	.rdesc = &r9a09g057_vqmmc_regulator,
+	.reg_init_data = &r9a09g057_regulator_init_data,
+	.regulator_init_state = false,
+	.regulator_init_voltage = 3300000,
+	.regulator_force_endis = r9a09g057_regulator_force_endis,
+	.regulator_force_voltage = r9a09g057_regulator_force_voltage,
+};
+
 static const struct renesas_sdhi_of_data of_data_rza2 = {
 	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
 			  TMIO_MMC_HAVE_CBSY,
@@ -260,6 +401,11 @@ static const struct renesas_sdhi_of_data_with_quirks of_rzg2l_compatible = {
 	.quirks = &sdhi_quirks_rzg2l,
 };
 
+static const struct renesas_sdhi_of_data_with_quirks of_r9a09g057_compatible = {
+	.of_data = &of_data_r9a09g057,
+	.quirks = &sdhi_quirks_rzg2l,
+};
+
 static const struct renesas_sdhi_of_data_with_quirks of_rcar_gen3_compatible = {
 	.of_data = &of_data_rcar_gen3,
 };
@@ -284,6 +430,7 @@ static const struct of_device_id renesas_sdhi_internal_dmac_of_match[] = {
 	{ .compatible = "renesas,sdhi-r8a77990", .data = &of_r8a77990_compatible, },
 	{ .compatible = "renesas,sdhi-r8a77995", .data = &of_rcar_gen3_nohs400_compatible, },
 	{ .compatible = "renesas,sdhi-r9a09g011", .data = &of_rzg2l_compatible, },
+	{ .compatible = "renesas,sdhi-r9a09g057", .data = &of_r9a09g057_compatible, },
 	{ .compatible = "renesas,rzg2l-sdhi", .data = &of_rzg2l_compatible, },
 	{ .compatible = "renesas,rcar-gen3-sdhi", .data = &of_rcar_gen3_compatible, },
 	{ .compatible = "renesas,rcar-gen4-sdhi", .data = &of_rcar_gen3_compatible, },
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 63000d9c7030..2648f444daa2 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 and RZ/V2H(P) */
 
 /* 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/V2H(P) */
+#define SD_STATUS_IOVS		BIT(16) /* only known on RZ/V2H(P) */
+
 /* Define some IRQ masks */
 /* This is the mask used at reset by the chip */
 #define TMIO_MASK_ALL           0x837f031d
-- 
2.34.1

Re: [PATCH v4 1/3] dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P) support

[Conor Dooley]

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

On Wed, Jun 26, 2024 at 02:23:39PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> The SD/MMC block on the RZ/V2H(P) ("R9A09G057") SoC is similar to that
> of the R-Car Gen3, but it has some differences:
> - HS400 is not supported.
> - It supports the SD_IOVS bit to control the IO voltage level.
> - It supports fixed address mode.
> 
> To accommodate these differences, a SoC-specific 'renesas,sdhi-r9a09g057'
> compatible string is added.
> 
> A 'vqmmc-regulator' object is introduced to handle the power enable (PWEN)
> and voltage level switching for the SD/MMC.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v3->v4
> - Dropped 'renesas,sdhi-use-internal-regulator' property
> - Moved 'vqmmc-regulator' to the top level
> 
> v2->v3
> - Renamed vqmmc-r9a09g057-regulator object to vqmmc-regulator
> - Added regulator-compatible property for vqmmc-regulator
> - Added 'renesas,sdhi-use-internal-regulator' property
> 
> v1->v2
> - Moved vqmmc object in the if block
> - Updated commit message
> ---
>  .../devicetree/bindings/mmc/renesas,sdhi.yaml | 26 ++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> index 3d0e61e59856..d632b67080bd 100644
> --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> @@ -18,6 +18,7 @@ properties:
>            - renesas,sdhi-r7s9210 # SH-Mobile AG5
>            - renesas,sdhi-r8a73a4 # R-Mobile APE6
>            - renesas,sdhi-r8a7740 # R-Mobile A1
> +          - renesas,sdhi-r9a09g057 # RZ/V2H(P)
>            - renesas,sdhi-sh73a0  # R-Mobile APE6
>        - items:
>            - enum:
> @@ -111,6 +112,15 @@ properties:
>  
>    max-frequency: true
>  
> +  vqmmc-regulator:
> +    type: object
> +    description: VQMMC SD regulator
> +    $ref: /schemas/regulator/regulator.yaml#
> +    unevaluatedProperties: false
> +    properties:
> +      regulator-compatible:
> +        pattern: "^vqmmc-r9a09g057-regulator"

The regulator core seems to say this property is deprecated:
	/**
	 * of_regulator_match - extract multiple regulator init data from device tree.
	 * @dev: device requesting the data
	 * @node: parent device node of the regulators
	 * @matches: match table for the regulators
	 * @num_matches: number of entries in match table
	 *
	 * This function uses a match table specified by the regulator driver to
	 * parse regulator init data from the device tree. @node is expected to
	 * contain a set of child nodes, each providing the init data for one
	 * regulator. The data parsed from a child node will be matched to a regulator
	 * based on either the deprecated property regulator-compatible if present,
                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

What am I missing? I also don't really understand why you're allowing a
pattern either, could you explain that?

Cheers,
Conor.

	 * or otherwise the child node's name. Note that the match table is modified
	 * in place and an additional of_node reference is taken for each matched
	 * regulator.
	 *
	 * Returns the number of matches found or a negative error code on failure.
	 */

> +
>  allOf:
>    - $ref: mmc-controller.yaml
>  
> @@ -118,7 +128,9 @@ allOf:
>        properties:
>          compatible:
>            contains:
> -            const: renesas,rzg2l-sdhi
> +            enum:
> +              - renesas,sdhi-r9a09g057
> +              - renesas,rzg2l-sdhi
>      then:
>        properties:
>          clocks:
> @@ -204,6 +216,18 @@ 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:
> +      required:
> +        - vqmmc-regulator
> +    else:
> +      properties:
> +        vqmmc-regulator: false
> +
>  required:
>    - compatible
>    - reg
> -- 
> 2.34.1
> 

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

RE: [PATCH v4 3/3] mmc: renesas_sdhi: Add support for RZ/V2H(P) SoC

[Biju Das]

Hi Wolfram and Prabhakar,

> -----Original Message-----
> From: Prabhakar <prabhakar.csengg@gmail.com>
> Sent: Wednesday, June 26, 2024 2:24 PM
> Subject: [PATCH v4 3/3] mmc: renesas_sdhi: Add support for RZ/V2H(P) SoC
> 
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> The SDHI/eMMC IPs found in the RZ/V2H(P) (a.k.a. r9a09g057) are very similar to those found in R-
> Car Gen3. However, they are not identical, necessitating an SoC-specific compatible string for
> fine-tuning driver support.
> 
> Key features of the RZ/V2H(P) SDHI/eMMC IPs include:
> - Voltage level control via the IOVS bit.
> - PWEN pin support via SD_STATUS register.
> - Lack of HS400 support.
> - Fixed address mode operation.
> 
> internal regulator support is added to control the voltage levels of SD pins via sd_iovs/sd_pwen
> bits in SD_STATUS register.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> # on RZ/G3S
> ---
> v3->v4
> - Dropped using 'renesas,sdhi-use-internal-regulator' property
> - Now using of_device_is_available() to check if regulator is available and enabled
> - Dropped extra spaces during operations
> - Included tested by tag from Claudiu
> - Rebased patch on top of https://patchwork.kernel.org/project/linux-renesas-
> soc/patch/20240626085015.32171-2-wsa+renesas@sang-engineering.com/
> 
> v2->v3
> - Moved regulator info to renesas_sdhi_of_data instead of quirks
> - Added support to configure the init state of regulator
> - Added function pointers to configure regulator
> - Added REGULATOR_CHANGE_VOLTAGE mask
> 
> v1->v2
> - Now controlling PWEN bit get/set_voltage
> ---
>  drivers/mmc/host/renesas_sdhi.h               |  13 ++
>  drivers/mmc/host/renesas_sdhi_core.c          |  98 ++++++++++++
>  drivers/mmc/host/renesas_sdhi_internal_dmac.c | 147 ++++++++++++++++++
>  drivers/mmc/host/tmio_mmc.h                   |   5 +
>  4 files changed, 263 insertions(+)
> 
> diff --git a/drivers/mmc/host/renesas_sdhi.h b/drivers/mmc/host/renesas_sdhi.h index
> f12a87442338..cd509e7142ba 100644
> --- a/drivers/mmc/host/renesas_sdhi.h
> +++ b/drivers/mmc/host/renesas_sdhi.h
> @@ -11,6 +11,8 @@
> 
>  #include <linux/dmaengine.h>
>  #include <linux/platform_device.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/machine.h>
>  #include <linux/workqueue.h>
>  #include "tmio_mmc.h"
> 
> @@ -36,6 +38,12 @@ struct renesas_sdhi_of_data {
>  	unsigned int max_blk_count;
>  	unsigned short max_segs;
>  	unsigned long sdhi_flags;
> +	struct regulator_desc *rdesc;
> +	struct regulator_init_data *reg_init_data;
> +	bool regulator_init_state;
> +	unsigned int regulator_init_voltage;
> +	int (*regulator_force_endis)(struct regulator_dev *rdev, bool enable);
> +	int (*regulator_force_voltage)(struct regulator_dev *rdev, unsigned
> +int voltage);
>  };
> 
>  #define SDHI_CALIB_TABLE_MAX 32
> @@ -95,6 +103,11 @@ struct renesas_sdhi {
> 
>  	struct reset_control *rstc;
>  	struct tmio_mmc_host *host;
> +
> +	bool use_internal_regulator;
> +	struct regulator_dev *internal_regulator;
> +	int (*regulator_force_endis)(struct regulator_dev *rdev, bool enable);
> +	int (*regulator_force_voltage)(struct regulator_dev *rdev, unsigned
> +int voltage);
>  };
> 
>  #define host_to_priv(host) \
> diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
> index 0fc159b52fa9..6f8e745477b5 100644
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -581,12 +581,50 @@ static void renesas_sdhi_reset(struct tmio_mmc_host *host, bool preserve)
> 
>  	if (!preserve) {
>  		if (priv->rstc) {
> +			bool regulator_enabled;
> +
> +			/* to restore back the internal regulator after reset make a copy of it */
> +			if (priv->use_internal_regulator)
> +				regulator_enabled = regulator_is_enabled(host->mmc->supply.vqmmc);

On all other SoCs, SD power enable is GPIO and we define this as GPIO hog, so that
It always supply Vmmc(3.3V) to the card for Card detection.

In this case, the SoC dedicated pin(PWEN) provides VMMC(3.3V). By reset this
value is always 1 (3.3V).

So should we implement handling of PWEN bit as part of vqmmc regulator
as it serves different functionality?

If I understand correctly,
VMMC(Always on 3.3V) for card detection -->PWEN pin
Vqmmc(1.8v-3.3V) for UHS operation for voltage switch-->IOVS pin

So according to me to implementing both the stuff in single regulator
is not correct? Like GPIO hog, PWEN bit should always on,
so that it always supply VMMC. What is your thoughts on this?

Please correct me if I am wrong.

Cheers,
Biju

Re: [PATCH v4 3/3] mmc: renesas_sdhi: Add support for RZ/V2H(P) SoC

[Wolfram Sang]

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


> If I understand correctly,
> VMMC(Always on 3.3V) for card detection -->PWEN pin
> Vqmmc(1.8v-3.3V) for UHS operation for voltage switch-->IOVS pin

If these assumptions are correct, then I am sorry to have missed it. I
thougaht PWEN is for vqmmc. If it is not, then we need two regulators,
one for vmmc and one for vqmmc. This is what the TMIO core expects and
already handles correctly on its own, or? Except for the reset handling
where we need to preserve some value in the SDHI core.


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

Re: [PATCH v4 1/3] dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P) support

[Wolfram Sang]

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

On Wed, Jun 26, 2024 at 02:23:39PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> The SD/MMC block on the RZ/V2H(P) ("R9A09G057") SoC is similar to that
> of the R-Car Gen3, but it has some differences:
> - HS400 is not supported.
> - It supports the SD_IOVS bit to control the IO voltage level.
> - It supports fixed address mode.
> 
> To accommodate these differences, a SoC-specific 'renesas,sdhi-r9a09g057'
> compatible string is added.
> 
> A 'vqmmc-regulator' object is introduced to handle the power enable (PWEN)
> and voltage level switching for the SD/MMC.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Can we have an example here? I can read DTS snippets better than YAML
code :/ Also wondering about the "regulator-compatible" property but
maybe the example makes the problem clear?


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

Re: [PATCH v4 3/3] mmc: renesas_sdhi: Add support for RZ/V2H(P) SoC

[Wolfram Sang]

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

Hi Prabhakar,

On Wed, Jun 26, 2024 at 02:23:41PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> The SDHI/eMMC IPs found in the RZ/V2H(P) (a.k.a. r9a09g057) are very
> similar to those found in R-Car Gen3. However, they are not identical,
> necessitating an SoC-specific compatible string for fine-tuning driver
> support.
> 
> Key features of the RZ/V2H(P) SDHI/eMMC IPs include:
> - Voltage level control via the IOVS bit.
> - PWEN pin support via SD_STATUS register.
> - Lack of HS400 support.
> - Fixed address mode operation.
> 
> internal regulator support is added to control the voltage levels of SD
> pins via sd_iovs/sd_pwen bits in SD_STATUS register.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> # on RZ/G3S
> ---
> v3->v4
> - Dropped using 'renesas,sdhi-use-internal-regulator' property
> - Now using of_device_is_available() to check if regulator is available and enabled
> - Dropped extra spaces during operations 
> - Included tested by tag from Claudiu
> - Rebased patch on top of https://patchwork.kernel.org/project/linux-renesas-soc/patch/20240626085015.32171-2-wsa+renesas@sang-engineering.com/
> 
> v2->v3
> - Moved regulator info to renesas_sdhi_of_data instead of quirks
> - Added support to configure the init state of regulator
> - Added function pointers to configure regulator
> - Added REGULATOR_CHANGE_VOLTAGE mask
> 
> v1->v2
> - Now controlling PWEN bit get/set_voltage
> ---
>  drivers/mmc/host/renesas_sdhi.h               |  13 ++
>  drivers/mmc/host/renesas_sdhi_core.c          |  98 ++++++++++++
>  drivers/mmc/host/renesas_sdhi_internal_dmac.c | 147 ++++++++++++++++++
>  drivers/mmc/host/tmio_mmc.h                   |   5 +
>  4 files changed, 263 insertions(+)
> 
> diff --git a/drivers/mmc/host/renesas_sdhi.h b/drivers/mmc/host/renesas_sdhi.h
> index f12a87442338..cd509e7142ba 100644
> --- a/drivers/mmc/host/renesas_sdhi.h
> +++ b/drivers/mmc/host/renesas_sdhi.h
> @@ -11,6 +11,8 @@
>  
>  #include <linux/dmaengine.h>
>  #include <linux/platform_device.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/machine.h>
>  #include <linux/workqueue.h>
>  #include "tmio_mmc.h"
>  
> @@ -36,6 +38,12 @@ struct renesas_sdhi_of_data {
>  	unsigned int max_blk_count;
>  	unsigned short max_segs;
>  	unsigned long sdhi_flags;
> +	struct regulator_desc *rdesc;
> +	struct regulator_init_data *reg_init_data;
> +	bool regulator_init_state;
> +	unsigned int regulator_init_voltage;
> +	int (*regulator_force_endis)(struct regulator_dev *rdev, bool enable);
> +	int (*regulator_force_voltage)(struct regulator_dev *rdev, unsigned int voltage);

I am open for discussing this but maybe here only

+	struct renesas_sdhi_regulator *internal_regulator

or something and create the new struct with the additions above?

> +	int (*regulator_force_endis)(struct regulator_dev *rdev, bool enable);
> +	int (*regulator_force_voltage)(struct regulator_dev *rdev, unsigned int voltage);

Do we need these functions because the regulator framework cannot force
these actions because it caches the old state? I wonder if we can avoid
these functions...

And the questions from the other threads need further discussions as
well.

But in general, I still like this approach.

Thank you,

   Wolfram


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

RE: [PATCH v4 3/3] mmc: renesas_sdhi: Add support for RZ/V2H(P) SoC

[Biju Das]

Hi Wolfram,

Thanks for the feedback.

> -----Original Message-----
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Sent: Wednesday, July 3, 2024 10:36 AM
> Subject: Re: [PATCH v4 3/3] mmc: renesas_sdhi: Add support for RZ/V2H(P) SoC
> 
> 
> > If I understand correctly,
> > VMMC(Always on 3.3V) for card detection -->PWEN pin
> > Vqmmc(1.8v-3.3V) for UHS operation for voltage switch-->IOVS pin
> 
> If these assumptions are correct, then I am sorry to have missed it. I thougaht PWEN is for vqmmc.

The assumptions are correct. PWEN is basically vmmc, which needs to be on always for 
the card detection.

> If it is not, then we need two regulators, one for vmmc and one for vqmmc. This is what the TMIO core
> expects and already handles correctly on its own, or? Except for the reset handling where we need to
> preserve some value in the SDHI core.

Yes, probably for PWEN, vmmc regulator with "always on" and "boot on" property enabled in regulator node.
So that it will be always on.

Cheers,
Biju

Re: [PATCH v4 3/3] mmc: renesas_sdhi: Add support for RZ/V2H(P) SoC

[Lad, Prabhakar]

Hi Wolfram,

Thank you for the review.

On Wed, Jul 3, 2024 at 10:43 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> Hi Prabhakar,
>
> On Wed, Jun 26, 2024 at 02:23:41PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > The SDHI/eMMC IPs found in the RZ/V2H(P) (a.k.a. r9a09g057) are very
> > similar to those found in R-Car Gen3. However, they are not identical,
> > necessitating an SoC-specific compatible string for fine-tuning driver
> > support.
> >
> > Key features of the RZ/V2H(P) SDHI/eMMC IPs include:
> > - Voltage level control via the IOVS bit.
> > - PWEN pin support via SD_STATUS register.
> > - Lack of HS400 support.
> > - Fixed address mode operation.
> >
> > internal regulator support is added to control the voltage levels of SD
> > pins via sd_iovs/sd_pwen bits in SD_STATUS register.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> # on RZ/G3S
> > ---
> > v3->v4
> > - Dropped using 'renesas,sdhi-use-internal-regulator' property
> > - Now using of_device_is_available() to check if regulator is available and enabled
> > - Dropped extra spaces during operations
> > - Included tested by tag from Claudiu
> > - Rebased patch on top of https://patchwork.kernel.org/project/linux-renesas-soc/patch/20240626085015.32171-2-wsa+renesas@sang-engineering.com/
> >
> > v2->v3
> > - Moved regulator info to renesas_sdhi_of_data instead of quirks
> > - Added support to configure the init state of regulator
> > - Added function pointers to configure regulator
> > - Added REGULATOR_CHANGE_VOLTAGE mask
> >
> > v1->v2
> > - Now controlling PWEN bit get/set_voltage
> > ---
> >  drivers/mmc/host/renesas_sdhi.h               |  13 ++
> >  drivers/mmc/host/renesas_sdhi_core.c          |  98 ++++++++++++
> >  drivers/mmc/host/renesas_sdhi_internal_dmac.c | 147 ++++++++++++++++++
> >  drivers/mmc/host/tmio_mmc.h                   |   5 +
> >  4 files changed, 263 insertions(+)
> >
> > diff --git a/drivers/mmc/host/renesas_sdhi.h b/drivers/mmc/host/renesas_sdhi.h
> > index f12a87442338..cd509e7142ba 100644
> > --- a/drivers/mmc/host/renesas_sdhi.h
> > +++ b/drivers/mmc/host/renesas_sdhi.h
> > @@ -11,6 +11,8 @@
> >
> >  #include <linux/dmaengine.h>
> >  #include <linux/platform_device.h>
> > +#include <linux/regulator/driver.h>
> > +#include <linux/regulator/machine.h>
> >  #include <linux/workqueue.h>
> >  #include "tmio_mmc.h"
> >
> > @@ -36,6 +38,12 @@ struct renesas_sdhi_of_data {
> >       unsigned int max_blk_count;
> >       unsigned short max_segs;
> >       unsigned long sdhi_flags;
> > +     struct regulator_desc *rdesc;
> > +     struct regulator_init_data *reg_init_data;
> > +     bool regulator_init_state;
> > +     unsigned int regulator_init_voltage;
> > +     int (*regulator_force_endis)(struct regulator_dev *rdev, bool enable);
> > +     int (*regulator_force_voltage)(struct regulator_dev *rdev, unsigned int voltage);
>
> I am open for discussing this but maybe here only
>
> +       struct renesas_sdhi_regulator *internal_regulator
>
> or something and create the new struct with the additions above?
>
> > +     int (*regulator_force_endis)(struct regulator_dev *rdev, bool enable);
> > +     int (*regulator_force_voltage)(struct regulator_dev *rdev, unsigned int voltage);
>
> Do we need these functions because the regulator framework cannot force
> these actions because it caches the old state? I wonder if we can avoid
> these functions...
>
Yes, for the voltage setting, it caches the values. However, for the
regulator enable/disable, we can use is_enabled(), which probes the
hardware.

The reset value for PWEN is 1. The regulator_force_endis() callback is
mainly added for a scenario where, consider a code flow where the
regulator is disabled (using regulator_disable()) and now we land in
the reset callback (i.e., renesas_sdhi_reset()). Here, after issuing
the reset, the PWEN value will be 1, but we need to restore it back.
Hence, this callback is necessary. Note that is_enabled() cannot be
used, as it probes the hardware when it switches states after a reset.

The reset value for IOVS is 3.3V. Below is the scenario for which
regulator_force_voltage() is added:

-----> Current value: 1.8V (cached by the regulator)
--------------> After reset:
------------------> Hardware has 3.3V, but the regulator core cache
still has 1.8V.
----------------------> When requested to switch to 1.8V from MMC
core: The regulator core returns success, as it sees 1.8V in the
cached state.
----------------------------> As a result, the SD card won't work.

Cheers,
Prabhakar

Re: [PATCH v4 1/3] dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P) support

[Lad, Prabhakar]

Hi Wolfram,

On Wed, Jul 3, 2024 at 10:38 AM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> On Wed, Jun 26, 2024 at 02:23:39PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > The SD/MMC block on the RZ/V2H(P) ("R9A09G057") SoC is similar to that
> > of the R-Car Gen3, but it has some differences:
> > - HS400 is not supported.
> > - It supports the SD_IOVS bit to control the IO voltage level.
> > - It supports fixed address mode.
> >
> > To accommodate these differences, a SoC-specific 'renesas,sdhi-r9a09g057'
> > compatible string is added.
> >
> > A 'vqmmc-regulator' object is introduced to handle the power enable (PWEN)
> > and voltage level switching for the SD/MMC.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Can we have an example here? I can read DTS snippets better than YAML
> code :/ Also wondering about the "regulator-compatible" property but
> maybe the example makes the problem clear?
>
You mean in the commit message or comment section? (I had added below
in the cover letter)

SoC DTSI node:
sdhi1: mmc@15c10000 {
        compatible = "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 167>,
                        <&cpg CPG_MOD 169>,
                        <&cpg CPG_MOD 168>,
                        <&cpg CPG_MOD 170>;
        clock-names = "core", "clkh", "cd", "aclk";
        resets = <&cpg 168>;
        power-domains = <&cpg>;
        status = "disabled";

        vqmmc_sdhi1: vqmmc-regulator {
                regulator-compatible = "vqmmc-r9a09g057-regulator";
                regulator-name = "sdhi1-vqmmc-regulator";
                regulator-min-microvolt = <1800000>;
                regulator-max-microvolt = <3300000>;
                status = "disabled";
        };
};

Board DTS:
&sdhi1 {
        pinctrl-0 = <&sdhi1_pins>;
        pinctrl-1 = <&sdhi1_pins>;
        pinctrl-names = "default", "state_uhs";
        vmmc-supply = <&reg_3p3v>;
        vqmmc-supply = <&vqmmc_sdhi1>;
        bus-width = <4>;
        sd-uhs-sdr50;
        sd-uhs-sdr104;
        status = "okay";
};

&vqmmc_sdhi1 {
     status = "okay";
};

Based on feedback from Conor, we cannot use the regulator-compatible
property. This would require us to implement separate drivers (one for
VMMC and another for VQMMC), which I believe would necessitate the use
of regmap. Currently, this seems unnecessary for controlling the two
bits as a regulator. As Geert previously pointed out, the PWEN and
IOVS pins can always be multiplexed as GPIOs on the RZ/V2H SoC (as is
done on R-Car devices). Therefore, I am inclined to drop the internal
regulator support for now.

Let me know your thoughts.

Cheers,
Prabhakar

Re: [PATCH v4 1/3] dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P) support

[Lad, Prabhakar]

Hi Wolfram,

On Thu, Jul 4, 2024 at 7:27 PM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
>
> Hi Wolfram,
>
> On Wed, Jul 3, 2024 at 10:38 AM Wolfram Sang
> <wsa+renesas@sang-engineering.com> wrote:
> >
> > On Wed, Jun 26, 2024 at 02:23:39PM +0100, Prabhakar wrote:
> > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > The SD/MMC block on the RZ/V2H(P) ("R9A09G057") SoC is similar to that
> > > of the R-Car Gen3, but it has some differences:
> > > - HS400 is not supported.
> > > - It supports the SD_IOVS bit to control the IO voltage level.
> > > - It supports fixed address mode.
> > >
> > > To accommodate these differences, a SoC-specific 'renesas,sdhi-r9a09g057'
> > > compatible string is added.
> > >
> > > A 'vqmmc-regulator' object is introduced to handle the power enable (PWEN)
> > > and voltage level switching for the SD/MMC.
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Can we have an example here? I can read DTS snippets better than YAML
> > code :/ Also wondering about the "regulator-compatible" property but
> > maybe the example makes the problem clear?
> >
> You mean in the commit message or comment section? (I had added below
> in the cover letter)
>
> SoC DTSI node:
> sdhi1: mmc@15c10000 {
>         compatible = "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 167>,
>                         <&cpg CPG_MOD 169>,
>                         <&cpg CPG_MOD 168>,
>                         <&cpg CPG_MOD 170>;
>         clock-names = "core", "clkh", "cd", "aclk";
>         resets = <&cpg 168>;
>         power-domains = <&cpg>;
>         status = "disabled";
>
>         vqmmc_sdhi1: vqmmc-regulator {
>                 regulator-compatible = "vqmmc-r9a09g057-regulator";
>                 regulator-name = "sdhi1-vqmmc-regulator";
>                 regulator-min-microvolt = <1800000>;
>                 regulator-max-microvolt = <3300000>;
>                 status = "disabled";
>         };
> };
>
> Board DTS:
> &sdhi1 {
>         pinctrl-0 = <&sdhi1_pins>;
>         pinctrl-1 = <&sdhi1_pins>;
>         pinctrl-names = "default", "state_uhs";
>         vmmc-supply = <&reg_3p3v>;
>         vqmmc-supply = <&vqmmc_sdhi1>;
>         bus-width = <4>;
>         sd-uhs-sdr50;
>         sd-uhs-sdr104;
>         status = "okay";
> };
>
> &vqmmc_sdhi1 {
>      status = "okay";
> };
>
> Based on feedback from Conor, we cannot use the regulator-compatible
> property. This would require us to implement separate drivers (one for
> VMMC and another for VQMMC), which I believe would necessitate the use
> of regmap. Currently, this seems unnecessary for controlling the two
> bits as a regulator. As Geert previously pointed out, the PWEN and
> IOVS pins can always be multiplexed as GPIOs on the RZ/V2H SoC (as is
> done on R-Car devices). Therefore, I am inclined to drop the internal
> regulator support for now.
>
> Let me know your thoughts.
>
Gentle ping.

Cheers,
Prabhakar