[PATCH V2 0/7] clockevent: Migrate to new 'set-state' interface

[PATCH V2 0/7] clockevent: Migrate to new 'set-state' interface

[Viresh Kumar]

Hi Thomas/Daniel,

I have incorporated all the improvements Daniel suggested on V1 and so
here is V2.

The first patch allows set-state callbacks to be optional, otherwise it
leads to unnecessary noop callbacks for drivers which don't want to
implement them. Over that, these noop-callbacks result in full function
calls for nothing really useful.

Rest of the series converts few clockevent drivers to the new set-state
interface. This would enable these drivers to use new states (like:
ONESHOT_STOPPED, etc.) of a clockevent device (if required), as the
set-mode interface is marked obsolete now and wouldn't be expanded to
handle new states.

Once all the drivers are migrated to the new interface in future, we can
remove the code supporting '->mode' in clockevents core.

Drivers converted in this series are selected based on the diff they
generate. These are different diffs we shall have for most of the
drivers and any suggestions/improvements for these patches will be
applied to other drivers as well.

This is based of latest tip/master from few days back due to dependency
on clockevent_state_*() helpers.

Only the first patch is tested on hardware, others are ONLY compile
tested.

V1->V2:
- New commit: 1/7 (Daniel)
- Added all Acks from Daniel
- Updated 2/7 to return 0 directly from timer_shutdown()
- Naming fixes suggested for kona driver (Ray Jui)
- Minor changes in commit logs
- Not adding noop-callbacks

Cc: Andres Salomon <dilinger@queued.net>
Cc: bcm-kernel-feedback-list@broadcom.com
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Lee Jones <lee@kernel.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Maxime Coquelin <maxime.coquelin@st.com>
Cc: Patrice Chotard <patrice.chotard@st.com>
Cc: Ray Jui <rjui@broadcom.com>
Cc: Scott Branden <sbranden@broadcom.com>
Cc: Srinivas Kandagatla <srinivas.kandagatla@gmail.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>

Viresh Kumar (7):
  clockevents: Allow set-state callbacks to be optional
  clocksource: arm_arch_timer: Migrate to new 'set-state' interface
  clocksource: arm_global_timer: Migrate to new 'set-state' interface
  clocksource: bcm2835: Migrate to new 'set-state' interface
  clocksource: bcm_kona: Migrate to new 'set-state' interface
  clocksource: cs5535: Migrate to new 'set-state' interface
  clocksource: em_sti: Migrate to new 'set-state' interface

 drivers/clocksource/arm_arch_timer.c   | 52 ++++++++++++++--------------------
 drivers/clocksource/arm_global_timer.c | 37 +++++++++++-------------
 drivers/clocksource/bcm2835_timer.c    | 16 -----------
 drivers/clocksource/bcm_kona_timer.c   | 17 ++++-------
 drivers/clocksource/cs5535-clockevt.c  | 24 +++++++++-------
 drivers/clocksource/em_sti.c           | 39 +++++++++----------------
 kernel/time/clockevents.c              | 24 ++++++----------
 7 files changed, 81 insertions(+), 128 deletions(-)

-- 
2.4.0

[PATCH V2 1/7] clockevents: Allow set-state callbacks to be optional

[Viresh Kumar]

Its mandatory for the drivers to provide set_state_{oneshot|periodic}()
(only if related modes are supported) and set_state_shutdown() callbacks
today, if they are implementing the new set-state interface.

But this leads to unnecessary noop callbacks for drivers which don't
want to implement them. Over that, it will lead to a full function call
for nothing really useful.

Lets make all set-state callbacks optional.

Suggested-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 kernel/time/clockevents.c | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/kernel/time/clockevents.c b/kernel/time/clockevents.c
index 2397b97320d8..37ba06f01818 100644
--- a/kernel/time/clockevents.c
+++ b/kernel/time/clockevents.c
@@ -120,19 +120,25 @@ static int __clockevents_switch_state(struct clock_event_device *dev,
 		/* The clockevent device is getting replaced. Shut it down. */
 
 	case CLOCK_EVT_STATE_SHUTDOWN:
-		return dev->set_state_shutdown(dev);
+		if (dev->set_state_shutdown)
+			return dev->set_state_shutdown(dev);
+		return 0;
 
 	case CLOCK_EVT_STATE_PERIODIC:
 		/* Core internal bug */
 		if (!(dev->features & CLOCK_EVT_FEAT_PERIODIC))
 			return -ENOSYS;
-		return dev->set_state_periodic(dev);
+		if (dev->set_state_periodic)
+			return dev->set_state_periodic(dev);
+		return 0;
 
 	case CLOCK_EVT_STATE_ONESHOT:
 		/* Core internal bug */
 		if (!(dev->features & CLOCK_EVT_FEAT_ONESHOT))
 			return -ENOSYS;
-		return dev->set_state_oneshot(dev);
+		if (dev->set_state_oneshot)
+			return dev->set_state_oneshot(dev);
+		return 0;
 
 	case CLOCK_EVT_STATE_ONESHOT_STOPPED:
 		/* Core internal bug */
@@ -471,18 +477,6 @@ static int clockevents_sanity_check(struct clock_event_device *dev)
 	if (dev->features & CLOCK_EVT_FEAT_DUMMY)
 		return 0;
 
-	/* New state-specific callbacks */
-	if (!dev->set_state_shutdown)
-		return -EINVAL;
-
-	if ((dev->features & CLOCK_EVT_FEAT_PERIODIC) &&
-	    !dev->set_state_periodic)
-		return -EINVAL;
-
-	if ((dev->features & CLOCK_EVT_FEAT_ONESHOT) &&
-	    !dev->set_state_oneshot)
-		return -EINVAL;
-
 	return 0;
 }
 
-- 
2.4.0

Re: [PATCH V2 1/7] clockevents: Allow set-state callbacks to be optional

[Daniel Lezcano]

On 06/12/2015 10:00 AM, Viresh Kumar wrote:
> Its mandatory for the drivers to provide set_state_{oneshot|periodic}()
> (only if related modes are supported) and set_state_shutdown() callbacks
> today, if they are implementing the new set-state interface.
>
> But this leads to unnecessary noop callbacks for drivers which don't
> want to implement them. Over that, it will lead to a full function call
> for nothing really useful.
>
> Lets make all set-state callbacks optional.
>
> Suggested-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

As this patch is needed for the other changes in this patchset, is it 
possible to take it through my tree Thomas ?




-- 
  <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

Re: [PATCH V2 1/7] clockevents: Allow set-state callbacks to be optional

[Thomas Gleixner]

On Mon, 29 Jun 2015, Daniel Lezcano wrote:

> On 06/12/2015 10:00 AM, Viresh Kumar wrote:
> > Its mandatory for the drivers to provide set_state_{oneshot|periodic}()
> > (only if related modes are supported) and set_state_shutdown() callbacks
> > today, if they are implementing the new set-state interface.
> > 
> > But this leads to unnecessary noop callbacks for drivers which don't
> > want to implement them. Over that, it will lead to a full function call
> > for nothing really useful.
> > 
> > Lets make all set-state callbacks optional.
> > 
> > Suggested-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> 
> As this patch is needed for the other changes in this patchset, is it possible
> to take it through my tree Thomas ?

I have not other dependencies in that area ATM, so it can bake in
yours and I'll pull it over then.

Thanks,

	tglx

[PATCH V2 2/7] clocksource: arm_arch_timer: Migrate to new 'set-state' interface

[Viresh Kumar]

Migrate arm_arch_timer driver to the new 'set-state' interface provided
by the clockevents core, the earlier 'set-mode' interface is marked
obsolete now.

This also enables us to implement callbacks for new states of clockevent
devices, for example: ONESHOT_STOPPED.

Cc: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/clocksource/arm_arch_timer.c | 52 +++++++++++++++---------------------
 1 file changed, 22 insertions(+), 30 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 0aa135ddbf80..d6e3e49399dd 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -181,44 +181,36 @@ static irqreturn_t arch_timer_handler_virt_mem(int irq, void *dev_id)
 	return timer_handler(ARCH_TIMER_MEM_VIRT_ACCESS, evt);
 }
 
-static __always_inline void timer_set_mode(const int access, int mode,
-				  struct clock_event_device *clk)
+static __always_inline int timer_shutdown(const int access,
+					  struct clock_event_device *clk)
 {
 	unsigned long ctrl;
-	switch (mode) {
-	case CLOCK_EVT_MODE_UNUSED:
-	case CLOCK_EVT_MODE_SHUTDOWN:
-		ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL, clk);
-		ctrl &= ~ARCH_TIMER_CTRL_ENABLE;
-		arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
-		break;
-	default:
-		break;
-	}
+
+	ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL, clk);
+	ctrl &= ~ARCH_TIMER_CTRL_ENABLE;
+	arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
+
+	return 0;
 }
 
-static void arch_timer_set_mode_virt(enum clock_event_mode mode,
-				     struct clock_event_device *clk)
+static int arch_timer_shutdown_virt(struct clock_event_device *clk)
 {
-	timer_set_mode(ARCH_TIMER_VIRT_ACCESS, mode, clk);
+	return timer_shutdown(ARCH_TIMER_VIRT_ACCESS, clk);
 }
 
-static void arch_timer_set_mode_phys(enum clock_event_mode mode,
-				     struct clock_event_device *clk)
+static int arch_timer_shutdown_phys(struct clock_event_device *clk)
 {
-	timer_set_mode(ARCH_TIMER_PHYS_ACCESS, mode, clk);
+	return timer_shutdown(ARCH_TIMER_PHYS_ACCESS, clk);
 }
 
-static void arch_timer_set_mode_virt_mem(enum clock_event_mode mode,
-					 struct clock_event_device *clk)
+static int arch_timer_shutdown_virt_mem(struct clock_event_device *clk)
 {
-	timer_set_mode(ARCH_TIMER_MEM_VIRT_ACCESS, mode, clk);
+	return timer_shutdown(ARCH_TIMER_MEM_VIRT_ACCESS, clk);
 }
 
-static void arch_timer_set_mode_phys_mem(enum clock_event_mode mode,
-					 struct clock_event_device *clk)
+static int arch_timer_shutdown_phys_mem(struct clock_event_device *clk)
 {
-	timer_set_mode(ARCH_TIMER_MEM_PHYS_ACCESS, mode, clk);
+	return timer_shutdown(ARCH_TIMER_MEM_PHYS_ACCESS, clk);
 }
 
 static __always_inline void set_next_event(const int access, unsigned long evt,
@@ -273,11 +265,11 @@ static void __arch_timer_setup(unsigned type,
 		clk->cpumask = cpumask_of(smp_processor_id());
 		if (arch_timer_use_virtual) {
 			clk->irq = arch_timer_ppi[VIRT_PPI];
-			clk->set_mode = arch_timer_set_mode_virt;
+			clk->set_state_shutdown = arch_timer_shutdown_virt;
 			clk->set_next_event = arch_timer_set_next_event_virt;
 		} else {
 			clk->irq = arch_timer_ppi[PHYS_SECURE_PPI];
-			clk->set_mode = arch_timer_set_mode_phys;
+			clk->set_state_shutdown = arch_timer_shutdown_phys;
 			clk->set_next_event = arch_timer_set_next_event_phys;
 		}
 	} else {
@@ -286,17 +278,17 @@ static void __arch_timer_setup(unsigned type,
 		clk->rating = 400;
 		clk->cpumask = cpu_all_mask;
 		if (arch_timer_mem_use_virtual) {
-			clk->set_mode = arch_timer_set_mode_virt_mem;
+			clk->set_state_shutdown = arch_timer_shutdown_virt_mem;
 			clk->set_next_event =
 				arch_timer_set_next_event_virt_mem;
 		} else {
-			clk->set_mode = arch_timer_set_mode_phys_mem;
+			clk->set_state_shutdown = arch_timer_shutdown_phys_mem;
 			clk->set_next_event =
 				arch_timer_set_next_event_phys_mem;
 		}
 	}
 
-	clk->set_mode(CLOCK_EVT_MODE_SHUTDOWN, clk);
+	clk->set_state_shutdown(clk);
 
 	clockevents_config_and_register(clk, arch_timer_rate, 0xf, 0x7fffffff);
 }
@@ -506,7 +498,7 @@ static void arch_timer_stop(struct clock_event_device *clk)
 			disable_percpu_irq(arch_timer_ppi[PHYS_NONSECURE_PPI]);
 	}
 
-	clk->set_mode(CLOCK_EVT_MODE_UNUSED, clk);
+	clk->set_state_shutdown(clk);
 }
 
 static int arch_timer_cpu_notify(struct notifier_block *self,
-- 
2.4.0

[PATCH V2 3/7] clocksource: arm_global_timer: Migrate to new 'set-state' interface

[Viresh Kumar]

Migrate arm_global_timer driver to the new 'set-state' interface
provided by the clockevents core, the earlier 'set-mode' interface is
marked obsolete now.

This also enables us to implement callbacks for new states of clockevent
devices, for example: ONESHOT_STOPPED.

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Srinivas Kandagatla <srinivas.kandagatla@gmail.com>
Cc: Maxime Coquelin <maxime.coquelin@st.com>
Cc: Patrice Chotard <patrice.chotard@st.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/clocksource/arm_global_timer.c | 37 ++++++++++++++++------------------
 1 file changed, 17 insertions(+), 20 deletions(-)

diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c
index e6833771a716..29ea50ac366a 100644
--- a/drivers/clocksource/arm_global_timer.c
+++ b/drivers/clocksource/arm_global_timer.c
@@ -107,26 +107,21 @@ static void gt_compare_set(unsigned long delta, int periodic)
 	writel(ctrl, gt_base + GT_CONTROL);
 }
 
-static void gt_clockevent_set_mode(enum clock_event_mode mode,
-				   struct clock_event_device *clk)
+static int gt_clockevent_shutdown(struct clock_event_device *evt)
 {
 	unsigned long ctrl;
 
-	switch (mode) {
-	case CLOCK_EVT_MODE_PERIODIC:
-		gt_compare_set(DIV_ROUND_CLOSEST(gt_clk_rate, HZ), 1);
-		break;
-	case CLOCK_EVT_MODE_ONESHOT:
-	case CLOCK_EVT_MODE_UNUSED:
-	case CLOCK_EVT_MODE_SHUTDOWN:
-		ctrl = readl(gt_base + GT_CONTROL);
-		ctrl &= ~(GT_CONTROL_COMP_ENABLE |
-				GT_CONTROL_IRQ_ENABLE | GT_CONTROL_AUTO_INC);
-		writel(ctrl, gt_base + GT_CONTROL);
-		break;
-	default:
-		break;
-	}
+	ctrl = readl(gt_base + GT_CONTROL);
+	ctrl &= ~(GT_CONTROL_COMP_ENABLE | GT_CONTROL_IRQ_ENABLE |
+		  GT_CONTROL_AUTO_INC);
+	writel(ctrl, gt_base + GT_CONTROL);
+	return 0;
+}
+
+static int gt_clockevent_set_periodic(struct clock_event_device *evt)
+{
+	gt_compare_set(DIV_ROUND_CLOSEST(gt_clk_rate, HZ), 1);
+	return 0;
 }
 
 static int gt_clockevent_set_next_event(unsigned long evt,
@@ -155,7 +150,7 @@ static irqreturn_t gt_clockevent_interrupt(int irq, void *dev_id)
 	 *	the Global Timer flag _after_ having incremented
 	 *	the Comparator register	value to a higher value.
 	 */
-	if (evt->mode == CLOCK_EVT_MODE_ONESHOT)
+	if (clockevent_state_oneshot(evt))
 		gt_compare_set(ULONG_MAX, 0);
 
 	writel_relaxed(GT_INT_STATUS_EVENT_FLAG, gt_base + GT_INT_STATUS);
@@ -171,7 +166,9 @@ static int gt_clockevents_init(struct clock_event_device *clk)
 	clk->name = "arm_global_timer";
 	clk->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT |
 		CLOCK_EVT_FEAT_PERCPU;
-	clk->set_mode = gt_clockevent_set_mode;
+	clk->set_state_shutdown = gt_clockevent_shutdown;
+	clk->set_state_periodic = gt_clockevent_set_periodic;
+	clk->set_state_oneshot = gt_clockevent_shutdown;
 	clk->set_next_event = gt_clockevent_set_next_event;
 	clk->cpumask = cpumask_of(cpu);
 	clk->rating = 300;
@@ -184,7 +181,7 @@ static int gt_clockevents_init(struct clock_event_device *clk)
 
 static void gt_clockevents_stop(struct clock_event_device *clk)
 {
-	gt_clockevent_set_mode(CLOCK_EVT_MODE_UNUSED, clk);
+	gt_clockevent_shutdown(clk);
 	disable_percpu_irq(clk->irq);
 }
 
-- 
2.4.0

Re: [PATCH V2 3/7] clocksource: arm_global_timer: Migrate to new 'set-state' interface

[Maxime Coquelin]

Hi Viresh,

On 06/12/2015 10:00 AM, Viresh Kumar wrote:
> Migrate arm_global_timer driver to the new 'set-state' interface
> provided by the clockevents core, the earlier 'set-mode' interface is
> marked obsolete now.
>
> This also enables us to implement callbacks for new states of clockevent
> devices, for example: ONESHOT_STOPPED.
>
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@gmail.com>
> Cc: Maxime Coquelin <maxime.coquelin@st.com>
> Cc: Patrice Chotard <patrice.chotard@st.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>   drivers/clocksource/arm_global_timer.c | 37 ++++++++++++++++------------------
>   1 file changed, 17 insertions(+), 20 deletions(-)
>
>

You can add:

Acked-by: Maxime Coquelin <maxime.coquelin@st.com>

Thanks!
Maxime

Re: [PATCH V2 3/7] clocksource: arm_global_timer: Migrate to new 'set-state' interface

[Viresh Kumar]

On 12-06-15, 10:22, Maxime Coquelin wrote:
> You can add:
> 
> Acked-by: Maxime Coquelin <maxime.coquelin@st.com>

Thanks Maxime.

-- 
viresh

Re: [PATCH V2 3/7] clocksource: arm_global_timer: Migrate to new 'set-state' interface

[srini]

On 12/06/15 09:00, Viresh Kumar wrote:
> Migrate arm_global_timer driver to the new 'set-state' interface
> provided by the clockevents core, the earlier 'set-mode' interface is
> marked obsolete now.
>
> This also enables us to implement callbacks for new states of clockevent
> devices, for example: ONESHOT_STOPPED.
>
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@gmail.com>
> Cc: Maxime Coquelin <maxime.coquelin@st.com>
> Cc: Patrice Chotard <patrice.chotard@st.com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>   drivers/clocksource/arm_global_timer.c | 37 ++++++++++++++++------------------
>   1 file changed, 17 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/clocksource/arm_global_timer.c b/drivers/clocksource/arm_global_timer.c
> index e6833771a716..29ea50ac366a 100644
> --- a/drivers/clocksource/arm_global_timer.c
> +++ b/drivers/clocksource/arm_global_timer.c
>
>   
Hi Viresh,
Thanks for the patch,
Should have been careful with the my last Ack :-)

Acked-by: Srinivas Kandagatla<srinivas.kandagatla@linaro.org 
<mailto:srinivas.kandagatla@linaro.org>>

--srini

Re: [PATCH V2 3/7] clocksource: arm_global_timer: Migrate to new 'set-state' interface

[Viresh Kumar]

On 12-06-15, 10:10, srini wrote:
> Should have been careful with the my last Ack :-)

That's fine.

> Acked-by: Srinivas Kandagatla<srinivas.kandagatla@linaro.org

Thanks.

-- 
viresh

[PATCH V2 4/7] clocksource: bcm2835: Migrate to new 'set-state' interface

[Viresh Kumar]

Migrate bcm2835 driver to the new 'set-state' interface provided by
the clockevents core, the earlier 'set-mode' interface is marked
obsolete now.

This also enables us to implement callbacks for new states of clockevent
devices, for example: ONESHOT_STOPPED.

We weren't doing anything in the ->set_mode() callback. So, this patch
doesn't provide any set-state callbacks.

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Lee Jones <lee@kernel.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/clocksource/bcm2835_timer.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/clocksource/bcm2835_timer.c b/drivers/clocksource/bcm2835_timer.c
index 26ed331b1aad..6f2822928963 100644
--- a/drivers/clocksource/bcm2835_timer.c
+++ b/drivers/clocksource/bcm2835_timer.c
@@ -54,21 +54,6 @@ static u64 notrace bcm2835_sched_read(void)
 	return readl_relaxed(system_clock);
 }
 
-static void bcm2835_time_set_mode(enum clock_event_mode mode,
-	struct clock_event_device *evt_dev)
-{
-	switch (mode) {
-	case CLOCK_EVT_MODE_ONESHOT:
-	case CLOCK_EVT_MODE_UNUSED:
-	case CLOCK_EVT_MODE_SHUTDOWN:
-	case CLOCK_EVT_MODE_RESUME:
-		break;
-	default:
-		WARN(1, "%s: unhandled event mode %d\n", __func__, mode);
-		break;
-	}
-}
-
 static int bcm2835_time_set_next_event(unsigned long event,
 	struct clock_event_device *evt_dev)
 {
@@ -129,7 +114,6 @@ static void __init bcm2835_timer_init(struct device_node *node)
 	timer->evt.name = node->name;
 	timer->evt.rating = 300;
 	timer->evt.features = CLOCK_EVT_FEAT_ONESHOT;
-	timer->evt.set_mode = bcm2835_time_set_mode;
 	timer->evt.set_next_event = bcm2835_time_set_next_event;
 	timer->evt.cpumask = cpumask_of(0);
 	timer->act.name = node->name;
-- 
2.4.0

Re: [PATCH V2 4/7] clocksource: bcm2835: Migrate to new 'set-state' interface

[Stephen Warren]

On 06/12/2015 02:00 AM, Viresh Kumar wrote:
> Migrate bcm2835 driver to the new 'set-state' interface provided by
> the clockevents core, the earlier 'set-mode' interface is marked
> obsolete now.
> 
> This also enables us to implement callbacks for new states of clockevent
> devices, for example: ONESHOT_STOPPED.
> 
> We weren't doing anything in the ->set_mode() callback. So, this patch
> doesn't provide any set-state callbacks.

This generates a panic at boot (on top of 4.1.0-rc8+, which certainly at
least booted fine):

> [    0.008586] clocksource timer: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 1911260446275 ns
> [    0.018080] ------------[ cut here ]------------
> [    0.022843] kernel BUG at kernel/time/clockevents.c:480!
> [    0.028299] Internal error: Oops - BUG: 0 [#1] ARM
> [    0.033237] CPU: 0 PID: 0 Comm: swapper Not tainted 4.1.0-rc8+ #46
> [    0.039567] Hardware name: BCM2835
> [    0.043092] task: c06fb648 ti: c06f6000 task.ti: c06f6000
> [    0.048668] PC is at clockevents_register_device+0x15c/0x174
> [    0.054481] LR is at clockevents_config_and_register+0x2c/0x30
> [    0.060467] pc : [<c00665e0>]    lr : [<c00666b4>]    psr: 600001d3
> [    0.060467] sp : c06f7f18  ip : c0066498  fp : c06f7f34
> [    0.072243] r10: cdffc660  r9 : 410fb767  r8 : c06db128
> [    0.077611] r7 : 0000001b  r6 : f0003018  r5 : cde38ed4  r4 : cd422020
> [    0.084294] r3 : 00000002  r2 : 00000000  r1 : 000003e7  r0 : cd422020
> [    0.090979] Flags: nZCv  IRQs off  FIQs off  Mode SVC_32  ISA ARM  Segment kernel
> [    0.098627] Control: 00c5387d  Table: 00004008  DAC: 00000015
> [    0.104521] Process swapper (pid: 0, stack limit = 0xc06f6188)
> [    0.110502] Stack: (0xc06f7f18 to 0xc06f8000)
> [    0.114993] 7f00:                                                       000003e7 cd422020
> [    0.123354] 7f20: cde38ed4 f0003018 c06f7f4c c06f7f38 c00666b4 c0066490 ffffffff cd422000
> [    0.131715] 7f40: c06f7f7c c06f7f50 c06c3358 c0066694 00000020 c03429ac c06f7f7c 000f4240
> [    0.140076] 7f60: cde38ed4 00000001 c06f7f84 ffffffff c06f7fa4 c06f7f80 c06c3124 c06c321c
> [    0.148436] 7f80: 00000001 c06f4ea0 ffffffff 00000000 00000001 c075aea0 c06f7fb4 c06f7fa8
> [    0.156798] 7fa0: c06988e4 c06c30dc c06f7ff4 c06f7fb8 c0695b90 c06988c0 ffffffff ffffffff
> [    0.165159] 7fc0: c06956e4 00000000 00000000 c06db128 c075b014 c06f8024 c06db124 c06fc910
> [    0.173518] 7fe0: 00004008 006d9fcc 00000000 c06f7ff8 00008074 c069597c 00000000 00000000
> [    0.181910] [<c00665e0>] (clockevents_register_device) from [<c00666b4>] (clockevents_config_and_register+0x2c/0x30)
> [    0.192661] [<c00666b4>] (clockevents_config_and_register) from [<c06c3358>] (bcm2835_timer_init+0x148/0x19c)
> [    0.202785] [<c06c3358>] (bcm2835_timer_init) from [<c06c3124>] (clocksource_of_init+0x54/0x98)
> [    0.211693] [<c06c3124>] (clocksource_of_init) from [<c06988e4>] (time_init+0x30/0x38)
> [    0.219798] [<c06988e4>] (time_init) from [<c0695b90>] (start_kernel+0x220/0x3a4)
> [    0.227459] [<c0695b90>] (start_kernel) from [<00008074>] (0x8074)
> [    0.233803] Code: e5943078 e3530000 1affffd5 eaffffd2 (e7f001f2) 
> [    0.240103] ---[ end trace cb88537fdc8fa200 ]---
> [    0.244866] Kernel panic - not syncing: Attempted to kill the idle task!
> [    0.251730] ---[ end Kernel panic - not syncing: Attempted to kill the idle task!

Re: [PATCH V2 4/7] clocksource: bcm2835: Migrate to new 'set-state' interface

[Viresh Kumar]

On 15-06-15, 20:57, Stephen Warren wrote:
> On 06/12/2015 02:00 AM, Viresh Kumar wrote:
> > Migrate bcm2835 driver to the new 'set-state' interface provided by
> > the clockevents core, the earlier 'set-mode' interface is marked
> > obsolete now.
> > 
> > This also enables us to implement callbacks for new states of clockevent
> > devices, for example: ONESHOT_STOPPED.
> > 
> > We weren't doing anything in the ->set_mode() callback. So, this patch
> > doesn't provide any set-state callbacks.
> 
> This generates a panic at boot (on top of 4.1.0-rc8+, which certainly at
> least booted fine):
> 
> > [    0.008586] clocksource timer: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 1911260446275 ns
> > [    0.018080] ------------[ cut here ]------------
> > [    0.022843] kernel BUG at kernel/time/clockevents.c:480!
> > [    0.028299] Internal error: Oops - BUG: 0 [#1] ARM
> > [    0.033237] CPU: 0 PID: 0 Comm: swapper Not tainted 4.1.0-rc8+ #46
> > [    0.039567] Hardware name: BCM2835
> > [    0.043092] task: c06fb648 ti: c06f6000 task.ti: c06f6000
> > [    0.048668] PC is at clockevents_register_device+0x15c/0x174

This failed the sanity checks of clockevents core. Did you apply the
first patch as well? Yes, its very much required.

Also, there were dependencies on the latest tip, prepared for 4.2
merge window and would have been better if you tested on top of that.

But those dependencies are for some helpers which aren't used in this
patch. So, it might work over rc8 + the first patch from this series..

In case it doesn't, please test it over tip/master once.

-- 
viresh

Re: [PATCH V2 4/7] clocksource: bcm2835: Migrate to new 'set-state' interface

[Stephen Warren]

On 06/15/2015 09:17 PM, Viresh Kumar wrote:
> On 15-06-15, 20:57, Stephen Warren wrote:
>> On 06/12/2015 02:00 AM, Viresh Kumar wrote:
>>> Migrate bcm2835 driver to the new 'set-state' interface provided by
>>> the clockevents core, the earlier 'set-mode' interface is marked
>>> obsolete now.
>>>
>>> This also enables us to implement callbacks for new states of clockevent
>>> devices, for example: ONESHOT_STOPPED.
>>>
>>> We weren't doing anything in the ->set_mode() callback. So, this patch
>>> doesn't provide any set-state callbacks.
>>
>> This generates a panic at boot (on top of 4.1.0-rc8+, which certainly at
>> least booted fine):
>>
>>> [    0.008586] clocksource timer: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 1911260446275 ns
>>> [    0.018080] ------------[ cut here ]------------
>>> [    0.022843] kernel BUG at kernel/time/clockevents.c:480!
>>> [    0.028299] Internal error: Oops - BUG: 0 [#1] ARM
>>> [    0.033237] CPU: 0 PID: 0 Comm: swapper Not tainted 4.1.0-rc8+ #46
>>> [    0.039567] Hardware name: BCM2835
>>> [    0.043092] task: c06fb648 ti: c06f6000 task.ti: c06f6000
>>> [    0.048668] PC is at clockevents_register_device+0x15c/0x174
> 
> This failed the sanity checks of clockevents core. Did you apply the
> first patch as well? Yes, its very much required.
> 
> Also, there were dependencies on the latest tip, prepared for 4.2
> merge window and would have been better if you tested on top of that.
> 
> But those dependencies are for some helpers which aren't used in this
> patch. So, it might work over rc8 + the first patch from this series..
> 
> In case it doesn't, please test it over tip/master once.

I see. You didn't Cc me on patch 1, and didn't mention the dependency in
this patch. That usually means they're all independent, e.g. the same
change in n different drivers.

Anyway, I tracked down the whole series and applied it on top of
next-20150615 and everything seems OK (kernel boots, and UART, USB kbd &
SD card work), so this patch,
Tested-by: Stephen Warren <swarren@wwwdotorg.org>

Re: [PATCH V2 4/7] clocksource: bcm2835: Migrate to new 'set-state' interface

[Viresh Kumar]

On 15-06-15, 22:16, Stephen Warren wrote:
> I see. You didn't Cc me on patch 1, and didn't mention the dependency in
> this patch. That usually means they're all independent, e.g. the same
> change in n different drivers.

Yeah, I cc'd you on the cover-letter and missed it on the first patch.
My fault. Actually during V1 the first patch wasn't there and so there
was no dependency, but in V2 this patch came in and I completely
forgot you and other guys for that patch.

> Anyway, I tracked down the whole series and applied it on top of
> next-20150615 and everything seems OK (kernel boots, and UART, USB kbd &
> SD card work), so this patch,
> Tested-by: Stephen Warren <swarren@wwwdotorg.org>

Thanks a lot Stephen.

-- 
viresh

[PATCH V2 5/7] clocksource: bcm_kona: Migrate to new 'set-state' interface

[Viresh Kumar]

Migrate bcm_kona driver to the new 'set-state' interface provided by
the clockevents core, the earlier 'set-mode' interface is marked
obsolete now.

This also enables us to implement callbacks for new states of clockevent
devices, for example: ONESHOT_STOPPED.

Oneshot callback isn't required as it was empty.

Acked-by: Ray Jui <rjui@broadcom.com>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: Ray Jui <rjui@broadcom.com>
Cc: Scott Branden <sbranden@broadcom.com>
Cc: bcm-kernel-feedback-list@broadcom.com
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/clocksource/bcm_kona_timer.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/clocksource/bcm_kona_timer.c b/drivers/clocksource/bcm_kona_timer.c
index f1e33d08dd83..e717e87df9bc 100644
--- a/drivers/clocksource/bcm_kona_timer.c
+++ b/drivers/clocksource/bcm_kona_timer.c
@@ -127,25 +127,18 @@ static int kona_timer_set_next_event(unsigned long clc,
 	return 0;
 }
 
-static void kona_timer_set_mode(enum clock_event_mode mode,
-			     struct clock_event_device *unused)
+static int kona_timer_shutdown(struct clock_event_device *evt)
 {
-	switch (mode) {
-	case CLOCK_EVT_MODE_ONESHOT:
-		/* by default mode is one shot don't do any thing */
-		break;
-	case CLOCK_EVT_MODE_UNUSED:
-	case CLOCK_EVT_MODE_SHUTDOWN:
-	default:
-		kona_timer_disable_and_clear(timers.tmr_regs);
-	}
+	kona_timer_disable_and_clear(timers.tmr_regs);
+	return 0;
 }
 
 static struct clock_event_device kona_clockevent_timer = {
 	.name = "timer 1",
 	.features = CLOCK_EVT_FEAT_ONESHOT,
 	.set_next_event = kona_timer_set_next_event,
-	.set_mode = kona_timer_set_mode
+	.set_state_shutdown = kona_timer_shutdown,
+	.tick_resume = kona_timer_shutdown,
 };
 
 static void __init kona_timer_clockevents_init(void)
-- 
2.4.0

[PATCH V2 6/7] clocksource: cs5535: Migrate to new 'set-state' interface

[Viresh Kumar]

Migrate cs5535 driver to the new 'set-state' interface provided by
the clockevents core, the earlier 'set-mode' interface is marked
obsolete now.

This also enables us to implement callbacks for new states of clockevent
devices, for example: ONESHOT_STOPPED.

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Andres Salomon <dilinger@queued.net>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/clocksource/cs5535-clockevt.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/clocksource/cs5535-clockevt.c b/drivers/clocksource/cs5535-clockevt.c
index db2105290898..9a7e37cf56b0 100644
--- a/drivers/clocksource/cs5535-clockevt.c
+++ b/drivers/clocksource/cs5535-clockevt.c
@@ -42,7 +42,6 @@ MODULE_PARM_DESC(irq, "Which IRQ to use for the clock source MFGPT ticks.");
  *  256         128   .125            512.000
  */
 
-static unsigned int cs5535_tick_mode = CLOCK_EVT_MODE_SHUTDOWN;
 static struct cs5535_mfgpt_timer *cs5535_event_clock;
 
 /* Selected from the table above */
@@ -77,15 +76,17 @@ static void start_timer(struct cs5535_mfgpt_timer *timer, uint16_t delta)
 			MFGPT_SETUP_CNTEN | MFGPT_SETUP_CMP2);
 }
 
-static void mfgpt_set_mode(enum clock_event_mode mode,
-		struct clock_event_device *evt)
+static int mfgpt_shutdown(struct clock_event_device *evt)
 {
 	disable_timer(cs5535_event_clock);
+	return 0;
+}
 
-	if (mode == CLOCK_EVT_MODE_PERIODIC)
-		start_timer(cs5535_event_clock, MFGPT_PERIODIC);
-
-	cs5535_tick_mode = mode;
+static int mfgpt_set_periodic(struct clock_event_device *evt)
+{
+	disable_timer(cs5535_event_clock);
+	start_timer(cs5535_event_clock, MFGPT_PERIODIC);
+	return 0;
 }
 
 static int mfgpt_next_event(unsigned long delta, struct clock_event_device *evt)
@@ -97,7 +98,10 @@ static int mfgpt_next_event(unsigned long delta, struct clock_event_device *evt)
 static struct clock_event_device cs5535_clockevent = {
 	.name = DRV_NAME,
 	.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
-	.set_mode = mfgpt_set_mode,
+	.set_state_shutdown = mfgpt_shutdown,
+	.set_state_periodic = mfgpt_set_periodic,
+	.set_state_oneshot = mfgpt_shutdown,
+	.tick_resume = mfgpt_shutdown,
 	.set_next_event = mfgpt_next_event,
 	.rating = 250,
 };
@@ -113,7 +117,7 @@ static irqreturn_t mfgpt_tick(int irq, void *dev_id)
 	/* Turn off the clock (and clear the event) */
 	disable_timer(cs5535_event_clock);
 
-	if (cs5535_tick_mode == CLOCK_EVT_MODE_SHUTDOWN)
+	if (clockevent_state_shutdown(&cs5535_clockevent))
 		return IRQ_HANDLED;
 
 	/* Clear the counter */
@@ -121,7 +125,7 @@ static irqreturn_t mfgpt_tick(int irq, void *dev_id)
 
 	/* Restart the clock in periodic mode */
 
-	if (cs5535_tick_mode == CLOCK_EVT_MODE_PERIODIC)
+	if (clockevent_state_periodic(&cs5535_clockevent))
 		cs5535_mfgpt_write(cs5535_event_clock, MFGPT_REG_SETUP,
 				MFGPT_SETUP_CNTEN | MFGPT_SETUP_CMP2);
 
-- 
2.4.0

[PATCH V2 7/7] clocksource: em_sti: Migrate to new 'set-state' interface

[Viresh Kumar]

Migrate em_sti driver to the new 'set-state' interface provided by
the clockevents core, the earlier 'set-mode' interface is marked
obsolete now.

This also enables us to implement callbacks for new states of clockevent
devices, for example: ONESHOT_STOPPED.

NOTE: This also drops a special check:

	if (old_mode == CLOCK_EVT_MODE_ONESHOT)
		em_sti_stop(p, USER_CLOCKEVENT);

as it doesn't look like that important. This driver only supports
ONESHOT and we can only move only to SHUTDOWN from ONESHOT and.
Also on second call (on shutdown), em_sti_stop() would return without
disabling the device again.

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/clocksource/em_sti.c | 39 ++++++++++++++-------------------------
 1 file changed, 14 insertions(+), 25 deletions(-)

diff --git a/drivers/clocksource/em_sti.c b/drivers/clocksource/em_sti.c
index dc3c6ee04aaa..7a97a34dba70 100644
--- a/drivers/clocksource/em_sti.c
+++ b/drivers/clocksource/em_sti.c
@@ -251,33 +251,21 @@ static struct em_sti_priv *ced_to_em_sti(struct clock_event_device *ced)
 	return container_of(ced, struct em_sti_priv, ced);
 }
 
-static void em_sti_clock_event_mode(enum clock_event_mode mode,
-				    struct clock_event_device *ced)
+static int em_sti_clock_event_shutdown(struct clock_event_device *ced)
 {
 	struct em_sti_priv *p = ced_to_em_sti(ced);
+	em_sti_stop(p, USER_CLOCKEVENT);
+	return 0;
+}
 
-	/* deal with old setting first */
-	switch (ced->mode) {
-	case CLOCK_EVT_MODE_ONESHOT:
-		em_sti_stop(p, USER_CLOCKEVENT);
-		break;
-	default:
-		break;
-	}
+static int em_sti_clock_event_set_oneshot(struct clock_event_device *ced)
+{
+	struct em_sti_priv *p = ced_to_em_sti(ced);
 
-	switch (mode) {
-	case CLOCK_EVT_MODE_ONESHOT:
-		dev_info(&p->pdev->dev, "used for oneshot clock events\n");
-		em_sti_start(p, USER_CLOCKEVENT);
-		clockevents_config(&p->ced, p->rate);
-		break;
-	case CLOCK_EVT_MODE_SHUTDOWN:
-	case CLOCK_EVT_MODE_UNUSED:
-		em_sti_stop(p, USER_CLOCKEVENT);
-		break;
-	default:
-		break;
-	}
+	dev_info(&p->pdev->dev, "used for oneshot clock events\n");
+	em_sti_start(p, USER_CLOCKEVENT);
+	clockevents_config(&p->ced, p->rate);
+	return 0;
 }
 
 static int em_sti_clock_event_next(unsigned long delta,
@@ -303,11 +291,12 @@ static void em_sti_register_clockevent(struct em_sti_priv *p)
 	ced->rating = 200;
 	ced->cpumask = cpu_possible_mask;
 	ced->set_next_event = em_sti_clock_event_next;
-	ced->set_mode = em_sti_clock_event_mode;
+	ced->set_state_shutdown = em_sti_clock_event_shutdown;
+	ced->set_state_oneshot = em_sti_clock_event_set_oneshot;
 
 	dev_info(&p->pdev->dev, "used for clock events\n");
 
-	/* Register with dummy 1 Hz value, gets updated in ->set_mode() */
+	/* Register with dummy 1 Hz value, gets updated in ->set_state_oneshot() */
 	clockevents_config_and_register(ced, 1, 2, 0xffffffff);
 }
 
-- 
2.4.0

Re: [PATCH V2 0/7] clockevent: Migrate to new 'set-state' interface

[Daniel Lezcano]

On 06/12/2015 10:00 AM, Viresh Kumar wrote:
> Hi Thomas/Daniel,
>
> I have incorporated all the improvements Daniel suggested on V1 and so
> here is V2.
>
> The first patch allows set-state callbacks to be optional, otherwise it
> leads to unnecessary noop callbacks for drivers which don't want to
> implement them. Over that, these noop-callbacks result in full function
> calls for nothing really useful.
>
> Rest of the series converts few clockevent drivers to the new set-state
> interface. This would enable these drivers to use new states (like:
> ONESHOT_STOPPED, etc.) of a clockevent device (if required), as the
> set-mode interface is marked obsolete now and wouldn't be expanded to
> handle new states.
>
> Once all the drivers are migrated to the new interface in future, we can
> remove the code supporting '->mode' in clockevents core.
>
> Drivers converted in this series are selected based on the diff they
> generate. These are different diffs we shall have for most of the
> drivers and any suggestions/improvements for these patches will be
> applied to other drivers as well.
>
> This is based of latest tip/master from few days back due to dependency
> on clockevent_state_*() helpers.

Applied to my tree for 3.5

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

Re: [PATCH V2 0/7] clockevent: Migrate to new 'set-state' interface

[Daniel Lezcano]

On 06/18/2015 11:10 AM, Daniel Lezcano wrote:
> On 06/12/2015 10:00 AM, Viresh Kumar wrote:
>> Hi Thomas/Daniel,
>>
>> I have incorporated all the improvements Daniel suggested on V1 and so
>> here is V2.
>>
>> The first patch allows set-state callbacks to be optional, otherwise it
>> leads to unnecessary noop callbacks for drivers which don't want to
>> implement them. Over that, these noop-callbacks result in full function
>> calls for nothing really useful.
>>
>> Rest of the series converts few clockevent drivers to the new set-state
>> interface. This would enable these drivers to use new states (like:
>> ONESHOT_STOPPED, etc.) of a clockevent device (if required), as the
>> set-mode interface is marked obsolete now and wouldn't be expanded to
>> handle new states.
>>
>> Once all the drivers are migrated to the new interface in future, we can
>> remove the code supporting '->mode' in clockevents core.
>>
>> Drivers converted in this series are selected based on the diff they
>> generate. These are different diffs we shall have for most of the
>> drivers and any suggestions/improvements for these patches will be
>> applied to other drivers as well.
>>
>> This is based of latest tip/master from few days back due to dependency
>> on clockevent_state_*() helpers.
>
> Applied to my tree for 3.5

Pfff, 4.3

-- 
  <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