linux-clk.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: dongxuyang@eswincomputing.com, mturquette@baylibre.com,
	sboyd@kernel.org, robh@kernel.org, krzk+dt@kernel.org,
	conor+dt@kernel.org, linux-clk@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: ningyu@eswincomputing.com, linmin@eswincomputing.com,
	huangyifeng@eswincomputing.com
Subject: Re: [PATCH v3 1/2] dt-bindings: clock: eswin: Documentation for eic7700 SoC
Date: Tue, 24 Jun 2025 13:00:53 +0200	[thread overview]
Message-ID: <cd98add5-dd2f-455f-b534-c83e62a97bd0@kernel.org> (raw)
In-Reply-To: <20250624103256.345-1-dongxuyang@eswincomputing.com>

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

  reply	other threads:[~2025-06-24 11:00 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-24 10:32 [PATCH v3 0/2] Add driver support for ESWIN eic700 SoC clock controller dongxuyang
2025-06-24 10:32 ` [PATCH v3 1/2] dt-bindings: clock: eswin: Documentation for eic7700 SoC dongxuyang
2025-06-24 11:00   ` Krzysztof Kozlowski [this message]
2025-06-25  6:07     ` Krzysztof Kozlowski
2025-06-24 10:33 ` [PATCH v3 2/2] clock: eswin: Add eic7700 clock driver dongxuyang
2025-06-24 11:04   ` Krzysztof Kozlowski
2025-07-04  9:46     ` 董绪洋
2025-07-07  9:12   ` Bo Gan
2025-07-08  9:09     ` Xuyang Dong
2025-07-09 22:52       ` Bo Gan
2025-07-10  0:52         ` Xuyang Dong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cd98add5-dd2f-455f-b534-c83e62a97bd0@kernel.org \
    --to=krzk@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dongxuyang@eswincomputing.com \
    --cc=huangyifeng@eswincomputing.com \
    --cc=krzk+dt@kernel.org \
    --cc=linmin@eswincomputing.com \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mturquette@baylibre.com \
    --cc=ningyu@eswincomputing.com \
    --cc=robh@kernel.org \
    --cc=sboyd@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).