* Add SMP/SMT support for sysost driver.
@ 2021-06-04 16:31 周琰杰 (Zhou Yanjie)
2021-06-04 16:31 ` [PATCH v2 1/2] clocksource: Ingenic: Rename unreasonable array names 周琰杰 (Zhou Yanjie)
2021-06-04 16:31 ` [PATCH v2 2/2] clocksource: Ingenic: Add SMP/SMT support for sysost driver 周琰杰 (Zhou Yanjie)
0 siblings, 2 replies; 11+ messages in thread
From: 周琰杰 (Zhou Yanjie) @ 2021-06-04 16:31 UTC (permalink / raw)
To: daniel.lezcano, tglx
Cc: linux-mips, linux-kernel, dongsheng.qiu, aric.pzqi, rick.tyliu,
sihui.liu, jun.jiang, sernia.zhou, paul
v1->v2:
1.split changes about x1000 and array name to separate patch.
2.Fix bug in ingenic_ost_global_timer_recalc_rate().
3.Add a backpointer to the ingenic_ost structure.
4.Remove unnecessary spinlock.
5.Use "ret = ost->irq" instead "ret = -EINVAL".
6.Use "%d" instead "%x" in pr_crit().
周琰杰 (Zhou Yanjie) (2):
clocksource: Ingenic: Rename unreasonable array names.
clocksource: Ingenic: Add SMP/SMT support for sysost driver.
drivers/clocksource/ingenic-sysost.c | 323 ++++++++++++++++++++++++++---------
1 file changed, 240 insertions(+), 83 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/2] clocksource: Ingenic: Rename unreasonable array names.
2021-06-04 16:31 Add SMP/SMT support for sysost driver 周琰杰 (Zhou Yanjie)
@ 2021-06-04 16:31 ` 周琰杰 (Zhou Yanjie)
2021-06-16 13:56 ` Daniel Lezcano
2021-06-18 16:03 ` [tip: timers/core] clocksource/drivers/ingenic: " tip-bot2 for 周琰杰
2021-06-04 16:31 ` [PATCH v2 2/2] clocksource: Ingenic: Add SMP/SMT support for sysost driver 周琰杰 (Zhou Yanjie)
1 sibling, 2 replies; 11+ messages in thread
From: 周琰杰 (Zhou Yanjie) @ 2021-06-04 16:31 UTC (permalink / raw)
To: daniel.lezcano, tglx
Cc: linux-mips, linux-kernel, dongsheng.qiu, aric.pzqi, rick.tyliu,
sihui.liu, jun.jiang, sernia.zhou, paul
1.Rename the "ingenic_ost_clk_info[]" to "x1000_ost_clk_info[]"
to facilitate the addition of OST support for X2000 SoC in a
later commit.
2.When the OST support for X2000 SoC is added, there will be two
compatible strings, so renaming "ingenic_ost_of_match[]" to
"ingenic_ost_of_matches[]" is more reasonable.
3.Remove the unnecessary comma in "ingenic_ost_of_matches[]" to
reduce code size as much as possible.
Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
---
Notes:
v2:
New patch.
drivers/clocksource/ingenic-sysost.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/clocksource/ingenic-sysost.c b/drivers/clocksource/ingenic-sysost.c
index e77d584..a129840 100644
--- a/drivers/clocksource/ingenic-sysost.c
+++ b/drivers/clocksource/ingenic-sysost.c
@@ -186,7 +186,7 @@ static const struct clk_ops ingenic_ost_global_timer_ops = {
static const char * const ingenic_ost_clk_parents[] = { "ext" };
-static const struct ingenic_ost_clk_info ingenic_ost_clk_info[] = {
+static const struct ingenic_ost_clk_info x1000_ost_clk_info[] = {
[OST_CLK_PERCPU_TIMER] = {
.init_data = {
.name = "percpu timer",
@@ -414,14 +414,14 @@ static const struct ingenic_soc_info x1000_soc_info = {
.num_channels = 2,
};
-static const struct of_device_id __maybe_unused ingenic_ost_of_match[] __initconst = {
- { .compatible = "ingenic,x1000-ost", .data = &x1000_soc_info, },
+static const struct of_device_id __maybe_unused ingenic_ost_of_matches[] __initconst = {
+ { .compatible = "ingenic,x1000-ost", .data = &x1000_soc_info },
{ /* sentinel */ }
};
static int __init ingenic_ost_probe(struct device_node *np)
{
- const struct of_device_id *id = of_match_node(ingenic_ost_of_match, np);
+ const struct of_device_id *id = of_match_node(ingenic_ost_of_matches, np);
struct ingenic_ost *ost;
unsigned int i;
int ret;
@@ -462,7 +462,7 @@ static int __init ingenic_ost_probe(struct device_node *np)
ost->clocks->num = ost->soc_info->num_channels;
for (i = 0; i < ost->clocks->num; i++) {
- ret = ingenic_ost_register_clock(ost, i, &ingenic_ost_clk_info[i], ost->clocks);
+ ret = ingenic_ost_register_clock(ost, i, &x1000_ost_clk_info[i], ost->clocks);
if (ret) {
pr_crit("%s: Cannot register clock %d\n", __func__, i);
goto err_unregister_ost_clocks;
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/2] clocksource: Ingenic: Add SMP/SMT support for sysost driver.
2021-06-04 16:31 Add SMP/SMT support for sysost driver 周琰杰 (Zhou Yanjie)
2021-06-04 16:31 ` [PATCH v2 1/2] clocksource: Ingenic: Rename unreasonable array names 周琰杰 (Zhou Yanjie)
@ 2021-06-04 16:31 ` 周琰杰 (Zhou Yanjie)
2021-06-10 15:30 ` Paul Cercueil
1 sibling, 1 reply; 11+ messages in thread
From: 周琰杰 (Zhou Yanjie) @ 2021-06-04 16:31 UTC (permalink / raw)
To: daniel.lezcano, tglx
Cc: linux-mips, linux-kernel, dongsheng.qiu, aric.pzqi, rick.tyliu,
sihui.liu, jun.jiang, sernia.zhou, paul
The OST in Ingenic XBurst®2 SoCs such as X2000 and X2100, has a global
timer and two or four percpu timers, add support for the percpu timers.
Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
---
Notes:
v1->v2:
1.Fix bug in ingenic_ost_global_timer_recalc_rate().
2.Add a backpointer to the ingenic_ost structure.
3.Remove unnecessary spinlock.
4.Use "ret = ost->irq" instead "ret = -EINVAL".
5.Use "%d" instead "%x" in pr_crit().
drivers/clocksource/ingenic-sysost.c | 315 ++++++++++++++++++++++++++---------
1 file changed, 236 insertions(+), 79 deletions(-)
diff --git a/drivers/clocksource/ingenic-sysost.c b/drivers/clocksource/ingenic-sysost.c
index a129840..6f080e4 100644
--- a/drivers/clocksource/ingenic-sysost.c
+++ b/drivers/clocksource/ingenic-sysost.c
@@ -4,6 +4,7 @@
* Copyright (c) 2020 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
*/
+#include <linux/bitfield.h>
#include <linux/bitops.h>
#include <linux/clk.h>
#include <linux/clk-provider.h>
@@ -13,6 +14,8 @@
#include <linux/mfd/syscon.h>
#include <linux/of_address.h>
#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/overflow.h>
#include <linux/sched_clock.h>
#include <linux/slab.h>
#include <linux/syscore_ops.h>
@@ -21,10 +24,14 @@
/* OST register offsets */
#define OST_REG_OSTCCR 0x00
+#define OST_REG_OSTER 0x04
#define OST_REG_OSTCR 0x08
#define OST_REG_OSTFR 0x0c
+#define OST_REG_OSTCNTH 0x0c
#define OST_REG_OSTMR 0x10
+#define OST_REG_OSTCNTL 0x10
#define OST_REG_OST1DFR 0x14
+#define OST_REG_OSTCNTB 0x14
#define OST_REG_OST1CNT 0x18
#define OST_REG_OST2CNTL 0x20
#define OST_REG_OSTCNT2HBUF 0x24
@@ -55,13 +62,23 @@
#define OSTECR_OST1ENC BIT(0)
#define OSTECR_OST2ENC BIT(1)
+enum ingenic_ost_version {
+ ID_X1000,
+ ID_X2000,
+};
+
struct ingenic_soc_info {
+ enum ingenic_ost_version version;
+ const struct ingenic_ost_clk_info *clk_info;
+
unsigned int num_channels;
+ unsigned int base_offset;
};
struct ingenic_ost_clk_info {
struct clk_init_data init_data;
- u8 ostccr_reg;
+ unsigned int idx;
+ u32 ostcntl_reg;
};
struct ingenic_ost_clk {
@@ -71,15 +88,27 @@ struct ingenic_ost_clk {
const struct ingenic_ost_clk_info *info;
};
+struct ingenic_ost_timer {
+ void __iomem *base;
+ unsigned int cpu;
+ unsigned int channel;
+ struct clock_event_device cevt;
+ struct ingenic_ost *ost;
+ struct clk *clk;
+ char name[20];
+};
+
struct ingenic_ost {
void __iomem *base;
const struct ingenic_soc_info *soc_info;
- struct clk *clk, *percpu_timer_clk, *global_timer_clk;
- struct clock_event_device cevt;
+ struct clk *clk, *global_timer_clk;
+ struct device_node *np;
struct clocksource cs;
- char name[20];
struct clk_hw_onecell_data *clocks;
+ struct ingenic_ost_timer __percpu *timers;
+
+ int irq;
};
static struct ingenic_ost *ingenic_ost;
@@ -94,11 +123,12 @@ static unsigned long ingenic_ost_percpu_timer_recalc_rate(struct clk_hw *hw,
{
struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
const struct ingenic_ost_clk_info *info = ost_clk->info;
+ struct ingenic_ost_timer *timer = per_cpu_ptr(ost_clk->ost->timers, info->idx);
unsigned int prescale;
- prescale = readl(ost_clk->ost->base + info->ostccr_reg);
+ prescale = readl(timer->base + OST_REG_OSTCCR);
- prescale = (prescale & OSTCCR_PRESCALE1_MASK) >> OSTCCR_PRESCALE1_LSB;
+ prescale = FIELD_GET(OSTCCR_PRESCALE1_MASK, prescale);
return parent_rate >> (prescale * 2);
}
@@ -108,11 +138,15 @@ static unsigned long ingenic_ost_global_timer_recalc_rate(struct clk_hw *hw,
{
struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
const struct ingenic_ost_clk_info *info = ost_clk->info;
+ struct ingenic_ost_timer *timer = per_cpu_ptr(ost_clk->ost->timers, info->idx);
unsigned int prescale;
- prescale = readl(ost_clk->ost->base + info->ostccr_reg);
+ prescale = readl(timer->base + OST_REG_OSTCCR);
- prescale = (prescale & OSTCCR_PRESCALE2_MASK) >> OSTCCR_PRESCALE2_LSB;
+ if (ost_clk->ost->soc_info->version >= ID_X2000)
+ prescale = FIELD_GET(OSTCCR_PRESCALE1_MASK, prescale);
+ else
+ prescale = FIELD_GET(OSTCCR_PRESCALE2_MASK, prescale);
return parent_rate >> (prescale * 2);
}
@@ -147,12 +181,13 @@ static int ingenic_ost_percpu_timer_set_rate(struct clk_hw *hw, unsigned long re
{
struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
const struct ingenic_ost_clk_info *info = ost_clk->info;
+ struct ingenic_ost_timer *timer = per_cpu_ptr(ost_clk->ost->timers, info->idx);
u8 prescale = ingenic_ost_get_prescale(parent_rate, req_rate);
int val;
- val = readl(ost_clk->ost->base + info->ostccr_reg);
+ val = readl(timer->base + OST_REG_OSTCCR);
val = (val & ~OSTCCR_PRESCALE1_MASK) | (prescale << OSTCCR_PRESCALE1_LSB);
- writel(val, ost_clk->ost->base + info->ostccr_reg);
+ writel(val, timer->base + OST_REG_OSTCCR);
return 0;
}
@@ -162,12 +197,18 @@ static int ingenic_ost_global_timer_set_rate(struct clk_hw *hw, unsigned long re
{
struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
const struct ingenic_ost_clk_info *info = ost_clk->info;
+ struct ingenic_ost_timer *timer = per_cpu_ptr(ost_clk->ost->timers, info->idx);
u8 prescale = ingenic_ost_get_prescale(parent_rate, req_rate);
int val;
- val = readl(ost_clk->ost->base + info->ostccr_reg);
- val = (val & ~OSTCCR_PRESCALE2_MASK) | (prescale << OSTCCR_PRESCALE2_LSB);
- writel(val, ost_clk->ost->base + info->ostccr_reg);
+ val = readl(timer->base + OST_REG_OSTCCR);
+
+ if (ost_clk->ost->soc_info->version >= ID_X2000)
+ val = (val & ~OSTCCR_PRESCALE1_MASK) | (prescale << OSTCCR_PRESCALE1_LSB);
+ else
+ val = (val & ~OSTCCR_PRESCALE2_MASK) | (prescale << OSTCCR_PRESCALE2_LSB);
+
+ writel(val, timer->base + OST_REG_OSTCCR);
return 0;
}
@@ -195,7 +236,42 @@ static const struct ingenic_ost_clk_info x1000_ost_clk_info[] = {
.ops = &ingenic_ost_percpu_timer_ops,
.flags = CLK_SET_RATE_UNGATE,
},
- .ostccr_reg = OST_REG_OSTCCR,
+ .idx = 0,
+ },
+
+ [OST_CLK_GLOBAL_TIMER] = {
+ .init_data = {
+ .name = "global timer",
+ .parent_names = ingenic_ost_clk_parents,
+ .num_parents = ARRAY_SIZE(ingenic_ost_clk_parents),
+ .ops = &ingenic_ost_global_timer_ops,
+ .flags = CLK_SET_RATE_UNGATE,
+ },
+ .ostcntl_reg = OST_REG_OST2CNTL,
+ },
+};
+
+static const struct ingenic_ost_clk_info x2000_ost_clk_info[] = {
+ [OST_CLK_PERCPU_TIMER0] = {
+ .init_data = {
+ .name = "percpu timer0",
+ .parent_names = ingenic_ost_clk_parents,
+ .num_parents = ARRAY_SIZE(ingenic_ost_clk_parents),
+ .ops = &ingenic_ost_percpu_timer_ops,
+ .flags = CLK_SET_RATE_UNGATE,
+ },
+ .idx = 0,
+ },
+
+ [OST_CLK_PERCPU_TIMER1] = {
+ .init_data = {
+ .name = "percpu timer1",
+ .parent_names = ingenic_ost_clk_parents,
+ .num_parents = ARRAY_SIZE(ingenic_ost_clk_parents),
+ .ops = &ingenic_ost_percpu_timer_ops,
+ .flags = CLK_SET_RATE_UNGATE,
+ },
+ .idx = 1,
},
[OST_CLK_GLOBAL_TIMER] = {
@@ -206,7 +282,7 @@ static const struct ingenic_ost_clk_info x1000_ost_clk_info[] = {
.ops = &ingenic_ost_global_timer_ops,
.flags = CLK_SET_RATE_UNGATE,
},
- .ostccr_reg = OST_REG_OSTCCR,
+ .ostcntl_reg = OST_REG_OSTCNTL,
},
};
@@ -215,7 +291,7 @@ static u64 notrace ingenic_ost_global_timer_read_cntl(void)
struct ingenic_ost *ost = ingenic_ost;
unsigned int count;
- count = readl(ost->base + OST_REG_OST2CNTL);
+ count = readl(ost->base + ost->soc_info->clk_info->ostcntl_reg);
return count;
}
@@ -225,16 +301,21 @@ static u64 notrace ingenic_ost_clocksource_read(struct clocksource *cs)
return ingenic_ost_global_timer_read_cntl();
}
-static inline struct ingenic_ost *to_ingenic_ost(struct clock_event_device *evt)
+static inline struct ingenic_ost_timer *
+to_ingenic_ost_timer(struct clock_event_device *evt)
{
- return container_of(evt, struct ingenic_ost, cevt);
+ return container_of(evt, struct ingenic_ost_timer, cevt);
}
static int ingenic_ost_cevt_set_state_shutdown(struct clock_event_device *evt)
{
- struct ingenic_ost *ost = to_ingenic_ost(evt);
+ struct ingenic_ost_timer *timer = to_ingenic_ost_timer(evt);
+ struct ingenic_ost *ost = timer->ost;
- writel(OSTECR_OST1ENC, ost->base + OST_REG_OSTECR);
+ if (ost->soc_info->version >= ID_X2000)
+ writel(0, timer->base + OST_REG_OSTER);
+ else
+ writel(OSTECR_OST1ENC, timer->base + OST_REG_OSTECR);
return 0;
}
@@ -242,26 +323,34 @@ static int ingenic_ost_cevt_set_state_shutdown(struct clock_event_device *evt)
static int ingenic_ost_cevt_set_next(unsigned long next,
struct clock_event_device *evt)
{
- struct ingenic_ost *ost = to_ingenic_ost(evt);
-
- writel((u32)~OSTFR_FFLAG, ost->base + OST_REG_OSTFR);
- writel(next, ost->base + OST_REG_OST1DFR);
- writel(OSTCR_OST1CLR, ost->base + OST_REG_OSTCR);
- writel(OSTESR_OST1ENS, ost->base + OST_REG_OSTESR);
- writel((u32)~OSTMR_FMASK, ost->base + OST_REG_OSTMR);
+ struct ingenic_ost_timer *timer = to_ingenic_ost_timer(evt);
+ struct ingenic_ost *ost = timer->ost;
+
+ writel((u32)~OSTFR_FFLAG, timer->base + OST_REG_OSTFR);
+ writel(next, timer->base + OST_REG_OST1DFR);
+ writel(OSTCR_OST1CLR, timer->base + OST_REG_OSTCR);
+
+ if (ost->soc_info->version >= ID_X2000) {
+ writel(OSTESR_OST1ENS, timer->base + OST_REG_OSTER);
+ } else {
+ writel(OSTESR_OST1ENS, timer->base + OST_REG_OSTESR);
+ writel((u32)~OSTMR_FMASK, timer->base + OST_REG_OSTMR);
+ }
return 0;
}
static irqreturn_t ingenic_ost_cevt_cb(int irq, void *dev_id)
{
- struct clock_event_device *evt = dev_id;
- struct ingenic_ost *ost = to_ingenic_ost(evt);
+ struct ingenic_ost_timer *timer = dev_id;
+ struct ingenic_ost *ost = timer->ost;
- writel(OSTECR_OST1ENC, ost->base + OST_REG_OSTECR);
+ if (ost->soc_info->version >= ID_X2000)
+ writel(0, timer->base + OST_REG_OSTER);
+ else
+ writel(OSTECR_OST1ENC, timer->base + OST_REG_OSTECR);
- if (evt->event_handler)
- evt->event_handler(evt);
+ timer->cevt.event_handler(&timer->cevt);
return IRQ_HANDLED;
}
@@ -271,6 +360,7 @@ static int __init ingenic_ost_register_clock(struct ingenic_ost *ost,
struct clk_hw_onecell_data *clocks)
{
struct ingenic_ost_clk *ost_clk;
+ struct ingenic_ost_timer *timer = per_cpu_ptr(ost->timers, info->idx);
int val, err;
ost_clk = kzalloc(sizeof(*ost_clk), GFP_KERNEL);
@@ -283,9 +373,9 @@ static int __init ingenic_ost_register_clock(struct ingenic_ost *ost,
ost_clk->ost = ost;
/* Reset clock divider */
- val = readl(ost->base + info->ostccr_reg);
- val &= ~(OSTCCR_PRESCALE1_MASK | OSTCCR_PRESCALE2_MASK);
- writel(val, ost->base + info->ostccr_reg);
+ val = readl(timer->base + OST_REG_OSTCCR);
+ val &= ~(OSTCCR_PRESCALE1_MASK);
+ writel(val, timer->base + OST_REG_OSTCCR);
err = clk_hw_register(NULL, &ost_clk->hw);
if (err) {
@@ -309,57 +399,51 @@ static struct clk * __init ingenic_ost_get_clock(struct device_node *np, int id)
return of_clk_get_from_provider(&args);
}
-static int __init ingenic_ost_percpu_timer_init(struct device_node *np,
- struct ingenic_ost *ost)
+static int __init ingenic_ost_setup_cevt(unsigned int cpu)
{
- unsigned int timer_virq, channel = OST_CLK_PERCPU_TIMER;
+ struct ingenic_ost *ost = ingenic_ost;
+ struct ingenic_ost_timer *timer = this_cpu_ptr(ost->timers);
unsigned long rate;
int err;
- ost->percpu_timer_clk = ingenic_ost_get_clock(np, channel);
- if (IS_ERR(ost->percpu_timer_clk))
- return PTR_ERR(ost->percpu_timer_clk);
+ timer->clk = ingenic_ost_get_clock(ost->np, timer->channel);
+ if (IS_ERR(timer->clk))
+ return PTR_ERR(timer->clk);
- err = clk_prepare_enable(ost->percpu_timer_clk);
+ err = clk_prepare_enable(timer->clk);
if (err)
goto err_clk_put;
- rate = clk_get_rate(ost->percpu_timer_clk);
+ rate = clk_get_rate(timer->clk);
if (!rate) {
err = -EINVAL;
goto err_clk_disable;
}
- timer_virq = of_irq_get(np, 0);
- if (!timer_virq) {
- err = -EINVAL;
- goto err_clk_disable;
- }
+ snprintf(timer->name, sizeof(timer->name), "OST percpu timer%u", cpu);
- snprintf(ost->name, sizeof(ost->name), "OST percpu timer");
+ /* Unmask full comparison match interrupt */
+ writel((u32)~OSTMR_FMASK, timer->base + OST_REG_OSTMR);
- err = request_irq(timer_virq, ingenic_ost_cevt_cb, IRQF_TIMER,
- ost->name, &ost->cevt);
- if (err)
- goto err_irq_dispose_mapping;
+ timer->cpu = smp_processor_id();
+ timer->cevt.cpumask = cpumask_of(smp_processor_id());
+ timer->cevt.features = CLOCK_EVT_FEAT_ONESHOT;
+ timer->cevt.name = timer->name;
+ timer->cevt.rating = 400;
+ timer->cevt.set_state_shutdown = ingenic_ost_cevt_set_state_shutdown;
+ timer->cevt.set_next_event = ingenic_ost_cevt_set_next;
- ost->cevt.cpumask = cpumask_of(smp_processor_id());
- ost->cevt.features = CLOCK_EVT_FEAT_ONESHOT;
- ost->cevt.name = ost->name;
- ost->cevt.rating = 400;
- ost->cevt.set_state_shutdown = ingenic_ost_cevt_set_state_shutdown;
- ost->cevt.set_next_event = ingenic_ost_cevt_set_next;
+ clockevents_config_and_register(&timer->cevt, rate, 4, 0xffffffff);
- clockevents_config_and_register(&ost->cevt, rate, 4, 0xffffffff);
+ if (ost->soc_info->version >= ID_X2000)
+ enable_percpu_irq(ost->irq, IRQ_TYPE_NONE);
return 0;
-err_irq_dispose_mapping:
- irq_dispose_mapping(timer_virq);
err_clk_disable:
- clk_disable_unprepare(ost->percpu_timer_clk);
+ clk_disable_unprepare(timer->clk);
err_clk_put:
- clk_put(ost->percpu_timer_clk);
+ clk_put(timer->clk);
return err;
}
@@ -385,11 +469,14 @@ static int __init ingenic_ost_global_timer_init(struct device_node *np,
goto err_clk_disable;
}
- /* Clear counter CNT registers */
- writel(OSTCR_OST2CLR, ost->base + OST_REG_OSTCR);
-
- /* Enable OST channel */
- writel(OSTESR_OST2ENS, ost->base + OST_REG_OSTESR);
+ /* Clear counter CNT registers and enable OST channel */
+ if (ost->soc_info->version >= ID_X2000) {
+ writel(OSTCR_OST1CLR, ost->base + OST_REG_OSTCR);
+ writel(OSTESR_OST1ENS, ost->base + OST_REG_OSTER);
+ } else {
+ writel(OSTCR_OST2CLR, ost->base + OST_REG_OSTCR);
+ writel(OSTESR_OST2ENS, ost->base + OST_REG_OSTESR);
+ }
cs->name = "ingenic-ost";
cs->rating = 400;
@@ -411,18 +498,33 @@ static int __init ingenic_ost_global_timer_init(struct device_node *np,
}
static const struct ingenic_soc_info x1000_soc_info = {
+ .version = ID_X1000,
+ .clk_info = x1000_ost_clk_info,
+
.num_channels = 2,
};
+static const struct ingenic_soc_info x2000_soc_info = {
+ .version = ID_X2000,
+ .clk_info = x2000_ost_clk_info,
+
+ .num_channels = 3,
+ .base_offset = 0x100,
+};
+
static const struct of_device_id __maybe_unused ingenic_ost_of_matches[] __initconst = {
{ .compatible = "ingenic,x1000-ost", .data = &x1000_soc_info },
+ { .compatible = "ingenic,x2000-ost", .data = &x2000_soc_info },
{ /* sentinel */ }
};
static int __init ingenic_ost_probe(struct device_node *np)
{
const struct of_device_id *id = of_match_node(ingenic_ost_of_matches, np);
+ struct ingenic_ost_timer *timer;
struct ingenic_ost *ost;
+ void __iomem *base;
+ unsigned int cpu;
unsigned int i;
int ret;
@@ -430,18 +532,43 @@ static int __init ingenic_ost_probe(struct device_node *np)
if (!ost)
return -ENOMEM;
+ ost->timers = alloc_percpu(struct ingenic_ost_timer);
+ if (!ost->timers) {
+ ret = -ENOMEM;
+ goto err_free_ost;
+ }
+
+ ost->np = np;
+ ost->soc_info = id->data;
+
ost->base = of_io_request_and_map(np, 0, of_node_full_name(np));
if (IS_ERR(ost->base)) {
pr_err("%s: Failed to map OST registers\n", __func__);
ret = PTR_ERR(ost->base);
- goto err_free_ost;
+ goto err_free_timers;
+ }
+
+ if (ost->soc_info->version >= ID_X2000) {
+ base = of_io_request_and_map(np, 1, of_node_full_name(np));
+ if (IS_ERR(base)) {
+ pr_err("%s: Failed to map OST registers\n", __func__);
+ ret = PTR_ERR(base);
+ goto err_free_timers;
+ }
+ }
+
+ ost->irq = irq_of_parse_and_map(np, 0);
+ if (ost->irq < 0) {
+ pr_crit("%s: Cannot to get OST IRQ\n", __func__);
+ ret = ost->irq;
+ goto err_free_timers;
}
ost->clk = of_clk_get_by_name(np, "ost");
if (IS_ERR(ost->clk)) {
- ret = PTR_ERR(ost->clk);
pr_crit("%s: Cannot get OST clock\n", __func__);
- goto err_free_ost;
+ ret = PTR_ERR(ost->clk);
+ goto err_free_timers;
}
ret = clk_prepare_enable(ost->clk);
@@ -450,8 +577,6 @@ static int __init ingenic_ost_probe(struct device_node *np)
goto err_put_clk;
}
- ost->soc_info = id->data;
-
ost->clocks = kzalloc(struct_size(ost->clocks, hws, ost->soc_info->num_channels),
GFP_KERNEL);
if (!ost->clocks) {
@@ -461,8 +586,21 @@ static int __init ingenic_ost_probe(struct device_node *np)
ost->clocks->num = ost->soc_info->num_channels;
- for (i = 0; i < ost->clocks->num; i++) {
- ret = ingenic_ost_register_clock(ost, i, &x1000_ost_clk_info[i], ost->clocks);
+ for (cpu = 0; cpu < num_possible_cpus(); cpu++) {
+ timer = per_cpu_ptr(ost->timers, cpu);
+
+ if (ost->soc_info->version >= ID_X2000)
+ timer->base = base + ost->soc_info->base_offset * cpu;
+ else
+ timer->base = ost->base;
+
+ timer->ost = ost;
+ timer->cpu = cpu;
+ timer->channel = OST_CLK_PERCPU_TIMER + cpu;
+ }
+
+ for (i = 0; i < num_possible_cpus() + 1; i++) {
+ ret = ingenic_ost_register_clock(ost, i, &ost->soc_info->clk_info[i], ost->clocks);
if (ret) {
pr_crit("%s: Cannot register clock %d\n", __func__, i);
goto err_unregister_ost_clocks;
@@ -488,6 +626,8 @@ static int __init ingenic_ost_probe(struct device_node *np)
clk_disable_unprepare(ost->clk);
err_put_clk:
clk_put(ost->clk);
+err_free_timers:
+ free_percpu(ost->timers);
err_free_ost:
kfree(ost);
return ret;
@@ -513,13 +653,29 @@ static int __init ingenic_ost_init(struct device_node *np)
ret = ingenic_ost_global_timer_init(np, ost);
if (ret) {
- pr_crit("%s: Unable to init global timer: %x\n", __func__, ret);
+ pr_crit("%s: Unable to init global timer: %d\n", __func__, ret);
goto err_free_ingenic_ost;
}
- ret = ingenic_ost_percpu_timer_init(np, ost);
- if (ret)
+ if (ost->soc_info->version >= ID_X2000)
+ ret = request_percpu_irq(ost->irq, ingenic_ost_cevt_cb,
+ "OST percpu timer", ost->timers);
+ else
+ ret = request_irq(ost->irq, ingenic_ost_cevt_cb, IRQF_TIMER,
+ "OST percpu timer", ost->timers);
+
+ if (ret) {
+ pr_crit("%s: Unable to request percpu IRQ: %d\n", __func__, ret);
+ goto err_ost_global_timer_cleanup;
+ }
+
+ /* Setup clock events on each CPU core */
+ ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "Ingenic XBurst: online",
+ ingenic_ost_setup_cevt, NULL);
+ if (ret < 0) {
+ pr_crit("%s: Unable to init percpu timers: %d\n", __func__, ret);
goto err_ost_global_timer_cleanup;
+ }
/* Register the sched_clock at the end as there's no way to undo it */
rate = clk_get_rate(ost->global_timer_clk);
@@ -537,3 +693,4 @@ static int __init ingenic_ost_init(struct device_node *np)
}
TIMER_OF_DECLARE(x1000_ost, "ingenic,x1000-ost", ingenic_ost_init);
+TIMER_OF_DECLARE(x2000_ost, "ingenic,x2000-ost", ingenic_ost_init);
--
2.7.4
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] clocksource: Ingenic: Add SMP/SMT support for sysost driver.
2021-06-04 16:31 ` [PATCH v2 2/2] clocksource: Ingenic: Add SMP/SMT support for sysost driver 周琰杰 (Zhou Yanjie)
@ 2021-06-10 15:30 ` Paul Cercueil
2021-06-11 15:31 ` Zhou Yanjie
0 siblings, 1 reply; 11+ messages in thread
From: Paul Cercueil @ 2021-06-10 15:30 UTC (permalink / raw)
To: 周琰杰
Cc: daniel.lezcano, tglx, linux-mips, linux-kernel, dongsheng.qiu,
aric.pzqi, rick.tyliu, sihui.liu, jun.jiang, sernia.zhou
Hi Zhou,
Le sam., juin 5 2021 at 00:31:46 +0800, 周琰杰 (Zhou Yanjie)
<zhouyanjie@wanyeetech.com> a écrit :
> The OST in Ingenic XBurst®2 SoCs such as X2000 and X2100, has a
> global
> timer and two or four percpu timers, add support for the percpu
> timers.
>
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> ---
>
> Notes:
> v1->v2:
> 1.Fix bug in ingenic_ost_global_timer_recalc_rate().
> 2.Add a backpointer to the ingenic_ost structure.
> 3.Remove unnecessary spinlock.
> 4.Use "ret = ost->irq" instead "ret = -EINVAL".
> 5.Use "%d" instead "%x" in pr_crit().
I can't shake the feeling that you are doing way too many things in one
single commit.
From what I can see, this commit can be split in 4 patches:
- Fix the "%x" in pr_crit(),
- Add the global timer support to the X1000,
- Add "ingenic_ost_timer" and update the code to use it,
- Finally add X2000 support.
> drivers/clocksource/ingenic-sysost.c | 315
> ++++++++++++++++++++++++++---------
> 1 file changed, 236 insertions(+), 79 deletions(-)
>
> diff --git a/drivers/clocksource/ingenic-sysost.c
> b/drivers/clocksource/ingenic-sysost.c
> index a129840..6f080e4 100644
> --- a/drivers/clocksource/ingenic-sysost.c
> +++ b/drivers/clocksource/ingenic-sysost.c
> @@ -4,6 +4,7 @@
> * Copyright (c) 2020 周琰杰 (Zhou Yanjie)
> <zhouyanjie@wanyeetech.com>
> */
>
> +#include <linux/bitfield.h>
> #include <linux/bitops.h>
> #include <linux/clk.h>
> #include <linux/clk-provider.h>
> @@ -13,6 +14,8 @@
> #include <linux/mfd/syscon.h>
> #include <linux/of_address.h>
> #include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/overflow.h>
> #include <linux/sched_clock.h>
> #include <linux/slab.h>
> #include <linux/syscore_ops.h>
> @@ -21,10 +24,14 @@
>
> /* OST register offsets */
> #define OST_REG_OSTCCR 0x00
> +#define OST_REG_OSTER 0x04
> #define OST_REG_OSTCR 0x08
> #define OST_REG_OSTFR 0x0c
> +#define OST_REG_OSTCNTH 0x0c
> #define OST_REG_OSTMR 0x10
> +#define OST_REG_OSTCNTL 0x10
> #define OST_REG_OST1DFR 0x14
> +#define OST_REG_OSTCNTB 0x14
> #define OST_REG_OST1CNT 0x18
> #define OST_REG_OST2CNTL 0x20
> #define OST_REG_OSTCNT2HBUF 0x24
> @@ -55,13 +62,23 @@
> #define OSTECR_OST1ENC BIT(0)
> #define OSTECR_OST2ENC BIT(1)
>
> +enum ingenic_ost_version {
> + ID_X1000,
> + ID_X2000,
> +};
> +
> struct ingenic_soc_info {
> + enum ingenic_ost_version version;
> + const struct ingenic_ost_clk_info *clk_info;
> +
> unsigned int num_channels;
> + unsigned int base_offset;
> };
>
> struct ingenic_ost_clk_info {
> struct clk_init_data init_data;
> - u8 ostccr_reg;
> + unsigned int idx;
> + u32 ostcntl_reg;
> };
>
> struct ingenic_ost_clk {
> @@ -71,15 +88,27 @@ struct ingenic_ost_clk {
> const struct ingenic_ost_clk_info *info;
> };
>
> +struct ingenic_ost_timer {
> + void __iomem *base;
> + unsigned int cpu;
> + unsigned int channel;
> + struct clock_event_device cevt;
> + struct ingenic_ost *ost;
> + struct clk *clk;
> + char name[20];
> +};
> +
> struct ingenic_ost {
> void __iomem *base;
> const struct ingenic_soc_info *soc_info;
> - struct clk *clk, *percpu_timer_clk, *global_timer_clk;
> - struct clock_event_device cevt;
> + struct clk *clk, *global_timer_clk;
> + struct device_node *np;
> struct clocksource cs;
> - char name[20];
>
> struct clk_hw_onecell_data *clocks;
> + struct ingenic_ost_timer __percpu *timers;
> +
> + int irq;
> };
>
> static struct ingenic_ost *ingenic_ost;
> @@ -94,11 +123,12 @@ static unsigned long
> ingenic_ost_percpu_timer_recalc_rate(struct clk_hw *hw,
> {
> struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
> const struct ingenic_ost_clk_info *info = ost_clk->info;
> + struct ingenic_ost_timer *timer = per_cpu_ptr(ost_clk->ost->timers,
> info->idx);
> unsigned int prescale;
>
> - prescale = readl(ost_clk->ost->base + info->ostccr_reg);
> + prescale = readl(timer->base + OST_REG_OSTCCR);
>
> - prescale = (prescale & OSTCCR_PRESCALE1_MASK) >>
> OSTCCR_PRESCALE1_LSB;
> + prescale = FIELD_GET(OSTCCR_PRESCALE1_MASK, prescale);
>
> return parent_rate >> (prescale * 2);
> }
> @@ -108,11 +138,15 @@ static unsigned long
> ingenic_ost_global_timer_recalc_rate(struct clk_hw *hw,
> {
> struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
> const struct ingenic_ost_clk_info *info = ost_clk->info;
> + struct ingenic_ost_timer *timer = per_cpu_ptr(ost_clk->ost->timers,
> info->idx);
> unsigned int prescale;
>
> - prescale = readl(ost_clk->ost->base + info->ostccr_reg);
> + prescale = readl(timer->base + OST_REG_OSTCCR);
>
> - prescale = (prescale & OSTCCR_PRESCALE2_MASK) >>
> OSTCCR_PRESCALE2_LSB;
> + if (ost_clk->ost->soc_info->version >= ID_X2000)
> + prescale = FIELD_GET(OSTCCR_PRESCALE1_MASK, prescale);
> + else
> + prescale = FIELD_GET(OSTCCR_PRESCALE2_MASK, prescale);
>
> return parent_rate >> (prescale * 2);
> }
> @@ -147,12 +181,13 @@ static int
> ingenic_ost_percpu_timer_set_rate(struct clk_hw *hw, unsigned long re
> {
> struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
> const struct ingenic_ost_clk_info *info = ost_clk->info;
> + struct ingenic_ost_timer *timer = per_cpu_ptr(ost_clk->ost->timers,
> info->idx);
> u8 prescale = ingenic_ost_get_prescale(parent_rate, req_rate);
> int val;
>
> - val = readl(ost_clk->ost->base + info->ostccr_reg);
> + val = readl(timer->base + OST_REG_OSTCCR);
> val = (val & ~OSTCCR_PRESCALE1_MASK) | (prescale <<
> OSTCCR_PRESCALE1_LSB);
> - writel(val, ost_clk->ost->base + info->ostccr_reg);
> + writel(val, timer->base + OST_REG_OSTCCR);
>
> return 0;
> }
> @@ -162,12 +197,18 @@ static int
> ingenic_ost_global_timer_set_rate(struct clk_hw *hw, unsigned long re
> {
> struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
> const struct ingenic_ost_clk_info *info = ost_clk->info;
> + struct ingenic_ost_timer *timer = per_cpu_ptr(ost_clk->ost->timers,
> info->idx);
> u8 prescale = ingenic_ost_get_prescale(parent_rate, req_rate);
> int val;
>
> - val = readl(ost_clk->ost->base + info->ostccr_reg);
> - val = (val & ~OSTCCR_PRESCALE2_MASK) | (prescale <<
> OSTCCR_PRESCALE2_LSB);
> - writel(val, ost_clk->ost->base + info->ostccr_reg);
> + val = readl(timer->base + OST_REG_OSTCCR);
> +
> + if (ost_clk->ost->soc_info->version >= ID_X2000)
> + val = (val & ~OSTCCR_PRESCALE1_MASK) | (prescale <<
> OSTCCR_PRESCALE1_LSB);
> + else
> + val = (val & ~OSTCCR_PRESCALE2_MASK) | (prescale <<
> OSTCCR_PRESCALE2_LSB);
> +
> + writel(val, timer->base + OST_REG_OSTCCR);
>
> return 0;
> }
> @@ -195,7 +236,42 @@ static const struct ingenic_ost_clk_info
> x1000_ost_clk_info[] = {
> .ops = &ingenic_ost_percpu_timer_ops,
> .flags = CLK_SET_RATE_UNGATE,
> },
> - .ostccr_reg = OST_REG_OSTCCR,
> + .idx = 0,
> + },
> +
> + [OST_CLK_GLOBAL_TIMER] = {
> + .init_data = {
> + .name = "global timer",
> + .parent_names = ingenic_ost_clk_parents,
> + .num_parents = ARRAY_SIZE(ingenic_ost_clk_parents),
> + .ops = &ingenic_ost_global_timer_ops,
> + .flags = CLK_SET_RATE_UNGATE,
> + },
> + .ostcntl_reg = OST_REG_OST2CNTL,
> + },
> +};
> +
> +static const struct ingenic_ost_clk_info x2000_ost_clk_info[] = {
> + [OST_CLK_PERCPU_TIMER0] = {
> + .init_data = {
> + .name = "percpu timer0",
> + .parent_names = ingenic_ost_clk_parents,
> + .num_parents = ARRAY_SIZE(ingenic_ost_clk_parents),
> + .ops = &ingenic_ost_percpu_timer_ops,
> + .flags = CLK_SET_RATE_UNGATE,
> + },
> + .idx = 0,
> + },
> +
> + [OST_CLK_PERCPU_TIMER1] = {
> + .init_data = {
> + .name = "percpu timer1",
> + .parent_names = ingenic_ost_clk_parents,
> + .num_parents = ARRAY_SIZE(ingenic_ost_clk_parents),
> + .ops = &ingenic_ost_percpu_timer_ops,
> + .flags = CLK_SET_RATE_UNGATE,
> + },
> + .idx = 1,
> },
>
> [OST_CLK_GLOBAL_TIMER] = {
> @@ -206,7 +282,7 @@ static const struct ingenic_ost_clk_info
> x1000_ost_clk_info[] = {
> .ops = &ingenic_ost_global_timer_ops,
> .flags = CLK_SET_RATE_UNGATE,
> },
> - .ostccr_reg = OST_REG_OSTCCR,
> + .ostcntl_reg = OST_REG_OSTCNTL,
> },
> };
>
> @@ -215,7 +291,7 @@ static u64 notrace
> ingenic_ost_global_timer_read_cntl(void)
> struct ingenic_ost *ost = ingenic_ost;
> unsigned int count;
>
> - count = readl(ost->base + OST_REG_OST2CNTL);
> + count = readl(ost->base + ost->soc_info->clk_info->ostcntl_reg);
>
> return count;
> }
> @@ -225,16 +301,21 @@ static u64 notrace
> ingenic_ost_clocksource_read(struct clocksource *cs)
> return ingenic_ost_global_timer_read_cntl();
> }
>
> -static inline struct ingenic_ost *to_ingenic_ost(struct
> clock_event_device *evt)
> +static inline struct ingenic_ost_timer *
> +to_ingenic_ost_timer(struct clock_event_device *evt)
> {
> - return container_of(evt, struct ingenic_ost, cevt);
> + return container_of(evt, struct ingenic_ost_timer, cevt);
> }
>
> static int ingenic_ost_cevt_set_state_shutdown(struct
> clock_event_device *evt)
> {
> - struct ingenic_ost *ost = to_ingenic_ost(evt);
> + struct ingenic_ost_timer *timer = to_ingenic_ost_timer(evt);
> + struct ingenic_ost *ost = timer->ost;
>
> - writel(OSTECR_OST1ENC, ost->base + OST_REG_OSTECR);
> + if (ost->soc_info->version >= ID_X2000)
> + writel(0, timer->base + OST_REG_OSTER);
> + else
> + writel(OSTECR_OST1ENC, timer->base + OST_REG_OSTECR);
>
> return 0;
> }
> @@ -242,26 +323,34 @@ static int
> ingenic_ost_cevt_set_state_shutdown(struct clock_event_device *evt)
> static int ingenic_ost_cevt_set_next(unsigned long next,
> struct clock_event_device *evt)
> {
> - struct ingenic_ost *ost = to_ingenic_ost(evt);
> -
> - writel((u32)~OSTFR_FFLAG, ost->base + OST_REG_OSTFR);
> - writel(next, ost->base + OST_REG_OST1DFR);
> - writel(OSTCR_OST1CLR, ost->base + OST_REG_OSTCR);
> - writel(OSTESR_OST1ENS, ost->base + OST_REG_OSTESR);
> - writel((u32)~OSTMR_FMASK, ost->base + OST_REG_OSTMR);
> + struct ingenic_ost_timer *timer = to_ingenic_ost_timer(evt);
> + struct ingenic_ost *ost = timer->ost;
> +
> + writel((u32)~OSTFR_FFLAG, timer->base + OST_REG_OSTFR);
> + writel(next, timer->base + OST_REG_OST1DFR);
> + writel(OSTCR_OST1CLR, timer->base + OST_REG_OSTCR);
> +
> + if (ost->soc_info->version >= ID_X2000) {
> + writel(OSTESR_OST1ENS, timer->base + OST_REG_OSTER);
> + } else {
> + writel(OSTESR_OST1ENS, timer->base + OST_REG_OSTESR);
> + writel((u32)~OSTMR_FMASK, timer->base + OST_REG_OSTMR);
> + }
>
> return 0;
> }
>
> static irqreturn_t ingenic_ost_cevt_cb(int irq, void *dev_id)
> {
> - struct clock_event_device *evt = dev_id;
> - struct ingenic_ost *ost = to_ingenic_ost(evt);
> + struct ingenic_ost_timer *timer = dev_id;
> + struct ingenic_ost *ost = timer->ost;
>
> - writel(OSTECR_OST1ENC, ost->base + OST_REG_OSTECR);
> + if (ost->soc_info->version >= ID_X2000)
> + writel(0, timer->base + OST_REG_OSTER);
> + else
> + writel(OSTECR_OST1ENC, timer->base + OST_REG_OSTECR);
>
> - if (evt->event_handler)
> - evt->event_handler(evt);
> + timer->cevt.event_handler(&timer->cevt);
>
> return IRQ_HANDLED;
> }
> @@ -271,6 +360,7 @@ static int __init
> ingenic_ost_register_clock(struct ingenic_ost *ost,
> struct clk_hw_onecell_data *clocks)
> {
> struct ingenic_ost_clk *ost_clk;
> + struct ingenic_ost_timer *timer = per_cpu_ptr(ost->timers,
> info->idx);
> int val, err;
>
> ost_clk = kzalloc(sizeof(*ost_clk), GFP_KERNEL);
> @@ -283,9 +373,9 @@ static int __init
> ingenic_ost_register_clock(struct ingenic_ost *ost,
> ost_clk->ost = ost;
>
> /* Reset clock divider */
> - val = readl(ost->base + info->ostccr_reg);
> - val &= ~(OSTCCR_PRESCALE1_MASK | OSTCCR_PRESCALE2_MASK);
> - writel(val, ost->base + info->ostccr_reg);
> + val = readl(timer->base + OST_REG_OSTCCR);
> + val &= ~(OSTCCR_PRESCALE1_MASK);
> + writel(val, timer->base + OST_REG_OSTCCR);
>
> err = clk_hw_register(NULL, &ost_clk->hw);
> if (err) {
> @@ -309,57 +399,51 @@ static struct clk * __init
> ingenic_ost_get_clock(struct device_node *np, int id)
> return of_clk_get_from_provider(&args);
> }
>
> -static int __init ingenic_ost_percpu_timer_init(struct device_node
> *np,
> - struct ingenic_ost *ost)
> +static int __init ingenic_ost_setup_cevt(unsigned int cpu)
> {
> - unsigned int timer_virq, channel = OST_CLK_PERCPU_TIMER;
> + struct ingenic_ost *ost = ingenic_ost;
> + struct ingenic_ost_timer *timer = this_cpu_ptr(ost->timers);
> unsigned long rate;
> int err;
>
> - ost->percpu_timer_clk = ingenic_ost_get_clock(np, channel);
> - if (IS_ERR(ost->percpu_timer_clk))
> - return PTR_ERR(ost->percpu_timer_clk);
> + timer->clk = ingenic_ost_get_clock(ost->np, timer->channel);
> + if (IS_ERR(timer->clk))
> + return PTR_ERR(timer->clk);
>
> - err = clk_prepare_enable(ost->percpu_timer_clk);
> + err = clk_prepare_enable(timer->clk);
> if (err)
> goto err_clk_put;
>
> - rate = clk_get_rate(ost->percpu_timer_clk);
> + rate = clk_get_rate(timer->clk);
> if (!rate) {
> err = -EINVAL;
> goto err_clk_disable;
> }
>
> - timer_virq = of_irq_get(np, 0);
> - if (!timer_virq) {
> - err = -EINVAL;
> - goto err_clk_disable;
> - }
> + snprintf(timer->name, sizeof(timer->name), "OST percpu timer%u",
> cpu);
>
> - snprintf(ost->name, sizeof(ost->name), "OST percpu timer");
> + /* Unmask full comparison match interrupt */
> + writel((u32)~OSTMR_FMASK, timer->base + OST_REG_OSTMR);
>
> - err = request_irq(timer_virq, ingenic_ost_cevt_cb, IRQF_TIMER,
> - ost->name, &ost->cevt);
> - if (err)
> - goto err_irq_dispose_mapping;
> + timer->cpu = smp_processor_id();
> + timer->cevt.cpumask = cpumask_of(smp_processor_id());
> + timer->cevt.features = CLOCK_EVT_FEAT_ONESHOT;
> + timer->cevt.name = timer->name;
> + timer->cevt.rating = 400;
> + timer->cevt.set_state_shutdown =
> ingenic_ost_cevt_set_state_shutdown;
> + timer->cevt.set_next_event = ingenic_ost_cevt_set_next;
>
> - ost->cevt.cpumask = cpumask_of(smp_processor_id());
> - ost->cevt.features = CLOCK_EVT_FEAT_ONESHOT;
> - ost->cevt.name = ost->name;
> - ost->cevt.rating = 400;
> - ost->cevt.set_state_shutdown = ingenic_ost_cevt_set_state_shutdown;
> - ost->cevt.set_next_event = ingenic_ost_cevt_set_next;
> + clockevents_config_and_register(&timer->cevt, rate, 4, 0xffffffff);
>
> - clockevents_config_and_register(&ost->cevt, rate, 4, 0xffffffff);
> + if (ost->soc_info->version >= ID_X2000)
> + enable_percpu_irq(ost->irq, IRQ_TYPE_NONE);
>
> return 0;
>
> -err_irq_dispose_mapping:
> - irq_dispose_mapping(timer_virq);
> err_clk_disable:
> - clk_disable_unprepare(ost->percpu_timer_clk);
> + clk_disable_unprepare(timer->clk);
> err_clk_put:
> - clk_put(ost->percpu_timer_clk);
> + clk_put(timer->clk);
> return err;
> }
>
> @@ -385,11 +469,14 @@ static int __init
> ingenic_ost_global_timer_init(struct device_node *np,
> goto err_clk_disable;
> }
>
> - /* Clear counter CNT registers */
> - writel(OSTCR_OST2CLR, ost->base + OST_REG_OSTCR);
> -
> - /* Enable OST channel */
> - writel(OSTESR_OST2ENS, ost->base + OST_REG_OSTESR);
> + /* Clear counter CNT registers and enable OST channel */
> + if (ost->soc_info->version >= ID_X2000) {
> + writel(OSTCR_OST1CLR, ost->base + OST_REG_OSTCR);
> + writel(OSTESR_OST1ENS, ost->base + OST_REG_OSTER);
> + } else {
> + writel(OSTCR_OST2CLR, ost->base + OST_REG_OSTCR);
> + writel(OSTESR_OST2ENS, ost->base + OST_REG_OSTESR);
> + }
>
> cs->name = "ingenic-ost";
> cs->rating = 400;
> @@ -411,18 +498,33 @@ static int __init
> ingenic_ost_global_timer_init(struct device_node *np,
> }
>
> static const struct ingenic_soc_info x1000_soc_info = {
> + .version = ID_X1000,
> + .clk_info = x1000_ost_clk_info,
> +
> .num_channels = 2,
> };
>
> +static const struct ingenic_soc_info x2000_soc_info = {
> + .version = ID_X2000,
> + .clk_info = x2000_ost_clk_info,
> +
> + .num_channels = 3,
> + .base_offset = 0x100,
> +};
> +
> static const struct of_device_id __maybe_unused
> ingenic_ost_of_matches[] __initconst = {
> { .compatible = "ingenic,x1000-ost", .data = &x1000_soc_info },
> + { .compatible = "ingenic,x2000-ost", .data = &x2000_soc_info },
> { /* sentinel */ }
> };
>
> static int __init ingenic_ost_probe(struct device_node *np)
> {
> const struct of_device_id *id =
> of_match_node(ingenic_ost_of_matches, np);
> + struct ingenic_ost_timer *timer;
> struct ingenic_ost *ost;
> + void __iomem *base;
> + unsigned int cpu;
> unsigned int i;
> int ret;
>
> @@ -430,18 +532,43 @@ static int __init ingenic_ost_probe(struct
> device_node *np)
> if (!ost)
> return -ENOMEM;
>
> + ost->timers = alloc_percpu(struct ingenic_ost_timer);
> + if (!ost->timers) {
> + ret = -ENOMEM;
> + goto err_free_ost;
> + }
> +
> + ost->np = np;
> + ost->soc_info = id->data;
> +
> ost->base = of_io_request_and_map(np, 0, of_node_full_name(np));
> if (IS_ERR(ost->base)) {
> pr_err("%s: Failed to map OST registers\n", __func__);
> ret = PTR_ERR(ost->base);
> - goto err_free_ost;
> + goto err_free_timers;
> + }
> +
> + if (ost->soc_info->version >= ID_X2000) {
> + base = of_io_request_and_map(np, 1, of_node_full_name(np));
> + if (IS_ERR(base)) {
> + pr_err("%s: Failed to map OST registers\n", __func__);
> + ret = PTR_ERR(base);
> + goto err_free_timers;
> + }
> + }
The DT documentation only mentions one memory resource. Here, you map a
second one, which is not used anywhere. I'm really confused about what
you're trying to do here.
> +
> + ost->irq = irq_of_parse_and_map(np, 0);
> + if (ost->irq < 0) {
> + pr_crit("%s: Cannot to get OST IRQ\n", __func__);
> + ret = ost->irq;
> + goto err_free_timers;
> }
>
> ost->clk = of_clk_get_by_name(np, "ost");
> if (IS_ERR(ost->clk)) {
> - ret = PTR_ERR(ost->clk);
> pr_crit("%s: Cannot get OST clock\n", __func__);
> - goto err_free_ost;
> + ret = PTR_ERR(ost->clk);
> + goto err_free_timers;
> }
>
> ret = clk_prepare_enable(ost->clk);
> @@ -450,8 +577,6 @@ static int __init ingenic_ost_probe(struct
> device_node *np)
> goto err_put_clk;
> }
>
> - ost->soc_info = id->data;
> -
> ost->clocks = kzalloc(struct_size(ost->clocks, hws,
> ost->soc_info->num_channels),
> GFP_KERNEL);
> if (!ost->clocks) {
> @@ -461,8 +586,21 @@ static int __init ingenic_ost_probe(struct
> device_node *np)
>
> ost->clocks->num = ost->soc_info->num_channels;
>
> - for (i = 0; i < ost->clocks->num; i++) {
> - ret = ingenic_ost_register_clock(ost, i, &x1000_ost_clk_info[i],
> ost->clocks);
> + for (cpu = 0; cpu < num_possible_cpus(); cpu++) {
> + timer = per_cpu_ptr(ost->timers, cpu);
> +
> + if (ost->soc_info->version >= ID_X2000)
> + timer->base = base + ost->soc_info->base_offset * cpu;
> + else
> + timer->base = ost->base;
> +
> + timer->ost = ost;
> + timer->cpu = cpu;
> + timer->channel = OST_CLK_PERCPU_TIMER + cpu;
> + }
> +
> + for (i = 0; i < num_possible_cpus() + 1; i++) {
> + ret = ingenic_ost_register_clock(ost, i,
> &ost->soc_info->clk_info[i], ost->clocks);
> if (ret) {
> pr_crit("%s: Cannot register clock %d\n", __func__, i);
> goto err_unregister_ost_clocks;
> @@ -488,6 +626,8 @@ static int __init ingenic_ost_probe(struct
> device_node *np)
> clk_disable_unprepare(ost->clk);
> err_put_clk:
> clk_put(ost->clk);
> +err_free_timers:
> + free_percpu(ost->timers);
> err_free_ost:
> kfree(ost);
> return ret;
> @@ -513,13 +653,29 @@ static int __init ingenic_ost_init(struct
> device_node *np)
>
> ret = ingenic_ost_global_timer_init(np, ost);
> if (ret) {
> - pr_crit("%s: Unable to init global timer: %x\n", __func__, ret);
> + pr_crit("%s: Unable to init global timer: %d\n", __func__, ret);
This is a fix, so it needs to be a separate commit with a Fixes: tag.
Cheers,
-Paul
> goto err_free_ingenic_ost;
> }
>
> - ret = ingenic_ost_percpu_timer_init(np, ost);
> - if (ret)
> + if (ost->soc_info->version >= ID_X2000)
> + ret = request_percpu_irq(ost->irq, ingenic_ost_cevt_cb,
> + "OST percpu timer", ost->timers);
> + else
> + ret = request_irq(ost->irq, ingenic_ost_cevt_cb, IRQF_TIMER,
> + "OST percpu timer", ost->timers);
> +
> + if (ret) {
> + pr_crit("%s: Unable to request percpu IRQ: %d\n", __func__, ret);
> + goto err_ost_global_timer_cleanup;
> + }
> +
> + /* Setup clock events on each CPU core */
> + ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "Ingenic XBurst:
> online",
> + ingenic_ost_setup_cevt, NULL);
> + if (ret < 0) {
> + pr_crit("%s: Unable to init percpu timers: %d\n", __func__, ret);
> goto err_ost_global_timer_cleanup;
> + }
>
> /* Register the sched_clock at the end as there's no way to undo it
> */
> rate = clk_get_rate(ost->global_timer_clk);
> @@ -537,3 +693,4 @@ static int __init ingenic_ost_init(struct
> device_node *np)
> }
>
> TIMER_OF_DECLARE(x1000_ost, "ingenic,x1000-ost", ingenic_ost_init);
> +TIMER_OF_DECLARE(x2000_ost, "ingenic,x2000-ost", ingenic_ost_init);
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] clocksource: Ingenic: Add SMP/SMT support for sysost driver.
2021-06-10 15:30 ` Paul Cercueil
@ 2021-06-11 15:31 ` Zhou Yanjie
2021-06-11 15:44 ` Paul Cercueil
0 siblings, 1 reply; 11+ messages in thread
From: Zhou Yanjie @ 2021-06-11 15:31 UTC (permalink / raw)
To: Paul Cercueil, 周琰杰
Cc: daniel.lezcano, tglx, linux-mips, linux-kernel, dongsheng.qiu,
aric.pzqi, rick.tyliu, sihui.liu, jun.jiang, sernia.zhou
Hi Paul,
On 2021/6/10 下午11:30, Paul Cercueil wrote:
> Hi Zhou,
>
> Le sam., juin 5 2021 at 00:31:46 +0800, 周琰杰 (Zhou Yanjie)
> <zhouyanjie@wanyeetech.com> a écrit :
>> The OST in Ingenic XBurst®2 SoCs such as X2000 and X2100, has a global
>> timer and two or four percpu timers, add support for the percpu timers.
>>
>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>> ---
>>
>> Notes:
>> v1->v2:
>> 1.Fix bug in ingenic_ost_global_timer_recalc_rate().
>> 2.Add a backpointer to the ingenic_ost structure.
>> 3.Remove unnecessary spinlock.
>> 4.Use "ret = ost->irq" instead "ret = -EINVAL".
>> 5.Use "%d" instead "%x" in pr_crit().
>
> I can't shake the feeling that you are doing way too many things in
> one single commit.
>
> From what I can see, this commit can be split in 4 patches:
>
> - Fix the "%x" in pr_crit(),
> - Add the global timer support to the X1000,
> - Add "ingenic_ost_timer" and update the code to use it,
> - Finally add X2000 support.
>
>
Sure.
>
>> drivers/clocksource/ingenic-sysost.c | 315
>> ++++++++++++++++++++++++++---------
>> 1 file changed, 236 insertions(+), 79 deletions(-)
>>
>> diff --git a/drivers/clocksource/ingenic-sysost.c
>> b/drivers/clocksource/ingenic-sysost.c
>> index a129840..6f080e4 100644
>> --- a/drivers/clocksource/ingenic-sysost.c
>> +++ b/drivers/clocksource/ingenic-sysost.c
>> @@ -4,6 +4,7 @@
>> * Copyright (c) 2020 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>> */
>>
>> +#include <linux/bitfield.h>
>> #include <linux/bitops.h>
>> #include <linux/clk.h>
>> #include <linux/clk-provider.h>
>> @@ -13,6 +14,8 @@
>> #include <linux/mfd/syscon.h>
>> #include <linux/of_address.h>
>> #include <linux/of_irq.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/overflow.h>
>> #include <linux/sched_clock.h>
>> #include <linux/slab.h>
>> #include <linux/syscore_ops.h>
>> @@ -21,10 +24,14 @@
>>
>> /* OST register offsets */
>> #define OST_REG_OSTCCR 0x00
>> +#define OST_REG_OSTER 0x04
>> #define OST_REG_OSTCR 0x08
>> #define OST_REG_OSTFR 0x0c
>> +#define OST_REG_OSTCNTH 0x0c
>> #define OST_REG_OSTMR 0x10
>> +#define OST_REG_OSTCNTL 0x10
>> #define OST_REG_OST1DFR 0x14
>> +#define OST_REG_OSTCNTB 0x14
>> #define OST_REG_OST1CNT 0x18
>> #define OST_REG_OST2CNTL 0x20
>> #define OST_REG_OSTCNT2HBUF 0x24
>> @@ -55,13 +62,23 @@
>> #define OSTECR_OST1ENC BIT(0)
>> #define OSTECR_OST2ENC BIT(1)
>>
>> +enum ingenic_ost_version {
>> + ID_X1000,
>> + ID_X2000,
>> +};
>> +
>> struct ingenic_soc_info {
>> + enum ingenic_ost_version version;
>> + const struct ingenic_ost_clk_info *clk_info;
>> +
>> unsigned int num_channels;
>> + unsigned int base_offset;
>> };
>>
>> struct ingenic_ost_clk_info {
>> struct clk_init_data init_data;
>> - u8 ostccr_reg;
>> + unsigned int idx;
>> + u32 ostcntl_reg;
>> };
>>
>> struct ingenic_ost_clk {
>> @@ -71,15 +88,27 @@ struct ingenic_ost_clk {
>> const struct ingenic_ost_clk_info *info;
>> };
>>
>> +struct ingenic_ost_timer {
>> + void __iomem *base;
>> + unsigned int cpu;
>> + unsigned int channel;
>> + struct clock_event_device cevt;
>> + struct ingenic_ost *ost;
>> + struct clk *clk;
>> + char name[20];
>> +};
>> +
>> struct ingenic_ost {
>> void __iomem *base;
>> const struct ingenic_soc_info *soc_info;
>> - struct clk *clk, *percpu_timer_clk, *global_timer_clk;
>> - struct clock_event_device cevt;
>> + struct clk *clk, *global_timer_clk;
>> + struct device_node *np;
>> struct clocksource cs;
>> - char name[20];
>>
>> struct clk_hw_onecell_data *clocks;
>> + struct ingenic_ost_timer __percpu *timers;
>> +
>> + int irq;
>> };
>>
>> static struct ingenic_ost *ingenic_ost;
>> @@ -94,11 +123,12 @@ static unsigned long
>> ingenic_ost_percpu_timer_recalc_rate(struct clk_hw *hw,
>> {
>> struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
>> const struct ingenic_ost_clk_info *info = ost_clk->info;
>> + struct ingenic_ost_timer *timer =
>> per_cpu_ptr(ost_clk->ost->timers, info->idx);
>> unsigned int prescale;
>>
>> - prescale = readl(ost_clk->ost->base + info->ostccr_reg);
>> + prescale = readl(timer->base + OST_REG_OSTCCR);
>>
>> - prescale = (prescale & OSTCCR_PRESCALE1_MASK) >>
>> OSTCCR_PRESCALE1_LSB;
>> + prescale = FIELD_GET(OSTCCR_PRESCALE1_MASK, prescale);
>>
>> return parent_rate >> (prescale * 2);
>> }
>> @@ -108,11 +138,15 @@ static unsigned long
>> ingenic_ost_global_timer_recalc_rate(struct clk_hw *hw,
>> {
>> struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
>> const struct ingenic_ost_clk_info *info = ost_clk->info;
>> + struct ingenic_ost_timer *timer =
>> per_cpu_ptr(ost_clk->ost->timers, info->idx);
>> unsigned int prescale;
>>
>> - prescale = readl(ost_clk->ost->base + info->ostccr_reg);
>> + prescale = readl(timer->base + OST_REG_OSTCCR);
>>
>> - prescale = (prescale & OSTCCR_PRESCALE2_MASK) >>
>> OSTCCR_PRESCALE2_LSB;
>> + if (ost_clk->ost->soc_info->version >= ID_X2000)
>> + prescale = FIELD_GET(OSTCCR_PRESCALE1_MASK, prescale);
>> + else
>> + prescale = FIELD_GET(OSTCCR_PRESCALE2_MASK, prescale);
>>
>> return parent_rate >> (prescale * 2);
>> }
>> @@ -147,12 +181,13 @@ static int
>> ingenic_ost_percpu_timer_set_rate(struct clk_hw *hw, unsigned long re
>> {
>> struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
>> const struct ingenic_ost_clk_info *info = ost_clk->info;
>> + struct ingenic_ost_timer *timer =
>> per_cpu_ptr(ost_clk->ost->timers, info->idx);
>> u8 prescale = ingenic_ost_get_prescale(parent_rate, req_rate);
>> int val;
>>
>> - val = readl(ost_clk->ost->base + info->ostccr_reg);
>> + val = readl(timer->base + OST_REG_OSTCCR);
>> val = (val & ~OSTCCR_PRESCALE1_MASK) | (prescale <<
>> OSTCCR_PRESCALE1_LSB);
>> - writel(val, ost_clk->ost->base + info->ostccr_reg);
>> + writel(val, timer->base + OST_REG_OSTCCR);
>>
>> return 0;
>> }
>> @@ -162,12 +197,18 @@ static int
>> ingenic_ost_global_timer_set_rate(struct clk_hw *hw, unsigned long re
>> {
>> struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
>> const struct ingenic_ost_clk_info *info = ost_clk->info;
>> + struct ingenic_ost_timer *timer =
>> per_cpu_ptr(ost_clk->ost->timers, info->idx);
>> u8 prescale = ingenic_ost_get_prescale(parent_rate, req_rate);
>> int val;
>>
>> - val = readl(ost_clk->ost->base + info->ostccr_reg);
>> - val = (val & ~OSTCCR_PRESCALE2_MASK) | (prescale <<
>> OSTCCR_PRESCALE2_LSB);
>> - writel(val, ost_clk->ost->base + info->ostccr_reg);
>> + val = readl(timer->base + OST_REG_OSTCCR);
>> +
>> + if (ost_clk->ost->soc_info->version >= ID_X2000)
>> + val = (val & ~OSTCCR_PRESCALE1_MASK) | (prescale <<
>> OSTCCR_PRESCALE1_LSB);
>> + else
>> + val = (val & ~OSTCCR_PRESCALE2_MASK) | (prescale <<
>> OSTCCR_PRESCALE2_LSB);
>> +
>> + writel(val, timer->base + OST_REG_OSTCCR);
>>
>> return 0;
>> }
>> @@ -195,7 +236,42 @@ static const struct ingenic_ost_clk_info
>> x1000_ost_clk_info[] = {
>> .ops = &ingenic_ost_percpu_timer_ops,
>> .flags = CLK_SET_RATE_UNGATE,
>> },
>> - .ostccr_reg = OST_REG_OSTCCR,
>> + .idx = 0,
>> + },
>> +
>> + [OST_CLK_GLOBAL_TIMER] = {
>> + .init_data = {
>> + .name = "global timer",
>> + .parent_names = ingenic_ost_clk_parents,
>> + .num_parents = ARRAY_SIZE(ingenic_ost_clk_parents),
>> + .ops = &ingenic_ost_global_timer_ops,
>> + .flags = CLK_SET_RATE_UNGATE,
>> + },
>> + .ostcntl_reg = OST_REG_OST2CNTL,
>> + },
>> +};
>> +
>> +static const struct ingenic_ost_clk_info x2000_ost_clk_info[] = {
>> + [OST_CLK_PERCPU_TIMER0] = {
>> + .init_data = {
>> + .name = "percpu timer0",
>> + .parent_names = ingenic_ost_clk_parents,
>> + .num_parents = ARRAY_SIZE(ingenic_ost_clk_parents),
>> + .ops = &ingenic_ost_percpu_timer_ops,
>> + .flags = CLK_SET_RATE_UNGATE,
>> + },
>> + .idx = 0,
>> + },
>> +
>> + [OST_CLK_PERCPU_TIMER1] = {
>> + .init_data = {
>> + .name = "percpu timer1",
>> + .parent_names = ingenic_ost_clk_parents,
>> + .num_parents = ARRAY_SIZE(ingenic_ost_clk_parents),
>> + .ops = &ingenic_ost_percpu_timer_ops,
>> + .flags = CLK_SET_RATE_UNGATE,
>> + },
>> + .idx = 1,
>> },
>>
>> [OST_CLK_GLOBAL_TIMER] = {
>> @@ -206,7 +282,7 @@ static const struct ingenic_ost_clk_info
>> x1000_ost_clk_info[] = {
>> .ops = &ingenic_ost_global_timer_ops,
>> .flags = CLK_SET_RATE_UNGATE,
>> },
>> - .ostccr_reg = OST_REG_OSTCCR,
>> + .ostcntl_reg = OST_REG_OSTCNTL,
>> },
>> };
>>
>> @@ -215,7 +291,7 @@ static u64 notrace
>> ingenic_ost_global_timer_read_cntl(void)
>> struct ingenic_ost *ost = ingenic_ost;
>> unsigned int count;
>>
>> - count = readl(ost->base + OST_REG_OST2CNTL);
>> + count = readl(ost->base + ost->soc_info->clk_info->ostcntl_reg);
>>
>> return count;
>> }
>> @@ -225,16 +301,21 @@ static u64 notrace
>> ingenic_ost_clocksource_read(struct clocksource *cs)
>> return ingenic_ost_global_timer_read_cntl();
>> }
>>
>> -static inline struct ingenic_ost *to_ingenic_ost(struct
>> clock_event_device *evt)
>> +static inline struct ingenic_ost_timer *
>> +to_ingenic_ost_timer(struct clock_event_device *evt)
>> {
>> - return container_of(evt, struct ingenic_ost, cevt);
>> + return container_of(evt, struct ingenic_ost_timer, cevt);
>> }
>>
>> static int ingenic_ost_cevt_set_state_shutdown(struct
>> clock_event_device *evt)
>> {
>> - struct ingenic_ost *ost = to_ingenic_ost(evt);
>> + struct ingenic_ost_timer *timer = to_ingenic_ost_timer(evt);
>> + struct ingenic_ost *ost = timer->ost;
>>
>> - writel(OSTECR_OST1ENC, ost->base + OST_REG_OSTECR);
>> + if (ost->soc_info->version >= ID_X2000)
>> + writel(0, timer->base + OST_REG_OSTER);
>> + else
>> + writel(OSTECR_OST1ENC, timer->base + OST_REG_OSTECR);
>>
>> return 0;
>> }
>> @@ -242,26 +323,34 @@ static int
>> ingenic_ost_cevt_set_state_shutdown(struct clock_event_device *evt)
>> static int ingenic_ost_cevt_set_next(unsigned long next,
>> struct clock_event_device *evt)
>> {
>> - struct ingenic_ost *ost = to_ingenic_ost(evt);
>> -
>> - writel((u32)~OSTFR_FFLAG, ost->base + OST_REG_OSTFR);
>> - writel(next, ost->base + OST_REG_OST1DFR);
>> - writel(OSTCR_OST1CLR, ost->base + OST_REG_OSTCR);
>> - writel(OSTESR_OST1ENS, ost->base + OST_REG_OSTESR);
>> - writel((u32)~OSTMR_FMASK, ost->base + OST_REG_OSTMR);
>> + struct ingenic_ost_timer *timer = to_ingenic_ost_timer(evt);
>> + struct ingenic_ost *ost = timer->ost;
>> +
>> + writel((u32)~OSTFR_FFLAG, timer->base + OST_REG_OSTFR);
>> + writel(next, timer->base + OST_REG_OST1DFR);
>> + writel(OSTCR_OST1CLR, timer->base + OST_REG_OSTCR);
>> +
>> + if (ost->soc_info->version >= ID_X2000) {
>> + writel(OSTESR_OST1ENS, timer->base + OST_REG_OSTER);
>> + } else {
>> + writel(OSTESR_OST1ENS, timer->base + OST_REG_OSTESR);
>> + writel((u32)~OSTMR_FMASK, timer->base + OST_REG_OSTMR);
>> + }
>>
>> return 0;
>> }
>>
>> static irqreturn_t ingenic_ost_cevt_cb(int irq, void *dev_id)
>> {
>> - struct clock_event_device *evt = dev_id;
>> - struct ingenic_ost *ost = to_ingenic_ost(evt);
>> + struct ingenic_ost_timer *timer = dev_id;
>> + struct ingenic_ost *ost = timer->ost;
>>
>> - writel(OSTECR_OST1ENC, ost->base + OST_REG_OSTECR);
>> + if (ost->soc_info->version >= ID_X2000)
>> + writel(0, timer->base + OST_REG_OSTER);
>> + else
>> + writel(OSTECR_OST1ENC, timer->base + OST_REG_OSTECR);
>>
>> - if (evt->event_handler)
>> - evt->event_handler(evt);
>> + timer->cevt.event_handler(&timer->cevt);
>>
>> return IRQ_HANDLED;
>> }
>> @@ -271,6 +360,7 @@ static int __init
>> ingenic_ost_register_clock(struct ingenic_ost *ost,
>> struct clk_hw_onecell_data *clocks)
>> {
>> struct ingenic_ost_clk *ost_clk;
>> + struct ingenic_ost_timer *timer = per_cpu_ptr(ost->timers,
>> info->idx);
>> int val, err;
>>
>> ost_clk = kzalloc(sizeof(*ost_clk), GFP_KERNEL);
>> @@ -283,9 +373,9 @@ static int __init
>> ingenic_ost_register_clock(struct ingenic_ost *ost,
>> ost_clk->ost = ost;
>>
>> /* Reset clock divider */
>> - val = readl(ost->base + info->ostccr_reg);
>> - val &= ~(OSTCCR_PRESCALE1_MASK | OSTCCR_PRESCALE2_MASK);
>> - writel(val, ost->base + info->ostccr_reg);
>> + val = readl(timer->base + OST_REG_OSTCCR);
>> + val &= ~(OSTCCR_PRESCALE1_MASK);
>> + writel(val, timer->base + OST_REG_OSTCCR);
>>
>> err = clk_hw_register(NULL, &ost_clk->hw);
>> if (err) {
>> @@ -309,57 +399,51 @@ static struct clk * __init
>> ingenic_ost_get_clock(struct device_node *np, int id)
>> return of_clk_get_from_provider(&args);
>> }
>>
>> -static int __init ingenic_ost_percpu_timer_init(struct device_node *np,
>> - struct ingenic_ost *ost)
>> +static int __init ingenic_ost_setup_cevt(unsigned int cpu)
>> {
>> - unsigned int timer_virq, channel = OST_CLK_PERCPU_TIMER;
>> + struct ingenic_ost *ost = ingenic_ost;
>> + struct ingenic_ost_timer *timer = this_cpu_ptr(ost->timers);
>> unsigned long rate;
>> int err;
>>
>> - ost->percpu_timer_clk = ingenic_ost_get_clock(np, channel);
>> - if (IS_ERR(ost->percpu_timer_clk))
>> - return PTR_ERR(ost->percpu_timer_clk);
>> + timer->clk = ingenic_ost_get_clock(ost->np, timer->channel);
>> + if (IS_ERR(timer->clk))
>> + return PTR_ERR(timer->clk);
>>
>> - err = clk_prepare_enable(ost->percpu_timer_clk);
>> + err = clk_prepare_enable(timer->clk);
>> if (err)
>> goto err_clk_put;
>>
>> - rate = clk_get_rate(ost->percpu_timer_clk);
>> + rate = clk_get_rate(timer->clk);
>> if (!rate) {
>> err = -EINVAL;
>> goto err_clk_disable;
>> }
>>
>> - timer_virq = of_irq_get(np, 0);
>> - if (!timer_virq) {
>> - err = -EINVAL;
>> - goto err_clk_disable;
>> - }
>> + snprintf(timer->name, sizeof(timer->name), "OST percpu timer%u",
>> cpu);
>>
>> - snprintf(ost->name, sizeof(ost->name), "OST percpu timer");
>> + /* Unmask full comparison match interrupt */
>> + writel((u32)~OSTMR_FMASK, timer->base + OST_REG_OSTMR);
>>
>> - err = request_irq(timer_virq, ingenic_ost_cevt_cb, IRQF_TIMER,
>> - ost->name, &ost->cevt);
>> - if (err)
>> - goto err_irq_dispose_mapping;
>> + timer->cpu = smp_processor_id();
>> + timer->cevt.cpumask = cpumask_of(smp_processor_id());
>> + timer->cevt.features = CLOCK_EVT_FEAT_ONESHOT;
>> + timer->cevt.name = timer->name;
>> + timer->cevt.rating = 400;
>> + timer->cevt.set_state_shutdown =
>> ingenic_ost_cevt_set_state_shutdown;
>> + timer->cevt.set_next_event = ingenic_ost_cevt_set_next;
>>
>> - ost->cevt.cpumask = cpumask_of(smp_processor_id());
>> - ost->cevt.features = CLOCK_EVT_FEAT_ONESHOT;
>> - ost->cevt.name = ost->name;
>> - ost->cevt.rating = 400;
>> - ost->cevt.set_state_shutdown = ingenic_ost_cevt_set_state_shutdown;
>> - ost->cevt.set_next_event = ingenic_ost_cevt_set_next;
>> + clockevents_config_and_register(&timer->cevt, rate, 4, 0xffffffff);
>>
>> - clockevents_config_and_register(&ost->cevt, rate, 4, 0xffffffff);
>> + if (ost->soc_info->version >= ID_X2000)
>> + enable_percpu_irq(ost->irq, IRQ_TYPE_NONE);
>>
>> return 0;
>>
>> -err_irq_dispose_mapping:
>> - irq_dispose_mapping(timer_virq);
>> err_clk_disable:
>> - clk_disable_unprepare(ost->percpu_timer_clk);
>> + clk_disable_unprepare(timer->clk);
>> err_clk_put:
>> - clk_put(ost->percpu_timer_clk);
>> + clk_put(timer->clk);
>> return err;
>> }
>>
>> @@ -385,11 +469,14 @@ static int __init
>> ingenic_ost_global_timer_init(struct device_node *np,
>> goto err_clk_disable;
>> }
>>
>> - /* Clear counter CNT registers */
>> - writel(OSTCR_OST2CLR, ost->base + OST_REG_OSTCR);
>> -
>> - /* Enable OST channel */
>> - writel(OSTESR_OST2ENS, ost->base + OST_REG_OSTESR);
>> + /* Clear counter CNT registers and enable OST channel */
>> + if (ost->soc_info->version >= ID_X2000) {
>> + writel(OSTCR_OST1CLR, ost->base + OST_REG_OSTCR);
>> + writel(OSTESR_OST1ENS, ost->base + OST_REG_OSTER);
>> + } else {
>> + writel(OSTCR_OST2CLR, ost->base + OST_REG_OSTCR);
>> + writel(OSTESR_OST2ENS, ost->base + OST_REG_OSTESR);
>> + }
>>
>> cs->name = "ingenic-ost";
>> cs->rating = 400;
>> @@ -411,18 +498,33 @@ static int __init
>> ingenic_ost_global_timer_init(struct device_node *np,
>> }
>>
>> static const struct ingenic_soc_info x1000_soc_info = {
>> + .version = ID_X1000,
>> + .clk_info = x1000_ost_clk_info,
>> +
>> .num_channels = 2,
>> };
>>
>> +static const struct ingenic_soc_info x2000_soc_info = {
>> + .version = ID_X2000,
>> + .clk_info = x2000_ost_clk_info,
>> +
>> + .num_channels = 3,
>> + .base_offset = 0x100,
>> +};
>> +
>> static const struct of_device_id __maybe_unused
>> ingenic_ost_of_matches[] __initconst = {
>> { .compatible = "ingenic,x1000-ost", .data = &x1000_soc_info },
>> + { .compatible = "ingenic,x2000-ost", .data = &x2000_soc_info },
>> { /* sentinel */ }
>> };
>>
>> static int __init ingenic_ost_probe(struct device_node *np)
>> {
>> const struct of_device_id *id =
>> of_match_node(ingenic_ost_of_matches, np);
>> + struct ingenic_ost_timer *timer;
>> struct ingenic_ost *ost;
>> + void __iomem *base;
>> + unsigned int cpu;
>> unsigned int i;
>> int ret;
>>
>> @@ -430,18 +532,43 @@ static int __init ingenic_ost_probe(struct
>> device_node *np)
>> if (!ost)
>> return -ENOMEM;
>>
>> + ost->timers = alloc_percpu(struct ingenic_ost_timer);
>> + if (!ost->timers) {
>> + ret = -ENOMEM;
>> + goto err_free_ost;
>> + }
>> +
>> + ost->np = np;
>> + ost->soc_info = id->data;
>> +
>> ost->base = of_io_request_and_map(np, 0, of_node_full_name(np));
>> if (IS_ERR(ost->base)) {
>> pr_err("%s: Failed to map OST registers\n", __func__);
>> ret = PTR_ERR(ost->base);
>> - goto err_free_ost;
>> + goto err_free_timers;
>> + }
>> +
>> + if (ost->soc_info->version >= ID_X2000) {
>> + base = of_io_request_and_map(np, 1, of_node_full_name(np));
>> + if (IS_ERR(base)) {
>> + pr_err("%s: Failed to map OST registers\n", __func__);
>> + ret = PTR_ERR(base);
>> + goto err_free_timers;
>> + }
>> + }
>
> The DT documentation only mentions one memory resource. Here, you map
> a second one, which is not used anywhere. I'm really confused about
> what you're trying to do here.
>
X2000 and X2100 divide the OST into two parts. The global timer is
the first part, which is still located at the address of 0x12000000,
and the percpu timers are the second part, the starting address is
0x12100000, and each timer is offset by 0x100 (percpu timer0 is at
0x12100000, percpu timer1 is at 0x12100100, percpu timer2 is at
0x12100200, percpu timer3 is at 0x12100300). This one is used in
line 593 of the code.
>> +
>> + ost->irq = irq_of_parse_and_map(np, 0);
>> + if (ost->irq < 0) {
>> + pr_crit("%s: Cannot to get OST IRQ\n", __func__);
>> + ret = ost->irq;
>> + goto err_free_timers;
>> }
>>
>> ost->clk = of_clk_get_by_name(np, "ost");
>> if (IS_ERR(ost->clk)) {
>> - ret = PTR_ERR(ost->clk);
>> pr_crit("%s: Cannot get OST clock\n", __func__);
>> - goto err_free_ost;
>> + ret = PTR_ERR(ost->clk);
>> + goto err_free_timers;
>> }
>>
>> ret = clk_prepare_enable(ost->clk);
>> @@ -450,8 +577,6 @@ static int __init ingenic_ost_probe(struct
>> device_node *np)
>> goto err_put_clk;
>> }
>>
>> - ost->soc_info = id->data;
>> -
>> ost->clocks = kzalloc(struct_size(ost->clocks, hws,
>> ost->soc_info->num_channels),
>> GFP_KERNEL);
>> if (!ost->clocks) {
>> @@ -461,8 +586,21 @@ static int __init ingenic_ost_probe(struct
>> device_node *np)
>>
>> ost->clocks->num = ost->soc_info->num_channels;
>>
>> - for (i = 0; i < ost->clocks->num; i++) {
>> - ret = ingenic_ost_register_clock(ost, i,
>> &x1000_ost_clk_info[i], ost->clocks);
>> + for (cpu = 0; cpu < num_possible_cpus(); cpu++) {
>> + timer = per_cpu_ptr(ost->timers, cpu);
>> +
>> + if (ost->soc_info->version >= ID_X2000)
>> + timer->base = base + ost->soc_info->base_offset * cpu;
>> + else
>> + timer->base = ost->base;
>> +
>> + timer->ost = ost;
>> + timer->cpu = cpu;
>> + timer->channel = OST_CLK_PERCPU_TIMER + cpu;
>> + }
>> +
>> + for (i = 0; i < num_possible_cpus() + 1; i++) {
>> + ret = ingenic_ost_register_clock(ost, i,
>> &ost->soc_info->clk_info[i], ost->clocks);
>> if (ret) {
>> pr_crit("%s: Cannot register clock %d\n", __func__, i);
>> goto err_unregister_ost_clocks;
>> @@ -488,6 +626,8 @@ static int __init ingenic_ost_probe(struct
>> device_node *np)
>> clk_disable_unprepare(ost->clk);
>> err_put_clk:
>> clk_put(ost->clk);
>> +err_free_timers:
>> + free_percpu(ost->timers);
>> err_free_ost:
>> kfree(ost);
>> return ret;
>> @@ -513,13 +653,29 @@ static int __init ingenic_ost_init(struct
>> device_node *np)
>>
>> ret = ingenic_ost_global_timer_init(np, ost);
>> if (ret) {
>> - pr_crit("%s: Unable to init global timer: %x\n", __func__,
>> ret);
>> + pr_crit("%s: Unable to init global timer: %d\n", __func__,
>> ret);
>
> This is a fix, so it needs to be a separate commit with a Fixes: tag.
Sure.
Thanks and best regards!
>
> Cheers,
> -Paul
>
>> goto err_free_ingenic_ost;
>> }
>>
>> - ret = ingenic_ost_percpu_timer_init(np, ost);
>> - if (ret)
>> + if (ost->soc_info->version >= ID_X2000)
>> + ret = request_percpu_irq(ost->irq, ingenic_ost_cevt_cb,
>> + "OST percpu timer", ost->timers);
>> + else
>> + ret = request_irq(ost->irq, ingenic_ost_cevt_cb, IRQF_TIMER,
>> + "OST percpu timer", ost->timers);
>> +
>> + if (ret) {
>> + pr_crit("%s: Unable to request percpu IRQ: %d\n", __func__,
>> ret);
>> + goto err_ost_global_timer_cleanup;
>> + }
>> +
>> + /* Setup clock events on each CPU core */
>> + ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "Ingenic XBurst:
>> online",
>> + ingenic_ost_setup_cevt, NULL);
>> + if (ret < 0) {
>> + pr_crit("%s: Unable to init percpu timers: %d\n", __func__,
>> ret);
>> goto err_ost_global_timer_cleanup;
>> + }
>>
>> /* Register the sched_clock at the end as there's no way to undo
>> it */
>> rate = clk_get_rate(ost->global_timer_clk);
>> @@ -537,3 +693,4 @@ static int __init ingenic_ost_init(struct
>> device_node *np)
>> }
>>
>> TIMER_OF_DECLARE(x1000_ost, "ingenic,x1000-ost", ingenic_ost_init);
>> +TIMER_OF_DECLARE(x2000_ost, "ingenic,x2000-ost", ingenic_ost_init);
>> --
>> 2.7.4
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] clocksource: Ingenic: Add SMP/SMT support for sysost driver.
2021-06-11 15:31 ` Zhou Yanjie
@ 2021-06-11 15:44 ` Paul Cercueil
2021-06-12 15:48 ` 周琰杰
0 siblings, 1 reply; 11+ messages in thread
From: Paul Cercueil @ 2021-06-11 15:44 UTC (permalink / raw)
To: Zhou Yanjie
Cc: Paul Cercueil, 周琰杰,
daniel.lezcano, tglx, linux-mips, linux-kernel, dongsheng.qiu,
aric.pzqi, rick.tyliu, sihui.liu, jun.jiang, sernia.zhou
Hi Zhou,
Le ven., juin 11 2021 at 23:31:57 +0800, Zhou Yanjie
<zhouyu@wanyeetech.com> a écrit :
> Hi Paul,
>
> On 2021/6/10 下午11:30, Paul Cercueil wrote:
>> Hi Zhou,
>>
>> Le sam., juin 5 2021 at 00:31:46 +0800, 周琰杰 (Zhou Yanjie)
>> \x7f<zhouyanjie@wanyeetech.com> a écrit :
>>> The OST in Ingenic XBurst®2 SoCs such as X2000 and X2100, has a
>>> global
>>> timer and two or four percpu timers, add support for the percpu
>>> timers.
>>>
>>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
>>> ---
>>>
>>> Notes:
>>> v1->v2:
>>> 1.Fix bug in ingenic_ost_global_timer_recalc_rate().
>>> 2.Add a backpointer to the ingenic_ost structure.
>>> 3.Remove unnecessary spinlock.
>>> 4.Use "ret = ost->irq" instead "ret = -EINVAL".
>>> 5.Use "%d" instead "%x" in pr_crit().
>>
>> I can't shake the feeling that you are doing way too many things in
>> \x7fone single commit.
>>
>> From what I can see, this commit can be split in 4 patches:
>>
>> - Fix the "%x" in pr_crit(),
>> - Add the global timer support to the X1000,
>> - Add "ingenic_ost_timer" and update the code to use it,
>> - Finally add X2000 support.
>>
>>
>
> Sure.
>
>
>>
>>> drivers/clocksource/ingenic-sysost.c | 315
>>> \x7f\x7f++++++++++++++++++++++++++---------
>>> 1 file changed, 236 insertions(+), 79 deletions(-)
>>>
>>> diff --git a/drivers/clocksource/ingenic-sysost.c
>>> \x7f\x7fb/drivers/clocksource/ingenic-sysost.c
>>> index a129840..6f080e4 100644
>>> --- a/drivers/clocksource/ingenic-sysost.c
>>> +++ b/drivers/clocksource/ingenic-sysost.c
>>> @@ -4,6 +4,7 @@
>>> * Copyright (c) 2020 周琰杰 (Zhou Yanjie)
>>> <zhouyanjie@wanyeetech.com>
>>> */
>>>
>>> +#include <linux/bitfield.h>
>>> #include <linux/bitops.h>
>>> #include <linux/clk.h>
>>> #include <linux/clk-provider.h>
>>> @@ -13,6 +14,8 @@
>>> #include <linux/mfd/syscon.h>
>>> #include <linux/of_address.h>
>>> #include <linux/of_irq.h>
>>> +#include <linux/of_platform.h>
>>> +#include <linux/overflow.h>
>>> #include <linux/sched_clock.h>
>>> #include <linux/slab.h>
>>> #include <linux/syscore_ops.h>
>>> @@ -21,10 +24,14 @@
>>>
>>> /* OST register offsets */
>>> #define OST_REG_OSTCCR 0x00
>>> +#define OST_REG_OSTER 0x04
>>> #define OST_REG_OSTCR 0x08
>>> #define OST_REG_OSTFR 0x0c
>>> +#define OST_REG_OSTCNTH 0x0c
>>> #define OST_REG_OSTMR 0x10
>>> +#define OST_REG_OSTCNTL 0x10
>>> #define OST_REG_OST1DFR 0x14
>>> +#define OST_REG_OSTCNTB 0x14
>>> #define OST_REG_OST1CNT 0x18
>>> #define OST_REG_OST2CNTL 0x20
>>> #define OST_REG_OSTCNT2HBUF 0x24
>>> @@ -55,13 +62,23 @@
>>> #define OSTECR_OST1ENC BIT(0)
>>> #define OSTECR_OST2ENC BIT(1)
>>>
>>> +enum ingenic_ost_version {
>>> + ID_X1000,
>>> + ID_X2000,
>>> +};
>>> +
>>> struct ingenic_soc_info {
>>> + enum ingenic_ost_version version;
>>> + const struct ingenic_ost_clk_info *clk_info;
>>> +
>>> unsigned int num_channels;
>>> + unsigned int base_offset;
>>> };
>>>
>>> struct ingenic_ost_clk_info {
>>> struct clk_init_data init_data;
>>> - u8 ostccr_reg;
>>> + unsigned int idx;
>>> + u32 ostcntl_reg;
>>> };
>>>
>>> struct ingenic_ost_clk {
>>> @@ -71,15 +88,27 @@ struct ingenic_ost_clk {
>>> const struct ingenic_ost_clk_info *info;
>>> };
>>>
>>> +struct ingenic_ost_timer {
>>> + void __iomem *base;
>>> + unsigned int cpu;
>>> + unsigned int channel;
>>> + struct clock_event_device cevt;
>>> + struct ingenic_ost *ost;
>>> + struct clk *clk;
>>> + char name[20];
>>> +};
>>> +
>>> struct ingenic_ost {
>>> void __iomem *base;
>>> const struct ingenic_soc_info *soc_info;
>>> - struct clk *clk, *percpu_timer_clk, *global_timer_clk;
>>> - struct clock_event_device cevt;
>>> + struct clk *clk, *global_timer_clk;
>>> + struct device_node *np;
>>> struct clocksource cs;
>>> - char name[20];
>>>
>>> struct clk_hw_onecell_data *clocks;
>>> + struct ingenic_ost_timer __percpu *timers;
>>> +
>>> + int irq;
>>> };
>>>
>>> static struct ingenic_ost *ingenic_ost;
>>> @@ -94,11 +123,12 @@ static unsigned long
>>> \x7f\x7fingenic_ost_percpu_timer_recalc_rate(struct clk_hw *hw,
>>> {
>>> struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
>>> const struct ingenic_ost_clk_info *info = ost_clk->info;
>>> + struct ingenic_ost_timer *timer =
>>> \x7f\x7fper_cpu_ptr(ost_clk->ost->timers, info->idx);
>>> unsigned int prescale;
>>>
>>> - prescale = readl(ost_clk->ost->base + info->ostccr_reg);
>>> + prescale = readl(timer->base + OST_REG_OSTCCR);
>>>
>>> - prescale = (prescale & OSTCCR_PRESCALE1_MASK) >>
>>> \x7f\x7fOSTCCR_PRESCALE1_LSB;
>>> + prescale = FIELD_GET(OSTCCR_PRESCALE1_MASK, prescale);
>>>
>>> return parent_rate >> (prescale * 2);
>>> }
>>> @@ -108,11 +138,15 @@ static unsigned long
>>> \x7f\x7fingenic_ost_global_timer_recalc_rate(struct clk_hw *hw,
>>> {
>>> struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
>>> const struct ingenic_ost_clk_info *info = ost_clk->info;
>>> + struct ingenic_ost_timer *timer =
>>> \x7f\x7fper_cpu_ptr(ost_clk->ost->timers, info->idx);
>>> unsigned int prescale;
>>>
>>> - prescale = readl(ost_clk->ost->base + info->ostccr_reg);
>>> + prescale = readl(timer->base + OST_REG_OSTCCR);
>>>
>>> - prescale = (prescale & OSTCCR_PRESCALE2_MASK) >>
>>> \x7f\x7fOSTCCR_PRESCALE2_LSB;
>>> + if (ost_clk->ost->soc_info->version >= ID_X2000)
>>> + prescale = FIELD_GET(OSTCCR_PRESCALE1_MASK, prescale);
>>> + else
>>> + prescale = FIELD_GET(OSTCCR_PRESCALE2_MASK, prescale);
>>>
>>> return parent_rate >> (prescale * 2);
>>> }
>>> @@ -147,12 +181,13 @@ static int
>>> \x7f\x7fingenic_ost_percpu_timer_set_rate(struct clk_hw *hw, unsigned
>>> long re
>>> {
>>> struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
>>> const struct ingenic_ost_clk_info *info = ost_clk->info;
>>> + struct ingenic_ost_timer *timer =
>>> \x7f\x7fper_cpu_ptr(ost_clk->ost->timers, info->idx);
>>> u8 prescale = ingenic_ost_get_prescale(parent_rate, req_rate);
>>> int val;
>>>
>>> - val = readl(ost_clk->ost->base + info->ostccr_reg);
>>> + val = readl(timer->base + OST_REG_OSTCCR);
>>> val = (val & ~OSTCCR_PRESCALE1_MASK) | (prescale <<
>>> \x7f\x7fOSTCCR_PRESCALE1_LSB);
>>> - writel(val, ost_clk->ost->base + info->ostccr_reg);
>>> + writel(val, timer->base + OST_REG_OSTCCR);
>>>
>>> return 0;
>>> }
>>> @@ -162,12 +197,18 @@ static int
>>> \x7f\x7fingenic_ost_global_timer_set_rate(struct clk_hw *hw, unsigned
>>> long re
>>> {
>>> struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
>>> const struct ingenic_ost_clk_info *info = ost_clk->info;
>>> + struct ingenic_ost_timer *timer =
>>> \x7f\x7fper_cpu_ptr(ost_clk->ost->timers, info->idx);
>>> u8 prescale = ingenic_ost_get_prescale(parent_rate, req_rate);
>>> int val;
>>>
>>> - val = readl(ost_clk->ost->base + info->ostccr_reg);
>>> - val = (val & ~OSTCCR_PRESCALE2_MASK) | (prescale <<
>>> \x7f\x7fOSTCCR_PRESCALE2_LSB);
>>> - writel(val, ost_clk->ost->base + info->ostccr_reg);
>>> + val = readl(timer->base + OST_REG_OSTCCR);
>>> +
>>> + if (ost_clk->ost->soc_info->version >= ID_X2000)
>>> + val = (val & ~OSTCCR_PRESCALE1_MASK) | (prescale <<
>>> \x7f\x7fOSTCCR_PRESCALE1_LSB);
>>> + else
>>> + val = (val & ~OSTCCR_PRESCALE2_MASK) | (prescale <<
>>> \x7f\x7fOSTCCR_PRESCALE2_LSB);
>>> +
>>> + writel(val, timer->base + OST_REG_OSTCCR);
>>>
>>> return 0;
>>> }
>>> @@ -195,7 +236,42 @@ static const struct ingenic_ost_clk_info
>>> \x7f\x7fx1000_ost_clk_info[] = {
>>> .ops = &ingenic_ost_percpu_timer_ops,
>>> .flags = CLK_SET_RATE_UNGATE,
>>> },
>>> - .ostccr_reg = OST_REG_OSTCCR,
>>> + .idx = 0,
>>> + },
>>> +
>>> + [OST_CLK_GLOBAL_TIMER] = {
>>> + .init_data = {
>>> + .name = "global timer",
>>> + .parent_names = ingenic_ost_clk_parents,
>>> + .num_parents = ARRAY_SIZE(ingenic_ost_clk_parents),
>>> + .ops = &ingenic_ost_global_timer_ops,
>>> + .flags = CLK_SET_RATE_UNGATE,
>>> + },
>>> + .ostcntl_reg = OST_REG_OST2CNTL,
>>> + },
>>> +};
>>> +
>>> +static const struct ingenic_ost_clk_info x2000_ost_clk_info[] = {
>>> + [OST_CLK_PERCPU_TIMER0] = {
>>> + .init_data = {
>>> + .name = "percpu timer0",
>>> + .parent_names = ingenic_ost_clk_parents,
>>> + .num_parents = ARRAY_SIZE(ingenic_ost_clk_parents),
>>> + .ops = &ingenic_ost_percpu_timer_ops,
>>> + .flags = CLK_SET_RATE_UNGATE,
>>> + },
>>> + .idx = 0,
>>> + },
>>> +
>>> + [OST_CLK_PERCPU_TIMER1] = {
>>> + .init_data = {
>>> + .name = "percpu timer1",
>>> + .parent_names = ingenic_ost_clk_parents,
>>> + .num_parents = ARRAY_SIZE(ingenic_ost_clk_parents),
>>> + .ops = &ingenic_ost_percpu_timer_ops,
>>> + .flags = CLK_SET_RATE_UNGATE,
>>> + },
>>> + .idx = 1,
>>> },
>>>
>>> [OST_CLK_GLOBAL_TIMER] = {
>>> @@ -206,7 +282,7 @@ static const struct ingenic_ost_clk_info
>>> \x7f\x7fx1000_ost_clk_info[] = {
>>> .ops = &ingenic_ost_global_timer_ops,
>>> .flags = CLK_SET_RATE_UNGATE,
>>> },
>>> - .ostccr_reg = OST_REG_OSTCCR,
>>> + .ostcntl_reg = OST_REG_OSTCNTL,
>>> },
>>> };
>>>
>>> @@ -215,7 +291,7 @@ static u64 notrace
>>> \x7f\x7fingenic_ost_global_timer_read_cntl(void)
>>> struct ingenic_ost *ost = ingenic_ost;
>>> unsigned int count;
>>>
>>> - count = readl(ost->base + OST_REG_OST2CNTL);
>>> + count = readl(ost->base +
>>> ost->soc_info->clk_info->ostcntl_reg);
>>>
>>> return count;
>>> }
>>> @@ -225,16 +301,21 @@ static u64 notrace
>>> \x7f\x7fingenic_ost_clocksource_read(struct clocksource *cs)
>>> return ingenic_ost_global_timer_read_cntl();
>>> }
>>>
>>> -static inline struct ingenic_ost *to_ingenic_ost(struct
>>> \x7f\x7fclock_event_device *evt)
>>> +static inline struct ingenic_ost_timer *
>>> +to_ingenic_ost_timer(struct clock_event_device *evt)
>>> {
>>> - return container_of(evt, struct ingenic_ost, cevt);
>>> + return container_of(evt, struct ingenic_ost_timer, cevt);
>>> }
>>>
>>> static int ingenic_ost_cevt_set_state_shutdown(struct
>>> \x7f\x7fclock_event_device *evt)
>>> {
>>> - struct ingenic_ost *ost = to_ingenic_ost(evt);
>>> + struct ingenic_ost_timer *timer = to_ingenic_ost_timer(evt);
>>> + struct ingenic_ost *ost = timer->ost;
>>>
>>> - writel(OSTECR_OST1ENC, ost->base + OST_REG_OSTECR);
>>> + if (ost->soc_info->version >= ID_X2000)
>>> + writel(0, timer->base + OST_REG_OSTER);
>>> + else
>>> + writel(OSTECR_OST1ENC, timer->base + OST_REG_OSTECR);
>>>
>>> return 0;
>>> }
>>> @@ -242,26 +323,34 @@ static int
>>> \x7f\x7fingenic_ost_cevt_set_state_shutdown(struct clock_event_device
>>> *evt)
>>> static int ingenic_ost_cevt_set_next(unsigned long next,
>>> struct clock_event_device *evt)
>>> {
>>> - struct ingenic_ost *ost = to_ingenic_ost(evt);
>>> -
>>> - writel((u32)~OSTFR_FFLAG, ost->base + OST_REG_OSTFR);
>>> - writel(next, ost->base + OST_REG_OST1DFR);
>>> - writel(OSTCR_OST1CLR, ost->base + OST_REG_OSTCR);
>>> - writel(OSTESR_OST1ENS, ost->base + OST_REG_OSTESR);
>>> - writel((u32)~OSTMR_FMASK, ost->base + OST_REG_OSTMR);
>>> + struct ingenic_ost_timer *timer = to_ingenic_ost_timer(evt);
>>> + struct ingenic_ost *ost = timer->ost;
>>> +
>>> + writel((u32)~OSTFR_FFLAG, timer->base + OST_REG_OSTFR);
>>> + writel(next, timer->base + OST_REG_OST1DFR);
>>> + writel(OSTCR_OST1CLR, timer->base + OST_REG_OSTCR);
>>> +
>>> + if (ost->soc_info->version >= ID_X2000) {
>>> + writel(OSTESR_OST1ENS, timer->base + OST_REG_OSTER);
>>> + } else {
>>> + writel(OSTESR_OST1ENS, timer->base + OST_REG_OSTESR);
>>> + writel((u32)~OSTMR_FMASK, timer->base + OST_REG_OSTMR);
>>> + }
>>>
>>> return 0;
>>> }
>>>
>>> static irqreturn_t ingenic_ost_cevt_cb(int irq, void *dev_id)
>>> {
>>> - struct clock_event_device *evt = dev_id;
>>> - struct ingenic_ost *ost = to_ingenic_ost(evt);
>>> + struct ingenic_ost_timer *timer = dev_id;
>>> + struct ingenic_ost *ost = timer->ost;
>>>
>>> - writel(OSTECR_OST1ENC, ost->base + OST_REG_OSTECR);
>>> + if (ost->soc_info->version >= ID_X2000)
>>> + writel(0, timer->base + OST_REG_OSTER);
>>> + else
>>> + writel(OSTECR_OST1ENC, timer->base + OST_REG_OSTECR);
>>>
>>> - if (evt->event_handler)
>>> - evt->event_handler(evt);
>>> + timer->cevt.event_handler(&timer->cevt);
>>>
>>> return IRQ_HANDLED;
>>> }
>>> @@ -271,6 +360,7 @@ static int __init
>>> \x7f\x7fingenic_ost_register_clock(struct ingenic_ost *ost,
>>> struct clk_hw_onecell_data *clocks)
>>> {
>>> struct ingenic_ost_clk *ost_clk;
>>> + struct ingenic_ost_timer *timer = per_cpu_ptr(ost->timers,
>>> \x7f\x7finfo->idx);
>>> int val, err;
>>>
>>> ost_clk = kzalloc(sizeof(*ost_clk), GFP_KERNEL);
>>> @@ -283,9 +373,9 @@ static int __init
>>> \x7f\x7fingenic_ost_register_clock(struct ingenic_ost *ost,
>>> ost_clk->ost = ost;
>>>
>>> /* Reset clock divider */
>>> - val = readl(ost->base + info->ostccr_reg);
>>> - val &= ~(OSTCCR_PRESCALE1_MASK | OSTCCR_PRESCALE2_MASK);
>>> - writel(val, ost->base + info->ostccr_reg);
>>> + val = readl(timer->base + OST_REG_OSTCCR);
>>> + val &= ~(OSTCCR_PRESCALE1_MASK);
>>> + writel(val, timer->base + OST_REG_OSTCCR);
>>>
>>> err = clk_hw_register(NULL, &ost_clk->hw);
>>> if (err) {
>>> @@ -309,57 +399,51 @@ static struct clk * __init
>>> \x7f\x7fingenic_ost_get_clock(struct device_node *np, int id)
>>> return of_clk_get_from_provider(&args);
>>> }
>>>
>>> -static int __init ingenic_ost_percpu_timer_init(struct device_node
>>> *np,
>>> - struct ingenic_ost *ost)
>>> +static int __init ingenic_ost_setup_cevt(unsigned int cpu)
>>> {
>>> - unsigned int timer_virq, channel = OST_CLK_PERCPU_TIMER;
>>> + struct ingenic_ost *ost = ingenic_ost;
>>> + struct ingenic_ost_timer *timer = this_cpu_ptr(ost->timers);
>>> unsigned long rate;
>>> int err;
>>>
>>> - ost->percpu_timer_clk = ingenic_ost_get_clock(np, channel);
>>> - if (IS_ERR(ost->percpu_timer_clk))
>>> - return PTR_ERR(ost->percpu_timer_clk);
>>> + timer->clk = ingenic_ost_get_clock(ost->np, timer->channel);
>>> + if (IS_ERR(timer->clk))
>>> + return PTR_ERR(timer->clk);
>>>
>>> - err = clk_prepare_enable(ost->percpu_timer_clk);
>>> + err = clk_prepare_enable(timer->clk);
>>> if (err)
>>> goto err_clk_put;
>>>
>>> - rate = clk_get_rate(ost->percpu_timer_clk);
>>> + rate = clk_get_rate(timer->clk);
>>> if (!rate) {
>>> err = -EINVAL;
>>> goto err_clk_disable;
>>> }
>>>
>>> - timer_virq = of_irq_get(np, 0);
>>> - if (!timer_virq) {
>>> - err = -EINVAL;
>>> - goto err_clk_disable;
>>> - }
>>> + snprintf(timer->name, sizeof(timer->name), "OST percpu
>>> timer%u", \x7f\x7fcpu);
>>>
>>> - snprintf(ost->name, sizeof(ost->name), "OST percpu timer");
>>> + /* Unmask full comparison match interrupt */
>>> + writel((u32)~OSTMR_FMASK, timer->base + OST_REG_OSTMR);
>>>
>>> - err = request_irq(timer_virq, ingenic_ost_cevt_cb, IRQF_TIMER,
>>> - ost->name, &ost->cevt);
>>> - if (err)
>>> - goto err_irq_dispose_mapping;
>>> + timer->cpu = smp_processor_id();
>>> + timer->cevt.cpumask = cpumask_of(smp_processor_id());
>>> + timer->cevt.features = CLOCK_EVT_FEAT_ONESHOT;
>>> + timer->cevt.name = timer->name;
>>> + timer->cevt.rating = 400;
>>> + timer->cevt.set_state_shutdown =
>>> \x7f\x7fingenic_ost_cevt_set_state_shutdown;
>>> + timer->cevt.set_next_event = ingenic_ost_cevt_set_next;
>>>
>>> - ost->cevt.cpumask = cpumask_of(smp_processor_id());
>>> - ost->cevt.features = CLOCK_EVT_FEAT_ONESHOT;
>>> - ost->cevt.name = ost->name;
>>> - ost->cevt.rating = 400;
>>> - ost->cevt.set_state_shutdown =
>>> ingenic_ost_cevt_set_state_shutdown;
>>> - ost->cevt.set_next_event = ingenic_ost_cevt_set_next;
>>> + clockevents_config_and_register(&timer->cevt, rate, 4,
>>> 0xffffffff);
>>>
>>> - clockevents_config_and_register(&ost->cevt, rate, 4,
>>> 0xffffffff);
>>> + if (ost->soc_info->version >= ID_X2000)
>>> + enable_percpu_irq(ost->irq, IRQ_TYPE_NONE);
>>>
>>> return 0;
>>>
>>> -err_irq_dispose_mapping:
>>> - irq_dispose_mapping(timer_virq);
>>> err_clk_disable:
>>> - clk_disable_unprepare(ost->percpu_timer_clk);
>>> + clk_disable_unprepare(timer->clk);
>>> err_clk_put:
>>> - clk_put(ost->percpu_timer_clk);
>>> + clk_put(timer->clk);
>>> return err;
>>> }
>>>
>>> @@ -385,11 +469,14 @@ static int __init
>>> \x7f\x7fingenic_ost_global_timer_init(struct device_node *np,
>>> goto err_clk_disable;
>>> }
>>>
>>> - /* Clear counter CNT registers */
>>> - writel(OSTCR_OST2CLR, ost->base + OST_REG_OSTCR);
>>> -
>>> - /* Enable OST channel */
>>> - writel(OSTESR_OST2ENS, ost->base + OST_REG_OSTESR);
>>> + /* Clear counter CNT registers and enable OST channel */
>>> + if (ost->soc_info->version >= ID_X2000) {
>>> + writel(OSTCR_OST1CLR, ost->base + OST_REG_OSTCR);
>>> + writel(OSTESR_OST1ENS, ost->base + OST_REG_OSTER);
>>> + } else {
>>> + writel(OSTCR_OST2CLR, ost->base + OST_REG_OSTCR);
>>> + writel(OSTESR_OST2ENS, ost->base + OST_REG_OSTESR);
>>> + }
>>>
>>> cs->name = "ingenic-ost";
>>> cs->rating = 400;
>>> @@ -411,18 +498,33 @@ static int __init
>>> \x7f\x7fingenic_ost_global_timer_init(struct device_node *np,
>>> }
>>>
>>> static const struct ingenic_soc_info x1000_soc_info = {
>>> + .version = ID_X1000,
>>> + .clk_info = x1000_ost_clk_info,
>>> +
>>> .num_channels = 2,
>>> };
>>>
>>> +static const struct ingenic_soc_info x2000_soc_info = {
>>> + .version = ID_X2000,
>>> + .clk_info = x2000_ost_clk_info,
>>> +
>>> + .num_channels = 3,
>>> + .base_offset = 0x100,
>>> +};
>>> +
>>> static const struct of_device_id __maybe_unused
>>> \x7f\x7fingenic_ost_of_matches[] __initconst = {
>>> { .compatible = "ingenic,x1000-ost", .data = &x1000_soc_info },
>>> + { .compatible = "ingenic,x2000-ost", .data = &x2000_soc_info },
>>> { /* sentinel */ }
>>> };
>>>
>>> static int __init ingenic_ost_probe(struct device_node *np)
>>> {
>>> const struct of_device_id *id =
>>> \x7f\x7fof_match_node(ingenic_ost_of_matches, np);
>>> + struct ingenic_ost_timer *timer;
>>> struct ingenic_ost *ost;
>>> + void __iomem *base;
>>> + unsigned int cpu;
>>> unsigned int i;
>>> int ret;
>>>
>>> @@ -430,18 +532,43 @@ static int __init ingenic_ost_probe(struct
>>> \x7f\x7fdevice_node *np)
>>> if (!ost)
>>> return -ENOMEM;
>>>
>>> + ost->timers = alloc_percpu(struct ingenic_ost_timer);
>>> + if (!ost->timers) {
>>> + ret = -ENOMEM;
>>> + goto err_free_ost;
>>> + }
>>> +
>>> + ost->np = np;
>>> + ost->soc_info = id->data;
>>> +
>>> ost->base = of_io_request_and_map(np, 0,
>>> of_node_full_name(np));
>>> if (IS_ERR(ost->base)) {
>>> pr_err("%s: Failed to map OST registers\n", __func__);
>>> ret = PTR_ERR(ost->base);
>>> - goto err_free_ost;
>>> + goto err_free_timers;
>>> + }
>>> +
>>> + if (ost->soc_info->version >= ID_X2000) {
>>> + base = of_io_request_and_map(np, 1, of_node_full_name(np));
>>> + if (IS_ERR(base)) {
>>> + pr_err("%s: Failed to map OST registers\n", __func__);
>>> + ret = PTR_ERR(base);
>>> + goto err_free_timers;
>>> + }
>>> + }
>>
>> The DT documentation only mentions one memory resource. Here, you
>> map \x7fa second one, which is not used anywhere. I'm really confused
>> about \x7fwhat you're trying to do here.
>>
>
> X2000 and X2100 divide the OST into two parts. The global timer is
> the first part, which is still located at the address of 0x12000000,
> and the percpu timers are the second part, the starting address is
> 0x12100000, and each timer is offset by 0x100 (percpu timer0 is at
> 0x12100000, percpu timer1 is at 0x12100100, percpu timer2 is at
> 0x12100200, percpu timer3 is at 0x12100300). This one is used in
> line 593 of the code.
Then you need two different DT nodes, one at each start address. Either
use different drivers (since the register sets are different), or *if*
it can be implemented cleanly in ingenic-sysost.c, different compatible
strings - one for the global timer and one for the percpu timers.
-Paul
>>> +
>>> + ost->irq = irq_of_parse_and_map(np, 0);
>>> + if (ost->irq < 0) {
>>> + pr_crit("%s: Cannot to get OST IRQ\n", __func__);
>>> + ret = ost->irq;
>>> + goto err_free_timers;
>>> }
>>>
>>> ost->clk = of_clk_get_by_name(np, "ost");
>>> if (IS_ERR(ost->clk)) {
>>> - ret = PTR_ERR(ost->clk);
>>> pr_crit("%s: Cannot get OST clock\n", __func__);
>>> - goto err_free_ost;
>>> + ret = PTR_ERR(ost->clk);
>>> + goto err_free_timers;
>>> }
>>>
>>> ret = clk_prepare_enable(ost->clk);
>>> @@ -450,8 +577,6 @@ static int __init ingenic_ost_probe(struct
>>> \x7f\x7fdevice_node *np)
>>> goto err_put_clk;
>>> }
>>>
>>> - ost->soc_info = id->data;
>>> -
>>> ost->clocks = kzalloc(struct_size(ost->clocks, hws,
>>> \x7f\x7fost->soc_info->num_channels),
>>> GFP_KERNEL);
>>> if (!ost->clocks) {
>>> @@ -461,8 +586,21 @@ static int __init ingenic_ost_probe(struct
>>> \x7f\x7fdevice_node *np)
>>>
>>> ost->clocks->num = ost->soc_info->num_channels;
>>>
>>> - for (i = 0; i < ost->clocks->num; i++) {
>>> - ret = ingenic_ost_register_clock(ost, i,
>>> \x7f\x7f&x1000_ost_clk_info[i], ost->clocks);
>>> + for (cpu = 0; cpu < num_possible_cpus(); cpu++) {
>>> + timer = per_cpu_ptr(ost->timers, cpu);
>>> +
>>> + if (ost->soc_info->version >= ID_X2000)
>>> + timer->base = base + ost->soc_info->base_offset * cpu;
>>> + else
>>> + timer->base = ost->base;
>>> +
>>> + timer->ost = ost;
>>> + timer->cpu = cpu;
>>> + timer->channel = OST_CLK_PERCPU_TIMER + cpu;
>>> + }
>>> +
>>> + for (i = 0; i < num_possible_cpus() + 1; i++) {
>>> + ret = ingenic_ost_register_clock(ost, i,
>>> \x7f\x7f&ost->soc_info->clk_info[i], ost->clocks);
>>> if (ret) {
>>> pr_crit("%s: Cannot register clock %d\n", __func__, i);
>>> goto err_unregister_ost_clocks;
>>> @@ -488,6 +626,8 @@ static int __init ingenic_ost_probe(struct
>>> \x7f\x7fdevice_node *np)
>>> clk_disable_unprepare(ost->clk);
>>> err_put_clk:
>>> clk_put(ost->clk);
>>> +err_free_timers:
>>> + free_percpu(ost->timers);
>>> err_free_ost:
>>> kfree(ost);
>>> return ret;
>>> @@ -513,13 +653,29 @@ static int __init ingenic_ost_init(struct
>>> \x7f\x7fdevice_node *np)
>>>
>>> ret = ingenic_ost_global_timer_init(np, ost);
>>> if (ret) {
>>> - pr_crit("%s: Unable to init global timer: %x\n", __func__,
>>> \x7f\x7fret);
>>> + pr_crit("%s: Unable to init global timer: %d\n", __func__,
>>> \x7f\x7fret);
>>
>> This is a fix, so it needs to be a separate commit with a Fixes: tag.
>
>
> Sure.
>
>
> Thanks and best regards!
>
>
>>
>> Cheers,
>> -Paul
>>
>>> goto err_free_ingenic_ost;
>>> }
>>>
>>> - ret = ingenic_ost_percpu_timer_init(np, ost);
>>> - if (ret)
>>> + if (ost->soc_info->version >= ID_X2000)
>>> + ret = request_percpu_irq(ost->irq, ingenic_ost_cevt_cb,
>>> + "OST percpu timer", ost->timers);
>>> + else
>>> + ret = request_irq(ost->irq, ingenic_ost_cevt_cb,
>>> IRQF_TIMER,
>>> + "OST percpu timer", ost->timers);
>>> +
>>> + if (ret) {
>>> + pr_crit("%s: Unable to request percpu IRQ: %d\n",
>>> __func__, \x7f\x7fret);
>>> + goto err_ost_global_timer_cleanup;
>>> + }
>>> +
>>> + /* Setup clock events on each CPU core */
>>> + ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "Ingenic XBurst:
>>> \x7f\x7fonline",
>>> + ingenic_ost_setup_cevt, NULL);
>>> + if (ret < 0) {
>>> + pr_crit("%s: Unable to init percpu timers: %d\n",
>>> __func__, \x7f\x7fret);
>>> goto err_ost_global_timer_cleanup;
>>> + }
>>>
>>> /* Register the sched_clock at the end as there's no way to
>>> undo \x7f\x7fit */
>>> rate = clk_get_rate(ost->global_timer_clk);
>>> @@ -537,3 +693,4 @@ static int __init ingenic_ost_init(struct
>>> \x7f\x7fdevice_node *np)
>>> }
>>>
>>> TIMER_OF_DECLARE(x1000_ost, "ingenic,x1000-ost",
>>> ingenic_ost_init);
>>> +TIMER_OF_DECLARE(x2000_ost, "ingenic,x2000-ost",
>>> ingenic_ost_init);
>>> --
>>> 2.7.4
>>>
>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] clocksource: Ingenic: Add SMP/SMT support for sysost driver.
2021-06-11 15:44 ` Paul Cercueil
@ 2021-06-12 15:48 ` 周琰杰
2021-06-14 16:42 ` Paul Cercueil
0 siblings, 1 reply; 11+ messages in thread
From: 周琰杰 @ 2021-06-12 15:48 UTC (permalink / raw)
To: Paul Cercueil
Cc: Paul Cercueil, 周琰杰,
daniel.lezcano, tglx, linux-mips, linux-kernel, dongsheng.qiu,
aric.pzqi, rick.tyliu, sihui.liu, jun.jiang, sernia.zhou
Hi Paul,
于 Fri, 11 Jun 2021 16:44:04 +0100
Paul Cercueil <paul@opendingux.net> 写道:
> Hi Zhou,
>
> Le ven., juin 11 2021 at 23:31:57 +0800, Zhou Yanjie
> <zhouyu@wanyeetech.com> a écrit :
> > Hi Paul,
> >
> > On 2021/6/10 下午11:30, Paul Cercueil wrote:
> >> Hi Zhou,
> >>
> >> Le sam., juin 5 2021 at 00:31:46 +0800, 周琰杰 (Zhou Yanjie)
> >> \x7f<zhouyanjie@wanyeetech.com> a écrit :
> >>> The OST in Ingenic XBurst®2 SoCs such as X2000 and X2100, has a
> >>> global
> >>> timer and two or four percpu timers, add support for the percpu
> >>> timers.
> >>>
> >>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> >>> ---
> >>>
> >>> Notes:
> >>> v1->v2:
> >>> 1.Fix bug in ingenic_ost_global_timer_recalc_rate().
> >>> 2.Add a backpointer to the ingenic_ost structure.
> >>> 3.Remove unnecessary spinlock.
> >>> 4.Use "ret = ost->irq" instead "ret = -EINVAL".
> >>> 5.Use "%d" instead "%x" in pr_crit().
> >>
> >> I can't shake the feeling that you are doing way too many things
> >> in \x7fone single commit.
> >>
> >> From what I can see, this commit can be split in 4 patches:
> >>
> >> - Fix the "%x" in pr_crit(),
> >> - Add the global timer support to the X1000,
> >> - Add "ingenic_ost_timer" and update the code to use it,
> >> - Finally add X2000 support.
> >>
> >>
> >
> > Sure.
> >
> >
> >>
> >>> drivers/clocksource/ingenic-sysost.c | 315
> >>> \x7f\x7f++++++++++++++++++++++++++---------
> >>> 1 file changed, 236 insertions(+), 79 deletions(-)
> >>>
> >>> diff --git a/drivers/clocksource/ingenic-sysost.c
> >>> \x7f\x7fb/drivers/clocksource/ingenic-sysost.c
> >>> index a129840..6f080e4 100644
> >>> --- a/drivers/clocksource/ingenic-sysost.c
> >>> +++ b/drivers/clocksource/ingenic-sysost.c
> >>> @@ -4,6 +4,7 @@
> >>> * Copyright (c) 2020 周琰杰 (Zhou Yanjie)
> >>> <zhouyanjie@wanyeetech.com>
> >>> */
> >>>
> >>> +#include <linux/bitfield.h>
> >>> #include <linux/bitops.h>
> >>> #include <linux/clk.h>
> >>> #include <linux/clk-provider.h>
> >>> @@ -13,6 +14,8 @@
> >>> #include <linux/mfd/syscon.h>
> >>> #include <linux/of_address.h>
> >>> #include <linux/of_irq.h>
> >>> +#include <linux/of_platform.h>
> >>> +#include <linux/overflow.h>
> >>> #include <linux/sched_clock.h>
> >>> #include <linux/slab.h>
> >>> #include <linux/syscore_ops.h>
> >>> @@ -21,10 +24,14 @@
> >>>
> >>> /* OST register offsets */
> >>> #define OST_REG_OSTCCR 0x00
> >>> +#define OST_REG_OSTER 0x04
> >>> #define OST_REG_OSTCR 0x08
> >>> #define OST_REG_OSTFR 0x0c
> >>> +#define OST_REG_OSTCNTH 0x0c
> >>> #define OST_REG_OSTMR 0x10
> >>> +#define OST_REG_OSTCNTL 0x10
> >>> #define OST_REG_OST1DFR 0x14
> >>> +#define OST_REG_OSTCNTB 0x14
> >>> #define OST_REG_OST1CNT 0x18
> >>> #define OST_REG_OST2CNTL 0x20
> >>> #define OST_REG_OSTCNT2HBUF 0x24
> >>> @@ -55,13 +62,23 @@
> >>> #define OSTECR_OST1ENC BIT(0)
> >>> #define OSTECR_OST2ENC BIT(1)
> >>>
> >>> +enum ingenic_ost_version {
> >>> + ID_X1000,
> >>> + ID_X2000,
> >>> +};
> >>> +
> >>> struct ingenic_soc_info {
> >>> + enum ingenic_ost_version version;
> >>> + const struct ingenic_ost_clk_info *clk_info;
> >>> +
> >>> unsigned int num_channels;
> >>> + unsigned int base_offset;
> >>> };
> >>>
> >>> struct ingenic_ost_clk_info {
> >>> struct clk_init_data init_data;
> >>> - u8 ostccr_reg;
> >>> + unsigned int idx;
> >>> + u32 ostcntl_reg;
> >>> };
> >>>
> >>> struct ingenic_ost_clk {
> >>> @@ -71,15 +88,27 @@ struct ingenic_ost_clk {
> >>> const struct ingenic_ost_clk_info *info;
> >>> };
> >>>
> >>> +struct ingenic_ost_timer {
> >>> + void __iomem *base;
> >>> + unsigned int cpu;
> >>> + unsigned int channel;
> >>> + struct clock_event_device cevt;
> >>> + struct ingenic_ost *ost;
> >>> + struct clk *clk;
> >>> + char name[20];
> >>> +};
> >>> +
> >>> struct ingenic_ost {
> >>> void __iomem *base;
> >>> const struct ingenic_soc_info *soc_info;
> >>> - struct clk *clk, *percpu_timer_clk, *global_timer_clk;
> >>> - struct clock_event_device cevt;
> >>> + struct clk *clk, *global_timer_clk;
> >>> + struct device_node *np;
> >>> struct clocksource cs;
> >>> - char name[20];
> >>>
> >>> struct clk_hw_onecell_data *clocks;
> >>> + struct ingenic_ost_timer __percpu *timers;
> >>> +
> >>> + int irq;
> >>> };
> >>>
> >>> static struct ingenic_ost *ingenic_ost;
> >>> @@ -94,11 +123,12 @@ static unsigned long
> >>> \x7f\x7fingenic_ost_percpu_timer_recalc_rate(struct clk_hw *hw,
> >>> {
> >>> struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
> >>> const struct ingenic_ost_clk_info *info = ost_clk->info;
> >>> + struct ingenic_ost_timer *timer =
> >>> \x7f\x7fper_cpu_ptr(ost_clk->ost->timers, info->idx);
> >>> unsigned int prescale;
> >>>
> >>> - prescale = readl(ost_clk->ost->base + info->ostccr_reg);
> >>> + prescale = readl(timer->base + OST_REG_OSTCCR);
> >>>
> >>> - prescale = (prescale & OSTCCR_PRESCALE1_MASK) >>
> >>> \x7f\x7fOSTCCR_PRESCALE1_LSB;
> >>> + prescale = FIELD_GET(OSTCCR_PRESCALE1_MASK, prescale);
> >>>
> >>> return parent_rate >> (prescale * 2);
> >>> }
> >>> @@ -108,11 +138,15 @@ static unsigned long
> >>> \x7f\x7fingenic_ost_global_timer_recalc_rate(struct clk_hw *hw,
> >>> {
> >>> struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
> >>> const struct ingenic_ost_clk_info *info = ost_clk->info;
> >>> + struct ingenic_ost_timer *timer =
> >>> \x7f\x7fper_cpu_ptr(ost_clk->ost->timers, info->idx);
> >>> unsigned int prescale;
> >>>
> >>> - prescale = readl(ost_clk->ost->base + info->ostccr_reg);
> >>> + prescale = readl(timer->base + OST_REG_OSTCCR);
> >>>
> >>> - prescale = (prescale & OSTCCR_PRESCALE2_MASK) >>
> >>> \x7f\x7fOSTCCR_PRESCALE2_LSB;
> >>> + if (ost_clk->ost->soc_info->version >= ID_X2000)
> >>> + prescale = FIELD_GET(OSTCCR_PRESCALE1_MASK, prescale);
> >>> + else
> >>> + prescale = FIELD_GET(OSTCCR_PRESCALE2_MASK, prescale);
> >>>
> >>> return parent_rate >> (prescale * 2);
> >>> }
> >>> @@ -147,12 +181,13 @@ static int
> >>> \x7f\x7fingenic_ost_percpu_timer_set_rate(struct clk_hw *hw, unsigned
> >>> long re
> >>> {
> >>> struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
> >>> const struct ingenic_ost_clk_info *info = ost_clk->info;
> >>> + struct ingenic_ost_timer *timer =
> >>> \x7f\x7fper_cpu_ptr(ost_clk->ost->timers, info->idx);
> >>> u8 prescale = ingenic_ost_get_prescale(parent_rate,
> >>> req_rate); int val;
> >>>
> >>> - val = readl(ost_clk->ost->base + info->ostccr_reg);
> >>> + val = readl(timer->base + OST_REG_OSTCCR);
> >>> val = (val & ~OSTCCR_PRESCALE1_MASK) | (prescale <<
> >>> \x7f\x7fOSTCCR_PRESCALE1_LSB);
> >>> - writel(val, ost_clk->ost->base + info->ostccr_reg);
> >>> + writel(val, timer->base + OST_REG_OSTCCR);
> >>>
> >>> return 0;
> >>> }
> >>> @@ -162,12 +197,18 @@ static int
> >>> \x7f\x7fingenic_ost_global_timer_set_rate(struct clk_hw *hw, unsigned
> >>> long re
> >>> {
> >>> struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
> >>> const struct ingenic_ost_clk_info *info = ost_clk->info;
> >>> + struct ingenic_ost_timer *timer =
> >>> \x7f\x7fper_cpu_ptr(ost_clk->ost->timers, info->idx);
> >>> u8 prescale = ingenic_ost_get_prescale(parent_rate,
> >>> req_rate); int val;
> >>>
> >>> - val = readl(ost_clk->ost->base + info->ostccr_reg);
> >>> - val = (val & ~OSTCCR_PRESCALE2_MASK) | (prescale <<
> >>> \x7f\x7fOSTCCR_PRESCALE2_LSB);
> >>> - writel(val, ost_clk->ost->base + info->ostccr_reg);
> >>> + val = readl(timer->base + OST_REG_OSTCCR);
> >>> +
> >>> + if (ost_clk->ost->soc_info->version >= ID_X2000)
> >>> + val = (val & ~OSTCCR_PRESCALE1_MASK) | (prescale <<
> >>> \x7f\x7fOSTCCR_PRESCALE1_LSB);
> >>> + else
> >>> + val = (val & ~OSTCCR_PRESCALE2_MASK) | (prescale <<
> >>> \x7f\x7fOSTCCR_PRESCALE2_LSB);
> >>> +
> >>> + writel(val, timer->base + OST_REG_OSTCCR);
> >>>
> >>> return 0;
> >>> }
> >>> @@ -195,7 +236,42 @@ static const struct ingenic_ost_clk_info
> >>> \x7f\x7fx1000_ost_clk_info[] = {
> >>> .ops = &ingenic_ost_percpu_timer_ops,
> >>> .flags = CLK_SET_RATE_UNGATE,
> >>> },
> >>> - .ostccr_reg = OST_REG_OSTCCR,
> >>> + .idx = 0,
> >>> + },
> >>> +
> >>> + [OST_CLK_GLOBAL_TIMER] = {
> >>> + .init_data = {
> >>> + .name = "global timer",
> >>> + .parent_names = ingenic_ost_clk_parents,
> >>> + .num_parents = ARRAY_SIZE(ingenic_ost_clk_parents),
> >>> + .ops = &ingenic_ost_global_timer_ops,
> >>> + .flags = CLK_SET_RATE_UNGATE,
> >>> + },
> >>> + .ostcntl_reg = OST_REG_OST2CNTL,
> >>> + },
> >>> +};
> >>> +
> >>> +static const struct ingenic_ost_clk_info x2000_ost_clk_info[] = {
> >>> + [OST_CLK_PERCPU_TIMER0] = {
> >>> + .init_data = {
> >>> + .name = "percpu timer0",
> >>> + .parent_names = ingenic_ost_clk_parents,
> >>> + .num_parents = ARRAY_SIZE(ingenic_ost_clk_parents),
> >>> + .ops = &ingenic_ost_percpu_timer_ops,
> >>> + .flags = CLK_SET_RATE_UNGATE,
> >>> + },
> >>> + .idx = 0,
> >>> + },
> >>> +
> >>> + [OST_CLK_PERCPU_TIMER1] = {
> >>> + .init_data = {
> >>> + .name = "percpu timer1",
> >>> + .parent_names = ingenic_ost_clk_parents,
> >>> + .num_parents = ARRAY_SIZE(ingenic_ost_clk_parents),
> >>> + .ops = &ingenic_ost_percpu_timer_ops,
> >>> + .flags = CLK_SET_RATE_UNGATE,
> >>> + },
> >>> + .idx = 1,
> >>> },
> >>>
> >>> [OST_CLK_GLOBAL_TIMER] = {
> >>> @@ -206,7 +282,7 @@ static const struct ingenic_ost_clk_info
> >>> \x7f\x7fx1000_ost_clk_info[] = {
> >>> .ops = &ingenic_ost_global_timer_ops,
> >>> .flags = CLK_SET_RATE_UNGATE,
> >>> },
> >>> - .ostccr_reg = OST_REG_OSTCCR,
> >>> + .ostcntl_reg = OST_REG_OSTCNTL,
> >>> },
> >>> };
> >>>
> >>> @@ -215,7 +291,7 @@ static u64 notrace
> >>> \x7f\x7fingenic_ost_global_timer_read_cntl(void)
> >>> struct ingenic_ost *ost = ingenic_ost;
> >>> unsigned int count;
> >>>
> >>> - count = readl(ost->base + OST_REG_OST2CNTL);
> >>> + count = readl(ost->base +
> >>> ost->soc_info->clk_info->ostcntl_reg);
> >>>
> >>> return count;
> >>> }
> >>> @@ -225,16 +301,21 @@ static u64 notrace
> >>> \x7f\x7fingenic_ost_clocksource_read(struct clocksource *cs)
> >>> return ingenic_ost_global_timer_read_cntl();
> >>> }
> >>>
> >>> -static inline struct ingenic_ost *to_ingenic_ost(struct
> >>> \x7f\x7fclock_event_device *evt)
> >>> +static inline struct ingenic_ost_timer *
> >>> +to_ingenic_ost_timer(struct clock_event_device *evt)
> >>> {
> >>> - return container_of(evt, struct ingenic_ost, cevt);
> >>> + return container_of(evt, struct ingenic_ost_timer, cevt);
> >>> }
> >>>
> >>> static int ingenic_ost_cevt_set_state_shutdown(struct
> >>> \x7f\x7fclock_event_device *evt)
> >>> {
> >>> - struct ingenic_ost *ost = to_ingenic_ost(evt);
> >>> + struct ingenic_ost_timer *timer = to_ingenic_ost_timer(evt);
> >>> + struct ingenic_ost *ost = timer->ost;
> >>>
> >>> - writel(OSTECR_OST1ENC, ost->base + OST_REG_OSTECR);
> >>> + if (ost->soc_info->version >= ID_X2000)
> >>> + writel(0, timer->base + OST_REG_OSTER);
> >>> + else
> >>> + writel(OSTECR_OST1ENC, timer->base + OST_REG_OSTECR);
> >>>
> >>> return 0;
> >>> }
> >>> @@ -242,26 +323,34 @@ static int
> >>> \x7f\x7fingenic_ost_cevt_set_state_shutdown(struct clock_event_device
> >>> *evt)
> >>> static int ingenic_ost_cevt_set_next(unsigned long next,
> >>> struct clock_event_device *evt)
> >>> {
> >>> - struct ingenic_ost *ost = to_ingenic_ost(evt);
> >>> -
> >>> - writel((u32)~OSTFR_FFLAG, ost->base + OST_REG_OSTFR);
> >>> - writel(next, ost->base + OST_REG_OST1DFR);
> >>> - writel(OSTCR_OST1CLR, ost->base + OST_REG_OSTCR);
> >>> - writel(OSTESR_OST1ENS, ost->base + OST_REG_OSTESR);
> >>> - writel((u32)~OSTMR_FMASK, ost->base + OST_REG_OSTMR);
> >>> + struct ingenic_ost_timer *timer = to_ingenic_ost_timer(evt);
> >>> + struct ingenic_ost *ost = timer->ost;
> >>> +
> >>> + writel((u32)~OSTFR_FFLAG, timer->base + OST_REG_OSTFR);
> >>> + writel(next, timer->base + OST_REG_OST1DFR);
> >>> + writel(OSTCR_OST1CLR, timer->base + OST_REG_OSTCR);
> >>> +
> >>> + if (ost->soc_info->version >= ID_X2000) {
> >>> + writel(OSTESR_OST1ENS, timer->base + OST_REG_OSTER);
> >>> + } else {
> >>> + writel(OSTESR_OST1ENS, timer->base + OST_REG_OSTESR);
> >>> + writel((u32)~OSTMR_FMASK, timer->base + OST_REG_OSTMR);
> >>> + }
> >>>
> >>> return 0;
> >>> }
> >>>
> >>> static irqreturn_t ingenic_ost_cevt_cb(int irq, void *dev_id)
> >>> {
> >>> - struct clock_event_device *evt = dev_id;
> >>> - struct ingenic_ost *ost = to_ingenic_ost(evt);
> >>> + struct ingenic_ost_timer *timer = dev_id;
> >>> + struct ingenic_ost *ost = timer->ost;
> >>>
> >>> - writel(OSTECR_OST1ENC, ost->base + OST_REG_OSTECR);
> >>> + if (ost->soc_info->version >= ID_X2000)
> >>> + writel(0, timer->base + OST_REG_OSTER);
> >>> + else
> >>> + writel(OSTECR_OST1ENC, timer->base + OST_REG_OSTECR);
> >>>
> >>> - if (evt->event_handler)
> >>> - evt->event_handler(evt);
> >>> + timer->cevt.event_handler(&timer->cevt);
> >>>
> >>> return IRQ_HANDLED;
> >>> }
> >>> @@ -271,6 +360,7 @@ static int __init
> >>> \x7f\x7fingenic_ost_register_clock(struct ingenic_ost *ost,
> >>> struct clk_hw_onecell_data *clocks)
> >>> {
> >>> struct ingenic_ost_clk *ost_clk;
> >>> + struct ingenic_ost_timer *timer = per_cpu_ptr(ost->timers,
> >>> \x7f\x7finfo->idx);
> >>> int val, err;
> >>>
> >>> ost_clk = kzalloc(sizeof(*ost_clk), GFP_KERNEL);
> >>> @@ -283,9 +373,9 @@ static int __init
> >>> \x7f\x7fingenic_ost_register_clock(struct ingenic_ost *ost,
> >>> ost_clk->ost = ost;
> >>>
> >>> /* Reset clock divider */
> >>> - val = readl(ost->base + info->ostccr_reg);
> >>> - val &= ~(OSTCCR_PRESCALE1_MASK | OSTCCR_PRESCALE2_MASK);
> >>> - writel(val, ost->base + info->ostccr_reg);
> >>> + val = readl(timer->base + OST_REG_OSTCCR);
> >>> + val &= ~(OSTCCR_PRESCALE1_MASK);
> >>> + writel(val, timer->base + OST_REG_OSTCCR);
> >>>
> >>> err = clk_hw_register(NULL, &ost_clk->hw);
> >>> if (err) {
> >>> @@ -309,57 +399,51 @@ static struct clk * __init
> >>> \x7f\x7fingenic_ost_get_clock(struct device_node *np, int id)
> >>> return of_clk_get_from_provider(&args);
> >>> }
> >>>
> >>> -static int __init ingenic_ost_percpu_timer_init(struct
> >>> device_node *np,
> >>> - struct ingenic_ost *ost)
> >>> +static int __init ingenic_ost_setup_cevt(unsigned int cpu)
> >>> {
> >>> - unsigned int timer_virq, channel = OST_CLK_PERCPU_TIMER;
> >>> + struct ingenic_ost *ost = ingenic_ost;
> >>> + struct ingenic_ost_timer *timer = this_cpu_ptr(ost->timers);
> >>> unsigned long rate;
> >>> int err;
> >>>
> >>> - ost->percpu_timer_clk = ingenic_ost_get_clock(np, channel);
> >>> - if (IS_ERR(ost->percpu_timer_clk))
> >>> - return PTR_ERR(ost->percpu_timer_clk);
> >>> + timer->clk = ingenic_ost_get_clock(ost->np, timer->channel);
> >>> + if (IS_ERR(timer->clk))
> >>> + return PTR_ERR(timer->clk);
> >>>
> >>> - err = clk_prepare_enable(ost->percpu_timer_clk);
> >>> + err = clk_prepare_enable(timer->clk);
> >>> if (err)
> >>> goto err_clk_put;
> >>>
> >>> - rate = clk_get_rate(ost->percpu_timer_clk);
> >>> + rate = clk_get_rate(timer->clk);
> >>> if (!rate) {
> >>> err = -EINVAL;
> >>> goto err_clk_disable;
> >>> }
> >>>
> >>> - timer_virq = of_irq_get(np, 0);
> >>> - if (!timer_virq) {
> >>> - err = -EINVAL;
> >>> - goto err_clk_disable;
> >>> - }
> >>> + snprintf(timer->name, sizeof(timer->name), "OST percpu
> >>> timer%u", \x7f\x7fcpu);
> >>>
> >>> - snprintf(ost->name, sizeof(ost->name), "OST percpu timer");
> >>> + /* Unmask full comparison match interrupt */
> >>> + writel((u32)~OSTMR_FMASK, timer->base + OST_REG_OSTMR);
> >>>
> >>> - err = request_irq(timer_virq, ingenic_ost_cevt_cb,
> >>> IRQF_TIMER,
> >>> - ost->name, &ost->cevt);
> >>> - if (err)
> >>> - goto err_irq_dispose_mapping;
> >>> + timer->cpu = smp_processor_id();
> >>> + timer->cevt.cpumask = cpumask_of(smp_processor_id());
> >>> + timer->cevt.features = CLOCK_EVT_FEAT_ONESHOT;
> >>> + timer->cevt.name = timer->name;
> >>> + timer->cevt.rating = 400;
> >>> + timer->cevt.set_state_shutdown =
> >>> \x7f\x7fingenic_ost_cevt_set_state_shutdown;
> >>> + timer->cevt.set_next_event = ingenic_ost_cevt_set_next;
> >>>
> >>> - ost->cevt.cpumask = cpumask_of(smp_processor_id());
> >>> - ost->cevt.features = CLOCK_EVT_FEAT_ONESHOT;
> >>> - ost->cevt.name = ost->name;
> >>> - ost->cevt.rating = 400;
> >>> - ost->cevt.set_state_shutdown =
> >>> ingenic_ost_cevt_set_state_shutdown;
> >>> - ost->cevt.set_next_event = ingenic_ost_cevt_set_next;
> >>> + clockevents_config_and_register(&timer->cevt, rate, 4,
> >>> 0xffffffff);
> >>>
> >>> - clockevents_config_and_register(&ost->cevt, rate, 4,
> >>> 0xffffffff);
> >>> + if (ost->soc_info->version >= ID_X2000)
> >>> + enable_percpu_irq(ost->irq, IRQ_TYPE_NONE);
> >>>
> >>> return 0;
> >>>
> >>> -err_irq_dispose_mapping:
> >>> - irq_dispose_mapping(timer_virq);
> >>> err_clk_disable:
> >>> - clk_disable_unprepare(ost->percpu_timer_clk);
> >>> + clk_disable_unprepare(timer->clk);
> >>> err_clk_put:
> >>> - clk_put(ost->percpu_timer_clk);
> >>> + clk_put(timer->clk);
> >>> return err;
> >>> }
> >>>
> >>> @@ -385,11 +469,14 @@ static int __init
> >>> \x7f\x7fingenic_ost_global_timer_init(struct device_node *np,
> >>> goto err_clk_disable;
> >>> }
> >>>
> >>> - /* Clear counter CNT registers */
> >>> - writel(OSTCR_OST2CLR, ost->base + OST_REG_OSTCR);
> >>> -
> >>> - /* Enable OST channel */
> >>> - writel(OSTESR_OST2ENS, ost->base + OST_REG_OSTESR);
> >>> + /* Clear counter CNT registers and enable OST channel */
> >>> + if (ost->soc_info->version >= ID_X2000) {
> >>> + writel(OSTCR_OST1CLR, ost->base + OST_REG_OSTCR);
> >>> + writel(OSTESR_OST1ENS, ost->base + OST_REG_OSTER);
> >>> + } else {
> >>> + writel(OSTCR_OST2CLR, ost->base + OST_REG_OSTCR);
> >>> + writel(OSTESR_OST2ENS, ost->base + OST_REG_OSTESR);
> >>> + }
> >>>
> >>> cs->name = "ingenic-ost";
> >>> cs->rating = 400;
> >>> @@ -411,18 +498,33 @@ static int __init
> >>> \x7f\x7fingenic_ost_global_timer_init(struct device_node *np,
> >>> }
> >>>
> >>> static const struct ingenic_soc_info x1000_soc_info = {
> >>> + .version = ID_X1000,
> >>> + .clk_info = x1000_ost_clk_info,
> >>> +
> >>> .num_channels = 2,
> >>> };
> >>>
> >>> +static const struct ingenic_soc_info x2000_soc_info = {
> >>> + .version = ID_X2000,
> >>> + .clk_info = x2000_ost_clk_info,
> >>> +
> >>> + .num_channels = 3,
> >>> + .base_offset = 0x100,
> >>> +};
> >>> +
> >>> static const struct of_device_id __maybe_unused
> >>> \x7f\x7fingenic_ost_of_matches[] __initconst = {
> >>> { .compatible = "ingenic,x1000-ost", .data =
> >>> &x1000_soc_info },
> >>> + { .compatible = "ingenic,x2000-ost", .data =
> >>> &x2000_soc_info }, { /* sentinel */ }
> >>> };
> >>>
> >>> static int __init ingenic_ost_probe(struct device_node *np)
> >>> {
> >>> const struct of_device_id *id =
> >>> \x7f\x7fof_match_node(ingenic_ost_of_matches, np);
> >>> + struct ingenic_ost_timer *timer;
> >>> struct ingenic_ost *ost;
> >>> + void __iomem *base;
> >>> + unsigned int cpu;
> >>> unsigned int i;
> >>> int ret;
> >>>
> >>> @@ -430,18 +532,43 @@ static int __init ingenic_ost_probe(struct
> >>> \x7f\x7fdevice_node *np)
> >>> if (!ost)
> >>> return -ENOMEM;
> >>>
> >>> + ost->timers = alloc_percpu(struct ingenic_ost_timer);
> >>> + if (!ost->timers) {
> >>> + ret = -ENOMEM;
> >>> + goto err_free_ost;
> >>> + }
> >>> +
> >>> + ost->np = np;
> >>> + ost->soc_info = id->data;
> >>> +
> >>> ost->base = of_io_request_and_map(np, 0,
> >>> of_node_full_name(np));
> >>> if (IS_ERR(ost->base)) {
> >>> pr_err("%s: Failed to map OST registers\n", __func__);
> >>> ret = PTR_ERR(ost->base);
> >>> - goto err_free_ost;
> >>> + goto err_free_timers;
> >>> + }
> >>> +
> >>> + if (ost->soc_info->version >= ID_X2000) {
> >>> + base = of_io_request_and_map(np, 1,
> >>> of_node_full_name(np));
> >>> + if (IS_ERR(base)) {
> >>> + pr_err("%s: Failed to map OST registers\n",
> >>> __func__);
> >>> + ret = PTR_ERR(base);
> >>> + goto err_free_timers;
> >>> + }
> >>> + }
> >>
> >> The DT documentation only mentions one memory resource. Here, you
> >> map \x7fa second one, which is not used anywhere. I'm really confused
> >> about \x7fwhat you're trying to do here.
> >>
> >
> > X2000 and X2100 divide the OST into two parts. The global timer is
> > the first part, which is still located at the address of 0x12000000,
> > and the percpu timers are the second part, the starting address is
> > 0x12100000, and each timer is offset by 0x100 (percpu timer0 is at
> > 0x12100000, percpu timer1 is at 0x12100100, percpu timer2 is at
> > 0x12100200, percpu timer3 is at 0x12100300). This one is used in
> > line 593 of the code.
>
> Then you need two different DT nodes, one at each start address.
> Either use different drivers (since the register sets are different),
> or *if* it can be implemented cleanly in ingenic-sysost.c, different
> compatible strings - one for the global timer and one for the percpu
> timers.
Sorry, I did not describe it clearly. Although the global timer and
percpu timers are divided into two parts, they still belong to the same
hardware module. The base address of the entire module is 0x12000000,
but the control register of the global timer is not shifted. The percpu
timers are shifted by 0x100000 as a whole (the situation here is
similar to PDMA, which is also divided into two parts: the
channel-related registers are not shifted, while the system
control-related registers are shifted by 0x1000 as a whole). I think
maybe we can follow PDMA's approach and add corresponding instructions
in the DT documentation. This can avoid confusion caused by splitting
different parts of the same hardware module into two nodes, and at the
same time keep the code of the devicetree as simple and clear as
possible.
What do you think about that?
Thanks and best regards!
>
> -Paul
>
> >>> +
> >>> + ost->irq = irq_of_parse_and_map(np, 0);
> >>> + if (ost->irq < 0) {
> >>> + pr_crit("%s: Cannot to get OST IRQ\n", __func__);
> >>> + ret = ost->irq;
> >>> + goto err_free_timers;
> >>> }
> >>>
> >>> ost->clk = of_clk_get_by_name(np, "ost");
> >>> if (IS_ERR(ost->clk)) {
> >>> - ret = PTR_ERR(ost->clk);
> >>> pr_crit("%s: Cannot get OST clock\n", __func__);
> >>> - goto err_free_ost;
> >>> + ret = PTR_ERR(ost->clk);
> >>> + goto err_free_timers;
> >>> }
> >>>
> >>> ret = clk_prepare_enable(ost->clk);
> >>> @@ -450,8 +577,6 @@ static int __init ingenic_ost_probe(struct
> >>> \x7f\x7fdevice_node *np)
> >>> goto err_put_clk;
> >>> }
> >>>
> >>> - ost->soc_info = id->data;
> >>> -
> >>> ost->clocks = kzalloc(struct_size(ost->clocks, hws,
> >>> \x7f\x7fost->soc_info->num_channels),
> >>> GFP_KERNEL);
> >>> if (!ost->clocks) {
> >>> @@ -461,8 +586,21 @@ static int __init ingenic_ost_probe(struct
> >>> \x7f\x7fdevice_node *np)
> >>>
> >>> ost->clocks->num = ost->soc_info->num_channels;
> >>>
> >>> - for (i = 0; i < ost->clocks->num; i++) {
> >>> - ret = ingenic_ost_register_clock(ost, i,
> >>> \x7f\x7f&x1000_ost_clk_info[i], ost->clocks);
> >>> + for (cpu = 0; cpu < num_possible_cpus(); cpu++) {
> >>> + timer = per_cpu_ptr(ost->timers, cpu);
> >>> +
> >>> + if (ost->soc_info->version >= ID_X2000)
> >>> + timer->base = base + ost->soc_info->base_offset *
> >>> cpu;
> >>> + else
> >>> + timer->base = ost->base;
> >>> +
> >>> + timer->ost = ost;
> >>> + timer->cpu = cpu;
> >>> + timer->channel = OST_CLK_PERCPU_TIMER + cpu;
> >>> + }
> >>> +
> >>> + for (i = 0; i < num_possible_cpus() + 1; i++) {
> >>> + ret = ingenic_ost_register_clock(ost, i,
> >>> \x7f\x7f&ost->soc_info->clk_info[i], ost->clocks);
> >>> if (ret) {
> >>> pr_crit("%s: Cannot register clock %d\n", __func__,
> >>> i); goto err_unregister_ost_clocks;
> >>> @@ -488,6 +626,8 @@ static int __init ingenic_ost_probe(struct
> >>> \x7f\x7fdevice_node *np)
> >>> clk_disable_unprepare(ost->clk);
> >>> err_put_clk:
> >>> clk_put(ost->clk);
> >>> +err_free_timers:
> >>> + free_percpu(ost->timers);
> >>> err_free_ost:
> >>> kfree(ost);
> >>> return ret;
> >>> @@ -513,13 +653,29 @@ static int __init ingenic_ost_init(struct
> >>> \x7f\x7fdevice_node *np)
> >>>
> >>> ret = ingenic_ost_global_timer_init(np, ost);
> >>> if (ret) {
> >>> - pr_crit("%s: Unable to init global timer: %x\n",
> >>> __func__, \x7f\x7fret);
> >>> + pr_crit("%s: Unable to init global timer: %d\n",
> >>> __func__, \x7f\x7fret);
> >>
> >> This is a fix, so it needs to be a separate commit with a Fixes:
> >> tag.
> >
> >
> > Sure.
> >
> >
> > Thanks and best regards!
> >
> >
> >>
> >> Cheers,
> >> -Paul
> >>
> >>> goto err_free_ingenic_ost;
> >>> }
> >>>
> >>> - ret = ingenic_ost_percpu_timer_init(np, ost);
> >>> - if (ret)
> >>> + if (ost->soc_info->version >= ID_X2000)
> >>> + ret = request_percpu_irq(ost->irq, ingenic_ost_cevt_cb,
> >>> + "OST percpu timer", ost->timers);
> >>> + else
> >>> + ret = request_irq(ost->irq, ingenic_ost_cevt_cb,
> >>> IRQF_TIMER,
> >>> + "OST percpu timer", ost->timers);
> >>> +
> >>> + if (ret) {
> >>> + pr_crit("%s: Unable to request percpu IRQ: %d\n",
> >>> __func__, \x7f\x7fret);
> >>> + goto err_ost_global_timer_cleanup;
> >>> + }
> >>> +
> >>> + /* Setup clock events on each CPU core */
> >>> + ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "Ingenic
> >>> XBurst: \x7f\x7fonline",
> >>> + ingenic_ost_setup_cevt, NULL);
> >>> + if (ret < 0) {
> >>> + pr_crit("%s: Unable to init percpu timers: %d\n",
> >>> __func__, \x7f\x7fret);
> >>> goto err_ost_global_timer_cleanup;
> >>> + }
> >>>
> >>> /* Register the sched_clock at the end as there's no way to
> >>> undo \x7f\x7fit */
> >>> rate = clk_get_rate(ost->global_timer_clk);
> >>> @@ -537,3 +693,4 @@ static int __init ingenic_ost_init(struct
> >>> \x7f\x7fdevice_node *np)
> >>> }
> >>>
> >>> TIMER_OF_DECLARE(x1000_ost, "ingenic,x1000-ost",
> >>> ingenic_ost_init);
> >>> +TIMER_OF_DECLARE(x2000_ost, "ingenic,x2000-ost",
> >>> ingenic_ost_init);
> >>> --
> >>> 2.7.4
> >>>
> >>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] clocksource: Ingenic: Add SMP/SMT support for sysost driver.
2021-06-12 15:48 ` 周琰杰
@ 2021-06-14 16:42 ` Paul Cercueil
2021-06-16 14:57 ` Daniel Lezcano
0 siblings, 1 reply; 11+ messages in thread
From: Paul Cercueil @ 2021-06-14 16:42 UTC (permalink / raw)
To: 周琰杰
Cc: Paul Cercueil, daniel.lezcano, tglx, linux-mips, linux-kernel,
dongsheng.qiu, aric.pzqi, rick.tyliu, sihui.liu, jun.jiang,
sernia.zhou
Hi Zhou,
Le sam., juin 12 2021 at 23:48:38 +0800, 周琰杰
<zhouyanjie@wanyeetech.com> a écrit :
> Hi Paul,
>
> 于 Fri, 11 Jun 2021 16:44:04 +0100
> Paul Cercueil <paul@opendingux.net> 写道:
>
>> Hi Zhou,
>>
>> Le ven., juin 11 2021 at 23:31:57 +0800, Zhou Yanjie
>> <zhouyu@wanyeetech.com> a écrit :
>> > Hi Paul,
>> >
>> > On 2021/6/10 下午11:30, Paul Cercueil wrote:
>> >> Hi Zhou,
>> >>
>> >> Le sam., juin 5 2021 at 00:31:46 +0800, 周琰杰 (Zhou Yanjie)
>> >> \x7f<zhouyanjie@wanyeetech.com> a écrit :
>> >>> The OST in Ingenic XBurst®2 SoCs such as X2000 and X2100, has a
>> >>> global
>> >>> timer and two or four percpu timers, add support for the percpu
>> >>> timers.
>> >>>
>> >>> Signed-off-by: 周琰杰 (Zhou Yanjie)
>> <zhouyanjie@wanyeetech.com>
>> >>> ---
>> >>>
>> >>> Notes:
>> >>> v1->v2:
>> >>> 1.Fix bug in ingenic_ost_global_timer_recalc_rate().
>> >>> 2.Add a backpointer to the ingenic_ost structure.
>> >>> 3.Remove unnecessary spinlock.
>> >>> 4.Use "ret = ost->irq" instead "ret = -EINVAL".
>> >>> 5.Use "%d" instead "%x" in pr_crit().
>> >>
>> >> I can't shake the feeling that you are doing way too many things
>> >> in \x7fone single commit.
>> >>
>> >> From what I can see, this commit can be split in 4 patches:
>> >>
>> >> - Fix the "%x" in pr_crit(),
>> >> - Add the global timer support to the X1000,
>> >> - Add "ingenic_ost_timer" and update the code to use it,
>> >> - Finally add X2000 support.
>> >>
>> >>
>> >
>> > Sure.
>> >
>> >
>> >>
>> >>> drivers/clocksource/ingenic-sysost.c | 315
>> >>> \x7f\x7f++++++++++++++++++++++++++---------
>> >>> 1 file changed, 236 insertions(+), 79 deletions(-)
>> >>>
>> >>> diff --git a/drivers/clocksource/ingenic-sysost.c
>> >>> \x7f\x7fb/drivers/clocksource/ingenic-sysost.c
>> >>> index a129840..6f080e4 100644
>> >>> --- a/drivers/clocksource/ingenic-sysost.c
>> >>> +++ b/drivers/clocksource/ingenic-sysost.c
>> >>> @@ -4,6 +4,7 @@
>> >>> * Copyright (c) 2020 周琰杰 (Zhou Yanjie)
>> >>> <zhouyanjie@wanyeetech.com>
>> >>> */
>> >>>
>> >>> +#include <linux/bitfield.h>
>> >>> #include <linux/bitops.h>
>> >>> #include <linux/clk.h>
>> >>> #include <linux/clk-provider.h>
>> >>> @@ -13,6 +14,8 @@
>> >>> #include <linux/mfd/syscon.h>
>> >>> #include <linux/of_address.h>
>> >>> #include <linux/of_irq.h>
>> >>> +#include <linux/of_platform.h>
>> >>> +#include <linux/overflow.h>
>> >>> #include <linux/sched_clock.h>
>> >>> #include <linux/slab.h>
>> >>> #include <linux/syscore_ops.h>
>> >>> @@ -21,10 +24,14 @@
>> >>>
>> >>> /* OST register offsets */
>> >>> #define OST_REG_OSTCCR 0x00
>> >>> +#define OST_REG_OSTER 0x04
>> >>> #define OST_REG_OSTCR 0x08
>> >>> #define OST_REG_OSTFR 0x0c
>> >>> +#define OST_REG_OSTCNTH 0x0c
>> >>> #define OST_REG_OSTMR 0x10
>> >>> +#define OST_REG_OSTCNTL 0x10
>> >>> #define OST_REG_OST1DFR 0x14
>> >>> +#define OST_REG_OSTCNTB 0x14
>> >>> #define OST_REG_OST1CNT 0x18
>> >>> #define OST_REG_OST2CNTL 0x20
>> >>> #define OST_REG_OSTCNT2HBUF 0x24
>> >>> @@ -55,13 +62,23 @@
>> >>> #define OSTECR_OST1ENC BIT(0)
>> >>> #define OSTECR_OST2ENC BIT(1)
>> >>>
>> >>> +enum ingenic_ost_version {
>> >>> + ID_X1000,
>> >>> + ID_X2000,
>> >>> +};
>> >>> +
>> >>> struct ingenic_soc_info {
>> >>> + enum ingenic_ost_version version;
>> >>> + const struct ingenic_ost_clk_info *clk_info;
>> >>> +
>> >>> unsigned int num_channels;
>> >>> + unsigned int base_offset;
>> >>> };
>> >>>
>> >>> struct ingenic_ost_clk_info {
>> >>> struct clk_init_data init_data;
>> >>> - u8 ostccr_reg;
>> >>> + unsigned int idx;
>> >>> + u32 ostcntl_reg;
>> >>> };
>> >>>
>> >>> struct ingenic_ost_clk {
>> >>> @@ -71,15 +88,27 @@ struct ingenic_ost_clk {
>> >>> const struct ingenic_ost_clk_info *info;
>> >>> };
>> >>>
>> >>> +struct ingenic_ost_timer {
>> >>> + void __iomem *base;
>> >>> + unsigned int cpu;
>> >>> + unsigned int channel;
>> >>> + struct clock_event_device cevt;
>> >>> + struct ingenic_ost *ost;
>> >>> + struct clk *clk;
>> >>> + char name[20];
>> >>> +};
>> >>> +
>> >>> struct ingenic_ost {
>> >>> void __iomem *base;
>> >>> const struct ingenic_soc_info *soc_info;
>> >>> - struct clk *clk, *percpu_timer_clk, *global_timer_clk;
>> >>> - struct clock_event_device cevt;
>> >>> + struct clk *clk, *global_timer_clk;
>> >>> + struct device_node *np;
>> >>> struct clocksource cs;
>> >>> - char name[20];
>> >>>
>> >>> struct clk_hw_onecell_data *clocks;
>> >>> + struct ingenic_ost_timer __percpu *timers;
>> >>> +
>> >>> + int irq;
>> >>> };
>> >>>
>> >>> static struct ingenic_ost *ingenic_ost;
>> >>> @@ -94,11 +123,12 @@ static unsigned long
>> >>> \x7f\x7fingenic_ost_percpu_timer_recalc_rate(struct clk_hw *hw,
>> >>> {
>> >>> struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
>> >>> const struct ingenic_ost_clk_info *info = ost_clk->info;
>> >>> + struct ingenic_ost_timer *timer =
>> >>> \x7f\x7fper_cpu_ptr(ost_clk->ost->timers, info->idx);
>> >>> unsigned int prescale;
>> >>>
>> >>> - prescale = readl(ost_clk->ost->base + info->ostccr_reg);
>> >>> + prescale = readl(timer->base + OST_REG_OSTCCR);
>> >>>
>> >>> - prescale = (prescale & OSTCCR_PRESCALE1_MASK) >>
>> >>> \x7f\x7fOSTCCR_PRESCALE1_LSB;
>> >>> + prescale = FIELD_GET(OSTCCR_PRESCALE1_MASK, prescale);
>> >>>
>> >>> return parent_rate >> (prescale * 2);
>> >>> }
>> >>> @@ -108,11 +138,15 @@ static unsigned long
>> >>> \x7f\x7fingenic_ost_global_timer_recalc_rate(struct clk_hw *hw,
>> >>> {
>> >>> struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
>> >>> const struct ingenic_ost_clk_info *info = ost_clk->info;
>> >>> + struct ingenic_ost_timer *timer =
>> >>> \x7f\x7fper_cpu_ptr(ost_clk->ost->timers, info->idx);
>> >>> unsigned int prescale;
>> >>>
>> >>> - prescale = readl(ost_clk->ost->base + info->ostccr_reg);
>> >>> + prescale = readl(timer->base + OST_REG_OSTCCR);
>> >>>
>> >>> - prescale = (prescale & OSTCCR_PRESCALE2_MASK) >>
>> >>> \x7f\x7fOSTCCR_PRESCALE2_LSB;
>> >>> + if (ost_clk->ost->soc_info->version >= ID_X2000)
>> >>> + prescale = FIELD_GET(OSTCCR_PRESCALE1_MASK, prescale);
>> >>> + else
>> >>> + prescale = FIELD_GET(OSTCCR_PRESCALE2_MASK, prescale);
>> >>>
>> >>> return parent_rate >> (prescale * 2);
>> >>> }
>> >>> @@ -147,12 +181,13 @@ static int
>> >>> \x7f\x7fingenic_ost_percpu_timer_set_rate(struct clk_hw *hw, unsigned
>> >>> long re
>> >>> {
>> >>> struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
>> >>> const struct ingenic_ost_clk_info *info = ost_clk->info;
>> >>> + struct ingenic_ost_timer *timer =
>> >>> \x7f\x7fper_cpu_ptr(ost_clk->ost->timers, info->idx);
>> >>> u8 prescale = ingenic_ost_get_prescale(parent_rate,
>> >>> req_rate); int val;
>> >>>
>> >>> - val = readl(ost_clk->ost->base + info->ostccr_reg);
>> >>> + val = readl(timer->base + OST_REG_OSTCCR);
>> >>> val = (val & ~OSTCCR_PRESCALE1_MASK) | (prescale <<
>> >>> \x7f\x7fOSTCCR_PRESCALE1_LSB);
>> >>> - writel(val, ost_clk->ost->base + info->ostccr_reg);
>> >>> + writel(val, timer->base + OST_REG_OSTCCR);
>> >>>
>> >>> return 0;
>> >>> }
>> >>> @@ -162,12 +197,18 @@ static int
>> >>> \x7f\x7fingenic_ost_global_timer_set_rate(struct clk_hw *hw, unsigned
>> >>> long re
>> >>> {
>> >>> struct ingenic_ost_clk *ost_clk = to_ost_clk(hw);
>> >>> const struct ingenic_ost_clk_info *info = ost_clk->info;
>> >>> + struct ingenic_ost_timer *timer =
>> >>> \x7f\x7fper_cpu_ptr(ost_clk->ost->timers, info->idx);
>> >>> u8 prescale = ingenic_ost_get_prescale(parent_rate,
>> >>> req_rate); int val;
>> >>>
>> >>> - val = readl(ost_clk->ost->base + info->ostccr_reg);
>> >>> - val = (val & ~OSTCCR_PRESCALE2_MASK) | (prescale <<
>> >>> \x7f\x7fOSTCCR_PRESCALE2_LSB);
>> >>> - writel(val, ost_clk->ost->base + info->ostccr_reg);
>> >>> + val = readl(timer->base + OST_REG_OSTCCR);
>> >>> +
>> >>> + if (ost_clk->ost->soc_info->version >= ID_X2000)
>> >>> + val = (val & ~OSTCCR_PRESCALE1_MASK) | (prescale <<
>> >>> \x7f\x7fOSTCCR_PRESCALE1_LSB);
>> >>> + else
>> >>> + val = (val & ~OSTCCR_PRESCALE2_MASK) | (prescale <<
>> >>> \x7f\x7fOSTCCR_PRESCALE2_LSB);
>> >>> +
>> >>> + writel(val, timer->base + OST_REG_OSTCCR);
>> >>>
>> >>> return 0;
>> >>> }
>> >>> @@ -195,7 +236,42 @@ static const struct ingenic_ost_clk_info
>> >>> \x7f\x7fx1000_ost_clk_info[] = {
>> >>> .ops = &ingenic_ost_percpu_timer_ops,
>> >>> .flags = CLK_SET_RATE_UNGATE,
>> >>> },
>> >>> - .ostccr_reg = OST_REG_OSTCCR,
>> >>> + .idx = 0,
>> >>> + },
>> >>> +
>> >>> + [OST_CLK_GLOBAL_TIMER] = {
>> >>> + .init_data = {
>> >>> + .name = "global timer",
>> >>> + .parent_names = ingenic_ost_clk_parents,
>> >>> + .num_parents = ARRAY_SIZE(ingenic_ost_clk_parents),
>> >>> + .ops = &ingenic_ost_global_timer_ops,
>> >>> + .flags = CLK_SET_RATE_UNGATE,
>> >>> + },
>> >>> + .ostcntl_reg = OST_REG_OST2CNTL,
>> >>> + },
>> >>> +};
>> >>> +
>> >>> +static const struct ingenic_ost_clk_info x2000_ost_clk_info[]
>> = {
>> >>> + [OST_CLK_PERCPU_TIMER0] = {
>> >>> + .init_data = {
>> >>> + .name = "percpu timer0",
>> >>> + .parent_names = ingenic_ost_clk_parents,
>> >>> + .num_parents = ARRAY_SIZE(ingenic_ost_clk_parents),
>> >>> + .ops = &ingenic_ost_percpu_timer_ops,
>> >>> + .flags = CLK_SET_RATE_UNGATE,
>> >>> + },
>> >>> + .idx = 0,
>> >>> + },
>> >>> +
>> >>> + [OST_CLK_PERCPU_TIMER1] = {
>> >>> + .init_data = {
>> >>> + .name = "percpu timer1",
>> >>> + .parent_names = ingenic_ost_clk_parents,
>> >>> + .num_parents = ARRAY_SIZE(ingenic_ost_clk_parents),
>> >>> + .ops = &ingenic_ost_percpu_timer_ops,
>> >>> + .flags = CLK_SET_RATE_UNGATE,
>> >>> + },
>> >>> + .idx = 1,
>> >>> },
>> >>>
>> >>> [OST_CLK_GLOBAL_TIMER] = {
>> >>> @@ -206,7 +282,7 @@ static const struct ingenic_ost_clk_info
>> >>> \x7f\x7fx1000_ost_clk_info[] = {
>> >>> .ops = &ingenic_ost_global_timer_ops,
>> >>> .flags = CLK_SET_RATE_UNGATE,
>> >>> },
>> >>> - .ostccr_reg = OST_REG_OSTCCR,
>> >>> + .ostcntl_reg = OST_REG_OSTCNTL,
>> >>> },
>> >>> };
>> >>>
>> >>> @@ -215,7 +291,7 @@ static u64 notrace
>> >>> \x7f\x7fingenic_ost_global_timer_read_cntl(void)
>> >>> struct ingenic_ost *ost = ingenic_ost;
>> >>> unsigned int count;
>> >>>
>> >>> - count = readl(ost->base + OST_REG_OST2CNTL);
>> >>> + count = readl(ost->base +
>> >>> ost->soc_info->clk_info->ostcntl_reg);
>> >>>
>> >>> return count;
>> >>> }
>> >>> @@ -225,16 +301,21 @@ static u64 notrace
>> >>> \x7f\x7fingenic_ost_clocksource_read(struct clocksource *cs)
>> >>> return ingenic_ost_global_timer_read_cntl();
>> >>> }
>> >>>
>> >>> -static inline struct ingenic_ost *to_ingenic_ost(struct
>> >>> \x7f\x7fclock_event_device *evt)
>> >>> +static inline struct ingenic_ost_timer *
>> >>> +to_ingenic_ost_timer(struct clock_event_device *evt)
>> >>> {
>> >>> - return container_of(evt, struct ingenic_ost, cevt);
>> >>> + return container_of(evt, struct ingenic_ost_timer, cevt);
>> >>> }
>> >>>
>> >>> static int ingenic_ost_cevt_set_state_shutdown(struct
>> >>> \x7f\x7fclock_event_device *evt)
>> >>> {
>> >>> - struct ingenic_ost *ost = to_ingenic_ost(evt);
>> >>> + struct ingenic_ost_timer *timer =
>> to_ingenic_ost_timer(evt);
>> >>> + struct ingenic_ost *ost = timer->ost;
>> >>>
>> >>> - writel(OSTECR_OST1ENC, ost->base + OST_REG_OSTECR);
>> >>> + if (ost->soc_info->version >= ID_X2000)
>> >>> + writel(0, timer->base + OST_REG_OSTER);
>> >>> + else
>> >>> + writel(OSTECR_OST1ENC, timer->base + OST_REG_OSTECR);
>> >>>
>> >>> return 0;
>> >>> }
>> >>> @@ -242,26 +323,34 @@ static int
>> >>> \x7f\x7fingenic_ost_cevt_set_state_shutdown(struct clock_event_device
>> >>> *evt)
>> >>> static int ingenic_ost_cevt_set_next(unsigned long next,
>> >>> struct clock_event_device *evt)
>> >>> {
>> >>> - struct ingenic_ost *ost = to_ingenic_ost(evt);
>> >>> -
>> >>> - writel((u32)~OSTFR_FFLAG, ost->base + OST_REG_OSTFR);
>> >>> - writel(next, ost->base + OST_REG_OST1DFR);
>> >>> - writel(OSTCR_OST1CLR, ost->base + OST_REG_OSTCR);
>> >>> - writel(OSTESR_OST1ENS, ost->base + OST_REG_OSTESR);
>> >>> - writel((u32)~OSTMR_FMASK, ost->base + OST_REG_OSTMR);
>> >>> + struct ingenic_ost_timer *timer =
>> to_ingenic_ost_timer(evt);
>> >>> + struct ingenic_ost *ost = timer->ost;
>> >>> +
>> >>> + writel((u32)~OSTFR_FFLAG, timer->base + OST_REG_OSTFR);
>> >>> + writel(next, timer->base + OST_REG_OST1DFR);
>> >>> + writel(OSTCR_OST1CLR, timer->base + OST_REG_OSTCR);
>> >>> +
>> >>> + if (ost->soc_info->version >= ID_X2000) {
>> >>> + writel(OSTESR_OST1ENS, timer->base + OST_REG_OSTER);
>> >>> + } else {
>> >>> + writel(OSTESR_OST1ENS, timer->base + OST_REG_OSTESR);
>> >>> + writel((u32)~OSTMR_FMASK, timer->base + OST_REG_OSTMR);
>> >>> + }
>> >>>
>> >>> return 0;
>> >>> }
>> >>>
>> >>> static irqreturn_t ingenic_ost_cevt_cb(int irq, void *dev_id)
>> >>> {
>> >>> - struct clock_event_device *evt = dev_id;
>> >>> - struct ingenic_ost *ost = to_ingenic_ost(evt);
>> >>> + struct ingenic_ost_timer *timer = dev_id;
>> >>> + struct ingenic_ost *ost = timer->ost;
>> >>>
>> >>> - writel(OSTECR_OST1ENC, ost->base + OST_REG_OSTECR);
>> >>> + if (ost->soc_info->version >= ID_X2000)
>> >>> + writel(0, timer->base + OST_REG_OSTER);
>> >>> + else
>> >>> + writel(OSTECR_OST1ENC, timer->base + OST_REG_OSTECR);
>> >>>
>> >>> - if (evt->event_handler)
>> >>> - evt->event_handler(evt);
>> >>> + timer->cevt.event_handler(&timer->cevt);
>> >>>
>> >>> return IRQ_HANDLED;
>> >>> }
>> >>> @@ -271,6 +360,7 @@ static int __init
>> >>> \x7f\x7fingenic_ost_register_clock(struct ingenic_ost *ost,
>> >>> struct clk_hw_onecell_data *clocks)
>> >>> {
>> >>> struct ingenic_ost_clk *ost_clk;
>> >>> + struct ingenic_ost_timer *timer = per_cpu_ptr(ost->timers,
>> >>> \x7f\x7finfo->idx);
>> >>> int val, err;
>> >>>
>> >>> ost_clk = kzalloc(sizeof(*ost_clk), GFP_KERNEL);
>> >>> @@ -283,9 +373,9 @@ static int __init
>> >>> \x7f\x7fingenic_ost_register_clock(struct ingenic_ost *ost,
>> >>> ost_clk->ost = ost;
>> >>>
>> >>> /* Reset clock divider */
>> >>> - val = readl(ost->base + info->ostccr_reg);
>> >>> - val &= ~(OSTCCR_PRESCALE1_MASK | OSTCCR_PRESCALE2_MASK);
>> >>> - writel(val, ost->base + info->ostccr_reg);
>> >>> + val = readl(timer->base + OST_REG_OSTCCR);
>> >>> + val &= ~(OSTCCR_PRESCALE1_MASK);
>> >>> + writel(val, timer->base + OST_REG_OSTCCR);
>> >>>
>> >>> err = clk_hw_register(NULL, &ost_clk->hw);
>> >>> if (err) {
>> >>> @@ -309,57 +399,51 @@ static struct clk * __init
>> >>> \x7f\x7fingenic_ost_get_clock(struct device_node *np, int id)
>> >>> return of_clk_get_from_provider(&args);
>> >>> }
>> >>>
>> >>> -static int __init ingenic_ost_percpu_timer_init(struct
>> >>> device_node *np,
>> >>> - struct ingenic_ost *ost)
>> >>> +static int __init ingenic_ost_setup_cevt(unsigned int cpu)
>> >>> {
>> >>> - unsigned int timer_virq, channel = OST_CLK_PERCPU_TIMER;
>> >>> + struct ingenic_ost *ost = ingenic_ost;
>> >>> + struct ingenic_ost_timer *timer =
>> this_cpu_ptr(ost->timers);
>> >>> unsigned long rate;
>> >>> int err;
>> >>>
>> >>> - ost->percpu_timer_clk = ingenic_ost_get_clock(np, channel);
>> >>> - if (IS_ERR(ost->percpu_timer_clk))
>> >>> - return PTR_ERR(ost->percpu_timer_clk);
>> >>> + timer->clk = ingenic_ost_get_clock(ost->np,
>> timer->channel);
>> >>> + if (IS_ERR(timer->clk))
>> >>> + return PTR_ERR(timer->clk);
>> >>>
>> >>> - err = clk_prepare_enable(ost->percpu_timer_clk);
>> >>> + err = clk_prepare_enable(timer->clk);
>> >>> if (err)
>> >>> goto err_clk_put;
>> >>>
>> >>> - rate = clk_get_rate(ost->percpu_timer_clk);
>> >>> + rate = clk_get_rate(timer->clk);
>> >>> if (!rate) {
>> >>> err = -EINVAL;
>> >>> goto err_clk_disable;
>> >>> }
>> >>>
>> >>> - timer_virq = of_irq_get(np, 0);
>> >>> - if (!timer_virq) {
>> >>> - err = -EINVAL;
>> >>> - goto err_clk_disable;
>> >>> - }
>> >>> + snprintf(timer->name, sizeof(timer->name), "OST percpu
>> >>> timer%u", \x7f\x7fcpu);
>> >>>
>> >>> - snprintf(ost->name, sizeof(ost->name), "OST percpu timer");
>> >>> + /* Unmask full comparison match interrupt */
>> >>> + writel((u32)~OSTMR_FMASK, timer->base + OST_REG_OSTMR);
>> >>>
>> >>> - err = request_irq(timer_virq, ingenic_ost_cevt_cb,
>> >>> IRQF_TIMER,
>> >>> - ost->name, &ost->cevt);
>> >>> - if (err)
>> >>> - goto err_irq_dispose_mapping;
>> >>> + timer->cpu = smp_processor_id();
>> >>> + timer->cevt.cpumask = cpumask_of(smp_processor_id());
>> >>> + timer->cevt.features = CLOCK_EVT_FEAT_ONESHOT;
>> >>> + timer->cevt.name = timer->name;
>> >>> + timer->cevt.rating = 400;
>> >>> + timer->cevt.set_state_shutdown =
>> >>> \x7f\x7fingenic_ost_cevt_set_state_shutdown;
>> >>> + timer->cevt.set_next_event = ingenic_ost_cevt_set_next;
>> >>>
>> >>> - ost->cevt.cpumask = cpumask_of(smp_processor_id());
>> >>> - ost->cevt.features = CLOCK_EVT_FEAT_ONESHOT;
>> >>> - ost->cevt.name = ost->name;
>> >>> - ost->cevt.rating = 400;
>> >>> - ost->cevt.set_state_shutdown =
>> >>> ingenic_ost_cevt_set_state_shutdown;
>> >>> - ost->cevt.set_next_event = ingenic_ost_cevt_set_next;
>> >>> + clockevents_config_and_register(&timer->cevt, rate, 4,
>> >>> 0xffffffff);
>> >>>
>> >>> - clockevents_config_and_register(&ost->cevt, rate, 4,
>> >>> 0xffffffff);
>> >>> + if (ost->soc_info->version >= ID_X2000)
>> >>> + enable_percpu_irq(ost->irq, IRQ_TYPE_NONE);
>> >>>
>> >>> return 0;
>> >>>
>> >>> -err_irq_dispose_mapping:
>> >>> - irq_dispose_mapping(timer_virq);
>> >>> err_clk_disable:
>> >>> - clk_disable_unprepare(ost->percpu_timer_clk);
>> >>> + clk_disable_unprepare(timer->clk);
>> >>> err_clk_put:
>> >>> - clk_put(ost->percpu_timer_clk);
>> >>> + clk_put(timer->clk);
>> >>> return err;
>> >>> }
>> >>>
>> >>> @@ -385,11 +469,14 @@ static int __init
>> >>> \x7f\x7fingenic_ost_global_timer_init(struct device_node *np,
>> >>> goto err_clk_disable;
>> >>> }
>> >>>
>> >>> - /* Clear counter CNT registers */
>> >>> - writel(OSTCR_OST2CLR, ost->base + OST_REG_OSTCR);
>> >>> -
>> >>> - /* Enable OST channel */
>> >>> - writel(OSTESR_OST2ENS, ost->base + OST_REG_OSTESR);
>> >>> + /* Clear counter CNT registers and enable OST channel */
>> >>> + if (ost->soc_info->version >= ID_X2000) {
>> >>> + writel(OSTCR_OST1CLR, ost->base + OST_REG_OSTCR);
>> >>> + writel(OSTESR_OST1ENS, ost->base + OST_REG_OSTER);
>> >>> + } else {
>> >>> + writel(OSTCR_OST2CLR, ost->base + OST_REG_OSTCR);
>> >>> + writel(OSTESR_OST2ENS, ost->base + OST_REG_OSTESR);
>> >>> + }
>> >>>
>> >>> cs->name = "ingenic-ost";
>> >>> cs->rating = 400;
>> >>> @@ -411,18 +498,33 @@ static int __init
>> >>> \x7f\x7fingenic_ost_global_timer_init(struct device_node *np,
>> >>> }
>> >>>
>> >>> static const struct ingenic_soc_info x1000_soc_info = {
>> >>> + .version = ID_X1000,
>> >>> + .clk_info = x1000_ost_clk_info,
>> >>> +
>> >>> .num_channels = 2,
>> >>> };
>> >>>
>> >>> +static const struct ingenic_soc_info x2000_soc_info = {
>> >>> + .version = ID_X2000,
>> >>> + .clk_info = x2000_ost_clk_info,
>> >>> +
>> >>> + .num_channels = 3,
>> >>> + .base_offset = 0x100,
>> >>> +};
>> >>> +
>> >>> static const struct of_device_id __maybe_unused
>> >>> \x7f\x7fingenic_ost_of_matches[] __initconst = {
>> >>> { .compatible = "ingenic,x1000-ost", .data =
>> >>> &x1000_soc_info },
>> >>> + { .compatible = "ingenic,x2000-ost", .data =
>> >>> &x2000_soc_info }, { /* sentinel */ }
>> >>> };
>> >>>
>> >>> static int __init ingenic_ost_probe(struct device_node *np)
>> >>> {
>> >>> const struct of_device_id *id =
>> >>> \x7f\x7fof_match_node(ingenic_ost_of_matches, np);
>> >>> + struct ingenic_ost_timer *timer;
>> >>> struct ingenic_ost *ost;
>> >>> + void __iomem *base;
>> >>> + unsigned int cpu;
>> >>> unsigned int i;
>> >>> int ret;
>> >>>
>> >>> @@ -430,18 +532,43 @@ static int __init ingenic_ost_probe(struct
>> >>> \x7f\x7fdevice_node *np)
>> >>> if (!ost)
>> >>> return -ENOMEM;
>> >>>
>> >>> + ost->timers = alloc_percpu(struct ingenic_ost_timer);
>> >>> + if (!ost->timers) {
>> >>> + ret = -ENOMEM;
>> >>> + goto err_free_ost;
>> >>> + }
>> >>> +
>> >>> + ost->np = np;
>> >>> + ost->soc_info = id->data;
>> >>> +
>> >>> ost->base = of_io_request_and_map(np, 0,
>> >>> of_node_full_name(np));
>> >>> if (IS_ERR(ost->base)) {
>> >>> pr_err("%s: Failed to map OST registers\n", __func__);
>> >>> ret = PTR_ERR(ost->base);
>> >>> - goto err_free_ost;
>> >>> + goto err_free_timers;
>> >>> + }
>> >>> +
>> >>> + if (ost->soc_info->version >= ID_X2000) {
>> >>> + base = of_io_request_and_map(np, 1,
>> >>> of_node_full_name(np));
>> >>> + if (IS_ERR(base)) {
>> >>> + pr_err("%s: Failed to map OST registers\n",
>> >>> __func__);
>> >>> + ret = PTR_ERR(base);
>> >>> + goto err_free_timers;
>> >>> + }
>> >>> + }
>> >>
>> >> The DT documentation only mentions one memory resource. Here, you
>> >> map \x7fa second one, which is not used anywhere. I'm really
>> confused
>> >> about \x7fwhat you're trying to do here.
>> >>
>> >
>> > X2000 and X2100 divide the OST into two parts. The global timer is
>> > the first part, which is still located at the address of
>> 0x12000000,
>> > and the percpu timers are the second part, the starting address is
>> > 0x12100000, and each timer is offset by 0x100 (percpu timer0 is at
>> > 0x12100000, percpu timer1 is at 0x12100100, percpu timer2 is at
>> > 0x12100200, percpu timer3 is at 0x12100300). This one is used in
>> > line 593 of the code.
>>
>> Then you need two different DT nodes, one at each start address.
>> Either use different drivers (since the register sets are
>> different),
>> or *if* it can be implemented cleanly in ingenic-sysost.c, different
>> compatible strings - one for the global timer and one for the percpu
>> timers.
>
> Sorry, I did not describe it clearly. Although the global timer and
> percpu timers are divided into two parts, they still belong to the
> same
> hardware module. The base address of the entire module is 0x12000000,
> but the control register of the global timer is not shifted. The
> percpu
> timers are shifted by 0x100000 as a whole (the situation here is
> similar to PDMA, which is also divided into two parts: the
> channel-related registers are not shifted, while the system
> control-related registers are shifted by 0x1000 as a whole). I think
> maybe we can follow PDMA's approach and add corresponding instructions
> in the DT documentation. This can avoid confusion caused by splitting
> different parts of the same hardware module into two nodes, and at the
> same time keep the code of the devicetree as simple and clear as
> possible.
>
> What do you think about that?
Looking at the programming manual, these are not the same hardware
module. They are on different addresses, are functionally independent,
and are even described in different chapters of the PM; so I stick to
what I said earlier.
Cheers,
-Paul
> Thanks and best regards!
>
>>
>> -Paul
>>
>> >>> +
>> >>> + ost->irq = irq_of_parse_and_map(np, 0);
>> >>> + if (ost->irq < 0) {
>> >>> + pr_crit("%s: Cannot to get OST IRQ\n", __func__);
>> >>> + ret = ost->irq;
>> >>> + goto err_free_timers;
>> >>> }
>> >>>
>> >>> ost->clk = of_clk_get_by_name(np, "ost");
>> >>> if (IS_ERR(ost->clk)) {
>> >>> - ret = PTR_ERR(ost->clk);
>> >>> pr_crit("%s: Cannot get OST clock\n", __func__);
>> >>> - goto err_free_ost;
>> >>> + ret = PTR_ERR(ost->clk);
>> >>> + goto err_free_timers;
>> >>> }
>> >>>
>> >>> ret = clk_prepare_enable(ost->clk);
>> >>> @@ -450,8 +577,6 @@ static int __init ingenic_ost_probe(struct
>> >>> \x7f\x7fdevice_node *np)
>> >>> goto err_put_clk;
>> >>> }
>> >>>
>> >>> - ost->soc_info = id->data;
>> >>> -
>> >>> ost->clocks = kzalloc(struct_size(ost->clocks, hws,
>> >>> \x7f\x7fost->soc_info->num_channels),
>> >>> GFP_KERNEL);
>> >>> if (!ost->clocks) {
>> >>> @@ -461,8 +586,21 @@ static int __init ingenic_ost_probe(struct
>> >>> \x7f\x7fdevice_node *np)
>> >>>
>> >>> ost->clocks->num = ost->soc_info->num_channels;
>> >>>
>> >>> - for (i = 0; i < ost->clocks->num; i++) {
>> >>> - ret = ingenic_ost_register_clock(ost, i,
>> >>> \x7f\x7f&x1000_ost_clk_info[i], ost->clocks);
>> >>> + for (cpu = 0; cpu < num_possible_cpus(); cpu++) {
>> >>> + timer = per_cpu_ptr(ost->timers, cpu);
>> >>> +
>> >>> + if (ost->soc_info->version >= ID_X2000)
>> >>> + timer->base = base + ost->soc_info->base_offset *
>> >>> cpu;
>> >>> + else
>> >>> + timer->base = ost->base;
>> >>> +
>> >>> + timer->ost = ost;
>> >>> + timer->cpu = cpu;
>> >>> + timer->channel = OST_CLK_PERCPU_TIMER + cpu;
>> >>> + }
>> >>> +
>> >>> + for (i = 0; i < num_possible_cpus() + 1; i++) {
>> >>> + ret = ingenic_ost_register_clock(ost, i,
>> >>> \x7f\x7f&ost->soc_info->clk_info[i], ost->clocks);
>> >>> if (ret) {
>> >>> pr_crit("%s: Cannot register clock %d\n", __func__,
>> >>> i); goto err_unregister_ost_clocks;
>> >>> @@ -488,6 +626,8 @@ static int __init ingenic_ost_probe(struct
>> >>> \x7f\x7fdevice_node *np)
>> >>> clk_disable_unprepare(ost->clk);
>> >>> err_put_clk:
>> >>> clk_put(ost->clk);
>> >>> +err_free_timers:
>> >>> + free_percpu(ost->timers);
>> >>> err_free_ost:
>> >>> kfree(ost);
>> >>> return ret;
>> >>> @@ -513,13 +653,29 @@ static int __init ingenic_ost_init(struct
>> >>> \x7f\x7fdevice_node *np)
>> >>>
>> >>> ret = ingenic_ost_global_timer_init(np, ost);
>> >>> if (ret) {
>> >>> - pr_crit("%s: Unable to init global timer: %x\n",
>> >>> __func__, \x7f\x7fret);
>> >>> + pr_crit("%s: Unable to init global timer: %d\n",
>> >>> __func__, \x7f\x7fret);
>> >>
>> >> This is a fix, so it needs to be a separate commit with a Fixes:
>> >> tag.
>> >
>> >
>> > Sure.
>> >
>> >
>> > Thanks and best regards!
>> >
>> >
>> >>
>> >> Cheers,
>> >> -Paul
>> >>
>> >>> goto err_free_ingenic_ost;
>> >>> }
>> >>>
>> >>> - ret = ingenic_ost_percpu_timer_init(np, ost);
>> >>> - if (ret)
>> >>> + if (ost->soc_info->version >= ID_X2000)
>> >>> + ret = request_percpu_irq(ost->irq, ingenic_ost_cevt_cb,
>> >>> + "OST percpu timer", ost->timers);
>> >>> + else
>> >>> + ret = request_irq(ost->irq, ingenic_ost_cevt_cb,
>> >>> IRQF_TIMER,
>> >>> + "OST percpu timer", ost->timers);
>> >>> +
>> >>> + if (ret) {
>> >>> + pr_crit("%s: Unable to request percpu IRQ: %d\n",
>> >>> __func__, \x7f\x7fret);
>> >>> + goto err_ost_global_timer_cleanup;
>> >>> + }
>> >>> +
>> >>> + /* Setup clock events on each CPU core */
>> >>> + ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "Ingenic
>> >>> XBurst: \x7f\x7fonline",
>> >>> + ingenic_ost_setup_cevt, NULL);
>> >>> + if (ret < 0) {
>> >>> + pr_crit("%s: Unable to init percpu timers: %d\n",
>> >>> __func__, \x7f\x7fret);
>> >>> goto err_ost_global_timer_cleanup;
>> >>> + }
>> >>>
>> >>> /* Register the sched_clock at the end as there's no way to
>> >>> undo \x7f\x7fit */
>> >>> rate = clk_get_rate(ost->global_timer_clk);
>> >>> @@ -537,3 +693,4 @@ static int __init ingenic_ost_init(struct
>> >>> \x7f\x7fdevice_node *np)
>> >>> }
>> >>>
>> >>> TIMER_OF_DECLARE(x1000_ost, "ingenic,x1000-ost",
>> >>> ingenic_ost_init);
>> >>> +TIMER_OF_DECLARE(x2000_ost, "ingenic,x2000-ost",
>> >>> ingenic_ost_init);
>> >>> --
>> >>> 2.7.4
>> >>>
>> >>
>>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 1/2] clocksource: Ingenic: Rename unreasonable array names.
2021-06-04 16:31 ` [PATCH v2 1/2] clocksource: Ingenic: Rename unreasonable array names 周琰杰 (Zhou Yanjie)
@ 2021-06-16 13:56 ` Daniel Lezcano
2021-06-18 16:03 ` [tip: timers/core] clocksource/drivers/ingenic: " tip-bot2 for 周琰杰
1 sibling, 0 replies; 11+ messages in thread
From: Daniel Lezcano @ 2021-06-16 13:56 UTC (permalink / raw)
To: 周琰杰 (Zhou Yanjie), tglx
Cc: linux-mips, linux-kernel, dongsheng.qiu, aric.pzqi, rick.tyliu,
sihui.liu, jun.jiang, sernia.zhou, paul
On 04/06/2021 18:31, 周琰杰 (Zhou Yanjie) wrote:
> 1.Rename the "ingenic_ost_clk_info[]" to "x1000_ost_clk_info[]"
> to facilitate the addition of OST support for X2000 SoC in a
> later commit.
> 2.When the OST support for X2000 SoC is added, there will be two
> compatible strings, so renaming "ingenic_ost_of_match[]" to
> "ingenic_ost_of_matches[]" is more reasonable.
> 3.Remove the unnecessary comma in "ingenic_ost_of_matches[]" to
> reduce code size as much as possible.
>
> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
> ---
I have applied this patch but not the 2/2.
Thanks
-- Daniel
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/2] clocksource: Ingenic: Add SMP/SMT support for sysost driver.
2021-06-14 16:42 ` Paul Cercueil
@ 2021-06-16 14:57 ` Daniel Lezcano
0 siblings, 0 replies; 11+ messages in thread
From: Daniel Lezcano @ 2021-06-16 14:57 UTC (permalink / raw)
To: Paul Cercueil, 周琰杰
Cc: Paul Cercueil, tglx, linux-mips, linux-kernel, dongsheng.qiu,
aric.pzqi, rick.tyliu, sihui.liu, jun.jiang, sernia.zhou
Hi,
On 14/06/2021 18:42, Paul Cercueil wrote:
> Hi Zhou,
[ please trim ... ]
>>> >> The DT documentation only mentions one memory resource. Here, you
>>> >> map \x7fa second one, which is not used anywhere. I'm really confused
>>> >> about \x7fwhat you're trying to do here.
>>> >>
>>> >
>>> > X2000 and X2100 divide the OST into two parts. The global timer is
>>> > the first part, which is still located at the address of 0x12000000,
>>> > and the percpu timers are the second part, the starting address is
>>> > 0x12100000, and each timer is offset by 0x100 (percpu timer0 is at
>>> > 0x12100000, percpu timer1 is at 0x12100100, percpu timer2 is at
>>> > 0x12100200, percpu timer3 is at 0x12100300). This one is used in
>>> > line 593 of the code.
>>>
>>> Then you need two different DT nodes, one at each start address.
>>> Either use different drivers (since the register sets are different),
>>> or *if* it can be implemented cleanly in ingenic-sysost.c, different
>>> compatible strings - one for the global timer and one for the percpu
>>> timers.
>>
>> Sorry, I did not describe it clearly. Although the global timer and
>> percpu timers are divided into two parts, they still belong to the same
>> hardware module. The base address of the entire module is 0x12000000,
>> but the control register of the global timer is not shifted. The percpu
>> timers are shifted by 0x100000 as a whole (the situation here is
>> similar to PDMA, which is also divided into two parts: the
>> channel-related registers are not shifted, while the system
>> control-related registers are shifted by 0x1000 as a whole). I think
>> maybe we can follow PDMA's approach and add corresponding instructions
>> in the DT documentation. This can avoid confusion caused by splitting
>> different parts of the same hardware module into two nodes, and at the
>> same time keep the code of the devicetree as simple and clear as
>> possible.
>>
>> What do you think about that?
>
> Looking at the programming manual, these are not the same hardware
> module. They are on different addresses, are functionally independent,
> and are even described in different chapters of the PM; so I stick to
> what I said earlier.
>
> Cheers,
> -Paul
>
>> Thanks and best regards!
>>
>>>
>>> -Paul
[ ... ]
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 11+ messages in thread
* [tip: timers/core] clocksource/drivers/ingenic: Rename unreasonable array names
2021-06-04 16:31 ` [PATCH v2 1/2] clocksource: Ingenic: Rename unreasonable array names 周琰杰 (Zhou Yanjie)
2021-06-16 13:56 ` Daniel Lezcano
@ 2021-06-18 16:03 ` tip-bot2 for 周琰杰
1 sibling, 0 replies; 11+ messages in thread
From: tip-bot2 for 周琰杰 @ 2021-06-18 16:03 UTC (permalink / raw)
To: linux-tip-commits; +Cc: zhouyanjie, Daniel Lezcano, x86, linux-kernel
The following commit has been merged into the timers/core branch of tip:
Commit-ID: 870a6e1539829356baf70b57c933d0b309cfac21
Gitweb: https://git.kernel.org/tip/870a6e1539829356baf70b57c933d0b309cfac21
Author: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
AuthorDate: Sat, 05 Jun 2021 00:31:45 +08:00
Committer: Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Tue, 15 Jun 2021 14:14:14 +02:00
clocksource/drivers/ingenic: Rename unreasonable array names
1.Rename the "ingenic_ost_clk_info[]" to "x1000_ost_clk_info[]" to
facilitate the addition of OST support for X2000 SoC in a later
commit
2.When the OST support for X2000 SoC is added, there will be two
compatible strings, so renaming "ingenic_ost_of_match[]" to
"ingenic_ost_of_matches[]" is more reasonable
3.Remove the unnecessary comma in "ingenic_ost_of_matches[]" to reduce
code size as much as possible.
Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Link: https://lore.kernel.org/r/1622824306-30987-2-git-send-email-zhouyanjie@wanyeetech.com
---
drivers/clocksource/ingenic-sysost.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/clocksource/ingenic-sysost.c b/drivers/clocksource/ingenic-sysost.c
index e77d584..a129840 100644
--- a/drivers/clocksource/ingenic-sysost.c
+++ b/drivers/clocksource/ingenic-sysost.c
@@ -186,7 +186,7 @@ static const struct clk_ops ingenic_ost_global_timer_ops = {
static const char * const ingenic_ost_clk_parents[] = { "ext" };
-static const struct ingenic_ost_clk_info ingenic_ost_clk_info[] = {
+static const struct ingenic_ost_clk_info x1000_ost_clk_info[] = {
[OST_CLK_PERCPU_TIMER] = {
.init_data = {
.name = "percpu timer",
@@ -414,14 +414,14 @@ static const struct ingenic_soc_info x1000_soc_info = {
.num_channels = 2,
};
-static const struct of_device_id __maybe_unused ingenic_ost_of_match[] __initconst = {
- { .compatible = "ingenic,x1000-ost", .data = &x1000_soc_info, },
+static const struct of_device_id __maybe_unused ingenic_ost_of_matches[] __initconst = {
+ { .compatible = "ingenic,x1000-ost", .data = &x1000_soc_info },
{ /* sentinel */ }
};
static int __init ingenic_ost_probe(struct device_node *np)
{
- const struct of_device_id *id = of_match_node(ingenic_ost_of_match, np);
+ const struct of_device_id *id = of_match_node(ingenic_ost_of_matches, np);
struct ingenic_ost *ost;
unsigned int i;
int ret;
@@ -462,7 +462,7 @@ static int __init ingenic_ost_probe(struct device_node *np)
ost->clocks->num = ost->soc_info->num_channels;
for (i = 0; i < ost->clocks->num; i++) {
- ret = ingenic_ost_register_clock(ost, i, &ingenic_ost_clk_info[i], ost->clocks);
+ ret = ingenic_ost_register_clock(ost, i, &x1000_ost_clk_info[i], ost->clocks);
if (ret) {
pr_crit("%s: Cannot register clock %d\n", __func__, i);
goto err_unregister_ost_clocks;
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-06-18 16:03 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-04 16:31 Add SMP/SMT support for sysost driver 周琰杰 (Zhou Yanjie)
2021-06-04 16:31 ` [PATCH v2 1/2] clocksource: Ingenic: Rename unreasonable array names 周琰杰 (Zhou Yanjie)
2021-06-16 13:56 ` Daniel Lezcano
2021-06-18 16:03 ` [tip: timers/core] clocksource/drivers/ingenic: " tip-bot2 for 周琰杰
2021-06-04 16:31 ` [PATCH v2 2/2] clocksource: Ingenic: Add SMP/SMT support for sysost driver 周琰杰 (Zhou Yanjie)
2021-06-10 15:30 ` Paul Cercueil
2021-06-11 15:31 ` Zhou Yanjie
2021-06-11 15:44 ` Paul Cercueil
2021-06-12 15:48 ` 周琰杰
2021-06-14 16:42 ` Paul Cercueil
2021-06-16 14:57 ` Daniel Lezcano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).