On Thu, Feb 18, 2016 at 06:18:56PM +0100, Geert Uytterhoeven wrote: > Hi Rob, > > On Thu, Feb 18, 2016 at 3:38 PM, Rob Herring <robh@xxxxxxxxxx> wrote: > > On Mon, Feb 15, 2016 at 10:16:50PM +0100, Geert Uytterhoeven wrote: > >> The Renesas R-Car System Controller provides power management for the > >> CPU cores and various coprocessors, following the generic PM domain > >> bindings in Documentation/devicetree/bindings/power/power_domain.txt. > >> > >> This supports R-Car Gen1, Gen2, and Gen3. > >> > >> Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx> > >> --- > >> - Keeping the power register block offset and the bit number as separate > >> "reg" cells, increasing "#address-cells" from 2 to 3, > >> > >> - Merging the interrupt bit (which needs only 5 bits) in the other "reg" > >> cell, decreasing "#address-cells" from 2 to 1. > > > > I think I'd move to not encoding mulitple things into reg. This seems > > like a bit of abuse of reg. Otherwise, I don't have much to comment on. > > Thanks! > > (quoting the encoding of the reg properties) > > +== PM Domain Nodes == > > + > > +Each of the PM domain nodes represents a PM domain, as documented by the > > +generic PM domain bindings in > > +Documentation/devicetree/bindings/power/power_domain.txt. > > + > > +Required properties: > > + - #power-domain-cells: Must be 0. > > + - reg: This property must contain 2 values: > > + - The first value is the number of the interrupt bit representing > > + the power area in the various Interrupt Registers (e.g. SYSCISR, > > + Interrupt Status Register), > > + - The second value encodes the power register block offset (which is > > + a multiple of 64), and the number of the bit representing the > > + power area in the various Power Control Registers (e.g. PWROFFSR, > > + Power Shutoff Status Register). This value is created by ORing > > + these two numbers. > > Not encoding multiple things into reg means adding more properties to provide > that information, iff we want to describe the PM Domain Nodes in DT. > I considered the reg property a two-dimensional address space. > > Taking the lessons from CCF and the new CPG/MSSR bindings into account > (which was BTW designed after the SYSC DT bindings), perhaps the PM Domain > hierarchy should be moved from DT to C, in the driver, too? > > That would mean we have in DT: > 1) "#power-domain-cells = <1>" > 2) defines for the various domains, e.g. "#define R8A7791_PD_CA15_SCU 12" > 3) e.g. "power-domains = <&sysc R8A7791_PD_CA15_SCU>" > 4) and we can get rid of the fallback compatibility strings again. > > Thoughts? Seems fine to me. Rob
