[PATCH 2/2] net: xilinx_emaclite: fix freezes due to unordered I/O

2017-02-14 Thread Anssi Hannula
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 123c1407af87 ("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: Anssi Hannula 
Fixes: 123c1407af87 ("net: emaclite: Do not use microblaze and ppc IO
functions")

---

Not sure what would be the correct way to fix this. This way seems to
work, at least, but maybe something else would be better?

 drivers/net/ethernet/xilinx/xilinx_emaclite.c | 116 ++
 1 file changed, 62 insertions(+), 54 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_emaclite.c 
b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
index bd6470f..69e31ce 100644
--- a/drivers/net/ethernet/xilinx/xilinx_emaclite.c
+++ b/drivers/net/ethernet/xilinx/xilinx_emaclite.c
@@ -100,6 +100,14 @@
 /* BUFFER_ALIGN(adr) calculates the number of bytes to the next alignment. */
 #define BUFFER_ALIGN(adr) ((ALIGNMENT - ((u32) adr)) % ALIGNMENT)
 
+#ifdef __BIG_ENDIAN
+#define xemaclite_readlioread32be
+#define xemaclite_writel   iowrite32be
+#else
+#define xemaclite_readlioread32
+#define xemaclite_writel   iowrite32
+#endif
+
 /**
  * struct net_local - Our private per device data
  * @ndev:  instance of the network device
@@ -156,15 +164,15 @@ static void xemaclite_enable_interrupts(struct net_local 
*drvdata)
u32 reg_data;
 
/* Enable the Tx interrupts for the first Buffer */
-   reg_data = __raw_readl(drvdata->base_addr + XEL_TSR_OFFSET);
-   __raw_writel(reg_data | XEL_TSR_XMIT_IE_MASK,
-drvdata->base_addr + XEL_TSR_OFFSET);
+   reg_data = xemaclite_readl(drvdata->base_addr + XEL_TSR_OFFSET);
+   xemaclite_writel(reg_data | XEL_TSR_XMIT_IE_MASK,
+drvdata->base_addr + XEL_TSR_OFFSET);
 
/* 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 */
-   __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,17 +187,17 @@ static void xemaclite_disable_interrupts(struct net_local 
*drvdata)
u32 reg_data;
 
/* 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 */
-   reg_data = __raw_readl(drvdata->base_addr + XEL_TSR_OFFSET);
-   __raw_writel(reg_data & (~XEL_TSR_XMIT_IE_MASK),
-drvdata->base_addr + XEL_TSR_OFFSET);
+   reg_data = xemaclite_readl(drvdata->base_addr + XEL_TSR_OFFSET);
+   xemaclite_writel(reg_data & (~XEL_TSR_XMIT_IE_MASK),
+drvdata->base_addr + XEL_TSR_OFFSET);
 
/* Disable the Rx interrupts for the first buffer */
-   reg_data = __raw_readl(drvdata->base_addr + XEL_RSR_OFFSET);
-   __raw_writel(reg_data & (~XEL_RSR_RECV_IE_MASK),
-drvdata->base_addr + XEL_RSR_OFFSET);
+   reg_data = xemaclite_readl(drvdata->base_addr + XEL_RSR_OFFSET);
+   xemaclite_writel(reg_data & (~XEL_RSR_RECV_IE_MASK),
+drvdata->base_addr + XEL_RSR_OFFSET);
 }
 
 /**
@@ -321,7 +329,7 @@ static int xemaclite_send_data(struct net_local *drvdata, 
u8 *data,
byte_count = ETH_FRAME_LEN;
 
/* 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 |
 XEL_TSR_XMIT_ACTIVE_MASK)) == 0) {
 
@@ -334,7 +342,7 @@ static int xemaclite_send_data(struct net_local *drvdata

Re: [PATCH 2/2] net: xilinx_emaclite: fix freezes due to unordered I/O

2017-02-15 Thread David Miller
From: Anssi Hannula 
Date: Tue, 14 Feb 2017 19:11:45 +0200

> 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 123c1407af87 ("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: Anssi Hannula 
> Fixes: 123c1407af87 ("net: emaclite: Do not use microblaze and ppc IO
> functions")

Applied.