| Message ID | 1479304148-2965-9-git-send-email-fu.wei@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Nov 16, 2016 at 09:49:01PM +0800, fu.wei@linaro.org wrote: > From: Fu Wei <fu.wei@linaro.org> > > The patch refactor original arch_timer_needs_probing function: > (1) Separate out arch_timer_needs_probing from arch_timer_common_init, > and call it only if acpi disabled. > (2) Rename arch_timer_needs_probing to arch_timer_needs_of_probing. Please write a real commit message. > Signed-off-by: Fu Wei <fu.wei@linaro.org> > --- > drivers/clocksource/arm_arch_timer.c | 34 +++++++++++++++++++--------------- > 1 file changed, 19 insertions(+), 15 deletions(-) > > diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c > index fe4e812..9ddc091 100644 > --- a/drivers/clocksource/arm_arch_timer.c > +++ b/drivers/clocksource/arm_arch_timer.c > @@ -792,15 +792,28 @@ static const struct of_device_id arch_timer_mem_of_match[] __initconst = { > {}, > }; > > -static bool __init > -arch_timer_needs_probing(int type, const struct of_device_id *matches) > +static bool __init arch_timer_needs_of_probing(void) > { > struct device_node *dn; > bool needs_probing = false; > + unsigned int mask = ARCH_TIMER_TYPE_CP15 | ARCH_TIMER_TYPE_MEM; > + > + /* We have two timers, and both device-tree nodes are probed. */ > + if ((arch_timers_present & mask) == mask) > + return false; > + > + /* > + * Only one type of timer is probed, > + * check if we have another type of timer node in device-tree. > + */ > + if (arch_timers_present & ARCH_TIMER_TYPE_CP15) > + dn = of_find_matching_node(NULL, arch_timer_mem_of_match); > + else > + dn = of_find_matching_node(NULL, arch_timer_of_match); > > - dn = of_find_matching_node(NULL, matches); > - if (dn && of_device_is_available(dn) && !(arch_timers_present & type)) > + if (dn && of_device_is_available(dn)) > needs_probing = true; > + > of_node_put(dn); > > return needs_probing; > @@ -808,17 +821,8 @@ arch_timer_needs_probing(int type, const struct of_device_id *matches) > > static int __init arch_timer_common_init(void) > { > - unsigned int mask = ARCH_TIMER_TYPE_CP15 | ARCH_TIMER_TYPE_MEM; > - > - /* Wait until both nodes are probed if we have two timers */ > - if ((arch_timers_present & mask) != mask) { > - if (arch_timer_needs_probing(ARCH_TIMER_TYPE_MEM, > - arch_timer_mem_of_match)) > - return 0; > - if (arch_timer_needs_probing(ARCH_TIMER_TYPE_CP15, > - arch_timer_of_match)) > - return 0; > - } Why can't we just move this into the DT-specific caller of arch_timer_common_init()? Thanks Mark. > + if (acpi_disabled && arch_timer_needs_of_probing()) > + return 0; > > arch_timer_banner(arch_timers_present); > arch_counter_register(arch_timers_present); > -- > 2.7.4 >
Hi Mark, On 19 November 2016 at 03:56, Mark Rutland <mark.rutland@arm.com> wrote: > On Wed, Nov 16, 2016 at 09:49:01PM +0800, fu.wei@linaro.org wrote: >> From: Fu Wei <fu.wei@linaro.org> >> >> The patch refactor original arch_timer_needs_probing function: >> (1) Separate out arch_timer_needs_probing from arch_timer_common_init, >> and call it only if acpi disabled. >> (2) Rename arch_timer_needs_probing to arch_timer_needs_of_probing. > > Please write a real commit message. OK, how about this: clocksource/drivers/arm_arch_timer: Refactor arch_timer_needs_probing, only use it in the DT init code Currently, the arch_timer_common_init uses arch_timer_needs_probing call to wait for both timers been probed which is only for booting from DT. For booting from ACPI, we don't need these in arch_timer_common_init, these code should be only in the DT init code. This patch seperates arch_timer_needs_probing related code out into a new function named arch_timer_needs_of_probing, and then reworks arch_timer_of_init to use it, so that we can keep this only in the DT init code. > >> Signed-off-by: Fu Wei <fu.wei@linaro.org> >> --- >> drivers/clocksource/arm_arch_timer.c | 34 +++++++++++++++++++--------------- >> 1 file changed, 19 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c >> index fe4e812..9ddc091 100644 >> --- a/drivers/clocksource/arm_arch_timer.c >> +++ b/drivers/clocksource/arm_arch_timer.c >> @@ -792,15 +792,28 @@ static const struct of_device_id arch_timer_mem_of_match[] __initconst = { >> {}, >> }; >> >> -static bool __init >> -arch_timer_needs_probing(int type, const struct of_device_id *matches) >> +static bool __init arch_timer_needs_of_probing(void) >> { >> struct device_node *dn; >> bool needs_probing = false; >> + unsigned int mask = ARCH_TIMER_TYPE_CP15 | ARCH_TIMER_TYPE_MEM; >> + >> + /* We have two timers, and both device-tree nodes are probed. */ >> + if ((arch_timers_present & mask) == mask) >> + return false; >> + >> + /* >> + * Only one type of timer is probed, >> + * check if we have another type of timer node in device-tree. >> + */ >> + if (arch_timers_present & ARCH_TIMER_TYPE_CP15) >> + dn = of_find_matching_node(NULL, arch_timer_mem_of_match); >> + else >> + dn = of_find_matching_node(NULL, arch_timer_of_match); >> >> - dn = of_find_matching_node(NULL, matches); >> - if (dn && of_device_is_available(dn) && !(arch_timers_present & type)) >> + if (dn && of_device_is_available(dn)) >> needs_probing = true; >> + >> of_node_put(dn); >> >> return needs_probing; >> @@ -808,17 +821,8 @@ arch_timer_needs_probing(int type, const struct of_device_id *matches) >> >> static int __init arch_timer_common_init(void) >> { >> - unsigned int mask = ARCH_TIMER_TYPE_CP15 | ARCH_TIMER_TYPE_MEM; >> - >> - /* Wait until both nodes are probed if we have two timers */ >> - if ((arch_timers_present & mask) != mask) { >> - if (arch_timer_needs_probing(ARCH_TIMER_TYPE_MEM, >> - arch_timer_mem_of_match)) >> - return 0; >> - if (arch_timer_needs_probing(ARCH_TIMER_TYPE_CP15, >> - arch_timer_of_match)) >> - return 0; >> - } > > Why can't we just move this into the DT-specific caller of > arch_timer_common_init()? yes, I have thought about this, but that means we have to split arch_timer_init, and use arch_timer_register and arch_timer_common_init directly in arch_timer_of_init. In another word, I also need to modify arch_timer_of_init, not only arch_timer_common_init, but let me have a try in v17, see if you like it. > > Thanks > Mark. > >> + if (acpi_disabled && arch_timer_needs_of_probing()) >> + return 0; >> >> arch_timer_banner(arch_timers_present); >> arch_counter_register(arch_timers_present); >> -- >> 2.7.4 >>
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index fe4e812..9ddc091 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -792,15 +792,28 @@ static const struct of_device_id arch_timer_mem_of_match[] __initconst = { {}, }; -static bool __init -arch_timer_needs_probing(int type, const struct of_device_id *matches) +static bool __init arch_timer_needs_of_probing(void) { struct device_node *dn; bool needs_probing = false; + unsigned int mask = ARCH_TIMER_TYPE_CP15 | ARCH_TIMER_TYPE_MEM; + + /* We have two timers, and both device-tree nodes are probed. */ + if ((arch_timers_present & mask) == mask) + return false; + + /* + * Only one type of timer is probed, + * check if we have another type of timer node in device-tree. + */ + if (arch_timers_present & ARCH_TIMER_TYPE_CP15) + dn = of_find_matching_node(NULL, arch_timer_mem_of_match); + else + dn = of_find_matching_node(NULL, arch_timer_of_match); - dn = of_find_matching_node(NULL, matches); - if (dn && of_device_is_available(dn) && !(arch_timers_present & type)) + if (dn && of_device_is_available(dn)) needs_probing = true; + of_node_put(dn); return needs_probing; @@ -808,17 +821,8 @@ arch_timer_needs_probing(int type, const struct of_device_id *matches) static int __init arch_timer_common_init(void) { - unsigned int mask = ARCH_TIMER_TYPE_CP15 | ARCH_TIMER_TYPE_MEM; - - /* Wait until both nodes are probed if we have two timers */ - if ((arch_timers_present & mask) != mask) { - if (arch_timer_needs_probing(ARCH_TIMER_TYPE_MEM, - arch_timer_mem_of_match)) - return 0; - if (arch_timer_needs_probing(ARCH_TIMER_TYPE_CP15, - arch_timer_of_match)) - return 0; - } + if (acpi_disabled && arch_timer_needs_of_probing()) + return 0; arch_timer_banner(arch_timers_present); arch_counter_register(arch_timers_present);