2018-03-08 15:27:18

by Tomer Maimon

[permalink] [raw]
Subject: [PATCH v5 0/2] clocksource/drivers/npcm: Add NPCM7xx timer driver

Addressed comments from:.
- Daniel Lezcano: https://www.spinics.net/lists/kernel/msg2741546.html
https://www.spinics.net/lists/kernel/msg2741548.html
- Joel Stanley: https://www.spinics.net/lists/kernel/msg2736646.html

Changes since version 4:
- Remove clock-frequency option.
- Rename driver name.
- Enable TIMER_OF_CLOCK flag.
- Modify register size in the dt-binding.

Changes since version 3:
- Modify prefix to npcm7xx_
- Modify licensing to SPDX license format

Changes since version 2:
- The shutdown function called oneshot function that can cause a crash in the system,
Modify shutdown call to unique shutdown function.
- Adding support to resume function
- Using timer-of.c API
- Modify NPCM750 compatible name.

Changes since version 1:
- Rename driver name
- Removing unnecessary dependencies in configuration
- Adding prefix to the macros
- No changes to dt-binding documentation since v1

Modified driver have been tested on the NPCM750 evaluation board.

Tomer Maimon (2):
dt-binding: timer: document NPCM7xx timer DT bindings
clocksource/drivers/npcm: Add NPCM7xx timer driver

.../bindings/timer/nuvoton,npcm7xx-timer.txt | 21 ++
drivers/clocksource/Kconfig | 8 +
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-npcm7xx.c | 215 +++++++++++++++++++++
4 files changed, 245 insertions(+)
create mode 100644 Documentation/devicetree/bindings/timer/nuvoton,npcm7xx-timer.txt
create mode 100644 drivers/clocksource/timer-npcm7xx.c

--
2.14.1



2018-03-08 15:28:15

by Tomer Maimon

[permalink] [raw]
Subject: [PATCH v5 2/2] clocksource/drivers/npcm: Add NPCM7xx timer driver

Add Nuvoton BMC NPCM7xx timer driver.

The clocksource Enable 24-bit TIMER0 and TIMER1 counters,
while TIMER0 serve as clockevent and TIMER1 serve as clocksource.

Signed-off-by: Tomer Maimon <[email protected]>
Reviewed-by: Brendan Higgins <brendanhiggins@xxxxxxxxxx>
---
drivers/clocksource/Kconfig | 8 ++
drivers/clocksource/Makefile | 1 +
drivers/clocksource/timer-npcm7xx.c | 215 ++++++++++++++++++++++++++++++++++++
3 files changed, 224 insertions(+)
create mode 100644 drivers/clocksource/timer-npcm7xx.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index b3b4ed9b6874..76194bc20bdf 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -130,6 +130,14 @@ config VT8500_TIMER
help
Enables support for the VT8500 driver.

+config NPCM7XX_TIMER
+ bool "NPCM7xx timer driver" if COMPILE_TEST
+ depends on HAS_IOMEM
+ select CLKSRC_MMIO
+ help
+ Enable 24-bit TIMER0 and TIMER1 counters in the NPCM7xx architecture,
+ While TIMER0 serves as clockevent and TIMER1 serves as clocksource.
+
config CADENCE_TTC_TIMER
bool "Cadence TTC timer driver" if COMPILE_TEST
depends on COMMON_CLK
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index d6dec4489d66..74387877f7cf 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_CLKSRC_NPS) += timer-nps.o
obj-$(CONFIG_OXNAS_RPS_TIMER) += timer-oxnas-rps.o
obj-$(CONFIG_OWL_TIMER) += owl-timer.o
obj-$(CONFIG_SPRD_TIMER) += timer-sprd.o
+obj-$(CONFIG_NPCM7XX_TIMER) += timer-npcm7xx.o

obj-$(CONFIG_ARC_TIMERS) += arc_timer.o
obj-$(CONFIG_ARM_ARCH_TIMER) += arm_arch_timer.o
diff --git a/drivers/clocksource/timer-npcm7xx.c b/drivers/clocksource/timer-npcm7xx.c
new file mode 100644
index 000000000000..f98d912bd259
--- /dev/null
+++ b/drivers/clocksource/timer-npcm7xx.c
@@ -0,0 +1,215 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2014-2018 Nuvoton Technologies [email protected]
+ * All rights reserved.
+ *
+ * Copyright 2017 Google, Inc.
+ */
+
+#include <linux/kernel.h>
+#include <linux/sched.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/err.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/clockchips.h>
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include "timer-of.h"
+
+/* Timers registers */
+#define NPCM7XX_REG_TCSR0 0x0 /* Timer 0 Control and Status Register */
+#define NPCM7XX_REG_TICR0 0x8 /* Timer 0 Initial Count Register */
+#define NPCM7XX_REG_TCSR1 0x4 /* Timer 1 Control and Status Register */
+#define NPCM7XX_REG_TICR1 0xc /* Timer 1 Initial Count Register */
+#define NPCM7XX_REG_TDR1 0x14 /* Timer 1 Data Register */
+#define NPCM7XX_REG_TISR 0x18 /* Timer Interrupt Status Register */
+
+/* Timers control */
+#define NPCM7XX_Tx_RESETINT 0x1f
+#define NPCM7XX_Tx_PERIOD BIT(27)
+#define NPCM7XX_Tx_INTEN BIT(29)
+#define NPCM7XX_Tx_COUNTEN BIT(30)
+#define NPCM7XX_Tx_ONESHOT 0x0
+#define NPCM7XX_Tx_OPER GENMASK(3, 27)
+#define NPCM7XX_Tx_MIN_PRESCALE 0x1
+#define NPCM7XX_Tx_TDR_MASK_BITS 24
+#define NPCM7XX_Tx_MAX_CNT 0xFFFFFF
+#define NPCM7XX_T0_CLR_INT 0x1
+#define NPCM7XX_Tx_CLR_CSR 0x0
+
+/* Timers operating mode */
+#define NPCM7XX_START_PERIODIC_Tx (NPCM7XX_Tx_PERIOD | NPCM7XX_Tx_COUNTEN | \
+ NPCM7XX_Tx_INTEN | \
+ NPCM7XX_Tx_MIN_PRESCALE)
+
+#define NPCM7XX_START_ONESHOT_Tx (NPCM7XX_Tx_ONESHOT | NPCM7XX_Tx_COUNTEN | \
+ NPCM7XX_Tx_INTEN | \
+ NPCM7XX_Tx_MIN_PRESCALE)
+
+#define NPCM7XX_START_Tx (NPCM7XX_Tx_COUNTEN | NPCM7XX_Tx_PERIOD | \
+ NPCM7XX_Tx_MIN_PRESCALE)
+
+#define NPCM7XX_DEFAULT_CSR (NPCM7XX_Tx_CLR_CSR | NPCM7XX_Tx_MIN_PRESCALE)
+
+static int npcm7xx_timer_resume(struct clock_event_device *evt)
+{
+ struct timer_of *to = to_timer_of(evt);
+ u32 val;
+
+ val = readl(timer_of_base(to) + NPCM7XX_REG_TCSR0);
+ val |= NPCM7XX_Tx_COUNTEN;
+ writel(val, timer_of_base(to) + NPCM7XX_REG_TCSR0);
+
+ return 0;
+}
+
+static int npcm7xx_timer_shutdown(struct clock_event_device *evt)
+{
+ struct timer_of *to = to_timer_of(evt);
+ u32 val;
+
+ val = readl(timer_of_base(to) + NPCM7XX_REG_TCSR0);
+ val &= ~NPCM7XX_Tx_COUNTEN;
+ writel(val, timer_of_base(to) + NPCM7XX_REG_TCSR0);
+
+ return 0;
+}
+
+static int npcm7xx_timer_oneshot(struct clock_event_device *evt)
+{
+ struct timer_of *to = to_timer_of(evt);
+ u32 val;
+
+ val = readl(timer_of_base(to) + NPCM7XX_REG_TCSR0);
+ val &= ~NPCM7XX_Tx_OPER;
+
+ val = readl(timer_of_base(to) + NPCM7XX_REG_TCSR0);
+ val |= NPCM7XX_START_ONESHOT_Tx;
+ writel(val, timer_of_base(to) + NPCM7XX_REG_TCSR0);
+
+ return 0;
+}
+
+static int npcm7xx_timer_periodic(struct clock_event_device *evt)
+{
+ struct timer_of *to = to_timer_of(evt);
+ u32 val;
+
+ val = readl(timer_of_base(to) + NPCM7XX_REG_TCSR0);
+ val &= ~NPCM7XX_Tx_OPER;
+
+ writel(timer_of_period(to), timer_of_base(to) + NPCM7XX_REG_TICR0);
+ val |= NPCM7XX_START_PERIODIC_Tx;
+
+ writel(val, timer_of_base(to) + NPCM7XX_REG_TCSR0);
+
+ return 0;
+}
+
+static int npcm7xx_clockevent_set_next_event(unsigned long evt,
+ struct clock_event_device *clk)
+{
+ struct timer_of *to = to_timer_of(clk);
+ u32 val;
+
+ writel(evt, timer_of_base(to) + NPCM7XX_REG_TICR0);
+ val = readl(timer_of_base(to) + NPCM7XX_REG_TCSR0);
+ val |= NPCM7XX_START_Tx;
+ writel(val, timer_of_base(to) + NPCM7XX_REG_TCSR0);
+
+ return 0;
+}
+
+static irqreturn_t npcm7xx_timer0_interrupt(int irq, void *dev_id)
+{
+ struct clock_event_device *evt = (struct clock_event_device *)dev_id;
+ struct timer_of *to = to_timer_of(evt);
+
+ writel(NPCM7XX_T0_CLR_INT, timer_of_base(to) + NPCM7XX_REG_TISR);
+
+ evt->event_handler(evt);
+
+ return IRQ_HANDLED;
+}
+
+static struct timer_of npcm7xx_to = {
+ .flags = TIMER_OF_IRQ | TIMER_OF_BASE | TIMER_OF_CLOCK,
+
+ .clkevt = {
+ .name = "npcm7xx-timer0",
+ .features = CLOCK_EVT_FEAT_PERIODIC |
+ CLOCK_EVT_FEAT_ONESHOT,
+ .set_next_event = npcm7xx_clockevent_set_next_event,
+ .set_state_shutdown = npcm7xx_timer_shutdown,
+ .set_state_periodic = npcm7xx_timer_periodic,
+ .set_state_oneshot = npcm7xx_timer_oneshot,
+ .tick_resume = npcm7xx_timer_resume,
+ .rating = 300,
+ },
+
+ .of_irq = {
+ .handler = npcm7xx_timer0_interrupt,
+ .flags = IRQF_TIMER | IRQF_IRQPOLL,
+ },
+};
+
+static void __init npcm7xx_clockevents_init(void)
+{
+ writel(NPCM7XX_DEFAULT_CSR,
+ timer_of_base(&npcm7xx_to) + NPCM7XX_REG_TCSR0);
+
+ writel(NPCM7XX_Tx_RESETINT,
+ timer_of_base(&npcm7xx_to) + NPCM7XX_REG_TISR);
+
+ npcm7xx_to.clkevt.cpumask = cpumask_of(0);
+ clockevents_config_and_register(&npcm7xx_to.clkevt,
+ timer_of_rate(&npcm7xx_to),
+ 0x1, NPCM7XX_Tx_MAX_CNT);
+}
+
+static void __init npcm7xx_clocksource_init(void)
+{
+ u32 val;
+
+ writel(NPCM7XX_DEFAULT_CSR,
+ timer_of_base(&npcm7xx_to) + NPCM7XX_REG_TCSR1);
+ writel(NPCM7XX_Tx_MAX_CNT,
+ timer_of_base(&npcm7xx_to) + NPCM7XX_REG_TICR1);
+
+ val = readl(timer_of_base(&npcm7xx_to) + NPCM7XX_REG_TCSR1);
+ val |= NPCM7XX_START_Tx;
+ writel(val, timer_of_base(&npcm7xx_to) + NPCM7XX_REG_TCSR1);
+
+ clocksource_mmio_init(timer_of_base(&npcm7xx_to) +
+ NPCM7XX_REG_TDR1,
+ "npcm7xx-timer1", timer_of_rate(&npcm7xx_to),
+ 200, (unsigned int)NPCM7XX_Tx_TDR_MASK_BITS,
+ clocksource_mmio_readl_down);
+}
+
+static int __init npcm7xx_timer_init(struct device_node *np)
+{
+ int ret;
+
+ ret = timer_of_init(np, &npcm7xx_to);
+ if (ret)
+ return ret;
+
+ /* Clock input is divided by PRESCALE + 1 before it is fed */
+ /* to the counter */
+ npcm7xx_to.of_clk.rate = npcm7xx_to.of_clk.rate /
+ (NPCM7XX_Tx_MIN_PRESCALE + 1);
+
+ npcm7xx_clocksource_init();
+ npcm7xx_clockevents_init();
+
+ pr_info("Enabling NPCM7xx clocksource timer base: 0x%x, IRQ: %d ",
+ (int)timer_of_base(&npcm7xx_to), timer_of_irq(&npcm7xx_to));
+
+ return 0;
+}
+
+TIMER_OF_DECLARE(npcm7xx, "nuvoton,npcm750-timer", npcm7xx_timer_init);
+
--
2.14.1


2018-03-08 15:28:21

by Tomer Maimon

[permalink] [raw]
Subject: [PATCH v5 1/2] dt-binding: timer: document NPCM7xx timer DT bindings

Added device tree binding documentation for Nuvoton NPCM7xx timer.

Signed-off-by: Tomer Maimon <[email protected]>
Acked-by: Rob Herring <[email protected]>
Reviewed-by: Brendan Higgins <[email protected]>
---
.../bindings/timer/nuvoton,npcm7xx-timer.txt | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
create mode 100644 Documentation/devicetree/bindings/timer/nuvoton,npcm7xx-timer.txt

diff --git a/Documentation/devicetree/bindings/timer/nuvoton,npcm7xx-timer.txt b/Documentation/devicetree/bindings/timer/nuvoton,npcm7xx-timer.txt
new file mode 100644
index 000000000000..ea22dfe485be
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/nuvoton,npcm7xx-timer.txt
@@ -0,0 +1,21 @@
+Nuvoton NPCM7xx timer
+
+Nuvoton NPCM7xx have three timer modules, each timer module provides five 24-bit
+timer counters.
+
+Required properties:
+- compatible : "nuvoton,npcm750-timer" for Poleg NPCM750.
+- reg : Offset and length of the register set for the device.
+- interrupts : Contain the timer interrupt with flags for
+ falling edge.
+- clocks : phandle of timer reference clock (usually a 25 MHz clock).
+
+Example:
+
+timer@f0008000 {
+ compatible = "nuvoton,npcm750-timer";
+ interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
+ reg = <0xf0008000 0x50>;
+ clocks = <&clk NPCM7XX_CLK_TIMER>;
+};
+
--
2.14.1


2018-03-08 15:34:35

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH v5 2/2] clocksource/drivers/npcm: Add NPCM7xx timer driver

On 08/03/2018 16:24, Tomer Maimon wrote:
> Add Nuvoton BMC NPCM7xx timer driver.
>
> The clocksource Enable 24-bit TIMER0 and TIMER1 counters,
> while TIMER0 serve as clockevent and TIMER1 serve as clocksource.
>
> Signed-off-by: Tomer Maimon <[email protected]>
> Reviewed-by: Brendan Higgins <brendanhiggins@xxxxxxxxxx>
> ---

[ ... ]

> +
> + pr_info("Enabling NPCM7xx clocksource timer base: 0x%x, IRQ: %d ",
> + (int)timer_of_base(&npcm7xx_to), timer_of_irq(&npcm7xx_to));

You can use '%p' instead of '0x%x' and remove the (int) cast.

I'm fine with the driver. If there is no comment, I will take it and do
the change (if you are ok), no need on this case to resend a v6.



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


2018-03-08 22:40:17

by Tomer Maimon

[permalink] [raw]
Subject: Re: [PATCH v5 2/2] clocksource/drivers/npcm: Add NPCM7xx timer driver

On 8 March 2018 at 17:33, Daniel Lezcano <[email protected]> wrote:
> On 08/03/2018 16:24, Tomer Maimon wrote:
>> Add Nuvoton BMC NPCM7xx timer driver.
>>
>> The clocksource Enable 24-bit TIMER0 and TIMER1 counters,
>> while TIMER0 serve as clockevent and TIMER1 serve as clocksource.
>>
>> Signed-off-by: Tomer Maimon <[email protected]>
>> Reviewed-by: Brendan Higgins <brendanhiggins@xxxxxxxxxx>
>> ---
>
> [ ... ]
>
>> +
>> + pr_info("Enabling NPCM7xx clocksource timer base: 0x%x, IRQ: %d ",
>> + (int)timer_of_base(&npcm7xx_to), timer_of_irq(&npcm7xx_to));
>
> You can use '%p' instead of '0x%x' and remove the (int) cast.
I think we need to use in the pr_info %px, if I use %p I get in the
kernel boot print "(ptrval)" instead of the address.
please refer: https://lwn.net/Articles/740249/
>
> I'm fine with the driver. If there is no comment, I will take it and do
> the change (if you are ok), no need on this case to resend a v6.
Yes great, Thank a lot!
>
>
>
> --
> <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
>

2018-03-09 13:26:48

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH v5 0/2] clocksource/drivers/npcm: Add NPCM7xx timer driver


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


2018-03-09 13:27:33

by Daniel Lezcano

[permalink] [raw]
Subject: Re: [PATCH v5 2/2] clocksource/drivers/npcm: Add NPCM7xx timer driver

On 08/03/2018 23:37, Tomer Maimon wrote:
> On 8 March 2018 at 17:33, Daniel Lezcano <[email protected]> wrote:
>> On 08/03/2018 16:24, Tomer Maimon wrote:
>>> Add Nuvoton BMC NPCM7xx timer driver.
>>>
>>> The clocksource Enable 24-bit TIMER0 and TIMER1 counters,
>>> while TIMER0 serve as clockevent and TIMER1 serve as clocksource.
>>>
>>> Signed-off-by: Tomer Maimon <[email protected]>
>>> Reviewed-by: Brendan Higgins <brendanhiggins@xxxxxxxxxx>
>>> ---
>>
>> [ ... ]
>>
>>> +
>>> + pr_info("Enabling NPCM7xx clocksource timer base: 0x%x, IRQ: %d ",
>>> + (int)timer_of_base(&npcm7xx_to), timer_of_irq(&npcm7xx_to));
>>
>> You can use '%p' instead of '0x%x' and remove the (int) cast.
> I think we need to use in the pr_info %px, if I use %p I get in the
> kernel boot print "(ptrval)" instead of the address.
> please refer: https://lwn.net/Articles/740249/

Ah yes, right.

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