2015-11-09 10:07:17

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 01/15] clocksource/drivers/h8300_timer8: Fix compilation error with dev_warn

The dev_warn is using the platform driver which was removed in the previous
patch.

Let's replace dev_warn by pr_warn.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/clocksource/h8300_timer8.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/h8300_timer8.c b/drivers/clocksource/h8300_timer8.c
index f0680eb..35b0e8f 100644
--- a/drivers/clocksource/h8300_timer8.c
+++ b/drivers/clocksource/h8300_timer8.c
@@ -98,7 +98,7 @@ static void timer8_set_next(struct timer8_priv *p, unsigned long delta)

raw_spin_lock_irqsave(&p->lock, flags);
if (delta >= 0x10000)
- dev_warn(&p->pdev->dev, "delta out of range\n");
+ pr_warn("delta out of range\n");
now = timer8_get_counter(p);
p->tcora = delta;
ctrl_outb(ctrl_inb(p->mapbase + _8TCR) | 0x40, p->mapbase + _8TCR);
--
1.9.1


2015-11-09 10:07:20

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 02/15] clocksource/drivers/h8300_tpu: Remove unused macros

Some macros are unused, delete them.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/clocksource/h8300_tpu.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/clocksource/h8300_tpu.c b/drivers/clocksource/h8300_tpu.c
index ed0b493..576dae6 100644
--- a/drivers/clocksource/h8300_tpu.c
+++ b/drivers/clocksource/h8300_tpu.c
@@ -20,16 +20,9 @@
#include <linux/of_address.h>
#include <linux/of_irq.h>

-#define TCR 0
-#define TMDR 1
-#define TIOR 2
-#define TER 4
-#define TSR 5
-#define TCNT 6
-#define TGRA 8
-#define TGRB 10
-#define TGRC 12
-#define TGRD 14
+#define TCR 0x0
+#define TSR 0x5
+#define TCNT 0x6

struct tpu_priv {
struct clocksource cs;
--
1.9.1

2015-11-09 10:13:30

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 03/15] clocksource/drivers/h8300_tpu: Remove pointless headers for TPU

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/clocksource/h8300_tpu.c | 5 -----
1 file changed, 5 deletions(-)

diff --git a/drivers/clocksource/h8300_tpu.c b/drivers/clocksource/h8300_tpu.c
index 576dae6..c1eef42 100644
--- a/drivers/clocksource/h8300_tpu.c
+++ b/drivers/clocksource/h8300_tpu.c
@@ -6,14 +6,9 @@
*/

#include <linux/errno.h>
-#include <linux/sched.h>
#include <linux/kernel.h>
-#include <linux/interrupt.h>
#include <linux/init.h>
-#include <linux/platform_device.h>
-#include <linux/slab.h>
#include <linux/clocksource.h>
-#include <linux/module.h>
#include <linux/clk.h>
#include <linux/io.h>
#include <linux/of.h>
--
1.9.1

2015-11-09 10:13:08

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 04/15] clocksource/drivers/h8300_timer8: Remove unused headers

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/clocksource/h8300_timer8.c | 5 -----
1 file changed, 5 deletions(-)

diff --git a/drivers/clocksource/h8300_timer8.c b/drivers/clocksource/h8300_timer8.c
index 35b0e8f..62a7f8c 100644
--- a/drivers/clocksource/h8300_timer8.c
+++ b/drivers/clocksource/h8300_timer8.c
@@ -8,21 +8,16 @@
*/

#include <linux/errno.h>
-#include <linux/sched.h>
#include <linux/kernel.h>
#include <linux/interrupt.h>
#include <linux/init.h>
-#include <linux/slab.h>
#include <linux/clockchips.h>
-#include <linux/module.h>
#include <linux/clk.h>
#include <linux/io.h>
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_irq.h>

-#include <asm/irq.h>
-
#define _8TCR 0
#define _8TCSR 2
#define TCORA 4
--
1.9.1

2015-11-09 10:12:50

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 05/15] clocksource/drivers/h8300_timer8: Remove unused macros

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/clocksource/h8300_timer8.c | 4 ----
1 file changed, 4 deletions(-)

diff --git a/drivers/clocksource/h8300_timer8.c b/drivers/clocksource/h8300_timer8.c
index 62a7f8c..88b9b06 100644
--- a/drivers/clocksource/h8300_timer8.c
+++ b/drivers/clocksource/h8300_timer8.c
@@ -24,7 +24,6 @@
#define TCORB 6
#define _8TCNT 8

-#define FLAG_REPROGRAM (1 << 0)
#define FLAG_SKIPEVENT (1 << 1)
#define FLAG_IRQCONTEXT (1 << 2)
#define FLAG_STARTED (1 << 3)
@@ -32,9 +31,6 @@
#define ONESHOT 0
#define PERIODIC 1

-#define RELATIVE 0
-#define ABSOLUTE 1
-
#define SCALE 64

struct timer8_priv {
--
1.9.1

2015-11-09 10:07:27

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 06/15] clocksource/drivers/h8300_timer8: Remove PERIODIC and ONESHOT macro

Specify the delta as parameter for the timer8_clock_event_start function
instead of using a macro to tell PERIODIC or ONESHOT.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/clocksource/h8300_timer8.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/clocksource/h8300_timer8.c b/drivers/clocksource/h8300_timer8.c
index 88b9b06..2433325 100644
--- a/drivers/clocksource/h8300_timer8.c
+++ b/drivers/clocksource/h8300_timer8.c
@@ -28,9 +28,6 @@
#define FLAG_IRQCONTEXT (1 << 2)
#define FLAG_STARTED (1 << 3)

-#define ONESHOT 0
-#define PERIODIC 1
-
#define SCALE 64

struct timer8_priv {
@@ -147,7 +144,7 @@ static inline struct timer8_priv *ced_to_priv(struct clock_event_device *ced)
return container_of(ced, struct timer8_priv, ced);
}

-static void timer8_clock_event_start(struct timer8_priv *p, int periodic)
+static void timer8_clock_event_start(struct timer8_priv *p, unsigned long delta)
{
struct clock_event_device *ced = &p->ced;

@@ -158,7 +155,7 @@ static void timer8_clock_event_start(struct timer8_priv *p, int periodic)
ced->max_delta_ns = clockevent_delta2ns(0xffff, ced);
ced->min_delta_ns = clockevent_delta2ns(0x0001, ced);

- timer8_set_next(p, periodic?(p->rate + HZ/2) / HZ:0x10000);
+ timer8_set_next(p, delta);
}

static int timer8_clock_event_shutdown(struct clock_event_device *ced)
@@ -173,7 +170,7 @@ static int timer8_clock_event_periodic(struct clock_event_device *ced)

pr_info("%s: used for periodic clock events\n", ced->name);
timer8_stop(p);
- timer8_clock_event_start(p, PERIODIC);
+ timer8_clock_event_start(p, (p->rate + HZ/2) / HZ);

return 0;
}
@@ -184,7 +181,7 @@ static int timer8_clock_event_oneshot(struct clock_event_device *ced)

pr_info("%s: used for oneshot clock events\n", ced->name);
timer8_stop(p);
- timer8_clock_event_start(p, ONESHOT);
+ timer8_clock_event_start(p, 0x10000);

return 0;
}
--
1.9.1

2015-11-09 10:07:31

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 07/15] clocksource/drivers/h8300_timer8: Fix irq return value check

The value returned in case of error for the 'irq_of_parse_and_map' function is
zero in case of error. Fix the check in the init code.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/clocksource/h8300_timer8.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/h8300_timer8.c b/drivers/clocksource/h8300_timer8.c
index 2433325..3eedeff 100644
--- a/drivers/clocksource/h8300_timer8.c
+++ b/drivers/clocksource/h8300_timer8.c
@@ -230,7 +230,7 @@ static void __init h8300_8timer_init(struct device_node *node)
}

irq = irq_of_parse_and_map(node, 0);
- if (irq < 0) {
+ if (!irq) {
pr_err("failed to get irq for clockevent\n");
goto unmap_reg;
}
--
1.9.1

2015-11-09 10:09:40

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 08/15] clocksource/drivers/h8300_timer8: Remove pointless irq re-entrant safe code

The current code assumes the interrupt function is re-entrant.

That is not correct. An interrupt handler is never invoked concurrently. The
interrupt line is masked on all processors.

Remove the chewing flags in the code.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/clocksource/h8300_timer8.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/clocksource/h8300_timer8.c b/drivers/clocksource/h8300_timer8.c
index 3eedeff..7111b99 100644
--- a/drivers/clocksource/h8300_timer8.c
+++ b/drivers/clocksource/h8300_timer8.c
@@ -24,8 +24,6 @@
#define TCORB 6
#define _8TCNT 8

-#define FLAG_SKIPEVENT (1 << 1)
-#define FLAG_IRQCONTEXT (1 << 2)
#define FLAG_STARTED (1 << 3)

#define SCALE 64
@@ -67,14 +65,13 @@ static irqreturn_t timer8_interrupt(int irq, void *dev_id)

ctrl_outb(ctrl_inb(p->mapbase + _8TCSR) & ~0x40,
p->mapbase + _8TCSR);
- p->flags |= FLAG_IRQCONTEXT;
+
ctrl_outw(p->tcora, p->mapbase + TCORA);
- if (!(p->flags & FLAG_SKIPEVENT)) {
- if (clockevent_state_oneshot(&p->ced))
- ctrl_outw(0x0000, p->mapbase + _8TCR);
- p->ced.event_handler(&p->ced);
- }
- p->flags &= ~(FLAG_SKIPEVENT | FLAG_IRQCONTEXT);
+
+ if (clockevent_state_oneshot(&p->ced))
+ ctrl_outw(0x0000, p->mapbase + _8TCR);
+
+ p->ced.event_handler(&p->ced);

return IRQ_HANDLED;
}
--
1.9.1

2015-11-09 10:09:36

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 09/15] clocksource/drivers/h8300_timer8: Remove irq and lock legacy code

The time framawork takes care of disabling the interrupts and takes a lock
to prevent races.

Remove the legacy code in the driver taking care of the races.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/clocksource/h8300_timer8.c | 16 ----------------
1 file changed, 16 deletions(-)

diff --git a/drivers/clocksource/h8300_timer8.c b/drivers/clocksource/h8300_timer8.c
index 7111b99..24d91b1 100644
--- a/drivers/clocksource/h8300_timer8.c
+++ b/drivers/clocksource/h8300_timer8.c
@@ -31,7 +31,6 @@
struct timer8_priv {
struct clock_event_device ced;
unsigned long mapbase;
- raw_spinlock_t lock;
unsigned long flags;
unsigned int rate;
unsigned int tcora;
@@ -78,10 +77,8 @@ static irqreturn_t timer8_interrupt(int irq, void *dev_id)

static void timer8_set_next(struct timer8_priv *p, unsigned long delta)
{
- unsigned long flags;
unsigned long now;

- raw_spin_lock_irqsave(&p->lock, flags);
if (delta >= 0x10000)
pr_warn("delta out of range\n");
now = timer8_get_counter(p);
@@ -91,8 +88,6 @@ static void timer8_set_next(struct timer8_priv *p, unsigned long delta)
ctrl_outw(delta, p->mapbase + TCORA);
else
ctrl_outw(now + 1, p->mapbase + TCORA);
-
- raw_spin_unlock_irqrestore(&p->lock, flags);
}

static int timer8_enable(struct timer8_priv *p)
@@ -108,9 +103,6 @@ static int timer8_enable(struct timer8_priv *p)
static int timer8_start(struct timer8_priv *p)
{
int ret = 0;
- unsigned long flags;
-
- raw_spin_lock_irqsave(&p->lock, flags);

if (!(p->flags & FLAG_STARTED))
ret = timer8_enable(p);
@@ -120,20 +112,12 @@ static int timer8_start(struct timer8_priv *p)
p->flags |= FLAG_STARTED;

out:
- raw_spin_unlock_irqrestore(&p->lock, flags);
-
return ret;
}

static void timer8_stop(struct timer8_priv *p)
{
- unsigned long flags;
-
- raw_spin_lock_irqsave(&p->lock, flags);
-
ctrl_outw(0x0000, p->mapbase + _8TCR);
-
- raw_spin_unlock_irqrestore(&p->lock, flags);
}

static inline struct timer8_priv *ced_to_priv(struct clock_event_device *ced)
--
1.9.1

2015-11-09 10:09:34

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 10/15] clocksource/drivers/h8300_timer8: Retrieve the clock rate at init time

The current code retrieves the rate value when the timer is enabled which
occurs each time a timer is re-armed. Except if the clock frequency has changed
magically I don't see why this should be done each time.

Retrieve the clock rate value at init time only.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/clocksource/h8300_timer8.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/clocksource/h8300_timer8.c b/drivers/clocksource/h8300_timer8.c
index 24d91b1..187c416 100644
--- a/drivers/clocksource/h8300_timer8.c
+++ b/drivers/clocksource/h8300_timer8.c
@@ -34,7 +34,6 @@ struct timer8_priv {
unsigned long flags;
unsigned int rate;
unsigned int tcora;
- struct clk *pclk;
};

static unsigned long timer8_get_counter(struct timer8_priv *p)
@@ -92,7 +91,6 @@ static void timer8_set_next(struct timer8_priv *p, unsigned long delta)

static int timer8_enable(struct timer8_priv *p)
{
- p->rate = clk_get_rate(p->pclk) / SCALE;
ctrl_outw(0xffff, p->mapbase + TCORA);
ctrl_outw(0x0000, p->mapbase + _8TCNT);
ctrl_outw(0x0c02, p->mapbase + _8TCR);
@@ -102,16 +100,15 @@ static int timer8_enable(struct timer8_priv *p)

static int timer8_start(struct timer8_priv *p)
{
- int ret = 0;
+ int ret;

- if (!(p->flags & FLAG_STARTED))
- ret = timer8_enable(p);
+ if ((p->flags & FLAG_STARTED))
+ return 0;

- if (ret)
- goto out;
- p->flags |= FLAG_STARTED;
+ ret = timer8_enable(p);
+ if (!ret)
+ p->flags |= FLAG_STARTED;

- out:
return ret;
}

@@ -217,7 +214,12 @@ static void __init h8300_8timer_init(struct device_node *node)
}

timer8_priv.mapbase = (unsigned long)base;
- timer8_priv.pclk = clk;
+
+ rate = clk_get_rate(clk) / SCALE;
+ if (!rate) {
+ pr_err("Failed to get rate for the clocksource\n");
+ goto unmap_reg;
+ }

ret = request_irq(irq, timer8_interrupt,
IRQF_TIMER, timer8_priv.ced.name, &timer8_priv);
@@ -225,10 +227,10 @@ static void __init h8300_8timer_init(struct device_node *node)
pr_err("failed to request irq %d for clockevent\n", irq);
goto unmap_reg;
}
- rate = clk_get_rate(clk) / SCALE;
+
clockevents_config_and_register(&timer8_priv.ced, rate, 1, 0x0000ffff);
- return;

+ return;
unmap_reg:
iounmap(base);
free_clk:
--
1.9.1

2015-11-09 10:07:35

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 11/15] clocksource/drivers/h8300_timer16: Remove pointless headers

The headers are not needed, remove them.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/clocksource/h8300_timer16.c | 10 ----------
1 file changed, 10 deletions(-)

diff --git a/drivers/clocksource/h8300_timer16.c b/drivers/clocksource/h8300_timer16.c
index cdf0d83..1c9dd02 100644
--- a/drivers/clocksource/h8300_timer16.c
+++ b/drivers/clocksource/h8300_timer16.c
@@ -4,25 +4,15 @@
* Copyright 2015 Yoshinori Sato <[email protected]>
*/

-#include <linux/errno.h>
-#include <linux/kernel.h>
-#include <linux/param.h>
-#include <linux/string.h>
-#include <linux/slab.h>
#include <linux/interrupt.h>
#include <linux/init.h>
-#include <linux/platform_device.h>
#include <linux/clocksource.h>
-#include <linux/module.h>
#include <linux/clk.h>
#include <linux/io.h>
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_irq.h>

-#include <asm/segment.h>
-#include <asm/irq.h>
-
#define TSTR 0
#define TSNC 1
#define TMDR 2
--
1.9.1

2015-11-09 10:07:43

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 12/15] clocksource/drivers/h8300_timer16: Remove unused macros

The macros are no longer used in the code, remove them.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/clocksource/h8300_timer16.c | 18 ------------------
1 file changed, 18 deletions(-)

diff --git a/drivers/clocksource/h8300_timer16.c b/drivers/clocksource/h8300_timer16.c
index 1c9dd02..bc9289b 100644
--- a/drivers/clocksource/h8300_timer16.c
+++ b/drivers/clocksource/h8300_timer16.c
@@ -14,29 +14,11 @@
#include <linux/of_irq.h>

#define TSTR 0
-#define TSNC 1
-#define TMDR 2
-#define TOLR 3
#define TISRA 4
-#define TISRB 5
#define TISRC 6

#define TCR 0
-#define TIOR 1
#define TCNT 2
-#define GRA 4
-#define GRB 6
-
-#define FLAG_REPROGRAM (1 << 0)
-#define FLAG_SKIPEVENT (1 << 1)
-#define FLAG_IRQCONTEXT (1 << 2)
-#define FLAG_STARTED (1 << 3)
-
-#define ONESHOT 0
-#define PERIODIC 1
-
-#define RELATIVE 0
-#define ABSOLUTE 1

struct timer16_priv {
struct clocksource cs;
--
1.9.1

2015-11-09 10:08:58

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 13/15] clocksource/drivers/h8300_timer16: Remove unused fields in timer16_priv

The fields are not used in the code, remove them.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/clocksource/h8300_timer16.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/clocksource/h8300_timer16.c b/drivers/clocksource/h8300_timer16.c
index bc9289b..6705bf7 100644
--- a/drivers/clocksource/h8300_timer16.c
+++ b/drivers/clocksource/h8300_timer16.c
@@ -25,8 +25,6 @@ struct timer16_priv {
unsigned long total_cycles;
unsigned long mapbase;
unsigned long mapcommon;
- unsigned long flags;
- unsigned short gra;
unsigned short cs_enabled;
unsigned char enb;
unsigned char imfa;
--
1.9.1

2015-11-09 10:07:40

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 14/15] clocksource/drivers/h8300_timer16: Fix irq return value check

The function irq_of_parse_and_map returns zero in case of failure.

Fix the return code test to check against zero.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/clocksource/h8300_timer16.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/h8300_timer16.c b/drivers/clocksource/h8300_timer16.c
index 6705bf7..129dca0 100644
--- a/drivers/clocksource/h8300_timer16.c
+++ b/drivers/clocksource/h8300_timer16.c
@@ -155,7 +155,7 @@ static void __init h8300_16timer_init(struct device_node *node)
}

irq = irq_of_parse_and_map(node, 0);
- if (irq < 0) {
+ if (!irq) {
pr_err("failed to get irq for clockevent\n");
goto unmap_comm;
}
--
1.9.1

2015-11-09 10:08:37

by Daniel Lezcano

[permalink] [raw]
Subject: [PATCH 15/15] clocksource/drivers/h8300_timer16: Remove pointless lock

The lock in the timer16_clocksource_read is not needed, remove it.

Signed-off-by: Daniel Lezcano <[email protected]>
---
drivers/clocksource/h8300_timer16.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/clocksource/h8300_timer16.c b/drivers/clocksource/h8300_timer16.c
index 129dca0..f396605 100644
--- a/drivers/clocksource/h8300_timer16.c
+++ b/drivers/clocksource/h8300_timer16.c
@@ -30,7 +30,6 @@ struct timer16_priv {
unsigned char imfa;
unsigned char imiea;
unsigned char ovf;
- raw_spinlock_t lock;
struct clk *clk;
};

@@ -75,13 +74,10 @@ static inline struct timer16_priv *cs_to_priv(struct clocksource *cs)
static cycle_t timer16_clocksource_read(struct clocksource *cs)
{
struct timer16_priv *p = cs_to_priv(cs);
- unsigned long flags, raw;
- unsigned long value;
+ unsigned long raw, value;

- raw_spin_lock_irqsave(&p->lock, flags);
value = p->total_cycles;
raw = timer16_get_counter(p);
- raw_spin_unlock_irqrestore(&p->lock, flags);

return value + raw;
}
--
1.9.1

2015-11-09 10:13:53

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH 01/15] clocksource/drivers/h8300_timer8: Fix compilation error with dev_warn


Series compiled but not tested.

-- 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-11-11 14:51:07

by Yoshinori Sato

[permalink] [raw]
Subject: Re: [PATCH 02/15] clocksource/drivers/h8300_tpu: Remove unused macros

On Mon, 09 Nov 2015 19:06:40 +0900,
Daniel Lezcano wrote:
>
> Some macros are unused, delete them.
>
> Signed-off-by: Daniel Lezcano <[email protected]>

Acked-by: Yoshinori Sato <[email protected]>

> ---
> drivers/clocksource/h8300_tpu.c | 13 +++----------
> 1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/clocksource/h8300_tpu.c b/drivers/clocksource/h8300_tpu.c
> index ed0b493..576dae6 100644
> --- a/drivers/clocksource/h8300_tpu.c
> +++ b/drivers/clocksource/h8300_tpu.c
> @@ -20,16 +20,9 @@
> #include <linux/of_address.h>
> #include <linux/of_irq.h>
>
> -#define TCR 0
> -#define TMDR 1
> -#define TIOR 2
> -#define TER 4
> -#define TSR 5
> -#define TCNT 6
> -#define TGRA 8
> -#define TGRB 10
> -#define TGRC 12
> -#define TGRD 14
> +#define TCR 0x0
> +#define TSR 0x5
> +#define TCNT 0x6
>
> struct tpu_priv {
> struct clocksource cs;
> --
> 1.9.1
>

2015-11-11 14:51:05

by Yoshinori Sato

[permalink] [raw]
Subject: Re: [PATCH 03/15] clocksource/drivers/h8300_tpu: Remove pointless headers for TPU

On Mon, 09 Nov 2015 19:06:41 +0900,
Daniel Lezcano wrote:
>
> Signed-off-by: Daniel Lezcano <[email protected]>

Acked-by: Yoshinori Sato <[email protected]>

> ---
> drivers/clocksource/h8300_tpu.c | 5 -----
> 1 file changed, 5 deletions(-)
>
> diff --git a/drivers/clocksource/h8300_tpu.c b/drivers/clocksource/h8300_tpu.c
> index 576dae6..c1eef42 100644
> --- a/drivers/clocksource/h8300_tpu.c
> +++ b/drivers/clocksource/h8300_tpu.c
> @@ -6,14 +6,9 @@
> */
>
> #include <linux/errno.h>
> -#include <linux/sched.h>
> #include <linux/kernel.h>
> -#include <linux/interrupt.h>
> #include <linux/init.h>
> -#include <linux/platform_device.h>
> -#include <linux/slab.h>
> #include <linux/clocksource.h>
> -#include <linux/module.h>
> #include <linux/clk.h>
> #include <linux/io.h>
> #include <linux/of.h>
> --
> 1.9.1
>

2015-11-11 14:50:41

by Yoshinori Sato

[permalink] [raw]
Subject: [PATCH 0/4] h8300: clock driver update

Overall looks good.
And I found some problem (in original code). This patches fix it.
Please apply after your patches.

Thanks.

Yoshinori Sato (4):
h8300: clocksource: Use overflow interrupt
h8300: clocksource: Counter overflow fix
h8300: clocksource: More simplify timer8_set_next
h8300: clocksource: remove unused local-variable.

drivers/clocksource/h8300_timer16.c | 30 ++++++++++--------
drivers/clocksource/h8300_timer8.c | 63 ++++++++++++-------------------------
2 files changed, 37 insertions(+), 56 deletions(-)

--
2.6.1

2015-11-11 14:52:26

by Yoshinori Sato

[permalink] [raw]
Subject: [PATCH 1/4] h8300: clocksource: Use overflow interrupt

Overflow interrupt is used for moving up of a count.

Signed-off-by: Yoshinori Sato <[email protected]>
---
drivers/clocksource/h8300_timer16.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/clocksource/h8300_timer16.c b/drivers/clocksource/h8300_timer16.c
index f396605..53daf6a 100644
--- a/drivers/clocksource/h8300_timer16.c
+++ b/drivers/clocksource/h8300_timer16.c
@@ -14,7 +14,6 @@
#include <linux/of_irq.h>

#define TSTR 0
-#define TISRA 4
#define TISRC 6

#define TCR 0
@@ -27,10 +26,8 @@ struct timer16_priv {
unsigned long mapcommon;
unsigned short cs_enabled;
unsigned char enb;
- unsigned char imfa;
- unsigned char imiea;
unsigned char ovf;
- struct clk *clk;
+ unsigned char ovie;
};

static unsigned long timer16_get_counter(struct timer16_priv *p)
@@ -59,8 +56,8 @@ static irqreturn_t timer16_interrupt(int irq, void *dev_id)
{
struct timer16_priv *p = (struct timer16_priv *)dev_id;

- ctrl_outb(ctrl_inb(p->mapcommon + TISRA) & ~p->imfa,
- p->mapcommon + TISRA);
+ ctrl_outb(ctrl_inb(p->mapcommon + TISRC) & ~p->ovf,
+ p->mapcommon + TISRC);
p->total_cycles += 0x10000;

return IRQ_HANDLED;
@@ -91,6 +88,8 @@ static int timer16_enable(struct clocksource *cs)
p->total_cycles = 0;
ctrl_outw(0x0000, p->mapbase + TCNT);
ctrl_outb(0x83, p->mapbase + TCR);
+ ctrl_outb(ctrl_inb(p->mapcommon + TISRC) | p->ovie,
+ p->mapcommon + TISRC);
ctrl_outb(ctrl_inb(p->mapcommon + TSTR) | p->enb,
p->mapcommon + TSTR);

@@ -104,6 +103,8 @@ static void timer16_disable(struct clocksource *cs)

WARN_ON(!p->cs_enabled);

+ ctrl_outb(ctrl_inb(p->mapcommon + TISRC) & ~p->ovie,
+ p->mapcommon + TISRC);
ctrl_outb(ctrl_inb(p->mapcommon + TSTR) & ~p->enb,
p->mapcommon + TSTR);

@@ -118,6 +119,7 @@ static struct timer16_priv timer16_priv = {
.enable = timer16_enable,
.disable = timer16_disable,
.mask = CLOCKSOURCE_MASK(sizeof(unsigned long) * 8),
+ .max_cycles = 0xffffffff,
.flags = CLOCK_SOURCE_IS_CONTINUOUS,
},
};
@@ -160,9 +162,9 @@ static void __init h8300_16timer_init(struct device_node *node)

timer16_priv.mapbase = (unsigned long)base[REG_CH];
timer16_priv.mapcommon = (unsigned long)base[REG_COMM];
+ timer16_priv.ovf = 1 << ch;
+ timer16_priv.ovie = 1 << (4 + ch);
timer16_priv.enb = 1 << ch;
- timer16_priv.imfa = 1 << ch;
- timer16_priv.imiea = 1 << (4 + ch);

ret = request_irq(irq, timer16_interrupt,
IRQF_TIMER, timer16_priv.cs.name, &timer16_priv);
@@ -172,7 +174,7 @@ static void __init h8300_16timer_init(struct device_node *node)
}

clocksource_register_hz(&timer16_priv.cs,
- clk_get_rate(timer16_priv.clk) / 8);
+ clk_get_rate(clk) / 8);
return;

unmap_comm:
--
2.6.1

2015-11-11 14:51:50

by Yoshinori Sato

[permalink] [raw]
Subject: [PATCH 2/4] h8300: clocksource: Counter overflow fix

Signed-off-by: Yoshinori Sato <[email protected]>
---
drivers/clocksource/h8300_timer16.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/clocksource/h8300_timer16.c b/drivers/clocksource/h8300_timer16.c
index 53daf6a..639acf3 100644
--- a/drivers/clocksource/h8300_timer16.c
+++ b/drivers/clocksource/h8300_timer16.c
@@ -32,8 +32,8 @@ struct timer16_priv {

static unsigned long timer16_get_counter(struct timer16_priv *p)
{
- unsigned long v1, v2, v3;
- int o1, o2;
+ unsigned short v1, v2, v3;
+ unsigned char o1, o2;

o1 = ctrl_inb(p->mapcommon + TISRC) & p->ovf;

@@ -47,8 +47,10 @@ static unsigned long timer16_get_counter(struct timer16_priv *p)
} while (unlikely((o1 != o2) || (v1 > v2 && v1 < v3)
|| (v2 > v3 && v2 < v1) || (v3 > v1 && v3 < v2)));

- v2 |= 0x10000;
- return v2;
+ if (unlikely(!o1))
+ return v2;
+ else
+ return v2 + 0x10000;
}


--
2.6.1

2015-11-11 14:50:47

by Yoshinori Sato

[permalink] [raw]
Subject: [PATCH 3/4] h8300: clocksource: More simplify timer8_set_next

Signed-off-by: Yoshinori Sato <[email protected]>
---
drivers/clocksource/h8300_timer8.c | 48 +++++++++++---------------------------
1 file changed, 13 insertions(+), 35 deletions(-)

diff --git a/drivers/clocksource/h8300_timer8.c b/drivers/clocksource/h8300_timer8.c
index 187c416..d85ae9a 100644
--- a/drivers/clocksource/h8300_timer8.c
+++ b/drivers/clocksource/h8300_timer8.c
@@ -24,6 +24,9 @@
#define TCORB 6
#define _8TCNT 8

+#define CMIEA BIT(6)
+#define CMFA BIT(6)
+
#define FLAG_STARTED (1 << 3)

#define SCALE 64
@@ -36,57 +39,32 @@ struct timer8_priv {
unsigned int tcora;
};

-static unsigned long timer8_get_counter(struct timer8_priv *p)
-{
- unsigned long v1, v2, v3;
- int o1, o2;
-
- o1 = ctrl_inb(p->mapbase + _8TCSR) & 0x20;
-
- /* Make sure the timer value is stable. Stolen from acpi_pm.c */
- do {
- o2 = o1;
- v1 = ctrl_inw(p->mapbase + _8TCNT);
- v2 = ctrl_inw(p->mapbase + _8TCNT);
- v3 = ctrl_inw(p->mapbase + _8TCNT);
- o1 = ctrl_inb(p->mapbase + _8TCSR) & 0x20;
- } while (unlikely((o1 != o2) || (v1 > v2 && v1 < v3)
- || (v2 > v3 && v2 < v1) || (v3 > v1 && v3 < v2)));
-
- v2 |= o1 << 10;
- return v2;
-}
-
static irqreturn_t timer8_interrupt(int irq, void *dev_id)
{
struct timer8_priv *p = dev_id;

- ctrl_outb(ctrl_inb(p->mapbase + _8TCSR) & ~0x40,
- p->mapbase + _8TCSR);
-
- ctrl_outw(p->tcora, p->mapbase + TCORA);
-
if (clockevent_state_oneshot(&p->ced))
ctrl_outw(0x0000, p->mapbase + _8TCR);

p->ced.event_handler(&p->ced);

+ ctrl_outb(ctrl_inb(p->mapbase + _8TCSR) & ~CMFA,
+ p->mapbase + _8TCSR);
return IRQ_HANDLED;
}

static void timer8_set_next(struct timer8_priv *p, unsigned long delta)
{
- unsigned long now;
-
if (delta >= 0x10000)
pr_warn("delta out of range\n");
- now = timer8_get_counter(p);
- p->tcora = delta;
- ctrl_outb(ctrl_inb(p->mapbase + _8TCR) | 0x40, p->mapbase + _8TCR);
- if (delta > now)
- ctrl_outw(delta, p->mapbase + TCORA);
- else
- ctrl_outw(now + 1, p->mapbase + TCORA);
+ ctrl_outb(ctrl_inb(p->mapbase + _8TCR) & ~CMIEA,
+ p->mapbase + _8TCR);
+ ctrl_outw(delta, p->mapbase + TCORA);
+ ctrl_outw(0x0000, p->mapbase + _8TCNT);
+ ctrl_outb(ctrl_inb(p->mapbase + _8TCSR) & ~CMFA,
+ p->mapbase + _8TCSR);
+ ctrl_outb(ctrl_inb(p->mapbase + _8TCR) | CMIEA,
+ p->mapbase + _8TCR);
}

static int timer8_enable(struct timer8_priv *p)
--
2.6.1

2015-11-11 14:50:45

by Yoshinori Sato

[permalink] [raw]
Subject: [PATCH 4/4] h8300: clocksource: remove unused local-variable.

Signed-off-by: Yoshinori Sato <[email protected]>
---
drivers/clocksource/h8300_timer8.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/clocksource/h8300_timer8.c b/drivers/clocksource/h8300_timer8.c
index d85ae9a..a725758 100644
--- a/drivers/clocksource/h8300_timer8.c
+++ b/drivers/clocksource/h8300_timer8.c
@@ -169,8 +169,6 @@ static void __init h8300_8timer_init(struct device_node *node)
{
void __iomem *base;
int irq;
- int ret = 0;
- int rate;
struct clk *clk;

clk = of_clk_get(node, 0);
@@ -193,20 +191,21 @@ static void __init h8300_8timer_init(struct device_node *node)

timer8_priv.mapbase = (unsigned long)base;

- rate = clk_get_rate(clk) / SCALE;
- if (!rate) {
+ timer8_priv.rate = clk_get_rate(clk);
+ if (!timer8_priv.rate) {
pr_err("Failed to get rate for the clocksource\n");
goto unmap_reg;
}
+ timer8_priv.rate /= SCALE;

- ret = request_irq(irq, timer8_interrupt,
- IRQF_TIMER, timer8_priv.ced.name, &timer8_priv);
- if (ret < 0) {
+ if (request_irq(irq, timer8_interrupt,
+ IRQF_TIMER, timer8_priv.ced.name, &timer8_priv) < 0) {
pr_err("failed to request irq %d for clockevent\n", irq);
goto unmap_reg;
}

- clockevents_config_and_register(&timer8_priv.ced, rate, 1, 0x0000ffff);
+ clockevents_config_and_register(&timer8_priv.ced,
+ timer8_priv.rate, 1, 0x0000ffff);

return;
unmap_reg:
--
2.6.1

2015-11-12 16:28:09

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH 0/4] h8300: clock driver update

On 11/11/2015 03:50 PM, Yoshinori Sato wrote:
> Overall looks good.
> And I found some problem (in original code). This patches fix it.
> Please apply after your patches.


Hi Yoshinori,

could you add a better description in your patches. No need to resend,
just answer to the patches with the changelog.

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-11-12 16:50:55

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH 1/4] h8300: clocksource: Use overflow interrupt

On 11/11/2015 03:50 PM, Yoshinori Sato wrote:
> Overflow interrupt is used for moving up of a count.
>
> Signed-off-by: Yoshinori Sato <[email protected]>
> ---

[ ... ]

> @@ -118,6 +119,7 @@ static struct timer16_priv timer16_priv = {
> .enable = timer16_enable,
> .disable = timer16_disable,
> .mask = CLOCKSOURCE_MASK(sizeof(unsigned long) * 8),
> + .max_cycles = 0xffffffff,

This is computed automatically when registering the clocksource.

clocksource_register_hz
__clocksource_register_scale
__clocksource_update_freq_scale
clocksource_update_max_deferment
=> clocks_calc_max_nsecs


> .flags = CLOCK_SOURCE_IS_CONTINUOUS,
> },
> };

[ ... ]

--
<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-11-17 12:20:49

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH 0/4] h8300: clock driver update

On 11/11/2015 03:50 PM, Yoshinori Sato wrote:
> Overall looks good.
> And I found some problem (in original code). This patches fix it.
> Please apply after your patches.

Hi Yoshinori,

could you elaborate in the changelogs of the different patches what they
do ?

Thanks
-- Daniel

> Yoshinori Sato (4):
> h8300: clocksource: Use overflow interrupt
> h8300: clocksource: Counter overflow fix
> h8300: clocksource: More simplify timer8_set_next
> h8300: clocksource: remove unused local-variable.
>
> drivers/clocksource/h8300_timer16.c | 30 ++++++++++--------
> drivers/clocksource/h8300_timer8.c | 63 ++++++++++++-------------------------
> 2 files changed, 37 insertions(+), 56 deletions(-)
>


--
<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-11-18 16:58:26

by Yoshinori Sato

[permalink] [raw]
Subject: Re: [PATCH 0/4] h8300: clock driver update

On Tue, 17 Nov 2015 21:20:45 +0900,
Daniel Lezcano wrote:
>
> On 11/11/2015 03:50 PM, Yoshinori Sato wrote:
> > Overall looks good.
> > And I found some problem (in original code). This patches fix it.
> > Please apply after your patches.
>
> Hi Yoshinori,
>
> could you elaborate in the changelogs of the different patches what
> they do ?
>
> Thanks
> -- Daniel
>

I found one problem.
I will resend after fix it.

Thanks.

> > Yoshinori Sato (4):
> > h8300: clocksource: Use overflow interrupt
> > h8300: clocksource: Counter overflow fix
> > h8300: clocksource: More simplify timer8_set_next
> > h8300: clocksource: remove unused local-variable.
> >
> > drivers/clocksource/h8300_timer16.c | 30 ++++++++++--------
> > drivers/clocksource/h8300_timer8.c | 63 ++++++++++++-------------------------
> > 2 files changed, 37 insertions(+), 56 deletions(-)
> >
>
>
> --
> <http://www.linaro.org/> Linaro.org $B("(B 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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

--
Yoshinori Sato
<[email protected]>