Skip to content

L
linux
Commit acf138f1 authored by Anssi Hannula's avatar Anssi Hannula Committed by David S. Miller
Browse files
Options

net: xilinx_emaclite: fix freezes due to unordered I/O 

The xilinx_emaclite uses __raw_writel and __raw_readl for register
accesses. Those functions do not imply any kind of memory barriers and
they may be reordered.

The driver does not seem to take that into account, though, and the
driver does not satisfy the ordering requirements of the hardware.
For clear examples, see xemaclite_mdio_write() and xemaclite_mdio_read()
which try to set MDIO address before initiating the transaction.

I'm seeing system freezes with the driver with GCC 5.4 and current
Linux kernels on Zynq-7000 SoC immediately when trying to use the
interface.

In commit 123c1407 ("net: emaclite: Do not use microblaze and ppc
IO functions") the driver was switched from non-generic
in_be32/out_be32 (memory barriers, big endian) to
__raw_readl/__raw_writel (no memory barriers, native endian), so
apparently the device follows system endianness and the driver was
originally written with the assumption of memory barriers.

Rather than try to hunt for each case of missing barrier, just switch
the driver to use iowrite32/ioread32/iowrite32be/ioread32be depending
on endianness instead.

Tested on little-endian Zynq-7000 ARM SoC FPGA.
Signed-off-by: default avatarAnssi Hannula <anssi.hannula@bitwise.fi>
Fixes: 123c1407 ("net: emaclite: Do not use microblaze and ppc IO
functions")
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent cd224553
Show whitespace changes
Inline Side-by-side
Showing with 62 additions and 54 deletions
drivers/net/ethernet/xilinx/xilinx_emaclite.c
View file @ acf138f1
...@@ -100,6 +100,14 @@ ...@@ -100,6 +100,14 @@
/* BUFFER_ALIGN(adr) calculates the number of bytes to the next alignment. */ /* BUFFER_ALIGN(adr) calculates the number of bytes to the next alignment. */
#define BUFFER_ALIGN(adr) ((ALIGNMENT - ((u32) adr)) % ALIGNMENT) #define BUFFER_ALIGN(adr) ((ALIGNMENT - ((u32) adr)) % ALIGNMENT)
#ifdef __BIG_ENDIAN
#define xemaclite_readl ioread32be
#define xemaclite_writel iowrite32be
#else
#define xemaclite_readl ioread32
#define xemaclite_writel iowrite32
#endif
/** /**
* struct net_local - Our private per device data * struct net_local - Our private per device data
* @ndev: instance of the network device * @ndev: instance of the network device
...@@ -156,15 +164,15 @@ static void xemaclite_enable_interrupts(struct net_local *drvdata) ...@@ -156,15 +164,15 @@ static void xemaclite_enable_interrupts(struct net_local *drvdata)
u32 reg_data; u32 reg_data;
/* Enable the Tx interrupts for the first Buffer */ /* Enable the Tx interrupts for the first Buffer */
reg_data = __raw_readl(drvdata->base_addr + XEL_TSR_OFFSET); reg_data = xemaclite_readl(drvdata->base_addr + XEL_TSR_OFFSET);
__raw_writel(reg_data | XEL_TSR_XMIT_IE_MASK, xemaclite_writel(reg_data | XEL_TSR_XMIT_IE_MASK,
drvdata->base_addr + XEL_TSR_OFFSET); drvdata->base_addr + XEL_TSR_OFFSET);
/* Enable the Rx interrupts for the first buffer */ /* Enable the Rx interrupts for the first buffer */
__raw_writel(XEL_RSR_RECV_IE_MASK, drvdata->base_addr + XEL_RSR_OFFSET); xemaclite_writel(XEL_RSR_RECV_IE_MASK, drvdata->base_addr + XEL_RSR_OFFSET);
/* Enable the Global Interrupt Enable */ /* Enable the Global Interrupt Enable */
__raw_writel(XEL_GIER_GIE_MASK, drvdata->base_addr + XEL_GIER_OFFSET); xemaclite_writel(XEL_GIER_GIE_MASK, drvdata->base_addr + XEL_GIER_OFFSET);
} }
/** /**
...@@ -179,16 +187,16 @@ static void xemaclite_disable_interrupts(struct net_local *drvdata) ...@@ -179,16 +187,16 @@ static void xemaclite_disable_interrupts(struct net_local *drvdata)
u32 reg_data; u32 reg_data;
/* Disable the Global Interrupt Enable */ /* Disable the Global Interrupt Enable */
__raw_writel(XEL_GIER_GIE_MASK, drvdata->base_addr + XEL_GIER_OFFSET); xemaclite_writel(XEL_GIER_GIE_MASK, drvdata->base_addr + XEL_GIER_OFFSET);
/* Disable the Tx interrupts for the first buffer */ /* Disable the Tx interrupts for the first buffer */
reg_data = __raw_readl(drvdata->base_addr + XEL_TSR_OFFSET); reg_data = xemaclite_readl(drvdata->base_addr + XEL_TSR_OFFSET);
__raw_writel(reg_data & (~XEL_TSR_XMIT_IE_MASK), xemaclite_writel(reg_data & (~XEL_TSR_XMIT_IE_MASK),
drvdata->base_addr + XEL_TSR_OFFSET); drvdata->base_addr + XEL_TSR_OFFSET);
/* Disable the Rx interrupts for the first buffer */ /* Disable the Rx interrupts for the first buffer */
reg_data = __raw_readl(drvdata->base_addr + XEL_RSR_OFFSET); reg_data = xemaclite_readl(drvdata->base_addr + XEL_RSR_OFFSET);
__raw_writel(reg_data & (~XEL_RSR_RECV_IE_MASK), xemaclite_writel(reg_data & (~XEL_RSR_RECV_IE_MASK),
drvdata->base_addr + XEL_RSR_OFFSET); drvdata->base_addr + XEL_RSR_OFFSET);
} }
...@@ -321,7 +329,7 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data, ...@@ -321,7 +329,7 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data,
byte_count = ETH_FRAME_LEN; byte_count = ETH_FRAME_LEN;
/* Check if the expected buffer is available */ /* Check if the expected buffer is available */
reg_data = __raw_readl(addr + XEL_TSR_OFFSET); reg_data = xemaclite_readl(addr + XEL_TSR_OFFSET);
if ((reg_data & (XEL_TSR_XMIT_BUSY_MASK | if ((reg_data & (XEL_TSR_XMIT_BUSY_MASK |
XEL_TSR_XMIT_ACTIVE_MASK)) == 0) { XEL_TSR_XMIT_ACTIVE_MASK)) == 0) {
...@@ -334,7 +342,7 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data, ...@@ -334,7 +342,7 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data,
addr = (void __iomem __force *)((u32 __force)addr ^ addr = (void __iomem __force *)((u32 __force)addr ^
XEL_BUFFER_OFFSET); XEL_BUFFER_OFFSET);
reg_data = __raw_readl(addr + XEL_TSR_OFFSET); reg_data = xemaclite_readl(addr + XEL_TSR_OFFSET);
if ((reg_data & (XEL_TSR_XMIT_BUSY_MASK | if ((reg_data & (XEL_TSR_XMIT_BUSY_MASK |
XEL_TSR_XMIT_ACTIVE_MASK)) != 0) XEL_TSR_XMIT_ACTIVE_MASK)) != 0)
...@@ -345,16 +353,16 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data, ...@@ -345,16 +353,16 @@ static int xemaclite_send_data(struct net_local *drvdata, u8 *data,
/* Write the frame to the buffer */ /* Write the frame to the buffer */
xemaclite_aligned_write(data, (u32 __force *) addr, byte_count); xemaclite_aligned_write(data, (u32 __force *) addr, byte_count);
__raw_writel((byte_count & XEL_TPLR_LENGTH_MASK), xemaclite_writel((byte_count & XEL_TPLR_LENGTH_MASK),
addr + XEL_TPLR_OFFSET); addr + XEL_TPLR_OFFSET);
/* Update the Tx Status Register to indicate that there is a /* Update the Tx Status Register to indicate that there is a
* frame to send. Set the XEL_TSR_XMIT_ACTIVE_MASK flag which * frame to send. Set the XEL_TSR_XMIT_ACTIVE_MASK flag which
* is used by the interrupt handler to check whether a frame * is used by the interrupt handler to check whether a frame
* has been transmitted */ * has been transmitted */
reg_data = __raw_readl(addr + XEL_TSR_OFFSET); reg_data = xemaclite_readl(addr + XEL_TSR_OFFSET);
reg_data |= (XEL_TSR_XMIT_BUSY_MASK | XEL_TSR_XMIT_ACTIVE_MASK); reg_data |= (XEL_TSR_XMIT_BUSY_MASK | XEL_TSR_XMIT_ACTIVE_MASK);
__raw_writel(reg_data, addr + XEL_TSR_OFFSET); xemaclite_writel(reg_data, addr + XEL_TSR_OFFSET);
return 0; return 0;
} }
...@@ -379,7 +387,7 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen) ...@@ -379,7 +387,7 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
addr = (drvdata->base_addr + drvdata->next_rx_buf_to_use); addr = (drvdata->base_addr + drvdata->next_rx_buf_to_use);
/* Verify which buffer has valid data */ /* Verify which buffer has valid data */
reg_data = __raw_readl(addr + XEL_RSR_OFFSET); reg_data = xemaclite_readl(addr + XEL_RSR_OFFSET);
if ((reg_data & XEL_RSR_RECV_DONE_MASK) == XEL_RSR_RECV_DONE_MASK) { if ((reg_data & XEL_RSR_RECV_DONE_MASK) == XEL_RSR_RECV_DONE_MASK) {
if (drvdata->rx_ping_pong != 0) if (drvdata->rx_ping_pong != 0)
...@@ -396,14 +404,14 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen) ...@@ -396,14 +404,14 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
return 0; /* No data was available */ return 0; /* No data was available */
/* Verify that buffer has valid data */ /* Verify that buffer has valid data */
reg_data = __raw_readl(addr + XEL_RSR_OFFSET); reg_data = xemaclite_readl(addr + XEL_RSR_OFFSET);
if ((reg_data & XEL_RSR_RECV_DONE_MASK) != if ((reg_data & XEL_RSR_RECV_DONE_MASK) !=
XEL_RSR_RECV_DONE_MASK) XEL_RSR_RECV_DONE_MASK)
return 0; /* No data was available */ return 0; /* No data was available */
} }
/* Get the protocol type of the ethernet frame that arrived */ /* Get the protocol type of the ethernet frame that arrived */
proto_type = ((ntohl(__raw_readl(addr + XEL_HEADER_OFFSET + proto_type = ((ntohl(xemaclite_readl(addr + XEL_HEADER_OFFSET +
XEL_RXBUFF_OFFSET)) >> XEL_HEADER_SHIFT) & XEL_RXBUFF_OFFSET)) >> XEL_HEADER_SHIFT) &
XEL_RPLR_LENGTH_MASK); XEL_RPLR_LENGTH_MASK);
...@@ -412,7 +420,7 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen) ...@@ -412,7 +420,7 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
if (proto_type > ETH_DATA_LEN) { if (proto_type > ETH_DATA_LEN) {
if (proto_type == ETH_P_IP) { if (proto_type == ETH_P_IP) {
length = ((ntohl(__raw_readl(addr + length = ((ntohl(xemaclite_readl(addr +
XEL_HEADER_IP_LENGTH_OFFSET + XEL_HEADER_IP_LENGTH_OFFSET +
XEL_RXBUFF_OFFSET)) >> XEL_RXBUFF_OFFSET)) >>
XEL_HEADER_SHIFT) & XEL_HEADER_SHIFT) &
...@@ -438,9 +446,9 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen) ...@@ -438,9 +446,9 @@ static u16 xemaclite_recv_data(struct net_local *drvdata, u8 *data, int maxlen)
data, length); data, length);
/* Acknowledge the frame */ /* Acknowledge the frame */
reg_data = __raw_readl(addr + XEL_RSR_OFFSET); reg_data = xemaclite_readl(addr + XEL_RSR_OFFSET);
reg_data &= ~XEL_RSR_RECV_DONE_MASK; reg_data &= ~XEL_RSR_RECV_DONE_MASK;
__raw_writel(reg_data, addr + XEL_RSR_OFFSET); xemaclite_writel(reg_data, addr + XEL_RSR_OFFSET);
return length; return length;
} }
...@@ -467,14 +475,14 @@ static void xemaclite_update_address(struct net_local *drvdata, ...@@ -467,14 +475,14 @@ static void xemaclite_update_address(struct net_local *drvdata,
xemaclite_aligned_write(address_ptr, (u32 __force *) addr, ETH_ALEN); xemaclite_aligned_write(address_ptr, (u32 __force *) addr, ETH_ALEN);
__raw_writel(ETH_ALEN, addr + XEL_TPLR_OFFSET); xemaclite_writel(ETH_ALEN, addr + XEL_TPLR_OFFSET);
/* Update the MAC address in the EmacLite */ /* Update the MAC address in the EmacLite */
reg_data = __raw_readl(addr + XEL_TSR_OFFSET); reg_data = xemaclite_readl(addr + XEL_TSR_OFFSET);
__raw_writel(reg_data | XEL_TSR_PROG_MAC_ADDR, addr + XEL_TSR_OFFSET); xemaclite_writel(reg_data | XEL_TSR_PROG_MAC_ADDR, addr + XEL_TSR_OFFSET);
/* Wait for EmacLite to finish with the MAC address update */ /* Wait for EmacLite to finish with the MAC address update */
while ((__raw_readl(addr + XEL_TSR_OFFSET) & while ((xemaclite_readl(addr + XEL_TSR_OFFSET) &
XEL_TSR_PROG_MAC_ADDR) != 0) XEL_TSR_PROG_MAC_ADDR) != 0)
; ;
} }
...@@ -644,31 +652,31 @@ static irqreturn_t xemaclite_interrupt(int irq, void *dev_id) ...@@ -644,31 +652,31 @@ static irqreturn_t xemaclite_interrupt(int irq, void *dev_id)
u32 tx_status; u32 tx_status;
/* Check if there is Rx Data available */ /* Check if there is Rx Data available */
if ((__raw_readl(base_addr + XEL_RSR_OFFSET) & if ((xemaclite_readl(base_addr + XEL_RSR_OFFSET) &
XEL_RSR_RECV_DONE_MASK) || XEL_RSR_RECV_DONE_MASK) ||
(__raw_readl(base_addr + XEL_BUFFER_OFFSET + XEL_RSR_OFFSET) (xemaclite_readl(base_addr + XEL_BUFFER_OFFSET + XEL_RSR_OFFSET)
& XEL_RSR_RECV_DONE_MASK)) & XEL_RSR_RECV_DONE_MASK))
xemaclite_rx_handler(dev); xemaclite_rx_handler(dev);
/* Check if the Transmission for the first buffer is completed */ /* Check if the Transmission for the first buffer is completed */
tx_status = __raw_readl(base_addr + XEL_TSR_OFFSET); tx_status = xemaclite_readl(base_addr + XEL_TSR_OFFSET);
if (((tx_status & XEL_TSR_XMIT_BUSY_MASK) == 0) && if (((tx_status & XEL_TSR_XMIT_BUSY_MASK) == 0) &&
(tx_status & XEL_TSR_XMIT_ACTIVE_MASK) != 0) { (tx_status & XEL_TSR_XMIT_ACTIVE_MASK) != 0) {
tx_status &= ~XEL_TSR_XMIT_ACTIVE_MASK; tx_status &= ~XEL_TSR_XMIT_ACTIVE_MASK;
__raw_writel(tx_status, base_addr + XEL_TSR_OFFSET); xemaclite_writel(tx_status, base_addr + XEL_TSR_OFFSET);
tx_complete = true; tx_complete = true;
} }
/* Check if the Transmission for the second buffer is completed */ /* Check if the Transmission for the second buffer is completed */
tx_status = __raw_readl(base_addr + XEL_BUFFER_OFFSET + XEL_TSR_OFFSET); tx_status = xemaclite_readl(base_addr + XEL_BUFFER_OFFSET + XEL_TSR_OFFSET);
if (((tx_status & XEL_TSR_XMIT_BUSY_MASK) == 0) && if (((tx_status & XEL_TSR_XMIT_BUSY_MASK) == 0) &&
(tx_status & XEL_TSR_XMIT_ACTIVE_MASK) != 0) { (tx_status & XEL_TSR_XMIT_ACTIVE_MASK) != 0) {
tx_status &= ~XEL_TSR_XMIT_ACTIVE_MASK; tx_status &= ~XEL_TSR_XMIT_ACTIVE_MASK;
__raw_writel(tx_status, base_addr + XEL_BUFFER_OFFSET + xemaclite_writel(tx_status, base_addr + XEL_BUFFER_OFFSET +
XEL_TSR_OFFSET); XEL_TSR_OFFSET);
tx_complete = true; tx_complete = true;
...@@ -702,7 +710,7 @@ static int xemaclite_mdio_wait(struct net_local *lp) ...@@ -702,7 +710,7 @@ static int xemaclite_mdio_wait(struct net_local *lp)
/* wait for the MDIO interface to not be busy or timeout /* wait for the MDIO interface to not be busy or timeout
after some time. after some time.
*/ */
while (__raw_readl(lp->base_addr + XEL_MDIOCTRL_OFFSET) & while (xemaclite_readl(lp->base_addr + XEL_MDIOCTRL_OFFSET) &
XEL_MDIOCTRL_MDIOSTS_MASK) { XEL_MDIOCTRL_MDIOSTS_MASK) {
if (time_before_eq(end, jiffies)) { if (time_before_eq(end, jiffies)) {
WARN_ON(1); WARN_ON(1);
...@@ -738,17 +746,17 @@ static int xemaclite_mdio_read(struct mii_bus *bus, int phy_id, int reg) ...@@ -738,17 +746,17 @@ static int xemaclite_mdio_read(struct mii_bus *bus, int phy_id, int reg)
* MDIO Address register. Set the Status bit in the MDIO Control * MDIO Address register. Set the Status bit in the MDIO Control
* register to start a MDIO read transaction. * register to start a MDIO read transaction.
*/ */
ctrl_reg = __raw_readl(lp->base_addr + XEL_MDIOCTRL_OFFSET); ctrl_reg = xemaclite_readl(lp->base_addr + XEL_MDIOCTRL_OFFSET);
__raw_writel(XEL_MDIOADDR_OP_MASK | xemaclite_writel(XEL_MDIOADDR_OP_MASK |
((phy_id << XEL_MDIOADDR_PHYADR_SHIFT) | reg), ((phy_id << XEL_MDIOADDR_PHYADR_SHIFT) | reg),
lp->base_addr + XEL_MDIOADDR_OFFSET); lp->base_addr + XEL_MDIOADDR_OFFSET);
__raw_writel(ctrl_reg | XEL_MDIOCTRL_MDIOSTS_MASK, xemaclite_writel(ctrl_reg | XEL_MDIOCTRL_MDIOSTS_MASK,
lp->base_addr + XEL_MDIOCTRL_OFFSET); lp->base_addr + XEL_MDIOCTRL_OFFSET);
if (xemaclite_mdio_wait(lp)) if (xemaclite_mdio_wait(lp))
return -ETIMEDOUT; return -ETIMEDOUT;
rc = __raw_readl(lp->base_addr + XEL_MDIORD_OFFSET); rc = xemaclite_readl(lp->base_addr + XEL_MDIORD_OFFSET);
dev_dbg(&lp->ndev->dev, dev_dbg(&lp->ndev->dev,
"xemaclite_mdio_read(phy_id=%i, reg=%x) == %x\n", "xemaclite_mdio_read(phy_id=%i, reg=%x) == %x\n",
...@@ -785,12 +793,12 @@ static int xemaclite_mdio_write(struct mii_bus *bus, int phy_id, int reg, ...@@ -785,12 +793,12 @@ static int xemaclite_mdio_write(struct mii_bus *bus, int phy_id, int reg,
* Data register. Finally, set the Status bit in the MDIO Control * Data register. Finally, set the Status bit in the MDIO Control
* register to start a MDIO write transaction. * register to start a MDIO write transaction.
*/ */
ctrl_reg = __raw_readl(lp->base_addr + XEL_MDIOCTRL_OFFSET); ctrl_reg = xemaclite_readl(lp->base_addr + XEL_MDIOCTRL_OFFSET);
__raw_writel(~XEL_MDIOADDR_OP_MASK & xemaclite_writel(~XEL_MDIOADDR_OP_MASK &
((phy_id << XEL_MDIOADDR_PHYADR_SHIFT) | reg), ((phy_id << XEL_MDIOADDR_PHYADR_SHIFT) | reg),
lp->base_addr + XEL_MDIOADDR_OFFSET); lp->base_addr + XEL_MDIOADDR_OFFSET);
__raw_writel(val, lp->base_addr + XEL_MDIOWR_OFFSET); xemaclite_writel(val, lp->base_addr + XEL_MDIOWR_OFFSET);
__raw_writel(ctrl_reg | XEL_MDIOCTRL_MDIOSTS_MASK, xemaclite_writel(ctrl_reg | XEL_MDIOCTRL_MDIOSTS_MASK,
lp->base_addr + XEL_MDIOCTRL_OFFSET); lp->base_addr + XEL_MDIOCTRL_OFFSET);
return 0; return 0;
...@@ -838,7 +846,7 @@ static int xemaclite_mdio_setup(struct net_local *lp, struct device *dev) ...@@ -838,7 +846,7 @@ static int xemaclite_mdio_setup(struct net_local *lp, struct device *dev)
/* Enable the MDIO bus by asserting the enable bit in MDIO Control /* Enable the MDIO bus by asserting the enable bit in MDIO Control
* register. * register.
*/ */
__raw_writel(XEL_MDIOCTRL_MDIOEN_MASK, xemaclite_writel(XEL_MDIOCTRL_MDIOEN_MASK,
lp->base_addr + XEL_MDIOCTRL_OFFSET); lp->base_addr + XEL_MDIOCTRL_OFFSET);
bus = mdiobus_alloc(); bus = mdiobus_alloc();
...@@ -1144,8 +1152,8 @@ static int xemaclite_of_probe(struct platform_device *ofdev) ...@@ -1144,8 +1152,8 @@ static int xemaclite_of_probe(struct platform_device *ofdev)
} }
/* Clear the Tx CSR's in case this is a restart */ /* Clear the Tx CSR's in case this is a restart */
__raw_writel(0, lp->base_addr + XEL_TSR_OFFSET); xemaclite_writel(0, lp->base_addr + XEL_TSR_OFFSET);
__raw_writel(0, lp->base_addr + XEL_BUFFER_OFFSET + XEL_TSR_OFFSET); xemaclite_writel(0, lp->base_addr + XEL_BUFFER_OFFSET + XEL_TSR_OFFSET);
/* Set the MAC address in the EmacLite device */ /* Set the MAC address in the EmacLite device */
xemaclite_update_address(lp, ndev->dev_addr); xemaclite_update_address(lp, ndev->dev_addr);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7