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

From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: vijayakannan.ayyathurai@intel.com, tglx@linutronix.de,
	robh+dt@kernel.org, catalin.marinas@arm.com, will@kernel.org
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	andriy.shevchenko@linux.intel.com, mgross@linux.intel.com,
	wan.ahmad.zainie.wan.mohamad@intel.com,
	lakshmi.bai.raja.subramanian@intel.com, chen.yong.seow@intel.com
Subject: Re: [PATCH v2 2/2] clocksource: Add Intel Keem Bay Timer Support
Date: Mon, 18 Jan 2021 16:56:17 +0100	[thread overview]
Message-ID: <73330c8b-4633-4a13-b4b5-7405ff1661fe@linaro.org> (raw)
In-Reply-To: <2a7038bc170e31a668995c367bbae018bf805dbe.1609306622.git.vijayakannan.ayyathurai@intel.com>

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

[ ... ]

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