Skip to contentSkip to conversation

s32g: Use a syscon for GPR

PATCH In review Dan Carpenter 6 participants 13 messages Jan 30 – Apr 22

Subsystems (8)
lore
Share & tools
lore
#0 lore raw

v6 cover s32g: Use a syscon for GPR

The s32g devices have a GPR register region which holds a number of miscellaneous registers. 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 */

I have included the whole list of registers below.

We still have to maintain backwards compatibility to this format, of course, but it would be better to access these registers through a syscon. Putting all the registers together is more organized and shows how the hardware actually is implemented.

Secondly, in some versions of this chipset those registers can only be accessed via SCMI. It's relatively straight forward to handle this by writing a syscon driver and registering it with of_syscon_register_regmap() but it's complicated to deal with if the registers aren't grouped together.

Changes since v5:
* Fix O vs 0 typo in yaml file
* Add Jan Petrous's Reviewed-by tag

Changes since v4:
* Return an error if regmap_write() fails
* Add Rob's Reviewed-by tag to the yaml patch

Changes since v3:
* Fix the yaml file format
* Add netdev to the CC list on all emails so the CI triggers

Changes since v2:
* Improve the documentation in .../bindings/net/nxp,s32-dwmac.yaml
* "[PATCH v2 2/4] dt-bindings: mfd: syscon: Document the GPR syscon for the NXP S32 SoCs" was applied so drop it.

Changes since v1:
* Add [email protected] to the CC list.
* Fix forward porting bug. s/PHY_INTF_SEL_RGMII/S32_PHY_INTF_SEL_RGMII/
* Use the correct SoC names nxp,s32g2-gpr and nxp,s32g3-gpr instead of nxp,s32g-gpr which is the SoC family.
* Fix the phandle name by adding the vendor prefix
* Fix the documentation for the phandle
* Remove #address-cells and #size-cells from the syscon block

Here is the whole list of registers in the GPR region

Starting from 0x4007C000

0 Software-Triggered Faults (SW_NCF)
4 GMAC Control (GMAC_0_CTRL_STS)
28 CMU Status 1 (CMU_STATUS_REG1)
2C CMUs Status 2 (CMU_STATUS_REG2)
30 FCCU EOUT Override Clear (FCCU_EOUT_OVERRIDE_CLEAR_REG) 38 SRC POR Control (SRC_POR_CTRL_REG)
54 GPR21 (GPR21)
5C GPR23 (GPR23)
60 GPR24 Register (GPR24)
CC Debug Control (DEBUG_CONTROL)
F0 Timestamp Control (TIMESTAMP_CONTROL_REGISTER)
F4 FlexRay OS Tick Input Select (FLEXRAY_OS_TICK_INPUT_SELECT_REG) FC GPR63 Register (GPR63)

Starting from 0x4007CA00

0 Coherency Enable for PFE Ports (PFE_COH_EN)
4 PFE EMAC Interface Mode (PFE_EMACX_INTF_SEL)
20 PFE EMACX Power Control (PFE_PWR_CTRL)
28 Error Injection on Cortex-M7 AHB and AXI Pipe (CM7_TCM_AHB_SLICE)
2C Error Injection AHBP Gasket Cortex-M7 (ERROR_INJECTION_AHBP_GASKET_CM7) 40 LLCE Subsystem Status (LLCE_STAT)
44 LLCE Power Control (LLCE_CTRL)
48 DDR Urgent Control (DDR_URGENT_CTRL)
4C FTM Global Load Control (FLXTIM_CTRL)
50 FTM LDOK Status (FLXTIM_STAT)
54 Top CMU Status (CMU_STAT)
58 Accelerator NoC No Pending Trans Status (NOC_NOPEND_TRANS)
90 SerDes RD/WD Toggle Control (PCIE_TOGGLE)
94 SerDes Toggle Done Status (PCIE_TOGGLEDONE_STAT)
E0 Generic Control 0 (GENCTRL0)
E4 Generic Control 1 (GENCTRL1)
F0 Generic Status 0 (GENSTAT0)
FC Cortex-M7 AXI Parity Error and AHBP Gasket Error Alarm (CM7_AXI_AHBP_GASKET_ERROR_ALARM)

Starting from 4007C800

4 GPR01 Register (GPR01)
30 GPR12 Register (GPR12)
58 GPR22 Register (GPR22)
70 GPR28 Register (GPR28)
74 GPR29 Register (GPR29)

Starting from 4007CB00

4 WKUP Pad Pullup/Pulldown Select (WKUP_PUS)

Dan Carpenter (3):
net: stmmac: s32: use a syscon for S32_PHY_INTF_SEL_RGMII
dt-bindings: net: nxp,s32-dwmac: Use the GPR syscon
dts: s32g: Add GPR syscon region

 .../bindings/net/nxp,s32-dwmac.yaml           | 13 +++++++++
 arch/arm64/boot/dts/freescale/s32g2.dtsi      |  6 ++++
 arch/arm64/boot/dts/freescale/s32g3.dtsi      |  6 ++++
 .../net/ethernet/stmicro/stmmac/dwmac-s32.c   | 28 +++++++++++++++----
 4 files changed, 47 insertions(+), 6 deletions(-)
Signature · 1 line
2.51.0
#1 lore raw

PATCH v6 1/3 net: stmmac: s32: use a syscon for S32_PHY_INTF_SEL_RGMII

On the s32 chipsets the GMAC_0_CTRL_STS register is in GPR region. Originally, accessing this register was done in a sort of ad-hoc way, but we want to use the syscon interface to do it.

This is a little bit ugly because we have to maintain backwards compatibility to the old device trees so we have to support both ways to access this register.

---
v6: Add Jan's R-b tag
v5: Return an error if regmap_write() fails
v4: no change
v3: no change
v2: Fix forward porting bug. s/PHY_INTF_SEL_RGMII/S32_PHY_INTF_SEL_RGMII/

 .../net/ethernet/stmicro/stmmac/dwmac-s32.c   | 28 +++++++++++++++----
 1 file changed, 22 insertions(+), 6 deletions(-)
drivers/net/ethernet/stmicro/stmmac/dwmac-s32.c +22 -6
index 5a485ee98fa7..af594a096676 100644
@@ -11,12 +11,14 @@
 #include <linux/device.h>
 #include <linux/ethtool.h>
 #include <linux/io.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of_mdio.h>
 #include <linux/of_address.h>
 #include <linux/phy.h>
 #include <linux/phylink.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/stmmac.h>
 
 #include "stmmac_platform.h"
@@ -32,6 +34,8 @@
 struct s32_priv_data {
 	void __iomem *ioaddr;
 	void __iomem *ctrl_sts;
+	struct regmap *sts_regmap;
+	unsigned int sts_offset;
 	struct device *dev;
 	phy_interface_t *intf_mode;
 	struct clk *tx_clk;
@@ -40,11 +44,17 @@ struct s32_priv_data {
 
 static int s32_gmac_write_phy_intf_select(struct s32_priv_data *gmac)
 {
-	writel(S32_PHY_INTF_SEL_RGMII, gmac->ctrl_sts);
+	int ret = 0;
+
+	if (gmac->ctrl_sts)
+		writel(S32_PHY_INTF_SEL_RGMII, gmac->ctrl_sts);
+	else
+		ret = regmap_write(gmac->sts_regmap, gmac->sts_offset,
+				   S32_PHY_INTF_SEL_RGMII);
 
 	dev_dbg(gmac->dev, "PHY mode set to %s\n", phy_modes(*gmac->intf_mode));
 
-	return 0;
+	return ret;
 }
 
 static int s32_gmac_init(struct device *dev, void *priv)
@@ -125,10 +135,16 @@ static int s32_dwmac_probe(struct platform_device *pdev)
 				     "dt configuration failed\n");
 
 	/* PHY interface mode control reg */
-	gmac->ctrl_sts = devm_platform_get_and_ioremap_resource(pdev, 1, NULL);
-	if (IS_ERR(gmac->ctrl_sts))
-		return dev_err_probe(dev, PTR_ERR(gmac->ctrl_sts),
-				     "S32CC config region is missing\n");
+	gmac->sts_regmap = syscon_regmap_lookup_by_phandle_args(dev->of_node,
+					"nxp,phy-sel", 1, &gmac->sts_offset);
+	if (gmac->sts_regmap == ERR_PTR(-EPROBE_DEFER))
+		return PTR_ERR(gmac->sts_regmap);
+	if (IS_ERR(gmac->sts_regmap)) {
+		gmac->ctrl_sts = devm_platform_get_and_ioremap_resource(pdev, 1, NULL);
+		if (IS_ERR(gmac->ctrl_sts))
+			return dev_err_probe(dev, PTR_ERR(gmac->ctrl_sts),
+					     "S32CC config region is missing\n");
+	}
 
 	/* tx clock */
 	gmac->tx_clk = devm_clk_get(&pdev->dev, "tx");
Signature · 1 line
2.51.0
#2 lore raw

PATCH v6 2/3 dt-bindings: net: nxp,s32-dwmac: Use the GPR syscon

In reply to Dan Carpenter

The S32 chipsets have 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 it's cleaner to use a syscon interface to access these registers.

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.

---
v6: Fix O vs 0 typo in GMAC_0
v5: Add Rob's R-b tag
v4: Fix the formatting issue Rob pointed out
v3: Better documentation about what GMAC_0_CTRL_STS register does.
v2: Add the vendor prefix to the phandle
Fix the documentation

 .../devicetree/bindings/net/nxp,s32-dwmac.yaml      | 13 +++++++++++++
 1 file changed, 13 insertions(+)
Documentation/devicetree/bindings/net/nxp,s32-dwmac.yaml +13 -0
index 2b8b74c5feec..1b2934f3c87c 100644
@@ -32,6 +32,18 @@ properties:
       - description: Main GMAC registers
       - description: GMAC PHY mode control register
 
+  nxp,phy-sel:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      - items:
+          - description: phandle to the GPR syscon node
+          - description: offset of PHY selection register
+    description:
+      This phandle points to the GMAC_0_CTRL_STS register which controls the
+      GMAC_0 configuration options.  The register lets you select the PHY
+      interface and the PHY mode.  It also controls if the FTM_0 or FTM_1
+      FlexTimer Modules connect to GMAC_0.
+
   interrupts:
     maxItems: 1
 
@@ -74,6 +86,7 @@ examples:
         compatible = "nxp,s32g2-dwmac";
         reg = <0x0 0x4033c000 0x0 0x2000>, /* gmac IP */
               <0x0 0x4007c004 0x0 0x4>;    /* GMAC_0_CTRL_STS */
+        nxp,phy-sel = <&gpr 0x4>;
         interrupt-parent = <&gic>;
         interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
         interrupt-names = "macirq";
Signature · 1 line
2.51.0
#3 lore raw

PATCH v6 3/3 dts: s32g: Add GPR syscon region

In reply to Dan Carpenter

Add the GPR syscon region for the s32 chipset.

---
v6: no change
v5: no change
v4: no change
v3: no change
v2: Remove #address-cells and #size-cells

 arch/arm64/boot/dts/freescale/s32g2.dtsi | 6 ++++++
 arch/arm64/boot/dts/freescale/s32g3.dtsi | 6 ++++++
 2 files changed, 12 insertions(+)
arch/arm64/boot/dts/freescale/s32g2.dtsi +6 -0
index 51d00dac12de..b954952d962b 100644
@@ -325,6 +325,11 @@ usdhc0-200mhz-grp4 {
 			};
 		};
 
+		gpr: syscon@4007c000 {
+			compatible = "nxp,s32g2-gpr", "syscon";
+			reg = <0x4007c000 0x3000>;
+		};
+
 		ocotp: nvmem@400a4000 {
 			compatible = "nxp,s32g2-ocotp";
 			reg = <0x400a4000 0x400>;
@@ -731,6 +736,7 @@ gmac0: ethernet@4033c000 {
 			compatible = "nxp,s32g2-dwmac";
 			reg = <0x4033c000 0x2000>, /* gmac IP */
 			      <0x4007c004 0x4>;    /* GMAC_0_CTRL_STS */
+			nxp,phy-sel = <&gpr 0x4>;
 			interrupt-parent = <&gic>;
 			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "macirq";
arch/arm64/boot/dts/freescale/s32g3.dtsi +6 -0
index e314f3c7d61d..be03db737384 100644
@@ -383,6 +383,11 @@ usdhc0-200mhz-grp4 {
 			};
 		};
 
+		gpr: syscon@4007c000 {
+			compatible = "nxp,s32g3-gpr", "syscon";
+			reg = <0x4007c000 0x3000>;
+		};
+
 		ocotp: nvmem@400a4000 {
 			compatible = "nxp,s32g3-ocotp", "nxp,s32g2-ocotp";
 			reg = <0x400a4000 0x400>;
@@ -808,6 +813,7 @@ gmac0: ethernet@4033c000 {
 			compatible = "nxp,s32g2-dwmac";
 			reg = <0x4033c000 0x2000>, /* gmac IP */
 			      <0x4007c004 0x4>;    /* GMAC_0_CTRL_STS */
+			nxp,phy-sel = <&gpr 0x4>;
 			interrupt-parent = <&gic>;
 			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "macirq";
Signature · 1 line
2.51.0
#4 lore raw

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

Hi Dan,

Quoted 24 lines · Dan Carpenter

Add the GPR syscon region for the s32 chipset.

---

[snip]

arch/arm64/boot/dts/freescale/s32g3.dtsi +5 -0
index e314f3c7d61d..be03db737384 100644
@@ -383,6 +383,11 @@ usdhc0-200mhz-grp4 {
 			};
 		};
 
+		gpr: syscon@4007c000 {
+			compatible = "nxp,s32g3-gpr", "syscon";
+			reg = <0x4007c000 0x3000>;
+		};
+
 		ocotp: nvmem@400a4000 {
 			compatible = "nxp,s32g3-ocotp", "nxp,s32g2-ocotp";
 			reg = <0x400a4000 0x400>;

arch/arm64/boot/dts/freescale/s32g3.dtsi gmac0: ethernet@4033c000 { · line 819

Quoted 7 lines
arch/arm64/boot/dts/freescale/s32g3.dtsi +1 -0
@@ -808,6 +813,7 @@ gmac0: ethernet@4033c000 {
 			compatible = "nxp,s32g2-dwmac";
 			reg = <0x4033c000 0x2000>, /* gmac IP */
 			      <0x4007c004 0x4>;    /* GMAC_0_CTRL_STS */
+			nxp,phy-sel = <&gpr 0x4>;
 			interrupt-parent = <&gic>;
 			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "macirq";

I gave this a test on an S32G-VNP-RDB3 and didn't see any issues on the dwmac-s32 side, but this appears to trigger a panic when reading the new debugfs regmap/*/registers file for the syscon node:

    # grep 4007c000 /proc/vmallocinfo
    0xffff800083da8000-0xffff800083dac000   16384 ioremap_prot+0x74/0xe0 phys=0x000000004007c000 ioremap
    # cat /sys/kernel/debug/regmap/dummy-syscon@0x000000004007c000/registers
    Internal error: synchronous external abort: 0000000096000210 [#1]  SMP
    [...]
    CPU: 0 UID: 0 PID: 4344 Comm: cat Tainted: G   M        E  X   ------  ---  6.12.0+ #226 PREEMPT_RT
    Tainted: [M]=MACHINE_CHECK, [E]=UNSIGNED_MODULE, [X]=AUX
    [...]
    pc : regmap_mmio_read32le+0x44/0xa0
    lr : regmap_mmio_read32le+0x44/0xa0
    [...]
    x23: ffff00080c080000 x22: ffff000802ac4c00 x21: ffff800087b13c9c
    x20: ffff800080a46494 x19: ffff800083da810c x18: 0000000000000004
    [...]
    x5 : ffff800080a46448 x4 : ffff800083da8000 x3 : ffff800080a46494
    x2 : ffff800080a47230 x1 : ffff800083da810c x0 : 0000000000000020
    Call trace:
     regmap_mmio_read32le+0x44/0xa0 (P)
     regmap_mmio_read+0x4c/0x80
     [...]
    Code: 52800400 8b214093 aa1303e1 97f4caf0 (b9400275)
    ---[ end trace 0000000000000000 ]---
    Kernel panic - not syncing: synchronous external abort: Fatal exception

Running this through decodecode gives:

    All code
    ========
       0:   52800400        mov     w0, #0x20                       // #32
       4:   8b214093        add     x19, x4, w1, uxtw
       8:   aa1303e1        mov     x1, x19
       c:   97f4caf0        bl      0xffffffffffd32bcc
      10:*  b9400275        ldr     w21, [x19]              <-- trapping instruction

    Code starting with the faulting instruction
    ===========================================
       0:   b9400275        ldr     w21, [x19]

x19's offset from the base address in /proc/vmallocinfo is 0x10c, which points to a bad read at physical address 0x4007c10c; I also confirmed that the preceding memory reads back without issues:

    # head -c 990 /sys/kernel/debug/regmap/dummy-syscon@0x000000004007c000/registers | tail -1
    0104: 00000000
    # head -c 1005 /sys/kernel/debug/regmap/dummy-syscon@0x000000004007c000/registers | tail -1
    0108: 00000000
    # head -c 1020 /sys/kernel/debug/regmap/dummy-syscon@0x000000004007c000/registers | tail -1
    <panic>

Best,
Jared

#5 lore raw

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

Fixing Dan's address based on mailmap update, sorry for the noise.

Quoted 91 lines · Jared Kangas

Hi Dan,

Quoted 24 lines · Dan Carpenter

Add the GPR syscon region for the s32 chipset.

---

[snip]

arch/arm64/boot/dts/freescale/s32g3.dtsi +5 -0
index e314f3c7d61d..be03db737384 100644
@@ -383,6 +383,11 @@ usdhc0-200mhz-grp4 {
 			};
 		};
 
+		gpr: syscon@4007c000 {
+			compatible = "nxp,s32g3-gpr", "syscon";
+			reg = <0x4007c000 0x3000>;
+		};
+
 		ocotp: nvmem@400a4000 {
 			compatible = "nxp,s32g3-ocotp", "nxp,s32g2-ocotp";
 			reg = <0x400a4000 0x400>;

arch/arm64/boot/dts/freescale/s32g3.dtsi gmac0: ethernet@4033c000 { · line 819

Quoted 7 lines
arch/arm64/boot/dts/freescale/s32g3.dtsi +1 -0
@@ -808,6 +813,7 @@ gmac0: ethernet@4033c000 {
 			compatible = "nxp,s32g2-dwmac";
 			reg = <0x4033c000 0x2000>, /* gmac IP */
 			      <0x4007c004 0x4>;    /* GMAC_0_CTRL_STS */
+			nxp,phy-sel = <&gpr 0x4>;
 			interrupt-parent = <&gic>;
 			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "macirq";

I gave this a test on an S32G-VNP-RDB3 and didn't see any issues on the dwmac-s32 side, but this appears to trigger a panic when reading the new debugfs regmap/*/registers file for the syscon node:

    # grep 4007c000 /proc/vmallocinfo
    0xffff800083da8000-0xffff800083dac000   16384 ioremap_prot+0x74/0xe0 phys=0x000000004007c000 ioremap
    # cat /sys/kernel/debug/regmap/dummy-syscon@0x000000004007c000/registers
    Internal error: synchronous external abort: 0000000096000210 [#1]  SMP
    [...]
    CPU: 0 UID: 0 PID: 4344 Comm: cat Tainted: G   M        E  X   ------  ---  6.12.0+ #226 PREEMPT_RT
    Tainted: [M]=MACHINE_CHECK, [E]=UNSIGNED_MODULE, [X]=AUX
    [...]
    pc : regmap_mmio_read32le+0x44/0xa0
    lr : regmap_mmio_read32le+0x44/0xa0
    [...]
    x23: ffff00080c080000 x22: ffff000802ac4c00 x21: ffff800087b13c9c
    x20: ffff800080a46494 x19: ffff800083da810c x18: 0000000000000004
    [...]
    x5 : ffff800080a46448 x4 : ffff800083da8000 x3 : ffff800080a46494
    x2 : ffff800080a47230 x1 : ffff800083da810c x0 : 0000000000000020
    Call trace:
     regmap_mmio_read32le+0x44/0xa0 (P)
     regmap_mmio_read+0x4c/0x80
     [...]
    Code: 52800400 8b214093 aa1303e1 97f4caf0 (b9400275)
    ---[ end trace 0000000000000000 ]---
    Kernel panic - not syncing: synchronous external abort: Fatal exception

Running this through decodecode gives:

    All code
    ========
       0:   52800400        mov     w0, #0x20                       // #32
       4:   8b214093        add     x19, x4, w1, uxtw
       8:   aa1303e1        mov     x1, x19
       c:   97f4caf0        bl      0xffffffffffd32bcc
      10:*  b9400275        ldr     w21, [x19]              <-- trapping instruction

    Code starting with the faulting instruction
    ===========================================
       0:   b9400275        ldr     w21, [x19]

x19's offset from the base address in /proc/vmallocinfo is 0x10c, which points to a bad read at physical address 0x4007c10c; I also confirmed that the preceding memory reads back without issues:

    # head -c 990 /sys/kernel/debug/regmap/dummy-syscon@0x000000004007c000/registers | tail -1
    0104: 00000000
    # head -c 1005 /sys/kernel/debug/regmap/dummy-syscon@0x000000004007c000/registers | tail -1
    0108: 00000000
    # head -c 1020 /sys/kernel/debug/regmap/dummy-syscon@0x000000004007c000/registers | tail -1
    <panic>

Best,
Jared

#6 lore raw

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

Quoted 83 lines · Jared Kangas

Fixing Dan's address based on mailmap update, sorry for the noise.

Quoted 80 lines · Jared Kangas

Hi Dan,

Quoted 24 lines · Dan Carpenter

Add the GPR syscon region for the s32 chipset.

---

[snip]

arch/arm64/boot/dts/freescale/s32g3.dtsi +5 -0
index e314f3c7d61d..be03db737384 100644
@@ -383,6 +383,11 @@ usdhc0-200mhz-grp4 {
 			};
 		};
 
+		gpr: syscon@4007c000 {
+			compatible = "nxp,s32g3-gpr", "syscon";
+			reg = <0x4007c000 0x3000>;
+		};
+
 		ocotp: nvmem@400a4000 {
 			compatible = "nxp,s32g3-ocotp", "nxp,s32g2-ocotp";
 			reg = <0x400a4000 0x400>;

arch/arm64/boot/dts/freescale/s32g3.dtsi gmac0: ethernet@4033c000 { · line 819

Quoted 7 lines
arch/arm64/boot/dts/freescale/s32g3.dtsi +1 -0
@@ -808,6 +813,7 @@ gmac0: ethernet@4033c000 {
 			compatible = "nxp,s32g2-dwmac";
 			reg = <0x4033c000 0x2000>, /* gmac IP */
 			      <0x4007c004 0x4>;    /* GMAC_0_CTRL_STS */
+			nxp,phy-sel = <&gpr 0x4>;
 			interrupt-parent = <&gic>;
 			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "macirq";

I gave this a test on an S32G-VNP-RDB3 and didn't see any issues on the dwmac-s32 side, but this appears to trigger a panic when reading the new debugfs regmap/*/registers file for the syscon node:

    # grep 4007c000 /proc/vmallocinfo
    0xffff800083da8000-0xffff800083dac000   16384 ioremap_prot+0x74/0xe0 phys=0x000000004007c000 ioremap
    # cat /sys/kernel/debug/regmap/dummy-syscon@0x000000004007c000/registers
    Internal error: synchronous external abort: 0000000096000210 [#1]  SMP
    [...]
    CPU: 0 UID: 0 PID: 4344 Comm: cat Tainted: G   M        E  X   ------  ---  6.12.0+ #226 PREEMPT_RT
    Tainted: [M]=MACHINE_CHECK, [E]=UNSIGNED_MODULE, [X]=AUX
    [...]
    pc : regmap_mmio_read32le+0x44/0xa0
    lr : regmap_mmio_read32le+0x44/0xa0
    [...]
    x23: ffff00080c080000 x22: ffff000802ac4c00 x21: ffff800087b13c9c
    x20: ffff800080a46494 x19: ffff800083da810c x18: 0000000000000004
    [...]
    x5 : ffff800080a46448 x4 : ffff800083da8000 x3 : ffff800080a46494
    x2 : ffff800080a47230 x1 : ffff800083da810c x0 : 0000000000000020
    Call trace:
     regmap_mmio_read32le+0x44/0xa0 (P)
     regmap_mmio_read+0x4c/0x80
     [...]
    Code: 52800400 8b214093 aa1303e1 97f4caf0 (b9400275)
    ---[ end trace 0000000000000000 ]---
    Kernel panic - not syncing: synchronous external abort: Fatal exception

Running this through decodecode gives:

    All code
    ========
       0:   52800400        mov     w0, #0x20                       // #32
       4:   8b214093        add     x19, x4, w1, uxtw
       8:   aa1303e1        mov     x1, x19
       c:   97f4caf0        bl      0xffffffffffd32bcc
      10:*  b9400275        ldr     w21, [x19]              <-- trapping instruction

    Code starting with the faulting instruction
    ===========================================
       0:   b9400275        ldr     w21, [x19]

x19's offset from the base address in /proc/vmallocinfo is 0x10c, which points to a bad read at physical address 0x4007c10c; I also confirmed that the preceding memory reads back without issues:

Oh, ugh... I didn't realize that this wasn't merged. I don't have a way to test this any more. The simplest fix would be to do change the 0x3000 to 0x100. The GPR63 register is at 0xFC.

		reg = <0x4007c000 0x100>;

That's probably the best fix as well. The later register areas would be their own syscons.

regards,
dan carpenter

Tested-by
#7 lore raw

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

Quoted 14 lines · Dan Carpenter
Quoted 12 lines · Jared Kangas

Fixing Dan's address based on mailmap update, sorry for the noise.

Quoted 9 lines · Jared Kangas

Hi Dan,

[snip]

I gave this a test on an S32G-VNP-RDB3 and didn't see any issues on the dwmac-s32 side, but this appears to trigger a panic when reading the new debugfs regmap/*/registers file for the syscon node:

[snip]

Quoted 8 lines

Oh, ugh... I didn't realize that this wasn't merged. I don't have a way to test this any more. The simplest fix would be to do change the 0x3000 to 0x100. The GPR63 register is at 0xFC.

		reg = <0x4007c000 0x100>;

That's probably the best fix as well. The later register areas would be their own syscons.

Tried that out and it looks good to me. With the write routed through syscon:

    # xxd -g4 /proc/device-tree/soc@0/syscon@4007c000/reg
    00000000: 4007c000 00000100                    @.......
    # cat /sys/kernel/debug/regmap/dummy-syscon@0x000000004007c000/registers
    00: 00000000
    04: 00000002
    08: 000000e7
    0c: 00000001
    10: ffffffff
    14: 1fffffff
    18: 00007fff
    1c: 00000000
    20: 00000000
    ...
    f4: 00000000
    f8: 00000000
    fc: 00000000

No more crashes and 04's value lines up with the S32_PHY_INTF_SEL_RGMII (0x2) write, so if you're able to post a revision, feel free to add my T-b:

maintainer
#8 lore raw
In reply to Dan Carpenter
Quoted 94 lines · Dan Carpenter

The s32g devices have a GPR register region which holds a number of miscellaneous registers. 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 */

I have included the whole list of registers below.

We still have to maintain backwards compatibility to this format, of course, but it would be better to access these registers through a syscon. Putting all the registers together is more organized and shows how the hardware actually is implemented.

Secondly, in some versions of this chipset those registers can only be accessed via SCMI. It's relatively straight forward to handle this by writing a syscon driver and registering it with of_syscon_register_regmap() but it's complicated to deal with if the registers aren't grouped together.

Changes since v5:
* Fix O vs 0 typo in yaml file
* Add Jan Petrous's Reviewed-by tag

Changes since v4:
* Return an error if regmap_write() fails
* Add Rob's Reviewed-by tag to the yaml patch

Changes since v3:
* Fix the yaml file format
* Add netdev to the CC list on all emails so the CI triggers

Changes since v2:
* Improve the documentation in .../bindings/net/nxp,s32-dwmac.yaml
* "[PATCH v2 2/4] dt-bindings: mfd: syscon: Document the GPR syscon for the NXP S32 SoCs" was applied so drop it.

Changes since v1:
* Add [email protected] to the CC list.
* Fix forward porting bug. s/PHY_INTF_SEL_RGMII/S32_PHY_INTF_SEL_RGMII/
* Use the correct SoC names nxp,s32g2-gpr and nxp,s32g3-gpr instead of nxp,s32g-gpr which is the SoC family.
* Fix the phandle name by adding the vendor prefix
* Fix the documentation for the phandle
* Remove #address-cells and #size-cells from the syscon block

Here is the whole list of registers in the GPR region

Starting from 0x4007C000

0 Software-Triggered Faults (SW_NCF)
4 GMAC Control (GMAC_0_CTRL_STS)
28 CMU Status 1 (CMU_STATUS_REG1)
2C CMUs Status 2 (CMU_STATUS_REG2)
30 FCCU EOUT Override Clear (FCCU_EOUT_OVERRIDE_CLEAR_REG) 38 SRC POR Control (SRC_POR_CTRL_REG)
54 GPR21 (GPR21)
5C GPR23 (GPR23)
60 GPR24 Register (GPR24)
CC Debug Control (DEBUG_CONTROL)
F0 Timestamp Control (TIMESTAMP_CONTROL_REGISTER)
F4 FlexRay OS Tick Input Select (FLEXRAY_OS_TICK_INPUT_SELECT_REG) FC GPR63 Register (GPR63)

Starting from 0x4007CA00

0 Coherency Enable for PFE Ports (PFE_COH_EN)
4 PFE EMAC Interface Mode (PFE_EMACX_INTF_SEL)
20 PFE EMACX Power Control (PFE_PWR_CTRL)
28 Error Injection on Cortex-M7 AHB and AXI Pipe (CM7_TCM_AHB_SLICE)
2C Error Injection AHBP Gasket Cortex-M7 (ERROR_INJECTION_AHBP_GASKET_CM7) 40 LLCE Subsystem Status (LLCE_STAT)
44 LLCE Power Control (LLCE_CTRL)
48 DDR Urgent Control (DDR_URGENT_CTRL)
4C FTM Global Load Control (FLXTIM_CTRL)
50 FTM LDOK Status (FLXTIM_STAT)
54 Top CMU Status (CMU_STAT)
58 Accelerator NoC No Pending Trans Status (NOC_NOPEND_TRANS)
90 SerDes RD/WD Toggle Control (PCIE_TOGGLE)
94 SerDes Toggle Done Status (PCIE_TOGGLEDONE_STAT)
E0 Generic Control 0 (GENCTRL0)
E4 Generic Control 1 (GENCTRL1)
F0 Generic Status 0 (GENSTAT0)
FC Cortex-M7 AXI Parity Error and AHBP Gasket Error Alarm (CM7_AXI_AHBP_GASKET_ERROR_ALARM)

Starting from 4007C800

4 GPR01 Register (GPR01)
30 GPR12 Register (GPR12)
58 GPR22 Register (GPR22)
70 GPR28 Register (GPR28)
74 GPR29 Register (GPR29)

Quoted 8 lines

Starting from 4007CB00

4 WKUP Pad Pullup/Pulldown Select (WKUP_PUS)

Dan Carpenter (3):
net: stmmac: s32: use a syscon for S32_PHY_INTF_SEL_RGMII
dt-bindings: net: nxp,s32-dwmac: Use the GPR syscon
dts: s32g: Add GPR syscon region

It looks like patch 3/3 depends on 1/3 but it should land in a different tree, as patches 1 && 2 looks suitable for 'net-next' and 3/3 should probably go via the arm/freescale tree.

We either need explicit ack from freescale maintainers or this should be split across subtrees, right?

Thanks,

Paolo

#9 lore raw
Quoted 10 lines · Paolo Abeni
Quoted 5 lines · Dan Carpenter

Dan Carpenter (3):
net: stmmac: s32: use a syscon for S32_PHY_INTF_SEL_RGMII
dt-bindings: net: nxp,s32-dwmac: Use the GPR syscon
dts: s32g: Add GPR syscon region

It looks like patch 3/3 depends on 1/3 but it should land in a different tree, as patches 1 && 2 looks suitable for 'net-next' and 3/3 should probably go via the arm/freescale tree.

Yep. The driver needs to be updated first before the device tree. (The new driver is compatible with old device trees, of course).

Quoted 4 lines

We either need explicit ack from freescale maintainers or this should be split across subtrees, right?

regards,
dan carpenter

maintainer
#10 lore raw
In reply to Paolo Abeni
Quoted 6 lines · On Tue, 3 Feb 2026 12:18:54 +0100 Paolo Abeni wrote:

It looks like patch 3/3 depends on 1/3 but it should land in a different tree, as patches 1 && 2 looks suitable for 'net-next' and 3/3 should probably go via the arm/freescale tree.

We either need explicit ack from freescale maintainers or this should be split across subtrees, right?

FWIW normally we don't touch the dts patch at all, unless the maintainer chimes in and tells us to take it. No waiting for acks. Off the top of my head us taking dts patches only happens if the person posting the patch _is_ the platform maintainer..

For this series we're waiting for the bindings to be reviewed.

#11 lore raw
Quoted 8 lines · Jakub Kicinski
Quoted 6 lines · On Tue, 3 Feb 2026 12:18:54 +0100 Paolo Abeni wrote:

It looks like patch 3/3 depends on 1/3 but it should land in a different tree, as patches 1 && 2 looks suitable for 'net-next' and 3/3 should probably go via the arm/freescale tree.

We either need explicit ack from freescale maintainers or this should be split across subtrees, right?

Quoted 6 lines

FWIW normally we don't touch the dts patch at all, unless the maintainer chimes in and tells us to take it. No waiting for acks. Off the top of my head us taking dts patches only happens if the person posting the patch _is_ the platform maintainer..

For this series we're waiting for the bindings to be reviewed.

Ugh. Crap. Rob already reviewed them. I included his Reviewed-by tag in v5 but I accidentally removed it that from v6. I'm so sorry.

https://lore.kernel.org/all/939fdaf94038613486bdbba510a92a1e57e18c71.1769592679.git.dan.carpenter@linaro.org/

regards,
dan carpenter

#12 lore raw
In reply to Dan Carpenter
This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This series was applied to netdev/net-next.git (main) by Jakub Kicinski <[email protected]>:

Quoted 9 lines · On Fri, 30 Jan 2026 16:19:35 +0300 you wrote:

The s32g devices have a GPR register region which holds a number of miscellaneous registers. 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 */

[...]

Here is the summary with links:
- [v6,1/3] net: stmmac: s32: use a syscon for S32_PHY_INTF_SEL_RGMII

    https://git.kernel.org/netdev/net-next/c/b0eeeb1b70c0
  - [v6,2/3] dt-bindings: net: nxp,s32-dwmac: Use the GPR syscon
    https://git.kernel.org/netdev/net-next/c/0d9588039543

- [v6,3/3] dts: s32g: Add GPR syscon region
(no matching commit)

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html