[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <75e0b0a3-6b6c-427e-a748-329dc1237da7@kernel.org>
Date: Mon, 21 Oct 2024 11:38:49 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Ivaylo Ivanov <ivo.ivanov.ivanov1@...il.com>,
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>, Tomasz Figa <tomasz.figa@...il.com>
Cc: 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 v1 1/3] dt-bindings: clock: Add Exynos8895 SoC CMU
bindings
On 20/10/2024 19:48, Ivaylo Ivanov wrote:
> Provide dt-schema documentation for Exynos8895 SoC clock controller.
> Add device tree clock binding definitions for the following CMU blocks:
> - CMU_TOP
> - CMU_FSYS0/1
> - CMU_PERIC0/1
> - CMU_PERIS
>
> Signed-off-by: Ivaylo Ivanov <ivo.ivanov.ivanov1@...il.com>
A nit, subject: drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
> +
> +title: Samsung Exynos8895 SoC clock controller
> +
> +maintainers:
> + - Ivaylo Ivanov <ivo.ivanov.ivanov1@...il.com>
> + - Chanwoo Choi <cw00.choi@...sung.com>
> + - Krzysztof Kozlowski <krzk@...nel.org>
> + - Sylwester Nawrocki <s.nawrocki@...sung.com>
> + - Tomasz Figa <tomasz.figa@...il.com>
Please drop Sylwester and Tomasz, they opted out from clocks.
> +
> +description: |
> + Exynos8895 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 (26 MHz). This external clock must be defined
> + as a fixed-rate clock in dts.
> +
> + CMU_TOP 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_TOP.
> +
> + 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/samsung,exynos8895.h' header.
> +
> +properties:
> + compatible:
> + enum:
> + - samsung,exynos8895-cmu-top
> + - samsung,exynos8895-cmu-fsys0
> + - samsung,exynos8895-cmu-fsys1
> + - samsung,exynos8895-cmu-peric0
> + - samsung,exynos8895-cmu-peric1
> + - samsung,exynos8895-cmu-peris
Alphabetical order.
> +
> + clocks:
> + minItems: 1
> + maxItems: 16
> +
> + clock-names:
> + minItems: 1
> + maxItems: 16
> +
> + "#clock-cells":
> + const: 1
> +
> + reg:
> + maxItems: 1
> +
required: block should go here (I know that other Samsung clock bindings
do not follow this convention).
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: samsung,exynos8895-cmu-top
> +
> + then:
> + properties:
> + clocks:
> + items:
> + - description: External reference clock (26 MHz)
> +
> + clock-names:
> + items:
> + - const: oscclk
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: samsung,exynos8895-cmu-fsys0
> +
> + then:
> + properties:
> + clocks:
> + items:
> + - description: External reference clock (26 MHz)
> + - description: CMU_FSYS0 BUS clock (from CMU_TOP)
> + - description: CMU_FSYS0 DPGTC clock (from CMU_TOP)
> + - description: CMU_FSYS0 MMC_EMBD clock (from CMU_TOP)
> + - description: CMU_FSYS0 UFS_EMBD clock (from CMU_TOP)
> + - description: CMU_FSYS0 USBDRD30 clock (from CMU_TOP)
> +
> + clock-names:
> + items:
> + - const: oscclk
> + - const: bus
> + - const: dpgtc
> + - const: mmc_embd
mmc
> + - const: ufs_embd
ufs
> + - const: usbdrd30
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: samsung,exynos8895-cmu-fsys1
> +
> + then:
> + properties:
> + clocks:
> + items:
> + - description: External reference clock (26 MHz)
> + - description: CMU_FSYS1 BUS clock (from CMU_TOP)
> + - description: CMU_FSYS1 MMC_CARD clock (from CMU_TOP)
> + - description: CMU_FSYS1 PCIE clock (from CMU_TOP)
> + - description: CMU_FSYS1 UFS_CARD clock (from CMU_TOP)
> +
> + clock-names:
> + items:
> + - const: oscclk
> + - const: bus
> + - const: mmc_card
mmc
Although now I wonder, why this is different FSYS. Is it for different
mmc controller?
> + - const: pcie
> + - const: ufs_card
ufs
Keep the order as in GS101 file.
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: samsung,exynos8895-cmu-peric0
> +
> + then:
> + properties:
> + clocks:
> + items:
> + - description: External reference clock (26 MHz)
> + - description: CMU_PERIC0 BUS clock (from CMU_TOP)
> + - description: CMU_PERIC0 UART_DBG clock (from CMU_TOP)
> + - description: CMU_PERIC0 USI00 clock (from CMU_TOP)
> + - description: CMU_PERIC0 USI01 clock (from CMU_TOP)
> + - description: CMU_PERIC0 USI02 clock (from CMU_TOP)
> + - description: CMU_PERIC0 USI03 clock (from CMU_TOP)
> +
> + clock-names:
> + items:
> + - const: oscclk
> + - const: bus
> + - const: uart_dbg
uart
> + - const: usi00
usi0
> + - const: usi01
usi1
> + - const: usi02
usi2
> + - const: usi03
usi3
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: samsung,exynos8895-cmu-peric1
> +
> + then:
> + properties:
> + clocks:
> + items:
> + - description: External reference clock (26 MHz)
> + - description: CMU_PERIC1 BUS clock (from CMU_TOP)
> + - description: CMU_PERIC1 SPEEDY2 clock (from CMU_TOP)
> + - description: CMU_PERIC1 SPI_CAM0 clock (from CMU_TOP)
> + - description: CMU_PERIC1 SPI_CAM1 clock (from CMU_TOP)
> + - description: CMU_PERIC1 UART_BT clock (from CMU_TOP)
> + - description: CMU_PERIC1 USI04 clock (from CMU_TOP)
> + - description: CMU_PERIC1 USI05 clock (from CMU_TOP)
> + - description: CMU_PERIC1 USI06 clock (from CMU_TOP)
> + - description: CMU_PERIC1 USI07 clock (from CMU_TOP)
> + - description: CMU_PERIC1 USI08 clock (from CMU_TOP)
> + - description: CMU_PERIC1 USI09 clock (from CMU_TOP)
> + - description: CMU_PERIC1 USI10 clock (from CMU_TOP)
> + - description: CMU_PERIC1 USI11 clock (from CMU_TOP)
> + - description: CMU_PERIC1 USI12 clock (from CMU_TOP)
> + - description: CMU_PERIC1 USI13 clock (from CMU_TOP)
> +
> + clock-names:
> + items:
> + - const: oscclk
> + - const: bus
> + - const: speedy2
speedy
> + - const: cam0
> + - const: cam1
> + - const: uart_bt
uart
> + - const: usi04
usi4, etc
> + - const: usi05
> + - const: usi06
> + - const: usi07
> + - const: usi08
> + - const: usi09
> + - const: usi10
> + - const: usi11
> + - const: usi12
> + - const: usi13
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: samsung,exynos8895-cmu-peris
> +
> + then:
> + properties:
> + clocks:
> + items:
> + - description: External reference clock (26 MHz)
> + - description: CMU_PERIS BUS clock (from CMU_TOP)
> +
> + clock-names:
> + items:
> + - const: oscclk
> + - const: bus
> +
> +required:
> + - compatible
> + - "#clock-cells"
> + - clocks
> + - clock-names
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + # Clock controller node for CMU_FSYS1
> + - |
> + #include <dt-bindings/clock/samsung,exynos8895.h>
> +
> + cmu_fsys1: clock-controller@...00000 {
> + compatible = "samsung,exynos8895-cmu-fsys1";
> + reg = <0x11400000 0x8000>;
> + #clock-cells = <1>;
> +
> + clocks = <&oscclk>,
> + <&cmu_top CLK_DOUT_CMU_FSYS1_BUS>,
> + <&cmu_top CLK_DOUT_CMU_FSYS1_MMC_CARD>,
> + <&cmu_top CLK_DOUT_CMU_FSYS1_PCIE>,
> + <&cmu_top CLK_DOUT_CMU_FSYS1_UFS_CARD>;
> + clock-names = "oscclk", "bus", "mmc_card",
> + "pcie", "ufs_card";
> + };
> +
Best regards,
Krzysztof
Powered by blists - more mailing lists