Re: [PATCH v13 2/2] rcar-csi2: add Renesas R-Car MIPI CSI-2 receiver driver

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

 



Hi Niklas,

On Sun, Apr 15, 2018 at 10:47:37PM +0200, Niklas Söderlund wrote:
> Hi Sakari,
> 
> Thanks for your feedback.
> 
> On 2018-04-04 23:13:57 +0300, Sakari Ailus wrote:
> 
> [snip]
> 
> > > > +	pm_runtime_enable(&pdev->dev);
> > > 
> > > Is CONFIG_PM mandatory on Renesas SoCs? If not, you end up with the
> > > device uninitialised at probe, and pm_runtime_get_sync will not
> > > initialise it either if CONFIG_PM is not enabled. I guess you could
> > > call your runtime_resume function unconditionally, and mark the device
> > > as active in runtime_pm using pm_runtime_set_active.
> > 
> > There doesn't seem to be any runtime_resume function. Was there supposed
> > to be one?
> 
> No there is not suppose to be one.

Ok.

> 
> > 
> > Assuming runtime PM would actually do something here, you might add
> > pm_runtime_idle() to power the device off after probing.
> > 
> > I guess pm_runtime_set_active() should precede pm_runtime_enable().
> 
> The CSI-2 is in the always on power domain so the calls to 
> pm_runtime_get_sync() and pm_runtime_put() are there in the s_stream() 
> callback to enable and disable the module clock. I'm no expert on PM but 
> in my testing the pm_ calls in this driver seems to be correct.
> 
> 1. In probe I call pm_runtime_enable(). And rudimentary tests shows the 
>    clock is off (but I might miss something) as I wish it to be until 
>    stream on time.
> 2. In s_stream() I call pm_runtime_get_sync() before writing any 
>    register when starting a stream. And likewise I call pm_runtime_put() 
>    when stopping and I no longer need to write to a register.
> 3. In remove() I call pm_runtime_disable().
> 
> Am I missing something obvious here?

Looking at the code again, it seems fine in this respect.

-- 
Sakari Ailus
e-mail: sakari.ailus@xxxxxx



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux