net: ks8851-ml: Fix IO operations, again

This patch reverts b39ff8953438 ("net: ks8851-ml: Fix 16-bit IO operation")
and 53961b64faad ("net: ks8851-ml: Fix 16-bit data access"), because it
turns out these were only necessary due to buggy hardware. This patch adds
a check for such a buggy hardware to prevent any such mistakes again.

While working further on the KS8851 driver, it came to light that the
KS8851-16MLL is capable of switching bus endianness by a hardware strap,
EESK pin. If this strap is incorrect, the IO accesses require such endian
swapping as is being reverted by this patch. Such swapping also impacts
the performance significantly.

Hence, in addition to removing it, detect that the hardware is broken,
report to user, and fail to bind with such hardware.

Fixes: b39ff8953438 ("net: ks8851-ml: Fix 16-bit IO operation")
Fixes: 53961b64faad ("net: ks8851-ml: Fix 16-bit data access")
Signed-off-by: Marek Vasut <marex@denx.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Lukas Wunner <lukas@wunner.de>
Cc: Petr Stetiar <ynezz@true.cz>
Cc: YueHaibing <yuehaibing@huawei.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/drivers/net/ethernet/micrel/ks8851_mll.c b/drivers/net/ethernet/micrel/ks8851_mll.c
index 58579baf3f7a007f65910938b7e3a7f3ddaed718..45cc840d8e2e14131188d14c3b98a9ffde7d8ad7 100644 (file)
--- a/drivers/net/ethernet/micrel/ks8851_mll.c
+++ b/drivers/net/ethernet/micrel/ks8851_mll.c
@@ -156,6 +156,50 @@ static int msg_enable;
  * chip is busy transferring packet data (RX/TX FIFO accesses).
  */
 
+/**
+ * ks_check_endian - Check whether endianness of the bus is correct
+ * @ks   : The chip information
+ *
+ * The KS8851-16MLL EESK pin allows selecting the endianness of the 16bit
+ * bus. To maintain optimum performance, the bus endianness should be set
+ * such that it matches the endianness of the CPU.
+ */
+
+static int ks_check_endian(struct ks_net *ks)
+{
+       u16 cider;
+
+       /*
+        * Read CIDER register first, however read it the "wrong" way around.
+        * If the endian strap on the KS8851-16MLL in incorrect and the chip
+        * is operating in different endianness than the CPU, then the meaning
+        * of BE[3:0] byte-enable bits is also swapped such that:
+        *    BE[3,2,1,0] becomes BE[1,0,3,2]
+        *
+        * Luckily for us, the byte-enable bits are the top four MSbits of
+        * the address register and the CIDER register is at offset 0xc0.
+        * Hence, by reading address 0xc0c0, which is not impacted by endian
+        * swapping, we assert either BE[3:2] or BE[1:0] while reading the
+        * CIDER register.
+        *
+        * If the bus configuration is correct, reading 0xc0c0 asserts
+        * BE[3:2] and this read returns 0x0000, because to read register
+        * with bottom two LSbits of address set to 0, BE[1:0] must be
+        * asserted.
+        *
+        * If the bus configuration is NOT correct, reading 0xc0c0 asserts
+        * BE[1:0] and this read returns non-zero 0x8872 value.
+        */
+       iowrite16(BE3 | BE2 | KS_CIDER, ks->hw_addr_cmd);
+       cider = ioread16(ks->hw_addr);
+       if (!cider)
+               return 0;
+
+       netdev_err(ks->netdev, "incorrect EESK endian strap setting\n");
+
+       return -EINVAL;
+}
+
 /**
  * ks_rdreg16 - read 16 bit register from device
  * @ks   : The chip information
@@ -166,7 +210,7 @@ static int msg_enable;
 
 static u16 ks_rdreg16(struct ks_net *ks, int offset)
 {
-       ks->cmd_reg_cache = (u16)offset | ((BE3 | BE2) >> (offset & 0x02));
+       ks->cmd_reg_cache = (u16)offset | ((BE1 | BE0) << (offset & 0x02));
        iowrite16(ks->cmd_reg_cache, ks->hw_addr_cmd);
        return ioread16(ks->hw_addr);
 }
@@ -181,7 +225,7 @@ static u16 ks_rdreg16(struct ks_net *ks, int offset)
 
 static void ks_wrreg16(struct ks_net *ks, int offset, u16 value)
 {
-       ks->cmd_reg_cache = (u16)offset | ((BE3 | BE2) >> (offset & 0x02));
+       ks->cmd_reg_cache = (u16)offset | ((BE1 | BE0) << (offset & 0x02));
        iowrite16(ks->cmd_reg_cache, ks->hw_addr_cmd);
        iowrite16(value, ks->hw_addr);
 }
@@ -197,7 +241,7 @@ static inline void ks_inblk(struct ks_net *ks, u16 *wptr, u32 len)
 {
        len >>= 1;
        while (len--)
-               *wptr++ = be16_to_cpu(ioread16(ks->hw_addr));
+               *wptr++ = (u16)ioread16(ks->hw_addr);
 }
 
 /**
@@ -211,7 +255,7 @@ static inline void ks_outblk(struct ks_net *ks, u16 *wptr, u32 len)
 {
        len >>= 1;
        while (len--)
-               iowrite16(cpu_to_be16(*wptr++), ks->hw_addr);
+               iowrite16(*wptr++, ks->hw_addr);
 }
 
 static void ks_disable_int(struct ks_net *ks)
@@ -1218,6 +1262,10 @@ static int ks8851_probe(struct platform_device *pdev)
                goto err_free;
        }
 
+       err = ks_check_endian(ks);
+       if (err)
+               goto err_free;
+
        netdev->irq = platform_get_irq(pdev, 0);
 
        if ((int)netdev->irq < 0) {