[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a132b5d3-ae93-6ba6-e324-130897567477@amlogic.com>
Date: Thu, 24 Aug 2023 10:35:59 +0800
From: Yu Tu <yu.tu@...ogic.com>
To: Jerome Brunet <jbrunet@...libre.com>, linux-clk@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-amlogic@...ts.infradead.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, Rob Herring <robh+dt@...nel.org>,
Neil Armstrong <neil.armstrong@...aro.org>,
Kevin Hilman <khilman@...libre.com>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Martin Blumenstingl <martin.blumenstingl@...glemail.com>
Cc: kelvin.zhang@...ogic.com, qi.duan@...ogic.com
Subject: Re: [PATCH V10 2/4] dt-bindings: clock: document Amlogic S4 SoC
peripherals clock controller
On 2023/8/23 17:14, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On Wed 23 Aug 2023 at 16:32, Yu Tu <yu.tu@...ogic.com> wrote:
>
>> On 2023/8/23 16:01, Jerome Brunet wrote:
>>> [ EXTERNAL EMAIL ]
>>> On Tue 22 Aug 2023 at 16:27, Yu Tu <yu.tu@...ogic.com> wrote:
>>>
>>>> Add the S4 peripherals clock controller dt-bindings in the S4 SoC
>>>> family.
>>>>
>>>> Signed-off-by: Yu Tu <yu.tu@...ogic.com>
>>>> ---
>>>> .../clock/amlogic,s4-peripherals-clkc.yaml | 96 +++++++
>>>> .../clock/amlogic,s4-peripherals-clkc.h | 236 ++++++++++++++++++
>>>> 2 files changed, 332 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/clock/amlogic,s4-peripherals-clkc.yaml
>>>> create mode 100644 include/dt-bindings/clock/amlogic,s4-peripherals-clkc.h
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/clock/amlogic,s4-peripherals-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,s4-peripherals-clkc.yaml
>>>> new file mode 100644
>>>> index 000000000000..e166563e7b14
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/clock/amlogic,s4-peripherals-clkc.yaml
>>>> @@ -0,0 +1,96 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +# Copyright (C) 2022-2023 Amlogic, Inc. All rights reserved
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/clock/amlogic,s4-peripherals-clkc.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Amlogic S4 Peripherals Clock Controller
>>>> +
>>>> +maintainers:
>>>> + - Yu Tu <yu.tu@...ogic.com>
>>>> +
>>>> +properties:
>>>> + compatible:
>>>> + const: amlogic,s4-peripherals-clkc
>>>> +
>>>> + reg:
>>>> + maxItems: 1
>>>> +
>>>> + clocks:
>>>> + items:
>>>> + - description: input fixed pll div2
>>>> + - description: input fixed pll div2p5
>>>> + - description: input fixed pll div3
>>>> + - description: input fixed pll div4
>>>> + - description: input fixed pll div5
>>>> + - description: input fixed pll div7
>>>> + - description: input hifi pll
>>>> + - description: input gp0 pll
>>>> + - description: input mpll0
>>>> + - description: input mpll1
>>>> + - description: input mpll2
>>>> + - description: input mpll3
>>>> + - description: input hdmi pll
>>>> + - description: input oscillator (usually at 24MHz)
>>>> + - description: input external 32kHz reference (optional)
>>> The bindings described here does not make this last clock optional, but
>>> requires it. This is going to be a problem since most platforms won't
>>> have this clock.
>>
>> Hi Jerome,
>> Do you mean that we can delete the description of "external 32kHz",
>> because we hardly use it?
>
> Absolutely not.
> Optional ressources need description too.
OKay. I got it.
>
>>
>>> You are missing minItems.
>>> Same below
>>
>> I will add it in next version.
>>
>
> I'm saying that because you did not provide minItems here, it gets
> implicitly set to the number of clocks you have declared, making the
> external 32k a required clock, not an optional one.
>
> You need to set minItems so the clocks is actually optional.
> Try removing the 32k in your example below and check how it goes with
> 'make dt_binding_check'
thank you I see. I tried. I had to follow your advice, or there'd be a
warning.
>
>>>
>>>> +
>>>> + clock-names:
>>>> + items:
>>>> + - const: fclk_div2
>>>> + - const: fclk_div2p5
>>>> + - const: fclk_div3
>>>> + - const: fclk_div4
>>>> + - const: fclk_div5
>>>> + - const: fclk_div7
>>>> + - const: hifi_pll
>>>> + - const: gp0_pll
>>>> + - const: mpll0
>>>> + - const: mpll1
>>>> + - const: mpll2
>>>> + - const: mpll3
>>>> + - const: hdmi_pll
>>>> + - const: xtal
>>>> + - const: ext_32k
>>>> +
>>>> + "#clock-cells":
>>>> + const: 1
>>>> +
>>>> +required:
>>>> + - compatible
>>>> + - reg
>>>> + - clocks
>>>> + - clock-names
>>>> + - "#clock-cells"
>>>> +
>>>> +additionalProperties: false
>>>> +
>>>> +examples:
>>>> + - |
>>>> + #include <dt-bindings/clock/amlogic,s4-peripherals-clkc.h>
>>>> +
>>>> + clkc_periphs: clock-controller@...00000 {
>>>> + compatible = "amlogic,s4-peripherals-clkc";
>>>> + reg = <0xfe000000 0x49c>;
>>>> + clocks = <&clkc_pll 3>,
>>>> + <&clkc_pll 13>,
>>>> + <&clkc_pll 5>,
>>>> + <&clkc_pll 7>,
>>>> + <&clkc_pll 9>,
>>>> + <&clkc_pll 11>,
>>>> + <&clkc_pll 17>,
>>>> + <&clkc_pll 15>,
>>>> + <&clkc_pll 25>,
>>>> + <&clkc_pll 27>,
>>>> + <&clkc_pll 29>,
>>>> + <&clkc_pll 31>,
>>>> + <&clkc_pll 20>,
>>>> + <&xtal>,
>>>> + <&ext_32k>;
>>>> + clock-names = "fclk_div2", "fclk_div2p5", "fclk_div3", "fclk_div4",
>>>> + "fclk_div5", "fclk_div7", "hifi_pll", "gp0_pll",
>>>> + "mpll0", "mpll1", "mpll2", "mpll3", "hdmi_pll", "xtal",
>>>> + "ext_32k";
>>>> + #clock-cells = <1>;
>>>> + };
>>>> +...
>>>> diff --git a/include/dt-bindings/clock/amlogic,s4-peripherals-clkc.h b/include/dt-bindings/clock/amlogic,s4-peripherals-clkc.h
>>>> new file mode 100644
>>>> index 000000000000..861a331963ac
>>>> --- /dev/null
>>>> +++ b/include/dt-bindings/clock/amlogic,s4-peripherals-clkc.h
>>>> @@ -0,0 +1,236 @@
>>>> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
>>>> +/*
>>>> + * Copyright (c) 2022-2023 Amlogic, Inc. All rights reserved.
>>>> + * Author: Yu Tu <yu.tu@...ogic.com>
>>>> + */
>>>> +
>>>> +#ifndef _DT_BINDINGS_CLOCK_AMLOGIC_S4_PERIPHERALS_CLKC_H
>>>> +#define _DT_BINDINGS_CLOCK_AMLOGIC_S4_PERIPHERALS_CLKC_H
>>>> +
>>>> +#define CLKID_RTC_32K_CLKIN 0
>>>> +#define CLKID_RTC_32K_DIV 1
>>>> +#define CLKID_RTC_32K_SEL 2
>>>> +#define CLKID_RTC_32K_XATL 3
>>>> +#define CLKID_RTC 4
>>>> +#define CLKID_SYS_CLK_B_SEL 5
>>>> +#define CLKID_SYS_CLK_B_DIV 6
>>>> +#define CLKID_SYS_CLK_B 7
>>>> +#define CLKID_SYS_CLK_A_SEL 8
>>>> +#define CLKID_SYS_CLK_A_DIV 9
>>>> +#define CLKID_SYS_CLK_A 10
>>>> +#define CLKID_SYS 11
>>>> +#define CLKID_CECA_32K_CLKIN 12
>>>> +#define CLKID_CECA_32K_DIV 13
>>>> +#define CLKID_CECA_32K_SEL_PRE 14
>>>> +#define CLKID_CECA_32K_SEL 15
>>>> +#define CLKID_CECA_32K_CLKOUT 16
>>>> +#define CLKID_CECB_32K_CLKIN 17
>>>> +#define CLKID_CECB_32K_DIV 18
>>>> +#define CLKID_CECB_32K_SEL_PRE 19
>>>> +#define CLKID_CECB_32K_SEL 20
>>>> +#define CLKID_CECB_32K_CLKOUT 21
>>>> +#define CLKID_SC_CLK_SEL 22
>>>> +#define CLKID_SC_CLK_DIV 23
>>>> +#define CLKID_SC 24
>>>> +#define CLKID_12_24M 25
>>>> +#define CLKID_12M_CLK_DIV 26
>>>> +#define CLKID_12_24M_CLK_SEL 27
>>>> +#define CLKID_VID_PLL_DIV 28
>>>> +#define CLKID_VID_PLL_SEL 29
>>>> +#define CLKID_VID_PLL 30
>>>> +#define CLKID_VCLK_SEL 31
>>>> +#define CLKID_VCLK2_SEL 32
>>>> +#define CLKID_VCLK_INPUT 33
>>>> +#define CLKID_VCLK2_INPUT 34
>>>> +#define CLKID_VCLK_DIV 35
>>>> +#define CLKID_VCLK2_DIV 36
>>>> +#define CLKID_VCLK 37
>>>> +#define CLKID_VCLK2 38
>>>> +#define CLKID_VCLK_DIV1 39
>>>> +#define CLKID_VCLK_DIV2_EN 40
>>>> +#define CLKID_VCLK_DIV4_EN 41
>>>> +#define CLKID_VCLK_DIV6_EN 42
>>>> +#define CLKID_VCLK_DIV12_EN 43
>>>> +#define CLKID_VCLK2_DIV1 44
>>>> +#define CLKID_VCLK2_DIV2_EN 45
>>>> +#define CLKID_VCLK2_DIV4_EN 46
>>>> +#define CLKID_VCLK2_DIV6_EN 47
>>>> +#define CLKID_VCLK2_DIV12_EN 48
>>>> +#define CLKID_VCLK_DIV2 49
>>>> +#define CLKID_VCLK_DIV4 50
>>>> +#define CLKID_VCLK_DIV6 51
>>>> +#define CLKID_VCLK_DIV12 52
>>>> +#define CLKID_VCLK2_DIV2 53
>>>> +#define CLKID_VCLK2_DIV4 54
>>>> +#define CLKID_VCLK2_DIV6 55
>>>> +#define CLKID_VCLK2_DIV12 56
>>>> +#define CLKID_CTS_ENCI_SEL 57
>>>> +#define CLKID_CTS_ENCP_SEL 58
>>>> +#define CLKID_CTS_VDAC_SEL 59
>>>> +#define CLKID_HDMI_TX_SEL 60
>>>> +#define CLKID_CTS_ENCI 61
>>>> +#define CLKID_CTS_ENCP 62
>>>> +#define CLKID_CTS_VDAC 63
>>>> +#define CLKID_HDMI_TX 64
>>>> +#define CLKID_HDMI_SEL 65
>>>> +#define CLKID_HDMI_DIV 66
>>>> +#define CLKID_HDMI 67
>>>> +#define CLKID_TS_CLK_DIV 68
>>>> +#define CLKID_TS 69
>>>> +#define CLKID_MALI_0_SEL 70
>>>> +#define CLKID_MALI_0_DIV 71
>>>> +#define CLKID_MALI_0 72
>>>> +#define CLKID_MALI_1_SEL 73
>>>> +#define CLKID_MALI_1_DIV 74
>>>> +#define CLKID_MALI_1 75
>>>> +#define CLKID_MALI_SEL 76
>>>> +#define CLKID_VDEC_P0_SEL 77
>>>> +#define CLKID_VDEC_P0_DIV 78
>>>> +#define CLKID_VDEC_P0 79
>>>> +#define CLKID_VDEC_P1_SEL 80
>>>> +#define CLKID_VDEC_P1_DIV 81
>>>> +#define CLKID_VDEC_P1 82
>>>> +#define CLKID_VDEC_SEL 83
>>>> +#define CLKID_HEVCF_P0_SEL 84
>>>> +#define CLKID_HEVCF_P0_DIV 85
>>>> +#define CLKID_HEVCF_P0 86
>>>> +#define CLKID_HEVCF_P1_SEL 87
>>>> +#define CLKID_HEVCF_P1_DIV 88
>>>> +#define CLKID_HEVCF_P1 89
>>>> +#define CLKID_HEVCF_SEL 90
>>>> +#define CLKID_VPU_0_SEL 91
>>>> +#define CLKID_VPU_0_DIV 92
>>>> +#define CLKID_VPU_0 93
>>>> +#define CLKID_VPU_1_SEL 94
>>>> +#define CLKID_VPU_1_DIV 95
>>>> +#define CLKID_VPU_1 96
>>>> +#define CLKID_VPU 97
>>>> +#define CLKID_VPU_CLKB_TMP_SEL 98
>>>> +#define CLKID_VPU_CLKB_TMP_DIV 99
>>>> +#define CLKID_VPU_CLKB_TMP 100
>>>> +#define CLKID_VPU_CLKB_DIV 101
>>>> +#define CLKID_VPU_CLKB 102
>>>> +#define CLKID_VPU_CLKC_P0_SEL 103
>>>> +#define CLKID_VPU_CLKC_P0_DIV 104
>>>> +#define CLKID_VPU_CLKC_P0 105
>>>> +#define CLKID_VPU_CLKC_P1_SEL 106
>>>> +#define CLKID_VPU_CLKC_P1_DIV 107
>>>> +#define CLKID_VPU_CLKC_P1 108
>>>> +#define CLKID_VPU_CLKC_SEL 109
>>>> +#define CLKID_VAPB_0_SEL 110
>>>> +#define CLKID_VAPB_0_DIV 111
>>>> +#define CLKID_VAPB_0 112
>>>> +#define CLKID_VAPB_1_SEL 113
>>>> +#define CLKID_VAPB_1_DIV 114
>>>> +#define CLKID_VAPB_1 115
>>>> +#define CLKID_VAPB 116
>>>> +#define CLKID_GE2D 117
>>>> +#define CLKID_VDIN_MEAS_SEL 118
>>>> +#define CLKID_VDIN_MEAS_DIV 119
>>>> +#define CLKID_VDIN_MEAS 120
>>>> +#define CLKID_SD_EMMC_C_CLK_SEL 121
>>>> +#define CLKID_SD_EMMC_C_CLK_DIV 122
>>>> +#define CLKID_SD_EMMC_C 123
>>>> +#define CLKID_SD_EMMC_A_CLK_SEL 124
>>>> +#define CLKID_SD_EMMC_A_CLK_DIV 125
>>>> +#define CLKID_SD_EMMC_A 126
>>>> +#define CLKID_SD_EMMC_B_CLK_SEL 127
>>>> +#define CLKID_SD_EMMC_B_CLK_DIV 128
>>>> +#define CLKID_SD_EMMC_B 129
>>>> +#define CLKID_SPICC0_SEL 130
>>>> +#define CLKID_SPICC0_DIV 131
>>>> +#define CLKID_SPICC0_EN 132
>>>> +#define CLKID_PWM_A_SEL 133
>>>> +#define CLKID_PWM_A_DIV 134
>>>> +#define CLKID_PWM_A 135
>>>> +#define CLKID_PWM_B_SEL 136
>>>> +#define CLKID_PWM_B_DIV 137
>>>> +#define CLKID_PWM_B 138
>>>> +#define CLKID_PWM_C_SEL 139
>>>> +#define CLKID_PWM_C_DIV 140
>>>> +#define CLKID_PWM_C 141
>>>> +#define CLKID_PWM_D_SEL 142
>>>> +#define CLKID_PWM_D_DIV 143
>>>> +#define CLKID_PWM_D 144
>>>> +#define CLKID_PWM_E_SEL 145
>>>> +#define CLKID_PWM_E_DIV 146
>>>> +#define CLKID_PWM_E 147
>>>> +#define CLKID_PWM_F_SEL 148
>>>> +#define CLKID_PWM_F_DIV 149
>>>> +#define CLKID_PWM_F 150
>>>> +#define CLKID_PWM_G_SEL 151
>>>> +#define CLKID_PWM_G_DIV 152
>>>> +#define CLKID_PWM_G 153
>>>> +#define CLKID_PWM_H_SEL 154
>>>> +#define CLKID_PWM_H_DIV 155
>>>> +#define CLKID_PWM_H 156
>>>> +#define CLKID_PWM_I_SEL 157
>>>> +#define CLKID_PWM_I_DIV 158
>>>> +#define CLKID_PWM_I 159
>>>> +#define CLKID_PWM_J_SEL 160
>>>> +#define CLKID_PWM_J_DIV 161
>>>> +#define CLKID_PWM_J 162
>>>> +#define CLKID_SARADC_SEL 163
>>>> +#define CLKID_SARADC_DIV 164
>>>> +#define CLKID_SARADC 165
>>>> +#define CLKID_GEN_SEL 166
>>>> +#define CLKID_GEN_DIV 167
>>>> +#define CLKID_GEN 168
>>>> +#define CLKID_DDR 169
>>>> +#define CLKID_DOS 170
>>>> +#define CLKID_ETHPHY 171
>>>> +#define CLKID_MALI 172
>>>> +#define CLKID_AOCPU 173
>>>> +#define CLKID_AUCPU 174
>>>> +#define CLKID_CEC 175
>>>> +#define CLKID_SDEMMC_A 176
>>>> +#define CLKID_SDEMMC_B 177
>>>> +#define CLKID_NAND 178
>>>> +#define CLKID_SMARTCARD 179
>>>> +#define CLKID_ACODEC 180
>>>> +#define CLKID_SPIFC 181
>>>> +#define CLKID_MSR 182
>>>> +#define CLKID_IR_CTRL 183
>>>> +#define CLKID_AUDIO 184
>>>> +#define CLKID_ETH 185
>>>> +#define CLKID_UART_A 186
>>>> +#define CLKID_UART_B 187
>>>> +#define CLKID_UART_C 188
>>>> +#define CLKID_UART_D 189
>>>> +#define CLKID_UART_E 190
>>>> +#define CLKID_AIFIFO 191
>>>> +#define CLKID_TS_DDR 192
>>>> +#define CLKID_TS_PLL 193
>>>> +#define CLKID_G2D 194
>>>> +#define CLKID_SPICC0 195
>>>> +#define CLKID_SPICC1 196
>>>> +#define CLKID_USB 197
>>>> +#define CLKID_I2C_M_A 198
>>>> +#define CLKID_I2C_M_B 199
>>>> +#define CLKID_I2C_M_C 200
>>>> +#define CLKID_I2C_M_D 201
>>>> +#define CLKID_I2C_M_E 202
>>>> +#define CLKID_HDMITX_APB 203
>>>> +#define CLKID_I2C_S_A 204
>>>> +#define CLKID_USB1_TO_DDR 205
>>>> +#define CLKID_HDCP22 206
>>>> +#define CLKID_MMC_APB 207
>>>> +#define CLKID_RSA 208
>>>> +#define CLKID_CPU_DEBUG 209
>>>> +#define CLKID_VPU_INTR 210
>>>> +#define CLKID_DEMOD 211
>>>> +#define CLKID_SAR_ADC 212
>>>> +#define CLKID_GIC 213
>>>> +#define CLKID_PWM_AB 214
>>>> +#define CLKID_PWM_CD 215
>>>> +#define CLKID_PWM_EF 216
>>>> +#define CLKID_PWM_GH 217
>>>> +#define CLKID_PWM_IJ 218
>>>> +#define CLKID_HDCP22_ESMCLK_SEL 219
>>>> +#define CLKID_HDCP22_ESMCLK_DIV 220
>>>> +#define CLKID_HDCP22_ESMCLK 221
>>>> +#define CLKID_HDCP22_SKPCLK_SEL 222
>>>> +#define CLKID_HDCP22_SKPCLK_DIV 223
>>>> +#define CLKID_HDCP22_SKPCLK 224
>>>> +
>>>> +#endif /* _DT_BINDINGS_CLOCK_AMLOGIC_S4_PERIPHERALS_CLKC_H */
>>>
>
Powered by blists - more mailing lists