[PATCH v2 00/20] Add support for the NXP automotive S32G PIT

[PATCH v2 00/20] Add support for the NXP automotive S32G PIT

[Daniel Lezcano]

The S32G platform has a couple of Programable Interrupt Timer
(PIT). This timer is the same IP as the one found on the Vybrid Family
platform. The main difference is there are multiple instances of this
timer on the S32G.

Before bringing the support for the S32G in the driver, a
pre-requesite must be accomplished to support multiple instances of
the timer in the driver. While at it, take also the opportunity to
clean up the code and make it consistent with the NXP STM timer which
looks similar to the PIT.

The two changes fix the raw_(readl|writel) usage where it is replaced
by readl|writel. In order to compile test the changes, the Kconfig
COMPILE_TEST option is added.

The other changes encapsulate more the codes into self-explanatory
functions, allow to pass the per instance information as parameters to
functions and use different names for the timers in order to be able
to distinguish them.

The last two changes provide the DT bindings and the S32G specific
changes to support multiple instances.

Changelog:
  - v2:
    - Added a cover letter (Ghennadi Procopciuc)
    - Fixed typo in descriptions (Ghennadi Procopciuc)
    - Clarified comment about channel usage (Ghennadi Procopciuc)
    - Added a description for the bindings (Rob Herring)
    - Initialized sched_clock global variable (Ghennadi Procopciuc)
    - Changed type for the 'cpu' parameter s/int/unsigned int/ (Ghennadi Procopciuc)
    - Fixed wrong bitwise negation when reading the counter (Ghennadi Procopciuc)
    - Changed type s/int/u32/ for writel (Ghennadi Procopciuc)
    - Clarified changelog with S32G support (Ghennadi Procopciuc)

  - v1: Initial post

Daniel Lezcano (20):
  clocksource/drivers/vf-pit: Replace raw_readl/writel to reald/writel
  clocksource/drivers/vf-pit: Add COMPILE_TEST option
  clocksource/drivers/vf-pit: Set the scene for multiple timers
  clocksource/drivers/vf-pit: Rework the base address usage
  clocksource/drivers/vf-pit: Pass the cpu number as parameter
  clocksource/drivers/vf-pit: Encapsulate the initialization of the
    cycles_per_jiffy
  clocksource/drivers/vf-pit: Allocate the struct timer at init time
  clocksource/drivers/vf-pit: Convert raw values to BIT macros
  clocksource/drivers/vf-pit: Register the clocksource from the driver
  clocksource/drivers/vf-pit: Encapsulate the macros
  clocksource/drivers/vf-pit: Encapsulate the PTLCVAL macro
  clocksource/drivers/vf-pit: Use the node name for the interrupt and
    timer names
  clocksource/drivers/vf-pit: Encapsulate clocksource enable / disable
  clocksource/drivers/vf-pit: Enable and disable module on error
  clocksource/drivers/vf-pit: Encapsulate set counter function
  clocksource/drivers/vf-pit: Consolidate calls to pit_*_disable/enable
  clocksource/drivers/vf-pit: Unify the function name for irq ack
  clocksource/drivers/vf-pit: Rename the VF PIT to NXP PIT
  dt: bindings: fsl,vf610-pit: Add compatible for s32g2 and s32g3
  clocksource/drivers/nxp-pit: Add NXP Automotive s32g2 / s32g3 support

 .../bindings/timer/fsl,vf610-pit.yaml         |   8 +-
 drivers/clocksource/Kconfig                   |   9 +-
 drivers/clocksource/Makefile                  |   2 +-
 drivers/clocksource/timer-nxp-pit.c           | 371 ++++++++++++++++++
 drivers/clocksource/timer-vf-pit.c            | 194 ---------
 5 files changed, 384 insertions(+), 200 deletions(-)
 create mode 100644 drivers/clocksource/timer-nxp-pit.c
 delete mode 100644 drivers/clocksource/timer-vf-pit.c

-- 
2.43.0

[PATCH v2 01/20] clocksource/drivers/vf-pit: Replace raw_readl/writel to reald/writel

[Daniel Lezcano]

The driver uses the raw_readl() and raw_writel() functions. Those are
not for MMIO devices. Replace them with readl() and writel()

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Arnd Bergmann <arnd@arndb.de>
---
 drivers/clocksource/timer-vf-pit.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/clocksource/timer-vf-pit.c b/drivers/clocksource/timer-vf-pit.c
index 911c92146eca..8041a8f62d1f 100644
--- a/drivers/clocksource/timer-vf-pit.c
+++ b/drivers/clocksource/timer-vf-pit.c
@@ -35,30 +35,30 @@ static unsigned long cycle_per_jiffy;
 
 static inline void pit_timer_enable(void)
 {
-	__raw_writel(PITTCTRL_TEN | PITTCTRL_TIE, clkevt_base + PITTCTRL);
+	writel(PITTCTRL_TEN | PITTCTRL_TIE, clkevt_base + PITTCTRL);
 }
 
 static inline void pit_timer_disable(void)
 {
-	__raw_writel(0, clkevt_base + PITTCTRL);
+	writel(0, clkevt_base + PITTCTRL);
 }
 
 static inline void pit_irq_acknowledge(void)
 {
-	__raw_writel(PITTFLG_TIF, clkevt_base + PITTFLG);
+	writel(PITTFLG_TIF, clkevt_base + PITTFLG);
 }
 
 static u64 notrace pit_read_sched_clock(void)
 {
-	return ~__raw_readl(clksrc_base + PITCVAL);
+	return ~readl(clksrc_base + PITCVAL);
 }
 
 static int __init pit_clocksource_init(unsigned long rate)
 {
 	/* set the max load value and start the clock source counter */
-	__raw_writel(0, clksrc_base + PITTCTRL);
-	__raw_writel(~0UL, clksrc_base + PITLDVAL);
-	__raw_writel(PITTCTRL_TEN, clksrc_base + PITTCTRL);
+	writel(0, clksrc_base + PITTCTRL);
+	writel(~0UL, clksrc_base + PITLDVAL);
+	writel(PITTCTRL_TEN, clksrc_base + PITTCTRL);
 
 	sched_clock_register(pit_read_sched_clock, 32, rate);
 	return clocksource_mmio_init(clksrc_base + PITCVAL, "vf-pit", rate,
@@ -76,7 +76,7 @@ static int pit_set_next_event(unsigned long delta,
 	 * hardware requirement.
 	 */
 	pit_timer_disable();
-	__raw_writel(delta - 1, clkevt_base + PITLDVAL);
+	writel(delta - 1, clkevt_base + PITLDVAL);
 	pit_timer_enable();
 
 	return 0;
@@ -125,8 +125,8 @@ static struct clock_event_device clockevent_pit = {
 
 static int __init pit_clockevent_init(unsigned long rate, int irq)
 {
-	__raw_writel(0, clkevt_base + PITTCTRL);
-	__raw_writel(PITTFLG_TIF, clkevt_base + PITTFLG);
+	writel(0, clkevt_base + PITTCTRL);
+	writel(PITTFLG_TIF, clkevt_base + PITTFLG);
 
 	BUG_ON(request_irq(irq, pit_timer_interrupt, IRQF_TIMER | IRQF_IRQPOLL,
 			   "VF pit timer", &clockevent_pit));
@@ -183,7 +183,7 @@ static int __init pit_timer_init(struct device_node *np)
 	cycle_per_jiffy = clk_rate / (HZ);
 
 	/* enable the pit module */
-	__raw_writel(~PITMCR_MDIS, timer_base + PITMCR);
+	writel(~PITMCR_MDIS, timer_base + PITMCR);
 
 	ret = pit_clocksource_init(clk_rate);
 	if (ret)
-- 
2.43.0

[PATCH v2 02/20] clocksource/drivers/vf-pit: Add COMPILE_TEST option

[Daniel Lezcano]

The VF PIT driver is a silent koption. In order to allow a better
compilation test coverage, let's add the COMPILE_TEST option so it can
be selected on other platforms than the Vybrid Family.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 645f517a1ac2..6f7d371904df 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -475,7 +475,7 @@ config FSL_FTM_TIMER
 	  Support for Freescale FlexTimer Module (FTM) timer.
 
 config VF_PIT_TIMER
-	bool
+	bool "Vybrid Family Programmable timer" if COMPILE_TEST
 	select CLKSRC_MMIO
 	help
 	  Support for Periodic Interrupt Timer on Freescale Vybrid Family SoCs.
-- 
2.43.0

[PATCH v2 03/20] clocksource/drivers/vf-pit: Set the scene for multiple timers

[Daniel Lezcano]

The driver is implemented as using a single timer and a single
clocksource. In order to take advantage of the multiple timers
supported in the PIT hardware and introduce different setup for a new
platform, let's encapsulate the data into a structure and pass this
structure around in the function parameter. The structure will be a
per timer instansiation in the next changes.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/timer-vf-pit.c | 121 +++++++++++++++++------------
 1 file changed, 72 insertions(+), 49 deletions(-)

diff --git a/drivers/clocksource/timer-vf-pit.c b/drivers/clocksource/timer-vf-pit.c
index 8041a8f62d1f..e4a8b32fff75 100644
--- a/drivers/clocksource/timer-vf-pit.c
+++ b/drivers/clocksource/timer-vf-pit.c
@@ -15,7 +15,7 @@
  */
 #define PITMCR		0x00
 #define PIT0_OFFSET	0x100
-#define PITn_OFFSET(n)	(PIT0_OFFSET + 0x10 * (n))
+#define PIT_CH(n)       (PIT0_OFFSET + 0x10 * (n))
 #define PITLDVAL	0x00
 #define PITCVAL		0x04
 #define PITTCTRL	0x08
@@ -29,23 +29,36 @@
 
 #define PITTFLG_TIF	0x1
 
+struct pit_timer {
+	void __iomem *clksrc_base;
+	void __iomem *clkevt_base;
+	unsigned long cycle_per_jiffy;
+	struct clock_event_device ced;
+	struct clocksource cs;
+};
+
+static struct pit_timer pit_timer;
+
 static void __iomem *clksrc_base;
-static void __iomem *clkevt_base;
-static unsigned long cycle_per_jiffy;
 
-static inline void pit_timer_enable(void)
+static inline struct pit_timer *ced_to_pit(struct clock_event_device *ced)
 {
-	writel(PITTCTRL_TEN | PITTCTRL_TIE, clkevt_base + PITTCTRL);
+	return container_of(ced, struct pit_timer, ced);
 }
 
-static inline void pit_timer_disable(void)
+static inline void pit_timer_enable(struct pit_timer *pit)
 {
-	writel(0, clkevt_base + PITTCTRL);
+	writel(PITTCTRL_TEN | PITTCTRL_TIE, pit->clkevt_base + PITTCTRL);
 }
 
-static inline void pit_irq_acknowledge(void)
+static inline void pit_timer_disable(struct pit_timer *pit)
 {
-	writel(PITTFLG_TIF, clkevt_base + PITTFLG);
+	writel(0, pit->clkevt_base + PITTCTRL);
+}
+
+static inline void pit_irq_acknowledge(struct pit_timer *pit)
+{
+	writel(PITTFLG_TIF, pit->clkevt_base + PITTFLG);
 }
 
 static u64 notrace pit_read_sched_clock(void)
@@ -53,21 +66,24 @@ static u64 notrace pit_read_sched_clock(void)
 	return ~readl(clksrc_base + PITCVAL);
 }
 
-static int __init pit_clocksource_init(unsigned long rate)
+static int __init pit_clocksource_init(struct pit_timer *pit, unsigned long rate)
 {
 	/* set the max load value and start the clock source counter */
-	writel(0, clksrc_base + PITTCTRL);
-	writel(~0UL, clksrc_base + PITLDVAL);
-	writel(PITTCTRL_TEN, clksrc_base + PITTCTRL);
+	writel(0, pit->clksrc_base + PITTCTRL);
+	writel(~0, pit->clksrc_base + PITLDVAL);
+	writel(PITTCTRL_TEN, pit->clksrc_base + PITTCTRL);
+
+	clksrc_base = pit->clksrc_base;
 
 	sched_clock_register(pit_read_sched_clock, 32, rate);
-	return clocksource_mmio_init(clksrc_base + PITCVAL, "vf-pit", rate,
+	return clocksource_mmio_init(pit->clksrc_base + PITCVAL, "vf-pit", rate,
 			300, 32, clocksource_mmio_readl_down);
 }
 
-static int pit_set_next_event(unsigned long delta,
-				struct clock_event_device *unused)
+static int pit_set_next_event(unsigned long delta, struct clock_event_device *ced)
 {
+	struct pit_timer *pit = ced_to_pit(ced);
+
 	/*
 	 * set a new value to PITLDVAL register will not restart the timer,
 	 * to abort the current cycle and start a timer period with the new
@@ -75,30 +91,37 @@ static int pit_set_next_event(unsigned long delta,
 	 * and the PITLAVAL should be set to delta minus one according to pit
 	 * hardware requirement.
 	 */
-	pit_timer_disable();
-	writel(delta - 1, clkevt_base + PITLDVAL);
-	pit_timer_enable();
+	pit_timer_disable(pit);
+	writel(delta - 1, pit->clkevt_base + PITLDVAL);
+	pit_timer_enable(pit);
 
 	return 0;
 }
 
-static int pit_shutdown(struct clock_event_device *evt)
+static int pit_shutdown(struct clock_event_device *ced)
 {
-	pit_timer_disable();
+	struct pit_timer *pit = ced_to_pit(ced);
+
+	pit_timer_disable(pit);
+
 	return 0;
 }
 
-static int pit_set_periodic(struct clock_event_device *evt)
+static int pit_set_periodic(struct clock_event_device *ced)
 {
-	pit_set_next_event(cycle_per_jiffy, evt);
+	struct pit_timer *pit = ced_to_pit(ced);
+
+	pit_set_next_event(pit->cycle_per_jiffy, ced);
+
 	return 0;
 }
 
 static irqreturn_t pit_timer_interrupt(int irq, void *dev_id)
 {
-	struct clock_event_device *evt = dev_id;
+	struct clock_event_device *ced = dev_id;
+	struct pit_timer *pit = ced_to_pit(ced);
 
-	pit_irq_acknowledge();
+	pit_irq_acknowledge(pit);
 
 	/*
 	 * pit hardware doesn't support oneshot, it will generate an interrupt
@@ -106,33 +129,33 @@ static irqreturn_t pit_timer_interrupt(int irq, void *dev_id)
 	 * and start the counter again. So software need to disable the timer
 	 * to stop the counter loop in ONESHOT mode.
 	 */
-	if (likely(clockevent_state_oneshot(evt)))
-		pit_timer_disable();
+	if (likely(clockevent_state_oneshot(ced)))
+		pit_timer_disable(pit);
 
-	evt->event_handler(evt);
+	ced->event_handler(ced);
 
 	return IRQ_HANDLED;
 }
 
-static struct clock_event_device clockevent_pit = {
-	.name		= "VF pit timer",
-	.features	= CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
-	.set_state_shutdown = pit_shutdown,
-	.set_state_periodic = pit_set_periodic,
-	.set_next_event	= pit_set_next_event,
-	.rating		= 300,
-};
-
-static int __init pit_clockevent_init(unsigned long rate, int irq)
+static int __init pit_clockevent_init(struct pit_timer *pit, unsigned long rate, int irq)
 {
-	writel(0, clkevt_base + PITTCTRL);
-	writel(PITTFLG_TIF, clkevt_base + PITTFLG);
+	writel(0, pit->clkevt_base + PITTCTRL);
+
+	writel(PITTFLG_TIF, pit->clkevt_base + PITTFLG);
 
 	BUG_ON(request_irq(irq, pit_timer_interrupt, IRQF_TIMER | IRQF_IRQPOLL,
-			   "VF pit timer", &clockevent_pit));
+			   "VF pit timer", &pit->ced));
+
+	pit->ced.cpumask = cpumask_of(0);
+	pit->ced.irq = irq;
+
+	pit->ced.name = "VF pit timer";
+	pit->ced.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
+	pit->ced.set_state_shutdown = pit_shutdown;
+	pit->ced.set_state_periodic = pit_set_periodic;
+	pit->ced.set_next_event	= pit_set_next_event;
+	pit->ced.rating	= 300;
 
-	clockevent_pit.cpumask = cpumask_of(0);
-	clockevent_pit.irq = irq;
 	/*
 	 * The value for the LDVAL register trigger is calculated as:
 	 * LDVAL trigger = (period / clock period) - 1
@@ -141,7 +164,7 @@ static int __init pit_clockevent_init(unsigned long rate, int irq)
 	 * LDVAL trigger value is 1. And then the min_delta is
 	 * minimal LDVAL trigger value + 1, and the max_delta is full 32-bit.
 	 */
-	clockevents_config_and_register(&clockevent_pit, rate, 2, 0xffffffff);
+	clockevents_config_and_register(&pit->ced, rate, 2, 0xffffffff);
 
 	return 0;
 }
@@ -164,8 +187,8 @@ static int __init pit_timer_init(struct device_node *np)
 	 * so choose PIT2 as clocksource, PIT3 as clockevent device,
 	 * and leave PIT0 and PIT1 unused for anyone else who needs them.
 	 */
-	clksrc_base = timer_base + PITn_OFFSET(2);
-	clkevt_base = timer_base + PITn_OFFSET(3);
+	pit_timer.clksrc_base = timer_base + PIT_CH(2);
+	pit_timer.clkevt_base = timer_base + PIT_CH(3);
 
 	irq = irq_of_parse_and_map(np, 0);
 	if (irq <= 0)
@@ -180,15 +203,15 @@ static int __init pit_timer_init(struct device_node *np)
 		return ret;
 
 	clk_rate = clk_get_rate(pit_clk);
-	cycle_per_jiffy = clk_rate / (HZ);
+	pit_timer.cycle_per_jiffy = clk_rate / (HZ);
 
 	/* enable the pit module */
 	writel(~PITMCR_MDIS, timer_base + PITMCR);
 
-	ret = pit_clocksource_init(clk_rate);
+	ret = pit_clocksource_init(&pit_timer, clk_rate);
 	if (ret)
 		return ret;
 
-	return pit_clockevent_init(clk_rate, irq);
+	return pit_clockevent_init(&pit_timer, clk_rate, irq);
 }
 TIMER_OF_DECLARE(vf610, "fsl,vf610-pit", pit_timer_init);
-- 
2.43.0

[PATCH v2 04/20] clocksource/drivers/vf-pit: Rework the base address usage

[Daniel Lezcano]

This change passes the base address to the clockevent and clocksource
initialization functions in order to use different base address in the
next changes.

No functional changes intended.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/timer-vf-pit.c | 35 +++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/drivers/clocksource/timer-vf-pit.c b/drivers/clocksource/timer-vf-pit.c
index e4a8b32fff75..6a5f940ad0bc 100644
--- a/drivers/clocksource/timer-vf-pit.c
+++ b/drivers/clocksource/timer-vf-pit.c
@@ -66,8 +66,16 @@ static u64 notrace pit_read_sched_clock(void)
 	return ~readl(clksrc_base + PITCVAL);
 }
 
-static int __init pit_clocksource_init(struct pit_timer *pit, unsigned long rate)
+static int __init pit_clocksource_init(struct pit_timer *pit, void __iomem *base,
+				       unsigned long rate)
 {
+	/*
+	 * The channels 0 and 1 can be chained to build a 64-bit
+	 * timer. Let's use the channel 2 as a clocksource and leave
+	 * the channels 0 and 1 unused for anyone else who needs them
+	 */
+	pit->clksrc_base = base + PIT_CH(2);
+
 	/* set the max load value and start the clock source counter */
 	writel(0, pit->clksrc_base + PITTCTRL);
 	writel(~0, pit->clksrc_base + PITLDVAL);
@@ -76,8 +84,9 @@ static int __init pit_clocksource_init(struct pit_timer *pit, unsigned long rate
 	clksrc_base = pit->clksrc_base;
 
 	sched_clock_register(pit_read_sched_clock, 32, rate);
+
 	return clocksource_mmio_init(pit->clksrc_base + PITCVAL, "vf-pit", rate,
-			300, 32, clocksource_mmio_readl_down);
+				     300, 32, clocksource_mmio_readl_down);
 }
 
 static int pit_set_next_event(unsigned long delta, struct clock_event_device *ced)
@@ -137,8 +146,16 @@ static irqreturn_t pit_timer_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static int __init pit_clockevent_init(struct pit_timer *pit, unsigned long rate, int irq)
+static int __init pit_clockevent_init(struct pit_timer *pit, void __iomem *base,
+				      unsigned long rate, int irq)
 {
+	/*
+	 * The channels 0 and 1 can be chained to build a 64-bit
+	 * timer. Let's use the channel 3 as a clockevent and leave
+	 * the channels 0 and 1 unused for anyone else who needs them
+	 */
+	pit->clkevt_base = base + PIT_CH(3);
+
 	writel(0, pit->clkevt_base + PITTCTRL);
 
 	writel(PITTFLG_TIF, pit->clkevt_base + PITTFLG);
@@ -182,14 +199,6 @@ static int __init pit_timer_init(struct device_node *np)
 		return -ENXIO;
 	}
 
-	/*
-	 * PIT0 and PIT1 can be chained to build a 64-bit timer,
-	 * so choose PIT2 as clocksource, PIT3 as clockevent device,
-	 * and leave PIT0 and PIT1 unused for anyone else who needs them.
-	 */
-	pit_timer.clksrc_base = timer_base + PIT_CH(2);
-	pit_timer.clkevt_base = timer_base + PIT_CH(3);
-
 	irq = irq_of_parse_and_map(np, 0);
 	if (irq <= 0)
 		return -EINVAL;
@@ -208,10 +217,10 @@ static int __init pit_timer_init(struct device_node *np)
 	/* enable the pit module */
 	writel(~PITMCR_MDIS, timer_base + PITMCR);
 
-	ret = pit_clocksource_init(&pit_timer, clk_rate);
+	ret = pit_clocksource_init(&pit_timer, timer_base, clk_rate);
 	if (ret)
 		return ret;
 
-	return pit_clockevent_init(&pit_timer, clk_rate, irq);
+	return pit_clockevent_init(&pit_timer, timer_base, clk_rate, irq);
 }
 TIMER_OF_DECLARE(vf610, "fsl,vf610-pit", pit_timer_init);
-- 
2.43.0

[PATCH v2 05/20] clocksource/drivers/vf-pit: Pass the cpu number as parameter

[Daniel Lezcano]

In order to initialize the timer with a cpumask tied to a cpu, let's
pass it as a parameter instead of hardwiring it in the init function.

No functional changes intended.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/timer-vf-pit.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/timer-vf-pit.c b/drivers/clocksource/timer-vf-pit.c
index 6a5f940ad0bc..9f3b72be987a 100644
--- a/drivers/clocksource/timer-vf-pit.c
+++ b/drivers/clocksource/timer-vf-pit.c
@@ -147,7 +147,7 @@ static irqreturn_t pit_timer_interrupt(int irq, void *dev_id)
 }
 
 static int __init pit_clockevent_init(struct pit_timer *pit, void __iomem *base,
-				      unsigned long rate, int irq)
+				      unsigned long rate, int irq, unsigned int cpu)
 {
 	/*
 	 * The channels 0 and 1 can be chained to build a 64-bit
@@ -163,7 +163,7 @@ static int __init pit_clockevent_init(struct pit_timer *pit, void __iomem *base,
 	BUG_ON(request_irq(irq, pit_timer_interrupt, IRQF_TIMER | IRQF_IRQPOLL,
 			   "VF pit timer", &pit->ced));
 
-	pit->ced.cpumask = cpumask_of(0);
+	pit->ced.cpumask = cpumask_of(cpu);
 	pit->ced.irq = irq;
 
 	pit->ced.name = "VF pit timer";
@@ -221,6 +221,6 @@ static int __init pit_timer_init(struct device_node *np)
 	if (ret)
 		return ret;
 
-	return pit_clockevent_init(&pit_timer, timer_base, clk_rate, irq);
+	return pit_clockevent_init(&pit_timer, timer_base, clk_rate, irq, 0);
 }
 TIMER_OF_DECLARE(vf610, "fsl,vf610-pit", pit_timer_init);
-- 
2.43.0

[PATCH v2 06/20] clocksource/drivers/vf-pit: Encapsulate the initialization of the cycles_per_jiffy

[Daniel Lezcano]

Move the cycles_per_jiffy initialization to the same place where the
other pit timer fields are initialized.

No functional changes intended.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/timer-vf-pit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-vf-pit.c b/drivers/clocksource/timer-vf-pit.c
index 9f3b72be987a..a11e6a63c79f 100644
--- a/drivers/clocksource/timer-vf-pit.c
+++ b/drivers/clocksource/timer-vf-pit.c
@@ -155,6 +155,7 @@ static int __init pit_clockevent_init(struct pit_timer *pit, void __iomem *base,
 	 * the channels 0 and 1 unused for anyone else who needs them
 	 */
 	pit->clkevt_base = base + PIT_CH(3);
+	pit->cycle_per_jiffy = rate / (HZ);
 
 	writel(0, pit->clkevt_base + PITTCTRL);
 
@@ -212,7 +213,6 @@ static int __init pit_timer_init(struct device_node *np)
 		return ret;
 
 	clk_rate = clk_get_rate(pit_clk);
-	pit_timer.cycle_per_jiffy = clk_rate / (HZ);
 
 	/* enable the pit module */
 	writel(~PITMCR_MDIS, timer_base + PITMCR);
-- 
2.43.0

[PATCH v2 07/20] clocksource/drivers/vf-pit: Allocate the struct timer at init time

[Daniel Lezcano]

Instead of having a static global structure for a timer, let's
allocate it dynamically so we can create multiple instances in the
future to support multiple timers. At the same time, add the
rollbacking code in case of error.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/timer-vf-pit.c | 48 +++++++++++++++++++++++-------
 1 file changed, 37 insertions(+), 11 deletions(-)

diff --git a/drivers/clocksource/timer-vf-pit.c b/drivers/clocksource/timer-vf-pit.c
index a11e6a63c79f..d408dcddb4e9 100644
--- a/drivers/clocksource/timer-vf-pit.c
+++ b/drivers/clocksource/timer-vf-pit.c
@@ -37,8 +37,6 @@ struct pit_timer {
 	struct clocksource cs;
 };
 
-static struct pit_timer pit_timer;
-
 static void __iomem *clksrc_base;
 
 static inline struct pit_timer *ced_to_pit(struct clock_event_device *ced)
@@ -189,38 +187,66 @@ static int __init pit_clockevent_init(struct pit_timer *pit, void __iomem *base,
 
 static int __init pit_timer_init(struct device_node *np)
 {
+	struct pit_timer *pit;
 	struct clk *pit_clk;
 	void __iomem *timer_base;
 	unsigned long clk_rate;
 	int irq, ret;
 
+	pit = kzalloc(sizeof(*pit), GFP_KERNEL);
+	if (!pit)
+		return -ENOMEM;
+
+	ret = -ENXIO;
 	timer_base = of_iomap(np, 0);
 	if (!timer_base) {
 		pr_err("Failed to iomap\n");
-		return -ENXIO;
+		goto out_kfree;
 	}
 
+	ret = -EINVAL;
 	irq = irq_of_parse_and_map(np, 0);
-	if (irq <= 0)
-		return -EINVAL;
+	if (irq <= 0) {
+		pr_err("Failed to irq_of_parse_and_map\n");
+		goto out_iounmap;
+	}
 
 	pit_clk = of_clk_get(np, 0);
-	if (IS_ERR(pit_clk))
-		return PTR_ERR(pit_clk);
+	if (IS_ERR(pit_clk)) {
+		ret = PTR_ERR(pit_clk);
+		goto out_iounmap;
+	}
 
 	ret = clk_prepare_enable(pit_clk);
 	if (ret)
-		return ret;
+		goto out_clk_put;
 
 	clk_rate = clk_get_rate(pit_clk);
 
 	/* enable the pit module */
 	writel(~PITMCR_MDIS, timer_base + PITMCR);
 
-	ret = pit_clocksource_init(&pit_timer, timer_base, clk_rate);
+	ret = pit_clocksource_init(pit, timer_base, clk_rate);
 	if (ret)
-		return ret;
+		goto out_disable_unprepare;
+
+	ret = pit_clockevent_init(pit, timer_base, clk_rate, irq, 0);
+	if (ret)
+		goto out_pit_clocksource_unregister;
+
+	return 0;
 
-	return pit_clockevent_init(&pit_timer, timer_base, clk_rate, irq, 0);
+out_pit_clocksource_unregister:
+	clocksource_unregister(&pit->cs);
+out_disable_unprepare:
+	clk_disable_unprepare(pit_clk);
+out_clk_put:
+	clk_put(pit_clk);
+out_iounmap:
+	iounmap(timer_base);
+out_kfree:
+	kfree(pit);
+
+	return ret;
 }
 TIMER_OF_DECLARE(vf610, "fsl,vf610-pit", pit_timer_init);
-- 
2.43.0

[PATCH v2 08/20] clocksource/drivers/vf-pit: Convert raw values to BIT macros

[Daniel Lezcano]

Use the BIT macros instead of the shifting syntax.

No functional change intended.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/timer-vf-pit.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/clocksource/timer-vf-pit.c b/drivers/clocksource/timer-vf-pit.c
index d408dcddb4e9..d1aec6aaeb02 100644
--- a/drivers/clocksource/timer-vf-pit.c
+++ b/drivers/clocksource/timer-vf-pit.c
@@ -21,11 +21,11 @@
 #define PITTCTRL	0x08
 #define PITTFLG		0x0c
 
-#define PITMCR_MDIS	(0x1 << 1)
+#define PITMCR_MDIS	BIT(1)
 
-#define PITTCTRL_TEN	(0x1 << 0)
-#define PITTCTRL_TIE	(0x1 << 1)
-#define PITCTRL_CHN	(0x1 << 2)
+#define PITTCTRL_TEN	BIT(0)
+#define PITTCTRL_TIE	BIT(1)
+#define PITCTRL_CHN	BIT(2)
 
 #define PITTFLG_TIF	0x1
 
-- 
2.43.0

[PATCH v2 09/20] clocksource/drivers/vf-pit: Register the clocksource from the driver

[Daniel Lezcano]

The function clocksource_mmio_init() uses its own global static
clocksource variable making no possible to have several instances of a
clocksource using this function. In order to support that, let's add
the clocksource structure to the pit structure and use the
clocksource_register_hz() function instead.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/timer-vf-pit.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/timer-vf-pit.c b/drivers/clocksource/timer-vf-pit.c
index d1aec6aaeb02..6a4043801eeb 100644
--- a/drivers/clocksource/timer-vf-pit.c
+++ b/drivers/clocksource/timer-vf-pit.c
@@ -44,6 +44,11 @@ static inline struct pit_timer *ced_to_pit(struct clock_event_device *ced)
 	return container_of(ced, struct pit_timer, ced);
 }
 
+static inline struct pit_timer *cs_to_pit(struct clocksource *cs)
+{
+	return container_of(cs, struct pit_timer, cs);
+}
+
 static inline void pit_timer_enable(struct pit_timer *pit)
 {
 	writel(PITTCTRL_TEN | PITTCTRL_TIE, pit->clkevt_base + PITTCTRL);
@@ -64,6 +69,13 @@ static u64 notrace pit_read_sched_clock(void)
 	return ~readl(clksrc_base + PITCVAL);
 }
 
+static u64 pit_timer_clocksource_read(struct clocksource *cs)
+{
+	struct pit_timer *pit = cs_to_pit(cs);
+
+	return (u64)~readl(pit->clksrc_base + PITCVAL);
+}
+
 static int __init pit_clocksource_init(struct pit_timer *pit, void __iomem *base,
 				       unsigned long rate)
 {
@@ -73,6 +85,11 @@ static int __init pit_clocksource_init(struct pit_timer *pit, void __iomem *base
 	 * the channels 0 and 1 unused for anyone else who needs them
 	 */
 	pit->clksrc_base = base + PIT_CH(2);
+	pit->cs.name = "vf-pit";
+	pit->cs.rating = 300;
+	pit->cs.read = pit_timer_clocksource_read;
+	pit->cs.mask = CLOCKSOURCE_MASK(32);
+	pit->cs.flags = CLOCK_SOURCE_IS_CONTINUOUS;
 
 	/* set the max load value and start the clock source counter */
 	writel(0, pit->clksrc_base + PITTCTRL);
@@ -83,8 +100,7 @@ static int __init pit_clocksource_init(struct pit_timer *pit, void __iomem *base
 
 	sched_clock_register(pit_read_sched_clock, 32, rate);
 
-	return clocksource_mmio_init(pit->clksrc_base + PITCVAL, "vf-pit", rate,
-				     300, 32, clocksource_mmio_readl_down);
+	return clocksource_register_hz(&pit->cs, rate);
 }
 
 static int pit_set_next_event(unsigned long delta, struct clock_event_device *ced)
-- 
2.43.0

[PATCH v2 10/20] clocksource/drivers/vf-pit: Encapsulate the macros

[Daniel Lezcano]

Pass the base address to the macro, so we can use the macro with
multiple instances of the timer because we deal with different base
address. At the same time, change writes to the register to the
existing corresponding functions.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/timer-vf-pit.c | 35 ++++++++++++++++--------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/clocksource/timer-vf-pit.c b/drivers/clocksource/timer-vf-pit.c
index 6a4043801eeb..8f0e26c0512d 100644
--- a/drivers/clocksource/timer-vf-pit.c
+++ b/drivers/clocksource/timer-vf-pit.c
@@ -16,18 +16,21 @@
 #define PITMCR		0x00
 #define PIT0_OFFSET	0x100
 #define PIT_CH(n)       (PIT0_OFFSET + 0x10 * (n))
-#define PITLDVAL	0x00
+
 #define PITCVAL		0x04
-#define PITTCTRL	0x08
-#define PITTFLG		0x0c
 
 #define PITMCR_MDIS	BIT(1)
 
-#define PITTCTRL_TEN	BIT(0)
-#define PITTCTRL_TIE	BIT(1)
-#define PITCTRL_CHN	BIT(2)
+#define PITLDVAL(__base)	(__base)
+#define PITTCTRL(__base)	((__base) + 0x08)
+
+
+#define PITTCTRL_TEN			BIT(0)
+#define PITTCTRL_TIE			BIT(1)
+
+#define PITTFLG(__base)	((__base) + 0x0c)
 
-#define PITTFLG_TIF	0x1
+#define PITTFLG_TIF			BIT(0)
 
 struct pit_timer {
 	void __iomem *clksrc_base;
@@ -51,17 +54,17 @@ static inline struct pit_timer *cs_to_pit(struct clocksource *cs)
 
 static inline void pit_timer_enable(struct pit_timer *pit)
 {
-	writel(PITTCTRL_TEN | PITTCTRL_TIE, pit->clkevt_base + PITTCTRL);
+	writel(PITTCTRL_TEN | PITTCTRL_TIE, PITTCTRL(pit->clkevt_base));
 }
 
 static inline void pit_timer_disable(struct pit_timer *pit)
 {
-	writel(0, pit->clkevt_base + PITTCTRL);
+	writel(0, PITTCTRL(pit->clkevt_base));
 }
 
 static inline void pit_irq_acknowledge(struct pit_timer *pit)
 {
-	writel(PITTFLG_TIF, pit->clkevt_base + PITTFLG);
+	writel(PITTFLG_TIF, PITTFLG(pit->clkevt_base));
 }
 
 static u64 notrace pit_read_sched_clock(void)
@@ -92,9 +95,9 @@ static int __init pit_clocksource_init(struct pit_timer *pit, void __iomem *base
 	pit->cs.flags = CLOCK_SOURCE_IS_CONTINUOUS;
 
 	/* set the max load value and start the clock source counter */
-	writel(0, pit->clksrc_base + PITTCTRL);
-	writel(~0, pit->clksrc_base + PITLDVAL);
-	writel(PITTCTRL_TEN, pit->clksrc_base + PITTCTRL);
+	pit_timer_disable(pit);
+	writel(~0, PITLDVAL(pit->clksrc_base));
+	writel(PITTCTRL_TEN, PITTCTRL(pit->clksrc_base));
 
 	clksrc_base = pit->clksrc_base;
 
@@ -115,7 +118,7 @@ static int pit_set_next_event(unsigned long delta, struct clock_event_device *ce
 	 * hardware requirement.
 	 */
 	pit_timer_disable(pit);
-	writel(delta - 1, pit->clkevt_base + PITLDVAL);
+	writel(delta - 1, PITLDVAL(pit->clkevt_base));
 	pit_timer_enable(pit);
 
 	return 0;
@@ -171,9 +174,9 @@ static int __init pit_clockevent_init(struct pit_timer *pit, void __iomem *base,
 	pit->clkevt_base = base + PIT_CH(3);
 	pit->cycle_per_jiffy = rate / (HZ);
 
-	writel(0, pit->clkevt_base + PITTCTRL);
+	pit_timer_disable(pit);
 
-	writel(PITTFLG_TIF, pit->clkevt_base + PITTFLG);
+	pit_irq_acknowledge(pit);
 
 	BUG_ON(request_irq(irq, pit_timer_interrupt, IRQF_TIMER | IRQF_IRQPOLL,
 			   "VF pit timer", &pit->ced));
-- 
2.43.0

[PATCH v2 11/20] clocksource/drivers/vf-pit: Encapsulate the PTLCVAL macro

[Daniel Lezcano]

Pass the channel and the base address to the PITLCVAL macro so it is
possible to use multiple instances of the timer with the macro.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/timer-vf-pit.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/clocksource/timer-vf-pit.c b/drivers/clocksource/timer-vf-pit.c
index 8f0e26c0512d..4f1b85ba5de3 100644
--- a/drivers/clocksource/timer-vf-pit.c
+++ b/drivers/clocksource/timer-vf-pit.c
@@ -17,13 +17,13 @@
 #define PIT0_OFFSET	0x100
 #define PIT_CH(n)       (PIT0_OFFSET + 0x10 * (n))
 
-#define PITCVAL		0x04
-
 #define PITMCR_MDIS	BIT(1)
 
 #define PITLDVAL(__base)	(__base)
 #define PITTCTRL(__base)	((__base) + 0x08)
 
+#define PITCVAL_OFFSET	0x04
+#define PITCVAL(__base)	((__base) + 0x04)
 
 #define PITTCTRL_TEN			BIT(0)
 #define PITTCTRL_TIE			BIT(1)
@@ -40,7 +40,7 @@ struct pit_timer {
 	struct clocksource cs;
 };
 
-static void __iomem *clksrc_base;
+static void __iomem *sched_clock_base;
 
 static inline struct pit_timer *ced_to_pit(struct clock_event_device *ced)
 {
@@ -69,14 +69,14 @@ static inline void pit_irq_acknowledge(struct pit_timer *pit)
 
 static u64 notrace pit_read_sched_clock(void)
 {
-	return ~readl(clksrc_base + PITCVAL);
+	return ~readl(sched_clock_base);
 }
 
 static u64 pit_timer_clocksource_read(struct clocksource *cs)
 {
 	struct pit_timer *pit = cs_to_pit(cs);
 
-	return (u64)~readl(pit->clksrc_base + PITCVAL);
+	return (u64)~readl(PITCVAL(pit->clksrc_base));
 }
 
 static int __init pit_clocksource_init(struct pit_timer *pit, void __iomem *base,
@@ -99,8 +99,7 @@ static int __init pit_clocksource_init(struct pit_timer *pit, void __iomem *base
 	writel(~0, PITLDVAL(pit->clksrc_base));
 	writel(PITTCTRL_TEN, PITTCTRL(pit->clksrc_base));
 
-	clksrc_base = pit->clksrc_base;
-
+	sched_clock_base = pit->clksrc_base + PITCVAL_OFFSET;
 	sched_clock_register(pit_read_sched_clock, 32, rate);
 
 	return clocksource_register_hz(&pit->cs, rate);
-- 
2.43.0

[PATCH v2 12/20] clocksource/drivers/vf-pit: Use the node name for the interrupt and timer names

[Daniel Lezcano]

In order to differentiate from userspace the pit timer given the
device tree, let's use the node name for the interrupt and the timer
names.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/timer-vf-pit.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/clocksource/timer-vf-pit.c b/drivers/clocksource/timer-vf-pit.c
index 4f1b85ba5de3..2a255b45561d 100644
--- a/drivers/clocksource/timer-vf-pit.c
+++ b/drivers/clocksource/timer-vf-pit.c
@@ -79,8 +79,8 @@ static u64 pit_timer_clocksource_read(struct clocksource *cs)
 	return (u64)~readl(PITCVAL(pit->clksrc_base));
 }
 
-static int __init pit_clocksource_init(struct pit_timer *pit, void __iomem *base,
-				       unsigned long rate)
+static int __init pit_clocksource_init(struct pit_timer *pit, const char *name,
+				       void __iomem *base, unsigned long rate)
 {
 	/*
 	 * The channels 0 and 1 can be chained to build a 64-bit
@@ -88,7 +88,7 @@ static int __init pit_clocksource_init(struct pit_timer *pit, void __iomem *base
 	 * the channels 0 and 1 unused for anyone else who needs them
 	 */
 	pit->clksrc_base = base + PIT_CH(2);
-	pit->cs.name = "vf-pit";
+	pit->cs.name = name;
 	pit->cs.rating = 300;
 	pit->cs.read = pit_timer_clocksource_read;
 	pit->cs.mask = CLOCKSOURCE_MASK(32);
@@ -162,8 +162,9 @@ static irqreturn_t pit_timer_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static int __init pit_clockevent_init(struct pit_timer *pit, void __iomem *base,
-				      unsigned long rate, int irq, unsigned int cpu)
+static int __init pit_clockevent_init(struct pit_timer *pit, const char *name,
+				      void __iomem *base, unsigned long rate,
+				      int irq, unsigned int cpu)
 {
 	/*
 	 * The channels 0 and 1 can be chained to build a 64-bit
@@ -178,12 +179,12 @@ static int __init pit_clockevent_init(struct pit_timer *pit, void __iomem *base,
 	pit_irq_acknowledge(pit);
 
 	BUG_ON(request_irq(irq, pit_timer_interrupt, IRQF_TIMER | IRQF_IRQPOLL,
-			   "VF pit timer", &pit->ced));
+			   name, &pit->ced));
 
 	pit->ced.cpumask = cpumask_of(cpu);
 	pit->ced.irq = irq;
 
-	pit->ced.name = "VF pit timer";
+	pit->ced.name = name;
 	pit->ced.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
 	pit->ced.set_state_shutdown = pit_shutdown;
 	pit->ced.set_state_periodic = pit_set_periodic;
@@ -208,6 +209,7 @@ static int __init pit_timer_init(struct device_node *np)
 	struct pit_timer *pit;
 	struct clk *pit_clk;
 	void __iomem *timer_base;
+	const char *name = of_node_full_name(np);
 	unsigned long clk_rate;
 	int irq, ret;
 
@@ -244,11 +246,11 @@ static int __init pit_timer_init(struct device_node *np)
 	/* enable the pit module */
 	writel(~PITMCR_MDIS, timer_base + PITMCR);
 
-	ret = pit_clocksource_init(pit, timer_base, clk_rate);
+	ret = pit_clocksource_init(pit, name, timer_base, clk_rate);
 	if (ret)
 		goto out_disable_unprepare;
 
-	ret = pit_clockevent_init(pit, timer_base, clk_rate, irq, 0);
+	ret = pit_clockevent_init(pit, name, timer_base, clk_rate, irq, 0);
 	if (ret)
 		goto out_pit_clocksource_unregister;
 
-- 
2.43.0

[PATCH v2 13/20] clocksource/drivers/vf-pit: Encapsulate clocksource enable / disable

[Daniel Lezcano]

For the sake of lisibility, let's encapsulate the writel calls to
enable and disable the timer into a function with a self-explainatory
name.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/timer-vf-pit.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/timer-vf-pit.c b/drivers/clocksource/timer-vf-pit.c
index 2a255b45561d..96377088a048 100644
--- a/drivers/clocksource/timer-vf-pit.c
+++ b/drivers/clocksource/timer-vf-pit.c
@@ -62,6 +62,16 @@ static inline void pit_timer_disable(struct pit_timer *pit)
 	writel(0, PITTCTRL(pit->clkevt_base));
 }
 
+static inline void pit_clocksource_enable(struct pit_timer *pit)
+{
+	writel(PITTCTRL_TEN, PITTCTRL(pit->clksrc_base));
+}
+
+static inline void pit_clocksource_disable(struct pit_timer *pit)
+{
+	pit_timer_disable(pit);
+}
+
 static inline void pit_irq_acknowledge(struct pit_timer *pit)
 {
 	writel(PITTFLG_TIF, PITTFLG(pit->clkevt_base));
@@ -95,9 +105,9 @@ static int __init pit_clocksource_init(struct pit_timer *pit, const char *name,
 	pit->cs.flags = CLOCK_SOURCE_IS_CONTINUOUS;
 
 	/* set the max load value and start the clock source counter */
-	pit_timer_disable(pit);
+	pit_clocksource_disable(pit);
 	writel(~0, PITLDVAL(pit->clksrc_base));
-	writel(PITTCTRL_TEN, PITTCTRL(pit->clksrc_base));
+	pit_clocksource_enable(pit);
 
 	sched_clock_base = pit->clksrc_base + PITCVAL_OFFSET;
 	sched_clock_register(pit_read_sched_clock, 32, rate);
-- 
2.43.0

[PATCH v2 14/20] clocksource/drivers/vf-pit: Enable and disable module on error

[Daniel Lezcano]

Encapsulate the calls to writel to enable and disable the PIT module
and make use of them. Add the missing module disablement in case of
error.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/timer-vf-pit.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/clocksource/timer-vf-pit.c b/drivers/clocksource/timer-vf-pit.c
index 96377088a048..609a4d9deb64 100644
--- a/drivers/clocksource/timer-vf-pit.c
+++ b/drivers/clocksource/timer-vf-pit.c
@@ -13,10 +13,12 @@
 /*
  * Each pit takes 0x10 Bytes register space
  */
-#define PITMCR		0x00
 #define PIT0_OFFSET	0x100
 #define PIT_CH(n)       (PIT0_OFFSET + 0x10 * (n))
 
+#define PITMCR(__base)	(__base)
+
+#define PITMCR_FRZ	BIT(0)
 #define PITMCR_MDIS	BIT(1)
 
 #define PITLDVAL(__base)	(__base)
@@ -52,6 +54,16 @@ static inline struct pit_timer *cs_to_pit(struct clocksource *cs)
 	return container_of(cs, struct pit_timer, cs);
 }
 
+static inline void pit_module_enable(void __iomem *base)
+{
+	writel(0, PITMCR(base));
+}
+
+static inline void pit_module_disable(void __iomem *base)
+{
+	writel(PITMCR_MDIS, PITMCR(base));
+}
+
 static inline void pit_timer_enable(struct pit_timer *pit)
 {
 	writel(PITTCTRL_TEN | PITTCTRL_TIE, PITTCTRL(pit->clkevt_base));
@@ -254,11 +266,11 @@ static int __init pit_timer_init(struct device_node *np)
 	clk_rate = clk_get_rate(pit_clk);
 
 	/* enable the pit module */
-	writel(~PITMCR_MDIS, timer_base + PITMCR);
+	pit_module_enable(timer_base);
 
 	ret = pit_clocksource_init(pit, name, timer_base, clk_rate);
 	if (ret)
-		goto out_disable_unprepare;
+		goto out_pit_module_disable;
 
 	ret = pit_clockevent_init(pit, name, timer_base, clk_rate, irq, 0);
 	if (ret)
@@ -268,7 +280,8 @@ static int __init pit_timer_init(struct device_node *np)
 
 out_pit_clocksource_unregister:
 	clocksource_unregister(&pit->cs);
-out_disable_unprepare:
+out_pit_module_disable:
+	pit_module_disable(timer_base);
 	clk_disable_unprepare(pit_clk);
 out_clk_put:
 	clk_put(pit_clk);
-- 
2.43.0

[PATCH v2 15/20] clocksource/drivers/vf-pit: Encapsulate set counter function

[Daniel Lezcano]

Encapsulate the writel() calls to set the counter into a
self-explainatory function.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/timer-vf-pit.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/timer-vf-pit.c b/drivers/clocksource/timer-vf-pit.c
index 609a4d9deb64..5551b61483f8 100644
--- a/drivers/clocksource/timer-vf-pit.c
+++ b/drivers/clocksource/timer-vf-pit.c
@@ -74,6 +74,11 @@ static inline void pit_timer_disable(struct pit_timer *pit)
 	writel(0, PITTCTRL(pit->clkevt_base));
 }
 
+static inline void pit_timer_set_counter(void __iomem *base, unsigned int cnt)
+{
+	writel(cnt, PITLDVAL(base));
+}
+
 static inline void pit_clocksource_enable(struct pit_timer *pit)
 {
 	writel(PITTCTRL_TEN, PITTCTRL(pit->clksrc_base));
@@ -118,7 +123,7 @@ static int __init pit_clocksource_init(struct pit_timer *pit, const char *name,
 
 	/* set the max load value and start the clock source counter */
 	pit_clocksource_disable(pit);
-	writel(~0, PITLDVAL(pit->clksrc_base));
+	pit_timer_set_counter(pit->clksrc_base, ~0);
 	pit_clocksource_enable(pit);
 
 	sched_clock_base = pit->clksrc_base + PITCVAL_OFFSET;
@@ -139,7 +144,7 @@ static int pit_set_next_event(unsigned long delta, struct clock_event_device *ce
 	 * hardware requirement.
 	 */
 	pit_timer_disable(pit);
-	writel(delta - 1, PITLDVAL(pit->clkevt_base));
+	pit_timer_set_counter(pit->clkevt_base, delta - 1);
 	pit_timer_enable(pit);
 
 	return 0;
-- 
2.43.0

[PATCH v2 16/20] clocksource/drivers/vf-pit: Consolidate calls to pit_*_disable/enable

[Daniel Lezcano]

The difference between the pit_clocksource_enable() and
pit_clocksource_disable() is only setting the TIF flag for the
clockevent. Let's group them and pass the TIF flag parameter to the
function so we save some lines of code. But as the base address is
different regarding if it is a clocksource or a clockevent, we pass
the base address in parameter instead of the struct pit_timer.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/timer-vf-pit.c | 34 ++++++++++++------------------
 1 file changed, 13 insertions(+), 21 deletions(-)

diff --git a/drivers/clocksource/timer-vf-pit.c b/drivers/clocksource/timer-vf-pit.c
index 5551b61483f8..3825159a0ca7 100644
--- a/drivers/clocksource/timer-vf-pit.c
+++ b/drivers/clocksource/timer-vf-pit.c
@@ -64,14 +64,16 @@ static inline void pit_module_disable(void __iomem *base)
 	writel(PITMCR_MDIS, PITMCR(base));
 }
 
-static inline void pit_timer_enable(struct pit_timer *pit)
+static inline void pit_timer_enable(void __iomem *base, bool tie)
 {
-	writel(PITTCTRL_TEN | PITTCTRL_TIE, PITTCTRL(pit->clkevt_base));
+	u32 val = PITTCTRL_TEN | (tie ? PITTCTRL_TIE : 0);
+
+	writel(val, PITTCTRL(base));
 }
 
-static inline void pit_timer_disable(struct pit_timer *pit)
+static inline void pit_timer_disable(void __iomem *base)
 {
-	writel(0, PITTCTRL(pit->clkevt_base));
+	writel(0, PITTCTRL(base));
 }
 
 static inline void pit_timer_set_counter(void __iomem *base, unsigned int cnt)
@@ -79,16 +81,6 @@ static inline void pit_timer_set_counter(void __iomem *base, unsigned int cnt)
 	writel(cnt, PITLDVAL(base));
 }
 
-static inline void pit_clocksource_enable(struct pit_timer *pit)
-{
-	writel(PITTCTRL_TEN, PITTCTRL(pit->clksrc_base));
-}
-
-static inline void pit_clocksource_disable(struct pit_timer *pit)
-{
-	pit_timer_disable(pit);
-}
-
 static inline void pit_irq_acknowledge(struct pit_timer *pit)
 {
 	writel(PITTFLG_TIF, PITTFLG(pit->clkevt_base));
@@ -122,9 +114,9 @@ static int __init pit_clocksource_init(struct pit_timer *pit, const char *name,
 	pit->cs.flags = CLOCK_SOURCE_IS_CONTINUOUS;
 
 	/* set the max load value and start the clock source counter */
-	pit_clocksource_disable(pit);
+	pit_timer_disable(pit->clksrc_base);
 	pit_timer_set_counter(pit->clksrc_base, ~0);
-	pit_clocksource_enable(pit);
+	pit_timer_enable(pit->clksrc_base, 0);
 
 	sched_clock_base = pit->clksrc_base + PITCVAL_OFFSET;
 	sched_clock_register(pit_read_sched_clock, 32, rate);
@@ -143,9 +135,9 @@ static int pit_set_next_event(unsigned long delta, struct clock_event_device *ce
 	 * and the PITLAVAL should be set to delta minus one according to pit
 	 * hardware requirement.
 	 */
-	pit_timer_disable(pit);
+	pit_timer_disable(pit->clkevt_base);
 	pit_timer_set_counter(pit->clkevt_base, delta - 1);
-	pit_timer_enable(pit);
+	pit_timer_enable(pit->clkevt_base, true);
 
 	return 0;
 }
@@ -154,7 +146,7 @@ static int pit_shutdown(struct clock_event_device *ced)
 {
 	struct pit_timer *pit = ced_to_pit(ced);
 
-	pit_timer_disable(pit);
+	pit_timer_disable(pit->clkevt_base);
 
 	return 0;
 }
@@ -182,7 +174,7 @@ static irqreturn_t pit_timer_interrupt(int irq, void *dev_id)
 	 * to stop the counter loop in ONESHOT mode.
 	 */
 	if (likely(clockevent_state_oneshot(ced)))
-		pit_timer_disable(pit);
+		pit_timer_disable(pit->clkevt_base);
 
 	ced->event_handler(ced);
 
@@ -201,7 +193,7 @@ static int __init pit_clockevent_init(struct pit_timer *pit, const char *name,
 	pit->clkevt_base = base + PIT_CH(3);
 	pit->cycle_per_jiffy = rate / (HZ);
 
-	pit_timer_disable(pit);
+	pit_timer_disable(pit->clkevt_base);
 
 	pit_irq_acknowledge(pit);
 
-- 
2.43.0

[PATCH v2 17/20] clocksource/drivers/vf-pit: Unify the function name for irq ack

[Daniel Lezcano]

Most the function are under the form pit_timer_*, let's change the
interrupt acknowledgement function name to have the same format.

No functional changes intended.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/timer-vf-pit.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/timer-vf-pit.c b/drivers/clocksource/timer-vf-pit.c
index 3825159a0ca7..2a0ee4109ead 100644
--- a/drivers/clocksource/timer-vf-pit.c
+++ b/drivers/clocksource/timer-vf-pit.c
@@ -81,7 +81,7 @@ static inline void pit_timer_set_counter(void __iomem *base, unsigned int cnt)
 	writel(cnt, PITLDVAL(base));
 }
 
-static inline void pit_irq_acknowledge(struct pit_timer *pit)
+static inline void pit_timer_irqack(struct pit_timer *pit)
 {
 	writel(PITTFLG_TIF, PITTFLG(pit->clkevt_base));
 }
@@ -165,7 +165,7 @@ static irqreturn_t pit_timer_interrupt(int irq, void *dev_id)
 	struct clock_event_device *ced = dev_id;
 	struct pit_timer *pit = ced_to_pit(ced);
 
-	pit_irq_acknowledge(pit);
+	pit_timer_irqack(pit);
 
 	/*
 	 * pit hardware doesn't support oneshot, it will generate an interrupt
@@ -195,7 +195,7 @@ static int __init pit_clockevent_init(struct pit_timer *pit, const char *name,
 
 	pit_timer_disable(pit->clkevt_base);
 
-	pit_irq_acknowledge(pit);
+	pit_timer_irqack(pit);
 
 	BUG_ON(request_irq(irq, pit_timer_interrupt, IRQF_TIMER | IRQF_IRQPOLL,
 			   name, &pit->ced));
-- 
2.43.0

[PATCH v2 18/20] clocksource/drivers/vf-pit: Rename the VF PIT to NXP PIT

[Daniel Lezcano]

The PIT acronym stands for Periodic Interrupt Timer which is found on
different NXP platforms not only on the Vybrid Family. Change the name
to be more generic for the NXP platforms in general. That will be
consistent with the NXP STM driver naming convention.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/Kconfig                             | 9 ++++++---
 drivers/clocksource/Makefile                            | 2 +-
 drivers/clocksource/{timer-vf-pit.c => timer-nxp-pit.c} | 0
 3 files changed, 7 insertions(+), 4 deletions(-)
 rename drivers/clocksource/{timer-vf-pit.c => timer-nxp-pit.c} (100%)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 6f7d371904df..0fd662f67d29 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -474,11 +474,14 @@ config FSL_FTM_TIMER
 	help
 	  Support for Freescale FlexTimer Module (FTM) timer.
 
-config VF_PIT_TIMER
-	bool "Vybrid Family Programmable timer" if COMPILE_TEST
+config NXP_PIT_TIMER
+	bool "NXP Periodic Interrupt Timer" if COMPILE_TEST
 	select CLKSRC_MMIO
 	help
-	  Support for Periodic Interrupt Timer on Freescale Vybrid Family SoCs.
+	  Support for Periodic Interrupt Timer on Freescale / NXP
+	  SoCs. This periodic timer is found on the Vybrid Family and
+	  the Automotive S32G2/3 platforms. It contains 4 channels
+	  where two can be coupled to form a 64 bits channel.
 
 config SYS_SUPPORTS_SH_CMT
 	bool
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 205bf3b0a8f3..77a0f08eb43b 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -49,7 +49,7 @@ obj-$(CONFIG_CLKSRC_LPC32XX)	+= timer-lpc32xx.o
 obj-$(CONFIG_CLKSRC_MPS2)	+= mps2-timer.o
 obj-$(CONFIG_CLKSRC_SAMSUNG_PWM)	+= samsung_pwm_timer.o
 obj-$(CONFIG_FSL_FTM_TIMER)	+= timer-fsl-ftm.o
-obj-$(CONFIG_VF_PIT_TIMER)	+= timer-vf-pit.o
+obj-$(CONFIG_NXP_PIT_TIMER)	+= timer-nxp-pit.o
 obj-$(CONFIG_CLKSRC_QCOM)	+= timer-qcom.o
 obj-$(CONFIG_MTK_TIMER)		+= timer-mediatek.o
 obj-$(CONFIG_MTK_CPUX_TIMER)	+= timer-mediatek-cpux.o
diff --git a/drivers/clocksource/timer-vf-pit.c b/drivers/clocksource/timer-nxp-pit.c
similarity index 100%
rename from drivers/clocksource/timer-vf-pit.c
rename to drivers/clocksource/timer-nxp-pit.c
-- 
2.43.0

[PATCH v2 19/20] dt: bindings: fsl,vf610-pit: Add compatible for s32g2 and s32g3

[Daniel Lezcano]

The Vybrid Family is a NXP (formerly Freescale) platform having a
Programmable Interrupt Timer (PIT). This timer is an IP found also on
the NXP Automotive platform S32G2 and S32G3.

Add the compatible for those platforms to describe the timer.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 .../devicetree/bindings/timer/fsl,vf610-pit.yaml          | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/timer/fsl,vf610-pit.yaml b/Documentation/devicetree/bindings/timer/fsl,vf610-pit.yaml
index bee2c35bd0e2..2aac63a58bfd 100644
--- a/Documentation/devicetree/bindings/timer/fsl,vf610-pit.yaml
+++ b/Documentation/devicetree/bindings/timer/fsl,vf610-pit.yaml
@@ -15,8 +15,12 @@ description:
 
 properties:
   compatible:
-    enum:
-      - fsl,vf610-pit
+    oneOf:
+      - const: fsl,vf610-pit
+      - const: nxp,s32g2-pit
+      - items:
+          - const: nxp,s32g3-pit
+          - const: nxp,s32g2-pit
 
   reg:
     maxItems: 1
-- 
2.43.0

[PATCH v2 20/20] clocksource/drivers/nxp-pit: Add NXP Automotive s32g2 / s32g3 support

[Daniel Lezcano]

The previous changes put in place the encapsulation of the code in
order to allow multiple instances of the driver.

The S32G platform has two Periodic Interrupt Timer (PIT). The IP is
exactly the same as the VF platform.

Each PIT has four channels which are 32 bits wide and counting
down. The two first channels can be chained to implement a 64 bits
counter. The channel usage is kept unchanged with the original driver,
channel 2 is used as a clocksource, channel 3 is used as a
clockevent. Other channels are unused.

In order to support the S32G platform which has two PIT, we initialize
the timer and bind it to a CPU. The S32G platforms can have 2, 4 or 8
CPUs and this kind of configuration can appear unusual as we may endup
with two PIT used as a clockevent for the two first CPUs while the
other CPUs use the architected timers. However, in the context of the
automotive, the platform can be partioned to assign 2 CPUs for Linux
and the others CPUs to third party OS. The PIT is then used with their
specifities like the ability to freeze the time which is needed for
instance for debugging purpose.

The setup found for this platform is each timer instance is bound to
CPU0 and CPU1.

A counter is incremented when a timer is successfully initialized and
assigned to a CPU. This counter is used as an index for the CPU number
and to detect when we reach the maximum possible instances for the
platform. That in turn triggers the CPU hotplug callbacks to achieve
the per CPU setup. It is the exact same mechanism found in the NXP STM
driver.

If the timers must be bound to different CPUs, it would require an
additionnal mechanism which is not part of these changes.

Tested on a s32g274a-rdb2.

Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/timer-nxp-pit.c | 115 +++++++++++++++++++++++-----
 1 file changed, 97 insertions(+), 18 deletions(-)

diff --git a/drivers/clocksource/timer-nxp-pit.c b/drivers/clocksource/timer-nxp-pit.c
index 2a0ee4109ead..f2534172b9d4 100644
--- a/drivers/clocksource/timer-nxp-pit.c
+++ b/drivers/clocksource/timer-nxp-pit.c
@@ -1,14 +1,16 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 /*
  * Copyright 2012-2013 Freescale Semiconductor, Inc.
+ * Copyright 2018,2021-2025 NXP
  */
-
 #include <linux/interrupt.h>
 #include <linux/clockchips.h>
+#include <linux/cpuhotplug.h>
 #include <linux/clk.h>
 #include <linux/of_address.h>
 #include <linux/of_irq.h>
 #include <linux/sched_clock.h>
+#include <linux/platform_device.h>
 
 /*
  * Each pit takes 0x10 Bytes register space
@@ -37,11 +39,23 @@
 struct pit_timer {
 	void __iomem *clksrc_base;
 	void __iomem *clkevt_base;
-	unsigned long cycle_per_jiffy;
 	struct clock_event_device ced;
 	struct clocksource cs;
+	int rate;
+};
+
+struct pit_timer_data {
+	int max_pit_instances;
 };
 
+static DEFINE_PER_CPU(struct pit_timer *, pit_timers);
+
+/*
+ * Global structure for multiple PITs initialization
+ */
+static int pit_instances;
+static int max_pit_instances = 1;
+
 static void __iomem *sched_clock_base;
 
 static inline struct pit_timer *ced_to_pit(struct clock_event_device *ced)
@@ -98,8 +112,8 @@ static u64 pit_timer_clocksource_read(struct clocksource *cs)
 	return (u64)~readl(PITCVAL(pit->clksrc_base));
 }
 
-static int __init pit_clocksource_init(struct pit_timer *pit, const char *name,
-				       void __iomem *base, unsigned long rate)
+static int pit_clocksource_init(struct pit_timer *pit, const char *name,
+				void __iomem *base, unsigned long rate)
 {
 	/*
 	 * The channels 0 and 1 can be chained to build a 64-bit
@@ -155,7 +169,7 @@ static int pit_set_periodic(struct clock_event_device *ced)
 {
 	struct pit_timer *pit = ced_to_pit(ced);
 
-	pit_set_next_event(pit->cycle_per_jiffy, ced);
+	pit_set_next_event(pit->rate / HZ, ced);
 
 	return 0;
 }
@@ -181,24 +195,28 @@ static irqreturn_t pit_timer_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static int __init pit_clockevent_init(struct pit_timer *pit, const char *name,
-				      void __iomem *base, unsigned long rate,
-				      int irq, unsigned int cpu)
+static int pit_clockevent_per_cpu_init(struct pit_timer *pit, const char *name,
+				       void __iomem *base, unsigned long rate,
+				       int irq, unsigned int cpu)
 {
+	int ret;
+
 	/*
 	 * The channels 0 and 1 can be chained to build a 64-bit
 	 * timer. Let's use the channel 3 as a clockevent and leave
 	 * the channels 0 and 1 unused for anyone else who needs them
 	 */
 	pit->clkevt_base = base + PIT_CH(3);
-	pit->cycle_per_jiffy = rate / (HZ);
+	pit->rate = rate;
 
 	pit_timer_disable(pit->clkevt_base);
 
 	pit_timer_irqack(pit);
 
-	BUG_ON(request_irq(irq, pit_timer_interrupt, IRQF_TIMER | IRQF_IRQPOLL,
-			   name, &pit->ced));
+	ret = request_irq(irq, pit_timer_interrupt, IRQF_TIMER | IRQF_NOBALANCING,
+			  name, &pit->ced);
+	if (ret)
+		return ret;
 
 	pit->ced.cpumask = cpumask_of(cpu);
 	pit->ced.irq = irq;
@@ -210,6 +228,23 @@ static int __init pit_clockevent_init(struct pit_timer *pit, const char *name,
 	pit->ced.set_next_event	= pit_set_next_event;
 	pit->ced.rating	= 300;
 
+	per_cpu(pit_timers, cpu) = pit;
+
+	return 0;
+}
+
+static int pit_clockevent_starting_cpu(unsigned int cpu)
+{
+	struct pit_timer *pit = per_cpu(pit_timers, cpu);
+	int ret;
+
+	if (!pit)
+		return 0;
+
+	ret = irq_force_affinity(pit->ced.irq, cpumask_of(cpu));
+	if (ret)
+		return ret;
+
 	/*
 	 * The value for the LDVAL register trigger is calculated as:
 	 * LDVAL trigger = (period / clock period) - 1
@@ -218,12 +253,12 @@ static int __init pit_clockevent_init(struct pit_timer *pit, const char *name,
 	 * LDVAL trigger value is 1. And then the min_delta is
 	 * minimal LDVAL trigger value + 1, and the max_delta is full 32-bit.
 	 */
-	clockevents_config_and_register(&pit->ced, rate, 2, 0xffffffff);
+	clockevents_config_and_register(&pit->ced, pit->rate, 2, 0xffffffff);
 
 	return 0;
 }
 
-static int __init pit_timer_init(struct device_node *np)
+static int pit_timer_init(struct device_node *np)
 {
 	struct pit_timer *pit;
 	struct clk *pit_clk;
@@ -262,16 +297,31 @@ static int __init pit_timer_init(struct device_node *np)
 
 	clk_rate = clk_get_rate(pit_clk);
 
-	/* enable the pit module */
-	pit_module_enable(timer_base);
+	pit_module_disable(timer_base);
 
 	ret = pit_clocksource_init(pit, name, timer_base, clk_rate);
-	if (ret)
+	if (ret) {
+		pr_err("Failed to initialize clocksource '%pOF'\n", np);
 		goto out_pit_module_disable;
+	}
 
-	ret = pit_clockevent_init(pit, name, timer_base, clk_rate, irq, 0);
-	if (ret)
+	ret = pit_clockevent_per_cpu_init(pit, name, timer_base, clk_rate, irq, pit_instances);
+	if (ret) {
+		pr_err("Failed to initialize clockevent '%pOF'\n", np);
 		goto out_pit_clocksource_unregister;
+	}
+
+	/* enable the pit module */
+	pit_module_enable(timer_base);
+
+	pit_instances++;
+
+	if (pit_instances == max_pit_instances) {
+		ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "PIT timer:starting",
+					pit_clockevent_starting_cpu, NULL);
+		if (ret < 0)
+			goto out_pit_clocksource_unregister;
+	}
 
 	return 0;
 
@@ -289,4 +339,33 @@ static int __init pit_timer_init(struct device_node *np)
 
 	return ret;
 }
+
+static int pit_timer_probe(struct platform_device *pdev)
+{
+	const struct pit_timer_data *pit_timer_data;
+
+	pit_timer_data = of_device_get_match_data(&pdev->dev);
+	if (pit_timer_data)
+		max_pit_instances = pit_timer_data->max_pit_instances;
+
+	return pit_timer_init(pdev->dev.of_node);
+}
+
+static struct pit_timer_data s32g2_data = { .max_pit_instances = 2 };
+
+static const struct of_device_id pit_timer_of_match[] = {
+	{ .compatible = "nxp,s32g2-pit", .data = &s32g2_data },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pit_timer_of_match);
+
+static struct platform_driver nxp_pit_driver = {
+	.driver = {
+		.name = "nxp-pit",
+		.of_match_table = pit_timer_of_match,
+	},
+	.probe = pit_timer_probe,
+};
+module_platform_driver(nxp_pit_driver);
+
 TIMER_OF_DECLARE(vf610, "fsl,vf610-pit", pit_timer_init);
-- 
2.43.0

Re: [PATCH v2 19/20] dt: bindings: fsl,vf610-pit: Add compatible for s32g2 and s32g3

[Rob Herring]

On Wed, Jul 30, 2025 at 10:27:21AM +0200, Daniel Lezcano wrote:
> The Vybrid Family is a NXP (formerly Freescale) platform having a
> Programmable Interrupt Timer (PIT). This timer is an IP found also on
> the NXP Automotive platform S32G2 and S32G3.
> 
> Add the compatible for those platforms to describe the timer.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  .../devicetree/bindings/timer/fsl,vf610-pit.yaml          | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/timer/fsl,vf610-pit.yaml b/Documentation/devicetree/bindings/timer/fsl,vf610-pit.yaml
> index bee2c35bd0e2..2aac63a58bfd 100644
> --- a/Documentation/devicetree/bindings/timer/fsl,vf610-pit.yaml
> +++ b/Documentation/devicetree/bindings/timer/fsl,vf610-pit.yaml
> @@ -15,8 +15,12 @@ description:
>  
>  properties:
>    compatible:
> -    enum:
> -      - fsl,vf610-pit
> +    oneOf:
> +      - const: fsl,vf610-pit
> +      - const: nxp,s32g2-pit

These 2 can be a single enum. Otherwise,

Acked-by: Rob Herring (Arm) <robh@kernel.org>

> +      - items:
> +          - const: nxp,s32g3-pit
> +          - const: nxp,s32g2-pit
>  
>    reg:
>      maxItems: 1
> -- 
> 2.43.0
> 

Re: [PATCH v2 19/20] dt: bindings: fsl,vf610-pit: Add compatible for s32g2 and s32g3

[Daniel Lezcano]

Hi Rob,

On 31/07/2025 01:36, Rob Herring wrote:
> On Wed, Jul 30, 2025 at 10:27:21AM +0200, Daniel Lezcano wrote:
>> The Vybrid Family is a NXP (formerly Freescale) platform having a
>> Programmable Interrupt Timer (PIT). This timer is an IP found also on
>> the NXP Automotive platform S32G2 and S32G3.
>>
>> Add the compatible for those platforms to describe the timer.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>>   .../devicetree/bindings/timer/fsl,vf610-pit.yaml          | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/timer/fsl,vf610-pit.yaml b/Documentation/devicetree/bindings/timer/fsl,vf610-pit.yaml
>> index bee2c35bd0e2..2aac63a58bfd 100644
>> --- a/Documentation/devicetree/bindings/timer/fsl,vf610-pit.yaml
>> +++ b/Documentation/devicetree/bindings/timer/fsl,vf610-pit.yaml
>> @@ -15,8 +15,12 @@ description:
>>   
>>   properties:
>>     compatible:
>> -    enum:
>> -      - fsl,vf610-pit
>> +    oneOf:
>> +      - const: fsl,vf610-pit
>> +      - const: nxp,s32g2-pit
> 
> These 2 can be a single enum. Otherwise,

Do you mean this ?

    enum:
      - fsl,vf610-pit
      - nxp,s32g2-pit

> Acked-by: Rob Herring (Arm) <robh@kernel.org>

Thanks for the review

>> +      - items:
>> +          - const: nxp,s32g3-pit
>> +          - const: nxp,s32g2-pit
>>   
>>     reg:
>>       maxItems: 1
>> -- 
>> 2.43.0
>>


-- 
<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 v2 19/20] dt: bindings: fsl,vf610-pit: Add compatible for s32g2 and s32g3

[Krzysztof Kozlowski]

On 31/07/2025 09:41, Daniel Lezcano wrote:
> 
> Hi Rob,
> 
> On 31/07/2025 01:36, Rob Herring wrote:
>> On Wed, Jul 30, 2025 at 10:27:21AM +0200, Daniel Lezcano wrote:
>>> The Vybrid Family is a NXP (formerly Freescale) platform having a
>>> Programmable Interrupt Timer (PIT). This timer is an IP found also on
>>> the NXP Automotive platform S32G2 and S32G3.
>>>
>>> Add the compatible for those platforms to describe the timer.
>>>
>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>> ---
>>>   .../devicetree/bindings/timer/fsl,vf610-pit.yaml          | 8 ++++++--
>>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/timer/fsl,vf610-pit.yaml b/Documentation/devicetree/bindings/timer/fsl,vf610-pit.yaml
>>> index bee2c35bd0e2..2aac63a58bfd 100644
>>> --- a/Documentation/devicetree/bindings/timer/fsl,vf610-pit.yaml
>>> +++ b/Documentation/devicetree/bindings/timer/fsl,vf610-pit.yaml
>>> @@ -15,8 +15,12 @@ description:
>>>   
>>>   properties:
>>>     compatible:
>>> -    enum:
>>> -      - fsl,vf610-pit
>>> +    oneOf:
>>> +      - const: fsl,vf610-pit
>>> +      - const: nxp,s32g2-pit
>>
>> These 2 can be a single enum. Otherwise,
> 
> Do you mean this ?
> 
>     enum:
>       - fsl,vf610-pit
>       - nxp,s32g2-pit
> 
Yes.
And also please correct the subject prefix to match subsystem, git log
--oneline or:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18



Best regards,
Krzysztof

Re: [PATCH v2 19/20] dt: bindings: fsl,vf610-pit: Add compatible for s32g2 and s32g3

[Daniel Lezcano]

On 31/07/2025 09:50, Krzysztof Kozlowski wrote:
> On 31/07/2025 09:41, Daniel Lezcano wrote:
>>
>> Hi Rob,
>>
>> On 31/07/2025 01:36, Rob Herring wrote:
>>> On Wed, Jul 30, 2025 at 10:27:21AM +0200, Daniel Lezcano wrote:
>>>> The Vybrid Family is a NXP (formerly Freescale) platform having a
>>>> Programmable Interrupt Timer (PIT). This timer is an IP found also on
>>>> the NXP Automotive platform S32G2 and S32G3.
>>>>
>>>> Add the compatible for those platforms to describe the timer.
>>>>
>>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>> ---
>>>>    .../devicetree/bindings/timer/fsl,vf610-pit.yaml          | 8 ++++++--
>>>>    1 file changed, 6 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/timer/fsl,vf610-pit.yaml b/Documentation/devicetree/bindings/timer/fsl,vf610-pit.yaml
>>>> index bee2c35bd0e2..2aac63a58bfd 100644
>>>> --- a/Documentation/devicetree/bindings/timer/fsl,vf610-pit.yaml
>>>> +++ b/Documentation/devicetree/bindings/timer/fsl,vf610-pit.yaml
>>>> @@ -15,8 +15,12 @@ description:
>>>>    
>>>>    properties:
>>>>      compatible:
>>>> -    enum:
>>>> -      - fsl,vf610-pit
>>>> +    oneOf:
>>>> +      - const: fsl,vf610-pit
>>>> +      - const: nxp,s32g2-pit
>>>
>>> These 2 can be a single enum. Otherwise,
>>
>> Do you mean this ?
>>
>>      enum:
>>        - fsl,vf610-pit
>>        - nxp,s32g2-pit
>>
> Yes.
> And also please correct the subject prefix to match subsystem, git log
> --oneline or:
> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

Sure I'll replace it by:

dt-bindings: timer: Add the PIT for s32g2 and s32g3

Thanks

-- 
<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 v2 07/20] clocksource/drivers/vf-pit: Allocate the struct timer at init time

[Ghennadi Procopciuc]

On 7/30/2025 11:27 AM, Daniel Lezcano wrote:

[...]

>  static int __init pit_timer_init(struct device_node *np)
>  {
> +	struct pit_timer *pit;
>  	struct clk *pit_clk;
>  	void __iomem *timer_base;
>  	unsigned long clk_rate;
>  	int irq, ret;
>  
> +	pit = kzalloc(sizeof(*pit), GFP_KERNEL);
> +	if (!pit)
> +		return -ENOMEM;
> +
> +	ret = -ENXIO;
>  	timer_base = of_iomap(np, 0);
>  	if (!timer_base) {
>  		pr_err("Failed to iomap\n");
> -		return -ENXIO;
> +		goto out_kfree;
>  	}
>  
> +	ret = -EINVAL;
>  	irq = irq_of_parse_and_map(np, 0);
> -	if (irq <= 0)
> -		return -EINVAL;
> +	if (irq <= 0) {
> +		pr_err("Failed to irq_of_parse_and_map\n");
> +		goto out_iounmap;
> +	}
>  
>  	pit_clk = of_clk_get(np, 0);
> -	if (IS_ERR(pit_clk))
> -		return PTR_ERR(pit_clk);
> +	if (IS_ERR(pit_clk)) {
> +		ret = PTR_ERR(pit_clk);
> +		goto out_iounmap;

This does not revert the use of irq_of_parse_and_map. In my opinion, it
should be explicitly reverted as part of the cleanup phase by calling
irq_of_parse_and_map.

> +	}
>  
>  	ret = clk_prepare_enable(pit_clk);
>  	if (ret)
> -		return ret;
> +		goto out_clk_put;
>  
>  	clk_rate = clk_get_rate(pit_clk);
>  
>  	/* enable the pit module */
>  	writel(~PITMCR_MDIS, timer_base + PITMCR);
>  
> -	ret = pit_clocksource_init(&pit_timer, timer_base, clk_rate);
> +	ret = pit_clocksource_init(pit, timer_base, clk_rate);
>  	if (ret)
> -		return ret;
> +		goto out_disable_unprepare;
> +
> +	ret = pit_clockevent_init(pit, timer_base, clk_rate, irq, 0);
> +	if (ret)
> +		goto out_pit_clocksource_unregister;
> +
> +	return 0;
>  
> -	return pit_clockevent_init(&pit_timer, timer_base, clk_rate, irq, 0);
> +out_pit_clocksource_unregister:
> +	clocksource_unregister(&pit->cs);
> +out_disable_unprepare:
> +	clk_disable_unprepare(pit_clk);
> +out_clk_put:
> +	clk_put(pit_clk);
> +out_iounmap:
> +	iounmap(timer_base);
> +out_kfree:
> +	kfree(pit);
> +
> +	return ret;
>  }
>  TIMER_OF_DECLARE(vf610, "fsl,vf610-pit", pit_timer_init);

-- 
Regards,
Ghennadi

Re: [PATCH v2 10/20] clocksource/drivers/vf-pit: Encapsulate the macros

[Ghennadi Procopciuc]

On 7/30/2025 11:27 AM, Daniel Lezcano wrote:
> Pass the base address to the macro, so we can use the macro with
> multiple instances of the timer because we deal with different base
> address. At the same time, change writes to the register to the
> existing corresponding functions.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  drivers/clocksource/timer-vf-pit.c | 35 ++++++++++++++++--------------
>  1 file changed, 19 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/clocksource/timer-vf-pit.c b/drivers/clocksource/timer-vf-pit.c
> index 6a4043801eeb..8f0e26c0512d 100644
> --- a/drivers/clocksource/timer-vf-pit.c
> +++ b/drivers/clocksource/timer-vf-pit.c
> @@ -16,18 +16,21 @@
>  #define PITMCR		0x00
>  #define PIT0_OFFSET	0x100
>  #define PIT_CH(n)       (PIT0_OFFSET + 0x10 * (n))
> -#define PITLDVAL	0x00
> +
>  #define PITCVAL		0x04
> -#define PITTCTRL	0x08
> -#define PITTFLG		0x0c
>  
>  #define PITMCR_MDIS	BIT(1)
>  
> -#define PITTCTRL_TEN	BIT(0)
> -#define PITTCTRL_TIE	BIT(1)
> -#define PITCTRL_CHN	BIT(2)
> +#define PITLDVAL(__base)	(__base)
> +#define PITTCTRL(__base)	((__base) + 0x08)
> +
> +
> +#define PITTCTRL_TEN			BIT(0)
> +#define PITTCTRL_TIE			BIT(1)

Checkpatch:

--------------------------------------------------------------------------
Commit eedb32b08f7f ("clocksource/drivers/vf-pit: Encapsulate the macros")
--------------------------------------------------------------------------
CHECK: Please don't use multiple blank lines
#38: FILE: drivers/clocksource/timer-vf-pit.c:27:
+
+

total: 0 errors, 0 warnings, 1 checks, 79 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or
--fix-inplace.

-- 
Regards,
Ghennadi

Re: [PATCH v2 11/20] clocksource/drivers/vf-pit: Encapsulate the PTLCVAL macro

[Ghennadi Procopciuc]

On 7/30/2025 11:27 AM, Daniel Lezcano wrote:
[...]
 >  static inline struct pit_timer *ced_to_pit(struct clock_event_device
*ced)
>  {
> @@ -69,14 +69,14 @@ static inline void pit_irq_acknowledge(struct pit_timer *pit)
>  
>  static u64 notrace pit_read_sched_clock(void)
>  {
> -	return ~readl(clksrc_base + PITCVAL);
> +	return ~readl(sched_clock_base);

This appears asymmetric compared to 'pit_timer_clocksource_read', where
an explicit cast is present but seems to be missing here.

>  }
>  
>  static u64 pit_timer_clocksource_read(struct clocksource *cs)
>  {
>  	struct pit_timer *pit = cs_to_pit(cs);
>  
> -	return (u64)~readl(pit->clksrc_base + PITCVAL);
> +	return (u64)~readl(PITCVAL(pit->clksrc_base));
>  }
>  
>  static int __init pit_clocksource_init(struct pit_timer *pit, void __iomem *base,
> @@ -99,8 +99,7 @@ static int __init pit_clocksource_init(struct pit_timer *pit, void __iomem *base
>  	writel(~0, PITLDVAL(pit->clksrc_base));
>  	writel(PITTCTRL_TEN, PITTCTRL(pit->clksrc_base));
>  
> -	clksrc_base = pit->clksrc_base;
> -
> +	sched_clock_base = pit->clksrc_base + PITCVAL_OFFSET;
>  	sched_clock_register(pit_read_sched_clock, 32, rate);
>  
>  	return clocksource_register_hz(&pit->cs, rate);

-- 
Regards,
Ghennadi

Re: [PATCH v2 17/20] clocksource/drivers/vf-pit: Unify the function name for irq ack

[Ghennadi Procopciuc]

On 7/30/2025 11:27 AM, Daniel Lezcano wrote:
> Most the function are under the form pit_timer_*, let's change the
> interrupt acknowledgement function name to have the same format.
> 
> No functional changes intended.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Checkpatch:
WARNING: 'acknowledgement' may be misspelled - perhaps 'acknowledgment'?
#8:
interrupt acknowledgement function name to have the same format.
          ^^^^^^^^^^^^^^^

total: 0 errors, 1 warnings, 0 checks, 24 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or
--fix-inplace.

Commit 4a2a139e0b3e ("clocksource/drivers/vf-pit: Unify the function
name for irq ack") has style problems, please review.

-- 
Regards,
Ghennadi

Re: [PATCH v2 18/20] clocksource/drivers/vf-pit: Rename the VF PIT to NXP PIT

[Ghennadi Procopciuc]

On 7/30/2025 11:27 AM, Daniel Lezcano wrote:
> The PIT acronym stands for Periodic Interrupt Timer which is found on
> different NXP platforms not only on the Vybrid Family. Change the name
> to be more generic for the NXP platforms in general. That will be
> consistent with the NXP STM driver naming convention.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  drivers/clocksource/Kconfig                             | 9 ++++++---
>  drivers/clocksource/Makefile                            | 2 +-
>  drivers/clocksource/{timer-vf-pit.c => timer-nxp-pit.c} | 0
>  3 files changed, 7 insertions(+), 4 deletions(-)
>  rename drivers/clocksource/{timer-vf-pit.c => timer-nxp-pit.c} (100%)
> 
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 6f7d371904df..0fd662f67d29 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -474,11 +474,14 @@ config FSL_FTM_TIMER
>  	help
>  	  Support for Freescale FlexTimer Module (FTM) timer.
>  
> -config VF_PIT_TIMER
> -	bool "Vybrid Family Programmable timer" if COMPILE_TEST
> +config NXP_PIT_TIMER
> +	bool "NXP Periodic Interrupt Timer" if COMPILE_TEST
>  	select CLKSRC_MMIO
>  	help
> -	  Support for Periodic Interrupt Timer on Freescale Vybrid Family SoCs.
> +	  Support for Periodic Interrupt Timer on Freescale / NXP
> +	  SoCs. This periodic timer is found on the Vybrid Family and
> +	  the Automotive S32G2/3 platforms. It contains 4 channels
> +	  where two can be coupled to form a 64 bits channel.

Checkpatch:
WARNING: please write a help paragraph that fully describes the config
symbol with at least 4 lines
#29: FILE: drivers/clocksource/Kconfig:477:
+config NXP_PIT_TIMER
+       bool "NXP Periodic Interrupt Timer" if COMPILE_TEST
        select CLKSRC_MMIO
        help
+         Support for Periodic Interrupt Timer on Freescale / NXP
+         Support for Periodic Interrupt Timer on Freescale / NXP
+         SoCs. This periodic timer is found on the Vybrid Family and
+         the Automotive S32G2/3 platforms. It contains 4 channels
+         where two can be coupled to form a 64 bits channel.


total: 0 errors, 2 warnings, 0 checks, 25 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or
--fix-inplace.

Commit 1e9876740925 ("clocksource/drivers/vf-pit: Rename the VF PIT to
NXP PIT") has style problems, please review.

-- 
Regards,
Ghennadi

Re: [PATCH v2 20/20] clocksource/drivers/nxp-pit: Add NXP Automotive s32g2 / s32g3 support

[Ghennadi Procopciuc]

On 7/30/2025 11:27 AM, Daniel Lezcano wrote:
[...]

> -static int __init pit_timer_init(struct device_node *np)
> +static int pit_timer_init(struct device_node *np)
>  {
>  	struct pit_timer *pit;
>  	struct clk *pit_clk;
> @@ -262,16 +297,31 @@ static int __init pit_timer_init(struct device_node *np)
>  
>  	clk_rate = clk_get_rate(pit_clk);
>  
> -	/* enable the pit module */
> -	pit_module_enable(timer_base);
> +	pit_module_disable(timer_base);
>  
>  	ret = pit_clocksource_init(pit, name, timer_base, clk_rate);
> -	if (ret)
> +	if (ret) {
> +		pr_err("Failed to initialize clocksource '%pOF'\n", np);
>  		goto out_pit_module_disable;
> +	}
>  
> -	ret = pit_clockevent_init(pit, name, timer_base, clk_rate, irq, 0);
> -	if (ret)
> +	ret = pit_clockevent_per_cpu_init(pit, name, timer_base, clk_rate, irq, pit_instances);
> +	if (ret) {
> +		pr_err("Failed to initialize clockevent '%pOF'\n", np);
>  		goto out_pit_clocksource_unregister;
> +	}
> +
> +	/* enable the pit module */
> +	pit_module_enable(timer_base);
> +
> +	pit_instances++;
> +
> +	if (pit_instances == max_pit_instances) {
> +		ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "PIT timer:starting",
> +					pit_clockevent_starting_cpu, NULL);
> +		if (ret < 0)
> +			goto out_pit_clocksource_unregister;

The function 'pit_clockevent_per_cpu_init' invokes 'request_irq', but
the corresponding 'free_irq' call is missing in the cleanup path.

> +	}
>  
>  	return 0;
>  
> @@ -289,4 +339,33 @@ static int __init pit_timer_init(struct device_node *np)
>  
>  	return ret;
>  }
-- 
Regards,
Ghennadi

Re: [PATCH v2 18/20] clocksource/drivers/vf-pit: Rename the VF PIT to NXP PIT

[Daniel Lezcano]

On 01/08/2025 09:35, Ghennadi Procopciuc wrote:
> On 7/30/2025 11:27 AM, Daniel Lezcano wrote:
>> The PIT acronym stands for Periodic Interrupt Timer which is found on
>> different NXP platforms not only on the Vybrid Family. Change the name
>> to be more generic for the NXP platforms in general. That will be
>> consistent with the NXP STM driver naming convention.
>>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>> ---
>>   drivers/clocksource/Kconfig                             | 9 ++++++---
>>   drivers/clocksource/Makefile                            | 2 +-
>>   drivers/clocksource/{timer-vf-pit.c => timer-nxp-pit.c} | 0
>>   3 files changed, 7 insertions(+), 4 deletions(-)
>>   rename drivers/clocksource/{timer-vf-pit.c => timer-nxp-pit.c} (100%)
>>
>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>> index 6f7d371904df..0fd662f67d29 100644
>> --- a/drivers/clocksource/Kconfig
>> +++ b/drivers/clocksource/Kconfig
>> @@ -474,11 +474,14 @@ config FSL_FTM_TIMER
>>   	help
>>   	  Support for Freescale FlexTimer Module (FTM) timer.
>>   
>> -config VF_PIT_TIMER
>> -	bool "Vybrid Family Programmable timer" if COMPILE_TEST
>> +config NXP_PIT_TIMER
>> +	bool "NXP Periodic Interrupt Timer" if COMPILE_TEST
>>   	select CLKSRC_MMIO
>>   	help
>> -	  Support for Periodic Interrupt Timer on Freescale Vybrid Family SoCs.
>> +	  Support for Periodic Interrupt Timer on Freescale / NXP
>> +	  SoCs. This periodic timer is found on the Vybrid Family and
>> +	  the Automotive S32G2/3 platforms. It contains 4 channels
>> +	  where two can be coupled to form a 64 bits channel.
> 
> Checkpatch:
> WARNING: please write a help paragraph that fully describes the config
> symbol with at least 4 lines

Sounds like a false positive.

> #29: FILE: drivers/clocksource/Kconfig:477:
> +config NXP_PIT_TIMER
> +       bool "NXP Periodic Interrupt Timer" if COMPILE_TEST
>          select CLKSRC_MMIO
>          help
> +         Support for Periodic Interrupt Timer on Freescale / NXP
> +         Support for Periodic Interrupt Timer on Freescale / NXP
> +         SoCs. This periodic timer is found on the Vybrid Family and
> +         the Automotive S32G2/3 platforms. It contains 4 channels
> +         where two can be coupled to form a 64 bits channel.
> 
> 
> total: 0 errors, 2 warnings, 0 checks, 25 lines checked
> 
> NOTE: For some of the reported defects, checkpatch may be able to
>        mechanically convert to the typical style using --fix or
> --fix-inplace.
> 
> Commit 1e9876740925 ("clocksource/drivers/vf-pit: Rename the VF PIT to
> NXP PIT") has style problems, please review.
> 


-- 
<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 v2 07/20] clocksource/drivers/vf-pit: Allocate the struct timer at init time

[Daniel Lezcano]

On 01/08/2025 09:33, Ghennadi Procopciuc wrote:
> On 7/30/2025 11:27 AM, Daniel Lezcano wrote:
> 
> [...]
> 

[ ... ]

>> +	ret = -EINVAL;
>>   	irq = irq_of_parse_and_map(np, 0);
>> -	if (irq <= 0)
>> -		return -EINVAL;
>> +	if (irq <= 0) {
>> +		pr_err("Failed to irq_of_parse_and_map\n");
>> +		goto out_iounmap;
>> +	}
>>   
>>   	pit_clk = of_clk_get(np, 0);
>> -	if (IS_ERR(pit_clk))
>> -		return PTR_ERR(pit_clk);
>> +	if (IS_ERR(pit_clk)) {
>> +		ret = PTR_ERR(pit_clk);
>> +		goto out_iounmap;
> 
> This does not revert the use of irq_of_parse_and_map. In my opinion, it
> should be explicitly reverted as part of the cleanup phase by calling
> irq_of_parse_and_map.

AFAICT, it does not make sense to undo irq_of_parse_and_map() and there 
is no revert function for that.

However, calling free_irq as commented in patch 20/20 makes sense.


-- 
<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 v2 07/20] clocksource/drivers/vf-pit: Allocate the struct timer at init time

[Ghennadi Procopciuc]

On 8/4/2025 12:12 PM, Daniel Lezcano wrote:
> On 01/08/2025 09:33, Ghennadi Procopciuc wrote:
>> On 7/30/2025 11:27 AM, Daniel Lezcano wrote:
>>
>> [...]
>>
> 
> [ ... ]
> 
>>> +    ret = -EINVAL;
>>>       irq = irq_of_parse_and_map(np, 0);
>>> -    if (irq <= 0)
>>> -        return -EINVAL;
>>> +    if (irq <= 0) {
>>> +        pr_err("Failed to irq_of_parse_and_map\n");
>>> +        goto out_iounmap;
>>> +    }
>>>         pit_clk = of_clk_get(np, 0);
>>> -    if (IS_ERR(pit_clk))
>>> -        return PTR_ERR(pit_clk);
>>> +    if (IS_ERR(pit_clk)) {
>>> +        ret = PTR_ERR(pit_clk);
>>> +        goto out_iounmap;
>>
>> This does not revert the use of irq_of_parse_and_map. In my opinion, it
>> should be explicitly reverted as part of the cleanup phase by calling
>> irq_of_parse_and_map.
> 
> AFAICT, it does not make sense to undo irq_of_parse_and_map() and there
> is no revert function for that.
> 
> However, calling free_irq as commented in patch 20/20 makes sense.
> 
> 

I noticed that irq_of_parse_and_map internally calls both
of_irq_parse_one and irq_create_of_mapping. While the former does not
have a corresponding revert function, the latter can be reversed using
irq_dispose_mapping.

Examples:
    drivers/char/ipmi/ipmi_powernv.c
    drivers/clocksource/timer-microchip-pit64b.c
    drivers/crypto/amcc/crypto4xx_core.c
    drivers/memory/fsl_ifc.c

-- 
Regards,
Ghennadi