Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

net: smc91x: fix SMC accesses

Commit b70661c70830 ("net: smc91x: use run-time configuration on all ARM
machines") broke some ARM platforms through several mistakes. Firstly,
the access size must correspond to the following rule:

(a) at least one of 16-bit or 8-bit access size must be supported
(b) 32-bit accesses are optional, and may be enabled in addition to
the above.

Secondly, it provides no emulation of 16-bit accesses, instead blindly
making 16-bit accesses even when the platform specifies that only 8-bit
is supported.

Reorganise smc91x.h so we can make use of the existing 16-bit access
emulation already provided - if 16-bit accesses are supported, use
16-bit accesses directly, otherwise if 8-bit accesses are supported,
use the provided 16-bit access emulation. If neither, BUG(). This
exactly reflects the driver behaviour prior to the commit being fixed.

Since the conversion incorrectly cut down the available access sizes on
several platforms, we also need to go through every platform and fix up
the overly-restrictive access size: Arnd assumed that if a platform can
perform 32-bit, 16-bit and 8-bit accesses, then only a 32-bit access
size needed to be specified - not so, all available access sizes must
be specified.

This likely fixes some performance regressions in doing this: if a
platform does not support 8-bit accesses, 8-bit accesses have been
emulated by performing a 16-bit read-modify-write access.

Tested on the Intel Assabet/Neponset platform, which supports only 8-bit
accesses, which was broken by the original commit.

Fixes: b70661c70830 ("net: smc91x: use run-time configuration on all ARM machines")
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Tested-by: Robert Jarzmik <robert.jarzmik@free.fr>
Signed-off-by: David S. Miller <davem@davemloft.net>

authored by

Russell King and committed by
David S. Miller
2fb04fdf 7d13eca0

+73 -26
+2 -1
arch/arm/mach-pxa/idp.c
··· 83 83 }; 84 84 85 85 static struct smc91x_platdata smc91x_platdata = { 86 - .flags = SMC91X_USE_32BIT | SMC91X_USE_DMA | SMC91X_NOWAIT, 86 + .flags = SMC91X_USE_8BIT | SMC91X_USE_16BIT | SMC91X_USE_32BIT | 87 + SMC91X_USE_DMA | SMC91X_NOWAIT, 87 88 }; 88 89 89 90 static struct platform_device smc91x_device = {
+2 -1
arch/arm/mach-pxa/xcep.c
··· 120 120 }; 121 121 122 122 static struct smc91x_platdata xcep_smc91x_info = { 123 - .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT | SMC91X_USE_DMA, 123 + .flags = SMC91X_USE_8BIT | SMC91X_USE_16BIT | SMC91X_USE_32BIT | 124 + SMC91X_NOWAIT | SMC91X_USE_DMA, 124 125 }; 125 126 126 127 static struct platform_device smc91x_device = {
+2 -1
arch/arm/mach-realview/core.c
··· 93 93 }; 94 94 95 95 static struct smc91x_platdata smc91x_platdata = { 96 - .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, 96 + .flags = SMC91X_USE_8BIT | SMC91X_USE_16BIT | SMC91X_USE_32BIT | 97 + SMC91X_NOWAIT, 97 98 }; 98 99 99 100 static struct platform_device realview_eth_device = {
+1 -1
arch/arm/mach-sa1100/pleb.c
··· 45 45 }; 46 46 47 47 static struct smc91x_platdata smc91x_platdata = { 48 - .flags = SMC91X_USE_16BIT | SMC91X_NOWAIT, 48 + .flags = SMC91X_USE_16BIT | SMC91X_USE_8BIT | SMC91X_NOWAIT, 49 49 }; 50 50 51 51 static struct platform_device smc91x_device = {
+2 -1
arch/blackfin/mach-bf561/boards/cm_bf561.c
··· 146 146 #include <linux/smc91x.h> 147 147 148 148 static struct smc91x_platdata smc91x_info = { 149 - .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, 149 + .flags = SMC91X_USE_8BIT | SMC91X_USE_16BIT | SMC91X_USE_32BIT | 150 + SMC91X_NOWAIT, 150 151 .leda = RPC_LED_100_10, 151 152 .ledb = RPC_LED_TX_RX, 152 153 };
+2 -1
arch/blackfin/mach-bf561/boards/ezkit.c
··· 134 134 #include <linux/smc91x.h> 135 135 136 136 static struct smc91x_platdata smc91x_info = { 137 - .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, 137 + .flags = SMC91X_USE_8BIT | SMC91X_USE_16BIT | SMC91X_USE_32BIT | 138 + SMC91X_NOWAIT, 138 139 .leda = RPC_LED_100_10, 139 140 .ledb = RPC_LED_TX_RX, 140 141 };
+7
drivers/net/ethernet/smsc/smc91x.c
··· 2275 2275 if (pd) { 2276 2276 memcpy(&lp->cfg, pd, sizeof(lp->cfg)); 2277 2277 lp->io_shift = SMC91X_IO_SHIFT(lp->cfg.flags); 2278 + 2279 + if (!SMC_8BIT(lp) && !SMC_16BIT(lp)) { 2280 + dev_err(&pdev->dev, 2281 + "at least one of 8-bit or 16-bit access support is required.\n"); 2282 + ret = -ENXIO; 2283 + goto out_free_netdev; 2284 + } 2278 2285 } 2279 2286 2280 2287 #if IS_BUILTIN(CONFIG_OF)
+45 -20
drivers/net/ethernet/smsc/smc91x.h
··· 37 37 #include <linux/smc91x.h> 38 38 39 39 /* 40 + * Any 16-bit access is performed with two 8-bit accesses if the hardware 41 + * can't do it directly. Most registers are 16-bit so those are mandatory. 42 + */ 43 + #define SMC_outw_b(x, a, r) \ 44 + do { \ 45 + unsigned int __val16 = (x); \ 46 + unsigned int __reg = (r); \ 47 + SMC_outb(__val16, a, __reg); \ 48 + SMC_outb(__val16 >> 8, a, __reg + (1 << SMC_IO_SHIFT)); \ 49 + } while (0) 50 + 51 + #define SMC_inw_b(a, r) \ 52 + ({ \ 53 + unsigned int __val16; \ 54 + unsigned int __reg = r; \ 55 + __val16 = SMC_inb(a, __reg); \ 56 + __val16 |= SMC_inb(a, __reg + (1 << SMC_IO_SHIFT)) << 8; \ 57 + __val16; \ 58 + }) 59 + 60 + /* 40 61 * Define your architecture specific bus configuration parameters here. 41 62 */ 42 63 ··· 76 55 #define SMC_IO_SHIFT (lp->io_shift) 77 56 78 57 #define SMC_inb(a, r) readb((a) + (r)) 79 - #define SMC_inw(a, r) readw((a) + (r)) 58 + #define SMC_inw(a, r) \ 59 + ({ \ 60 + unsigned int __smc_r = r; \ 61 + SMC_16BIT(lp) ? readw((a) + __smc_r) : \ 62 + SMC_8BIT(lp) ? SMC_inw_b(a, __smc_r) : \ 63 + ({ BUG(); 0; }); \ 64 + }) 65 + 80 66 #define SMC_inl(a, r) readl((a) + (r)) 81 67 #define SMC_outb(v, a, r) writeb(v, (a) + (r)) 68 + #define SMC_outw(v, a, r) \ 69 + do { \ 70 + unsigned int __v = v, __smc_r = r; \ 71 + if (SMC_16BIT(lp)) \ 72 + __SMC_outw(__v, a, __smc_r); \ 73 + else if (SMC_8BIT(lp)) \ 74 + SMC_outw_b(__v, a, __smc_r); \ 75 + else \ 76 + BUG(); \ 77 + } while (0) 78 + 82 79 #define SMC_outl(v, a, r) writel(v, (a) + (r)) 80 + #define SMC_insb(a, r, p, l) readsb((a) + (r), p, l) 81 + #define SMC_outsb(a, r, p, l) writesb((a) + (r), p, l) 83 82 #define SMC_insw(a, r, p, l) readsw((a) + (r), p, l) 84 83 #define SMC_outsw(a, r, p, l) writesw((a) + (r), p, l) 85 84 #define SMC_insl(a, r, p, l) readsl((a) + (r), p, l) ··· 107 66 #define SMC_IRQ_FLAGS (-1) /* from resource */ 108 67 109 68 /* We actually can't write halfwords properly if not word aligned */ 110 - static inline void SMC_outw(u16 val, void __iomem *ioaddr, int reg) 69 + static inline void __SMC_outw(u16 val, void __iomem *ioaddr, int reg) 111 70 { 112 71 if ((machine_is_mainstone() || machine_is_stargate2() || 113 72 machine_is_pxa_idp()) && reg & 2) { ··· 457 416 458 417 #if ! SMC_CAN_USE_16BIT 459 418 460 - /* 461 - * Any 16-bit access is performed with two 8-bit accesses if the hardware 462 - * can't do it directly. Most registers are 16-bit so those are mandatory. 463 - */ 464 - #define SMC_outw(x, ioaddr, reg) \ 465 - do { \ 466 - unsigned int __val16 = (x); \ 467 - SMC_outb( __val16, ioaddr, reg ); \ 468 - SMC_outb( __val16 >> 8, ioaddr, reg + (1 << SMC_IO_SHIFT));\ 469 - } while (0) 470 - #define SMC_inw(ioaddr, reg) \ 471 - ({ \ 472 - unsigned int __val16; \ 473 - __val16 = SMC_inb( ioaddr, reg ); \ 474 - __val16 |= SMC_inb( ioaddr, reg + (1 << SMC_IO_SHIFT)) << 8; \ 475 - __val16; \ 476 - }) 477 - 419 + #define SMC_outw(x, ioaddr, reg) SMC_outw_b(x, ioaddr, reg) 420 + #define SMC_inw(ioaddr, reg) SMC_inw_b(ioaddr, reg) 478 421 #define SMC_insw(a, r, p, l) BUG() 479 422 #define SMC_outsw(a, r, p, l) BUG() 480 423
+10
include/linux/smc91x.h
··· 1 1 #ifndef __SMC91X_H__ 2 2 #define __SMC91X_H__ 3 3 4 + /* 5 + * These bits define which access sizes a platform can support, rather 6 + * than the maximal access size. So, if your platform can do 16-bit 7 + * and 32-bit accesses to the SMC91x device, but not 8-bit, set both 8 + * SMC91X_USE_16BIT and SMC91X_USE_32BIT. 9 + * 10 + * The SMC91x driver requires at least one of SMC91X_USE_8BIT or 11 + * SMC91X_USE_16BIT to be supported - just setting SMC91X_USE_32BIT is 12 + * an invalid configuration. 13 + */ 4 14 #define SMC91X_USE_8BIT (1 << 0) 5 15 #define SMC91X_USE_16BIT (1 << 1) 6 16 #define SMC91X_USE_32BIT (1 << 2)