On 10/21/24 12:38, Krzysztof Kozlowski wrote: > 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@xxxxxxxxx> > > 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@xxxxxxxxx> >> + - Chanwoo Choi <cw00.choi@xxxxxxxxxxx> >> + - Krzysztof Kozlowski <krzk@xxxxxxxxxx> >> + - Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx> >> + - Tomasz Figa <tomasz.figa@xxxxxxxxx> > 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? FSYS0 provides clocks for embedded storages (UFS and eMMC), whereas FSYS1 clocks external storage cards (UFS and MMC cards). As far as I can tell, there's only one MMC controller, so perhaps it could be set to drive an eMMC or an SD card. On retail devices, UFS_EMBD and MMC_CARD are used. Thanks for the review, will fix the issues in the next patchset. Best regards, Ivo. > >> + - 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@11400000 { >> + 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 >
