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

From: Michael Tretter <m.tretter@pengutronix.de>
To: Rajan Vaja <RAJANV@xilinx.com>, Michal Simek <michal.simek@xilinx.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	 kernel@pengutronix.de
Subject: Re: [PATCH 0/4] soc: xilinx: pm_domains: cleanup and fix PM_INIT_FINALIZE
Date: Tue, 1 Jun 2021 10:49:10 +0200	[thread overview]
Message-ID: <20210601084910.GB21748@pengutronix.de> (raw)
In-Reply-To: <93cdc435-448f-233b-5b3f-b4ff1d47f897@xilinx.com>

Hi Rajan,

On Wed, 28 Apr 2021 15:17:25 +0200, Michal Simek wrote:
> On 4/20/21 4:18 PM, Sudeep Holla wrote:
> > On Mon, Apr 19, 2021 at 09:32:39AM +0200, Michael Tretter wrote:
> > 
> > Sorry for chiming in randomly. I always though the way PM_INIT_FINALIZE
> > is designed has issues(e.g. racy). I was involved in discussion with
> > Xilinx when we will designing more generic version of EEMI - SCMI
> > which is now supported in upstream. EEMI was in production already when
> > we started on SCMI 3-4 years back and wanted to get feedback.

Is it possible to use SCMI on the ZynqMP? I guess no, as I couldn't find any
code that would make this possible. Correct?

> > 
> > [...]
> > 
> >>
> >> What is the reason why all devices have to be requested before calling
> >> zynqmp_pm_init_finalize()?
> >>
> > 
> > Yes that is wrong assumption/expectation from the firmware.
> > 
> >> 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.
> >>
> > 
> > Agreed that was my understanding too.
> > 
> >> 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?
> >>
> > 
> > I was told it will work. But it will be always racy if there are multiple
> > channels to talk to firmware.
> > 
> > My argument firmware can turn off all the devices before giving control
> > to OS and no need for that. But there is some boot time optimisation
> > possible I am told which I could well be. But this interface for too
> > racy IMO, just happens to be fine with limited configurations it operates
> > in.
> 
> Rajan: Can you please do deep dive to this in pmufw and try to figured
> it out how to fix this on firmware side?

Did you have time to look into this?

There are 3 more cleanup patches in this series. Are there any objections
against these patches? I think the other patches are still useful by
themselves.

Michael

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel