Re: [PATCH v3 1/2] dt-bindings: clock: eswin: Documentation for eic7700 SoC

[Krzysztof Kozlowski <krzk@kernel.org>]

On 24/06/2025 12:32, dongxuyang@eswincomputing.com wrote:
> +
> +properties:
> +  compatible:
> +    const: eswin,eic7700-clock
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#clock-cells':
> +    const: 1
> +
> +  cpu-default-frequency:

Frequency has a type - hz, use it as unit suffix if this stays.

> +    $ref: /schemas/types.yaml#/definitions/uint32

Drop

Anyway, why do you need it? Why would this be a different per board and
why would you ever need to encode it in DT? If firmware initializes
device, just the registers.

> +    default: 1400000000
> +
> +required:
> +  - compatible
> +  - reg
> +  - '#clock-cells'


... and your example is not complete. Add all properties there.

...

> +#define EIC7700_MUX_U_VI_SHUTTER_XTAL_2MUX1_4	68
> +#define EIC7700_MUX_U_VI_SHUTTER_XTAL_2MUX1_5	69
> +#define EIC7700_MUX_U_VO_ACLK_XTAL_2MUX1		70
> +#define EIC7700_MUX_U_IESMCLK_XTAL_2MUX1		71
> +#define EIC7700_MUX_U_VO_PIXEL_XTAL_2MUX1		72
> +#define EIC7700_MUX_U_VO_MCLK_2MUX_EXT_MCLK		73
> +#define EIC7700_MUX_U_VC_ACLK_XTAL_2MUX1		74
> +#define EIC7700_MUX_U_JD_XTAL_2MUX1		75
> +#define EIC7700_MUX_U_JE_XTAL_2MUX1		76
> +#define EIC7700_MUX_U_VE_XTAL_2MUX1		77
> +#define EIC7700_MUX_U_VD_XTAL_2MUX1		78
> +#define EIC7700_MUX_U_SATA_PHY_2MUX1		79
> +#define EIC7700_MUX_U_AONDMA_AXI2MUX1_GFREE		80
> +#define EIC7700_MUX_U_CRYPTO_XTAL_2MUX1		81
> +#define EIC7700_MUX_U_RMII_REF_2MUX			82
> +#define EIC7700_MUX_U_ETH_CORE_2MUX1		83
> +#define EIC7700_MUX_U_VI_DW_ROOT_2MUX1		84
> +#define EIC7700_MUX_U_VI_DW_XTAL_2MUX1		85
> +#define EIC7700_MUX_U_NPU_E31_3MUX1_GFREE		86
> +#define EIC7700_MUX_U_DDR_ACLK_ROOT_2MUX1_GFREE		87
> +
> +/* divider clocks */
> +#define EIC7700_DIVDER_U_SYS_CFG_DIV_DYNM		100

DIVIDER... or just shorter DIV

> +#define EIC7700_DIVDER_U_NOC_NSP_DIV_DYNM		101
> +#define EIC7700_DIVDER_U_BOOTSPI_DIV_DYNM		102
> +#define EIC7700_DIVDER_U_SCPU_CORE_DIV_DYNM		103
> +#define EIC7700_DIVDER_U_LPCPU_CORE_DIV_DYNM	104
> +#define EIC7700_DIVDER_U_GPU_ACLK_DIV_DYNM		105
> +#define EIC7700_DIVDER_U_DSP_ACLK_DIV_DYNM		106
> +#define EIC7700_DIVDER_U_D2D_ACLK_DIV_DYNM		107
> +#define EIC7700_DIVDER_U_HSP_ACLK_DIV_DYNM		108
> +#define EIC7700_DIVDER_U_ETH_TXCLK_DIV_DYNM_0	109
> +#define EIC7700_DIVDER_U_ETH_TXCLK_DIV_DYNM_1	110
> +#define EIC7700_DIVDER_U_MSHC_CORE_DIV_DYNM_0	111
> +#define EIC7700_DIVDER_U_MSHC_CORE_DIV_DYNM_1	112
> +#define EIC7700_DIVDER_U_MSHC_CORE_DIV_DYNM_2	113
> +#define EIC7700_DIVDER_U_PCIE_ACLK_DIV_DYNM		114
> +#define EIC7700_DIVDER_U_NPU_ACLK_DIV_DYNM		115
> +#define EIC7700_DIVDER_U_NPU_LLC_SRC0_DIV_DYNM	116
> +#define EIC7700_DIVDER_U_NPU_LLC_SRC1_DIV_DYNM	117
> +#define EIC7700_DIVDER_U_NPU_CORECLK_DIV_DYNM	118
> +#define EIC7700_DIVDER_U_VI_ACLK_DIV_DYNM		119
> +#define EIC7700_DIVDER_U_VI_DVP_DIV_DYNM		120
> +#define EIC7700_DIVDER_U_VI_DIG_ISP_DIV_DYNM	121
> +#define EIC7700_DIVDER_U_VI_SHUTTER_DIV_DYNM_0	122
> +#define EIC7700_DIVDER_U_VI_SHUTTER_DIV_DYNM_1	123
> +#define EIC7700_DIVDER_U_VI_SHUTTER_DIV_DYNM_2	124
> +#define EIC7700_DIVDER_U_VI_SHUTTER_DIV_DYNM_3	125
> +#define EIC7700_DIVDER_U_VI_SHUTTER_DIV_DYNM_4	126
> +#define EIC7700_DIVDER_U_VI_SHUTTER_DIV_DYNM_5	127
> +#define EIC7700_DIVDER_U_VO_ACLK_DIV_DYNM		128
> +#define EIC7700_DIVDER_U_IESMCLK_DIV_DYNM		129
> +#define EIC7700_DIVDER_U_VO_PIXEL_DIV_DYNM		130
> +#define EIC7700_DIVDER_U_VO_MCLK_DIV_DYNM		131
> +#define EIC7700_DIVDER_U_VC_ACLK_DIV_DYNM		132
> +#define EIC7700_DIVDER_U_JD_DIV_DYNM		133
> +#define EIC7700_DIVDER_U_JE_DIV_DYNM		134
> +#define EIC7700_DIVDER_U_VE_DIV_DYNM		135
> +#define EIC7700_DIVDER_U_VD_DIV_DYNM		136
> +#define EIC7700_DIVDER_U_G2D_DIV_DYNM		137
> +#define EIC7700_DIVDER_U_AONDMA_AXI_DIV_DYNM	138
> +#define EIC7700_DIVDER_U_CRYPTO_DIV_DYNM	139
> +#define EIC7700_DIVDER_U_VI_DW_DIV_DYNM		140
> +#define EIC7700_DIVDER_U_NPU_E31_DIV_DYNM	141
> +#define EIC7700_DIVDER_U_SATA_PHY_REF_DIV_DYNM	142
> +#define EIC7700_DIVDER_U_DSP_0_ACLK_DIV_DYNM	143
> +#define EIC7700_DIVDER_U_DSP_1_ACLK_DIV_DYNM	144
> +#define EIC7700_DIVDER_U_DSP_2_ACLK_DIV_DYNM	145
> +#define EIC7700_DIVDER_U_DSP_3_ACLK_DIV_DYNM	146
> +#define EIC7700_DIVDER_U_DDR_ACLK_DIV_DYNM		147
> +#define EIC7700_DIVDER_U_AON_RTC_DIV_DYNM		148
> +#define EIC7700_DIVDER_U_U84_RTC_TOGGLE_DIV_DYNM	149
> +#define EIC7700_DIVDER_U_VO_CEC_DIV_DYNM		150
> +
> +/* gate clocks */
> +#define EIC7700_GATE_CLK_CPU_EXT_SRC_CORE_CLK_0	200

151, there should be no gaps in IDs.

> +#define EIC7700_GATE_CLK_CPU_EXT_SRC_CORE_CLK_1	201
> +#define EIC7700_GATE_CLK_CPU_EXT_SRC_CORE_CLK_2	202
> +#define EIC7700_GATE_CLK_CPU_EXT_SRC_CORE_CLK_3	203


...

> +
> +#define EIC7700_CLK_VC_JE_PCLK		685
> +#define EIC7700_CLK_VC_JD_PCLK		686
> +#define EIC7700_CLK_VC_VE_PCLK		687
> +#define EIC7700_CLK_VC_VD_PCLK		688
> +#define EIC7700_CLK_VC_MON_PCLK		689
> +
> +#define EIC7700_CLK_HSP_DMA0_CLK		690
> +#define EIC7700_CLK_HSP_DMA0_CLK_TEST	691
> +
> +#define EIC7700_NR_CLKS		700

Drop

> +
> +#endif /* _DT_BINDINGS_ESWIN_EIC7700_CLOCK_H_ */


Best regards,
Krzysztof