[PATCH 0/4] soc: xilinx: pm_domains: cleanup and fix PM_INIT_FINALIZE

[Rajan Vaja]

Hi Michal,

> -----Original Message-----
> From: Michael Tretter <m.tretter at pengutronix.de>
> Sent: 19 April 2021 01:03 PM
> To: Rajan Vaja <RAJANV at xilinx.com>
> Cc: linux-arm-kernel at lists.infradead.org; Michal Simek <michals at xilinx.com>; Jolly
> Shah <JOLLYS at xilinx.com>
> Subject: Re: [PATCH 0/4] soc: xilinx: pm_domains: cleanup and fix
> PM_INIT_FINALIZE
> 
> Hi Rajan,
> 
> On Thu, 15 Apr 2021 16:27:58 +0000, Rajan Vaja wrote:
> > Thanks for the patch.
> >
> > > -----Original Message-----
> > > From: Michael Tretter <m.tretter at pengutronix.de>
> > > Sent: 17 March 2021 09:34 PM
> > > To: linux-arm-kernel at lists.infradead.org
> > > Cc: Michal Simek <michals at xilinx.com>; Rajan Vaja <RAJANV at xilinx.com>; Jolly
> > > Shah <JOLLYS at xilinx.com>; m.tretter at pengutronix.de
> > > Subject: [PATCH 0/4] soc: xilinx: pm_domains: cleanup and fix
> PM_INIT_FINALIZE
> > >
> > > Patch 1 of this series fixes the ZynqMP PMU FW power management
> > > initialization, which was done by the wrong driver. PM_INIT_FINALIZE must be
> > > called from the zynqmp_pm_domains driver, which handles power domains,
> instead
> > > of the zynmp_power driver, which is responsible for suspend and shutdown.
> > [Rajan] I am fine with moving to genpd but zynqmp_pm_init_finalize() needs to
> be late call.
> > zynqmp_pm_init_finalize() should be called when Linux has requested all the
> devices through
> > genpd driver. Making it late call will make sure it.
> 
> What is the reason why all devices have to be requested before calling
> zynqmp_pm_init_finalize()?
[Rajan] This is required if device is not to be powered down. If zynqmp_pm_init_finalize()
Is called before requesting, device may go power down. So settings done in probe() may be
lost. In this case, driver needs to do config during runtime resume. However, some driver
may not do all config during runtime resume which may not work.

> 
> I was expecting that calling PM_INIT_FINALIZE only would tell the PMU_FW that
> Linux is using the PM API and the PMU_FW should power down/up PM slaves as
> requested by Linux. It is somewhat surprising that this isn't the case and all
> PM slaves have to be powered up before calling PM_INIT_FINALIZE.
[Rajan] Basically PMUFW will do power up/down as per request. However, device
config may be lost due to this power down. So device driver mat need to take care
during runtime suspend/resume



> 
> What would happen if some driver is built as a module? In that case, the
> module would be loaded and request the pm node only after PM_INIT_FINALIZE
> was
> called. Do we have to avoid/disallow such cases?
[Rajan] We allow that. When driver is init, it needs to setup device which will be fine
As probe do most of settings.

> 
> For USB, I am actually observing a similar situation: If I do not request the
> USB PM slave before I call PM_INIT_FINALIZE, I see communication errors with
> connected USB devices. Could this be related?
[Rajan] Yes, device config might have lost due to power down because of init finalize call.

Thanks,
Rajan

> 
> Thanks,
> 
> Michael
> 
> >
> > >
> > > Patches 2 to 4 are various cleanup patches to improve the readability and
> > > debugging experience of the zynqmp_pm_domains driver.
> > >
> > > Michael
> > >
> > > Michael Tretter (4):
> > >   soc: xilinx: move PM_INIT_FINALIZE to zynqmp_pm_domains driver
> > >   soc: xilinx: cleanup debug and error messages
> > >   soc: xilinx: use a properly named field instead of flags
> > >   soc: xilinx: add a to_zynqmp_pm_domain macro
> > >
> > >  drivers/soc/xilinx/zynqmp_pm_domains.c | 79 +++++++++++++-------------
> > >  drivers/soc/xilinx/zynqmp_power.c      |  1 -
> > >  2 files changed, 38 insertions(+), 42 deletions(-)
> > >
> > > --
> > > 2.29.2