* [PATCH v4] mmc: mxs-mmc: Introduce regulator support @ 2019-01-28 14:41 Martin Kepplinger 2019-01-28 21:15 ` Ulf Hansson 0 siblings, 1 reply; 6+ messages in thread From: Martin Kepplinger @ 2019-01-28 14:41 UTC (permalink / raw) To: linux-mmc, linux-arm-kernel, robin Cc: ulf.hansson, s.hauer, Martin Kepplinger, linux-kernel, linux-imx, shawnguo 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v4] mmc: mxs-mmc: Introduce regulator support 2019-01-28 14:41 [PATCH v4] mmc: mxs-mmc: Introduce regulator support Martin Kepplinger @ 2019-01-28 21:15 ` Ulf Hansson 2019-01-31 8:20 ` Robin van der Gracht 0 siblings, 1 reply; 6+ messages in thread From: Ulf Hansson @ 2019-01-28 21:15 UTC (permalink / raw) To: Martin Kepplinger Cc: Robin van der Gracht, Sascha Hauer, Martin Kepplinger, linux-mmc@vger.kernel.org, Linux Kernel Mailing List, dl-linux-imx, Shawn Guo, Linux ARM 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] mmc: mxs-mmc: Introduce regulator support 2019-01-28 21:15 ` Ulf Hansson @ 2019-01-31 8:20 ` Robin van der Gracht 2019-01-31 12:17 ` Ulf Hansson 0 siblings, 1 reply; 6+ messages in thread From: Robin van der Gracht @ 2019-01-31 8:20 UTC (permalink / raw) To: Ulf Hansson Cc: Sascha Hauer, Martin Kepplinger, linux-mmc@vger.kernel.org, Linux Kernel Mailing List, dl-linux-imx, Martin Kepplinger, Shawn Guo, Linux ARM 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] mmc: mxs-mmc: Introduce regulator support 2019-01-31 8:20 ` Robin van der Gracht @ 2019-01-31 12:17 ` Ulf Hansson 2019-01-31 13:09 ` Robin van der Gracht 0 siblings, 1 reply; 6+ messages in thread From: Ulf Hansson @ 2019-01-31 12:17 UTC (permalink / raw) To: Robin van der Gracht Cc: Sascha Hauer, Martin Kepplinger, linux-mmc@vger.kernel.org, Linux Kernel Mailing List, dl-linux-imx, Martin Kepplinger, Shawn Guo, Linux ARM 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] mmc: mxs-mmc: Introduce regulator support 2019-01-31 12:17 ` Ulf Hansson @ 2019-01-31 13:09 ` Robin van der Gracht 2019-01-31 13:15 ` Martin Kepplinger 0 siblings, 1 reply; 6+ messages in thread From: Robin van der Gracht @ 2019-01-31 13:09 UTC (permalink / raw) To: Ulf Hansson Cc: Sascha Hauer, Martin Kepplinger, linux-mmc@vger.kernel.org, Linux Kernel Mailing List, dl-linux-imx, Martin Kepplinger, Shawn Guo, Linux ARM 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4] mmc: mxs-mmc: Introduce regulator support 2019-01-31 13:09 ` Robin van der Gracht @ 2019-01-31 13:15 ` Martin Kepplinger 0 siblings, 0 replies; 6+ messages in thread From: Martin Kepplinger @ 2019-01-31 13:15 UTC (permalink / raw) To: Robin van der Gracht, Ulf Hansson Cc: Sascha Hauer, linux-mmc@vger.kernel.org, Linux Kernel Mailing List, dl-linux-imx, Martin Kepplinger, Shawn Guo, Linux ARM [-- Attachment #1.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 #1.2: smime.p7s --] [-- Type: application/x-pkcs7-signature, Size: 3616 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-01-31 13:15 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-01-28 14:41 [PATCH v4] mmc: mxs-mmc: Introduce regulator support Martin Kepplinger 2019-01-28 21:15 ` Ulf Hansson 2019-01-31 8:20 ` Robin van der Gracht 2019-01-31 12:17 ` Ulf Hansson 2019-01-31 13:09 ` Robin van der Gracht 2019-01-31 13:15 ` Martin Kepplinger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).