2022-08-10 02:42:26

by Samuel Holland

[permalink] [raw]
Subject: [PATCH] mmc: sunxi-mmc: Fix clock refcount imbalance during unbind

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 <[email protected]>
---

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);
+ 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);

--
2.35.1


2022-08-14 08:41:39

by Jernej Škrabec

[permalink] [raw]
Subject: Re: [PATCH] mmc: sunxi-mmc: Fix clock refcount imbalance during unbind

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 <[email protected]>

Did you get any warnings for unbalanced disable calls?

Anyway:
Acked-by: Jernej Skrabec <[email protected]>

Best regards,
Jernej


2022-08-14 18:25:13

by Samuel Holland

[permalink] [raw]
Subject: Re: [PATCH] mmc: sunxi-mmc: Fix clock refcount imbalance during unbind

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 <[email protected]>
>
> 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 <[email protected]>
>
> Best regards,
> Jernej
>
>

2022-08-15 10:14:03

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH] mmc: sunxi-mmc: Fix clock refcount imbalance during unbind

On Wed, 10 Aug 2022 at 04:25, Samuel Holland <[email protected]> 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 <[email protected]>
> ---
>
> 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

2023-01-01 19:41:03

by Samuel Holland

[permalink] [raw]
Subject: Re: [PATCH] mmc: sunxi-mmc: Fix clock refcount imbalance during unbind

On 8/15/22 05:11, Ulf Hansson wrote:
> On Wed, 10 Aug 2022 at 04:25, Samuel Holland <[email protected]> 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 <[email protected]>
>> ---
>>
>> 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

2023-01-02 16:38:15

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH] mmc: sunxi-mmc: Fix clock refcount imbalance during unbind

On Sun, 1 Jan 2023 at 20:25, Samuel Holland <[email protected]> wrote:
>
> On 8/15/22 05:11, Ulf Hansson wrote:
> > On Wed, 10 Aug 2022 at 04:25, Samuel Holland <[email protected]> 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 <[email protected]>
> >> ---
> >>
> >> 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