linux-kernel - Re: [PATCH 02/16] dt-bindings: clock: Add ARTPEC-8 CMU bindings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fd261b5b4452cc797f50bb3fc8cfe93e4a38e477.camel@coasia.com>
Date: Mon, 21 Jul 2025 13:31:53 +0900
From: Hakyeong Kim <hgkim05@...sia.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, ksk4725@...sia.com, Jesper
 Nilsson <jesper.nilsson@...s.com>, Michael Turquette
 <mturquette@...libre.com>,  Stephen Boyd <sboyd@...nel.org>, Rob Herring
 <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,  Conor Dooley
 <conor+dt@...nel.org>, Sylwester Nawrocki <s.nawrocki@...sung.com>, Chanwoo
 Choi <cw00.choi@...sung.com>, Alim Akhtar <alim.akhtar@...sung.com>, Linus
 Walleij <linus.walleij@...aro.org>, Tomasz Figa <tomasz.figa@...il.com>,
 Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>,
 Arnd Bergmann <arnd@...db.de>,  Ravi Patel <ravi.patel@...sung.com>,
 SungMin Park <smn1196@...sia.com>
Cc: kenkim <kenkim@...sia.com>, Jongshin Park <pjsin865@...sia.com>, GunWoo
 Kim <gwk1013@...sia.com>, GyoungBo Min <mingyoungbo@...sia.com>, Pankaj
 Dubey <pankaj.dubey@...sung.com>, Shradha Todi <shradha.t@...sung.com>,
 Inbaraj E <inbaraj.e@...sung.com>, Swathi K S <swathi.ks@...sung.com>,
 Hrishikesh <hrishikesh.d@...sung.com>, Dongjin Yang
 <dj76.yang@...sung.com>, Sang Min Kim <hypmean.kim@...sung.com>,
 linux-kernel@...r.kernel.org,  linux-arm-kernel@...ts.infradead.org,
 linux-samsung-soc@...r.kernel.org,  linux-arm-kernel@...s.com,
 linux-clk@...r.kernel.org, devicetree@...r.kernel.org, 
 linux-gpio@...r.kernel.org, soc@...ts.linux.dev
Subject: Re: [PATCH 02/16] dt-bindings: clock: Add ARTPEC-8 CMU bindings

On Thu, 2025-07-10 at 09:10 +0200, Krzysztof Kozlowski wrote:
> On 10/07/2025 02:20, ksk4725@...sia.com wrote:
> > From: Hakyeong Kim <hgkim05@...sia.com>
> > 
> > Add dt-schema for ARTPEC-8 SoC clock controller.
> > 
> > Add device-tree binding definitions for following CMU blocks:
> > - CMU_CMU
> > - CMU_BUS
> > - CMU_CORE
> > - CMU_CPUCL
> > - CMU_FSYS
> > - CMU_IMEM
> > - CMU_PERI
> > 
> > Signed-off-by: Ravi Patel <ravi.patel@...sung.com>
> > Signed-off-by: Hakyeong Kim <hgkim05@...sia.com>
> 
> Confusing order, unless you really understand this, but considering
> you
> did not add your own SoB I claim you do not understand this. What
> does
> Ravi's SoB mean here?

Ok, I will update the SoB section in all the patches.

> 
> > ---
> >  .../bindings/clock/axis,artpec8-clock.yaml    | 224
> > ++++++++++++++++++
> >  1 file changed, 224 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/clock/axis,artpec8-clock.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/axis,artpec8-
> > clock.yaml b/Documentation/devicetree/bindings/clock/axis,artpec8-
> > clock.yaml
> > new file mode 100644
> > index 000000000000..baacea10599b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/axis,artpec8-
> > clock.yaml
> > @@ -0,0 +1,224 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/axis,artpec8-clock.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Axis ARTPEC-8 SoC clock controller
> > +
> > +maintainers:
> > +  - Jesper Nilsson <jesper.nilsson@...s.com>
> > +
> > +description: |
> > +  ARTPEC-8 clock controller is comprised of several CMU units,
> > generating
> > +  clocks for different domains. Those CMU units are modeled as
> > separate device
> > +  tree nodes, and might depend on each other. The root clock in
> > that root tree
> > +  is an external clock: OSCCLK (25 MHz). This external clock must
> > be defined
> > +  as a fixed-rate clock in dts.
> > +
> > +  CMU_CMU is a top-level CMU, where all base clocks are prepared
> > using PLLs and
> > +  dividers; all other clocks of function blocks (other CMUs) are
> > usually
> > +  derived from CMU_CMU.
> > +
> > +  Each clock is assigned an identifier and client nodes can use
> > this identifier
> > +  to specify the clock which they consume. All clocks available
> > for usage
> > +  in clock consumer nodes are defined as preprocessor macros in
> > +  'include/dt-bindings/clock/axis,artpec8-clk.h' header.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - axis,artpec8-cmu-cmu
> > +      - axis,artpec8-cmu-bus
> > +      - axis,artpec8-cmu-core
> > +      - axis,artpec8-cmu-cpucl
> > +      - axis,artpec8-cmu-fsys
> > +      - axis,artpec8-cmu-imem
> > +      - axis,artpec8-cmu-peri
> > +
> > +  clocks:
> > +    minItems: 1
> > +    maxItems: 5
> > +
> > +  clock-names:
> > +    minItems: 1
> > +    maxItems: 5
> > +
> > +  "#clock-cells":
> > +    const: 1
> > +
> > +  reg:
> > +    maxItems: 1
> 
> reg goes second, after compatible (Samsung bindings are not the best
> example because I converted them long time ago before many coding
> style
> practices were encouraged)

Ok, I will move the reg property.

> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - "#clock-cells"
> > +  - clocks
> > +  - clock-names
> > +
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> 
> Drop contains.

Ok, I will remove the contains wherever applicable.

> 
> > +            const: axis,artpec8-cmu-cmu
> > +
> > +    then:
> > +      properties:
> > +        clocks:
> > +          items:
> > +            - description: External reference clock (25 MHz)
> > +
> > +        clock-names:
> > +          items:
> > +            - const: fin_pll
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: axis,artpec8-cmu-bus
> > +
> > +    then:
> > +      properties:
> > +        clocks:
> > +          items:
> > +            - description: External reference clock (25 MHz)
> > +            - description: CMU_BUS BUS clock (from CMU_CMU)
> > +            - description: CMU_BUS DLP clock (from CMU_CMU)
> > +
> > +        clock-names:
> > +          items:
> > +            - const: fin_pll
> > +            - const: dout_clkcmu_bus_bus
> > +            - const: dout_clkcmu_bus_dlp
> 
> All these names should be changed to match what is the input. Look at
> latest bindings, we moved away from that style.

Ok, I will modify clock-names to "bus" and "dlp" and will update at the
other related places also.

Thanks,
Hakyeong Kim

> 
> 
> 
> Best regards,
> Krzysztof