| Message ID | 20220810022509.43743-1-samuel@sholland.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: sunxi-mmc: Fix clock refcount imbalance during unbind | expand |
Dne sreda, 10. avgust 2022 ob 04:25:09 CEST je Samuel Holland napisal(a): > If the controller is suspended by runtime PM, the clock is already > disabled, so do not try to disable it again during removal. Use > pm_runtime_disable() to flush any pending runtime PM transitions. > > Fixes: 9a8e1e8cc2c0 ("mmc: sunxi: Add runtime_pm support") > Signed-off-by: Samuel Holland <samuel@sholland.org> Did you get any warnings for unbalanced disable calls? Anyway: Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com> Best regards, Jernej
On 8/14/22 3:22 AM, Jernej Škrabec wrote: > Dne sreda, 10. avgust 2022 ob 04:25:09 CEST je Samuel Holland napisal(a): >> If the controller is suspended by runtime PM, the clock is already >> disabled, so do not try to disable it again during removal. Use >> pm_runtime_disable() to flush any pending runtime PM transitions. >> >> Fixes: 9a8e1e8cc2c0 ("mmc: sunxi: Add runtime_pm support") >> Signed-off-by: Samuel Holland <samuel@sholland.org> > > Did you get any warnings for unbalanced disable calls? Yes, I got warnings while I was doing unbind/bind cycles to force the SDIO WiFi driver to reload. Regards, Samuel > Anyway: > Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com> > > Best regards, > Jernej > >
On Wed, 10 Aug 2022 at 04:25, Samuel Holland <samuel@sholland.org> wrote: > > If the controller is suspended by runtime PM, the clock is already > disabled, so do not try to disable it again during removal. Use > pm_runtime_disable() to flush any pending runtime PM transitions. > > Fixes: 9a8e1e8cc2c0 ("mmc: sunxi: Add runtime_pm support") > Signed-off-by: Samuel Holland <samuel@sholland.org> > --- > > drivers/mmc/host/sunxi-mmc.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c > index b16e12e62e72..3db9f32d6a7b 100644 > --- a/drivers/mmc/host/sunxi-mmc.c > +++ b/drivers/mmc/host/sunxi-mmc.c > @@ -1492,9 +1492,11 @@ static int sunxi_mmc_remove(struct platform_device *pdev) > struct sunxi_mmc_host *host = mmc_priv(mmc); > > mmc_remove_host(mmc); > - pm_runtime_force_suspend(&pdev->dev); > - disable_irq(host->irq); > - sunxi_mmc_disable(host); Perhaps a better option than the below, would just be to drop the above calls to disable_irq() and sunxi_mmc_disable(), as that is already managed through the call to pm_runtime_force_suspend(). > + pm_runtime_disable(&pdev->dev); > + if (!pm_runtime_status_suspended(&pdev->dev)) { > + disable_irq(host->irq); > + sunxi_mmc_disable(host); > + } > dma_free_coherent(&pdev->dev, PAGE_SIZE, host->sg_cpu, host->sg_dma); > mmc_free_host(mmc); > Kind regards Uffe
On 8/15/22 05:11, Ulf Hansson wrote: > On Wed, 10 Aug 2022 at 04:25, Samuel Holland <samuel@sholland.org> wrote: >> >> If the controller is suspended by runtime PM, the clock is already >> disabled, so do not try to disable it again during removal. Use >> pm_runtime_disable() to flush any pending runtime PM transitions. >> >> Fixes: 9a8e1e8cc2c0 ("mmc: sunxi: Add runtime_pm support") >> Signed-off-by: Samuel Holland <samuel@sholland.org> >> --- >> >> drivers/mmc/host/sunxi-mmc.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c >> index b16e12e62e72..3db9f32d6a7b 100644 >> --- a/drivers/mmc/host/sunxi-mmc.c >> +++ b/drivers/mmc/host/sunxi-mmc.c >> @@ -1492,9 +1492,11 @@ static int sunxi_mmc_remove(struct platform_device *pdev) >> struct sunxi_mmc_host *host = mmc_priv(mmc); >> >> mmc_remove_host(mmc); >> - pm_runtime_force_suspend(&pdev->dev); >> - disable_irq(host->irq); >> - sunxi_mmc_disable(host); > > Perhaps a better option than the below, would just be to drop the > above calls to disable_irq() and sunxi_mmc_disable(), as that is > already managed through the call to pm_runtime_force_suspend(). I like the idea, but it will not work for !CONFIG_PM builds, which this driver currently supports. Regards, Samuel >> + pm_runtime_disable(&pdev->dev); >> + if (!pm_runtime_status_suspended(&pdev->dev)) { >> + disable_irq(host->irq); >> + sunxi_mmc_disable(host); >> + } >> dma_free_coherent(&pdev->dev, PAGE_SIZE, host->sg_cpu, host->sg_dma); >> mmc_free_host(mmc); >> > > Kind regards > Uffe
On Sun, 1 Jan 2023 at 20:25, Samuel Holland <samuel@sholland.org> wrote: > > On 8/15/22 05:11, Ulf Hansson wrote: > > On Wed, 10 Aug 2022 at 04:25, Samuel Holland <samuel@sholland.org> wrote: > >> > >> If the controller is suspended by runtime PM, the clock is already > >> disabled, so do not try to disable it again during removal. Use > >> pm_runtime_disable() to flush any pending runtime PM transitions. > >> > >> Fixes: 9a8e1e8cc2c0 ("mmc: sunxi: Add runtime_pm support") > >> Signed-off-by: Samuel Holland <samuel@sholland.org> > >> --- > >> > >> drivers/mmc/host/sunxi-mmc.c | 8 +++++--- > >> 1 file changed, 5 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c > >> index b16e12e62e72..3db9f32d6a7b 100644 > >> --- a/drivers/mmc/host/sunxi-mmc.c > >> +++ b/drivers/mmc/host/sunxi-mmc.c > >> @@ -1492,9 +1492,11 @@ static int sunxi_mmc_remove(struct platform_device *pdev) > >> struct sunxi_mmc_host *host = mmc_priv(mmc); > >> > >> mmc_remove_host(mmc); > >> - pm_runtime_force_suspend(&pdev->dev); > >> - disable_irq(host->irq); > >> - sunxi_mmc_disable(host); > > > > Perhaps a better option than the below, would just be to drop the > > above calls to disable_irq() and sunxi_mmc_disable(), as that is > > already managed through the call to pm_runtime_force_suspend(). > > I like the idea, but it will not work for !CONFIG_PM builds, which this > driver currently supports. Good point! I have applied the patch for fixes and added a stable tag to it, thanks! [...] Kind regards Uffe
diff --git a/drivers/mmc/host/sunxi-mmc.c b/drivers/mmc/host/sunxi-mmc.c index b16e12e62e72..3db9f32d6a7b 100644 --- a/drivers/mmc/host/sunxi-mmc.c +++ b/drivers/mmc/host/sunxi-mmc.c @@ -1492,9 +1492,11 @@ static int sunxi_mmc_remove(struct platform_device *pdev) struct sunxi_mmc_host *host = mmc_priv(mmc); mmc_remove_host(mmc); - pm_runtime_force_suspend(&pdev->dev); - disable_irq(host->irq); - sunxi_mmc_disable(host); + pm_runtime_disable(&pdev->dev); + if (!pm_runtime_status_suspended(&pdev->dev)) { + disable_irq(host->irq); + sunxi_mmc_disable(host); + } dma_free_coherent(&pdev->dev, PAGE_SIZE, host->sg_cpu, host->sg_dma); mmc_free_host(mmc);
If the controller is suspended by runtime PM, the clock is already disabled, so do not try to disable it again during removal. Use pm_runtime_disable() to flush any pending runtime PM transitions. Fixes: 9a8e1e8cc2c0 ("mmc: sunxi: Add runtime_pm support") Signed-off-by: Samuel Holland <samuel@sholland.org> --- drivers/mmc/host/sunxi-mmc.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)