Re: [PATCH v3 1/2] PCI: don't rely on of_platform_depopulate() for reused OF-nodes

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

On Tue, Aug 27, 2024 at 02:25:58PM +0200, Bartosz Golaszewski wrote:
> On Tue, Aug 27, 2024 at 10:40 AM Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > + Rob
> >
> > On Fri, Aug 23, 2024 at 11:33:22AM +0200, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > >
> > > of_platform_depopulate() doesn't play nice with reused OF nodes - it
> > > ignores the ones that are not marked explicitly as populated and it may
> > > happen that the PCI device goes away before the platform device in which
> > > case the PCI core clears the OF_POPULATED bit. We need to
> > > unconditionally unregister the platform devices for child nodes when
> > > stopping the PCI device.
> > >
> >
> > It sounds like the fix is in of_platform_depopulate() itself and this patch
> > works around the API issue in PCI driver.
> >
> > Rob, is that correct?
> >
> > - Mani
> 
> of_platform_depopulate() has more issues than just that. For one: it's
> asymmetric to of_platform_populate() as it takes a struct device as
> argument and not a device node. This causes issues for users like TI
> aemif that call of_platform_populate() on nodes without the compatible
> property that are never consumed by any device. AFAIK there's
> currently no way to depopulate them.
> 

Oouch!

> In this particular case I think that the OF_POPULATED bit should not
> be set when the PCI device is created but only when the platform
> device is.
> 
> However I'm afraid to change the semantics of of_platform_depopulate()
> et al for all users so I'm more inclined to have this fix in v6.11 to
> avoid releasing non functional code (pwrctl devices not being removed)
> and then possibly introduce a new variant of of_platform_depopulate()
> that would work slightly differently.
> 

Ok, sounds like a plan. Since Rob is also in favor of this patch, it is good to
get this series merged for 6.11.

Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

-- 
மணிவண்ணன் சதாசிவம்