Skip to content

Commit 8262e6f

Browse files
Marek Vasutdavem330
authored andcommitted
net: ks8851-ml: Fix IO operations, again
This patch reverts 5829210 ("net: ks8851-ml: Fix 16-bit IO operation") and edacb09 ("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: 5829210 ("net: ks8851-ml: Fix 16-bit IO operation") Fixes: edacb09 ("net: ks8851-ml: Fix 16-bit data access") Signed-off-by: Marek Vasut <[email protected]> Cc: David S. Miller <[email protected]> Cc: Lukas Wunner <[email protected]> Cc: Petr Stetiar <[email protected]> Cc: YueHaibing <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 328f5bb commit 8262e6f

File tree

1 file changed

+52
-4
lines changed

1 file changed

+52
-4
lines changed

drivers/net/ethernet/micrel/ks8851_mll.c

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,50 @@ static int msg_enable;
156156
* chip is busy transferring packet data (RX/TX FIFO accesses).
157157
*/
158158

159+
/**
160+
* ks_check_endian - Check whether endianness of the bus is correct
161+
* @ks : The chip information
162+
*
163+
* The KS8851-16MLL EESK pin allows selecting the endianness of the 16bit
164+
* bus. To maintain optimum performance, the bus endianness should be set
165+
* such that it matches the endianness of the CPU.
166+
*/
167+
168+
static int ks_check_endian(struct ks_net *ks)
169+
{
170+
u16 cider;
171+
172+
/*
173+
* Read CIDER register first, however read it the "wrong" way around.
174+
* If the endian strap on the KS8851-16MLL in incorrect and the chip
175+
* is operating in different endianness than the CPU, then the meaning
176+
* of BE[3:0] byte-enable bits is also swapped such that:
177+
* BE[3,2,1,0] becomes BE[1,0,3,2]
178+
*
179+
* Luckily for us, the byte-enable bits are the top four MSbits of
180+
* the address register and the CIDER register is at offset 0xc0.
181+
* Hence, by reading address 0xc0c0, which is not impacted by endian
182+
* swapping, we assert either BE[3:2] or BE[1:0] while reading the
183+
* CIDER register.
184+
*
185+
* If the bus configuration is correct, reading 0xc0c0 asserts
186+
* BE[3:2] and this read returns 0x0000, because to read register
187+
* with bottom two LSbits of address set to 0, BE[1:0] must be
188+
* asserted.
189+
*
190+
* If the bus configuration is NOT correct, reading 0xc0c0 asserts
191+
* BE[1:0] and this read returns non-zero 0x8872 value.
192+
*/
193+
iowrite16(BE3 | BE2 | KS_CIDER, ks->hw_addr_cmd);
194+
cider = ioread16(ks->hw_addr);
195+
if (!cider)
196+
return 0;
197+
198+
netdev_err(ks->netdev, "incorrect EESK endian strap setting\n");
199+
200+
return -EINVAL;
201+
}
202+
159203
/**
160204
* ks_rdreg16 - read 16 bit register from device
161205
* @ks : The chip information
@@ -166,7 +210,7 @@ static int msg_enable;
166210

167211
static u16 ks_rdreg16(struct ks_net *ks, int offset)
168212
{
169-
ks->cmd_reg_cache = (u16)offset | ((BE3 | BE2) >> (offset & 0x02));
213+
ks->cmd_reg_cache = (u16)offset | ((BE1 | BE0) << (offset & 0x02));
170214
iowrite16(ks->cmd_reg_cache, ks->hw_addr_cmd);
171215
return ioread16(ks->hw_addr);
172216
}
@@ -181,7 +225,7 @@ static u16 ks_rdreg16(struct ks_net *ks, int offset)
181225

182226
static void ks_wrreg16(struct ks_net *ks, int offset, u16 value)
183227
{
184-
ks->cmd_reg_cache = (u16)offset | ((BE3 | BE2) >> (offset & 0x02));
228+
ks->cmd_reg_cache = (u16)offset | ((BE1 | BE0) << (offset & 0x02));
185229
iowrite16(ks->cmd_reg_cache, ks->hw_addr_cmd);
186230
iowrite16(value, ks->hw_addr);
187231
}
@@ -197,7 +241,7 @@ static inline void ks_inblk(struct ks_net *ks, u16 *wptr, u32 len)
197241
{
198242
len >>= 1;
199243
while (len--)
200-
*wptr++ = be16_to_cpu(ioread16(ks->hw_addr));
244+
*wptr++ = (u16)ioread16(ks->hw_addr);
201245
}
202246

203247
/**
@@ -211,7 +255,7 @@ static inline void ks_outblk(struct ks_net *ks, u16 *wptr, u32 len)
211255
{
212256
len >>= 1;
213257
while (len--)
214-
iowrite16(cpu_to_be16(*wptr++), ks->hw_addr);
258+
iowrite16(*wptr++, ks->hw_addr);
215259
}
216260

217261
static void ks_disable_int(struct ks_net *ks)
@@ -1218,6 +1262,10 @@ static int ks8851_probe(struct platform_device *pdev)
12181262
goto err_free;
12191263
}
12201264

1265+
err = ks_check_endian(ks);
1266+
if (err)
1267+
goto err_free;
1268+
12211269
netdev->irq = platform_get_irq(pdev, 0);
12221270

12231271
if ((int)netdev->irq < 0) {

0 commit comments

Comments
 (0)