| Message ID | 1453042744-16196-3-git-send-email-stefan.wahren@i2se.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
This patch looks good as well. On 16-01-17 06:59 AM, Stefan Wahren wrote: > The RPi firmware-based clocks driver can actually disable > unused clocks, so when switching to use it we ended up losing > our MMC clock once all devices were probed. > > This patch adopts the changes from 1e5a0a9a58e2 ("mmc: sdhci-bcm2835: > Actually enable the clock") to sdhci-iproc. > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> Acked-by: Scott Branden <sbranden@broadcom.com> > --- > drivers/mmc/host/sdhci-iproc.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c > index e22060a..55bc348 100644 > --- a/drivers/mmc/host/sdhci-iproc.c > +++ b/drivers/mmc/host/sdhci-iproc.c > @@ -207,6 +207,11 @@ static int sdhci_iproc_probe(struct platform_device *pdev) > ret = PTR_ERR(pltfm_host->clk); > goto err; > } > + ret = clk_prepare_enable(pltfm_host->clk); > + if (ret) { > + dev_err(&pdev->dev, "failed to enable host clk\n"); > + goto err; > + } > > if (iproc_host->data->pdata->quirks & SDHCI_QUIRK_MISSING_CAPS) { > host->caps = iproc_host->data->caps; > @@ -215,10 +220,12 @@ static int sdhci_iproc_probe(struct platform_device *pdev) > > ret = sdhci_add_host(host); > if (ret) > - goto err; > + goto err_clk; > > return 0; > > +err_clk: > + clk_disable_unprepare(pltfm_host->clk); > err: > sdhci_pltfm_free(pdev); > return ret; > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 17 January 2016 at 15:59, Stefan Wahren <stefan.wahren@i2se.com> wrote: > The RPi firmware-based clocks driver can actually disable > unused clocks, so when switching to use it we ended up losing > our MMC clock once all devices were probed. > > This patch adopts the changes from 1e5a0a9a58e2 ("mmc: sdhci-bcm2835: > Actually enable the clock") to sdhci-iproc. > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> > --- > drivers/mmc/host/sdhci-iproc.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c > index e22060a..55bc348 100644 > --- a/drivers/mmc/host/sdhci-iproc.c > +++ b/drivers/mmc/host/sdhci-iproc.c > @@ -207,6 +207,11 @@ static int sdhci_iproc_probe(struct platform_device *pdev) > ret = PTR_ERR(pltfm_host->clk); > goto err; > } > + ret = clk_prepare_enable(pltfm_host->clk); > + if (ret) { > + dev_err(&pdev->dev, "failed to enable host clk\n"); > + goto err; > + } > > if (iproc_host->data->pdata->quirks & SDHCI_QUIRK_MISSING_CAPS) { > host->caps = iproc_host->data->caps; > @@ -215,10 +220,12 @@ static int sdhci_iproc_probe(struct platform_device *pdev) > > ret = sdhci_add_host(host); > if (ret) > - goto err; > + goto err_clk; > > return 0; > > +err_clk: > + clk_disable_unprepare(pltfm_host->clk); > err: > sdhci_pltfm_free(pdev); > return ret; > -- > 1.7.9.5 > There's a missing clk_disable_unprepare() from the ->remove() callback. Otherwise this looks good. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, > Ulf Hansson <ulf.hansson@linaro.org> hat am 27. Januar 2016 um 15:16 > geschrieben: > > > On 17 January 2016 at 15:59, Stefan Wahren <stefan.wahren@i2se.com> wrote: > > The RPi firmware-based clocks driver can actually disable > > unused clocks, so when switching to use it we ended up losing > > our MMC clock once all devices were probed. > > > > This patch adopts the changes from 1e5a0a9a58e2 ("mmc: sdhci-bcm2835: > > Actually enable the clock") to sdhci-iproc. > > > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> > > --- > > drivers/mmc/host/sdhci-iproc.c | 9 ++++++++- > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c > > index e22060a..55bc348 100644 > > --- a/drivers/mmc/host/sdhci-iproc.c > > +++ b/drivers/mmc/host/sdhci-iproc.c > > @@ -207,6 +207,11 @@ static int sdhci_iproc_probe(struct platform_device > > *pdev) > > ret = PTR_ERR(pltfm_host->clk); > > goto err; > > } > > + ret = clk_prepare_enable(pltfm_host->clk); > > + if (ret) { > > + dev_err(&pdev->dev, "failed to enable host clk\n"); > > + goto err; > > + } > > > > if (iproc_host->data->pdata->quirks & SDHCI_QUIRK_MISSING_CAPS) { > > host->caps = iproc_host->data->caps; > > @@ -215,10 +220,12 @@ static int sdhci_iproc_probe(struct platform_device > > *pdev) > > > > ret = sdhci_add_host(host); > > if (ret) > > - goto err; > > + goto err_clk; > > > > return 0; > > > > +err_clk: > > + clk_disable_unprepare(pltfm_host->clk); > > err: > > sdhci_pltfm_free(pdev); > > return ret; > > -- > > 1.7.9.5 > > > > There's a missing clk_disable_unprepare() from the ->remove() > callback. Otherwise this looks good. this shouldn't be necessary, because clk_disable_unprepare() is already called by sdhci_pltfm_unregister(). Regards Stefan > > Kind regards > Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 27 January 2016 at 20:11, Stefan Wahren <stefan.wahren@i2se.com> wrote: > Hi, > >> Ulf Hansson <ulf.hansson@linaro.org> hat am 27. Januar 2016 um 15:16 >> geschrieben: >> >> >> On 17 January 2016 at 15:59, Stefan Wahren <stefan.wahren@i2se.com> wrote: >> > The RPi firmware-based clocks driver can actually disable >> > unused clocks, so when switching to use it we ended up losing >> > our MMC clock once all devices were probed. >> > >> > This patch adopts the changes from 1e5a0a9a58e2 ("mmc: sdhci-bcm2835: >> > Actually enable the clock") to sdhci-iproc. >> > >> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> >> > --- >> > drivers/mmc/host/sdhci-iproc.c | 9 ++++++++- >> > 1 file changed, 8 insertions(+), 1 deletion(-) >> > >> > diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c >> > index e22060a..55bc348 100644 >> > --- a/drivers/mmc/host/sdhci-iproc.c >> > +++ b/drivers/mmc/host/sdhci-iproc.c >> > @@ -207,6 +207,11 @@ static int sdhci_iproc_probe(struct platform_device >> > *pdev) >> > ret = PTR_ERR(pltfm_host->clk); >> > goto err; >> > } >> > + ret = clk_prepare_enable(pltfm_host->clk); >> > + if (ret) { >> > + dev_err(&pdev->dev, "failed to enable host clk\n"); >> > + goto err; >> > + } >> > >> > if (iproc_host->data->pdata->quirks & SDHCI_QUIRK_MISSING_CAPS) { >> > host->caps = iproc_host->data->caps; >> > @@ -215,10 +220,12 @@ static int sdhci_iproc_probe(struct platform_device >> > *pdev) >> > >> > ret = sdhci_add_host(host); >> > if (ret) >> > - goto err; >> > + goto err_clk; >> > >> > return 0; >> > >> > +err_clk: >> > + clk_disable_unprepare(pltfm_host->clk); >> > err: >> > sdhci_pltfm_free(pdev); >> > return ret; >> > -- >> > 1.7.9.5 >> > >> >> There's a missing clk_disable_unprepare() from the ->remove() >> callback. Otherwise this looks good. > > this shouldn't be necessary, because clk_disable_unprepare() is already called > by sdhci_pltfm_unregister(). Huh. That's yet another strange behaviour of the sdhci core. So there's no clk enabling done via the registering API, but disabling is done at unregistering. It's fragile. That also means there is a clk disable unbalance issue at ->remove() for sdhci-iproc, right!? Anyway, the patch is of course correct. Applied for next! Thanks and kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/host/sdhci-iproc.c b/drivers/mmc/host/sdhci-iproc.c index e22060a..55bc348 100644 --- a/drivers/mmc/host/sdhci-iproc.c +++ b/drivers/mmc/host/sdhci-iproc.c @@ -207,6 +207,11 @@ static int sdhci_iproc_probe(struct platform_device *pdev) ret = PTR_ERR(pltfm_host->clk); goto err; } + ret = clk_prepare_enable(pltfm_host->clk); + if (ret) { + dev_err(&pdev->dev, "failed to enable host clk\n"); + goto err; + } if (iproc_host->data->pdata->quirks & SDHCI_QUIRK_MISSING_CAPS) { host->caps = iproc_host->data->caps; @@ -215,10 +220,12 @@ static int sdhci_iproc_probe(struct platform_device *pdev) ret = sdhci_add_host(host); if (ret) - goto err; + goto err_clk; return 0; +err_clk: + clk_disable_unprepare(pltfm_host->clk); err: sdhci_pltfm_free(pdev); return ret;
The RPi firmware-based clocks driver can actually disable unused clocks, so when switching to use it we ended up losing our MMC clock once all devices were probed. This patch adopts the changes from 1e5a0a9a58e2 ("mmc: sdhci-bcm2835: Actually enable the clock") to sdhci-iproc. Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> --- drivers/mmc/host/sdhci-iproc.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)