[PATCH v2 06/10] dt-bindings: clock: airoha: Document new property airoha,chip-scu

Christian Marangi posted 10 patches 7 months, 3 weeks ago
There is a newer version of this series
[PATCH v2 06/10] dt-bindings: clock: airoha: Document new property airoha,chip-scu
Posted by Christian Marangi 7 months, 3 weeks ago
Document new property airoha,chip-scu used on new Airoha SoC to
reference the Chip SCU syscon node used for PCIe configuration.

Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 .../devicetree/bindings/clock/airoha,en7523-scu.yaml      | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
index fe2c5c1baf43..bce77a14c938 100644
--- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
+++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
@@ -49,6 +49,11 @@ properties:
     description: ID of the controller reset line
     const: 1
 
+  airoha,chip-scu:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: Phandle to the Chip SCU syscon node for PCIe
+      configuration
+
 required:
   - compatible
   - reg
@@ -66,6 +71,8 @@ allOf:
 
         '#reset-cells': false
 
+        airoha,chip-scu: false
+
   - if:
       properties:
         compatible:
@@ -97,5 +104,6 @@ examples:
         reg = <0x0 0x1fb00000 0x0 0x970>;
               #clock-cells = <1>;
               #reset-cells = <1>;
+        airoha,chip-scu = <&chip_scu>;
       };
     };
-- 
2.48.1
Re: [PATCH v2 06/10] dt-bindings: clock: airoha: Document new property airoha,chip-scu
Posted by Krzysztof Kozlowski 7 months, 2 weeks ago
On Tue, Jun 17, 2025 at 03:04:49PM +0200, Christian Marangi wrote:
> Document new property airoha,chip-scu used on new Airoha SoC to
> reference the Chip SCU syscon node used for PCIe configuration.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  .../devicetree/bindings/clock/airoha,en7523-scu.yaml      | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> index fe2c5c1baf43..bce77a14c938 100644
> --- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> +++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> @@ -49,6 +49,11 @@ properties:
>      description: ID of the controller reset line
>      const: 1
>  
> +  airoha,chip-scu:

So the scu has phandle to scu... That's not what we discussed. Your
changelog also is very vague here, no links to previous discussions does
not make reviewing it easier.

You clearly said you have SCU node wich clocks and now you claim you
have here some different device thus you need phandle. This is what your
schema says.

No.

Where is the DTS with COMPLETE picture?

Best regards,
Krzysztof
Re: [PATCH v2 06/10] dt-bindings: clock: airoha: Document new property airoha,chip-scu
Posted by Christian Marangi 7 months, 2 weeks ago
On Fri, Jun 27, 2025 at 09:59:34AM +0200, Krzysztof Kozlowski wrote:
> On Tue, Jun 17, 2025 at 03:04:49PM +0200, Christian Marangi wrote:
> > Document new property airoha,chip-scu used on new Airoha SoC to
> > reference the Chip SCU syscon node used for PCIe configuration.
> > 
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > ---
> >  .../devicetree/bindings/clock/airoha,en7523-scu.yaml      | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> > index fe2c5c1baf43..bce77a14c938 100644
> > --- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> > +++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
> > @@ -49,6 +49,11 @@ properties:
> >      description: ID of the controller reset line
> >      const: 1
> >  
> > +  airoha,chip-scu:
> 
> So the scu has phandle to scu... That's not what we discussed. Your
> changelog also is very vague here, no links to previous discussions does
> not make reviewing it easier.
>

Do you think it might be better to add to the changlog link to the
previous version?

> You clearly said you have SCU node wich clocks and now you claim you
> have here some different device thus you need phandle. This is what your
> schema says.
> 

There is "SCU" and "Chip SCU". This new schema is to keep consistency
with an7581 as MFD is quite problematic.

Also I implemented the current mdio schema with 2 line with compatible
and reg from suggestion of Rob review.

> No.
> 
> Where is the DTS with COMPLETE picture?
> 
> 

Here the current DTS [1]. Nothing is stable for this and we can change
it but I want to stress that the current HW block are VERY CONFUSING and
SCRAMBELED. So it's really a matter of finding the least bad solution.

In SCU there are:
- PART fot the clock register
- 2 MDIO controller register

In chip SCU:
- Other part of the clock register
- Thermal driver register
- PART of the pinctrl register

[1] https://github.com/Ansuel/openwrt/blob/openwrt-24.10-airoha-an7581-stable/target/linux/airoha/dts/an7583.dtsi#L361

-- 
	Ansuel
Re: [PATCH v2 06/10] dt-bindings: clock: airoha: Document new property airoha,chip-scu
Posted by Krzysztof Kozlowski 6 months, 3 weeks ago
On 27/06/2025 10:20, Christian Marangi wrote:
> 
> Here the current DTS [1]. Nothing is stable for this and we can change
> it but I want to stress that the current HW block are VERY CONFUSING and
> SCRAMBELED. So it's really a matter of finding the least bad solution.
> 
> In SCU there are:
> - PART fot the clock register
> - 2 MDIO controller register
> 
> In chip SCU:
> - Other part of the clock register
> - Thermal driver register
> - PART of the pinctrl register
> 
> [1] https://github.com/Ansuel/openwrt/blob/openwrt-24.10-airoha-an7581-stable/target/linux/airoha/dts/an7583.dtsi#L361


Thanks and it proves: that's a no. You cannot have two devices with same
unit address. It means that chip-scu and scu ARE THE SAME devices.

> 


Best regards,
Krzysztof
Re: [PATCH v2 06/10] dt-bindings: clock: airoha: Document new property airoha,chip-scu
Posted by Christian Marangi 6 months, 3 weeks ago
On Wed, Jul 16, 2025 at 04:29:12PM +0200, Krzysztof Kozlowski wrote:
> On 27/06/2025 10:20, Christian Marangi wrote:
> > 
> > Here the current DTS [1]. Nothing is stable for this and we can change
> > it but I want to stress that the current HW block are VERY CONFUSING and
> > SCRAMBELED. So it's really a matter of finding the least bad solution.
> > 
> > In SCU there are:
> > - PART fot the clock register
> > - 2 MDIO controller register
> > 
> > In chip SCU:
> > - Other part of the clock register
> > - Thermal driver register
> > - PART of the pinctrl register
> > 
> > [1] https://github.com/Ansuel/openwrt/blob/openwrt-24.10-airoha-an7581-stable/target/linux/airoha/dts/an7583.dtsi#L361
> 
> 
> Thanks and it proves: that's a no. You cannot have two devices with same
> unit address. It means that chip-scu and scu ARE THE SAME devices.
> 

Thanks for checking it. Hope it's clear that

scuclk: system-controller@1fa20000

is a typo and should be

scuclk: system-controller@1fb00000
(to follow the reg property reg = <0x0 0x1fb00000 0x0 0x970>;
 with the 0x970 taken from the documentation)

With this in mind and if your comment still apply do you have any hint
how to better reorganize the 2 node?

-- 
	Ansuel