[PATCHv2] drivers: bus: simple-pm-bus: Fix compatibility with simple-bus for auxdata

[PATCHv2] drivers: bus: simple-pm-bus: Fix compatibility with simple-bus for auxdata

[Tony Lindgren]

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(-)

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;
 }
-- 
2.30.0

Re: [PATCHv2] drivers: bus: simple-pm-bus: Fix compatibility with simple-bus for auxdata

[Geert Uytterhoeven]

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

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCHv2] drivers: bus: simple-pm-bus: Fix compatibility with simple-bus for auxdata

[Arnd Bergmann]

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

Re: [PATCHv2] drivers: bus: simple-pm-bus: Fix compatibility with simple-bus for auxdata

[Tony Lindgren]

* 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

Re: [PATCHv2] drivers: bus: simple-pm-bus: Fix compatibility with simple-bus for auxdata

[Rob Herring]

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

Re: [PATCHv2] drivers: bus: simple-pm-bus: Fix compatibility with simple-bus for auxdata

[Tony Lindgren]

* 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

Re: [PATCHv2] drivers: bus: simple-pm-bus: Fix compatibility with simple-bus for auxdata

[Rob Herring]

+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