Skip to contentSkip to conversation

mmc:sunxi-mmc:add support on discrete device power supply

PATCH In review Michael Wu 2 participants 5 messages Dec 22, 2021 – Dec 31, 2021

Subsystems (2)
MMC, SD and SDIO Arm/Allwinner sunXi SoC support
lore
Share & tools
lore

Michael Wu

author
#0 lore raw

PATCH 1/3 mmc:sunxi-mmc:add support on discrete device power supply

Because some platform has no regulator, only use discrete devices to supply power,For this situation, to use sd/mmc card, we add ocr manually

Signed-off-by Michael Wu <[email protected]>
---
 drivers/mmc/host/sunxi-mmc.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/mmc/host/sunxi-mmc.c +8 -0
index 2702736a1c57..afeefead6501 100644
@@ -1300,6 +1300,14 @@ static int sunxi_mmc_resource_request(struct sunxi_mmc_host *host,
 	if (ret)
 		return ret;
 
+	/**
+	 * Some platforms has no regulator. Discrete devices are used instead.
+	 * To support sd/mmc card, we need to add ocr manually.
+	 */
+	if (!host->mmc->ocr_avail)
+		host->mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
+
+
 	host->reg_base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(host->reg_base))
 		return PTR_ERR(host->reg_base);
Signature · 1 line
2.29.0
#1 lore raw
Quoted 14 lines · Michael Wu

Because some platform has no regulator, only use discrete devices to supply power,For this situation, to use sd/mmc card, we add ocr manually

Signed-off-by Michael Wu <[email protected]>
---
 drivers/mmc/host/sunxi-mmc.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/mmc/host/sunxi-mmc.c +0 -0
index 2702736a1c57..afeefead6501 100644

drivers/mmc/host/sunxi-mmc.c static int sunxi_mmc_resource_request(struct sunxi_mmc_host *host, · line 1309

Quoted 10 lines
drivers/mmc/host/sunxi-mmc.c +7 -0
@@ -1300,6 +1300,14 @@ static int sunxi_mmc_resource_request(struct sunxi_mmc_host *host,
        if (ret)
                return ret;
 
+       /**
+        * Some platforms has no regulator. Discrete devices are used instead.
+        * To support sd/mmc card, we need to add ocr manually.
+        */
+       if (!host->mmc->ocr_avail)
+               host->mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
+

Rather than doing this, I suggest you hook up a fixed vmmc regulator in the DTS.

Nevertheless, it seems reasonable to check that the ocr_avail gets set up correctly. And if it doesn't, perhaps we should print a warning and return an error code.

drivers/mmc/host/sunxi-mmc.c static int sunxi_mmc_resource_request(struct sunxi_mmc_host *host, · line 1313

Quoted 4 lines
drivers/mmc/host/sunxi-mmc.c +1 -0
+
        host->reg_base = devm_platform_ioremap_resource(pdev, 0);
        if (IS_ERR(host->reg_base))
                return PTR_ERR(host->reg_base);

Kind regards
Uffe

author
#2 lore raw
Quoted 37 lines

From: Ulf Hansson
Date: 2021-12-29 00:49
To: Michael Wu
CC: mripard; wens; samuel; andre.przywara; jernej.skrabec; linux-mmc; linux-arm-kernel; linux-sunxi; linux-kernel
Subject: Re: [PATCH 1/3] mmc:sunxi-mmc:add support on discrete device power supply

Quoted 14 lines · Michael Wu

Because some platform has no regulator, only use discrete devices to supply power,For this situation, to use sd/mmc card, we add ocr manually

Signed-off-by Michael Wu <[email protected]>
---
 drivers/mmc/host/sunxi-mmc.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/mmc/host/sunxi-mmc.c +0 -0
index 2702736a1c57..afeefead6501 100644

drivers/mmc/host/sunxi-mmc.c static int sunxi_mmc_resource_request(struct sunxi_mmc_host *host, · line 1309

Quoted 10 lines
drivers/mmc/host/sunxi-mmc.c +7 -0
@@ -1300,6 +1300,14 @@ static int sunxi_mmc_resource_request(struct sunxi_mmc_host *host,
        if (ret)
                return ret;
 
+       /**
+        * Some platforms has no regulator. Discrete devices are used instead.
+        * To support sd/mmc card, we need to add ocr manually.
+        */
+       if (!host->mmc->ocr_avail)
+               host->mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
+

Rather than doing this, I suggest you hook up a fixed vmmc regulator in the DTS.

Nevertheless, it seems reasonable to check that the ocr_avail gets set up correctly. And if it doesn't, perhaps we should print a warning and return an error code.

Quoted 7 lines

drivers/mmc/host/sunxi-mmc.c static int sunxi_mmc_resource_request(struct sunxi_mmc_host *host, · line 1313

Quoted 4 lines
drivers/mmc/host/sunxi-mmc.c +1 -0
+
        host->reg_base = devm_platform_ioremap_resource(pdev, 0);
        if (IS_ERR(host->reg_base))
                return PTR_ERR(host->reg_base);

Kind regards
Uffe

Dear Uffe,
Thanks for your suggestion. It is a better solution.
I've modified my patch. Please check if it's reasonable. If it is, I'll re-sumbit it later.

---
Subject: [PATCH v2] mmc: sunxi-mmc: check ocr_avail on resource request

Some platforms have no regulator, discrete power devices are used instead. However, sunxi_mmc_probe does not catch this exception when regulator is absent in DTS. This leads to sd or eMMC init failure.
To solve this, a fixed vmmc regulator must be hooked up in DTS, like this:

reg_dummy_vmmc: dummy_vmmc {
	compatible = "regulator-fixed";
	regulator-name = "dummy-vmmc";
	regulator-min-microvolt = <500000>;
	regulator-max-microvolt = <3500000>;
};
mmc0:mmc@4020000 {
	compatible = "allwinner,sun50i-a100-emmc";
	device_type = "mmc0";
	vmmc-supply = <&reg_dummy_vmmc>;
}

In this patch, we print an error message and abort the probe process if the regulator is not specified in DTS.

Signed-off-by Michael Wu <[email protected]>

---

drivers/mmc/host/sunxi-mmc.c +5 -0
index 2702736..0da74bd 100644
@@ -1300,6 +1300,11 @@
 	if (ret)
 		return ret;
 
+	if (!host->mmc->ocr_avail) {
+		dev_err(&pdev->dev, "Could not get mmc regulator\n");
+		return -EINVAL;
+	}
+
 	host->reg_base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(host->reg_base))
 		return PTR_ERR(host->reg_base);

Best Regards,
Michael Wu

#3 lore raw
Quoted 57 lines · On Fri, 31 Dec 2021 at 09:28, [email protected]…
Quoted 38 lines

From: Ulf Hansson
Date: 2021-12-29 00:49
To: Michael Wu
CC: mripard; wens; samuel; andre.przywara; jernej.skrabec; linux-mmc; linux-arm-kernel; linux-sunxi; linux-kernel
Subject: Re: [PATCH 1/3] mmc:sunxi-mmc:add support on discrete device power supply

Quoted 14 lines · Michael Wu

Because some platform has no regulator, only use discrete devices to supply power,For this situation, to use sd/mmc card, we add ocr manually

Signed-off-by Michael Wu <[email protected]>
---
 drivers/mmc/host/sunxi-mmc.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/mmc/host/sunxi-mmc.c +0 -0
index 2702736a1c57..afeefead6501 100644

drivers/mmc/host/sunxi-mmc.c static int sunxi_mmc_resource_request(struct sunxi_mmc_host *host, · line 1309

Quoted 10 lines
drivers/mmc/host/sunxi-mmc.c +7 -0
@@ -1300,6 +1300,14 @@ static int sunxi_mmc_resource_request(struct sunxi_mmc_host *host,
        if (ret)
                return ret;
 
+       /**
+        * Some platforms has no regulator. Discrete devices are used instead.
+        * To support sd/mmc card, we need to add ocr manually.
+        */
+       if (!host->mmc->ocr_avail)
+               host->mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
+

Rather than doing this, I suggest you hook up a fixed vmmc regulator in the DTS.

Nevertheless, it seems reasonable to check that the ocr_avail gets set up correctly. And if it doesn't, perhaps we should print a warning and return an error code.

Quoted 7 lines

drivers/mmc/host/sunxi-mmc.c line 1310

Quoted 4 lines
drivers/mmc/host/sunxi-mmc.c +1 -0
+
        host->reg_base = devm_platform_ioremap_resource(pdev, 0);
        if (IS_ERR(host->reg_base))
                return PTR_ERR(host->reg_base);

Kind regards
Uffe

Dear Uffe,
Thanks for your suggestion. It is a better solution.
I've modified my patch. Please check if it's reasonable. If it is, I'll re-sumbit it later.

---
Subject: [PATCH v2] mmc: sunxi-mmc: check ocr_avail on resource request

Some platforms have no regulator, discrete power devices are used instead. However, sunxi_mmc_probe does not catch this exception when regulator is absent in DTS. This leads to sd or eMMC init failure.
To solve this, a fixed vmmc regulator must be hooked up in DTS, like this:

Quoted 5 lines
reg_dummy_vmmc: dummy_vmmc {
        compatible = "regulator-fixed";
        regulator-name = "dummy-vmmc";
        regulator-min-microvolt = <500000>;
        regulator-max-microvolt = <3500000>;

The min/max should be set to the same value as you can't really change the voltage levels.

If you know the voltage level that is supplied for your platform, then state this value - otherwise I would suggest picking 3.3V, which is rather commonly used for MMC/SD.

Quoted 29 lines

};

mmc0:mmc@4020000 {
        compatible = "allwinner,sun50i-a100-emmc";
        device_type = "mmc0";
        vmmc-supply = <&reg_dummy_vmmc>;
}

In this patch, we print an error message and abort the probe process if the regulator is not specified in DTS.

Signed-off-by Michael Wu <[email protected]>

---

drivers/mmc/host/sunxi-mmc.c +5 -0
index 2702736..0da74bd 100644
@@ -1300,6 +1300,11 @@
        if (ret)
                return ret;
 
+       if (!host->mmc->ocr_avail) {
+               dev_err(&pdev->dev, "Could not get mmc regulator\n");
+               return -EINVAL;
+       }
+
        host->reg_base = devm_platform_ioremap_resource(pdev, 0);
        if (IS_ERR(host->reg_base))
                return PTR_ERR(host->reg_base);
Quoted 2 lines

Best Regards,
Michael Wu

Yep, this looks good to me!

Kind regards
Uffe

author
#4 lore raw
Quoted 65 lines
Quoted 57 lines · On Fri, 31 Dec 2021 at 09:28, [email protected]…
Quoted 38 lines

From: Ulf Hansson
Date: 2021-12-29 00:49
To: Michael Wu
CC: mripard; wens; samuel; andre.przywara; jernej.skrabec; linux-mmc; linux-arm-kernel; linux-sunxi; linux-kernel
Subject: Re: [PATCH 1/3] mmc:sunxi-mmc:add support on discrete device power supply

Quoted 14 lines · Michael Wu

Because some platform has no regulator, only use discrete devices to supply power,For this situation, to use sd/mmc card, we add ocr manually

Signed-off-by Michael Wu <[email protected]>
---
 drivers/mmc/host/sunxi-mmc.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/mmc/host/sunxi-mmc.c +0 -0
index 2702736a1c57..afeefead6501 100644

drivers/mmc/host/sunxi-mmc.c static int sunxi_mmc_resource_request(struct sunxi_mmc_host *host, · line 1309

Quoted 10 lines
drivers/mmc/host/sunxi-mmc.c +7 -0
@@ -1300,6 +1300,14 @@ static int sunxi_mmc_resource_request(struct sunxi_mmc_host *host,
        if (ret)
                return ret;
 
+       /**
+        * Some platforms has no regulator. Discrete devices are used instead.
+        * To support sd/mmc card, we need to add ocr manually.
+        */
+       if (!host->mmc->ocr_avail)
+               host->mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
+

Rather than doing this, I suggest you hook up a fixed vmmc regulator in the DTS.

Nevertheless, it seems reasonable to check that the ocr_avail gets set up correctly. And if it doesn't, perhaps we should print a warning and return an error code.

Quoted 7 lines

drivers/mmc/host/sunxi-mmc.c line 1310

Quoted 4 lines
drivers/mmc/host/sunxi-mmc.c +1 -0
+
        host->reg_base = devm_platform_ioremap_resource(pdev, 0);
        if (IS_ERR(host->reg_base))
                return PTR_ERR(host->reg_base);

Kind regards
Uffe

Dear Uffe,
Thanks for your suggestion. It is a better solution.
I've modified my patch. Please check if it's reasonable. If it is, I'll re-sumbit it later.

---
Subject: [PATCH v2] mmc: sunxi-mmc: check ocr_avail on resource request

Some platforms have no regulator, discrete power devices are used instead. However, sunxi_mmc_probe does not catch this exception when regulator is absent in DTS. This leads to sd or eMMC init failure.
To solve this, a fixed vmmc regulator must be hooked up in DTS, like this:

Quoted 5 lines
reg_dummy_vmmc: dummy_vmmc {
        compatible = "regulator-fixed";
        regulator-name = "dummy-vmmc";
        regulator-min-microvolt = <500000>;
        regulator-max-microvolt = <3500000>;
Quoted 6 lines

The min/max should be set to the same value as you can't really change the voltage levels.

If you know the voltage level that is supplied for your platform, then state this value - otherwise I would suggest picking 3.3V, which is rather commonly used for MMC/SD.

Okay. The supplied voltage is 2.7V~3.6V. l'll correct the min/max value to 3.3V:

        regulator-min-microvolt = <3300000>;
        regulator-max-microvolt = <3300000>;
Quoted 29 lines
Quoted 29 lines

};

mmc0:mmc@4020000 {
        compatible = "allwinner,sun50i-a100-emmc";
        device_type = "mmc0";
        vmmc-supply = <&reg_dummy_vmmc>;
}

In this patch, we print an error message and abort the probe process if the regulator is not specified in DTS.

Signed-off-by Michael Wu <[email protected]>

---

drivers/mmc/host/sunxi-mmc.c +5 -0
index 2702736..0da74bd 100644
@@ -1300,6 +1300,11 @@
        if (ret)
                return ret;
 
+       if (!host->mmc->ocr_avail) {
+               dev_err(&pdev->dev, "Could not get mmc regulator\n");
+               return -EINVAL;
+       }
+
        host->reg_base = devm_platform_ioremap_resource(pdev, 0);
        if (IS_ERR(host->reg_base))
                return PTR_ERR(host->reg_base);
Quoted 7 lines
Quoted 2 lines

Best Regards,
Michael Wu

Yep, this looks good to me!

Kind regards
Uffe

I'll submit the patch once I get back to work. Thanks for your effort and happy new year :)

Best Regards,
Michael Wu
_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel