On Mon, Jan 18, 2021 at 8:33 AM Tony Lindgren <[email protected]> wrote:
>
> After converting am335x to probe devices with simple-pm-bus I noticed
> that we are not passing auxdata for of_platform_populate() like we do
> with simple-bus.
>
> While device tree using SoCs should no longer need platform data, there
> are still quite a few drivers that still need it as can be seen with
> git grep OF_DEV_AUXDATA. We want to have simple-pm-bus be usable as a
> replacement for simple-bus also for cases where OF_DEV_AUXDATA is still
> needed.
>
> Let's fix the issue by passing auxdata as platform data to simple-pm-bus.
> That way the SoCs needing this can pass the auxdata with OF_DEV_AUXDATA.
> And let's pass the auxdata for omaps to fix the issue for am335x.
>
> As an alternative solution, adding simple-pm-bus handling directly to
> drivers/of/platform.c was considered, but we would still need simple-pm-bus
> device driver. So passing auxdata as platform data seems like the simplest
> solution.
>
> Fixes: 5a230524f879 ("ARM: dts: Use simple-pm-bus for genpd for am3 l4_wkup")
> Signed-off-by: Tony Lindgren <[email protected]>
> ---
> Changes since v1: Updated description, added devicetree list to Cc
This looks fine to me for now
Acked-by: Arnd Bergmann <[email protected]>
But I think we should take the time to discuss how to phase out auxdata
over time. There are still a number of users, but it's not that many in the
end. For some of them I see a clear solution, for other ones I do not:
lpc32xx: Used only for pl080 DMA data with the old method, needs to
be converted to use the proper DT binding that was added a few years
ago.
kirkwood: I don't see what this does at all, as there is no pdata, and
there is no clkdev lookup for "mvebu-audio"
orion: similar to kirkwood, these seem to have been added for
clkdev lookup, but the orion_clkdev_init() function seems to
not be called for the orion5x_dt variant.
omap2: I'll leave these for Tony to comment
spear3xx: pl022 and pl080 should just use the normal DT
binding, see lpc32xx.
u300: platform is scheduled for removal
integrator_ap: pl010_set_mctrl() needs a callback to
integrator_uart_set_mctrl(). I see no good alternative, but
a workaround might be to call into syscon directly from the
driver on versatile machines. For all I can tell, pl010 is only
used on versatile and ep93xx, so that would not harm a
commonly used driver.
versatile/integrator_cp: similar problem but for mmci, which is
used more widely. Used for card detection, which could
theoretically be implemented with a fake gpio driver, but that
might be excessive.
mips/pic32: used for setting up DMA for sdhci, could be done
in a platform-specific sdhci front-end.
arm-cci: used to pass cci address after ioremap(), avoiding
this would revert e9c112c94b01 ("perf/arm-cci: Untangle
global cci_ctrl_base").
Arnd
* Arnd Bergmann <[email protected]> [210118 08:30]:
> On Mon, Jan 18, 2021 at 8:33 AM Tony Lindgren <[email protected]> wrote:
> >
> > After converting am335x to probe devices with simple-pm-bus I noticed
> > that we are not passing auxdata for of_platform_populate() like we do
> > with simple-bus.
> >
> > While device tree using SoCs should no longer need platform data, there
> > are still quite a few drivers that still need it as can be seen with
> > git grep OF_DEV_AUXDATA. We want to have simple-pm-bus be usable as a
> > replacement for simple-bus also for cases where OF_DEV_AUXDATA is still
> > needed.
> >
> > Let's fix the issue by passing auxdata as platform data to simple-pm-bus.
> > That way the SoCs needing this can pass the auxdata with OF_DEV_AUXDATA.
> > And let's pass the auxdata for omaps to fix the issue for am335x.
> >
> > As an alternative solution, adding simple-pm-bus handling directly to
> > drivers/of/platform.c was considered, but we would still need simple-pm-bus
> > device driver. So passing auxdata as platform data seems like the simplest
> > solution.
> >
> > Fixes: 5a230524f879 ("ARM: dts: Use simple-pm-bus for genpd for am3 l4_wkup")
> > Signed-off-by: Tony Lindgren <[email protected]>
> > ---
> > Changes since v1: Updated description, added devicetree list to Cc
>
> This looks fine to me for now
>
> Acked-by: Arnd Bergmann <[email protected]>
Thanks for the review.
> But I think we should take the time to discuss how to phase out auxdata
> over time. There are still a number of users, but it's not that many in the
> end. For some of them I see a clear solution, for other ones I do not:
Yes agreed we should remove the auxdata use.
> omap2: I'll leave these for Tony to comment
The three hardest ones to update (because of PM dependencies):
- PRM power managment interrupts that also pinctrl driver uses
- The enable/disable of clockdomain autoidle that at least ti-sysc uses
- Smartreflex PM dependencies to voltage controller
For the ones above, I'll try to come up with something eventually.
The others should be just straight forward driver updates needed.
The hsmmc dependencies would be ideally fixed by moving to use sdhci
driver, but at least custom voltage handling and sdio support needs
work.
Regards,
Tony
+Linus W
On Mon, Jan 18, 2021 at 2:30 AM Arnd Bergmann <[email protected]> wrote:
>
> On Mon, Jan 18, 2021 at 8:33 AM Tony Lindgren <[email protected]> wrote:
> >
> > After converting am335x to probe devices with simple-pm-bus I noticed
> > that we are not passing auxdata for of_platform_populate() like we do
> > with simple-bus.
> >
> > While device tree using SoCs should no longer need platform data, there
> > are still quite a few drivers that still need it as can be seen with
> > git grep OF_DEV_AUXDATA. We want to have simple-pm-bus be usable as a
> > replacement for simple-bus also for cases where OF_DEV_AUXDATA is still
> > needed.
> >
> > Let's fix the issue by passing auxdata as platform data to simple-pm-bus.
> > That way the SoCs needing this can pass the auxdata with OF_DEV_AUXDATA.
> > And let's pass the auxdata for omaps to fix the issue for am335x.
> >
> > As an alternative solution, adding simple-pm-bus handling directly to
> > drivers/of/platform.c was considered, but we would still need simple-pm-bus
> > device driver. So passing auxdata as platform data seems like the simplest
> > solution.
> >
> > Fixes: 5a230524f879 ("ARM: dts: Use simple-pm-bus for genpd for am3 l4_wkup")
> > Signed-off-by: Tony Lindgren <[email protected]>
> > ---
> > Changes since v1: Updated description, added devicetree list to Cc
>
> This looks fine to me for now
>
> Acked-by: Arnd Bergmann <[email protected]>
>
> But I think we should take the time to discuss how to phase out auxdata
> over time. There are still a number of users, but it's not that many in the
> end. For some of them I see a clear solution, for other ones I do not:
Thanks for summarizing.
> lpc32xx: Used only for pl080 DMA data with the old method, needs to
> be converted to use the proper DT binding that was added a few years
> ago.
>
> kirkwood: I don't see what this does at all, as there is no pdata, and
> there is no clkdev lookup for "mvebu-audio"
Probably nothing. I reached that conclusion on u300 too. Clocks got
added in DT and someone forgot to remove auxdata. Granted, it's pretty
non-obvious what the purpose is if there is no platform_data.
>
> orion: similar to kirkwood, these seem to have been added for
> clkdev lookup, but the orion_clkdev_init() function seems to
> not be called for the orion5x_dt variant.
>
> omap2: I'll leave these for Tony to comment
>
> spear3xx: pl022 and pl080 should just use the normal DT
> binding, see lpc32xx.
>
> u300: platform is scheduled for removal
>
> integrator_ap: pl010_set_mctrl() needs a callback to
> integrator_uart_set_mctrl(). I see no good alternative, but
> a workaround might be to call into syscon directly from the
> driver on versatile machines. For all I can tell, pl010 is only
> used on versatile and ep93xx, so that would not harm a
> commonly used driver.
That was my conclusion.
> versatile/integrator_cp: similar problem but for mmci, which is
> used more widely. Used for card detection, which could
> theoretically be implemented with a fake gpio driver, but that
> might be excessive.
>
> mips/pic32: used for setting up DMA for sdhci, could be done
> in a platform-specific sdhci front-end.
>
> arm-cci: used to pass cci address after ioremap(), avoiding
> this would revert e9c112c94b01 ("perf/arm-cci: Untangle
> global cci_ctrl_base").
Create a regmap and then secondary drivers needing register access can
lookup the regmap? Or just ioremap it twice... I'll take a closer look
at this one.
Rob
On Mon, Jan 18, 2021 at 2:41 AM Tony Lindgren <[email protected]> wrote:
>
> * Arnd Bergmann <[email protected]> [210118 08:30]:
> > On Mon, Jan 18, 2021 at 8:33 AM Tony Lindgren <[email protected]> wrote:
> > >
> > > After converting am335x to probe devices with simple-pm-bus I noticed
> > > that we are not passing auxdata for of_platform_populate() like we do
> > > with simple-bus.
> > >
> > > While device tree using SoCs should no longer need platform data, there
> > > are still quite a few drivers that still need it as can be seen with
> > > git grep OF_DEV_AUXDATA. We want to have simple-pm-bus be usable as a
> > > replacement for simple-bus also for cases where OF_DEV_AUXDATA is still
> > > needed.
> > >
> > > Let's fix the issue by passing auxdata as platform data to simple-pm-bus.
> > > That way the SoCs needing this can pass the auxdata with OF_DEV_AUXDATA.
> > > And let's pass the auxdata for omaps to fix the issue for am335x.
> > >
> > > As an alternative solution, adding simple-pm-bus handling directly to
> > > drivers/of/platform.c was considered, but we would still need simple-pm-bus
> > > device driver. So passing auxdata as platform data seems like the simplest
> > > solution.
> > >
> > > Fixes: 5a230524f879 ("ARM: dts: Use simple-pm-bus for genpd for am3 l4_wkup")
> > > Signed-off-by: Tony Lindgren <[email protected]>
> > > ---
> > > Changes since v1: Updated description, added devicetree list to Cc
> >
> > This looks fine to me for now
> >
> > Acked-by: Arnd Bergmann <[email protected]>
>
> Thanks for the review.
>
> > But I think we should take the time to discuss how to phase out auxdata
> > over time. There are still a number of users, but it's not that many in the
> > end. For some of them I see a clear solution, for other ones I do not:
>
> Yes agreed we should remove the auxdata use.
>
> > omap2: I'll leave these for Tony to comment
>
> The three hardest ones to update (because of PM dependencies):
>
> - PRM power managment interrupts that also pinctrl driver uses
I haven't looked at it, but can't one driver go find the other node
and the interrupts it needs? There's nothing wrong with a driver
looking outside 'its node' for information.
Rob
* Rob Herring <[email protected]> [210119 14:51]:
> On Mon, Jan 18, 2021 at 2:41 AM Tony Lindgren <[email protected]> wrote:
> > - PRM power managment interrupts that also pinctrl driver uses
>
> I haven't looked at it, but can't one driver go find the other node
> and the interrupts it needs? There's nothing wrong with a driver
> looking outside 'its node' for information.
Yes sure once there are interrupt nodes for it :) It should eventually
be a chained irqchip or something like that. FYI, the current stuff is
the code in mach-omap2/prm_common.c.
Regards,
Tony