Re: [PATCH v7 2/5] dt-bindings: clock: renesas,r9a06g032-sysctrl: documentation

[Geert Uytterhoeven]

Hi Michel,

On Thu, May 31, 2018 at 12:16 PM, M P <buserror@xxxxxxxxx> wrote:
> On Fri, 25 May 2018 at 10:23, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>> On Thu, May 24, 2018 at 11:28 AM, Michel Pollet
>> <michel.pollet@xxxxxxxxxxxxxx> wrote:
>> > The Renesas R9A06G032 SYSCTRL node description.
>> >
>> > Signed-off-by: Michel Pollet <michel.pollet@xxxxxxxxxxxxxx>
>>
>> Thanks for your patch!
>>
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/clock/renesas,r9a06g032-sysctrl.txt
>> > @@ -0,0 +1,32 @@
>> > +* Renesas R9A06G032 SYSCTRL
>> > +
>> > +Required Properties:
>> > +
>> > +  - compatible: Must be:
>> > +    - "renesas,r9a06g032-sysctrl"
>> > +  - reg: Base address and length of the SYSCTRL IO block.
>> > +  - #clock-cells: Must be 1
>>
>> No clocks/clock-names for the external clock inputs?
>>
>> "RZ/N1 has 3 clock sources, 1 reference clock inputs for RGMII, and 2
>>  reference clock outputs for RMII/MII."
>>
>> Given the documentation explicitly mentions the module clocks are to be
>> used for power-management, you may want to add #power-domain-cells as well,
>> and let the driver register clock domain. But that can be added later
>> (although it will break backwards compatibility with old DTBs).
>>
>> As PWRCTRL_* registers allow to reset individual modules, #reset-cells is
>> another thing to add later.  It's good to start thinking early about how to
>> reference resets, though.
>> E.g. on other Renesas-SoCs, module resets uses the same numerical
>> references as module clocks.
>
> As you said, could we add all that later, as appropriate? Here I tried
> to trim it
> down to the the bare minimum -- my previous version of the driver had
> separate reset  descriptors, but this one has been all compacted to do just
> what it's supposed to do: clocks.

Yes, it can be added later.
I just wanted to mention it, so you could already think about it, and to avoid a
possible "but we could have nicely integrated reset and clock support if we
did ..." later.

> Or, so you want to add another DT index to refer to other reset indexes etc?
> ie not use the of_clk_src_onecell_get provider? That COULD work and yes, the
> indexes would stay the same, I'd just have to get the reset descriptor from the
> clock object. We haven't had a use for individual resets so far.

Reset indices come from DT, too, but the reset framework doesn't use an
xlate function itself, but just passes the index to the reset driver.
How you obtain the register and bits to reset the device is competlely up
to to the reset driver.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

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