diff mbox

[03/22] clocksource/drivers/rockchip: Make the driver more compatible

Message ID 1446469011-22710-3-git-send-email-daniel.lezcano@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Lezcano Nov. 2, 2015, 12:56 p.m. UTC 
From: Caesar Wang <wxt@rock-chips.com>

Build the arm64 SoCs (e.g.: RK3368) on Rockchip platform,
There are some failure with build up on timer driver for rockchip.

Says:
/tmp/ccdAnNy5.s:47: Error: missing immediate expression at  operand 1 --
`dsb`
...

The problem was different semantics of dsb on btw arm32 and arm64,
Here we can convert the dsb with insteading of dsb(sy).The "sy" param
is the default which you are allow to omit, so on arm32 dsb()and dsb(sy)
are the same.

Signed-off-by: Caesar Wang <wxt@rock-chips.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/rockchip_timer.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Arnd Bergmann Nov. 2, 2015, 3:33 p.m. UTC | #1 
On Monday 02 November 2015 13:56:31 Daniel Lezcano wrote:
>  static inline void rk_timer_disable(struct clock_event_device *ce)
>  {
>         writel_relaxed(TIMER_DISABLE, rk_base(ce) + TIMER_CONTROL_REG);
> -       dsb();
> +       dsb(sy);
>  }
>  
>  static inline void rk_timer_enable(struct clock_event_device *ce, u32 flags)
>  {
>         writel_relaxed(TIMER_ENABLE | TIMER_INT_UNMASK | flags,
>                        rk_base(ce) + TIMER_CONTROL_REG);
> -       dsb();
> +       dsb(sy);
>  }
>  
> 

This will fail the compile test, because dsb() is not available on non-ARM
architectures. Would it be enough to just use the normal writel() accessor
here?

	Arnd
Daniel Lezcano Nov. 2, 2015, 4:32 p.m. UTC | #2 
On 11/02/2015 04:33 PM, Arnd Bergmann wrote:
> On Monday 02 November 2015 13:56:31 Daniel Lezcano wrote:
>>   static inline void rk_timer_disable(struct clock_event_device *ce)
>>   {
>>          writel_relaxed(TIMER_DISABLE, rk_base(ce) + TIMER_CONTROL_REG);
>> -       dsb();
>> +       dsb(sy);
>>   }
>>
>>   static inline void rk_timer_enable(struct clock_event_device *ce, u32 flags)
>>   {
>>          writel_relaxed(TIMER_ENABLE | TIMER_INT_UNMASK | flags,
>>                         rk_base(ce) + TIMER_CONTROL_REG);
>> -       dsb();
>> +       dsb(sy);
>>   }
>>
>>
>
> This will fail the compile test, because dsb() is not available on non-ARM
> architectures. Would it be enough to just use the normal writel() accessor
> here?

That's a good question and I believe we can remove it but I have to 
setup a rockchip board before doing the changes in order to test.

I the meantime added the COMPILE_TEST option but restricted it to ARM 
and ARM64.
Arnd Bergmann Nov. 2, 2015, 9:44 p.m. UTC | #3 
On Monday 02 November 2015 17:32:22 Daniel Lezcano wrote:
> On 11/02/2015 04:33 PM, Arnd Bergmann wrote:
> > On Monday 02 November 2015 13:56:31 Daniel Lezcano wrote:
> >>   static inline void rk_timer_disable(struct clock_event_device *ce)
> >>   {
> >>          writel_relaxed(TIMER_DISABLE, rk_base(ce) + TIMER_CONTROL_REG);
> >> -       dsb();
> >> +       dsb(sy);
> >>   }
> >>
> >>   static inline void rk_timer_enable(struct clock_event_device *ce, u32 flags)
> >>   {
> >>          writel_relaxed(TIMER_ENABLE | TIMER_INT_UNMASK | flags,
> >>                         rk_base(ce) + TIMER_CONTROL_REG);
> >> -       dsb();
> >> +       dsb(sy);
> >>   }
> >>
> >>
> >
> > This will fail the compile test, because dsb() is not available on non-ARM
> > architectures. Would it be enough to just use the normal writel() accessor
> > here?
> 
> That's a good question and I believe we can remove it but I have to 
> setup a rockchip board before doing the changes in order to test.
> 
> I the meantime added the COMPILE_TEST option but restricted it to ARM 
> and ARM64.
> 

Ok. I saw that addition after commenting here, it looks correct this
way, I was just slightly confused by seeing patch 2 first and thought
it was for the same driver.

In general, it would of course be best to allow all drivers to be built
on x86, but your series is already a huge improvement as it is.

	Arnd
diff mbox

Patch

diff --git a/drivers/clocksource/rockchip_timer.c b/drivers/clocksource/rockchip_timer.c
index d3c1742..724c321 100644
--- a/drivers/clocksource/rockchip_timer.c
+++ b/drivers/clocksource/rockchip_timer.c
@@ -49,14 +49,14 @@  static inline void __iomem *rk_base(struct clock_event_device *ce)
 static inline void rk_timer_disable(struct clock_event_device *ce)
 {
 	writel_relaxed(TIMER_DISABLE, rk_base(ce) + TIMER_CONTROL_REG);
-	dsb();
+	dsb(sy);
 }
 
 static inline void rk_timer_enable(struct clock_event_device *ce, u32 flags)
 {
 	writel_relaxed(TIMER_ENABLE | TIMER_INT_UNMASK | flags,
 		       rk_base(ce) + TIMER_CONTROL_REG);
-	dsb();
+	dsb(sy);
 }
 
 static void rk_timer_update_counter(unsigned long cycles,
@@ -64,13 +64,13 @@  static void rk_timer_update_counter(unsigned long cycles,
 {
 	writel_relaxed(cycles, rk_base(ce) + TIMER_LOAD_COUNT0);
 	writel_relaxed(0, rk_base(ce) + TIMER_LOAD_COUNT1);
-	dsb();
+	dsb(sy);
 }
 
 static void rk_timer_interrupt_clear(struct clock_event_device *ce)
 {
 	writel_relaxed(1, rk_base(ce) + TIMER_INT_STATUS);
-	dsb();
+	dsb(sy);
 }
 
 static inline int rk_timer_set_next_event(unsigned long cycles,