diff mbox series

[v3,1/4] dt-bindings: clock: convert stm32 rcc bindings to json-schema

Message ID 20250114091128.528757-2-dario.binacchi@amarulasolutions.com (mailing list archive)
State New
Headers show
Series Support spread spectrum clocking for stm32f{4,7} platforms | expand

Commit Message

Dario Binacchi Jan. 14, 2025, 9:11 a.m. UTC
The patch converts st,stm32-rcc.txt to the JSON schema, but it does more
than that. The old bindings, in fact, only covered the stm32f{4,7}
platforms and not the stm32h7. Therefore, to avoid patch submission tests
failing, it was necessary to add the corresponding compatible (i. e.
st,stm32h743-rcc) and specify that, in this case, 3 are the clocks instead
of the 2 required for the stm32f{4,7} platforms.
Additionally, the old bindings made no mention of the st,syscfg property,
which is used by both the stm32f{4,7} and the stm32h7 platforms.

The patch also fixes the files referencing to the old st,stm32-rcc.txt.

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

Changes in v3:
- Add 'Reviewed-by' tag of Krzysztof Kozlowski

Changes in v2:
- Fixup patches:
  2/6 dt-bindings: reset: st,stm32-rcc: update reference due to rename
  3/6 dt-bindings: clock: stm32fx: update reference due to rename
- Update the commit message
- Reduce the description section of the yaml file
- List the items with description for the clocks property
- Use only one example
- Rename rcc to clock-controller@58024400 for the node of the example

 .../bindings/clock/st,stm32-rcc.txt           | 138 ------------------
 .../bindings/clock/st,stm32-rcc.yaml          | 111 ++++++++++++++
 .../bindings/reset/st,stm32-rcc.txt           |   2 +-
 include/dt-bindings/clock/stm32fx-clock.h     |   2 +-
 4 files changed, 113 insertions(+), 140 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/st,stm32-rcc.txt
 create mode 100644 Documentation/devicetree/bindings/clock/st,stm32-rcc.yaml

Comments

Krzysztof Kozlowski Jan. 14, 2025, 9:29 a.m. UTC | #1
On 14/01/2025 10:11, Dario Binacchi wrote:
> The patch converts st,stm32-rcc.txt to the JSON schema, but it does more
> than that. The old bindings, in fact, only covered the stm32f{4,7}
> platforms and not the stm32h7. Therefore, to avoid patch submission tests
> failing, it was necessary to add the corresponding compatible (i. e.
> st,stm32h743-rcc) and specify that, in this case, 3 are the clocks instead
> of the 2 required for the stm32f{4,7} platforms.
> Additionally, the old bindings made no mention of the st,syscfg property,
> which is used by both the stm32f{4,7} and the stm32h7 platforms.
> 
> The patch also fixes the files referencing to the old st,stm32-rcc.txt.
> 
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Drop the tag. It was conditional.

> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - const: st,stm32f42xx-rcc
> +          - const: st,stm32-rcc
> +      - items:
> +          - enum:
> +              - st,stm32f469-rcc
> +          - const: st,stm32f42xx-rcc
> +          - const: st,stm32-rcc
> +      - items:
> +          - const: st,stm32f746-rcc
> +          - const: st,stm32-rcc

Nothing improved here.

Best regards,
Krzysztof
Dario Binacchi Jan. 14, 2025, 9:36 a.m. UTC | #2
On Tue, Jan 14, 2025 at 10:29 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 14/01/2025 10:11, Dario Binacchi wrote:
> > The patch converts st,stm32-rcc.txt to the JSON schema, but it does more
> > than that. The old bindings, in fact, only covered the stm32f{4,7}
> > platforms and not the stm32h7. Therefore, to avoid patch submission tests
> > failing, it was necessary to add the corresponding compatible (i. e.
> > st,stm32h743-rcc) and specify that, in this case, 3 are the clocks instead
> > of the 2 required for the stm32f{4,7} platforms.
> > Additionally, the old bindings made no mention of the st,syscfg property,
> > which is used by both the stm32f{4,7} and the stm32h7 platforms.
> >
> > The patch also fixes the files referencing to the old st,stm32-rcc.txt.
> >
> > Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
>
> Drop the tag. It was conditional.
>
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - items:
> > +          - const: st,stm32f42xx-rcc
> > +          - const: st,stm32-rcc
> > +      - items:
> > +          - enum:
> > +              - st,stm32f469-rcc
> > +          - const: st,stm32f42xx-rcc
> > +          - const: st,stm32-rcc
> > +      - items:
> > +          - const: st,stm32f746-rcc
> > +          - const: st,stm32-rcc
>
> Nothing improved here.

In my humble opinion, there is nothing to improve. Any modification
made causes the tests to fail.

$ git grep st,stm32f746-rcc arch/
arch/arm/boot/dts/st/stm32f746.dtsi:                    compatible =
"st,stm32f746-rcc", "st,stm32-rcc";
arch/arm/boot/dts/st/stm32f769-disco.dts:       compatible =
"st,stm32f769-rcc", "st,stm32f746-rcc", "st,stm32-rcc";

Or am I missing something?

Thanks and regards,
Dario
>
> Best regards,
> Krzysztof
Krzysztof Kozlowski Jan. 14, 2025, 9:48 a.m. UTC | #3
On 14/01/2025 10:36, Dario Binacchi wrote:
>> Nothing improved here.
> 
> In my humble opinion, there is nothing to improve. Any modification
> made causes the tests to fail.
> 
> $ git grep st,stm32f746-rcc arch/
> arch/arm/boot/dts/st/stm32f746.dtsi:                    compatible =
> "st,stm32f746-rcc", "st,stm32-rcc";
> arch/arm/boot/dts/st/stm32f769-disco.dts:       compatible =
> "st,stm32f769-rcc", "st,stm32f746-rcc", "st,stm32-rcc";
> 
> Or am I missing something?

How can I know what you are missing if you do not show the code?

Best regards,
Krzysztof
Dario Binacchi Jan. 14, 2025, 10 a.m. UTC | #4
On Tue, Jan 14, 2025 at 10:48 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 14/01/2025 10:36, Dario Binacchi wrote:
> >> Nothing improved here.
> >
> > In my humble opinion, there is nothing to improve. Any modification
> > made causes the tests to fail.
> >
> > $ git grep st,stm32f746-rcc arch/
> > arch/arm/boot/dts/st/stm32f746.dtsi:                    compatible =
> > "st,stm32f746-rcc", "st,stm32-rcc";
> > arch/arm/boot/dts/st/stm32f769-disco.dts:       compatible =
> > "st,stm32f769-rcc", "st,stm32f746-rcc", "st,stm32-rcc";
> >
> > Or am I missing something?
>
> How can I know what you are missing if you do not show the code?

Sorry, but I still can't understand. I run multiple tests, trying to
modify things
based on what I understood of your suggestions, but the tests failed
every time.

Could you kindly provide an example of what you'd like me to do?

Thanks and regards,
Dario
>
> Best regards,
> Krzysztof
Krzysztof Kozlowski Jan. 14, 2025, 10:13 a.m. UTC | #5
On 14/01/2025 11:00, Dario Binacchi wrote:
> On Tue, Jan 14, 2025 at 10:48 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 14/01/2025 10:36, Dario Binacchi wrote:
>>>> Nothing improved here.
>>>
>>> In my humble opinion, there is nothing to improve. Any modification
>>> made causes the tests to fail.
>>>
>>> $ git grep st,stm32f746-rcc arch/
>>> arch/arm/boot/dts/st/stm32f746.dtsi:                    compatible =
>>> "st,stm32f746-rcc", "st,stm32-rcc";
>>> arch/arm/boot/dts/st/stm32f769-disco.dts:       compatible =
>>> "st,stm32f769-rcc", "st,stm32f746-rcc", "st,stm32-rcc";
>>>
>>> Or am I missing something?
>>
>> How can I know what you are missing if you do not show the code?
> 
> Sorry, but I still can't understand. I run multiple tests, trying to

You don't understand that I cannot improve your code if I do not see the
code? So let me rephrase: In order to tell what is wrong with some sort
of code, I need to see it. I cannot tell what is wrong with code without
seeing it.

> modify things
> based on what I understood of your suggestions, but the tests failed
> every time.
> 
> Could you kindly provide an example of what you'd like me to do?
Any qcom binding? Any other SoC binding with multiple devices?

Best regards,
Krzysztof
Dario Binacchi Jan. 14, 2025, 11:25 a.m. UTC | #6
On Tue, Jan 14, 2025 at 11:13 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 14/01/2025 11:00, Dario Binacchi wrote:
> > On Tue, Jan 14, 2025 at 10:48 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On 14/01/2025 10:36, Dario Binacchi wrote:
> >>>> Nothing improved here.
> >>>
> >>> In my humble opinion, there is nothing to improve. Any modification
> >>> made causes the tests to fail.
> >>>
> >>> $ git grep st,stm32f746-rcc arch/
> >>> arch/arm/boot/dts/st/stm32f746.dtsi:                    compatible =
> >>> "st,stm32f746-rcc", "st,stm32-rcc";
> >>> arch/arm/boot/dts/st/stm32f769-disco.dts:       compatible =
> >>> "st,stm32f769-rcc", "st,stm32f746-rcc", "st,stm32-rcc";
> >>>
> >>> Or am I missing something?
> >>
> >> How can I know what you are missing if you do not show the code?
> >
> > Sorry, but I still can't understand. I run multiple tests, trying to
>
> You don't understand that I cannot improve your code if I do not see the
> code? So let me rephrase: In order to tell what is wrong with some sort
> of code, I need to see it. I cannot tell what is wrong with code without
> seeing it.

You told me that this code was not exactly correct for the parts
marked with *********:

properties:
  compatible:
    oneOf:
      - items:
          - const: st,stm32f42xx-rcc
          - const: st,stm32-rcc
      - items:
          - enum:
              - st,stm32f469-rcc
          - const: st,stm32f42xx-rcc
          - const: st,stm32-rcc
      - items:
          - const: st,stm32f746-rcc ********
          - const: st,stm32-rcc
      - items:
          - enum:
              - st,stm32f769-rcc
          - const: st,stm32f746-rcc
          - const: st,stm32-rcc
      - items:
          - const: st,stm32h743-rcc *********
          - const: st,stm32-rcc

I haven't found a way to make changes to those elements without causing the
tests to fail. Could you kindly provide more explicit guidance on the kind of
modification you're expecting?

Thanks and regards,
Dario

>
> > modify things
> > based on what I understood of your suggestions, but the tests failed
> > every time.
> >
> > Could you kindly provide an example of what you'd like me to do?
> Any qcom binding? Any other SoC binding with multiple devices?
>
> Best regards,
> Krzysztof
--

Dario Binacchi

Senior Embedded Linux Developer

dario.binacchi@amarulasolutions.com
Krzysztof Kozlowski Jan. 14, 2025, 12:10 p.m. UTC | #7
On 14/01/2025 12:25, Dario Binacchi wrote:
> On Tue, Jan 14, 2025 at 11:13 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 14/01/2025 11:00, Dario Binacchi wrote:
>>> On Tue, Jan 14, 2025 at 10:48 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>
>>>> On 14/01/2025 10:36, Dario Binacchi wrote:
>>>>>> Nothing improved here.
>>>>>
>>>>> In my humble opinion, there is nothing to improve. Any modification
>>>>> made causes the tests to fail.
>>>>>
>>>>> $ git grep st,stm32f746-rcc arch/
>>>>> arch/arm/boot/dts/st/stm32f746.dtsi:                    compatible =
>>>>> "st,stm32f746-rcc", "st,stm32-rcc";
>>>>> arch/arm/boot/dts/st/stm32f769-disco.dts:       compatible =
>>>>> "st,stm32f769-rcc", "st,stm32f746-rcc", "st,stm32-rcc";
>>>>>
>>>>> Or am I missing something?
>>>>
>>>> How can I know what you are missing if you do not show the code?
>>>
>>> Sorry, but I still can't understand. I run multiple tests, trying to
>>
>> You don't understand that I cannot improve your code if I do not see the
>> code? So let me rephrase: In order to tell what is wrong with some sort
>> of code, I need to see it. I cannot tell what is wrong with code without
>> seeing it.
> 
> You told me that this code was not exactly correct for the parts
> marked with *********:

All places with same fallbacks should be unified. You have at least two
of such groups.

> 
> properties:
>   compatible:
>     oneOf:
>       - items:
>           - const: st,stm32f42xx-rcc
>           - const: st,stm32-rcc
>       - items:
>           - enum:
>               - st,stm32f469-rcc
>           - const: st,stm32f42xx-rcc
>           - const: st,stm32-rcc
>       - items:
>           - const: st,stm32f746-rcc ********
>           - const: st,stm32-rcc
>       - items:
>           - enum:
>               - st,stm32f769-rcc
>           - const: st,stm32f746-rcc
>           - const: st,stm32-rcc
>       - items:
>           - const: st,stm32h743-rcc *********
>           - const: st,stm32-rcc
> 
> I haven't found a way to make changes to those elements without causing the
> tests to fail. Could you kindly provide more explicit guidance on the kind of
> modification you're expecting?
> 

items:
  - enum:
      - foo
      - bar
   - const: baz

Like I said: all Qcom bindings. Probably all NXP as well.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/clock/st,stm32-rcc.txt b/Documentation/devicetree/bindings/clock/st,stm32-rcc.txt
deleted file mode 100644
index cfa04b614d8a..000000000000
--- a/Documentation/devicetree/bindings/clock/st,stm32-rcc.txt
+++ /dev/null
@@ -1,138 +0,0 @@ 
-STMicroelectronics STM32 Reset and Clock Controller
-===================================================
-
-The RCC IP is both a reset and a clock controller.
-
-Please refer to clock-bindings.txt for common clock controller binding usage.
-Please also refer to reset.txt for common reset controller binding usage.
-
-Required properties:
-- compatible: Should be:
-  "st,stm32f42xx-rcc"
-  "st,stm32f469-rcc"
-  "st,stm32f746-rcc"
-  "st,stm32f769-rcc"
-
-- reg: should be register base and length as documented in the
-  datasheet
-- #reset-cells: 1, see below
-- #clock-cells: 2, device nodes should specify the clock in their "clocks"
-  property, containing a phandle to the clock device node, an index selecting
-  between gated clocks and other clocks and an index specifying the clock to
-  use.
-- clocks: External oscillator clock phandle
-  - high speed external clock signal (HSE)
-  - external I2S clock (I2S_CKIN)
-
-Example:
-
-	rcc: rcc@40023800 {
-		#reset-cells = <1>;
-		#clock-cells = <2>
-		compatible = "st,stm32f42xx-rcc", "st,stm32-rcc";
-		reg = <0x40023800 0x400>;
-		clocks = <&clk_hse>, <&clk_i2s_ckin>;
-	};
-
-Specifying gated clocks
-=======================
-
-The primary index must be set to 0.
-
-The secondary index is the bit number within the RCC register bank, starting
-from the first RCC clock enable register (RCC_AHB1ENR, address offset 0x30).
-
-It is calculated as: index = register_offset / 4 * 32 + bit_offset.
-Where bit_offset is the bit offset within the register (LSB is 0, MSB is 31).
-
-To simplify the usage and to share bit definition with the reset and clock
-drivers of the RCC IP, macros are available to generate the index in
-human-readble format.
-
-For STM32F4 series, the macro are available here:
- - include/dt-bindings/mfd/stm32f4-rcc.h
-
-Example:
-
-	/* Gated clock, AHB1 bit 0 (GPIOA) */
-	... {
-		clocks = <&rcc 0 STM32F4_AHB1_CLOCK(GPIOA)>
-	};
-
-	/* Gated clock, AHB2 bit 4 (CRYP) */
-	... {
-		clocks = <&rcc 0 STM32F4_AHB2_CLOCK(CRYP)>
-	};
-
-Specifying other clocks
-=======================
-
-The primary index must be set to 1.
-
-The secondary index is bound with the following magic numbers:
-
-	0	SYSTICK
-	1	FCLK
-	2	CLK_LSI		(low-power clock source)
-	3	CLK_LSE		(generated from a 32.768 kHz low-speed external
-				 crystal or ceramic resonator)
-	4	CLK_HSE_RTC	(HSE division factor for RTC clock)
-	5	CLK_RTC		(real-time clock)
-	6	PLL_VCO_I2S	(vco frequency of I2S pll)
-	7	PLL_VCO_SAI	(vco frequency of SAI pll)
-	8	CLK_LCD		(LCD-TFT)
-	9	CLK_I2S		(I2S clocks)
-	10	CLK_SAI1	(audio clocks)
-	11	CLK_SAI2
-	12	CLK_I2SQ_PDIV	(post divisor of pll i2s q divisor)
-	13	CLK_SAIQ_PDIV	(post divisor of pll sai q divisor)
-
-	14	CLK_HSI		(Internal ocscillator clock)
-	15	CLK_SYSCLK	(System Clock)
-	16	CLK_HDMI_CEC	(HDMI-CEC clock)
-	17	CLK_SPDIF	(SPDIF-Rx clock)
-	18	CLK_USART1	(U(s)arts clocks)
-	19	CLK_USART2
-	20	CLK_USART3
-	21	CLK_UART4
-	22	CLK_UART5
-	23	CLK_USART6
-	24	CLK_UART7
-	25	CLK_UART8
-	26	CLK_I2C1	(I2S clocks)
-	27	CLK_I2C2
-	28	CLK_I2C3
-	29	CLK_I2C4
-	30	CLK_LPTIMER	(LPTimer1 clock)
-	31	CLK_PLL_SRC
-	32	CLK_DFSDM1
-	33	CLK_ADFSDM1
-	34	CLK_F769_DSI
-)
-
-Example:
-
-	/* Misc clock, FCLK */
-	... {
-		clocks = <&rcc 1 STM32F4_APB1_CLOCK(TIM2)>
-	};
-
-
-Specifying softreset control of devices
-=======================================
-
-Device nodes should specify the reset channel required in their "resets"
-property, containing a phandle to the reset device node and an index specifying
-which channel to use.
-The index is the bit number within the RCC registers bank, starting from RCC
-base address.
-It is calculated as: index = register_offset / 4 * 32 + bit_offset.
-Where bit_offset is the bit offset within the register.
-For example, for CRC reset:
-  crc = AHB1RSTR_offset / 4 * 32 + CRCRST_bit_offset = 0x10 / 4 * 32 + 12 = 140
-
-example:
-
-	timer2 {
-		resets	= <&rcc STM32F4_APB1_RESET(TIM2)>;
-	};
diff --git a/Documentation/devicetree/bindings/clock/st,stm32-rcc.yaml b/Documentation/devicetree/bindings/clock/st,stm32-rcc.yaml
new file mode 100644
index 000000000000..779e547700be
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/st,stm32-rcc.yaml
@@ -0,0 +1,111 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/st,stm32-rcc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STMicroelectronics STM32 Reset Clock Controller
+
+maintainers:
+  - Dario Binacchi <dario.binacchi@amarulasolutions.com>
+
+description: |
+  The RCC IP is both a reset and a clock controller.
+  The reset phandle argument is the bit number within the RCC registers bank,
+  starting from RCC base address.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - const: st,stm32f42xx-rcc
+          - const: st,stm32-rcc
+      - items:
+          - enum:
+              - st,stm32f469-rcc
+          - const: st,stm32f42xx-rcc
+          - const: st,stm32-rcc
+      - items:
+          - const: st,stm32f746-rcc
+          - const: st,stm32-rcc
+      - items:
+          - enum:
+              - st,stm32f769-rcc
+          - const: st,stm32f746-rcc
+          - const: st,stm32-rcc
+      - items:
+          - const: st,stm32h743-rcc
+          - const: st,stm32-rcc
+
+  reg:
+    maxItems: 1
+
+  '#reset-cells':
+    const: 1
+
+  '#clock-cells':
+    enum: [1, 2]
+
+  clocks:
+    minItems: 2
+    maxItems: 3
+
+  st,syscfg:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      Phandle to system configuration controller. It can be used to control the
+      power domain circuitry.
+
+required:
+  - compatible
+  - reg
+  - '#reset-cells'
+  - '#clock-cells'
+  - clocks
+  - st,syscfg
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: st,stm32h743-rcc
+    then:
+      properties:
+        '#clock-cells':
+          const: 1
+          description: |
+            The clock index for the specified type.
+        clocks:
+          items:
+            - description: high speed external (HSE) clock input
+            - description: low speed external (LSE) clock input
+            - description: Inter-IC sound (I2S) clock input
+    else:
+      properties:
+        '#clock-cells':
+          const: 2
+          description: |
+            - The first cell is the clock type, possible values are 0 for
+              gated clocks and 1 otherwise.
+            - The second cell is the clock index for the specified type.
+        clocks:
+          items:
+            - description: high speed external (HSE) clock input
+            - description: Inter-IC sound (I2S) clock input
+
+additionalProperties: false
+
+examples:
+  # Reset and Clock Control Module node:
+  - |
+    clock-controller@58024400 {
+        compatible = "st,stm32h743-rcc", "st,stm32-rcc";
+        reg = <0x58024400 0x400>;
+        #clock-cells = <1>;
+        #reset-cells = <1>;
+        clocks = <&clk_hse>, <&clk_lse>, <&clk_i2s>;
+        st,syscfg = <&pwrcfg>;
+    };
+
+...
diff --git a/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
index 01db34375192..384035e8e60b 100644
--- a/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
+++ b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
@@ -3,4 +3,4 @@  STMicroelectronics STM32 Peripheral Reset Controller
 
 The RCC IP is both a reset and a clock controller.
 
-Please see Documentation/devicetree/bindings/clock/st,stm32-rcc.txt
+Please see Documentation/devicetree/bindings/clock/st,stm32-rcc.yaml
diff --git a/include/dt-bindings/clock/stm32fx-clock.h b/include/dt-bindings/clock/stm32fx-clock.h
index e5dad050d518..b6ff9c68cb3f 100644
--- a/include/dt-bindings/clock/stm32fx-clock.h
+++ b/include/dt-bindings/clock/stm32fx-clock.h
@@ -10,7 +10,7 @@ 
  * List of clocks which are not derived from system clock (SYSCLOCK)
  *
  * The index of these clocks is the secondary index of DT bindings
- * (see Documentation/devicetree/bindings/clock/st,stm32-rcc.txt)
+ * (see Documentation/devicetree/bindings/clock/st,stm32-rcc.yaml)
  *
  * e.g:
 	<assigned-clocks = <&rcc 1 CLK_LSE>;