Re: [PATCH v2 2/2] clocksource: Add Intel Keem Bay Timer Support

From: Daniel Lezcano
Date: Mon Jan 18 2021 - 11:01:52 EST


On 30/12/2020 07:25, vijayakannan.ayyathurai@xxxxxxxxx wrote:
> From: Vijayakannan Ayyathurai <vijayakannan.ayyathurai@xxxxxxxxx>

[ ... ]

> +static struct timer_of keembay_ce_to = {
> + .flags = TIMER_OF_IRQ | TIMER_OF_BASE | TIMER_OF_CLOCK,
> + .clkevt = {
> + .name = "keembay_sys_clkevt",
> + .features = CLOCK_EVT_FEAT_PERIODIC |
> + CLOCK_EVT_FEAT_ONESHOT |
> + CLOCK_EVT_FEAT_DYNIRQ,
> + .rating = TIM_RATING,
> + .set_next_event = keembay_timer_set_next_event,
> + .set_state_periodic = keembay_timer_periodic,
> + .set_state_shutdown = keembay_timer_shutdown,
> + },
> + .of_base = {
> + .index = 0,
> + },
> + .of_irq = {
> + .handler = keembay_timer_isr,
> + .flags = IRQF_TIMER | IRQF_IRQPOLL,

Is the IRQPOLL flag really needed here ?

> + },
> +};
> +
> +static int __init keembay_clockevent_init(struct device_node *np,
> + struct keembay_init_data *data)
> +{
> + u32 val;
> + int ret;
> +
> + data->mask = TIM_CONFIG_PRESCALER_ENABLE;
> + data->cfg = &keembay_ce_to;
> + ret = keembay_timer_setup(np, data);
> + if (ret)
> + return ret;
> +
> + val = readl(data->base + TIM_RELOAD_VAL_OFFSET);
> +
> + keembay_ce_to.clkevt.cpumask = cpumask_of(0);
> + keembay_ce_to.of_clk.rate = keembay_ce_to.of_clk.rate / (val + 1);
> +
> + keembay_timer_disable(timer_of_base(&keembay_ce_to));
> +
> + clockevents_config_and_register(&keembay_ce_to.clkevt,
> + timer_of_rate(&keembay_ce_to), 1, U32_MAX);
> + return 0;
> +}
> +
> +static struct timer_of keembay_cs_to = {
> + .flags = TIMER_OF_BASE | TIMER_OF_CLOCK,
> + .of_base = {
> + .index = 1,
> + },
> +};
> +
> +static u64 notrace keembay_clocksource_read(struct clocksource *cs)
> +{
> + return lo_hi_readq(timer_of_base(&keembay_cs_to));
> +}
> +
> +static struct clocksource keembay_counter = {
> + .name = "keembay_sys_counter",
> + .rating = TIM_RATING,
> + .read = keembay_clocksource_read,
> + .mask = CLOCKSOURCE_MASK(TIM_CLKSRC_BITS),
> + .flags = CLOCK_SOURCE_IS_CONTINUOUS |
> + CLOCK_SOURCE_SUSPEND_NONSTOP,
> +};
> +
> +static int __init keembay_clocksource_init(struct device_node *np,
> + struct keembay_init_data *data)
> +{
> + int ret;
> +
> + data->mask = TIM_CONFIG_ENABLE;
> + data->cfg = &keembay_cs_to;
> + ret = keembay_timer_setup(np, data);
> + if (ret)
> + return ret;
> +
> + return clocksource_register_hz(&keembay_counter, timer_of_rate(&keembay_cs_to));
> +}
> +
> +static int __init keembay_timer_init(struct device_node *np)
> +{
> + struct keembay_init_data data;
> + int ret;
> +
> + data.base = of_iomap(np, 2);
> + if (!data.base)
> + return -ENXIO;
> +
> + ret = keembay_clocksource_init(np, &data);
> + if (ret)
> + goto exit;
> +
> + ret = keembay_clockevent_init(np, &data);

Is this missing ?

if (ret)
goto exit;

return 0;

> +
> +exit:
> + keembay_timer_cleanup(np, &data);
> +
> + return ret;
> +}
> +
> +TIMER_OF_DECLARE(keembay_timer, "intel,keembay-timer", keembay_timer_init);
>


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