* [PATCH 1/9] ARC: timer: gfrc, rtc: Read BCR to detect whether hardware exists ...
2016-10-31 22:48 ` [PATCH 0/9] Move ARC timer code into drivers/clocksource/ Vineet Gupta
@ 2016-10-31 22:48 ` Vineet Gupta
2016-11-03 17:00 ` Daniel Lezcano
2016-10-31 22:48 ` [PATCH 2/9] ARC: timer: rtc: implement read loop in "C" vs. inline asm Vineet Gupta
` (8 subsequent siblings)
9 siblings, 1 reply; 52+ messages in thread
From: Vineet Gupta @ 2016-10-31 22:48 UTC (permalink / raw)
To: Daniel Lezcano
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin,
Vineet Gupta
... don't rely on cpuinfo populated in arc boot code. This paves wat for
moving this code in drivers/clocksource/
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
arch/arc/kernel/time.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/arc/kernel/time.c b/arch/arc/kernel/time.c
index f927b8dc6edd..a2db010cde18 100644
--- a/arch/arc/kernel/time.c
+++ b/arch/arc/kernel/time.c
@@ -118,10 +118,11 @@ static struct clocksource arc_counter_gfrc = {
static int __init arc_cs_setup_gfrc(struct device_node *node)
{
- int exists = cpuinfo_arc700[0].extn.gfrc;
+ struct mcip_bcr mp;
int ret;
- if (WARN(!exists, "Global-64-bit-Ctr clocksource not detected"))
+ READ_BCR(ARC_REG_MCIP_BCR, mp);
+ if (WARN(!mp.gfrc, "Global-64-bit-Ctr clocksource not detected"))
return -ENXIO;
ret = arc_get_timer_clk(node);
@@ -174,10 +175,11 @@ static struct clocksource arc_counter_rtc = {
static int __init arc_cs_setup_rtc(struct device_node *node)
{
- int exists = cpuinfo_arc700[smp_processor_id()].extn.rtc;
+ struct bcr_timer timer;
int ret;
- if (WARN(!exists, "Local-64-bit-Ctr clocksource not detected"))
+ READ_BCR(ARC_REG_TIMERS_BCR, timer);
+ if (WARN(!timer.rtc, "Local-64-bit-Ctr clocksource not detected"))
return -ENXIO;
/* Local to CPU hence not usable in SMP */
--
2.7.4
^ permalink raw reply related [flat|nested] 52+ messages in thread
* Re: [PATCH 1/9] ARC: timer: gfrc, rtc: Read BCR to detect whether hardware exists ...
2016-10-31 22:48 ` [PATCH 1/9] ARC: timer: gfrc, rtc: Read BCR to detect whether hardware exists Vineet Gupta
@ 2016-11-03 17:00 ` Daniel Lezcano
2016-11-03 17:41 ` Vineet Gupta
0 siblings, 1 reply; 52+ messages in thread
From: Daniel Lezcano @ 2016-11-03 17:00 UTC (permalink / raw)
To: Vineet Gupta
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin
On Mon, Oct 31, 2016 at 03:48:08PM -0700, Vineet Gupta wrote:
> ... don't rely on cpuinfo populated in arc boot code. This paves wat for
> moving this code in drivers/clocksource/
>
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> ---
> arch/arc/kernel/time.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arc/kernel/time.c b/arch/arc/kernel/time.c
> index f927b8dc6edd..a2db010cde18 100644
> --- a/arch/arc/kernel/time.c
> +++ b/arch/arc/kernel/time.c
> @@ -118,10 +118,11 @@ static struct clocksource arc_counter_gfrc = {
>
> static int __init arc_cs_setup_gfrc(struct device_node *node)
> {
> - int exists = cpuinfo_arc700[0].extn.gfrc;
> + struct mcip_bcr mp;
> int ret;
>
> - if (WARN(!exists, "Global-64-bit-Ctr clocksource not detected"))
> + READ_BCR(ARC_REG_MCIP_BCR, mp);
> + if (WARN(!mp.gfrc, "Global-64-bit-Ctr clocksource not detected"))
Take the opportunity to replace this WARN by a pr_err.
> return -ENXIO;
>
> ret = arc_get_timer_clk(node);
> @@ -174,10 +175,11 @@ static struct clocksource arc_counter_rtc = {
>
> static int __init arc_cs_setup_rtc(struct device_node *node)
> {
> - int exists = cpuinfo_arc700[smp_processor_id()].extn.rtc;
> + struct bcr_timer timer;
> int ret;
>
> - if (WARN(!exists, "Local-64-bit-Ctr clocksource not detected"))
> + READ_BCR(ARC_REG_TIMERS_BCR, timer);
> + if (WARN(!timer.rtc, "Local-64-bit-Ctr clocksource not detected"))
> return -ENXIO;o
Ditto and ^^
So the READ_BCR() is only there to check the timer is physically present ?
> /* Local to CPU hence not usable in SMP */
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 1/9] ARC: timer: gfrc, rtc: Read BCR to detect whether hardware exists ...
2016-11-03 17:00 ` Daniel Lezcano
@ 2016-11-03 17:41 ` Vineet Gupta
0 siblings, 0 replies; 52+ messages in thread
From: Vineet Gupta @ 2016-11-03 17:41 UTC (permalink / raw)
To: Daniel Lezcano
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin
On 11/03/2016 10:00 AM, Daniel Lezcano wrote:
> On Mon, Oct 31, 2016 at 03:48:08PM -0700, Vineet Gupta wrote:
>> ... don't rely on cpuinfo populated in arc boot code. This paves wat for
>> moving this code in drivers/clocksource/
>>
>> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
>> ---
>> arch/arc/kernel/time.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arc/kernel/time.c b/arch/arc/kernel/time.c
>> index f927b8dc6edd..a2db010cde18 100644
>> --- a/arch/arc/kernel/time.c
>> +++ b/arch/arc/kernel/time.c
>> @@ -118,10 +118,11 @@ static struct clocksource arc_counter_gfrc = {
>>
>> static int __init arc_cs_setup_gfrc(struct device_node *node)
>> {
>> - int exists = cpuinfo_arc700[0].extn.gfrc;
>> + struct mcip_bcr mp;
>> int ret;
>>
>> - if (WARN(!exists, "Global-64-bit-Ctr clocksource not detected"))
>> + READ_BCR(ARC_REG_MCIP_BCR, mp);
>> + if (WARN(!mp.gfrc, "Global-64-bit-Ctr clocksource not detected"))
>
> Take the opportunity to replace this WARN by a pr_err.
OK.
>
>> return -ENXIO;
>>
>> ret = arc_get_timer_clk(node);
>> @@ -174,10 +175,11 @@ static struct clocksource arc_counter_rtc = {
>>
>> static int __init arc_cs_setup_rtc(struct device_node *node)
>> {
>> - int exists = cpuinfo_arc700[smp_processor_id()].extn.rtc;
>> + struct bcr_timer timer;
>> int ret;
>>
>> - if (WARN(!exists, "Local-64-bit-Ctr clocksource not detected"))
>> + READ_BCR(ARC_REG_TIMERS_BCR, timer);
>> + if (WARN(!timer.rtc, "Local-64-bit-Ctr clocksource not detected"))
>> return -ENXIO;o
>
> Ditto and ^^
>
> So the READ_BCR() is only there to check the timer is physically present ?
Yep, due to configurable nature of cores, we have Build Config Registers to detect
at runtime what is present or not. This allows for boot printing at the minimum.
This is defined in arcregs.h and in newly introduced soc/arc/aux.h
>
>> /* Local to CPU hence not usable in SMP */
>> --
>> 2.7.4
>>
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH 2/9] ARC: timer: rtc: implement read loop in "C" vs. inline asm
2016-10-31 22:48 ` [PATCH 0/9] Move ARC timer code into drivers/clocksource/ Vineet Gupta
2016-10-31 22:48 ` [PATCH 1/9] ARC: timer: gfrc, rtc: Read BCR to detect whether hardware exists Vineet Gupta
@ 2016-10-31 22:48 ` Vineet Gupta
2016-11-03 17:02 ` Daniel Lezcano
2016-10-31 22:48 ` [PATCH 3/9] ARC: timer: gfrc: boot print alongside other timers Vineet Gupta
` (7 subsequent siblings)
9 siblings, 1 reply; 52+ messages in thread
From: Vineet Gupta @ 2016-10-31 22:48 UTC (permalink / raw)
To: Daniel Lezcano
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin,
Vineet Gupta
To allow for easy movement into drivers/clocksource
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
arch/arc/kernel/time.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/arch/arc/kernel/time.c b/arch/arc/kernel/time.c
index a2db010cde18..2c51e3cafad0 100644
--- a/arch/arc/kernel/time.c
+++ b/arch/arc/kernel/time.c
@@ -153,14 +153,11 @@ static cycle_t arc_read_rtc(struct clocksource *cs)
cycle_t full;
} stamp;
-
- __asm__ __volatile(
- "1: \n"
- " lr %0, [AUX_RTC_LOW] \n"
- " lr %1, [AUX_RTC_HIGH] \n"
- " lr %2, [AUX_RTC_CTRL] \n"
- " bbit0.nt %2, 31, 1b \n"
- : "=r" (stamp.low), "=r" (stamp.high), "=r" (status));
+ do {
+ stamp.low = read_aux_reg(AUX_RTC_LOW);
+ stamp.high = read_aux_reg(AUX_RTC_HIGH);
+ status = read_aux_reg(AUX_RTC_CTRL);
+ } while (!(status & 0x80000000UL));
return stamp.full;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 52+ messages in thread
* Re: [PATCH 2/9] ARC: timer: rtc: implement read loop in "C" vs. inline asm
2016-10-31 22:48 ` [PATCH 2/9] ARC: timer: rtc: implement read loop in "C" vs. inline asm Vineet Gupta
@ 2016-11-03 17:02 ` Daniel Lezcano
2016-11-03 17:45 ` Vineet Gupta
0 siblings, 1 reply; 52+ messages in thread
From: Daniel Lezcano @ 2016-11-03 17:02 UTC (permalink / raw)
To: Vineet Gupta
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin
On Mon, Oct 31, 2016 at 03:48:09PM -0700, Vineet Gupta wrote:
> To allow for easy movement into drivers/clocksource
>
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> ---
> arch/arc/kernel/time.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arc/kernel/time.c b/arch/arc/kernel/time.c
> index a2db010cde18..2c51e3cafad0 100644
> --- a/arch/arc/kernel/time.c
> +++ b/arch/arc/kernel/time.c
> @@ -153,14 +153,11 @@ static cycle_t arc_read_rtc(struct clocksource *cs)
> cycle_t full;
> } stamp;
>
> -
> - __asm__ __volatile(
> - "1: \n"
> - " lr %0, [AUX_RTC_LOW] \n"
> - " lr %1, [AUX_RTC_HIGH] \n"
> - " lr %2, [AUX_RTC_CTRL] \n"
> - " bbit0.nt %2, 31, 1b \n"
> - : "=r" (stamp.low), "=r" (stamp.high), "=r" (status));
> + do {
> + stamp.low = read_aux_reg(AUX_RTC_LOW);
> + stamp.high = read_aux_reg(AUX_RTC_HIGH);
> + status = read_aux_reg(AUX_RTC_CTRL);
> + } while (!(status & 0x80000000UL));
Replace the literal 0x80000000UL by a macro.
What is the 'status' for ?
> return stamp.full;
> }
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 2/9] ARC: timer: rtc: implement read loop in "C" vs. inline asm
2016-11-03 17:02 ` Daniel Lezcano
@ 2016-11-03 17:45 ` Vineet Gupta
0 siblings, 0 replies; 52+ messages in thread
From: Vineet Gupta @ 2016-11-03 17:45 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Noam Camus, tglx, linux-snps-arc, Alexey.Brodkin, linux-kernel
On 11/03/2016 10:02 AM, Daniel Lezcano wrote:
> On Mon, Oct 31, 2016 at 03:48:09PM -0700, Vineet Gupta wrote:
>> To allow for easy movement into drivers/clocksource
>>
>> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
>> ---
>> arch/arc/kernel/time.c | 13 +++++--------
>> 1 file changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arc/kernel/time.c b/arch/arc/kernel/time.c
>> index a2db010cde18..2c51e3cafad0 100644
>> --- a/arch/arc/kernel/time.c
>> +++ b/arch/arc/kernel/time.c
>> @@ -153,14 +153,11 @@ static cycle_t arc_read_rtc(struct clocksource *cs)
>> cycle_t full;
>> } stamp;
>>
>> -
>> - __asm__ __volatile(
>> - "1: \n"
>> - " lr %0, [AUX_RTC_LOW] \n"
>> - " lr %1, [AUX_RTC_HIGH] \n"
>> - " lr %2, [AUX_RTC_CTRL] \n"
>> - " bbit0.nt %2, 31, 1b \n"
>> - : "=r" (stamp.low), "=r" (stamp.high), "=r" (status));
>> + do {
>> + stamp.low = read_aux_reg(AUX_RTC_LOW);
>> + stamp.high = read_aux_reg(AUX_RTC_HIGH);
>> + status = read_aux_reg(AUX_RTC_CTRL);
>> + } while (!(status & 0x80000000UL));
>
> Replace the literal 0x80000000UL by a macro.
OK !
> What is the 'status' for ?
Hardware keeps a internal state machine for atomic readout of low/high. So if an
interrupt is taken between reading low and high, or if high increments after low
is read, then the bit forces a loop to retry.
>
>> return stamp.full;
>> }
>> --
>> 2.7.4
>>
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH 3/9] ARC: timer: gfrc: boot print alongside other timers
2016-10-31 22:48 ` [PATCH 0/9] Move ARC timer code into drivers/clocksource/ Vineet Gupta
2016-10-31 22:48 ` [PATCH 1/9] ARC: timer: gfrc, rtc: Read BCR to detect whether hardware exists Vineet Gupta
2016-10-31 22:48 ` [PATCH 2/9] ARC: timer: rtc: implement read loop in "C" vs. inline asm Vineet Gupta
@ 2016-10-31 22:48 ` Vineet Gupta
2016-11-03 17:09 ` Daniel Lezcano
2016-10-31 22:48 ` [PATCH 4/9] ARC: time: move time_init() out of the driver Vineet Gupta
` (6 subsequent siblings)
9 siblings, 1 reply; 52+ messages in thread
From: Vineet Gupta @ 2016-10-31 22:48 UTC (permalink / raw)
To: Daniel Lezcano
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin,
Vineet Gupta
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
arch/arc/kernel/setup.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
index 0385df77a697..595d06900061 100644
--- a/arch/arc/kernel/setup.c
+++ b/arch/arc/kernel/setup.c
@@ -234,11 +234,11 @@ static char *arc_cpu_mumbojumbo(int cpu_id, char *buf, int len)
is_isa_arcompact() ? "ARCompact" : "ARCv2",
IS_AVAIL1(cpu->isa.be, "[Big-Endian]"));
- n += scnprintf(buf + n, len - n, "Timers\t\t: %s%s%s%s\nISA Extn\t: ",
+ n += scnprintf(buf + n, len - n, "Timers\t\t: %s%s%s%s%s%s\nISA Extn\t: ",
IS_AVAIL1(cpu->extn.timer0, "Timer0 "),
IS_AVAIL1(cpu->extn.timer1, "Timer1 "),
- IS_AVAIL2(cpu->extn.rtc, "Local-64-bit-Ctr ",
- CONFIG_ARC_HAS_RTC));
+ IS_AVAIL2(cpu->extn.rtc, "RTC [UP 64-bit] ", CONFIG_ARC_HAS_RTC),
+ IS_AVAIL2(cpu->extn.gfrc, "GFRC [SMP 64-bit] ", CONFIG_ARC_HAS_GFRC));
n += i = scnprintf(buf + n, len - n, "%s%s%s%s%s",
IS_AVAIL2(cpu->isa.atomic, "atomic ", CONFIG_ARC_HAS_LLSC),
--
2.7.4
^ permalink raw reply related [flat|nested] 52+ messages in thread
* Re: [PATCH 3/9] ARC: timer: gfrc: boot print alongside other timers
2016-10-31 22:48 ` [PATCH 3/9] ARC: timer: gfrc: boot print alongside other timers Vineet Gupta
@ 2016-11-03 17:09 ` Daniel Lezcano
2016-11-03 17:47 ` Vineet Gupta
0 siblings, 1 reply; 52+ messages in thread
From: Daniel Lezcano @ 2016-11-03 17:09 UTC (permalink / raw)
To: Vineet Gupta
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin
On Mon, Oct 31, 2016 at 03:48:10PM -0700, Vineet Gupta wrote:
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
Why not add a message in drivers/clocksource/clksrc-probe.c when a timer inits
successfully ? and then get rid of this code.
> ---
> arch/arc/kernel/setup.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
> index 0385df77a697..595d06900061 100644
> --- a/arch/arc/kernel/setup.c
> +++ b/arch/arc/kernel/setup.c
> @@ -234,11 +234,11 @@ static char *arc_cpu_mumbojumbo(int cpu_id, char *buf, int len)
> is_isa_arcompact() ? "ARCompact" : "ARCv2",
> IS_AVAIL1(cpu->isa.be, "[Big-Endian]"));
>
> - n += scnprintf(buf + n, len - n, "Timers\t\t: %s%s%s%s\nISA Extn\t: ",
> + n += scnprintf(buf + n, len - n, "Timers\t\t: %s%s%s%s%s%s\nISA Extn\t: ",
> IS_AVAIL1(cpu->extn.timer0, "Timer0 "),
> IS_AVAIL1(cpu->extn.timer1, "Timer1 "),
> - IS_AVAIL2(cpu->extn.rtc, "Local-64-bit-Ctr ",
> - CONFIG_ARC_HAS_RTC));
> + IS_AVAIL2(cpu->extn.rtc, "RTC [UP 64-bit] ", CONFIG_ARC_HAS_RTC),
> + IS_AVAIL2(cpu->extn.gfrc, "GFRC [SMP 64-bit] ", CONFIG_ARC_HAS_GFRC));
>
> n += i = scnprintf(buf + n, len - n, "%s%s%s%s%s",
> IS_AVAIL2(cpu->isa.atomic, "atomic ", CONFIG_ARC_HAS_LLSC),
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 3/9] ARC: timer: gfrc: boot print alongside other timers
2016-11-03 17:09 ` Daniel Lezcano
@ 2016-11-03 17:47 ` Vineet Gupta
2016-11-03 17:51 ` Daniel Lezcano
0 siblings, 1 reply; 52+ messages in thread
From: Vineet Gupta @ 2016-11-03 17:47 UTC (permalink / raw)
To: Daniel Lezcano
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin
On 11/03/2016 10:09 AM, Daniel Lezcano wrote:
> On Mon, Oct 31, 2016 at 03:48:10PM -0700, Vineet Gupta wrote:
>> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
>
> Why not add a message in drivers/clocksource/clksrc-probe.c when a timer inits
> successfully ? and then get rid of this code.
At boot I have bunch of cod which prints all the major hardware blocks and thus
would like to print it. We also print if a feature is present but not supported
etc. So I like to keep all of that here.
Also same code is used for /proc/cpuinfo.
>
>> ---
>> arch/arc/kernel/setup.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
>> index 0385df77a697..595d06900061 100644
>> --- a/arch/arc/kernel/setup.c
>> +++ b/arch/arc/kernel/setup.c
>> @@ -234,11 +234,11 @@ static char *arc_cpu_mumbojumbo(int cpu_id, char *buf, int len)
>> is_isa_arcompact() ? "ARCompact" : "ARCv2",
>> IS_AVAIL1(cpu->isa.be, "[Big-Endian]"));
>>
>> - n += scnprintf(buf + n, len - n, "Timers\t\t: %s%s%s%s\nISA Extn\t: ",
>> + n += scnprintf(buf + n, len - n, "Timers\t\t: %s%s%s%s%s%s\nISA Extn\t: ",
>> IS_AVAIL1(cpu->extn.timer0, "Timer0 "),
>> IS_AVAIL1(cpu->extn.timer1, "Timer1 "),
>> - IS_AVAIL2(cpu->extn.rtc, "Local-64-bit-Ctr ",
>> - CONFIG_ARC_HAS_RTC));
>> + IS_AVAIL2(cpu->extn.rtc, "RTC [UP 64-bit] ", CONFIG_ARC_HAS_RTC),
>> + IS_AVAIL2(cpu->extn.gfrc, "GFRC [SMP 64-bit] ", CONFIG_ARC_HAS_GFRC));
>>
>> n += i = scnprintf(buf + n, len - n, "%s%s%s%s%s",
>> IS_AVAIL2(cpu->isa.atomic, "atomic ", CONFIG_ARC_HAS_LLSC),
>> --
>> 2.7.4
>>
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 3/9] ARC: timer: gfrc: boot print alongside other timers
2016-11-03 17:47 ` Vineet Gupta
@ 2016-11-03 17:51 ` Daniel Lezcano
0 siblings, 0 replies; 52+ messages in thread
From: Daniel Lezcano @ 2016-11-03 17:51 UTC (permalink / raw)
To: Vineet Gupta
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin
On Thu, Nov 03, 2016 at 10:47:19AM -0700, Vineet Gupta wrote:
> On 11/03/2016 10:09 AM, Daniel Lezcano wrote:
> > On Mon, Oct 31, 2016 at 03:48:10PM -0700, Vineet Gupta wrote:
> >> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> >
> > Why not add a message in drivers/clocksource/clksrc-probe.c when a timer inits
> > successfully ? and then get rid of this code.
>
> At boot I have bunch of cod which prints all the major hardware blocks and thus
> would like to print it. We also print if a feature is present but not supported
> etc. So I like to keep all of that here.
>
> Also same code is used for /proc/cpuinfo.
Ok, I see. Thanks for the clarification.
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH 4/9] ARC: time: move time_init() out of the driver
2016-10-31 22:48 ` [PATCH 0/9] Move ARC timer code into drivers/clocksource/ Vineet Gupta
` (2 preceding siblings ...)
2016-10-31 22:48 ` [PATCH 3/9] ARC: timer: gfrc: boot print alongside other timers Vineet Gupta
@ 2016-10-31 22:48 ` Vineet Gupta
2016-11-03 17:15 ` Daniel Lezcano
2016-10-31 22:48 ` [PATCH 5/9] ARC: breakout aux handling into a seperate header Vineet Gupta
` (5 subsequent siblings)
9 siblings, 1 reply; 52+ messages in thread
From: Vineet Gupta @ 2016-10-31 22:48 UTC (permalink / raw)
To: Daniel Lezcano
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin,
Vineet Gupta
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
arch/arc/kernel/setup.c | 11 +++++++++++
arch/arc/kernel/time.c | 9 ---------
2 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
index 595d06900061..5865bd34a7fa 100644
--- a/arch/arc/kernel/setup.c
+++ b/arch/arc/kernel/setup.c
@@ -10,6 +10,8 @@
#include <linux/fs.h>
#include <linux/delay.h>
#include <linux/root_dev.h>
+#include <linux/clk-provider.h>
+#include <linux/clocksource.h>
#include <linux/console.h>
#include <linux/module.h>
#include <linux/cpu.h>
@@ -449,6 +451,15 @@ void __init setup_arch(char **cmdline_p)
arc_unwind_init();
}
+/*
+ * Called from start_kernel() - boot CPU only
+ */
+void __init time_init(void)
+{
+ of_clk_init(NULL);
+ clocksource_probe();
+}
+
static int __init customize_machine(void)
{
if (machine_desc->init_machine)
diff --git a/arch/arc/kernel/time.c b/arch/arc/kernel/time.c
index 2c51e3cafad0..00ece39a8ae7 100644
--- a/arch/arc/kernel/time.c
+++ b/arch/arc/kernel/time.c
@@ -367,12 +367,3 @@ static int __init arc_of_timer_init(struct device_node *np)
return ret;
}
CLOCKSOURCE_OF_DECLARE(arc_clkevt, "snps,arc-timer", arc_of_timer_init);
-
-/*
- * Called from start_kernel() - boot CPU only
- */
-void __init time_init(void)
-{
- of_clk_init(NULL);
- clocksource_probe();
-}
--
2.7.4
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH 5/9] ARC: breakout aux handling into a seperate header
2016-10-31 22:48 ` [PATCH 0/9] Move ARC timer code into drivers/clocksource/ Vineet Gupta
` (3 preceding siblings ...)
2016-10-31 22:48 ` [PATCH 4/9] ARC: time: move time_init() out of the driver Vineet Gupta
@ 2016-10-31 22:48 ` Vineet Gupta
2016-11-01 7:49 ` Noam Camus
2016-10-31 22:48 ` [PATCH 6/9] ARC: move mcip.h into include/soc and adjust the includes Vineet Gupta
` (4 subsequent siblings)
9 siblings, 1 reply; 52+ messages in thread
From: Vineet Gupta @ 2016-10-31 22:48 UTC (permalink / raw)
To: Daniel Lezcano
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin,
Vineet Gupta
ARC timers use aux registers for programming and this paves way for
moving ARC timer drivers into drivers/clocksource
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
arch/arc/include/asm/arcregs.h | 85 +-----------------------------------------
arch/arc/include/asm/mcip.h | 2 +-
include/soc/arc/aux.h | 52 ++++++++++++++++++++++++++
include/soc/nps/common.h | 4 +-
4 files changed, 56 insertions(+), 87 deletions(-)
create mode 100644 include/soc/arc/aux.h
diff --git a/arch/arc/include/asm/arcregs.h b/arch/arc/include/asm/arcregs.h
index 7f3f9f63708c..ccf5dc96713b 100644
--- a/arch/arc/include/asm/arcregs.h
+++ b/arch/arc/include/asm/arcregs.h
@@ -110,90 +110,7 @@
#ifndef __ASSEMBLY__
-/*
- ******************************************************************
- * Inline ASM macros to read/write AUX Regs
- * Essentially invocation of lr/sr insns from "C"
- */
-
-#if 1
-
-#define read_aux_reg(reg) __builtin_arc_lr(reg)
-
-/* gcc builtin sr needs reg param to be long immediate */
-#define write_aux_reg(reg_immed, val) \
- __builtin_arc_sr((unsigned int)(val), reg_immed)
-
-#else
-
-#define read_aux_reg(reg) \
-({ \
- unsigned int __ret; \
- __asm__ __volatile__( \
- " lr %0, [%1]" \
- : "=r"(__ret) \
- : "i"(reg)); \
- __ret; \
-})
-
-/*
- * Aux Reg address is specified as long immediate by caller
- * e.g.
- * write_aux_reg(0x69, some_val);
- * This generates tightest code.
- */
-#define write_aux_reg(reg_imm, val) \
-({ \
- __asm__ __volatile__( \
- " sr %0, [%1] \n" \
- : \
- : "ir"(val), "i"(reg_imm)); \
-})
-
-/*
- * Aux Reg address is specified in a variable
- * * e.g.
- * reg_num = 0x69
- * write_aux_reg2(reg_num, some_val);
- * This has to generate glue code to load the reg num from
- * memory to a reg hence not recommended.
- */
-#define write_aux_reg2(reg_in_var, val) \
-({ \
- unsigned int tmp; \
- \
- __asm__ __volatile__( \
- " ld %0, [%2] \n\t" \
- " sr %1, [%0] \n\t" \
- : "=&r"(tmp) \
- : "r"(val), "memory"(®_in_var)); \
-})
-
-#endif
-
-#define READ_BCR(reg, into) \
-{ \
- unsigned int tmp; \
- tmp = read_aux_reg(reg); \
- if (sizeof(tmp) == sizeof(into)) { \
- into = *((typeof(into) *)&tmp); \
- } else { \
- extern void bogus_undefined(void); \
- bogus_undefined(); \
- } \
-}
-
-#define WRITE_AUX(reg, into) \
-{ \
- unsigned int tmp; \
- if (sizeof(tmp) == sizeof(into)) { \
- tmp = (*(unsigned int *)&(into)); \
- write_aux_reg(reg, tmp); \
- } else { \
- extern void bogus_undefined(void); \
- bogus_undefined(); \
- } \
-}
+#include <soc/arc/aux.h>
/* Helpers */
#define TO_KB(bytes) ((bytes) >> 10)
diff --git a/arch/arc/include/asm/mcip.h b/arch/arc/include/asm/mcip.h
index c8fbe4114bad..fc28d0944801 100644
--- a/arch/arc/include/asm/mcip.h
+++ b/arch/arc/include/asm/mcip.h
@@ -13,7 +13,7 @@
#ifdef CONFIG_ISA_ARCV2
-#include <asm/arcregs.h>
+#include <soc/arc/aux.h>
#define ARC_REG_MCIP_BCR 0x0d0
#define ARC_REG_MCIP_CMD 0x600
diff --git a/include/soc/arc/aux.h b/include/soc/arc/aux.h
new file mode 100644
index 000000000000..8c41c096a51b
--- /dev/null
+++ b/include/soc/arc/aux.h
@@ -0,0 +1,52 @@
+/*
+ * Copyright (C) 2016-2017 Synopsys, Inc. (www.synopsys.com)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#ifndef __SOC_ARC_AUX_H__
+#define __SOC_ARC_AUX_H__
+
+#ifdef CONFIG_ARC
+
+#define read_aux_reg(r) __builtin_arc_lr(r)
+
+/* gcc builtin sr needs reg param to be long immediate */
+#define write_aux_reg(r, v) __builtin_arc_sr((unsigned int)(v), r)
+
+#else
+
+#define read_aux_reg(r) 0
+#define write_aux_reg(r, v)
+
+#endif
+
+#define READ_BCR(reg, into) \
+{ \
+ unsigned int tmp; \
+ tmp = read_aux_reg(reg); \
+ if (sizeof(tmp) == sizeof(into)) { \
+ into = *((typeof(into) *)&tmp); \
+ } else { \
+ extern void bogus_undefined(void); \
+ bogus_undefined(); \
+ } \
+}
+
+#define WRITE_AUX(reg, into) \
+{ \
+ unsigned int tmp; \
+ if (sizeof(tmp) == sizeof(into)) { \
+ tmp = (*(unsigned int *)&(into)); \
+ write_aux_reg(reg, tmp); \
+ } else { \
+ extern void bogus_undefined(void); \
+ bogus_undefined(); \
+ } \
+}
+
+
+#endif
diff --git a/include/soc/nps/common.h b/include/soc/nps/common.h
index 9b1d43d671a3..b7ba05b3993f 100644
--- a/include/soc/nps/common.h
+++ b/include/soc/nps/common.h
@@ -47,6 +47,8 @@
#ifndef __ASSEMBLY__
+#include <soc/arc/aux.h>
+
/* In order to increase compilation test coverage */
#ifdef CONFIG_ARC
static inline void nps_ack_gic(void)
@@ -59,8 +61,6 @@ static inline void nps_ack_gic(void)
}
#else
static inline void nps_ack_gic(void) { }
-#define write_aux_reg(r, v)
-#define read_aux_reg(r) 0
#endif
/* CPU global ID */
--
2.7.4
^ permalink raw reply related [flat|nested] 52+ messages in thread
* RE: [PATCH 5/9] ARC: breakout aux handling into a seperate header
2016-10-31 22:48 ` [PATCH 5/9] ARC: breakout aux handling into a seperate header Vineet Gupta
@ 2016-11-01 7:49 ` Noam Camus
0 siblings, 0 replies; 52+ messages in thread
From: Noam Camus @ 2016-11-01 7:49 UTC (permalink / raw)
To: Vineet Gupta, Daniel Lezcano
Cc: tglx, linux-kernel, linux-snps-arc, Alexey.Brodkin
>From: Vineet Gupta [mailto:Vineet.Gupta1@synopsys.com]
>Sent: Tuesday, November 1, 2016 12:48 AM
>ARC timers use aux registers for programming and this paves way for moving ARC timer drivers into drivers/clocksource
Maybe in this patch or just another one could you move from timer.c to the new soc header all timer related Aux registers definitions?
This could be used by timer-nps driver.
i.e.:
/* Timer related Aux registers */
#define ARC_REG_TIMER0_LIMIT 0x23 /* timer 0 limit */
#define ARC_REG_TIMER0_CTRL 0x22 /* timer 0 control */
#define ARC_REG_TIMER0_CNT 0x21 /* timer 0 count */
#define ARC_REG_TIMER1_LIMIT 0x102 /* timer 1 limit */
#define ARC_REG_TIMER1_CTRL 0x101 /* timer 1 control */
#define ARC_REG_TIMER1_CNT 0x100 /* timer 1 count */
#define TIMER_CTRL_IE (1 << 0) /* Interrupt when Count reaches limit */
#define TIMER_CTRL_NH (1 << 1) /* Count only when CPU NOT halted */
#define ARC_TIMER_MAX 0xFFFFFFFF
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH 6/9] ARC: move mcip.h into include/soc and adjust the includes
2016-10-31 22:48 ` [PATCH 0/9] Move ARC timer code into drivers/clocksource/ Vineet Gupta
` (4 preceding siblings ...)
2016-10-31 22:48 ` [PATCH 5/9] ARC: breakout aux handling into a seperate header Vineet Gupta
@ 2016-10-31 22:48 ` Vineet Gupta
2016-11-03 17:20 ` Daniel Lezcano
2016-10-31 22:48 ` [PATCH 7/9] ARC: breakout timer stuff into a seperate header Vineet Gupta
` (3 subsequent siblings)
9 siblings, 1 reply; 52+ messages in thread
From: Vineet Gupta @ 2016-10-31 22:48 UTC (permalink / raw)
To: Daniel Lezcano
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin,
Vineet Gupta
Also remove the depedency on ARCv2, to increase compile coverage for
!ARCV2 builds
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
arch/arc/kernel/mcip.c | 2 +-
arch/arc/kernel/time.c | 2 +-
arch/arc/plat-axs10x/axs10x.c | 2 +-
{arch/arc/include/asm => include/soc/arc}/mcip.h | 8 ++------
4 files changed, 5 insertions(+), 9 deletions(-)
rename {arch/arc/include/asm => include/soc/arc}/mcip.h (96%)
diff --git a/arch/arc/kernel/mcip.c b/arch/arc/kernel/mcip.c
index c424d5abc318..0651e0a2e8b1 100644
--- a/arch/arc/kernel/mcip.c
+++ b/arch/arc/kernel/mcip.c
@@ -11,8 +11,8 @@
#include <linux/smp.h>
#include <linux/irq.h>
#include <linux/spinlock.h>
+#include <soc/arc/mcip.h>
#include <asm/irqflags-arcv2.h>
-#include <asm/mcip.h>
#include <asm/setup.h>
static DEFINE_RAW_SPINLOCK(mcip_lock);
diff --git a/arch/arc/kernel/time.c b/arch/arc/kernel/time.c
index 00ece39a8ae7..f1ebe45bfcdf 100644
--- a/arch/arc/kernel/time.c
+++ b/arch/arc/kernel/time.c
@@ -40,7 +40,7 @@
#include <asm/irq.h>
#include <asm/arcregs.h>
-#include <asm/mcip.h>
+#include <soc/arc/mcip.h>
/* Timer related Aux registers */
#define ARC_REG_TIMER0_LIMIT 0x23 /* timer 0 limit */
diff --git a/arch/arc/plat-axs10x/axs10x.c b/arch/arc/plat-axs10x/axs10x.c
index 86548701023c..38ff349d7f2a 100644
--- a/arch/arc/plat-axs10x/axs10x.c
+++ b/arch/arc/plat-axs10x/axs10x.c
@@ -21,7 +21,7 @@
#include <asm/asm-offsets.h>
#include <asm/io.h>
#include <asm/mach_desc.h>
-#include <asm/mcip.h>
+#include <soc/arc/mcip.h>
#define AXS_MB_CGU 0xE0010000
#define AXS_MB_CREG 0xE0011000
diff --git a/arch/arc/include/asm/mcip.h b/include/soc/arc/mcip.h
similarity index 96%
rename from arch/arc/include/asm/mcip.h
rename to include/soc/arc/mcip.h
index fc28d0944801..6902c2a8bd23 100644
--- a/arch/arc/include/asm/mcip.h
+++ b/include/soc/arc/mcip.h
@@ -8,10 +8,8 @@
* published by the Free Software Foundation.
*/
-#ifndef __ASM_MCIP_H
-#define __ASM_MCIP_H
-
-#ifdef CONFIG_ISA_ARCV2
+#ifndef __SOC_ARC_MCIP_H
+#define __SOC_ARC_MCIP_H
#include <soc/arc/aux.h>
@@ -103,5 +101,3 @@ static inline void __mcip_cmd_data(unsigned int cmd, unsigned int param,
}
#endif
-
-#endif
--
2.7.4
^ permalink raw reply related [flat|nested] 52+ messages in thread
* Re: [PATCH 6/9] ARC: move mcip.h into include/soc and adjust the includes
2016-10-31 22:48 ` [PATCH 6/9] ARC: move mcip.h into include/soc and adjust the includes Vineet Gupta
@ 2016-11-03 17:20 ` Daniel Lezcano
0 siblings, 0 replies; 52+ messages in thread
From: Daniel Lezcano @ 2016-11-03 17:20 UTC (permalink / raw)
To: Vineet Gupta
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin
On Mon, Oct 31, 2016 at 03:48:13PM -0700, Vineet Gupta wrote:
> Also remove the depedency on ARCv2, to increase compile coverage for
> !ARCV2 builds
s/depedency/dependency/
Acked-by: Daniel Lezcano <daniel.lezcnao@linaro.org>
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> ---
> arch/arc/kernel/mcip.c | 2 +-
> arch/arc/kernel/time.c | 2 +-
> arch/arc/plat-axs10x/axs10x.c | 2 +-
> {arch/arc/include/asm => include/soc/arc}/mcip.h | 8 ++------
> 4 files changed, 5 insertions(+), 9 deletions(-)
> rename {arch/arc/include/asm => include/soc/arc}/mcip.h (96%)
>
> diff --git a/arch/arc/kernel/mcip.c b/arch/arc/kernel/mcip.c
> index c424d5abc318..0651e0a2e8b1 100644
> --- a/arch/arc/kernel/mcip.c
> +++ b/arch/arc/kernel/mcip.c
> @@ -11,8 +11,8 @@
> #include <linux/smp.h>
> #include <linux/irq.h>
> #include <linux/spinlock.h>
> +#include <soc/arc/mcip.h>
> #include <asm/irqflags-arcv2.h>
> -#include <asm/mcip.h>
> #include <asm/setup.h>
>
> static DEFINE_RAW_SPINLOCK(mcip_lock);
> diff --git a/arch/arc/kernel/time.c b/arch/arc/kernel/time.c
> index 00ece39a8ae7..f1ebe45bfcdf 100644
> --- a/arch/arc/kernel/time.c
> +++ b/arch/arc/kernel/time.c
> @@ -40,7 +40,7 @@
> #include <asm/irq.h>
> #include <asm/arcregs.h>
>
> -#include <asm/mcip.h>
> +#include <soc/arc/mcip.h>
>
> /* Timer related Aux registers */
> #define ARC_REG_TIMER0_LIMIT 0x23 /* timer 0 limit */
> diff --git a/arch/arc/plat-axs10x/axs10x.c b/arch/arc/plat-axs10x/axs10x.c
> index 86548701023c..38ff349d7f2a 100644
> --- a/arch/arc/plat-axs10x/axs10x.c
> +++ b/arch/arc/plat-axs10x/axs10x.c
> @@ -21,7 +21,7 @@
> #include <asm/asm-offsets.h>
> #include <asm/io.h>
> #include <asm/mach_desc.h>
> -#include <asm/mcip.h>
> +#include <soc/arc/mcip.h>
>
> #define AXS_MB_CGU 0xE0010000
> #define AXS_MB_CREG 0xE0011000
> diff --git a/arch/arc/include/asm/mcip.h b/include/soc/arc/mcip.h
> similarity index 96%
> rename from arch/arc/include/asm/mcip.h
> rename to include/soc/arc/mcip.h
> index fc28d0944801..6902c2a8bd23 100644
> --- a/arch/arc/include/asm/mcip.h
> +++ b/include/soc/arc/mcip.h
> @@ -8,10 +8,8 @@
> * published by the Free Software Foundation.
> */
>
> -#ifndef __ASM_MCIP_H
> -#define __ASM_MCIP_H
> -
> -#ifdef CONFIG_ISA_ARCV2
> +#ifndef __SOC_ARC_MCIP_H
> +#define __SOC_ARC_MCIP_H
>
> #include <soc/arc/aux.h>
>
> @@ -103,5 +101,3 @@ static inline void __mcip_cmd_data(unsigned int cmd, unsigned int param,
> }
>
> #endif
> -
> -#endif
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH 7/9] ARC: breakout timer stuff into a seperate header
2016-10-31 22:48 ` [PATCH 0/9] Move ARC timer code into drivers/clocksource/ Vineet Gupta
` (5 preceding siblings ...)
2016-10-31 22:48 ` [PATCH 6/9] ARC: move mcip.h into include/soc and adjust the includes Vineet Gupta
@ 2016-10-31 22:48 ` Vineet Gupta
2016-11-03 17:25 ` Daniel Lezcano
2016-10-31 22:48 ` [PATCH 8/9] ARC: timer: rename config options Vineet Gupta
` (2 subsequent siblings)
9 siblings, 1 reply; 52+ messages in thread
From: Vineet Gupta @ 2016-10-31 22:48 UTC (permalink / raw)
To: Daniel Lezcano
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin,
Vineet Gupta
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
arch/arc/include/asm/arcregs.h | 9 +--------
arch/arc/kernel/time.c | 2 +-
include/soc/arc/timers.h | 24 ++++++++++++++++++++++++
3 files changed, 26 insertions(+), 9 deletions(-)
create mode 100644 include/soc/arc/timers.h
diff --git a/arch/arc/include/asm/arcregs.h b/arch/arc/include/asm/arcregs.h
index ccf5dc96713b..a17aa4558832 100644
--- a/arch/arc/include/asm/arcregs.h
+++ b/arch/arc/include/asm/arcregs.h
@@ -20,7 +20,6 @@
#define ARC_REG_FP_V2_BCR 0xc8 /* ARCv2 FPU */
#define ARC_REG_SLC_BCR 0xce
#define ARC_REG_DCCM_BUILD 0x74 /* DCCM size (common) */
-#define ARC_REG_TIMERS_BCR 0x75
#define ARC_REG_AP_BCR 0x76
#define ARC_REG_ICCM_BUILD 0x78 /* ICCM size (common) */
#define ARC_REG_XY_MEM_BCR 0x79
@@ -206,13 +205,7 @@ struct bcr_fp_arcv2 {
#endif
};
-struct bcr_timer {
-#ifdef CONFIG_CPU_BIG_ENDIAN
- unsigned int pad2:15, rtsc:1, pad1:5, rtc:1, t1:1, t0:1, ver:8;
-#else
- unsigned int ver:8, t0:1, t1:1, rtc:1, pad1:5, rtsc:1, pad2:15;
-#endif
-};
+#include <soc/arc/timers.h>
struct bcr_bpu_arcompact {
#ifdef CONFIG_CPU_BIG_ENDIAN
diff --git a/arch/arc/kernel/time.c b/arch/arc/kernel/time.c
index f1ebe45bfcdf..dad3a9933c4c 100644
--- a/arch/arc/kernel/time.c
+++ b/arch/arc/kernel/time.c
@@ -38,8 +38,8 @@
#include <linux/of.h>
#include <linux/of_irq.h>
#include <asm/irq.h>
-#include <asm/arcregs.h>
+#include <soc/arc/timers.h>
#include <soc/arc/mcip.h>
/* Timer related Aux registers */
diff --git a/include/soc/arc/timers.h b/include/soc/arc/timers.h
new file mode 100644
index 000000000000..79484fafabfa
--- /dev/null
+++ b/include/soc/arc/timers.h
@@ -0,0 +1,24 @@
+/*
+ * Copyright (C) 2016-17 Synopsys, Inc. (www.synopsys.com)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __SOC_ARC_TIMERS_H
+#define __SOC_ARC_TIMERS_H
+
+#include <soc/arc/aux.h>
+
+#define ARC_REG_TIMERS_BCR 0x75
+
+struct bcr_timer {
+#ifdef CONFIG_CPU_BIG_ENDIAN
+ unsigned int pad2:15, rtsc:1, pad1:5, rtc:1, t1:1, t0:1, ver:8;
+#else
+ unsigned int ver:8, t0:1, t1:1, rtc:1, pad1:5, rtsc:1, pad2:15;
+#endif
+};
+
+#endif
--
2.7.4
^ permalink raw reply related [flat|nested] 52+ messages in thread
* Re: [PATCH 7/9] ARC: breakout timer stuff into a seperate header
2016-10-31 22:48 ` [PATCH 7/9] ARC: breakout timer stuff into a seperate header Vineet Gupta
@ 2016-11-03 17:25 ` Daniel Lezcano
0 siblings, 0 replies; 52+ messages in thread
From: Daniel Lezcano @ 2016-11-03 17:25 UTC (permalink / raw)
To: Vineet Gupta
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin
s/seperate/separate/
On Mon, Oct 31, 2016 at 03:48:14PM -0700, Vineet Gupta wrote:
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> ---
> arch/arc/include/asm/arcregs.h | 9 +--------
> arch/arc/kernel/time.c | 2 +-
> include/soc/arc/timers.h | 24 ++++++++++++++++++++++++
> 3 files changed, 26 insertions(+), 9 deletions(-)
> create mode 100644 include/soc/arc/timers.h
>
> diff --git a/arch/arc/include/asm/arcregs.h b/arch/arc/include/asm/arcregs.h
> index ccf5dc96713b..a17aa4558832 100644
> --- a/arch/arc/include/asm/arcregs.h
> +++ b/arch/arc/include/asm/arcregs.h
> @@ -20,7 +20,6 @@
> #define ARC_REG_FP_V2_BCR 0xc8 /* ARCv2 FPU */
> #define ARC_REG_SLC_BCR 0xce
> #define ARC_REG_DCCM_BUILD 0x74 /* DCCM size (common) */
> -#define ARC_REG_TIMERS_BCR 0x75
> #define ARC_REG_AP_BCR 0x76
> #define ARC_REG_ICCM_BUILD 0x78 /* ICCM size (common) */
> #define ARC_REG_XY_MEM_BCR 0x79
> @@ -206,13 +205,7 @@ struct bcr_fp_arcv2 {
> #endif
> };
>
> -struct bcr_timer {
> -#ifdef CONFIG_CPU_BIG_ENDIAN
> - unsigned int pad2:15, rtsc:1, pad1:5, rtc:1, t1:1, t0:1, ver:8;
> -#else
> - unsigned int ver:8, t0:1, t1:1, rtc:1, pad1:5, rtsc:1, pad2:15;
> -#endif
> -};
> +#include <soc/arc/timers.h>
>
> struct bcr_bpu_arcompact {
> #ifdef CONFIG_CPU_BIG_ENDIAN
> diff --git a/arch/arc/kernel/time.c b/arch/arc/kernel/time.c
> index f1ebe45bfcdf..dad3a9933c4c 100644
> --- a/arch/arc/kernel/time.c
> +++ b/arch/arc/kernel/time.c
> @@ -38,8 +38,8 @@
> #include <linux/of.h>
> #include <linux/of_irq.h>
> #include <asm/irq.h>
> -#include <asm/arcregs.h>
>
> +#include <soc/arc/timers.h>
> #include <soc/arc/mcip.h>
>
> /* Timer related Aux registers */
> diff --git a/include/soc/arc/timers.h b/include/soc/arc/timers.h
> new file mode 100644
> index 000000000000..79484fafabfa
> --- /dev/null
> +++ b/include/soc/arc/timers.h
> @@ -0,0 +1,24 @@
> +/*
> + * Copyright (C) 2016-17 Synopsys, Inc. (www.synopsys.com)
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __SOC_ARC_TIMERS_H
> +#define __SOC_ARC_TIMERS_H
> +
> +#include <soc/arc/aux.h>
> +
> +#define ARC_REG_TIMERS_BCR 0x75
> +
> +struct bcr_timer {
> +#ifdef CONFIG_CPU_BIG_ENDIAN
> + unsigned int pad2:15, rtsc:1, pad1:5, rtc:1, t1:1, t0:1, ver:8;
> +#else
> + unsigned int ver:8, t0:1, t1:1, rtc:1, pad1:5, rtsc:1, pad2:15;
> +#endif
> +};
> +
> +#endif
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH 8/9] ARC: timer: rename config options
2016-10-31 22:48 ` [PATCH 0/9] Move ARC timer code into drivers/clocksource/ Vineet Gupta
` (6 preceding siblings ...)
2016-10-31 22:48 ` [PATCH 7/9] ARC: breakout timer stuff into a seperate header Vineet Gupta
@ 2016-10-31 22:48 ` Vineet Gupta
2016-10-31 22:48 ` [PATCH 9/9] clocksource: import ARC timer driver Vineet Gupta
2016-11-03 17:28 ` [PATCH 0/9] Move ARC timer code into drivers/clocksource/ Daniel Lezcano
9 siblings, 0 replies; 52+ messages in thread
From: Vineet Gupta @ 2016-10-31 22:48 UTC (permalink / raw)
To: Daniel Lezcano
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin,
Vineet Gupta
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
arch/arc/Kconfig | 4 ++--
arch/arc/configs/nsimosci_hs_smp_defconfig | 2 +-
arch/arc/configs/vdk_hs38_smp_defconfig | 2 +-
arch/arc/kernel/setup.c | 4 ++--
arch/arc/kernel/time.c | 4 ++--
5 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
index bd204bfa29ed..b4499f97035a 100644
--- a/arch/arc/Kconfig
+++ b/arch/arc/Kconfig
@@ -410,12 +410,12 @@ config ARC_HAS_DIV_REM
bool "Insn: div, divu, rem, remu"
default y
-config ARC_HAS_RTC
+config ARC_TIMER_RTC
bool "Local 64-bit r/o cycle counter"
default n
depends on !SMP
-config ARC_HAS_GFRC
+config ARC_TIMER_GFRC
bool "SMP synchronized 64-bit cycle counter"
default y
depends on SMP
diff --git a/arch/arc/configs/nsimosci_hs_smp_defconfig b/arch/arc/configs/nsimosci_hs_smp_defconfig
index 6da71ba253a9..9782fe512244 100644
--- a/arch/arc/configs/nsimosci_hs_smp_defconfig
+++ b/arch/arc/configs/nsimosci_hs_smp_defconfig
@@ -21,7 +21,7 @@ CONFIG_MODULES=y
CONFIG_ARC_PLAT_SIM=y
CONFIG_ISA_ARCV2=y
CONFIG_SMP=y
-# CONFIG_ARC_HAS_GFRC is not set
+# CONFIG_ARC_TIMER_GFRC is not set
CONFIG_ARC_BUILTIN_DTB_NAME="nsimosci_hs_idu"
CONFIG_PREEMPT=y
# CONFIG_COMPACTION is not set
diff --git a/arch/arc/configs/vdk_hs38_smp_defconfig b/arch/arc/configs/vdk_hs38_smp_defconfig
index 969b206d6c67..a927389d02d8 100644
--- a/arch/arc/configs/vdk_hs38_smp_defconfig
+++ b/arch/arc/configs/vdk_hs38_smp_defconfig
@@ -15,7 +15,7 @@ CONFIG_ARC_PLAT_AXS10X=y
CONFIG_AXS103=y
CONFIG_ISA_ARCV2=y
CONFIG_SMP=y
-# CONFIG_ARC_HAS_GFRC is not set
+# CONFIG_ARC_TIMER_GFRC is not set
CONFIG_ARC_UBOOT_SUPPORT=y
CONFIG_ARC_BUILTIN_DTB_NAME="vdk_hs38_smp"
CONFIG_PREEMPT=y
diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
index 5865bd34a7fa..4855335b956b 100644
--- a/arch/arc/kernel/setup.c
+++ b/arch/arc/kernel/setup.c
@@ -239,8 +239,8 @@ static char *arc_cpu_mumbojumbo(int cpu_id, char *buf, int len)
n += scnprintf(buf + n, len - n, "Timers\t\t: %s%s%s%s%s%s\nISA Extn\t: ",
IS_AVAIL1(cpu->extn.timer0, "Timer0 "),
IS_AVAIL1(cpu->extn.timer1, "Timer1 "),
- IS_AVAIL2(cpu->extn.rtc, "RTC [UP 64-bit] ", CONFIG_ARC_HAS_RTC),
- IS_AVAIL2(cpu->extn.gfrc, "GFRC [SMP 64-bit] ", CONFIG_ARC_HAS_GFRC));
+ IS_AVAIL2(cpu->extn.rtc, "RTC [UP 64-bit] ", CONFIG_ARC_TIMER_RTC),
+ IS_AVAIL2(cpu->extn.gfrc, "GFRC [SMP 64-bit] ", CONFIG_ARC_TIMER_GFRC));
n += i = scnprintf(buf + n, len - n, "%s%s%s%s%s",
IS_AVAIL2(cpu->isa.atomic, "atomic ", CONFIG_ARC_HAS_LLSC),
diff --git a/arch/arc/kernel/time.c b/arch/arc/kernel/time.c
index dad3a9933c4c..676b14b7a9be 100644
--- a/arch/arc/kernel/time.c
+++ b/arch/arc/kernel/time.c
@@ -81,7 +81,7 @@ static int noinline arc_get_timer_clk(struct device_node *node)
/********** Clock Source Device *********/
-#ifdef CONFIG_ARC_HAS_GFRC
+#ifdef CONFIG_ARC_TIMER_GFRC
static cycle_t arc_read_gfrc(struct clocksource *cs)
{
@@ -135,7 +135,7 @@ CLOCKSOURCE_OF_DECLARE(arc_gfrc, "snps,archs-timer-gfrc", arc_cs_setup_gfrc);
#endif
-#ifdef CONFIG_ARC_HAS_RTC
+#ifdef CONFIG_ARC_TIMER_RTC
#define AUX_RTC_CTRL 0x103
#define AUX_RTC_LOW 0x104
--
2.7.4
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH 9/9] clocksource: import ARC timer driver
2016-10-31 22:48 ` [PATCH 0/9] Move ARC timer code into drivers/clocksource/ Vineet Gupta
` (7 preceding siblings ...)
2016-10-31 22:48 ` [PATCH 8/9] ARC: timer: rename config options Vineet Gupta
@ 2016-10-31 22:48 ` Vineet Gupta
2016-11-01 0:01 ` kbuild test robot
2016-11-01 20:42 ` Daniel Lezcano
2016-11-03 17:28 ` [PATCH 0/9] Move ARC timer code into drivers/clocksource/ Daniel Lezcano
9 siblings, 2 replies; 52+ messages in thread
From: Vineet Gupta @ 2016-10-31 22:48 UTC (permalink / raw)
To: Daniel Lezcano
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin,
Vineet Gupta
Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
MAINTAINERS | 1 +
arch/arc/Kconfig | 12 +--------
arch/arc/kernel/Makefile | 2 +-
drivers/clocksource/Kconfig | 24 ++++++++++++++++++
drivers/clocksource/Makefile | 1 +
.../time.c => drivers/clocksource/arc_timer.c | 29 ++++++----------------
6 files changed, 35 insertions(+), 34 deletions(-)
rename arch/arc/kernel/time.c => drivers/clocksource/arc_timer.c (90%)
diff --git a/MAINTAINERS b/MAINTAINERS
index 3d838cf49f81..57b56ff1dd68 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11632,6 +11632,7 @@ S: Supported
F: arch/arc/
F: Documentation/devicetree/bindings/arc/*
F: Documentation/devicetree/bindings/interrupt-controller/snps,arc*
+F: drivers/clocksource/arc_timer.c
F: drivers/tty/serial/arc_uart.c
T: git git://git.kernel.org/pub/scm/linux/kernel/git/vgupta/arc.git
diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
index b4499f97035a..8768a509d5e7 100644
--- a/arch/arc/Kconfig
+++ b/arch/arc/Kconfig
@@ -8,9 +8,9 @@
config ARC
def_bool y
+ select ARC_TIMER
select ARCH_SUPPORTS_ATOMIC_RMW if ARC_HAS_LLSC
select BUILDTIME_EXTABLE_SORT
- select CLKSRC_OF
select CLONE_BACKWARDS
select COMMON_CLK
select GENERIC_ATOMIC64 if !ISA_ARCV2 || !(ARC_HAS_LL64 && ARC_HAS_LLSC)
@@ -410,16 +410,6 @@ config ARC_HAS_DIV_REM
bool "Insn: div, divu, rem, remu"
default y
-config ARC_TIMER_RTC
- bool "Local 64-bit r/o cycle counter"
- default n
- depends on !SMP
-
-config ARC_TIMER_GFRC
- bool "SMP synchronized 64-bit cycle counter"
- default y
- depends on SMP
-
config ARC_NUMBER_OF_INTERRUPTS
int "Number of interrupts"
range 8 240
diff --git a/arch/arc/kernel/Makefile b/arch/arc/kernel/Makefile
index cfcdedf52ff8..8942c5c3b4c5 100644
--- a/arch/arc/kernel/Makefile
+++ b/arch/arc/kernel/Makefile
@@ -8,7 +8,7 @@
# Pass UTS_MACHINE for user_regset definition
CFLAGS_ptrace.o += -DUTS_MACHINE='"$(UTS_MACHINE)"'
-obj-y := arcksyms.o setup.o irq.o time.o reset.o ptrace.o process.o devtree.o
+obj-y := arcksyms.o setup.o irq.o reset.o ptrace.o process.o devtree.o
obj-y += signal.o traps.o sys.o troubleshoot.o stacktrace.o disasm.o
obj-$(CONFIG_ISA_ARCOMPACT) += entry-compact.o intc-compact.o
obj-$(CONFIG_ISA_ARCV2) += entry-arcv2.o intc-arcv2.o
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index e2c6e43cf8ca..73feaadc1924 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -282,6 +282,30 @@ config CLKSRC_MPS2
select CLKSRC_MMIO
select CLKSRC_OF
+config ARC_TIMER
+ bool "Enable timers for ARC Cores"
+ select CLKSRC_OF if OF
+
+if ARC_TIMER
+
+config ARC_TIMER_RTC
+ bool "64-bit cycle counter in HS38 cores"
+ default y if ARC
+ depends on !SMP
+ help
+ This counter provides 64-bit resolution vs. the 32-bit TIMER1.
+ It is implemented inside the core thus can't be used in SMP systems
+
+config ARC_TIMER_GFRC
+ bool "64-bit cycle counter in ARConnect block in HS38x cores"
+ default y if ARC
+ depends on SMP
+ help
+ This counter can be used as clocksource in SMP HS38 SoCs.
+ It sits outside the core thus can be used in SMP systems
+
+endif
+
config ARM_ARCH_TIMER
bool
select CLKSRC_OF if OF
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index cf87f407f1ad..e78480cb47f4 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -69,3 +69,4 @@ obj-$(CONFIG_H8300_TMR16) += h8300_timer16.o
obj-$(CONFIG_H8300_TPU) += h8300_tpu.o
obj-$(CONFIG_CLKSRC_ST_LPC) += clksrc_st_lpc.o
obj-$(CONFIG_X86_NUMACHIP) += numachip.o
+obj-$(CONFIG_ARC_TIMER) += arc_timer.o
diff --git a/arch/arc/kernel/time.c b/drivers/clocksource/arc_timer.c
similarity index 90%
rename from arch/arc/kernel/time.c
rename to drivers/clocksource/arc_timer.c
index 676b14b7a9be..ec37b6c5e903 100644
--- a/arch/arc/kernel/time.c
+++ b/drivers/clocksource/arc_timer.c
@@ -1,32 +1,18 @@
/*
+ * Copyright (C) 2016-17 Synopsys, Inc. (www.synopsys.com)
* Copyright (C) 2004, 2007-2010, 2011-2012 Synopsys, Inc. (www.synopsys.com)
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
- *
- * vineetg: Jan 1011
- * -sched_clock( ) no longer jiffies based. Uses the same clocksource
- * as gtod
- *
- * Rajeshwarr/Vineetg: Mar 2008
- * -Implemented CONFIG_GENERIC_TIME (rather deleted arch specific code)
- * for arch independent gettimeofday()
- * -Implemented CONFIG_GENERIC_CLOCKEVENTS as base for hrtimers
- *
- * Vineetg: Mar 2008: Forked off from time.c which now is time-jiff.c
*/
-/* ARC700 has two 32bit independent prog Timers: TIMER0 and TIMER1
- * Each can programmed to go from @count to @limit and optionally
- * interrupt when that happens.
- * A write to Control Register clears the Interrupt
- *
- * We've designated TIMER0 for events (clockevents)
- * while TIMER1 for free running (clocksource)
+/* ARC700 has two 32bit independent prog Timers: TIMER0 and TIMER1, Each can be
+ * programmed to go from @count to @limit and optionally interrupt.
+ * We've designated TIMER0 for clockevents and TIMER1 for clocksource
*
- * Newer ARC700 cores have 64bit clk fetching RTSC insn, preferred over TIMER1
- * which however is currently broken
+ * ARCv2 based HS38 cores have RTC (in-core) and GFRC (inside ARConnect/MCIP)
+ * which are suitable for UP and SMP based clocksources respectively
*/
#include <linux/interrupt.h>
@@ -37,7 +23,6 @@
#include <linux/cpu.h>
#include <linux/of.h>
#include <linux/of_irq.h>
-#include <asm/irq.h>
#include <soc/arc/timers.h>
#include <soc/arc/mcip.h>
@@ -281,7 +266,7 @@ static irqreturn_t timer_irq_handler(int irq, void *dev_id)
* irq_set_chip_and_handler() asked for handle_percpu_devid_irq()
*/
struct clock_event_device *evt = this_cpu_ptr(&arc_clockevent_device);
- int irq_reenable = clockevent_state_periodic(evt);
+ int irq_reenable __maybe_unused = clockevent_state_periodic(evt);
/*
* Any write to CTRL reg ACks the interrupt, we rewrite the
--
2.7.4
^ permalink raw reply related [flat|nested] 52+ messages in thread
* Re: [PATCH 9/9] clocksource: import ARC timer driver
2016-10-31 22:48 ` [PATCH 9/9] clocksource: import ARC timer driver Vineet Gupta
@ 2016-11-01 0:01 ` kbuild test robot
2016-11-01 0:45 ` Vineet Gupta
2016-11-01 20:42 ` Daniel Lezcano
1 sibling, 1 reply; 52+ messages in thread
From: kbuild test robot @ 2016-11-01 0:01 UTC (permalink / raw)
To: Vineet Gupta
Cc: kbuild-all, Daniel Lezcano, tglx, linux-kernel, linux-snps-arc,
Noam Camus, Alexey.Brodkin, Vineet Gupta
[-- Attachment #1: Type: text/plain, Size: 15110 bytes --]
Hi Vineet,
[auto build test ERROR on linus/master]
[also build test ERROR on v4.9-rc3]
[cannot apply to arc/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
[Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
[Check https://git-scm.com/docs/git-format-patch for more information]
url: https://github.com/0day-ci/linux/commits/Vineet-Gupta/Move-ARC-timer-code-into-drivers-clocksource/20161101-065452
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=ia64
All error/warnings (new ones prefixed by >>):
>> drivers/clocksource/arc_timer.c:237:19: warning: 'struct clock_event_device' declared inside parameter list will not be visible outside of this definition or declaration
struct clock_event_device *dev)
^~~~~~~~~~~~~~~~~~
drivers/clocksource/arc_timer.c:243:45: warning: 'struct clock_event_device' declared inside parameter list will not be visible outside of this definition or declaration
static int arc_clkevent_set_periodic(struct clock_event_device *dev)
^~~~~~~~~~~~~~~~~~
In file included from include/asm-generic/percpu.h:6:0,
from arch/ia64/include/asm/percpu.h:45,
from arch/ia64/include/asm/processor.h:77,
from arch/ia64/include/asm/thread_info.h:11,
from include/linux/thread_info.h:58,
from include/asm-generic/preempt.h:4,
from ./arch/ia64/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:59,
from include/linux/interrupt.h:8,
from drivers/clocksource/arc_timer.c:18:
>> drivers/clocksource/arc_timer.c:253:30: error: variable 'arc_clockevent_device' has initializer but incomplete type
static DEFINE_PER_CPU(struct clock_event_device, arc_clockevent_device) = {
^
include/linux/percpu-defs.h:95:13: note: in definition of macro 'DEFINE_PER_CPU_SECTION'
__typeof__(type) name
^~~~
>> drivers/clocksource/arc_timer.c:253:8: note: in expansion of macro 'DEFINE_PER_CPU'
static DEFINE_PER_CPU(struct clock_event_device, arc_clockevent_device) = {
^~~~~~~~~~~~~~
>> drivers/clocksource/arc_timer.c:254:2: error: unknown field 'name' specified in initializer
.name = "ARC Timer0",
^
>> drivers/clocksource/arc_timer.c:254:12: warning: excess elements in struct initializer
.name = "ARC Timer0",
^~~~~~~~~~~~
drivers/clocksource/arc_timer.c:254:12: note: (near initialization for 'arc_clockevent_device')
>> drivers/clocksource/arc_timer.c:255:2: error: unknown field 'features' specified in initializer
.features = CLOCK_EVT_FEAT_ONESHOT |
^
>> drivers/clocksource/arc_timer.c:255:15: error: 'CLOCK_EVT_FEAT_ONESHOT' undeclared here (not in a function)
.features = CLOCK_EVT_FEAT_ONESHOT |
^~~~~~~~~~~~~~~~~~~~~~
>> drivers/clocksource/arc_timer.c:256:7: error: 'CLOCK_EVT_FEAT_PERIODIC' undeclared here (not in a function)
CLOCK_EVT_FEAT_PERIODIC,
^~~~~~~~~~~~~~~~~~~~~~~
drivers/clocksource/arc_timer.c:255:15: warning: excess elements in struct initializer
.features = CLOCK_EVT_FEAT_ONESHOT |
^~~~~~~~~~~~~~~~~~~~~~
drivers/clocksource/arc_timer.c:255:15: note: (near initialization for 'arc_clockevent_device')
>> drivers/clocksource/arc_timer.c:257:2: error: unknown field 'rating' specified in initializer
.rating = 300,
^
drivers/clocksource/arc_timer.c:257:14: warning: excess elements in struct initializer
.rating = 300,
^~~
drivers/clocksource/arc_timer.c:257:14: note: (near initialization for 'arc_clockevent_device')
>> drivers/clocksource/arc_timer.c:258:2: error: unknown field 'set_next_event' specified in initializer
.set_next_event = arc_clkevent_set_next_event,
^
drivers/clocksource/arc_timer.c:258:21: warning: excess elements in struct initializer
.set_next_event = arc_clkevent_set_next_event,
^~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/clocksource/arc_timer.c:258:21: note: (near initialization for 'arc_clockevent_device')
>> drivers/clocksource/arc_timer.c:259:2: error: unknown field 'set_state_periodic' specified in initializer
.set_state_periodic = arc_clkevent_set_periodic,
^
drivers/clocksource/arc_timer.c:259:24: warning: excess elements in struct initializer
.set_state_periodic = arc_clkevent_set_periodic,
^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/clocksource/arc_timer.c:259:24: note: (near initialization for 'arc_clockevent_device')
drivers/clocksource/arc_timer.c: In function 'timer_irq_handler':
>> drivers/clocksource/arc_timer.c:268:9: error: invalid use of undefined type 'struct clock_event_device'
struct clock_event_device *evt = this_cpu_ptr(&arc_clockevent_device);
^~~~~~~~~~~~~~~~~~
>> drivers/clocksource/arc_timer.c:269:36: error: implicit declaration of function 'clockevent_state_periodic' [-Werror=implicit-function-declaration]
int irq_reenable __maybe_unused = clockevent_state_periodic(evt);
^~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/clocksource/arc_timer.c:278:5: error: dereferencing pointer to incomplete type 'struct clock_event_device'
evt->event_handler(evt);
^~
drivers/clocksource/arc_timer.c: In function 'arc_timer_starting_cpu':
drivers/clocksource/arc_timer.c:286:9: error: invalid use of undefined type 'struct clock_event_device'
struct clock_event_device *evt = this_cpu_ptr(&arc_clockevent_device);
^~~~~~~~~~~~~~~~~~
>> drivers/clocksource/arc_timer.c:290:2: error: implicit declaration of function 'clockevents_config_and_register' [-Werror=implicit-function-declaration]
clockevents_config_and_register(evt, arc_timer_freq, 0, ARC_TIMER_MAX);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/clocksource/arc_timer.c: In function 'arc_clockevent_setup':
drivers/clocksource/arc_timer.c:306:9: error: invalid use of undefined type 'struct clock_event_device'
struct clock_event_device *evt = this_cpu_ptr(&arc_clockevent_device);
^~~~~~~~~~~~~~~~~~
In file included from include/asm-generic/percpu.h:6:0,
from arch/ia64/include/asm/percpu.h:45,
from arch/ia64/include/asm/processor.h:77,
from arch/ia64/include/asm/thread_info.h:11,
from include/linux/thread_info.h:58,
from include/asm-generic/preempt.h:4,
from ./arch/ia64/include/generated/asm/preempt.h:1,
from include/linux/preempt.h:59,
from include/linux/interrupt.h:8,
from drivers/clocksource/arc_timer.c:18:
drivers/clocksource/arc_timer.c: At top level:
>> drivers/clocksource/arc_timer.c:253:50: error: storage size of 'arc_clockevent_device' isn't known
static DEFINE_PER_CPU(struct clock_event_device, arc_clockevent_device) = {
^
include/linux/percpu-defs.h:95:19: note: in definition of macro 'DEFINE_PER_CPU_SECTION'
__typeof__(type) name
^~~~
>> drivers/clocksource/arc_timer.c:253:8: note: in expansion of macro 'DEFINE_PER_CPU'
static DEFINE_PER_CPU(struct clock_event_device, arc_clockevent_device) = {
^~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/arc_clockevent_device +253 drivers/clocksource/arc_timer.c
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 231
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 232 write_aux_reg(ARC_REG_TIMER0_CTRL, TIMER_CTRL_IE | TIMER_CTRL_NH);
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 233 }
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 234
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 235
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 236 static int arc_clkevent_set_next_event(unsigned long delta,
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 @237 struct clock_event_device *dev)
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 238 {
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 239 arc_timer_event_setup(delta);
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 240 return 0;
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 241 }
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 242
aeec6cdad arch/arc/kernel/time.c Viresh Kumar 2015-07-16 243 static int arc_clkevent_set_periodic(struct clock_event_device *dev)
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 244 {
c9a98e184 arch/arc/kernel/time.c Vineet Gupta 2014-06-25 245 /*
c9a98e184 arch/arc/kernel/time.c Vineet Gupta 2014-06-25 246 * At X Hz, 1 sec = 1000ms -> X cycles;
c9a98e184 arch/arc/kernel/time.c Vineet Gupta 2014-06-25 247 * 10ms -> X / 100 cycles
c9a98e184 arch/arc/kernel/time.c Vineet Gupta 2014-06-25 248 */
77c8d0d6b arch/arc/kernel/time.c Vineet Gupta 2016-01-01 249 arc_timer_event_setup(arc_timer_freq / HZ);
aeec6cdad arch/arc/kernel/time.c Viresh Kumar 2015-07-16 250 return 0;
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 251 }
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 252
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 @253 static DEFINE_PER_CPU(struct clock_event_device, arc_clockevent_device) = {
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 @254 .name = "ARC Timer0",
aeec6cdad arch/arc/kernel/time.c Viresh Kumar 2015-07-16 @255 .features = CLOCK_EVT_FEAT_ONESHOT |
aeec6cdad arch/arc/kernel/time.c Viresh Kumar 2015-07-16 @256 CLOCK_EVT_FEAT_PERIODIC,
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 @257 .rating = 300,
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 @258 .set_next_event = arc_clkevent_set_next_event,
aeec6cdad arch/arc/kernel/time.c Viresh Kumar 2015-07-16 @259 .set_state_periodic = arc_clkevent_set_periodic,
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 260 };
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 261
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 262 static irqreturn_t timer_irq_handler(int irq, void *dev_id)
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 263 {
f8b34c3fd arch/arc/kernel/time.c Vineet Gupta 2014-01-25 264 /*
f8b34c3fd arch/arc/kernel/time.c Vineet Gupta 2014-01-25 265 * Note that generic IRQ core could have passed @evt for @dev_id if
f8b34c3fd arch/arc/kernel/time.c Vineet Gupta 2014-01-25 266 * irq_set_chip_and_handler() asked for handle_percpu_devid_irq()
f8b34c3fd arch/arc/kernel/time.c Vineet Gupta 2014-01-25 267 */
f8b34c3fd arch/arc/kernel/time.c Vineet Gupta 2014-01-25 @268 struct clock_event_device *evt = this_cpu_ptr(&arc_clockevent_device);
31aaee97f drivers/clocksource/arc_timer.c Vineet Gupta 2016-10-31 @269 int irq_reenable __maybe_unused = clockevent_state_periodic(evt);
f8b34c3fd arch/arc/kernel/time.c Vineet Gupta 2014-01-25 270
f8b34c3fd arch/arc/kernel/time.c Vineet Gupta 2014-01-25 271 /*
f8b34c3fd arch/arc/kernel/time.c Vineet Gupta 2014-01-25 272 * Any write to CTRL reg ACks the interrupt, we rewrite the
f8b34c3fd arch/arc/kernel/time.c Vineet Gupta 2014-01-25 273 * Count when [N]ot [H]alted bit.
f8b34c3fd arch/arc/kernel/time.c Vineet Gupta 2014-01-25 274 * And re-arm it if perioid by [I]nterrupt [E]nable bit
f8b34c3fd arch/arc/kernel/time.c Vineet Gupta 2014-01-25 275 */
f8b34c3fd arch/arc/kernel/time.c Vineet Gupta 2014-01-25 276 write_aux_reg(ARC_REG_TIMER0_CTRL, irq_reenable | TIMER_CTRL_NH);
f8b34c3fd arch/arc/kernel/time.c Vineet Gupta 2014-01-25 277
f8b34c3fd arch/arc/kernel/time.c Vineet Gupta 2014-01-25 @278 evt->event_handler(evt);
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 279
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 280 return IRQ_HANDLED;
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 281 }
d8005e6b9 arch/arc/kernel/time.c Vineet Gupta 2013-01-18 282
ecd8081f6 arch/arc/kernel/time.c Anna-Maria Gleixner 2016-07-13 283
ecd8081f6 arch/arc/kernel/time.c Anna-Maria Gleixner 2016-07-13 284 static int arc_timer_starting_cpu(unsigned int cpu)
eec3c58ef arch/arc/kernel/time.c Noam Camus 2016-01-01 285 {
eec3c58ef arch/arc/kernel/time.c Noam Camus 2016-01-01 286 struct clock_event_device *evt = this_cpu_ptr(&arc_clockevent_device);
eec3c58ef arch/arc/kernel/time.c Noam Camus 2016-01-01 287
eec3c58ef arch/arc/kernel/time.c Noam Camus 2016-01-01 288 evt->cpumask = cpumask_of(smp_processor_id());
eec3c58ef arch/arc/kernel/time.c Noam Camus 2016-01-01 289
ecd8081f6 arch/arc/kernel/time.c Anna-Maria Gleixner 2016-07-13 @290 clockevents_config_and_register(evt, arc_timer_freq, 0, ARC_TIMER_MAX);
eec3c58ef arch/arc/kernel/time.c Noam Camus 2016-01-01 291 enable_percpu_irq(arc_timer_irq, 0);
ecd8081f6 arch/arc/kernel/time.c Anna-Maria Gleixner 2016-07-13 292 return 0;
eec3c58ef arch/arc/kernel/time.c Noam Camus 2016-01-01 293 }
:::::: The code at line 253 was first introduced by commit
:::::: d8005e6b95268cbb50db3773d5f180c32a9434fe ARC: Timers/counters/delay management
:::::: TO: Vineet Gupta <vgupta@synopsys.com>
:::::: CC: Vineet Gupta <vgupta@synopsys.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45102 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 9/9] clocksource: import ARC timer driver
2016-11-01 0:01 ` kbuild test robot
@ 2016-11-01 0:45 ` Vineet Gupta
0 siblings, 0 replies; 52+ messages in thread
From: Vineet Gupta @ 2016-11-01 0:45 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, Daniel Lezcano, tglx, linux-kernel, linux-snps-arc,
Noam Camus, Alexey.Brodkin
On 10/31/2016 05:01 PM, kbuild test robot wrote:
> Hi Vineet,
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.9-rc3]
> [cannot apply to arc/for-next]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> [Suggest to use git(>=2.9.0) format-patch --base=<commit> (or --base=auto for convenience) to record what (public, well-known) commit your patch series was built on]
> [Check https://git-scm.com/docs/git-format-patch for more information]
>
> url: https://github.com/0day-ci/linux/commits/Vineet-Gupta/Move-ARC-timer-code-into-drivers-clocksource/20161101-065452
> config: ia64-allmodconfig (attached as .config)
> compiler: ia64-linux-gcc (GCC) 6.2.0
> reproduce:
> wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=ia64
>
> All error/warnings (new ones prefixed by >>):
>
>>> >> drivers/clocksource/arc_timer.c:237:19: warning: 'struct clock_event_device' declared inside parameter list will not be visible outside of this definition or declaration
> struct clock_event_device *dev)
> ^~~~~~~~~~~~~~~~~~
> drivers/clocksource/arc_timer.c:243:45: warning: 'struct clock_event_device' declared inside parameter list will not be visible outside of this definition or declaration
So I'd build tested the series for ARM (32 bit) and x86 (64 bit).
Seems this requires a depends on GENERIC_CLOCKEVENTS. Will fix it along with other
comments !
Thx for reporting.
-Vineet
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 9/9] clocksource: import ARC timer driver
2016-10-31 22:48 ` [PATCH 9/9] clocksource: import ARC timer driver Vineet Gupta
2016-11-01 0:01 ` kbuild test robot
@ 2016-11-01 20:42 ` Daniel Lezcano
2016-11-01 20:57 ` Vineet Gupta
1 sibling, 1 reply; 52+ messages in thread
From: Daniel Lezcano @ 2016-11-01 20:42 UTC (permalink / raw)
To: Vineet Gupta
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin
On Mon, Oct 31, 2016 at 03:48:16PM -0700, Vineet Gupta wrote:
> Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
> ---
> MAINTAINERS | 1 +
> arch/arc/Kconfig | 12 +--------
> arch/arc/kernel/Makefile | 2 +-
> drivers/clocksource/Kconfig | 24 ++++++++++++++++++
> drivers/clocksource/Makefile | 1 +
> .../time.c => drivers/clocksource/arc_timer.c | 29 ++++++----------------
> 6 files changed, 35 insertions(+), 34 deletions(-)
> rename arch/arc/kernel/time.c => drivers/clocksource/arc_timer.c (90%)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3d838cf49f81..57b56ff1dd68 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11632,6 +11632,7 @@ S: Supported
> F: arch/arc/
> F: Documentation/devicetree/bindings/arc/*
> F: Documentation/devicetree/bindings/interrupt-controller/snps,arc*
> +F: drivers/clocksource/arc_timer.c
> F: drivers/tty/serial/arc_uart.c
> T: git git://git.kernel.org/pub/scm/linux/kernel/git/vgupta/arc.git
>
> diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
> index b4499f97035a..8768a509d5e7 100644
> --- a/arch/arc/Kconfig
> +++ b/arch/arc/Kconfig
> @@ -8,9 +8,9 @@
>
> config ARC
> def_bool y
> + select ARC_TIMER
> select ARCH_SUPPORTS_ATOMIC_RMW if ARC_HAS_LLSC
> select BUILDTIME_EXTABLE_SORT
> - select CLKSRC_OF
> select CLONE_BACKWARDS
> select COMMON_CLK
> select GENERIC_ATOMIC64 if !ISA_ARCV2 || !(ARC_HAS_LL64 && ARC_HAS_LLSC)
> @@ -410,16 +410,6 @@ config ARC_HAS_DIV_REM
> bool "Insn: div, divu, rem, remu"
> default y
>
> -config ARC_TIMER_RTC
> - bool "Local 64-bit r/o cycle counter"
> - default n
> - depends on !SMP
> -
> -config ARC_TIMER_GFRC
> - bool "SMP synchronized 64-bit cycle counter"
> - default y
> - depends on SMP
> -
> config ARC_NUMBER_OF_INTERRUPTS
> int "Number of interrupts"
> range 8 240
> diff --git a/arch/arc/kernel/Makefile b/arch/arc/kernel/Makefile
> index cfcdedf52ff8..8942c5c3b4c5 100644
> --- a/arch/arc/kernel/Makefile
> +++ b/arch/arc/kernel/Makefile
> @@ -8,7 +8,7 @@
> # Pass UTS_MACHINE for user_regset definition
> CFLAGS_ptrace.o += -DUTS_MACHINE='"$(UTS_MACHINE)"'
>
> -obj-y := arcksyms.o setup.o irq.o time.o reset.o ptrace.o process.o devtree.o
> +obj-y := arcksyms.o setup.o irq.o reset.o ptrace.o process.o devtree.o
> obj-y += signal.o traps.o sys.o troubleshoot.o stacktrace.o disasm.o
> obj-$(CONFIG_ISA_ARCOMPACT) += entry-compact.o intc-compact.o
> obj-$(CONFIG_ISA_ARCV2) += entry-arcv2.o intc-arcv2.o
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index e2c6e43cf8ca..73feaadc1924 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -282,6 +282,30 @@ config CLKSRC_MPS2
> select CLKSRC_MMIO
> select CLKSRC_OF
>
> +config ARC_TIMER
> + bool "Enable timers for ARC Cores"
> + select CLKSRC_OF if OF
> +
> +if ARC_TIMER
> +
> +config ARC_TIMER_RTC
> + bool "64-bit cycle counter in HS38 cores"
> + default y if ARC
> + depends on !SMP
> + help
> + This counter provides 64-bit resolution vs. the 32-bit TIMER1.
> + It is implemented inside the core thus can't be used in SMP systems
> +
> +config ARC_TIMER_GFRC
> + bool "64-bit cycle counter in ARConnect block in HS38x cores"
> + default y if ARC
> + depends on SMP
> + help
> + This counter can be used as clocksource in SMP HS38 SoCs.
> + It sits outside the core thus can be used in SMP systems
> +
> +endif
> +
Please stay consistent with the rest of the Kconfig.
config ARC_TIMER_RTC
bool "64-bit cycle counter in HS38 cores" if COMPILE_TEST
select CLKSRC_OF
help
This counter provides 64-bit resolution vs. the 32-bit TIMER1.
It is implemented inside the core thus can't be used in SMP systems.
config ARC_TIMER_GFRC
bool "64-bit cycle counter in ARConnect block in HS38x cores" if COMPILE_TEST
select CLKSRC_OF
help
This counter can be used as clocksource in SMP HS38 SoCs.
It sits outside the core thus can be used in SMP systems
Then in the ARC's Kconfig you select ARC_TIMER_RTC or ARC_TIMER_GFRC depending
it is SMP or not.
One question:
Why ARC_TIMER_RTC can't be used in a SMP system ? Doesn't have each core its
own clocksource ? It seems you are assuming a clocksource can be used on SMP
only if the clocksource is unique and shared across the cores.
> config ARM_ARCH_TIMER
> bool
> select CLKSRC_OF if OF
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index cf87f407f1ad..e78480cb47f4 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -69,3 +69,4 @@ obj-$(CONFIG_H8300_TMR16) += h8300_timer16.o
> obj-$(CONFIG_H8300_TPU) += h8300_tpu.o
> obj-$(CONFIG_CLKSRC_ST_LPC) += clksrc_st_lpc.o
> obj-$(CONFIG_X86_NUMACHIP) += numachip.o
> +obj-$(CONFIG_ARC_TIMER) += arc_timer.o
> diff --git a/arch/arc/kernel/time.c b/drivers/clocksource/arc_timer.c
> similarity index 90%
> rename from arch/arc/kernel/time.c
> rename to drivers/clocksource/arc_timer.c
> index 676b14b7a9be..ec37b6c5e903 100644
> --- a/arch/arc/kernel/time.c
> +++ b/drivers/clocksource/arc_timer.c
> @@ -1,32 +1,18 @@
> /*
> + * Copyright (C) 2016-17 Synopsys, Inc. (www.synopsys.com)
> * Copyright (C) 2004, 2007-2010, 2011-2012 Synopsys, Inc. (www.synopsys.com)
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License version 2 as
> * published by the Free Software Foundation.
> - *
> - * vineetg: Jan 1011
> - * -sched_clock( ) no longer jiffies based. Uses the same clocksource
> - * as gtod
> - *
> - * Rajeshwarr/Vineetg: Mar 2008
> - * -Implemented CONFIG_GENERIC_TIME (rather deleted arch specific code)
> - * for arch independent gettimeofday()
> - * -Implemented CONFIG_GENERIC_CLOCKEVENTS as base for hrtimers
> - *
> - * Vineetg: Mar 2008: Forked off from time.c which now is time-jiff.c
> */
>
> -/* ARC700 has two 32bit independent prog Timers: TIMER0 and TIMER1
> - * Each can programmed to go from @count to @limit and optionally
> - * interrupt when that happens.
> - * A write to Control Register clears the Interrupt
> - *
> - * We've designated TIMER0 for events (clockevents)
> - * while TIMER1 for free running (clocksource)
> +/* ARC700 has two 32bit independent prog Timers: TIMER0 and TIMER1, Each can be
> + * programmed to go from @count to @limit and optionally interrupt.
> + * We've designated TIMER0 for clockevents and TIMER1 for clocksource
> *
> - * Newer ARC700 cores have 64bit clk fetching RTSC insn, preferred over TIMER1
> - * which however is currently broken
> + * ARCv2 based HS38 cores have RTC (in-core) and GFRC (inside ARConnect/MCIP)
> + * which are suitable for UP and SMP based clocksources respectively
> */
>
> #include <linux/interrupt.h>
> @@ -37,7 +23,6 @@
> #include <linux/cpu.h>
> #include <linux/of.h>
> #include <linux/of_irq.h>
> -#include <asm/irq.h>
>
> #include <soc/arc/timers.h>
> #include <soc/arc/mcip.h>
> @@ -281,7 +266,7 @@ static irqreturn_t timer_irq_handler(int irq, void *dev_id)
> * irq_set_chip_and_handler() asked for handle_percpu_devid_irq()
> */
> struct clock_event_device *evt = this_cpu_ptr(&arc_clockevent_device);
> - int irq_reenable = clockevent_state_periodic(evt);
> + int irq_reenable __maybe_unused = clockevent_state_periodic(evt);
>
> /*
> * Any write to CTRL reg ACks the interrupt, we rewrite the
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 9/9] clocksource: import ARC timer driver
2016-11-01 20:42 ` Daniel Lezcano
@ 2016-11-01 20:57 ` Vineet Gupta
2016-11-02 0:19 ` Daniel Lezcano
2016-11-03 17:33 ` Daniel Lezcano
0 siblings, 2 replies; 52+ messages in thread
From: Vineet Gupta @ 2016-11-01 20:57 UTC (permalink / raw)
To: Daniel Lezcano
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin
Hi Daniel,
On 11/01/2016 01:42 PM, Daniel Lezcano wrote:
> Please stay consistent with the rest of the Kconfig.
>
> config ARC_TIMER_RTC
> bool "64-bit cycle counter in HS38 cores" if COMPILE_TEST
> select CLKSRC_OF
> help
> This counter provides 64-bit resolution vs. the 32-bit TIMER1.
> It is implemented inside the core thus can't be used in SMP systems.
>
> config ARC_TIMER_GFRC
> bool "64-bit cycle counter in ARConnect block in HS38x cores" if COMPILE_TEST
> select CLKSRC_OF
> help
> This counter can be used as clocksource in SMP HS38 SoCs.
> It sits outside the core thus can be used in SMP systems
>
Yes I did so already :-) Although I also added a default y if ARC to both, but as
you say that is better done in ARC Kconfig.
> Then in the ARC's Kconfig you select ARC_TIMER_RTC or ARC_TIMER_GFRC depending
> it is SMP or not.
>
> One question:
>
> Why ARC_TIMER_RTC can't be used in a SMP system ? Doesn't have each core its
> own clocksource ? It seems you are assuming a clocksource can be used on SMP
> only if the clocksource is unique and shared across the cores.
Thats what I thought so far. Thing is, the individual core's counters could get
out of sync, simply because non masters cores were halted to begin with and came
up at different points in real time. so a gtod might return different value
depending on what core it landed on. Does clocksource also does ticks broadcasts
and such to keep things in sync ?
Because of the git mv you, diff didn't include bulk of driver code which would
make for bulk of review anyways. So perhaps in v2 I don't do the git mv. OK ?
Thx,
-Vineet
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 9/9] clocksource: import ARC timer driver
2016-11-01 20:57 ` Vineet Gupta
@ 2016-11-02 0:19 ` Daniel Lezcano
2016-11-02 1:03 ` Vineet Gupta
2016-11-03 17:33 ` Daniel Lezcano
1 sibling, 1 reply; 52+ messages in thread
From: Daniel Lezcano @ 2016-11-02 0:19 UTC (permalink / raw)
To: Vineet Gupta
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin
On Tue, Nov 01, 2016 at 01:57:05PM -0700, Vineet Gupta wrote:
> Hi Daniel,
>
> On 11/01/2016 01:42 PM, Daniel Lezcano wrote:
> > Please stay consistent with the rest of the Kconfig.
> >
> > config ARC_TIMER_RTC
> > bool "64-bit cycle counter in HS38 cores" if COMPILE_TEST
> > select CLKSRC_OF
> > help
> > This counter provides 64-bit resolution vs. the 32-bit TIMER1.
> > It is implemented inside the core thus can't be used in SMP systems.
> >
> > config ARC_TIMER_GFRC
> > bool "64-bit cycle counter in ARConnect block in HS38x cores" if COMPILE_TEST
> > select CLKSRC_OF
> > help
> > This counter can be used as clocksource in SMP HS38 SoCs.
> > It sits outside the core thus can be used in SMP systems
> >
>
> Yes I did so already :-) Although I also added a default y if ARC to both, but as
> you say that is better done in ARC Kconfig.
>
> > Then in the ARC's Kconfig you select ARC_TIMER_RTC or ARC_TIMER_GFRC depending
> > it is SMP or not.
> >
> > One question:
> >
> > Why ARC_TIMER_RTC can't be used in a SMP system ? Doesn't have each core its
> > own clocksource ? It seems you are assuming a clocksource can be used on SMP
> > only if the clocksource is unique and shared across the cores.
>
> Thats what I thought so far. Thing is, the individual core's counters could get
> out of sync, simply because non masters cores were halted to begin with and came
> up at different points in real time. so a gtod might return different value
> depending on what core it landed on. Does clocksource also does ticks broadcasts
> and such to keep things in sync ?
Sounds like it is similar than the TSC. Do you agree to have a try by setting
the CONFIG_HAVE_UNSTABLE_SCHED_CLOCK option ?
If you can use those per cpu clocksource, performances on your system may
improve with the sched_clock().
> Because of the git mv you, diff didn't include bulk of driver code which would
> make for bulk of review anyways. So perhaps in v2 I don't do the git mv. OK ?
That means I will review and comment existing code. It is not a problem for me
if you agree to do the changes.
-- Daniel
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 9/9] clocksource: import ARC timer driver
2016-11-02 0:19 ` Daniel Lezcano
@ 2016-11-02 1:03 ` Vineet Gupta
2016-11-03 16:40 ` Vineet Gupta
0 siblings, 1 reply; 52+ messages in thread
From: Vineet Gupta @ 2016-11-02 1:03 UTC (permalink / raw)
To: Daniel Lezcano
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin
On 11/01/2016 05:19 PM, Daniel Lezcano wrote:
>>>
>>> One question:
>>>
>>> Why ARC_TIMER_RTC can't be used in a SMP system ? Doesn't have each core its
>>> own clocksource ? It seems you are assuming a clocksource can be used on SMP
>>> only if the clocksource is unique and shared across the cores.
>>
>> Thats what I thought so far. Thing is, the individual core's counters could get
>> out of sync, simply because non masters cores were halted to begin with and came
>> up at different points in real time. so a gtod might return different value
>> depending on what core it landed on. Does clocksource also does ticks broadcasts
>> and such to keep things in sync ?
>
> Sounds like it is similar than the TSC. Do you agree to have a try by setting
> the CONFIG_HAVE_UNSTABLE_SCHED_CLOCK option ?
I'm not sure why we would want to enable extra stuff - I see work queues and bunch
of per cpu counting / math to adjust for the variance, if this was enabled. Anyhow
see more below.
> If you can use those per cpu clocksource, performances on your system may
> improve with the sched_clock().
Couple of things
1. Currently we don't hookup sched clock to any counter at all (on my todo list
for a while). So we only get jiffies64 based value - I know that sucks - causes
scheduling to be not super accurate etc - potentially affects benchmarks etc - but
that can be fixed easily / independent of this.
2. Say we did have sched_clock() driven by hardware - in SMP system I would still
prefer it to be driven by "common" GFRC and not "per cpu" RTC. The overhead of
HAVE_UNSTABLE_SCHED_CLOCK looks way way more than reading GFRC counter like this.
local_irq_save(flags);
__mcip_cmd(CMD_GFRC_READ_LO, 0);
stamp.l = read_aux_reg(ARC_REG_MCIP_READBACK);
__mcip_cmd(CMD_GFRC_READ_HI, 0);
stamp.h = read_aux_reg(ARC_REG_MCIP_READBACK);
local_irq_restore(flags);
GFRC reading by 2 cores concurrently doesn't require any synchronization at all.
The irq disabling around it is to make sure we didn't get a bogus readout lest an
interrupt came in between the read of 2 words. But if sched_clock can guarantee
that irqs are disable - I can probably even remove it at least for the purpose of
sched clock.
However I think we are digressing here a bit. IMHO, what clock we choose to drive
sched should not really be driven by the driver. It must be for the arch to decide.
We should first focus on how the clockevent/sources are programmed first and then
dive into sched_clock_xx as that doesn't exist at the moment for ARC.
>
>> Because of the git mv you, diff didn't include bulk of driver code which would
>> make for bulk of review anyways. So perhaps in v2 I don't do the git mv. OK ?
>
> That means I will review and comment existing code. It is not a problem for me
> if you agree to do the changes.
Sure, the whole point is to make things better as an outcome of review. I have no
issues changing code provided we don't add major performance regressions.
Thx,
-Vineet
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 9/9] clocksource: import ARC timer driver
2016-11-02 1:03 ` Vineet Gupta
@ 2016-11-03 16:40 ` Vineet Gupta
2016-11-03 16:50 ` Daniel Lezcano
0 siblings, 1 reply; 52+ messages in thread
From: Vineet Gupta @ 2016-11-03 16:40 UTC (permalink / raw)
To: Daniel Lezcano
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin
On 11/01/2016 06:03 PM, Vineet Gupta wrote:
>>> Because of the git mv you, diff didn't include bulk of driver code which would
>>> >> make for bulk of review anyways. So perhaps in v2 I don't do the git mv. OK ?
>> >
>> > That means I will review and comment existing code. It is not a problem for me
>> > if you agree to do the changes.
> Sure, the whole point is to make things better as an outcome of review. I have no
> issues changing code provided we don't add major performance regressions.
So just wondering if I could have some comments on the initial import of driver
before I send out a v2.
The issue is git mv didn't show bulk of code being moved. Shall I send a v2 with a
different ordering so I introduce the driver first, with new headers, new Kconfig
items etc and then as a subsequent patch prune those bits from arch/arc/* ?
-Vineet
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 9/9] clocksource: import ARC timer driver
2016-11-03 16:40 ` Vineet Gupta
@ 2016-11-03 16:50 ` Daniel Lezcano
2016-11-03 17:57 ` Vineet Gupta
0 siblings, 1 reply; 52+ messages in thread
From: Daniel Lezcano @ 2016-11-03 16:50 UTC (permalink / raw)
To: Vineet Gupta
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin
On Thu, Nov 03, 2016 at 09:40:23AM -0700, Vineet Gupta wrote:
> On 11/01/2016 06:03 PM, Vineet Gupta wrote:
> >>> Because of the git mv you, diff didn't include bulk of driver code which would
> >>> >> make for bulk of review anyways. So perhaps in v2 I don't do the git mv. OK ?
> >> >
> >> > That means I will review and comment existing code. It is not a problem for me
> >> > if you agree to do the changes.
> > Sure, the whole point is to make things better as an outcome of review. I have no
> > issues changing code provided we don't add major performance regressions.
>
> So just wondering if I could have some comments on the initial import of driver
> before I send out a v2.
Yeah, ok. Let me comment the other patches of the series and then you can send a V2.
> The issue is git mv didn't show bulk of code being moved. Shall I send a v2 with a
> different ordering so I introduce the driver first, with new headers, new Kconfig
> items etc and then as a subsequent patch prune those bits from arch/arc/* ?
>
> -Vineet
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 9/9] clocksource: import ARC timer driver
2016-11-03 16:50 ` Daniel Lezcano
@ 2016-11-03 17:57 ` Vineet Gupta
2016-11-03 18:11 ` Daniel Lezcano
0 siblings, 1 reply; 52+ messages in thread
From: Vineet Gupta @ 2016-11-03 17:57 UTC (permalink / raw)
To: Daniel Lezcano
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin
Hi Daniel,
On 11/03/2016 09:50 AM, Daniel Lezcano wrote:
> On Thu, Nov 03, 2016 at 09:40:23AM -0700, Vineet Gupta wrote:
>> On 11/01/2016 06:03 PM, Vineet Gupta wrote:
>>>>> Because of the git mv you, diff didn't include bulk of driver code which would
>>>>>>> make for bulk of review anyways. So perhaps in v2 I don't do the git mv. OK ?
>>>>>
>>>>> That means I will review and comment existing code. It is not a problem for me
>>>>> if you agree to do the changes.
>>> Sure, the whole point is to make things better as an outcome of review. I have no
>>> issues changing code provided we don't add major performance regressions.
>>
>> So just wondering if I could have some comments on the initial import of driver
>> before I send out a v2.
>
> Yeah, ok. Let me comment the other patches of the series and then you can send a V2.
Thx for taking a quick look - this is a good start. How about the actual driver
itself, do you want to take a quick look there as well before v2 ?
>
>> The issue is git mv didn't show bulk of code being moved. Shall I send a v2 with a
>> different ordering so I introduce the driver first, with new headers, new Kconfig
>> items etc and then as a subsequent patch prune those bits from arch/arc/* ?
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 9/9] clocksource: import ARC timer driver
2016-11-03 17:57 ` Vineet Gupta
@ 2016-11-03 18:11 ` Daniel Lezcano
2016-11-03 18:43 ` Vineet Gupta
0 siblings, 1 reply; 52+ messages in thread
From: Daniel Lezcano @ 2016-11-03 18:11 UTC (permalink / raw)
To: Vineet Gupta
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin
On Thu, Nov 03, 2016 at 10:57:48AM -0700, Vineet Gupta wrote:
> Hi Daniel,
>
> On 11/03/2016 09:50 AM, Daniel Lezcano wrote:
> > On Thu, Nov 03, 2016 at 09:40:23AM -0700, Vineet Gupta wrote:
> >> On 11/01/2016 06:03 PM, Vineet Gupta wrote:
> >>>>> Because of the git mv you, diff didn't include bulk of driver code which would
> >>>>>>> make for bulk of review anyways. So perhaps in v2 I don't do the git mv. OK ?
> >>>>>
> >>>>> That means I will review and comment existing code. It is not a problem for me
> >>>>> if you agree to do the changes.
> >>> Sure, the whole point is to make things better as an outcome of review. I have no
> >>> issues changing code provided we don't add major performance regressions.
> >>
> >> So just wondering if I could have some comments on the initial import of driver
> >> before I send out a v2.
> >
> > Yeah, ok. Let me comment the other patches of the series and then you can send a V2.
>
> Thx for taking a quick look - this is a good start. How about the actual driver
> itself, do you want to take a quick look there as well before v2 ?
At the first glance, with your changes it is acceptable to be moved. Perhaps,
you can have a look to remove the BIG_ENDIAN stuff in the clock read function.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 9/9] clocksource: import ARC timer driver
2016-11-03 18:11 ` Daniel Lezcano
@ 2016-11-03 18:43 ` Vineet Gupta
0 siblings, 0 replies; 52+ messages in thread
From: Vineet Gupta @ 2016-11-03 18:43 UTC (permalink / raw)
To: Daniel Lezcano
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin
On 11/03/2016 11:11 AM, Daniel Lezcano wrote:
>> Thx for taking a quick look - this is a good start. How about the actual driver
>> > itself, do you want to take a quick look there as well before v2 ?
> At the first glance, with your changes it is acceptable to be moved. Perhaps,
> you can have a look to remove the BIG_ENDIAN stuff in the clock read function.
>
OK addressed that as well !
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 9/9] clocksource: import ARC timer driver
2016-11-01 20:57 ` Vineet Gupta
2016-11-02 0:19 ` Daniel Lezcano
@ 2016-11-03 17:33 ` Daniel Lezcano
2016-11-03 18:14 ` Daniel Lezcano
2016-11-03 18:47 ` Vineet Gupta
1 sibling, 2 replies; 52+ messages in thread
From: Daniel Lezcano @ 2016-11-03 17:33 UTC (permalink / raw)
To: Vineet Gupta
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin
On Tue, Nov 01, 2016 at 01:57:05PM -0700, Vineet Gupta wrote:
> Hi Daniel,
>
> On 11/01/2016 01:42 PM, Daniel Lezcano wrote:
> > Please stay consistent with the rest of the Kconfig.
> >
> > config ARC_TIMER_RTC
> > bool "64-bit cycle counter in HS38 cores" if COMPILE_TEST
> > select CLKSRC_OF
> > help
> > This counter provides 64-bit resolution vs. the 32-bit TIMER1.
> > It is implemented inside the core thus can't be used in SMP systems.
> >
> > config ARC_TIMER_GFRC
> > bool "64-bit cycle counter in ARConnect block in HS38x cores" if COMPILE_TEST
> > select CLKSRC_OF
> > help
> > This counter can be used as clocksource in SMP HS38 SoCs.
> > It sits outside the core thus can be used in SMP systems
> >
>
> Yes I did so already :-) Although I also added a default y if ARC to both, but as
> you say that is better done in ARC Kconfig.
>
> > Then in the ARC's Kconfig you select ARC_TIMER_RTC or ARC_TIMER_GFRC depending
> > it is SMP or not.
> >
> > One question:
> >
> > Why ARC_TIMER_RTC can't be used in a SMP system ? Doesn't have each core its
> > own clocksource ? It seems you are assuming a clocksource can be used on SMP
> > only if the clocksource is unique and shared across the cores.
>
As now the clksrc-probe is correctly handling the errors, if the rtc and the
gfrc are both defined in the DT, you can fail to init the rtc one with a simple
test in the init function:
if (IS_DEFINED(CONFIG_SMP))
return -EINVAL;
So, you can inconditionaly compile in both RTC and GFRC, no ? That would be
cleaner and prevent a different kernel config.
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 9/9] clocksource: import ARC timer driver
2016-11-03 17:33 ` Daniel Lezcano
@ 2016-11-03 18:14 ` Daniel Lezcano
2016-11-03 18:47 ` Vineet Gupta
1 sibling, 0 replies; 52+ messages in thread
From: Daniel Lezcano @ 2016-11-03 18:14 UTC (permalink / raw)
To: Vineet Gupta
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin
On Thu, Nov 03, 2016 at 06:33:17PM +0100, Daniel Lezcano wrote:
[ ... ]
> As now the clksrc-probe is correctly handling the errors, if the rtc and the
> gfrc are both defined in the DT, you can fail to init the rtc one with a simple
> test in the init function:
>
> if (IS_DEFINED(CONFIG_SMP))
> return -EINVAL;
>
> So, you can inconditionaly compile in both RTC and GFRC, no ? That would be
> cleaner and prevent a different kernel config.
Ah, actually I suggested something which is already there :)
Perhaps, the if SMP does not make sense in the Kconfig, no ?
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 9/9] clocksource: import ARC timer driver
2016-11-03 17:33 ` Daniel Lezcano
2016-11-03 18:14 ` Daniel Lezcano
@ 2016-11-03 18:47 ` Vineet Gupta
1 sibling, 0 replies; 52+ messages in thread
From: Vineet Gupta @ 2016-11-03 18:47 UTC (permalink / raw)
To: Daniel Lezcano
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin
On 11/03/2016 10:33 AM, Daniel Lezcano wrote:
> As now the clksrc-probe is correctly handling the errors, if the rtc and the
> gfrc are both defined in the DT, you can fail to init the rtc one with a simple
> test in the init function:
>
> if (IS_DEFINED(CONFIG_SMP))
> return -EINVAL;
>
> So, you can inconditionaly compile in both RTC and GFRC, no ? That would be
> cleaner and prevent a different kernel config.
That's a very good idea. So now I envision
CONFIG_ARC_TIMERS # legacy TIMER0 / TIMER1
CONFIG_ARC_64BIT_TIMERS # rtc, gfrc
I need this distinction at the min to be able to select them from ARC Kconfig.
-Vineet
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH 0/9] Move ARC timer code into drivers/clocksource/
2016-10-31 22:48 ` [PATCH 0/9] Move ARC timer code into drivers/clocksource/ Vineet Gupta
` (8 preceding siblings ...)
2016-10-31 22:48 ` [PATCH 9/9] clocksource: import ARC timer driver Vineet Gupta
@ 2016-11-03 17:28 ` Daniel Lezcano
9 siblings, 0 replies; 52+ messages in thread
From: Daniel Lezcano @ 2016-11-03 17:28 UTC (permalink / raw)
To: Vineet Gupta
Cc: tglx, linux-kernel, linux-snps-arc, Noam Camus, Alexey.Brodkin
On Mon, Oct 31, 2016 at 03:48:07PM -0700, Vineet Gupta wrote:
> Hi,
>
> This series addresses the long pending move of ARC timer code into
> drivers/clocksource/.
>
> - patches [1-4]/9 are improvements to arc code, paving way for later code motion.
> - patches [5-8]/9 refactor the arc headers to be shared between arc and drivers
> - patch 9/9 moves out the driver code/build/Kconfig bits
>
> As review progresses/concludes, I'd like to merge [1-8] for this merge window and
> clocksource maintainers can pick up the actual switch for 4.10 or even 4.9 as they
> prefer.
>
The different patches deserve a longer change description.
^ permalink raw reply [flat|nested] 52+ messages in thread