* [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes @ 2016-08-25 14:43 ` Arnd Bergmann 0 siblings, 0 replies; 22+ messages in thread From: Arnd Bergmann @ 2016-08-25 14:43 UTC (permalink / raw) To: Russell King Cc: linux-arm-kernel, Arnd Bergmann, Nicolas Pitre, David S. Miller, Robert Jarzmik, Yoshinori Sato, netdev, linux-kernel The ARM specific I/O operations are almost the same as the generic ones, with the exception of the SMC_outw macro that works around a problem of some platforms that cannot write to 16-bit registers at an address that is not 32-bit aligned. By inspection, I found that this is handled already in the register abstractions for almost all cases, the exceptions being SMC_SET_MAC_ADDR() and SMC_SET_MCAST(). I assume that all platforms that require the hack for the other registers also need it here, so the ones listed explictly here are the only ones that work correctly, while the other ones either don't need the hack at all, or they will set an incorrect MAC address (which can often go unnoticed). This changes the two macros that set the unaligned registers to use 32-bit writes if possible, which should do the right thing in all combinations. The ARM specific SMC_outw gets removed as a consequence. The only difference between the ARM behavior and the default is the selection of the LED settings. The fact that we have different defaults based on the CPU architectures here is a bit suspicious, but probably harmless, and I have no plan of touching that. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/net/ethernet/smsc/smc91x.h | 50 +++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 20 deletions(-) While this patch fixes one bug on Neponset, it probably doesn't address the one that Russell ran into first, so this is for review only for now, until the remaining problem(s) have been worked out. diff --git a/drivers/net/ethernet/smsc/smc91x.h b/drivers/net/ethernet/smsc/smc91x.h index 22333477d0b5..908473d9ede0 100644 --- a/drivers/net/ethernet/smsc/smc91x.h +++ b/drivers/net/ethernet/smsc/smc91x.h @@ -58,6 +58,7 @@ #define SMC_inw(a, r) readw((a) + (r)) #define SMC_inl(a, r) readl((a) + (r)) #define SMC_outb(v, a, r) writeb(v, (a) + (r)) +#define SMC_outw(v, a, r) writew(v, (a) + (r)) #define SMC_outl(v, a, r) writel(v, (a) + (r)) #define SMC_insw(a, r, p, l) readsw((a) + (r), p, l) #define SMC_outsw(a, r, p, l) writesw((a) + (r), p, l) @@ -65,19 +66,6 @@ #define SMC_outsl(a, r, p, l) writesl((a) + (r), p, l) #define SMC_IRQ_FLAGS (-1) /* from resource */ -/* We actually can't write halfwords properly if not word aligned */ -static inline void SMC_outw(u16 val, void __iomem *ioaddr, int reg) -{ - if ((machine_is_mainstone() || machine_is_stargate2() || - machine_is_pxa_idp()) && reg & 2) { - unsigned int v = val << 16; - v |= readl(ioaddr + (reg & ~2)) & 0xffff; - writel(v, ioaddr + (reg & ~2)); - } else { - writew(val, ioaddr + reg); - } -} - #elif defined(CONFIG_SH_SH4202_MICRODEV) #define SMC_CAN_USE_8BIT 0 @@ -1029,18 +1017,40 @@ static const char * chip_ids[ 16 ] = { #define SMC_SET_MAC_ADDR(lp, addr) \ do { \ - 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)); \ + if (SMC_32BIT(lp)) { \ + SMC_outl((addr[0] )|(addr[1] << 8) | \ + (addr[2] << 16)|(addr[3] << 24), \ + ioaddr, ADDR0_REG(lp)); \ + } else { \ + 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_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)); \ + if (SMC_32BIT(lp)) { \ + SMC_outl((mt[0] ) | (mt[1] << 8) | \ + (mt[2] << 16) | (mt[3] << 24), \ + ioaddr, MCAST_REG1(lp)); \ + SMC_outl((mt[4] ) | (mt[5] << 8) | \ + (mt[6] << 16) | (mt[7] << 24), \ + ioaddr, MCAST_REG3(lp)); \ + } else { \ + 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) \ -- 2.9.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes @ 2016-08-25 14:43 ` Arnd Bergmann 0 siblings, 0 replies; 22+ messages in thread From: Arnd Bergmann @ 2016-08-25 14:43 UTC (permalink / raw) To: linux-arm-kernel The ARM specific I/O operations are almost the same as the generic ones, with the exception of the SMC_outw macro that works around a problem of some platforms that cannot write to 16-bit registers at an address that is not 32-bit aligned. By inspection, I found that this is handled already in the register abstractions for almost all cases, the exceptions being SMC_SET_MAC_ADDR() and SMC_SET_MCAST(). I assume that all platforms that require the hack for the other registers also need it here, so the ones listed explictly here are the only ones that work correctly, while the other ones either don't need the hack at all, or they will set an incorrect MAC address (which can often go unnoticed). This changes the two macros that set the unaligned registers to use 32-bit writes if possible, which should do the right thing in all combinations. The ARM specific SMC_outw gets removed as a consequence. The only difference between the ARM behavior and the default is the selection of the LED settings. The fact that we have different defaults based on the CPU architectures here is a bit suspicious, but probably harmless, and I have no plan of touching that. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/net/ethernet/smsc/smc91x.h | 50 +++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 20 deletions(-) While this patch fixes one bug on Neponset, it probably doesn't address the one that Russell ran into first, so this is for review only for now, until the remaining problem(s) have been worked out. diff --git a/drivers/net/ethernet/smsc/smc91x.h b/drivers/net/ethernet/smsc/smc91x.h index 22333477d0b5..908473d9ede0 100644 --- a/drivers/net/ethernet/smsc/smc91x.h +++ b/drivers/net/ethernet/smsc/smc91x.h @@ -58,6 +58,7 @@ #define SMC_inw(a, r) readw((a) + (r)) #define SMC_inl(a, r) readl((a) + (r)) #define SMC_outb(v, a, r) writeb(v, (a) + (r)) +#define SMC_outw(v, a, r) writew(v, (a) + (r)) #define SMC_outl(v, a, r) writel(v, (a) + (r)) #define SMC_insw(a, r, p, l) readsw((a) + (r), p, l) #define SMC_outsw(a, r, p, l) writesw((a) + (r), p, l) @@ -65,19 +66,6 @@ #define SMC_outsl(a, r, p, l) writesl((a) + (r), p, l) #define SMC_IRQ_FLAGS (-1) /* from resource */ -/* We actually can't write halfwords properly if not word aligned */ -static inline void SMC_outw(u16 val, void __iomem *ioaddr, int reg) -{ - if ((machine_is_mainstone() || machine_is_stargate2() || - machine_is_pxa_idp()) && reg & 2) { - unsigned int v = val << 16; - v |= readl(ioaddr + (reg & ~2)) & 0xffff; - writel(v, ioaddr + (reg & ~2)); - } else { - writew(val, ioaddr + reg); - } -} - #elif defined(CONFIG_SH_SH4202_MICRODEV) #define SMC_CAN_USE_8BIT 0 @@ -1029,18 +1017,40 @@ static const char * chip_ids[ 16 ] = { #define SMC_SET_MAC_ADDR(lp, addr) \ do { \ - 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)); \ + if (SMC_32BIT(lp)) { \ + SMC_outl((addr[0] )|(addr[1] << 8) | \ + (addr[2] << 16)|(addr[3] << 24), \ + ioaddr, ADDR0_REG(lp)); \ + } else { \ + 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_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)); \ + if (SMC_32BIT(lp)) { \ + SMC_outl((mt[0] ) | (mt[1] << 8) | \ + (mt[2] << 16) | (mt[3] << 24), \ + ioaddr, MCAST_REG1(lp)); \ + SMC_outl((mt[4] ) | (mt[5] << 8) | \ + (mt[6] << 16) | (mt[7] << 24), \ + ioaddr, MCAST_REG3(lp)); \ + } else { \ + 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) \ -- 2.9.0 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes 2016-08-25 14:43 ` Arnd Bergmann (?) @ 2016-08-25 14:45 ` Arnd Bergmann -1 siblings, 0 replies; 22+ messages in thread From: Arnd Bergmann @ 2016-08-25 14:45 UTC (permalink / raw) To: Russell King Cc: linux-arm-kernel, Nicolas Pitre, David S. Miller, Robert Jarzmik, Yoshinori Sato, netdev, linux-kernel On Thursday, August 25, 2016 4:43:04 PM CEST Arnd Bergmann wrote: > drivers/net/ethernet/smsc/smc91x.h | 50 +++++++++++++++++++++++--------------- > 1 file changed, 30 insertions(+), 20 deletions(-) > > While this patch fixes one bug on Neponset, it probably doesn't address > the one that Russell ran into first, so this is for review only for now, > until the remaining problem(s) have been worked out. > The comment should have been on another patch, my mistake. please see v2. Arnd ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes @ 2016-08-25 14:45 ` Arnd Bergmann 0 siblings, 0 replies; 22+ messages in thread From: Arnd Bergmann @ 2016-08-25 14:45 UTC (permalink / raw) To: linux-arm-kernel On Thursday, August 25, 2016 4:43:04 PM CEST Arnd Bergmann wrote: > drivers/net/ethernet/smsc/smc91x.h | 50 +++++++++++++++++++++++--------------- > 1 file changed, 30 insertions(+), 20 deletions(-) > > While this patch fixes one bug on Neponset, it probably doesn't address > the one that Russell ran into first, so this is for review only for now, > until the remaining problem(s) have been worked out. > The comment should have been on another patch, my mistake. please see v2. Arnd ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes @ 2016-08-25 14:45 ` Arnd Bergmann 0 siblings, 0 replies; 22+ messages in thread From: Arnd Bergmann @ 2016-08-25 14:45 UTC (permalink / raw) To: Russell King Cc: Yoshinori Sato, Nicolas Pitre, netdev, linux-kernel, Robert Jarzmik, David S. Miller, linux-arm-kernel On Thursday, August 25, 2016 4:43:04 PM CEST Arnd Bergmann wrote: > drivers/net/ethernet/smsc/smc91x.h | 50 +++++++++++++++++++++++--------------- > 1 file changed, 30 insertions(+), 20 deletions(-) > > While this patch fixes one bug on Neponset, it probably doesn't address > the one that Russell ran into first, so this is for review only for now, > until the remaining problem(s) have been worked out. > The comment should have been on another patch, my mistake. please see v2. Arnd ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes 2016-08-25 14:45 ` Arnd Bergmann @ 2016-08-25 18:02 ` Robert Jarzmik -1 siblings, 0 replies; 22+ messages in thread From: Robert Jarzmik @ 2016-08-25 18:02 UTC (permalink / raw) To: Arnd Bergmann Cc: Russell King, linux-arm-kernel, Nicolas Pitre, David S. Miller, Yoshinori Sato, netdev, linux-kernel Arnd Bergmann <arnd@arndb.de> writes: > On Thursday, August 25, 2016 4:43:04 PM CEST Arnd Bergmann wrote: >> drivers/net/ethernet/smsc/smc91x.h | 50 +++++++++++++++++++++++--------------- >> 1 file changed, 30 insertions(+), 20 deletions(-) >> >> While this patch fixes one bug on Neponset, it probably doesn't address >> the one that Russell ran into first, so this is for review only for now, >> until the remaining problem(s) have been worked out. >> > > The comment should have been on another patch, my mistake. please > see v2. > > Arnd Hi Arnd, I didn't review the patch thoroughly, but I launched your 2 patches in my pxa little farm. The result is that lubbock and mainstone are all right, but zylonite is broken (ie. networkless). I removed then these 2 patches and zylonite worked again. I have also an error message on the console on a "broken" zylonite : Changing smcs91x MAC address to 08:00:3e:26:0a:5b: ifconfig: SIOCSIFHWADDR: Device or resource busy I reran the test twice (2 times with your patches, 2 times without), the result looks consistent, ie. zylonite doesn't really like them. Cheers. -- Robert PS: even if zylonite defconfig is not in the tree, it uses this driver, with : CONFIG_SMC91X=y ...and... static struct smc91x_platdata zylonite_smc91x_info = { .flags = SMC91X_USE_8BIT | SMC91X_USE_16BIT | SMC91X_NOWAIT | SMC91X_USE_DMA, }; ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes @ 2016-08-25 18:02 ` Robert Jarzmik 0 siblings, 0 replies; 22+ messages in thread From: Robert Jarzmik @ 2016-08-25 18:02 UTC (permalink / raw) To: linux-arm-kernel Arnd Bergmann <arnd@arndb.de> writes: > On Thursday, August 25, 2016 4:43:04 PM CEST Arnd Bergmann wrote: >> drivers/net/ethernet/smsc/smc91x.h | 50 +++++++++++++++++++++++--------------- >> 1 file changed, 30 insertions(+), 20 deletions(-) >> >> While this patch fixes one bug on Neponset, it probably doesn't address >> the one that Russell ran into first, so this is for review only for now, >> until the remaining problem(s) have been worked out. >> > > The comment should have been on another patch, my mistake. please > see v2. > > Arnd Hi Arnd, I didn't review the patch thoroughly, but I launched your 2 patches in my pxa little farm. The result is that lubbock and mainstone are all right, but zylonite is broken (ie. networkless). I removed then these 2 patches and zylonite worked again. I have also an error message on the console on a "broken" zylonite : Changing smcs91x MAC address to 08:00:3e:26:0a:5b: ifconfig: SIOCSIFHWADDR: Device or resource busy I reran the test twice (2 times with your patches, 2 times without), the result looks consistent, ie. zylonite doesn't really like them. Cheers. -- Robert PS: even if zylonite defconfig is not in the tree, it uses this driver, with : CONFIG_SMC91X=y ...and... static struct smc91x_platdata zylonite_smc91x_info = { .flags = SMC91X_USE_8BIT | SMC91X_USE_16BIT | SMC91X_NOWAIT | SMC91X_USE_DMA, }; ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes 2016-08-25 18:02 ` Robert Jarzmik @ 2016-08-25 22:37 ` Russell King - ARM Linux -1 siblings, 0 replies; 22+ messages in thread From: Russell King - ARM Linux @ 2016-08-25 22:37 UTC (permalink / raw) To: Robert Jarzmik Cc: Arnd Bergmann, linux-arm-kernel, Nicolas Pitre, David S. Miller, Yoshinori Sato, netdev, linux-kernel On Thu, Aug 25, 2016 at 08:02:35PM +0200, Robert Jarzmik wrote: > Arnd Bergmann <arnd@arndb.de> writes: > > > On Thursday, August 25, 2016 4:43:04 PM CEST Arnd Bergmann wrote: > >> drivers/net/ethernet/smsc/smc91x.h | 50 +++++++++++++++++++++++--------------- > >> 1 file changed, 30 insertions(+), 20 deletions(-) > >> > >> While this patch fixes one bug on Neponset, it probably doesn't address > >> the one that Russell ran into first, so this is for review only for now, > >> until the remaining problem(s) have been worked out. > >> > > > > The comment should have been on another patch, my mistake. please > > see v2. > > > > Arnd > > Hi Arnd, > > I didn't review the patch thoroughly, but I launched your 2 patches in my pxa > little farm. > > The result is that lubbock and mainstone are all right, but zylonite is broken > (ie. networkless). I removed then these 2 patches and zylonite worked again. > > I have also an error message on the console on a "broken" zylonite : > Changing smcs91x MAC address to 08:00:3e:26:0a:5b: ifconfig: SIOCSIFHWADDR: > Device or resource busy > > I reran the test twice (2 times with your patches, 2 times without), the result > looks consistent, ie. zylonite doesn't really like them. Please try the patch below. I sent this to Will a few days ago, as he said (on irc) that he was also seeing problems on a platform he had, but I've yet to hear back. I've not posted it yet because I haven't got around to writing a commit description for it. It does require that at least one of 8-bit or 16-bit accesses are supported, but I think that's already true. 8<======== From: Russell King <rmk+kernel@armlinux.org.uk> Subject: [PATCH] net: smc91x: fix SMC accesses Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> --- drivers/net/ethernet/smsc/smc91x.h | 65 ++++++++++++++++++++++++++------------ 1 file changed, 45 insertions(+), 20 deletions(-) diff --git a/drivers/net/ethernet/smsc/smc91x.h b/drivers/net/ethernet/smsc/smc91x.h index 1a55c7976df0..e17671c9d1b0 100644 --- a/drivers/net/ethernet/smsc/smc91x.h +++ b/drivers/net/ethernet/smsc/smc91x.h @@ -37,6 +37,27 @@ #include <linux/smc91x.h> /* + * 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. + */ +#define SMC_outw_b(x, a, r) \ + do { \ + unsigned int __val16 = (x); \ + unsigned int __reg = (r); \ + SMC_outb(__val16, a, __reg); \ + SMC_outb(__val16 >> 8, a, __reg + (1 << SMC_IO_SHIFT)); \ + } while (0) + +#define SMC_inw_b(a, r) \ + ({ \ + unsigned int __val16; \ + unsigned int __reg = r; \ + __val16 = SMC_inb(a, __reg); \ + __val16 |= SMC_inb(a, __reg + (1 << SMC_IO_SHIFT)) << 8; \ + __val16; \ + }) + +/* * Define your architecture specific bus configuration parameters here. */ @@ -55,10 +76,30 @@ #define SMC_IO_SHIFT (lp->io_shift) #define SMC_inb(a, r) readb((a) + (r)) -#define SMC_inw(a, r) readw((a) + (r)) +#define SMC_inw(a, r) \ + ({ \ + unsigned int __smc_r = r; \ + SMC_16BIT(lp) ? readw((a) + __smc_r) : \ + SMC_8BIT(lp) ? SMC_inw_b(a, __smc_r) : \ + ({ BUG(); 0; }); \ + }) + #define SMC_inl(a, r) readl((a) + (r)) #define SMC_outb(v, a, r) writeb(v, (a) + (r)) +#define SMC_outw(v, a, r) \ + do { \ + unsigned int __v = v, __smc_r = r; \ + if (SMC_16BIT(lp)) \ + __SMC_outw(__v, a, __smc_r); \ + else if (SMC_8BIT(lp)) \ + SMC_outw_b(__v, a, __smc_r); \ + else \ + BUG(); \ + } while (0) + #define SMC_outl(v, a, r) writel(v, (a) + (r)) +#define SMC_insb(a, r, p, l) readsb((a) + (r), p, l) +#define SMC_outsb(a, r, p, l) writesb((a) + (r), p, l) #define SMC_insw(a, r, p, l) readsw((a) + (r), p, l) #define SMC_outsw(a, r, p, l) writesw((a) + (r), p, l) #define SMC_insl(a, r, p, l) readsl((a) + (r), p, l) @@ -66,7 +107,7 @@ #define SMC_IRQ_FLAGS (-1) /* from resource */ /* We actually can't write halfwords properly if not word aligned */ -static inline void SMC_outw(u16 val, void __iomem *ioaddr, int reg) +static inline void __SMC_outw(u16 val, void __iomem *ioaddr, int reg) { if ((machine_is_mainstone() || machine_is_stargate2() || machine_is_pxa_idp()) && reg & 2) { @@ -416,24 +457,8 @@ smc_pxa_dma_insw(void __iomem *ioaddr, struct smc_local *lp, int reg, int dma, #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. - */ -#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 ); \ - __val16 |= SMC_inb( ioaddr, reg + (1 << SMC_IO_SHIFT)) << 8; \ - __val16; \ - }) - +#define SMC_outw(x, ioaddr, reg) SMC_outw_b(x, ioaddr, reg) +#define SMC_inw(ioaddr, reg) SMC_inw_b(ioaddr, reg) #define SMC_insw(a, r, p, l) BUG() #define SMC_outsw(a, r, p, l) BUG() -- 2.1.0 -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes @ 2016-08-25 22:37 ` Russell King - ARM Linux 0 siblings, 0 replies; 22+ messages in thread From: Russell King - ARM Linux @ 2016-08-25 22:37 UTC (permalink / raw) To: linux-arm-kernel On Thu, Aug 25, 2016 at 08:02:35PM +0200, Robert Jarzmik wrote: > Arnd Bergmann <arnd@arndb.de> writes: > > > On Thursday, August 25, 2016 4:43:04 PM CEST Arnd Bergmann wrote: > >> drivers/net/ethernet/smsc/smc91x.h | 50 +++++++++++++++++++++++--------------- > >> 1 file changed, 30 insertions(+), 20 deletions(-) > >> > >> While this patch fixes one bug on Neponset, it probably doesn't address > >> the one that Russell ran into first, so this is for review only for now, > >> until the remaining problem(s) have been worked out. > >> > > > > The comment should have been on another patch, my mistake. please > > see v2. > > > > Arnd > > Hi Arnd, > > I didn't review the patch thoroughly, but I launched your 2 patches in my pxa > little farm. > > The result is that lubbock and mainstone are all right, but zylonite is broken > (ie. networkless). I removed then these 2 patches and zylonite worked again. > > I have also an error message on the console on a "broken" zylonite : > Changing smcs91x MAC address to 08:00:3e:26:0a:5b: ifconfig: SIOCSIFHWADDR: > Device or resource busy > > I reran the test twice (2 times with your patches, 2 times without), the result > looks consistent, ie. zylonite doesn't really like them. Please try the patch below. I sent this to Will a few days ago, as he said (on irc) that he was also seeing problems on a platform he had, but I've yet to hear back. I've not posted it yet because I haven't got around to writing a commit description for it. It does require that at least one of 8-bit or 16-bit accesses are supported, but I think that's already true. 8<======== From: Russell King <rmk+kernel@armlinux.org.uk> Subject: [PATCH] net: smc91x: fix SMC accesses Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> --- drivers/net/ethernet/smsc/smc91x.h | 65 ++++++++++++++++++++++++++------------ 1 file changed, 45 insertions(+), 20 deletions(-) diff --git a/drivers/net/ethernet/smsc/smc91x.h b/drivers/net/ethernet/smsc/smc91x.h index 1a55c7976df0..e17671c9d1b0 100644 --- a/drivers/net/ethernet/smsc/smc91x.h +++ b/drivers/net/ethernet/smsc/smc91x.h @@ -37,6 +37,27 @@ #include <linux/smc91x.h> /* + * 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. + */ +#define SMC_outw_b(x, a, r) \ + do { \ + unsigned int __val16 = (x); \ + unsigned int __reg = (r); \ + SMC_outb(__val16, a, __reg); \ + SMC_outb(__val16 >> 8, a, __reg + (1 << SMC_IO_SHIFT)); \ + } while (0) + +#define SMC_inw_b(a, r) \ + ({ \ + unsigned int __val16; \ + unsigned int __reg = r; \ + __val16 = SMC_inb(a, __reg); \ + __val16 |= SMC_inb(a, __reg + (1 << SMC_IO_SHIFT)) << 8; \ + __val16; \ + }) + +/* * Define your architecture specific bus configuration parameters here. */ @@ -55,10 +76,30 @@ #define SMC_IO_SHIFT (lp->io_shift) #define SMC_inb(a, r) readb((a) + (r)) -#define SMC_inw(a, r) readw((a) + (r)) +#define SMC_inw(a, r) \ + ({ \ + unsigned int __smc_r = r; \ + SMC_16BIT(lp) ? readw((a) + __smc_r) : \ + SMC_8BIT(lp) ? SMC_inw_b(a, __smc_r) : \ + ({ BUG(); 0; }); \ + }) + #define SMC_inl(a, r) readl((a) + (r)) #define SMC_outb(v, a, r) writeb(v, (a) + (r)) +#define SMC_outw(v, a, r) \ + do { \ + unsigned int __v = v, __smc_r = r; \ + if (SMC_16BIT(lp)) \ + __SMC_outw(__v, a, __smc_r); \ + else if (SMC_8BIT(lp)) \ + SMC_outw_b(__v, a, __smc_r); \ + else \ + BUG(); \ + } while (0) + #define SMC_outl(v, a, r) writel(v, (a) + (r)) +#define SMC_insb(a, r, p, l) readsb((a) + (r), p, l) +#define SMC_outsb(a, r, p, l) writesb((a) + (r), p, l) #define SMC_insw(a, r, p, l) readsw((a) + (r), p, l) #define SMC_outsw(a, r, p, l) writesw((a) + (r), p, l) #define SMC_insl(a, r, p, l) readsl((a) + (r), p, l) @@ -66,7 +107,7 @@ #define SMC_IRQ_FLAGS (-1) /* from resource */ /* We actually can't write halfwords properly if not word aligned */ -static inline void SMC_outw(u16 val, void __iomem *ioaddr, int reg) +static inline void __SMC_outw(u16 val, void __iomem *ioaddr, int reg) { if ((machine_is_mainstone() || machine_is_stargate2() || machine_is_pxa_idp()) && reg & 2) { @@ -416,24 +457,8 @@ smc_pxa_dma_insw(void __iomem *ioaddr, struct smc_local *lp, int reg, int dma, #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. - */ -#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 ); \ - __val16 |= SMC_inb( ioaddr, reg + (1 << SMC_IO_SHIFT)) << 8; \ - __val16; \ - }) - +#define SMC_outw(x, ioaddr, reg) SMC_outw_b(x, ioaddr, reg) +#define SMC_inw(ioaddr, reg) SMC_inw_b(ioaddr, reg) #define SMC_insw(a, r, p, l) BUG() #define SMC_outsw(a, r, p, l) BUG() -- 2.1.0 -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently@9.6Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes 2016-08-25 22:37 ` Russell King - ARM Linux @ 2016-08-26 9:41 ` Arnd Bergmann -1 siblings, 0 replies; 22+ messages in thread From: Arnd Bergmann @ 2016-08-26 9:41 UTC (permalink / raw) To: linux-arm-kernel Cc: Russell King - ARM Linux, Robert Jarzmik, Yoshinori Sato, Nicolas Pitre, netdev, linux-kernel, David S. Miller On Thursday, August 25, 2016 11:37:43 PM CEST Russell King - ARM Linux wrote: > On Thu, Aug 25, 2016 at 08:02:35PM +0200, Robert Jarzmik wrote: > > Arnd Bergmann <arnd@arndb.de> writes: > /* > + * 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. > + */ > +#define SMC_outw_b(x, a, r) \ > + do { \ > + unsigned int __val16 = (x); \ > + unsigned int __reg = (r); \ > + SMC_outb(__val16, a, __reg); \ > + SMC_outb(__val16 >> 8, a, __reg + (1 << SMC_IO_SHIFT)); \ > + } while (0) > + > +#define SMC_inw_b(a, r) \ > + ({ \ > + unsigned int __val16; \ > + unsigned int __reg = r; \ > + __val16 = SMC_inb(a, __reg); \ > + __val16 |= SMC_inb(a, __reg + (1 << SMC_IO_SHIFT)) << 8; \ > + __val16; \ > + }) > + > +/* > * Define your architecture specific bus configuration parameters here. > */ > > @@ -55,10 +76,30 @@ > #define SMC_IO_SHIFT (lp->io_shift) > > #define SMC_inb(a, r) readb((a) + (r)) > -#define SMC_inw(a, r) readw((a) + (r)) > +#define SMC_inw(a, r) \ > + ({ \ > + unsigned int __smc_r = r; \ > + SMC_16BIT(lp) ? readw((a) + __smc_r) : \ > + SMC_8BIT(lp) ? SMC_inw_b(a, __smc_r) : \ > + ({ BUG(); 0; }); \ > + }) > + I think this breaks machines that declare a device that just lists SMC91X_USE_32BIT but not SMC91X_USE_16BIT. Right now, the way this is interpreted is to use 32-bit accessors for most things, but not avoiding 16-bit reads. That is a bit fishy though, and we could instead change the platform data to always set both SMC91X_USE_32BIT and SMC91X_USE_16BIT. The affected platforms are DT based machines with 32-bit I/O and these board files: arch/arm/mach-pxa/idp.c: .flags = SMC91X_USE_32BIT | SMC91X_USE_DMA | SMC91X_NOWAIT, arch/arm/mach-pxa/xcep.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT | SMC91X_USE_DMA, arch/arm/mach-realview/core.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, arch/blackfin/mach-bf561/boards/cm_bf561.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, arch/blackfin/mach-bf561/boards/ezkit.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, Arnd ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes @ 2016-08-26 9:41 ` Arnd Bergmann 0 siblings, 0 replies; 22+ messages in thread From: Arnd Bergmann @ 2016-08-26 9:41 UTC (permalink / raw) To: linux-arm-kernel On Thursday, August 25, 2016 11:37:43 PM CEST Russell King - ARM Linux wrote: > On Thu, Aug 25, 2016 at 08:02:35PM +0200, Robert Jarzmik wrote: > > Arnd Bergmann <arnd@arndb.de> writes: > /* > + * 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. > + */ > +#define SMC_outw_b(x, a, r) \ > + do { \ > + unsigned int __val16 = (x); \ > + unsigned int __reg = (r); \ > + SMC_outb(__val16, a, __reg); \ > + SMC_outb(__val16 >> 8, a, __reg + (1 << SMC_IO_SHIFT)); \ > + } while (0) > + > +#define SMC_inw_b(a, r) \ > + ({ \ > + unsigned int __val16; \ > + unsigned int __reg = r; \ > + __val16 = SMC_inb(a, __reg); \ > + __val16 |= SMC_inb(a, __reg + (1 << SMC_IO_SHIFT)) << 8; \ > + __val16; \ > + }) > + > +/* > * Define your architecture specific bus configuration parameters here. > */ > > @@ -55,10 +76,30 @@ > #define SMC_IO_SHIFT (lp->io_shift) > > #define SMC_inb(a, r) readb((a) + (r)) > -#define SMC_inw(a, r) readw((a) + (r)) > +#define SMC_inw(a, r) \ > + ({ \ > + unsigned int __smc_r = r; \ > + SMC_16BIT(lp) ? readw((a) + __smc_r) : \ > + SMC_8BIT(lp) ? SMC_inw_b(a, __smc_r) : \ > + ({ BUG(); 0; }); \ > + }) > + I think this breaks machines that declare a device that just lists SMC91X_USE_32BIT but not SMC91X_USE_16BIT. Right now, the way this is interpreted is to use 32-bit accessors for most things, but not avoiding 16-bit reads. That is a bit fishy though, and we could instead change the platform data to always set both SMC91X_USE_32BIT and SMC91X_USE_16BIT. The affected platforms are DT based machines with 32-bit I/O and these board files: arch/arm/mach-pxa/idp.c: .flags = SMC91X_USE_32BIT | SMC91X_USE_DMA | SMC91X_NOWAIT, arch/arm/mach-pxa/xcep.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT | SMC91X_USE_DMA, arch/arm/mach-realview/core.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, arch/blackfin/mach-bf561/boards/cm_bf561.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, arch/blackfin/mach-bf561/boards/ezkit.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, Arnd ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes 2016-08-26 9:41 ` Arnd Bergmann @ 2016-08-26 9:56 ` Arnd Bergmann -1 siblings, 0 replies; 22+ messages in thread From: Arnd Bergmann @ 2016-08-26 9:56 UTC (permalink / raw) To: linux-arm-kernel Cc: Russell King - ARM Linux, Robert Jarzmik, Yoshinori Sato, Nicolas Pitre, netdev, linux-kernel, David S. Miller On Friday, August 26, 2016 11:41:21 AM CEST Arnd Bergmann wrote: > > I think this breaks machines that declare a device that just lists > SMC91X_USE_32BIT but not SMC91X_USE_16BIT. Right now, the way this > is interpreted is to use 32-bit accessors for most things, but > not avoiding 16-bit reads. I guess my patch has the same problem here, with the if (SMC_CAN_USE_8BIT && !SMC_16BIT(lp)) check that is true when a platform device is declared with 32-bit I/O only but all three are enabled at compile-time. The best check I can think of here (aside from redefining how the flags work) would be #define SMC_8BIT_ONLY(p) \ (((p)->cfg.flags & (SMC91X_USE_8BIT |SMC91X_USE_16BIT | SMC91X_USE_32BIT)) == \ SMC91X_USE_8BIT) Arnd ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes @ 2016-08-26 9:56 ` Arnd Bergmann 0 siblings, 0 replies; 22+ messages in thread From: Arnd Bergmann @ 2016-08-26 9:56 UTC (permalink / raw) To: linux-arm-kernel On Friday, August 26, 2016 11:41:21 AM CEST Arnd Bergmann wrote: > > I think this breaks machines that declare a device that just lists > SMC91X_USE_32BIT but not SMC91X_USE_16BIT. Right now, the way this > is interpreted is to use 32-bit accessors for most things, but > not avoiding 16-bit reads. I guess my patch has the same problem here, with the if (SMC_CAN_USE_8BIT && !SMC_16BIT(lp)) check that is true when a platform device is declared with 32-bit I/O only but all three are enabled at compile-time. The best check I can think of here (aside from redefining how the flags work) would be #define SMC_8BIT_ONLY(p) \ (((p)->cfg.flags & (SMC91X_USE_8BIT |SMC91X_USE_16BIT | SMC91X_USE_32BIT)) == \ SMC91X_USE_8BIT) Arnd ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes 2016-08-26 9:41 ` Arnd Bergmann @ 2016-08-26 11:38 ` Russell King - ARM Linux -1 siblings, 0 replies; 22+ messages in thread From: Russell King - ARM Linux @ 2016-08-26 11:38 UTC (permalink / raw) To: Arnd Bergmann Cc: linux-arm-kernel, Robert Jarzmik, Yoshinori Sato, Nicolas Pitre, netdev, linux-kernel, David S. Miller On Fri, Aug 26, 2016 at 11:41:21AM +0200, Arnd Bergmann wrote: > On Thursday, August 25, 2016 11:37:43 PM CEST Russell King - ARM Linux wrote: > > On Thu, Aug 25, 2016 at 08:02:35PM +0200, Robert Jarzmik wrote: > > > Arnd Bergmann <arnd@arndb.de> writes: > > /* > > + * 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. > > + */ > > +#define SMC_outw_b(x, a, r) \ > > + do { \ > > + unsigned int __val16 = (x); \ > > + unsigned int __reg = (r); \ > > + SMC_outb(__val16, a, __reg); \ > > + SMC_outb(__val16 >> 8, a, __reg + (1 << SMC_IO_SHIFT)); \ > > + } while (0) > > + > > +#define SMC_inw_b(a, r) \ > > + ({ \ > > + unsigned int __val16; \ > > + unsigned int __reg = r; \ > > + __val16 = SMC_inb(a, __reg); \ > > + __val16 |= SMC_inb(a, __reg + (1 << SMC_IO_SHIFT)) << 8; \ > > + __val16; \ > > + }) > > + > > +/* > > * Define your architecture specific bus configuration parameters here. > > */ > > > > @@ -55,10 +76,30 @@ > > #define SMC_IO_SHIFT (lp->io_shift) > > > > #define SMC_inb(a, r) readb((a) + (r)) > > -#define SMC_inw(a, r) readw((a) + (r)) > > +#define SMC_inw(a, r) \ > > + ({ \ > > + unsigned int __smc_r = r; \ > > + SMC_16BIT(lp) ? readw((a) + __smc_r) : \ > > + SMC_8BIT(lp) ? SMC_inw_b(a, __smc_r) : \ > > + ({ BUG(); 0; }); \ > > + }) > > + > > I think this breaks machines that declare a device that just lists > SMC91X_USE_32BIT but not SMC91X_USE_16BIT. Right now, the way this > is interpreted is to use 32-bit accessors for most things, but > not avoiding 16-bit reads. Your comment is wrong. It breaks machines that declare a device with SMC91X_USE_32BIT but not _either_ of SMC91X_USE_16BIT _or_ SMC91X_USE_8BIT. As I already noted, but you obviously ignored. In that note, I pointed out that this _was_ already the case with the original driver before your patch: #if ! SMC_CAN_USE_16BIT ... code implementing SMC_outw() and SMC_inw() in terms of SMC_outb() and SMC_inb(), and defining SMC_insw() and SMC_outsw() as BUG() #endif #if !defined(SMC_insw) || !defined(SMC_outsw) #define SMC_insw(a, r, p, l) BUG() #define SMC_outsw(a, r, p, l) BUG() #endif #if ! SMC_CAN_USE_8BIT #define SMC_inb(ioaddr, reg) ({ BUG(); 0; }) #define SMC_outb(x, ioaddr, reg) BUG() #define SMC_insb(a, r, p, l) BUG() #define SMC_outsb(a, r, p, l) BUG() #endif #if !defined(SMC_insb) || !defined(SMC_outsb) #define SMC_insb(a, r, p, l) BUG() #define SMC_outsb(a, r, p, l) BUG() #endif So, if _neither_ SMC_CAN_USE_16BIT nor SMC_CAN_USE_8BIT are defined, trying to use the 16-bit accessors cause a BUG(). > That is a bit fishy though, and we could instead change the platform > data to always set both SMC91X_USE_32BIT and SMC91X_USE_16BIT. > > The affected platforms are DT based machines with 32-bit I/O and > these board files: > > arch/arm/mach-pxa/idp.c: .flags = SMC91X_USE_32BIT | SMC91X_USE_DMA | SMC91X_NOWAIT, > arch/arm/mach-pxa/xcep.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT | SMC91X_USE_DMA, > arch/arm/mach-realview/core.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, > arch/blackfin/mach-bf561/boards/cm_bf561.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, > arch/blackfin/mach-bf561/boards/ezkit.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, Right, this looks like another one of your bugs in this conversion. #elif defined(CONFIG_ARCH_INNOKOM) || \ defined(CONFIG_ARCH_PXA_IDP) || \ defined(CONFIG_ARCH_RAMSES) || \ defined(CONFIG_ARCH_PCM027) #define SMC_CAN_USE_8BIT 1 #define SMC_CAN_USE_16BIT 1 #define SMC_CAN_USE_32BIT 1 So, IDP can use all of 32-bit, 16-bit and 8-bit accesses, meanwhile your conversion is telling the driver that it can only use 32-bit => your conversion of IDP is broken. Before your conversion, realview depended on the default settings of smc91x, which is again: #define SMC_CAN_USE_8BIT 1 #define SMC_CAN_USE_16BIT 1 #define SMC_CAN_USE_32BIT 1 So realview follows IDP - it can do 32-bit, 16-bit and 8-bit accesses, meanwhile your conversion tells the driver it can _only_ do 32-bit accesses, which is again broken. XCEP falls into the same category, as do the other two blackfin cases from what I can see. The point of the SMC_CAN_USE_xxx macros is to indicate to the SMC91x driver which sizes of access can be used on the hardware concerned. If SMC_CAN_USE_16BIT is set, then 16-bit accesses are possible. If it's not set, then 8-bit accesses will be used if they're supported, otherwise it's a buggy configuration. SMC_CAN_USE_32BIT says that 32-bit accesses are possible, but these are only used in a small number of cases as an optimisation. It is a bug to configure the smc91x driver with both SMC_CAN_USE_16BIT and SMC_CAN_USE_8BIT clear. Hence, it's a bug to tell the driver (via the platform data) that only 32-bit accesses can be performed. So, while my patch may be fixing some of the brokenness, the rest of the brokenness also needs fixing by adjusting all the platform data to properly reflect which access sizes are possible, rather than what you seem to have done, which is to indicate what the largest possible access size is. This is especially important as there are platforms around where 16-bit accesses to the SMC91x can be performed, but not 8-bit: #elif defined(CONFIG_MACH_LOGICPD_PXA270) || \ defined(CONFIG_MACH_NOMADIK_8815NHK) #define SMC_CAN_USE_8BIT 0 #define SMC_CAN_USE_16BIT 1 #define SMC_CAN_USE_32BIT 0 #elif defined(CONFIG_SH_SH4202_MICRODEV) #define SMC_CAN_USE_8BIT 0 #define SMC_CAN_USE_16BIT 1 #define SMC_CAN_USE_32BIT 0 #elif defined(CONFIG_M32R) #define SMC_CAN_USE_8BIT 0 #define SMC_CAN_USE_16BIT 1 #define SMC_CAN_USE_32BIT 0 #elif defined(CONFIG_ARCH_MSM) #define SMC_CAN_USE_8BIT 0 #define SMC_CAN_USE_16BIT 1 #define SMC_CAN_USE_32BIT 0 #elif defined(CONFIG_COLDFIRE) #define SMC_CAN_USE_8BIT 0 #define SMC_CAN_USE_16BIT 1 #define SMC_CAN_USE_32BIT 0 ... etc ... -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes @ 2016-08-26 11:38 ` Russell King - ARM Linux 0 siblings, 0 replies; 22+ messages in thread From: Russell King - ARM Linux @ 2016-08-26 11:38 UTC (permalink / raw) To: linux-arm-kernel On Fri, Aug 26, 2016 at 11:41:21AM +0200, Arnd Bergmann wrote: > On Thursday, August 25, 2016 11:37:43 PM CEST Russell King - ARM Linux wrote: > > On Thu, Aug 25, 2016 at 08:02:35PM +0200, Robert Jarzmik wrote: > > > Arnd Bergmann <arnd@arndb.de> writes: > > /* > > + * 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. > > + */ > > +#define SMC_outw_b(x, a, r) \ > > + do { \ > > + unsigned int __val16 = (x); \ > > + unsigned int __reg = (r); \ > > + SMC_outb(__val16, a, __reg); \ > > + SMC_outb(__val16 >> 8, a, __reg + (1 << SMC_IO_SHIFT)); \ > > + } while (0) > > + > > +#define SMC_inw_b(a, r) \ > > + ({ \ > > + unsigned int __val16; \ > > + unsigned int __reg = r; \ > > + __val16 = SMC_inb(a, __reg); \ > > + __val16 |= SMC_inb(a, __reg + (1 << SMC_IO_SHIFT)) << 8; \ > > + __val16; \ > > + }) > > + > > +/* > > * Define your architecture specific bus configuration parameters here. > > */ > > > > @@ -55,10 +76,30 @@ > > #define SMC_IO_SHIFT (lp->io_shift) > > > > #define SMC_inb(a, r) readb((a) + (r)) > > -#define SMC_inw(a, r) readw((a) + (r)) > > +#define SMC_inw(a, r) \ > > + ({ \ > > + unsigned int __smc_r = r; \ > > + SMC_16BIT(lp) ? readw((a) + __smc_r) : \ > > + SMC_8BIT(lp) ? SMC_inw_b(a, __smc_r) : \ > > + ({ BUG(); 0; }); \ > > + }) > > + > > I think this breaks machines that declare a device that just lists > SMC91X_USE_32BIT but not SMC91X_USE_16BIT. Right now, the way this > is interpreted is to use 32-bit accessors for most things, but > not avoiding 16-bit reads. Your comment is wrong. It breaks machines that declare a device with SMC91X_USE_32BIT but not _either_ of SMC91X_USE_16BIT _or_ SMC91X_USE_8BIT. As I already noted, but you obviously ignored. In that note, I pointed out that this _was_ already the case with the original driver before your patch: #if ! SMC_CAN_USE_16BIT ... code implementing SMC_outw() and SMC_inw() in terms of SMC_outb() and SMC_inb(), and defining SMC_insw() and SMC_outsw() as BUG() #endif #if !defined(SMC_insw) || !defined(SMC_outsw) #define SMC_insw(a, r, p, l) BUG() #define SMC_outsw(a, r, p, l) BUG() #endif #if ! SMC_CAN_USE_8BIT #define SMC_inb(ioaddr, reg) ({ BUG(); 0; }) #define SMC_outb(x, ioaddr, reg) BUG() #define SMC_insb(a, r, p, l) BUG() #define SMC_outsb(a, r, p, l) BUG() #endif #if !defined(SMC_insb) || !defined(SMC_outsb) #define SMC_insb(a, r, p, l) BUG() #define SMC_outsb(a, r, p, l) BUG() #endif So, if _neither_ SMC_CAN_USE_16BIT nor SMC_CAN_USE_8BIT are defined, trying to use the 16-bit accessors cause a BUG(). > That is a bit fishy though, and we could instead change the platform > data to always set both SMC91X_USE_32BIT and SMC91X_USE_16BIT. > > The affected platforms are DT based machines with 32-bit I/O and > these board files: > > arch/arm/mach-pxa/idp.c: .flags = SMC91X_USE_32BIT | SMC91X_USE_DMA | SMC91X_NOWAIT, > arch/arm/mach-pxa/xcep.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT | SMC91X_USE_DMA, > arch/arm/mach-realview/core.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, > arch/blackfin/mach-bf561/boards/cm_bf561.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, > arch/blackfin/mach-bf561/boards/ezkit.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, Right, this looks like another one of your bugs in this conversion. #elif defined(CONFIG_ARCH_INNOKOM) || \ defined(CONFIG_ARCH_PXA_IDP) || \ defined(CONFIG_ARCH_RAMSES) || \ defined(CONFIG_ARCH_PCM027) #define SMC_CAN_USE_8BIT 1 #define SMC_CAN_USE_16BIT 1 #define SMC_CAN_USE_32BIT 1 So, IDP can use all of 32-bit, 16-bit and 8-bit accesses, meanwhile your conversion is telling the driver that it can only use 32-bit => your conversion of IDP is broken. Before your conversion, realview depended on the default settings of smc91x, which is again: #define SMC_CAN_USE_8BIT 1 #define SMC_CAN_USE_16BIT 1 #define SMC_CAN_USE_32BIT 1 So realview follows IDP - it can do 32-bit, 16-bit and 8-bit accesses, meanwhile your conversion tells the driver it can _only_ do 32-bit accesses, which is again broken. XCEP falls into the same category, as do the other two blackfin cases from what I can see. The point of the SMC_CAN_USE_xxx macros is to indicate to the SMC91x driver which sizes of access can be used on the hardware concerned. If SMC_CAN_USE_16BIT is set, then 16-bit accesses are possible. If it's not set, then 8-bit accesses will be used if they're supported, otherwise it's a buggy configuration. SMC_CAN_USE_32BIT says that 32-bit accesses are possible, but these are only used in a small number of cases as an optimisation. It is a bug to configure the smc91x driver with both SMC_CAN_USE_16BIT and SMC_CAN_USE_8BIT clear. Hence, it's a bug to tell the driver (via the platform data) that only 32-bit accesses can be performed. So, while my patch may be fixing some of the brokenness, the rest of the brokenness also needs fixing by adjusting all the platform data to properly reflect which access sizes are possible, rather than what you seem to have done, which is to indicate what the largest possible access size is. This is especially important as there are platforms around where 16-bit accesses to the SMC91x can be performed, but not 8-bit: #elif defined(CONFIG_MACH_LOGICPD_PXA270) || \ defined(CONFIG_MACH_NOMADIK_8815NHK) #define SMC_CAN_USE_8BIT 0 #define SMC_CAN_USE_16BIT 1 #define SMC_CAN_USE_32BIT 0 #elif defined(CONFIG_SH_SH4202_MICRODEV) #define SMC_CAN_USE_8BIT 0 #define SMC_CAN_USE_16BIT 1 #define SMC_CAN_USE_32BIT 0 #elif defined(CONFIG_M32R) #define SMC_CAN_USE_8BIT 0 #define SMC_CAN_USE_16BIT 1 #define SMC_CAN_USE_32BIT 0 #elif defined(CONFIG_ARCH_MSM) #define SMC_CAN_USE_8BIT 0 #define SMC_CAN_USE_16BIT 1 #define SMC_CAN_USE_32BIT 0 #elif defined(CONFIG_COLDFIRE) #define SMC_CAN_USE_8BIT 0 #define SMC_CAN_USE_16BIT 1 #define SMC_CAN_USE_32BIT 0 ... etc ... -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes 2016-08-26 11:38 ` Russell King - ARM Linux @ 2016-08-26 12:29 ` Russell King - ARM Linux -1 siblings, 0 replies; 22+ messages in thread From: Russell King - ARM Linux @ 2016-08-26 12:29 UTC (permalink / raw) To: Arnd Bergmann Cc: Yoshinori Sato, Nicolas Pitre, netdev, linux-kernel, Robert Jarzmik, David S. Miller, linux-arm-kernel On Fri, Aug 26, 2016 at 12:38:48PM +0100, Russell King - ARM Linux wrote: > On Fri, Aug 26, 2016 at 11:41:21AM +0200, Arnd Bergmann wrote: > > On Thursday, August 25, 2016 11:37:43 PM CEST Russell King - ARM Linux wrote: > > > On Thu, Aug 25, 2016 at 08:02:35PM +0200, Robert Jarzmik wrote: > > > > Arnd Bergmann <arnd@arndb.de> writes: > > > /* > > > + * 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. > > > + */ > > > +#define SMC_outw_b(x, a, r) \ > > > + do { \ > > > + unsigned int __val16 = (x); \ > > > + unsigned int __reg = (r); \ > > > + SMC_outb(__val16, a, __reg); \ > > > + SMC_outb(__val16 >> 8, a, __reg + (1 << SMC_IO_SHIFT)); \ > > > + } while (0) > > > + > > > +#define SMC_inw_b(a, r) \ > > > + ({ \ > > > + unsigned int __val16; \ > > > + unsigned int __reg = r; \ > > > + __val16 = SMC_inb(a, __reg); \ > > > + __val16 |= SMC_inb(a, __reg + (1 << SMC_IO_SHIFT)) << 8; \ > > > + __val16; \ > > > + }) > > > + > > > +/* > > > * Define your architecture specific bus configuration parameters here. > > > */ > > > > > > @@ -55,10 +76,30 @@ > > > #define SMC_IO_SHIFT (lp->io_shift) > > > > > > #define SMC_inb(a, r) readb((a) + (r)) > > > -#define SMC_inw(a, r) readw((a) + (r)) > > > +#define SMC_inw(a, r) \ > > > + ({ \ > > > + unsigned int __smc_r = r; \ > > > + SMC_16BIT(lp) ? readw((a) + __smc_r) : \ > > > + SMC_8BIT(lp) ? SMC_inw_b(a, __smc_r) : \ > > > + ({ BUG(); 0; }); \ > > > + }) > > > + > > > > I think this breaks machines that declare a device that just lists > > SMC91X_USE_32BIT but not SMC91X_USE_16BIT. Right now, the way this > > is interpreted is to use 32-bit accessors for most things, but > > not avoiding 16-bit reads. > > Your comment is wrong. It breaks machines that declare a device > with SMC91X_USE_32BIT but not _either_ of SMC91X_USE_16BIT _or_ > SMC91X_USE_8BIT. As I already noted, but you obviously ignored. > > In that note, I pointed out that this _was_ already the case with > the original driver before your patch: > > #if ! SMC_CAN_USE_16BIT > ... code implementing SMC_outw() and SMC_inw() in terms of SMC_outb() > and SMC_inb(), and defining SMC_insw() and SMC_outsw() as BUG() > #endif > #if !defined(SMC_insw) || !defined(SMC_outsw) > #define SMC_insw(a, r, p, l) BUG() > #define SMC_outsw(a, r, p, l) BUG() > #endif > > #if ! SMC_CAN_USE_8BIT > #define SMC_inb(ioaddr, reg) ({ BUG(); 0; }) > #define SMC_outb(x, ioaddr, reg) BUG() > #define SMC_insb(a, r, p, l) BUG() > #define SMC_outsb(a, r, p, l) BUG() > #endif > > #if !defined(SMC_insb) || !defined(SMC_outsb) > #define SMC_insb(a, r, p, l) BUG() > #define SMC_outsb(a, r, p, l) BUG() > #endif > > So, if _neither_ SMC_CAN_USE_16BIT nor SMC_CAN_USE_8BIT are defined, > trying to use the 16-bit accessors cause a BUG(). > > > That is a bit fishy though, and we could instead change the platform > > data to always set both SMC91X_USE_32BIT and SMC91X_USE_16BIT. > > > > The affected platforms are DT based machines with 32-bit I/O and > > these board files: > > > > arch/arm/mach-pxa/idp.c: .flags = SMC91X_USE_32BIT | SMC91X_USE_DMA | SMC91X_NOWAIT, > > arch/arm/mach-pxa/xcep.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT | SMC91X_USE_DMA, > > arch/arm/mach-realview/core.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, > > arch/blackfin/mach-bf561/boards/cm_bf561.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, > > arch/blackfin/mach-bf561/boards/ezkit.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, > > Right, this looks like another one of your bugs in this conversion. > > #elif defined(CONFIG_ARCH_INNOKOM) || \ > defined(CONFIG_ARCH_PXA_IDP) || \ > defined(CONFIG_ARCH_RAMSES) || \ > defined(CONFIG_ARCH_PCM027) > > #define SMC_CAN_USE_8BIT 1 > #define SMC_CAN_USE_16BIT 1 > #define SMC_CAN_USE_32BIT 1 > > So, IDP can use all of 32-bit, 16-bit and 8-bit accesses, meanwhile your > conversion is telling the driver that it can only use 32-bit => your > conversion of IDP is broken. > > Before your conversion, realview depended on the default settings of > smc91x, which is again: > > #define SMC_CAN_USE_8BIT 1 > #define SMC_CAN_USE_16BIT 1 > #define SMC_CAN_USE_32BIT 1 > > So realview follows IDP - it can do 32-bit, 16-bit and 8-bit accesses, > meanwhile your conversion tells the driver it can _only_ do 32-bit > accesses, which is again broken. > > XCEP falls into the same category, as do the other two blackfin > cases from what I can see. > > The point of the SMC_CAN_USE_xxx macros is to indicate to the SMC91x > driver which sizes of access can be used on the hardware concerned. > If SMC_CAN_USE_16BIT is set, then 16-bit accesses are possible. If > it's not set, then 8-bit accesses will be used if they're supported, > otherwise it's a buggy configuration. > > SMC_CAN_USE_32BIT says that 32-bit accesses are possible, but these > are only used in a small number of cases as an optimisation. > > It is a bug to configure the smc91x driver with both SMC_CAN_USE_16BIT > and SMC_CAN_USE_8BIT clear. > > Hence, it's a bug to tell the driver (via the platform data) that only > 32-bit accesses can be performed. > > So, while my patch may be fixing some of the brokenness, the rest of > the brokenness also needs fixing by adjusting all the platform data > to properly reflect which access sizes are possible, rather than what > you seem to have done, which is to indicate what the largest possible > access size is. > > This is especially important as there are platforms around where 16-bit > accesses to the SMC91x can be performed, but not 8-bit: > > #elif defined(CONFIG_MACH_LOGICPD_PXA270) || \ > defined(CONFIG_MACH_NOMADIK_8815NHK) > > #define SMC_CAN_USE_8BIT 0 > #define SMC_CAN_USE_16BIT 1 > #define SMC_CAN_USE_32BIT 0 > > #elif defined(CONFIG_SH_SH4202_MICRODEV) > > #define SMC_CAN_USE_8BIT 0 > #define SMC_CAN_USE_16BIT 1 > #define SMC_CAN_USE_32BIT 0 > > #elif defined(CONFIG_M32R) > > #define SMC_CAN_USE_8BIT 0 > #define SMC_CAN_USE_16BIT 1 > #define SMC_CAN_USE_32BIT 0 > > #elif defined(CONFIG_ARCH_MSM) > > #define SMC_CAN_USE_8BIT 0 > #define SMC_CAN_USE_16BIT 1 > #define SMC_CAN_USE_32BIT 0 > > #elif defined(CONFIG_COLDFIRE) > > #define SMC_CAN_USE_8BIT 0 > #define SMC_CAN_USE_16BIT 1 > #define SMC_CAN_USE_32BIT 0 > > ... etc ... So, based on your patch introducing the breakage, these changes are necessary to restore the per-platform capabilities that were originally configured into the driver. I've also checked over the tree, and added the others that only set SMC91X_USE_32BIT. Lastly, I've added a comment to linux/smc91x.h indicating how these bits are supposed to be used. There may be other platforms which need updating - setting these bits incorrectly can lead to more complex IO accesses than are necessary, which, in a SMP environment, would be racy. For instance, a missing SMC91X_USE_8BIT results in the interrupt acknowledgement being written using a read-modify-write sequence with a 16-bit access under local_irq_save(). That's also another illustration why supporting at least one of 8-bit or 16-bit access is mandatory - there is no emulation of the 8 or 16-bit accesses in terms of 32-bit accessors. arch/arm/mach-pxa/idp.c | 3 ++- arch/arm/mach-pxa/xcep.c | 3 ++- arch/arm/mach-realview/core.c | 3 ++- arch/arm/mach-sa1100/pleb.c | 2 +- arch/blackfin/mach-bf561/boards/cm_bf561.c | 3 ++- arch/blackfin/mach-bf561/boards/ezkit.c | 3 ++- include/linux/smc91x.h | 10 ++++++++++ 7 files changed, 21 insertions(+), 6 deletions(-) diff --git a/arch/arm/mach-pxa/idp.c b/arch/arm/mach-pxa/idp.c index c410d84b243d..30100ac94368 100644 --- a/arch/arm/mach-pxa/idp.c +++ b/arch/arm/mach-pxa/idp.c @@ -83,7 +83,8 @@ static struct resource smc91x_resources[] = { }; static struct smc91x_platdata smc91x_platdata = { - .flags = SMC91X_USE_32BIT | SMC91X_USE_DMA | SMC91X_NOWAIT, + .flags = SMC91X_USE_8BIT | SMC91X_USE_16BIT | SMC91X_USE_32BIT | + SMC91X_USE_DMA | SMC91X_NOWAIT, }; static struct platform_device smc91x_device = { diff --git a/arch/arm/mach-pxa/xcep.c b/arch/arm/mach-pxa/xcep.c index 3f06cd90567a..056369ef250e 100644 --- a/arch/arm/mach-pxa/xcep.c +++ b/arch/arm/mach-pxa/xcep.c @@ -120,7 +120,8 @@ static struct resource smc91x_resources[] = { }; static struct smc91x_platdata xcep_smc91x_info = { - .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT | SMC91X_USE_DMA, + .flags = SMC91X_USE_8BIT | SMC91X_USE_16BIT | SMC91X_USE_32BIT | + SMC91X_NOWAIT | SMC91X_USE_DMA, }; static struct platform_device smc91x_device = { diff --git a/arch/arm/mach-realview/core.c b/arch/arm/mach-realview/core.c index baf174542e36..b287deaf582c 100644 --- a/arch/arm/mach-realview/core.c +++ b/arch/arm/mach-realview/core.c @@ -93,7 +93,8 @@ static struct smsc911x_platform_config smsc911x_config = { }; static struct smc91x_platdata smc91x_platdata = { - .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, + .flags = SMC91X_USE_8BIT | SMC91X_USE_16BIT | SMC91X_USE_32BIT | + SMC91X_NOWAIT, }; static struct platform_device realview_eth_device = { diff --git a/arch/arm/mach-sa1100/pleb.c b/arch/arm/mach-sa1100/pleb.c index 1525d7b5f1b7..88149f85bc49 100644 --- a/arch/arm/mach-sa1100/pleb.c +++ b/arch/arm/mach-sa1100/pleb.c @@ -45,7 +45,7 @@ static struct resource smc91x_resources[] = { }; static struct smc91x_platdata smc91x_platdata = { - .flags = SMC91X_USE_16BIT | SMC91X_NOWAIT, + .flags = SMC91X_USE_16BIT | SMC91X_USE_8BIT | SMC91X_NOWAIT, }; static struct platform_device smc91x_device = { diff --git a/arch/blackfin/mach-bf561/boards/cm_bf561.c b/arch/blackfin/mach-bf561/boards/cm_bf561.c index c6db52ba3a06..10c57771822d 100644 --- a/arch/blackfin/mach-bf561/boards/cm_bf561.c +++ b/arch/blackfin/mach-bf561/boards/cm_bf561.c @@ -146,7 +146,8 @@ static struct platform_device hitachi_fb_device = { #include <linux/smc91x.h> static struct smc91x_platdata smc91x_info = { - .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, + .flags = SMC91X_USE_8BIT | SMC91X_USE_16BIT | SMC91X_USE_32BIT | + SMC91X_NOWAIT, .leda = RPC_LED_100_10, .ledb = RPC_LED_TX_RX, }; diff --git a/arch/blackfin/mach-bf561/boards/ezkit.c b/arch/blackfin/mach-bf561/boards/ezkit.c index f35525b55819..57d1c43726d9 100644 --- a/arch/blackfin/mach-bf561/boards/ezkit.c +++ b/arch/blackfin/mach-bf561/boards/ezkit.c @@ -134,7 +134,8 @@ static struct platform_device net2272_bfin_device = { #include <linux/smc91x.h> static struct smc91x_platdata smc91x_info = { - .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, + .flags = SMC91X_USE_8BIT | SMC91X_USE_16BIT | SMC91X_USE_32BIT | + SMC91X_NOWAIT, .leda = RPC_LED_100_10, .ledb = RPC_LED_TX_RX, }; diff --git a/include/linux/smc91x.h b/include/linux/smc91x.h index 76199b75d584..e302c447e057 100644 --- a/include/linux/smc91x.h +++ b/include/linux/smc91x.h @@ -1,6 +1,16 @@ #ifndef __SMC91X_H__ #define __SMC91X_H__ +/* + * These bits define which access sizes a platform can support, rather + * than the maximal access size. So, if your platform can do 16-bit + * and 32-bit accesses to the SMC91x device, but not 8-bit, set both + * SMC91X_USE_16BIT and SMC91X_USE_32BIT. + * + * The SMC91x driver requires at least one of SMC91X_USE_8BIT or + * SMC91X_USE_16BIT to be supported - just setting SMC91X_USE_32BIT is + * an invalid configuration. + */ #define SMC91X_USE_8BIT (1 << 0) #define SMC91X_USE_16BIT (1 << 1) #define SMC91X_USE_32BIT (1 << 2) -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes @ 2016-08-26 12:29 ` Russell King - ARM Linux 0 siblings, 0 replies; 22+ messages in thread From: Russell King - ARM Linux @ 2016-08-26 12:29 UTC (permalink / raw) To: linux-arm-kernel On Fri, Aug 26, 2016 at 12:38:48PM +0100, Russell King - ARM Linux wrote: > On Fri, Aug 26, 2016 at 11:41:21AM +0200, Arnd Bergmann wrote: > > On Thursday, August 25, 2016 11:37:43 PM CEST Russell King - ARM Linux wrote: > > > On Thu, Aug 25, 2016 at 08:02:35PM +0200, Robert Jarzmik wrote: > > > > Arnd Bergmann <arnd@arndb.de> writes: > > > /* > > > + * 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. > > > + */ > > > +#define SMC_outw_b(x, a, r) \ > > > + do { \ > > > + unsigned int __val16 = (x); \ > > > + unsigned int __reg = (r); \ > > > + SMC_outb(__val16, a, __reg); \ > > > + SMC_outb(__val16 >> 8, a, __reg + (1 << SMC_IO_SHIFT)); \ > > > + } while (0) > > > + > > > +#define SMC_inw_b(a, r) \ > > > + ({ \ > > > + unsigned int __val16; \ > > > + unsigned int __reg = r; \ > > > + __val16 = SMC_inb(a, __reg); \ > > > + __val16 |= SMC_inb(a, __reg + (1 << SMC_IO_SHIFT)) << 8; \ > > > + __val16; \ > > > + }) > > > + > > > +/* > > > * Define your architecture specific bus configuration parameters here. > > > */ > > > > > > @@ -55,10 +76,30 @@ > > > #define SMC_IO_SHIFT (lp->io_shift) > > > > > > #define SMC_inb(a, r) readb((a) + (r)) > > > -#define SMC_inw(a, r) readw((a) + (r)) > > > +#define SMC_inw(a, r) \ > > > + ({ \ > > > + unsigned int __smc_r = r; \ > > > + SMC_16BIT(lp) ? readw((a) + __smc_r) : \ > > > + SMC_8BIT(lp) ? SMC_inw_b(a, __smc_r) : \ > > > + ({ BUG(); 0; }); \ > > > + }) > > > + > > > > I think this breaks machines that declare a device that just lists > > SMC91X_USE_32BIT but not SMC91X_USE_16BIT. Right now, the way this > > is interpreted is to use 32-bit accessors for most things, but > > not avoiding 16-bit reads. > > Your comment is wrong. It breaks machines that declare a device > with SMC91X_USE_32BIT but not _either_ of SMC91X_USE_16BIT _or_ > SMC91X_USE_8BIT. As I already noted, but you obviously ignored. > > In that note, I pointed out that this _was_ already the case with > the original driver before your patch: > > #if ! SMC_CAN_USE_16BIT > ... code implementing SMC_outw() and SMC_inw() in terms of SMC_outb() > and SMC_inb(), and defining SMC_insw() and SMC_outsw() as BUG() > #endif > #if !defined(SMC_insw) || !defined(SMC_outsw) > #define SMC_insw(a, r, p, l) BUG() > #define SMC_outsw(a, r, p, l) BUG() > #endif > > #if ! SMC_CAN_USE_8BIT > #define SMC_inb(ioaddr, reg) ({ BUG(); 0; }) > #define SMC_outb(x, ioaddr, reg) BUG() > #define SMC_insb(a, r, p, l) BUG() > #define SMC_outsb(a, r, p, l) BUG() > #endif > > #if !defined(SMC_insb) || !defined(SMC_outsb) > #define SMC_insb(a, r, p, l) BUG() > #define SMC_outsb(a, r, p, l) BUG() > #endif > > So, if _neither_ SMC_CAN_USE_16BIT nor SMC_CAN_USE_8BIT are defined, > trying to use the 16-bit accessors cause a BUG(). > > > That is a bit fishy though, and we could instead change the platform > > data to always set both SMC91X_USE_32BIT and SMC91X_USE_16BIT. > > > > The affected platforms are DT based machines with 32-bit I/O and > > these board files: > > > > arch/arm/mach-pxa/idp.c: .flags = SMC91X_USE_32BIT | SMC91X_USE_DMA | SMC91X_NOWAIT, > > arch/arm/mach-pxa/xcep.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT | SMC91X_USE_DMA, > > arch/arm/mach-realview/core.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, > > arch/blackfin/mach-bf561/boards/cm_bf561.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, > > arch/blackfin/mach-bf561/boards/ezkit.c: .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, > > Right, this looks like another one of your bugs in this conversion. > > #elif defined(CONFIG_ARCH_INNOKOM) || \ > defined(CONFIG_ARCH_PXA_IDP) || \ > defined(CONFIG_ARCH_RAMSES) || \ > defined(CONFIG_ARCH_PCM027) > > #define SMC_CAN_USE_8BIT 1 > #define SMC_CAN_USE_16BIT 1 > #define SMC_CAN_USE_32BIT 1 > > So, IDP can use all of 32-bit, 16-bit and 8-bit accesses, meanwhile your > conversion is telling the driver that it can only use 32-bit => your > conversion of IDP is broken. > > Before your conversion, realview depended on the default settings of > smc91x, which is again: > > #define SMC_CAN_USE_8BIT 1 > #define SMC_CAN_USE_16BIT 1 > #define SMC_CAN_USE_32BIT 1 > > So realview follows IDP - it can do 32-bit, 16-bit and 8-bit accesses, > meanwhile your conversion tells the driver it can _only_ do 32-bit > accesses, which is again broken. > > XCEP falls into the same category, as do the other two blackfin > cases from what I can see. > > The point of the SMC_CAN_USE_xxx macros is to indicate to the SMC91x > driver which sizes of access can be used on the hardware concerned. > If SMC_CAN_USE_16BIT is set, then 16-bit accesses are possible. If > it's not set, then 8-bit accesses will be used if they're supported, > otherwise it's a buggy configuration. > > SMC_CAN_USE_32BIT says that 32-bit accesses are possible, but these > are only used in a small number of cases as an optimisation. > > It is a bug to configure the smc91x driver with both SMC_CAN_USE_16BIT > and SMC_CAN_USE_8BIT clear. > > Hence, it's a bug to tell the driver (via the platform data) that only > 32-bit accesses can be performed. > > So, while my patch may be fixing some of the brokenness, the rest of > the brokenness also needs fixing by adjusting all the platform data > to properly reflect which access sizes are possible, rather than what > you seem to have done, which is to indicate what the largest possible > access size is. > > This is especially important as there are platforms around where 16-bit > accesses to the SMC91x can be performed, but not 8-bit: > > #elif defined(CONFIG_MACH_LOGICPD_PXA270) || \ > defined(CONFIG_MACH_NOMADIK_8815NHK) > > #define SMC_CAN_USE_8BIT 0 > #define SMC_CAN_USE_16BIT 1 > #define SMC_CAN_USE_32BIT 0 > > #elif defined(CONFIG_SH_SH4202_MICRODEV) > > #define SMC_CAN_USE_8BIT 0 > #define SMC_CAN_USE_16BIT 1 > #define SMC_CAN_USE_32BIT 0 > > #elif defined(CONFIG_M32R) > > #define SMC_CAN_USE_8BIT 0 > #define SMC_CAN_USE_16BIT 1 > #define SMC_CAN_USE_32BIT 0 > > #elif defined(CONFIG_ARCH_MSM) > > #define SMC_CAN_USE_8BIT 0 > #define SMC_CAN_USE_16BIT 1 > #define SMC_CAN_USE_32BIT 0 > > #elif defined(CONFIG_COLDFIRE) > > #define SMC_CAN_USE_8BIT 0 > #define SMC_CAN_USE_16BIT 1 > #define SMC_CAN_USE_32BIT 0 > > ... etc ... So, based on your patch introducing the breakage, these changes are necessary to restore the per-platform capabilities that were originally configured into the driver. I've also checked over the tree, and added the others that only set SMC91X_USE_32BIT. Lastly, I've added a comment to linux/smc91x.h indicating how these bits are supposed to be used. There may be other platforms which need updating - setting these bits incorrectly can lead to more complex IO accesses than are necessary, which, in a SMP environment, would be racy. For instance, a missing SMC91X_USE_8BIT results in the interrupt acknowledgement being written using a read-modify-write sequence with a 16-bit access under local_irq_save(). That's also another illustration why supporting at least one of 8-bit or 16-bit access is mandatory - there is no emulation of the 8 or 16-bit accesses in terms of 32-bit accessors. arch/arm/mach-pxa/idp.c | 3 ++- arch/arm/mach-pxa/xcep.c | 3 ++- arch/arm/mach-realview/core.c | 3 ++- arch/arm/mach-sa1100/pleb.c | 2 +- arch/blackfin/mach-bf561/boards/cm_bf561.c | 3 ++- arch/blackfin/mach-bf561/boards/ezkit.c | 3 ++- include/linux/smc91x.h | 10 ++++++++++ 7 files changed, 21 insertions(+), 6 deletions(-) diff --git a/arch/arm/mach-pxa/idp.c b/arch/arm/mach-pxa/idp.c index c410d84b243d..30100ac94368 100644 --- a/arch/arm/mach-pxa/idp.c +++ b/arch/arm/mach-pxa/idp.c @@ -83,7 +83,8 @@ static struct resource smc91x_resources[] = { }; static struct smc91x_platdata smc91x_platdata = { - .flags = SMC91X_USE_32BIT | SMC91X_USE_DMA | SMC91X_NOWAIT, + .flags = SMC91X_USE_8BIT | SMC91X_USE_16BIT | SMC91X_USE_32BIT | + SMC91X_USE_DMA | SMC91X_NOWAIT, }; static struct platform_device smc91x_device = { diff --git a/arch/arm/mach-pxa/xcep.c b/arch/arm/mach-pxa/xcep.c index 3f06cd90567a..056369ef250e 100644 --- a/arch/arm/mach-pxa/xcep.c +++ b/arch/arm/mach-pxa/xcep.c @@ -120,7 +120,8 @@ static struct resource smc91x_resources[] = { }; static struct smc91x_platdata xcep_smc91x_info = { - .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT | SMC91X_USE_DMA, + .flags = SMC91X_USE_8BIT | SMC91X_USE_16BIT | SMC91X_USE_32BIT | + SMC91X_NOWAIT | SMC91X_USE_DMA, }; static struct platform_device smc91x_device = { diff --git a/arch/arm/mach-realview/core.c b/arch/arm/mach-realview/core.c index baf174542e36..b287deaf582c 100644 --- a/arch/arm/mach-realview/core.c +++ b/arch/arm/mach-realview/core.c @@ -93,7 +93,8 @@ static struct smsc911x_platform_config smsc911x_config = { }; static struct smc91x_platdata smc91x_platdata = { - .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, + .flags = SMC91X_USE_8BIT | SMC91X_USE_16BIT | SMC91X_USE_32BIT | + SMC91X_NOWAIT, }; static struct platform_device realview_eth_device = { diff --git a/arch/arm/mach-sa1100/pleb.c b/arch/arm/mach-sa1100/pleb.c index 1525d7b5f1b7..88149f85bc49 100644 --- a/arch/arm/mach-sa1100/pleb.c +++ b/arch/arm/mach-sa1100/pleb.c @@ -45,7 +45,7 @@ static struct resource smc91x_resources[] = { }; static struct smc91x_platdata smc91x_platdata = { - .flags = SMC91X_USE_16BIT | SMC91X_NOWAIT, + .flags = SMC91X_USE_16BIT | SMC91X_USE_8BIT | SMC91X_NOWAIT, }; static struct platform_device smc91x_device = { diff --git a/arch/blackfin/mach-bf561/boards/cm_bf561.c b/arch/blackfin/mach-bf561/boards/cm_bf561.c index c6db52ba3a06..10c57771822d 100644 --- a/arch/blackfin/mach-bf561/boards/cm_bf561.c +++ b/arch/blackfin/mach-bf561/boards/cm_bf561.c @@ -146,7 +146,8 @@ static struct platform_device hitachi_fb_device = { #include <linux/smc91x.h> static struct smc91x_platdata smc91x_info = { - .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, + .flags = SMC91X_USE_8BIT | SMC91X_USE_16BIT | SMC91X_USE_32BIT | + SMC91X_NOWAIT, .leda = RPC_LED_100_10, .ledb = RPC_LED_TX_RX, }; diff --git a/arch/blackfin/mach-bf561/boards/ezkit.c b/arch/blackfin/mach-bf561/boards/ezkit.c index f35525b55819..57d1c43726d9 100644 --- a/arch/blackfin/mach-bf561/boards/ezkit.c +++ b/arch/blackfin/mach-bf561/boards/ezkit.c @@ -134,7 +134,8 @@ static struct platform_device net2272_bfin_device = { #include <linux/smc91x.h> static struct smc91x_platdata smc91x_info = { - .flags = SMC91X_USE_32BIT | SMC91X_NOWAIT, + .flags = SMC91X_USE_8BIT | SMC91X_USE_16BIT | SMC91X_USE_32BIT | + SMC91X_NOWAIT, .leda = RPC_LED_100_10, .ledb = RPC_LED_TX_RX, }; diff --git a/include/linux/smc91x.h b/include/linux/smc91x.h index 76199b75d584..e302c447e057 100644 --- a/include/linux/smc91x.h +++ b/include/linux/smc91x.h @@ -1,6 +1,16 @@ #ifndef __SMC91X_H__ #define __SMC91X_H__ +/* + * These bits define which access sizes a platform can support, rather + * than the maximal access size. So, if your platform can do 16-bit + * and 32-bit accesses to the SMC91x device, but not 8-bit, set both + * SMC91X_USE_16BIT and SMC91X_USE_32BIT. + * + * The SMC91x driver requires at least one of SMC91X_USE_8BIT or + * SMC91X_USE_16BIT to be supported - just setting SMC91X_USE_32BIT is + * an invalid configuration. + */ #define SMC91X_USE_8BIT (1 << 0) #define SMC91X_USE_16BIT (1 << 1) #define SMC91X_USE_32BIT (1 << 2) -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently@9.6Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes 2016-08-25 22:37 ` Russell King - ARM Linux (?) @ 2016-08-27 15:30 ` Robert Jarzmik -1 siblings, 0 replies; 22+ messages in thread From: Robert Jarzmik @ 2016-08-27 15:30 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Arnd Bergmann, linux-arm-kernel, Nicolas Pitre, David S. Miller, Yoshinori Sato, netdev, linux-kernel Russell King - ARM Linux <linux@armlinux.org.uk> writes: > On Thu, Aug 25, 2016 at 08:02:35PM +0200, Robert Jarzmik wrote: >> Arnd Bergmann <arnd@arndb.de> writes: >> >> > On Thursday, August 25, 2016 4:43:04 PM CEST Arnd Bergmann wrote: >> >> drivers/net/ethernet/smsc/smc91x.h | 50 +++++++++++++++++++++++--------------- >> >> 1 file changed, 30 insertions(+), 20 deletions(-) >> >> >> >> While this patch fixes one bug on Neponset, it probably doesn't address >> >> the one that Russell ran into first, so this is for review only for now, >> >> until the remaining problem(s) have been worked out. >> >> >> > >> > The comment should have been on another patch, my mistake. please >> > see v2. >> > >> > Arnd >> >> Hi Arnd, >> >> I didn't review the patch thoroughly, but I launched your 2 patches in my pxa >> little farm. >> >> The result is that lubbock and mainstone are all right, but zylonite is broken >> (ie. networkless). I removed then these 2 patches and zylonite worked again. >> >> I have also an error message on the console on a "broken" zylonite : >> Changing smcs91x MAC address to 08:00:3e:26:0a:5b: ifconfig: SIOCSIFHWADDR: >> Device or resource busy >> >> I reran the test twice (2 times with your patches, 2 times without), the result >> looks consistent, ie. zylonite doesn't really like them. > > Please try the patch below. I sent this to Will a few days ago, as > he said (on irc) that he was also seeing problems on a platform he > had, but I've yet to hear back. I've not posted it yet because I > haven't got around to writing a commit description for it. > > It does require that at least one of 8-bit or 16-bit accesses are > supported, but I think that's already true. Hi Russell, With your patch : - lubbock, mainstone and zylonite boards are working correctly => ie. all my boards are working correctly - the message "ifconfig: SIOCSIFHWADDR: Device or resource busy" is still there => this message is here even without your patch, so it's rather not relevant => this message is triggered by an "/sbin/ifconfig eth0 hw ether 08:00:3e:26:0a:5b" command So from a PXA testing point of view it's all good, ie. Tested-by: Robert Jarzmik <robert.jarzmik@free.fr> Cheers. -- Robert ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes @ 2016-08-27 15:30 ` Robert Jarzmik 0 siblings, 0 replies; 22+ messages in thread From: Robert Jarzmik @ 2016-08-27 15:30 UTC (permalink / raw) To: linux-arm-kernel Russell King - ARM Linux <linux@armlinux.org.uk> writes: > On Thu, Aug 25, 2016 at 08:02:35PM +0200, Robert Jarzmik wrote: >> Arnd Bergmann <arnd@arndb.de> writes: >> >> > On Thursday, August 25, 2016 4:43:04 PM CEST Arnd Bergmann wrote: >> >> drivers/net/ethernet/smsc/smc91x.h | 50 +++++++++++++++++++++++--------------- >> >> 1 file changed, 30 insertions(+), 20 deletions(-) >> >> >> >> While this patch fixes one bug on Neponset, it probably doesn't address >> >> the one that Russell ran into first, so this is for review only for now, >> >> until the remaining problem(s) have been worked out. >> >> >> > >> > The comment should have been on another patch, my mistake. please >> > see v2. >> > >> > Arnd >> >> Hi Arnd, >> >> I didn't review the patch thoroughly, but I launched your 2 patches in my pxa >> little farm. >> >> The result is that lubbock and mainstone are all right, but zylonite is broken >> (ie. networkless). I removed then these 2 patches and zylonite worked again. >> >> I have also an error message on the console on a "broken" zylonite : >> Changing smcs91x MAC address to 08:00:3e:26:0a:5b: ifconfig: SIOCSIFHWADDR: >> Device or resource busy >> >> I reran the test twice (2 times with your patches, 2 times without), the result >> looks consistent, ie. zylonite doesn't really like them. > > Please try the patch below. I sent this to Will a few days ago, as > he said (on irc) that he was also seeing problems on a platform he > had, but I've yet to hear back. I've not posted it yet because I > haven't got around to writing a commit description for it. > > It does require that at least one of 8-bit or 16-bit accesses are > supported, but I think that's already true. Hi Russell, With your patch : - lubbock, mainstone and zylonite boards are working correctly => ie. all my boards are working correctly - the message "ifconfig: SIOCSIFHWADDR: Device or resource busy" is still there => this message is here even without your patch, so it's rather not relevant => this message is triggered by an "/sbin/ifconfig eth0 hw ether 08:00:3e:26:0a:5b" command So from a PXA testing point of view it's all good, ie. Tested-by: Robert Jarzmik <robert.jarzmik@free.fr> Cheers. -- Robert ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes @ 2016-08-27 15:30 ` Robert Jarzmik 0 siblings, 0 replies; 22+ messages in thread From: Robert Jarzmik @ 2016-08-27 15:30 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Arnd Bergmann, Yoshinori Sato, Nicolas Pitre, netdev, linux-kernel, David S. Miller, linux-arm-kernel Russell King - ARM Linux <linux@armlinux.org.uk> writes: > On Thu, Aug 25, 2016 at 08:02:35PM +0200, Robert Jarzmik wrote: >> Arnd Bergmann <arnd@arndb.de> writes: >> >> > On Thursday, August 25, 2016 4:43:04 PM CEST Arnd Bergmann wrote: >> >> drivers/net/ethernet/smsc/smc91x.h | 50 +++++++++++++++++++++++--------------- >> >> 1 file changed, 30 insertions(+), 20 deletions(-) >> >> >> >> While this patch fixes one bug on Neponset, it probably doesn't address >> >> the one that Russell ran into first, so this is for review only for now, >> >> until the remaining problem(s) have been worked out. >> >> >> > >> > The comment should have been on another patch, my mistake. please >> > see v2. >> > >> > Arnd >> >> Hi Arnd, >> >> I didn't review the patch thoroughly, but I launched your 2 patches in my pxa >> little farm. >> >> The result is that lubbock and mainstone are all right, but zylonite is broken >> (ie. networkless). I removed then these 2 patches and zylonite worked again. >> >> I have also an error message on the console on a "broken" zylonite : >> Changing smcs91x MAC address to 08:00:3e:26:0a:5b: ifconfig: SIOCSIFHWADDR: >> Device or resource busy >> >> I reran the test twice (2 times with your patches, 2 times without), the result >> looks consistent, ie. zylonite doesn't really like them. > > Please try the patch below. I sent this to Will a few days ago, as > he said (on irc) that he was also seeing problems on a platform he > had, but I've yet to hear back. I've not posted it yet because I > haven't got around to writing a commit description for it. > > It does require that at least one of 8-bit or 16-bit accesses are > supported, but I think that's already true. Hi Russell, With your patch : - lubbock, mainstone and zylonite boards are working correctly => ie. all my boards are working correctly - the message "ifconfig: SIOCSIFHWADDR: Device or resource busy" is still there => this message is here even without your patch, so it's rather not relevant => this message is triggered by an "/sbin/ifconfig eth0 hw ether 08:00:3e:26:0a:5b" command So from a PXA testing point of view it's all good, ie. Tested-by: Robert Jarzmik <robert.jarzmik@free.fr> Cheers. -- Robert ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes 2016-08-27 15:30 ` Robert Jarzmik @ 2016-08-27 15:38 ` Russell King - ARM Linux -1 siblings, 0 replies; 22+ messages in thread From: Russell King - ARM Linux @ 2016-08-27 15:38 UTC (permalink / raw) To: Robert Jarzmik Cc: Arnd Bergmann, linux-arm-kernel, Nicolas Pitre, David S. Miller, Yoshinori Sato, netdev, linux-kernel On Sat, Aug 27, 2016 at 05:30:42PM +0200, Robert Jarzmik wrote: > Hi Russell, > > With your patch : > - lubbock, mainstone and zylonite boards are working correctly > => ie. all my boards are working correctly > > - the message "ifconfig: SIOCSIFHWADDR: Device or resource busy" is still there > => this message is here even without your patch, so it's rather not relevant > => this message is triggered by an "/sbin/ifconfig eth0 hw ether > 08:00:3e:26:0a:5b" command > > So from a PXA testing point of view it's all good, ie. > Tested-by: Robert Jarzmik <robert.jarzmik@free.fr> Thanks for testing, that's good news. You'll get "SIOCSIFHWADDR: Device or resource busy" if the device is already up and has no support for changing the address while the NIC is "live" - we only program the MAC address when bringing the smc91x device up (or resetting it after a timeout). See net/ethernet/eth.c::eth_prepare_mac_addr_change() and smc_enable(). -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes @ 2016-08-27 15:38 ` Russell King - ARM Linux 0 siblings, 0 replies; 22+ messages in thread From: Russell King - ARM Linux @ 2016-08-27 15:38 UTC (permalink / raw) To: linux-arm-kernel On Sat, Aug 27, 2016 at 05:30:42PM +0200, Robert Jarzmik wrote: > Hi Russell, > > With your patch : > - lubbock, mainstone and zylonite boards are working correctly > => ie. all my boards are working correctly > > - the message "ifconfig: SIOCSIFHWADDR: Device or resource busy" is still there > => this message is here even without your patch, so it's rather not relevant > => this message is triggered by an "/sbin/ifconfig eth0 hw ether > 08:00:3e:26:0a:5b" command > > So from a PXA testing point of view it's all good, ie. > Tested-by: Robert Jarzmik <robert.jarzmik@free.fr> Thanks for testing, that's good news. You'll get "SIOCSIFHWADDR: Device or resource busy" if the device is already up and has no support for changing the address while the NIC is "live" - we only program the MAC address when bringing the smc91x device up (or resetting it after a timeout). See net/ethernet/eth.c::eth_prepare_mac_addr_change() and smc_enable(). -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2016-08-27 15:39 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-08-25 14:43 [PATCH] smc91x: remove ARM hack for unaligned 16-bit writes Arnd Bergmann 2016-08-25 14:43 ` Arnd Bergmann 2016-08-25 14:45 ` Arnd Bergmann 2016-08-25 14:45 ` Arnd Bergmann 2016-08-25 14:45 ` Arnd Bergmann 2016-08-25 18:02 ` Robert Jarzmik 2016-08-25 18:02 ` Robert Jarzmik 2016-08-25 22:37 ` Russell King - ARM Linux 2016-08-25 22:37 ` Russell King - ARM Linux 2016-08-26 9:41 ` Arnd Bergmann 2016-08-26 9:41 ` Arnd Bergmann 2016-08-26 9:56 ` Arnd Bergmann 2016-08-26 9:56 ` Arnd Bergmann 2016-08-26 11:38 ` Russell King - ARM Linux 2016-08-26 11:38 ` Russell King - ARM Linux 2016-08-26 12:29 ` Russell King - ARM Linux 2016-08-26 12:29 ` Russell King - ARM Linux 2016-08-27 15:30 ` Robert Jarzmik 2016-08-27 15:30 ` Robert Jarzmik 2016-08-27 15:30 ` Robert Jarzmik 2016-08-27 15:38 ` Russell King - ARM Linux 2016-08-27 15:38 ` Russell King - ARM Linux
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.