| Message ID | 1452155155-16232-7-git-send-email-wxt@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hello Caesar, On 01/07/2016 05:25 AM, Caesar Wang wrote: > This patch enables support for power-on sequencing of SDIO > peripherals through DT. > I think the subject line and this first paragraph are misleading since the simple power sequence provider already supports power-on sequencing. This patch does not add or enable support but extends the current support to also enable a regulator as a part of the SDIO chip power on sequencing. > In general, it's quite common that wifi modules and other similar > peripherals have several signals in addition to the SDIO interface that > needs wiggling before the module will power on. > > For example: > we need enable wifi module power to via the WL_REG_ON > pin, we need enable it as the regulator if this pin is connected to > the gpio of cpu. > This part confuses me, so does your chip have an actual regulator that needs to be enabled or is just a fake regulator whose gpio property is used not to enable the regulator but to toggle the WL_REG_ON pin of the WiFi chip? > Maybe, someone will say that can pull up/down from dts. > Unfortunately some SoCs can't support pinctrl pull up/down in > internal. > Can you please elaborate on this? AFAIU this limitation is the reason why you went with the regulator approach so I think it deserve a more deep explanation. > Anyway, we can add this patch to supprt the power-on sequencing for s/supprt/support > sdio. > > Signed-off-by: Caesar Wang <wxt@rock-chips.com> > Best regards,
Am Freitag, 8. Januar 2016, 09:22:31 schrieb Javier Martinez Canillas: > > For example: > > we need enable wifi module power to via the WL_REG_ON > > pin, we need enable it as the regulator if this pin is connected to > > the gpio of cpu. > > This part confuses me, so does your chip have an actual regulator that > needs to be enabled or is just a fake regulator whose gpio property is > used not to enable the regulator but to toggle the WL_REG_ON pin of > the WiFi chip? another option would be to use the reset-gpio-handles. rk3288-veyron and I think some Exynos as well use it that way. > > Maybe, someone will say that can pull up/down from dts. > > Unfortunately some SoCs can't support pinctrl pull up/down in > > internal. > > Can you please elaborate on this? AFAIU this limitation is the reason > why you went with the regulator approach so I think it deserve a more > deep explanation. On the rk3036 each pin has an individual unchangable pull direction. So it's either no bias or pulling in the predefined direction (the pin_default bias option). Heiko
Hello Heiko, On 01/08/2016 11:42 PM, Heiko Stuebner wrote: > Am Freitag, 8. Januar 2016, 09:22:31 schrieb Javier Martinez Canillas: >>> For example: >>> we need enable wifi module power to via the WL_REG_ON >>> pin, we need enable it as the regulator if this pin is connected to >>> the gpio of cpu. >> >> This part confuses me, so does your chip have an actual regulator that >> needs to be enabled or is just a fake regulator whose gpio property is >> used not to enable the regulator but to toggle the WL_REG_ON pin of >> the WiFi chip? > > another option would be to use the reset-gpio-handles. rk3288-veyron and I > think some Exynos as well use it that way. > Yes I know, my point was that the reset-gpios property should be used instead of a fake regulator if what's needed is to toggle a chip pin. > >>> Maybe, someone will say that can pull up/down from dts. >>> Unfortunately some SoCs can't support pinctrl pull up/down in >>> internal. >> >> Can you please elaborate on this? AFAIU this limitation is the reason >> why you went with the regulator approach so I think it deserve a more >> deep explanation. > > On the rk3036 each pin has an individual unchangable pull direction. So it's > either no bias or pulling in the predefined direction (the pin_default bias > option). > I think each change has to be justified on its own so I would say that having a regulator enabled as a part of a SDIO chip's power sequencing is something needed for many platforms, and that this provider should be extended to support that (something like commit msg in patch 05/12). And then in the kylin DTS change (patch 08/12), I would explain why a chained regulators approach is used/needed instead of the reset-gpios due any platform limitations. > > Heiko > Best regards,
Hi Javier, ? 2016?01?12? 00:02, Javier Martinez Canillas ??: > Hello Heiko, > > On 01/08/2016 11:42 PM, Heiko Stuebner wrote: >> Am Freitag, 8. Januar 2016, 09:22:31 schrieb Javier Martinez Canillas: >>>> For example: >>>> we need enable wifi module power to via the WL_REG_ON >>>> pin, we need enable it as the regulator if this pin is connected to >>>> the gpio of cpu. >>> This part confuses me, so does your chip have an actual regulator that >>> needs to be enabled or is just a fake regulator whose gpio property is >>> used not to enable the regulator but to toggle the WL_REG_ON pin of >>> the WiFi chip? >> another option would be to use the reset-gpio-handles. rk3288-veyron and I >> think some Exynos as well use it that way. >> > Yes I know, my point was that the reset-gpios property should be used > instead of a fake regulator if what's needed is to toggle a chip pin. > >>>> Maybe, someone will say that can pull up/down from dts. >>>> Unfortunately some SoCs can't support pinctrl pull up/down in >>>> internal. >>> Can you please elaborate on this? AFAIU this limitation is the reason >>> why you went with the regulator approach so I think it deserve a more >>> deep explanation. >> On the rk3036 each pin has an individual unchangable pull direction. So it's >> either no bias or pulling in the predefined direction (the pin_default bias >> option). >> > I think each change has to be justified on its own so I would say that > having a regulator enabled as a part of a SDIO chip's power sequencing > is something needed for many platforms, and that this provider should > be extended to support that (something like commit msg in patch 05/12). > > And then in the kylin DTS change (patch 08/12), I would explain why a > chained regulators approach is used/needed instead of the reset-gpios > due any platform limitations. Okay, I 'm agreed with your points in here. The reset-gpios/pwrsq can meet the demand of some wlan chips trigger condition. No matter whatever is the BT_EN or WL_EN triggers pin. >> Heiko >> > Best regards,
diff --git a/drivers/mmc/core/pwrseq_simple.c b/drivers/mmc/core/pwrseq_simple.c index d10538b..455dd0c 100644 --- a/drivers/mmc/core/pwrseq_simple.c +++ b/drivers/mmc/core/pwrseq_simple.c @@ -14,6 +14,7 @@ #include <linux/err.h> #include <linux/of_gpio.h> #include <linux/gpio/consumer.h> +#include <linux/regulator/consumer.h> #include <linux/mmc/host.h> @@ -24,6 +25,7 @@ struct mmc_pwrseq_simple { bool clk_enabled; struct clk *ext_clk; struct gpio_descs *reset_gpios; + struct regulator *regulator; }; static void mmc_pwrseq_simple_set_gpios_value(struct mmc_pwrseq_simple *pwrseq, @@ -45,6 +47,13 @@ static void mmc_pwrseq_simple_pre_power_on(struct mmc_host *host) struct mmc_pwrseq_simple *pwrseq = container_of(host->pwrseq, struct mmc_pwrseq_simple, pwrseq); + if (!IS_ERR(pwrseq->regulator)) { + dev_dbg(host->parent, "Enabling external regulator\n"); + if (regulator_enable(pwrseq->regulator)) + dev_err(host->parent, + "Failed to enable external regulator\n"); + } + if (!IS_ERR(pwrseq->ext_clk) && !pwrseq->clk_enabled) { clk_prepare_enable(pwrseq->ext_clk); pwrseq->clk_enabled = true; @@ -117,6 +126,13 @@ struct mmc_pwrseq *mmc_pwrseq_simple_alloc(struct mmc_host *host, goto clk_put; } + pwrseq->regulator = devm_regulator_get(dev, "ext-vcc"); + if (IS_ERR(pwrseq->regulator) && + PTR_ERR(pwrseq->regulator) != -EPROBE_DEFER) { + ret = PTR_ERR(pwrseq->regulator); + goto clk_put; + } + pwrseq->pwrseq.ops = &mmc_pwrseq_simple_ops; return &pwrseq->pwrseq;
This patch enables support for power-on sequencing of SDIO peripherals through DT. In general, it's quite common that wifi modules and other similar peripherals have several signals in addition to the SDIO interface that needs wiggling before the module will power on. For example: we need enable wifi module power to via the WL_REG_ON pin, we need enable it as the regulator if this pin is connected to the gpio of cpu. Maybe, someone will say that can pull up/down from dts. Unfortunately some SoCs can't support pinctrl pull up/down in internal. Anyway, we can add this patch to supprt the power-on sequencing for sdio. Signed-off-by: Caesar Wang <wxt@rock-chips.com> --- Changes in v2: - This fix inmmc-power-sequences, as Heiko comment on https://patchwork.kernel.org/patch/7903161/ drivers/mmc/core/pwrseq_simple.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)