* [PATCHv3 0/3] m68k: coldfire: fix non-standard readX()/writeX() functions
@ 2026-09-07 13:37 Greg Ungerer
2026-09-07 13:37 ` [PATCHv3 1/3] net: fec: do not use readl()/writel() for ColdFire Greg Ungerer
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Greg Ungerer @ 2026-09-07 13:37 UTC (permalink / raw)
To: linux-m68k
Cc: linux-kernel, arnd, wei.fang, frank.li, shenwei.wang, imx, netdev,
nico, linux-can, linux-spi, olteanv, Greg Ungerer
This collection of patches is aimed at fixing the non-standard ColdFire
set of readX()/writeX() IO access functions. Instead switching to using the
asm-generic definitions in include/asm-generic/io.h. The difficulty comes
in trying not to break any drivers with this change.
The implementation of the readX()/writeX() family of IO access functions
is non-standard on ColdFire platforms. They either return big-endian (that
is native endian) data, or on platforms with PCI bus support check the
supplied address and return either big or little endian data based on that
check. This is non-standard, they are expected to always return
little-endian byte ordered data. Unfortunately this behavior also means
that ioreadX()/iowroteX() and their big-endian counter parts
ioreadXbe()/iowriteXbe() are currently broken because they are implemented
using the readX()/writeX() functions.
Patches 1 and 2 in this series are specific net driver changes that can be
applied independently of the final ColdFire readX()/writeX() change.
Patch 3 is the actual switch to ColdFire building using asm-generic
readX()/writeX(), but also contains three driver fixes that are not easily
handled independently.
Note that I don't have access to all supported hardware needed to fully
test all these changes. I have tested what I have, a bunch of the standard
Freescale ColdFire eval boards, and inspected generated code for differences.
Note that the v1 and v2 versions of this series carried a change to the
sdhci-esdhc-mcf.c driver that is now in mainline - so dropped from this
current v3 series.
The v2 series moved from an RFC to a patch series. There was only minor
changes overall to address comments. Changes include formatting, separating
out the mcf5441x regmap in spi-fsl-dspi.c (patch 3) and reordering the
quirks in flexcan-core.c (patch 3).
This v3 series has no new code changes. It is sent with an expanded CC
list in the hope of getting the net driver changes (patches 1 and 2)
picked up so the final readl/writel fix (patch 3) that depends on them
can be applied.
Signed-off-by: Greg Ungerer <gerg@linux-m68k.org>
---
arch/m68k/include/asm/io_no.h | 68 -------
drivers/dma/mcf-edma-main.c | 14 -
drivers/net/can/flexcan/flexcan-core.c | 1
drivers/net/ethernet/freescale/fec.h | 15 +
drivers/net/ethernet/freescale/fec_main.c | 257 +++++++++++++++---------------
drivers/net/ethernet/freescale/fec_ptp.c | 78 ++++-----
drivers/net/ethernet/smsc/smc91x.h | 12 -
drivers/spi/spi-fsl-dspi.c | 14 +
8 files changed, 218 insertions(+), 241 deletions(-)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCHv3 1/3] net: fec: do not use readl()/writel() for ColdFire
2026-09-07 13:37 [PATCHv3 0/3] m68k: coldfire: fix non-standard readX()/writeX() functions Greg Ungerer
@ 2026-09-07 13:37 ` Greg Ungerer
2026-09-08 13:43 ` sashiko-bot
2026-09-11 0:42 ` Jakub Kicinski
2026-09-07 13:37 ` [PATCHv3 2/3] net: smc91x: do not use readw()/writew() on ColdFire platforms Greg Ungerer
2026-09-07 13:37 ` [PATCHv3 3/3] m68k: coldfire: fix non-standard readX()/writeX() functions Greg Ungerer
2 siblings, 2 replies; 11+ messages in thread
From: Greg Ungerer @ 2026-09-07 13:37 UTC (permalink / raw)
To: linux-m68k
Cc: linux-kernel, arnd, wei.fang, frank.li, shenwei.wang, imx, netdev,
nico, linux-can, linux-spi, olteanv, Greg Ungerer, andrew+netdev,
davem, edumazet, kuba, pabeni, Andrew Lunn
Modify the FEC driver to not directly use readl() and writel() to access
hardware registers but instead local fec_readl() and fec_writel() methods.
This allows for different architecture users of this driver to have
different underlying access functions - to support both little and big
endian hardware.
The FEC hardware block in ColdFire SoC parts is accessed big-endian.
The usual kernel readl()/writel() IO memory access methods are defined to
access little endian data. Change access for ColdFire to use __raw_readl()
and __raw_writel() access methods - which do not modify or swap bytes
on access.
The FEC driver works today because the m68k architecture io.h has a
kludge in the definitions of the readl() and writel() functions for
ColdFire that allow big-endian access if the address of the register to
access is within the SoC's internal peripheral registers. This is being
fixed in the near future to define readl() and writel() correctly - with
no byte swapping. Thus the motivation for this fix here.
__raw_readl()/__raw_writel() access methods are used instead of the more
commonly used ioread32be()/iowrite32be() here because those are broken too,
because of the current readl()/writel() kludge. They are implemented in
asm-generic/io.h in terms of readl()/writel().
Note that even when readl() and writel() are fixed on ColdFire they will
not be the right thing to use within the FEC driver on ColdFire hardware.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Greg Ungerer <gerg@linux-m68k.org>
---
v2: changed from RFC to PATCH
minor reordering of local variables for preferred layout
v3: no change
Resending with expanded CC list as per get_maintainers.pl, hoping to get
this picked up.
drivers/net/ethernet/freescale/fec.h | 15 ++
drivers/net/ethernet/freescale/fec_main.c | 257 +++++++++++-----------
drivers/net/ethernet/freescale/fec_ptp.c | 78 +++----
3 files changed, 183 insertions(+), 167 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 7176803146f3..af31d946a638 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -701,5 +701,20 @@ int fec_ptp_set(struct net_device *ndev, struct kernel_hwtstamp_config *config,
struct netlink_ext_ack *extack);
void fec_ptp_get(struct net_device *ndev, struct kernel_hwtstamp_config *config);
+/*
+ * ColdFire SoC peripheral blocks are big-endian, so use the raw IO access
+ * functions for them.
+ */
+#ifdef CONFIG_COLDFIRE
+#define fec_readl __raw_readl
+#define fec_writel __raw_writel
+#define fec_readl_poll_timeout_atomic(addr, val, cond, delay_us, timeout_us) \
+ readx_poll_timeout_atomic(__raw_readl, addr, val, cond, delay_us, timeout_us)
+#else
+#define fec_readl readl
+#define fec_writel writel
+#define fec_readl_poll_timeout_atomic readl_poll_timeout_atomic
+#endif
+
/****************************************************************************/
#endif /* FEC_H */
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 794ec427b0ee..a497dd686fd4 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -497,11 +497,11 @@ static void fec_txq_trigger_xmit(struct fec_enet_private *fep,
struct fec_enet_priv_tx_q *txq)
{
if (!(fep->quirks & FEC_QUIRK_ERR007885) ||
- !readl(txq->bd.reg_desc_active) ||
- !readl(txq->bd.reg_desc_active) ||
- !readl(txq->bd.reg_desc_active) ||
- !readl(txq->bd.reg_desc_active))
- writel(0, txq->bd.reg_desc_active);
+ !fec_readl(txq->bd.reg_desc_active) ||
+ !fec_readl(txq->bd.reg_desc_active) ||
+ !fec_readl(txq->bd.reg_desc_active) ||
+ !fec_readl(txq->bd.reg_desc_active))
+ fec_writel(0, txq->bd.reg_desc_active);
}
static struct bufdesc *
@@ -1069,7 +1069,7 @@ static void fec_enet_active_rxring(struct net_device *ndev)
int i;
for (i = 0; i < fep->num_rx_queues; i++)
- writel(0, fep->rx_queue[i]->bd.reg_desc_active);
+ fec_writel(0, fep->rx_queue[i]->bd.reg_desc_active);
}
static void fec_enet_enable_ring(struct net_device *ndev)
@@ -1081,23 +1081,23 @@ static void fec_enet_enable_ring(struct net_device *ndev)
for (i = 0; i < fep->num_rx_queues; i++) {
rxq = fep->rx_queue[i];
- writel(rxq->bd.dma, fep->hwp + FEC_R_DES_START(i));
- writel(fep->max_buf_size, fep->hwp + FEC_R_BUFF_SIZE(i));
+ fec_writel(rxq->bd.dma, fep->hwp + FEC_R_DES_START(i));
+ fec_writel(fep->max_buf_size, fep->hwp + FEC_R_BUFF_SIZE(i));
/* enable DMA1/2 */
if (i)
- writel(RCMR_MATCHEN | RCMR_CMP(i),
- fep->hwp + FEC_RCMR(i));
+ fec_writel(RCMR_MATCHEN | RCMR_CMP(i),
+ fep->hwp + FEC_RCMR(i));
}
for (i = 0; i < fep->num_tx_queues; i++) {
txq = fep->tx_queue[i];
- writel(txq->bd.dma, fep->hwp + FEC_X_DES_START(i));
+ fec_writel(txq->bd.dma, fep->hwp + FEC_X_DES_START(i));
/* enable DMA1/2 */
if (i)
- writel(DMA_CLASS_EN | IDLE_SLOPE(i),
- fep->hwp + FEC_DMA_CFG(i));
+ fec_writel(DMA_CLASS_EN | IDLE_SLOPE(i),
+ fep->hwp + FEC_DMA_CFG(i));
}
}
@@ -1112,15 +1112,15 @@ static void fec_ctrl_reset(struct fec_enet_private *fep, bool allow_wol)
if (!allow_wol || !(fep->wol_flag & FEC_WOL_FLAG_SLEEP_ON)) {
if (fep->quirks & FEC_QUIRK_HAS_MULTI_QUEUES ||
((fep->quirks & FEC_QUIRK_NO_HARD_RESET) && fep->link)) {
- writel(0, fep->hwp + FEC_ECNTRL);
+ fec_writel(0, fep->hwp + FEC_ECNTRL);
} else {
- writel(FEC_ECR_RESET, fep->hwp + FEC_ECNTRL);
+ fec_writel(FEC_ECR_RESET, fep->hwp + FEC_ECNTRL);
udelay(10);
}
} else {
- val = readl(fep->hwp + FEC_ECNTRL);
+ val = fec_readl(fep->hwp + FEC_ECNTRL);
val |= (FEC_ECR_MAGICEN | FEC_ECR_SLEEP);
- writel(val, fep->hwp + FEC_ECNTRL);
+ fec_writel(val, fep->hwp + FEC_ECNTRL);
}
}
@@ -1128,11 +1128,11 @@ static void fec_set_hw_mac_addr(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- writel(ndev->dev_addr[3] | (ndev->dev_addr[2] << 8) |
- (ndev->dev_addr[1] << 16) | (ndev->dev_addr[0] << 24),
- fep->hwp + FEC_ADDR_LOW);
- writel((ndev->dev_addr[5] << 16) | (ndev->dev_addr[4] << 24),
- fep->hwp + FEC_ADDR_HIGH);
+ fec_writel(ndev->dev_addr[3] | (ndev->dev_addr[2] << 8) |
+ (ndev->dev_addr[1] << 16) | (ndev->dev_addr[0] << 24),
+ fep->hwp + FEC_ADDR_LOW);
+ fec_writel((ndev->dev_addr[5] << 16) | (ndev->dev_addr[4] << 24),
+ fep->hwp + FEC_ADDR_HIGH);
}
/*
@@ -1162,7 +1162,7 @@ fec_restart(struct net_device *ndev)
fec_set_hw_mac_addr(ndev);
/* Clear any outstanding interrupt, except MDIO. */
- writel((0xffffffff & ~FEC_ENET_MII), fep->hwp + FEC_IEVENT);
+ fec_writel((0xffffffff & ~FEC_ENET_MII), fep->hwp + FEC_IEVENT);
fec_enet_bd_init(ndev);
@@ -1171,19 +1171,19 @@ fec_restart(struct net_device *ndev)
/* Enable MII mode */
if (fep->full_duplex == DUPLEX_FULL) {
/* FD enable */
- writel(0x04, fep->hwp + FEC_X_CNTRL);
+ fec_writel(0x04, fep->hwp + FEC_X_CNTRL);
} else {
/* No Rcv on Xmit */
rcntl |= FEC_RCR_DRT;
- writel(0x0, fep->hwp + FEC_X_CNTRL);
+ fec_writel(0x0, fep->hwp + FEC_X_CNTRL);
}
/* Set MII speed */
- writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
+ fec_writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
#if !defined(CONFIG_M5272)
if (fep->quirks & FEC_QUIRK_HAS_RACC) {
- u32 val = readl(fep->hwp + FEC_RACC);
+ u32 val = fec_readl(fep->hwp + FEC_RACC);
/* align IP header */
val |= FEC_RACC_SHIFT16;
@@ -1192,8 +1192,8 @@ fec_restart(struct net_device *ndev)
val |= FEC_RACC_OPTIONS;
else
val &= ~FEC_RACC_OPTIONS;
- writel(val, fep->hwp + FEC_RACC);
- writel(min(fep->rx_frame_size, fep->max_buf_size), fep->hwp + FEC_FTRL);
+ fec_writel(val, fep->hwp + FEC_RACC);
+ fec_writel(min(fep->rx_frame_size, fep->max_buf_size), fep->hwp + FEC_FTRL);
}
#endif
@@ -1227,8 +1227,8 @@ fec_restart(struct net_device *ndev)
if (fep->quirks & FEC_QUIRK_USE_GASKET) {
u32 cfgr;
/* disable the gasket and wait */
- writel(0, fep->hwp + FEC_MIIGSK_ENR);
- while (readl(fep->hwp + FEC_MIIGSK_ENR) & 4)
+ fec_writel(0, fep->hwp + FEC_MIIGSK_ENR);
+ while (fec_readl(fep->hwp + FEC_MIIGSK_ENR) & 4)
udelay(1);
/*
@@ -1240,10 +1240,10 @@ fec_restart(struct net_device *ndev)
? BM_MIIGSK_CFGR_RMII : BM_MIIGSK_CFGR_MII;
if (ndev->phydev && ndev->phydev->speed == SPEED_10)
cfgr |= BM_MIIGSK_CFGR_FRCONT_10M;
- writel(cfgr, fep->hwp + FEC_MIIGSK_CFGR);
+ fec_writel(cfgr, fep->hwp + FEC_MIIGSK_CFGR);
/* re-enable the gasket */
- writel(2, fep->hwp + FEC_MIIGSK_ENR);
+ fec_writel(2, fep->hwp + FEC_MIIGSK_ENR);
}
#endif
}
@@ -1256,25 +1256,25 @@ fec_restart(struct net_device *ndev)
rcntl |= FEC_RCR_FLOWCTL;
/* set FIFO threshold parameter to reduce overrun */
- writel(FEC_ENET_RSEM_V, fep->hwp + FEC_R_FIFO_RSEM);
- writel(FEC_ENET_RSFL_V, fep->hwp + FEC_R_FIFO_RSFL);
- writel(FEC_ENET_RAEM_V, fep->hwp + FEC_R_FIFO_RAEM);
- writel(FEC_ENET_RAFL_V, fep->hwp + FEC_R_FIFO_RAFL);
+ fec_writel(FEC_ENET_RSEM_V, fep->hwp + FEC_R_FIFO_RSEM);
+ fec_writel(FEC_ENET_RSFL_V, fep->hwp + FEC_R_FIFO_RSFL);
+ fec_writel(FEC_ENET_RAEM_V, fep->hwp + FEC_R_FIFO_RAEM);
+ fec_writel(FEC_ENET_RAFL_V, fep->hwp + FEC_R_FIFO_RAFL);
/* OPD */
- writel(FEC_ENET_OPD_V, fep->hwp + FEC_OPD);
+ fec_writel(FEC_ENET_OPD_V, fep->hwp + FEC_OPD);
} else {
rcntl &= ~FEC_RCR_FLOWCTL;
}
#endif /* !defined(CONFIG_M5272) */
- writel(rcntl, fep->hwp + FEC_R_CNTRL);
+ fec_writel(rcntl, fep->hwp + FEC_R_CNTRL);
/* Setup multicast filter. */
set_multicast_list(ndev);
#ifndef CONFIG_M5272
- writel(0, fep->hwp + FEC_HASH_TABLE_HIGH);
- writel(0, fep->hwp + FEC_HASH_TABLE_LOW);
+ fec_writel(0, fep->hwp + FEC_HASH_TABLE_HIGH);
+ fec_writel(0, fep->hwp + FEC_HASH_TABLE_LOW);
#endif
if (fep->quirks & FEC_QUIRK_ENET_MAC) {
@@ -1290,9 +1290,9 @@ fec_restart(struct net_device *ndev)
*/
if ((fep->quirks & FEC_QUIRK_JUMBO_FRAME) &&
(ndev->mtu > (PKT_MAXBUF_SIZE - VLAN_ETH_HLEN - ETH_FCS_LEN)))
- writel(0xF, fep->hwp + FEC_X_WMRK);
+ fec_writel(0xF, fep->hwp + FEC_X_WMRK);
else
- writel(FEC_TXWMRK_STRFWD, fep->hwp + FEC_X_WMRK);
+ fec_writel(FEC_TXWMRK_STRFWD, fep->hwp + FEC_X_WMRK);
}
if (fep->bufdesc_ex)
@@ -1307,11 +1307,11 @@ fec_restart(struct net_device *ndev)
#ifndef CONFIG_M5272
/* Enable the MIB statistic event counters */
- writel(0 << 31, fep->hwp + FEC_MIB_CTRLSTAT);
+ fec_writel(0 << 31, fep->hwp + FEC_MIB_CTRLSTAT);
#endif
/* And last, enable the transmit and receive processing */
- writel(ecntl, fep->hwp + FEC_ECNTRL);
+ fec_writel(ecntl, fep->hwp + FEC_ECNTRL);
fec_enet_active_rxring(ndev);
if (fep->bufdesc_ex) {
@@ -1321,9 +1321,9 @@ fec_restart(struct net_device *ndev)
/* Enable interrupts we wish to service */
if (fep->link)
- writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
+ fec_writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
else
- writel(0, fep->hwp + FEC_IMASK);
+ fec_writel(0, fep->hwp + FEC_IMASK);
/* Init the interrupt coalescing */
if (fep->quirks & FEC_QUIRK_HAS_COALESCE)
@@ -1384,29 +1384,30 @@ static void fec_irqs_disable(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- writel(0, fep->hwp + FEC_IMASK);
+ fec_writel(0, fep->hwp + FEC_IMASK);
}
static void fec_irqs_disable_except_wakeup(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- writel(0, fep->hwp + FEC_IMASK);
- writel(FEC_ENET_WAKEUP, fep->hwp + FEC_IMASK);
+ fec_writel(0, fep->hwp + FEC_IMASK);
+ fec_writel(FEC_ENET_WAKEUP, fep->hwp + FEC_IMASK);
}
static void
fec_stop(struct net_device *ndev)
{
struct fec_enet_private *fep = netdev_priv(ndev);
- u32 rmii_mode = readl(fep->hwp + FEC_R_CNTRL) & FEC_RCR_RMII;
- u32 val;
+ u32 rmii_mode, val;
+
+ rmii_mode = fec_readl(fep->hwp + FEC_R_CNTRL) & FEC_RCR_RMII;
/* We cannot expect a graceful transmit stop without link !!! */
if (fep->link) {
- writel(1, fep->hwp + FEC_X_CNTRL); /* Graceful transmit stop */
+ fec_writel(1, fep->hwp + FEC_X_CNTRL); /* Graceful transmit stop */
udelay(10);
- if (!(readl(fep->hwp + FEC_IEVENT) & FEC_ENET_GRA))
+ if (!(fec_readl(fep->hwp + FEC_IEVENT) & FEC_ENET_GRA))
netdev_err(ndev, "Graceful transmit stop did not complete!\n");
}
@@ -1414,20 +1415,20 @@ fec_stop(struct net_device *ndev)
fec_ptp_save_state(fep);
fec_ctrl_reset(fep, true);
- writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
- writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
+ fec_writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
+ fec_writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
/* We have to keep ENET enabled to have MII interrupt stay working */
if (fep->quirks & FEC_QUIRK_ENET_MAC &&
!(fep->wol_flag & FEC_WOL_FLAG_SLEEP_ON)) {
- writel(FEC_ECR_ETHEREN, fep->hwp + FEC_ECNTRL);
- writel(rmii_mode, fep->hwp + FEC_R_CNTRL);
+ fec_writel(FEC_ECR_ETHEREN, fep->hwp + FEC_ECNTRL);
+ fec_writel(rmii_mode, fep->hwp + FEC_R_CNTRL);
}
if (fep->bufdesc_ex) {
- val = readl(fep->hwp + FEC_ECNTRL);
+ val = fec_readl(fep->hwp + FEC_ECNTRL);
val |= FEC_ECR_EN1588;
- writel(val, fep->hwp + FEC_ECNTRL);
+ fec_writel(val, fep->hwp + FEC_ECNTRL);
fec_ptp_start_cyclecounter(ndev);
fec_ptp_restore_state(fep);
@@ -1713,8 +1714,8 @@ static int fec_enet_tx_queue(struct fec_enet_private *fep,
/* ERR006358: Keep the transmitter going */
if (bdp != txq->bd.cur &&
- readl(txq->bd.reg_desc_active) == 0)
- writel(0, txq->bd.reg_desc_active);
+ fec_readl(txq->bd.reg_desc_active) == 0)
+ fec_writel(0, txq->bd.reg_desc_active);
if (txq->xsk_pool) {
struct xsk_buff_pool *pool = txq->xsk_pool;
@@ -1923,7 +1924,7 @@ static int fec_enet_rx_queue(struct fec_enet_private *fep,
break;
pkt_received++;
- writel(FEC_ENET_RXF_GET(queue), fep->hwp + FEC_IEVENT);
+ fec_writel(FEC_ENET_RXF_GET(queue), fep->hwp + FEC_IEVENT);
/* Check for errors. */
status ^= BD_ENET_RX_LAST;
@@ -1991,7 +1992,7 @@ static int fec_enet_rx_queue(struct fec_enet_private *fep,
* incoming frames. On a heavily loaded network, we should be
* able to keep up at the expense of system resources.
*/
- writel(0, rxq->bd.reg_desc_active);
+ fec_writel(0, rxq->bd.reg_desc_active);
}
rxq->bd.cur = bdp;
@@ -2053,7 +2054,7 @@ static int fec_enet_rx_queue_xdp(struct fec_enet_private *fep, int queue,
break;
pkt_received++;
- writel(FEC_ENET_RXF_GET(queue), fep->hwp + FEC_IEVENT);
+ fec_writel(FEC_ENET_RXF_GET(queue), fep->hwp + FEC_IEVENT);
/* Check for errors. */
status ^= BD_ENET_RX_LAST;
@@ -2166,7 +2167,7 @@ static int fec_enet_rx_queue_xdp(struct fec_enet_private *fep, int queue,
* incoming frames. On a heavily loaded network, we should be
* able to keep up at the expense of system resources.
*/
- writel(0, rxq->bd.reg_desc_active);
+ fec_writel(0, rxq->bd.reg_desc_active);
}
rxq->bd.cur = bdp;
@@ -2296,7 +2297,7 @@ static int fec_enet_rx_queue_xsk(struct fec_enet_private *fep, int queue,
if (unlikely(pkt_received >= budget))
break;
- writel(FEC_ENET_RXF_GET(queue), fep->hwp + FEC_IEVENT);
+ fec_writel(FEC_ENET_RXF_GET(queue), fep->hwp + FEC_IEVENT);
index = fec_enet_get_bd_index(bdp, &rxq->bd);
xsk = rxq->rx_buf[index].xdp;
@@ -2428,7 +2429,7 @@ static int fec_enet_rx_queue_xsk(struct fec_enet_private *fep, int queue,
* incoming frames. On a heavily loaded network, we should be
* able to keep up at the expense of system resources.
*/
- writel(0, rxq->bd.reg_desc_active);
+ fec_writel(0, rxq->bd.reg_desc_active);
}
rxq->bd.cur = bdp;
@@ -2475,12 +2476,12 @@ static bool fec_enet_collect_events(struct fec_enet_private *fep)
{
uint int_events;
- int_events = readl(fep->hwp + FEC_IEVENT);
+ int_events = fec_readl(fep->hwp + FEC_IEVENT);
/* Don't clear MDIO events, we poll for those */
int_events &= ~FEC_ENET_MII;
- writel(int_events, fep->hwp + FEC_IEVENT);
+ fec_writel(int_events, fep->hwp + FEC_IEVENT);
return int_events != 0;
}
@@ -2497,7 +2498,7 @@ fec_enet_interrupt(int irq, void *dev_id)
if (napi_schedule_prep(&fep->napi)) {
/* Disable interrupts */
- writel(0, fep->hwp + FEC_IMASK);
+ fec_writel(0, fep->hwp + FEC_IMASK);
__napi_schedule(&fep->napi);
}
}
@@ -2520,7 +2521,7 @@ static int fec_enet_rx_napi(struct napi_struct *napi, int budget)
if (max_done < budget) {
napi_complete_done(napi, max_done);
- writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
+ fec_writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
return max_done;
}
@@ -2576,9 +2577,9 @@ static int fec_get_mac(struct net_device *ndev)
*/
if (!is_valid_ether_addr(iap)) {
*((__be32 *) &tmpaddr[0]) =
- cpu_to_be32(readl(fep->hwp + FEC_ADDR_LOW));
+ cpu_to_be32(fec_readl(fep->hwp + FEC_ADDR_LOW));
*((__be16 *) &tmpaddr[4]) =
- cpu_to_be16(readl(fep->hwp + FEC_ADDR_HIGH) >> 16);
+ cpu_to_be16(fec_readl(fep->hwp + FEC_ADDR_HIGH) >> 16);
iap = &tmpaddr[0];
}
@@ -2630,8 +2631,8 @@ static int fec_enet_eee_mode_set(struct net_device *ndev, u32 lpi_timer,
wake_cycle = 0;
}
- writel(sleep_cycle, fep->hwp + FEC_LPI_SLEEP);
- writel(wake_cycle, fep->hwp + FEC_LPI_WAKE);
+ fec_writel(sleep_cycle, fep->hwp + FEC_LPI_SLEEP);
+ fec_writel(wake_cycle, fep->hwp + FEC_LPI_WAKE);
return 0;
}
@@ -2701,11 +2702,11 @@ static int fec_enet_mdio_wait(struct fec_enet_private *fep)
uint ievent;
int ret;
- ret = readl_poll_timeout_atomic(fep->hwp + FEC_IEVENT, ievent,
- ievent & FEC_ENET_MII, 2, 30000);
+ ret = fec_readl_poll_timeout_atomic(fep->hwp + FEC_IEVENT, ievent,
+ ievent & FEC_ENET_MII, 2, 30000);
if (!ret)
- writel(FEC_ENET_MII, fep->hwp + FEC_IEVENT);
+ fec_writel(FEC_ENET_MII, fep->hwp + FEC_IEVENT);
return ret;
}
@@ -2726,9 +2727,9 @@ static int fec_enet_mdio_read_c22(struct mii_bus *bus, int mii_id, int regnum)
frame_addr = regnum;
/* start a read op */
- writel(frame_start | frame_op |
- FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
- FEC_MMFR_TA, fep->hwp + FEC_MII_DATA);
+ fec_writel(frame_start | frame_op |
+ FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
+ FEC_MMFR_TA, fep->hwp + FEC_MII_DATA);
/* wait for end of transfer */
ret = fec_enet_mdio_wait(fep);
@@ -2737,7 +2738,7 @@ static int fec_enet_mdio_read_c22(struct mii_bus *bus, int mii_id, int regnum)
goto out;
}
- ret = FEC_MMFR_DATA(readl(fep->hwp + FEC_MII_DATA));
+ ret = FEC_MMFR_DATA(fec_readl(fep->hwp + FEC_MII_DATA));
out:
pm_runtime_put_autosuspend(dev);
@@ -2759,10 +2760,10 @@ static int fec_enet_mdio_read_c45(struct mii_bus *bus, int mii_id,
frame_start = FEC_MMFR_ST_C45;
/* write address */
- writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
- FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(devad) |
- FEC_MMFR_TA | (regnum & 0xFFFF),
- fep->hwp + FEC_MII_DATA);
+ fec_writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
+ FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(devad) |
+ FEC_MMFR_TA | (regnum & 0xFFFF),
+ fep->hwp + FEC_MII_DATA);
/* wait for end of transfer */
ret = fec_enet_mdio_wait(fep);
@@ -2774,9 +2775,9 @@ static int fec_enet_mdio_read_c45(struct mii_bus *bus, int mii_id,
frame_op = FEC_MMFR_OP_READ_C45;
/* start a read op */
- writel(frame_start | frame_op |
- FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(devad) |
- FEC_MMFR_TA, fep->hwp + FEC_MII_DATA);
+ fec_writel(frame_start | frame_op |
+ FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(devad) |
+ FEC_MMFR_TA, fep->hwp + FEC_MII_DATA);
/* wait for end of transfer */
ret = fec_enet_mdio_wait(fep);
@@ -2785,7 +2786,7 @@ static int fec_enet_mdio_read_c45(struct mii_bus *bus, int mii_id,
goto out;
}
- ret = FEC_MMFR_DATA(readl(fep->hwp + FEC_MII_DATA));
+ ret = FEC_MMFR_DATA(fec_readl(fep->hwp + FEC_MII_DATA));
out:
pm_runtime_put_autosuspend(dev);
@@ -2809,10 +2810,10 @@ static int fec_enet_mdio_write_c22(struct mii_bus *bus, int mii_id, int regnum,
frame_addr = regnum;
/* start a write op */
- writel(frame_start | FEC_MMFR_OP_WRITE |
- FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
- FEC_MMFR_TA | FEC_MMFR_DATA(value),
- fep->hwp + FEC_MII_DATA);
+ fec_writel(frame_start | FEC_MMFR_OP_WRITE |
+ FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(frame_addr) |
+ FEC_MMFR_TA | FEC_MMFR_DATA(value),
+ fep->hwp + FEC_MII_DATA);
/* wait for end of transfer */
ret = fec_enet_mdio_wait(fep);
@@ -2838,10 +2839,10 @@ static int fec_enet_mdio_write_c45(struct mii_bus *bus, int mii_id,
frame_start = FEC_MMFR_ST_C45;
/* write address */
- writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
- FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(devad) |
- FEC_MMFR_TA | (regnum & 0xFFFF),
- fep->hwp + FEC_MII_DATA);
+ fec_writel(frame_start | FEC_MMFR_OP_ADDR_WRITE |
+ FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(devad) |
+ FEC_MMFR_TA | (regnum & 0xFFFF),
+ fep->hwp + FEC_MII_DATA);
/* wait for end of transfer */
ret = fec_enet_mdio_wait(fep);
@@ -2851,10 +2852,10 @@ static int fec_enet_mdio_write_c45(struct mii_bus *bus, int mii_id,
}
/* start a write op */
- writel(frame_start | FEC_MMFR_OP_WRITE |
- FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(devad) |
- FEC_MMFR_TA | FEC_MMFR_DATA(value),
- fep->hwp + FEC_MII_DATA);
+ fec_writel(frame_start | FEC_MMFR_OP_WRITE |
+ FEC_MMFR_PA(mii_id) | FEC_MMFR_RA(devad) |
+ FEC_MMFR_TA | FEC_MMFR_DATA(value),
+ fep->hwp + FEC_MII_DATA);
/* wait for end of transfer */
ret = fec_enet_mdio_wait(fep);
@@ -3132,13 +3133,13 @@ static int fec_enet_mii_init(struct platform_device *pdev)
* - writing MMFR:
* - mscr[7:0]_not_zero
*/
- writel(0, fep->hwp + FEC_MII_DATA);
+ fec_writel(0, fep->hwp + FEC_MII_DATA);
}
- writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
+ fec_writel(fep->phy_speed, fep->hwp + FEC_MII_SPEED);
/* Clear any pending transaction complete indication */
- writel(FEC_ENET_MII, fep->hwp + FEC_IEVENT);
+ fec_writel(FEC_ENET_MII, fep->hwp + FEC_IEVENT);
fep->mii_bus = mdiobus_alloc();
if (fep->mii_bus == NULL) {
@@ -3320,7 +3321,7 @@ static void fec_enet_get_regs(struct net_device *ndev,
continue;
off >>= 2;
- buf[off] = readl(&theregs[off]);
+ buf[off] = fec_readl(&theregs[off]);
}
pm_runtime_put_autosuspend(dev);
@@ -3487,7 +3488,7 @@ static void fec_enet_update_ethtool_stats(struct net_device *dev)
int i;
for (i = 0; i < ARRAY_SIZE(fec_stats); i++)
- fep->ethtool_stats[i] = readl(fep->hwp + fec_stats[i].offset);
+ fep->ethtool_stats[i] = fec_readl(fep->hwp + fec_stats[i].offset);
}
static void fec_enet_get_xdp_stats(struct fec_enet_private *fep, u64 *data)
@@ -3588,10 +3589,10 @@ static void fec_enet_clear_ethtool_stats(struct net_device *dev)
int i, j;
/* Disable MIB statistics counters */
- writel(FEC_MIB_CTRLSTAT_DISABLE, fep->hwp + FEC_MIB_CTRLSTAT);
+ fec_writel(FEC_MIB_CTRLSTAT_DISABLE, fep->hwp + FEC_MIB_CTRLSTAT);
for (i = 0; i < ARRAY_SIZE(fec_stats); i++)
- writel(0, fep->hwp + fec_stats[i].offset);
+ fec_writel(0, fep->hwp + fec_stats[i].offset);
for (i = fep->num_rx_queues - 1; i >= 0; i--) {
rxq = fep->rx_queue[i];
@@ -3600,7 +3601,7 @@ static void fec_enet_clear_ethtool_stats(struct net_device *dev)
}
/* Don't disable MIB statistics counters */
- writel(0, fep->hwp + FEC_MIB_CTRLSTAT);
+ fec_writel(0, fep->hwp + FEC_MIB_CTRLSTAT);
}
#else /* !defined(CONFIG_M5272) */
@@ -3649,13 +3650,13 @@ static void fec_enet_itr_coal_set(struct net_device *ndev)
tx_itr |= FEC_ITR_ICTT(tx_ictt);
}
- writel(tx_itr, fep->hwp + FEC_TXIC0);
- writel(rx_itr, fep->hwp + FEC_RXIC0);
+ fec_writel(tx_itr, fep->hwp + FEC_TXIC0);
+ fec_writel(rx_itr, fep->hwp + FEC_RXIC0);
if (fep->quirks & FEC_QUIRK_HAS_MULTI_QUEUES) {
- writel(tx_itr, fep->hwp + FEC_TXIC1);
- writel(rx_itr, fep->hwp + FEC_RXIC1);
- writel(tx_itr, fep->hwp + FEC_TXIC2);
- writel(rx_itr, fep->hwp + FEC_RXIC2);
+ fec_writel(tx_itr, fep->hwp + FEC_TXIC1);
+ fec_writel(rx_itr, fep->hwp + FEC_RXIC1);
+ fec_writel(tx_itr, fep->hwp + FEC_TXIC2);
+ fec_writel(rx_itr, fep->hwp + FEC_RXIC2);
}
}
@@ -4281,22 +4282,22 @@ static void set_multicast_list(struct net_device *ndev)
unsigned int hash_high = 0, hash_low = 0;
if (ndev->flags & IFF_PROMISC) {
- tmp = readl(fep->hwp + FEC_R_CNTRL);
+ tmp = fec_readl(fep->hwp + FEC_R_CNTRL);
tmp |= 0x8;
- writel(tmp, fep->hwp + FEC_R_CNTRL);
+ fec_writel(tmp, fep->hwp + FEC_R_CNTRL);
return;
}
- tmp = readl(fep->hwp + FEC_R_CNTRL);
+ tmp = fec_readl(fep->hwp + FEC_R_CNTRL);
tmp &= ~0x8;
- writel(tmp, fep->hwp + FEC_R_CNTRL);
+ fec_writel(tmp, fep->hwp + FEC_R_CNTRL);
if (ndev->flags & IFF_ALLMULTI) {
/* Catch all multicast addresses, so set the
* filter to all 1's
*/
- writel(0xffffffff, fep->hwp + FEC_GRP_HASH_TABLE_HIGH);
- writel(0xffffffff, fep->hwp + FEC_GRP_HASH_TABLE_LOW);
+ fec_writel(0xffffffff, fep->hwp + FEC_GRP_HASH_TABLE_HIGH);
+ fec_writel(0xffffffff, fep->hwp + FEC_GRP_HASH_TABLE_LOW);
return;
}
@@ -4317,8 +4318,8 @@ static void set_multicast_list(struct net_device *ndev)
hash_low |= 1 << hash;
}
- writel(hash_high, fep->hwp + FEC_GRP_HASH_TABLE_HIGH);
- writel(hash_low, fep->hwp + FEC_GRP_HASH_TABLE_LOW);
+ fec_writel(hash_high, fep->hwp + FEC_GRP_HASH_TABLE_HIGH);
+ fec_writel(hash_low, fep->hwp + FEC_GRP_HASH_TABLE_LOW);
}
/* Set a MAC change in hardware. */
@@ -5003,7 +5004,7 @@ static int fec_enet_init(struct net_device *ndev)
ndev->netdev_ops = &fec_netdev_ops;
ndev->ethtool_ops = &fec_enet_ethtool_ops;
- writel(FEC_RX_DISABLED_IMASK, fep->hwp + FEC_IMASK);
+ fec_writel(FEC_RX_DISABLED_IMASK, fep->hwp + FEC_IMASK);
netif_napi_add(ndev, &fep->napi, fec_enet_rx_napi);
if (fep->quirks & FEC_QUIRK_HAS_VLAN)
@@ -5609,9 +5610,9 @@ static int fec_resume(struct device *dev)
enable_irq(fep->wake_irq);
}
- val = readl(fep->hwp + FEC_ECNTRL);
+ val = fec_readl(fep->hwp + FEC_ECNTRL);
val &= ~(FEC_ECR_MAGICEN | FEC_ECR_SLEEP);
- writel(val, fep->hwp + FEC_ECNTRL);
+ fec_writel(val, fep->hwp + FEC_ECNTRL);
fep->wol_flag &= ~FEC_WOL_FLAG_SLEEP_ON;
}
fec_restart(ndev);
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index 56801c2009d5..07062599b522 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -102,14 +102,14 @@ static u64 fec_ptp_read(struct cyclecounter *cc)
container_of(cc, struct fec_enet_private, cc);
u32 tempval;
- tempval = readl(fep->hwp + FEC_ATIME_CTRL);
+ tempval = fec_readl(fep->hwp + FEC_ATIME_CTRL);
tempval |= FEC_T_CTRL_CAPTURE;
- writel(tempval, fep->hwp + FEC_ATIME_CTRL);
+ fec_writel(tempval, fep->hwp + FEC_ATIME_CTRL);
if (fep->quirks & FEC_QUIRK_BUG_CAPTURE)
udelay(1);
- return readl(fep->hwp + FEC_ATIME);
+ return fec_readl(fep->hwp + FEC_ATIME);
}
/**
@@ -142,17 +142,17 @@ static int fec_ptp_enable_pps(struct fec_enet_private *fep, uint enable)
if (enable) {
/* clear capture or output compare interrupt status if have.
*/
- writel(FEC_T_TF_MASK, fep->hwp + FEC_TCSR(fep->pps_channel));
+ fec_writel(FEC_T_TF_MASK, fep->hwp + FEC_TCSR(fep->pps_channel));
/* It is recommended to double check the TMODE field in the
* TCSR register to be cleared before the first compare counter
* is written into TCCR register. Just add a double check.
*/
- val = readl(fep->hwp + FEC_TCSR(fep->pps_channel));
+ val = fec_readl(fep->hwp + FEC_TCSR(fep->pps_channel));
do {
val &= ~(FEC_T_TMODE_MASK);
- writel(val, fep->hwp + FEC_TCSR(fep->pps_channel));
- val = readl(fep->hwp + FEC_TCSR(fep->pps_channel));
+ fec_writel(val, fep->hwp + FEC_TCSR(fep->pps_channel));
+ val = fec_readl(fep->hwp + FEC_TCSR(fep->pps_channel));
} while (val & FEC_T_TMODE_MASK);
/* Dummy read counter to update the counter */
@@ -194,31 +194,31 @@ static int fec_ptp_enable_pps(struct fec_enet_private *fep, uint enable)
* is bigger than fep->cc.mask would be a error.
*/
val &= fep->cc.mask;
- writel(val, fep->hwp + FEC_TCCR(fep->pps_channel));
+ fec_writel(val, fep->hwp + FEC_TCCR(fep->pps_channel));
/* Calculate the second the compare event timestamp */
fep->next_counter = (val + fep->reload_period) & fep->cc.mask;
/* * Enable compare event when overflow */
- val = readl(fep->hwp + FEC_ATIME_CTRL);
+ val = fec_readl(fep->hwp + FEC_ATIME_CTRL);
val |= FEC_T_CTRL_PINPER;
- writel(val, fep->hwp + FEC_ATIME_CTRL);
+ fec_writel(val, fep->hwp + FEC_ATIME_CTRL);
/* Compare channel setting. */
- val = readl(fep->hwp + FEC_TCSR(fep->pps_channel));
+ val = fec_readl(fep->hwp + FEC_TCSR(fep->pps_channel));
val |= (1 << FEC_T_TF_OFFSET | 1 << FEC_T_TIE_OFFSET);
val &= ~(1 << FEC_T_TDRE_OFFSET);
val &= ~(FEC_T_TMODE_MASK);
val |= (FEC_HIGH_PULSE << FEC_T_TMODE_OFFSET);
- writel(val, fep->hwp + FEC_TCSR(fep->pps_channel));
+ fec_writel(val, fep->hwp + FEC_TCSR(fep->pps_channel));
/* Write the second compare event timestamp and calculate
* the third timestamp. Refer the TCCR register detail in the spec.
*/
- writel(fep->next_counter, fep->hwp + FEC_TCCR(fep->pps_channel));
+ fec_writel(fep->next_counter, fep->hwp + FEC_TCCR(fep->pps_channel));
fep->next_counter = (fep->next_counter + fep->reload_period) & fep->cc.mask;
} else {
- writel(0, fep->hwp + FEC_TCSR(fep->pps_channel));
+ fec_writel(0, fep->hwp + FEC_TCSR(fep->pps_channel));
}
fep->pps_enable = enable;
@@ -258,26 +258,26 @@ static int fec_ptp_pps_perout(struct fec_enet_private *fep)
compare_val = fep->perout_stime - curr_time + ptp_hc;
compare_val &= fep->cc.mask;
- writel(compare_val, fep->hwp + FEC_TCCR(fep->pps_channel));
+ fec_writel(compare_val, fep->hwp + FEC_TCCR(fep->pps_channel));
fep->next_counter = (compare_val + fep->reload_period) & fep->cc.mask;
/* Enable compare event when overflow */
- temp_val = readl(fep->hwp + FEC_ATIME_CTRL);
+ temp_val = fec_readl(fep->hwp + FEC_ATIME_CTRL);
temp_val |= FEC_T_CTRL_PINPER;
- writel(temp_val, fep->hwp + FEC_ATIME_CTRL);
+ fec_writel(temp_val, fep->hwp + FEC_ATIME_CTRL);
/* Compare channel setting. */
- temp_val = readl(fep->hwp + FEC_TCSR(fep->pps_channel));
+ temp_val = fec_readl(fep->hwp + FEC_TCSR(fep->pps_channel));
temp_val |= (1 << FEC_T_TF_OFFSET | 1 << FEC_T_TIE_OFFSET);
temp_val &= ~(1 << FEC_T_TDRE_OFFSET);
temp_val &= ~(FEC_T_TMODE_MASK);
temp_val |= (FEC_TMODE_TOGGLE << FEC_T_TMODE_OFFSET);
- writel(temp_val, fep->hwp + FEC_TCSR(fep->pps_channel));
+ fec_writel(temp_val, fep->hwp + FEC_TCSR(fep->pps_channel));
/* Write the second compare event timestamp and calculate
* the third timestamp. Refer the TCCR register detail in the spec.
*/
- writel(fep->next_counter, fep->hwp + FEC_TCCR(fep->pps_channel));
+ fec_writel(fep->next_counter, fep->hwp + FEC_TCCR(fep->pps_channel));
fep->next_counter = (fep->next_counter + fep->reload_period) & fep->cc.mask;
spin_unlock_irqrestore(&fep->tmreg_lock, flags);
@@ -314,13 +314,13 @@ void fec_ptp_start_cyclecounter(struct net_device *ndev)
spin_lock_irqsave(&fep->tmreg_lock, flags);
/* 1ns counter */
- writel(inc << FEC_T_INC_OFFSET, fep->hwp + FEC_ATIME_INC);
+ fec_writel(inc << FEC_T_INC_OFFSET, fep->hwp + FEC_ATIME_INC);
/* use 31-bit timer counter */
- writel(FEC_COUNTER_PERIOD, fep->hwp + FEC_ATIME_EVT_PERIOD);
+ fec_writel(FEC_COUNTER_PERIOD, fep->hwp + FEC_ATIME_EVT_PERIOD);
- writel(FEC_T_CTRL_ENABLE | FEC_T_CTRL_PERIOD_RST,
- fep->hwp + FEC_ATIME_CTRL);
+ fec_writel(FEC_T_CTRL_ENABLE | FEC_T_CTRL_PERIOD_RST,
+ fep->hwp + FEC_ATIME_CTRL);
memset(&fep->cc, 0, sizeof(fep->cc));
fep->cc.read = fec_ptp_read;
@@ -397,11 +397,11 @@ static int fec_ptp_adjfine(struct ptp_clock_info *ptp, long scaled_ppm)
spin_lock_irqsave(&fep->tmreg_lock, flags);
- tmp = readl(fep->hwp + FEC_ATIME_INC) & FEC_T_INC_MASK;
+ tmp = fec_readl(fep->hwp + FEC_ATIME_INC) & FEC_T_INC_MASK;
tmp |= corr_ns << FEC_T_INC_CORR_OFFSET;
- writel(tmp, fep->hwp + FEC_ATIME_INC);
+ fec_writel(tmp, fep->hwp + FEC_ATIME_INC);
corr_period = corr_period > 1 ? corr_period - 1 : corr_period;
- writel(corr_period, fep->hwp + FEC_ATIME_CORR);
+ fec_writel(corr_period, fep->hwp + FEC_ATIME_CORR);
/* dummy read to update the timer. */
timecounter_read(&fep->tc);
@@ -493,7 +493,7 @@ static int fec_ptp_settime(struct ptp_clock_info *ptp,
counter = ns & fep->cc.mask;
spin_lock_irqsave(&fep->tmreg_lock, flags);
- writel(counter, fep->hwp + FEC_ATIME);
+ fec_writel(counter, fep->hwp + FEC_ATIME);
timecounter_init(&fep->tc, &fep->cc, ns);
spin_unlock_irqrestore(&fep->tmreg_lock, flags);
mutex_unlock(&fep->ptp_clk_mutex);
@@ -508,7 +508,7 @@ static int fec_ptp_pps_disable(struct fec_enet_private *fep, uint channel)
spin_lock_irqsave(&fep->tmreg_lock, flags);
fep->perout_enable = false;
- writel(0, fep->hwp + FEC_TCSR(channel));
+ fec_writel(0, fep->hwp + FEC_TCSR(channel));
spin_unlock_irqrestore(&fep->tmreg_lock, flags);
return 0;
@@ -701,15 +701,15 @@ static irqreturn_t fec_pps_interrupt(int irq, void *dev_id)
u8 channel = fep->pps_channel;
struct ptp_clock_event event;
- val = readl(fep->hwp + FEC_TCSR(channel));
+ val = fec_readl(fep->hwp + FEC_TCSR(channel));
if (val & FEC_T_TF_MASK) {
/* Write the next next compare(not the next according the spec)
* value to the register
*/
- writel(fep->next_counter, fep->hwp + FEC_TCCR(channel));
+ fec_writel(fep->next_counter, fep->hwp + FEC_TCCR(channel));
do {
- writel(val, fep->hwp + FEC_TCSR(channel));
- } while (readl(fep->hwp + FEC_TCSR(channel)) & FEC_T_TF_MASK);
+ fec_writel(val, fep->hwp + FEC_TCSR(channel));
+ } while (fec_readl(fep->hwp + FEC_TCSR(channel)) & FEC_T_TF_MASK);
/* Update the counter; */
fep->next_counter = (fep->next_counter + fep->reload_period) &
@@ -813,8 +813,8 @@ void fec_ptp_save_state(struct fec_enet_private *fep)
fep->ptp_saved_state.ns_phc = timecounter_read(&fep->tc);
fep->ptp_saved_state.ns_sys = ktime_get_ns();
- fep->ptp_saved_state.at_corr = readl(fep->hwp + FEC_ATIME_CORR);
- atime_inc_corr = readl(fep->hwp + FEC_ATIME_INC) & FEC_T_INC_CORR_MASK;
+ fep->ptp_saved_state.at_corr = fec_readl(fep->hwp + FEC_ATIME_CORR);
+ atime_inc_corr = fec_readl(fep->hwp + FEC_ATIME_INC) & FEC_T_INC_CORR_MASK;
fep->ptp_saved_state.at_inc_corr = (u8)(atime_inc_corr >> FEC_T_INC_CORR_OFFSET);
spin_unlock_irqrestore(&fep->tmreg_lock, flags);
@@ -823,7 +823,7 @@ void fec_ptp_save_state(struct fec_enet_private *fep)
/* Restore PTP functionality after a reset */
void fec_ptp_restore_state(struct fec_enet_private *fep)
{
- u32 atime_inc = readl(fep->hwp + FEC_ATIME_INC) & FEC_T_INC_MASK;
+ u32 atime_inc = fec_readl(fep->hwp + FEC_ATIME_INC) & FEC_T_INC_MASK;
unsigned long flags;
u32 counter;
u64 ns;
@@ -833,13 +833,13 @@ void fec_ptp_restore_state(struct fec_enet_private *fep)
/* Reset turned it off, so adjust our status flag */
fep->pps_enable = 0;
- writel(fep->ptp_saved_state.at_corr, fep->hwp + FEC_ATIME_CORR);
+ fec_writel(fep->ptp_saved_state.at_corr, fep->hwp + FEC_ATIME_CORR);
atime_inc |= ((u32)fep->ptp_saved_state.at_inc_corr) << FEC_T_INC_CORR_OFFSET;
- writel(atime_inc, fep->hwp + FEC_ATIME_INC);
+ fec_writel(atime_inc, fep->hwp + FEC_ATIME_INC);
ns = ktime_get_ns() - fep->ptp_saved_state.ns_sys + fep->ptp_saved_state.ns_phc;
counter = ns & fep->cc.mask;
- writel(counter, fep->hwp + FEC_ATIME);
+ fec_writel(counter, fep->hwp + FEC_ATIME);
timecounter_init(&fep->tc, &fep->cc, ns);
spin_unlock_irqrestore(&fep->tmreg_lock, flags);
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCHv3 2/3] net: smc91x: do not use readw()/writew() on ColdFire platforms
2026-09-07 13:37 [PATCHv3 0/3] m68k: coldfire: fix non-standard readX()/writeX() functions Greg Ungerer
2026-09-07 13:37 ` [PATCHv3 1/3] net: fec: do not use readl()/writel() for ColdFire Greg Ungerer
@ 2026-09-07 13:37 ` Greg Ungerer
2026-09-07 13:37 ` [PATCHv3 3/3] m68k: coldfire: fix non-standard readX()/writeX() functions Greg Ungerer
2 siblings, 0 replies; 11+ messages in thread
From: Greg Ungerer @ 2026-09-07 13:37 UTC (permalink / raw)
To: linux-m68k
Cc: linux-kernel, arnd, wei.fang, frank.li, shenwei.wang, imx, netdev,
nico, linux-can, linux-spi, olteanv, Greg Ungerer, andrew+netdev,
davem, edumazet, kuba, pabeni
Modify the access macros and functions used to access the smsc hardware
registers when used on ColdFire SoC platforms so they do not use readw()
or writew(), or derived functions like ioread16be() and iowrite16be().
The current set of readX()/writeX() access methods for ColdFire have
historically been non-standard, in that they mostly access memory
big-endian instead of the expected little-endian. Before fixing the
ColdFire readX() and writeX() supporting code to properly work with
little-endian data existing driver uses need to be fixed. Convert the
smsc driver ColdFire uses of these to use the raw access macros - which
are well defined to be (native) big-endian on ColdFire. This change
requires some byte swapping at time of access to retain existing correct
behavior.
Signed-off-by: Greg Ungerer <gerg@linux-m68k.org>
---
v2: changed from RFC to PATCH
v3: no change
Resending with expanded CC list as per get_maintainers.pl, hoping to get
this picked up.
drivers/net/ethernet/smsc/smc91x.h | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/smsc/smc91x.h b/drivers/net/ethernet/smsc/smc91x.h
index 38aa4374e813..1290335629c1 100644
--- a/drivers/net/ethernet/smsc/smc91x.h
+++ b/drivers/net/ethernet/smsc/smc91x.h
@@ -142,22 +142,26 @@ static inline void _SMC_outw_align4(u16 val, void __iomem *ioaddr, int reg,
#define SMC_CAN_USE_32BIT 0
#define SMC_NOWAIT 1
+/*
+ * Access SMSC device registers using raw IO access primitives. Byte
+ * swap as required for device registers, but not data.
+ */
static inline void mcf_insw(void __iomem *a, unsigned char *p, int l)
{
u16 *wp = (u16 *) p;
while (l-- > 0)
- *wp++ = readw(a);
+ *wp++ = __raw_readw(a);
}
static inline void mcf_outsw(void __iomem *a, unsigned char *p, int l)
{
u16 *wp = (u16 *) p;
while (l-- > 0)
- writew(*wp++, a);
+ __raw_writew(*wp++, a);
}
-#define SMC_inw(a, r) ioread16be((a) + (r))
-#define SMC_outw(lp, v, a, r) iowrite16be(v, (a) + (r))
+#define SMC_inw(a, r) swab16(__raw_readw((a) + (r)))
+#define SMC_outw(lp, v, a, r) __raw_writew(swab16(v), (a) + (r))
#define SMC_insw(a, r, p, l) mcf_insw(a + r, p, l)
#define SMC_outsw(a, r, p, l) mcf_outsw(a + r, p, l)
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCHv3 3/3] m68k: coldfire: fix non-standard readX()/writeX() functions
2026-09-07 13:37 [PATCHv3 0/3] m68k: coldfire: fix non-standard readX()/writeX() functions Greg Ungerer
2026-09-07 13:37 ` [PATCHv3 1/3] net: fec: do not use readl()/writel() for ColdFire Greg Ungerer
2026-09-07 13:37 ` [PATCHv3 2/3] net: smc91x: do not use readw()/writew() on ColdFire platforms Greg Ungerer
@ 2026-09-07 13:37 ` Greg Ungerer
2026-09-08 13:43 ` sashiko-bot
2 siblings, 1 reply; 11+ messages in thread
From: Greg Ungerer @ 2026-09-07 13:37 UTC (permalink / raw)
To: linux-m68k
Cc: linux-kernel, arnd, wei.fang, frank.li, shenwei.wang, imx, netdev,
nico, linux-can, linux-spi, olteanv, Greg Ungerer,
Marc Kleine-Budde, Frank Li
Remove the local ColdFire definitions of readb()/readw()/readl() and
writeb()/writew()/writel() and use the asm-generic versions of them.
The implementation of the readX()/writeX() family of IO access functions
is non-standard on ColdFire platforms. They either return big-endian (that
is native endian) data, or on platforms with PCI bus support check the
supplied address and return either big or little endian data based on that
check. This is non-standard, they are expected to always return
little-endian byte ordered data. Unfortunately this behavior also means
that ioreadX()/iowroteX() and their big-endian counter parts
ioreadXbe()/iowriteXbe() are currently broken because they are implemented
using the readX()/writeX() functions.
The change to use the asm-generic versions of readX()/writeX() itself is
quite straight forward, just remove the ColdFire local versions of them. But
this of course has implications for any remaining drivers that use any of
these IO access functions. A number of drivers can be independently fixed,
before this final fix to readX()/writeX() for ColdFire. A small number of
drivers cannot easily be independently fixed and remain in a working
state. Those drivers are fixed here as well:
drivers/dma/mcf-edma-main.c
Supports big-endian access by setting the big-endian flag of
the drivers struct fsl_edma_engine. But locally should be using
ioread32be() and iowrite32be() instead of ioread32() and iowrite32().
drivers/net/can/flexcan/flexcan-core.c
Setting the driver quirks flag for big-endian access will force
driver to use correct access functions.
drivers/spi/spi-fsl-dspi.c
Setting the regmap format_endian flags to use native endian will
force driver to use appropriate big or little endian access on
whatever platform it is built for.
These drivers have only been compile tested.
Acked-by: Marc Kleine-Budde <mkl@pengutronix.de> # for drivers/net/can/flexcan
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Greg Ungerer <gerg@linux-m68k.org>
---
v2: changed from RFC to PATCH
use separate regmap config for mcf5441x (spi-fsl-dspi.c)
reorder quirks for mcf5441x (flexcan-core.c)
v3: no change
arch/m68k/include/asm/io_no.h | 68 +++-----------------------
drivers/dma/mcf-edma-main.c | 14 +++---
drivers/net/can/flexcan/flexcan-core.c | 1 +
drivers/spi/spi-fsl-dspi.c | 14 +++++-
4 files changed, 27 insertions(+), 70 deletions(-)
diff --git a/arch/m68k/include/asm/io_no.h b/arch/m68k/include/asm/io_no.h
index 4f0f34b06e37..2f12f4ed0da5 100644
--- a/arch/m68k/include/asm/io_no.h
+++ b/arch/m68k/include/asm/io_no.h
@@ -3,15 +3,10 @@
#define _M68KNOMMU_IO_H
/*
- * Convert a physical memory address into a IO memory address.
- * For us this is trivially a type cast.
- */
-#define iomem(a) ((void __iomem *) (a))
-
-/*
- * The non-MMU m68k and ColdFire IO and memory mapped hardware access
- * functions have always worked in CPU native endian. We need to define
- * that behavior here first before we include asm-generic/io.h.
+ * Historically the raw native endian IO access macros for non-MMU m68k and
+ * ColdFire have accepted any integral value as the address argument.
+ * The asm-generic versions of these expect "void __iomem *" for the address,
+ * so define local permissive versions here.
*/
#define __raw_readb(addr) \
({ u8 __v = (*(__force volatile u8 *) (addr)); __v; })
@@ -45,66 +40,15 @@
* applies just the same of there is no MMU but something like a PCI bus
* is present.
*/
-static int __cf_internalio(unsigned long addr)
+static inline int __cf_internalio(unsigned long addr)
{
return (addr >= IOMEMBASE) && (addr <= IOMEMBASE + IOMEMSIZE - 1);
}
-static int cf_internalio(const volatile void __iomem *addr)
+static inline int cf_internalio(const volatile void __iomem *addr)
{
return __cf_internalio((unsigned long) addr);
}
-
-/*
- * We need to treat built-in peripherals and bus based address ranges
- * differently. Local built-in peripherals (and the ColdFire SoC parts
- * have quite a lot of them) are always native endian - which is big
- * endian on m68k/ColdFire. Bus based address ranges, like the PCI bus,
- * are accessed little endian - so we need to byte swap those.
- */
-#define readw readw
-static inline u16 readw(const volatile void __iomem *addr)
-{
- if (cf_internalio(addr))
- return __raw_readw(addr);
- return swab16(__raw_readw(addr));
-}
-
-#define readl readl
-static inline u32 readl(const volatile void __iomem *addr)
-{
- if (cf_internalio(addr))
- return __raw_readl(addr);
- return swab32(__raw_readl(addr));
-}
-
-#define writew writew
-static inline void writew(u16 value, volatile void __iomem *addr)
-{
- if (cf_internalio(addr))
- __raw_writew(value, addr);
- else
- __raw_writew(swab16(value), addr);
-}
-
-#define writel writel
-static inline void writel(u32 value, volatile void __iomem *addr)
-{
- if (cf_internalio(addr))
- __raw_writel(value, addr);
- else
- __raw_writel(swab32(value), addr);
-}
-
-#else
-
-#define readb __raw_readb
-#define readw __raw_readw
-#define readl __raw_readl
-#define writeb __raw_writeb
-#define writew __raw_writew
-#define writel __raw_writel
-
#endif /* IOMEMBASE */
#if defined(CONFIG_COLDFIRE)
diff --git a/drivers/dma/mcf-edma-main.c b/drivers/dma/mcf-edma-main.c
index 9e1c6400c77b..4ed0ce644e37 100644
--- a/drivers/dma/mcf-edma-main.c
+++ b/drivers/dma/mcf-edma-main.c
@@ -21,9 +21,9 @@ static irqreturn_t mcf_edma_tx_handler(int irq, void *dev_id)
unsigned int ch;
u64 intmap;
- intmap = ioread32(regs->inth);
+ intmap = ioread32be(regs->inth);
intmap <<= 32;
- intmap |= ioread32(regs->intl);
+ intmap |= ioread32be(regs->intl);
if (!intmap)
return IRQ_NONE;
@@ -43,7 +43,7 @@ static irqreturn_t mcf_edma_err_handler(int irq, void *dev_id)
struct edma_regs *regs = &mcf_edma->regs;
unsigned int err, ch;
- err = ioread32(regs->errl);
+ err = ioread32be(regs->errl);
if (!err)
return IRQ_NONE;
@@ -55,7 +55,7 @@ static irqreturn_t mcf_edma_err_handler(int irq, void *dev_id)
}
}
- err = ioread32(regs->errh);
+ err = ioread32be(regs->errh);
if (!err)
return IRQ_NONE;
@@ -203,8 +203,8 @@ static int mcf_edma_probe(struct platform_device *pdev)
edma_write_tcdreg(mcf_chan, cpu_to_le32(0), csr);
}
- iowrite32(~0, regs->inth);
- iowrite32(~0, regs->intl);
+ iowrite32be(~0, regs->inth);
+ iowrite32be(~0, regs->intl);
ret = mcf_edma->drvdata->setup_irq(pdev, mcf_edma);
if (ret)
@@ -248,7 +248,7 @@ static int mcf_edma_probe(struct platform_device *pdev)
}
/* Enable round robin arbitration */
- iowrite32(EDMA_CR_ERGA | EDMA_CR_ERCA, regs->cr);
+ iowrite32be(EDMA_CR_ERGA | EDMA_CR_ERCA, regs->cr);
return 0;
}
diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
index f5d22c61503f..f0571b97817b 100644
--- a/drivers/net/can/flexcan/flexcan-core.c
+++ b/drivers/net/can/flexcan/flexcan-core.c
@@ -295,6 +295,7 @@ static_assert(sizeof(struct flexcan_regs) == 0x4 * 18 + 0xfb8);
static const struct flexcan_devtype_data fsl_mcf5441x_devtype_data = {
.quirks = FLEXCAN_QUIRK_BROKEN_PERR_STATE |
+ FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN |
FLEXCAN_QUIRK_NR_IRQ_3 | FLEXCAN_QUIRK_NR_MB_16 |
FLEXCAN_QUIRK_SUPPORT_RX_MAILBOX |
FLEXCAN_QUIRK_SUPPORT_RX_FIFO,
diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 23e6bc5f202e..5fb5f5dd1b0d 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -192,6 +192,7 @@ enum {
DSPI_XSPI_REGMAP,
S32G_DSPI_XSPI_REGMAP,
DSPI_PUSHR,
+ MCF5441X_DSPI_REGMAP,
};
static const struct regmap_config dspi_regmap_config[] = {
@@ -238,6 +239,17 @@ static const struct regmap_config dspi_regmap_config[] = {
.reg_stride = 2,
.max_register = 0x2,
},
+ [MCF5441X_DSPI_REGMAP] = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .max_register = SPI_RXFR3,
+ .volatile_table = &dspi_volatile_table,
+ .rd_table = &dspi_access_table,
+ .wr_table = &dspi_access_table,
+ .reg_format_endian = REGMAP_ENDIAN_BIG,
+ .val_format_endian = REGMAP_ENDIAN_BIG,
+ },
};
static const struct fsl_dspi_devtype_data devtype_data[] = {
@@ -303,7 +315,7 @@ static const struct fsl_dspi_devtype_data devtype_data[] = {
.trans_mode = DSPI_DMA_MODE,
.max_clock_factor = 8,
.fifo_size = 16,
- .regmap = &dspi_regmap_config[DSPI_REGMAP],
+ .regmap = &dspi_regmap_config[MCF5441X_DSPI_REGMAP],
},
[S32G] = {
.trans_mode = DSPI_XSPI_MODE,
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCHv3 1/3] net: fec: do not use readl()/writel() for ColdFire
2026-09-07 13:37 ` [PATCHv3 1/3] net: fec: do not use readl()/writel() for ColdFire Greg Ungerer
@ 2026-09-08 13:43 ` sashiko-bot
2026-09-11 13:09 ` Greg Ungerer
2026-09-11 0:42 ` Jakub Kicinski
1 sibling, 1 reply; 11+ messages in thread
From: sashiko-bot @ 2026-09-08 13:43 UTC (permalink / raw)
To: Greg Ungerer
Cc: Oleksij Rempel, Frank.Li, Vincent Mailhol, imx, linux-can,
Oliver Hartkopp, Marc Kleine-Budde
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Heap buffer overflow in TX bounce buffer due to unvalidated packet and fragment lengths.
- [High] The hardware PPS interrupt handler accesses and modifies shared PTP state without acquiring the required lock, racing with configuration paths on other CPUs.
- [High] Unbounded hardware polling loops can cause a hard lockup if the hardware enters an error state.
- [High] Integer underflow in RX packet length calculation leads to out-of-bounds memory access.
- [High] Out-of-bounds read and write in `fec_enet_clear_csum` due to missing `pskb_may_pull`.
- [High] Missing dma_rmb() after reading descriptor status in RX paths.
--
commit b19b9a07ca8eff68179fb339b69d5c4690fdf442
Author: Greg Ungerer <gerg@linux-m68k.org>
net: fec: do not use readl()/writel() for ColdFire
This patch modifies the FEC driver to use local fec_readl() and fec_writel()
methods instead of direct hardware register accesses. This avoids incorrect
byte swapping on big-endian architectures like ColdFire.
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
[ ... ]
> @@ -1923,7 +1924,7 @@ static int fec_enet_rx_queue(struct fec_enet_private *fep,
> break;
> pkt_received++;
>
> - writel(FEC_ENET_RXF_GET(queue), fep->hwp + FEC_IEVENT);
> + fec_writel(FEC_ENET_RXF_GET(queue), fep->hwp + FEC_IEVENT);
[Severity: High]
This is a pre-existing issue, but is a memory barrier missing in the RX
loop?
The loop checks the status to confirm ownership in the while condition, but
without a dma_rmb() immediately after, couldn't speculative execution on
architectures with weak memory ordering cause the subsequent read of
cbd_datlen to happen before the DMA transfer is completely visible?
>
> /* Check for errors. */
> status ^= BD_ENET_RX_LAST;
[ ... ]
> diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
> --- a/drivers/net/ethernet/freescale/fec_ptp.c
> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
[ ... ]
> @@ -142,17 +142,17 @@ static int fec_ptp_enable_pps(struct fec_enet_private *fep, uint enable)
> if (enable) {
> /* clear capture or output compare interrupt status if have.
> */
> - writel(FEC_T_TF_MASK, fep->hwp + FEC_TCSR(fep->pps_channel));
> + fec_writel(FEC_T_TF_MASK, fep->hwp + FEC_TCSR(fep->pps_channel));
>
> /* It is recommended to double check the TMODE field in the
> * TCSR register to be cleared before the first compare counter
> * is written into TCCR register. Just add a double check.
> */
> - val = readl(fep->hwp + FEC_TCSR(fep->pps_channel));
> + val = fec_readl(fep->hwp + FEC_TCSR(fep->pps_channel));
> do {
> val &= ~(FEC_T_TMODE_MASK);
> - writel(val, fep->hwp + FEC_TCSR(fep->pps_channel));
> - val = readl(fep->hwp + FEC_TCSR(fep->pps_channel));
> + fec_writel(val, fep->hwp + FEC_TCSR(fep->pps_channel));
> + val = fec_readl(fep->hwp + FEC_TCSR(fep->pps_channel));
> } while (val & FEC_T_TMODE_MASK);
[Severity: High]
This is a pre-existing issue, but is it safe to have an unbounded polling
loop here while IRQs are disabled?
If the hardware transitions into a faulty state and perpetually returns a
value with FEC_T_TMODE_MASK set, could this loop cause a hard lockup or
kernel panic?
>
> /* Dummy read counter to update the counter */
[ ... ]
> @@ -194,31 +194,31 @@ static int fec_ptp_enable_pps(struct fec_enet_private *fep, uint enable)
> * is bigger than fep->cc.mask would be a error.
> */
> val &= fep->cc.mask;
> - writel(val, fep->hwp + FEC_TCCR(fep->pps_channel));
> + fec_writel(val, fep->hwp + FEC_TCCR(fep->pps_channel));
>
> /* Calculate the second the compare event timestamp */
> fep->next_counter = (val + fep->reload_period) & fep->cc.mask;
[Severity: High]
This is a pre-existing issue, but I noticed a potential data race between
this configuration path and the interrupt handler.
Here, fep->next_counter is updated while holding fep->tmreg_lock. However,
the interrupt handler fec_pps_interrupt() modifies fep->next_counter
locklessly. Could this cause the PTP timer to miss events or corrupt the
hardware state if this is triggered concurrently from userspace?
>
> /* * Enable compare event when overflow */
> - val = readl(fep->hwp + FEC_ATIME_CTRL);
[ ... ]
> @@ -701,15 +701,15 @@ static irqreturn_t fec_pps_interrupt(int irq, void *dev_id)
> u8 channel = fep->pps_channel;
> struct ptp_clock_event event;
>
> - val = readl(fep->hwp + FEC_TCSR(channel));
> + val = fec_readl(fep->hwp + FEC_TCSR(channel));
> if (val & FEC_T_TF_MASK) {
> /* Write the next next compare(not the next according the spec)
> * value to the register
> */
> - writel(fep->next_counter, fep->hwp + FEC_TCCR(channel));
> + fec_writel(fep->next_counter, fep->hwp + FEC_TCCR(channel));
> do {
> - writel(val, fep->hwp + FEC_TCSR(channel));
> - } while (readl(fep->hwp + FEC_TCSR(channel)) & FEC_T_TF_MASK);
> + fec_writel(val, fep->hwp + FEC_TCSR(channel));
> + } while (fec_readl(fep->hwp + FEC_TCSR(channel)) & FEC_T_TF_MASK);
[Severity: High]
This is a pre-existing issue, but similar to the loop in
fec_ptp_enable_pps(), does this unbounded loop risk a hard lockup?
Since this runs in hardirq context, a faulty hardware state that fails to
clear FEC_T_TF_MASK would keep the CPU spinning indefinitely.
[ ... ]
I also noticed a few other pre-existing issues in this driver that fall
outside the immediate diff context of this patch:
[Severity: Critical]
This is a pre-existing issue, but does fec_enet_txq_submit_frag_skb()
overflow the TX bounce buffer?
index = fec_enet_get_bd_index(bdp, &txq->bd);
if (((unsigned long) bufaddr) & fep->tx_align ||
fep->quirks & FEC_QUIRK_SWAP_FRAME) {
memcpy(txq->tx_bounce[index], bufaddr, frag_len);
bufaddr = txq->tx_bounce[index];
Since fragments can be up to PAGE_SIZE (4KB or 64KB), but the tx_bounce
buffers are statically sized to 2048 bytes, couldn't a large unaligned
packet or jumbo frame cause an out-of-bounds heap write?
[Severity: High]
This is a pre-existing issue, but I have a question about the packet length
calculation in fec_enet_rx_queue_xdp() and fec_enet_rx_queue():
pkt_len = fec16_to_cpu(bdp->cbd_datlen);
...
pkt_len -= sub_len;
xdp_prepare_buff(&xdp, page_address(page), data_start, pkt_len, false);
If a corrupted or maliciously crafted DMA descriptor reports a pkt_len less
than sub_len, pkt_len will underflow. Won't this pass a very large length
to xdp_prepare_buff() (or skb_put() in fec_build_skb()), potentially
allowing a BPF program to read/write out of bounds or causing an
skb_over_panic()?
[Severity: High]
This is a pre-existing issue, but I noticed a potential out-of-bounds
access on non-linear SKBs in fec_enet_clear_csum():
static bool is_ipv4_pkt(struct sk_buff *skb)
{
return skb->protocol == htons(ETH_P_IP) && ip_hdr(skb)->version == 4;
}
If an AF_PACKET socket transmits a packet where the IP header is in a
non-linear fragment, wouldn't ip_hdr(skb)->version and the subsequent
ip_hdr(skb)->check = 0 assignment read and write past the linear buffer
into unallocated memory, since pskb_may_pull() hasn't been called?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260907134037.1855408-2-gerg@linux-m68k.org?part=1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCHv3 3/3] m68k: coldfire: fix non-standard readX()/writeX() functions
2026-09-07 13:37 ` [PATCHv3 3/3] m68k: coldfire: fix non-standard readX()/writeX() functions Greg Ungerer
@ 2026-09-08 13:43 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-09-08 13:43 UTC (permalink / raw)
To: Greg Ungerer
Cc: Oliver Hartkopp, Marc Kleine-Budde, Frank.Li, imx, linux-can,
Oleksij Rempel, Vincent Mailhol
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Replacing ColdFire's non-standard I/O accessors breaks eDMA descriptor loading by exposing a hidden endianness bug in `fsl-edma-common.h`, writing little-endian data to big-endian hardware.
Pre-existing issues:
- [High] The eDMA error interrupt handler silently ignores upper channel errors if no lower channel errors are present, and returns IRQ_NONE after successfully handling lower channel errors if no upper channel errors are present.
- [High] Using 32-bit `BIT()` macro with a shift count up to 63 causes undefined behavior, breaking interrupt handling for channels 32-63.
--
commit 90504ddd291ad9040d7cc40def38400a2da51323
Author: Greg Ungerer <gerg@linux-m68k.org>
m68k: coldfire: fix non-standard readX()/writeX() functions
This patch removes the local ColdFire definitions of read/write I/O
access functions and uses the asm-generic versions instead. It also
updates several drivers that previously relied on the non-standard
behavior to use the correct endian-specific accessors or quirks.
[ ... ]
> diff --git a/drivers/dma/mcf-edma-main.c b/drivers/dma/mcf-edma-main.c
> index 9e1c6400c77be..4ed0ce644e37e 100644
> --- a/drivers/dma/mcf-edma-main.c
> +++ b/drivers/dma/mcf-edma-main.c
> @@ -21,9 +21,9 @@ static irqreturn_t mcf_edma_tx_handler(int irq, void *dev_id)
> unsigned int ch;
> u64 intmap;
>
> - intmap = ioread32(regs->inth);
> + intmap = ioread32be(regs->inth);
> intmap <<= 32;
> - intmap |= ioread32(regs->intl);
> + intmap |= ioread32be(regs->intl);
> if (!intmap)
> return IRQ_NONE;
[Severity: High]
This is a pre-existing issue, but does using the 32-bit BIT() macro just
below this block cause undefined behavior for channels 32-63?
In mcf_edma_tx_handler():
for (ch = 0; ch < mcf_edma->n_chans; ch++) {
if (intmap & BIT(ch)) {
iowrite8(EDMA_MASK_CH(ch), regs->cint);
On 32-bit platforms, BIT(ch) evaluates modulo 32. When checking completions
for upper channels (e.g., ch=32), BIT(32) will evaluate to bit 0 instead
of bit 32.
If the wrong bit is checked, the interrupt for channels 32-63 won't be
cleared, leading to an interrupt storm. Should this use BIT_ULL(ch)
since intmap is a u64?
> @@ -43,7 +43,7 @@ static irqreturn_t mcf_edma_err_handler(int irq, void *dev_id)
> struct edma_regs *regs = &mcf_edma->regs;
> unsigned int err, ch;
>
> - err = ioread32(regs->errl);
> + err = ioread32be(regs->errl);
> if (!err)
> return IRQ_NONE;
[Severity: High]
This isn't a bug introduced by this patch, but does this error handler
incorrectly short-circuit if lower channels are error-free?
If a DMA error occurs on channels 32-63 (regs->errh), but not on 0-31
(regs->errl), the handler will hit this first if (!err) check and return
IRQ_NONE.
If it returns IRQ_NONE, the error is never cleared, leaving the IRQ
line asserted. Will this result in an interrupt storm that hangs the
CPU until the kernel permanently disables the IRQ?
[ ... ]
> @@ -203,8 +203,8 @@ static int mcf_edma_probe(struct platform_device *pdev)
> edma_write_tcdreg(mcf_chan, cpu_to_le32(0), csr);
> }
>
> - iowrite32(~0, regs->inth);
> - iowrite32(~0, regs->intl);
> + iowrite32be(~0, regs->inth);
> + iowrite32be(~0, regs->intl);
[Severity: High]
Does removing the non-standard read/write behaviors expose a hidden
endianness bug in how fsl-edma-common.h programs the eDMA descriptors?
The mcf-edma probe function writes TCD registers via edma_write_tcdreg()
in this hunk:
edma_write_tcdreg(mcf_chan, cpu_to_le32(0), csr);
Under the hood in fsl-edma-common.h, edma_write_tcdreg_c() casts this
little-endian value directly to a native u32 before passing it to
edma_writel() (which maps to iowrite32be on ColdFire).
Previously, the non-standard ColdFire I/O macros lacked byte-swapping,
so the driver's use of iowrite32be() implicitly double-swapped the
incorrectly cast u32 value back to a native Big-Endian value.
Now that the generic writel() corrects this and byte-swaps properly, will
the eDMA controller read garbled (byte-swapped) descriptors and corrupt
memory when DMA transfers are submitted?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260907134037.1855408-4-gerg@linux-m68k.org?part=1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCHv3 1/3] net: fec: do not use readl()/writel() for ColdFire
2026-09-07 13:37 ` [PATCHv3 1/3] net: fec: do not use readl()/writel() for ColdFire Greg Ungerer
2026-09-08 13:43 ` sashiko-bot
@ 2026-09-11 0:42 ` Jakub Kicinski
2026-09-11 7:39 ` Arnd Bergmann
2026-09-11 13:03 ` Greg Ungerer
1 sibling, 2 replies; 11+ messages in thread
From: Jakub Kicinski @ 2026-09-11 0:42 UTC (permalink / raw)
To: Greg Ungerer
Cc: linux-m68k, linux-kernel, arnd, wei.fang, frank.li, shenwei.wang,
imx, netdev, nico, linux-can, linux-spi, olteanv, andrew+netdev,
davem, edumazet, pabeni, Andrew Lunn
On Mon, 7 Sep 2026 23:37:09 +1000 Greg Ungerer wrote:
> The FEC driver works today because the m68k architecture io.h has a
> kludge in the definitions of the readl() and writel() functions for
> ColdFire that allow big-endian access if the address of the register to
> access is within the SoC's internal peripheral registers. This is being
> fixed in the near future to define readl() and writel() correctly - with
> no byte swapping. Thus the motivation for this fix here.
What is the motivation for this cleanup? Is someone still making
new SKUs of Coldfire boards? Or (and please don't take this the wrong
way) it was a long standing TODO that was tempting to feed to an LLM?
IMO keeping the hacks in m68k is a better choice. FEC was used on more
modern SoCs, definitely on PPC ones. So we'll be able to get rid of
m68k before we can get rid of FEC. Sprinkling m68k workarounds in
the FEC driver is backwards.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCHv3 1/3] net: fec: do not use readl()/writel() for ColdFire
2026-09-11 0:42 ` Jakub Kicinski
@ 2026-09-11 7:39 ` Arnd Bergmann
2026-09-11 13:03 ` Greg Ungerer
1 sibling, 0 replies; 11+ messages in thread
From: Arnd Bergmann @ 2026-09-11 7:39 UTC (permalink / raw)
To: Jakub Kicinski, Greg Ungerer
Cc: linux-m68k, linux-kernel, Wei Fang, Frank Li, Shenwei Wang, imx,
Netdev, Nicolas Pitre, linux-can, linux-spi, Vladimir Oltean,
andrew+netdev, David S . Miller, Eric Dumazet, Paolo Abeni,
Andrew Lunn
On Fri, Sep 11, 2026, at 02:42, Jakub Kicinski wrote:
> On Mon, 7 Sep 2026 23:37:09 +1000 Greg Ungerer wrote:
>> The FEC driver works today because the m68k architecture io.h has a
>> kludge in the definitions of the readl() and writel() functions for
>> ColdFire that allow big-endian access if the address of the register to
>> access is within the SoC's internal peripheral registers. This is being
>> fixed in the near future to define readl() and writel() correctly - with
>> no byte swapping. Thus the motivation for this fix here.
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> What is the motivation for this cleanup? Is someone still making
> new SKUs of Coldfire boards? Or (and please don't take this the wrong
> way) it was a long standing TODO that was tempting to feed to an LLM?
>
> IMO keeping the hacks in m68k is a better choice. FEC was used on more
> modern SoCs, definitely on PPC ones. So we'll be able to get rid of
> m68k before we can get rid of FEC. Sprinkling m68k workarounds in
> the FEC driver is backwards.
Coldfire at the moment is weird because it defines the readl()/writel()
helpers the opposite way from everything else. Greg has been working
towards fixing this so we can avoid special hacks for it in other
parts of the kernel and share the code with Arm and other SoCs using
the same peripherals.
fec is an exception to this because it happened to rely on the
unusual macros: all the Arm SoCs using this hardware have little-endian
registers (regardless of whether the CPU runs as BE or LE), while
all the coldfire chips use big-endian registers (and don't support
LE kernels). The powerpc variant of fec also uses big-endian registers
but has a separate copy of the driver that hardcodes this. Other
Freescale drivers already have a runtime endianess detection that
is needed because they built both BE and LE variants of the hardware
on Arm SoCs that could run the same kernel.
I very much hope we can merge this bit.
Arnd
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCHv3 1/3] net: fec: do not use readl()/writel() for ColdFire
2026-09-11 0:42 ` Jakub Kicinski
2026-09-11 7:39 ` Arnd Bergmann
@ 2026-09-11 13:03 ` Greg Ungerer
2026-09-11 23:04 ` Jakub Kicinski
1 sibling, 1 reply; 11+ messages in thread
From: Greg Ungerer @ 2026-09-11 13:03 UTC (permalink / raw)
To: Jakub Kicinski
Cc: linux-m68k, linux-kernel, arnd, wei.fang, frank.li, shenwei.wang,
imx, netdev, nico, linux-can, linux-spi, olteanv, andrew+netdev,
davem, edumazet, pabeni, Andrew Lunn
Hi Jakub,
On 11/9/26 10:42, Jakub Kicinski wrote:
> On Mon, 7 Sep 2026 23:37:09 +1000 Greg Ungerer wrote:
>> The FEC driver works today because the m68k architecture io.h has a
>> kludge in the definitions of the readl() and writel() functions for
>> ColdFire that allow big-endian access if the address of the register to
>> access is within the SoC's internal peripheral registers. This is being
>> fixed in the near future to define readl() and writel() correctly - with
>> no byte swapping. Thus the motivation for this fix here.
>
> What is the motivation for this cleanup?
Sorry, maybe you were not in the CC list of the cover email. From that:
This collection of patches is aimed at fixing the non-standard ColdFire
set of readX()/writeX() IO access functions. Instead switching to using the
asm-generic definitions in include/asm-generic/io.h. The difficulty comes
in trying not to break any drivers with this change.
The implementation of the readX()/writeX() family of IO access functions
is non-standard on ColdFire platforms. They either return big-endian (that
is native endian) data, or on platforms with PCI bus support check the
supplied address and return either big or little endian data based on that
check. This is non-standard, they are expected to always return
little-endian byte ordered data. Unfortunately this behavior also means
that ioreadX()/iowroteX() and their big-endian counter parts
ioreadXbe()/iowriteXbe() are currently broken because they are implemented
using the readX()/writeX() functions.
Patches 1 and 2 in this series are specific net driver changes that can be
applied independently of the final ColdFire readX()/writeX() change.
> Is someone still making
> new SKUs of Coldfire boards? Or (and please don't take this the wrong
> way) it was a long standing TODO that was tempting to feed to an LLM?
It is a long standing todo/fix, but there is nothing LLM anywhere here.
This was all manually coded and tested. This patch is 95% textual
substitution, so it wasn't exactly very difficult.
> IMO keeping the hacks in m68k is a better choice. FEC was used on more
> modern SoCs, definitely on PPC ones. So we'll be able to get rid of
> m68k before we can get rid of FEC. Sprinkling m68k workarounds in
> the FEC driver is backwards.
These changes are not work arounds, they are made to make the FEC driver
explicitly aware of endianess of the underlying processor and FEC hardware
block. The code as it is now, that is carrying hacks inside readX and writeX
for m68k, is trying to hide that from the driver.
Regards
Greg
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCHv3 1/3] net: fec: do not use readl()/writel() for ColdFire
2026-09-08 13:43 ` sashiko-bot
@ 2026-09-11 13:09 ` Greg Ungerer
0 siblings, 0 replies; 11+ messages in thread
From: Greg Ungerer @ 2026-09-11 13:09 UTC (permalink / raw)
To: sashiko-reviews
Cc: Oleksij Rempel, Frank.Li, Vincent Mailhol, imx, linux-can,
Oliver Hartkopp, Marc Kleine-Budde
On 8/9/26 23:43, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] Heap buffer overflow in TX bounce buffer due to unvalidated packet and fragment lengths.
> - [High] The hardware PPS interrupt handler accesses and modifies shared PTP state without acquiring the required lock, racing with configuration paths on other CPUs.
> - [High] Unbounded hardware polling loops can cause a hard lockup if the hardware enters an error state.
> - [High] Integer underflow in RX packet length calculation leads to out-of-bounds memory access.
> - [High] Out-of-bounds read and write in `fec_enet_clear_csum` due to missing `pskb_may_pull`.
> - [High] Missing dma_rmb() after reading descriptor status in RX paths.
Given these are pre-existing issues and not in any way related to what
this patch is doing I am not going to make any changes to it based on this.
Certainly a heads up for the drievr maintainers that there is some issues
worth looking into though.
Regards
Greg
>
> commit b19b9a07ca8eff68179fb339b69d5c4690fdf442
> Author: Greg Ungerer <gerg@linux-m68k.org>
> net: fec: do not use readl()/writel() for ColdFire
>
> This patch modifies the FEC driver to use local fec_readl() and fec_writel()
> methods instead of direct hardware register accesses. This avoids incorrect
> byte swapping on big-endian architectures like ColdFire.
>
>> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
>> --- a/drivers/net/ethernet/freescale/fec_main.c
>> +++ b/drivers/net/ethernet/freescale/fec_main.c
> [ ... ]
>> @@ -1923,7 +1924,7 @@ static int fec_enet_rx_queue(struct fec_enet_private *fep,
>> break;
>> pkt_received++;
>>
>> - writel(FEC_ENET_RXF_GET(queue), fep->hwp + FEC_IEVENT);
>> + fec_writel(FEC_ENET_RXF_GET(queue), fep->hwp + FEC_IEVENT);
>
> [Severity: High]
> This is a pre-existing issue, but is a memory barrier missing in the RX
> loop?
>
> The loop checks the status to confirm ownership in the while condition, but
> without a dma_rmb() immediately after, couldn't speculative execution on
> architectures with weak memory ordering cause the subsequent read of
> cbd_datlen to happen before the DMA transfer is completely visible?
>
>>
>> /* Check for errors. */
>> status ^= BD_ENET_RX_LAST;
>
> [ ... ]
>
>> diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
>> --- a/drivers/net/ethernet/freescale/fec_ptp.c
>> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
> [ ... ]
>> @@ -142,17 +142,17 @@ static int fec_ptp_enable_pps(struct fec_enet_private *fep, uint enable)
>> if (enable) {
>> /* clear capture or output compare interrupt status if have.
>> */
>> - writel(FEC_T_TF_MASK, fep->hwp + FEC_TCSR(fep->pps_channel));
>> + fec_writel(FEC_T_TF_MASK, fep->hwp + FEC_TCSR(fep->pps_channel));
>>
>> /* It is recommended to double check the TMODE field in the
>> * TCSR register to be cleared before the first compare counter
>> * is written into TCCR register. Just add a double check.
>> */
>> - val = readl(fep->hwp + FEC_TCSR(fep->pps_channel));
>> + val = fec_readl(fep->hwp + FEC_TCSR(fep->pps_channel));
>> do {
>> val &= ~(FEC_T_TMODE_MASK);
>> - writel(val, fep->hwp + FEC_TCSR(fep->pps_channel));
>> - val = readl(fep->hwp + FEC_TCSR(fep->pps_channel));
>> + fec_writel(val, fep->hwp + FEC_TCSR(fep->pps_channel));
>> + val = fec_readl(fep->hwp + FEC_TCSR(fep->pps_channel));
>> } while (val & FEC_T_TMODE_MASK);
>
> [Severity: High]
> This is a pre-existing issue, but is it safe to have an unbounded polling
> loop here while IRQs are disabled?
>
> If the hardware transitions into a faulty state and perpetually returns a
> value with FEC_T_TMODE_MASK set, could this loop cause a hard lockup or
> kernel panic?
>
>>
>> /* Dummy read counter to update the counter */
> [ ... ]
>> @@ -194,31 +194,31 @@ static int fec_ptp_enable_pps(struct fec_enet_private *fep, uint enable)
>> * is bigger than fep->cc.mask would be a error.
>> */
>> val &= fep->cc.mask;
>> - writel(val, fep->hwp + FEC_TCCR(fep->pps_channel));
>> + fec_writel(val, fep->hwp + FEC_TCCR(fep->pps_channel));
>>
>> /* Calculate the second the compare event timestamp */
>> fep->next_counter = (val + fep->reload_period) & fep->cc.mask;
>
> [Severity: High]
> This is a pre-existing issue, but I noticed a potential data race between
> this configuration path and the interrupt handler.
>
> Here, fep->next_counter is updated while holding fep->tmreg_lock. However,
> the interrupt handler fec_pps_interrupt() modifies fep->next_counter
> locklessly. Could this cause the PTP timer to miss events or corrupt the
> hardware state if this is triggered concurrently from userspace?
>
>>
>> /* * Enable compare event when overflow */
>> - val = readl(fep->hwp + FEC_ATIME_CTRL);
> [ ... ]
>> @@ -701,15 +701,15 @@ static irqreturn_t fec_pps_interrupt(int irq, void *dev_id)
>> u8 channel = fep->pps_channel;
>> struct ptp_clock_event event;
>>
>> - val = readl(fep->hwp + FEC_TCSR(channel));
>> + val = fec_readl(fep->hwp + FEC_TCSR(channel));
>> if (val & FEC_T_TF_MASK) {
>> /* Write the next next compare(not the next according the spec)
>> * value to the register
>> */
>> - writel(fep->next_counter, fep->hwp + FEC_TCCR(channel));
>> + fec_writel(fep->next_counter, fep->hwp + FEC_TCCR(channel));
>> do {
>> - writel(val, fep->hwp + FEC_TCSR(channel));
>> - } while (readl(fep->hwp + FEC_TCSR(channel)) & FEC_T_TF_MASK);
>> + fec_writel(val, fep->hwp + FEC_TCSR(channel));
>> + } while (fec_readl(fep->hwp + FEC_TCSR(channel)) & FEC_T_TF_MASK);
>
> [Severity: High]
> This is a pre-existing issue, but similar to the loop in
> fec_ptp_enable_pps(), does this unbounded loop risk a hard lockup?
>
> Since this runs in hardirq context, a faulty hardware state that fails to
> clear FEC_T_TF_MASK would keep the CPU spinning indefinitely.
>
> [ ... ]
>
> I also noticed a few other pre-existing issues in this driver that fall
> outside the immediate diff context of this patch:
>
> [Severity: Critical]
> This is a pre-existing issue, but does fec_enet_txq_submit_frag_skb()
> overflow the TX bounce buffer?
>
> index = fec_enet_get_bd_index(bdp, &txq->bd);
> if (((unsigned long) bufaddr) & fep->tx_align ||
> fep->quirks & FEC_QUIRK_SWAP_FRAME) {
> memcpy(txq->tx_bounce[index], bufaddr, frag_len);
> bufaddr = txq->tx_bounce[index];
>
> Since fragments can be up to PAGE_SIZE (4KB or 64KB), but the tx_bounce
> buffers are statically sized to 2048 bytes, couldn't a large unaligned
> packet or jumbo frame cause an out-of-bounds heap write?
>
> [Severity: High]
> This is a pre-existing issue, but I have a question about the packet length
> calculation in fec_enet_rx_queue_xdp() and fec_enet_rx_queue():
>
> pkt_len = fec16_to_cpu(bdp->cbd_datlen);
> ...
> pkt_len -= sub_len;
> xdp_prepare_buff(&xdp, page_address(page), data_start, pkt_len, false);
>
> If a corrupted or maliciously crafted DMA descriptor reports a pkt_len less
> than sub_len, pkt_len will underflow. Won't this pass a very large length
> to xdp_prepare_buff() (or skb_put() in fec_build_skb()), potentially
> allowing a BPF program to read/write out of bounds or causing an
> skb_over_panic()?
>
> [Severity: High]
> This is a pre-existing issue, but I noticed a potential out-of-bounds
> access on non-linear SKBs in fec_enet_clear_csum():
>
> static bool is_ipv4_pkt(struct sk_buff *skb)
> {
> return skb->protocol == htons(ETH_P_IP) && ip_hdr(skb)->version == 4;
> }
>
> If an AF_PACKET socket transmits a packet where the IP header is in a
> non-linear fragment, wouldn't ip_hdr(skb)->version and the subsequent
> ip_hdr(skb)->check = 0 assignment read and write past the linear buffer
> into unallocated memory, since pskb_may_pull() hasn't been called?
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCHv3 1/3] net: fec: do not use readl()/writel() for ColdFire
2026-09-11 13:03 ` Greg Ungerer
@ 2026-09-11 23:04 ` Jakub Kicinski
0 siblings, 0 replies; 11+ messages in thread
From: Jakub Kicinski @ 2026-09-11 23:04 UTC (permalink / raw)
To: Greg Ungerer
Cc: linux-m68k, linux-kernel, arnd, wei.fang, frank.li, shenwei.wang,
imx, netdev, nico, linux-can, linux-spi, olteanv, andrew+netdev,
davem, edumazet, pabeni, Andrew Lunn
On Fri, 11 Sep 2026 23:03:56 +1000 Greg Ungerer wrote:
> On 11/9/26 10:42, Jakub Kicinski wrote:
> > On Mon, 7 Sep 2026 23:37:09 +1000 Greg Ungerer wrote:
> >> The FEC driver works today because the m68k architecture io.h has a
> >> kludge in the definitions of the readl() and writel() functions for
> >> ColdFire that allow big-endian access if the address of the register to
> >> access is within the SoC's internal peripheral registers. This is being
> >> fixed in the near future to define readl() and writel() correctly - with
> >> no byte swapping. Thus the motivation for this fix here.
> >
> > What is the motivation for this cleanup?
>
> Sorry, maybe you were not in the CC list of the cover email. From that:
>
> This collection of patches is aimed at fixing the non-standard ColdFire
> set of readX()/writeX() IO access functions. Instead switching to using the
> asm-generic definitions in include/asm-generic/io.h. The difficulty comes
> in trying not to break any drivers with this change.
>
> The implementation of the readX()/writeX() family of IO access functions
> is non-standard on ColdFire platforms. They either return big-endian (that
> is native endian) data, or on platforms with PCI bus support check the
> supplied address and return either big or little endian data based on that
> check. This is non-standard, they are expected to always return
> little-endian byte ordered data. Unfortunately this behavior also means
> that ioreadX()/iowroteX() and their big-endian counter parts
> ioreadXbe()/iowriteXbe() are currently broken because they are implemented
> using the readX()/writeX() functions.
>
> Patches 1 and 2 in this series are specific net driver changes that can be
> applied independently of the final ColdFire readX()/writeX() change.
I read it, it doesn't really answer the question of priority.
> > Is someone still making
> > new SKUs of Coldfire boards? Or (and please don't take this the wrong
> > way) it was a long standing TODO that was tempting to feed to an LLM?
>
> It is a long standing todo/fix, but there is nothing LLM anywhere here.
> This was all manually coded and tested. This patch is 95% textual
> substitution, so it wasn't exactly very difficult.
>
> > IMO keeping the hacks in m68k is a better choice. FEC was used on more
> > modern SoCs, definitely on PPC ones. So we'll be able to get rid of
> > m68k before we can get rid of FEC. Sprinkling m68k workarounds in
> > the FEC driver is backwards.
>
> These changes are not work arounds, they are made to make the FEC driver
> explicitly aware of endianess of the underlying processor and FEC hardware
> block. The code as it is now, that is carrying hacks inside readX and writeX
> for m68k, is trying to hide that from the driver.
Whatever, let me not waste any more time on this.
If Arnd thinks its worth it:
Acked-by: Jakub Kicinski <kuba@kernel.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-09-11 23:04 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-07 13:37 [PATCHv3 0/3] m68k: coldfire: fix non-standard readX()/writeX() functions Greg Ungerer
2026-09-07 13:37 ` [PATCHv3 1/3] net: fec: do not use readl()/writel() for ColdFire Greg Ungerer
2026-09-08 13:43 ` sashiko-bot
2026-09-11 13:09 ` Greg Ungerer
2026-09-11 0:42 ` Jakub Kicinski
2026-09-11 7:39 ` Arnd Bergmann
2026-09-11 13:03 ` Greg Ungerer
2026-09-11 23:04 ` Jakub Kicinski
2026-09-07 13:37 ` [PATCHv3 2/3] net: smc91x: do not use readw()/writew() on ColdFire platforms Greg Ungerer
2026-09-07 13:37 ` [PATCHv3 3/3] m68k: coldfire: fix non-standard readX()/writeX() functions Greg Ungerer
2026-09-08 13:43 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox