Re: sdhci_am654 and runtime-pm issues

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Apr 03, 2023 at 12:19:20PM +0200, Ulf Hansson wrote:
> On Sat, 1 Apr 2023 at 15:16, Uwe Kleine-König
> <u.kleine-koenig@xxxxxxxxxxxxxx> wrote:
> >
> > Hello,
> >
> > I looked at the sdhci_am654's probe function and concerning runtime-pm
> > this is at least unconvential:
> >
> > It does
> >
> >         clk_xin = devm_clk_get(dev, "clk_xin");
> >         if (IS_ERR(clk_xin))
> >                 ...
> >
> >         /* Clocks are enabled using pm_runtime */
> >         pm_runtime_enable(dev);
> >         ret = pm_runtime_resume_and_get(dev);
> >         ...
> >
> > I'm not fluent in runtime-pm stuff (so I added Rafael to Cc), but I
> > thought it is to be used the other way around, i.e. put the device in
> > operational state and then runtime-pm cares to suspend the device under
> > some conditions (e.g. CONFIG_PM being enabled).
> >
> > With CONFIG_PM unset the driver is broken for sure, as then
> > pm_runtime_enable() and pm_runtime_resume_and_get() have no effect. So
> > the clk stays off.
> 
> In principle, you are correct. I wouldn't recommend the above pattern
> in general, but it doesn't mean that it can't work.
> 
> Some platforms are selecting "PM" from some of their toplevel Kconfig,
> as they simply can't work without it. That means that the code you
> refer to above, doesn't have to be broken.

If I understand correctly the driver under discussion is supposed to be
used on SoCs in the CONFIG_ARCH_K3 family. This isn't one of those that
enforce PM.

But yes, adding a depends on PM would be an action that improves the
situation. Having said that even on such platforms you can disable
runtime PM, so even a select on PM isn't a complete fix.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Memonry Technology]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux