[PATCH 0/3] clocksource: sun5i: clean up and convert to platform driver

[PATCH 0/3] clocksource: sun5i: clean up and convert to platform driver

[Mans Rullgard]

The first two of these patches remove a bunch of duplicated code/data in
the sun5i hstimer driver.  To keep the diff clearer, the first patch
introduces an otherwise pointless struct which is then removed in the
second.

The third patch converts the driver to a platform_device driver.  This
is to make it work again on A20 and A31 (I think) where it broke when
the ccu driver was changed to a platform driver.

I have only tested this on an A20 where the hstimer is registered as
expected and can be selected through sysfs.  I hope it doesn't break
something else.

Some additional discussion here:
https://lore.kernel.org/linux-clk/20211119033338.25486-4-samuel@sholland.org/

Mans Rullgard (3):
  clocksource: sun5i: remove duplication of code and data
  clocksource: sun5i: remove pointless struct
  clocksource: sun5i: convert to platform device driver

 drivers/clocksource/timer-sun5i.c | 288 +++++++++++++-----------------
 1 file changed, 123 insertions(+), 165 deletions(-)

-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] clocksource: sun5i: remove duplication of code and data

[Mans Rullgard]

Move the clocksource and clock_event_device structs into the main
struct sun5i_timer, and update the code for the new layout.  This
removes a lot of duplication of both code and data.

Signed-off-by: Mans Rullgard <mans@mansr.com>
---
 drivers/clocksource/timer-sun5i.c | 206 +++++++++++-------------------
 1 file changed, 76 insertions(+), 130 deletions(-)

diff --git a/drivers/clocksource/timer-sun5i.c b/drivers/clocksource/timer-sun5i.c
index 7d5fa9069906..e0ca97cf80cb 100644
--- a/drivers/clocksource/timer-sun5i.c
+++ b/drivers/clocksource/timer-sun5i.c
@@ -35,31 +35,26 @@
 
 #define TIMER_SYNC_TICKS	3
 
-struct sun5i_timer {
+/* Pointless struct to minimise diff */
+struct _sun5i_timer {
 	void __iomem		*base;
 	struct clk		*clk;
 	struct notifier_block	clk_rate_cb;
 	u32			ticks_per_jiffy;
 };
 
-#define to_sun5i_timer(x) \
-	container_of(x, struct sun5i_timer, clk_rate_cb)
-
-struct sun5i_timer_clksrc {
-	struct sun5i_timer	timer;
+struct sun5i_timer {
+	struct _sun5i_timer	timer;
 	struct clocksource	clksrc;
-};
-
-#define to_sun5i_timer_clksrc(x) \
-	container_of(x, struct sun5i_timer_clksrc, clksrc)
-
-struct sun5i_timer_clkevt {
-	struct sun5i_timer		timer;
 	struct clock_event_device	clkevt;
 };
 
-#define to_sun5i_timer_clkevt(x) \
-	container_of(x, struct sun5i_timer_clkevt, clkevt)
+#define nb_to_sun5i_timer(x) \
+	container_of(x, struct sun5i_timer, timer.clk_rate_cb)
+#define clksrc_to_sun5i_timer(x) \
+	container_of(x, struct sun5i_timer, clksrc)
+#define clkevt_to_sun5i_timer(x) \
+	container_of(x, struct sun5i_timer, clkevt)
 
 /*
  * When we disable a timer, we need to wait at least for 2 cycles of
@@ -67,7 +62,7 @@ struct sun5i_timer_clkevt {
  * that is already setup and runs at the same frequency than the other
  * timers, and we never will be disabled.
  */
-static void sun5i_clkevt_sync(struct sun5i_timer_clkevt *ce)
+static void sun5i_clkevt_sync(struct sun5i_timer *ce)
 {
 	u32 old = readl(ce->timer.base + TIMER_CNTVAL_LO_REG(1));
 
@@ -75,7 +70,7 @@ static void sun5i_clkevt_sync(struct sun5i_timer_clkevt *ce)
 		cpu_relax();
 }
 
-static void sun5i_clkevt_time_stop(struct sun5i_timer_clkevt *ce, u8 timer)
+static void sun5i_clkevt_time_stop(struct sun5i_timer *ce, u8 timer)
 {
 	u32 val = readl(ce->timer.base + TIMER_CTL_REG(timer));
 	writel(val & ~TIMER_CTL_ENABLE, ce->timer.base + TIMER_CTL_REG(timer));
@@ -83,12 +78,12 @@ static void sun5i_clkevt_time_stop(struct sun5i_timer_clkevt *ce, u8 timer)
 	sun5i_clkevt_sync(ce);
 }
 
-static void sun5i_clkevt_time_setup(struct sun5i_timer_clkevt *ce, u8 timer, u32 delay)
+static void sun5i_clkevt_time_setup(struct sun5i_timer *ce, u8 timer, u32 delay)
 {
 	writel(delay, ce->timer.base + TIMER_INTVAL_LO_REG(timer));
 }
 
-static void sun5i_clkevt_time_start(struct sun5i_timer_clkevt *ce, u8 timer, bool periodic)
+static void sun5i_clkevt_time_start(struct sun5i_timer *ce, u8 timer, bool periodic)
 {
 	u32 val = readl(ce->timer.base + TIMER_CTL_REG(timer));
 
@@ -103,7 +98,7 @@ static void sun5i_clkevt_time_start(struct sun5i_timer_clkevt *ce, u8 timer, boo
 
 static int sun5i_clkevt_shutdown(struct clock_event_device *clkevt)
 {
-	struct sun5i_timer_clkevt *ce = to_sun5i_timer_clkevt(clkevt);
+	struct sun5i_timer *ce = clkevt_to_sun5i_timer(clkevt);
 
 	sun5i_clkevt_time_stop(ce, 0);
 	return 0;
@@ -111,7 +106,7 @@ static int sun5i_clkevt_shutdown(struct clock_event_device *clkevt)
 
 static int sun5i_clkevt_set_oneshot(struct clock_event_device *clkevt)
 {
-	struct sun5i_timer_clkevt *ce = to_sun5i_timer_clkevt(clkevt);
+	struct sun5i_timer *ce = clkevt_to_sun5i_timer(clkevt);
 
 	sun5i_clkevt_time_stop(ce, 0);
 	sun5i_clkevt_time_start(ce, 0, false);
@@ -120,7 +115,7 @@ static int sun5i_clkevt_set_oneshot(struct clock_event_device *clkevt)
 
 static int sun5i_clkevt_set_periodic(struct clock_event_device *clkevt)
 {
-	struct sun5i_timer_clkevt *ce = to_sun5i_timer_clkevt(clkevt);
+	struct sun5i_timer *ce = clkevt_to_sun5i_timer(clkevt);
 
 	sun5i_clkevt_time_stop(ce, 0);
 	sun5i_clkevt_time_setup(ce, 0, ce->timer.ticks_per_jiffy);
@@ -131,7 +126,7 @@ static int sun5i_clkevt_set_periodic(struct clock_event_device *clkevt)
 static int sun5i_clkevt_next_event(unsigned long evt,
 				   struct clock_event_device *clkevt)
 {
-	struct sun5i_timer_clkevt *ce = to_sun5i_timer_clkevt(clkevt);
+	struct sun5i_timer *ce = clkevt_to_sun5i_timer(clkevt);
 
 	sun5i_clkevt_time_stop(ce, 0);
 	sun5i_clkevt_time_setup(ce, 0, evt - TIMER_SYNC_TICKS);
@@ -142,7 +137,7 @@ static int sun5i_clkevt_next_event(unsigned long evt,
 
 static irqreturn_t sun5i_timer_interrupt(int irq, void *dev_id)
 {
-	struct sun5i_timer_clkevt *ce = dev_id;
+	struct sun5i_timer *ce = dev_id;
 
 	writel(0x1, ce->timer.base + TIMER_IRQ_ST_REG);
 	ce->clkevt.event_handler(&ce->clkevt);
@@ -152,17 +147,16 @@ static irqreturn_t sun5i_timer_interrupt(int irq, void *dev_id)
 
 static u64 sun5i_clksrc_read(struct clocksource *clksrc)
 {
-	struct sun5i_timer_clksrc *cs = to_sun5i_timer_clksrc(clksrc);
+	struct sun5i_timer *cs = clksrc_to_sun5i_timer(clksrc);
 
 	return ~readl(cs->timer.base + TIMER_CNTVAL_LO_REG(1));
 }
 
-static int sun5i_rate_cb_clksrc(struct notifier_block *nb,
-				unsigned long event, void *data)
+static int sun5i_rate_cb(struct notifier_block *nb,
+			 unsigned long event, void *data)
 {
 	struct clk_notifier_data *ndata = data;
-	struct sun5i_timer *timer = to_sun5i_timer(nb);
-	struct sun5i_timer_clksrc *cs = container_of(timer, struct sun5i_timer_clksrc, timer);
+	struct sun5i_timer *cs = nb_to_sun5i_timer(nb);
 
 	switch (event) {
 	case PRE_RATE_CHANGE:
@@ -171,6 +165,8 @@ static int sun5i_rate_cb_clksrc(struct notifier_block *nb,
 
 	case POST_RATE_CHANGE:
 		clocksource_register_hz(&cs->clksrc, ndata->new_rate);
+		clockevents_update_freq(&cs->clkevt, ndata->new_rate);
+		cs->timer.ticks_per_jiffy = DIV_ROUND_UP(ndata->new_rate, HZ);
 		break;
 
 	default:
@@ -181,41 +177,12 @@ static int sun5i_rate_cb_clksrc(struct notifier_block *nb,
 }
 
 static int __init sun5i_setup_clocksource(struct device_node *node,
-					  void __iomem *base,
-					  struct clk *clk, int irq)
+					  struct sun5i_timer *cs,
+					  unsigned long rate)
 {
-	struct sun5i_timer_clksrc *cs;
-	unsigned long rate;
+	void __iomem *base = cs->timer.base;
 	int ret;
 
-	cs = kzalloc(sizeof(*cs), GFP_KERNEL);
-	if (!cs)
-		return -ENOMEM;
-
-	ret = clk_prepare_enable(clk);
-	if (ret) {
-		pr_err("Couldn't enable parent clock\n");
-		goto err_free;
-	}
-
-	rate = clk_get_rate(clk);
-	if (!rate) {
-		pr_err("Couldn't get parent clock rate\n");
-		ret = -EINVAL;
-		goto err_disable_clk;
-	}
-
-	cs->timer.base = base;
-	cs->timer.clk = clk;
-	cs->timer.clk_rate_cb.notifier_call = sun5i_rate_cb_clksrc;
-	cs->timer.clk_rate_cb.next = NULL;
-
-	ret = clk_notifier_register(clk, &cs->timer.clk_rate_cb);
-	if (ret) {
-		pr_err("Unable to register clock notifier.\n");
-		goto err_disable_clk;
-	}
-
 	writel(~0, base + TIMER_INTVAL_LO_REG(1));
 	writel(TIMER_CTL_ENABLE | TIMER_CTL_RELOAD,
 	       base + TIMER_CTL_REG(1));
@@ -229,72 +196,20 @@ static int __init sun5i_setup_clocksource(struct device_node *node,
 	ret = clocksource_register_hz(&cs->clksrc, rate);
 	if (ret) {
 		pr_err("Couldn't register clock source.\n");
-		goto err_remove_notifier;
+		return ret;
 	}
 
 	return 0;
-
-err_remove_notifier:
-	clk_notifier_unregister(clk, &cs->timer.clk_rate_cb);
-err_disable_clk:
-	clk_disable_unprepare(clk);
-err_free:
-	kfree(cs);
-	return ret;
-}
-
-static int sun5i_rate_cb_clkevt(struct notifier_block *nb,
-				unsigned long event, void *data)
-{
-	struct clk_notifier_data *ndata = data;
-	struct sun5i_timer *timer = to_sun5i_timer(nb);
-	struct sun5i_timer_clkevt *ce = container_of(timer, struct sun5i_timer_clkevt, timer);
-
-	if (event == POST_RATE_CHANGE) {
-		clockevents_update_freq(&ce->clkevt, ndata->new_rate);
-		ce->timer.ticks_per_jiffy = DIV_ROUND_UP(ndata->new_rate, HZ);
-	}
-
-	return NOTIFY_DONE;
 }
 
-static int __init sun5i_setup_clockevent(struct device_node *node, void __iomem *base,
-					 struct clk *clk, int irq)
+static int __init sun5i_setup_clockevent(struct device_node *node,
+					 struct sun5i_timer *ce,
+					 unsigned long rate, int irq)
 {
-	struct sun5i_timer_clkevt *ce;
-	unsigned long rate;
+	void __iomem *base = ce->timer.base;
 	int ret;
 	u32 val;
 
-	ce = kzalloc(sizeof(*ce), GFP_KERNEL);
-	if (!ce)
-		return -ENOMEM;
-
-	ret = clk_prepare_enable(clk);
-	if (ret) {
-		pr_err("Couldn't enable parent clock\n");
-		goto err_free;
-	}
-
-	rate = clk_get_rate(clk);
-	if (!rate) {
-		pr_err("Couldn't get parent clock rate\n");
-		ret = -EINVAL;
-		goto err_disable_clk;
-	}
-
-	ce->timer.base = base;
-	ce->timer.ticks_per_jiffy = DIV_ROUND_UP(rate, HZ);
-	ce->timer.clk = clk;
-	ce->timer.clk_rate_cb.notifier_call = sun5i_rate_cb_clkevt;
-	ce->timer.clk_rate_cb.next = NULL;
-
-	ret = clk_notifier_register(clk, &ce->timer.clk_rate_cb);
-	if (ret) {
-		pr_err("Unable to register clock notifier.\n");
-		goto err_disable_clk;
-	}
-
 	ce->clkevt.name = node->name;
 	ce->clkevt.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
 	ce->clkevt.set_next_event = sun5i_clkevt_next_event;
@@ -317,27 +232,25 @@ static int __init sun5i_setup_clockevent(struct device_node *node, void __iomem
 			  "sun5i_timer0", ce);
 	if (ret) {
 		pr_err("Unable to register interrupt\n");
-		goto err_remove_notifier;
+		return ret;
 	}
 
 	return 0;
-
-err_remove_notifier:
-	clk_notifier_unregister(clk, &ce->timer.clk_rate_cb);
-err_disable_clk:
-	clk_disable_unprepare(clk);
-err_free:
-	kfree(ce);
-	return ret;
 }
 
 static int __init sun5i_timer_init(struct device_node *node)
 {
+	struct sun5i_timer *st;
 	struct reset_control *rstc;
 	void __iomem *timer_base;
 	struct clk *clk;
+	unsigned long rate;
 	int irq, ret;
 
+	st = kzalloc(sizeof(*st), GFP_KERNEL);
+	if (!st)
+		return -ENOMEM;
+
 	timer_base = of_io_request_and_map(node, 0, of_node_full_name(node));
 	if (IS_ERR(timer_base)) {
 		pr_err("Can't map registers\n");
@@ -356,15 +269,48 @@ static int __init sun5i_timer_init(struct device_node *node)
 		return PTR_ERR(clk);
 	}
 
+	ret = clk_prepare_enable(clk);
+	if (ret) {
+		pr_err("Couldn't enable parent clock\n");
+		goto err_free;
+	}
+
+	rate = clk_get_rate(clk);
+	if (!rate) {
+		pr_err("Couldn't get parent clock rate\n");
+		ret = -EINVAL;
+		goto err_disable_clk;
+	}
+
+	st->timer.base = timer_base;
+	st->timer.ticks_per_jiffy = DIV_ROUND_UP(rate, HZ);
+	st->timer.clk = clk;
+	st->timer.clk_rate_cb.notifier_call = sun5i_rate_cb;
+	st->timer.clk_rate_cb.next = NULL;
+
+	ret = clk_notifier_register(clk, &st->timer.clk_rate_cb);
+	if (ret) {
+		pr_err("Unable to register clock notifier.\n");
+		goto err_disable_clk;
+	}
+
 	rstc = of_reset_control_get(node, NULL);
 	if (!IS_ERR(rstc))
 		reset_control_deassert(rstc);
 
-	ret = sun5i_setup_clocksource(node, timer_base, clk, irq);
+	ret = sun5i_setup_clocksource(node, st, rate);
 	if (ret)
-		return ret;
+		goto err_remove_notifier;
 
-	return sun5i_setup_clockevent(node, timer_base, clk, irq);
+	return sun5i_setup_clockevent(node, st, rate, irq);
+
+err_remove_notifier:
+	clk_notifier_unregister(clk, &st->timer.clk_rate_cb);
+err_disable_clk:
+	clk_disable_unprepare(clk);
+err_free:
+	kfree(st);
+	return ret;
 }
 TIMER_OF_DECLARE(sun5i_a13, "allwinner,sun5i-a13-hstimer",
 			   sun5i_timer_init);
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] clocksource: sun5i: remove duplication of code and data

[Jernej Škrabec]

Dne petek, 30. junij 2023 ob 22:01:26 CEST je Mans Rullgard napisal(a):
> Move the clocksource and clock_event_device structs into the main
> struct sun5i_timer, and update the code for the new layout.  This
> removes a lot of duplication of both code and data.
> 
> Signed-off-by: Mans Rullgard <mans@mansr.com>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] clocksource: sun5i: remove pointless struct

[Mans Rullgard]

Remove the pointless struct added in the previous patch to make
the diff smaller.

Signed-off-by: Mans Rullgard <mans@mansr.com>
---
 drivers/clocksource/timer-sun5i.c | 49 ++++++++++++++-----------------
 1 file changed, 22 insertions(+), 27 deletions(-)

diff --git a/drivers/clocksource/timer-sun5i.c b/drivers/clocksource/timer-sun5i.c
index e0ca97cf80cb..3ca427e54daf 100644
--- a/drivers/clocksource/timer-sun5i.c
+++ b/drivers/clocksource/timer-sun5i.c
@@ -35,22 +35,17 @@
 
 #define TIMER_SYNC_TICKS	3
 
-/* Pointless struct to minimise diff */
-struct _sun5i_timer {
+struct sun5i_timer {
 	void __iomem		*base;
 	struct clk		*clk;
 	struct notifier_block	clk_rate_cb;
 	u32			ticks_per_jiffy;
-};
-
-struct sun5i_timer {
-	struct _sun5i_timer	timer;
 	struct clocksource	clksrc;
 	struct clock_event_device	clkevt;
 };
 
 #define nb_to_sun5i_timer(x) \
-	container_of(x, struct sun5i_timer, timer.clk_rate_cb)
+	container_of(x, struct sun5i_timer, clk_rate_cb)
 #define clksrc_to_sun5i_timer(x) \
 	container_of(x, struct sun5i_timer, clksrc)
 #define clkevt_to_sun5i_timer(x) \
@@ -64,28 +59,28 @@ struct sun5i_timer {
  */
 static void sun5i_clkevt_sync(struct sun5i_timer *ce)
 {
-	u32 old = readl(ce->timer.base + TIMER_CNTVAL_LO_REG(1));
+	u32 old = readl(ce->base + TIMER_CNTVAL_LO_REG(1));
 
-	while ((old - readl(ce->timer.base + TIMER_CNTVAL_LO_REG(1))) < TIMER_SYNC_TICKS)
+	while ((old - readl(ce->base + TIMER_CNTVAL_LO_REG(1))) < TIMER_SYNC_TICKS)
 		cpu_relax();
 }
 
 static void sun5i_clkevt_time_stop(struct sun5i_timer *ce, u8 timer)
 {
-	u32 val = readl(ce->timer.base + TIMER_CTL_REG(timer));
-	writel(val & ~TIMER_CTL_ENABLE, ce->timer.base + TIMER_CTL_REG(timer));
+	u32 val = readl(ce->base + TIMER_CTL_REG(timer));
+	writel(val & ~TIMER_CTL_ENABLE, ce->base + TIMER_CTL_REG(timer));
 
 	sun5i_clkevt_sync(ce);
 }
 
 static void sun5i_clkevt_time_setup(struct sun5i_timer *ce, u8 timer, u32 delay)
 {
-	writel(delay, ce->timer.base + TIMER_INTVAL_LO_REG(timer));
+	writel(delay, ce->base + TIMER_INTVAL_LO_REG(timer));
 }
 
 static void sun5i_clkevt_time_start(struct sun5i_timer *ce, u8 timer, bool periodic)
 {
-	u32 val = readl(ce->timer.base + TIMER_CTL_REG(timer));
+	u32 val = readl(ce->base + TIMER_CTL_REG(timer));
 
 	if (periodic)
 		val &= ~TIMER_CTL_ONESHOT;
@@ -93,7 +88,7 @@ static void sun5i_clkevt_time_start(struct sun5i_timer *ce, u8 timer, bool perio
 		val |= TIMER_CTL_ONESHOT;
 
 	writel(val | TIMER_CTL_ENABLE | TIMER_CTL_RELOAD,
-	       ce->timer.base + TIMER_CTL_REG(timer));
+	       ce->base + TIMER_CTL_REG(timer));
 }
 
 static int sun5i_clkevt_shutdown(struct clock_event_device *clkevt)
@@ -118,7 +113,7 @@ static int sun5i_clkevt_set_periodic(struct clock_event_device *clkevt)
 	struct sun5i_timer *ce = clkevt_to_sun5i_timer(clkevt);
 
 	sun5i_clkevt_time_stop(ce, 0);
-	sun5i_clkevt_time_setup(ce, 0, ce->timer.ticks_per_jiffy);
+	sun5i_clkevt_time_setup(ce, 0, ce->ticks_per_jiffy);
 	sun5i_clkevt_time_start(ce, 0, true);
 	return 0;
 }
@@ -139,7 +134,7 @@ static irqreturn_t sun5i_timer_interrupt(int irq, void *dev_id)
 {
 	struct sun5i_timer *ce = dev_id;
 
-	writel(0x1, ce->timer.base + TIMER_IRQ_ST_REG);
+	writel(0x1, ce->base + TIMER_IRQ_ST_REG);
 	ce->clkevt.event_handler(&ce->clkevt);
 
 	return IRQ_HANDLED;
@@ -149,7 +144,7 @@ static u64 sun5i_clksrc_read(struct clocksource *clksrc)
 {
 	struct sun5i_timer *cs = clksrc_to_sun5i_timer(clksrc);
 
-	return ~readl(cs->timer.base + TIMER_CNTVAL_LO_REG(1));
+	return ~readl(cs->base + TIMER_CNTVAL_LO_REG(1));
 }
 
 static int sun5i_rate_cb(struct notifier_block *nb,
@@ -166,7 +161,7 @@ static int sun5i_rate_cb(struct notifier_block *nb,
 	case POST_RATE_CHANGE:
 		clocksource_register_hz(&cs->clksrc, ndata->new_rate);
 		clockevents_update_freq(&cs->clkevt, ndata->new_rate);
-		cs->timer.ticks_per_jiffy = DIV_ROUND_UP(ndata->new_rate, HZ);
+		cs->ticks_per_jiffy = DIV_ROUND_UP(ndata->new_rate, HZ);
 		break;
 
 	default:
@@ -180,7 +175,7 @@ static int __init sun5i_setup_clocksource(struct device_node *node,
 					  struct sun5i_timer *cs,
 					  unsigned long rate)
 {
-	void __iomem *base = cs->timer.base;
+	void __iomem *base = cs->base;
 	int ret;
 
 	writel(~0, base + TIMER_INTVAL_LO_REG(1));
@@ -206,7 +201,7 @@ static int __init sun5i_setup_clockevent(struct device_node *node,
 					 struct sun5i_timer *ce,
 					 unsigned long rate, int irq)
 {
-	void __iomem *base = ce->timer.base;
+	void __iomem *base = ce->base;
 	int ret;
 	u32 val;
 
@@ -282,13 +277,13 @@ static int __init sun5i_timer_init(struct device_node *node)
 		goto err_disable_clk;
 	}
 
-	st->timer.base = timer_base;
-	st->timer.ticks_per_jiffy = DIV_ROUND_UP(rate, HZ);
-	st->timer.clk = clk;
-	st->timer.clk_rate_cb.notifier_call = sun5i_rate_cb;
-	st->timer.clk_rate_cb.next = NULL;
+	st->base = timer_base;
+	st->ticks_per_jiffy = DIV_ROUND_UP(rate, HZ);
+	st->clk = clk;
+	st->clk_rate_cb.notifier_call = sun5i_rate_cb;
+	st->clk_rate_cb.next = NULL;
 
-	ret = clk_notifier_register(clk, &st->timer.clk_rate_cb);
+	ret = clk_notifier_register(clk, &st->clk_rate_cb);
 	if (ret) {
 		pr_err("Unable to register clock notifier.\n");
 		goto err_disable_clk;
@@ -305,7 +300,7 @@ static int __init sun5i_timer_init(struct device_node *node)
 	return sun5i_setup_clockevent(node, st, rate, irq);
 
 err_remove_notifier:
-	clk_notifier_unregister(clk, &st->timer.clk_rate_cb);
+	clk_notifier_unregister(clk, &st->clk_rate_cb);
 err_disable_clk:
 	clk_disable_unprepare(clk);
 err_free:
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] clocksource: sun5i: remove pointless struct

[Jernej Škrabec]

Dne petek, 30. junij 2023 ob 22:01:27 CEST je Mans Rullgard napisal(a):
> Remove the pointless struct added in the previous patch to make
> the diff smaller.
> 
> Signed-off-by: Mans Rullgard <mans@mansr.com>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] clocksource: sun5i: convert to platform device driver

[Mans Rullgard]

Convert the sun5i hstimer driver to a platform device driver.
This makes it work again on A20 and other systems where the
clock is provided by a platform device driver.

Fixes: 7ec03b588d22 ("clk: sunxi-ng: Convert early providers to platform drivers")
Signed-off-by: Mans Rullgard <mans@mansr.com>
---
 drivers/clocksource/timer-sun5i.c | 121 +++++++++++++++++-------------
 1 file changed, 69 insertions(+), 52 deletions(-)

diff --git a/drivers/clocksource/timer-sun5i.c b/drivers/clocksource/timer-sun5i.c
index 3ca427e54daf..69fee3540d37 100644
--- a/drivers/clocksource/timer-sun5i.c
+++ b/drivers/clocksource/timer-sun5i.c
@@ -16,9 +16,7 @@
 #include <linux/irqreturn.h>
 #include <linux/reset.h>
 #include <linux/slab.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-#include <linux/of_irq.h>
+#include <linux/platform_device.h>
 
 #define TIMER_IRQ_EN_REG		0x00
 #define TIMER_IRQ_EN(val)			BIT(val)
@@ -171,10 +169,10 @@ static int sun5i_rate_cb(struct notifier_block *nb,
 	return NOTIFY_DONE;
 }
 
-static int __init sun5i_setup_clocksource(struct device_node *node,
-					  struct sun5i_timer *cs,
-					  unsigned long rate)
+static int sun5i_setup_clocksource(struct platform_device *pdev,
+				   unsigned long rate)
 {
+	struct sun5i_timer *cs = platform_get_drvdata(pdev);
 	void __iomem *base = cs->base;
 	int ret;
 
@@ -182,7 +180,7 @@ static int __init sun5i_setup_clocksource(struct device_node *node,
 	writel(TIMER_CTL_ENABLE | TIMER_CTL_RELOAD,
 	       base + TIMER_CTL_REG(1));
 
-	cs->clksrc.name = node->name;
+	cs->clksrc.name = pdev->dev.of_node->name;
 	cs->clksrc.rating = 340;
 	cs->clksrc.read = sun5i_clksrc_read;
 	cs->clksrc.mask = CLOCKSOURCE_MASK(32);
@@ -190,22 +188,23 @@ static int __init sun5i_setup_clocksource(struct device_node *node,
 
 	ret = clocksource_register_hz(&cs->clksrc, rate);
 	if (ret) {
-		pr_err("Couldn't register clock source.\n");
+		dev_err(&pdev->dev, "Couldn't register clock source.\n");
 		return ret;
 	}
 
 	return 0;
 }
 
-static int __init sun5i_setup_clockevent(struct device_node *node,
-					 struct sun5i_timer *ce,
-					 unsigned long rate, int irq)
+static int sun5i_setup_clockevent(struct platform_device *pdev,
+				  unsigned long rate, int irq)
 {
+	struct device *dev = &pdev->dev;
+	struct sun5i_timer *ce = platform_get_drvdata(pdev);
 	void __iomem *base = ce->base;
 	int ret;
 	u32 val;
 
-	ce->clkevt.name = node->name;
+	ce->clkevt.name = dev->of_node->name;
 	ce->clkevt.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
 	ce->clkevt.set_next_event = sun5i_clkevt_next_event;
 	ce->clkevt.set_state_shutdown = sun5i_clkevt_shutdown;
@@ -223,18 +222,20 @@ static int __init sun5i_setup_clockevent(struct device_node *node,
 	clockevents_config_and_register(&ce->clkevt, rate,
 					TIMER_SYNC_TICKS, 0xffffffff);
 
-	ret = request_irq(irq, sun5i_timer_interrupt, IRQF_TIMER | IRQF_IRQPOLL,
-			  "sun5i_timer0", ce);
+	ret = devm_request_irq(dev, irq, sun5i_timer_interrupt,
+			       IRQF_TIMER | IRQF_IRQPOLL,
+			       "sun5i_timer0", ce);
 	if (ret) {
-		pr_err("Unable to register interrupt\n");
+		dev_err(dev, "Unable to register interrupt\n");
 		return ret;
 	}
 
 	return 0;
 }
 
-static int __init sun5i_timer_init(struct device_node *node)
+static int sun5i_timer_probe(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
 	struct sun5i_timer *st;
 	struct reset_control *rstc;
 	void __iomem *timer_base;
@@ -242,39 +243,34 @@ static int __init sun5i_timer_init(struct device_node *node)
 	unsigned long rate;
 	int irq, ret;
 
-	st = kzalloc(sizeof(*st), GFP_KERNEL);
+	st = devm_kzalloc(dev, sizeof(*st), GFP_KERNEL);
 	if (!st)
 		return -ENOMEM;
 
-	timer_base = of_io_request_and_map(node, 0, of_node_full_name(node));
+	platform_set_drvdata(pdev, st);
+
+	timer_base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(timer_base)) {
-		pr_err("Can't map registers\n");
+		dev_err(dev, "Can't map registers\n");
 		return PTR_ERR(timer_base);
 	}
 
-	irq = irq_of_parse_and_map(node, 0);
-	if (irq <= 0) {
-		pr_err("Can't parse IRQ\n");
-		return -EINVAL;
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(dev, "Can't get IRQ\n");
+		return irq;
 	}
 
-	clk = of_clk_get(node, 0);
+	clk = devm_clk_get_enabled(dev, NULL);
 	if (IS_ERR(clk)) {
-		pr_err("Can't get timer clock\n");
+		dev_err(dev, "Can't get timer clock\n");
 		return PTR_ERR(clk);
 	}
 
-	ret = clk_prepare_enable(clk);
-	if (ret) {
-		pr_err("Couldn't enable parent clock\n");
-		goto err_free;
-	}
-
 	rate = clk_get_rate(clk);
 	if (!rate) {
-		pr_err("Couldn't get parent clock rate\n");
-		ret = -EINVAL;
-		goto err_disable_clk;
+		dev_err(dev, "Couldn't get parent clock rate\n");
+		return -EINVAL;
 	}
 
 	st->base = timer_base;
@@ -283,31 +279,52 @@ static int __init sun5i_timer_init(struct device_node *node)
 	st->clk_rate_cb.notifier_call = sun5i_rate_cb;
 	st->clk_rate_cb.next = NULL;
 
-	ret = clk_notifier_register(clk, &st->clk_rate_cb);
+	ret = devm_clk_notifier_register(dev, clk, &st->clk_rate_cb);
 	if (ret) {
-		pr_err("Unable to register clock notifier.\n");
-		goto err_disable_clk;
+		dev_err(dev, "Unable to register clock notifier.\n");
+		return ret;
 	}
 
-	rstc = of_reset_control_get(node, NULL);
-	if (!IS_ERR(rstc))
+	rstc = devm_reset_control_get_optional_exclusive(dev, NULL);
+	if (rstc)
 		reset_control_deassert(rstc);
 
-	ret = sun5i_setup_clocksource(node, st, rate);
+	ret = sun5i_setup_clocksource(pdev, rate);
 	if (ret)
-		goto err_remove_notifier;
+		return ret;
 
-	return sun5i_setup_clockevent(node, st, rate, irq);
+	ret = sun5i_setup_clockevent(pdev, rate, irq);
+	if (ret)
+		goto err_unreg_clocksource;
+
+	return 0;
 
-err_remove_notifier:
-	clk_notifier_unregister(clk, &st->clk_rate_cb);
-err_disable_clk:
-	clk_disable_unprepare(clk);
-err_free:
-	kfree(st);
+err_unreg_clocksource:
+	clocksource_unregister(&st->clksrc);
 	return ret;
 }
-TIMER_OF_DECLARE(sun5i_a13, "allwinner,sun5i-a13-hstimer",
-			   sun5i_timer_init);
-TIMER_OF_DECLARE(sun7i_a20, "allwinner,sun7i-a20-hstimer",
-			   sun5i_timer_init);
+
+static void sun5i_timer_remove(struct platform_device *pdev)
+{
+	struct sun5i_timer *st = platform_get_drvdata(pdev);
+
+	clocksource_unregister(&st->clksrc);
+}
+
+static const struct of_device_id sun5i_timer_of_match[] = {
+	{ .compatible = "allwinner,sun5i-a13-hstimer" },
+	{ .compatible = "allwinner,sun7i-a20-hstimer" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sun5i_timer_of_match);
+
+static struct platform_driver sun5i_timer_driver = {
+	.probe		= sun5i_timer_probe,
+	.remove_new	= sun5i_timer_remove,
+	.driver	= {
+		.name	= "sun5i-timer",
+		.of_match_table = sun5i_timer_of_match,
+		.suppress_bind_attrs = true,
+	},
+};
+module_platform_driver(sun5i_timer_driver);
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] clocksource: sun5i: convert to platform device driver

[Jernej Škrabec]

Dne petek, 30. junij 2023 ob 22:01:28 CEST je Mans Rullgard napisal(a):
> Convert the sun5i hstimer driver to a platform device driver.
> This makes it work again on A20 and other systems where the
> clock is provided by a platform device driver.
> 
> Fixes: 7ec03b588d22 ("clk: sunxi-ng: Convert early providers to platform
> drivers") Signed-off-by: Mans Rullgard <mans@mansr.com>

Acked-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Best regards,
Jernej



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] clocksource: sun5i: clean up and convert to platform driver

[Maxime Ripard]

On Fri, 30 Jun 2023 21:01:25 +0100, Mans Rullgard wrote:
> The first two of these patches remove a bunch of duplicated code/data in
> the sun5i hstimer driver.  To keep the diff clearer, the first patch
> introduces an otherwise pointless struct which is then removed in the
> second.
> 
> 
> [ ... ]

Acked-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] clocksource: sun5i: clean up and convert to platform driver

[Daniel Lezcano]

On 30/06/2023 22:01, Mans Rullgard wrote:
> The first two of these patches remove a bunch of duplicated code/data in
> the sun5i hstimer driver.  To keep the diff clearer, the first patch
> introduces an otherwise pointless struct which is then removed in the
> second.
> 
> The third patch converts the driver to a platform_device driver.  This
> is to make it work again on A20 and A31 (I think) where it broke when
> the ccu driver was changed to a platform driver.
> 
> I have only tested this on an A20 where the hstimer is registered as
> expected and can be selected through sysfs.  I hope it doesn't break
> something else.
> 
> Some additional discussion here:
> https://lore.kernel.org/linux-clk/20211119033338.25486-4-samuel@sholland.org/
> 
> Mans Rullgard (3):
>    clocksource: sun5i: remove duplication of code and data
>    clocksource: sun5i: remove pointless struct
>    clocksource: sun5i: convert to platform device driver
> 
>   drivers/clocksource/timer-sun5i.c | 288 +++++++++++++-----------------
>   1 file changed, 123 insertions(+), 165 deletions(-)

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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel