RE: [PATCH v2 1/4] dt-bindings: clock: add Exynos Auto v920 SoC CMU bindings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

Hello Alim,

> -----Original Message-----
> From: Alim Akhtar <alim.akhtar@xxxxxxxxxxx>
> Sent: Monday, July 8, 2024 7:30 PM
> To: 'Sunyeal Hong' <sunyeal.hong@xxxxxxxxxxx>; 'Krzysztof Kozlowski'
> <krzk@xxxxxxxxxx>; 'Sylwester Nawrocki' <s.nawrocki@xxxxxxxxxxx>; 'Chanwoo
> Choi' <cw00.choi@xxxxxxxxxxx>; 'Michael Turquette'
> <mturquette@xxxxxxxxxxxx>; 'Stephen Boyd' <sboyd@xxxxxxxxxx>; 'Rob
> Herring' <robh@xxxxxxxxxx>; 'Conor Dooley' <conor+dt@xxxxxxxxxx>
> Cc: linux-samsung-soc@xxxxxxxxxxxxxxx; linux-clk@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: RE: [PATCH v2 1/4] dt-bindings: clock: add Exynos Auto v920 SoC
> CMU bindings
> 
> Hello Sunyeal
> 
> > -----Original Message-----
> > From: Sunyeal Hong <sunyeal.hong@xxxxxxxxxxx>
> > Sent: Monday, July 8, 2024 4:43 AM
> > To: Krzysztof Kozlowski <krzk@xxxxxxxxxx>; Sylwester Nawrocki
> > <s.nawrocki@xxxxxxxxxxx>; Chanwoo Choi <cw00.choi@xxxxxxxxxxx>; Alim
> > Akhtar <alim.akhtar@xxxxxxxxxxx>; Michael Turquette
> > <mturquette@xxxxxxxxxxxx>; Stephen Boyd <sboyd@xxxxxxxxxx>; Rob
> > Herring <robh@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>
> > Cc: linux-samsung-soc@xxxxxxxxxxxxxxx; linux-clk@xxxxxxxxxxxxxxx;
> > devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> > linux- kernel@xxxxxxxxxxxxxxx; Sunyeal Hong <sunyeal.hong@xxxxxxxxxxx>
> > Subject: [PATCH v2 1/4] dt-bindings: clock: add Exynos Auto v920 SoC
> > CMU bindings
> >
> > Add dt-schema for Exynos Auto v920 SoC clock controller.
> Prefer to have Exynos Auto -> ExynosAuto to match with the naming
> convention and the UM.
> 
Okay, I will update.
> > Add device tree clock binding definitions for below CMU blocks.
> >
> > - CMU_TOP
> > - CMU_PERIC0
> >
> > Signed-off-by: Sunyeal Hong <sunyeal.hong@xxxxxxxxxxx>
> > ---
> >  .../clock/samsung,exynosautov920-clock.yaml   | 115 +++++++++++
> >  .../clock/samsung,exynosautov920.h            | 191 ++++++++++++++++++
> >  2 files changed, 306 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/clock/samsung,exynosautov920-
> > clock.yaml
> >  create mode 100644 include/dt-bindings/clock/samsung,exynosautov920.h
> >
> > diff --git
> > a/Documentation/devicetree/bindings/clock/samsung,exynosautov920-
> > clock.yaml
> > b/Documentation/devicetree/bindings/clock/samsung,exynosautov920-
> > clock.yaml
> > new file mode 100644
> > index 000000000000..ade74d6e90c0
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/samsung,exynosautov920-
> > clo
> > +++ ck.yaml
> > @@ -0,0 +1,115 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +http://devicetree.org/schemas/clock/samsung,exynosautov920-
> > clock.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Samsung Exynos Auto v920 SoC clock controller
> > +
> > +maintainers:
> > +  - Sunyeal Hong <sunyeal.hong@xxxxxxxxxxx>
> > +  - Chanwoo Choi <cw00.choi@xxxxxxxxxxx>
> > +  - Krzysztof Kozlowski <krzk@xxxxxxxxxx>
> > +  - Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>
> > +
> > +description: |
> > +  Exynos Auto v920 clock controller is comprised of several CMU
> > +units, generating
> > +  clocks for different domains. Those CMU units are modeled as
> > +separate device
> > +  tree nodes, and might depend on each other. Root clocks in that
> > +clock tree are
> > +  two external clocks:: OSCCLK/XTCXO (38.4 MHz) and RTCCLK/XrtcXTI
> > (32768 Hz).
> > +  The external OSCCLK must be defined as fixed-rate clock in dts.
> > +
> > +  CMU_TOP is a top-level CMU, where all base clocks are prepared
> > + using PLLs and  dividers; all other clocks of function blocks (other
> > + CMUs) are usually  derived from CMU_TOP.
> > +
> > +  Each clock is assigned an identifier and client nodes can use this
> > + identifier  to specify the clock which they consume. All clocks
> > + available for usage  in clock consumer nodes are defined as
> > + preprocessor macros in  'include/dt-
> > bindings/clock/samsung,exynosautov920.h' header.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - samsung,exynosautov920-cmu-top
> > +      - samsung,exynosautov920-cmu-peric0
> > +
> > +  clocks:
> > +    minItems: 1
> > +    maxItems: 3
> > +
> > +  clock-names:
> > +    minItems: 1
> > +    maxItems: 3
> > +
> > +  "#clock-cells":
> > +    const: 1
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: samsung,exynosautov920-cmu-top
> > +
> > +    then:
> > +      properties:
> > +        clocks:
> > +          items:
> > +            - description: External reference clock (38.4 MHz)
> > +
> > +        clock-names:
> > +          items:
> > +            - const: oscclk
> > +
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: samsung,exynosautov920-cmu-peric0
> > +
> > +    then:
> > +      properties:
> > +        clocks:
> > +          items:
> > +            - description: External reference clock (38.4 MHz)
> > +            - description: CMU_PERIC0 NOC clock (from CMU_TOP)
> > +            - description: CMU_PERIC0 IP clock (from CMU_TOP)
> > +
> > +        clock-names:
> > +          items:
> > +            - const: oscclk
> > +            - const: noc
> > +            - const: ip
> These are too generic name, please add peric0_noc and peric0_ip, and this
> is to match with the UM.
> I am sure in future you would like to add other IPs like USI, I2C etc for
> the peric0 block
Like Jaewon and Rob's reviews, I think it's right to use a general clock name.
> > +
> > +required:
> > +  - compatible
> > +  - "#clock-cells"
> > +  - clocks
> > +  - clock-names
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  # Clock controller node for CMU_PERIC0
> > +  - |
> > +    #include <dt-bindings/clock/samsung,exynosautov920.h>
> > +
> > +    cmu_peric0: clock-controller@10800000 {
> > +        compatible = "samsung,exynosautov920-cmu-peric0";
> > +        reg = <0x10800000 0x8000>;
> > +        #clock-cells = <1>;
> > +
> > +        clocks = <&xtcxo>,
> > +                 <&cmu_top DOUT_CLKCMU_PERIC0_NOC>,
> > +                 <&cmu_top DOUT_CLKCMU_PERIC0_IP>;
> > +        clock-names = "oscclk",
> > +                      "noc",
> > +                      "ip";
> > +    };
> > +
> > +...
> > diff --git a/include/dt-bindings/clock/samsung,exynosautov920.h
> > b/include/dt-bindings/clock/samsung,exynosautov920.h
> > new file mode 100644
> > index 000000000000..9daa617c3636
> > --- /dev/null
> > +++ b/include/dt-bindings/clock/samsung,exynosautov920.h
> > @@ -0,0 +1,191 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> > +/*
> > + * Copyright (c) 2024 Samsung Electronics Co., Ltd.
> > + * Author: Sunyeal Hong <sunyeal.hong@xxxxxxxxxxx>
> > + *
> > + * Device Tree binding constants for Exynos Auto V920 clock controller.
> > + */
> > +
> > +#ifndef _DT_BINDINGS_CLOCK_EXYNOSAUTOV920_H
> > +#define _DT_BINDINGS_CLOCK_EXYNOSAUTOV920_H
> > +
> > +/* CMU_TOP */
> > +#define FOUT_SHARED0_PLL		1
> > +#define FOUT_SHARED1_PLL		2
> > +#define FOUT_SHARED2_PLL		3
> > +#define FOUT_SHARED3_PLL		4
> > +#define FOUT_SHARED4_PLL		5
> > +#define FOUT_SHARED5_PLL		6
> > +#define FOUT_MMC_PLL			7
> > +
> > +/* MUX in CMU_TOP */
> > +#define MOUT_SHARED0_PLL		101
> > +#define MOUT_SHARED1_PLL		102
> > +#define MOUT_SHARED2_PLL		103
> > +#define MOUT_SHARED3_PLL		104
> > +#define MOUT_SHARED4_PLL		105
> > +#define MOUT_SHARED5_PLL		106
> > +#define MOUT_MMC_PLL			107
> > +#define MOUT_CLKCMU_CMU_BOOST		108
> > +#define MOUT_CLKCMU_CMU_CMUREF		109
> > +#define MOUT_CLKCMU_ACC_NOC		110
> > +#define MOUT_CLKCMU_ACC_ORB		111
> > +#define MOUT_CLKCMU_APM_NOC		112
> > +#define MOUT_CLKCMU_AUD_CPU		113
> > +#define MOUT_CLKCMU_AUD_NOC		114
> > +#define MOUT_CLKCMU_CPUCL0_SWITCH	115
> > +#define MOUT_CLKCMU_CPUCL0_CLUSTER	116
> > +#define MOUT_CLKCMU_CPUCL0_DBG		117
> > +#define MOUT_CLKCMU_CPUCL1_SWITCH	118
> > +#define MOUT_CLKCMU_CPUCL1_CLUSTER	119
> > +#define MOUT_CLKCMU_CPUCL2_SWITCH	120
> > +#define MOUT_CLKCMU_CPUCL2_CLUSTER	121
> > +#define MOUT_CLKCMU_DNC_NOC		122
> > +#define MOUT_CLKCMU_DPTX_NOC		123
> > +#define MOUT_CLKCMU_DPTX_DPGTC		124
> > +#define MOUT_CLKCMU_DPTX_DPOSC		125
> > +#define MOUT_CLKCMU_DPUB_NOC		126
> > +#define MOUT_CLKCMU_DPUB_DSIM		127
> > +#define MOUT_CLKCMU_DPUF0_NOC		128
> > +#define MOUT_CLKCMU_DPUF1_NOC		129
> > +#define MOUT_CLKCMU_DPUF2_NOC		130
> > +#define MOUT_CLKCMU_DSP_NOC		131
> > +#define MOUT_CLKCMU_G3D_SWITCH		132
> > +#define MOUT_CLKCMU_G3D_NOCP		133
> > +#define MOUT_CLKCMU_GNPU_NOC		134
> > +#define MOUT_CLKCMU_HSI0_NOC		135
> > +#define MOUT_CLKCMU_HSI1_NOC		136
> > +#define MOUT_CLKCMU_HSI1_USBDRD		137
> > +#define MOUT_CLKCMU_HSI1_MMC_CARD	138
> > +#define MOUT_CLKCMU_HSI2_NOC		139
> > +#define MOUT_CLKCMU_HSI2_NOC_UFS	140
> > +#define MOUT_CLKCMU_HSI2_UFS_EMBD	141
> > +#define MOUT_CLKCMU_HSI2_ETHERNET	142
> > +#define MOUT_CLKCMU_ISP_NOC		143
> > +#define MOUT_CLKCMU_M2M_NOC		144
> > +#define MOUT_CLKCMU_M2M_JPEG		145
> > +#define MOUT_CLKCMU_MFC_MFC		146
> > +#define MOUT_CLKCMU_MFC_WFD		147
> > +#define MOUT_CLKCMU_MFD_NOC		148
> > +#define MOUT_CLKCMU_MIF_SWITCH		149
> > +#define MOUT_CLKCMU_MIF_NOCP		150
> > +#define MOUT_CLKCMU_MISC_NOC		151
> > +#define MOUT_CLKCMU_NOCL0_NOC		152
> > +#define MOUT_CLKCMU_NOCL1_NOC		153
> > +#define MOUT_CLKCMU_NOCL2_NOC		154
> > +#define MOUT_CLKCMU_PERIC0_NOC		155
> > +#define MOUT_CLKCMU_PERIC0_IP		156
> > +#define MOUT_CLKCMU_PERIC1_NOC		157
> > +#define MOUT_CLKCMU_PERIC1_IP		158
> > +#define MOUT_CLKCMU_SDMA_NOC		159
> > +#define MOUT_CLKCMU_SNW_NOC		160
> > +#define MOUT_CLKCMU_SSP_NOC		161
> > +#define MOUT_CLKCMU_TAA_NOC		162
> > +
> > +/* DIV in CMU_TOP */
> > +#define DOUT_SHARED0_DIV1		201
> > +#define DOUT_SHARED0_DIV2		202
> > +#define DOUT_SHARED0_DIV3		203
> > +#define DOUT_SHARED0_DIV4		204
> > +#define DOUT_SHARED1_DIV1		205
> > +#define DOUT_SHARED1_DIV2		206
> > +#define DOUT_SHARED1_DIV3		207
> > +#define DOUT_SHARED1_DIV4		208
> > +#define DOUT_SHARED2_DIV1		209
> > +#define DOUT_SHARED2_DIV2		210
> > +#define DOUT_SHARED2_DIV3		211
> > +#define DOUT_SHARED2_DIV4		212
> > +#define DOUT_SHARED3_DIV1		213
> > +#define DOUT_SHARED3_DIV2		214
> > +#define DOUT_SHARED3_DIV3		215
> > +#define DOUT_SHARED3_DIV4		216
> > +#define DOUT_SHARED4_DIV1		217
> > +#define DOUT_SHARED4_DIV2		218
> > +#define DOUT_SHARED4_DIV3		219
> > +#define DOUT_SHARED4_DIV4		220
> > +#define DOUT_SHARED5_DIV1		221
> > +#define DOUT_SHARED5_DIV2		222
> > +#define DOUT_SHARED5_DIV3		223
> > +#define DOUT_SHARED5_DIV4		224
> > +#define DOUT_CLKCMU_CMU_BOOST		225
> > +#define DOUT_CLKCMU_ACC_NOC		226
> > +#define DOUT_CLKCMU_ACC_ORB		227
> > +#define DOUT_CLKCMU_APM_NOC		228
> > +#define DOUT_CLKCMU_AUD_CPU		229
> > +#define DOUT_CLKCMU_AUD_NOC		230
> > +#define DOUT_CLKCMU_CPUCL0_SWITCH	231
> > +#define DOUT_CLKCMU_CPUCL0_CLUSTER	232
> > +#define DOUT_CLKCMU_CPUCL0_DBG		233
> > +#define DOUT_CLKCMU_CPUCL1_SWITCH	234
> > +#define DOUT_CLKCMU_CPUCL1_CLUSTER	235
> > +#define DOUT_CLKCMU_CPUCL2_SWITCH	236
> > +#define DOUT_CLKCMU_CPUCL2_CLUSTER	237
> > +#define DOUT_CLKCMU_DNC_NOC		238
> > +#define DOUT_CLKCMU_DPTX_NOC		239
> > +#define DOUT_CLKCMU_DPTX_DPGTC		240
> > +#define DOUT_CLKCMU_DPTX_DPOSC		241
> > +#define DOUT_CLKCMU_DPUB_NOC		242
> > +#define DOUT_CLKCMU_DPUB_DSIM		243
> > +#define DOUT_CLKCMU_DPUF0_NOC		244
> > +#define DOUT_CLKCMU_DPUF1_NOC		245
> > +#define DOUT_CLKCMU_DPUF2_NOC		246
> > +#define DOUT_CLKCMU_DSP_NOC		247
> > +#define DOUT_CLKCMU_G3D_SWITCH		248
> > +#define DOUT_CLKCMU_G3D_NOCP		249
> > +#define DOUT_CLKCMU_GNPU_NOC		250
> > +#define DOUT_CLKCMU_HSI0_NOC		251
> > +#define DOUT_CLKCMU_HSI1_NOC		252
> > +#define DOUT_CLKCMU_HSI1_USBDRD		253
> > +#define DOUT_CLKCMU_HSI1_MMC_CARD	254
> > +#define DOUT_CLKCMU_HSI2_NOC		255
> > +#define DOUT_CLKCMU_HSI2_NOC_UFS	256
> > +#define DOUT_CLKCMU_HSI2_UFS_EMBD	257
> > +#define DOUT_CLKCMU_HSI2_ETHERNET	258
> > +#define DOUT_CLKCMU_ISP_NOC		259
> > +#define DOUT_CLKCMU_M2M_NOC		260
> > +#define DOUT_CLKCMU_M2M_JPEG		261
> > +#define DOUT_CLKCMU_MFC_MFC		262
> > +#define DOUT_CLKCMU_MFC_WFD		263
> > +#define DOUT_CLKCMU_MFD_NOC		264
> > +#define DOUT_CLKCMU_MIF_NOCP		265
> > +#define DOUT_CLKCMU_MISC_NOC		266
> > +#define DOUT_CLKCMU_NOCL0_NOC		267
> > +#define DOUT_CLKCMU_NOCL1_NOC		268
> > +#define DOUT_CLKCMU_NOCL2_NOC		269
> > +#define DOUT_CLKCMU_PERIC0_NOC		270
> > +#define DOUT_CLKCMU_PERIC0_IP		271
> > +#define DOUT_CLKCMU_PERIC1_NOC		272
> > +#define DOUT_CLKCMU_PERIC1_IP		273
> > +#define DOUT_CLKCMU_SDMA_NOC		274
> > +#define DOUT_CLKCMU_SNW_NOC		275
> > +#define DOUT_CLKCMU_SSP_NOC		276
> > +#define DOUT_CLKCMU_TAA_NOC		277
> > +
> > +/* CMU_PERIC0 */
> > +#define CLK_MOUT_PERIC0_IP_USER		1
> > +#define CLK_MOUT_PERIC0_NOC_USER	2
> > +#define CLK_MOUT_PERIC0_USI00_USI	3
> > +#define CLK_MOUT_PERIC0_USI01_USI	4
> > +#define CLK_MOUT_PERIC0_USI02_USI	5
> > +#define CLK_MOUT_PERIC0_USI03_USI	6
> > +#define CLK_MOUT_PERIC0_USI04_USI	7
> > +#define CLK_MOUT_PERIC0_USI05_USI	8
> > +#define CLK_MOUT_PERIC0_USI06_USI	9
> > +#define CLK_MOUT_PERIC0_USI07_USI	10
> > +#define CLK_MOUT_PERIC0_USI08_USI	11
> > +#define CLK_MOUT_PERIC0_USI_I2C		12
> > +#define CLK_MOUT_PERIC0_I3C		13
> > +
> > +#define CLK_DOUT_PERIC0_USI00_USI	14
> > +#define CLK_DOUT_PERIC0_USI01_USI	15
> > +#define CLK_DOUT_PERIC0_USI02_USI	16
> > +#define CLK_DOUT_PERIC0_USI03_USI	17
> > +#define CLK_DOUT_PERIC0_USI04_USI	18
> > +#define CLK_DOUT_PERIC0_USI05_USI	19
> > +#define CLK_DOUT_PERIC0_USI06_USI	20
> > +#define CLK_DOUT_PERIC0_USI07_USI	21
> > +#define CLK_DOUT_PERIC0_USI08_USI	22
> > +#define CLK_DOUT_PERIC0_USI_I2C		23
> > +#define CLK_DOUT_PERIC0_I3C		24
> > +
> > +#endif /* _DT_BINDINGS_CLOCK_EXYNOSAUTOV920_H */
> > --
> > 2.45.2
> 

If there is anything you would like to review further, please check it out.

Thanks,
Sunyeal Hong
[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]

  Powered by Linux