Re: [PATCH v3/resend 3/4] drivers: bus: Add Simple Power-Managed Bus DT Bindings

[Arnd Bergmann <arnd@arndb.de>]

On Friday 23 January 2015 09:56:51 Geert Uytterhoeven wrote:
> >> diff --git a/Documentation/devicetree/bindings/bus/simple-pm-bus.txt b/Documentation/devicetree/bindings/bus/simple-pm-bus.txt
> >> new file mode 100644
> >> index 0000000000000000..d03abf7fd8e3997a
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/bus/simple-pm-bus.txt
> >> @@ -0,0 +1,52 @@
> >> +Simple Power-Managed Bus
> >> +========================
> >> +
> >> +A Simple Power-Managed Bus is a transparent bus that doesn't need a real
> >> +driver, as it's typically initialized by the boot loader.
> >> +
> >> +However, its bus controller is part of a PM domain, or under the control of a
> >> +functional clock.  Hence, the bus controller's PM domain and/or clock must be
> >> +enabled for child devices connected to the bus (either on-SoC or externally)
> >> +to function.
> >> +
> >> +
> >> +Generic compatible values and properties
> >> +----------------------------------------
> >> +
> >> +Required properties:
> >> +  - compatible: Must be at least one of the vendor-specific compatible values
> >> +             from a vendor-specific section below, and "simple-bus" as a
> >> +             fallback.
> >
> > What happened to the idea of using something like "simple-pm-bus"?
> 
> I think that's a decision to make by the (successor of the) ePAPR committee.
> At least it would be nice to get some feedback from the DT review team
> about this.
> 
> If we go that road, the vendor-specific compatible value should still be
> documented, else checkpatch will complain when encountering it in a DTS.
> Then, should it become
> 
>     compatible = "renesas,bsc-sh73a0", "renesas,bsc", "simple-pm-bus",
> "simple-bus";
> 
> or should "simple-bus" just be added to of_default_bus_match_table[], so we
> can drop "simple-bus" from the list in the DTS:
> 
>     compatible = "renesas,bsc-sh73a0", "renesas,bsc", "simple-pm-bus";

I was thinking of the reverse: drop "simple-bus" bus from the list here,
but not add "simple-pm-bus" to of_default_bus_match_table. This will
cause child devices to no longer be probed automatically, and you will
have to call of_platform_populate() from simple_pm_bus_probe(), after
pm_runtime_enable(). This seems like a cleaner model to me, for two
reasons:

- In the binding, claiming compatibility with "simple-bus" feels
  wrong to me, because you have a bus that is not as simple as others

- The ordering between pm_runtime_enable() and the probing of the
  child devices is guaranteed, which I think it is not with your
  current code.

Does this make sense, or am I missing an important reason why there
has to be a "simple-bus" compatible string here?

	Arnd