Hi Laurent,

On Tue, Sep 1, 2015 at 9:30 PM, Laurent Pinchart
<[email protected]> wrote:
> 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 :-)

You are right that they may not be searched very frequently. But at
least the CPG is consistent with MSTP with this implementation. =)

>> 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 ?

You know that I personally always prefered an incremental approach
with little speculation in DT. =)

So using SoC specific compat strings over generic ones would in
general be my preferred choice. I suspect the initial developer of
this patch simply followed the same style as Gen2 with a generic
compat string.

Does this mean that you for the r8a7795 CPG prefer SoC specific compat
string to begin with over a more generic one? If we go for SoC
specific compat string then perhaps we also should change file names?

Thanks,

/ magnus
--
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

Reply via email to