Re: [PATCH v2 33/35] clocksource/drivers/atcpit100: Add andestech atcpit100 timer

[Daniel Lezcano]

On 27/11/2017 13:28, Greentime Hu wrote:
> From: Rick Chen 
> 
> ATCPIT100 is often used on the Andes architecture,
> This timer provide 4 PIT channels. Each PIT channel is a
> multi-function timer, can be configured as 32,16,8 bit timers
> or PWM as well.
> 
> For system timer it will set channel 1 32-bit timer0 as clock
> source and count downwards until underflow and restart again.
> 
> It also set channel 0 32-bit timer0 as clock event and count
> downwards until condition match. It will generate an interrupt
> for handling periodically.
> 
> Signed-off-by: Rick Chen 
> Signed-off-by: Greentime Hu 
> ---

Looks good.

Please resend this patch folded with the Makefile change and the DT
binding (fixed) as suggested by Arnd. I will merge them.

Thanks

  -- Daniel


-- 
  Linaro.org │ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH v2 33/35] clocksource/drivers/atcpit100: Add andestech atcpit100 timer

[Linus Walleij]

On Mon, Nov 27, 2017 at 1:28 PM, Greentime Hu  wrote:

> From: Rick Chen 
>
> ATCPIT100 is often used on the Andes architecture,
> This timer provide 4 PIT channels. Each PIT channel is a
> multi-function timer, can be configured as 32,16,8 bit timers
> or PWM as well.
>
> For system timer it will set channel 1 32-bit timer0 as clock
> source and count downwards until underflow and restart again.
>
> It also set channel 0 32-bit timer0 as clock event and count
> downwards until condition match. It will generate an interrupt
> for handling periodically.
>
> Signed-off-by: Rick Chen 
> Signed-off-by: Greentime Hu 

The driver looks nice overall.

> +static struct timer_of to = {
> +   .flags = TIMER_OF_IRQ | TIMER_OF_CLOCK | TIMER_OF_BASE,
> +
> +   .clkevt = {
> +   .name = "atcpit100_tick",
> +   .rating = 300,
> +   .features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
> +   .set_state_shutdown = atcpit100_clkevt_shutdown,
> +   .set_state_periodic = atcpit100_clkevt_set_periodic,
> +   .set_state_oneshot = atcpit100_clkevt_set_oneshot,
> +   .tick_resume = atcpit100_clkevt_shutdown,
> +   .set_next_event = atcpit100_clkevt_next_event,
> +   .cpumask = cpu_all_mask,
> +   },
> +
> +   .of_irq = {
> +   .handler = atcpit100_timer_interrupt,
> +   .flags = IRQF_TIMER | IRQF_IRQPOLL,
> +   },

I would add:

.of_clk = {
.name = "PCLK",
};

To be explicit on what we use.

(I hope I understand this OF timer right.)

Otherwise it looks good!
Reviewed-by: Linus Walleij 

Yours,
Linus Walleij

[PATCH v2 33/35] clocksource/drivers/atcpit100: Add andestech atcpit100 timer

[Greentime Hu]

From: Rick Chen 

ATCPIT100 is often used on the Andes architecture,
This timer provide 4 PIT channels. Each PIT channel is a
multi-function timer, can be configured as 32,16,8 bit timers
or PWM as well.

For system timer it will set channel 1 32-bit timer0 as clock
source and count downwards until underflow and restart again.

It also set channel 0 32-bit timer0 as clock event and count
downwards until condition match. It will generate an interrupt
for handling periodically.

Signed-off-by: Rick Chen 
Signed-off-by: Greentime Hu 
---
 drivers/clocksource/timer-atcpit100.c |  247 +
 1 file changed, 247 insertions(+)
 create mode 100644 drivers/clocksource/timer-atcpit100.c

diff --git a/drivers/clocksource/timer-atcpit100.c 
b/drivers/clocksource/timer-atcpit100.c
new file mode 100644
index 000..76ddd2f
--- /dev/null
+++ b/drivers/clocksource/timer-atcpit100.c
@@ -0,0 +1,247 @@
+/*
+ *  Andestech ATCPIT100 Timer Device Driver Implementation
+ *
+ * Copyright (C) 2017 Andes Technology Corporation
+ * Rick Chen, Andes Technology Corporation 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "timer-of.h"
+
+/*
+ * Definition of register offsets
+ */
+
+/* ID and Revision Register */
+#define ID_REV 0x0
+
+/* Configuration Register */
+#define CFG0x10
+
+/* Interrupt Enable Register */
+#define INT_EN 0x14
+#define CH_INT_EN(c, i)((1<