Hi all,
here's a v2. I reused the subject from my v1 submission although the focus of
this series moved a bit towards the timer subsystem.
I replaced Stephen's fix to prevent per cpu devices from becoming
the broadcast device with Thomas' proposal, but I kept the original commit
message. I split the whole approach in small chunks that I found reasonable.
In 4/4, I picked up Grant's comment regarding the DT node name.
v2:
- drop 1/2 of the original series
- implement preventing per cpu devices from becoming broadcast device according
to Thomas' proposal
- introduce new CLOCK_EVT_FEAT_PERCPU flag
- set the flag for the arm_global_timer
- filter per cpu devices based on this flag when choosing the broadcast
device
- rename DT node to avoid '_'s
Soren Brinkmann (4):
clockchips: Add FEAT_PERCPU clockevent flag
clocksource/arm_global_timer: Set FEAT_PERCPU flag
tick: broadcast: Deny per-cpu clockevents from being broadcast sources
arm: zynq: Enable arm_global_timer
arch/arm/boot/dts/zynq-7000.dtsi | 8 ++++++++
arch/arm/mach-zynq/Kconfig | 1 +
drivers/clocksource/arm_global_timer.c | 3 ++-
include/linux/clockchips.h | 1 +
kernel/time/tick-broadcast.c | 1 +
5 files changed, 13 insertions(+), 1 deletion(-)
--
1.8.4
The arm_global_timer is a per cpu device. Set the appropriate flag.
Signed-off-by: Soren Brinkmann <[email protected]>
---
drivers/clocksource/arm_global_timer.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c
index b66c1f3..c639b1a 100644
--- a/drivers/clocksource/arm_global_timer.c
+++ b/drivers/clocksource/arm_global_timer.c
@@ -169,7 +169,8 @@ static int gt_clockevents_init(struct clock_event_device *clk)
int cpu = smp_processor_id();
clk->name = "arm_global_timer";
- clk->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
+ clk->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT |
+ CLOCK_EVT_FEAT_PERCPU;
clk->set_mode = gt_clockevent_set_mode;
clk->set_next_event = gt_clockevent_set_next_event;
clk->cpumask = cpumask_of(cpu);
--
1.8.4
Zynq is based on an ARM Cortex-A9 MPCore, which features the
arm_global_timer in its SCU. Therefore enable the timer for Zynq.
Signed-off-by: Soren Brinkmann <[email protected]>
Acked-by: Daniel Lezcano <[email protected]>
---
v2:
- rename DT node: global_timer@... => timer@...
- add Daniel's ACK
---
arch/arm/boot/dts/zynq-7000.dtsi | 8 ++++++++
arch/arm/mach-zynq/Kconfig | 1 +
2 files changed, 9 insertions(+)
diff --git a/arch/arm/boot/dts/zynq-7000.dtsi b/arch/arm/boot/dts/zynq-7000.dtsi
index e32b92b..e7f73b2 100644
--- a/arch/arm/boot/dts/zynq-7000.dtsi
+++ b/arch/arm/boot/dts/zynq-7000.dtsi
@@ -92,6 +92,14 @@
};
};
+ global_timer: timer@f8f00200 {
+ compatible = "arm,cortex-a9-global-timer";
+ reg = <0xf8f00200 0x20>;
+ interrupts = <1 11 0x301>;
+ interrupt-parent = <&intc>;
+ clocks = <&clkc 4>;
+ };
+
ttc0: ttc0@f8001000 {
interrupt-parent = <&intc>;
interrupts = < 0 10 4 0 11 4 0 12 4 >;
diff --git a/arch/arm/mach-zynq/Kconfig b/arch/arm/mach-zynq/Kconfig
index 04f8a4a..6b04260 100644
--- a/arch/arm/mach-zynq/Kconfig
+++ b/arch/arm/mach-zynq/Kconfig
@@ -13,5 +13,6 @@ config ARCH_ZYNQ
select HAVE_SMP
select SPARSE_IRQ
select CADENCE_TTC_TIMER
+ select ARM_GLOBAL_TIMER
help
Support for Xilinx Zynq ARM Cortex A9 Platform
--
1.8.4
Add the flag CLOCK_EVT_FEAT_PERCPU which is supposed to be set for per
cpu clockevent devices.
Signed-off-by: Soren Brinkmann <[email protected]>
---
include/linux/clockchips.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h
index 0857922..493aa02 100644
--- a/include/linux/clockchips.h
+++ b/include/linux/clockchips.h
@@ -60,6 +60,7 @@ enum clock_event_mode {
* Core shall set the interrupt affinity dynamically in broadcast mode
*/
#define CLOCK_EVT_FEAT_DYNIRQ 0x000020
+#define CLOCK_EVT_FEAT_PERCPU 0x000040
/**
* struct clock_event_device - clock event device descriptor
--
1.8.4
On most ARM systems the per-cpu clockevents are truly per-cpu in
the sense that they can't be controlled on any other CPU besides
the CPU that they interrupt. If one of these clockevents were to
become a broadcast source we will run into a lot of trouble
because the broadcast source is enabled on the first CPU to go
into deep idle (if that CPU suffers from FEAT_C3_STOP) and that
could be a different CPU than what the clockevent is interrupting
(or even worse the CPU that the clockevent interrupts could be
offline).
Theoretically it's possible to support per-cpu clockevents as the
broadcast source but so far we haven't needed this and supporting
it is rather complicated. Let's just deny the possibility for now
until this becomes a reality (let's hope it never does!).
Signed-off-by: Soren Brinkmann <[email protected]>
---
kernel/time/tick-broadcast.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
index 218bcb5..9532690 100644
--- a/kernel/time/tick-broadcast.c
+++ b/kernel/time/tick-broadcast.c
@@ -70,6 +70,7 @@ static bool tick_check_broadcast_device(struct clock_event_device *curdev,
struct clock_event_device *newdev)
{
if ((newdev->features & CLOCK_EVT_FEAT_DUMMY) ||
+ (newdev->features & CLOCK_EVT_FEAT_PERCPU) ||
(newdev->features & CLOCK_EVT_FEAT_C3STOP))
return false;
--
1.8.4
On 09/18/2013 08:48 PM, Soren Brinkmann wrote:
> Add the flag CLOCK_EVT_FEAT_PERCPU which is supposed to be set for per
> cpu clockevent devices.
>
> Signed-off-by: Soren Brinkmann <[email protected]>
Acked-by: Daniel Lezcano <[email protected]>
> ---
> include/linux/clockchips.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/linux/clockchips.h b/include/linux/clockchips.h
> index 0857922..493aa02 100644
> --- a/include/linux/clockchips.h
> +++ b/include/linux/clockchips.h
> @@ -60,6 +60,7 @@ enum clock_event_mode {
> * Core shall set the interrupt affinity dynamically in broadcast mode
> */
> #define CLOCK_EVT_FEAT_DYNIRQ 0x000020
> +#define CLOCK_EVT_FEAT_PERCPU 0x000040
>
> /**
> * struct clock_event_device - clock event device descriptor
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
On 09/18/2013 08:48 PM, Soren Brinkmann wrote:
> The arm_global_timer is a per cpu device. Set the appropriate flag.
>
> Signed-off-by: Soren Brinkmann <[email protected]>
Acked-by: Daniel Lezcano <[email protected]>
> ---
> drivers/clocksource/arm_global_timer.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c
> index b66c1f3..c639b1a 100644
> --- a/drivers/clocksource/arm_global_timer.c
> +++ b/drivers/clocksource/arm_global_timer.c
> @@ -169,7 +169,8 @@ static int gt_clockevents_init(struct clock_event_device *clk)
> int cpu = smp_processor_id();
>
> clk->name = "arm_global_timer";
> - clk->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
> + clk->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT |
> + CLOCK_EVT_FEAT_PERCPU;
> clk->set_mode = gt_clockevent_set_mode;
> clk->set_next_event = gt_clockevent_set_next_event;
> clk->cpumask = cpumask_of(cpu);
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
On 09/18/2013 08:48 PM, Soren Brinkmann wrote:
> On most ARM systems the per-cpu clockevents are truly per-cpu in
> the sense that they can't be controlled on any other CPU besides
> the CPU that they interrupt. If one of these clockevents were to
> become a broadcast source we will run into a lot of trouble
> because the broadcast source is enabled on the first CPU to go
> into deep idle (if that CPU suffers from FEAT_C3_STOP) and that
> could be a different CPU than what the clockevent is interrupting
> (or even worse the CPU that the clockevent interrupts could be
> offline).
>
> Theoretically it's possible to support per-cpu clockevents as the
> broadcast source but so far we haven't needed this and supporting
> it is rather complicated. Let's just deny the possibility for now
> until this becomes a reality (let's hope it never does!).
>
> Signed-off-by: Soren Brinkmann <[email protected]>
Acked-by: Daniel Lezcano <[email protected]>
> ---
> kernel/time/tick-broadcast.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/time/tick-broadcast.c b/kernel/time/tick-broadcast.c
> index 218bcb5..9532690 100644
> --- a/kernel/time/tick-broadcast.c
> +++ b/kernel/time/tick-broadcast.c
> @@ -70,6 +70,7 @@ static bool tick_check_broadcast_device(struct clock_event_device *curdev,
> struct clock_event_device *newdev)
> {
> if ((newdev->features & CLOCK_EVT_FEAT_DUMMY) ||
> + (newdev->features & CLOCK_EVT_FEAT_PERCPU) ||
> (newdev->features & CLOCK_EVT_FEAT_C3STOP))
> return false;
>
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
On 09/18/2013 08:48 PM, Soren Brinkmann wrote:
> Hi all,
>
> here's a v2. I reused the subject from my v1 submission although the focus of
> this series moved a bit towards the timer subsystem.
> I replaced Stephen's fix to prevent per cpu devices from becoming
> the broadcast device with Thomas' proposal, but I kept the original commit
> message. I split the whole approach in small chunks that I found reasonable.
>
> In 4/4, I picked up Grant's comment regarding the DT node name.
>
> v2:
> - drop 1/2 of the original series
> - implement preventing per cpu devices from becoming broadcast device according
> to Thomas' proposal
> - introduce new CLOCK_EVT_FEAT_PERCPU flag
> - set the flag for the arm_global_timer
> - filter per cpu devices based on this flag when choosing the broadcast
> device
> - rename DT node to avoid '_'s
>
>
> Soren Brinkmann (4):
> clockchips: Add FEAT_PERCPU clockevent flag
> clocksource/arm_global_timer: Set FEAT_PERCPU flag
> tick: broadcast: Deny per-cpu clockevents from being broadcast sources
> arm: zynq: Enable arm_global_timer
>
> arch/arm/boot/dts/zynq-7000.dtsi | 8 ++++++++
> arch/arm/mach-zynq/Kconfig | 1 +
> drivers/clocksource/arm_global_timer.c | 3 ++-
> include/linux/clockchips.h | 1 +
> kernel/time/tick-broadcast.c | 1 +
> 5 files changed, 13 insertions(+), 1 deletion(-)
This patches falls under different maintainer's umbrella.
I acked-by the patches, so if someone is willing to take the patches I
am ok with that. I am ok also to pick them into my tree with the
acked-by maintainer. Just let me know ...
Thanks
-- Daniel
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
On Wed, Sep 25, 2013 at 05:20:43PM +0200, Daniel Lezcano wrote:
> On 09/18/2013 08:48 PM, Soren Brinkmann wrote:
> > Hi all,
> >
> > here's a v2. I reused the subject from my v1 submission although the focus of
> > this series moved a bit towards the timer subsystem.
> > I replaced Stephen's fix to prevent per cpu devices from becoming
> > the broadcast device with Thomas' proposal, but I kept the original commit
> > message. I split the whole approach in small chunks that I found reasonable.
> >
> > In 4/4, I picked up Grant's comment regarding the DT node name.
> >
> > v2:
> > - drop 1/2 of the original series
> > - implement preventing per cpu devices from becoming broadcast device according
> > to Thomas' proposal
> > - introduce new CLOCK_EVT_FEAT_PERCPU flag
> > - set the flag for the arm_global_timer
> > - filter per cpu devices based on this flag when choosing the broadcast
> > device
> > - rename DT node to avoid '_'s
> >
> >
> > Soren Brinkmann (4):
> > clockchips: Add FEAT_PERCPU clockevent flag
> > clocksource/arm_global_timer: Set FEAT_PERCPU flag
> > tick: broadcast: Deny per-cpu clockevents from being broadcast sources
> > arm: zynq: Enable arm_global_timer
> >
> > arch/arm/boot/dts/zynq-7000.dtsi | 8 ++++++++
> > arch/arm/mach-zynq/Kconfig | 1 +
> > drivers/clocksource/arm_global_timer.c | 3 ++-
> > include/linux/clockchips.h | 1 +
> > kernel/time/tick-broadcast.c | 1 +
> > 5 files changed, 13 insertions(+), 1 deletion(-)
>
> This patches falls under different maintainer's umbrella.
>
> I acked-by the patches, so if someone is willing to take the patches I
> am ok with that. I am ok also to pick them into my tree with the
> acked-by maintainer. Just let me know ...
Thanks Daniel. I agree you/timer folks and Michal/armsoc have to figure
out how to resolve this the easiest way. I'd almost say merging this all
through the timers tree might be the easiest. The conflict which might
occur in the Zynq dts file should be trivial since the patch is purely
additive.
But well, up to you.
Sören
Hi Daniel,
On 09/25/2013 05:20 PM, Daniel Lezcano wrote:
> On 09/18/2013 08:48 PM, Soren Brinkmann wrote:
>> Hi all,
>>
>> here's a v2. I reused the subject from my v1 submission although the focus of
>> this series moved a bit towards the timer subsystem.
>> I replaced Stephen's fix to prevent per cpu devices from becoming
>> the broadcast device with Thomas' proposal, but I kept the original commit
>> message. I split the whole approach in small chunks that I found reasonable.
>>
>> In 4/4, I picked up Grant's comment regarding the DT node name.
>>
>> v2:
>> - drop 1/2 of the original series
>> - implement preventing per cpu devices from becoming broadcast device according
>> to Thomas' proposal
>> - introduce new CLOCK_EVT_FEAT_PERCPU flag
>> - set the flag for the arm_global_timer
>> - filter per cpu devices based on this flag when choosing the broadcast
>> device
>> - rename DT node to avoid '_'s
>>
>>
>> Soren Brinkmann (4):
>> clockchips: Add FEAT_PERCPU clockevent flag
>> clocksource/arm_global_timer: Set FEAT_PERCPU flag
>> tick: broadcast: Deny per-cpu clockevents from being broadcast sources
>> arm: zynq: Enable arm_global_timer
>>
>> arch/arm/boot/dts/zynq-7000.dtsi | 8 ++++++++
>> arch/arm/mach-zynq/Kconfig | 1 +
>> drivers/clocksource/arm_global_timer.c | 3 ++-
>> include/linux/clockchips.h | 1 +
>> kernel/time/tick-broadcast.c | 1 +
>> 5 files changed, 13 insertions(+), 1 deletion(-)
>
> This patches falls under different maintainer's umbrella.
>
> I acked-by the patches, so if someone is willing to take the patches I
> am ok with that. I am ok also to pick them into my tree with the
> acked-by maintainer. Just let me know ...
>
Yes, please add also this zynq specific patch through your tree.
Here is my ACK for that.
Acked-by: Michal Simek <[email protected]>
Thanks,
Michal
On 09/26/2013 10:16 AM, Michal Simek wrote:
> Hi Daniel,
>
> On 09/25/2013 05:20 PM, Daniel Lezcano wrote:
>> On 09/18/2013 08:48 PM, Soren Brinkmann wrote:
>>> Hi all,
>>>
>>> here's a v2. I reused the subject from my v1 submission although the focus of
>>> this series moved a bit towards the timer subsystem.
>>> I replaced Stephen's fix to prevent per cpu devices from becoming
>>> the broadcast device with Thomas' proposal, but I kept the original commit
>>> message. I split the whole approach in small chunks that I found reasonable.
>>>
>>> In 4/4, I picked up Grant's comment regarding the DT node name.
>>>
>>> v2:
>>> - drop 1/2 of the original series
>>> - implement preventing per cpu devices from becoming broadcast device according
>>> to Thomas' proposal
>>> - introduce new CLOCK_EVT_FEAT_PERCPU flag
>>> - set the flag for the arm_global_timer
>>> - filter per cpu devices based on this flag when choosing the broadcast
>>> device
>>> - rename DT node to avoid '_'s
>>>
>>>
>>> Soren Brinkmann (4):
>>> clockchips: Add FEAT_PERCPU clockevent flag
>>> clocksource/arm_global_timer: Set FEAT_PERCPU flag
>>> tick: broadcast: Deny per-cpu clockevents from being broadcast sources
>>> arm: zynq: Enable arm_global_timer
>>>
>>> arch/arm/boot/dts/zynq-7000.dtsi | 8 ++++++++
>>> arch/arm/mach-zynq/Kconfig | 1 +
>>> drivers/clocksource/arm_global_timer.c | 3 ++-
>>> include/linux/clockchips.h | 1 +
>>> kernel/time/tick-broadcast.c | 1 +
>>> 5 files changed, 13 insertions(+), 1 deletion(-)
>>
>> This patches falls under different maintainer's umbrella.
>>
>> I acked-by the patches, so if someone is willing to take the patches I
>> am ok with that. I am ok also to pick them into my tree with the
>> acked-by maintainer. Just let me know ...
>>
>
> Yes, please add also this zynq specific patch through your tree.
>
> Here is my ACK for that.
> Acked-by: Michal Simek <[email protected]>
Thomas do you mind to ack the patches if you agree with it ?
Thanks
-- Daniel
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
On 18/09/13 19:48, Soren Brinkmann wrote:
> The arm_global_timer is a per cpu device. Set the appropriate flag.
>
> Signed-off-by: Soren Brinkmann <[email protected]>
> ---
> drivers/clocksource/arm_global_timer.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c
> index b66c1f3..c639b1a 100644
> --- a/drivers/clocksource/arm_global_timer.c
> +++ b/drivers/clocksource/arm_global_timer.c
> @@ -169,7 +169,8 @@ static int gt_clockevents_init(struct clock_event_device *clk)
> int cpu = smp_processor_id();
>
> clk->name = "arm_global_timer";
> - clk->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
> + clk->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT |
> + CLOCK_EVT_FEAT_PERCPU;
> clk->set_mode = gt_clockevent_set_mode;
> clk->set_next_event = gt_clockevent_set_next_event;
> clk->cpumask = cpumask_of(cpu);
>
Acked-by: Srinivas Kandagatla <[email protected]>