| Message ID | E1QM1ZG-0003DV-PA@rmk-PC.arm.linux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 16 May 2011 18:26, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > --- a/arch/arm/mach-omap1/time.c > +++ b/arch/arm/mach-omap1/time.c ... > static inline unsigned long notrace omap_mpu_timer_read(int nr) > { > - volatile omap_mpu_timer_regs_t* timer = omap_mpu_timer_base(nr); > - return timer->read_tim; > + omap_mpu_timer_regs_t __iomem *timer = omap_mpu_timer_base(nr); > + return readl(&timer->read_tim); > } We should start using the *_relaxed() accessors a bit more to avoid the barriers overhead in the standard I/O accessors.
On Tue, May 17, 2011 at 10:59:28PM +0100, Catalin Marinas wrote: > On 16 May 2011 18:26, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > > --- a/arch/arm/mach-omap1/time.c > > +++ b/arch/arm/mach-omap1/time.c > ... > > static inline unsigned long notrace omap_mpu_timer_read(int nr) > > { > > - volatile omap_mpu_timer_regs_t* timer = omap_mpu_timer_base(nr); > > - return timer->read_tim; > > + omap_mpu_timer_regs_t __iomem *timer = omap_mpu_timer_base(nr); > > + return readl(&timer->read_tim); > > } > > We should start using the *_relaxed() accessors a bit more to avoid > the barriers overhead in the standard I/O accessors. I thought about that, but when you look at patch 6, it'd change this. I wanted to use the same accessor here as it ends up with in patch 6. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2011/5/17 Catalin Marinas <catalin.marinas@arm.com>: > On 16 May 2011 18:26, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: >> --- a/arch/arm/mach-omap1/time.c >> +++ b/arch/arm/mach-omap1/time.c > ... >> static inline unsigned long notrace omap_mpu_timer_read(int nr) >> { >> - volatile omap_mpu_timer_regs_t* timer = omap_mpu_timer_base(nr); >> - return timer->read_tim; >> + omap_mpu_timer_regs_t __iomem *timer = omap_mpu_timer_base(nr); >> + return readl(&timer->read_tim); >> } > > We should start using the *_relaxed() accessors a bit more to avoid > the barriers overhead in the standard I/O accessors. Speaking of which. The documentation for the *_relaxed calls are in Documentation/DocBook/deviceiobook.tmpl and reads like this: "PCI ordering rules also guarantee that PIO read responses arrive after any outstanding DMA writes from that bus, since for some devices the result of a readb call may signal to the driver that a DMA transaction is complete. In many cases, however, the driver may want to indicate that the next readb call has no relation to any previous DMA writes performed by the device. The driver can use readb_relaxed for these cases, although only some platforms will honor the relaxed semantics. Using the relaxed read functions will provide significant performance benefits on platforms that support it. The qla2xxx driver provides examples of how to use readX_relaxed. In many cases, a majority of the driver's readX calls can safely be converted to readX_relaxed calls, since only a few will indicate or depend on DMA completion." I guess that in the ARM case "PCI DMA" corresponds to "bus mastering" but is this strictly speaking properly describing the semantics of the ARM *_relaxed operations? If it isn't a hint on how it should be understood would be much appreciated. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-omap1/time.c b/arch/arm/mach-omap1/time.c index e2c29b4..e7ab616 100644 --- a/arch/arm/mach-omap1/time.c +++ b/arch/arm/mach-omap1/time.c @@ -68,49 +68,50 @@ typedef struct { } omap_mpu_timer_regs_t; #define omap_mpu_timer_base(n) \ -((volatile omap_mpu_timer_regs_t*)OMAP1_IO_ADDRESS(OMAP_MPU_TIMER_BASE + \ +((omap_mpu_timer_regs_t __iomem *)OMAP1_IO_ADDRESS(OMAP_MPU_TIMER_BASE + \ (n)*OMAP_MPU_TIMER_OFFSET)) static inline unsigned long notrace omap_mpu_timer_read(int nr) { - volatile omap_mpu_timer_regs_t* timer = omap_mpu_timer_base(nr); - return timer->read_tim; + omap_mpu_timer_regs_t __iomem *timer = omap_mpu_timer_base(nr); + return readl(&timer->read_tim); } static inline void omap_mpu_set_autoreset(int nr) { - volatile omap_mpu_timer_regs_t* timer = omap_mpu_timer_base(nr); + omap_mpu_timer_regs_t __iomem *timer = omap_mpu_timer_base(nr); - timer->cntl = timer->cntl | MPU_TIMER_AR; + writel(readl(&timer->cntl) | MPU_TIMER_AR, &timer->cntl); } static inline void omap_mpu_remove_autoreset(int nr) { - volatile omap_mpu_timer_regs_t* timer = omap_mpu_timer_base(nr); + omap_mpu_timer_regs_t __iomem *timer = omap_mpu_timer_base(nr); - timer->cntl = timer->cntl & ~MPU_TIMER_AR; + writel(readl(&timer->cntl) & ~MPU_TIMER_AR, &timer->cntl); } static inline void omap_mpu_timer_start(int nr, unsigned long load_val, int autoreset) { - volatile omap_mpu_timer_regs_t* timer = omap_mpu_timer_base(nr); - unsigned int timerflags = (MPU_TIMER_CLOCK_ENABLE | MPU_TIMER_ST); + omap_mpu_timer_regs_t __iomem *timer = omap_mpu_timer_base(nr); + unsigned int timerflags = MPU_TIMER_CLOCK_ENABLE | MPU_TIMER_ST; - if (autoreset) timerflags |= MPU_TIMER_AR; + if (autoreset) + timerflags |= MPU_TIMER_AR; - timer->cntl = MPU_TIMER_CLOCK_ENABLE; + writel(MPU_TIMER_CLOCK_ENABLE, &timer->cntl); udelay(1); - timer->load_tim = load_val; + writel(load_val, &timer->load_tim); udelay(1); - timer->cntl = timerflags; + writel(timerflags, &timer->cntl); } static inline void omap_mpu_timer_stop(int nr) { - volatile omap_mpu_timer_regs_t* timer = omap_mpu_timer_base(nr); + omap_mpu_timer_regs_t __iomem *timer = omap_mpu_timer_base(nr); - timer->cntl &= ~MPU_TIMER_ST; + writel(readl(&timer->cntl) & ~MPU_TIMER_ST, &timer->cntl); } /*