2015-06-12 08:00:35

by Viresh Kumar

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

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 <[email protected]>
Cc: [email protected]
Cc: Florian Fainelli <[email protected]>
Cc: Lee Jones <[email protected]>
Cc: Magnus Damm <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Maxime Coquelin <[email protected]>
Cc: Patrice Chotard <[email protected]>
Cc: Ray Jui <[email protected]>
Cc: Scott Branden <[email protected]>
Cc: Srinivas Kandagatla <[email protected]>
Cc: Stephen Warren <[email protected]>

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


2015-06-12 08:00:41

by Viresh Kumar

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

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 <[email protected]>
Signed-off-by: Viresh Kumar <[email protected]>
---
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

2015-06-12 08:03:09

by Viresh Kumar

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

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 <[email protected]>
Signed-off-by: Viresh Kumar <[email protected]>
---
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

2015-06-12 08:00:48

by Viresh Kumar

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

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 <[email protected]>
Cc: Srinivas Kandagatla <[email protected]>
Cc: Maxime Coquelin <[email protected]>
Cc: Patrice Chotard <[email protected]>
Signed-off-by: Viresh Kumar <[email protected]>
---
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

2015-06-12 08:00:51

by Viresh Kumar

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

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 <[email protected]>
Cc: Stephen Warren <[email protected]>
Cc: Lee Jones <[email protected]>
Signed-off-by: Viresh Kumar <[email protected]>
---
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

2015-06-12 08:01:09

by Viresh Kumar

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

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 <[email protected]>
Cc: Florian Fainelli <[email protected]>
Cc: Ray Jui <[email protected]>
Cc: Scott Branden <[email protected]>
Cc: [email protected]
Signed-off-by: Viresh Kumar <[email protected]>
---
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

2015-06-12 08:01:04

by Viresh Kumar

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

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 <[email protected]>
Cc: Andres Salomon <[email protected]>
Signed-off-by: Viresh Kumar <[email protected]>
---
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

2015-06-12 08:01:11

by Viresh Kumar

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

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 <[email protected]>
Cc: Magnus Damm <[email protected]>
Signed-off-by: Viresh Kumar <[email protected]>
---
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

2015-06-12 08:22:50

by Maxime Coquelin

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

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 <[email protected]>
> Cc: Srinivas Kandagatla <[email protected]>
> Cc: Maxime Coquelin <[email protected]>
> Cc: Patrice Chotard <[email protected]>
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
> drivers/clocksource/arm_global_timer.c | 37 ++++++++++++++++------------------
> 1 file changed, 17 insertions(+), 20 deletions(-)
>
>

You can add:

Acked-by: Maxime Coquelin <[email protected]>

Thanks!
Maxime

2015-06-12 08:29:02

by Viresh Kumar

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

On 12-06-15, 10:22, Maxime Coquelin wrote:
> You can add:
>
> Acked-by: Maxime Coquelin <[email protected]>

Thanks Maxime.

--
viresh

2015-06-12 09:10:52

by srini

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


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 <[email protected]>
> Cc: Srinivas Kandagatla <[email protected]>
> Cc: Maxime Coquelin <[email protected]>
> Cc: Patrice Chotard <[email protected]>
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
> 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<[email protected]
<mailto:[email protected]>>

--srini

2015-06-12 09:11:56

by Viresh Kumar

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

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

That's fine.

> Acked-by: Srinivas Kandagatla<[email protected]

Thanks.

--
viresh

2015-06-16 02:57:26

by Stephen Warren

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

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!

2015-06-16 03:18:10

by Viresh Kumar

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

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

2015-06-16 04:16:35

by Stephen Warren

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

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 <[email protected]>

2015-06-16 04:19:20

by Viresh Kumar

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

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 <[email protected]>

Thanks a lot Stephen.

--
viresh

2015-06-18 09:10:45

by Daniel Lezcano

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

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

2015-06-18 09:14:44

by Daniel Lezcano

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

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

2015-06-29 08:47:32

by Daniel Lezcano

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

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 <[email protected]>
> Signed-off-by: Viresh Kumar <[email protected]>

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

2015-06-29 09:15:37

by Thomas Gleixner

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

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 <[email protected]>
> > Signed-off-by: Viresh Kumar <[email protected]>
>
> 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