Hi Magnus, On Tuesday 01 September 2015 19:59:01 Magnus Damm wrote: > On Tue, Sep 1, 2015 at 3:00 PM, Laurent Pinchart wrote: > > On Monday 31 August 2015 21:49:04 Magnus Damm wrote: > >> From: Gaku Inami <[email protected]> > >> > >> This V5 patch adds initial CPG support for R-Car Generation 3 and in > >> particular the R8A7795 SoC. > >> > >> The R-Car Gen3 clock hardware has a register write protection feature > >> that needs to be shared between the CPG function needs to be shared > >> between the CPG and MSTP hardware somehow. So far this feature is simply > >> ignored. > >> > >> Signed-off-by: Gaku Inami <[email protected]> > >> Signed-off-by: Kuninori Morimoto <[email protected]> > >> Signed-off-by: Magnus Damm <[email protected]> > >> --- > >> > >> Changes since V4: (Magnus Damm <[email protected]>) > >> - Simplified clks array handling - thanks Geert! > >> - Updated th DT binding documentation to reflect latest state > >> - of_property_count_strings() -> of_property_count_u32_elems() fix > >> > >> Changes since V3: (Magnus Damm <[email protected]>) > >> - Reworked driver to incorporate most feedback from Stephen Boyd - > >> thanks!! > >> - Major things like syscon and driver model require more discussion. > >> - Added hunk to build drivers/clk/shmobile if ARCH_RENESAS is set. > >> > >> Changes since V2: (Magnus Damm <[email protected]>) > >> - Reworked driver to rely on clock index instead of strings. > >> - Dropped use of "clock-output-names". > >> > >> Earlier versions: Kuninori Morimoto <[email protected]> > >> Initial version: Gaku Inami <[email protected]> > >> > >> Documentation/devicetree/bindings/clock/renesas,rcar-gen3-cpg-clocks.txt > >> | 32 + > >> drivers/clk/Makefile | 1 > >> drivers/clk/shmobile/Makefile | 1 > >> drivers/clk/shmobile/clk-rcar-gen3.c | 245 ++++++++++ > >> 4 files changed, 279 insertions(+) > >> > >> --- /dev/null > >> +++ > >> work/Documentation/devicetree/bindings/clock/renesas,rcar-gen3-cpg-clocks > >> .txt > >> 2015-08-31 15:49:10.000000000 +0900 @@ -0,0 +1,32 @@ > >> +* Renesas R-Car Gen3 Clock Pulse Generator (CPG) > >> + > >> +The CPG generates core clocks for the R-Car Gen3 SoCs. It includes three > >> PLLs > >> +and several fixed ratio dividers. > >> + > >> +Required Properties: > >> + > >> + - compatible: Must be one of > >> + - "renesas,r8a7795-cpg-clocks" for the r8a7795 CPG > >> + - "renesas,rcar-gen3-cpg-clocks" for the generic R-Car Gen3 CPG > >> + > >> + - reg: Base address and length of the memory resource used by the CPG > >> + > >> + - clocks: References to the parent clocks: first to the EXTAL clock > >> + - #clock-cells: Must be 1 > >> + - clock-indices: Indices of the exported clocks > > > > Do we actually need the clock-indices property, as CPG clocks are numbered > > sequentially ? It seems to me like we could drop the property, especially > > given that the number of clocks is hardcoded in the driver anyway. > > There are two reasons why they are there: > 1) I like to be able to search DT files to see which node that is > providing what clock.
The clocks are listed in the CPG section of include/dt-bindings/clock/r8a7795- clock.h so that should already give a hint. Additionally I don't think you'll search the CPG core clocks very often in the DT sources as they're usually not used directly by devices :-) > 2) When adding support for additional SoCs we may add new index to the > driver. New SoC may get sparsely populated index lists and old ones > will omit the recently added index. If the CPG core differs between SoCs shouldn't that be handled by the DT compatibility string ? > >> +/* > >> + * MD EXTAL PLL0 PLL1 PLL2 PLL3 > >> PLL4 > >> + * 14 13 19 17 (MHz) *1 *1 *1 > >> + *------------------------------------------------------------------- > >> + * 0 0 0 0 16.66 x 1 x180/2 x192/2 x144/2 x192 > >> x144 > >> + * 0 0 0 1 16.66 x 1 x180/2 x192/2 x144/2 x128 > >> x144 > >> + * 0 0 1 0 Prohibited setting > >> + * 0 0 1 1 16.66 x 1 x180/2 x192/2 x144/2 x192 > >> x144 > >> + * 0 1 0 0 20 x 1 x150/2 x156/2 x120/2 x156 > >> x120 > >> + * 0 1 0 1 20 x 1 x150/2 x156/2 x120/2 x106 > >> x120 > >> + * 0 1 1 0 Prohibited setting > >> + * 0 1 1 1 20 x 1 x150/2 x156/2 x120/2 x156 > >> x120 > >> + * 1 0 0 0 25 x 1 x120/2 x128/2 x96/2 x128 x96 > >> + * 1 0 0 1 25 x 1 x120/2 x128/2 x96/2 x84 x96 > >> + * 1 0 1 0 Prohibited setting > >> + * 1 0 1 1 25 x 1 x120/2 x128/2 x96/2 x128 x96 > >> + * 1 1 0 0 33.33 / 2 x180/2 x192/2 x144/2 x192 > >> x144 > >> + * 1 1 0 1 33.33 / 2 x180/2 x192/2 x144/2 x128 > >> x144 > >> + * 1 1 1 0 Prohibited setting > >> + * 1 1 1 1 33.33 / 2 x180/2 x192/2 x144/2 x192 > >> x144 > >> + * > >> + * *1 : datasheet indicates VCO output (PLLx = VCO/2) > > > > As explained in a separate e-mail there's a few clocks on R8A7795 that > > derive directly from PLL1 VCO. I thus wonder whether we shouldn't expose > > the PLL1 clock as the VCO output and create VCO/2 using a fixed factor > > clock in DT. > > Do you think that would reduce complexity or simplify the code? If so > I think we should do it. Otherwise I think it makes sense to simply > follow the data sheet. I've commented on this in a reply to Geert's e-mail. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
