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

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

[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 <michael@allwinnertech.com>
---
 drivers/mmc/host/sunxi-mmc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 2702736a1c57..afeefead6501 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -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);
-- 
2.29.0

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

[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 <michael@allwinnertech.com>
---
 drivers/mmc/host/sunxi-mmc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 2702736a1c57..afeefead6501 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -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);
-- 
2.29.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Ulf Hansson]

On Wed, 22 Dec 2021 at 04:07, Michael Wu <michael@allwinnertech.com> wrote:
>
> 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 <michael@allwinnertech.com>
> ---
>  drivers/mmc/host/sunxi-mmc.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> index 2702736a1c57..afeefead6501 100644
> --- a/drivers/mmc/host/sunxi-mmc.c
> +++ b/drivers/mmc/host/sunxi-mmc.c
> @@ -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.

> +
>         host->reg_base = devm_platform_ioremap_resource(pdev, 0);
>         if (IS_ERR(host->reg_base))
>                 return PTR_ERR(host->reg_base);

Kind regards
Uffe

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

[Ulf Hansson]

On Wed, 22 Dec 2021 at 04:07, Michael Wu <michael@allwinnertech.com> wrote:
>
> 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 <michael@allwinnertech.com>
> ---
>  drivers/mmc/host/sunxi-mmc.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> index 2702736a1c57..afeefead6501 100644
> --- a/drivers/mmc/host/sunxi-mmc.c
> +++ b/drivers/mmc/host/sunxi-mmc.c
> @@ -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.

> +
>         host->reg_base = devm_platform_ioremap_resource(pdev, 0);
>         if (IS_ERR(host->reg_base))
>                 return PTR_ERR(host->reg_base);

Kind regards
Uffe

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[michael]

> 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
> On Wed, 22 Dec 2021 at 04:07, Michael Wu <michael@allwinnertech.com> wrote:
> >
> > 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 <michael@allwinnertech.com>
> > ---
> >  drivers/mmc/host/sunxi-mmc.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> > index 2702736a1c57..afeefead6501 100644
> > --- a/drivers/mmc/host/sunxi-mmc.c
> > +++ b/drivers/mmc/host/sunxi-mmc.c
> > @@ -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.
> 
> > +
> >         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 <michael@allwinnertech.com>
---

diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 2702736..0da74bd 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -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

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

[michael]

> 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
> On Wed, 22 Dec 2021 at 04:07, Michael Wu <michael@allwinnertech.com> wrote:
> >
> > 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 <michael@allwinnertech.com>
> > ---
> >  drivers/mmc/host/sunxi-mmc.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> > index 2702736a1c57..afeefead6501 100644
> > --- a/drivers/mmc/host/sunxi-mmc.c
> > +++ b/drivers/mmc/host/sunxi-mmc.c
> > @@ -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.
> 
> > +
> >         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 <michael@allwinnertech.com>
---

diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
index 2702736..0da74bd 100644
--- a/drivers/mmc/host/sunxi-mmc.c
+++ b/drivers/mmc/host/sunxi-mmc.c
@@ -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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Ulf Hansson]

On Fri, 31 Dec 2021 at 09:28, michael@allwinnertech.com
<michael@allwinnertech.com> wrote:
>
> > 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
> > On Wed, 22 Dec 2021 at 04:07, Michael Wu <michael@allwinnertech.com> wrote:
> > >
> > > 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 <michael@allwinnertech.com>
> > > ---
> > >  drivers/mmc/host/sunxi-mmc.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> > > index 2702736a1c57..afeefead6501 100644
> > > --- a/drivers/mmc/host/sunxi-mmc.c
> > > +++ b/drivers/mmc/host/sunxi-mmc.c
> > > @@ -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.
> >
> > > +
> > >         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>;

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.

> };
> 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 <michael@allwinnertech.com>
> ---
>
> diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> index 2702736..0da74bd 100644
> --- a/drivers/mmc/host/sunxi-mmc.c
> +++ b/drivers/mmc/host/sunxi-mmc.c
> @@ -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

Yep, this looks good to me!

Kind regards
Uffe

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

[Ulf Hansson]

On Fri, 31 Dec 2021 at 09:28, michael@allwinnertech.com
<michael@allwinnertech.com> wrote:
>
> > 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
> > On Wed, 22 Dec 2021 at 04:07, Michael Wu <michael@allwinnertech.com> wrote:
> > >
> > > 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 <michael@allwinnertech.com>
> > > ---
> > >  drivers/mmc/host/sunxi-mmc.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> > > index 2702736a1c57..afeefead6501 100644
> > > --- a/drivers/mmc/host/sunxi-mmc.c
> > > +++ b/drivers/mmc/host/sunxi-mmc.c
> > > @@ -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.
> >
> > > +
> > >         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>;

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.

> };
> 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 <michael@allwinnertech.com>
> ---
>
> diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> index 2702736..0da74bd 100644
> --- a/drivers/mmc/host/sunxi-mmc.c
> +++ b/drivers/mmc/host/sunxi-mmc.c
> @@ -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

Yep, this looks good to me!

Kind regards
Uffe

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[michael]

> On Fri, 31 Dec 2021 at 09:28, michael@allwinnertech.com
> <michael@allwinnertech.com> wrote:
> >
> > > 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
> > > On Wed, 22 Dec 2021 at 04:07, Michael Wu <michael@allwinnertech.com> wrote:
> > > >
> > > > 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 <michael@allwinnertech.com>
> > > > ---
> > > >  drivers/mmc/host/sunxi-mmc.c | 8 ++++++++
> > > >  1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> > > > index 2702736a1c57..afeefead6501 100644
> > > > --- a/drivers/mmc/host/sunxi-mmc.c
> > > > +++ b/drivers/mmc/host/sunxi-mmc.c
> > > > @@ -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.
> > >
> > > > +
> > > >         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>;
> 
> 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>;

> > };
> > 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 <michael@allwinnertech.com>
> > ---
> >
> > diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> > index 2702736..0da74bd 100644
> > --- a/drivers/mmc/host/sunxi-mmc.c
> > +++ b/drivers/mmc/host/sunxi-mmc.c
> > @@ -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
> 
> 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
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[michael]

> On Fri, 31 Dec 2021 at 09:28, michael@allwinnertech.com
> <michael@allwinnertech.com> wrote:
> >
> > > 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
> > > On Wed, 22 Dec 2021 at 04:07, Michael Wu <michael@allwinnertech.com> wrote:
> > > >
> > > > 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 <michael@allwinnertech.com>
> > > > ---
> > > >  drivers/mmc/host/sunxi-mmc.c | 8 ++++++++
> > > >  1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> > > > index 2702736a1c57..afeefead6501 100644
> > > > --- a/drivers/mmc/host/sunxi-mmc.c
> > > > +++ b/drivers/mmc/host/sunxi-mmc.c
> > > > @@ -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.
> > >
> > > > +
> > > >         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>;
> 
> 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>;

> > };
> > 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 <michael@allwinnertech.com>
> > ---
> >
> > diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c
> > index 2702736..0da74bd 100644
> > --- a/drivers/mmc/host/sunxi-mmc.c
> > +++ b/drivers/mmc/host/sunxi-mmc.c
> > @@ -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
> 
> 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