[PATCH 0/4] s32g: Use a syscon for GPR

[PATCH 0/4] s32g: Use a syscon for GPR

[Dan Carpenter]

*** 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

[PATCH 4/4] dts: s32g: Add GPR syscon region

[Dan Carpenter]

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

Re: [PATCH 0/4] s32g: Use a syscon for GPR

[Dan Carpenter]

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

Re: [PATCH 4/4] dts: s32g: Add GPR syscon region

[Frank Li]

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
>

Re: [PATCH 4/4] dts: s32g: Add GPR syscon region

[Russell King (Oracle)]

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!