RE: [PATCH v2 1/4] dt-bindings: clock: add Exynos Auto v920 SoC CMU bindings

[Alim Akhtar]

Hi Rob,

> -----Original Message-----
> From: Rob Herring <robh@xxxxxxxxxx>
> Sent: Tuesday, July 9, 2024 9:53 PM
> To: Alim Akhtar <alim.akhtar@xxxxxxxxxxx>
> Cc: 'Sunyeal Hong' <sunyeal.hong@xxxxxxxxxxx>; 'Krzysztof Kozlowski'
> <krzk@xxxxxxxxxx>; 'Sylwester Nawrocki' <s.nawrocki@xxxxxxxxxxx>;
> 'Chanwoo Choi' <cw00.choi@xxxxxxxxxxx>; 'Michael Turquette'
> <mturquette@xxxxxxxxxxxx>; 'Stephen Boyd' <sboyd@xxxxxxxxxx>; 'Conor
> Dooley' <conor+dt@xxxxxxxxxx>; linux-samsung-soc@xxxxxxxxxxxxxxx; linux-
> clk@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v2 1/4] dt-bindings: clock: add Exynos Auto v920 SoC
> CMU bindings
> 
> On Mon, Jul 08, 2024 at 03:59:40PM +0530, Alim Akhtar wrote:
> > Hello Sunyeal
> >
> > > -----Original Message-----
> > > From: Sunyeal Hong <sunyeal.hong@xxxxxxxxxxx>
[snip]
> > > +            - description: External reference clock (38.4 MHz)
> > > +            - description: CMU_PERIC0 NOC clock (from CMU_TOP)
> > > +            - description: CMU_PERIC0 IP clock (from CMU_TOP)
> > > +
> > > +        clock-names:
> > > +          items:
> > > +            - const: oscclk
> > > +            - const: noc
> > > +            - const: ip
> > These are too generic name, please add peric0_noc and peric0_ip, and
this
> is to match with the UM.
> > I am sure in future you would like to add other IPs like USI, I2C etc
> > for the peric0 block
> 
> Names are local to the block, so adding the block name is redundant.
> 
Yes, noc and IP clocks are input to CMU_PERIC0 block which is coming from
another block (CMU_TOP)
The name mentioned in the UM as DIV_CLKCMU_PERIC0_NOC and
DIV_CLKCMU_PERIC0_IP. To match UM,
suggested to add prefix block name. It is redundant though.
I can think of a case where clock names are big and taking several
characters, it make sense to drop the prefix. 

> Wouldn't USI and I2C clocks be outputs? This property is input clocks.
> 
Yes it is, what I meant was the clock ID mentioned in
include/dt-bindings/clock/samsung,exynosautov920.h
Also contains _PERIC0_ as prefix.

I am fine with the recommended name (just keeping NoC and IP) for the local
clocks to a block.

> The names and descriptions should be defined at the top level and then
here
> should be just 'minItems: 3' (And above 'maxItems: 1').
> 
> Rob