| Message ID | 20210118073340.62141-1-tony@atomide.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PATCHv2] drivers: bus: simple-pm-bus: Fix compatibility with simple-bus for auxdata | expand |
On Mon, Jan 18, 2021 at 8:33 AM Tony Lindgren <tony@atomide.com> 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 <tony@atomide.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
On Mon, Jan 18, 2021 at 8:33 AM Tony Lindgren <tony@atomide.com> 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 <tony@atomide.com> > --- > Changes since v1: Updated description, added devicetree list to Cc This looks fine to me for now Acked-by: Arnd Bergmann <arnd@arndb.de> 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 <arnd@kernel.org> [210118 08:30]: > On Mon, Jan 18, 2021 at 8:33 AM Tony Lindgren <tony@atomide.com> 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 <tony@atomide.com> > > --- > > Changes since v1: Updated description, added devicetree list to Cc > > This looks fine to me for now > > Acked-by: Arnd Bergmann <arnd@arndb.de> 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
On Mon, Jan 18, 2021 at 2:41 AM Tony Lindgren <tony@atomide.com> wrote: > > * Arnd Bergmann <arnd@kernel.org> [210118 08:30]: > > On Mon, Jan 18, 2021 at 8:33 AM Tony Lindgren <tony@atomide.com> 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 <tony@atomide.com> > > > --- > > > Changes since v1: Updated description, added devicetree list to Cc > > > > This looks fine to me for now > > > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > 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 <robh+dt@kernel.org> [210119 14:51]: > On Mon, Jan 18, 2021 at 2:41 AM Tony Lindgren <tony@atomide.com> 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
+Linus W On Mon, Jan 18, 2021 at 2:30 AM Arnd Bergmann <arnd@kernel.org> wrote: > > On Mon, Jan 18, 2021 at 8:33 AM Tony Lindgren <tony@atomide.com> 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 <tony@atomide.com> > > --- > > Changes since v1: Updated description, added devicetree list to Cc > > This looks fine to me for now > > Acked-by: Arnd Bergmann <arnd@arndb.de> > > 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
diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c --- a/arch/arm/mach-omap2/pdata-quirks.c +++ b/arch/arm/mach-omap2/pdata-quirks.c @@ -522,6 +522,7 @@ static struct of_dev_auxdata omap_auxdata_lookup[] = { &dra7_ipu1_dsp_iommu_pdata), #endif /* Common auxdata */ + OF_DEV_AUXDATA("simple-pm-bus", 0, NULL, omap_auxdata_lookup), OF_DEV_AUXDATA("ti,sysc", 0, NULL, &ti_sysc_pdata), OF_DEV_AUXDATA("pinctrl-single", 0, NULL, &pcs_pdata), OF_DEV_AUXDATA("ti,omap-prm-inst", 0, NULL, &ti_prm_pdata), diff --git a/drivers/bus/simple-pm-bus.c b/drivers/bus/simple-pm-bus.c --- a/drivers/bus/simple-pm-bus.c +++ b/drivers/bus/simple-pm-bus.c @@ -16,6 +16,7 @@ static int simple_pm_bus_probe(struct platform_device *pdev) { + const struct of_dev_auxdata *lookup = dev_get_platdata(&pdev->dev); struct device_node *np = pdev->dev.of_node; dev_dbg(&pdev->dev, "%s\n", __func__); @@ -23,7 +24,7 @@ static int simple_pm_bus_probe(struct platform_device *pdev) pm_runtime_enable(&pdev->dev); if (np) - of_platform_populate(np, NULL, NULL, &pdev->dev); + of_platform_populate(np, NULL, lookup, &pdev->dev); return 0; }
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 <tony@atomide.com> --- Changes since v1: Updated description, added devicetree list to Cc --- arch/arm/mach-omap2/pdata-quirks.c | 1 + drivers/bus/simple-pm-bus.c | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-)