[PATCH v4] mmc: mxs-mmc: Introduce regulator support

[PATCH v4] mmc: mxs-mmc: Introduce regulator support

[Martin Kepplinger]

From: Martin Kepplinger <martin.kepplinger@ginzinger.com>

This adds support for explicitly switching the mmc's power on and off
which is needed for example for WL1837 WL1271 wifi controllers on imx28.

While the wifi's vmmc-supply regulator can be configured in devicetree,
"ip link set wlan0 down" doesn't turn off the VMMC regulator which leads
to hangs when loading firmware, for example.

Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
---


revision history                                                               
----------------                                                               
v4: re-added forgotten regulator_enable() during probe
v3: improve API usage as suggested by Ulf
v2: tested patch with changes suggested by Robin                               
v1: question, why https://patchwork.kernel.org/patch/4365751/ didn't get in


 drivers/mmc/host/mxs-mmc.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
index add1e70195ea..23d275269d61 100644
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -517,6 +517,22 @@ static void mxs_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	else
 		host->bus_width = 0;
 
+	switch (ios->power_mode) {
+	case MMC_POWER_OFF:
+		if (!IS_ERR(host->mmc->supply.vmmc))
+			mmc_regulator_set_ocr(host->mmc,
+					      host->mmc->supply.vmmc, 0);
+		break;
+	case MMC_POWER_UP:
+		if (!IS_ERR(host->mmc->supply.vmmc))
+			mmc_regulator_set_ocr(host->mmc,
+					      host->mmc->supply.vmmc,
+					      ios->vdd);
+		break;
+	default:
+		break;
+	}
+
 	if (ios->clock)
 		mxs_ssp_set_clk_rate(&host->ssp, ios->clock);
 }
@@ -588,7 +604,6 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 	struct mmc_host *mmc;
 	struct resource *iores;
 	int ret = 0, irq_err;
-	struct regulator *reg_vmmc;
 	struct mxs_ssp *ssp;
 
 	irq_err = platform_get_irq(pdev, 0);
@@ -614,14 +629,15 @@ static int mxs_mmc_probe(struct platform_device *pdev)
 	host->mmc = mmc;
 	host->sdio_irq_en = 0;
 
-	reg_vmmc = devm_regulator_get(&pdev->dev, "vmmc");
-	if (!IS_ERR(reg_vmmc)) {
-		ret = regulator_enable(reg_vmmc);
-		if (ret) {
-			dev_err(&pdev->dev,
-				"Failed to enable vmmc regulator: %d\n", ret);
-			goto out_mmc_free;
-		}
+	ret = mmc_regulator_get_supply(mmc);
+	if (ret == -EPROBE_DEFER)
+		goto out_mmc_free;
+
+	ret = regulator_enable(mmc->supply.vmmc);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"Failed to enable vmmc regulator: %d\n", ret);
+		goto out_mmc_free;
 	}
 
 	ssp->clk = devm_clk_get(&pdev->dev, NULL);
-- 
2.20.1

Re: [PATCH v4] mmc: mxs-mmc: Introduce regulator support

[Ulf Hansson]

On Mon, 28 Jan 2019 at 15:41, Martin Kepplinger <martink@posteo.de> wrote:
>
> From: Martin Kepplinger <martin.kepplinger@ginzinger.com>
>
> This adds support for explicitly switching the mmc's power on and off
> which is needed for example for WL1837 WL1271 wifi controllers on imx28.
>
> While the wifi's vmmc-supply regulator can be configured in devicetree,
> "ip link set wlan0 down" doesn't turn off the VMMC regulator which leads
> to hangs when loading firmware, for example.
>
> Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
> ---
>
>
> revision history
> ----------------
> v4: re-added forgotten regulator_enable() during probe
> v3: improve API usage as suggested by Ulf
> v2: tested patch with changes suggested by Robin
> v1: question, why https://patchwork.kernel.org/patch/4365751/ didn't get in
>
>
>  drivers/mmc/host/mxs-mmc.c | 34 +++++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
> index add1e70195ea..23d275269d61 100644
> --- a/drivers/mmc/host/mxs-mmc.c
> +++ b/drivers/mmc/host/mxs-mmc.c
> @@ -517,6 +517,22 @@ static void mxs_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>         else
>                 host->bus_width = 0;
>
> +       switch (ios->power_mode) {
> +       case MMC_POWER_OFF:
> +               if (!IS_ERR(host->mmc->supply.vmmc))
> +                       mmc_regulator_set_ocr(host->mmc,
> +                                             host->mmc->supply.vmmc, 0);
> +               break;
> +       case MMC_POWER_UP:
> +               if (!IS_ERR(host->mmc->supply.vmmc))
> +                       mmc_regulator_set_ocr(host->mmc,
> +                                             host->mmc->supply.vmmc,
> +                                             ios->vdd);
> +               break;
> +       default:
> +               break;
> +       }
> +
>         if (ios->clock)
>                 mxs_ssp_set_clk_rate(&host->ssp, ios->clock);
>  }
> @@ -588,7 +604,6 @@ static int mxs_mmc_probe(struct platform_device *pdev)
>         struct mmc_host *mmc;
>         struct resource *iores;
>         int ret = 0, irq_err;
> -       struct regulator *reg_vmmc;
>         struct mxs_ssp *ssp;
>
>         irq_err = platform_get_irq(pdev, 0);
> @@ -614,14 +629,15 @@ static int mxs_mmc_probe(struct platform_device *pdev)
>         host->mmc = mmc;
>         host->sdio_irq_en = 0;
>
> -       reg_vmmc = devm_regulator_get(&pdev->dev, "vmmc");
> -       if (!IS_ERR(reg_vmmc)) {
> -               ret = regulator_enable(reg_vmmc);
> -               if (ret) {
> -                       dev_err(&pdev->dev,
> -                               "Failed to enable vmmc regulator: %d\n", ret);
> -                       goto out_mmc_free;
> -               }
> +       ret = mmc_regulator_get_supply(mmc);
> +       if (ret == -EPROBE_DEFER)
> +               goto out_mmc_free;
> +
> +       ret = regulator_enable(mmc->supply.vmmc);

This is wrong, as it may cause the regulator usage count to become
wrongly balanced.

Instead, via ->set_ios() when calling mmc_regulator_set_ocr(), it will
take care of enabling and disabling the regulator depending of the
requested vdd voltage level.

> +       if (ret) {
> +               dev_err(&pdev->dev,
> +                       "Failed to enable vmmc regulator: %d\n", ret);
> +               goto out_mmc_free;
>         }
>
>         ssp->clk = devm_clk_get(&pdev->dev, NULL);
> --
> 2.20.1
>

BTW, you didn't really answer my earlier question about the TI WiFi
chip. Doesn't you need a special clock for WiFi chip as well? How do
you intend to manage that?

Kind regards
Uffe

Re: [PATCH v4] mmc: mxs-mmc: Introduce regulator support

[Robin van der Gracht]

On Mon, 28 Jan 2019 22:15:23 +0100
Ulf Hansson <ulf.hansson@linaro.org> wrote:

> On Mon, 28 Jan 2019 at 15:41, Martin Kepplinger <martink@posteo.de> wrote:
> >
> > From: Martin Kepplinger <martin.kepplinger@ginzinger.com>
> >
> > This adds support for explicitly switching the mmc's power on and off
> > which is needed for example for WL1837 WL1271 wifi controllers on imx28.
> >
> > While the wifi's vmmc-supply regulator can be configured in devicetree,
> > "ip link set wlan0 down" doesn't turn off the VMMC regulator which leads
> > to hangs when loading firmware, for example.
> >
> > Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
> > ---
> >
> >
> > revision history
> > ----------------
> > v4: re-added forgotten regulator_enable() during probe
> > v3: improve API usage as suggested by Ulf
> > v2: tested patch with changes suggested by Robin
> > v1: question, why https://patchwork.kernel.org/patch/4365751/ didn't get in
> >
> >
> >  drivers/mmc/host/mxs-mmc.c | 34 +++++++++++++++++++++++++---------
> >  1 file changed, 25 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
> > index add1e70195ea..23d275269d61 100644
> > --- a/drivers/mmc/host/mxs-mmc.c
> > +++ b/drivers/mmc/host/mxs-mmc.c
> > @@ -517,6 +517,22 @@ static void mxs_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> >         else
> >                 host->bus_width = 0;
> >
> > +       switch (ios->power_mode) {
> > +       case MMC_POWER_OFF:
> > +               if (!IS_ERR(host->mmc->supply.vmmc))
> > +                       mmc_regulator_set_ocr(host->mmc,
> > +                                             host->mmc->supply.vmmc, 0);
> > +               break;
> > +       case MMC_POWER_UP:
> > +               if (!IS_ERR(host->mmc->supply.vmmc))
> > +                       mmc_regulator_set_ocr(host->mmc,
> > +                                             host->mmc->supply.vmmc,
> > +                                             ios->vdd);
> > +               break;
> > +       default:
> > +               break;
> > +       }
> > +
> >         if (ios->clock)
> >                 mxs_ssp_set_clk_rate(&host->ssp, ios->clock);
> >  }
> > @@ -588,7 +604,6 @@ static int mxs_mmc_probe(struct platform_device *pdev)
> >         struct mmc_host *mmc;
> >         struct resource *iores;
> >         int ret = 0, irq_err;
> > -       struct regulator *reg_vmmc;
> >         struct mxs_ssp *ssp;
> >
> >         irq_err = platform_get_irq(pdev, 0);
> > @@ -614,14 +629,15 @@ static int mxs_mmc_probe(struct platform_device *pdev)
> >         host->mmc = mmc;
> >         host->sdio_irq_en = 0;
> >
> > -       reg_vmmc = devm_regulator_get(&pdev->dev, "vmmc");
> > -       if (!IS_ERR(reg_vmmc)) {
> > -               ret = regulator_enable(reg_vmmc);
> > -               if (ret) {
> > -                       dev_err(&pdev->dev,
> > -                               "Failed to enable vmmc regulator: %d\n", ret);
> > -                       goto out_mmc_free;
> > -               }
> > +       ret = mmc_regulator_get_supply(mmc);
> > +       if (ret == -EPROBE_DEFER)
> > +               goto out_mmc_free;
> > +
> > +       ret = regulator_enable(mmc->supply.vmmc);  
> 
> This is wrong, as it may cause the regulator usage count to become
> wrongly balanced.
> 
> Instead, via ->set_ios() when calling mmc_regulator_set_ocr(), it will
> take care of enabling and disabling the regulator depending of the
> requested vdd voltage level.
> 
> > +       if (ret) {
> > +               dev_err(&pdev->dev,
> > +                       "Failed to enable vmmc regulator: %d\n", ret);
> > +               goto out_mmc_free;
> >         }
> >
> >         ssp->clk = devm_clk_get(&pdev->dev, NULL);
> > --
> > 2.20.1
> >  
> 
> BTW, you didn't really answer my earlier question about the TI WiFi
> chip. Doesn't you need a special clock for WiFi chip as well? How do
> you intend to manage that?

I used an external 32K oscillator (SLOW_CLK) for my wl1271. Other
clocks ware generated on the module.

I had to supply a 'vmmc-supply' in your wl1271 devicetree node,
which will be used to power on/off the wlan module. The supply should
be a (delayed) GPIO controlled 'fixed-regulator' attached to the
wlan_en pin on the module.

1: Documentation/devicetree/bindings/net/wireless/ti,wlcore.txt

Kind regards,

-- 
Robin van der Gracht
Protonic Holland
tel.: +31 (0) 229 212928
fax.: +31 (0) 229 210930
Factorij 36 / 1689 AL Zwaag

Re: [PATCH v4] mmc: mxs-mmc: Introduce regulator support

[Ulf Hansson]

On Thu, 31 Jan 2019 at 09:20, Robin van der Gracht <robin@protonic.nl> wrote:
>
> On Mon, 28 Jan 2019 22:15:23 +0100
> Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> > On Mon, 28 Jan 2019 at 15:41, Martin Kepplinger <martink@posteo.de> wrote:
> > >
> > > From: Martin Kepplinger <martin.kepplinger@ginzinger.com>
> > >
> > > This adds support for explicitly switching the mmc's power on and off
> > > which is needed for example for WL1837 WL1271 wifi controllers on imx28.
> > >
> > > While the wifi's vmmc-supply regulator can be configured in devicetree,
> > > "ip link set wlan0 down" doesn't turn off the VMMC regulator which leads
> > > to hangs when loading firmware, for example.
> > >
> > > Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
> > > ---
> > >
> > >
> > > revision history
> > > ----------------
> > > v4: re-added forgotten regulator_enable() during probe
> > > v3: improve API usage as suggested by Ulf
> > > v2: tested patch with changes suggested by Robin
> > > v1: question, why https://patchwork.kernel.org/patch/4365751/ didn't get in
> > >
> > >
> > >  drivers/mmc/host/mxs-mmc.c | 34 +++++++++++++++++++++++++---------
> > >  1 file changed, 25 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
> > > index add1e70195ea..23d275269d61 100644
> > > --- a/drivers/mmc/host/mxs-mmc.c
> > > +++ b/drivers/mmc/host/mxs-mmc.c
> > > @@ -517,6 +517,22 @@ static void mxs_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> > >         else
> > >                 host->bus_width = 0;
> > >
> > > +       switch (ios->power_mode) {
> > > +       case MMC_POWER_OFF:
> > > +               if (!IS_ERR(host->mmc->supply.vmmc))
> > > +                       mmc_regulator_set_ocr(host->mmc,
> > > +                                             host->mmc->supply.vmmc, 0);
> > > +               break;
> > > +       case MMC_POWER_UP:
> > > +               if (!IS_ERR(host->mmc->supply.vmmc))
> > > +                       mmc_regulator_set_ocr(host->mmc,
> > > +                                             host->mmc->supply.vmmc,
> > > +                                             ios->vdd);
> > > +               break;
> > > +       default:
> > > +               break;
> > > +       }
> > > +
> > >         if (ios->clock)
> > >                 mxs_ssp_set_clk_rate(&host->ssp, ios->clock);
> > >  }
> > > @@ -588,7 +604,6 @@ static int mxs_mmc_probe(struct platform_device *pdev)
> > >         struct mmc_host *mmc;
> > >         struct resource *iores;
> > >         int ret = 0, irq_err;
> > > -       struct regulator *reg_vmmc;
> > >         struct mxs_ssp *ssp;
> > >
> > >         irq_err = platform_get_irq(pdev, 0);
> > > @@ -614,14 +629,15 @@ static int mxs_mmc_probe(struct platform_device *pdev)
> > >         host->mmc = mmc;
> > >         host->sdio_irq_en = 0;
> > >
> > > -       reg_vmmc = devm_regulator_get(&pdev->dev, "vmmc");
> > > -       if (!IS_ERR(reg_vmmc)) {
> > > -               ret = regulator_enable(reg_vmmc);
> > > -               if (ret) {
> > > -                       dev_err(&pdev->dev,
> > > -                               "Failed to enable vmmc regulator: %d\n", ret);
> > > -                       goto out_mmc_free;
> > > -               }
> > > +       ret = mmc_regulator_get_supply(mmc);
> > > +       if (ret == -EPROBE_DEFER)
> > > +               goto out_mmc_free;
> > > +
> > > +       ret = regulator_enable(mmc->supply.vmmc);
> >
> > This is wrong, as it may cause the regulator usage count to become
> > wrongly balanced.
> >
> > Instead, via ->set_ios() when calling mmc_regulator_set_ocr(), it will
> > take care of enabling and disabling the regulator depending of the
> > requested vdd voltage level.
> >
> > > +       if (ret) {
> > > +               dev_err(&pdev->dev,
> > > +                       "Failed to enable vmmc regulator: %d\n", ret);
> > > +               goto out_mmc_free;
> > >         }
> > >
> > >         ssp->clk = devm_clk_get(&pdev->dev, NULL);
> > > --
> > > 2.20.1
> > >
> >
> > BTW, you didn't really answer my earlier question about the TI WiFi
> > chip. Doesn't you need a special clock for WiFi chip as well? How do
> > you intend to manage that?
>
> I used an external 32K oscillator (SLOW_CLK) for my wl1271. Other
> clocks ware generated on the module.

Right. How do you control that clock? Did you model it as clock via
the common clock framework?

>
> I had to supply a 'vmmc-supply' in your wl1271 devicetree node,
> which will be used to power on/off the wlan module. The supply should
> be a (delayed) GPIO controlled 'fixed-regulator' attached to the
> wlan_en pin on the module.

Right, thanks for explaining.

>
> 1: Documentation/devicetree/bindings/net/wireless/ti,wlcore.txt
>

This sounds like a good fit for mmc pwrseq simple. There are already
similar users for it.

Have a look at: /drivers/mmc/core/pwrseq*
If the mmc host driver calls mmc_of_parse() during ->probe(), a pwrseq
instance will be hooked up to it. Once the mmc core tries to power up
the card it will make use of the attached pwrseq for the mmc host in
question.

In this way, you can control the clock and GPIO line, in more exact
ways that is needed by the WiFi chip.

Here is a DT example (look for "mmc-pwrseq-simple"):
arch/arm/boot/dts/imx6qdl-sr-som-ti.dtsi

This should do the trick for you. On the other hand, I don't mind that
you still add regulator support to the driver, along the lines of what
$subject patch does, however it may not be exactly what you need for
the WiFi case.

Kind regards
Uffe

Re: [PATCH v4] mmc: mxs-mmc: Introduce regulator support

[Robin van der Gracht]

On Thu, 31 Jan 2019 13:17:23 +0100
Ulf Hansson <ulf.hansson@linaro.org> wrote:

> On Thu, 31 Jan 2019 at 09:20, Robin van der Gracht <robin@protonic.nl> wrote:
> >
> > On Mon, 28 Jan 2019 22:15:23 +0100
> > Ulf Hansson <ulf.hansson@linaro.org> wrote:
> >  
> > > On Mon, 28 Jan 2019 at 15:41, Martin Kepplinger <martink@posteo.de> wrote:  
> > > >
> > > > From: Martin Kepplinger <martin.kepplinger@ginzinger.com>
> > > >
> > > > This adds support for explicitly switching the mmc's power on and off
> > > > which is needed for example for WL1837 WL1271 wifi controllers on imx28.
> > > >
> > > > While the wifi's vmmc-supply regulator can be configured in devicetree,
> > > > "ip link set wlan0 down" doesn't turn off the VMMC regulator which leads
> > > > to hangs when loading firmware, for example.
> > > >
> > > > Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
> > > > ---
> > > >
> > > >
> > > > revision history
> > > > ----------------
> > > > v4: re-added forgotten regulator_enable() during probe
> > > > v3: improve API usage as suggested by Ulf
> > > > v2: tested patch with changes suggested by Robin
> > > > v1: question, why https://patchwork.kernel.org/patch/4365751/ didn't get in
> > > >
> > > >
> > > >  drivers/mmc/host/mxs-mmc.c | 34 +++++++++++++++++++++++++---------
> > > >  1 file changed, 25 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
> > > > index add1e70195ea..23d275269d61 100644
> > > > --- a/drivers/mmc/host/mxs-mmc.c
> > > > +++ b/drivers/mmc/host/mxs-mmc.c
> > > > @@ -517,6 +517,22 @@ static void mxs_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> > > >         else
> > > >                 host->bus_width = 0;
> > > >
> > > > +       switch (ios->power_mode) {
> > > > +       case MMC_POWER_OFF:
> > > > +               if (!IS_ERR(host->mmc->supply.vmmc))
> > > > +                       mmc_regulator_set_ocr(host->mmc,
> > > > +                                             host->mmc->supply.vmmc, 0);
> > > > +               break;
> > > > +       case MMC_POWER_UP:
> > > > +               if (!IS_ERR(host->mmc->supply.vmmc))
> > > > +                       mmc_regulator_set_ocr(host->mmc,
> > > > +                                             host->mmc->supply.vmmc,
> > > > +                                             ios->vdd);
> > > > +               break;
> > > > +       default:
> > > > +               break;
> > > > +       }
> > > > +
> > > >         if (ios->clock)
> > > >                 mxs_ssp_set_clk_rate(&host->ssp, ios->clock);
> > > >  }
> > > > @@ -588,7 +604,6 @@ static int mxs_mmc_probe(struct platform_device *pdev)
> > > >         struct mmc_host *mmc;
> > > >         struct resource *iores;
> > > >         int ret = 0, irq_err;
> > > > -       struct regulator *reg_vmmc;
> > > >         struct mxs_ssp *ssp;
> > > >
> > > >         irq_err = platform_get_irq(pdev, 0);
> > > > @@ -614,14 +629,15 @@ static int mxs_mmc_probe(struct platform_device *pdev)
> > > >         host->mmc = mmc;
> > > >         host->sdio_irq_en = 0;
> > > >
> > > > -       reg_vmmc = devm_regulator_get(&pdev->dev, "vmmc");
> > > > -       if (!IS_ERR(reg_vmmc)) {
> > > > -               ret = regulator_enable(reg_vmmc);
> > > > -               if (ret) {
> > > > -                       dev_err(&pdev->dev,
> > > > -                               "Failed to enable vmmc regulator: %d\n", ret);
> > > > -                       goto out_mmc_free;
> > > > -               }
> > > > +       ret = mmc_regulator_get_supply(mmc);
> > > > +       if (ret == -EPROBE_DEFER)
> > > > +               goto out_mmc_free;
> > > > +
> > > > +       ret = regulator_enable(mmc->supply.vmmc);  
> > >
> > > This is wrong, as it may cause the regulator usage count to become
> > > wrongly balanced.
> > >
> > > Instead, via ->set_ios() when calling mmc_regulator_set_ocr(), it will
> > > take care of enabling and disabling the regulator depending of the
> > > requested vdd voltage level.
> > >  
> > > > +       if (ret) {
> > > > +               dev_err(&pdev->dev,
> > > > +                       "Failed to enable vmmc regulator: %d\n", ret);
> > > > +               goto out_mmc_free;
> > > >         }
> > > >
> > > >         ssp->clk = devm_clk_get(&pdev->dev, NULL);
> > > > --
> > > > 2.20.1
> > > >  
> > >
> > > BTW, you didn't really answer my earlier question about the TI WiFi
> > > chip. Doesn't you need a special clock for WiFi chip as well? How do
> > > you intend to manage that?  
> >
> > I used an external 32K oscillator (SLOW_CLK) for my wl1271. Other
> > clocks ware generated on the module.  
> 
> Right. How do you control that clock? Did you model it as clock via
> the common clock framework?

No I didn't. The slow clock (sleep clock) was always 'on'.

> 
> >
> > I had to supply a 'vmmc-supply' in your wl1271 devicetree node,
> > which will be used to power on/off the wlan module. The supply should
> > be a (delayed) GPIO controlled 'fixed-regulator' attached to the
> > wlan_en pin on the module.  
> 
> Right, thanks for explaining.
> 
> >
> > 1: Documentation/devicetree/bindings/net/wireless/ti,wlcore.txt
> >  
> 
> This sounds like a good fit for mmc pwrseq simple. There are already
> similar users for it.
> 
> Have a look at: /drivers/mmc/core/pwrseq*
> If the mmc host driver calls mmc_of_parse() during ->probe(), a pwrseq
> instance will be hooked up to it. Once the mmc core tries to power up
> the card it will make use of the attached pwrseq for the mmc host in
> question.
> 
> In this way, you can control the clock and GPIO line, in more exact
> ways that is needed by the WiFi chip.

Ack. Makes more sense than using a regulator (even without specifying
'clocks').

> 
> Here is a DT example (look for "mmc-pwrseq-simple"):
> arch/arm/boot/dts/imx6qdl-sr-som-ti.dtsi
> 
> This should do the trick for you. On the other hand, I don't mind that
> you still add regulator support to the driver, along the lines of what
> $subject patch does, however it may not be exactly what you need for
> the WiFi case.

@Martin; What do you think? Will you work this out with Ulf?
Since I can't test this.

Regards,
Robin

Re: [PATCH v4] mmc: mxs-mmc: Introduce regulator support

[Martin Kepplinger]

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

On 31.01.19 14:09, Robin van der Gracht wrote:
> On Thu, 31 Jan 2019 13:17:23 +0100
> Ulf Hansson <ulf.hansson@linaro.org> wrote:
> 
>> On Thu, 31 Jan 2019 at 09:20, Robin van der Gracht <robin@protonic.nl> wrote:
>>>
>>> On Mon, 28 Jan 2019 22:15:23 +0100
>>> Ulf Hansson <ulf.hansson@linaro.org> wrote:

...

>>>>
>>>> BTW, you didn't really answer my earlier question about the TI WiFi
>>>> chip. Doesn't you need a special clock for WiFi chip as well? How do
>>>> you intend to manage that?
>>>
>>> I used an external 32K oscillator (SLOW_CLK) for my wl1271. Other
>>> clocks ware generated on the module.
>>
>> Right. How do you control that clock? Did you model it as clock via
>> the common clock framework?
> 
> No I didn't. The slow clock (sleep clock) was always 'on'.
> 
>>
>>>
>>> I had to supply a 'vmmc-supply' in your wl1271 devicetree node,
>>> which will be used to power on/off the wlan module. The supply should
>>> be a (delayed) GPIO controlled 'fixed-regulator' attached to the
>>> wlan_en pin on the module.
>>
>> Right, thanks for explaining.
>>
>>>
>>> 1: Documentation/devicetree/bindings/net/wireless/ti,wlcore.txt
>>>   
>>
>> This sounds like a good fit for mmc pwrseq simple. There are already
>> similar users for it.
>>
>> Have a look at: /drivers/mmc/core/pwrseq*
>> If the mmc host driver calls mmc_of_parse() during ->probe(), a pwrseq
>> instance will be hooked up to it. Once the mmc core tries to power up
>> the card it will make use of the attached pwrseq for the mmc host in
>> question.
>>
>> In this way, you can control the clock and GPIO line, in more exact
>> ways that is needed by the WiFi chip.
> 
> Ack. Makes more sense than using a regulator (even without specifying
> 'clocks').
> 

Thanks Ulf! Sounds promising.

>>
>> Here is a DT example (look for "mmc-pwrseq-simple"):
>> arch/arm/boot/dts/imx6qdl-sr-som-ti.dtsi
>>
>> This should do the trick for you. On the other hand, I don't mind that
>> you still add regulator support to the driver, along the lines of what
>> $subject patch does, however it may not be exactly what you need for
>> the WiFi case.
> 
> @Martin; What do you think? Will you work this out with Ulf?
> Since I can't test this.

I'll test Ulf's suggestion and go that way if I get it to work. Give me 
a few days though.

Thanks a lot for your help so far Robin,

                                martin

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3616 bytes --]