[PATCH 0/2] add Microchip PIT64B timer

[PATCH 0/2] add Microchip PIT64B timer

[Claudiu.Beznea]

From: Claudiu Beznea <claudiu.beznea@microchip.com>

Hi,

This series adds driver for Microchip PIT64B timer.
Timer could be used in continuous or oneshot mode. It has 2x32 bit registers
to emulate a 64 bit timer. The timer's period could be configured via LSB_PR
and MSB_PR registers. The current timer's value could be checked via TLSB and
TMSB registers. When (TMSB << 32) | TLSB value reach the (MSB_PR << 32) | LSB_PR
interrupt is raised. If in contiuous mode the TLSB and TMSB resets and restart
counting.

This drivers uses PIT64B capabilities for clocksource and clockevent.
The first requested PIT64B timer is used for clockevent. The second one is used
for clocksource. Individual PIT64B hardware resources were used for clocksource
and clockevent to be able to support high resolution timers with this PIT64B
implementation.

Thank you,
Claudiu Beznea

Claudiu Beznea (2):
  dt-bindings: arm: atmel: add bindings for PIT64B
  clocksource/drivers/timer-microchip-pit64b: add Microchip PIT64B
    support

 .../devicetree/bindings/arm/atmel-sysregs.txt      |   7 +
 drivers/clocksource/Kconfig                        |   6 +
 drivers/clocksource/Makefile                       |   1 +
 drivers/clocksource/timer-microchip-pit64b.c       | 469 +++++++++++++++++++++
 4 files changed, 483 insertions(+)
 create mode 100644 drivers/clocksource/timer-microchip-pit64b.c

-- 
2.7.4

[PATCH 1/5] dt-bindings: arm: atmel: add bindings for PIT64B

[Claudiu.Beznea]

From: Claudiu Beznea <claudiu.beznea@microchip.com>

Add device tree bindings for PIT64B timer.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 Documentation/devicetree/bindings/arm/atmel-sysregs.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/atmel-sysregs.txt b/Documentation/devicetree/bindings/arm/atmel-sysregs.txt
index 14f319f694b7..c7b7ead7966b 100644
--- a/Documentation/devicetree/bindings/arm/atmel-sysregs.txt
+++ b/Documentation/devicetree/bindings/arm/atmel-sysregs.txt
@@ -10,6 +10,13 @@ PIT Timer required properties:
 - interrupts: Should contain interrupt for the PIT which is the IRQ line
   shared across all System Controller members.
 
+PIT64B Timer required properties:
+- compatible: Should be "microchip,sam9x60-pit64b"
+- reg: Should contain registers location and length
+- clock-frequency: Should contain PIT64B timer frequency
+- interrupts: Should contain interrupt for PIT64B timer
+- clocks: Should contain the available clock sources for PIT64B timer.
+
 System Timer (ST) required properties:
 - compatible: Should be "atmel,at91rm9200-st", "syscon", "simple-mfd"
 - reg: Should contain registers location and length
-- 
2.7.4

[PATCH 2/5] clocksource/drivers/timer-microchip-pit64b: add Microchip PIT64B support

[Claudiu.Beznea]

From: Claudiu Beznea <claudiu.beznea@microchip.com>

Add driver for Microchip PIT64B timer. Timer could be used in continuous
mode or oneshot mode. The hardware has 2x32 bit registers for period
emulating a 64 bit timer. The LSB_PR and MSB_PR registers are used to set
the period value (compare value). TLSB and TMSB keeps the current value
of the counter. After a compare the TLSB and TMSB register resets. Apart
from this the hardware has SMOD bit in mode register that allow to
reconfigure the timer without reset and start commands (start command
while timer is active is ignored).
The driver uses PIT64B timer as clocksource or clockevent. First requested
timer would be registered as clockevent, second one would be registered as
clocksource. Individual PIT64B hardware resources were used for clocksource
and clockevent to be able to support high resolution timers with this
hardware implementation.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/clocksource/Kconfig                  |   6 +
 drivers/clocksource/Makefile                 |   1 +
 drivers/clocksource/timer-microchip-pit64b.c | 464 +++++++++++++++++++++++++++
 3 files changed, 471 insertions(+)
 create mode 100644 drivers/clocksource/timer-microchip-pit64b.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 5d93e580e5dc..2ad6f881a0bb 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -448,6 +448,12 @@ config OXNAS_RPS_TIMER
 config SYS_SUPPORTS_SH_CMT
         bool
 
+config MICROCHIP_PIT64B
+	bool "Microchip PIT64B support"
+	depends on OF || COMPILE_TEST
+	help
+	  This option enables Microchip PIT64B timer.
+
 config MTK_TIMER
 	bool "Mediatek timer driver" if COMPILE_TEST
 	depends on HAS_IOMEM
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index c4a8e9ef932a..c53fa12b9b94 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -35,6 +35,7 @@ obj-$(CONFIG_U300_TIMER)	+= timer-u300.o
 obj-$(CONFIG_SUN4I_TIMER)	+= timer-sun4i.o
 obj-$(CONFIG_SUN5I_HSTIMER)	+= timer-sun5i.o
 obj-$(CONFIG_MESON6_TIMER)	+= timer-meson6.o
+obj-$(CONFIG_MICROCHIP_PIT64B)	+= timer-microchip-pit64b.o
 obj-$(CONFIG_TEGRA_TIMER)	+= timer-tegra20.o
 obj-$(CONFIG_VT8500_TIMER)	+= timer-vt8500.o
 obj-$(CONFIG_NSPIRE_TIMER)	+= timer-zevio.o
diff --git a/drivers/clocksource/timer-microchip-pit64b.c b/drivers/clocksource/timer-microchip-pit64b.c
new file mode 100644
index 000000000000..6787aa98ef01
--- /dev/null
+++ b/drivers/clocksource/timer-microchip-pit64b.c
@@ -0,0 +1,464 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Copyright (C) 2019 Microchip Technology Inc.
+// Copyright (C) 2019 Claudiu Beznea (claudiu.beznea@microchip.com)
+
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/interrupt.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/sched_clock.h>
+#include <linux/slab.h>
+
+#define MCHP_PIT64B_CR		0x00	/* Control Register */
+#define MCHP_PIT64B_CR_START	BIT(0)
+#define MCHP_PIT64B_CR_SWRST	BIT(8)
+
+#define MCHP_PIT64B_MR		0x04	/* Mode Register */
+#define MCHP_PIT64B_MR_CONT	BIT(0)
+#define MCHP_PIT64B_MR_SGCLK	BIT(3)
+#define MCHP_PIT64B_MR_SMOD	BIT(4)
+#define MCHP_PIT64B_MR_PRES	GENMASK(11, 8)
+
+#define MCHP_PIT64B_LSB_PR	0x08	/* LSB Period Register */
+
+#define MCHP_PIT64B_MSB_PR	0x0C	/* MSB Period Register */
+
+#define MCHP_PIT64B_IER		0x10	/* Interrupt Enable Register */
+#define MCHP_PIT64B_IER_PERIOD	BIT(0)
+
+#define MCHP_PIT64B_ISR		0x1C	/* Interrupt Status Register */
+#define MCHP_PIT64B_ISR_PERIOD	BIT(0)
+
+#define MCHP_PIT64B_TLSBR	0x20	/* Timer LSB Register */
+
+#define MCHP_PIT64B_TMSBR	0x24	/* Timer MSB Register */
+
+#define MCHP_PIT64B_PRES_MAX	0x10
+#define MCHP_PIT64B_DEF_FREQ	2500000UL	/* 2.5 MHz */
+#define MCHP_PIT64B_LSBMASK	GENMASK_ULL(31, 0)
+#define MCHP_PIT64B_PRESCALER(p)	(MCHP_PIT64B_MR_PRES & ((p) << 8))
+
+#define MCHP_PIT64B_NAME	"pit64b"
+
+struct mchp_pit64b_common_data {
+	void __iomem *base;
+	struct clk *pclk;
+	struct clk *gclk;
+	u64 cycles;
+	u8 pres;
+};
+
+struct mchp_pit64b_clksrc_data {
+	struct clocksource *clksrc;
+	struct mchp_pit64b_common_data *cd;
+};
+
+struct mchp_pit64b_clkevt_data {
+	struct clock_event_device *clkevt;
+	struct mchp_pit64b_common_data *cd;
+};
+
+static struct mchp_pit64b_data {
+	struct mchp_pit64b_clksrc_data *csd;
+	struct mchp_pit64b_clkevt_data *ced;
+} data;
+
+static inline u32 mchp_pit64b_read(void __iomem *base, u32 offset)
+{
+	return readl_relaxed(base + offset);
+}
+
+static inline void mchp_pit64b_write(void __iomem *base, u32 offset, u32 val)
+{
+	writel_relaxed(val, base + offset);
+}
+
+static inline u64 mchp_pit64b_get_period(void __iomem *base)
+{
+	u32 lsb, msb;
+
+	/* LSB must be read first to guarantee an atomic read of the 64 bit
+	 * timer.
+	 */
+	lsb = mchp_pit64b_read(base, MCHP_PIT64B_TLSBR);
+	msb = mchp_pit64b_read(base, MCHP_PIT64B_TMSBR);
+
+	return (((u64)msb << 32) | lsb);
+}
+
+static inline void mchp_pit64b_set_period(void __iomem *base, u64 cycles)
+{
+	u32 lsb, msb;
+
+	lsb = cycles & MCHP_PIT64B_LSBMASK;
+	msb = cycles >> 32;
+
+	/* LSB must be write last to guarantee an atomic update of the timer
+	 * even when SMOD=1.
+	 */
+	mchp_pit64b_write(base, MCHP_PIT64B_MSB_PR, msb);
+	mchp_pit64b_write(base, MCHP_PIT64B_LSB_PR, lsb);
+}
+
+static inline void mchp_pit64b_reset(struct mchp_pit64b_common_data *data,
+				     u32 mode, bool irq_ena)
+{
+	mode |= MCHP_PIT64B_PRESCALER(data->pres);
+	if (data->gclk)
+		mode |= MCHP_PIT64B_MR_SGCLK;
+
+	mchp_pit64b_write(data->base, MCHP_PIT64B_CR, MCHP_PIT64B_CR_SWRST);
+	mchp_pit64b_write(data->base, MCHP_PIT64B_MR, mode);
+	mchp_pit64b_set_period(data->base, data->cycles);
+	if (irq_ena)
+		mchp_pit64b_write(data->base, MCHP_PIT64B_IER,
+				  MCHP_PIT64B_IER_PERIOD);
+	mchp_pit64b_write(data->base, MCHP_PIT64B_CR, MCHP_PIT64B_CR_START);
+}
+
+static u64 mchp_pit64b_read_clk(struct clocksource *cs)
+{
+	return mchp_pit64b_get_period(data.csd->cd->base);
+}
+
+static u64 mchp_sched_read_clk(void)
+{
+	return mchp_pit64b_get_period(data.csd->cd->base);
+}
+
+static struct clocksource mchp_pit64b_clksrc = {
+	.name = MCHP_PIT64B_NAME,
+	.mask = CLOCKSOURCE_MASK(64),
+	.flags = CLOCK_SOURCE_IS_CONTINUOUS,
+	.rating = 210,
+	.read = mchp_pit64b_read_clk,
+};
+
+static int mchp_pit64b_clkevt_shutdown(struct clock_event_device *cedev)
+{
+	mchp_pit64b_write(data.ced->cd->base, MCHP_PIT64B_CR,
+			  MCHP_PIT64B_CR_SWRST);
+
+	return 0;
+}
+
+static int mchp_pit64b_clkevt_set_periodic(struct clock_event_device *cedev)
+{
+	mchp_pit64b_reset(data.ced->cd, MCHP_PIT64B_MR_CONT, true);
+
+	return 0;
+}
+
+static int mchp_pit64b_clkevt_set_oneshot(struct clock_event_device *cedev)
+{
+	mchp_pit64b_reset(data.ced->cd, MCHP_PIT64B_MR_SMOD, true);
+
+	return 0;
+}
+
+static int mchp_pit64b_clkevt_set_next_event(unsigned long evt,
+					     struct clock_event_device *cedev)
+{
+	mchp_pit64b_set_period(data.ced->cd->base, evt);
+	mchp_pit64b_write(data.ced->cd->base, MCHP_PIT64B_CR,
+			  MCHP_PIT64B_CR_START);
+
+	return 0;
+}
+
+static void mchp_pit64b_clkevt_suspend(struct clock_event_device *cedev)
+{
+	mchp_pit64b_write(data.ced->cd->base, MCHP_PIT64B_CR,
+			  MCHP_PIT64B_CR_SWRST);
+	if (data.ced->cd->gclk)
+		clk_disable_unprepare(data.ced->cd->gclk);
+	clk_disable_unprepare(data.ced->cd->pclk);
+}
+
+static void mchp_pit64b_clkevt_resume(struct clock_event_device *cedev)
+{
+	u32 mode = MCHP_PIT64B_MR_SMOD;
+
+	clk_prepare_enable(data.ced->cd->pclk);
+	if (data.ced->cd->gclk)
+		clk_prepare_enable(data.ced->cd->gclk);
+
+	if (clockevent_state_periodic(data.ced->clkevt))
+		mode = MCHP_PIT64B_MR_CONT;
+
+	mchp_pit64b_reset(data.ced->cd, mode, true);
+}
+
+static struct clock_event_device mchp_pit64b_clkevt = {
+	.name = MCHP_PIT64B_NAME,
+	.features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC,
+	.rating = 150,
+	.set_state_shutdown = mchp_pit64b_clkevt_shutdown,
+	.set_state_periodic = mchp_pit64b_clkevt_set_periodic,
+	.set_state_oneshot = mchp_pit64b_clkevt_set_oneshot,
+	.set_next_event = mchp_pit64b_clkevt_set_next_event,
+	.suspend = mchp_pit64b_clkevt_suspend,
+	.resume = mchp_pit64b_clkevt_resume,
+};
+
+static irqreturn_t mchp_pit64b_interrupt(int irq, void *dev_id)
+{
+	struct mchp_pit64b_clkevt_data *irq_data = dev_id;
+
+	if (data.ced != irq_data)
+		return IRQ_NONE;
+
+	if (mchp_pit64b_read(irq_data->cd->base, MCHP_PIT64B_ISR) &
+	    MCHP_PIT64B_ISR_PERIOD) {
+		irq_data->clkevt->event_handler(irq_data->clkevt);
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
+static int __init mchp_pit64b_pres_compute(u32 *pres, u32 clk_rate,
+					   u32 max_rate)
+{
+	u32 tmp;
+
+	for (*pres = 0; *pres < MCHP_PIT64B_PRES_MAX; (*pres)++) {
+		tmp = clk_rate / (*pres + 1);
+		if (tmp <= max_rate)
+			break;
+	}
+
+	if (*pres == MCHP_PIT64B_PRES_MAX)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int __init mchp_pit64b_pres_prepare(struct mchp_pit64b_common_data *cd,
+					   unsigned long max_rate)
+{
+	unsigned long pclk_rate, diff = 0, best_diff = ULONG_MAX;
+	long gclk_round = 0;
+	u32 pres, best_pres;
+	int ret = 0;
+
+	pclk_rate = clk_get_rate(cd->pclk);
+	if (!pclk_rate)
+		return -EINVAL;
+
+	if (cd->gclk) {
+		gclk_round = clk_round_rate(cd->gclk, max_rate);
+		if (gclk_round < 0)
+			goto pclk;
+
+		if (pclk_rate / gclk_round < 3)
+			goto pclk;
+
+		ret = mchp_pit64b_pres_compute(&pres, gclk_round, max_rate);
+		if (ret)
+			best_diff = abs(gclk_round - max_rate);
+		else
+			best_diff = abs(gclk_round / (pres + 1) - max_rate);
+		best_pres = pres;
+	}
+
+pclk:
+	/* Check if requested rate could be obtained using PCLK. */
+	ret = mchp_pit64b_pres_compute(&pres, pclk_rate, max_rate);
+	if (ret)
+		diff = abs(pclk_rate - max_rate);
+	else
+		diff = abs(pclk_rate / (pres + 1) - max_rate);
+
+	if (best_diff > diff) {
+		/* Use PCLK. */
+		cd->gclk = NULL;
+		best_pres = pres;
+	} else {
+		clk_set_rate(cd->gclk, gclk_round);
+	}
+
+	cd->pres = best_pres;
+
+	pr_info("PIT64B: using clk=%s with prescaler %u, freq=%lu [Hz]\n",
+		cd->gclk ? "gclk" : "pclk", cd->pres,
+		cd->gclk ? gclk_round / (cd->pres + 1)
+			 : pclk_rate / (cd->pres + 1));
+
+	return 0;
+}
+
+static int __init mchp_pit64b_dt_init_clksrc(struct mchp_pit64b_common_data *cd)
+{
+	struct mchp_pit64b_clksrc_data *csd;
+	unsigned long clk_rate;
+	int ret;
+
+	csd = kzalloc(sizeof(*csd), GFP_KERNEL);
+	if (!csd)
+		return -ENOMEM;
+
+	csd->cd = cd;
+
+	if (csd->cd->gclk)
+		clk_rate = clk_get_rate(csd->cd->gclk);
+	else
+		clk_rate = clk_get_rate(csd->cd->pclk);
+
+	clk_rate = clk_rate / (cd->pres + 1);
+	csd->cd->cycles = ULLONG_MAX;
+	mchp_pit64b_reset(csd->cd, MCHP_PIT64B_MR_CONT, false);
+
+	data.csd = csd;
+
+	csd->clksrc = &mchp_pit64b_clksrc;
+
+	ret = clocksource_register_hz(csd->clksrc, clk_rate);
+	if (ret) {
+		pr_debug("clksrc: Failed to register PIT64B clocksource!\n");
+		goto free;
+	}
+
+	sched_clock_register(mchp_sched_read_clk, 64, clk_rate);
+
+	return 0;
+
+free:
+	kfree(csd);
+	data.csd = NULL;
+
+	return ret;
+}
+
+static int __init mchp_pit64b_dt_init_clkevt(struct mchp_pit64b_common_data *cd,
+					     u32 irq)
+{
+	struct mchp_pit64b_clkevt_data *ced;
+	unsigned long clk_rate;
+	int ret;
+
+	ced = kzalloc(sizeof(*ced), GFP_KERNEL);
+	if (!ced)
+		return -ENOMEM;
+
+	ced->cd = cd;
+
+	if (ced->cd->gclk)
+		clk_rate = clk_get_rate(ced->cd->gclk);
+	else
+		clk_rate = clk_get_rate(ced->cd->pclk);
+
+	clk_rate = clk_rate / (ced->cd->pres + 1);
+	ced->cd->cycles = DIV_ROUND_CLOSEST(clk_rate, HZ);
+
+	ret = request_irq(irq, mchp_pit64b_interrupt, IRQF_TIMER, "pit64b_tick",
+			  ced);
+	if (ret) {
+		pr_debug("clkevt: Failed to setup PIT64B IRQ\n");
+		goto free;
+	}
+
+	data.ced = ced;
+
+	/* Set up and register clockevents. */
+	ced->clkevt = &mchp_pit64b_clkevt;
+	ced->clkevt->cpumask = cpumask_of(0);
+	ced->clkevt->irq = irq;
+	clockevents_config_and_register(ced->clkevt, clk_rate, 1, ULONG_MAX);
+
+	return 0;
+
+free:
+	kfree(ced);
+	data.ced = NULL;
+
+	return ret;
+}
+
+static int __init mchp_pit64b_dt_init(struct device_node *node)
+{
+	struct mchp_pit64b_common_data *cd;
+	u32 irq, freq = MCHP_PIT64B_DEF_FREQ;
+	int ret;
+
+	if (data.csd && data.ced)
+		return -EBUSY;
+
+	cd = kzalloc(sizeof(*cd), GFP_KERNEL);
+	if (!cd)
+		return -ENOMEM;
+
+	cd->pclk = of_clk_get_by_name(node, "pclk");
+	if (IS_ERR(cd->pclk)) {
+		ret = PTR_ERR(cd->pclk);
+		goto free;
+	}
+
+	cd->gclk = of_clk_get_by_name(node, "gclk");
+	if (IS_ERR(cd->gclk))
+		cd->gclk = NULL;
+
+	ret = of_property_read_u32(node, "clock-frequency", &freq);
+	if (ret)
+		pr_debug("PIT64B: failed to read clock frequency. Using default!\n");
+
+	ret = mchp_pit64b_pres_prepare(cd, freq);
+	if (ret)
+		goto free;
+
+	cd->base = of_iomap(node, 0);
+	if (!cd->base) {
+		pr_debug("%s: Could not map PIT64B address!\n",
+			 MCHP_PIT64B_NAME);
+		ret = -ENXIO;
+		goto free;
+	}
+
+	ret = clk_prepare_enable(cd->pclk);
+	if (ret)
+		goto unmap;
+
+	if (cd->gclk) {
+		ret = clk_prepare_enable(cd->gclk);
+		if (ret)
+			goto pclk_unprepare;
+	}
+
+	if (!data.ced) {
+		irq = irq_of_parse_and_map(node, 0);
+		if (!irq) {
+			pr_debug("%s: Failed to get PIT64B clockevent IRQ!\n",
+				 MCHP_PIT64B_NAME);
+			ret = -ENODEV;
+			goto gclk_unprepare;
+		}
+		ret = mchp_pit64b_dt_init_clkevt(cd, irq);
+		if (ret)
+			goto irq_unmap;
+	} else {
+		ret = mchp_pit64b_dt_init_clksrc(cd);
+		if (ret)
+			goto gclk_unprepare;
+	}
+
+	return 0;
+
+irq_unmap:
+	irq_dispose_mapping(irq);
+gclk_unprepare:
+	if (cd->gclk)
+		clk_disable_unprepare(cd->gclk);
+pclk_unprepare:
+	clk_disable_unprepare(cd->pclk);
+unmap:
+	iounmap(cd->base);
+free:
+	kfree(cd);
+
+	return ret;
+}
+
+TIMER_OF_DECLARE(mchp_pit64b_clksrc, "microchip,sam9x60-pit64b",
+		 mchp_pit64b_dt_init);
-- 
2.7.4

[PATCH 3/5] MAINTAINERS: change section name to be more generic

[Claudiu.Beznea]

From: Claudiu Beznea <claudiu.beznea@microchip.com>

Change Microchip timers section name to be more generic.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4d04cebb4a71..0948d6592ea5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10016,7 +10016,7 @@ S:	Supported
 F:	drivers/misc/atmel-ssc.c
 F:	include/linux/atmel-ssc.h
 
-MICROCHIP TIMER COUNTER (TC) AND CLOCKSOURCE DRIVERS
+MICROCHIP TIMERS AND CLOCKSOURCE DRIVERS
 M:	Nicolas Ferre <nicolas.ferre@microchip.com>
 L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 S:	Supported
-- 
2.7.4

[PATCH 4/5] MAINTAINERS: add myself as maintainer

[Claudiu.Beznea]

From: Claudiu Beznea <claudiu.beznea@microchip.com>

Add myself as maintainer for Microchip timers and clocksource
drivers.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0948d6592ea5..85bc819867da 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10018,6 +10018,7 @@ F:	include/linux/atmel-ssc.h
 
 MICROCHIP TIMERS AND CLOCKSOURCE DRIVERS
 M:	Nicolas Ferre <nicolas.ferre@microchip.com>
+M:	Claudiu Beznea <claudiu.beznea@microchip.com>
 L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 S:	Supported
 F:	drivers/misc/atmel_tclib.c
-- 
2.7.4

[PATCH 5/5] MAINTAINERS: add timer-microchip-pit64c.c

[Claudiu.Beznea]

From: Claudiu Beznea <claudiu.beznea@microchip.com>

Add timer-microchip-pit64b.c as maintained file.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 85bc819867da..5af947c9f350 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10023,6 +10023,7 @@ L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 S:	Supported
 F:	drivers/misc/atmel_tclib.c
 F:	drivers/clocksource/tcb_clksrc.c
+F:	drivers/clocksource/timer-microchip-pit64b.c
 
 MICROCHIP USBA UDC DRIVER
 M:	Cristian Birsan <cristian.birsan@microchip.com>
-- 
2.7.4

Re: [PATCH 1/5] dt-bindings: arm: atmel: add bindings for PIT64B

[Rob Herring]

On Thu, 14 Mar 2019 16:26:35 +0000, <Claudiu.Beznea@microchip.com> wrote:
> From: Claudiu Beznea <claudiu.beznea@microchip.com>
> 
> Add device tree bindings for PIT64B timer.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>  Documentation/devicetree/bindings/arm/atmel-sysregs.txt | 7 +++++++
>  1 file changed, 7 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH 2/5] clocksource/drivers/timer-microchip-pit64b: add Microchip PIT64B support

[Nicolas.Ferre]

On 14/03/2019 at 17:26, Claudiu Beznea - M18063 wrote:
> From: Claudiu Beznea <claudiu.beznea@microchip.com>
> 
> Add driver for Microchip PIT64B timer. Timer could be used in continuous
> mode or oneshot mode. The hardware has 2x32 bit registers for period
> emulating a 64 bit timer. The LSB_PR and MSB_PR registers are used to set
> the period value (compare value). TLSB and TMSB keeps the current value
> of the counter. After a compare the TLSB and TMSB register resets. Apart
> from this the hardware has SMOD bit in mode register that allow to
> reconfigure the timer without reset and start commands (start command
> while timer is active is ignored).
> The driver uses PIT64B timer as clocksource or clockevent. First requested
> timer would be registered as clockevent, second one would be registered as
> clocksource. Individual PIT64B hardware resources were used for clocksource
> and clockevent to be able to support high resolution timers with this
> hardware implementation.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>

Look good to me:
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>


> ---
>   drivers/clocksource/Kconfig                  |   6 +
>   drivers/clocksource/Makefile                 |   1 +
>   drivers/clocksource/timer-microchip-pit64b.c | 464 +++++++++++++++++++++++++++
>   3 files changed, 471 insertions(+)
>   create mode 100644 drivers/clocksource/timer-microchip-pit64b.c
> 
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 5d93e580e5dc..2ad6f881a0bb 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -448,6 +448,12 @@ config OXNAS_RPS_TIMER
>   config SYS_SUPPORTS_SH_CMT
>           bool
>   
> +config MICROCHIP_PIT64B
> +	bool "Microchip PIT64B support"
> +	depends on OF || COMPILE_TEST
> +	help
> +	  This option enables Microchip PIT64B timer.
> +
>   config MTK_TIMER
>   	bool "Mediatek timer driver" if COMPILE_TEST
>   	depends on HAS_IOMEM
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index c4a8e9ef932a..c53fa12b9b94 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -35,6 +35,7 @@ obj-$(CONFIG_U300_TIMER)	+= timer-u300.o
>   obj-$(CONFIG_SUN4I_TIMER)	+= timer-sun4i.o
>   obj-$(CONFIG_SUN5I_HSTIMER)	+= timer-sun5i.o
>   obj-$(CONFIG_MESON6_TIMER)	+= timer-meson6.o
> +obj-$(CONFIG_MICROCHIP_PIT64B)	+= timer-microchip-pit64b.o
>   obj-$(CONFIG_TEGRA_TIMER)	+= timer-tegra20.o
>   obj-$(CONFIG_VT8500_TIMER)	+= timer-vt8500.o
>   obj-$(CONFIG_NSPIRE_TIMER)	+= timer-zevio.o
> diff --git a/drivers/clocksource/timer-microchip-pit64b.c b/drivers/clocksource/timer-microchip-pit64b.c
> new file mode 100644
> index 000000000000..6787aa98ef01
> --- /dev/null
> +++ b/drivers/clocksource/timer-microchip-pit64b.c
> @@ -0,0 +1,464 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Copyright (C) 2019 Microchip Technology Inc.
> +// Copyright (C) 2019 Claudiu Beznea (claudiu.beznea@microchip.com)
> +
> +#include <linux/clk.h>
> +#include <linux/clockchips.h>
> +#include <linux/interrupt.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/sched_clock.h>
> +#include <linux/slab.h>
> +
> +#define MCHP_PIT64B_CR		0x00	/* Control Register */
> +#define MCHP_PIT64B_CR_START	BIT(0)
> +#define MCHP_PIT64B_CR_SWRST	BIT(8)
> +
> +#define MCHP_PIT64B_MR		0x04	/* Mode Register */
> +#define MCHP_PIT64B_MR_CONT	BIT(0)
> +#define MCHP_PIT64B_MR_SGCLK	BIT(3)
> +#define MCHP_PIT64B_MR_SMOD	BIT(4)
> +#define MCHP_PIT64B_MR_PRES	GENMASK(11, 8)
> +
> +#define MCHP_PIT64B_LSB_PR	0x08	/* LSB Period Register */
> +
> +#define MCHP_PIT64B_MSB_PR	0x0C	/* MSB Period Register */
> +
> +#define MCHP_PIT64B_IER		0x10	/* Interrupt Enable Register */
> +#define MCHP_PIT64B_IER_PERIOD	BIT(0)
> +
> +#define MCHP_PIT64B_ISR		0x1C	/* Interrupt Status Register */
> +#define MCHP_PIT64B_ISR_PERIOD	BIT(0)
> +
> +#define MCHP_PIT64B_TLSBR	0x20	/* Timer LSB Register */
> +
> +#define MCHP_PIT64B_TMSBR	0x24	/* Timer MSB Register */
> +
> +#define MCHP_PIT64B_PRES_MAX	0x10
> +#define MCHP_PIT64B_DEF_FREQ	2500000UL	/* 2.5 MHz */
> +#define MCHP_PIT64B_LSBMASK	GENMASK_ULL(31, 0)
> +#define MCHP_PIT64B_PRESCALER(p)	(MCHP_PIT64B_MR_PRES & ((p) << 8))
> +
> +#define MCHP_PIT64B_NAME	"pit64b"
> +
> +struct mchp_pit64b_common_data {
> +	void __iomem *base;
> +	struct clk *pclk;
> +	struct clk *gclk;
> +	u64 cycles;
> +	u8 pres;
> +};
> +
> +struct mchp_pit64b_clksrc_data {
> +	struct clocksource *clksrc;
> +	struct mchp_pit64b_common_data *cd;
> +};
> +
> +struct mchp_pit64b_clkevt_data {
> +	struct clock_event_device *clkevt;
> +	struct mchp_pit64b_common_data *cd;
> +};
> +
> +static struct mchp_pit64b_data {
> +	struct mchp_pit64b_clksrc_data *csd;
> +	struct mchp_pit64b_clkevt_data *ced;
> +} data;
> +
> +static inline u32 mchp_pit64b_read(void __iomem *base, u32 offset)
> +{
> +	return readl_relaxed(base + offset);
> +}
> +
> +static inline void mchp_pit64b_write(void __iomem *base, u32 offset, u32 val)
> +{
> +	writel_relaxed(val, base + offset);
> +}
> +
> +static inline u64 mchp_pit64b_get_period(void __iomem *base)
> +{
> +	u32 lsb, msb;
> +
> +	/* LSB must be read first to guarantee an atomic read of the 64 bit
> +	 * timer.
> +	 */
> +	lsb = mchp_pit64b_read(base, MCHP_PIT64B_TLSBR);
> +	msb = mchp_pit64b_read(base, MCHP_PIT64B_TMSBR);
> +
> +	return (((u64)msb << 32) | lsb);
> +}
> +
> +static inline void mchp_pit64b_set_period(void __iomem *base, u64 cycles)
> +{
> +	u32 lsb, msb;
> +
> +	lsb = cycles & MCHP_PIT64B_LSBMASK;
> +	msb = cycles >> 32;
> +
> +	/* LSB must be write last to guarantee an atomic update of the timer
> +	 * even when SMOD=1.
> +	 */
> +	mchp_pit64b_write(base, MCHP_PIT64B_MSB_PR, msb);
> +	mchp_pit64b_write(base, MCHP_PIT64B_LSB_PR, lsb);
> +}
> +
> +static inline void mchp_pit64b_reset(struct mchp_pit64b_common_data *data,
> +				     u32 mode, bool irq_ena)
> +{
> +	mode |= MCHP_PIT64B_PRESCALER(data->pres);
> +	if (data->gclk)
> +		mode |= MCHP_PIT64B_MR_SGCLK;
> +
> +	mchp_pit64b_write(data->base, MCHP_PIT64B_CR, MCHP_PIT64B_CR_SWRST);
> +	mchp_pit64b_write(data->base, MCHP_PIT64B_MR, mode);
> +	mchp_pit64b_set_period(data->base, data->cycles);
> +	if (irq_ena)
> +		mchp_pit64b_write(data->base, MCHP_PIT64B_IER,
> +				  MCHP_PIT64B_IER_PERIOD);
> +	mchp_pit64b_write(data->base, MCHP_PIT64B_CR, MCHP_PIT64B_CR_START);
> +}
> +
> +static u64 mchp_pit64b_read_clk(struct clocksource *cs)
> +{
> +	return mchp_pit64b_get_period(data.csd->cd->base);
> +}
> +
> +static u64 mchp_sched_read_clk(void)
> +{
> +	return mchp_pit64b_get_period(data.csd->cd->base);
> +}
> +
> +static struct clocksource mchp_pit64b_clksrc = {
> +	.name = MCHP_PIT64B_NAME,
> +	.mask = CLOCKSOURCE_MASK(64),
> +	.flags = CLOCK_SOURCE_IS_CONTINUOUS,
> +	.rating = 210,
> +	.read = mchp_pit64b_read_clk,
> +};
> +
> +static int mchp_pit64b_clkevt_shutdown(struct clock_event_device *cedev)
> +{
> +	mchp_pit64b_write(data.ced->cd->base, MCHP_PIT64B_CR,
> +			  MCHP_PIT64B_CR_SWRST);
> +
> +	return 0;
> +}
> +
> +static int mchp_pit64b_clkevt_set_periodic(struct clock_event_device *cedev)
> +{
> +	mchp_pit64b_reset(data.ced->cd, MCHP_PIT64B_MR_CONT, true);
> +
> +	return 0;
> +}
> +
> +static int mchp_pit64b_clkevt_set_oneshot(struct clock_event_device *cedev)
> +{
> +	mchp_pit64b_reset(data.ced->cd, MCHP_PIT64B_MR_SMOD, true);
> +
> +	return 0;
> +}
> +
> +static int mchp_pit64b_clkevt_set_next_event(unsigned long evt,
> +					     struct clock_event_device *cedev)
> +{
> +	mchp_pit64b_set_period(data.ced->cd->base, evt);
> +	mchp_pit64b_write(data.ced->cd->base, MCHP_PIT64B_CR,
> +			  MCHP_PIT64B_CR_START);
> +
> +	return 0;
> +}
> +
> +static void mchp_pit64b_clkevt_suspend(struct clock_event_device *cedev)
> +{
> +	mchp_pit64b_write(data.ced->cd->base, MCHP_PIT64B_CR,
> +			  MCHP_PIT64B_CR_SWRST);
> +	if (data.ced->cd->gclk)
> +		clk_disable_unprepare(data.ced->cd->gclk);
> +	clk_disable_unprepare(data.ced->cd->pclk);
> +}
> +
> +static void mchp_pit64b_clkevt_resume(struct clock_event_device *cedev)
> +{
> +	u32 mode = MCHP_PIT64B_MR_SMOD;
> +
> +	clk_prepare_enable(data.ced->cd->pclk);
> +	if (data.ced->cd->gclk)
> +		clk_prepare_enable(data.ced->cd->gclk);
> +
> +	if (clockevent_state_periodic(data.ced->clkevt))
> +		mode = MCHP_PIT64B_MR_CONT;
> +
> +	mchp_pit64b_reset(data.ced->cd, mode, true);
> +}
> +
> +static struct clock_event_device mchp_pit64b_clkevt = {
> +	.name = MCHP_PIT64B_NAME,
> +	.features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC,
> +	.rating = 150,
> +	.set_state_shutdown = mchp_pit64b_clkevt_shutdown,
> +	.set_state_periodic = mchp_pit64b_clkevt_set_periodic,
> +	.set_state_oneshot = mchp_pit64b_clkevt_set_oneshot,
> +	.set_next_event = mchp_pit64b_clkevt_set_next_event,
> +	.suspend = mchp_pit64b_clkevt_suspend,
> +	.resume = mchp_pit64b_clkevt_resume,
> +};
> +
> +static irqreturn_t mchp_pit64b_interrupt(int irq, void *dev_id)
> +{
> +	struct mchp_pit64b_clkevt_data *irq_data = dev_id;
> +
> +	if (data.ced != irq_data)
> +		return IRQ_NONE;
> +
> +	if (mchp_pit64b_read(irq_data->cd->base, MCHP_PIT64B_ISR) &
> +	    MCHP_PIT64B_ISR_PERIOD) {
> +		irq_data->clkevt->event_handler(irq_data->clkevt);
> +		return IRQ_HANDLED;
> +	}
> +
> +	return IRQ_NONE;
> +}
> +
> +static int __init mchp_pit64b_pres_compute(u32 *pres, u32 clk_rate,
> +					   u32 max_rate)
> +{
> +	u32 tmp;
> +
> +	for (*pres = 0; *pres < MCHP_PIT64B_PRES_MAX; (*pres)++) {
> +		tmp = clk_rate / (*pres + 1);
> +		if (tmp <= max_rate)
> +			break;
> +	}
> +
> +	if (*pres == MCHP_PIT64B_PRES_MAX)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int __init mchp_pit64b_pres_prepare(struct mchp_pit64b_common_data *cd,
> +					   unsigned long max_rate)
> +{
> +	unsigned long pclk_rate, diff = 0, best_diff = ULONG_MAX;
> +	long gclk_round = 0;
> +	u32 pres, best_pres;
> +	int ret = 0;
> +
> +	pclk_rate = clk_get_rate(cd->pclk);
> +	if (!pclk_rate)
> +		return -EINVAL;
> +
> +	if (cd->gclk) {
> +		gclk_round = clk_round_rate(cd->gclk, max_rate);
> +		if (gclk_round < 0)
> +			goto pclk;
> +
> +		if (pclk_rate / gclk_round < 3)
> +			goto pclk;
> +
> +		ret = mchp_pit64b_pres_compute(&pres, gclk_round, max_rate);
> +		if (ret)
> +			best_diff = abs(gclk_round - max_rate);
> +		else
> +			best_diff = abs(gclk_round / (pres + 1) - max_rate);
> +		best_pres = pres;
> +	}
> +
> +pclk:
> +	/* Check if requested rate could be obtained using PCLK. */
> +	ret = mchp_pit64b_pres_compute(&pres, pclk_rate, max_rate);
> +	if (ret)
> +		diff = abs(pclk_rate - max_rate);
> +	else
> +		diff = abs(pclk_rate / (pres + 1) - max_rate);
> +
> +	if (best_diff > diff) {
> +		/* Use PCLK. */
> +		cd->gclk = NULL;
> +		best_pres = pres;
> +	} else {
> +		clk_set_rate(cd->gclk, gclk_round);
> +	}
> +
> +	cd->pres = best_pres;
> +
> +	pr_info("PIT64B: using clk=%s with prescaler %u, freq=%lu [Hz]\n",
> +		cd->gclk ? "gclk" : "pclk", cd->pres,
> +		cd->gclk ? gclk_round / (cd->pres + 1)
> +			 : pclk_rate / (cd->pres + 1));
> +
> +	return 0;
> +}
> +
> +static int __init mchp_pit64b_dt_init_clksrc(struct mchp_pit64b_common_data *cd)
> +{
> +	struct mchp_pit64b_clksrc_data *csd;
> +	unsigned long clk_rate;
> +	int ret;
> +
> +	csd = kzalloc(sizeof(*csd), GFP_KERNEL);
> +	if (!csd)
> +		return -ENOMEM;
> +
> +	csd->cd = cd;
> +
> +	if (csd->cd->gclk)
> +		clk_rate = clk_get_rate(csd->cd->gclk);
> +	else
> +		clk_rate = clk_get_rate(csd->cd->pclk);
> +
> +	clk_rate = clk_rate / (cd->pres + 1);
> +	csd->cd->cycles = ULLONG_MAX;
> +	mchp_pit64b_reset(csd->cd, MCHP_PIT64B_MR_CONT, false);
> +
> +	data.csd = csd;
> +
> +	csd->clksrc = &mchp_pit64b_clksrc;
> +
> +	ret = clocksource_register_hz(csd->clksrc, clk_rate);
> +	if (ret) {
> +		pr_debug("clksrc: Failed to register PIT64B clocksource!\n");
> +		goto free;
> +	}
> +
> +	sched_clock_register(mchp_sched_read_clk, 64, clk_rate);
> +
> +	return 0;
> +
> +free:
> +	kfree(csd);
> +	data.csd = NULL;
> +
> +	return ret;
> +}
> +
> +static int __init mchp_pit64b_dt_init_clkevt(struct mchp_pit64b_common_data *cd,
> +					     u32 irq)
> +{
> +	struct mchp_pit64b_clkevt_data *ced;
> +	unsigned long clk_rate;
> +	int ret;
> +
> +	ced = kzalloc(sizeof(*ced), GFP_KERNEL);
> +	if (!ced)
> +		return -ENOMEM;
> +
> +	ced->cd = cd;
> +
> +	if (ced->cd->gclk)
> +		clk_rate = clk_get_rate(ced->cd->gclk);
> +	else
> +		clk_rate = clk_get_rate(ced->cd->pclk);
> +
> +	clk_rate = clk_rate / (ced->cd->pres + 1);
> +	ced->cd->cycles = DIV_ROUND_CLOSEST(clk_rate, HZ);
> +
> +	ret = request_irq(irq, mchp_pit64b_interrupt, IRQF_TIMER, "pit64b_tick",
> +			  ced);
> +	if (ret) {
> +		pr_debug("clkevt: Failed to setup PIT64B IRQ\n");
> +		goto free;
> +	}
> +
> +	data.ced = ced;
> +
> +	/* Set up and register clockevents. */
> +	ced->clkevt = &mchp_pit64b_clkevt;
> +	ced->clkevt->cpumask = cpumask_of(0);
> +	ced->clkevt->irq = irq;
> +	clockevents_config_and_register(ced->clkevt, clk_rate, 1, ULONG_MAX);
> +
> +	return 0;
> +
> +free:
> +	kfree(ced);
> +	data.ced = NULL;
> +
> +	return ret;
> +}
> +
> +static int __init mchp_pit64b_dt_init(struct device_node *node)
> +{
> +	struct mchp_pit64b_common_data *cd;
> +	u32 irq, freq = MCHP_PIT64B_DEF_FREQ;
> +	int ret;
> +
> +	if (data.csd && data.ced)
> +		return -EBUSY;
> +
> +	cd = kzalloc(sizeof(*cd), GFP_KERNEL);
> +	if (!cd)
> +		return -ENOMEM;
> +
> +	cd->pclk = of_clk_get_by_name(node, "pclk");
> +	if (IS_ERR(cd->pclk)) {
> +		ret = PTR_ERR(cd->pclk);
> +		goto free;
> +	}
> +
> +	cd->gclk = of_clk_get_by_name(node, "gclk");
> +	if (IS_ERR(cd->gclk))
> +		cd->gclk = NULL;
> +
> +	ret = of_property_read_u32(node, "clock-frequency", &freq);
> +	if (ret)
> +		pr_debug("PIT64B: failed to read clock frequency. Using default!\n");
> +
> +	ret = mchp_pit64b_pres_prepare(cd, freq);
> +	if (ret)
> +		goto free;
> +
> +	cd->base = of_iomap(node, 0);
> +	if (!cd->base) {
> +		pr_debug("%s: Could not map PIT64B address!\n",
> +			 MCHP_PIT64B_NAME);
> +		ret = -ENXIO;
> +		goto free;
> +	}
> +
> +	ret = clk_prepare_enable(cd->pclk);
> +	if (ret)
> +		goto unmap;
> +
> +	if (cd->gclk) {
> +		ret = clk_prepare_enable(cd->gclk);
> +		if (ret)
> +			goto pclk_unprepare;
> +	}
> +
> +	if (!data.ced) {
> +		irq = irq_of_parse_and_map(node, 0);
> +		if (!irq) {
> +			pr_debug("%s: Failed to get PIT64B clockevent IRQ!\n",
> +				 MCHP_PIT64B_NAME);
> +			ret = -ENODEV;
> +			goto gclk_unprepare;
> +		}
> +		ret = mchp_pit64b_dt_init_clkevt(cd, irq);
> +		if (ret)
> +			goto irq_unmap;
> +	} else {
> +		ret = mchp_pit64b_dt_init_clksrc(cd);
> +		if (ret)
> +			goto gclk_unprepare;
> +	}
> +
> +	return 0;
> +
> +irq_unmap:
> +	irq_dispose_mapping(irq);
> +gclk_unprepare:
> +	if (cd->gclk)
> +		clk_disable_unprepare(cd->gclk);
> +pclk_unprepare:
> +	clk_disable_unprepare(cd->pclk);
> +unmap:
> +	iounmap(cd->base);
> +free:
> +	kfree(cd);
> +
> +	return ret;
> +}
> +
> +TIMER_OF_DECLARE(mchp_pit64b_clksrc, "microchip,sam9x60-pit64b",
> +		 mchp_pit64b_dt_init);
> 


-- 
Nicolas Ferre

Re: [PATCH 1/5] dt-bindings: arm: atmel: add bindings for PIT64B

[Nicolas.Ferre]

On 14/03/2019 at 17:26, Claudiu Beznea - M18063 wrote:
> From: Claudiu Beznea <claudiu.beznea@microchip.com>
> 
> Add device tree bindings for PIT64B timer.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>

Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

> ---
>   Documentation/devicetree/bindings/arm/atmel-sysregs.txt | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/atmel-sysregs.txt b/Documentation/devicetree/bindings/arm/atmel-sysregs.txt
> index 14f319f694b7..c7b7ead7966b 100644
> --- a/Documentation/devicetree/bindings/arm/atmel-sysregs.txt
> +++ b/Documentation/devicetree/bindings/arm/atmel-sysregs.txt
> @@ -10,6 +10,13 @@ PIT Timer required properties:
>   - interrupts: Should contain interrupt for the PIT which is the IRQ line
>     shared across all System Controller members.
>   
> +PIT64B Timer required properties:
> +- compatible: Should be "microchip,sam9x60-pit64b"
> +- reg: Should contain registers location and length
> +- clock-frequency: Should contain PIT64B timer frequency
> +- interrupts: Should contain interrupt for PIT64B timer
> +- clocks: Should contain the available clock sources for PIT64B timer.
> +
>   System Timer (ST) required properties:
>   - compatible: Should be "atmel,at91rm9200-st", "syscon", "simple-mfd"
>   - reg: Should contain registers location and length
> 


-- 
Nicolas Ferre

Re: [PATCH 3/5] MAINTAINERS: change section name to be more generic

[Nicolas.Ferre]

On 14/03/2019 at 17:26, Claudiu Beznea - M18063 wrote:
> From: Claudiu Beznea <claudiu.beznea@microchip.com>
> 
> Change Microchip timers section name to be more generic.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>

Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

> ---
>   MAINTAINERS | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4d04cebb4a71..0948d6592ea5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10016,7 +10016,7 @@ S:	Supported
>   F:	drivers/misc/atmel-ssc.c
>   F:	include/linux/atmel-ssc.h
>   
> -MICROCHIP TIMER COUNTER (TC) AND CLOCKSOURCE DRIVERS
> +MICROCHIP TIMERS AND CLOCKSOURCE DRIVERS
>   M:	Nicolas Ferre <nicolas.ferre@microchip.com>
>   L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
>   S:	Supported
> 


-- 
Nicolas Ferre

Re: [PATCH 4/5] MAINTAINERS: add myself as maintainer

[Nicolas.Ferre]

On 14/03/2019 at 17:26, Claudiu Beznea - M18063 wrote:
> From: Claudiu Beznea <claudiu.beznea@microchip.com>
> 
> Add myself as maintainer for Microchip timers and clocksource
> drivers.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>

Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

Thanks Claudiu!

> ---
>   MAINTAINERS | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0948d6592ea5..85bc819867da 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10018,6 +10018,7 @@ F:	include/linux/atmel-ssc.h
>   
>   MICROCHIP TIMERS AND CLOCKSOURCE DRIVERS
>   M:	Nicolas Ferre <nicolas.ferre@microchip.com>
> +M:	Claudiu Beznea <claudiu.beznea@microchip.com>
>   L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
>   S:	Supported
>   F:	drivers/misc/atmel_tclib.c
> 


-- 
Nicolas Ferre

Re: [PATCH 5/5] MAINTAINERS: add timer-microchip-pit64c.c

[Nicolas.Ferre]

On 14/03/2019 at 17:26, Claudiu Beznea - M18063 wrote:
> From: Claudiu Beznea <claudiu.beznea@microchip.com>
> 
> Add timer-microchip-pit64b.c as maintained file.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>

Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

> ---
>   MAINTAINERS | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 85bc819867da..5af947c9f350 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10023,6 +10023,7 @@ L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
>   S:	Supported
>   F:	drivers/misc/atmel_tclib.c
>   F:	drivers/clocksource/tcb_clksrc.c
> +F:	drivers/clocksource/timer-microchip-pit64b.c
>   
>   MICROCHIP USBA UDC DRIVER
>   M:	Cristian Birsan <cristian.birsan@microchip.com>
> 


-- 
Nicolas Ferre

Re: [PATCH 2/5] clocksource/drivers/timer-microchip-pit64b: add Microchip PIT64B support

[Daniel Lezcano]

Hi Claudiu,

On 14/03/2019 17:26, Claudiu.Beznea@microchip.com wrote:
> From: Claudiu Beznea <claudiu.beznea@microchip.com>
> 
> Add driver for Microchip PIT64B timer. Timer could be used in continuous
> mode or oneshot mode. The hardware has 2x32 bit registers for period
> emulating a 64 bit timer. The LSB_PR and MSB_PR registers are used to set
> the period value (compare value). TLSB and TMSB keeps the current value
> of the counter. After a compare the TLSB and TMSB register resets. Apart
> from this the hardware has SMOD bit in mode register that allow to
> reconfigure the timer without reset and start commands (start command
> while timer is active is ignored).
> The driver uses PIT64B timer as clocksource or clockevent. First requested
> timer would be registered as clockevent, second one would be registered as
> clocksource.

Even if that was done this way before, assuming the DT describes the
clockevent at the first place and then the clocksource, it is a fragile
approach.

What about using one of these approach?

eg.

arch/arm/boot/dts/at91sam9261ek.dts

chosen {
	bootargs = "rootfstype=ubifs ubi.mtd=5 root=ubi0:rootfs rw";
                stdout-path = "serial0:115200n8";

	clocksource {
		timer = <&timer0>;
	};

        clockevent {
		timer = <&timer1>;
        };
};

or

arch/arm/boot/dts/integratorap.dts

aliases {
	arm,timer-primary = &timer2;
	arm,timer-secondary = &timer1;
};

So we can have control of what is the clocksource or the clockevent.
That is particulary handy in case of multiple channels.

Not sure if we can replace the 'arm,timer_primary' to 'clocksource'.

Rob? What is your opinion?

> Individual PIT64B hardware resources were used for clocksource
> and clockevent to be able to support high resolution timers with this
> hardware implementation.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>  drivers/clocksource/Kconfig                  |   6 +
>  drivers/clocksource/Makefile                 |   1 +
>  drivers/clocksource/timer-microchip-pit64b.c | 464 +++++++++++++++++++++++++++
>  3 files changed, 471 insertions(+)
>  create mode 100644 drivers/clocksource/timer-microchip-pit64b.c
> 
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 5d93e580e5dc..2ad6f881a0bb 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -448,6 +448,12 @@ config OXNAS_RPS_TIMER
>  config SYS_SUPPORTS_SH_CMT
>          bool
>  
> +config MICROCHIP_PIT64B
> +	bool "Microchip PIT64B support"
> +	depends on OF || COMPILE_TEST
> +	help
> +	  This option enables Microchip PIT64B timer.
> +
>  config MTK_TIMER
>  	bool "Mediatek timer driver" if COMPILE_TEST
>  	depends on HAS_IOMEM
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index c4a8e9ef932a..c53fa12b9b94 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -35,6 +35,7 @@ obj-$(CONFIG_U300_TIMER)	+= timer-u300.o
>  obj-$(CONFIG_SUN4I_TIMER)	+= timer-sun4i.o
>  obj-$(CONFIG_SUN5I_HSTIMER)	+= timer-sun5i.o
>  obj-$(CONFIG_MESON6_TIMER)	+= timer-meson6.o
> +obj-$(CONFIG_MICROCHIP_PIT64B)	+= timer-microchip-pit64b.o
>  obj-$(CONFIG_TEGRA_TIMER)	+= timer-tegra20.o
>  obj-$(CONFIG_VT8500_TIMER)	+= timer-vt8500.o
>  obj-$(CONFIG_NSPIRE_TIMER)	+= timer-zevio.o
> diff --git a/drivers/clocksource/timer-microchip-pit64b.c b/drivers/clocksource/timer-microchip-pit64b.c
> new file mode 100644
> index 000000000000..6787aa98ef01
> --- /dev/null
> +++ b/drivers/clocksource/timer-microchip-pit64b.c
> @@ -0,0 +1,464 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Copyright (C) 2019 Microchip Technology Inc.
> +// Copyright (C) 2019 Claudiu Beznea (claudiu.beznea@microchip.com)
> +
> +#include <linux/clk.h>
> +#include <linux/clockchips.h>
> +#include <linux/interrupt.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/sched_clock.h>
> +#include <linux/slab.h>
> +
> +#define MCHP_PIT64B_CR		0x00	/* Control Register */
> +#define MCHP_PIT64B_CR_START	BIT(0)
> +#define MCHP_PIT64B_CR_SWRST	BIT(8)
> +
> +#define MCHP_PIT64B_MR		0x04	/* Mode Register */
> +#define MCHP_PIT64B_MR_CONT	BIT(0)
> +#define MCHP_PIT64B_MR_SGCLK	BIT(3)
> +#define MCHP_PIT64B_MR_SMOD	BIT(4)
> +#define MCHP_PIT64B_MR_PRES	GENMASK(11, 8)
> +
> +#define MCHP_PIT64B_LSB_PR	0x08	/* LSB Period Register */
> +
> +#define MCHP_PIT64B_MSB_PR	0x0C	/* MSB Period Register */
> +
> +#define MCHP_PIT64B_IER		0x10	/* Interrupt Enable Register */
> +#define MCHP_PIT64B_IER_PERIOD	BIT(0)
> +
> +#define MCHP_PIT64B_ISR		0x1C	/* Interrupt Status Register */
> +#define MCHP_PIT64B_ISR_PERIOD	BIT(0)
> +
> +#define MCHP_PIT64B_TLSBR	0x20	/* Timer LSB Register */
> +
> +#define MCHP_PIT64B_TMSBR	0x24	/* Timer MSB Register */
> +
> +#define MCHP_PIT64B_PRES_MAX	0x10
> +#define MCHP_PIT64B_DEF_FREQ	2500000UL	/* 2.5 MHz */
> +#define MCHP_PIT64B_LSBMASK	GENMASK_ULL(31, 0)
> +#define MCHP_PIT64B_PRESCALER(p)	(MCHP_PIT64B_MR_PRES & ((p) << 8))
> +
> +#define MCHP_PIT64B_NAME	"pit64b"
> +
> +struct mchp_pit64b_common_data {
> +	void __iomem *base;
> +	struct clk *pclk;
> +	struct clk *gclk;
> +	u64 cycles;
> +	u8 pres;
> +};
> +
> +struct mchp_pit64b_clksrc_data {
> +	struct clocksource *clksrc;
> +	struct mchp_pit64b_common_data *cd;
> +};
> +
> +struct mchp_pit64b_clkevt_data {
> +	struct clock_event_device *clkevt;
> +	struct mchp_pit64b_common_data *cd;
> +};
> +
> +static struct mchp_pit64b_data {
> +	struct mchp_pit64b_clksrc_data *csd;
> +	struct mchp_pit64b_clkevt_data *ced;
> +} data;
> +
> +static inline u32 mchp_pit64b_read(void __iomem *base, u32 offset)
> +{
> +	return readl_relaxed(base + offset);
> +}
> +
> +static inline void mchp_pit64b_write(void __iomem *base, u32 offset, u32 val)
> +{
> +	writel_relaxed(val, base + offset);
> +}
> +
> +static inline u64 mchp_pit64b_get_period(void __iomem *base)
> +{
> +	u32 lsb, msb;
> +
> +	/* LSB must be read first to guarantee an atomic read of the 64 bit
> +	 * timer.
> +	 */
> +	lsb = mchp_pit64b_read(base, MCHP_PIT64B_TLSBR);
> +	msb = mchp_pit64b_read(base, MCHP_PIT64B_TMSBR);
> +
> +	return (((u64)msb << 32) | lsb);
> +}
> +
> +static inline void mchp_pit64b_set_period(void __iomem *base, u64 cycles)
> +{
> +	u32 lsb, msb;
> +
> +	lsb = cycles & MCHP_PIT64B_LSBMASK;
> +	msb = cycles >> 32;
> +
> +	/* LSB must be write last to guarantee an atomic update of the timer
> +	 * even when SMOD=1.
> +	 */
> +	mchp_pit64b_write(base, MCHP_PIT64B_MSB_PR, msb);
> +	mchp_pit64b_write(base, MCHP_PIT64B_LSB_PR, lsb);
> +}
> +
> +static inline void mchp_pit64b_reset(struct mchp_pit64b_common_data *data,
> +				     u32 mode, bool irq_ena)
> +{
> +	mode |= MCHP_PIT64B_PRESCALER(data->pres);
> +	if (data->gclk)
> +		mode |= MCHP_PIT64B_MR_SGCLK;
> +
> +	mchp_pit64b_write(data->base, MCHP_PIT64B_CR, MCHP_PIT64B_CR_SWRST);
> +	mchp_pit64b_write(data->base, MCHP_PIT64B_MR, mode);
> +	mchp_pit64b_set_period(data->base, data->cycles);
> +	if (irq_ena)
> +		mchp_pit64b_write(data->base, MCHP_PIT64B_IER,
> +				  MCHP_PIT64B_IER_PERIOD);
> +	mchp_pit64b_write(data->base, MCHP_PIT64B_CR, MCHP_PIT64B_CR_START);
> +}
> +
> +static u64 mchp_pit64b_read_clk(struct clocksource *cs)
> +{
> +	return mchp_pit64b_get_period(data.csd->cd->base);
> +}
> +
> +static u64 mchp_sched_read_clk(void)
> +{
> +	return mchp_pit64b_get_period(data.csd->cd->base);
> +}
> +
> +static struct clocksource mchp_pit64b_clksrc = {
> +	.name = MCHP_PIT64B_NAME,
> +	.mask = CLOCKSOURCE_MASK(64),
> +	.flags = CLOCK_SOURCE_IS_CONTINUOUS,
> +	.rating = 210,
> +	.read = mchp_pit64b_read_clk,
> +};
> +
> +static int mchp_pit64b_clkevt_shutdown(struct clock_event_device *cedev)
> +{
> +	mchp_pit64b_write(data.ced->cd->base, MCHP_PIT64B_CR,
> +			  MCHP_PIT64B_CR_SWRST);
> +
> +	return 0;
> +}
> +
> +static int mchp_pit64b_clkevt_set_periodic(struct clock_event_device *cedev)
> +{
> +	mchp_pit64b_reset(data.ced->cd, MCHP_PIT64B_MR_CONT, true);
> +
> +	return 0;
> +}
> +
> +static int mchp_pit64b_clkevt_set_oneshot(struct clock_event_device *cedev)
> +{
> +	mchp_pit64b_reset(data.ced->cd, MCHP_PIT64B_MR_SMOD, true);
> +
> +	return 0;
> +}
> +
> +static int mchp_pit64b_clkevt_set_next_event(unsigned long evt,
> +					     struct clock_event_device *cedev)
> +{
> +	mchp_pit64b_set_period(data.ced->cd->base, evt);
> +	mchp_pit64b_write(data.ced->cd->base, MCHP_PIT64B_CR,
> +			  MCHP_PIT64B_CR_START);
> +
> +	return 0;
> +}
> +
> +static void mchp_pit64b_clkevt_suspend(struct clock_event_device *cedev)
> +{
> +	mchp_pit64b_write(data.ced->cd->base, MCHP_PIT64B_CR,
> +			  MCHP_PIT64B_CR_SWRST);
> +	if (data.ced->cd->gclk)
> +		clk_disable_unprepare(data.ced->cd->gclk);
> +	clk_disable_unprepare(data.ced->cd->pclk);
> +}
> +
> +static void mchp_pit64b_clkevt_resume(struct clock_event_device *cedev)
> +{
> +	u32 mode = MCHP_PIT64B_MR_SMOD;
> +
> +	clk_prepare_enable(data.ced->cd->pclk);
> +	if (data.ced->cd->gclk)
> +		clk_prepare_enable(data.ced->cd->gclk);
> +
> +	if (clockevent_state_periodic(data.ced->clkevt))
> +		mode = MCHP_PIT64B_MR_CONT;
> +
> +	mchp_pit64b_reset(data.ced->cd, mode, true);
> +}
> +
> +static struct clock_event_device mchp_pit64b_clkevt = {
> +	.name = MCHP_PIT64B_NAME,
> +	.features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC,
> +	.rating = 150,
> +	.set_state_shutdown = mchp_pit64b_clkevt_shutdown,
> +	.set_state_periodic = mchp_pit64b_clkevt_set_periodic,
> +	.set_state_oneshot = mchp_pit64b_clkevt_set_oneshot,
> +	.set_next_event = mchp_pit64b_clkevt_set_next_event,
> +	.suspend = mchp_pit64b_clkevt_suspend,
> +	.resume = mchp_pit64b_clkevt_resume,
> +};
> +
> +static irqreturn_t mchp_pit64b_interrupt(int irq, void *dev_id)
> +{
> +	struct mchp_pit64b_clkevt_data *irq_data = dev_id;
> +
> +	if (data.ced != irq_data)
> +		return IRQ_NONE;
> +
> +	if (mchp_pit64b_read(irq_data->cd->base, MCHP_PIT64B_ISR) &
> +	    MCHP_PIT64B_ISR_PERIOD) {
> +		irq_data->clkevt->event_handler(irq_data->clkevt);
> +		return IRQ_HANDLED;
> +	}
> +
> +	return IRQ_NONE;
> +}
> +
> +static int __init mchp_pit64b_pres_compute(u32 *pres, u32 clk_rate,
> +					   u32 max_rate)
> +{
> +	u32 tmp;
> +
> +	for (*pres = 0; *pres < MCHP_PIT64B_PRES_MAX; (*pres)++) {
> +		tmp = clk_rate / (*pres + 1);
> +		if (tmp <= max_rate)
> +			break;
> +	}
> +
> +	if (*pres == MCHP_PIT64B_PRES_MAX)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int __init mchp_pit64b_pres_prepare(struct mchp_pit64b_common_data *cd,
> +					   unsigned long max_rate)
> +{
> +	unsigned long pclk_rate, diff = 0, best_diff = ULONG_MAX;
> +	long gclk_round = 0;
> +	u32 pres, best_pres;
> +	int ret = 0;
> +
> +	pclk_rate = clk_get_rate(cd->pclk);
> +	if (!pclk_rate)
> +		return -EINVAL;
> +
> +	if (cd->gclk) {
> +		gclk_round = clk_round_rate(cd->gclk, max_rate);
> +		if (gclk_round < 0)
> +			goto pclk;
> +
> +		if (pclk_rate / gclk_round < 3)
> +			goto pclk;
> +
> +		ret = mchp_pit64b_pres_compute(&pres, gclk_round, max_rate);
> +		if (ret)
> +			best_diff = abs(gclk_round - max_rate);
> +		else
> +			best_diff = abs(gclk_round / (pres + 1) - max_rate);
> +		best_pres = pres;
> +	}
> +
> +pclk:
> +	/* Check if requested rate could be obtained using PCLK. */
> +	ret = mchp_pit64b_pres_compute(&pres, pclk_rate, max_rate);
> +	if (ret)
> +		diff = abs(pclk_rate - max_rate);
> +	else
> +		diff = abs(pclk_rate / (pres + 1) - max_rate);
> +
> +	if (best_diff > diff) {
> +		/* Use PCLK. */
> +		cd->gclk = NULL;
> +		best_pres = pres;
> +	} else {
> +		clk_set_rate(cd->gclk, gclk_round);
> +	}
> +
> +	cd->pres = best_pres;
> +
> +	pr_info("PIT64B: using clk=%s with prescaler %u, freq=%lu [Hz]\n",
> +		cd->gclk ? "gclk" : "pclk", cd->pres,
> +		cd->gclk ? gclk_round / (cd->pres + 1)
> +			 : pclk_rate / (cd->pres + 1));
> +
> +	return 0;
> +}
> +
> +static int __init mchp_pit64b_dt_init_clksrc(struct mchp_pit64b_common_data *cd)
> +{
> +	struct mchp_pit64b_clksrc_data *csd;
> +	unsigned long clk_rate;
> +	int ret;
> +
> +	csd = kzalloc(sizeof(*csd), GFP_KERNEL);
> +	if (!csd)
> +		return -ENOMEM;
> +
> +	csd->cd = cd;
> +
> +	if (csd->cd->gclk)
> +		clk_rate = clk_get_rate(csd->cd->gclk);
> +	else
> +		clk_rate = clk_get_rate(csd->cd->pclk);
> +
> +	clk_rate = clk_rate / (cd->pres + 1);
> +	csd->cd->cycles = ULLONG_MAX;
> +	mchp_pit64b_reset(csd->cd, MCHP_PIT64B_MR_CONT, false);
> +
> +	data.csd = csd;
> +
> +	csd->clksrc = &mchp_pit64b_clksrc;
> +
> +	ret = clocksource_register_hz(csd->clksrc, clk_rate);
> +	if (ret) {
> +		pr_debug("clksrc: Failed to register PIT64B clocksource!\n");
> +		goto free;
> +	}
> +
> +	sched_clock_register(mchp_sched_read_clk, 64, clk_rate);
> +
> +	return 0;
> +
> +free:
> +	kfree(csd);
> +	data.csd = NULL;
> +
> +	return ret;
> +}
> +
> +static int __init mchp_pit64b_dt_init_clkevt(struct mchp_pit64b_common_data *cd,
> +					     u32 irq)
> +{
> +	struct mchp_pit64b_clkevt_data *ced;
> +	unsigned long clk_rate;
> +	int ret;
> +
> +	ced = kzalloc(sizeof(*ced), GFP_KERNEL);
> +	if (!ced)
> +		return -ENOMEM;
> +
> +	ced->cd = cd;
> +
> +	if (ced->cd->gclk)
> +		clk_rate = clk_get_rate(ced->cd->gclk);
> +	else
> +		clk_rate = clk_get_rate(ced->cd->pclk);
> +
> +	clk_rate = clk_rate / (ced->cd->pres + 1);
> +	ced->cd->cycles = DIV_ROUND_CLOSEST(clk_rate, HZ);
> +
> +	ret = request_irq(irq, mchp_pit64b_interrupt, IRQF_TIMER, "pit64b_tick",
> +			  ced);
> +	if (ret) {
> +		pr_debug("clkevt: Failed to setup PIT64B IRQ\n");
> +		goto free;
> +	}
> +
> +	data.ced = ced;
> +
> +	/* Set up and register clockevents. */
> +	ced->clkevt = &mchp_pit64b_clkevt;
> +	ced->clkevt->cpumask = cpumask_of(0);
> +	ced->clkevt->irq = irq;
> +	clockevents_config_and_register(ced->clkevt, clk_rate, 1, ULONG_MAX);
> +
> +	return 0;
> +
> +free:
> +	kfree(ced);
> +	data.ced = NULL;
> +
> +	return ret;
> +}
> +
> +static int __init mchp_pit64b_dt_init(struct device_node *node)
> +{
> +	struct mchp_pit64b_common_data *cd;
> +	u32 irq, freq = MCHP_PIT64B_DEF_FREQ;
> +	int ret;
> +
> +	if (data.csd && data.ced)
> +		return -EBUSY;
> +
> +	cd = kzalloc(sizeof(*cd), GFP_KERNEL);
> +	if (!cd)
> +		return -ENOMEM;
> +
> +	cd->pclk = of_clk_get_by_name(node, "pclk");
> +	if (IS_ERR(cd->pclk)) {
> +		ret = PTR_ERR(cd->pclk);
> +		goto free;
> +	}
> +
> +	cd->gclk = of_clk_get_by_name(node, "gclk");
> +	if (IS_ERR(cd->gclk))
> +		cd->gclk = NULL;
> +
> +	ret = of_property_read_u32(node, "clock-frequency", &freq);
> +	if (ret)
> +		pr_debug("PIT64B: failed to read clock frequency. Using default!\n");
> +
> +	ret = mchp_pit64b_pres_prepare(cd, freq);
> +	if (ret)
> +		goto free;
> +
> +	cd->base = of_iomap(node, 0);
> +	if (!cd->base) {
> +		pr_debug("%s: Could not map PIT64B address!\n",
> +			 MCHP_PIT64B_NAME);
> +		ret = -ENXIO;
> +		goto free;
> +	}
> +
> +	ret = clk_prepare_enable(cd->pclk);
> +	if (ret)
> +		goto unmap;
> +
> +	if (cd->gclk) {
> +		ret = clk_prepare_enable(cd->gclk);
> +		if (ret)
> +			goto pclk_unprepare;
> +	}
> +
> +	if (!data.ced) {
> +		irq = irq_of_parse_and_map(node, 0);
> +		if (!irq) {
> +			pr_debug("%s: Failed to get PIT64B clockevent IRQ!\n",
> +				 MCHP_PIT64B_NAME);
> +			ret = -ENODEV;
> +			goto gclk_unprepare;
> +		}
> +		ret = mchp_pit64b_dt_init_clkevt(cd, irq);
> +		if (ret)
> +			goto irq_unmap;
> +	} else {
> +		ret = mchp_pit64b_dt_init_clksrc(cd);
> +		if (ret)
> +			goto gclk_unprepare;
> +	}
> +
> +	return 0;
> +
> +irq_unmap:
> +	irq_dispose_mapping(irq);
> +gclk_unprepare:
> +	if (cd->gclk)
> +		clk_disable_unprepare(cd->gclk);
> +pclk_unprepare:
> +	clk_disable_unprepare(cd->pclk);
> +unmap:
> +	iounmap(cd->base);
> +free:
> +	kfree(cd);
> +
> +	return ret;
> +}
> +
> +TIMER_OF_DECLARE(mchp_pit64b_clksrc, "microchip,sam9x60-pit64b",
> +		 mchp_pit64b_dt_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

Re: [PATCH 2/5] clocksource/drivers/timer-microchip-pit64b: add Microchip PIT64B support

[Claudiu.Beznea]

Hi Daniel,

On 08.04.2019 11:43, Daniel Lezcano wrote:
> External E-Mail
> 
> 
> Hi Claudiu,
> 
> On 14/03/2019 17:26, Claudiu.Beznea@microchip.com wrote:
>> From: Claudiu Beznea <claudiu.beznea@microchip.com>
>>
>> Add driver for Microchip PIT64B timer. Timer could be used in continuous
>> mode or oneshot mode. The hardware has 2x32 bit registers for period
>> emulating a 64 bit timer. The LSB_PR and MSB_PR registers are used to set
>> the period value (compare value). TLSB and TMSB keeps the current value
>> of the counter. After a compare the TLSB and TMSB register resets. Apart
>> from this the hardware has SMOD bit in mode register that allow to
>> reconfigure the timer without reset and start commands (start command
>> while timer is active is ignored).
>> The driver uses PIT64B timer as clocksource or clockevent. First requested
>> timer would be registered as clockevent, second one would be registered as
>> clocksource.
> 
> Even if that was done this way before, assuming the DT describes the
> clockevent at the first place and then the clocksource, it is a fragile
> approach.
> 
> What about using one of these approach?
> 
> eg.
> 
> arch/arm/boot/dts/at91sam9261ek.dts
> 
> chosen {
> 	bootargs = "rootfstype=ubifs ubi.mtd=5 root=ubi0:rootfs rw";
>                 stdout-path = "serial0:115200n8";
> 
> 	clocksource {
> 		timer = <&timer0>;
> 	};
> 
>         clockevent {
> 		timer = <&timer1>;
>         };
> };
> 
> or
> 
> arch/arm/boot/dts/integratorap.dts
> 
> aliases {
> 	arm,timer-primary = &timer2;
> 	arm,timer-secondary = &timer1;
> };
> 
> So we can have control of what is the clocksource or the clockevent.
> That is particulary handy in case of multiple channels.

Sure, I will look over these.

Thank you,
Claudiu Beznea

> 
> Not sure if we can replace the 'arm,timer_primary' to 'clocksource'.
> 
> Rob? What is your opinion?
> 
>> Individual PIT64B hardware resources were used for clocksource
>> and clockevent to be able to support high resolution timers with this
>> hardware implementation.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
>>  drivers/clocksource/Kconfig                  |   6 +
>>  drivers/clocksource/Makefile                 |   1 +
>>  drivers/clocksource/timer-microchip-pit64b.c | 464 +++++++++++++++++++++++++++
>>  3 files changed, 471 insertions(+)
>>  create mode 100644 drivers/clocksource/timer-microchip-pit64b.c
>>
>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>> index 5d93e580e5dc..2ad6f881a0bb 100644
>> --- a/drivers/clocksource/Kconfig
>> +++ b/drivers/clocksource/Kconfig
>> @@ -448,6 +448,12 @@ config OXNAS_RPS_TIMER
>>  config SYS_SUPPORTS_SH_CMT
>>          bool
>>  
>> +config MICROCHIP_PIT64B
>> +	bool "Microchip PIT64B support"
>> +	depends on OF || COMPILE_TEST
>> +	help
>> +	  This option enables Microchip PIT64B timer.
>> +
>>  config MTK_TIMER
>>  	bool "Mediatek timer driver" if COMPILE_TEST
>>  	depends on HAS_IOMEM
>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>> index c4a8e9ef932a..c53fa12b9b94 100644
>> --- a/drivers/clocksource/Makefile
>> +++ b/drivers/clocksource/Makefile
>> @@ -35,6 +35,7 @@ obj-$(CONFIG_U300_TIMER)	+= timer-u300.o
>>  obj-$(CONFIG_SUN4I_TIMER)	+= timer-sun4i.o
>>  obj-$(CONFIG_SUN5I_HSTIMER)	+= timer-sun5i.o
>>  obj-$(CONFIG_MESON6_TIMER)	+= timer-meson6.o
>> +obj-$(CONFIG_MICROCHIP_PIT64B)	+= timer-microchip-pit64b.o
>>  obj-$(CONFIG_TEGRA_TIMER)	+= timer-tegra20.o
>>  obj-$(CONFIG_VT8500_TIMER)	+= timer-vt8500.o
>>  obj-$(CONFIG_NSPIRE_TIMER)	+= timer-zevio.o
>> diff --git a/drivers/clocksource/timer-microchip-pit64b.c b/drivers/clocksource/timer-microchip-pit64b.c
>> new file mode 100644
>> index 000000000000..6787aa98ef01
>> --- /dev/null
>> +++ b/drivers/clocksource/timer-microchip-pit64b.c
>> @@ -0,0 +1,464 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +//
>> +// Copyright (C) 2019 Microchip Technology Inc.
>> +// Copyright (C) 2019 Claudiu Beznea (claudiu.beznea@microchip.com)
>> +
>> +#include <linux/clk.h>
>> +#include <linux/clockchips.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/sched_clock.h>
>> +#include <linux/slab.h>
>> +
>> +#define MCHP_PIT64B_CR		0x00	/* Control Register */
>> +#define MCHP_PIT64B_CR_START	BIT(0)
>> +#define MCHP_PIT64B_CR_SWRST	BIT(8)
>> +
>> +#define MCHP_PIT64B_MR		0x04	/* Mode Register */
>> +#define MCHP_PIT64B_MR_CONT	BIT(0)
>> +#define MCHP_PIT64B_MR_SGCLK	BIT(3)
>> +#define MCHP_PIT64B_MR_SMOD	BIT(4)
>> +#define MCHP_PIT64B_MR_PRES	GENMASK(11, 8)
>> +
>> +#define MCHP_PIT64B_LSB_PR	0x08	/* LSB Period Register */
>> +
>> +#define MCHP_PIT64B_MSB_PR	0x0C	/* MSB Period Register */
>> +
>> +#define MCHP_PIT64B_IER		0x10	/* Interrupt Enable Register */
>> +#define MCHP_PIT64B_IER_PERIOD	BIT(0)
>> +
>> +#define MCHP_PIT64B_ISR		0x1C	/* Interrupt Status Register */
>> +#define MCHP_PIT64B_ISR_PERIOD	BIT(0)
>> +
>> +#define MCHP_PIT64B_TLSBR	0x20	/* Timer LSB Register */
>> +
>> +#define MCHP_PIT64B_TMSBR	0x24	/* Timer MSB Register */
>> +
>> +#define MCHP_PIT64B_PRES_MAX	0x10
>> +#define MCHP_PIT64B_DEF_FREQ	2500000UL	/* 2.5 MHz */
>> +#define MCHP_PIT64B_LSBMASK	GENMASK_ULL(31, 0)
>> +#define MCHP_PIT64B_PRESCALER(p)	(MCHP_PIT64B_MR_PRES & ((p) << 8))
>> +
>> +#define MCHP_PIT64B_NAME	"pit64b"
>> +
>> +struct mchp_pit64b_common_data {
>> +	void __iomem *base;
>> +	struct clk *pclk;
>> +	struct clk *gclk;
>> +	u64 cycles;
>> +	u8 pres;
>> +};
>> +
>> +struct mchp_pit64b_clksrc_data {
>> +	struct clocksource *clksrc;
>> +	struct mchp_pit64b_common_data *cd;
>> +};
>> +
>> +struct mchp_pit64b_clkevt_data {
>> +	struct clock_event_device *clkevt;
>> +	struct mchp_pit64b_common_data *cd;
>> +};
>> +
>> +static struct mchp_pit64b_data {
>> +	struct mchp_pit64b_clksrc_data *csd;
>> +	struct mchp_pit64b_clkevt_data *ced;
>> +} data;
>> +
>> +static inline u32 mchp_pit64b_read(void __iomem *base, u32 offset)
>> +{
>> +	return readl_relaxed(base + offset);
>> +}
>> +
>> +static inline void mchp_pit64b_write(void __iomem *base, u32 offset, u32 val)
>> +{
>> +	writel_relaxed(val, base + offset);
>> +}
>> +
>> +static inline u64 mchp_pit64b_get_period(void __iomem *base)
>> +{
>> +	u32 lsb, msb;
>> +
>> +	/* LSB must be read first to guarantee an atomic read of the 64 bit
>> +	 * timer.
>> +	 */
>> +	lsb = mchp_pit64b_read(base, MCHP_PIT64B_TLSBR);
>> +	msb = mchp_pit64b_read(base, MCHP_PIT64B_TMSBR);
>> +
>> +	return (((u64)msb << 32) | lsb);
>> +}
>> +
>> +static inline void mchp_pit64b_set_period(void __iomem *base, u64 cycles)
>> +{
>> +	u32 lsb, msb;
>> +
>> +	lsb = cycles & MCHP_PIT64B_LSBMASK;
>> +	msb = cycles >> 32;
>> +
>> +	/* LSB must be write last to guarantee an atomic update of the timer
>> +	 * even when SMOD=1.
>> +	 */
>> +	mchp_pit64b_write(base, MCHP_PIT64B_MSB_PR, msb);
>> +	mchp_pit64b_write(base, MCHP_PIT64B_LSB_PR, lsb);
>> +}
>> +
>> +static inline void mchp_pit64b_reset(struct mchp_pit64b_common_data *data,
>> +				     u32 mode, bool irq_ena)
>> +{
>> +	mode |= MCHP_PIT64B_PRESCALER(data->pres);
>> +	if (data->gclk)
>> +		mode |= MCHP_PIT64B_MR_SGCLK;
>> +
>> +	mchp_pit64b_write(data->base, MCHP_PIT64B_CR, MCHP_PIT64B_CR_SWRST);
>> +	mchp_pit64b_write(data->base, MCHP_PIT64B_MR, mode);
>> +	mchp_pit64b_set_period(data->base, data->cycles);
>> +	if (irq_ena)
>> +		mchp_pit64b_write(data->base, MCHP_PIT64B_IER,
>> +				  MCHP_PIT64B_IER_PERIOD);
>> +	mchp_pit64b_write(data->base, MCHP_PIT64B_CR, MCHP_PIT64B_CR_START);
>> +}
>> +
>> +static u64 mchp_pit64b_read_clk(struct clocksource *cs)
>> +{
>> +	return mchp_pit64b_get_period(data.csd->cd->base);
>> +}
>> +
>> +static u64 mchp_sched_read_clk(void)
>> +{
>> +	return mchp_pit64b_get_period(data.csd->cd->base);
>> +}
>> +
>> +static struct clocksource mchp_pit64b_clksrc = {
>> +	.name = MCHP_PIT64B_NAME,
>> +	.mask = CLOCKSOURCE_MASK(64),
>> +	.flags = CLOCK_SOURCE_IS_CONTINUOUS,
>> +	.rating = 210,
>> +	.read = mchp_pit64b_read_clk,
>> +};
>> +
>> +static int mchp_pit64b_clkevt_shutdown(struct clock_event_device *cedev)
>> +{
>> +	mchp_pit64b_write(data.ced->cd->base, MCHP_PIT64B_CR,
>> +			  MCHP_PIT64B_CR_SWRST);
>> +
>> +	return 0;
>> +}
>> +
>> +static int mchp_pit64b_clkevt_set_periodic(struct clock_event_device *cedev)
>> +{
>> +	mchp_pit64b_reset(data.ced->cd, MCHP_PIT64B_MR_CONT, true);
>> +
>> +	return 0;
>> +}
>> +
>> +static int mchp_pit64b_clkevt_set_oneshot(struct clock_event_device *cedev)
>> +{
>> +	mchp_pit64b_reset(data.ced->cd, MCHP_PIT64B_MR_SMOD, true);
>> +
>> +	return 0;
>> +}
>> +
>> +static int mchp_pit64b_clkevt_set_next_event(unsigned long evt,
>> +					     struct clock_event_device *cedev)
>> +{
>> +	mchp_pit64b_set_period(data.ced->cd->base, evt);
>> +	mchp_pit64b_write(data.ced->cd->base, MCHP_PIT64B_CR,
>> +			  MCHP_PIT64B_CR_START);
>> +
>> +	return 0;
>> +}
>> +
>> +static void mchp_pit64b_clkevt_suspend(struct clock_event_device *cedev)
>> +{
>> +	mchp_pit64b_write(data.ced->cd->base, MCHP_PIT64B_CR,
>> +			  MCHP_PIT64B_CR_SWRST);
>> +	if (data.ced->cd->gclk)
>> +		clk_disable_unprepare(data.ced->cd->gclk);
>> +	clk_disable_unprepare(data.ced->cd->pclk);
>> +}
>> +
>> +static void mchp_pit64b_clkevt_resume(struct clock_event_device *cedev)
>> +{
>> +	u32 mode = MCHP_PIT64B_MR_SMOD;
>> +
>> +	clk_prepare_enable(data.ced->cd->pclk);
>> +	if (data.ced->cd->gclk)
>> +		clk_prepare_enable(data.ced->cd->gclk);
>> +
>> +	if (clockevent_state_periodic(data.ced->clkevt))
>> +		mode = MCHP_PIT64B_MR_CONT;
>> +
>> +	mchp_pit64b_reset(data.ced->cd, mode, true);
>> +}
>> +
>> +static struct clock_event_device mchp_pit64b_clkevt = {
>> +	.name = MCHP_PIT64B_NAME,
>> +	.features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC,
>> +	.rating = 150,
>> +	.set_state_shutdown = mchp_pit64b_clkevt_shutdown,
>> +	.set_state_periodic = mchp_pit64b_clkevt_set_periodic,
>> +	.set_state_oneshot = mchp_pit64b_clkevt_set_oneshot,
>> +	.set_next_event = mchp_pit64b_clkevt_set_next_event,
>> +	.suspend = mchp_pit64b_clkevt_suspend,
>> +	.resume = mchp_pit64b_clkevt_resume,
>> +};
>> +
>> +static irqreturn_t mchp_pit64b_interrupt(int irq, void *dev_id)
>> +{
>> +	struct mchp_pit64b_clkevt_data *irq_data = dev_id;
>> +
>> +	if (data.ced != irq_data)
>> +		return IRQ_NONE;
>> +
>> +	if (mchp_pit64b_read(irq_data->cd->base, MCHP_PIT64B_ISR) &
>> +	    MCHP_PIT64B_ISR_PERIOD) {
>> +		irq_data->clkevt->event_handler(irq_data->clkevt);
>> +		return IRQ_HANDLED;
>> +	}
>> +
>> +	return IRQ_NONE;
>> +}
>> +
>> +static int __init mchp_pit64b_pres_compute(u32 *pres, u32 clk_rate,
>> +					   u32 max_rate)
>> +{
>> +	u32 tmp;
>> +
>> +	for (*pres = 0; *pres < MCHP_PIT64B_PRES_MAX; (*pres)++) {
>> +		tmp = clk_rate / (*pres + 1);
>> +		if (tmp <= max_rate)
>> +			break;
>> +	}
>> +
>> +	if (*pres == MCHP_PIT64B_PRES_MAX)
>> +		return -EINVAL;
>> +
>> +	return 0;
>> +}
>> +
>> +static int __init mchp_pit64b_pres_prepare(struct mchp_pit64b_common_data *cd,
>> +					   unsigned long max_rate)
>> +{
>> +	unsigned long pclk_rate, diff = 0, best_diff = ULONG_MAX;
>> +	long gclk_round = 0;
>> +	u32 pres, best_pres;
>> +	int ret = 0;
>> +
>> +	pclk_rate = clk_get_rate(cd->pclk);
>> +	if (!pclk_rate)
>> +		return -EINVAL;
>> +
>> +	if (cd->gclk) {
>> +		gclk_round = clk_round_rate(cd->gclk, max_rate);
>> +		if (gclk_round < 0)
>> +			goto pclk;
>> +
>> +		if (pclk_rate / gclk_round < 3)
>> +			goto pclk;
>> +
>> +		ret = mchp_pit64b_pres_compute(&pres, gclk_round, max_rate);
>> +		if (ret)
>> +			best_diff = abs(gclk_round - max_rate);
>> +		else
>> +			best_diff = abs(gclk_round / (pres + 1) - max_rate);
>> +		best_pres = pres;
>> +	}
>> +
>> +pclk:
>> +	/* Check if requested rate could be obtained using PCLK. */
>> +	ret = mchp_pit64b_pres_compute(&pres, pclk_rate, max_rate);
>> +	if (ret)
>> +		diff = abs(pclk_rate - max_rate);
>> +	else
>> +		diff = abs(pclk_rate / (pres + 1) - max_rate);
>> +
>> +	if (best_diff > diff) {
>> +		/* Use PCLK. */
>> +		cd->gclk = NULL;
>> +		best_pres = pres;
>> +	} else {
>> +		clk_set_rate(cd->gclk, gclk_round);
>> +	}
>> +
>> +	cd->pres = best_pres;
>> +
>> +	pr_info("PIT64B: using clk=%s with prescaler %u, freq=%lu [Hz]\n",
>> +		cd->gclk ? "gclk" : "pclk", cd->pres,
>> +		cd->gclk ? gclk_round / (cd->pres + 1)
>> +			 : pclk_rate / (cd->pres + 1));
>> +
>> +	return 0;
>> +}
>> +
>> +static int __init mchp_pit64b_dt_init_clksrc(struct mchp_pit64b_common_data *cd)
>> +{
>> +	struct mchp_pit64b_clksrc_data *csd;
>> +	unsigned long clk_rate;
>> +	int ret;
>> +
>> +	csd = kzalloc(sizeof(*csd), GFP_KERNEL);
>> +	if (!csd)
>> +		return -ENOMEM;
>> +
>> +	csd->cd = cd;
>> +
>> +	if (csd->cd->gclk)
>> +		clk_rate = clk_get_rate(csd->cd->gclk);
>> +	else
>> +		clk_rate = clk_get_rate(csd->cd->pclk);
>> +
>> +	clk_rate = clk_rate / (cd->pres + 1);
>> +	csd->cd->cycles = ULLONG_MAX;
>> +	mchp_pit64b_reset(csd->cd, MCHP_PIT64B_MR_CONT, false);
>> +
>> +	data.csd = csd;
>> +
>> +	csd->clksrc = &mchp_pit64b_clksrc;
>> +
>> +	ret = clocksource_register_hz(csd->clksrc, clk_rate);
>> +	if (ret) {
>> +		pr_debug("clksrc: Failed to register PIT64B clocksource!\n");
>> +		goto free;
>> +	}
>> +
>> +	sched_clock_register(mchp_sched_read_clk, 64, clk_rate);
>> +
>> +	return 0;
>> +
>> +free:
>> +	kfree(csd);
>> +	data.csd = NULL;
>> +
>> +	return ret;
>> +}
>> +
>> +static int __init mchp_pit64b_dt_init_clkevt(struct mchp_pit64b_common_data *cd,
>> +					     u32 irq)
>> +{
>> +	struct mchp_pit64b_clkevt_data *ced;
>> +	unsigned long clk_rate;
>> +	int ret;
>> +
>> +	ced = kzalloc(sizeof(*ced), GFP_KERNEL);
>> +	if (!ced)
>> +		return -ENOMEM;
>> +
>> +	ced->cd = cd;
>> +
>> +	if (ced->cd->gclk)
>> +		clk_rate = clk_get_rate(ced->cd->gclk);
>> +	else
>> +		clk_rate = clk_get_rate(ced->cd->pclk);
>> +
>> +	clk_rate = clk_rate / (ced->cd->pres + 1);
>> +	ced->cd->cycles = DIV_ROUND_CLOSEST(clk_rate, HZ);
>> +
>> +	ret = request_irq(irq, mchp_pit64b_interrupt, IRQF_TIMER, "pit64b_tick",
>> +			  ced);
>> +	if (ret) {
>> +		pr_debug("clkevt: Failed to setup PIT64B IRQ\n");
>> +		goto free;
>> +	}
>> +
>> +	data.ced = ced;
>> +
>> +	/* Set up and register clockevents. */
>> +	ced->clkevt = &mchp_pit64b_clkevt;
>> +	ced->clkevt->cpumask = cpumask_of(0);
>> +	ced->clkevt->irq = irq;
>> +	clockevents_config_and_register(ced->clkevt, clk_rate, 1, ULONG_MAX);
>> +
>> +	return 0;
>> +
>> +free:
>> +	kfree(ced);
>> +	data.ced = NULL;
>> +
>> +	return ret;
>> +}
>> +
>> +static int __init mchp_pit64b_dt_init(struct device_node *node)
>> +{
>> +	struct mchp_pit64b_common_data *cd;
>> +	u32 irq, freq = MCHP_PIT64B_DEF_FREQ;
>> +	int ret;
>> +
>> +	if (data.csd && data.ced)
>> +		return -EBUSY;
>> +
>> +	cd = kzalloc(sizeof(*cd), GFP_KERNEL);
>> +	if (!cd)
>> +		return -ENOMEM;
>> +
>> +	cd->pclk = of_clk_get_by_name(node, "pclk");
>> +	if (IS_ERR(cd->pclk)) {
>> +		ret = PTR_ERR(cd->pclk);
>> +		goto free;
>> +	}
>> +
>> +	cd->gclk = of_clk_get_by_name(node, "gclk");
>> +	if (IS_ERR(cd->gclk))
>> +		cd->gclk = NULL;
>> +
>> +	ret = of_property_read_u32(node, "clock-frequency", &freq);
>> +	if (ret)
>> +		pr_debug("PIT64B: failed to read clock frequency. Using default!\n");
>> +
>> +	ret = mchp_pit64b_pres_prepare(cd, freq);
>> +	if (ret)
>> +		goto free;
>> +
>> +	cd->base = of_iomap(node, 0);
>> +	if (!cd->base) {
>> +		pr_debug("%s: Could not map PIT64B address!\n",
>> +			 MCHP_PIT64B_NAME);
>> +		ret = -ENXIO;
>> +		goto free;
>> +	}
>> +
>> +	ret = clk_prepare_enable(cd->pclk);
>> +	if (ret)
>> +		goto unmap;
>> +
>> +	if (cd->gclk) {
>> +		ret = clk_prepare_enable(cd->gclk);
>> +		if (ret)
>> +			goto pclk_unprepare;
>> +	}
>> +
>> +	if (!data.ced) {
>> +		irq = irq_of_parse_and_map(node, 0);
>> +		if (!irq) {
>> +			pr_debug("%s: Failed to get PIT64B clockevent IRQ!\n",
>> +				 MCHP_PIT64B_NAME);
>> +			ret = -ENODEV;
>> +			goto gclk_unprepare;
>> +		}
>> +		ret = mchp_pit64b_dt_init_clkevt(cd, irq);
>> +		if (ret)
>> +			goto irq_unmap;
>> +	} else {
>> +		ret = mchp_pit64b_dt_init_clksrc(cd);
>> +		if (ret)
>> +			goto gclk_unprepare;
>> +	}
>> +
>> +	return 0;
>> +
>> +irq_unmap:
>> +	irq_dispose_mapping(irq);
>> +gclk_unprepare:
>> +	if (cd->gclk)
>> +		clk_disable_unprepare(cd->gclk);
>> +pclk_unprepare:
>> +	clk_disable_unprepare(cd->pclk);
>> +unmap:
>> +	iounmap(cd->base);
>> +free:
>> +	kfree(cd);
>> +
>> +	return ret;
>> +}
>> +
>> +TIMER_OF_DECLARE(mchp_pit64b_clksrc, "microchip,sam9x60-pit64b",
>> +		 mchp_pit64b_dt_init);
>>
> 
> 

Re: [PATCH 2/5] clocksource/drivers/timer-microchip-pit64b: add Microchip PIT64B support

[Alexandre Belloni]

Hi Daniel,

On 08/04/2019 10:43:26+0200, Daniel Lezcano wrote:
> Hi Claudiu,
> 
> On 14/03/2019 17:26, Claudiu.Beznea@microchip.com wrote:
> > From: Claudiu Beznea <claudiu.beznea@microchip.com>
> > 
> > Add driver for Microchip PIT64B timer. Timer could be used in continuous
> > mode or oneshot mode. The hardware has 2x32 bit registers for period
> > emulating a 64 bit timer. The LSB_PR and MSB_PR registers are used to set
> > the period value (compare value). TLSB and TMSB keeps the current value
> > of the counter. After a compare the TLSB and TMSB register resets. Apart
> > from this the hardware has SMOD bit in mode register that allow to
> > reconfigure the timer without reset and start commands (start command
> > while timer is active is ignored).
> > The driver uses PIT64B timer as clocksource or clockevent. First requested
> > timer would be registered as clockevent, second one would be registered as
> > clocksource.
> 
> Even if that was done this way before, assuming the DT describes the
> clockevent at the first place and then the clocksource, it is a fragile
> approach.
> 
> What about using one of these approach?
> 
> eg.
> 
> arch/arm/boot/dts/at91sam9261ek.dts
> 
> chosen {
> 	bootargs = "rootfstype=ubifs ubi.mtd=5 root=ubi0:rootfs rw";
>                 stdout-path = "serial0:115200n8";
> 
> 	clocksource {
> 		timer = <&timer0>;
> 	};
> 
>         clockevent {
> 		timer = <&timer1>;
>         };
> };
> 

I suggested and implemented exactly that back in 2017 and it was shot
down by both Rob and Mark:

https://lore.kernel.org/lkml/20171213185313.20017-1-alexandre.belloni@free-electrons.com/

At the time, you didn't do *anything* to get it accepted, you stayed
silent. I can respin the series but then I see two options:
 - either you back up the series and really push for it
 - or you simply take this driver as it is. There is nothing in it that
   couldn't be reworked later once you reached a conclusion with the DT
maintainers.

> or
> 
> arch/arm/boot/dts/integratorap.dts
> 
> aliases {
> 	arm,timer-primary = &timer2;
> 	arm,timer-secondary = &timer1;
> };
> 
> So we can have control of what is the clocksource or the clockevent.
> That is particulary handy in case of multiple channels.
> 
> Not sure if we can replace the 'arm,timer_primary' to 'clocksource'.
> 
> Rob? What is your opinion?
> 
> > Individual PIT64B hardware resources were used for clocksource
> > and clockevent to be able to support high resolution timers with this
> > hardware implementation.
> > 
> > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> > ---
> >  drivers/clocksource/Kconfig                  |   6 +
> >  drivers/clocksource/Makefile                 |   1 +
> >  drivers/clocksource/timer-microchip-pit64b.c | 464 +++++++++++++++++++++++++++
> >  3 files changed, 471 insertions(+)
> >  create mode 100644 drivers/clocksource/timer-microchip-pit64b.c
> > 
> > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > index 5d93e580e5dc..2ad6f881a0bb 100644
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -448,6 +448,12 @@ config OXNAS_RPS_TIMER
> >  config SYS_SUPPORTS_SH_CMT
> >          bool
> >  
> > +config MICROCHIP_PIT64B
> > +	bool "Microchip PIT64B support"
> > +	depends on OF || COMPILE_TEST
> > +	help
> > +	  This option enables Microchip PIT64B timer.
> > +
> >  config MTK_TIMER
> >  	bool "Mediatek timer driver" if COMPILE_TEST
> >  	depends on HAS_IOMEM
> > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> > index c4a8e9ef932a..c53fa12b9b94 100644
> > --- a/drivers/clocksource/Makefile
> > +++ b/drivers/clocksource/Makefile
> > @@ -35,6 +35,7 @@ obj-$(CONFIG_U300_TIMER)	+= timer-u300.o
> >  obj-$(CONFIG_SUN4I_TIMER)	+= timer-sun4i.o
> >  obj-$(CONFIG_SUN5I_HSTIMER)	+= timer-sun5i.o
> >  obj-$(CONFIG_MESON6_TIMER)	+= timer-meson6.o
> > +obj-$(CONFIG_MICROCHIP_PIT64B)	+= timer-microchip-pit64b.o
> >  obj-$(CONFIG_TEGRA_TIMER)	+= timer-tegra20.o
> >  obj-$(CONFIG_VT8500_TIMER)	+= timer-vt8500.o
> >  obj-$(CONFIG_NSPIRE_TIMER)	+= timer-zevio.o
> > diff --git a/drivers/clocksource/timer-microchip-pit64b.c b/drivers/clocksource/timer-microchip-pit64b.c
> > new file mode 100644
> > index 000000000000..6787aa98ef01
> > --- /dev/null
> > +++ b/drivers/clocksource/timer-microchip-pit64b.c
> > @@ -0,0 +1,464 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +//
> > +// Copyright (C) 2019 Microchip Technology Inc.
> > +// Copyright (C) 2019 Claudiu Beznea (claudiu.beznea@microchip.com)
> > +
> > +#include <linux/clk.h>
> > +#include <linux/clockchips.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/sched_clock.h>
> > +#include <linux/slab.h>
> > +
> > +#define MCHP_PIT64B_CR		0x00	/* Control Register */
> > +#define MCHP_PIT64B_CR_START	BIT(0)
> > +#define MCHP_PIT64B_CR_SWRST	BIT(8)
> > +
> > +#define MCHP_PIT64B_MR		0x04	/* Mode Register */
> > +#define MCHP_PIT64B_MR_CONT	BIT(0)
> > +#define MCHP_PIT64B_MR_SGCLK	BIT(3)
> > +#define MCHP_PIT64B_MR_SMOD	BIT(4)
> > +#define MCHP_PIT64B_MR_PRES	GENMASK(11, 8)
> > +
> > +#define MCHP_PIT64B_LSB_PR	0x08	/* LSB Period Register */
> > +
> > +#define MCHP_PIT64B_MSB_PR	0x0C	/* MSB Period Register */
> > +
> > +#define MCHP_PIT64B_IER		0x10	/* Interrupt Enable Register */
> > +#define MCHP_PIT64B_IER_PERIOD	BIT(0)
> > +
> > +#define MCHP_PIT64B_ISR		0x1C	/* Interrupt Status Register */
> > +#define MCHP_PIT64B_ISR_PERIOD	BIT(0)
> > +
> > +#define MCHP_PIT64B_TLSBR	0x20	/* Timer LSB Register */
> > +
> > +#define MCHP_PIT64B_TMSBR	0x24	/* Timer MSB Register */
> > +
> > +#define MCHP_PIT64B_PRES_MAX	0x10
> > +#define MCHP_PIT64B_DEF_FREQ	2500000UL	/* 2.5 MHz */
> > +#define MCHP_PIT64B_LSBMASK	GENMASK_ULL(31, 0)
> > +#define MCHP_PIT64B_PRESCALER(p)	(MCHP_PIT64B_MR_PRES & ((p) << 8))
> > +
> > +#define MCHP_PIT64B_NAME	"pit64b"
> > +
> > +struct mchp_pit64b_common_data {
> > +	void __iomem *base;
> > +	struct clk *pclk;
> > +	struct clk *gclk;
> > +	u64 cycles;
> > +	u8 pres;
> > +};
> > +
> > +struct mchp_pit64b_clksrc_data {
> > +	struct clocksource *clksrc;
> > +	struct mchp_pit64b_common_data *cd;
> > +};
> > +
> > +struct mchp_pit64b_clkevt_data {
> > +	struct clock_event_device *clkevt;
> > +	struct mchp_pit64b_common_data *cd;
> > +};
> > +
> > +static struct mchp_pit64b_data {
> > +	struct mchp_pit64b_clksrc_data *csd;
> > +	struct mchp_pit64b_clkevt_data *ced;
> > +} data;
> > +
> > +static inline u32 mchp_pit64b_read(void __iomem *base, u32 offset)
> > +{
> > +	return readl_relaxed(base + offset);
> > +}
> > +
> > +static inline void mchp_pit64b_write(void __iomem *base, u32 offset, u32 val)
> > +{
> > +	writel_relaxed(val, base + offset);
> > +}
> > +
> > +static inline u64 mchp_pit64b_get_period(void __iomem *base)
> > +{
> > +	u32 lsb, msb;
> > +
> > +	/* LSB must be read first to guarantee an atomic read of the 64 bit
> > +	 * timer.
> > +	 */
> > +	lsb = mchp_pit64b_read(base, MCHP_PIT64B_TLSBR);
> > +	msb = mchp_pit64b_read(base, MCHP_PIT64B_TMSBR);
> > +
> > +	return (((u64)msb << 32) | lsb);
> > +}
> > +
> > +static inline void mchp_pit64b_set_period(void __iomem *base, u64 cycles)
> > +{
> > +	u32 lsb, msb;
> > +
> > +	lsb = cycles & MCHP_PIT64B_LSBMASK;
> > +	msb = cycles >> 32;
> > +
> > +	/* LSB must be write last to guarantee an atomic update of the timer
> > +	 * even when SMOD=1.
> > +	 */
> > +	mchp_pit64b_write(base, MCHP_PIT64B_MSB_PR, msb);
> > +	mchp_pit64b_write(base, MCHP_PIT64B_LSB_PR, lsb);
> > +}
> > +
> > +static inline void mchp_pit64b_reset(struct mchp_pit64b_common_data *data,
> > +				     u32 mode, bool irq_ena)
> > +{
> > +	mode |= MCHP_PIT64B_PRESCALER(data->pres);
> > +	if (data->gclk)
> > +		mode |= MCHP_PIT64B_MR_SGCLK;
> > +
> > +	mchp_pit64b_write(data->base, MCHP_PIT64B_CR, MCHP_PIT64B_CR_SWRST);
> > +	mchp_pit64b_write(data->base, MCHP_PIT64B_MR, mode);
> > +	mchp_pit64b_set_period(data->base, data->cycles);
> > +	if (irq_ena)
> > +		mchp_pit64b_write(data->base, MCHP_PIT64B_IER,
> > +				  MCHP_PIT64B_IER_PERIOD);
> > +	mchp_pit64b_write(data->base, MCHP_PIT64B_CR, MCHP_PIT64B_CR_START);
> > +}
> > +
> > +static u64 mchp_pit64b_read_clk(struct clocksource *cs)
> > +{
> > +	return mchp_pit64b_get_period(data.csd->cd->base);
> > +}
> > +
> > +static u64 mchp_sched_read_clk(void)
> > +{
> > +	return mchp_pit64b_get_period(data.csd->cd->base);
> > +}
> > +
> > +static struct clocksource mchp_pit64b_clksrc = {
> > +	.name = MCHP_PIT64B_NAME,
> > +	.mask = CLOCKSOURCE_MASK(64),
> > +	.flags = CLOCK_SOURCE_IS_CONTINUOUS,
> > +	.rating = 210,
> > +	.read = mchp_pit64b_read_clk,
> > +};
> > +
> > +static int mchp_pit64b_clkevt_shutdown(struct clock_event_device *cedev)
> > +{
> > +	mchp_pit64b_write(data.ced->cd->base, MCHP_PIT64B_CR,
> > +			  MCHP_PIT64B_CR_SWRST);
> > +
> > +	return 0;
> > +}
> > +
> > +static int mchp_pit64b_clkevt_set_periodic(struct clock_event_device *cedev)
> > +{
> > +	mchp_pit64b_reset(data.ced->cd, MCHP_PIT64B_MR_CONT, true);
> > +
> > +	return 0;
> > +}
> > +
> > +static int mchp_pit64b_clkevt_set_oneshot(struct clock_event_device *cedev)
> > +{
> > +	mchp_pit64b_reset(data.ced->cd, MCHP_PIT64B_MR_SMOD, true);
> > +
> > +	return 0;
> > +}
> > +
> > +static int mchp_pit64b_clkevt_set_next_event(unsigned long evt,
> > +					     struct clock_event_device *cedev)
> > +{
> > +	mchp_pit64b_set_period(data.ced->cd->base, evt);
> > +	mchp_pit64b_write(data.ced->cd->base, MCHP_PIT64B_CR,
> > +			  MCHP_PIT64B_CR_START);
> > +
> > +	return 0;
> > +}
> > +
> > +static void mchp_pit64b_clkevt_suspend(struct clock_event_device *cedev)
> > +{
> > +	mchp_pit64b_write(data.ced->cd->base, MCHP_PIT64B_CR,
> > +			  MCHP_PIT64B_CR_SWRST);
> > +	if (data.ced->cd->gclk)
> > +		clk_disable_unprepare(data.ced->cd->gclk);
> > +	clk_disable_unprepare(data.ced->cd->pclk);
> > +}
> > +
> > +static void mchp_pit64b_clkevt_resume(struct clock_event_device *cedev)
> > +{
> > +	u32 mode = MCHP_PIT64B_MR_SMOD;
> > +
> > +	clk_prepare_enable(data.ced->cd->pclk);
> > +	if (data.ced->cd->gclk)
> > +		clk_prepare_enable(data.ced->cd->gclk);
> > +
> > +	if (clockevent_state_periodic(data.ced->clkevt))
> > +		mode = MCHP_PIT64B_MR_CONT;
> > +
> > +	mchp_pit64b_reset(data.ced->cd, mode, true);
> > +}
> > +
> > +static struct clock_event_device mchp_pit64b_clkevt = {
> > +	.name = MCHP_PIT64B_NAME,
> > +	.features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC,
> > +	.rating = 150,
> > +	.set_state_shutdown = mchp_pit64b_clkevt_shutdown,
> > +	.set_state_periodic = mchp_pit64b_clkevt_set_periodic,
> > +	.set_state_oneshot = mchp_pit64b_clkevt_set_oneshot,
> > +	.set_next_event = mchp_pit64b_clkevt_set_next_event,
> > +	.suspend = mchp_pit64b_clkevt_suspend,
> > +	.resume = mchp_pit64b_clkevt_resume,
> > +};
> > +
> > +static irqreturn_t mchp_pit64b_interrupt(int irq, void *dev_id)
> > +{
> > +	struct mchp_pit64b_clkevt_data *irq_data = dev_id;
> > +
> > +	if (data.ced != irq_data)
> > +		return IRQ_NONE;
> > +
> > +	if (mchp_pit64b_read(irq_data->cd->base, MCHP_PIT64B_ISR) &
> > +	    MCHP_PIT64B_ISR_PERIOD) {
> > +		irq_data->clkevt->event_handler(irq_data->clkevt);
> > +		return IRQ_HANDLED;
> > +	}
> > +
> > +	return IRQ_NONE;
> > +}
> > +
> > +static int __init mchp_pit64b_pres_compute(u32 *pres, u32 clk_rate,
> > +					   u32 max_rate)
> > +{
> > +	u32 tmp;
> > +
> > +	for (*pres = 0; *pres < MCHP_PIT64B_PRES_MAX; (*pres)++) {
> > +		tmp = clk_rate / (*pres + 1);
> > +		if (tmp <= max_rate)
> > +			break;
> > +	}
> > +
> > +	if (*pres == MCHP_PIT64B_PRES_MAX)
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > +
> > +static int __init mchp_pit64b_pres_prepare(struct mchp_pit64b_common_data *cd,
> > +					   unsigned long max_rate)
> > +{
> > +	unsigned long pclk_rate, diff = 0, best_diff = ULONG_MAX;
> > +	long gclk_round = 0;
> > +	u32 pres, best_pres;
> > +	int ret = 0;
> > +
> > +	pclk_rate = clk_get_rate(cd->pclk);
> > +	if (!pclk_rate)
> > +		return -EINVAL;
> > +
> > +	if (cd->gclk) {
> > +		gclk_round = clk_round_rate(cd->gclk, max_rate);
> > +		if (gclk_round < 0)
> > +			goto pclk;
> > +
> > +		if (pclk_rate / gclk_round < 3)
> > +			goto pclk;
> > +
> > +		ret = mchp_pit64b_pres_compute(&pres, gclk_round, max_rate);
> > +		if (ret)
> > +			best_diff = abs(gclk_round - max_rate);
> > +		else
> > +			best_diff = abs(gclk_round / (pres + 1) - max_rate);
> > +		best_pres = pres;
> > +	}
> > +
> > +pclk:
> > +	/* Check if requested rate could be obtained using PCLK. */
> > +	ret = mchp_pit64b_pres_compute(&pres, pclk_rate, max_rate);
> > +	if (ret)
> > +		diff = abs(pclk_rate - max_rate);
> > +	else
> > +		diff = abs(pclk_rate / (pres + 1) - max_rate);
> > +
> > +	if (best_diff > diff) {
> > +		/* Use PCLK. */
> > +		cd->gclk = NULL;
> > +		best_pres = pres;
> > +	} else {
> > +		clk_set_rate(cd->gclk, gclk_round);
> > +	}
> > +
> > +	cd->pres = best_pres;
> > +
> > +	pr_info("PIT64B: using clk=%s with prescaler %u, freq=%lu [Hz]\n",
> > +		cd->gclk ? "gclk" : "pclk", cd->pres,
> > +		cd->gclk ? gclk_round / (cd->pres + 1)
> > +			 : pclk_rate / (cd->pres + 1));
> > +
> > +	return 0;
> > +}
> > +
> > +static int __init mchp_pit64b_dt_init_clksrc(struct mchp_pit64b_common_data *cd)
> > +{
> > +	struct mchp_pit64b_clksrc_data *csd;
> > +	unsigned long clk_rate;
> > +	int ret;
> > +
> > +	csd = kzalloc(sizeof(*csd), GFP_KERNEL);
> > +	if (!csd)
> > +		return -ENOMEM;
> > +
> > +	csd->cd = cd;
> > +
> > +	if (csd->cd->gclk)
> > +		clk_rate = clk_get_rate(csd->cd->gclk);
> > +	else
> > +		clk_rate = clk_get_rate(csd->cd->pclk);
> > +
> > +	clk_rate = clk_rate / (cd->pres + 1);
> > +	csd->cd->cycles = ULLONG_MAX;
> > +	mchp_pit64b_reset(csd->cd, MCHP_PIT64B_MR_CONT, false);
> > +
> > +	data.csd = csd;
> > +
> > +	csd->clksrc = &mchp_pit64b_clksrc;
> > +
> > +	ret = clocksource_register_hz(csd->clksrc, clk_rate);
> > +	if (ret) {
> > +		pr_debug("clksrc: Failed to register PIT64B clocksource!\n");
> > +		goto free;
> > +	}
> > +
> > +	sched_clock_register(mchp_sched_read_clk, 64, clk_rate);
> > +
> > +	return 0;
> > +
> > +free:
> > +	kfree(csd);
> > +	data.csd = NULL;
> > +
> > +	return ret;
> > +}
> > +
> > +static int __init mchp_pit64b_dt_init_clkevt(struct mchp_pit64b_common_data *cd,
> > +					     u32 irq)
> > +{
> > +	struct mchp_pit64b_clkevt_data *ced;
> > +	unsigned long clk_rate;
> > +	int ret;
> > +
> > +	ced = kzalloc(sizeof(*ced), GFP_KERNEL);
> > +	if (!ced)
> > +		return -ENOMEM;
> > +
> > +	ced->cd = cd;
> > +
> > +	if (ced->cd->gclk)
> > +		clk_rate = clk_get_rate(ced->cd->gclk);
> > +	else
> > +		clk_rate = clk_get_rate(ced->cd->pclk);
> > +
> > +	clk_rate = clk_rate / (ced->cd->pres + 1);
> > +	ced->cd->cycles = DIV_ROUND_CLOSEST(clk_rate, HZ);
> > +
> > +	ret = request_irq(irq, mchp_pit64b_interrupt, IRQF_TIMER, "pit64b_tick",
> > +			  ced);
> > +	if (ret) {
> > +		pr_debug("clkevt: Failed to setup PIT64B IRQ\n");
> > +		goto free;
> > +	}
> > +
> > +	data.ced = ced;
> > +
> > +	/* Set up and register clockevents. */
> > +	ced->clkevt = &mchp_pit64b_clkevt;
> > +	ced->clkevt->cpumask = cpumask_of(0);
> > +	ced->clkevt->irq = irq;
> > +	clockevents_config_and_register(ced->clkevt, clk_rate, 1, ULONG_MAX);
> > +
> > +	return 0;
> > +
> > +free:
> > +	kfree(ced);
> > +	data.ced = NULL;
> > +
> > +	return ret;
> > +}
> > +
> > +static int __init mchp_pit64b_dt_init(struct device_node *node)
> > +{
> > +	struct mchp_pit64b_common_data *cd;
> > +	u32 irq, freq = MCHP_PIT64B_DEF_FREQ;
> > +	int ret;
> > +
> > +	if (data.csd && data.ced)
> > +		return -EBUSY;
> > +
> > +	cd = kzalloc(sizeof(*cd), GFP_KERNEL);
> > +	if (!cd)
> > +		return -ENOMEM;
> > +
> > +	cd->pclk = of_clk_get_by_name(node, "pclk");
> > +	if (IS_ERR(cd->pclk)) {
> > +		ret = PTR_ERR(cd->pclk);
> > +		goto free;
> > +	}
> > +
> > +	cd->gclk = of_clk_get_by_name(node, "gclk");
> > +	if (IS_ERR(cd->gclk))
> > +		cd->gclk = NULL;
> > +
> > +	ret = of_property_read_u32(node, "clock-frequency", &freq);
> > +	if (ret)
> > +		pr_debug("PIT64B: failed to read clock frequency. Using default!\n");
> > +
> > +	ret = mchp_pit64b_pres_prepare(cd, freq);
> > +	if (ret)
> > +		goto free;
> > +
> > +	cd->base = of_iomap(node, 0);
> > +	if (!cd->base) {
> > +		pr_debug("%s: Could not map PIT64B address!\n",
> > +			 MCHP_PIT64B_NAME);
> > +		ret = -ENXIO;
> > +		goto free;
> > +	}
> > +
> > +	ret = clk_prepare_enable(cd->pclk);
> > +	if (ret)
> > +		goto unmap;
> > +
> > +	if (cd->gclk) {
> > +		ret = clk_prepare_enable(cd->gclk);
> > +		if (ret)
> > +			goto pclk_unprepare;
> > +	}
> > +
> > +	if (!data.ced) {
> > +		irq = irq_of_parse_and_map(node, 0);
> > +		if (!irq) {
> > +			pr_debug("%s: Failed to get PIT64B clockevent IRQ!\n",
> > +				 MCHP_PIT64B_NAME);
> > +			ret = -ENODEV;
> > +			goto gclk_unprepare;
> > +		}
> > +		ret = mchp_pit64b_dt_init_clkevt(cd, irq);
> > +		if (ret)
> > +			goto irq_unmap;
> > +	} else {
> > +		ret = mchp_pit64b_dt_init_clksrc(cd);
> > +		if (ret)
> > +			goto gclk_unprepare;
> > +	}
> > +
> > +	return 0;
> > +
> > +irq_unmap:
> > +	irq_dispose_mapping(irq);
> > +gclk_unprepare:
> > +	if (cd->gclk)
> > +		clk_disable_unprepare(cd->gclk);
> > +pclk_unprepare:
> > +	clk_disable_unprepare(cd->pclk);
> > +unmap:
> > +	iounmap(cd->base);
> > +free:
> > +	kfree(cd);
> > +
> > +	return ret;
> > +}
> > +
> > +TIMER_OF_DECLARE(mchp_pit64b_clksrc, "microchip,sam9x60-pit64b",
> > +		 mchp_pit64b_dt_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
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 2/5] clocksource/drivers/timer-microchip-pit64b: add Microchip PIT64B support

[Daniel Lezcano]

On 08/04/2019 14:11, Alexandre Belloni wrote:
> Hi Daniel,
> 
> On 08/04/2019 10:43:26+0200, Daniel Lezcano wrote:
>> Hi Claudiu,
>>
>> On 14/03/2019 17:26, Claudiu.Beznea@microchip.com wrote:
>>> From: Claudiu Beznea <claudiu.beznea@microchip.com>
>>>
>>> Add driver for Microchip PIT64B timer. Timer could be used in continuous
>>> mode or oneshot mode. The hardware has 2x32 bit registers for period
>>> emulating a 64 bit timer. The LSB_PR and MSB_PR registers are used to set
>>> the period value (compare value). TLSB and TMSB keeps the current value
>>> of the counter. After a compare the TLSB and TMSB register resets. Apart
>>> from this the hardware has SMOD bit in mode register that allow to
>>> reconfigure the timer without reset and start commands (start command
>>> while timer is active is ignored).
>>> The driver uses PIT64B timer as clocksource or clockevent. First requested
>>> timer would be registered as clockevent, second one would be registered as
>>> clocksource.
>>
>> Even if that was done this way before, assuming the DT describes the
>> clockevent at the first place and then the clocksource, it is a fragile
>> approach.
>>
>> What about using one of these approach?
>>
>> eg.
>>
>> arch/arm/boot/dts/at91sam9261ek.dts
>>
>> chosen {
>> 	bootargs = "rootfstype=ubifs ubi.mtd=5 root=ubi0:rootfs rw";
>>                 stdout-path = "serial0:115200n8";
>>
>> 	clocksource {
>> 		timer = <&timer0>;
>> 	};
>>
>>         clockevent {
>> 		timer = <&timer1>;
>>         };
>> };
>>
> 
> I suggested and implemented exactly that back in 2017 and it was shot
> down by both Rob and Mark:
> 
> https://lore.kernel.org/lkml/20171213185313.20017-1-alexandre.belloni@free-electrons.com/
> 
> At the time, you didn't do *anything* to get it accepted, you stayed
> silent.

What about commit 51f0aeb2d21f1 ?


Author: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Date:   Wed Jun 8 17:08:57 2016 +0200

    ARM: dts: at91: at91sam9261ek: use TCB0 as timers

    Use tcb0 for timers as selected in at91_dt_defconfig.

    Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
    Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>

diff --git a/arch/arm/boot/dts/at91sam9261ek.dts
b/arch/arm/boot/dts/at91sam9261ek.dts
index 9733db3f739b..a29fc0494076 100644
--- a/arch/arm/boot/dts/at91sam9261ek.dts
+++ b/arch/arm/boot/dts/at91sam9261ek.dts
@@ -15,6 +15,14 @@
        chosen {
                bootargs = "rootfstype=ubifs ubi.mtd=5 root=ubi0:rootfs rw";
                stdout-path = "serial0:115200n8";
+
+               clocksource {
+                       timer = <&timer0>;
+               };
+
+               clockevent {
+                       timer = <&timer1>;
+               };
        };

        memory {
@@ -125,6 +133,18 @@
                };

                apb {
+                       tcb0: timer@fffa0000 {
+                               timer0: timer@0 {
+                                       compatible = "atmel,tcb-timer";
+                                       reg = <0>, <1>;
+                               };
+
+                               timer1: timer@2 {
+                                       compatible = "atmel,tcb-timer";
+                                       reg = <2>;
+                               };
+                       };
+
                        usb1: gadget@fffa4000 {
                                atmel,vbus-gpio = <&pioB 29
GPIO_ACTIVE_HIGH>;
                                status = "okay";



>  I can respin the series but then I see two options:
>  - either you back up the series and really push for it
>  - or you simply take this driver as it is. There is nothing in it that
>    couldn't be reworked later once you reached a conclusion with the DT
> maintainers.
> 
>> or
>>
>> arch/arm/boot/dts/integratorap.dts
>>
>> aliases {
>> 	arm,timer-primary = &timer2;
>> 	arm,timer-secondary = &timer1;
>> };
>>
>> So we can have control of what is the clocksource or the clockevent.
>> That is particulary handy in case of multiple channels.
>>
>> Not sure if we can replace the 'arm,timer_primary' to 'clocksource'.
>>
>> Rob? What is your opinion?
>>
>>> Individual PIT64B hardware resources were used for clocksource
>>> and clockevent to be able to support high resolution timers with this
>>> hardware implementation.
>>>
>>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>>> ---
>>>  drivers/clocksource/Kconfig                  |   6 +
>>>  drivers/clocksource/Makefile                 |   1 +
>>>  drivers/clocksource/timer-microchip-pit64b.c | 464 +++++++++++++++++++++++++++
>>>  3 files changed, 471 insertions(+)
>>>  create mode 100644 drivers/clocksource/timer-microchip-pit64b.c
>>>
>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>> index 5d93e580e5dc..2ad6f881a0bb 100644
>>> --- a/drivers/clocksource/Kconfig
>>> +++ b/drivers/clocksource/Kconfig
>>> @@ -448,6 +448,12 @@ config OXNAS_RPS_TIMER
>>>  config SYS_SUPPORTS_SH_CMT
>>>          bool
>>>  
>>> +config MICROCHIP_PIT64B
>>> +	bool "Microchip PIT64B support"
>>> +	depends on OF || COMPILE_TEST
>>> +	help
>>> +	  This option enables Microchip PIT64B timer.
>>> +
>>>  config MTK_TIMER
>>>  	bool "Mediatek timer driver" if COMPILE_TEST
>>>  	depends on HAS_IOMEM
>>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>>> index c4a8e9ef932a..c53fa12b9b94 100644
>>> --- a/drivers/clocksource/Makefile
>>> +++ b/drivers/clocksource/Makefile
>>> @@ -35,6 +35,7 @@ obj-$(CONFIG_U300_TIMER)	+= timer-u300.o
>>>  obj-$(CONFIG_SUN4I_TIMER)	+= timer-sun4i.o
>>>  obj-$(CONFIG_SUN5I_HSTIMER)	+= timer-sun5i.o
>>>  obj-$(CONFIG_MESON6_TIMER)	+= timer-meson6.o
>>> +obj-$(CONFIG_MICROCHIP_PIT64B)	+= timer-microchip-pit64b.o
>>>  obj-$(CONFIG_TEGRA_TIMER)	+= timer-tegra20.o
>>>  obj-$(CONFIG_VT8500_TIMER)	+= timer-vt8500.o
>>>  obj-$(CONFIG_NSPIRE_TIMER)	+= timer-zevio.o
>>> diff --git a/drivers/clocksource/timer-microchip-pit64b.c b/drivers/clocksource/timer-microchip-pit64b.c
>>> new file mode 100644
>>> index 000000000000..6787aa98ef01
>>> --- /dev/null
>>> +++ b/drivers/clocksource/timer-microchip-pit64b.c
>>> @@ -0,0 +1,464 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +//
>>> +// Copyright (C) 2019 Microchip Technology Inc.
>>> +// Copyright (C) 2019 Claudiu Beznea (claudiu.beznea@microchip.com)
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/clockchips.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/of_address.h>
>>> +#include <linux/of_irq.h>
>>> +#include <linux/sched_clock.h>
>>> +#include <linux/slab.h>
>>> +
>>> +#define MCHP_PIT64B_CR		0x00	/* Control Register */
>>> +#define MCHP_PIT64B_CR_START	BIT(0)
>>> +#define MCHP_PIT64B_CR_SWRST	BIT(8)
>>> +
>>> +#define MCHP_PIT64B_MR		0x04	/* Mode Register */
>>> +#define MCHP_PIT64B_MR_CONT	BIT(0)
>>> +#define MCHP_PIT64B_MR_SGCLK	BIT(3)
>>> +#define MCHP_PIT64B_MR_SMOD	BIT(4)
>>> +#define MCHP_PIT64B_MR_PRES	GENMASK(11, 8)
>>> +
>>> +#define MCHP_PIT64B_LSB_PR	0x08	/* LSB Period Register */
>>> +
>>> +#define MCHP_PIT64B_MSB_PR	0x0C	/* MSB Period Register */
>>> +
>>> +#define MCHP_PIT64B_IER		0x10	/* Interrupt Enable Register */
>>> +#define MCHP_PIT64B_IER_PERIOD	BIT(0)
>>> +
>>> +#define MCHP_PIT64B_ISR		0x1C	/* Interrupt Status Register */
>>> +#define MCHP_PIT64B_ISR_PERIOD	BIT(0)
>>> +
>>> +#define MCHP_PIT64B_TLSBR	0x20	/* Timer LSB Register */
>>> +
>>> +#define MCHP_PIT64B_TMSBR	0x24	/* Timer MSB Register */
>>> +
>>> +#define MCHP_PIT64B_PRES_MAX	0x10
>>> +#define MCHP_PIT64B_DEF_FREQ	2500000UL	/* 2.5 MHz */
>>> +#define MCHP_PIT64B_LSBMASK	GENMASK_ULL(31, 0)
>>> +#define MCHP_PIT64B_PRESCALER(p)	(MCHP_PIT64B_MR_PRES & ((p) << 8))
>>> +
>>> +#define MCHP_PIT64B_NAME	"pit64b"
>>> +
>>> +struct mchp_pit64b_common_data {
>>> +	void __iomem *base;
>>> +	struct clk *pclk;
>>> +	struct clk *gclk;
>>> +	u64 cycles;
>>> +	u8 pres;
>>> +};
>>> +
>>> +struct mchp_pit64b_clksrc_data {
>>> +	struct clocksource *clksrc;
>>> +	struct mchp_pit64b_common_data *cd;
>>> +};
>>> +
>>> +struct mchp_pit64b_clkevt_data {
>>> +	struct clock_event_device *clkevt;
>>> +	struct mchp_pit64b_common_data *cd;
>>> +};
>>> +
>>> +static struct mchp_pit64b_data {
>>> +	struct mchp_pit64b_clksrc_data *csd;
>>> +	struct mchp_pit64b_clkevt_data *ced;
>>> +} data;
>>> +
>>> +static inline u32 mchp_pit64b_read(void __iomem *base, u32 offset)
>>> +{
>>> +	return readl_relaxed(base + offset);
>>> +}
>>> +
>>> +static inline void mchp_pit64b_write(void __iomem *base, u32 offset, u32 val)
>>> +{
>>> +	writel_relaxed(val, base + offset);
>>> +}
>>> +
>>> +static inline u64 mchp_pit64b_get_period(void __iomem *base)
>>> +{
>>> +	u32 lsb, msb;
>>> +
>>> +	/* LSB must be read first to guarantee an atomic read of the 64 bit
>>> +	 * timer.
>>> +	 */
>>> +	lsb = mchp_pit64b_read(base, MCHP_PIT64B_TLSBR);
>>> +	msb = mchp_pit64b_read(base, MCHP_PIT64B_TMSBR);
>>> +
>>> +	return (((u64)msb << 32) | lsb);
>>> +}
>>> +
>>> +static inline void mchp_pit64b_set_period(void __iomem *base, u64 cycles)
>>> +{
>>> +	u32 lsb, msb;
>>> +
>>> +	lsb = cycles & MCHP_PIT64B_LSBMASK;
>>> +	msb = cycles >> 32;
>>> +
>>> +	/* LSB must be write last to guarantee an atomic update of the timer
>>> +	 * even when SMOD=1.
>>> +	 */
>>> +	mchp_pit64b_write(base, MCHP_PIT64B_MSB_PR, msb);
>>> +	mchp_pit64b_write(base, MCHP_PIT64B_LSB_PR, lsb);
>>> +}
>>> +
>>> +static inline void mchp_pit64b_reset(struct mchp_pit64b_common_data *data,
>>> +				     u32 mode, bool irq_ena)
>>> +{
>>> +	mode |= MCHP_PIT64B_PRESCALER(data->pres);
>>> +	if (data->gclk)
>>> +		mode |= MCHP_PIT64B_MR_SGCLK;
>>> +
>>> +	mchp_pit64b_write(data->base, MCHP_PIT64B_CR, MCHP_PIT64B_CR_SWRST);
>>> +	mchp_pit64b_write(data->base, MCHP_PIT64B_MR, mode);
>>> +	mchp_pit64b_set_period(data->base, data->cycles);
>>> +	if (irq_ena)
>>> +		mchp_pit64b_write(data->base, MCHP_PIT64B_IER,
>>> +				  MCHP_PIT64B_IER_PERIOD);
>>> +	mchp_pit64b_write(data->base, MCHP_PIT64B_CR, MCHP_PIT64B_CR_START);
>>> +}
>>> +
>>> +static u64 mchp_pit64b_read_clk(struct clocksource *cs)
>>> +{
>>> +	return mchp_pit64b_get_period(data.csd->cd->base);
>>> +}
>>> +
>>> +static u64 mchp_sched_read_clk(void)
>>> +{
>>> +	return mchp_pit64b_get_period(data.csd->cd->base);
>>> +}
>>> +
>>> +static struct clocksource mchp_pit64b_clksrc = {
>>> +	.name = MCHP_PIT64B_NAME,
>>> +	.mask = CLOCKSOURCE_MASK(64),
>>> +	.flags = CLOCK_SOURCE_IS_CONTINUOUS,
>>> +	.rating = 210,
>>> +	.read = mchp_pit64b_read_clk,
>>> +};
>>> +
>>> +static int mchp_pit64b_clkevt_shutdown(struct clock_event_device *cedev)
>>> +{
>>> +	mchp_pit64b_write(data.ced->cd->base, MCHP_PIT64B_CR,
>>> +			  MCHP_PIT64B_CR_SWRST);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int mchp_pit64b_clkevt_set_periodic(struct clock_event_device *cedev)
>>> +{
>>> +	mchp_pit64b_reset(data.ced->cd, MCHP_PIT64B_MR_CONT, true);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int mchp_pit64b_clkevt_set_oneshot(struct clock_event_device *cedev)
>>> +{
>>> +	mchp_pit64b_reset(data.ced->cd, MCHP_PIT64B_MR_SMOD, true);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int mchp_pit64b_clkevt_set_next_event(unsigned long evt,
>>> +					     struct clock_event_device *cedev)
>>> +{
>>> +	mchp_pit64b_set_period(data.ced->cd->base, evt);
>>> +	mchp_pit64b_write(data.ced->cd->base, MCHP_PIT64B_CR,
>>> +			  MCHP_PIT64B_CR_START);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static void mchp_pit64b_clkevt_suspend(struct clock_event_device *cedev)
>>> +{
>>> +	mchp_pit64b_write(data.ced->cd->base, MCHP_PIT64B_CR,
>>> +			  MCHP_PIT64B_CR_SWRST);
>>> +	if (data.ced->cd->gclk)
>>> +		clk_disable_unprepare(data.ced->cd->gclk);
>>> +	clk_disable_unprepare(data.ced->cd->pclk);
>>> +}
>>> +
>>> +static void mchp_pit64b_clkevt_resume(struct clock_event_device *cedev)
>>> +{
>>> +	u32 mode = MCHP_PIT64B_MR_SMOD;
>>> +
>>> +	clk_prepare_enable(data.ced->cd->pclk);
>>> +	if (data.ced->cd->gclk)
>>> +		clk_prepare_enable(data.ced->cd->gclk);
>>> +
>>> +	if (clockevent_state_periodic(data.ced->clkevt))
>>> +		mode = MCHP_PIT64B_MR_CONT;
>>> +
>>> +	mchp_pit64b_reset(data.ced->cd, mode, true);
>>> +}
>>> +
>>> +static struct clock_event_device mchp_pit64b_clkevt = {
>>> +	.name = MCHP_PIT64B_NAME,
>>> +	.features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC,
>>> +	.rating = 150,
>>> +	.set_state_shutdown = mchp_pit64b_clkevt_shutdown,
>>> +	.set_state_periodic = mchp_pit64b_clkevt_set_periodic,
>>> +	.set_state_oneshot = mchp_pit64b_clkevt_set_oneshot,
>>> +	.set_next_event = mchp_pit64b_clkevt_set_next_event,
>>> +	.suspend = mchp_pit64b_clkevt_suspend,
>>> +	.resume = mchp_pit64b_clkevt_resume,
>>> +};
>>> +
>>> +static irqreturn_t mchp_pit64b_interrupt(int irq, void *dev_id)
>>> +{
>>> +	struct mchp_pit64b_clkevt_data *irq_data = dev_id;
>>> +
>>> +	if (data.ced != irq_data)
>>> +		return IRQ_NONE;
>>> +
>>> +	if (mchp_pit64b_read(irq_data->cd->base, MCHP_PIT64B_ISR) &
>>> +	    MCHP_PIT64B_ISR_PERIOD) {
>>> +		irq_data->clkevt->event_handler(irq_data->clkevt);
>>> +		return IRQ_HANDLED;
>>> +	}
>>> +
>>> +	return IRQ_NONE;
>>> +}
>>> +
>>> +static int __init mchp_pit64b_pres_compute(u32 *pres, u32 clk_rate,
>>> +					   u32 max_rate)
>>> +{
>>> +	u32 tmp;
>>> +
>>> +	for (*pres = 0; *pres < MCHP_PIT64B_PRES_MAX; (*pres)++) {
>>> +		tmp = clk_rate / (*pres + 1);
>>> +		if (tmp <= max_rate)
>>> +			break;
>>> +	}
>>> +
>>> +	if (*pres == MCHP_PIT64B_PRES_MAX)
>>> +		return -EINVAL;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int __init mchp_pit64b_pres_prepare(struct mchp_pit64b_common_data *cd,
>>> +					   unsigned long max_rate)
>>> +{
>>> +	unsigned long pclk_rate, diff = 0, best_diff = ULONG_MAX;
>>> +	long gclk_round = 0;
>>> +	u32 pres, best_pres;
>>> +	int ret = 0;
>>> +
>>> +	pclk_rate = clk_get_rate(cd->pclk);
>>> +	if (!pclk_rate)
>>> +		return -EINVAL;
>>> +
>>> +	if (cd->gclk) {
>>> +		gclk_round = clk_round_rate(cd->gclk, max_rate);
>>> +		if (gclk_round < 0)
>>> +			goto pclk;
>>> +
>>> +		if (pclk_rate / gclk_round < 3)
>>> +			goto pclk;
>>> +
>>> +		ret = mchp_pit64b_pres_compute(&pres, gclk_round, max_rate);
>>> +		if (ret)
>>> +			best_diff = abs(gclk_round - max_rate);
>>> +		else
>>> +			best_diff = abs(gclk_round / (pres + 1) - max_rate);
>>> +		best_pres = pres;
>>> +	}
>>> +
>>> +pclk:
>>> +	/* Check if requested rate could be obtained using PCLK. */
>>> +	ret = mchp_pit64b_pres_compute(&pres, pclk_rate, max_rate);
>>> +	if (ret)
>>> +		diff = abs(pclk_rate - max_rate);
>>> +	else
>>> +		diff = abs(pclk_rate / (pres + 1) - max_rate);
>>> +
>>> +	if (best_diff > diff) {
>>> +		/* Use PCLK. */
>>> +		cd->gclk = NULL;
>>> +		best_pres = pres;
>>> +	} else {
>>> +		clk_set_rate(cd->gclk, gclk_round);
>>> +	}
>>> +
>>> +	cd->pres = best_pres;
>>> +
>>> +	pr_info("PIT64B: using clk=%s with prescaler %u, freq=%lu [Hz]\n",
>>> +		cd->gclk ? "gclk" : "pclk", cd->pres,
>>> +		cd->gclk ? gclk_round / (cd->pres + 1)
>>> +			 : pclk_rate / (cd->pres + 1));
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int __init mchp_pit64b_dt_init_clksrc(struct mchp_pit64b_common_data *cd)
>>> +{
>>> +	struct mchp_pit64b_clksrc_data *csd;
>>> +	unsigned long clk_rate;
>>> +	int ret;
>>> +
>>> +	csd = kzalloc(sizeof(*csd), GFP_KERNEL);
>>> +	if (!csd)
>>> +		return -ENOMEM;
>>> +
>>> +	csd->cd = cd;
>>> +
>>> +	if (csd->cd->gclk)
>>> +		clk_rate = clk_get_rate(csd->cd->gclk);
>>> +	else
>>> +		clk_rate = clk_get_rate(csd->cd->pclk);
>>> +
>>> +	clk_rate = clk_rate / (cd->pres + 1);
>>> +	csd->cd->cycles = ULLONG_MAX;
>>> +	mchp_pit64b_reset(csd->cd, MCHP_PIT64B_MR_CONT, false);
>>> +
>>> +	data.csd = csd;
>>> +
>>> +	csd->clksrc = &mchp_pit64b_clksrc;
>>> +
>>> +	ret = clocksource_register_hz(csd->clksrc, clk_rate);
>>> +	if (ret) {
>>> +		pr_debug("clksrc: Failed to register PIT64B clocksource!\n");
>>> +		goto free;
>>> +	}
>>> +
>>> +	sched_clock_register(mchp_sched_read_clk, 64, clk_rate);
>>> +
>>> +	return 0;
>>> +
>>> +free:
>>> +	kfree(csd);
>>> +	data.csd = NULL;
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static int __init mchp_pit64b_dt_init_clkevt(struct mchp_pit64b_common_data *cd,
>>> +					     u32 irq)
>>> +{
>>> +	struct mchp_pit64b_clkevt_data *ced;
>>> +	unsigned long clk_rate;
>>> +	int ret;
>>> +
>>> +	ced = kzalloc(sizeof(*ced), GFP_KERNEL);
>>> +	if (!ced)
>>> +		return -ENOMEM;
>>> +
>>> +	ced->cd = cd;
>>> +
>>> +	if (ced->cd->gclk)
>>> +		clk_rate = clk_get_rate(ced->cd->gclk);
>>> +	else
>>> +		clk_rate = clk_get_rate(ced->cd->pclk);
>>> +
>>> +	clk_rate = clk_rate / (ced->cd->pres + 1);
>>> +	ced->cd->cycles = DIV_ROUND_CLOSEST(clk_rate, HZ);
>>> +
>>> +	ret = request_irq(irq, mchp_pit64b_interrupt, IRQF_TIMER, "pit64b_tick",
>>> +			  ced);
>>> +	if (ret) {
>>> +		pr_debug("clkevt: Failed to setup PIT64B IRQ\n");
>>> +		goto free;
>>> +	}
>>> +
>>> +	data.ced = ced;
>>> +
>>> +	/* Set up and register clockevents. */
>>> +	ced->clkevt = &mchp_pit64b_clkevt;
>>> +	ced->clkevt->cpumask = cpumask_of(0);
>>> +	ced->clkevt->irq = irq;
>>> +	clockevents_config_and_register(ced->clkevt, clk_rate, 1, ULONG_MAX);
>>> +
>>> +	return 0;
>>> +
>>> +free:
>>> +	kfree(ced);
>>> +	data.ced = NULL;
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static int __init mchp_pit64b_dt_init(struct device_node *node)
>>> +{
>>> +	struct mchp_pit64b_common_data *cd;
>>> +	u32 irq, freq = MCHP_PIT64B_DEF_FREQ;
>>> +	int ret;
>>> +
>>> +	if (data.csd && data.ced)
>>> +		return -EBUSY;
>>> +
>>> +	cd = kzalloc(sizeof(*cd), GFP_KERNEL);
>>> +	if (!cd)
>>> +		return -ENOMEM;
>>> +
>>> +	cd->pclk = of_clk_get_by_name(node, "pclk");
>>> +	if (IS_ERR(cd->pclk)) {
>>> +		ret = PTR_ERR(cd->pclk);
>>> +		goto free;
>>> +	}
>>> +
>>> +	cd->gclk = of_clk_get_by_name(node, "gclk");
>>> +	if (IS_ERR(cd->gclk))
>>> +		cd->gclk = NULL;
>>> +
>>> +	ret = of_property_read_u32(node, "clock-frequency", &freq);
>>> +	if (ret)
>>> +		pr_debug("PIT64B: failed to read clock frequency. Using default!\n");
>>> +
>>> +	ret = mchp_pit64b_pres_prepare(cd, freq);
>>> +	if (ret)
>>> +		goto free;
>>> +
>>> +	cd->base = of_iomap(node, 0);
>>> +	if (!cd->base) {
>>> +		pr_debug("%s: Could not map PIT64B address!\n",
>>> +			 MCHP_PIT64B_NAME);
>>> +		ret = -ENXIO;
>>> +		goto free;
>>> +	}
>>> +
>>> +	ret = clk_prepare_enable(cd->pclk);
>>> +	if (ret)
>>> +		goto unmap;
>>> +
>>> +	if (cd->gclk) {
>>> +		ret = clk_prepare_enable(cd->gclk);
>>> +		if (ret)
>>> +			goto pclk_unprepare;
>>> +	}
>>> +
>>> +	if (!data.ced) {
>>> +		irq = irq_of_parse_and_map(node, 0);
>>> +		if (!irq) {
>>> +			pr_debug("%s: Failed to get PIT64B clockevent IRQ!\n",
>>> +				 MCHP_PIT64B_NAME);
>>> +			ret = -ENODEV;
>>> +			goto gclk_unprepare;
>>> +		}
>>> +		ret = mchp_pit64b_dt_init_clkevt(cd, irq);
>>> +		if (ret)
>>> +			goto irq_unmap;
>>> +	} else {
>>> +		ret = mchp_pit64b_dt_init_clksrc(cd);
>>> +		if (ret)
>>> +			goto gclk_unprepare;
>>> +	}
>>> +
>>> +	return 0;
>>> +
>>> +irq_unmap:
>>> +	irq_dispose_mapping(irq);
>>> +gclk_unprepare:
>>> +	if (cd->gclk)
>>> +		clk_disable_unprepare(cd->gclk);
>>> +pclk_unprepare:
>>> +	clk_disable_unprepare(cd->pclk);
>>> +unmap:
>>> +	iounmap(cd->base);
>>> +free:
>>> +	kfree(cd);
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +TIMER_OF_DECLARE(mchp_pit64b_clksrc, "microchip,sam9x60-pit64b",
>>> +		 mchp_pit64b_dt_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
>>
> 


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

Re: [PATCH 2/5] clocksource/drivers/timer-microchip-pit64b: add Microchip PIT64B support

[Alexandre Belloni]

On 08/04/2019 14:35:05+0200, Daniel Lezcano wrote:
> 
> What about commit 51f0aeb2d21f1 ?
> 

Well, do you see anything parsing that in drivers/clocksource ?


-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 2/5] clocksource/drivers/timer-microchip-pit64b: add Microchip PIT64B support

[Daniel Lezcano]

On 08/04/2019 14:42, Alexandre Belloni wrote:
> On 08/04/2019 14:35:05+0200, Daniel Lezcano wrote:
>>
>> What about commit 51f0aeb2d21f1 ?
>>
> 
> Well, do you see anything parsing that in drivers/clocksource ?

So to make it clear:

1. You say I said anything, emphasis this word in the previous answer.
But you are the one who should have argue and give the reasons of the
changes (and I'm sure they are valid). At the moment of the discussion
in the thread you mentioned, the DT change was already present.
 - Why did you not clarified this point in the thread discussion?
 - Why there is no Rob's acked-by in this commit?


2. You keep sending the atmel rework series again and again. And I'm
reviewing it again and again. And you object every single comment I do
on your code. I've already told you that.


3. I'm putting on the table again this clockevent/clocksource selection
from the DT hoping we can finally find a solution for *everyone* and
instead of jumping on the opportunity to discuss it, you blame me to not
have done this for you before.


4. Bonus, you resend your series again for the nth times two years after
the last discussion.


Do you want to see some progress?

Propose something generic telling if the node pointer is for a
clocksource or a clockevent. Get agreement from everyone and then resend
your atmel rework based on this.

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

Re: [PATCH 2/5] clocksource/drivers/timer-microchip-pit64b: add Microchip PIT64B support

[Alexandre Belloni]

On 08/04/2019 15:22:50+0200, Daniel Lezcano wrote:
> On 08/04/2019 14:42, Alexandre Belloni wrote:
> > On 08/04/2019 14:35:05+0200, Daniel Lezcano wrote:
> >>
> >> What about commit 51f0aeb2d21f1 ?
> >>
> > 
> > Well, do you see anything parsing that in drivers/clocksource ?
> 
> So to make it clear:
> 
> 1. You say I said anything, emphasis this word in the previous answer.
> But you are the one who should have argue and give the reasons of the
> changes (and I'm sure they are valid). At the moment of the discussion
> in the thread you mentioned, the DT change was already present.
>  - Why did you not clarified this point in the thread discussion?
>  - Why there is no Rob's acked-by in this commit?
> 

This was a left over that had absolutely no influence on anything. I'll
remove it. There was no ack because nobody expects Rob to review all the
DT changes.

> 
> 2. You keep sending the atmel rework series again and again. And I'm
> reviewing it again and again. And you object every single comment I do
> on your code. I've already told you that.
> 

Have a look, I stopped trying to send it as a new driver and now the
changes are minimal, to solve the specific issues we have.

> 
> 3. I'm putting on the table again this clockevent/clocksource selection
> from the DT hoping we can finally find a solution for *everyone* and
> instead of jumping on the opportunity to discuss it, you blame me to not
> have done this for you before.
> 

I've been jumping on the opportunity since 2016 and like I pointed, last
time, you didn't even discuss it.

> 
> 4. Bonus, you resend your series again for the nth times two years after
> the last discussion.
> 

This is not the same series. And it has not been two year, the last one
was sent in September 2018.

> 
> Do you want to see some progress?
> 
> Propose something generic telling if the node pointer is for a
> clocksource or a clockevent. Get agreement from everyone and then resend
> your atmel rework based on this.
> 

This will never work. We have been doing plenty of work and participated
to plenty of discussions since 2016. Every time, you asked us to have a
look at the other drivers. We did. We tried to get something generic
enough and you never backed us. It is the maintainer job to ensure a
proposed solution is generic enough as he is the one that has a global
view of the different drivers and HW platforms. You should not ask that
from platform maintainers. If you want to have a generic binding, you
have to be active and discuss it with Rob.

Anyway, this is now out of the way for the TCB rework because I'm not
touching that part anymore. But I want to ensures this does not impede
the PIT64 driver.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 2/5] clocksource/drivers/timer-microchip-pit64b: add Microchip PIT64B support

[Claudiu.Beznea]

Hi Daniel,

Taking into account the discussion on this tread and the fact that we have
no answer from Rob on this topic (I'm talking about [1]), what do you think
it would be best for this driver to be accepted the soonest? Would it be OK
for you to mimic the approach done by:

drivers/clocksource/timer-integrator-ap.c

with the following bindings in DT:

aliases {
	arm,timer-primary = &timer2;
	arm,timer-secondary = &timer1;
};

also in PIT64B driver?

Or do you think re-spinning the Alexandre's patches at [2] (which seems to
me like the generic way to do it) would be better?

Thank you,
Claudiu Beznea

[1]
https://lore.kernel.org/lkml/20190408151155.20279-1-alexandre.belloni@bootlin.com/#t
[2]
https://lore.kernel.org/lkml/20171213185313.20017-1-alexandre.belloni@free-electrons.com/

On 08.04.2019 15:11, Alexandre Belloni wrote:
> External E-Mail
> 
> 
> Hi Daniel,
> 
> On 08/04/2019 10:43:26+0200, Daniel Lezcano wrote:
>> Hi Claudiu,
>>
>> On 14/03/2019 17:26, Claudiu.Beznea@microchip.com wrote:
>>> From: Claudiu Beznea <claudiu.beznea@microchip.com>
>>>
>>> Add driver for Microchip PIT64B timer. Timer could be used in continuous
>>> mode or oneshot mode. The hardware has 2x32 bit registers for period
>>> emulating a 64 bit timer. The LSB_PR and MSB_PR registers are used to set
>>> the period value (compare value). TLSB and TMSB keeps the current value
>>> of the counter. After a compare the TLSB and TMSB register resets. Apart
>>> from this the hardware has SMOD bit in mode register that allow to
>>> reconfigure the timer without reset and start commands (start command
>>> while timer is active is ignored).
>>> The driver uses PIT64B timer as clocksource or clockevent. First requested
>>> timer would be registered as clockevent, second one would be registered as
>>> clocksource.
>>
>> Even if that was done this way before, assuming the DT describes the
>> clockevent at the first place and then the clocksource, it is a fragile
>> approach.
>>
>> What about using one of these approach?
>>
>> eg.
>>
>> arch/arm/boot/dts/at91sam9261ek.dts
>>
>> chosen {
>> 	bootargs = "rootfstype=ubifs ubi.mtd=5 root=ubi0:rootfs rw";
>>                 stdout-path = "serial0:115200n8";
>>
>> 	clocksource {
>> 		timer = <&timer0>;
>> 	};
>>
>>         clockevent {
>> 		timer = <&timer1>;
>>         };
>> };
>>
> 
> I suggested and implemented exactly that back in 2017 and it was shot
> down by both Rob and Mark:
> 
> https://lore.kernel.org/lkml/20171213185313.20017-1-alexandre.belloni@free-electrons.com/
> 
> At the time, you didn't do *anything* to get it accepted, you stayed
> silent. I can respin the series but then I see two options:
>  - either you back up the series and really push for it
>  - or you simply take this driver as it is. There is nothing in it that
>    couldn't be reworked later once you reached a conclusion with the DT
> maintainers.
> 
>> or
>>
>> arch/arm/boot/dts/integratorap.dts
>>
>> aliases {
>> 	arm,timer-primary = &timer2;
>> 	arm,timer-secondary = &timer1;
>> };
>>
>> So we can have control of what is the clocksource or the clockevent.
>> That is particulary handy in case of multiple channels.
>>
>> Not sure if we can replace the 'arm,timer_primary' to 'clocksource'.
>>
>> Rob? What is your opinion?
>>
>>> Individual PIT64B hardware resources were used for clocksource
>>> and clockevent to be able to support high resolution timers with this
>>> hardware implementation.
>>>
>>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>>> ---
>>>  drivers/clocksource/Kconfig                  |   6 +
>>>  drivers/clocksource/Makefile                 |   1 +
>>>  drivers/clocksource/timer-microchip-pit64b.c | 464 +++++++++++++++++++++++++++
>>>  3 files changed, 471 insertions(+)
>>>  create mode 100644 drivers/clocksource/timer-microchip-pit64b.c
>>>
>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>> index 5d93e580e5dc..2ad6f881a0bb 100644
>>> --- a/drivers/clocksource/Kconfig
>>> +++ b/drivers/clocksource/Kconfig
>>> @@ -448,6 +448,12 @@ config OXNAS_RPS_TIMER
>>>  config SYS_SUPPORTS_SH_CMT
>>>          bool
>>>  
>>> +config MICROCHIP_PIT64B
>>> +	bool "Microchip PIT64B support"
>>> +	depends on OF || COMPILE_TEST
>>> +	help
>>> +	  This option enables Microchip PIT64B timer.
>>> +
>>>  config MTK_TIMER
>>>  	bool "Mediatek timer driver" if COMPILE_TEST
>>>  	depends on HAS_IOMEM
>>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>>> index c4a8e9ef932a..c53fa12b9b94 100644
>>> --- a/drivers/clocksource/Makefile
>>> +++ b/drivers/clocksource/Makefile
>>> @@ -35,6 +35,7 @@ obj-$(CONFIG_U300_TIMER)	+= timer-u300.o
>>>  obj-$(CONFIG_SUN4I_TIMER)	+= timer-sun4i.o
>>>  obj-$(CONFIG_SUN5I_HSTIMER)	+= timer-sun5i.o
>>>  obj-$(CONFIG_MESON6_TIMER)	+= timer-meson6.o
>>> +obj-$(CONFIG_MICROCHIP_PIT64B)	+= timer-microchip-pit64b.o
>>>  obj-$(CONFIG_TEGRA_TIMER)	+= timer-tegra20.o
>>>  obj-$(CONFIG_VT8500_TIMER)	+= timer-vt8500.o
>>>  obj-$(CONFIG_NSPIRE_TIMER)	+= timer-zevio.o
>>> diff --git a/drivers/clocksource/timer-microchip-pit64b.c b/drivers/clocksource/timer-microchip-pit64b.c
>>> new file mode 100644
>>> index 000000000000..6787aa98ef01
>>> --- /dev/null
>>> +++ b/drivers/clocksource/timer-microchip-pit64b.c
>>> @@ -0,0 +1,464 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +//
>>> +// Copyright (C) 2019 Microchip Technology Inc.
>>> +// Copyright (C) 2019 Claudiu Beznea (claudiu.beznea@microchip.com)
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/clockchips.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/of_address.h>
>>> +#include <linux/of_irq.h>
>>> +#include <linux/sched_clock.h>
>>> +#include <linux/slab.h>
>>> +
>>> +#define MCHP_PIT64B_CR		0x00	/* Control Register */
>>> +#define MCHP_PIT64B_CR_START	BIT(0)
>>> +#define MCHP_PIT64B_CR_SWRST	BIT(8)
>>> +
>>> +#define MCHP_PIT64B_MR		0x04	/* Mode Register */
>>> +#define MCHP_PIT64B_MR_CONT	BIT(0)
>>> +#define MCHP_PIT64B_MR_SGCLK	BIT(3)
>>> +#define MCHP_PIT64B_MR_SMOD	BIT(4)
>>> +#define MCHP_PIT64B_MR_PRES	GENMASK(11, 8)
>>> +
>>> +#define MCHP_PIT64B_LSB_PR	0x08	/* LSB Period Register */
>>> +
>>> +#define MCHP_PIT64B_MSB_PR	0x0C	/* MSB Period Register */
>>> +
>>> +#define MCHP_PIT64B_IER		0x10	/* Interrupt Enable Register */
>>> +#define MCHP_PIT64B_IER_PERIOD	BIT(0)
>>> +
>>> +#define MCHP_PIT64B_ISR		0x1C	/* Interrupt Status Register */
>>> +#define MCHP_PIT64B_ISR_PERIOD	BIT(0)
>>> +
>>> +#define MCHP_PIT64B_TLSBR	0x20	/* Timer LSB Register */
>>> +
>>> +#define MCHP_PIT64B_TMSBR	0x24	/* Timer MSB Register */
>>> +
>>> +#define MCHP_PIT64B_PRES_MAX	0x10
>>> +#define MCHP_PIT64B_DEF_FREQ	2500000UL	/* 2.5 MHz */
>>> +#define MCHP_PIT64B_LSBMASK	GENMASK_ULL(31, 0)
>>> +#define MCHP_PIT64B_PRESCALER(p)	(MCHP_PIT64B_MR_PRES & ((p) << 8))
>>> +
>>> +#define MCHP_PIT64B_NAME	"pit64b"
>>> +
>>> +struct mchp_pit64b_common_data {
>>> +	void __iomem *base;
>>> +	struct clk *pclk;
>>> +	struct clk *gclk;
>>> +	u64 cycles;
>>> +	u8 pres;
>>> +};
>>> +
>>> +struct mchp_pit64b_clksrc_data {
>>> +	struct clocksource *clksrc;
>>> +	struct mchp_pit64b_common_data *cd;
>>> +};
>>> +
>>> +struct mchp_pit64b_clkevt_data {
>>> +	struct clock_event_device *clkevt;
>>> +	struct mchp_pit64b_common_data *cd;
>>> +};
>>> +
>>> +static struct mchp_pit64b_data {
>>> +	struct mchp_pit64b_clksrc_data *csd;
>>> +	struct mchp_pit64b_clkevt_data *ced;
>>> +} data;
>>> +
>>> +static inline u32 mchp_pit64b_read(void __iomem *base, u32 offset)
>>> +{
>>> +	return readl_relaxed(base + offset);
>>> +}
>>> +
>>> +static inline void mchp_pit64b_write(void __iomem *base, u32 offset, u32 val)
>>> +{
>>> +	writel_relaxed(val, base + offset);
>>> +}
>>> +
>>> +static inline u64 mchp_pit64b_get_period(void __iomem *base)
>>> +{
>>> +	u32 lsb, msb;
>>> +
>>> +	/* LSB must be read first to guarantee an atomic read of the 64 bit
>>> +	 * timer.
>>> +	 */
>>> +	lsb = mchp_pit64b_read(base, MCHP_PIT64B_TLSBR);
>>> +	msb = mchp_pit64b_read(base, MCHP_PIT64B_TMSBR);
>>> +
>>> +	return (((u64)msb << 32) | lsb);
>>> +}
>>> +
>>> +static inline void mchp_pit64b_set_period(void __iomem *base, u64 cycles)
>>> +{
>>> +	u32 lsb, msb;
>>> +
>>> +	lsb = cycles & MCHP_PIT64B_LSBMASK;
>>> +	msb = cycles >> 32;
>>> +
>>> +	/* LSB must be write last to guarantee an atomic update of the timer
>>> +	 * even when SMOD=1.
>>> +	 */
>>> +	mchp_pit64b_write(base, MCHP_PIT64B_MSB_PR, msb);
>>> +	mchp_pit64b_write(base, MCHP_PIT64B_LSB_PR, lsb);
>>> +}
>>> +
>>> +static inline void mchp_pit64b_reset(struct mchp_pit64b_common_data *data,
>>> +				     u32 mode, bool irq_ena)
>>> +{
>>> +	mode |= MCHP_PIT64B_PRESCALER(data->pres);
>>> +	if (data->gclk)
>>> +		mode |= MCHP_PIT64B_MR_SGCLK;
>>> +
>>> +	mchp_pit64b_write(data->base, MCHP_PIT64B_CR, MCHP_PIT64B_CR_SWRST);
>>> +	mchp_pit64b_write(data->base, MCHP_PIT64B_MR, mode);
>>> +	mchp_pit64b_set_period(data->base, data->cycles);
>>> +	if (irq_ena)
>>> +		mchp_pit64b_write(data->base, MCHP_PIT64B_IER,
>>> +				  MCHP_PIT64B_IER_PERIOD);
>>> +	mchp_pit64b_write(data->base, MCHP_PIT64B_CR, MCHP_PIT64B_CR_START);
>>> +}
>>> +
>>> +static u64 mchp_pit64b_read_clk(struct clocksource *cs)
>>> +{
>>> +	return mchp_pit64b_get_period(data.csd->cd->base);
>>> +}
>>> +
>>> +static u64 mchp_sched_read_clk(void)
>>> +{
>>> +	return mchp_pit64b_get_period(data.csd->cd->base);
>>> +}
>>> +
>>> +static struct clocksource mchp_pit64b_clksrc = {
>>> +	.name = MCHP_PIT64B_NAME,
>>> +	.mask = CLOCKSOURCE_MASK(64),
>>> +	.flags = CLOCK_SOURCE_IS_CONTINUOUS,
>>> +	.rating = 210,
>>> +	.read = mchp_pit64b_read_clk,
>>> +};
>>> +
>>> +static int mchp_pit64b_clkevt_shutdown(struct clock_event_device *cedev)
>>> +{
>>> +	mchp_pit64b_write(data.ced->cd->base, MCHP_PIT64B_CR,
>>> +			  MCHP_PIT64B_CR_SWRST);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int mchp_pit64b_clkevt_set_periodic(struct clock_event_device *cedev)
>>> +{
>>> +	mchp_pit64b_reset(data.ced->cd, MCHP_PIT64B_MR_CONT, true);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int mchp_pit64b_clkevt_set_oneshot(struct clock_event_device *cedev)
>>> +{
>>> +	mchp_pit64b_reset(data.ced->cd, MCHP_PIT64B_MR_SMOD, true);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int mchp_pit64b_clkevt_set_next_event(unsigned long evt,
>>> +					     struct clock_event_device *cedev)
>>> +{
>>> +	mchp_pit64b_set_period(data.ced->cd->base, evt);
>>> +	mchp_pit64b_write(data.ced->cd->base, MCHP_PIT64B_CR,
>>> +			  MCHP_PIT64B_CR_START);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static void mchp_pit64b_clkevt_suspend(struct clock_event_device *cedev)
>>> +{
>>> +	mchp_pit64b_write(data.ced->cd->base, MCHP_PIT64B_CR,
>>> +			  MCHP_PIT64B_CR_SWRST);
>>> +	if (data.ced->cd->gclk)
>>> +		clk_disable_unprepare(data.ced->cd->gclk);
>>> +	clk_disable_unprepare(data.ced->cd->pclk);
>>> +}
>>> +
>>> +static void mchp_pit64b_clkevt_resume(struct clock_event_device *cedev)
>>> +{
>>> +	u32 mode = MCHP_PIT64B_MR_SMOD;
>>> +
>>> +	clk_prepare_enable(data.ced->cd->pclk);
>>> +	if (data.ced->cd->gclk)
>>> +		clk_prepare_enable(data.ced->cd->gclk);
>>> +
>>> +	if (clockevent_state_periodic(data.ced->clkevt))
>>> +		mode = MCHP_PIT64B_MR_CONT;
>>> +
>>> +	mchp_pit64b_reset(data.ced->cd, mode, true);
>>> +}
>>> +
>>> +static struct clock_event_device mchp_pit64b_clkevt = {
>>> +	.name = MCHP_PIT64B_NAME,
>>> +	.features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC,
>>> +	.rating = 150,
>>> +	.set_state_shutdown = mchp_pit64b_clkevt_shutdown,
>>> +	.set_state_periodic = mchp_pit64b_clkevt_set_periodic,
>>> +	.set_state_oneshot = mchp_pit64b_clkevt_set_oneshot,
>>> +	.set_next_event = mchp_pit64b_clkevt_set_next_event,
>>> +	.suspend = mchp_pit64b_clkevt_suspend,
>>> +	.resume = mchp_pit64b_clkevt_resume,
>>> +};
>>> +
>>> +static irqreturn_t mchp_pit64b_interrupt(int irq, void *dev_id)
>>> +{
>>> +	struct mchp_pit64b_clkevt_data *irq_data = dev_id;
>>> +
>>> +	if (data.ced != irq_data)
>>> +		return IRQ_NONE;
>>> +
>>> +	if (mchp_pit64b_read(irq_data->cd->base, MCHP_PIT64B_ISR) &
>>> +	    MCHP_PIT64B_ISR_PERIOD) {
>>> +		irq_data->clkevt->event_handler(irq_data->clkevt);
>>> +		return IRQ_HANDLED;
>>> +	}
>>> +
>>> +	return IRQ_NONE;
>>> +}
>>> +
>>> +static int __init mchp_pit64b_pres_compute(u32 *pres, u32 clk_rate,
>>> +					   u32 max_rate)
>>> +{
>>> +	u32 tmp;
>>> +
>>> +	for (*pres = 0; *pres < MCHP_PIT64B_PRES_MAX; (*pres)++) {
>>> +		tmp = clk_rate / (*pres + 1);
>>> +		if (tmp <= max_rate)
>>> +			break;
>>> +	}
>>> +
>>> +	if (*pres == MCHP_PIT64B_PRES_MAX)
>>> +		return -EINVAL;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int __init mchp_pit64b_pres_prepare(struct mchp_pit64b_common_data *cd,
>>> +					   unsigned long max_rate)
>>> +{
>>> +	unsigned long pclk_rate, diff = 0, best_diff = ULONG_MAX;
>>> +	long gclk_round = 0;
>>> +	u32 pres, best_pres;
>>> +	int ret = 0;
>>> +
>>> +	pclk_rate = clk_get_rate(cd->pclk);
>>> +	if (!pclk_rate)
>>> +		return -EINVAL;
>>> +
>>> +	if (cd->gclk) {
>>> +		gclk_round = clk_round_rate(cd->gclk, max_rate);
>>> +		if (gclk_round < 0)
>>> +			goto pclk;
>>> +
>>> +		if (pclk_rate / gclk_round < 3)
>>> +			goto pclk;
>>> +
>>> +		ret = mchp_pit64b_pres_compute(&pres, gclk_round, max_rate);
>>> +		if (ret)
>>> +			best_diff = abs(gclk_round - max_rate);
>>> +		else
>>> +			best_diff = abs(gclk_round / (pres + 1) - max_rate);
>>> +		best_pres = pres;
>>> +	}
>>> +
>>> +pclk:
>>> +	/* Check if requested rate could be obtained using PCLK. */
>>> +	ret = mchp_pit64b_pres_compute(&pres, pclk_rate, max_rate);
>>> +	if (ret)
>>> +		diff = abs(pclk_rate - max_rate);
>>> +	else
>>> +		diff = abs(pclk_rate / (pres + 1) - max_rate);
>>> +
>>> +	if (best_diff > diff) {
>>> +		/* Use PCLK. */
>>> +		cd->gclk = NULL;
>>> +		best_pres = pres;
>>> +	} else {
>>> +		clk_set_rate(cd->gclk, gclk_round);
>>> +	}
>>> +
>>> +	cd->pres = best_pres;
>>> +
>>> +	pr_info("PIT64B: using clk=%s with prescaler %u, freq=%lu [Hz]\n",
>>> +		cd->gclk ? "gclk" : "pclk", cd->pres,
>>> +		cd->gclk ? gclk_round / (cd->pres + 1)
>>> +			 : pclk_rate / (cd->pres + 1));
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int __init mchp_pit64b_dt_init_clksrc(struct mchp_pit64b_common_data *cd)
>>> +{
>>> +	struct mchp_pit64b_clksrc_data *csd;
>>> +	unsigned long clk_rate;
>>> +	int ret;
>>> +
>>> +	csd = kzalloc(sizeof(*csd), GFP_KERNEL);
>>> +	if (!csd)
>>> +		return -ENOMEM;
>>> +
>>> +	csd->cd = cd;
>>> +
>>> +	if (csd->cd->gclk)
>>> +		clk_rate = clk_get_rate(csd->cd->gclk);
>>> +	else
>>> +		clk_rate = clk_get_rate(csd->cd->pclk);
>>> +
>>> +	clk_rate = clk_rate / (cd->pres + 1);
>>> +	csd->cd->cycles = ULLONG_MAX;
>>> +	mchp_pit64b_reset(csd->cd, MCHP_PIT64B_MR_CONT, false);
>>> +
>>> +	data.csd = csd;
>>> +
>>> +	csd->clksrc = &mchp_pit64b_clksrc;
>>> +
>>> +	ret = clocksource_register_hz(csd->clksrc, clk_rate);
>>> +	if (ret) {
>>> +		pr_debug("clksrc: Failed to register PIT64B clocksource!\n");
>>> +		goto free;
>>> +	}
>>> +
>>> +	sched_clock_register(mchp_sched_read_clk, 64, clk_rate);
>>> +
>>> +	return 0;
>>> +
>>> +free:
>>> +	kfree(csd);
>>> +	data.csd = NULL;
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static int __init mchp_pit64b_dt_init_clkevt(struct mchp_pit64b_common_data *cd,
>>> +					     u32 irq)
>>> +{
>>> +	struct mchp_pit64b_clkevt_data *ced;
>>> +	unsigned long clk_rate;
>>> +	int ret;
>>> +
>>> +	ced = kzalloc(sizeof(*ced), GFP_KERNEL);
>>> +	if (!ced)
>>> +		return -ENOMEM;
>>> +
>>> +	ced->cd = cd;
>>> +
>>> +	if (ced->cd->gclk)
>>> +		clk_rate = clk_get_rate(ced->cd->gclk);
>>> +	else
>>> +		clk_rate = clk_get_rate(ced->cd->pclk);
>>> +
>>> +	clk_rate = clk_rate / (ced->cd->pres + 1);
>>> +	ced->cd->cycles = DIV_ROUND_CLOSEST(clk_rate, HZ);
>>> +
>>> +	ret = request_irq(irq, mchp_pit64b_interrupt, IRQF_TIMER, "pit64b_tick",
>>> +			  ced);
>>> +	if (ret) {
>>> +		pr_debug("clkevt: Failed to setup PIT64B IRQ\n");
>>> +		goto free;
>>> +	}
>>> +
>>> +	data.ced = ced;
>>> +
>>> +	/* Set up and register clockevents. */
>>> +	ced->clkevt = &mchp_pit64b_clkevt;
>>> +	ced->clkevt->cpumask = cpumask_of(0);
>>> +	ced->clkevt->irq = irq;
>>> +	clockevents_config_and_register(ced->clkevt, clk_rate, 1, ULONG_MAX);
>>> +
>>> +	return 0;
>>> +
>>> +free:
>>> +	kfree(ced);
>>> +	data.ced = NULL;
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static int __init mchp_pit64b_dt_init(struct device_node *node)
>>> +{
>>> +	struct mchp_pit64b_common_data *cd;
>>> +	u32 irq, freq = MCHP_PIT64B_DEF_FREQ;
>>> +	int ret;
>>> +
>>> +	if (data.csd && data.ced)
>>> +		return -EBUSY;
>>> +
>>> +	cd = kzalloc(sizeof(*cd), GFP_KERNEL);
>>> +	if (!cd)
>>> +		return -ENOMEM;
>>> +
>>> +	cd->pclk = of_clk_get_by_name(node, "pclk");
>>> +	if (IS_ERR(cd->pclk)) {
>>> +		ret = PTR_ERR(cd->pclk);
>>> +		goto free;
>>> +	}
>>> +
>>> +	cd->gclk = of_clk_get_by_name(node, "gclk");
>>> +	if (IS_ERR(cd->gclk))
>>> +		cd->gclk = NULL;
>>> +
>>> +	ret = of_property_read_u32(node, "clock-frequency", &freq);
>>> +	if (ret)
>>> +		pr_debug("PIT64B: failed to read clock frequency. Using default!\n");
>>> +
>>> +	ret = mchp_pit64b_pres_prepare(cd, freq);
>>> +	if (ret)
>>> +		goto free;
>>> +
>>> +	cd->base = of_iomap(node, 0);
>>> +	if (!cd->base) {
>>> +		pr_debug("%s: Could not map PIT64B address!\n",
>>> +			 MCHP_PIT64B_NAME);
>>> +		ret = -ENXIO;
>>> +		goto free;
>>> +	}
>>> +
>>> +	ret = clk_prepare_enable(cd->pclk);
>>> +	if (ret)
>>> +		goto unmap;
>>> +
>>> +	if (cd->gclk) {
>>> +		ret = clk_prepare_enable(cd->gclk);
>>> +		if (ret)
>>> +			goto pclk_unprepare;
>>> +	}
>>> +
>>> +	if (!data.ced) {
>>> +		irq = irq_of_parse_and_map(node, 0);
>>> +		if (!irq) {
>>> +			pr_debug("%s: Failed to get PIT64B clockevent IRQ!\n",
>>> +				 MCHP_PIT64B_NAME);
>>> +			ret = -ENODEV;
>>> +			goto gclk_unprepare;
>>> +		}
>>> +		ret = mchp_pit64b_dt_init_clkevt(cd, irq);
>>> +		if (ret)
>>> +			goto irq_unmap;
>>> +	} else {
>>> +		ret = mchp_pit64b_dt_init_clksrc(cd);
>>> +		if (ret)
>>> +			goto gclk_unprepare;
>>> +	}
>>> +
>>> +	return 0;
>>> +
>>> +irq_unmap:
>>> +	irq_dispose_mapping(irq);
>>> +gclk_unprepare:
>>> +	if (cd->gclk)
>>> +		clk_disable_unprepare(cd->gclk);
>>> +pclk_unprepare:
>>> +	clk_disable_unprepare(cd->pclk);
>>> +unmap:
>>> +	iounmap(cd->base);
>>> +free:
>>> +	kfree(cd);
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +TIMER_OF_DECLARE(mchp_pit64b_clksrc, "microchip,sam9x60-pit64b",
>>> +		 mchp_pit64b_dt_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
>>
> 

Re: [PATCH 2/5] clocksource/drivers/timer-microchip-pit64b: add Microchip PIT64B support

[Daniel Lezcano]

Hi Claudiu,


On 30/05/2019 09:46, Claudiu.Beznea@microchip.com wrote:
> Hi Daniel,
> 
> Taking into account the discussion on this tread and the fact that we have
> no answer from Rob on this topic (I'm talking about [1]), what do you think
> it would be best for this driver to be accepted the soonest? Would it be OK
> for you to mimic the approach done by:
> 
> drivers/clocksource/timer-integrator-ap.c
> 
> with the following bindings in DT:
> 
> aliases {
> 	arm,timer-primary = &timer2;
> 	arm,timer-secondary = &timer1;
> };
> 
> also in PIT64B driver?
> 
> Or do you think re-spinning the Alexandre's patches at [2] (which seems to
> me like the generic way to do it) would be better?

This hardware / OS connection problem is getting really annoying for
everyone and this pattern is repeating itself since several years. It is
time we fix it properly.

The first solution looks hackish from my POV. The second approach looks
nicer and generic as you say. So I would vote for [2] but with the
flagging approach proposed by Mark [3].

I added Arnd in Cc in order to have its opinion.

[3]
https://lore.kernel.org/lkml/20171215113242.skmh5nzr7wqdmvnw@lakrids.cambridge.arm.com/

> [1]
> https://lore.kernel.org/lkml/20190408151155.20279-1-alexandre.belloni@bootlin.com/#t
> [2]
> https://lore.kernel.org/lkml/20171213185313.20017-1-alexandre.belloni@free-electrons.com/
> 






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

Re: Re: [PATCH 2/5] clocksource/drivers/timer-microchip-pit64b: add Microchip PIT64B support

[Claudiu.Beznea]

Hi Daniel,

On 31.05.2019 13:41, Daniel Lezcano wrote:
> 
> Hi Claudiu,
> 
> 
> On 30/05/2019 09:46, Claudiu.Beznea@microchip.com wrote:
>> Hi Daniel,
>>
>> Taking into account the discussion on this tread and the fact that we have
>> no answer from Rob on this topic (I'm talking about [1]), what do you think
>> it would be best for this driver to be accepted the soonest? Would it be OK
>> for you to mimic the approach done by:
>>
>> drivers/clocksource/timer-integrator-ap.c
>>
>> with the following bindings in DT:
>>
>> aliases {
>> 	arm,timer-primary = &timer2;
>> 	arm,timer-secondary = &timer1;
>> };
>>
>> also in PIT64B driver?
>>
>> Or do you think re-spinning the Alexandre's patches at [2] (which seems to
>> me like the generic way to do it) would be better?
> 
> This hardware / OS connection problem is getting really annoying for
> everyone and this pattern is repeating itself since several years. It is
> time we fix it properly.
> 
> The first solution looks hackish from my POV. The second approach looks
> nicer and generic as you say. So I would vote for [2]
> flagging approach proposed by Mark [3].

With this flagging approach this would mean a kind unification of
clocksource and clockevent functionalities under a single one, right? So
that the driver would register to the above layers only one device w/ 2
functionalities (clocksource and clockevent)? Please correct me if I'm
wrong? If so, from my point of view this would require major re-working of
clocksource and clockevent subsystems. Correctly if I wrongly understood,
please.

At the moment we register different functionalities (clocksource and
clockevent) to the above layers for hardware blocks (e.g. with
clocksource_register_hz() or clockevents_config_and_register()). If
hardware can support clocksource and clockevent we register both these
functionalities, if only one is supported we register only one of these.
The above layers would choose the best clocksource/clockevent device from
the available ones based on rating field for each clocksource/clockevent we
register. In all this current behavior I don't see how these flags would
interact with clocksource/clockevent subsystem. Could you please let me
know how do you see these and the way these new flags would interact with
the layers above the drivers?

Thank you,
Claudiu Beznea

> 
> I added Arnd in Cc in order to have its opinion.
> 
> [3]
> https://lore.kernel.org/lkml/20171215113242.skmh5nzr7wqdmvnw@lakrids.cambridge.arm.com/
> 
>> [1]
>> https://lore.kernel.org/lkml/20190408151155.20279-1-alexandre.belloni@bootlin.com/#t
>> [2]
>> https://lore.kernel.org/lkml/20171213185313.20017-1-alexandre.belloni@free-electrons.com/
>>
> 
> 
> 
> 
> 
> 

Re: [PATCH 2/5] clocksource/drivers/timer-microchip-pit64b: add Microchip PIT64B support

[Daniel Lezcano]

Hi Claudiu,

sorry for the late reply.


On 13/06/2019 16:12, Claudiu.Beznea@microchip.com wrote:
> Hi Daniel,
> 
> On 31.05.2019 13:41, Daniel Lezcano wrote:
>>
>> Hi Claudiu,
>>
>>
>> On 30/05/2019 09:46, Claudiu.Beznea@microchip.com wrote:
>>> Hi Daniel,
>>>
>>> Taking into account the discussion on this tread and the fact that we have
>>> no answer from Rob on this topic (I'm talking about [1]), what do you think
>>> it would be best for this driver to be accepted the soonest? Would it be OK
>>> for you to mimic the approach done by:
>>>
>>> drivers/clocksource/timer-integrator-ap.c
>>>
>>> with the following bindings in DT:
>>>
>>> aliases {
>>> 	arm,timer-primary = &timer2;
>>> 	arm,timer-secondary = &timer1;
>>> };
>>>
>>> also in PIT64B driver?
>>>
>>> Or do you think re-spinning the Alexandre's patches at [2] (which seems to
>>> me like the generic way to do it) would be better?
>>
>> This hardware / OS connection problem is getting really annoying for
>> everyone and this pattern is repeating itself since several years. It is
>> time we fix it properly.
>>
>> The first solution looks hackish from my POV. The second approach looks
>> nicer and generic as you say. So I would vote for [2]
>> flagging approach proposed by Mark [3].
> 
> With this flagging approach this would mean a kind unification of
> clocksource and clockevent functionalities under a single one, right? So
> that the driver would register to the above layers only one device w/ 2
> functionalities (clocksource and clockevent)? Please correct me if I'm
> wrong? If so, from my point of view this would require major re-working of
> clocksource and clockevent subsystems. Correctly if I wrongly understood,
> please.

Well, actually I was not expecting to change all the framework but just
pass a flag to the probe function telling if the node is for a
clocksource, a clockevent or both.



> At the moment we register different functionalities (clocksource and
> clockevent) to the above layers for hardware blocks (e.g. with
> clocksource_register_hz() or clockevents_config_and_register()). If
> hardware can support clocksource and clockevent we register both these
> functionalities, if only one is supported we register only one of these.
> The above layers would choose the best clocksource/clockevent device from
> the available ones based on rating field for each clocksource/clockevent we
> register. In all this current behavior I don't see how these flags would
> interact with clocksource/clockevent subsystem. Could you please let me
> know how do you see these and the way these new flags would interact with
> the layers above the drivers?
>>
>> I added Arnd in Cc in order to have its opinion.
>>
>> [3]
>> https://lore.kernel.org/lkml/20171215113242.skmh5nzr7wqdmvnw@lakrids.cambridge.arm.com/
>>
>>> [1]
>>> https://lore.kernel.org/lkml/20190408151155.20279-1-alexandre.belloni@bootlin.com/#t
>>> [2]
>>> https://lore.kernel.org/lkml/20171213185313.20017-1-alexandre.belloni@free-electrons.com/
>>>
>>
>>
>>
>>
>>
>>


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

Re: [PATCH 2/5] clocksource/drivers/timer-microchip-pit64b: add Microchip PIT64B support

[Claudiu.Beznea]

Hi Daniel,

On 20.06.2019 11:53, Daniel Lezcano wrote:
> Hi Claudiu,
> 
> sorry for the late reply.

No problem, I understand.

> 
> 
> On 13/06/2019 16:12, Claudiu.Beznea@microchip.com wrote:
>> Hi Daniel,
>>
>> On 31.05.2019 13:41, Daniel Lezcano wrote:
>>>
>>> Hi Claudiu,
>>>
>>>
>>> On 30/05/2019 09:46, Claudiu.Beznea@microchip.com wrote:
>>>> Hi Daniel,
>>>>
>>>> Taking into account the discussion on this tread and the fact that we have
>>>> no answer from Rob on this topic (I'm talking about [1]), what do you think
>>>> it would be best for this driver to be accepted the soonest? Would it be OK
>>>> for you to mimic the approach done by:
>>>>
>>>> drivers/clocksource/timer-integrator-ap.c
>>>>
>>>> with the following bindings in DT:
>>>>
>>>> aliases {
>>>> 	arm,timer-primary = &timer2;
>>>> 	arm,timer-secondary = &timer1;
>>>> };
>>>>
>>>> also in PIT64B driver?
>>>>
>>>> Or do you think re-spinning the Alexandre's patches at [2] (which seems to
>>>> me like the generic way to do it) would be better?
>>>
>>> This hardware / OS connection problem is getting really annoying for
>>> everyone and this pattern is repeating itself since several years. It is
>>> time we fix it properly.
>>>
>>> The first solution looks hackish from my POV. The second approach looks
>>> nicer and generic as you say. So I would vote for [2]
>>> flagging approach proposed by Mark [3].
>>
>> With this flagging approach this would mean a kind unification of
>> clocksource and clockevent functionalities under a single one, right? So
>> that the driver would register to the above layers only one device w/ 2
>> functionalities (clocksource and clockevent)? Please correct me if I'm
>> wrong? If so, from my point of view this would require major re-working of
>> clocksource and clockevent subsystems. Correctly if I wrongly understood,
>> please.
> 
> Well, actually I was not expecting to change all the framework but just
> pass a flag to the probe function telling if the node is for a
> clocksource, a clockevent or both.
> 

Giving so, whit these proposals I'm thinking at having something like this,
using Alexandre's new macros from [2] and passing a bitmask to timer's
probing functions (in the above example adapted only for pit64b driver
introduced in this thread):

diff --git a/drivers/clocksource/timer-microchip-pit64b.c b/drivers/clocksource/timer-microchip-pit64b.c
index 62339d8187ce..b283d51ad4c7 100644
--- a/drivers/clocksource/timer-microchip-pit64b.c
+++ b/drivers/clocksource/timer-microchip-pit64b.c
@@ -377,7 +377,7 @@ static int __init mchp_pit64b_dt_init_clkevt(struct mchp_pit64b_common_data *cd,
        return ret;
 }
 
-static int __init mchp_pit64b_dt_init(struct device_node *node)
+static int __init mchp_pit64b_dt_init(struct device_node *node, u32 props)
 {
        struct mchp_pit64b_common_data *cd;
        u32 irq, freq = MCHP_PIT64B_DEF_FREQ;
@@ -426,7 +426,11 @@ static int __init mchp_pit64b_dt_init(struct device_node *node)
                        goto pclk_unprepare;
        }
 
-       if (!data.ced) {
+       if (props & TIMER_OF_PROPERTY_CLOCKSOURCE) {
+               if (data.ced)
+                       goto gclk_unprepare;
+
                irq = irq_of_parse_and_map(node, 0);
                if (!irq) {
                        pr_debug("%s: Failed to get PIT64B clockevent IRQ!\n",
@@ -437,7 +441,13 @@ static int __init mchp_pit64b_dt_init(struct device_node *node)
                ret = mchp_pit64b_dt_init_clkevt(cd, irq);
                if (ret)
                        goto irq_unmap;
-       } else {
+       }
+
+       if (props & TIMER_OF_PROPERTY_CLOCKEVENT) {
+               if (data.csd)
+                       goto gclk_unprepare;
+
                ret = mchp_pit64b_dt_init_clksrc(cd);
                if (ret)
                        goto gclk_unprepare;
diff --git a/drivers/clocksource/timer-of.h b/drivers/clocksource/timer-of.h
index a5478f3e8589..faf95c98b6d2 100644
--- a/drivers/clocksource/timer-of.h
+++ b/drivers/clocksource/timer-of.h
@@ -8,6 +8,10 @@
 #define TIMER_OF_CLOCK 0x2
 #define TIMER_OF_IRQ   0x4
 
+#define TIMER_OF_PROPERTY_CLOCKSOURCE  BIT(0)
+#define TIMER_OF_PROPERTY_CLOCKEVENT   BIT(1)
+
 struct of_timer_irq {
        int irq;
        int index;
diff --git a/drivers/clocksource/timer-probe.c b/drivers/clocksource/timer-probe.c
index 028075720334..69c45f7d198c 100644
--- a/drivers/clocksource/timer-probe.c
+++ b/drivers/clocksource/timer-probe.c
@@ -18,6 +18,8 @@
 #include <linux/init.h>
 #include <linux/of.h>
 #include <linux/clocksource.h>
+#include <linux/interrupt.h>
+#include "timer-of.h"
 
 extern struct of_device_id __timer_of_table[];
 
@@ -28,8 +30,9 @@ void __init timer_probe(void)
 {
        struct device_node *np;
        const struct of_device_id *match;
-       of_init_fn_1_ret init_func_ret;
+       of_init_fn_2_timer_ret init_func_ret;
        unsigned timers = 0;
+       u32 props = TIMER_OF_PROPERTY_CLOCKSOURCE | TIMER_OF_PROPERTY_CLOCKEVENT;
        int ret;
 
        for_each_matching_node_and_match(np, __timer_of_table, &match) {
@@ -38,7 +41,12 @@ void __init timer_probe(void)
 
                init_func_ret = match->data;
 
-               ret = init_func_ret(np);
+               if (timer_of_is_clocksource(np) &&
+		    !timer_of_is_clockevent(np))
+                       props = TIMER_OF_PROPERTY_CLOCKSOURCE;
+               if (timer_of_is_clockevent(np) &&
+		    !timer_of_is_clocksource(np))
+                       props = TIMER_OF_PROPERTY_CLOCKEVENT;
+
+               ret = init_func_ret(np, props);
                if (ret) {
                        pr_err("Failed to initialize '%pOF': %d\n", np, ret);
                        continue;
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 308918928767..5c4de4833ed8 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -255,7 +255,7 @@ extern int clocksource_mmio_init(void __iomem *, const char *,
 extern int clocksource_i8253_init(void);
 
 #define TIMER_OF_DECLARE(name, compat, fn) \
-       OF_DECLARE_1_RET(timer, name, compat, fn)
+       OF_DECLARE_2_TIMER_RET(timer, name, compat, fn)
 
 #define CLOCKSOURCE_OF_DECLARE(name, compat, fn) \
        TIMER_OF_DECLARE(name, compat, fn)
diff --git a/include/linux/of.h b/include/linux/of.h
index 99b0ebf49632..50a3c27f7717 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1258,6 +1258,7 @@ static inline int of_get_available_child_count(const struct device_node *np)
 #endif
 
 typedef int (*of_init_fn_2)(struct device_node *, struct device_node *);
+typedef int (*of_init_fn_2_timer_ret)(struct device_node *, u32);
 typedef int (*of_init_fn_1_ret)(struct device_node *);
 typedef void (*of_init_fn_1)(struct device_node *);
 
@@ -1267,6 +1268,8 @@ typedef void (*of_init_fn_1)(struct device_node *);
                _OF_DECLARE(table, name, compat, fn, of_init_fn_1_ret)
 #define OF_DECLARE_2(table, name, compat, fn) \
                _OF_DECLARE(table, name, compat, fn, of_init_fn_2)
+#define OF_DECLARE_2_TIMER_RET(table, name, compat, fn) \
+               _OF_DECLARE(table, name, compat, fn, of_init_fn_2_timer_ret)
 
 /**
  * struct of_changeset_entry   - Holds a changeset entry


The only downside of this is that we're parsing these new DT bindings before
calling probe function, then we're checking the result of parsing again, in
probe function, and I'm thinking if it wouldn't be simpler to just parse these
binding in timer's probe function as all the other DT bindings are parsed
(moreover all timers probing functions should be changes to get this new argument).

More than this I see that there is one timer driver which is not probed via
timer_probe() (I'm pointing to drivers/clocksource/numachip.c).

Please let me know what do you think about it.

Thank you,
Claudiu Beznea

> 
> 
>> At the moment we register different functionalities (clocksource and
>> clockevent) to the above layers for hardware blocks (e.g. with
>> clocksource_register_hz() or clockevents_config_and_register()). If
>> hardware can support clocksource and clockevent we register both these
>> functionalities, if only one is supported we register only one of these.
>> The above layers would choose the best clocksource/clockevent device from
>> the available ones based on rating field for each clocksource/clockevent we
>> register. In all this current behavior I don't see how these flags would
>> interact with clocksource/clockevent subsystem. Could you please let me
>> know how do you see these and the way these new flags would interact with
>> the layers above the drivers?
>>>
>>> I added Arnd in Cc in order to have its opinion.
>>>
>>> [3]
>>> https://lore.kernel.org/lkml/20171215113242.skmh5nzr7wqdmvnw@lakrids.cambridge.arm.com/
>>>
>>>> [1]
>>>> https://lore.kernel.org/lkml/20190408151155.20279-1-alexandre.belloni@bootlin.com/#t
>>>> [2]
>>>> https://lore.kernel.org/lkml/20171213185313.20017-1-alexandre.belloni@free-electrons.com/
>>>>
>>>
>>>
>>>
>>>
>>>
>>>
> 
> 

Re: [PATCH 2/5] clocksource/drivers/timer-microchip-pit64b: add Microchip PIT64B support

[Daniel Lezcano]

On 21/06/2019 12:34, Claudiu.Beznea@microchip.com wrote:
> Hi Daniel,
> 
> On 20.06.2019 11:53, Daniel Lezcano wrote:
>> Hi Claudiu,
>>
>> sorry for the late reply.
> 
> No problem, I understand.
> 
>>
>>
>> On 13/06/2019 16:12, Claudiu.Beznea@microchip.com wrote:
>>> Hi Daniel,
>>>
>>> On 31.05.2019 13:41, Daniel Lezcano wrote:
>>>>
>>>> Hi Claudiu,
>>>>
>>>>
>>>> On 30/05/2019 09:46, Claudiu.Beznea@microchip.com wrote:
>>>>> Hi Daniel,
>>>>>
>>>>> Taking into account the discussion on this tread and the fact that we have
>>>>> no answer from Rob on this topic (I'm talking about [1]), what do you think
>>>>> it would be best for this driver to be accepted the soonest? Would it be OK
>>>>> for you to mimic the approach done by:
>>>>>
>>>>> drivers/clocksource/timer-integrator-ap.c
>>>>>
>>>>> with the following bindings in DT:
>>>>>
>>>>> aliases {
>>>>> 	arm,timer-primary = &timer2;
>>>>> 	arm,timer-secondary = &timer1;
>>>>> };
>>>>>
>>>>> also in PIT64B driver?
>>>>>
>>>>> Or do you think re-spinning the Alexandre's patches at [2] (which seems to
>>>>> me like the generic way to do it) would be better?
>>>>
>>>> This hardware / OS connection problem is getting really annoying for
>>>> everyone and this pattern is repeating itself since several years. It is
>>>> time we fix it properly.
>>>>
>>>> The first solution looks hackish from my POV. The second approach looks
>>>> nicer and generic as you say. So I would vote for [2]
>>>> flagging approach proposed by Mark [3].
>>>
>>> With this flagging approach this would mean a kind unification of
>>> clocksource and clockevent functionalities under a single one, right? So
>>> that the driver would register to the above layers only one device w/ 2
>>> functionalities (clocksource and clockevent)? Please correct me if I'm
>>> wrong? If so, from my point of view this would require major re-working of
>>> clocksource and clockevent subsystems. Correctly if I wrongly understood,
>>> please.
>>
>> Well, actually I was not expecting to change all the framework but just
>> pass a flag to the probe function telling if the node is for a
>> clocksource, a clockevent or both.
>>
> 
> Giving so, whit these proposals I'm thinking at having something like this,
> using Alexandre's new macros from [2] and passing a bitmask to timer's
> probing functions (in the above example adapted only for pit64b driver
> introduced in this thread):

Yes basically that is what I had in mind. Thanks for taking care of
that. However after seeing the code I realize the impact is larger than
expected as all the TIMER_OF_DECLARE will be impacted.

AFAICT, this driver can be converted to the timer-of API. So I think it
makes sense to keep this contained in the API.

In the function timer_of_init(), we can add the parsing of the node and
set the timer-of flags with:

#define TIMER_OF_IS_CLOCKSOURCE	 0x8
#define TIMER_OF_IS_CLOCKEVENT  0x10

In addition the API:

int timer_of_is_clocksource(struct timer_of *to)
{
	return (to & TIMER_OF_IS_CLOCKSOURCE);
}
int timer_of_is_clockevents(struct timer_of *to)
{
	return (to & TIMER_OF_IS_CLOCKEVENT);
}




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