| Message ID | 20230513192352.479627-1-marex@denx.de |
|---|---|
| State | New |
| Headers | show |
| Series | mmc: pwrseq: sd8787: Fix WILC CHIP_EN and RESETN toggling order | expand |
On 13.05.2023 22:23, Marek Vasut wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Chapter "5.3 Power-Up/Down Sequence" of WILC1000 [1] and WILC3000 [2] > states that CHIP_EN must be pulled HIGH first, RESETN second. Fix the > order of these signals in the driver. > > Use the mmc_pwrseq_ops as driver data as the delay between signals is > specific to SDIO card type anyway. > > [1] https://ww1.microchip.com/downloads/aemDocuments/documents/WSG/ProductDocuments/DataSheets/ATWILC1000-MR110XB-IEEE-802.11-b-g-n-Link-Controller-Module-DS70005326E.pdf > [2] https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/IEEE-802.11-b-g-n-Link-Controller-Module-with-Integrated-Bluetooth-5.0-DS70005327B.pdf > > Fixes: b2832b96fcf5 ("mmc: pwrseq: sd8787: add support for wilc1000") > Signed-off-by: Marek Vasut <marex@denx.de> Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com> > --- > Cc: Ajay Singh <ajay.kathat@microchip.com> > Cc: Claudiu Beznea <claudiu.beznea@microchip.com> > Cc: Ulf Hansson <ulf.hansson@linaro.org> > Cc: linux-mmc@vger.kernel.org > --- > drivers/mmc/core/pwrseq_sd8787.c | 34 ++++++++++++++++++++++++-------- > 1 file changed, 26 insertions(+), 8 deletions(-) > > diff --git a/drivers/mmc/core/pwrseq_sd8787.c b/drivers/mmc/core/pwrseq_sd8787.c > index 2e120ad83020f..0c5f5e371e1f8 100644 > --- a/drivers/mmc/core/pwrseq_sd8787.c > +++ b/drivers/mmc/core/pwrseq_sd8787.c > @@ -28,7 +28,6 @@ struct mmc_pwrseq_sd8787 { > struct mmc_pwrseq pwrseq; > struct gpio_desc *reset_gpio; > struct gpio_desc *pwrdn_gpio; > - u32 reset_pwrdwn_delay_ms; > }; > > #define to_pwrseq_sd8787(p) container_of(p, struct mmc_pwrseq_sd8787, pwrseq) > @@ -39,7 +38,7 @@ static void mmc_pwrseq_sd8787_pre_power_on(struct mmc_host *host) > > gpiod_set_value_cansleep(pwrseq->reset_gpio, 1); > > - msleep(pwrseq->reset_pwrdwn_delay_ms); > + msleep(300); > gpiod_set_value_cansleep(pwrseq->pwrdn_gpio, 1); > } > > @@ -51,17 +50,37 @@ static void mmc_pwrseq_sd8787_power_off(struct mmc_host *host) > gpiod_set_value_cansleep(pwrseq->reset_gpio, 0); > } > > +static void mmc_pwrseq_wilc1000_pre_power_on(struct mmc_host *host) > +{ > + struct mmc_pwrseq_sd8787 *pwrseq = to_pwrseq_sd8787(host->pwrseq); > + > + /* The pwrdn_gpio is really CHIP_EN, reset_gpio is RESETN */ > + gpiod_set_value_cansleep(pwrseq->pwrdn_gpio, 1); > + msleep(5); > + gpiod_set_value_cansleep(pwrseq->reset_gpio, 1); > +} > + > +static void mmc_pwrseq_wilc1000_power_off(struct mmc_host *host) > +{ > + struct mmc_pwrseq_sd8787 *pwrseq = to_pwrseq_sd8787(host->pwrseq); > + > + gpiod_set_value_cansleep(pwrseq->reset_gpio, 0); > + gpiod_set_value_cansleep(pwrseq->pwrdn_gpio, 0); > +} > + > static const struct mmc_pwrseq_ops mmc_pwrseq_sd8787_ops = { > .pre_power_on = mmc_pwrseq_sd8787_pre_power_on, > .power_off = mmc_pwrseq_sd8787_power_off, > }; > > -static const u32 sd8787_delay_ms = 300; > -static const u32 wilc1000_delay_ms = 5; > +static const struct mmc_pwrseq_ops mmc_pwrseq_wilc1000_ops = { > + .pre_power_on = mmc_pwrseq_wilc1000_pre_power_on, > + .power_off = mmc_pwrseq_wilc1000_power_off, > +}; > > static const struct of_device_id mmc_pwrseq_sd8787_of_match[] = { > - { .compatible = "mmc-pwrseq-sd8787", .data = &sd8787_delay_ms }, > - { .compatible = "mmc-pwrseq-wilc1000", .data = &wilc1000_delay_ms }, > + { .compatible = "mmc-pwrseq-sd8787", .data = &mmc_pwrseq_sd8787_ops }, > + { .compatible = "mmc-pwrseq-wilc1000", .data = &mmc_pwrseq_wilc1000_ops }, > {/* sentinel */}, > }; > MODULE_DEVICE_TABLE(of, mmc_pwrseq_sd8787_of_match); > @@ -77,7 +96,6 @@ static int mmc_pwrseq_sd8787_probe(struct platform_device *pdev) > return -ENOMEM; > > match = of_match_node(mmc_pwrseq_sd8787_of_match, pdev->dev.of_node); > - pwrseq->reset_pwrdwn_delay_ms = *(u32 *)match->data; > > pwrseq->pwrdn_gpio = devm_gpiod_get(dev, "powerdown", GPIOD_OUT_LOW); > if (IS_ERR(pwrseq->pwrdn_gpio)) > @@ -88,7 +106,7 @@ static int mmc_pwrseq_sd8787_probe(struct platform_device *pdev) > return PTR_ERR(pwrseq->reset_gpio); > > pwrseq->pwrseq.dev = dev; > - pwrseq->pwrseq.ops = &mmc_pwrseq_sd8787_ops; > + pwrseq->pwrseq.ops = match->data; > pwrseq->pwrseq.owner = THIS_MODULE; > platform_set_drvdata(pdev, pwrseq); > > -- > 2.39.2 >
On Sat, 13 May 2023 at 21:24, Marek Vasut <marex@denx.de> wrote: > > Chapter "5.3 Power-Up/Down Sequence" of WILC1000 [1] and WILC3000 [2] > states that CHIP_EN must be pulled HIGH first, RESETN second. Fix the > order of these signals in the driver. > > Use the mmc_pwrseq_ops as driver data as the delay between signals is > specific to SDIO card type anyway. > > [1] https://ww1.microchip.com/downloads/aemDocuments/documents/WSG/ProductDocuments/DataSheets/ATWILC1000-MR110XB-IEEE-802.11-b-g-n-Link-Controller-Module-DS70005326E.pdf > [2] https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/IEEE-802.11-b-g-n-Link-Controller-Module-with-Integrated-Bluetooth-5.0-DS70005327B.pdf > > Fixes: b2832b96fcf5 ("mmc: pwrseq: sd8787: add support for wilc1000") > Signed-off-by: Marek Vasut <marex@denx.de> Applied for fixes and by adding a stable tag, thanks! Kind regards Uffe > --- > Cc: Ajay Singh <ajay.kathat@microchip.com> > Cc: Claudiu Beznea <claudiu.beznea@microchip.com> > Cc: Ulf Hansson <ulf.hansson@linaro.org> > Cc: linux-mmc@vger.kernel.org > --- > drivers/mmc/core/pwrseq_sd8787.c | 34 ++++++++++++++++++++++++-------- > 1 file changed, 26 insertions(+), 8 deletions(-) > > diff --git a/drivers/mmc/core/pwrseq_sd8787.c b/drivers/mmc/core/pwrseq_sd8787.c > index 2e120ad83020f..0c5f5e371e1f8 100644 > --- a/drivers/mmc/core/pwrseq_sd8787.c > +++ b/drivers/mmc/core/pwrseq_sd8787.c > @@ -28,7 +28,6 @@ struct mmc_pwrseq_sd8787 { > struct mmc_pwrseq pwrseq; > struct gpio_desc *reset_gpio; > struct gpio_desc *pwrdn_gpio; > - u32 reset_pwrdwn_delay_ms; > }; > > #define to_pwrseq_sd8787(p) container_of(p, struct mmc_pwrseq_sd8787, pwrseq) > @@ -39,7 +38,7 @@ static void mmc_pwrseq_sd8787_pre_power_on(struct mmc_host *host) > > gpiod_set_value_cansleep(pwrseq->reset_gpio, 1); > > - msleep(pwrseq->reset_pwrdwn_delay_ms); > + msleep(300); > gpiod_set_value_cansleep(pwrseq->pwrdn_gpio, 1); > } > > @@ -51,17 +50,37 @@ static void mmc_pwrseq_sd8787_power_off(struct mmc_host *host) > gpiod_set_value_cansleep(pwrseq->reset_gpio, 0); > } > > +static void mmc_pwrseq_wilc1000_pre_power_on(struct mmc_host *host) > +{ > + struct mmc_pwrseq_sd8787 *pwrseq = to_pwrseq_sd8787(host->pwrseq); > + > + /* The pwrdn_gpio is really CHIP_EN, reset_gpio is RESETN */ > + gpiod_set_value_cansleep(pwrseq->pwrdn_gpio, 1); > + msleep(5); > + gpiod_set_value_cansleep(pwrseq->reset_gpio, 1); > +} > + > +static void mmc_pwrseq_wilc1000_power_off(struct mmc_host *host) > +{ > + struct mmc_pwrseq_sd8787 *pwrseq = to_pwrseq_sd8787(host->pwrseq); > + > + gpiod_set_value_cansleep(pwrseq->reset_gpio, 0); > + gpiod_set_value_cansleep(pwrseq->pwrdn_gpio, 0); > +} > + > static const struct mmc_pwrseq_ops mmc_pwrseq_sd8787_ops = { > .pre_power_on = mmc_pwrseq_sd8787_pre_power_on, > .power_off = mmc_pwrseq_sd8787_power_off, > }; > > -static const u32 sd8787_delay_ms = 300; > -static const u32 wilc1000_delay_ms = 5; > +static const struct mmc_pwrseq_ops mmc_pwrseq_wilc1000_ops = { > + .pre_power_on = mmc_pwrseq_wilc1000_pre_power_on, > + .power_off = mmc_pwrseq_wilc1000_power_off, > +}; > > static const struct of_device_id mmc_pwrseq_sd8787_of_match[] = { > - { .compatible = "mmc-pwrseq-sd8787", .data = &sd8787_delay_ms }, > - { .compatible = "mmc-pwrseq-wilc1000", .data = &wilc1000_delay_ms }, > + { .compatible = "mmc-pwrseq-sd8787", .data = &mmc_pwrseq_sd8787_ops }, > + { .compatible = "mmc-pwrseq-wilc1000", .data = &mmc_pwrseq_wilc1000_ops }, > {/* sentinel */}, > }; > MODULE_DEVICE_TABLE(of, mmc_pwrseq_sd8787_of_match); > @@ -77,7 +96,6 @@ static int mmc_pwrseq_sd8787_probe(struct platform_device *pdev) > return -ENOMEM; > > match = of_match_node(mmc_pwrseq_sd8787_of_match, pdev->dev.of_node); > - pwrseq->reset_pwrdwn_delay_ms = *(u32 *)match->data; > > pwrseq->pwrdn_gpio = devm_gpiod_get(dev, "powerdown", GPIOD_OUT_LOW); > if (IS_ERR(pwrseq->pwrdn_gpio)) > @@ -88,7 +106,7 @@ static int mmc_pwrseq_sd8787_probe(struct platform_device *pdev) > return PTR_ERR(pwrseq->reset_gpio); > > pwrseq->pwrseq.dev = dev; > - pwrseq->pwrseq.ops = &mmc_pwrseq_sd8787_ops; > + pwrseq->pwrseq.ops = match->data; > pwrseq->pwrseq.owner = THIS_MODULE; > platform_set_drvdata(pdev, pwrseq); > > -- > 2.39.2 >
diff --git a/drivers/mmc/core/pwrseq_sd8787.c b/drivers/mmc/core/pwrseq_sd8787.c index 2e120ad83020f..0c5f5e371e1f8 100644 --- a/drivers/mmc/core/pwrseq_sd8787.c +++ b/drivers/mmc/core/pwrseq_sd8787.c @@ -28,7 +28,6 @@ struct mmc_pwrseq_sd8787 { struct mmc_pwrseq pwrseq; struct gpio_desc *reset_gpio; struct gpio_desc *pwrdn_gpio; - u32 reset_pwrdwn_delay_ms; }; #define to_pwrseq_sd8787(p) container_of(p, struct mmc_pwrseq_sd8787, pwrseq) @@ -39,7 +38,7 @@ static void mmc_pwrseq_sd8787_pre_power_on(struct mmc_host *host) gpiod_set_value_cansleep(pwrseq->reset_gpio, 1); - msleep(pwrseq->reset_pwrdwn_delay_ms); + msleep(300); gpiod_set_value_cansleep(pwrseq->pwrdn_gpio, 1); } @@ -51,17 +50,37 @@ static void mmc_pwrseq_sd8787_power_off(struct mmc_host *host) gpiod_set_value_cansleep(pwrseq->reset_gpio, 0); } +static void mmc_pwrseq_wilc1000_pre_power_on(struct mmc_host *host) +{ + struct mmc_pwrseq_sd8787 *pwrseq = to_pwrseq_sd8787(host->pwrseq); + + /* The pwrdn_gpio is really CHIP_EN, reset_gpio is RESETN */ + gpiod_set_value_cansleep(pwrseq->pwrdn_gpio, 1); + msleep(5); + gpiod_set_value_cansleep(pwrseq->reset_gpio, 1); +} + +static void mmc_pwrseq_wilc1000_power_off(struct mmc_host *host) +{ + struct mmc_pwrseq_sd8787 *pwrseq = to_pwrseq_sd8787(host->pwrseq); + + gpiod_set_value_cansleep(pwrseq->reset_gpio, 0); + gpiod_set_value_cansleep(pwrseq->pwrdn_gpio, 0); +} + static const struct mmc_pwrseq_ops mmc_pwrseq_sd8787_ops = { .pre_power_on = mmc_pwrseq_sd8787_pre_power_on, .power_off = mmc_pwrseq_sd8787_power_off, }; -static const u32 sd8787_delay_ms = 300; -static const u32 wilc1000_delay_ms = 5; +static const struct mmc_pwrseq_ops mmc_pwrseq_wilc1000_ops = { + .pre_power_on = mmc_pwrseq_wilc1000_pre_power_on, + .power_off = mmc_pwrseq_wilc1000_power_off, +}; static const struct of_device_id mmc_pwrseq_sd8787_of_match[] = { - { .compatible = "mmc-pwrseq-sd8787", .data = &sd8787_delay_ms }, - { .compatible = "mmc-pwrseq-wilc1000", .data = &wilc1000_delay_ms }, + { .compatible = "mmc-pwrseq-sd8787", .data = &mmc_pwrseq_sd8787_ops }, + { .compatible = "mmc-pwrseq-wilc1000", .data = &mmc_pwrseq_wilc1000_ops }, {/* sentinel */}, }; MODULE_DEVICE_TABLE(of, mmc_pwrseq_sd8787_of_match); @@ -77,7 +96,6 @@ static int mmc_pwrseq_sd8787_probe(struct platform_device *pdev) return -ENOMEM; match = of_match_node(mmc_pwrseq_sd8787_of_match, pdev->dev.of_node); - pwrseq->reset_pwrdwn_delay_ms = *(u32 *)match->data; pwrseq->pwrdn_gpio = devm_gpiod_get(dev, "powerdown", GPIOD_OUT_LOW); if (IS_ERR(pwrseq->pwrdn_gpio)) @@ -88,7 +106,7 @@ static int mmc_pwrseq_sd8787_probe(struct platform_device *pdev) return PTR_ERR(pwrseq->reset_gpio); pwrseq->pwrseq.dev = dev; - pwrseq->pwrseq.ops = &mmc_pwrseq_sd8787_ops; + pwrseq->pwrseq.ops = match->data; pwrseq->pwrseq.owner = THIS_MODULE; platform_set_drvdata(pdev, pwrseq);
Chapter "5.3 Power-Up/Down Sequence" of WILC1000 [1] and WILC3000 [2] states that CHIP_EN must be pulled HIGH first, RESETN second. Fix the order of these signals in the driver. Use the mmc_pwrseq_ops as driver data as the delay between signals is specific to SDIO card type anyway. [1] https://ww1.microchip.com/downloads/aemDocuments/documents/WSG/ProductDocuments/DataSheets/ATWILC1000-MR110XB-IEEE-802.11-b-g-n-Link-Controller-Module-DS70005326E.pdf [2] https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/IEEE-802.11-b-g-n-Link-Controller-Module-with-Integrated-Bluetooth-5.0-DS70005327B.pdf Fixes: b2832b96fcf5 ("mmc: pwrseq: sd8787: add support for wilc1000") Signed-off-by: Marek Vasut <marex@denx.de> --- Cc: Ajay Singh <ajay.kathat@microchip.com> Cc: Claudiu Beznea <claudiu.beznea@microchip.com> Cc: Ulf Hansson <ulf.hansson@linaro.org> Cc: linux-mmc@vger.kernel.org --- drivers/mmc/core/pwrseq_sd8787.c | 34 ++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-)