devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/2] clocksource: npcm: add NPCM7xx timer driver
@ 2018-02-26 14:36 Tomer Maimon
  2018-02-26 14:36 ` [PATCH v4 1/2] dt-binding: timer: document NPCM7xx timer DT bindings Tomer Maimon
  2018-02-26 14:36 ` [PATCH v4 2/2] clocksource: npcm: add NPCM7xx timer driver Tomer Maimon
  0 siblings, 2 replies; 7+ messages in thread
From: Tomer Maimon @ 2018-02-26 14:36 UTC (permalink / raw)
  To: robh+dt, mark.rutland, daniel.lezcano, tglx, avifishman70,
	brendanhiggins, joel
  Cc: devicetree, linux-kernel, openbmc, Tomer Maimon

Addressed comments from:.
 - Brendan Higgins: https://www.spinics.net/lists/devicetree/msg202331.html

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: npcm: add NPCM7xx timer driver

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

-- 
2.14.1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v4 1/2] dt-binding: timer: document NPCM7xx timer DT bindings
  2018-02-26 14:36 [PATCH v4 0/2] clocksource: npcm: add NPCM7xx timer driver Tomer Maimon
@ 2018-02-26 14:36 ` Tomer Maimon
  2018-03-02  1:51   ` Joel Stanley
  2018-03-07 17:25   ` Daniel Lezcano
  2018-02-26 14:36 ` [PATCH v4 2/2] clocksource: npcm: add NPCM7xx timer driver Tomer Maimon
  1 sibling, 2 replies; 7+ messages in thread
From: Tomer Maimon @ 2018-02-26 14:36 UTC (permalink / raw)
  To: robh+dt, mark.rutland, daniel.lezcano, tglx, avifishman70,
	brendanhiggins, joel
  Cc: devicetree, linux-kernel, openbmc, Tomer Maimon

Added device tree binding documentation for Nuvoton NPCM7xx timer.

Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
Acked-by: Rob Herring <robh@kernel.org>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
---
 .../bindings/timer/nuvoton,npcm7xx-timer.txt       | 25 ++++++++++++++++++++++
 1 file changed, 25 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..c5150522e618
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/nuvoton,npcm7xx-timer.txt
@@ -0,0 +1,25 @@
+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.
+
+Required clocking property, have to be one of:
+- clocks          : phandle of timer reference clock.
+- clock-frequency : The frequency in Hz of the clock that drives the NPCM7xx
+                    timer (usually 25000000).
+
+Example:
+
+timer@f0008000 {
+    compatible = "nuvoton,npcm750-timer";
+    interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
+    reg = <0xf0008000 0x1000>;
+    clocks = <&clk NPCM7XX_CLK_TIMER>;
+};
+
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v4 2/2] clocksource: npcm: add NPCM7xx timer driver
  2018-02-26 14:36 [PATCH v4 0/2] clocksource: npcm: add NPCM7xx timer driver Tomer Maimon
  2018-02-26 14:36 ` [PATCH v4 1/2] dt-binding: timer: document NPCM7xx timer DT bindings Tomer Maimon
@ 2018-02-26 14:36 ` Tomer Maimon
  2018-03-07 17:28   ` Daniel Lezcano
  1 sibling, 1 reply; 7+ messages in thread
From: Tomer Maimon @ 2018-02-26 14:36 UTC (permalink / raw)
  To: robh+dt, mark.rutland, daniel.lezcano, tglx, avifishman70,
	brendanhiggins, joel
  Cc: devicetree, linux-kernel, openbmc, Tomer Maimon

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 <tmaimon77@gmail.com>
Reviewed-by: Brendan Higgins <brendanhiggins@xxxxxxxxxx>
---
 drivers/clocksource/Kconfig         |   8 ++
 drivers/clocksource/Makefile        |   1 +
 drivers/clocksource/npcm7xx-timer.c | 237 ++++++++++++++++++++++++++++++++++++
 3 files changed, 246 insertions(+)
 create mode 100644 drivers/clocksource/npcm7xx-timer.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index b3b4ed9b6874..bfc80653d89f 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 arcithcture,
+	  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..b16c1a2b9e0d 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)	+= npcm7xx-timer.o
 
 obj-$(CONFIG_ARC_TIMERS)		+= arc_timer.o
 obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
diff --git a/drivers/clocksource/npcm7xx-timer.c b/drivers/clocksource/npcm7xx-timer.c
new file mode 100644
index 000000000000..ab27fc82d8ef
--- /dev/null
+++ b/drivers/clocksource/npcm7xx-timer.c
@@ -0,0 +1,237 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2014-2018 Nuvoton Technologies tomer.maimon@nuvoton.com
+ * 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_rate(to) / HZ),
+		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_setnextevent(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,
+
+	.clkevt = {
+		.name		    = "npcm7xx-timer0",
+		.features	    = CLOCK_EVT_FEAT_PERIODIC |
+				      CLOCK_EVT_FEAT_ONESHOT,
+		.set_next_event	    = npcm7xx_clockevent_setnextevent,
+		.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(u32 rate)
+{
+	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, rate,
+					0x1, NPCM7XX_Tx_MAX_CNT);
+}
+
+static void __init npcm7xx_clocksource_init(u32 rate)
+{
+	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", rate,
+				200, (unsigned int)NPCM7XX_Tx_TDR_MASK_BITS,
+				clocksource_mmio_readl_down);
+}
+
+static int __init npcm7xx_timer_init(struct device_node *np)
+{
+	struct clk *clk;
+	int ret;
+	u32 rate;
+
+	clk = of_clk_get(np, 0);
+
+	if (IS_ERR(clk)) {
+		ret = of_property_read_u32(np, "clock-frequency", &rate);
+		if (ret)
+			return ret;
+	} else {
+		clk_prepare_enable(clk);
+		rate = clk_get_rate(clk);
+		npcm7xx_to.of_clk.clk = clk;
+	}
+
+	ret = timer_of_init(np, &npcm7xx_to);
+	if (ret)
+		goto err_timer_of_init;
+
+	/* Clock input is divided by PRESCALE + 1 before it is fed */
+	/* to the counter */
+	rate = rate / (NPCM7XX_Tx_MIN_PRESCALE + 1);
+	npcm7xx_to.of_clk.rate = rate;
+
+	npcm7xx_clocksource_init(rate);
+	npcm7xx_clockevents_init(rate);
+
+	pr_info("Enabling NPCM7xx clocksource timer base: %p, IRQ: %d\n",
+		 timer_of_base(&npcm7xx_to), timer_of_irq(&npcm7xx_to));
+
+	return 0;
+
+err_timer_of_init:
+	if (!IS_ERR(clk)) {
+		clk_disable_unprepare(clk);
+		clk_put(clk);
+	}
+
+	return ret;
+}
+
+TIMER_OF_DECLARE(npcm7xx, "nuvoton,npcm750-timer", npcm7xx_timer_init);
+
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] dt-binding: timer: document NPCM7xx timer DT bindings
  2018-02-26 14:36 ` [PATCH v4 1/2] dt-binding: timer: document NPCM7xx timer DT bindings Tomer Maimon
@ 2018-03-02  1:51   ` Joel Stanley
  2018-03-07 10:30     ` Tomer Maimon
  2018-03-07 17:25   ` Daniel Lezcano
  1 sibling, 1 reply; 7+ messages in thread
From: Joel Stanley @ 2018-03-02  1:51 UTC (permalink / raw)
  To: Tomer Maimon
  Cc: Rob Herring, Mark Rutland, Daniel Lezcano, Thomas Gleixner,
	Avi Fishman, Brendan Higgins, devicetree,
	Linux Kernel Mailing List, OpenBMC Maillist

Hi Tomer,

On Tue, Feb 27, 2018 at 1:06 AM, Tomer Maimon <tmaimon77@gmail.com> wrote:
> Added device tree binding documentation for Nuvoton NPCM7xx timer.
>
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> Acked-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
> ---
>  .../bindings/timer/nuvoton,npcm7xx-timer.txt       | 25 ++++++++++++++++++++++
>  1 file changed, 25 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..c5150522e618
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/nuvoton,npcm7xx-timer.txt
> @@ -0,0 +1,25 @@
> +Nuvoton NPCM7xx timer

Would it make more sense to all this Nuvoton NPCM Timer? This way the
name stays generic for when you add other systems.

> +
> +Nuvoton NPCM7xx have three timer modules, each timer module provides five 24-bit
> +timer counters.


I notice the timer has a register in the middle of it that is used for
the watchdog. Should we describe this in the same binding? Even if we
just add some text that adds

 ... and one watchdog register.

> +
> +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.
> +
> +Required clocking property, have to be one of:
> +- clocks          : phandle of timer reference clock.
> +- clock-frequency : The frequency in Hz of the clock that drives the NPCM7xx
> +                    timer (usually 25000000).
> +
> +Example:
> +
> +timer@f0008000 {
> +    compatible = "nuvoton,npcm750-timer";
> +    interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
> +    reg = <0xf0008000 0x1000>;

This can probably be <0xf0008000 0x50>, as tthe timer hardware only
has registers up to base + 0x50.

> +    clocks = <&clk NPCM7XX_CLK_TIMER>;
> +};
> +
> --
> 2.14.1
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] dt-binding: timer: document NPCM7xx timer DT bindings
  2018-03-02  1:51   ` Joel Stanley
@ 2018-03-07 10:30     ` Tomer Maimon
  0 siblings, 0 replies; 7+ messages in thread
From: Tomer Maimon @ 2018-03-07 10:30 UTC (permalink / raw)
  To: Joel Stanley
  Cc: Rob Herring, Mark Rutland, Daniel Lezcano, Thomas Gleixner,
	Avi Fishman, Brendan Higgins, devicetree,
	Linux Kernel Mailing List, OpenBMC Maillist

Sorry for the late response

On 2 March 2018 at 03:51, Joel Stanley <joel@jms.id.au> wrote:
> Hi Tomer,
>
> On Tue, Feb 27, 2018 at 1:06 AM, Tomer Maimon <tmaimon77@gmail.com> wrote:
>> Added device tree binding documentation for Nuvoton NPCM7xx timer.
>>
>> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
>> Acked-by: Rob Herring <robh@kernel.org>
>> Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
>> ---
>>  .../bindings/timer/nuvoton,npcm7xx-timer.txt       | 25 ++++++++++++++++++++++
>>  1 file changed, 25 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..c5150522e618
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/timer/nuvoton,npcm7xx-timer.txt
>> @@ -0,0 +1,25 @@
>> +Nuvoton NPCM7xx timer
>
> Would it make more sense to all this Nuvoton NPCM Timer? This way the
> name stays generic for when you add other systems.
>
we cant promise the timer will stay the same in the next generation, I
will leave this for NPCM7xx
>> +
>> +Nuvoton NPCM7xx have three timer modules, each timer module provides five 24-bit
>> +timer counters.
>
>
> I notice the timer has a register in the middle of it that is used for
> the watchdog. Should we describe this in the same binding? Even if we
> just add some text that adds
>
>  ... and one watchdog register.
>
I'm not sure, because the binding is only for the timer I dont think I
need to describe what is the
register contains
>> +
>> +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.
>> +
>> +Required clocking property, have to be one of:
>> +- clocks          : phandle of timer reference clock.
>> +- clock-frequency : The frequency in Hz of the clock that drives the NPCM7xx
>> +                    timer (usually 25000000).
>> +
>> +Example:
>> +
>> +timer@f0008000 {
>> +    compatible = "nuvoton,npcm750-timer";
>> +    interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
>> +    reg = <0xf0008000 0x1000>;
>
> This can probably be <0xf0008000 0x50>, as tthe timer hardware only
> has registers up to base + 0x50.
Thanks,
I will modify it in the next version, waiting fro more comments.
>
>> +    clocks = <&clk NPCM7XX_CLK_TIMER>;
>> +};
>> +
>> --
>> 2.14.1
>>

Cheers,

Tomer

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/2] dt-binding: timer: document NPCM7xx timer DT bindings
  2018-02-26 14:36 ` [PATCH v4 1/2] dt-binding: timer: document NPCM7xx timer DT bindings Tomer Maimon
  2018-03-02  1:51   ` Joel Stanley
@ 2018-03-07 17:25   ` Daniel Lezcano
  1 sibling, 0 replies; 7+ messages in thread
From: Daniel Lezcano @ 2018-03-07 17:25 UTC (permalink / raw)
  To: Tomer Maimon, robh+dt, mark.rutland, tglx, avifishman70,
	brendanhiggins, joel
  Cc: devicetree, linux-kernel, openbmc

On 26/02/2018 15:36, Tomer Maimon wrote:
> Added device tree binding documentation for Nuvoton NPCM7xx timer.
> 
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> Acked-by: Rob Herring <robh@kernel.org>
> Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
> ---
>  .../bindings/timer/nuvoton,npcm7xx-timer.txt       | 25 ++++++++++++++++++++++
>  1 file changed, 25 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..c5150522e618
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/nuvoton,npcm7xx-timer.txt
> @@ -0,0 +1,25 @@
> +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.
> +
> +Required clocking property, have to be one of:
> +- clocks          : phandle of timer reference clock.
> +- clock-frequency : The frequency in Hz of the clock that drives the NPCM7xx
> +                    timer (usually 25000000).

Why one of these and not the phandle only ? That will make the driver
cleaner.

> +
> +Example:
> +
> +timer@f0008000 {
> +    compatible = "nuvoton,npcm750-timer";
> +    interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
> +    reg = <0xf0008000 0x1000>;
> +    clocks = <&clk NPCM7XX_CLK_TIMER>;
> +};
> +
> 


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 2/2] clocksource: npcm: add NPCM7xx timer driver
  2018-02-26 14:36 ` [PATCH v4 2/2] clocksource: npcm: add NPCM7xx timer driver Tomer Maimon
@ 2018-03-07 17:28   ` Daniel Lezcano
  0 siblings, 0 replies; 7+ messages in thread
From: Daniel Lezcano @ 2018-03-07 17:28 UTC (permalink / raw)
  To: Tomer Maimon, robh+dt, mark.rutland, tglx, avifishman70,
	brendanhiggins, joel
  Cc: devicetree, linux-kernel, openbmc


Hi Tomer,

change the subject to: clocksource/drivers/npcm: Add NPCM7xx timer driver

On 26/02/2018 15:36, 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 <tmaimon77@gmail.com>
> Reviewed-by: Brendan Higgins <brendanhiggins@xxxxxxxxxx>


[ ... ]

> +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 arcithcture,

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..b16c1a2b9e0d 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)	+= npcm7xx-timer.o

Not mandatory but if you can change to timer-npcm7xx that would be nice.

>  obj-$(CONFIG_ARC_TIMERS)		+= arc_timer.o

[ ... ]

> +static int npcm7xx_clockevent_setnextevent(unsigned long evt,
> +		struct clock_event_device *clk)

npcm7xx_timer_set_next_event()

[ ... ]

> +static int __init npcm7xx_timer_init(struct device_node *np)
> +{
> +	struct clk *clk;
> +	int ret;
> +	u32 rate;
> +
> +	clk = of_clk_get(np, 0);
> +
> +	if (IS_ERR(clk)) {
> +		ret = of_property_read_u32(np, "clock-frequency", &rate);
> +		if (ret)
> +			return ret;
> +	} else {
> +		clk_prepare_enable(clk);
> +		rate = clk_get_rate(clk);
> +		npcm7xx_to.of_clk.clk = clk;
> +	}

[ ... ]

See comment on DT binding patch 1/2

> +err_timer_of_init:
> +	if (!IS_ERR(clk)) {
> +		clk_disable_unprepare(clk);
> +		clk_put(clk);
> +	}
> +
> +	return ret;
> +}
> +
> +TIMER_OF_DECLARE(npcm7xx, "nuvoton,npcm750-timer", npcm7xx_timer_init);
> +
> 


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2018-03-07 17:28 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-26 14:36 [PATCH v4 0/2] clocksource: npcm: add NPCM7xx timer driver Tomer Maimon
2018-02-26 14:36 ` [PATCH v4 1/2] dt-binding: timer: document NPCM7xx timer DT bindings Tomer Maimon
2018-03-02  1:51   ` Joel Stanley
2018-03-07 10:30     ` Tomer Maimon
2018-03-07 17:25   ` Daniel Lezcano
2018-02-26 14:36 ` [PATCH v4 2/2] clocksource: npcm: add NPCM7xx timer driver Tomer Maimon
2018-03-07 17:28   ` Daniel Lezcano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).