Re: [PATCH v3 1/5] dt-bindings: clock: samsung: Add Exynos9610 CMU bindings

[Krzysztof Kozlowski]

On Wed, Jul 22, 2026 at 11:44:02PM +0300, Alexandru Chimac wrote:
> This clock management unit has a topmost block (CMU_TOP)
> that generates top clocks for other blocks, alongside 20
> other blocks, out of which 11 are currently implemented.

This feels not properly wrapped.

Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597

> 
> Signed-off-by: Alexandru Chimac <alex@xxxxxxxxx>
> ---
>  .../bindings/clock/samsung,exynos9610-cmu.yaml     | 335 ++++++++++
>  include/dt-bindings/clock/samsung,exynos9610-cmu.h | 722 +++++++++++++++++++++
>  2 files changed, 1057 insertions(+)

...

> +  compatible:
> +    enum:
> +      - samsung,exynos9610-cmu-top
> +      - samsung,exynos9610-cmu-apm
> +      - samsung,exynos9610-cmu-cam
> +      - samsung,exynos9610-cmu-cmgp
> +      - samsung,exynos9610-cmu-core
> +      - samsung,exynos9610-cmu-cpucl0
> +      - samsung,exynos9610-cmu-dispaud
> +      - samsung,exynos9610-cmu-fsys
> +      - samsung,exynos9610-cmu-g2d
> +      - samsung,exynos9610-cmu-g3d
> +      - samsung,exynos9610-cmu-peri
> +      - samsung,exynos9610-cmu-usb

List should be ordered alphabetically.

> +
> +  clocks:
> +    minItems: 1
> +    maxItems: 5
> +
> +  clock-names:
> +    minItems: 1
> +    maxItems: 5
> +
> +  "#clock-cells":
> +    const: 1
> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - "#clock-cells"
> +  - clocks
> +  - clock-names
> +  - reg
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: samsung,exynos9610-cmu-top

And then the same order of "if:then:" cases.

> +
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: External reference clock (26MHz)
> +
> +        clock-names:
> +          items:
> +            - const: oscclk
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: samsung,exynos9610-cmu-apm
> +
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: External reference clock (26MHz)
> +            - description: CMU_APM bus clock (from CMU_TOP)
> +
> +        clock-names:
> +          items:
> +            - const: oscclk
> +            - const: dout_cmu_apm_bus

...


> +
> +additionalProperties: false
> +
> +examples:
> +  # Clock controller node for CMU_FSYS

Drop comment, compatible defines that this is CMU FSYS

> +  - |
> +    #include <dt-bindings/clock/samsung,exynos9610-cmu.h>
> +
> +    cmu_fsys: clock-controller@13400000 {

Drop unused label.

> +        compatible = "samsung,exynos9610-cmu-fsys";
> +        reg = <0x13400000 0x8000>;
> +        #clock-cells = <1>;
> +
> +        clocks = <&oscclk>,
> +                 <&cmu_top CLK_DOUT_CMU_FSYS_BUS>,
> +                 <&cmu_top CLK_DOUT_CMU_FSYS_MMC_CARD>,
> +                 <&cmu_top CLK_DOUT_CMU_FSYS_MMC_EMBD>,
> +                 <&cmu_top CLK_DOUT_CMU_FSYS_UFS_EMBD>;
> +        clock-names = "oscclk",
> +                      "dout_cmu_fsys_bus",
> +                      "dout_cmu_fsys_mmc_card",
> +                      "dout_cmu_fsys_mmc_embd",
> +                      "dout_cmu_fsys_ufs_embd";
> +    };
> +...

...

> +/* CMU_CAM muxes */
> +#define CLK_MOUT_PLL_CAM_BUS_USER			1
> +
> +/* CMU_CAM dividers */
> +#define CLK_DIV_CLK_CAM_BUSP				2
> +
> +/* CMU_CAM gates */
> +#define CLK_GAT_CLK_CAM_CMU_PCLK			3
> +#define CLK_GAT_CLK_CAM_OSCCLK_CLK			4
> +#define CLK_GOUT_CAM_BUSD				5
> +#define CLK_GOUT_CAM_BTM_ACLK				6
> +#define CLK_GOUT_CAM_BTM_PCLK				7
> +#define CLK_GOUT_CAM_LHS_ATB_CAMISP_CLK			8
> +#define CLK_GOUT_CAM_IS6P10P0_ACLK_3AA			9
> +#define CLK_GOUT_CAM_IS6P10P0_ACLK_CSIS0		10
> +#define CLK_GOUT_CAM_IS6P10P0_ACLK_CSIS1		11
> +#define CLK_GOUT_CAM_IS6P10P0_ACLK_CSIS2		12
> +#define CLK_GOUT_CAM_IS6P10P0_ACLK_CSIS3		13
> +#define CLK_GOUT_CAM_IS6P10P0_ACLK_RDMA			14
> +#define CLK_GOUT_CAM_IS6P10P0_ACLK_GLUE_CSIS0		15
> +#define CLK_GOUT_CAM_IS6P10P0_ACLK_GLUE_CSIS1		16
> +#define CLK_GOUT_CAM_IS6P10P0_ACLK_GLUE_CSIS2		17
> +#define CLK_GOUT_CAM_IS6P10P0_ACLK_GLUE_CSIS3		18
> +#define CLK_GOUT_CAM_IS6P10P0_ACLK_PAFSTAT_CORE		19
> +#define CLK_GOUT_CAM_IS6P10P0_ACLK_PPMU_CAM		20
> +#define CLK_GOUT_CAM_IS6P10P0_ACLK_DMA			21
> +#define CLK_GOUT_CAM_IS6P10P0_ACLK_SMMU_CAM		22
> +#define CLK_GOUT_CAM_IS6P10P0_ACLK_XIU_D_CAM		23
> +#define CLK_GOUT_CAM_IS6P10P0_PCLK_PGEN_LITE_CAM0	24
> +#define CLK_GOUT_CAM_IS6P10P0_PCLK_PGEN_LITE_CAM1	25
> +#define CLK_GOUT_CAM_IS6P10P0_PCLK_PPMU_CAM		26
> +#define CLK_GOUT_CAM_LHM_AXI_P_CLK			27
> +#define CLK_GOUT_CAM_LHS_ACEL_D_CLK			28
> +#define CLK_GOUT_CAM_BUSD_CLK				29
> +#define CLK_GOUT_CAM_BUSP_CLK				30
> +#define CLK_GOUT_CAM_SYSREG_PCLK			31
> +
> +/* CMU_CMGP muxes */
> +#define CLK_MOUT_CLK_CMGP_ADC			0

Use consistent indexing - either start from 0 or from 1. All other cases
started from 1.

> +#define CLK_MOUT_CLK_CMGP_I2C			1
> +#define CLK_MOUT_CLK_CMGP_USI00			2
> +#define CLK_MOUT_CLK_CMGP_USI01			3
> +#define CLK_MOUT_CLK_CMGP_USI02			4
> +#define CLK_MOUT_CLK_CMGP_USI03			5
> +#define CLK_MOUT_CLK_CMGP_USI04			6

Best regards,
Krzysztof