Hi Geert, On 10 April 2018 11:08, Geert wrote: > > Hi Michel, > > On Tue, Apr 10, 2018 at 11:56 AM, Michel Pollet > <michel.pollet@xxxxxxxxxxxxxx> 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.
