Re: [PATCH 1/7] serial: core: Add support of runtime PM

[Andy Shevchenko <andriy.shevchenko@intel.com>]

First of all, my apology for the long delay in answering / commenting on this.
Second, Cc'ed to Ilpo.

On Tue, Jan 18, 2022 at 11:41:03AM +0200, Tony Lindgren wrote:
> * Andy Shevchenko <andriy.shevchenko@intel.com> [211209 10:37]:
> > On Thu, Dec 09, 2021 at 09:29:44AM +0200, Tony Lindgren wrote:
> > > * Johan Hovold <johan@kernel.org> [211130 10:29]:
> > > > On Mon, Nov 15, 2021 at 10:41:57AM +0200, Tony Lindgren wrote:
> > > > > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > > > 
> > > > > 8250 driver has wrong implementation of runtime power management, i.e.
> > > > > it uses an irq_safe flag. The irq_safe flag takes a permanent usage count
> > > > > on the parent device preventing the parent from idling. This patch
> > > > > prepares for making runtime power management generic by adding runtime PM
> > > > > calls to serial core once for all UART drivers.
> > > > > 
> > > > > As we have serial drivers that do not enable runtime PM, and drivers that
> > > > > enable runtime PM, we add new functions for serial_pm_resume_and_get() and
> > > > > serial_pm_autosuspend() functions to handle errors and allow the use also
> > > > > for cases when runtime PM is not enabled. The other option considered was
> > > > > to not check for runtime PM enable errors. But some CPUs can hang when the
> > > > > clocks are not enabled for the device, so ignoring the errors is not a good
> > > > > option. Eventually with the serial port drivers updated, we should be able
> > > > > to just switch to using the standard runtime PM calls with no need for the
> > > > > wrapper functions.
> > > > 
> > > > A third option which needs to be considered is to always enable runtime
> > > > pm in core but to keep the ports active while they are opened unless a
> > > > driver opts in for more aggressive power management. This is how USB
> > > > devices are handled for example.
> > > > 
> > > > A next step could then be to move over uart_change_pm() to be handled
> > > > from the pm callbacks.
> > > 
> > > Yes that would be nice to do eventually :)
> 
> I think we should do the "third option" above as the first preparatory patch.
> It can be done separately from the rest of the series, and we avoid adding
> serial layer specific wrappers around runtime PM calls in the later patches.
> 
> Below is what I came up with for the preparatory patch, can you guys please
> take a look and see if you have better ideas?
> 
> For system suspend and resume, it seems we don't need to do anything as
> runtime PM is anyways disabled already in prepare.
> 
> Andy, care to give the following also a try for your serial port test
> cases?

I'm a bit off of the UART work nowadays, but luckily we have Ilpo who is
pretty much ramped up on the topic. Please, include him in all your work
WRT 8250 UART improvements. I hope Ilpo might have time to test the patch
mentioned in this message or give an idea how to do better, if possibly.

-- 
With Best Regards,
Andy Shevchenko