[PATCH 3/4] dt-bindings: net: nxp,s32-dwmac: Use the GPR syscon

Dan Carpenter posted 4 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH 3/4] dt-bindings: net: nxp,s32-dwmac: Use the GPR syscon
Posted by Dan Carpenter 2 months, 1 week ago
The S32 chipset has a GPR region which has a miscellaneous registers
including the GMAC_0_CTRL_STS register.  Originally this code accessed
that register in a sort of ad-hoc way, but we want to access it using
the syscon interface.

We still need to maintain the old method of accessing the GMAC register
but using a syscon will let us access other registers more cleanly.

Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml b/Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml
index 2b8b74c5feec..17f6c50dca03 100644
--- a/Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml
@@ -32,6 +32,11 @@ properties:
       - description: Main GMAC registers
       - description: GMAC PHY mode control register
 
+  phy-sel:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      - description: The offset into the s32 GPR syscon
+
   interrupts:
     maxItems: 1
 
@@ -74,6 +79,7 @@ examples:
         compatible = "nxp,s32g2-dwmac";
         reg = <0x0 0x4033c000 0x0 0x2000>, /* gmac IP */
               <0x0 0x4007c004 0x0 0x4>;    /* GMAC_0_CTRL_STS */
+        phy-sel = <&gpr 0x4>;
         interrupt-parent = <&gic>;
         interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
         interrupt-names = "macirq";
-- 
2.51.0
Re: [PATCH 3/4] dt-bindings: net: nxp,s32-dwmac: Use the GPR syscon
Posted by Krzysztof Kozlowski 2 months, 1 week ago
On 01/12/2025 14:08, Dan Carpenter wrote:
> The S32 chipset has a GPR region which has a miscellaneous registers
> including the GMAC_0_CTRL_STS register.  Originally this code accessed
> that register in a sort of ad-hoc way, but we want to access it using
> the syscon interface.
> 
> We still need to maintain the old method of accessing the GMAC register
> but using a syscon will let us access other registers more cleanly.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>  Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml b/Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml
> index 2b8b74c5feec..17f6c50dca03 100644
> --- a/Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml
> @@ -32,6 +32,11 @@ properties:
>        - description: Main GMAC registers
>        - description: GMAC PHY mode control register
>  
> +  phy-sel:

Missing vendor prefix.

> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      - description: The offset into the s32 GPR syscon

No, first item is not the offset but the phandle. You need syntax like here:

https://elixir.bootlin.com/linux/v5.18-rc1/source/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml#L42

The description of the first item (unlike in example above) should say
what is the purpose, how this device is using GPR region, what is it
needed for.

Best regards,
Krzysztof
Re: [PATCH 3/4] dt-bindings: net: nxp,s32-dwmac: Use the GPR syscon
Posted by Dan Carpenter 1 month, 3 weeks ago
On Mon, Dec 01, 2025 at 06:33:07PM +0100, Krzysztof Kozlowski wrote:
> On 01/12/2025 14:08, Dan Carpenter wrote:
> > The S32 chipset has a GPR region which has a miscellaneous registers
> > including the GMAC_0_CTRL_STS register.  Originally this code accessed
> > that register in a sort of ad-hoc way, but we want to access it using
> > the syscon interface.
> > 
> > We still need to maintain the old method of accessing the GMAC register
> > but using a syscon will let us access other registers more cleanly.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > ---
> >  Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml b/Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml
> > index 2b8b74c5feec..17f6c50dca03 100644
> > --- a/Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml
> > +++ b/Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml
> > @@ -32,6 +32,11 @@ properties:
> >        - description: Main GMAC registers
> >        - description: GMAC PHY mode control register
> >  
> > +  phy-sel:
> 
> Missing vendor prefix.
> 
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    items:
> > +      - description: The offset into the s32 GPR syscon
> 
> No, first item is not the offset but the phandle. You need syntax like here:
> 
> https://elixir.bootlin.com/linux/v5.18-rc1/source/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml#L42
> 
> The description of the first item (unlike in example above) should say
> what is the purpose, how this device is using GPR region, what is it
> needed for.

I had to do it a bit differently from the exynos-usi.yaml code.  When I
tried it that way I got the following warning that the "phy-sel" wasn't
a common suffix and it doesn't have a description.

$ make dt_binding_check DT_SCHEMA_FILES=net/nxp,s32-dwmac.yaml
  SCHEMA  Documentation/devicetree/bindings/processed-schema.json
  CHKDT   ./Documentation/devicetree/bindings
/home/dcarpenter/progs/kernel/nxp_gpr/Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml: properties:nxp,phy-sel: 'anyOf' conditional failed, one must be fixed:
        'description' is a dependency of '$ref'
        '/schemas/types.yaml#/definitions/phandle-array' does not match '^#/(definitions|\\$defs)/'
                hint: A vendor property can have a $ref to a a $defs schema
        hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
        from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
  LINT    ./Documentation/devicetree/bindings
  DTEX    Documentation/devicetree/bindings/net/nxp,s32-dwmac.example.dts
  DTC [C] Documentation/devicetree/bindings/net/nxp,s32-dwmac.example.dtb

The exynos-usi.yaml file doesn't generate a warning like that and I wasn't
able to figure out why that is.  But what worked for me was adding the
phandle description like this:

  nxp,phy-sel:
    description: phandle to the GPR syscon node
    $ref: /schemas/types.yaml#/definitions/phandle-array
    items:
      - description: offset of PHY selection register

regards,
dan carpenter
Re: [PATCH 3/4] dt-bindings: net: nxp,s32-dwmac: Use the GPR syscon
Posted by Dan Carpenter 1 month, 3 weeks ago
On Mon, Dec 15, 2025 at 01:59:09PM +0300, Dan Carpenter wrote:
> On Mon, Dec 01, 2025 at 06:33:07PM +0100, Krzysztof Kozlowski wrote:
> > On 01/12/2025 14:08, Dan Carpenter wrote:
> > > The S32 chipset has a GPR region which has a miscellaneous registers
> > > including the GMAC_0_CTRL_STS register.  Originally this code accessed
> > > that register in a sort of ad-hoc way, but we want to access it using
> > > the syscon interface.
> > > 
> > > We still need to maintain the old method of accessing the GMAC register
> > > but using a syscon will let us access other registers more cleanly.
> > > 
> > > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > > ---
> > >  Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml b/Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml
> > > index 2b8b74c5feec..17f6c50dca03 100644
> > > --- a/Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml
> > > +++ b/Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml
> > > @@ -32,6 +32,11 @@ properties:
> > >        - description: Main GMAC registers
> > >        - description: GMAC PHY mode control register
> > >  
> > > +  phy-sel:
> > 
> > Missing vendor prefix.
> > 
> > > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > > +    items:
> > > +      - description: The offset into the s32 GPR syscon
> > 
> > No, first item is not the offset but the phandle. You need syntax like here:
> > 
> > https://elixir.bootlin.com/linux/v5.18-rc1/source/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml#L42
> > 
> > The description of the first item (unlike in example above) should say
> > what is the purpose, how this device is using GPR region, what is it
> > needed for.
> 
> I had to do it a bit differently from the exynos-usi.yaml code.  When I
> tried it that way I got the following warning that the "phy-sel" wasn't
> a common suffix and it doesn't have a description.
> 
> $ make dt_binding_check DT_SCHEMA_FILES=net/nxp,s32-dwmac.yaml
>   SCHEMA  Documentation/devicetree/bindings/processed-schema.json
>   CHKDT   ./Documentation/devicetree/bindings
> /home/dcarpenter/progs/kernel/nxp_gpr/Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml: properties:nxp,phy-sel: 'anyOf' conditional failed, one must be fixed:
>         'description' is a dependency of '$ref'
>         '/schemas/types.yaml#/definitions/phandle-array' does not match '^#/(definitions|\\$defs)/'
>                 hint: A vendor property can have a $ref to a a $defs schema
>         hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
>         from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
>   LINT    ./Documentation/devicetree/bindings
>   DTEX    Documentation/devicetree/bindings/net/nxp,s32-dwmac.example.dts
>   DTC [C] Documentation/devicetree/bindings/net/nxp,s32-dwmac.example.dtb
> 
> The exynos-usi.yaml file doesn't generate a warning like that and I wasn't
> able to figure out why that is.

Oh, crap.  I'm an idiot.  I've been staring at this for an embarrassing
long time, and I didn't see that the exynos-usi.yaml has a description
after the - items description.  And then I send this and I immediately
see it.  :/

regards,
dan carpenter