Messages in this thread Patch in this message | | | Date | Wed, 16 Jun 2021 10:03:14 +0200 | From | Mauro Carvalho Chehab <> | Subject | Re: [PATCH 08/25] media: atmel: properly get pm_runtime |
| |
Em Thu, 10 Jun 2021 12:00:42 +0000 <Eugen.Hristev@microchip.com> escreveu:
> On 6/10/21 12:38 PM, Mauro Carvalho Chehab wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > > > Em Thu, 10 Jun 2021 09:04:07 +0000 > > <Eugen.Hristev@microchip.com> escreveu: > > > >>>> diff --git a/drivers/media/platform/atmel/atmel-isc-base.c b/drivers/media/platform/atmel/atmel-isc-base.c > >>>> index fe3ec8d0eaee..ce8e1351fa53 100644 > >>>> --- a/drivers/media/platform/atmel/atmel-isc-base.c > >>>> +++ b/drivers/media/platform/atmel/atmel-isc-base.c > >>>> @@ -294,9 +294,13 @@ static int isc_wait_clk_stable(struct clk_hw *hw) > >>>> static int isc_clk_prepare(struct clk_hw *hw) > >>>> { > >>>> struct isc_clk *isc_clk = to_isc_clk(hw); > >>>> + int ret; > >>>> > >>>> - if (isc_clk->id == ISC_ISPCK) > >>>> - pm_runtime_get_sync(isc_clk->dev); > >>>> + if (isc_clk->id == ISC_ISPCK) { > >>>> + ret = pm_runtime_resume_and_get(isc_clk->dev); > >>>> + if (ret < 0) > >>>> + return ret; > >>>> + } > >> > >> Hi Mauro, > >> > >> With this patch, the ISC is broken on latest media tree. It looks like > >> pm_runtime_resume_and_get for the ISC_ISPCK clock returns -ENOACCESS and > >> thus, the probe of the driver fails: > >> > >> atmel-sama5d2-isc f0008000.isc: failed to enable ispck: -13 > >> atmel-sama5d2-isc: probe of f0008000.isc failed with error -13 > >> > >> > >> Could you point out how I could fix this ? Maybe the isc_clk->dev is not > >> properly handled/initialized in some other part of the code ? > > > > Looking at RPM implementation: > > > > static int rpm_resume(struct device *dev, int rpmflags) > > { > > ... > > if (dev->power.runtime_error) > > retval = -EINVAL; > > else if (dev->power.disable_depth == 1 && dev->power.is_suspended > > && dev->power.runtime_status == RPM_ACTIVE) > > retval = 1; > > else if (dev->power.disable_depth > 0) > > retval = -EACCES; > > ... > > > > It sounds that RPM is disabled for this clock. Did you call > > pm_runtime_enable() before calling isc_clk_prepare()? > > > > Thanks, > > Mauro > > > > I tried to call pm_runtime_enable for the clk at clk_init time, but > doing that makes the runtime for the ISC fail like this: > > atmel-sama5d2-isc f0008000.isc: Unbalanced pm_runtime_enable!
Making RPM balanced is complex ;-)
Yet, clearly there's something weird at the PM code. I mean, ignoring a -ENOACCESS error like the original code sounds wrong, as it basically means that pm_runtime_get_sync() were doing nothing (except by incrementing a refcount).
Some drivers call clk_prepare()/clk_unprepare() at their .prepare/.unprepare ops. Those functions internally call RPM get logic recursively, to ensure that the RPM state of the parents are also at the right state.
Thanks, Mauro
| |