| Message ID | 1297463148-6612-1-git-send-email-wd@denx.de |
|---|---|
| State | Accepted |
| Commit | 495df3bad9f250a36470cce15f14c36c616172b6 |
| Delegated to: | Albert ARIBAUD |
| Headers | show |
Le 11/02/2011 23:25, Wolfgang Denk a écrit : > Commit 3c0659b "ARM: Avoid compiler optimization for readb, writeb > and friends." introduced I/O accessors with memory barriers. > Unfortunately the new write*() accessors introduced a bug: > > The problem is that the argument "v" gets evaluated twice. This > breaks code like used here (from "drivers/net/dnet.c"): > > for (i = 0; i< wrsz; i++) > writel(*bufp++,&dnet->regs->TX_DATA_FIFO); > > Use auxiliary variables to avoid such problems. > > Signed-off-by: Wolfgang Denk<wd@denx.de> > Cc: Albert Aribaud<albert.aribaud@free.fr> > Cc: Alexander Holler<holler@ahsoftware.de> > Cc: Dirk Behme<dirk.behme@googlemail.com> > --- > > This patch fixes a pretty nasty problem. Everybody who has > experienced strange failures on ARM lately should apply it ASAP and > re-test. As far as I can tell at least the following drivers are > affected: > > drivers/net/dnet.c: writel(*bufp++,&dnet->regs->TX_DATA_FIFO); > drivers/usb/musb/musb_core.c: writeb(*data++,&musbr->fifox[ep]); > drivers/mmc/pxa_mmc.c: writel(*src++, MMC_TXFIFO); > drivers/mmc/mxcmmc.c: writel(*buf++,&host->base->buffer_access); > drivers/spi/davinci_spi.c: writel(data1_reg_val | *txp++,&ds->regs->dat1); > > Albert, please apply ASAP! Applied, available on u-boot-arm/master. Amicalement,
Dear Albert ARIBAUD, In message <4D55B8C9.8070605@free.fr> you wrote: > > > This patch fixes a pretty nasty problem. Everybody who has > > experienced strange failures on ARM lately should apply it ASAP and > > re-test. As far as I can tell at least the following drivers are > > affected: > > > > drivers/net/dnet.c: writel(*bufp++,&dnet->regs->TX_DATA_FIFO); > > drivers/usb/musb/musb_core.c: writeb(*data++,&musbr->fifox[ep]); > > drivers/mmc/pxa_mmc.c: writel(*src++, MMC_TXFIFO); > > drivers/mmc/mxcmmc.c: writel(*buf++,&host->base->buffer_access); > > drivers/spi/davinci_spi.c: writel(data1_reg_val | *txp++,&ds->regs->dat1); > > > > Albert, please apply ASAP! > > Applied, available on u-boot-arm/master. Thanks, highly appreciated. Best regards, Wolfgang Denk
Hello, Am 11.02.2011 23:25, schrieb Wolfgang Denk: > +#define writeb(v,c) ({ u8 __v = v; __iowmb(); __arch_putb(__v,c); __v; }) > +#define writew(v,c) ({ u16 __v = v; __iowmb(); __arch_putw(__v,c); __v; }) > +#define writel(v,c) ({ u32 __v = v; __iowmb(); __arch_putl(__v,c); __v; }) Thanks a lot, can I have the do {} while() back? I still don't like such macro-making-sense-only constructs. (Just kidding). ;) Regards, Alexander
diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h index 3886f15..1fbc531 100644 --- a/arch/arm/include/asm/io.h +++ b/arch/arm/include/asm/io.h @@ -133,9 +133,9 @@ extern inline void __raw_readsl(unsigned int addr, void *data, int longlen) #define __iormb() dmb() #define __iowmb() dmb() -#define writeb(v,c) ({ __iowmb(); __arch_putb(v,c); v; }) -#define writew(v,c) ({ __iowmb(); __arch_putw(v,c); v; }) -#define writel(v,c) ({ __iowmb(); __arch_putl(v,c); v; }) +#define writeb(v,c) ({ u8 __v = v; __iowmb(); __arch_putb(__v,c); __v; }) +#define writew(v,c) ({ u16 __v = v; __iowmb(); __arch_putw(__v,c); __v; }) +#define writel(v,c) ({ u32 __v = v; __iowmb(); __arch_putl(__v,c); __v; }) #define readb(c) ({ u8 __v = __arch_getb(c); __iormb(); __v; }) #define readw(c) ({ u16 __v = __arch_getw(c); __iormb(); __v; })
Commit 3c0659b "ARM: Avoid compiler optimization for readb, writeb and friends." introduced I/O accessors with memory barriers. Unfortunately the new write*() accessors introduced a bug: The problem is that the argument "v" gets evaluated twice. This breaks code like used here (from "drivers/net/dnet.c"): for (i = 0; i < wrsz; i++) writel(*bufp++, &dnet->regs->TX_DATA_FIFO); Use auxiliary variables to avoid such problems. Signed-off-by: Wolfgang Denk <wd@denx.de> Cc: Albert Aribaud <albert.aribaud@free.fr> Cc: Alexander Holler <holler@ahsoftware.de> Cc: Dirk Behme <dirk.behme@googlemail.com> --- This patch fixes a pretty nasty problem. Everybody who has experienced strange failures on ARM lately should apply it ASAP and re-test. As far as I can tell at least the following drivers are affected: drivers/net/dnet.c: writel(*bufp++, &dnet->regs->TX_DATA_FIFO); drivers/usb/musb/musb_core.c: writeb(*data++, &musbr->fifox[ep]); drivers/mmc/pxa_mmc.c: writel(*src++, MMC_TXFIFO); drivers/mmc/mxcmmc.c: writel(*buf++, &host->base->buffer_access); drivers/spi/davinci_spi.c: writel(data1_reg_val | *txp++, &ds->regs->dat1); Albert, please apply ASAP! arch/arm/include/asm/io.h | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-)