Re: [PATCH 2/5] dt-bindings: clock: altr,agilex72-clkmgr: add Agilex72 clock manager binding

[Krzysztof Kozlowski <krzk@kernel.org>]

On 06/08/2026 11:25, evon.chiam@altera.com wrote:
> From: "Chiam, Evon" <evon.chiam@altera.com>
> 
> Add binding and clock ID header for Altera SoCFPGA Agilex72 clock manager.

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

A nit, subject: drop second/last, redundant "binding". The "dt-bindings"
prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v7.1-rc7/source/Documentation/devicetree/bindings/submitting-patches.rst#L23

> 
> Signed-off-by: Chiam, Evon <evon.chiam@altera.com>



> +
> +examples:
> +  - |
> +    /* External reference clocks supplied to the clock manager */
> +    clocks {
> +      cb_intosc_div2_clk: cb-intosc-div2-clk {
> +        #clock-cells = <0>;
> +        compatible = "fixed-clock";
> +        clock-frequency = <0>;
> +      };
> +
> +      /* cb-intosc divided by 2, then by 5 (total divide-by-10) */
> +      cb_intosc_div10_clk: cb-intosc-div10-clk {
> +        #clock-cells = <0>;
> +        compatible = "fixed-factor-clock";
> +        clocks = <&cb_intosc_div2_clk>;
> +        clock-mult = <1>;
> +        clock-div = <5>;
> +      };
> +
> +      f2s_free_clk: f2s-free-clk {
> +        #clock-cells = <0>;
> +        compatible = "fixed-clock";
> +        clock-frequency = <0>;
> +      };
> +
> +      osc1: osc1 {
> +        #clock-cells = <0>;
> +        compatible = "fixed-clock";
> +        clock-frequency = <0>;
> +      };
> +    };

None of above are relevant. Drop.

> +
> +    clkmgr: clock-controller@915c000 {
> +      compatible = "altr,agilex72-clkmgr";
> +      reg = <0x915c000 0x1000>,
> +            <0x915e000 0x1000>,
> +            <0x915f000 0x1000>,
> +            <0x9160000 0x1000>;
> +      reg-names = "clkmgr", "gppll0", "gppll1", "gppll2";
> +      #clock-cells = <1>;
> +    };


...

> +#define AGILEX72_COMP0_CLK             36
> +#define AGILEX72_CORE2_CLK             37
> +#define AGILEX72_CORE3_CLK             38
> +#define AGILEX72_MPU_CLK               39
> +#define AGILEX72_CCU_CLK               40
> +#define AGILEX72_APU_SYS_FREE_CLK      41
> +#define AGILEX72_HSP_SYS_FREE_CLK      42
> +#define AGILEX72_HSP_MAIN_FREE_CLK     43
> +#define AGILEX72_HSP_MAIN_CLK          44
> +#define AGILEX72_HSP_MP_CLK            45
> +#define AGILEX72_HSP_SP_CLK            46
> +#define AGILEX72_USB2OTG_HCLK          47
> +#define AGILEX72_LSP_SYS_FREE_CLK      48
> +#define AGILEX72_LSP_MAIN_FREE_CLK     49
> +#define AGILEX72_LSP_MAIN_CLK          50
> +#define AGILEX72_LSP_MP_CLK            51
> +#define AGILEX72_LSP_SP_CLK            52
> +#define AGILEX72_SPIM_0_CLK            53
> +#define AGILEX72_SPIM_1_CLK            54
> +#define AGILEX72_SPIS_0_CLK            55
> +#define AGILEX72_SPIS_1_CLK            56
> +#define AGILEX72_DMA_0_CORE_CLK        57
> +#define AGILEX72_DMA_0_HS_CLK          58
> +#define AGILEX72_DMA_1_CORE_CLK        59
> +#define AGILEX72_DMA_1_HS_CLK          60
> +#define AGILEX72_I3C_0_CORE_CLK        61
> +#define AGILEX72_I3C_1_CORE_CLK        62
> +#define AGILEX72_I2C_0_PCLK            63
> +#define AGILEX72_I2C_1_PCLK            64
> +#define AGILEX72_I2C_EMAC0_PCLK        65
> +#define AGILEX72_I2C_EMAC1_PCLK        66
> +#define AGILEX72_I2C_EMAC2_PCLK        67
> +#define AGILEX72_UART_0_PCLK           68
> +#define AGILEX72_UART_1_PCLK           69
> +#define AGILEX72_UART_2_PCLK           70
> +#define AGILEX72_SPTIMER_0_PCLK        71
> +#define AGILEX72_SPTIMER_1_PCLK        72
> +#define AGILEX72_CS_AT_CLK             73
> +#define AGILEX72_CS_PDBG_CLK           74
> +#define AGILEX72_CS_TRACE_CLK          75
> +#define AGILEX72_EMACA_DIV_CLK         76
> +#define AGILEX72_EMACB_DIV_CLK         77
> +#define AGILEX72_EMAC0_CLK             78
> +#define AGILEX72_EMAC1_CLK             79
> +#define AGILEX72_EMAC2_CLK             80
> +#define AGILEX72_EMAC_PTP_CLK          81
> +#define AGILEX72_GPIO_DB_CLK           82
> +#define AGILEX72_USB31_SUSPEND_CLK     83
> +#define AGILEX72_USB31_BUS_CLK_EARLY   84
> +#define AGILEX72_S2F_USER0_CLK         85
> +#define AGILEX72_S2F_USER1_CLK         86
> +#define AGILEX72_XSPI_PCLK             87
> +#define AGILEX72_XSPI_CLK              88
> +#define AGILEX72_XSPI_PHY_CLK          89
> +#define AGILEX72_SDMMC0_SDPHY_REG_CLK  90
> +#define AGILEX72_SDMMC1_SDPHY_REG_CLK  91
> +#define AGILEX72_SDMMC0_SDMCLK         92
> +#define AGILEX72_SDMMC1_SDMCLK         93
> +#define AGILEX72_SDMMC0_PHY_CLK        94
> +#define AGILEX72_SDMMC1_PHY_CLK        95
> +#define AGILEX72_USB31_REF_CLK         96

> +#define AGILEX72_NUM_CLKS              97

No, not a binding. Drop

> +
> +#endif  /* __DT_BINDINGS_ALTR_AGILEX72_CLKMGR_H */


Best regards,
Krzysztof