| Message ID | 1374661682-9349-7-git-send-email-ben.dooks@codethink.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 24, 2013 at 11:27:54AM +0100, Ben Dooks wrote: > Ensure the twd driver uses the correct calls to access the hardware > to ensure that we do not end up with data in the wrong endian format. > > Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> Reviewed-by: Will Deacon <will.deacon@arm.com> There are a bunch of other __raw_* users under arch/arm/, but I suppose we can let other people move those over as they see fit. It wouldn't hurt to fix arch/arm/include/asm/{cti,hardware/coresight.h} though. Will
On 24/07/13 15:49, Will Deacon wrote: > On Wed, Jul 24, 2013 at 11:27:54AM +0100, Ben Dooks wrote: >> Ensure the twd driver uses the correct calls to access the hardware >> to ensure that we do not end up with data in the wrong endian format. >> >> Signed-off-by: Ben Dooks<ben.dooks@codethink.co.uk> > > Reviewed-by: Will Deacon<will.deacon@arm.com> > > There are a bunch of other __raw_* users under arch/arm/, but I suppose we > can let other people move those over as they see fit. It wouldn't hurt to > fix arch/arm/include/asm/{cti,hardware/coresight.h} though. I will have a look at these. Quite a lot of the machine specific stuff I decided to leave alone as fixing everything was out of my remit, and is best left to a later effort.
On 24/07/13 15:49, Will Deacon wrote: > On Wed, Jul 24, 2013 at 11:27:54AM +0100, Ben Dooks wrote: >> Ensure the twd driver uses the correct calls to access the hardware >> to ensure that we do not end up with data in the wrong endian format. >> >> Signed-off-by: Ben Dooks<ben.dooks@codethink.co.uk> > > Reviewed-by: Will Deacon<will.deacon@arm.com> > > There are a bunch of other __raw_* users under arch/arm/, but I suppose we > can let other people move those over as they see fit. It wouldn't hurt to > fix arch/arm/include/asm/{cti,hardware/coresight.h} though. I didn't think to look for things under there. Do people think it would be worth just changing __raw_read/__raw_write functions to also take into account endian-ness?
On Wed, Jul 24, 2013 at 04:06:58PM +0100, Ben Dooks wrote: > On 24/07/13 15:49, Will Deacon wrote: > > On Wed, Jul 24, 2013 at 11:27:54AM +0100, Ben Dooks wrote: > >> Ensure the twd driver uses the correct calls to access the hardware > >> to ensure that we do not end up with data in the wrong endian format. > >> > >> Signed-off-by: Ben Dooks<ben.dooks@codethink.co.uk> > > > > Reviewed-by: Will Deacon<will.deacon@arm.com> > > > > There are a bunch of other __raw_* users under arch/arm/, but I suppose we > > can let other people move those over as they see fit. It wouldn't hurt to > > fix arch/arm/include/asm/{cti,hardware/coresight.h} though. > > I didn't think to look for things under there. > > Do people think it would be worth just changing __raw_read/__raw_write > functions to also take into account endian-ness? That's probably a bit too far and will likely break some of the many drivers which use these accessors. It also puts us at odds with other architectures offering these accessors, so we're better off just fixing the callsites as appropriate. Will
diff --git a/arch/arm/kernel/smp_twd.c b/arch/arm/kernel/smp_twd.c index 90525d9..804d3f8 100644 --- a/arch/arm/kernel/smp_twd.c +++ b/arch/arm/kernel/smp_twd.c @@ -45,7 +45,7 @@ static void twd_set_mode(enum clock_event_mode mode, case CLOCK_EVT_MODE_PERIODIC: ctrl = TWD_TIMER_CONTROL_ENABLE | TWD_TIMER_CONTROL_IT_ENABLE | TWD_TIMER_CONTROL_PERIODIC; - __raw_writel(DIV_ROUND_CLOSEST(twd_timer_rate, HZ), + writel_relaxed(DIV_ROUND_CLOSEST(twd_timer_rate, HZ), twd_base + TWD_TIMER_LOAD); break; case CLOCK_EVT_MODE_ONESHOT: @@ -58,18 +58,18 @@ static void twd_set_mode(enum clock_event_mode mode, ctrl = 0; } - __raw_writel(ctrl, twd_base + TWD_TIMER_CONTROL); + writel_relaxed(ctrl, twd_base + TWD_TIMER_CONTROL); } static int twd_set_next_event(unsigned long evt, struct clock_event_device *unused) { - unsigned long ctrl = __raw_readl(twd_base + TWD_TIMER_CONTROL); + unsigned long ctrl = readl_relaxed(twd_base + TWD_TIMER_CONTROL); ctrl |= TWD_TIMER_CONTROL_ENABLE; - __raw_writel(evt, twd_base + TWD_TIMER_COUNTER); - __raw_writel(ctrl, twd_base + TWD_TIMER_CONTROL); + writel_relaxed(evt, twd_base + TWD_TIMER_COUNTER); + writel_relaxed(ctrl, twd_base + TWD_TIMER_CONTROL); return 0; } @@ -82,8 +82,8 @@ static int twd_set_next_event(unsigned long evt, */ static int twd_timer_ack(void) { - if (__raw_readl(twd_base + TWD_TIMER_INTSTAT)) { - __raw_writel(1, twd_base + TWD_TIMER_INTSTAT); + if (readl_relaxed(twd_base + TWD_TIMER_INTSTAT)) { + writel_relaxed(1, twd_base + TWD_TIMER_INTSTAT); return 1; } @@ -209,15 +209,15 @@ static void __cpuinit twd_calibrate_rate(void) waitjiffies += 5; /* enable, no interrupt or reload */ - __raw_writel(0x1, twd_base + TWD_TIMER_CONTROL); + writel_relaxed(0x1, twd_base + TWD_TIMER_CONTROL); /* maximum value */ - __raw_writel(0xFFFFFFFFU, twd_base + TWD_TIMER_COUNTER); + writel_relaxed(0xFFFFFFFFU, twd_base + TWD_TIMER_COUNTER); while (get_jiffies_64() < waitjiffies) udelay(10); - count = __raw_readl(twd_base + TWD_TIMER_COUNTER); + count = readl_relaxed(twd_base + TWD_TIMER_COUNTER); twd_timer_rate = (0xFFFFFFFFU - count) * (HZ / 5); @@ -275,7 +275,7 @@ static int __cpuinit twd_timer_setup(struct clock_event_device *clk) * bother with the below. */ if (per_cpu(percpu_setup_called, cpu)) { - __raw_writel(0, twd_base + TWD_TIMER_CONTROL); + writel_relaxed(0, twd_base + TWD_TIMER_CONTROL); clockevents_register_device(*__this_cpu_ptr(twd_evt)); enable_percpu_irq(clk->irq, 0); return 0; @@ -288,7 +288,7 @@ static int __cpuinit twd_timer_setup(struct clock_event_device *clk) * The following is done once per CPU the first time .setup() is * called. */ - __raw_writel(0, twd_base + TWD_TIMER_CONTROL); + writel_relaxed(0, twd_base + TWD_TIMER_CONTROL); clk->name = "local_timer"; clk->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT |
Ensure the twd driver uses the correct calls to access the hardware to ensure that we do not end up with data in the wrong endian format. Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk> --- arch/arm/kernel/smp_twd.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)