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

[Alim Akhtar]

Hi Rob,

> -----Original Message-----
> From: Rob Herring <robh at kernel.org>
> Sent: Tuesday, July 9, 2024 9:53 PM
> To: Alim Akhtar <alim.akhtar at samsung.com>
> Cc: 'Sunyeal Hong' <sunyeal.hong at samsung.com>; 'Krzysztof Kozlowski'
> <krzk at kernel.org>; 'Sylwester Nawrocki' <s.nawrocki at samsung.com>;
> 'Chanwoo Choi' <cw00.choi at samsung.com>; 'Michael Turquette'
> <mturquette at baylibre.com>; 'Stephen Boyd' <sboyd at kernel.org>; 'Conor
> Dooley' <conor+dt at kernel.org>; linux-samsung-soc at vger.kernel.org; linux-
> clk at vger.kernel.org; devicetree at vger.kernel.org; linux-arm-
> kernel at lists.infradead.org; linux-kernel at vger.kernel.org
> 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 at samsung.com>
[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