| Message ID | 20220407074432.424578-4-vincent.whitchurch@axis.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | clocksource: Add MCT support for ARTPEC-8 | expand |
On 07/04/2022 09:44, Vincent Whitchurch wrote: > If the device tree indicates that the hardware requires that the > processor only use certain local timers, respect that. > > Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com> > --- > > Notes: > v3: > - Use array in devicetree > - Remove addition of global variable > - Split out FRC sharing changes > > drivers/clocksource/exynos_mct.c | 51 ++++++++++++++++++++++++++++---- > 1 file changed, 45 insertions(+), 6 deletions(-) > > diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c > index 12023831dedf..4093a71ff618 100644 > --- a/drivers/clocksource/exynos_mct.c > +++ b/drivers/clocksource/exynos_mct.c > @@ -33,7 +33,7 @@ > #define EXYNOS4_MCT_G_INT_ENB EXYNOS4_MCTREG(0x248) > #define EXYNOS4_MCT_G_WSTAT EXYNOS4_MCTREG(0x24C) > #define _EXYNOS4_MCT_L_BASE EXYNOS4_MCTREG(0x300) > -#define EXYNOS4_MCT_L_BASE(x) (_EXYNOS4_MCT_L_BASE + (0x100 * x)) > +#define EXYNOS4_MCT_L_BASE(x) (_EXYNOS4_MCT_L_BASE + (0x100 * (x))) > #define EXYNOS4_MCT_L_MASK (0xffffff00) > > #define MCT_L_TCNTB_OFFSET (0x00) > @@ -66,6 +66,8 @@ > #define MCT_L0_IRQ 4 > /* Max number of IRQ as per DT binding document */ > #define MCT_NR_IRQS 20 > +/* Max number of local timers */ > +#define MCT_NR_LOCAL (MCT_NR_IRQS - MCT_L0_IRQ) > > enum { > MCT_INT_SPI, > @@ -456,7 +458,6 @@ static int exynos4_mct_starting_cpu(unsigned int cpu) > per_cpu_ptr(&percpu_mct_tick, cpu); > struct clock_event_device *evt = &mevt->evt; > > - mevt->base = EXYNOS4_MCT_L_BASE(cpu); > snprintf(mevt->name, sizeof(mevt->name), "mct_tick%d", cpu); > > evt->name = mevt->name; > @@ -528,7 +529,9 @@ static int __init exynos4_timer_resources(struct device_node *np) > } > Document the arguments, especially focusing on the keys and the contents of local_idx. The code is getting to a state with 3 or 4 variables having similar meaning (IRQ number, local IRQ number, local IRQ index). > static int __init exynos4_timer_interrupts(struct device_node *np, > - unsigned int int_type) > + unsigned int int_type, > + u32 *local_idx, const u32 * > + size_t nr_local) > { > int nr_irqs, i, err, cpu; > > @@ -561,13 +564,19 @@ static int __init exynos4_timer_interrupts(struct device_node *np, > } else { > for_each_possible_cpu(cpu) { > int mct_irq; > + unsigned int irqidx; irq_idx > struct mct_clock_event_device *pcpu_mevt = > per_cpu_ptr(&percpu_mct_tick, cpu); > > + if (cpu >= nr_local) > + break; > + > + irqidx = MCT_L0_IRQ + local_idx[cpu]; > + > pcpu_mevt->evt.irq = -1; > - if (MCT_L0_IRQ + cpu >= ARRAY_SIZE(mct_irqs)) > + if (irqidx >= ARRAY_SIZE(mct_irqs)) > break; > - mct_irq = mct_irqs[MCT_L0_IRQ + cpu]; > + mct_irq = mct_irqs[irqidx]; > > irq_set_status_flags(mct_irq, IRQ_NOAUTOEN); > if (request_irq(mct_irq, > @@ -583,6 +592,15 @@ static int __init exynos4_timer_interrupts(struct device_node *np, > } > } > > + for_each_possible_cpu(cpu) { > + struct mct_clock_event_device *mevt = per_cpu_ptr(&percpu_mct_tick, cpu); > + > + if (cpu >= nr_local) It looks like an error condition, so this should not be handled silently because later base==0 will be used. Probably old code has similar problem... Best regards, Krzysztof
diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c index 12023831dedf..4093a71ff618 100644 --- a/drivers/clocksource/exynos_mct.c +++ b/drivers/clocksource/exynos_mct.c @@ -33,7 +33,7 @@ #define EXYNOS4_MCT_G_INT_ENB EXYNOS4_MCTREG(0x248) #define EXYNOS4_MCT_G_WSTAT EXYNOS4_MCTREG(0x24C) #define _EXYNOS4_MCT_L_BASE EXYNOS4_MCTREG(0x300) -#define EXYNOS4_MCT_L_BASE(x) (_EXYNOS4_MCT_L_BASE + (0x100 * x)) +#define EXYNOS4_MCT_L_BASE(x) (_EXYNOS4_MCT_L_BASE + (0x100 * (x))) #define EXYNOS4_MCT_L_MASK (0xffffff00) #define MCT_L_TCNTB_OFFSET (0x00) @@ -66,6 +66,8 @@ #define MCT_L0_IRQ 4 /* Max number of IRQ as per DT binding document */ #define MCT_NR_IRQS 20 +/* Max number of local timers */ +#define MCT_NR_LOCAL (MCT_NR_IRQS - MCT_L0_IRQ) enum { MCT_INT_SPI, @@ -456,7 +458,6 @@ static int exynos4_mct_starting_cpu(unsigned int cpu) per_cpu_ptr(&percpu_mct_tick, cpu); struct clock_event_device *evt = &mevt->evt; - mevt->base = EXYNOS4_MCT_L_BASE(cpu); snprintf(mevt->name, sizeof(mevt->name), "mct_tick%d", cpu); evt->name = mevt->name; @@ -528,7 +529,9 @@ static int __init exynos4_timer_resources(struct device_node *np) } static int __init exynos4_timer_interrupts(struct device_node *np, - unsigned int int_type) + unsigned int int_type, + u32 *local_idx, + size_t nr_local) { int nr_irqs, i, err, cpu; @@ -561,13 +564,19 @@ static int __init exynos4_timer_interrupts(struct device_node *np, } else { for_each_possible_cpu(cpu) { int mct_irq; + unsigned int irqidx; struct mct_clock_event_device *pcpu_mevt = per_cpu_ptr(&percpu_mct_tick, cpu); + if (cpu >= nr_local) + break; + + irqidx = MCT_L0_IRQ + local_idx[cpu]; + pcpu_mevt->evt.irq = -1; - if (MCT_L0_IRQ + cpu >= ARRAY_SIZE(mct_irqs)) + if (irqidx >= ARRAY_SIZE(mct_irqs)) break; - mct_irq = mct_irqs[MCT_L0_IRQ + cpu]; + mct_irq = mct_irqs[irqidx]; irq_set_status_flags(mct_irq, IRQ_NOAUTOEN); if (request_irq(mct_irq, @@ -583,6 +592,15 @@ static int __init exynos4_timer_interrupts(struct device_node *np, } } + for_each_possible_cpu(cpu) { + struct mct_clock_event_device *mevt = per_cpu_ptr(&percpu_mct_tick, cpu); + + if (cpu >= nr_local) + break; + + mevt->base = EXYNOS4_MCT_L_BASE(local_idx[cpu]); + } + /* Install hotplug callbacks which configure the timer on this CPU */ err = cpuhp_setup_state(CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING, "clockevents/exynos4/mct_timer:starting", @@ -613,13 +631,34 @@ static int __init exynos4_timer_interrupts(struct device_node *np, static int __init mct_init_dt(struct device_node *np, unsigned int int_type) { bool frc_shared = of_property_read_bool(np, "samsung,frc-shared"); + u32 local_idx[MCT_NR_LOCAL] = {0}; + int nr_local; int ret; + nr_local = of_property_count_u32_elems(np, "samsung,local-timers"); + if (nr_local == 0) + return -EINVAL; + if (nr_local > 0) { + if (nr_local > ARRAY_SIZE(local_idx)) + return -EINVAL; + + ret = of_property_read_u32_array(np, "samsung,local-timers", + local_idx, nr_local); + if (ret) + return ret; + } else { + int i; + + nr_local = ARRAY_SIZE(local_idx); + for (i = 0; i < nr_local; i++) + local_idx[i] = i; + } + ret = exynos4_timer_resources(np); if (ret) return ret; - ret = exynos4_timer_interrupts(np, int_type); + ret = exynos4_timer_interrupts(np, int_type, local_idx, nr_local); if (ret) return ret;
If the device tree indicates that the hardware requires that the processor only use certain local timers, respect that. Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com> --- Notes: v3: - Use array in devicetree - Remove addition of global variable - Split out FRC sharing changes drivers/clocksource/exynos_mct.c | 51 ++++++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 6 deletions(-)