[v2] net: fec: limit register access on i.MX6UL

Message ID 20221024080552.21004-1-jbe@pengutronix.de
State New
Headers
Series [v2] net: fec: limit register access on i.MX6UL |

Commit Message

Juergen Borleis Oct. 24, 2022, 8:05 a.m. UTC
  Using 'ethtool -d […]' on an i.MX6UL leads to a kernel crash:

   Unhandled fault: external abort on non-linefetch (0x1008) at […]

due to this SoC has less registers in its FEC implementation compared to other
i.MX6 variants. Thus, a run-time decision is required to avoid access to
non-existing registers.

Signed-off-by: Juergen Borleis <jbe@pengutronix.de>
---

Notes:
    v2: using of_machine_is_compatible() to detect the i.MX6UL SoC

 drivers/net/ethernet/freescale/fec_main.c | 46 ++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 2 deletions(-)
  

Comments

Andrew Lunn Oct. 25, 2022, 12:49 p.m. UTC | #1
On Mon, Oct 24, 2022 at 10:05:52AM +0200, Juergen Borleis wrote:
> Using 'ethtool -d […]' on an i.MX6UL leads to a kernel crash:
> 
>    Unhandled fault: external abort on non-linefetch (0x1008) at […]
> 
> due to this SoC has less registers in its FEC implementation compared to other
> i.MX6 variants. Thus, a run-time decision is required to avoid access to
> non-existing registers.
> 
> Signed-off-by: Juergen Borleis <jbe@pengutronix.de>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew
  
Jakub Kicinski Oct. 25, 2022, 11:08 p.m. UTC | #2
On Tue, 25 Oct 2022 14:49:45 +0200 Andrew Lunn wrote:
> On Mon, Oct 24, 2022 at 10:05:52AM +0200, Juergen Borleis wrote:
> > Using 'ethtool -d […]' on an i.MX6UL leads to a kernel crash:
> > 
> >    Unhandled fault: external abort on non-linefetch (0x1008) at […]
> > 
> > due to this SoC has less registers in its FEC implementation compared to other
> > i.MX6 variants. Thus, a run-time decision is required to avoid access to
> > non-existing registers.
> > 
> > Signed-off-by: Juergen Borleis <jbe@pengutronix.de>  
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

What would be the right Fixes tag for this? This one perhaps?

Fixes: a51d3ab50702 ("net: fec: use a more proper compatible string for i.MX6UL type device")
  
patchwork-bot+netdevbpf@kernel.org Oct. 27, 2022, 3:30 a.m. UTC | #3
Hello:

This patch was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 24 Oct 2022 10:05:52 +0200 you wrote:
> Using 'ethtool -d […]' on an i.MX6UL leads to a kernel crash:
> 
>    Unhandled fault: external abort on non-linefetch (0x1008) at […]
> 
> due to this SoC has less registers in its FEC implementation compared to other
> i.MX6 variants. Thus, a run-time decision is required to avoid access to
> non-existing registers.
> 
> [...]

Here is the summary with links:
  - [v2] net: fec: limit register access on i.MX6UL
    https://git.kernel.org/netdev/net/c/0a8b43b12dd7

You are awesome, thank you!
  

Patch

diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 98d5cd31..28ef4d3 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2432,6 +2432,31 @@  static u32 fec_enet_register_offset[] = {
 	IEEE_R_DROP, IEEE_R_FRAME_OK, IEEE_R_CRC, IEEE_R_ALIGN, IEEE_R_MACERR,
 	IEEE_R_FDXFC, IEEE_R_OCTETS_OK
 };
+/* for i.MX6ul */
+static u32 fec_enet_register_offset_6ul[] = {
+	FEC_IEVENT, FEC_IMASK, FEC_R_DES_ACTIVE_0, FEC_X_DES_ACTIVE_0,
+	FEC_ECNTRL, FEC_MII_DATA, FEC_MII_SPEED, FEC_MIB_CTRLSTAT, FEC_R_CNTRL,
+	FEC_X_CNTRL, FEC_ADDR_LOW, FEC_ADDR_HIGH, FEC_OPD, FEC_TXIC0, FEC_RXIC0,
+	FEC_HASH_TABLE_HIGH, FEC_HASH_TABLE_LOW, FEC_GRP_HASH_TABLE_HIGH,
+	FEC_GRP_HASH_TABLE_LOW, FEC_X_WMRK, FEC_R_DES_START_0,
+	FEC_X_DES_START_0, FEC_R_BUFF_SIZE_0, FEC_R_FIFO_RSFL, FEC_R_FIFO_RSEM,
+	FEC_R_FIFO_RAEM, FEC_R_FIFO_RAFL, FEC_RACC,
+	RMON_T_DROP, RMON_T_PACKETS, RMON_T_BC_PKT, RMON_T_MC_PKT,
+	RMON_T_CRC_ALIGN, RMON_T_UNDERSIZE, RMON_T_OVERSIZE, RMON_T_FRAG,
+	RMON_T_JAB, RMON_T_COL, RMON_T_P64, RMON_T_P65TO127, RMON_T_P128TO255,
+	RMON_T_P256TO511, RMON_T_P512TO1023, RMON_T_P1024TO2047,
+	RMON_T_P_GTE2048, RMON_T_OCTETS,
+	IEEE_T_DROP, IEEE_T_FRAME_OK, IEEE_T_1COL, IEEE_T_MCOL, IEEE_T_DEF,
+	IEEE_T_LCOL, IEEE_T_EXCOL, IEEE_T_MACERR, IEEE_T_CSERR, IEEE_T_SQE,
+	IEEE_T_FDXFC, IEEE_T_OCTETS_OK,
+	RMON_R_PACKETS, RMON_R_BC_PKT, RMON_R_MC_PKT, RMON_R_CRC_ALIGN,
+	RMON_R_UNDERSIZE, RMON_R_OVERSIZE, RMON_R_FRAG, RMON_R_JAB,
+	RMON_R_RESVD_O, RMON_R_P64, RMON_R_P65TO127, RMON_R_P128TO255,
+	RMON_R_P256TO511, RMON_R_P512TO1023, RMON_R_P1024TO2047,
+	RMON_R_P_GTE2048, RMON_R_OCTETS,
+	IEEE_R_DROP, IEEE_R_FRAME_OK, IEEE_R_CRC, IEEE_R_ALIGN, IEEE_R_MACERR,
+	IEEE_R_FDXFC, IEEE_R_OCTETS_OK
+};
 #else
 static __u32 fec_enet_register_version = 1;
 static u32 fec_enet_register_offset[] = {
@@ -2456,7 +2481,24 @@  static void fec_enet_get_regs(struct net_device *ndev,
 	u32 *buf = (u32 *)regbuf;
 	u32 i, off;
 	int ret;
+#if defined(CONFIG_M523x) || defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
+	defined(CONFIG_M520x) || defined(CONFIG_M532x) || defined(CONFIG_ARM) || \
+	defined(CONFIG_ARM64) || defined(CONFIG_COMPILE_TEST)
+	u32 *reg_list;
+	u32 reg_cnt;
 
+	if (!of_machine_is_compatible("fsl,imx6ul")) {
+		reg_list = fec_enet_register_offset;
+		reg_cnt = ARRAY_SIZE(fec_enet_register_offset);
+	} else {
+		reg_list = fec_enet_register_offset_6ul;
+		reg_cnt = ARRAY_SIZE(fec_enet_register_offset_6ul);
+	}
+#else
+	/* coldfire */
+	static u32 *reg_list = fec_enet_register_offset;
+	static const u32 reg_cnt = ARRAY_SIZE(fec_enet_register_offset);
+#endif
 	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0)
 		return;
@@ -2465,8 +2507,8 @@  static void fec_enet_get_regs(struct net_device *ndev,
 
 	memset(buf, 0, regs->len);
 
-	for (i = 0; i < ARRAY_SIZE(fec_enet_register_offset); i++) {
-		off = fec_enet_register_offset[i];
+	for (i = 0; i < reg_cnt; i++) {
+		off = reg_list[i];
 
 		if ((off == FEC_R_BOUND || off == FEC_R_FSTART) &&
 		    !(fep->quirks & FEC_QUIRK_HAS_FRREG))