devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] s32g: Use a syscon for GPR
@ 2025-12-01 13:08 Dan Carpenter
  2025-12-01 13:08 ` [PATCH 2/4] dt-bindings: mfd: syscon: Document the GPR syscon for the NXP S32 SoCs Dan Carpenter
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Dan Carpenter @ 2025-12-01 13:08 UTC (permalink / raw)
  To: Chester Lin
  Cc: Alexandre Torgue, Andrew Lunn, Conor Dooley, David S. Miller,
	devicetree, Eric Dumazet, Fabio Estevam, Ghennadi Procopciuc, imx,
	Jakub Kicinski, Jan Petrous, Krzysztof Kozlowski, Lee Jones,
	linux-arm-kernel, linux-kernel, linux-stm32, Matthias Brugger,
	Maxime Coquelin, netdev, NXP S32 Linux Team, Paolo Abeni,
	Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo,
	linaro-s32

*** BLURB HERE ***

Dan Carpenter (4):
  net: stmmac: s32: use the syscon interface PHY_INTF_SEL_RGMII
  dt-bindings: mfd: syscon: Document the GPR syscon for the NXP S32 SoCs
  dt-bindings: net: nxp,s32-dwmac: Use the GPR syscon
  dts: s32g: Add GPR syscon region

 .../devicetree/bindings/mfd/syscon.yaml       |  2 ++
 .../bindings/net/nxp,s32-dwmac.yaml           |  6 +++++
 arch/arm64/boot/dts/freescale/s32g2.dtsi      |  8 +++++++
 arch/arm64/boot/dts/freescale/s32g3.dtsi      |  8 +++++++
 .../net/ethernet/stmicro/stmmac/dwmac-s32.c   | 23 +++++++++++++++----
 5 files changed, 42 insertions(+), 5 deletions(-)

-- 
2.51.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/4] dt-bindings: mfd: syscon: Document the GPR syscon for the NXP S32 SoCs
  2025-12-01 13:08 [PATCH 0/4] s32g: Use a syscon for GPR Dan Carpenter
@ 2025-12-01 13:08 ` Dan Carpenter
  2025-12-01 17:30   ` Krzysztof Kozlowski
  2025-12-01 13:08 ` [PATCH 3/4] dt-bindings: net: nxp,s32-dwmac: Use the GPR syscon Dan Carpenter
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Dan Carpenter @ 2025-12-01 13:08 UTC (permalink / raw)
  To: Lee Jones
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, devicetree,
	linux-kernel, NXP S32 Linux Team, linaro-s32

The NXP S32 SoCs have a GPR region which is used by a variety of
drivers.  Some examples of the registers in this region are:
  * DDR_PMU_IRQ
  * GMAC0_PHY_INTF_SEL
  * GMAC1_PHY_INTF_SEL
  * PFE_EMACS_INTF_SEL
  * PFE_COH_EN
  * PFE_PWR_CTRL
  * PFE_EMACS_GENCTRL1
  * PFE_GENCTRL3

Use the syscon interface to access these registers.

Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 Documentation/devicetree/bindings/mfd/syscon.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
index 55efb83b1495..6e6b92227092 100644
--- a/Documentation/devicetree/bindings/mfd/syscon.yaml
+++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
@@ -102,6 +102,7 @@ select:
           - mstar,msc313-pmsleep
           - nuvoton,ma35d1-sys
           - nuvoton,wpcm450-shm
+          - nxp,s32-gpr
           - qcom,apq8064-mmss-sfpb
           - qcom,apq8064-sps-sic
           - rockchip,px30-qos
@@ -212,6 +213,7 @@ properties:
               - mstar,msc313-pmsleep
               - nuvoton,ma35d1-sys
               - nuvoton,wpcm450-shm
+              - nxp,s32-gpr
               - qcom,apq8064-mmss-sfpb
               - qcom,apq8064-sps-sic
               - rockchip,px30-qos
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/4] dt-bindings: net: nxp,s32-dwmac: Use the GPR syscon
  2025-12-01 13:08 [PATCH 0/4] s32g: Use a syscon for GPR Dan Carpenter
  2025-12-01 13:08 ` [PATCH 2/4] dt-bindings: mfd: syscon: Document the GPR syscon for the NXP S32 SoCs Dan Carpenter
@ 2025-12-01 13:08 ` Dan Carpenter
  2025-12-01 17:33   ` Krzysztof Kozlowski
  2025-12-01 13:08 ` [PATCH 4/4] dts: s32g: Add GPR syscon region Dan Carpenter
  2025-12-01 14:31 ` [PATCH 0/4] s32g: Use a syscon for GPR Dan Carpenter
  3 siblings, 1 reply; 11+ messages in thread
From: Dan Carpenter @ 2025-12-01 13:08 UTC (permalink / raw)
  To: Jan Petrous
  Cc: s32, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	netdev, devicetree, linux-kernel, linaro-s32

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/4] dts: s32g: Add GPR syscon region
  2025-12-01 13:08 [PATCH 0/4] s32g: Use a syscon for GPR Dan Carpenter
  2025-12-01 13:08 ` [PATCH 2/4] dt-bindings: mfd: syscon: Document the GPR syscon for the NXP S32 SoCs Dan Carpenter
  2025-12-01 13:08 ` [PATCH 3/4] dt-bindings: net: nxp,s32-dwmac: Use the GPR syscon Dan Carpenter
@ 2025-12-01 13:08 ` Dan Carpenter
  2025-12-01 16:42   ` Frank Li
  2025-12-01 14:31 ` [PATCH 0/4] s32g: Use a syscon for GPR Dan Carpenter
  3 siblings, 1 reply; 11+ messages in thread
From: Dan Carpenter @ 2025-12-01 13:08 UTC (permalink / raw)
  To: Chester Lin
  Cc: Matthias Brugger, Ghennadi Procopciuc, NXP S32 Linux Team,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-arm-kernel,
	imx, devicetree, linux-kernel, linaro-s32

Add the GPR syscon region for the s32 chipset.

Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 arch/arm64/boot/dts/freescale/s32g2.dtsi | 8 ++++++++
 arch/arm64/boot/dts/freescale/s32g3.dtsi | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
index 51d00dac12de..3c9472f6c174 100644
--- a/arch/arm64/boot/dts/freescale/s32g2.dtsi
+++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
@@ -325,6 +325,13 @@ usdhc0-200mhz-grp4 {
 			};
 		};
 
+		gpr: syscon@4007c000 {
+			compatible = "nxp,s32-gpr", "syscon";
+			reg = <0x4007c000 0x3000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+		};
+
 		ocotp: nvmem@400a4000 {
 			compatible = "nxp,s32g2-ocotp";
 			reg = <0x400a4000 0x400>;
@@ -731,6 +738,7 @@ gmac0: ethernet@4033c000 {
 			compatible = "nxp,s32g2-dwmac";
 			reg = <0x4033c000 0x2000>, /* gmac IP */
 			      <0x4007c004 0x4>;    /* GMAC_0_CTRL_STS */
+			phy-sel = <&gpr 0x4>;
 			interrupt-parent = <&gic>;
 			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "macirq";
diff --git a/arch/arm64/boot/dts/freescale/s32g3.dtsi b/arch/arm64/boot/dts/freescale/s32g3.dtsi
index eff7673e7f34..0ceca3caf133 100644
--- a/arch/arm64/boot/dts/freescale/s32g3.dtsi
+++ b/arch/arm64/boot/dts/freescale/s32g3.dtsi
@@ -383,6 +383,13 @@ usdhc0-200mhz-grp4 {
 			};
 		};
 
+		gpr: syscon@4007c000 {
+			compatible = "nxp,s32-gpr", "syscon";
+			reg = <0x4007c000 0x3000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+		};
+
 		ocotp: nvmem@400a4000 {
 			compatible = "nxp,s32g3-ocotp", "nxp,s32g2-ocotp";
 			reg = <0x400a4000 0x400>;
@@ -808,6 +815,7 @@ gmac0: ethernet@4033c000 {
 			compatible = "nxp,s32g2-dwmac";
 			reg = <0x4033c000 0x2000>, /* gmac IP */
 			      <0x4007c004 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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/4] s32g: Use a syscon for GPR
  2025-12-01 13:08 [PATCH 0/4] s32g: Use a syscon for GPR Dan Carpenter
                   ` (2 preceding siblings ...)
  2025-12-01 13:08 ` [PATCH 4/4] dts: s32g: Add GPR syscon region Dan Carpenter
@ 2025-12-01 14:31 ` Dan Carpenter
  3 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2025-12-01 14:31 UTC (permalink / raw)
  To: Chester Lin
  Cc: Alexandre Torgue, Andrew Lunn, Conor Dooley, David S. Miller,
	devicetree, Eric Dumazet, Fabio Estevam, Ghennadi Procopciuc, imx,
	Jakub Kicinski, Jan Petrous, Krzysztof Kozlowski, Lee Jones,
	linux-arm-kernel, linux-kernel, linux-stm32, Matthias Brugger,
	Maxime Coquelin, netdev, NXP S32 Linux Team, Paolo Abeni,
	Pengutronix Kernel Team, Rob Herring, Sascha Hauer, Shawn Guo,
	linaro-s32

On Mon, Dec 01, 2025 at 04:08:14PM +0300, Dan Carpenter wrote:
> *** BLURB HERE ***
> 

Sorry, I obviously meant to write a message here.

The s32g devices have a GPR register region which could be accessed
via a syscon.  Currently only the stmmac/dwmac-s32.c uses anything
from there and we just add a line to the device tree to access
that GMAC_0_CTRL_STS register:

			reg = <0x4033c000 0x2000>, /* gmac IP */
			      <0x4007c004 0x4>;    /* GMAC_0_CTRL_STS */

But it would be better to have a syscon instead of adding each
register to the device tree like this.

We still have to maintain backwards compatibility to this format,
of course.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] dts: s32g: Add GPR syscon region
  2025-12-01 13:08 ` [PATCH 4/4] dts: s32g: Add GPR syscon region Dan Carpenter
@ 2025-12-01 16:42   ` Frank Li
  2025-12-01 16:55     ` Russell King (Oracle)
  0 siblings, 1 reply; 11+ messages in thread
From: Frank Li @ 2025-12-01 16:42 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Chester Lin, Matthias Brugger, Ghennadi Procopciuc,
	NXP S32 Linux Team, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-arm-kernel, imx,
	devicetree, linux-kernel, linaro-s32

On Mon, Dec 01, 2025 at 04:08:33PM +0300, Dan Carpenter wrote:
> Add the GPR syscon region for the s32 chipset.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>  arch/arm64/boot/dts/freescale/s32g2.dtsi | 8 ++++++++
>  arch/arm64/boot/dts/freescale/s32g3.dtsi | 8 ++++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> index 51d00dac12de..3c9472f6c174 100644
> --- a/arch/arm64/boot/dts/freescale/s32g2.dtsi
> +++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> @@ -325,6 +325,13 @@ usdhc0-200mhz-grp4 {
>  			};
>  		};
>
> +		gpr: syscon@4007c000 {
> +			compatible = "nxp,s32-gpr", "syscon";
> +			reg = <0x4007c000 0x3000>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +		};
> +

Please cc whole thread to imx@lists.linux.dev.

I think it is not good method by using syscon here.

Suppose using standard phy interface or mux controller interface.

Frank

>  		ocotp: nvmem@400a4000 {
>  			compatible = "nxp,s32g2-ocotp";
>  			reg = <0x400a4000 0x400>;
> @@ -731,6 +738,7 @@ gmac0: ethernet@4033c000 {
>  			compatible = "nxp,s32g2-dwmac";
>  			reg = <0x4033c000 0x2000>, /* gmac IP */
>  			      <0x4007c004 0x4>;    /* GMAC_0_CTRL_STS */
> +			phy-sel = <&gpr 0x4>;
>  			interrupt-parent = <&gic>;
>  			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
>  			interrupt-names = "macirq";
> diff --git a/arch/arm64/boot/dts/freescale/s32g3.dtsi b/arch/arm64/boot/dts/freescale/s32g3.dtsi
> index eff7673e7f34..0ceca3caf133 100644
> --- a/arch/arm64/boot/dts/freescale/s32g3.dtsi
> +++ b/arch/arm64/boot/dts/freescale/s32g3.dtsi
> @@ -383,6 +383,13 @@ usdhc0-200mhz-grp4 {
>  			};
>  		};
>
> +		gpr: syscon@4007c000 {
> +			compatible = "nxp,s32-gpr", "syscon";
> +			reg = <0x4007c000 0x3000>;
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +		};
> +
>  		ocotp: nvmem@400a4000 {
>  			compatible = "nxp,s32g3-ocotp", "nxp,s32g2-ocotp";
>  			reg = <0x400a4000 0x400>;
> @@ -808,6 +815,7 @@ gmac0: ethernet@4033c000 {
>  			compatible = "nxp,s32g2-dwmac";
>  			reg = <0x4033c000 0x2000>, /* gmac IP */
>  			      <0x4007c004 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
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 4/4] dts: s32g: Add GPR syscon region
  2025-12-01 16:42   ` Frank Li
@ 2025-12-01 16:55     ` Russell King (Oracle)
  0 siblings, 0 replies; 11+ messages in thread
From: Russell King (Oracle) @ 2025-12-01 16:55 UTC (permalink / raw)
  To: Frank Li
  Cc: Dan Carpenter, Chester Lin, Matthias Brugger, Ghennadi Procopciuc,
	NXP S32 Linux Team, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, linux-arm-kernel, imx,
	devicetree, linux-kernel, linaro-s32

On Mon, Dec 01, 2025 at 11:42:00AM -0500, Frank Li wrote:
> On Mon, Dec 01, 2025 at 04:08:33PM +0300, Dan Carpenter wrote:
> > Add the GPR syscon region for the s32 chipset.
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> > ---
> >  arch/arm64/boot/dts/freescale/s32g2.dtsi | 8 ++++++++
> >  arch/arm64/boot/dts/freescale/s32g3.dtsi | 8 ++++++++
> >  2 files changed, 16 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/s32g2.dtsi b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> > index 51d00dac12de..3c9472f6c174 100644
> > --- a/arch/arm64/boot/dts/freescale/s32g2.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/s32g2.dtsi
> > @@ -325,6 +325,13 @@ usdhc0-200mhz-grp4 {
> >  			};
> >  		};
> >
> > +		gpr: syscon@4007c000 {
> > +			compatible = "nxp,s32-gpr", "syscon";
> > +			reg = <0x4007c000 0x3000>;
> > +			#address-cells = <1>;
> > +			#size-cells = <1>;
> > +		};
> > +
> 
> Please cc whole thread to imx@lists.linux.dev.
> 
> I think it is not good method by using syscon here.
> 
> Suppose using standard phy interface or mux controller interface.

I rather disagree, but I would like to see the definition of the
"ctrl_sts" register. The driver defines:

/* SoC PHY interface control register */
#define S32_PHY_INTF_SEL_MII    0x00
#define S32_PHY_INTF_SEL_SGMII  0x01
#define S32_PHY_INTF_SEL_RGMII  0x02
#define S32_PHY_INTF_SEL_RMII   0x08

This is mostly equivalent to the phy_intf_sel_i[2:0] input to the GMAC
block, who's bit combinations are defined by the PHY_INTF_SEL_xxx
constants. These seem to correspond to register bits 3:1, but with
the GMAC being configured for MII mode with an external SGMII PCS
when bit 0 is set.

If this is true, then no, there is no "phy" as such, and if we go
down the route of modelling the GMAC's phy_intf_sel_i[2:0] inputs
as a "phy" then we're going to end up with something that's a
drivers/phy PHY before a real seperate PHY for providing the
SGMII/1000BASE-X signalling. This falls into the category of needless
over-complication with no benefit.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/4] dt-bindings: mfd: syscon: Document the GPR syscon for the NXP S32 SoCs
  2025-12-01 13:08 ` [PATCH 2/4] dt-bindings: mfd: syscon: Document the GPR syscon for the NXP S32 SoCs Dan Carpenter
@ 2025-12-01 17:30   ` Krzysztof Kozlowski
  0 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2025-12-01 17:30 UTC (permalink / raw)
  To: Dan Carpenter, Lee Jones
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, devicetree,
	linux-kernel, NXP S32 Linux Team, linaro-s32

On 01/12/2025 14:08, Dan Carpenter wrote:
> The NXP S32 SoCs have a GPR region which is used by a variety of
> drivers.  Some examples of the registers in this region are:
>   * DDR_PMU_IRQ
>   * GMAC0_PHY_INTF_SEL
>   * GMAC1_PHY_INTF_SEL
>   * PFE_EMACS_INTF_SEL
>   * PFE_COH_EN
>   * PFE_PWR_CTRL
>   * PFE_EMACS_GENCTRL1
>   * PFE_GENCTRL3
> 
> Use the syscon interface to access these registers.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>  Documentation/devicetree/bindings/mfd/syscon.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
> index 55efb83b1495..6e6b92227092 100644
> --- a/Documentation/devicetree/bindings/mfd/syscon.yaml
> +++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
> @@ -102,6 +102,7 @@ select:
>            - mstar,msc313-pmsleep
>            - nuvoton,ma35d1-sys
>            - nuvoton,wpcm450-shm
> +          - nxp,s32-gpr
>            - qcom,apq8064-mmss-sfpb
>            - qcom,apq8064-sps-sic
>            - rockchip,px30-qos
> @@ -212,6 +213,7 @@ properties:
>                - mstar,msc313-pmsleep
>                - nuvoton,ma35d1-sys
>                - nuvoton,wpcm450-shm
> +              - nxp,s32-gpr
>                - qcom,apq8064-mmss-sf

You should have Soc specific compatible, not family one, so s32g2 and
probably separate s32g3.


>                - qcom,apq8064-sps-sic
>                - rockchip,px30-qos


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] dt-bindings: net: nxp,s32-dwmac: Use the GPR syscon
  2025-12-01 13:08 ` [PATCH 3/4] dt-bindings: net: nxp,s32-dwmac: Use the GPR syscon Dan Carpenter
@ 2025-12-01 17:33   ` Krzysztof Kozlowski
  2025-12-15 10:59     ` Dan Carpenter
  0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2025-12-01 17:33 UTC (permalink / raw)
  To: Dan Carpenter, Jan Petrous
  Cc: s32, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	netdev, devicetree, linux-kernel, linaro-s32

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] dt-bindings: net: nxp,s32-dwmac: Use the GPR syscon
  2025-12-01 17:33   ` Krzysztof Kozlowski
@ 2025-12-15 10:59     ` Dan Carpenter
  2025-12-15 11:03       ` Dan Carpenter
  0 siblings, 1 reply; 11+ messages in thread
From: Dan Carpenter @ 2025-12-15 10:59 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Jan Petrous, s32, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, netdev, devicetree, linux-kernel, linaro-s32

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/4] dt-bindings: net: nxp,s32-dwmac: Use the GPR syscon
  2025-12-15 10:59     ` Dan Carpenter
@ 2025-12-15 11:03       ` Dan Carpenter
  0 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2025-12-15 11:03 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Jan Petrous, s32, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, netdev, devicetree, linux-kernel, linaro-s32

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-12-15 11:04 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-01 13:08 [PATCH 0/4] s32g: Use a syscon for GPR Dan Carpenter
2025-12-01 13:08 ` [PATCH 2/4] dt-bindings: mfd: syscon: Document the GPR syscon for the NXP S32 SoCs Dan Carpenter
2025-12-01 17:30   ` Krzysztof Kozlowski
2025-12-01 13:08 ` [PATCH 3/4] dt-bindings: net: nxp,s32-dwmac: Use the GPR syscon Dan Carpenter
2025-12-01 17:33   ` Krzysztof Kozlowski
2025-12-15 10:59     ` Dan Carpenter
2025-12-15 11:03       ` Dan Carpenter
2025-12-01 13:08 ` [PATCH 4/4] dts: s32g: Add GPR syscon region Dan Carpenter
2025-12-01 16:42   ` Frank Li
2025-12-01 16:55     ` Russell King (Oracle)
2025-12-01 14:31 ` [PATCH 0/4] s32g: Use a syscon for GPR Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).