Hi Paul, 在 2020/7/1 上午3:38, Paul Cercueil 写道:
Hi Zhou,Le mer. 1 juil. 2020 à 1:15, 周琰杰 (Zhou Yanjie) <zhouyanjie@xxxxxxxxxxxxxx> a écrit :X1000 and SoCs after X1000 (such as X1500 and X1830) had a separate OST, it no longer belongs to TCU. This driver will register both a clocksource and a sched_clock to the system. Tested-by: 周正 (Zhou Zheng) <sernia.zhou@xxxxxxxxxxx> Co-developed-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@xxxxxxxxxxx> Signed-off-by: 漆鹏振 (Qi Pengzhen) <aric.pzqi@xxxxxxxxxxx> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@xxxxxxxxxxxxxx> --- Notes: v1->v2: Fix compile warnings. Reported-by: kernel test robot <lkp@xxxxxxxxx> v2->v3: No change. drivers/clocksource/Kconfig | 19 +- drivers/clocksource/Makefile | 1 +drivers/clocksource/ingenic-sysost.c | 509 +++++++++++++++++++++++++++++++++++3 files changed, 525 insertions(+), 4 deletions(-) create mode 100644 drivers/clocksource/ingenic-sysost.c diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig index 91418381fcd4..172397a00f3e 100644 --- a/drivers/clocksource/Kconfig +++ b/drivers/clocksource/Kconfig @@ -686,7 +686,7 @@ config MILBEAUT_TIMER Enables the support for Milbeaut timer driver. config INGENIC_TIMER - bool "Clocksource/timer using the TCU in Ingenic JZ SoCs" + bool "Clocksource/timer using the TCU in Ingenic SoCs"This cosmetic change is unrelated to the patch, please drop it.
Sure.
default MACH_INGENIC depends on MIPS || COMPILE_TEST depends on COMMON_CLK @@ -694,15 +694,26 @@ config INGENIC_TIMER select TIMER_OF select IRQ_DOMAIN help - Support for the timer/counter unit of the Ingenic JZ SoCs. + Support for the timer/counter unit of the Ingenic SoCs.Same here.
Sure.
+ +config INGENIC_SYSOST + bool "Clocksource/timer using the SYSOST in Ingenic SoCs" + default MACH_INGENIC + depends on MIPS || COMPILE_TEST + depends on COMMON_CLK + select MFD_SYSCON + select TIMER_OF + select IRQ_DOMAIN + help + This option enables support for SYSOST in Ingenic SoCs . config INGENIC_OST - bool "Clocksource for Ingenic OS Timer" + bool "Clocksource for Ingenic OST"Same here.
OK.
depends on MIPS || COMPILE_TEST depends on COMMON_CLK select MFD_SYSCON help - Support for the Operating System Timer of the Ingenic JZ SoCs. + Support for the Operating System Timer of the Ingenic SoCs.Same here.
Sure.
config MICROCHIP_PIT64B bool "Microchip PIT64B support" diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile index bdda1a2e4097..3994e221e262 100644 --- a/drivers/clocksource/Makefile +++ b/drivers/clocksource/Makefile @@ -82,6 +82,7 @@ obj-$(CONFIG_H8300_TMR8) += h8300_timer8.o obj-$(CONFIG_H8300_TMR16) += h8300_timer16.o obj-$(CONFIG_H8300_TPU) += h8300_tpu.o obj-$(CONFIG_INGENIC_OST) += ingenic-ost.o +obj-$(CONFIG_INGENIC_SYSOST) += ingenic-sysost.o obj-$(CONFIG_INGENIC_TIMER) += ingenic-timer.o obj-$(CONFIG_CLKSRC_ST_LPC) += clksrc_st_lpc.o obj-$(CONFIG_X86_NUMACHIP) += numachip.odiff --git a/drivers/clocksource/ingenic-sysost.c b/drivers/clocksource/ingenic-sysost.cnew file mode 100644 index 000000000000..d3e1b7582221 --- /dev/null +++ b/drivers/clocksource/ingenic-sysost.c @@ -0,0 +1,509 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Ingenic XBurst SoCs SYSOST clocks driver + * Copyright (c) 2020 周琰杰 (Zhou Yanjie) <zhouyanjie@xxxxxxxxxxxxxx> + */ + +#include <linux/bitops.h> +#include <linux/clk.h> +#include <linux/clk-provider.h> +#include <linux/clockchips.h> +#include <linux/clocksource.h> +#include <linux/interrupt.h> +#include <linux/mfd/syscon.h> +#include <linux/of_address.h> +#include <linux/of_irq.h> +#include <linux/sched_clock.h> +#include <linux/slab.h> +#include <linux/syscore_ops.h> + +#include <dt-bindings/clock/ingenic,ost.h> + +/* OST register offsets */ +#define OST_REG_OSTCCR 0x00 +#define OST_REG_OSTCR 0x08 +#define OST_REG_OSTFR 0x0c +#define OST_REG_OSTMR 0x10 +#define OST_REG_OST1DFR 0x14 +#define OST_REG_OST1CNT 0x18 +#define OST_REG_OST2CNTL 0x20 +#define OST_REG_OSTCNT2HBUF 0x24 +#define OST_REG_OSTESR 0x34 +#define OST_REG_OSTECR 0x38 + +/* bits within the OSTCCR register */ +#define OSTCCR_PRESCALE1_MASK 0x3 +#define OSTCCR_PRESCALE2_MASK 0xc +#define OSTCCR_PRESCALE1_LSB 0 +#define OSTCCR_PRESCALE2_LSB 2 + +/* bits within the OSTCR register */ +#define OSTCR_OST1CLR BIT(0) +#define OSTCR_OST2CLR BIT(1) + +/* bits within the OSTFR register */ +#define OSTFR_FFLAG BIT(0) + +/* bits within the OSTMR register */ +#define OSTMR_FMASK BIT(0) + +/* bits within the OSTESR register */ +#define OSTESR_OST1ENS BIT(0) +#define OSTESR_OST2ENS BIT(1) + +/* bits within the OSTECR register */ +#define OSTECR_OST1ENC BIT(0) +#define OSTECR_OST2ENC BIT(1) + +enum ost_clk_parent { + OST_PARENT_EXT, +}; + +struct ingenic_soc_info { + unsigned int num_channels; +};Unless you plan to add support for other SoCs which have more than 2 channels, I think you can drop this - just add a OST_NUM_CHANNELS macro.
When X2000 turned on SMT, a total of 3 channels are required, this is reserved for adding support for X2000 in the future.
+ +struct ingenic_ost_clk_info { + struct clk_init_data init_data; + u8 ostccr_reg; +}; + +struct ingenic_ost_clk { + struct clk_hw hw; + unsigned int idx; + struct ingenic_ost *ost; + const struct ingenic_ost_clk_info *info; +}; + +struct ingenic_ost { + void __iomem *base; + const struct ingenic_soc_info *soc_info; + struct clk *clk, *percpu_timer_clk, *global_timer_clk; + unsigned int percpu_timer_channel, global_timer_channel; + struct clock_event_device cevt; + struct clocksource cs; + char name[20]; + + struct clk_hw_onecell_data *clocks; +}; + +static struct ingenic_ost *ingenic_ost; + +static inline struct ingenic_ost_clk *to_ost_clk(struct clk_hw *hw) +{ + return container_of(hw, struct ingenic_ost_clk, hw); +} + +static unsigned long ingenic_ost_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct ingenic_ost_clk *ost_clk = to_ost_clk(hw); + const struct ingenic_ost_clk_info *info = ost_clk->info; + unsigned int prescale; + + prescale = readl(ost_clk->ost->base + info->ostccr_reg); + + if (ost_clk->idx == OST_CLK_PERCPU_TIMER)+ prescale = (prescale & OSTCCR_PRESCALE1_MASK) >> OSTCCR_PRESCALE1_LSB;+ else if (ost_clk->idx == OST_CLK_GLOBAL_TIMER)+ prescale = (prescale & OSTCCR_PRESCALE2_MASK) >> OSTCCR_PRESCALE2_LSB;Instead of checking which clock you're manipulating here, just set a different .recalc_rate to the two clocks.
Sure.
+ + return parent_rate >> (prescale * 2); +} ++static u8 ingenic_ost_get_prescale(unsigned long rate, unsigned long req_rate)+{ + u8 prescale; + + for (prescale = 0; prescale < 2; prescale++) + if ((rate >> (prescale * 2)) <= req_rate) + return prescale; + + return 2; /* /16 divider */ +} ++static long ingenic_ost_round_rate(struct clk_hw *hw, unsigned long req_rate,+ unsigned long *parent_rate) +{ + unsigned long rate = *parent_rate; + u8 prescale; + + if (req_rate > rate) + return rate; + + prescale = ingenic_ost_get_prescale(rate, req_rate); + + return rate >> (prescale * 2); +} ++static int ingenic_ost_set_rate(struct clk_hw *hw, unsigned long req_rate,+ unsigned long parent_rate) +{ + struct ingenic_ost_clk *ost_clk = to_ost_clk(hw); + const struct ingenic_ost_clk_info *info = ost_clk->info; + u8 prescale = ingenic_ost_get_prescale(parent_rate, req_rate); + int val; + + if (ost_clk->idx == OST_CLK_PERCPU_TIMER) { + val = readl(ost_clk->ost->base + info->ostccr_reg);+ val = (val & ~OSTCCR_PRESCALE1_MASK) | (prescale << OSTCCR_PRESCALE1_LSB);+ writel(val, ost_clk->ost->base + info->ostccr_reg); + } else if (ost_clk->idx == OST_CLK_GLOBAL_TIMER) { + 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); + }Same here - set a different .set_rate() callback to the two clocks.
Sure.
+ + return 0; +} + +static const struct clk_ops ingenic_ost_clk_ops = { + .recalc_rate = ingenic_ost_recalc_rate, + .round_rate = ingenic_ost_round_rate, + .set_rate = ingenic_ost_set_rate, +}; + +static const char * const ingenic_ost_clk_parents[] = { + [OST_PARENT_EXT] = "ext", +};just { "ext" } will be fine, no need to specify the index, which is zero anyway. Then you can drop the ost_clock_parent enum.
OK.
+ +#define DEF_TIMER(_name, _ostccr) { \ + .init_data = { \ + .name = _name, \ + .parent_names = ingenic_ost_clk_parents, \ + .num_parents = ARRAY_SIZE(ingenic_ost_clk_parents), \ + .ops = &ingenic_ost_clk_ops, \ + .flags = CLK_SET_RATE_UNGATE, \ + }, \ + .ostccr_reg = _ostccr, \ +} + +static const struct ingenic_ost_clk_info ingenic_ost_clk_info[] = { + [OST_CLK_PERCPU_TIMER] = DEF_TIMER("percpu timer", OST_REG_OSTCCR), + [OST_CLK_GLOBAL_TIMER] = DEF_TIMER("global timer", OST_REG_OSTCCR), +}; + +#undef DEF_TIMER + +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); + + return count; +} + +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)+{ + return container_of(evt, struct ingenic_ost, cevt); +} ++static int ingenic_ost_cevt_set_state_shutdown(struct clock_event_device *evt)+{ + struct ingenic_ost *ost = to_ingenic_ost(evt); + + writel(OSTECR_OST1ENC, ost->base + OST_REG_OSTECR); + + return 0; +} + +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); + + 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); + + writel(OSTECR_OST1ENC, ost->base + OST_REG_OSTECR); + + if (evt->event_handler) + evt->event_handler(evt); + + return IRQ_HANDLED; +} + +static int __init ingenic_ost_register_clock(struct ingenic_ost *ost, + unsigned int idx, const struct ingenic_ost_clk_info *info, + struct clk_hw_onecell_data *clocks) +{ + struct ingenic_ost_clk *ost_clk; + int val, err; + + ost_clk = kzalloc(sizeof(*ost_clk), GFP_KERNEL); + if (!ost_clk) + return -ENOMEM; + + ost_clk->hw.init = &info->init_data; + ost_clk->idx = idx; + ost_clk->info = info; + 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); + + err = clk_hw_register(NULL, &ost_clk->hw); + if (err) { + kfree(ost_clk); + return err; + } + + clocks->hws[idx] = &ost_clk->hw; + + return 0; +} ++static struct clk * __init ingenic_ost_get_clock(struct device_node *np, int id)+{ + struct of_phandle_args args; + + args.np = np; + args.args_count = 1; + args.args[0] = id; + + return of_clk_get_from_provider(&args); +} + +static int __init ingenic_ost_percpu_timer_init(struct device_node *np, + struct ingenic_ost *ost) +{ + unsigned int timer_virq, channel = ost->percpu_timer_channel; + 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); + + err = clk_prepare_enable(ost->percpu_timer_clk); + if (err) + goto err_clk_put; + + rate = clk_get_rate(ost->percpu_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(ost->name, sizeof(ost->name), "OST percpu timer"); + + err = request_irq(timer_virq, ingenic_ost_cevt_cb, IRQF_TIMER, + ost->name, &ost->cevt); + if (err) + goto err_irq_dispose_mapping; + + 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(&ost->cevt, rate, 4, 0xffffffff); + + return 0; + +err_irq_dispose_mapping: + irq_dispose_mapping(timer_virq); +err_clk_disable: + clk_disable_unprepare(ost->percpu_timer_clk); +err_clk_put: + clk_put(ost->percpu_timer_clk); + return err; +} + +static int __init ingenic_ost_global_timer_init(struct device_node *np, + struct ingenic_ost *ost) +{ + unsigned int channel = ost->global_timer_channel; + struct clocksource *cs = &ost->cs; + unsigned long rate; + int err; + + ost->global_timer_clk = ingenic_ost_get_clock(np, channel); + if (IS_ERR(ost->global_timer_clk)) + return PTR_ERR(ost->global_timer_clk); + + err = clk_prepare_enable(ost->global_timer_clk); + if (err) + goto err_clk_put; + + rate = clk_get_rate(ost->global_timer_clk); + if (!rate) { + err = -EINVAL; + 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); + + cs->name = "ingenic-ost"; + cs->rating = 400; + cs->flags = CLOCK_SOURCE_IS_CONTINUOUS; + cs->mask = CLOCKSOURCE_MASK(32); + cs->read = ingenic_ost_clocksource_read; + + err = clocksource_register_hz(cs, rate); + if (err) + goto err_clk_disable; + + return 0; + +err_clk_disable: + clk_disable_unprepare(ost->global_timer_clk); +err_clk_put: + clk_put(ost->global_timer_clk); + return err; +} + +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, }, + { /* 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);+ struct ingenic_ost *ost; + unsigned int i; + int ret; + + ost = kzalloc(sizeof(*ost), GFP_KERNEL); + if (!ost) + return -ENOMEM; + + ost->base = of_iomap(np, 0); + if (IS_ERR(ost->base)) { + pr_err("%s: Failed to map OST registers\n", __func__); + ret = PTR_ERR(ost->base); + goto err_free_ost; + } + + 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 = clk_prepare_enable(ost->clk); + if (ret) { + pr_crit("%s: Unable to enable OST clock\n", __func__); + 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) { + ret = -ENOMEM; + goto err_clk_disable; + } + + 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);+ if (ret) { + pr_crit("%s: Cannot register clock %d\n", __func__, i); + goto err_unregister_ost_clocks; + } + } ++ ret = of_clk_add_hw_provider(np, of_clk_hw_onecell_get, ost->clocks);+ if (ret) { + pr_crit("%s: Cannot add OF clock provider\n", __func__); + goto err_unregister_ost_clocks; + } + + ost->percpu_timer_channel = OST_CLK_PERCPU_TIMER; + ost->global_timer_channel = OST_CLK_GLOBAL_TIMER;If that's a constant, just use the constant where 'ost->percpu_timer_channel' and 'ost->global_timer_channel' are used, I think.
Sure.
+ + ingenic_ost = ost; + + return 0; + +err_unregister_ost_clocks: + for (i = 0; i < ost->clocks->num; i++) + if (ost->clocks->hws[i]) + clk_hw_unregister(ost->clocks->hws[i]); + kfree(ost->clocks); +err_clk_disable: + clk_disable_unprepare(ost->clk); +err_put_clk: + clk_put(ost->clk); +err_free_ost: + kfree(ost); + return ret; +} + +static int __init ingenic_ost_init(struct device_node *np) +{ + unsigned long rate; + int ret = ingenic_ost_probe(np);Don't call functions in variable declarations - they can be overlooked easily.
OK
+ + if (ret)+ pr_crit("%s: Failed to initialize OST clocks: %d\n", __func__, ret);return ret?
I'll add it in the next version.
+ + of_node_clear_flag(np, OF_POPULATED); + + ret = ingenic_ost_global_timer_init(np, ingenic_ost);The 'ingenic_ost' variable is just a dirty workaround for the sched_clock callback, which does not take any parameter. Avoid using it here. Make ingenic_ost_probe() return a pointer to your ingenic_ost instance (and a PTR_ERR() on error), and use that pointer for the rest.
Sure. Thanks and best regards!
Cheers, -Paul+ if (ret) {+ pr_crit("%s: Unable to init global timer: %x\n", __func__, ret);+ goto err_free_ingenic_ost; + } + + ret = ingenic_ost_percpu_timer_init(np, ingenic_ost); + if (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(ingenic_ost->global_timer_clk); + sched_clock_register(ingenic_ost_global_timer_read_cntl, 32, rate); + + return 0; + +err_ost_global_timer_cleanup: + clocksource_unregister(&ingenic_ost->cs); + clk_disable_unprepare(ingenic_ost->global_timer_clk); + clk_put(ingenic_ost->global_timer_clk); +err_free_ingenic_ost: + kfree(ingenic_ost); + return ret; +} + +TIMER_OF_DECLARE(x1000_ost, "ingenic,x1000-ost", ingenic_ost_init); -- 2.11.0
