mbox series

[v4,0/4] initial clock support for exynosauto v920 SoC

Message ID 20240722223333.1137947-1-sunyeal.hong@samsung.com
Headers show
Series initial clock support for exynosauto v920 SoC | expand

Message

Sunyeal Hong July 22, 2024, 10:33 p.m. UTC 
This patchset adds initial clock driver support for Exynos Auto v920 SoC.
This driver uses HW Auto Clock gating. So all gate clocks did not register.

Below CMU blocks are supported in this patchset and remains will be
implemented later.

- CMU_TOP
- CMU_PERIC0

Changes in v4:
 - Change PLL_531x fdiv type and mask bit
 - Change PLL_531x mdiv type

Changes in v3:
 - Change SoC name from Exynos Auto to ExynosAuto
 - Change the makefile order to the bottom of exynosautov9
 - Add PLL_531x formula for integer PLL

Changes in v2:
 - Fix typo from v209 to v920
 - Change USI clock to appropriate
 - Merge headers into binding patches
 - Change clock-name to the recommended name

Sunyeal Hong (4):
  dt-bindings: clock: add ExynosAuto v920 SoC CMU bindings
  arm64: dts: exynos: add initial CMU clock nodes in ExynosAuto v920
  clk: samsung: clk-pll: Add support for pll_531x
  clk: samsung: add top clock support for ExynosAuto v920 SoC

 .../clock/samsung,exynosautov920-clock.yaml   |  115 ++
 .../arm64/boot/dts/exynos/exynosautov920.dtsi |   40 +-
 drivers/clk/samsung/Makefile                  |    1 +
 drivers/clk/samsung/clk-exynosautov920.c      | 1173 +++++++++++++++++
 drivers/clk/samsung/clk-pll.c                 |   44 +
 drivers/clk/samsung/clk-pll.h                 |    1 +
 .../clock/samsung,exynosautov920.h            |  191 +++
 7 files changed, 1552 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/samsung,exynosautov920-clock.yaml
 create mode 100644 drivers/clk/samsung/clk-exynosautov920.c
 create mode 100644 include/dt-bindings/clock/samsung,exynosautov920.h

Comments

Rob Herring July 23, 2024, 8:57 p.m. UTC | #1 
On Tue, Jul 23, 2024 at 07:33:30AM +0900, Sunyeal Hong wrote:
> Add dt-schema for ExynosAuto v920 SoC clock controller.
> Add device tree clock binding definitions for below CMU blocks.
> 
> - CMU_TOP
> - CMU_PERIC0
> 
> Signed-off-by: Sunyeal Hong <sunyeal.hong@samsung.com>
> ---
>  .../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..90f9f17da959
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/samsung,exynosautov920-clock.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 ExynosAuto v920 SoC clock controller
> +
> +maintainers:
> +  - Sunyeal Hong <sunyeal.hong@samsung.com>
> +  - Chanwoo Choi <cw00.choi@samsung.com>
> +  - Krzysztof Kozlowski <krzk@kernel.org>
> +  - Sylwester Nawrocki <s.nawrocki@samsung.com>
> +
> +description: |
> +  ExynosAuto 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

Move the descriptions and names here. Then in the if/then schemas just 
set the number of items to 1 or 3 as appropriate.

Rob
Krzysztof Kozlowski July 24, 2024, 10:12 a.m. UTC | #2 
On 23/07/2024 00:33, Sunyeal Hong wrote:
> Add dt-schema for ExynosAuto v920 SoC clock controller.
> Add device tree clock binding definitions for below CMU blocks.
> 
> - CMU_TOP
> - CMU_PERIC0
> 
> Signed-off-by: Sunyeal Hong <sunyeal.hong@samsung.com>
> ---
>  .../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..90f9f17da959
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/samsung,exynosautov920-clock.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 ExynosAuto v920 SoC clock controller
> +
> +maintainers:
> +  - Sunyeal Hong <sunyeal.hong@samsung.com>
> +  - Chanwoo Choi <cw00.choi@samsung.com>
> +  - Krzysztof Kozlowski <krzk@kernel.org>
> +  - Sylwester Nawrocki <s.nawrocki@samsung.com>
> +
> +description: |
> +  ExynosAuto 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
> +
> +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..ad89728a4396
> --- /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@samsung.com>
> + *
> + * Device Tree binding constants for ExynosAuto 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

This is some odd numbering. Numbers start from 0 or 1 and are continuous.


Best regards,
Krzysztof
Krzysztof Kozlowski July 24, 2024, 10:13 a.m. UTC | #3 
On 23/07/2024 00:33, Sunyeal Hong wrote:
> Add cmu_top, cmu_peric0 clock nodes and
> switch USI clocks instead of dummy fixed-rate-clock.
> 
> Signed-off-by: Sunyeal Hong <sunyeal.hong@samsung.com>

Thank you for your patch. There is something to discuss/improve.

>  	cpus: cpus {
>  		#address-cells = <2>;
>  		#size-cells = <0>;
> @@ -182,6 +172,28 @@ chipid@10000000 {
>  			reg = <0x10000000 0x24>;
>  		};
>  
> +		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";
> +		};
> +
> +		cmu_top: clock-controller@11000000 {

This does not look ordered. Please read DTS coding style.

Best regards,
Krzysztof
Tudor Ambarus July 24, 2024, 11:17 a.m. UTC | #4 
Hi, Sunyeal,

I quickly skimmed over the series and I fail to see where/how the HW
auto clock gating is enabled/configured. Would you please add more
details on how this works?

On 7/22/24 11:33 PM, Sunyeal Hong wrote:
> Add cmu_top, cmu_peric0 clock nodes and
> switch USI clocks instead of dummy fixed-rate-clock.
> 
> Signed-off-by: Sunyeal Hong <sunyeal.hong@samsung.com>
> ---
>  .../arm64/boot/dts/exynos/exynosautov920.dtsi | 40 +++++++++++++------
>  1 file changed, 27 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/exynos/exynosautov920.dtsi b/arch/arm64/boot/dts/exynos/exynosautov920.dtsi
> index c1c8566d74f5..54fc32074379 100644
> --- a/arch/arm64/boot/dts/exynos/exynosautov920.dtsi
> +++ b/arch/arm64/boot/dts/exynos/exynosautov920.dtsi


cut

> @@ -224,7 +237,8 @@ serial_0: serial@10880000 {
>  				interrupts = <GIC_SPI 764 IRQ_TYPE_LEVEL_HIGH>;
>  				pinctrl-names = "default";
>  				pinctrl-0 = <&uart0_bus>;
> -				clocks = <&clock_usi>, <&clock_usi>;
> +				clocks = <&cmu_peric0 CLK_MOUT_PERIC0_NOC_USER>,

isn't this MUX common to multiple GATEs? Wouldn't turning it off affect
other users than the serial?

Thanks,
ta

> +					 <&cmu_peric0 CLK_DOUT_PERIC0_USI00_USI>;
>  				clock-names = "uart", "clk_uart_baud0";
>  				samsung,uart-fifosize = <256>;
>  				status = "disabled";
Sunyeal Hong July 25, 2024, 1:24 a.m. UTC | #5 
Hello Rob,

> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Wednesday, July 24, 2024 5:57 AM
> To: Sunyeal Hong <sunyeal.hong@samsung.com>
> Cc: Krzysztof Kozlowski <krzk@kernel.org>; Sylwester Nawrocki
> <s.nawrocki@samsung.com>; Chanwoo Choi <cw00.choi@samsung.com>; Alim
> Akhtar <alim.akhtar@samsung.com>; Michael Turquette
> <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; linux-samsung-soc@vger.kernel.org; linux-
> clk@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v4 1/4] dt-bindings: clock: add ExynosAuto v920 SoC
> CMU bindings
> 
> On Tue, Jul 23, 2024 at 07:33:30AM +0900, Sunyeal Hong wrote:
> > Add dt-schema for ExynosAuto v920 SoC clock controller.
> > Add device tree clock binding definitions for below CMU blocks.
> >
> > - CMU_TOP
> > - CMU_PERIC0
> >
> > Signed-off-by: Sunyeal Hong <sunyeal.hong@samsung.com>
> > ---
> >  .../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.y
> > aml  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..90f9f17da959
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/samsung,exynosautov920-c
> > +++ lock.yaml
> > @@ -0,0 +1,115 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +https://protect2.fireeye.com/v1/url?k=9932a88e-c6ae81a4-993323c1-000b
> > +abe598f7-779f1e959ac8eab9&q=1&e=539edfa4-b4e4-460a-93f4-1e6f17030945&
> > +u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fclock%2Fsamsung%2Cexynosaut
> > +ov920-clock.yaml%23
> > +$schema:
> > +https://protect2.fireeye.com/v1/url?k=4dbf6fb9-12234693-4dbee4f6-000b
> > +abe598f7-363a2f64c69b9542&q=1&e=539edfa4-b4e4-460a-93f4-1e6f17030945&
> > +u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
> > +
> > +title: Samsung ExynosAuto v920 SoC clock controller
> > +
> > +maintainers:
> > +  - Sunyeal Hong <sunyeal.hong@samsung.com>
> > +  - Chanwoo Choi <cw00.choi@samsung.com>
> > +  - Krzysztof Kozlowski <krzk@kernel.org>
> > +  - Sylwester Nawrocki <s.nawrocki@samsung.com>
> > +
> > +description: |
> > +  ExynosAuto 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
> 
> Move the descriptions and names here. Then in the if/then schemas just set
> the number of items to 1 or 3 as appropriate.
> 
> Rob

Thank you for your review. I will update by reflecting the fixes.

Thanks,
Sunyeal Hong
Sunyeal Hong July 25, 2024, 1:25 a.m. UTC | #6 
Hello Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Wednesday, July 24, 2024 7:12 PM
> To: Sunyeal Hong <sunyeal.hong@samsung.com>; Sylwester Nawrocki
> <s.nawrocki@samsung.com>; Chanwoo Choi <cw00.choi@samsung.com>; Alim
> Akhtar <alim.akhtar@samsung.com>; Michael Turquette
> <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; Rob Herring
> <robh@kernel.org>; Conor Dooley <conor+dt@kernel.org>
> Cc: linux-samsung-soc@vger.kernel.org; linux-clk@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v4 1/4] dt-bindings: clock: add ExynosAuto v920 SoC
> CMU bindings
> 
> On 23/07/2024 00:33, Sunyeal Hong wrote:
> > Add dt-schema for ExynosAuto v920 SoC clock controller.
> > Add device tree clock binding definitions for below CMU blocks.
> >
> > - CMU_TOP
> > - CMU_PERIC0
> >
> > Signed-off-by: Sunyeal Hong <sunyeal.hong@samsung.com>
> > ---
> >  .../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.y
> > aml  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..90f9f17da959
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/samsung,exynosautov920-c
> > +++ lock.yaml
> > @@ -0,0 +1,115 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +https://protect2.fireeye.com/v1/url?k=79ef652b-1864700b-79eeee64-74fe
> > +485fb347-9d0b27f7b9bcf4cc&q=1&e=af4d44eb-4030-4020-8a28-394e2a873516&
> > +u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fclock%2Fsamsung%2Cexynosaut
> > +ov920-clock.yaml%23
> > +$schema:
> > +https://protect2.fireeye.com/v1/url?k=4f1f645c-2e94717c-4f1eef13-74fe
> > +485fb347-e7ad6ce5885cf0ba&q=1&e=af4d44eb-4030-4020-8a28-394e2a873516&
> > +u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
> > +
> > +title: Samsung ExynosAuto v920 SoC clock controller
> > +
> > +maintainers:
> > +  - Sunyeal Hong <sunyeal.hong@samsung.com>
> > +  - Chanwoo Choi <cw00.choi@samsung.com>
> > +  - Krzysztof Kozlowski <krzk@kernel.org>
> > +  - Sylwester Nawrocki <s.nawrocki@samsung.com>
> > +
> > +description: |
> > +  ExynosAuto 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
> > +
> > +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..ad89728a4396
> > --- /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@samsung.com>
> > + *
> > + * Device Tree binding constants for ExynosAuto 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
> 
> This is some odd numbering. Numbers start from 0 or 1 and are continuous.
> 
Okay, I will update.
> 
> Best regards,
> Krzysztof

Thanks,
Sunyeal Hong.
Sunyeal Hong July 25, 2024, 1:28 a.m. UTC | #7 
Hello Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Wednesday, July 24, 2024 7:14 PM
> To: Sunyeal Hong <sunyeal.hong@samsung.com>; Sylwester Nawrocki
> <s.nawrocki@samsung.com>; Chanwoo Choi <cw00.choi@samsung.com>; Alim
> Akhtar <alim.akhtar@samsung.com>; Michael Turquette
> <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; Rob Herring
> <robh@kernel.org>; Conor Dooley <conor+dt@kernel.org>
> Cc: linux-samsung-soc@vger.kernel.org; linux-clk@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v4 2/4] arm64: dts: exynos: add initial CMU clock
> nodes in ExynosAuto v920
> 
> On 23/07/2024 00:33, Sunyeal Hong wrote:
> > Add cmu_top, cmu_peric0 clock nodes and switch USI clocks instead of
> > dummy fixed-rate-clock.
> >
> > Signed-off-by: Sunyeal Hong <sunyeal.hong@samsung.com>
> 
> Thank you for your patch. There is something to discuss/improve.
> 
> >  	cpus: cpus {
> >  		#address-cells = <2>;
> >  		#size-cells = <0>;
> > @@ -182,6 +172,28 @@ chipid@10000000 {
> >  			reg = <0x10000000 0x24>;
> >  		};
> >
> > +		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";
> > +		};
> > +
> > +		cmu_top: clock-controller@11000000 {
> 
> This does not look ordered. Please read DTS coding style.
> 
I will modify it by referring to the Order of Nodes item in the dts coding style guide document.

> Best regards,
> Krzysztof

Thanks,
Sunyeal Hong.
Sunyeal Hong July 25, 2024, 3:03 a.m. UTC | #8 
Hello Rob,

> -----Original Message-----
> From: sunyeal.hong <sunyeal.hong@samsung.com>
> Sent: Thursday, July 25, 2024 10:24 AM
> To: 'Rob Herring' <robh@kernel.org>
> Cc: 'Krzysztof Kozlowski' <krzk@kernel.org>; 'Sylwester Nawrocki'
> <s.nawrocki@samsung.com>; 'Chanwoo Choi' <cw00.choi@samsung.com>; 'Alim
> Akhtar' <alim.akhtar@samsung.com>; 'Michael Turquette'
> <mturquette@baylibre.com>; 'Stephen Boyd' <sboyd@kernel.org>; 'Conor
> Dooley' <conor+dt@kernel.org>; linux-samsung-soc@vger.kernel.org; linux-
> clk@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: RE: [PATCH v4 1/4] dt-bindings: clock: add ExynosAuto v920 SoC
> CMU bindings
> 
> Hello Rob,
> 
> > -----Original Message-----
> > From: Rob Herring <robh@kernel.org>
> > Sent: Wednesday, July 24, 2024 5:57 AM
> > To: Sunyeal Hong <sunyeal.hong@samsung.com>
> > Cc: Krzysztof Kozlowski <krzk@kernel.org>; Sylwester Nawrocki
> > <s.nawrocki@samsung.com>; Chanwoo Choi <cw00.choi@samsung.com>; Alim
> > Akhtar <alim.akhtar@samsung.com>; Michael Turquette
> > <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; Conor
> > Dooley <conor+dt@kernel.org>; linux-samsung-soc@vger.kernel.org;
> > linux- clk@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH v4 1/4] dt-bindings: clock: add ExynosAuto v920
> > SoC CMU bindings
> >
> > On Tue, Jul 23, 2024 at 07:33:30AM +0900, Sunyeal Hong wrote:
> > > Add dt-schema for ExynosAuto v920 SoC clock controller.
> > > Add device tree clock binding definitions for below CMU blocks.
> > >
> > > - CMU_TOP
> > > - CMU_PERIC0
> > >
> > > Signed-off-by: Sunyeal Hong <sunyeal.hong@samsung.com>
> > > ---
> > >  .../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
> > > .y
> > > aml  create mode 100644
> > > include/dt-bindings/clock/samsung,exynosautov920.h
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/clock/samsung,exynosautov920-clo
> > > ck
> > > .yaml
> > > b/Documentation/devicetree/bindings/clock/samsung,exynosautov920-clo
> > > ck
> > > .yaml
> > > new file mode 100644
> > > index 000000000000..90f9f17da959
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/clock/samsung,exynosautov920
> > > +++ -c
> > > +++ lock.yaml
> > > @@ -0,0 +1,115 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > > +---
> > > +$id:
> > > +https://protect2.fireeye.com/v1/url?k=9932a88e-c6ae81a4-993323c1-00
> > > +0b
> > > +abe598f7-779f1e959ac8eab9&q=1&e=539edfa4-b4e4-460a-93f4-1e6f1703094
> > > +5&
> > > +u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fclock%2Fsamsung%2Cexynosa
> > > +ut
> > > +ov920-clock.yaml%23
> > > +$schema:
> > > +https://protect2.fireeye.com/v1/url?k=4dbf6fb9-12234693-4dbee4f6-00
> > > +0b
> > > +abe598f7-363a2f64c69b9542&q=1&e=539edfa4-b4e4-460a-93f4-1e6f1703094
> > > +5&
> > > +u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
> > > +
> > > +title: Samsung ExynosAuto v920 SoC clock controller
> > > +
> > > +maintainers:
> > > +  - Sunyeal Hong <sunyeal.hong@samsung.com>
> > > +  - Chanwoo Choi <cw00.choi@samsung.com>
> > > +  - Krzysztof Kozlowski <krzk@kernel.org>
> > > +  - Sylwester Nawrocki <s.nawrocki@samsung.com>
> > > +
> > > +description: |
> > > +  ExynosAuto 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
> >
> > Move the descriptions and names here. Then in the if/then schemas just
> > set the number of items to 1 or 3 as appropriate.
> >
> > Rob
> 
> Thank you for your review. I will update by reflecting the fixes.
> 
> Thanks,
> Sunyeal Hong
> 
> 

I faced a new problem after modifying it as you reviewed.
For example, if I declare a new cmu block that uses only osclk and noc to dt, it seems that a problem occurs in check dtb.

- yaml
properties:
  compatible:
    enum:
      - samsung,exynosautov920-cmu-top
      - samsung,exynosautov920-cmu-peric0
	  - samsung,exynosautov920-cmu-misc

  clocks:
    minItems: 1
    items:
      - description: External reference clock (38.4 MHz)
      - description: Block IP clock (from CMU_TOP)
      - description: Block NOC clock (from CMU_TOP)

  clock-names:
    minItems: 1
    items:
      - const: oscclk
      - const: ip
      - const: noc

- dts
cmu_misc: clock-controller@10020000 {
	compatible = "samsung,exynosautov920-cmu-misc";
	reg = <0x10020000 0x8000>;
	#clock-cells = <1>;

	clocks = <&xtcxo>,
		 <&cmu_top DOUT_CLKCMU_MISC_NOC>;
	clock-names = "oscclk",
		      "noc";
};

In this case, can you tell me how to handle it?
And if a new clock item is added and a new cmu block uses only the clock item added and oscclk, a problem may occur.

Thanks,
Sunyeal Hong.
Sunyeal Hong July 25, 2024, 3:09 a.m. UTC | #9 
Hello Tudor,

> -----Original Message-----
> From: Tudor Ambarus <tudor.ambarus@linaro.org>
> Sent: Wednesday, July 24, 2024 8:18 PM
> To: Sunyeal Hong <sunyeal.hong@samsung.com>; Krzysztof Kozlowski
> <krzk@kernel.org>; Sylwester Nawrocki <s.nawrocki@samsung.com>; Chanwoo
> Choi <cw00.choi@samsung.com>; Alim Akhtar <alim.akhtar@samsung.com>;
> Michael Turquette <mturquette@baylibre.com>; Stephen Boyd
> <sboyd@kernel.org>; Rob Herring <robh@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>
> Cc: linux-samsung-soc@vger.kernel.org; linux-clk@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v4 2/4] arm64: dts: exynos: add initial CMU clock
> nodes in ExynosAuto v920
> 
> Hi, Sunyeal,
> 
> I quickly skimmed over the series and I fail to see where/how the HW auto
> clock gating is enabled/configured. Would you please add more details on
> how this works?
>
The HW auto clock gating function is activated in the bootloader stage. So we don't have to control it in the kernel.

> On 7/22/24 11:33 PM, Sunyeal Hong wrote:
> > Add cmu_top, cmu_peric0 clock nodes and switch USI clocks instead of
> > dummy fixed-rate-clock.
> >
> > Signed-off-by: Sunyeal Hong <sunyeal.hong@samsung.com>
> > ---
> >  .../arm64/boot/dts/exynos/exynosautov920.dtsi | 40
> > +++++++++++++------
> >  1 file changed, 27 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/exynos/exynosautov920.dtsi
> > b/arch/arm64/boot/dts/exynos/exynosautov920.dtsi
> > index c1c8566d74f5..54fc32074379 100644
> > --- a/arch/arm64/boot/dts/exynos/exynosautov920.dtsi
> > +++ b/arch/arm64/boot/dts/exynos/exynosautov920.dtsi
> 
> 
> cut
> 
> > @@ -224,7 +237,8 @@ serial_0: serial@10880000 {
> >  				interrupts = <GIC_SPI 764 IRQ_TYPE_LEVEL_HIGH>;
> >  				pinctrl-names = "default";
> >  				pinctrl-0 = <&uart0_bus>;
> > -				clocks = <&clock_usi>, <&clock_usi>;
> > +				clocks = <&cmu_peric0 CLK_MOUT_PERIC0_NOC_USER>,
> 
> isn't this MUX common to multiple GATEs? Wouldn't turning it off affect
> other users than the serial?
> 
> Thanks,
> ta
> 
I don’t think there will be any problems you are worried about through the enable count of CCF.

Thanks,
Sunyeal Hong.
> > +					 <&cmu_peric0 CLK_DOUT_PERIC0_USI00_USI>;
> >  				clock-names = "uart", "clk_uart_baud0";
> >  				samsung,uart-fifosize = <256>;
> >  				status = "disabled";
Krzysztof Kozlowski July 25, 2024, 6:21 a.m. UTC | #10 
On 25/07/2024 05:03, sunyeal.hong wrote:

> - dts
> cmu_misc: clock-controller@10020000 {
> 	compatible = "samsung,exynosautov920-cmu-misc";
> 	reg = <0x10020000 0x8000>;
> 	#clock-cells = <1>;
> 
> 	clocks = <&xtcxo>,
> 		 <&cmu_top DOUT_CLKCMU_MISC_NOC>;
> 	clock-names = "oscclk",
> 		      "noc";
> };
> 
> In this case, can you tell me how to handle it?
> And if a new clock item is added and a new cmu block uses only the clock item added and oscclk, a problem may occur.

The same problem was in your original version, so why suddenly it appeared?

Anyway, why clock would be missing? You just wrote in the bindings that
there is such input clock.

Best regards,
Krzysztof
Sunyeal Hong July 25, 2024, 6:35 a.m. UTC | #11 
Hello Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Thursday, July 25, 2024 3:21 PM
> To: sunyeal.hong <sunyeal.hong@samsung.com>; 'Rob Herring'
> <robh@kernel.org>
> Cc: 'Sylwester Nawrocki' <s.nawrocki@samsung.com>; 'Chanwoo Choi'
> <cw00.choi@samsung.com>; 'Alim Akhtar' <alim.akhtar@samsung.com>; 'Michael
> Turquette' <mturquette@baylibre.com>; 'Stephen Boyd' <sboyd@kernel.org>;
> 'Conor Dooley' <conor+dt@kernel.org>; linux-samsung-soc@vger.kernel.org;
> linux-clk@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v4 1/4] dt-bindings: clock: add ExynosAuto v920 SoC
> CMU bindings
> 
> On 25/07/2024 05:03, sunyeal.hong wrote:
> 
> > - dts
> > cmu_misc: clock-controller@10020000 {
> > 	compatible = "samsung,exynosautov920-cmu-misc";
> > 	reg = <0x10020000 0x8000>;
> > 	#clock-cells = <1>;
> >
> > 	clocks = <&xtcxo>,
> > 		 <&cmu_top DOUT_CLKCMU_MISC_NOC>;
> > 	clock-names = "oscclk",
> > 		      "noc";
> > };
> >
> > In this case, can you tell me how to handle it?
> > And if a new clock item is added and a new cmu block uses only the clock
> item added and oscclk, a problem may occur.
> 
> The same problem was in your original version, so why suddenly it appeared?
> 
> Anyway, why clock would be missing? You just wrote in the bindings that
> there is such input clock.
> 
> Best regards,
> Krzysztof
> 

If I reflect Rob's review, it will be changed as below.

- yaml
properties:
  compatible:
    enum:
      - samsung,exynosautov920-cmu-top
      - samsung,exynosautov920-cmu-peric0

  clocks:
    minItems: 1
    items:
      - description: External reference clock (38.4 MHz)
      - description: Block IP clock (from CMU_TOP)
      - description: Block NOC clock (from CMU_TOP)

  clock-names:
    minItems: 1
    items:
      - const: oscclk
      - const: ip
      - const: noc

  "#clock-cells":
    const: 1

  reg:
    maxItems: 1

if:
    properties:
      compatible:
        enum:
          - samsung,exynosautov920-cmu-misc

  then:
    properties:
      clocks:
        minItems: 2
        maxItems: 2

      clock-names:
        minItems: 2
        maxItems: 2

- device tree
cmu_misc: clock-controller@10020000 {
	compatible = "samsung,exynosautov920-cmu-misc";
	reg = <0x10020000 0x8000>;
	#clock-cells = <1>;

	clocks = <&xtcxo>,
		 <&cmu_top DOUT_CLKCMU_MISC_NOC>;
	clock-names = "oscclk",
		      "noc";
};

In this case, ip should be used after oscclk, but misc does not use ip, so there is a problem in dt check.

The code of v4 version has clock items for each block, so there was no problem like this.
- yaml(v4)

  if:
    properties:
      compatible:
        contains:
          const: samsung,exynosautov920-cmu-misc

  then:
    properties:
      clocks:
        items:
          - description: External reference clock (38.4 MHz)
          - description: CMU_MISC NOC clock (from CMU_MISC)

      clock-names:
        items:
          - const: oscclk
          - const: noc

If there is anything I misunderstand, please guide me.

Thanks,
Sunyeal Hong.
Krzysztof Kozlowski July 25, 2024, 6:37 a.m. UTC | #12 
On 25/07/2024 08:35, sunyeal.hong wrote:
> Hello Krzysztof,
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@kernel.org>
>> Sent: Thursday, July 25, 2024 3:21 PM
>> To: sunyeal.hong <sunyeal.hong@samsung.com>; 'Rob Herring'
>> <robh@kernel.org>
>> Cc: 'Sylwester Nawrocki' <s.nawrocki@samsung.com>; 'Chanwoo Choi'
>> <cw00.choi@samsung.com>; 'Alim Akhtar' <alim.akhtar@samsung.com>; 'Michael
>> Turquette' <mturquette@baylibre.com>; 'Stephen Boyd' <sboyd@kernel.org>;
>> 'Conor Dooley' <conor+dt@kernel.org>; linux-samsung-soc@vger.kernel.org;
>> linux-clk@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
>> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH v4 1/4] dt-bindings: clock: add ExynosAuto v920 SoC
>> CMU bindings
>>
>> On 25/07/2024 05:03, sunyeal.hong wrote:
>>
>>> - dts
>>> cmu_misc: clock-controller@10020000 {
>>> 	compatible = "samsung,exynosautov920-cmu-misc";
>>> 	reg = <0x10020000 0x8000>;
>>> 	#clock-cells = <1>;
>>>
>>> 	clocks = <&xtcxo>,
>>> 		 <&cmu_top DOUT_CLKCMU_MISC_NOC>;
>>> 	clock-names = "oscclk",
>>> 		      "noc";
>>> };
>>>
>>> In this case, can you tell me how to handle it?
>>> And if a new clock item is added and a new cmu block uses only the clock
>> item added and oscclk, a problem may occur.
>>
>> The same problem was in your original version, so why suddenly it appeared?
>>
>> Anyway, why clock would be missing? You just wrote in the bindings that
>> there is such input clock.
>>
>> Best regards,
>> Krzysztof
>>
> 
> If I reflect Rob's review, it will be changed as below.
> 
> - yaml
> properties:
>   compatible:
>     enum:
>       - samsung,exynosautov920-cmu-top
>       - samsung,exynosautov920-cmu-peric0
> 
>   clocks:
>     minItems: 1
>     items:
>       - description: External reference clock (38.4 MHz)
>       - description: Block IP clock (from CMU_TOP)
>       - description: Block NOC clock (from CMU_TOP)
> 
>   clock-names:
>     minItems: 1
>     items:
>       - const: oscclk
>       - const: ip
>       - const: noc
> 
>   "#clock-cells":
>     const: 1
> 
>   reg:
>     maxItems: 1
> 
> if:
>     properties:
>       compatible:
>         enum:
>           - samsung,exynosautov920-cmu-misc
> 
>   then:
>     properties:
>       clocks:
>         minItems: 2
>         maxItems: 2
> 
>       clock-names:
>         minItems: 2
>         maxItems: 2
> 
> - device tree
> cmu_misc: clock-controller@10020000 {
> 	compatible = "samsung,exynosautov920-cmu-misc";
> 	reg = <0x10020000 0x8000>;
> 	#clock-cells = <1>;
> 
> 	clocks = <&xtcxo>,
> 		 <&cmu_top DOUT_CLKCMU_MISC_NOC>;
> 	clock-names = "oscclk",
> 		      "noc";
> };
> 
> In this case, ip should be used after oscclk, but misc does not use ip, so there is a problem in dt check.
> 
> The code of v4 version has clock items for each block, so there was no problem like this.
> - yaml(v4)
> 
>   if:
>     properties:
>       compatible:
>         contains:
>           const: samsung,exynosautov920-cmu-misc
> 
>   then:
>     properties:
>       clocks:
>         items:
>           - description: External reference clock (38.4 MHz)
>           - description: CMU_MISC NOC clock (from CMU_MISC)
> 
>       clock-names:
>         items:
>           - const: oscclk
>           - const: noc
> 
> If there is anything I misunderstand, please guide me.
> 

You did not address my questions at all instead just copied again the
same. It is not how it works.

I am not going to discuss like this.

Best regards,
Krzysztof
Krzysztof Kozlowski July 25, 2024, 6:40 a.m. UTC | #13 
On 25/07/2024 08:37, Krzysztof Kozlowski wrote:
>>   then:
>>     properties:
>>       clocks:
>>         items:
>>           - description: External reference clock (38.4 MHz)
>>           - description: CMU_MISC NOC clock (from CMU_MISC)
>>
>>       clock-names:
>>         items:
>>           - const: oscclk
>>           - const: noc
>>
>> If there is anything I misunderstand, please guide me.
>>
> 
> You did not address my questions at all instead just copied again the
> same. It is not how it works.
> 
> I am not going to discuss like this.

And in case it is still unclear - just look at your bindings and DTS.
They say you have three clocks!

Best regards,
Krzysztof
Sunyeal Hong July 25, 2024, 7:14 a.m. UTC | #14 
Hello Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Thursday, July 25, 2024 3:41 PM
> To: sunyeal.hong <sunyeal.hong@samsung.com>; 'Rob Herring'
> <robh@kernel.org>
> Cc: 'Sylwester Nawrocki' <s.nawrocki@samsung.com>; 'Chanwoo Choi'
> <cw00.choi@samsung.com>; 'Alim Akhtar' <alim.akhtar@samsung.com>; 'Michael
> Turquette' <mturquette@baylibre.com>; 'Stephen Boyd' <sboyd@kernel.org>;
> 'Conor Dooley' <conor+dt@kernel.org>; linux-samsung-soc@vger.kernel.org;
> linux-clk@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v4 1/4] dt-bindings: clock: add ExynosAuto v920 SoC
> CMU bindings
> 
> On 25/07/2024 08:37, Krzysztof Kozlowski wrote:
> >>   then:
> >>     properties:
> >>       clocks:
> >>         items:
> >>           - description: External reference clock (38.4 MHz)
> >>           - description: CMU_MISC NOC clock (from CMU_MISC)
> >>
> >>       clock-names:
> >>         items:
> >>           - const: oscclk
> >>           - const: noc
> >>
> >> If there is anything I misunderstand, please guide me.
> >>
> >
> > You did not address my questions at all instead just copied again the
> > same. It is not how it works.
> >
> > I am not going to discuss like this.
> 
> And in case it is still unclear - just look at your bindings and DTS.
> They say you have three clocks!
> 
> Best regards,
> Krzysztof
> 

Let me answer your questions first.
In the existing V4 patch, clock items were declared in if then for each block, so there was no problem.
If modified according to Rob's comment, problems may occur as the input clock is configured differently for each block.

I think it would be better to keep the bindings in v4 patch form like the existing exynos models.

Thanks for your comment.

Best regards,
Sunyeal
Krzysztof Kozlowski July 25, 2024, 7:31 a.m. UTC | #15 
On 25/07/2024 09:14, sunyeal.hong wrote:
> Hello Krzysztof,
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@kernel.org>
>> Sent: Thursday, July 25, 2024 3:41 PM
>> To: sunyeal.hong <sunyeal.hong@samsung.com>; 'Rob Herring'
>> <robh@kernel.org>
>> Cc: 'Sylwester Nawrocki' <s.nawrocki@samsung.com>; 'Chanwoo Choi'
>> <cw00.choi@samsung.com>; 'Alim Akhtar' <alim.akhtar@samsung.com>; 'Michael
>> Turquette' <mturquette@baylibre.com>; 'Stephen Boyd' <sboyd@kernel.org>;
>> 'Conor Dooley' <conor+dt@kernel.org>; linux-samsung-soc@vger.kernel.org;
>> linux-clk@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
>> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH v4 1/4] dt-bindings: clock: add ExynosAuto v920 SoC
>> CMU bindings
>>
>> On 25/07/2024 08:37, Krzysztof Kozlowski wrote:
>>>>   then:
>>>>     properties:
>>>>       clocks:
>>>>         items:
>>>>           - description: External reference clock (38.4 MHz)
>>>>           - description: CMU_MISC NOC clock (from CMU_MISC)
>>>>
>>>>       clock-names:
>>>>         items:
>>>>           - const: oscclk
>>>>           - const: noc
>>>>
>>>> If there is anything I misunderstand, please guide me.
>>>>
>>>
>>> You did not address my questions at all instead just copied again the
>>> same. It is not how it works.
>>>
>>> I am not going to discuss like this.
>>
>> And in case it is still unclear - just look at your bindings and DTS.
>> They say you have three clocks!
>>
>> Best regards,
>> Krzysztof
>>
> 
> Let me answer your questions first.
> In the existing V4 patch, clock items were declared in if then for each block, so there was no problem.

No. Again, look at your binding and DTS.

1. What clocks did you define for cmu-top?
2. What clocks did you define for cmu-peric0?

Rob's advice is reasonable and you must follow it, unless you are not
telling us something. There is no other choice, no other compatibles, no
other devices.

> If modified according to Rob's comment, problems may occur as the input clock is configured differently for each block.

But it is not! Look at your binding.


Best regards,
Krzysztof
Sunyeal Hong July 25, 2024, 7:50 a.m. UTC | #16 
Hello Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Thursday, July 25, 2024 4:32 PM
> To: sunyeal.hong <sunyeal.hong@samsung.com>; 'Rob Herring'
> <robh@kernel.org>
> Cc: 'Sylwester Nawrocki' <s.nawrocki@samsung.com>; 'Chanwoo Choi'
> <cw00.choi@samsung.com>; 'Alim Akhtar' <alim.akhtar@samsung.com>; 'Michael
> Turquette' <mturquette@baylibre.com>; 'Stephen Boyd' <sboyd@kernel.org>;
> 'Conor Dooley' <conor+dt@kernel.org>; linux-samsung-soc@vger.kernel.org;
> linux-clk@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v4 1/4] dt-bindings: clock: add ExynosAuto v920 SoC
> CMU bindings
> 
> On 25/07/2024 09:14, sunyeal.hong wrote:
> > Hello Krzysztof,
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzk@kernel.org>
> >> Sent: Thursday, July 25, 2024 3:41 PM
> >> To: sunyeal.hong <sunyeal.hong@samsung.com>; 'Rob Herring'
> >> <robh@kernel.org>
> >> Cc: 'Sylwester Nawrocki' <s.nawrocki@samsung.com>; 'Chanwoo Choi'
> >> <cw00.choi@samsung.com>; 'Alim Akhtar' <alim.akhtar@samsung.com>;
> >> 'Michael Turquette' <mturquette@baylibre.com>; 'Stephen Boyd'
> >> <sboyd@kernel.org>; 'Conor Dooley' <conor+dt@kernel.org>;
> >> linux-samsung-soc@vger.kernel.org;
> >> linux-clk@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
> >> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> >> Subject: Re: [PATCH v4 1/4] dt-bindings: clock: add ExynosAuto v920
> >> SoC CMU bindings
> >>
> >> On 25/07/2024 08:37, Krzysztof Kozlowski wrote:
> >>>>   then:
> >>>>     properties:
> >>>>       clocks:
> >>>>         items:
> >>>>           - description: External reference clock (38.4 MHz)
> >>>>           - description: CMU_MISC NOC clock (from CMU_MISC)
> >>>>
> >>>>       clock-names:
> >>>>         items:
> >>>>           - const: oscclk
> >>>>           - const: noc
> >>>>
> >>>> If there is anything I misunderstand, please guide me.
> >>>>
> >>>
> >>> You did not address my questions at all instead just copied again
> >>> the same. It is not how it works.
> >>>
> >>> I am not going to discuss like this.
> >>
> >> And in case it is still unclear - just look at your bindings and DTS.
> >> They say you have three clocks!
> >>
> >> Best regards,
> >> Krzysztof
> >>
> >
> > Let me answer your questions first.
> > In the existing V4 patch, clock items were declared in if then for each
> block, so there was no problem.
> 
> No. Again, look at your binding and DTS.
> 
> 1. What clocks did you define for cmu-top?
Cmu-top has one clock(oscclk).
> 2. What clocks did you define for cmu-peric0?
Cmu-peric0 has three clocks(oscclk, noc and ip)
> 
> Rob's advice is reasonable and you must follow it, unless you are not
> telling us something. There is no other choice, no other compatibles, no
> other devices.
> 
Yes, that's right. In this patch, modifications are possible according to Rob's review.
> > If modified according to Rob's comment, problems may occur as the input
> clock is configured differently for each block.
> 
> But it is not! Look at your binding.
The reason I mentioned this was to ask how to handle problems that may occur when adding cmu for a new block in a new patch.
As you mentioned, this issue does not exist in this patch.
> 
> 
> Best regards,
> Krzysztof

Best regard,
sunyeal
Krzysztof Kozlowski July 25, 2024, 7:56 a.m. UTC | #17 
On 25/07/2024 09:50, sunyeal.hong wrote:
> Hello Krzysztof,
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@kernel.org>
>> Sent: Thursday, July 25, 2024 4:32 PM
>> To: sunyeal.hong <sunyeal.hong@samsung.com>; 'Rob Herring'
>> <robh@kernel.org>
>> Cc: 'Sylwester Nawrocki' <s.nawrocki@samsung.com>; 'Chanwoo Choi'
>> <cw00.choi@samsung.com>; 'Alim Akhtar' <alim.akhtar@samsung.com>; 'Michael
>> Turquette' <mturquette@baylibre.com>; 'Stephen Boyd' <sboyd@kernel.org>;
>> 'Conor Dooley' <conor+dt@kernel.org>; linux-samsung-soc@vger.kernel.org;
>> linux-clk@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
>> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH v4 1/4] dt-bindings: clock: add ExynosAuto v920 SoC
>> CMU bindings
>>
>> On 25/07/2024 09:14, sunyeal.hong wrote:
>>> Hello Krzysztof,
>>>
>>>> -----Original Message-----
>>>> From: Krzysztof Kozlowski <krzk@kernel.org>
>>>> Sent: Thursday, July 25, 2024 3:41 PM
>>>> To: sunyeal.hong <sunyeal.hong@samsung.com>; 'Rob Herring'
>>>> <robh@kernel.org>
>>>> Cc: 'Sylwester Nawrocki' <s.nawrocki@samsung.com>; 'Chanwoo Choi'
>>>> <cw00.choi@samsung.com>; 'Alim Akhtar' <alim.akhtar@samsung.com>;
>>>> 'Michael Turquette' <mturquette@baylibre.com>; 'Stephen Boyd'
>>>> <sboyd@kernel.org>; 'Conor Dooley' <conor+dt@kernel.org>;
>>>> linux-samsung-soc@vger.kernel.org;
>>>> linux-clk@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
>>>> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
>>>> Subject: Re: [PATCH v4 1/4] dt-bindings: clock: add ExynosAuto v920
>>>> SoC CMU bindings
>>>>
>>>> On 25/07/2024 08:37, Krzysztof Kozlowski wrote:
>>>>>>   then:
>>>>>>     properties:
>>>>>>       clocks:
>>>>>>         items:
>>>>>>           - description: External reference clock (38.4 MHz)
>>>>>>           - description: CMU_MISC NOC clock (from CMU_MISC)
>>>>>>
>>>>>>       clock-names:
>>>>>>         items:
>>>>>>           - const: oscclk
>>>>>>           - const: noc
>>>>>>
>>>>>> If there is anything I misunderstand, please guide me.
>>>>>>
>>>>>
>>>>> You did not address my questions at all instead just copied again
>>>>> the same. It is not how it works.
>>>>>
>>>>> I am not going to discuss like this.
>>>>
>>>> And in case it is still unclear - just look at your bindings and DTS.
>>>> They say you have three clocks!
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>>
>>>
>>> Let me answer your questions first.
>>> In the existing V4 patch, clock items were declared in if then for each
>> block, so there was no problem.
>>
>> No. Again, look at your binding and DTS.
>>
>> 1. What clocks did you define for cmu-top?
> Cmu-top has one clock(oscclk).
>> 2. What clocks did you define for cmu-peric0?
> Cmu-peric0 has three clocks(oscclk, noc and ip)
>>
>> Rob's advice is reasonable and you must follow it, unless you are not
>> telling us something. There is no other choice, no other compatibles, no
>> other devices.
>>
> Yes, that's right. In this patch, modifications are possible according to Rob's review.
>>> If modified according to Rob's comment, problems may occur as the input
>> clock is configured differently for each block.
>>
>> But it is not! Look at your binding.
> The reason I mentioned this was to ask how to handle problems that may occur when adding cmu for a new block in a new patch.
> As you mentioned, this issue does not exist in this patch.

A new block? And how do we know about it? Bindings are supposed to be
complete. We see bindings and you receive review.

Post complete bindings.

Best regards,
Krzysztof
Sunyeal Hong July 29, 2024, 2:48 a.m. UTC | #18 
Hello Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Thursday, July 25, 2024 4:56 PM
> To: sunyeal.hong <sunyeal.hong@samsung.com>; 'Rob Herring'
> <robh@kernel.org>
> Cc: 'Sylwester Nawrocki' <s.nawrocki@samsung.com>; 'Chanwoo Choi'
> <cw00.choi@samsung.com>; 'Alim Akhtar' <alim.akhtar@samsung.com>; 'Michael
> Turquette' <mturquette@baylibre.com>; 'Stephen Boyd' <sboyd@kernel.org>;
> 'Conor Dooley' <conor+dt@kernel.org>; linux-samsung-soc@vger.kernel.org;
> linux-clk@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v4 1/4] dt-bindings: clock: add ExynosAuto v920 SoC
> CMU bindings
> 
> On 25/07/2024 09:50, sunyeal.hong wrote:
> > Hello Krzysztof,
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzk@kernel.org>
> >> Sent: Thursday, July 25, 2024 4:32 PM
> >> To: sunyeal.hong <sunyeal.hong@samsung.com>; 'Rob Herring'
> >> <robh@kernel.org>
> >> Cc: 'Sylwester Nawrocki' <s.nawrocki@samsung.com>; 'Chanwoo Choi'
> >> <cw00.choi@samsung.com>; 'Alim Akhtar' <alim.akhtar@samsung.com>;
> >> 'Michael Turquette' <mturquette@baylibre.com>; 'Stephen Boyd'
> >> <sboyd@kernel.org>; 'Conor Dooley' <conor+dt@kernel.org>;
> >> linux-samsung-soc@vger.kernel.org;
> >> linux-clk@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
> >> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> >> Subject: Re: [PATCH v4 1/4] dt-bindings: clock: add ExynosAuto v920
> >> SoC CMU bindings
> >>
> >> On 25/07/2024 09:14, sunyeal.hong wrote:
> >>> Hello Krzysztof,
> >>>
> >>>> -----Original Message-----
> >>>> From: Krzysztof Kozlowski <krzk@kernel.org>
> >>>> Sent: Thursday, July 25, 2024 3:41 PM
> >>>> To: sunyeal.hong <sunyeal.hong@samsung.com>; 'Rob Herring'
> >>>> <robh@kernel.org>
> >>>> Cc: 'Sylwester Nawrocki' <s.nawrocki@samsung.com>; 'Chanwoo Choi'
> >>>> <cw00.choi@samsung.com>; 'Alim Akhtar' <alim.akhtar@samsung.com>;
> >>>> 'Michael Turquette' <mturquette@baylibre.com>; 'Stephen Boyd'
> >>>> <sboyd@kernel.org>; 'Conor Dooley' <conor+dt@kernel.org>;
> >>>> linux-samsung-soc@vger.kernel.org;
> >>>> linux-clk@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
> >>>> kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> >>>> Subject: Re: [PATCH v4 1/4] dt-bindings: clock: add ExynosAuto v920
> >>>> SoC CMU bindings
> >>>>
> >>>> On 25/07/2024 08:37, Krzysztof Kozlowski wrote:
> >>>>>>   then:
> >>>>>>     properties:
> >>>>>>       clocks:
> >>>>>>         items:
> >>>>>>           - description: External reference clock (38.4 MHz)
> >>>>>>           - description: CMU_MISC NOC clock (from CMU_MISC)
> >>>>>>
> >>>>>>       clock-names:
> >>>>>>         items:
> >>>>>>           - const: oscclk
> >>>>>>           - const: noc
> >>>>>>
> >>>>>> If there is anything I misunderstand, please guide me.
> >>>>>>
> >>>>>
> >>>>> You did not address my questions at all instead just copied again
> >>>>> the same. It is not how it works.
> >>>>>
> >>>>> I am not going to discuss like this.
> >>>>
> >>>> And in case it is still unclear - just look at your bindings and DTS.
> >>>> They say you have three clocks!
> >>>>
> >>>> Best regards,
> >>>> Krzysztof
> >>>>
> >>>
> >>> Let me answer your questions first.
> >>> In the existing V4 patch, clock items were declared in if then for
> >>> each
> >> block, so there was no problem.
> >>
> >> No. Again, look at your binding and DTS.
> >>
> >> 1. What clocks did you define for cmu-top?
> > Cmu-top has one clock(oscclk).
> >> 2. What clocks did you define for cmu-peric0?
> > Cmu-peric0 has three clocks(oscclk, noc and ip)
> >>
> >> Rob's advice is reasonable and you must follow it, unless you are not
> >> telling us something. There is no other choice, no other compatibles,
> >> no other devices.
> >>
> > Yes, that's right. In this patch, modifications are possible according
> to Rob's review.
> >>> If modified according to Rob's comment, problems may occur as the
> >>> input
> >> clock is configured differently for each block.
> >>
> >> But it is not! Look at your binding.
> > The reason I mentioned this was to ask how to handle problems that may
> occur when adding cmu for a new block in a new patch.
> > As you mentioned, this issue does not exist in this patch.
> 
> A new block? And how do we know about it? Bindings are supposed to be
> complete. We see bindings and you receive review.
> 
> Post complete bindings.
> 
> Best regards,
> Krzysztof
> 

I understand your intention. I will re-upload the patch based on Rob's review based on the current patch.

Best regards,
sunyeal