From mboxrd@z Thu Jan 1 00:00:00 1970 From: arnd@arndb.de (Arnd Bergmann) Date: Thu, 25 Aug 2016 17:39:00 +0200 Subject: [PATCH v2 1/2] smc91x: always use 8-bit access if necessary In-Reply-To: <20160825144633.1850889-1-arnd@arndb.de> References: <20160825144633.1850889-1-arnd@arndb.de> Message-ID: <3107598.uul2xx8LJA@wuerfel> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org As Russell King found out the hard way, a change I did to fix multiplatform builds with this driver broke the old Assabet/Neponset platform: It turns out that while the driver is runtime configurable in principle, the runtime configuration does not cover the specific case of machines that can not do any 16-bit I/O on the smc91x registers. The driver currently provides helpers to access 16-bit registers for architectures that are known at compile-time to only have 8-bit I/O, but my patch changed it to a runtime flag that never gets consulted most register accesses. This introduces new SMC_out16()/SMC_in16 helpers (if anyone can suggest a better name, I'm glad to modify this) that behaves like SMC_outw()/SMC_inw() most of the time, but uses a pair of 8-bit accesses on platforms that have no support for wider register accesses. Signed-off-by: Arnd Bergmann Reported-by: Russell King Fixes: b70661c70830d ("net: smc91x: use run-time configuration on all ARM machines") --- Having bad luck streak with this patch, the version I sent had a couple of mistakes from the last rebase before sending it out, this version should actually apply and build. diff --git a/drivers/net/ethernet/smsc/smc91x.h b/drivers/net/ethernet/smsc/smc91x.h index 1a55c7976df0..497411ed6de2 100644 --- a/drivers/net/ethernet/smsc/smc91x.h +++ b/drivers/net/ethernet/smsc/smc91x.h @@ -414,30 +414,32 @@ smc_pxa_dma_insw(void __iomem *ioaddr, struct smc_local *lp, int reg, int dma, #define SMC_outsl(a, r, p, l) BUG() #endif -#if ! SMC_CAN_USE_16BIT - /* - * Any 16-bit access is performed with two 8-bit accesses if the hardware - * can't do it directly. Most registers are 16-bit so those are mandatory. + * Any 16-bit register access is performed with two 8-bit accesses if the + * hardware can't do it directly. */ -#define SMC_outw(x, ioaddr, reg) \ - do { \ - unsigned int __val16 = (x); \ - SMC_outb( __val16, ioaddr, reg ); \ - SMC_outb( __val16 >> 8, ioaddr, reg + (1 << SMC_IO_SHIFT));\ - } while (0) -#define SMC_inw(ioaddr, reg) \ - ({ \ - unsigned int __val16; \ - __val16 = SMC_inb( ioaddr, reg ); \ +#define SMC_out16(x, ioaddr, reg) \ +do { \ + if (SMC_CAN_USE_8BIT && !SMC_16BIT(lp)) { \ + unsigned int __val16 = (x); \ + SMC_outb(__val16, ioaddr, reg ); \ + SMC_outb(__val16 >> 8, ioaddr, reg + (1 << SMC_IO_SHIFT)); \ + } else { \ + SMC_outw(x, ioaddr, reg); \ + } \ +} while (0) + +#define SMC_in16(ioaddr, reg) \ +({ \ + unsigned int __val16; \ + if (SMC_CAN_USE_8BIT && !SMC_16BIT(lp)) { \ + __val16 = SMC_inb( ioaddr, reg ); \ __val16 |= SMC_inb( ioaddr, reg + (1 << SMC_IO_SHIFT)) << 8; \ - __val16; \ - }) - -#define SMC_insw(a, r, p, l) BUG() -#define SMC_outsw(a, r, p, l) BUG() - -#endif + } else { \ + __val16 = SMC_inw(ioaddr, reg); \ + } \ + __val16; \ +}) #if !defined(SMC_insw) || !defined(SMC_outsw) #define SMC_insw(a, r, p, l) BUG() @@ -927,113 +929,113 @@ static const char * chip_ids[ 16 ] = { SMC_outw((x) << 8, ioaddr, INT_REG(lp)); \ } while (0) -#define SMC_CURRENT_BANK(lp) SMC_inw(ioaddr, BANK_SELECT) +#define SMC_CURRENT_BANK(lp) SMC_in16(ioaddr, BANK_SELECT) #define SMC_SELECT_BANK(lp, x) \ do { \ if (SMC_MUST_ALIGN_WRITE(lp)) \ SMC_outl((x)<<16, ioaddr, 12<> 8; \ - __v = SMC_inw(ioaddr, ADDR1_REG(lp)); \ + __v = SMC_in16(ioaddr, ADDR1_REG(lp)); \ addr[2] = __v; addr[3] = __v >> 8; \ - __v = SMC_inw(ioaddr, ADDR2_REG(lp)); \ + __v = SMC_in16(ioaddr, ADDR2_REG(lp)); \ addr[4] = __v; addr[5] = __v >> 8; \ } while (0) #endif #define SMC_SET_MAC_ADDR(lp, addr) \ do { \ - SMC_outw(addr[0]|(addr[1] << 8), ioaddr, ADDR0_REG(lp)); \ - SMC_outw(addr[2]|(addr[3] << 8), ioaddr, ADDR1_REG(lp)); \ - SMC_outw(addr[4]|(addr[5] << 8), ioaddr, ADDR2_REG(lp)); \ + SMC_out16(addr[0]|(addr[1] << 8), ioaddr, ADDR0_REG(lp)); \ + SMC_out16(addr[2]|(addr[3] << 8), ioaddr, ADDR1_REG(lp)); \ + SMC_out16(addr[4]|(addr[5] << 8), ioaddr, ADDR2_REG(lp)); \ } while (0) #define SMC_SET_MCAST(lp, x) \ do { \ const unsigned char *mt = (x); \ - SMC_outw(mt[0] | (mt[1] << 8), ioaddr, MCAST_REG1(lp)); \ - SMC_outw(mt[2] | (mt[3] << 8), ioaddr, MCAST_REG2(lp)); \ - SMC_outw(mt[4] | (mt[5] << 8), ioaddr, MCAST_REG3(lp)); \ - SMC_outw(mt[6] | (mt[7] << 8), ioaddr, MCAST_REG4(lp)); \ + SMC_out16(mt[0] | (mt[1] << 8), ioaddr, MCAST_REG1(lp)); \ + SMC_out16(mt[2] | (mt[3] << 8), ioaddr, MCAST_REG2(lp)); \ + SMC_out16(mt[4] | (mt[5] << 8), ioaddr, MCAST_REG3(lp)); \ + SMC_out16(mt[6] | (mt[7] << 8), ioaddr, MCAST_REG4(lp)); \ } while (0) #define SMC_PUT_PKT_HDR(lp, status, length) \ @@ -1042,8 +1044,8 @@ static const char * chip_ids[ 16 ] = { SMC_outl((status) | (length)<<16, ioaddr, \ DATA_REG(lp)); \ else { \ - SMC_outw(status, ioaddr, DATA_REG(lp)); \ - SMC_outw(length, ioaddr, DATA_REG(lp)); \ + SMC_out16(status, ioaddr, DATA_REG(lp)); \ + SMC_out16(length, ioaddr, DATA_REG(lp)); \ } \ } while (0) @@ -1054,8 +1056,8 @@ static const char * chip_ids[ 16 ] = { (status) = __val & 0xffff; \ (length) = __val >> 16; \ } else { \ - (status) = SMC_inw(ioaddr, DATA_REG(lp)); \ - (length) = SMC_inw(ioaddr, DATA_REG(lp)); \ + (status) = SMC_in16(ioaddr, DATA_REG(lp)); \ + (length) = SMC_in16(ioaddr, DATA_REG(lp)); \ } \ } while (0)