Message ID | 1475086637-1914-2-git-send-email-fu.wei@linaro.org |
---|---|
State | New |
Headers | show |
Hi Mark On 20 October 2016 at 22:45, Mark Rutland <mark.rutland@arm.com> wrote: > Hi, > > On Thu, Sep 29, 2016 at 02:17:09AM +0800, fu.wei@linaro.org wrote: >> diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h >> index caedb74..6f06481 100644 >> --- a/include/clocksource/arm_arch_timer.h >> +++ b/include/clocksource/arm_arch_timer.h >> @@ -19,6 +19,9 @@ > > Please add: > > #include <linux/bitops.h> > > ... immediately before the includes below; it's needed to ensure that > BIT() is defined in all cases. Previously we were relying on implicit > header includes, which is not good practice. yes, you are right! added this, thanks ! > >> #include <linux/timecounter.h> >> #include <linux/types.h> >> >> +#define ARCH_CP15_TIMER BIT(0) >> +#define ARCH_MEM_TIMER BIT(1) > > If we're going to expose these in a header, it would be better to rename > them to something that makes their usage/meaning clear. These should > probably be ARCH_TIMER_TYPE_{CP15,MEM}. > > I guess this can wait for subsequent cleanup. > >> +enum ppi_nr { >> + PHYS_SECURE_PPI, >> + PHYS_NONSECURE_PPI, >> + VIRT_PPI, >> + HYP_PPI, >> + MAX_TIMER_PPI >> +}; > > Please rename this to arch_timer_ppi_nr (updating the single user in > drivers/clocksource/arm_arch_timer.c). That'll avoid the potential for > name clashes in files this happens to get included in (potentially > transitively via other headers). OK, NP, I have fixed this. > > With those changes (regardless of the ARCH_TIMER_TYPE_* bits): > > Acked-by: Mark Rutland <mark.rutland@arm.com> For ARCH_TIMER_TYPE_*, maybe I should add a patch at the end of this patchset to fix it, OK ? Thanks for ACK :-) > > Thanks, > Mark. > >> + >> #define ARCH_TIMER_PHYS_ACCESS 0 >> #define ARCH_TIMER_VIRT_ACCESS 1 >> #define ARCH_TIMER_MEM_PHYS_ACCESS 2 >> -- >> 2.7.4 >> -- Best regards, Fu Wei Software Engineer Red Hat _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 5770054..aea6c10 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -51,8 +51,6 @@ #define CNTV_TVAL 0x38 #define CNTV_CTL 0x3c -#define ARCH_CP15_TIMER BIT(0) -#define ARCH_MEM_TIMER BIT(1) static unsigned arch_timers_present __initdata; static void __iomem *arch_counter_base; @@ -65,15 +63,6 @@ struct arch_timer { #define to_arch_timer(e) container_of(e, struct arch_timer, evt) static u32 arch_timer_rate; - -enum ppi_nr { - PHYS_SECURE_PPI, - PHYS_NONSECURE_PPI, - VIRT_PPI, - HYP_PPI, - MAX_TIMER_PPI -}; - static int arch_timer_ppi[MAX_TIMER_PPI]; static struct clock_event_device __percpu *arch_timer_evt; diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h index caedb74..6f06481 100644 --- a/include/clocksource/arm_arch_timer.h +++ b/include/clocksource/arm_arch_timer.h @@ -19,6 +19,9 @@ #include <linux/timecounter.h> #include <linux/types.h> +#define ARCH_CP15_TIMER BIT(0) +#define ARCH_MEM_TIMER BIT(1) + #define ARCH_TIMER_CTRL_ENABLE (1 << 0) #define ARCH_TIMER_CTRL_IT_MASK (1 << 1) #define ARCH_TIMER_CTRL_IT_STAT (1 << 2) @@ -34,6 +37,14 @@ enum arch_timer_reg { ARCH_TIMER_REG_TVAL, }; +enum ppi_nr { + PHYS_SECURE_PPI, + PHYS_NONSECURE_PPI, + VIRT_PPI, + HYP_PPI, + MAX_TIMER_PPI +}; + #define ARCH_TIMER_PHYS_ACCESS 0 #define ARCH_TIMER_VIRT_ACCESS 1 #define ARCH_TIMER_MEM_PHYS_ACCESS 2