linux-kernel - Re: [PATCH v2 1/5] dt-bindings: clock: add exynos8890 SoC

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: <20251022-affable-arrogant-coucal-3f7fbc@kuoka>
Date: Wed, 22 Oct 2025 09:44:40 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Ivaylo Ivanov <ivo.ivanov.ivanov1@...il.com>
Cc: Sylwester Nawrocki <s.nawrocki@...sung.com>, 
	Chanwoo Choi <cw00.choi@...sung.com>, Alim Akhtar <alim.akhtar@...sung.com>, 
	Michael Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>, Rob Herring <robh@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, linux-samsung-soc@...r.kernel.org, linux-clk@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/5] dt-bindings: clock: add exynos8890 SoC

On Fri, Oct 17, 2025 at 07:13:29PM +0300, Ivaylo Ivanov wrote:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: samsung,exynos8890-cmu-aud
> +
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: External reference clock (76.8 MHz)
> +            - description: CMU_AUD PLL clock (from CMU_TOP)
> +
> +        clock-names:
> +          items:
> +            - const: oscclk
> +            - const: pll
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: samsung,exynos8890-cmu-bus0
> +
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: External reference clock (76.8 MHz)
> +            - description: CMU_BUS0 ACLK 528MHz clock (from CMU_TOP)
> +            - description: CMU_BUS0 ACLK 200MHz clock (from CMU_TOP)
> +            - description: CMU_BUS0 PCLK 132MHz clock (from CMU_TOP)
> +
> +        clock-names:
> +          items:
> +            - const: oscclk
> +            - const: "528"
> +            - const: "200"
> +            - const: "132"

We do not want the frequency here, for sure not frequency alone. There
is no such code/syntax. Really.  Please do not invent your own style.
That's just pclk. You describe here the logical name of this clock
input.

ACLK is AXI bus clock, so if this block receives only one ACLK, then
this is just "axi" or "bus". Recently we were calling this "bus".

Same in other places. If two AXI bus clocks come in, they could be named
bus0 and bus1, or in this case - because these are sources for
generating further ACLKs - bus_528 and bus_200, to indicate that one
will be for AXI bus clocked 528 MHz and other for 200 MHz.

Please wait for some other opinions, because same rule I would like to
apply to ExynosAuto, Artpec and Google GS.

@Raghav Sharma, @Alim Akhtar, @Sam Protsenko, @Peter Griffin, @André
Draszik  - share your thoughs please?

And to clarify in simple terms for others or for the future:
1. HCLK would be the AHB bus, so also bus. Both ACLK and HCLK are for
memory accesses.
2. PCLK is APB bus, for registers.
3. SCLK is for main operation of the block (called special clock, but no
clue what is so special about it).

> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: samsung,exynos8890-cmu-bus1
> +
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: External reference clock (76.8 MHz)
> +            - description: CMU_BUS1 ACLK 528MHz clock (from CMU_BUS0)
> +            - description: CMU_BUS1 PCLK 132MHz clock (from CMU_TOP)
> +
> +        clock-names:
> +          items:
> +            - const: oscclk
> +            - const: "528"
> +            - const: "132"
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: samsung,exynos8890-cmu-ccore
> +
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: External reference clock (76.8 MHz)
> +            - description: CMU_CCORE ACLK 800MHz clock (from CMU_TOP)
> +            - description: CMU_CCORE ACLK 264MHz clock (from CMU_TOP)
> +            - description: CMU_CCORE ACLK G3D 800MHz clock (from CMU_TOP)
> +            - description: CMU_CCORE ACLK 528MHz clock (from CMU_TOP)
> +            - description: CMU_CCORE ACLK 132MHz clock (from CMU_TOP)
> +            - description: CMU_CCORE PCLK 66MHz clock (from CMU_TOP)
> +
> +        clock-names:
> +          items:
> +            - const: oscclk
> +            - const: "800"
> +            - const: "264"
> +            - const: g3d
> +            - const: "528"
> +            - const: "132"
> +            - const: "66"
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: samsung,exynos8890-cmu-disp0
> +
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: External reference clock (76.8 MHz)
> +            - description: CMU_DISP0 ACLK 0 400MHz clock (from CMU_TOP)
> +            - description: CMU_DISP0 ACLK 1 400MHz clock (from CMU_TOP)
> +            - description: CMU_DISP0 DECON0 ECLK0 clock (from CMU_TOP)
> +            - description: CMU_DISP0 DECON0 VCLK0 clock (from CMU_TOP)
> +            - description: CMU_DISP0 DECON0 VCLK1 clock (from CMU_TOP)
> +            - description: CMU_DISP0 HDMI audio clock (from CMU_TOP)
> +
> +        clock-names:
> +          items:
> +            - const: oscclk
> +            - const: "0_400"
> +            - const: "1_400"
> +            - const: eclk0
> +            - const: vclk0
> +            - const: vclk1
> +            - const: audio
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: samsung,exynos8890-cmu-disp1
> +
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: External reference clock (76.8 MHz)
> +            - description: CMU_DISP1 ACLK 0 400MHz clock (from CMU_TOP)
> +            - description: CMU_DISP1 ACLK 1 400MHz clock (from CMU_TOP)
> +            - description: CMU_DISP1 DECON1 ECLK0 clock (from CMU_TOP)
> +            - description: CMU_DISP1 DECON1 ECLK1 clock (from CMU_TOP)
> +
> +        clock-names:
> +          items:
> +            - const: oscclk
> +            - const: "0_400"
> +            - const: "1_400"
> +            - const: eclk0
> +            - const: eclk01
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: samsung,exynos8890-cmu-fsys0
> +
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: External reference clock (76.8 MHz)
> +            - description: CMU_FSYS0 ACLK 200MHz clock (from CMU_TOP)
> +            - description: CMU_FSYS0 USB30DRD clock (from CMU_TOP)
> +            - description: CMU_FSYS0 MMC0 clock (from CMU_TOP)
> +            - description: CMU_FSYS0 UFS UNIPRO20 clock (from CMU_TOP)
> +            - description: CMU_FSYS0 PHY 24MHz clock (from CMU_TOP)
> +            - description: CMU_FSYS0 UFS UNIPRO config clock (from CMU_TOP)
> +
> +        clock-names:
> +          items:
> +            - const: oscclk
> +            - const: "200"
> +            - const: usb
> +            - const: mmc
> +            - const: unipro20

Just "ufs"

> +            - const: 24m

No... really no. Half of these names feel random. ACLK 200 MHz is "200"
but PHY 24 MHz is 24m... That's SCLK but I don't know from where does it
come from, so what is the true source. Since it is used as ref clock for
PHY, I would rather assume this is not "SCLK" but some fixed oscillator
input. If you don't find the source of this clock giving any reasonable
name, let it be just "sclk".

I understand that without hardware manual it is difficult to figure all
this out and my requirements here do not make it easier. I appreciate
your work.

> +            - const: unipro_cfg

"ufs_cfg"

> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: samsung,exynos8890-cmu-fsys1
> +
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: External reference clock (76.8 MHz)
> +            - description: CMU_FSYS1 ACLK 200MHz clock (from CMU_TOP)
> +            - description: CMU_FSYS1 MMC2 clock (from CMU_TOP)
> +            - description: CMU_FSYS1 UFS UNIPRO20 clock (from CMU_TOP)
> +            - description: CMU_FSYS1 UFS UNIPRO config clock (from CMU_TOP)
> +            - description: CMU_FSYS1 PCIe PHY clock (from CMU_TOP)
> +            - description: CMU_FSYS1 PCIe PLL clock (from CMU_TOP)
> +
> +        clock-names:
> +          items:
> +            - const: oscclk
> +            - const: "200"
> +            - const: mmc2

mmc

> +            - const: unipro20
> +            - const: unipro_cfg

Please keep common part same with fsys0, so the lists share as much as
possible.  Same for other blocks.

Best regards,
Krzysztof