[PATCH] pmdomain: imx: gpc: set fwnode from parent device

[Ulf Hansson]

+ Pengfei Li

On Tue, 17 Oct 2023 at 12:17, Ulf Hansson <ulf.hansson at linaro.org> wrote:
>
> On Tue, 17 Oct 2023 at 10:00, Emil Kronborg Andersen <emkan at prevas.dk> wrote:
> >
> > Hi Ulf,
> >
> > On Mon, Oct 16, 2023 at 11:53 +0200, Ulf Hansson wrote:
> > > Unfortunately, it's not that easy. It's not required by
> > > pm_genpd_init() (or of_genpd_add_provider_*) to use a struct *device
> > > when registering a genpd provider. In fact, that's also the reason why
> > > pm_genpd_init() needs to initialize its own struct device.
> >
> > Do you see any way to do it properly at pmdomain level instead, or is
> > that not feasible? I agree with Saravana that a centralized solution,
> > akin to what was implemented for tty devices in commit 1a5ecc73b2bf
> > ("serdev: Set fwnode for serdev devices"), would be ideal. However, I
> > was not able to find other reports or patches of problems regarding
> > fw_devlink at pmdomain level, so it might not be a general problem here,
> > and fixes per driver might be okay in this case?
>
> Sure, we could fix it though a centralized solution, but it requires
> us to patch each and every genpd provider driver too. At this point I
> would therefore say, let's just fix it for those that need it.
>
> >
> > > That said, can you please explain the special condition that caused
> > > this thing to be needed in the first place?
> >
> > I noticed this issue when I updated from 5.15.93 to 6.1.37. Except for
> > the commit I bisected, i.e., 3fb16866b51d ("driver core: fw_devlink:
> > Make cycle detection more robust"), I found that the issue was
> > introduced somewhere between 6.1.16 and 6.1.15. Here, some commits
> > regarding fw_devlink was backported, which might be relevant:
> >
> > $ git log --oneline v6.1.15..v6.1.16 -- drivers/base/core.c | grep fw_devlink
> > 11d93294b7c3 driver core: fw_devlink: Avoid spurious error message
> > 3dd596616d10 driver core: fw_devlink: Make cycle detection more robust
> > a3c171751512 driver core: fw_devlink: Improve check for fwnode with no device/driver
> > 7a8ce4d2fbbc driver core: fw_devlink: Consolidate device link flag computation
> > 16aa2487cf15 driver core: fw_devlink: Allow marking a fwnode link as being part of a cycle
> > eaf9b5612a47 driver core: fw_devlink: Don't purge child fwnode's consumer links
> > 2455b81afe68 driver core: fw_devlink: Add DL_FLAG_CYCLE support to device links
>
> Thanks for sharing the commits. I haven't looked much closer to them,
> but maybe Saravana has some ideas about whether this potentially can
> be fixed in the fw_devlink parsing instead?

I had a bit closer look at this. If I understand things correctly,
this problem is because the imx_gpc_driver's ->probe() (imx_gpc_probe)
is allocating/adding a new platform device, per child-of-node to
represent a power-domain.

I suspect in most other cases where a platform device is being
allocated per child node, is when the child node has a
compatible-string and the parent driver is calling
of_platform_populate(), or similar. In those cases, the "dev.fwnode"
would be set accordingly, but not in the imx_gpc_driver case as it
needs to be set explicitly.

That said, it looks like we should move forward with the $subject
patch. However, it looks like the patch [1] from Pengfei Li is more
correct than the $subject patch.

Kind regards
Uffe

[1]
https://lore.kernel.org/all/20231020185949.537083-1-pengfei.li_1@nxp.com/