Hi Geert,

On 10 April 2018 11:08, Geert wrote:
>
> Hi Michel,
>
> On Tue, Apr 10, 2018 at 11:56 AM, Michel Pollet
> <michel.pol...@bp.renesas.com> wrote:
> > In the current SDK for the RZ/N1, we made a clock architecture that is
> entirely device-tree based.
> > The clock hierarchy is quite complex and was machine generated from
> > design documents, and some exceptions and grouping were added to the
> 'main' family rzn1.dtsi...
> >
> > Apart from a few fixed-clock nodes, all of the other nodes are
> > 'special' and require a driver. All of these drivers are sub-drivers to a 
> > 'main'
> clock driver. That has been working for 2 years already.
> >
> > One extra note: we don't 'own' all of these clocks, part of the
> > clocks/dividers can be enable/disabled by the CM3 core.
> >
> > Now, For upstreaming, I'm going to have to change that, since already
> > the 'clock' bits are going to go under the MFD sysctrl node. However
> > I'm trying to figure out if we can still use our rzn1-clocks.dtsi in
> > some form, as well as my drivers, or so I have to convert it to a C table in
> some way.
> >
> > Also note that all the clock refer to SYSCTRL registers/bits using
> > constant names from a header file, not hex constants etc.
> >
> > I would appreciate any ideas/suggestions before I commit blindly to a
> path...
> >
> > Here is the main autogenerated clock file:
> > https://github.com/renesas-rz/rzn1_linux/blob/rzn1-stable/arch/arm/boo
> > t/dts/rzn1-clocks.dtsi Here's the extra clock{} node in the main
> > rzn1.dtsi
> > https://github.com/renesas-
> rz/rzn1_linux/blob/89d6c9be056a462b95d52172
> > 21d70d6e5c25dfc2/arch/arm/boot/dts/rzn1.dtsi#L70
>
> Describing the full clock hierarchy in DT is no longer recommended.
> The modern way is to have a single clock provider node in DT, and have the
> driver register all clocks.
>
> Compare e.g. the single clock-controller node in
> arch/arm/boot/dts/r8a7791.dtsi in v4.15 (used with the {r8a7791,renesas}-
> cpg-mssr.c driver, with the complex clocks node in v4.14, used with a lot of
> subdrivers, and requiring continuous maintenance.
>
> So I think you're best of creating (generating) a C table instead, and keep 
> the
> DT simple and obviously correct.

So, do I understand correctly that I could duplicate the tree as a C structure, 
and have my
'clock' driver instantiate all my sub drivers as a clock tree directly?

I looked into r8a7791's code, but your clock tree is 'flat' from what I see 
there, you don't have
the weird and fun clock dependencies we have. Nor any of the special cases we 
have; also,
you have a single clock driver in there it seems, so it is a pretty simple case 
where your
indexing method works...

So I can definitely have a C struct to instantiate the table, but mostly that C 
table will be
duplicating the device tree hierarchy completely, and I'll still need as many 
sub-drivers
as I already have... The only change is really that that table will be in a .c 
-- is that
exactly what you want?

Here are the current drivers I used.
https://github.com/renesas-rz/rzn1_linux/tree/rzn1-stable/drivers/clk/rzn1

There is:
+ renesas,rzn1-clock-group -- many drivers only support claiming a single 
clock; the
   clock-group driver allows me to group clocks together so they are enabled as 
ones
   -- that way I don't have to patch the drivers.
+ renesas,rzn1-clock-gate -- this one is more or less the same as yours. 
Enables/disable
   a clock. It's parent will provide the rate.
+ renesas,rzn1-clock-divider -- this one has a register with min,max, and an 
optional
   table of 'fixed' values if the range is not linear.
+ renesas,rzn1-clock-bitselect -- this one is fun, a single bit can change not 
only the
   parent clock of *several IPs at the same time* but also the gate they have 
to use...
+ renesas,rzn1-clock-dualgate -- this one works with the previous one, use gate 
0 or
   1 depending on one bit...

So, should I just jam all of these together with a struct hierarchy in a single 
driver, and
use an index? I don't have too much trouble with the table as I can generate it 
as well,
I'm just making sure it's what you want...

>
> Gr{oetje,eeting}s,
>
>                         Geert

Thanks!
Michel
PS: I didn't make the hardware :-)



Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, 
Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered 
No. 04586709.

Reply via email to