| Message ID | 20210118073340.62141-1-tony@atomide.com |
|---|---|
| State | Accepted |
| Commit | eda080eabf5b9555e4d574ba035b0cb8aa42f052 |
| Headers | show |
| Series | [PATCHv2] drivers: bus: simple-pm-bus: Fix compatibility with simple-bus for auxdata | expand |
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(-)