diff mbox

[v8,09/16] clockevents/drivers: Add STM32 Timer driver

Message ID 1431158038-3813-10-git-send-email-mcoquelin.stm32@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Maxime Coquelin May 9, 2015, 7:53 a.m. UTC
STM32 MCUs feature 16 and 32 bits general purpose timers with prescalers.
The drivers detects whether the time is 16 or 32 bits, and applies a
1024 prescaler value if it is 16 bits.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
---
 drivers/clocksource/Kconfig       |   8 ++
 drivers/clocksource/Makefile      |   1 +
 drivers/clocksource/timer-stm32.c | 184 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 193 insertions(+)
 create mode 100644 drivers/clocksource/timer-stm32.c

Comments

Maxime Coquelin May 18, 2015, 12:59 p.m. UTC | #1
Daniel,

2015-05-09 9:53 GMT+02:00 Maxime Coquelin <mcoquelin.stm32@gmail.com>:
> STM32 MCUs feature 16 and 32 bits general purpose timers with prescalers.
> The drivers detects whether the time is 16 or 32 bits, and applies a
> 1024 prescaler value if it is 16 bits.
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> ---
>  drivers/clocksource/Kconfig       |   8 ++
>  drivers/clocksource/Makefile      |   1 +
>  drivers/clocksource/timer-stm32.c | 184 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 193 insertions(+)
>  create mode 100644 drivers/clocksource/timer-stm32.c
>

I forgot to as you to also pick this one an its documentation (patch 8).

Best regards,
Maxime
Daniel Lezcano May 18, 2015, 1:10 p.m. UTC | #2
On 05/09/2015 09:53 AM, Maxime Coquelin wrote:
> STM32 MCUs feature 16 and 32 bits general purpose timers with prescalers.
> The drivers detects whether the time is 16 or 32 bits, and applies a
> 1024 prescaler value if it is 16 bits.
>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> ---
>   drivers/clocksource/Kconfig       |   8 ++
>   drivers/clocksource/Makefile      |   1 +
>   drivers/clocksource/timer-stm32.c | 184 ++++++++++++++++++++++++++++++++++++++
>   3 files changed, 193 insertions(+)
>   create mode 100644 drivers/clocksource/timer-stm32.c
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index bf9364c..2443520 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -106,6 +106,14 @@ config CLKSRC_EFM32
>   	  Support to use the timers of EFM32 SoCs as clock source and clock
>   	  event device.
>
> +config CLKSRC_STM32
> +	bool "Clocksource for STM32 SoCs" if !ARCH_STM32
> +	depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST)

Are the interactive bool and the 'COMPILE_TEST' necessary ?


> +	select CLKSRC_MMIO
> +	default ARCH_STM32
> +	help
> +	  Support to use the timers of STM32 SoCs as clock event device.
> +
>   config ARM_ARCH_TIMER
>   	bool
>   	select CLKSRC_OF if OF
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index d510c54..888a7df 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -36,6 +36,7 @@ obj-$(CONFIG_ARCH_NSPIRE)	+= zevio-timer.o
>   obj-$(CONFIG_ARCH_BCM_MOBILE)	+= bcm_kona_timer.o
>   obj-$(CONFIG_CADENCE_TTC_TIMER)	+= cadence_ttc_timer.o
>   obj-$(CONFIG_CLKSRC_EFM32)	+= time-efm32.o
> +obj-$(CONFIG_CLKSRC_STM32)	+= timer-stm32.o
>   obj-$(CONFIG_CLKSRC_EXYNOS_MCT)	+= exynos_mct.o
>   obj-$(CONFIG_CLKSRC_SAMSUNG_PWM)	+= samsung_pwm_timer.o
>   obj-$(CONFIG_FSL_FTM_TIMER)	+= fsl_ftm_timer.o
> diff --git a/drivers/clocksource/timer-stm32.c b/drivers/clocksource/timer-stm32.c
> new file mode 100644
> index 0000000..fad2e2e
> --- /dev/null
> +++ b/drivers/clocksource/timer-stm32.c
> @@ -0,0 +1,184 @@
> +/*
> + * Copyright (C) Maxime Coquelin 2015
> + * Author:  Maxime Coquelin <mcoquelin.stm32@gmail.com>
> + * License terms:  GNU General Public License (GPL), version 2
> + *
> + * Inspired by time-efm32.c from Uwe Kleine-Koenig
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/clocksource.h>
> +#include <linux/clockchips.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/clk.h>
> +#include <linux/reset.h>
> +
> +#define TIM_CR1		0x00
> +#define TIM_DIER	0x0c
> +#define TIM_SR		0x10
> +#define TIM_EGR		0x14
> +#define TIM_PSC		0x28
> +#define TIM_ARR		0x2c
> +
> +#define TIM_CR1_CEN	BIT(0)
> +#define TIM_CR1_OPM	BIT(3)
> +#define TIM_CR1_ARPE	BIT(7)
> +
> +#define TIM_DIER_UIE	BIT(0)
> +
> +#define TIM_SR_UIF	BIT(0)
> +
> +#define TIM_EGR_UG	BIT(0)
> +
> +struct stm32_clock_event_ddata {
> +	struct clock_event_device evtdev;
> +	unsigned periodic_top;
> +	void __iomem *base;
> +};
> +
> +static void stm32_clock_event_set_mode(enum clock_event_mode mode,
> +				       struct clock_event_device *evtdev)
> +{
> +	struct stm32_clock_event_ddata *data =
> +		container_of(evtdev, struct stm32_clock_event_ddata, evtdev);
> +	void *base = data->base;
> +
> +	switch (mode) {
> +	case CLOCK_EVT_MODE_PERIODIC:
> +		writel_relaxed(data->periodic_top, base + TIM_ARR);
> +		writel_relaxed(TIM_CR1_ARPE | TIM_CR1_CEN, base + TIM_CR1);
> +		break;
> +
> +	case CLOCK_EVT_MODE_ONESHOT:
> +	default:
> +		writel_relaxed(0, base + TIM_CR1);
> +		break;
> +	}
> +}
> +
> +static int stm32_clock_event_set_next_event(unsigned long evt,
> +					    struct clock_event_device *evtdev)
> +{
> +	struct stm32_clock_event_ddata *data =
> +		container_of(evtdev, struct stm32_clock_event_ddata, evtdev);
> +
> +	writel_relaxed(evt, data->base + TIM_ARR);
> +	writel_relaxed(TIM_CR1_ARPE | TIM_CR1_OPM | TIM_CR1_CEN,
> +		       data->base + TIM_CR1);
> +
> +	return 0;
> +}
> +
> +static irqreturn_t stm32_clock_event_handler(int irq, void *dev_id)
> +{
> +	struct stm32_clock_event_ddata *data = dev_id;
> +
> +	writel_relaxed(0, data->base + TIM_SR);
> +
> +	data->evtdev.event_handler(&data->evtdev);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static struct stm32_clock_event_ddata clock_event_ddata = {
> +	.evtdev = {
> +		.name = "stm32 clockevent",
> +		.features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC,
> +		.set_mode = stm32_clock_event_set_mode,
> +		.set_next_event = stm32_clock_event_set_next_event,
> +		.rating = 200,
> +	},
> +};
> +
> +static void __init stm32_clockevent_init(struct device_node *np)
> +{
> +	struct stm32_clock_event_ddata *data = &clock_event_ddata;
> +	struct clk *clk;
> +	struct reset_control *rstc;
> +	unsigned long rate, max_delta;
> +	int irq, ret, bits, prescaler = 1;
> +
> +	clk = of_clk_get(np, 0);
> +	if (IS_ERR(clk)) {
> +		ret = PTR_ERR(clk);
> +		pr_err("failed to get clock for clockevent (%d)\n", ret);
> +		goto err_clk_get;
> +	}
> +
> +	ret = clk_prepare_enable(clk);
> +	if (ret) {
> +		pr_err("failed to enable timer clock for clockevent (%d)\n",
> +		       ret);
> +		goto err_clk_enable;
> +	}
> +
> +	rate = clk_get_rate(clk);
> +
> +	rstc = of_reset_control_get(np, NULL);
> +	if (!IS_ERR(rstc)) {
> +		reset_control_assert(rstc);
> +		reset_control_deassert(rstc);
> +	}
> +
> +	data->base = of_iomap(np, 0);
> +	if (!data->base) {
> +		pr_err("failed to map registers for clockevent\n");
> +		goto err_iomap;
> +	}
> +
> +	irq = irq_of_parse_and_map(np, 0);
> +	if (!irq) {
> +		pr_err("%s: failed to get irq.\n", np->full_name);
> +		goto err_get_irq;
> +	}
> +
> +	/* Detect whether the timer is 16 or 32 bits */
> +	writel_relaxed(~0UL, data->base + TIM_ARR);
> +	max_delta = readl_relaxed(data->base + TIM_ARR);
> +	if (max_delta == ~0UL) {
> +		prescaler = 1;
> +		bits = 32;
> +	} else {
> +		prescaler = 1024;
> +		bits = 16;
> +	}
> +	writel_relaxed(0, data->base + TIM_ARR);
> +
> +	writel_relaxed(prescaler - 1, data->base + TIM_PSC);
> +	writel_relaxed(TIM_EGR_UG, data->base + TIM_EGR);
> +	writel_relaxed(TIM_DIER_UIE, data->base + TIM_DIER);
> +	writel_relaxed(0, data->base + TIM_SR);
> +
> +	data->periodic_top = DIV_ROUND_CLOSEST(rate, prescaler * HZ);
> +
> +	clockevents_config_and_register(&data->evtdev,
> +					DIV_ROUND_CLOSEST(rate, prescaler),
> +					0x1, max_delta);
> +
> +	ret = request_irq(irq, stm32_clock_event_handler, IRQF_TIMER,
> +			"stm32 clockevent", data);
> +	if (ret) {
> +		pr_err("%s: failed to request irq.\n", np->full_name);
> +		goto err_get_irq;
> +	}
> +
> +	pr_info("%s: STM32 clockevent driver initialized (%d bits)\n",
> +			np->full_name, bits);
> +
> +	return;
> +
> +err_get_irq:
> +	iounmap(data->base);
> +err_iomap:
> +	clk_disable_unprepare(clk);
> +err_clk_enable:
> +	clk_put(clk);
> +err_clk_get:
> +	return;
> +}
> +
> +CLOCKSOURCE_OF_DECLARE(stm32, "st,stm32-timer", stm32_clockevent_init);
>
Maxime Coquelin May 18, 2015, 2:03 p.m. UTC | #3
2015-05-18 15:10 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
> On 05/09/2015 09:53 AM, Maxime Coquelin wrote:
>>
>> STM32 MCUs feature 16 and 32 bits general purpose timers with prescalers.
>> The drivers detects whether the time is 16 or 32 bits, and applies a
>> 1024 prescaler value if it is 16 bits.
>>
>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>> ---
>>   drivers/clocksource/Kconfig       |   8 ++
>>   drivers/clocksource/Makefile      |   1 +
>>   drivers/clocksource/timer-stm32.c | 184
>> ++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 193 insertions(+)
>>   create mode 100644 drivers/clocksource/timer-stm32.c
>>
>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>> index bf9364c..2443520 100644
>> --- a/drivers/clocksource/Kconfig
>> +++ b/drivers/clocksource/Kconfig
>> @@ -106,6 +106,14 @@ config CLKSRC_EFM32
>>           Support to use the timers of EFM32 SoCs as clock source and
>> clock
>>           event device.
>>
>> +config CLKSRC_STM32
>> +       bool "Clocksource for STM32 SoCs" if !ARCH_STM32
>> +       depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST)
>
>
> Are the interactive bool and the 'COMPILE_TEST' necessary ?
>

The interactive bool is necessary if we want to be able to
select/deselect it in COMPILE_TEST configuration.
And personnaly, I think COMPILE_TEST use makes sense.

Note that other timer drivers are doing the same thing today
(CLKSRC_EFM32, SH_TIMER_CMT, EM_TIMER_STI...).

Do you have a specific concern regarding COMPILE_TEST?

Kind regards,
Maxime
Daniel Lezcano May 19, 2015, 8:16 a.m. UTC | #4
On 05/18/2015 04:03 PM, Maxime Coquelin wrote:
> 2015-05-18 15:10 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
>> On 05/09/2015 09:53 AM, Maxime Coquelin wrote:
>>>
>>> STM32 MCUs feature 16 and 32 bits general purpose timers with prescalers.
>>> The drivers detects whether the time is 16 or 32 bits, and applies a
>>> 1024 prescaler value if it is 16 bits.
>>>
>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
>>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>>> ---
>>>    drivers/clocksource/Kconfig       |   8 ++
>>>    drivers/clocksource/Makefile      |   1 +
>>>    drivers/clocksource/timer-stm32.c | 184
>>> ++++++++++++++++++++++++++++++++++++++
>>>    3 files changed, 193 insertions(+)
>>>    create mode 100644 drivers/clocksource/timer-stm32.c
>>>
>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>> index bf9364c..2443520 100644
>>> --- a/drivers/clocksource/Kconfig
>>> +++ b/drivers/clocksource/Kconfig
>>> @@ -106,6 +106,14 @@ config CLKSRC_EFM32
>>>            Support to use the timers of EFM32 SoCs as clock source and
>>> clock
>>>            event device.
>>>
>>> +config CLKSRC_STM32
>>> +       bool "Clocksource for STM32 SoCs" if !ARCH_STM32
>>> +       depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST)
>>
>>
>> Are the interactive bool and the 'COMPILE_TEST' necessary ?
>>
>
> The interactive bool is necessary if we want to be able to
> select/deselect it in COMPILE_TEST configuration.
> And personnaly, I think COMPILE_TEST use makes sense.
>
> Note that other timer drivers are doing the same thing today
> (CLKSRC_EFM32, SH_TIMER_CMT, EM_TIMER_STI...).
>
> Do you have a specific concern regarding COMPILE_TEST?

Actually, we try to keep the timer selection non-interactive and let the 
platform's Kconfig to select the timer.

I like when the code is consistent. The COMPILE_TEST was introduced and 
created a precedence. I would like to get rid of the interactive timer 
selection but I did not have time to go through this yet.
Maxime Coquelin May 19, 2015, 8:55 a.m. UTC | #5
2015-05-19 10:16 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
> On 05/18/2015 04:03 PM, Maxime Coquelin wrote:
>>
>> 2015-05-18 15:10 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
>>>
>>> On 05/09/2015 09:53 AM, Maxime Coquelin wrote:
>>>>
>>>>
>>>> STM32 MCUs feature 16 and 32 bits general purpose timers with
>>>> prescalers.
>>>> The drivers detects whether the time is 16 or 32 bits, and applies a
>>>> 1024 prescaler value if it is 16 bits.
>>>>
>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>>>> ---
>>>>    drivers/clocksource/Kconfig       |   8 ++
>>>>    drivers/clocksource/Makefile      |   1 +
>>>>    drivers/clocksource/timer-stm32.c | 184
>>>> ++++++++++++++++++++++++++++++++++++++
>>>>    3 files changed, 193 insertions(+)
>>>>    create mode 100644 drivers/clocksource/timer-stm32.c
>>>>
>>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>>> index bf9364c..2443520 100644
>>>> --- a/drivers/clocksource/Kconfig
>>>> +++ b/drivers/clocksource/Kconfig
>>>> @@ -106,6 +106,14 @@ config CLKSRC_EFM32
>>>>            Support to use the timers of EFM32 SoCs as clock source and
>>>> clock
>>>>            event device.
>>>>
>>>> +config CLKSRC_STM32
>>>> +       bool "Clocksource for STM32 SoCs" if !ARCH_STM32
>>>> +       depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST)
>>>
>>>
>>>
>>> Are the interactive bool and the 'COMPILE_TEST' necessary ?
>>>
>>
>> The interactive bool is necessary if we want to be able to
>> select/deselect it in COMPILE_TEST configuration.
>> And personnaly, I think COMPILE_TEST use makes sense.
>>
>> Note that other timer drivers are doing the same thing today
>> (CLKSRC_EFM32, SH_TIMER_CMT, EM_TIMER_STI...).
>>
>> Do you have a specific concern regarding COMPILE_TEST?
>
>
> Actually, we try to keep the timer selection non-interactive and let the
> platform's Kconfig to select the timer.

Ok.

>
> I like when the code is consistent. The COMPILE_TEST was introduced and
> created a precedence. I would like to get rid of the interactive timer
> selection but I did not have time to go through this yet.

Indeed, consistency is important.
On my side, I don't have a strong opinion regarding the COMPILE_TEST thing.
IMHO, it is more a subsystem's maintainer choice.

So, if as a maintainer you don't use it and prefer not supporting it,
I'm fine to provide you a new version without COMPILE_TEST.
Doing that, the interactive selection will disappear too.

I can provide you a new version this evenning.

Best regards,
Maxime

>
>
>
> --
>  <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
>
Daniel Lezcano May 19, 2015, 9:06 a.m. UTC | #6
On 05/19/2015 10:55 AM, Maxime Coquelin wrote:
> 2015-05-19 10:16 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
>> On 05/18/2015 04:03 PM, Maxime Coquelin wrote:
>>>
>>> 2015-05-18 15:10 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
>>>>
>>>> On 05/09/2015 09:53 AM, Maxime Coquelin wrote:
>>>>>
>>>>>
>>>>> STM32 MCUs feature 16 and 32 bits general purpose timers with
>>>>> prescalers.
>>>>> The drivers detects whether the time is 16 or 32 bits, and applies a
>>>>> 1024 prescaler value if it is 16 bits.
>>>>>
>>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>>>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>>>>> ---
>>>>>     drivers/clocksource/Kconfig       |   8 ++
>>>>>     drivers/clocksource/Makefile      |   1 +
>>>>>     drivers/clocksource/timer-stm32.c | 184
>>>>> ++++++++++++++++++++++++++++++++++++++
>>>>>     3 files changed, 193 insertions(+)
>>>>>     create mode 100644 drivers/clocksource/timer-stm32.c
>>>>>
>>>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>>>> index bf9364c..2443520 100644
>>>>> --- a/drivers/clocksource/Kconfig
>>>>> +++ b/drivers/clocksource/Kconfig
>>>>> @@ -106,6 +106,14 @@ config CLKSRC_EFM32
>>>>>             Support to use the timers of EFM32 SoCs as clock source and
>>>>> clock
>>>>>             event device.
>>>>>
>>>>> +config CLKSRC_STM32
>>>>> +       bool "Clocksource for STM32 SoCs" if !ARCH_STM32
>>>>> +       depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST)
>>>>
>>>>
>>>>
>>>> Are the interactive bool and the 'COMPILE_TEST' necessary ?
>>>>
>>>
>>> The interactive bool is necessary if we want to be able to
>>> select/deselect it in COMPILE_TEST configuration.
>>> And personnaly, I think COMPILE_TEST use makes sense.
>>>
>>> Note that other timer drivers are doing the same thing today
>>> (CLKSRC_EFM32, SH_TIMER_CMT, EM_TIMER_STI...).
>>>
>>> Do you have a specific concern regarding COMPILE_TEST?
>>
>>
>> Actually, we try to keep the timer selection non-interactive and let the
>> platform's Kconfig to select the timer.
>
> Ok.
>
>>
>> I like when the code is consistent. The COMPILE_TEST was introduced and
>> created a precedence. I would like to get rid of the interactive timer
>> selection but I did not have time to go through this yet.
>
> Indeed, consistency is important.
> On my side, I don't have a strong opinion regarding the COMPILE_TEST thing.
> IMHO, it is more a subsystem's maintainer choice.
>
> So, if as a maintainer you don't use it and prefer not supporting it,
> I'm fine to provide you a new version without COMPILE_TEST.
> Doing that, the interactive selection will disappear too.
>
> I can provide you a new version this evenning.

Ok, great.

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
>>
Maxime Coquelin May 19, 2015, 9:44 a.m. UTC | #7
2015-05-19 11:06 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
> On 05/19/2015 10:55 AM, Maxime Coquelin wrote:
>>
>> 2015-05-19 10:16 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
>>>
>>> On 05/18/2015 04:03 PM, Maxime Coquelin wrote:
>>>>
>>>>
>>>> 2015-05-18 15:10 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
>>>>>
>>>>>
>>>>> On 05/09/2015 09:53 AM, Maxime Coquelin wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> STM32 MCUs feature 16 and 32 bits general purpose timers with
>>>>>> prescalers.
>>>>>> The drivers detects whether the time is 16 or 32 bits, and applies a
>>>>>> 1024 prescaler value if it is 16 bits.
>>>>>>
>>>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>>>>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>>>>>> ---
>>>>>>     drivers/clocksource/Kconfig       |   8 ++
>>>>>>     drivers/clocksource/Makefile      |   1 +
>>>>>>     drivers/clocksource/timer-stm32.c | 184
>>>>>> ++++++++++++++++++++++++++++++++++++++
>>>>>>     3 files changed, 193 insertions(+)
>>>>>>     create mode 100644 drivers/clocksource/timer-stm32.c
>>>>>>
>>>>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>>>>> index bf9364c..2443520 100644
>>>>>> --- a/drivers/clocksource/Kconfig
>>>>>> +++ b/drivers/clocksource/Kconfig
>>>>>> @@ -106,6 +106,14 @@ config CLKSRC_EFM32
>>>>>>             Support to use the timers of EFM32 SoCs as clock source
>>>>>> and
>>>>>> clock
>>>>>>             event device.
>>>>>>
>>>>>> +config CLKSRC_STM32
>>>>>> +       bool "Clocksource for STM32 SoCs" if !ARCH_STM32
>>>>>> +       depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST)
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Are the interactive bool and the 'COMPILE_TEST' necessary ?
>>>>>
>>>>
>>>> The interactive bool is necessary if we want to be able to
>>>> select/deselect it in COMPILE_TEST configuration.
>>>> And personnaly, I think COMPILE_TEST use makes sense.
>>>>
>>>> Note that other timer drivers are doing the same thing today
>>>> (CLKSRC_EFM32, SH_TIMER_CMT, EM_TIMER_STI...).
>>>>
>>>> Do you have a specific concern regarding COMPILE_TEST?
>>>
>>>
>>>
>>> Actually, we try to keep the timer selection non-interactive and let the
>>> platform's Kconfig to select the timer.
>>
>>
>> Ok.
>>
>>>
>>> I like when the code is consistent. The COMPILE_TEST was introduced and
>>> created a precedence. I would like to get rid of the interactive timer
>>> selection but I did not have time to go through this yet.
>>
>>
>> Indeed, consistency is important.
>> On my side, I don't have a strong opinion regarding the COMPILE_TEST
>> thing.
>> IMHO, it is more a subsystem's maintainer choice.
>>
>> So, if as a maintainer you don't use it and prefer not supporting it,
>> I'm fine to provide you a new version without COMPILE_TEST.
>> Doing that, the interactive selection will disappear too.
>>
>> I can provide you a new version this evenning.
>
>
> Ok, great.

Is the below Kconfig entry fine for you?

config CLKSRC_STM32
    def_bool y if ARCH_STM32
    select CLKSRC_MMIO

Best regards,
Maxime


>
> 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
>>>
>
>
> --
>  <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
>
Daniel Lezcano May 19, 2015, 9:59 a.m. UTC | #8
On 05/19/2015 11:44 AM, Maxime Coquelin wrote:
> 2015-05-19 11:06 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
>> On 05/19/2015 10:55 AM, Maxime Coquelin wrote:
>>>
>>> 2015-05-19 10:16 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
>>>>
>>>> On 05/18/2015 04:03 PM, Maxime Coquelin wrote:
>>>>>
>>>>>
>>>>> 2015-05-18 15:10 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
>>>>>>
>>>>>>
>>>>>> On 05/09/2015 09:53 AM, Maxime Coquelin wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> STM32 MCUs feature 16 and 32 bits general purpose timers with
>>>>>>> prescalers.
>>>>>>> The drivers detects whether the time is 16 or 32 bits, and applies a
>>>>>>> 1024 prescaler value if it is 16 bits.
>>>>>>>
>>>>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>>>>>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>>>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>>>>>>> ---
>>>>>>>      drivers/clocksource/Kconfig       |   8 ++
>>>>>>>      drivers/clocksource/Makefile      |   1 +
>>>>>>>      drivers/clocksource/timer-stm32.c | 184
>>>>>>> ++++++++++++++++++++++++++++++++++++++
>>>>>>>      3 files changed, 193 insertions(+)
>>>>>>>      create mode 100644 drivers/clocksource/timer-stm32.c
>>>>>>>
>>>>>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>>>>>> index bf9364c..2443520 100644
>>>>>>> --- a/drivers/clocksource/Kconfig
>>>>>>> +++ b/drivers/clocksource/Kconfig
>>>>>>> @@ -106,6 +106,14 @@ config CLKSRC_EFM32
>>>>>>>              Support to use the timers of EFM32 SoCs as clock source
>>>>>>> and
>>>>>>> clock
>>>>>>>              event device.
>>>>>>>
>>>>>>> +config CLKSRC_STM32
>>>>>>> +       bool "Clocksource for STM32 SoCs" if !ARCH_STM32
>>>>>>> +       depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST)
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Are the interactive bool and the 'COMPILE_TEST' necessary ?
>>>>>>
>>>>>
>>>>> The interactive bool is necessary if we want to be able to
>>>>> select/deselect it in COMPILE_TEST configuration.
>>>>> And personnaly, I think COMPILE_TEST use makes sense.
>>>>>
>>>>> Note that other timer drivers are doing the same thing today
>>>>> (CLKSRC_EFM32, SH_TIMER_CMT, EM_TIMER_STI...).
>>>>>
>>>>> Do you have a specific concern regarding COMPILE_TEST?
>>>>
>>>>
>>>>
>>>> Actually, we try to keep the timer selection non-interactive and let the
>>>> platform's Kconfig to select the timer.
>>>
>>>
>>> Ok.
>>>
>>>>
>>>> I like when the code is consistent. The COMPILE_TEST was introduced and
>>>> created a precedence. I would like to get rid of the interactive timer
>>>> selection but I did not have time to go through this yet.
>>>
>>>
>>> Indeed, consistency is important.
>>> On my side, I don't have a strong opinion regarding the COMPILE_TEST
>>> thing.
>>> IMHO, it is more a subsystem's maintainer choice.
>>>
>>> So, if as a maintainer you don't use it and prefer not supporting it,
>>> I'm fine to provide you a new version without COMPILE_TEST.
>>> Doing that, the interactive selection will disappear too.
>>>
>>> I can provide you a new version this evenning.
>>
>>
>> Ok, great.
>
> Is the below Kconfig entry fine for you?
>
> config CLKSRC_STM32
>      def_bool y if ARCH_STM32
>      select CLKSRC_MMIO

config CLKSRC_STM32
	bool
	select CLKSRC_MMIO

and in the arch/arm/mach-stm32/Kconfig add select CLKSRC_STM32

> Best regards,
> Maxime
>
>
>>
>> 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
>>>>
>>
>>
>> --
>>   <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
>>
Maxime Coquelin May 19, 2015, 10:02 a.m. UTC | #9
2015-05-19 11:59 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
> On 05/19/2015 11:44 AM, Maxime Coquelin wrote:
>>
>> 2015-05-19 11:06 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
>>>
>>> On 05/19/2015 10:55 AM, Maxime Coquelin wrote:
>>>>
>>>>
>>>> 2015-05-19 10:16 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
>>>>>
>>>>>
>>>>> On 05/18/2015 04:03 PM, Maxime Coquelin wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> 2015-05-18 15:10 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 05/09/2015 09:53 AM, Maxime Coquelin wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> STM32 MCUs feature 16 and 32 bits general purpose timers with
>>>>>>>> prescalers.
>>>>>>>> The drivers detects whether the time is 16 or 32 bits, and applies a
>>>>>>>> 1024 prescaler value if it is 16 bits.
>>>>>>>>
>>>>>>>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>>>>>>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>>>>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>>>>>>>> ---
>>>>>>>>      drivers/clocksource/Kconfig       |   8 ++
>>>>>>>>      drivers/clocksource/Makefile      |   1 +
>>>>>>>>      drivers/clocksource/timer-stm32.c | 184
>>>>>>>> ++++++++++++++++++++++++++++++++++++++
>>>>>>>>      3 files changed, 193 insertions(+)
>>>>>>>>      create mode 100644 drivers/clocksource/timer-stm32.c
>>>>>>>>
>>>>>>>> diff --git a/drivers/clocksource/Kconfig
>>>>>>>> b/drivers/clocksource/Kconfig
>>>>>>>> index bf9364c..2443520 100644
>>>>>>>> --- a/drivers/clocksource/Kconfig
>>>>>>>> +++ b/drivers/clocksource/Kconfig
>>>>>>>> @@ -106,6 +106,14 @@ config CLKSRC_EFM32
>>>>>>>>              Support to use the timers of EFM32 SoCs as clock source
>>>>>>>> and
>>>>>>>> clock
>>>>>>>>              event device.
>>>>>>>>
>>>>>>>> +config CLKSRC_STM32
>>>>>>>> +       bool "Clocksource for STM32 SoCs" if !ARCH_STM32
>>>>>>>> +       depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST)
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Are the interactive bool and the 'COMPILE_TEST' necessary ?
>>>>>>>
>>>>>>
>>>>>> The interactive bool is necessary if we want to be able to
>>>>>> select/deselect it in COMPILE_TEST configuration.
>>>>>> And personnaly, I think COMPILE_TEST use makes sense.
>>>>>>
>>>>>> Note that other timer drivers are doing the same thing today
>>>>>> (CLKSRC_EFM32, SH_TIMER_CMT, EM_TIMER_STI...).
>>>>>>
>>>>>> Do you have a specific concern regarding COMPILE_TEST?
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Actually, we try to keep the timer selection non-interactive and let
>>>>> the
>>>>> platform's Kconfig to select the timer.
>>>>
>>>>
>>>>
>>>> Ok.
>>>>
>>>>>
>>>>> I like when the code is consistent. The COMPILE_TEST was introduced and
>>>>> created a precedence. I would like to get rid of the interactive timer
>>>>> selection but I did not have time to go through this yet.
>>>>
>>>>
>>>>
>>>> Indeed, consistency is important.
>>>> On my side, I don't have a strong opinion regarding the COMPILE_TEST
>>>> thing.
>>>> IMHO, it is more a subsystem's maintainer choice.
>>>>
>>>> So, if as a maintainer you don't use it and prefer not supporting it,
>>>> I'm fine to provide you a new version without COMPILE_TEST.
>>>> Doing that, the interactive selection will disappear too.
>>>>
>>>> I can provide you a new version this evenning.
>>>
>>>
>>>
>>> Ok, great.
>>
>>
>> Is the below Kconfig entry fine for you?
>>
>> config CLKSRC_STM32
>>      def_bool y if ARCH_STM32
>>      select CLKSRC_MMIO
>
>
> config CLKSRC_STM32
>         bool
>         select CLKSRC_MMIO
>
> and in the arch/arm/mach-stm32/Kconfig add select CLKSRC_STM32

Ok, I will send a patch for arch/arm/Kconfig, as Arnd already applied
the one intruducing ARCH_STM32.

Thanks,
Maxime
>
>
>> Best regards,
>> Maxime
>>
>>
>>>
>>> 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
>>>>>
>>>
>>>
>>> --
>>>   <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
>>>
>
>
> --
>  <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
>
Arnd Bergmann May 19, 2015, 10:55 a.m. UTC | #10
On Tuesday 19 May 2015 12:02:59 Maxime Coquelin wrote:
> >
> > config CLKSRC_STM32
> >         bool
> >         select CLKSRC_MMIO
> >
> > and in the arch/arm/mach-stm32/Kconfig add select CLKSRC_STM32
> 
> Ok, I will send a patch for arch/arm/Kconfig, as Arnd already applied
> the one intruducing ARCH_STM32.
> 
> 

Please also make it possible to select this driver on other architectures
with COMPILE_TEST, so we get coverage from all the x86 test infrastructure.

	Arnd
Thomas Gleixner May 19, 2015, 12:56 p.m. UTC | #11
On Tue, 19 May 2015, Maxime Coquelin wrote:
> 2015-05-19 11:59 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
> >> Is the below Kconfig entry fine for you?
> >>
> >> config CLKSRC_STM32
> >>      def_bool y if ARCH_STM32
> >>      select CLKSRC_MMIO
> >
> >
> > config CLKSRC_STM32
> >         bool
> >         select CLKSRC_MMIO
> >
> > and in the arch/arm/mach-stm32/Kconfig add select CLKSRC_STM32
> 
> Ok, I will send a patch for arch/arm/Kconfig, as Arnd already applied
> the one intruducing ARCH_STM32.

Folks, can you please trim your replies. It's a PITA to scroll down
through several pages to find a single line of content.

Thanks,

	tglx
Russell King - ARM Linux May 19, 2015, 1 p.m. UTC | #12
On Tue, May 19, 2015 at 02:56:43PM +0200, Thomas Gleixner wrote:
> On Tue, 19 May 2015, Maxime Coquelin wrote:
> > 2015-05-19 11:59 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
> > >> Is the below Kconfig entry fine for you?
> > >>
> > >> config CLKSRC_STM32
> > >>      def_bool y if ARCH_STM32
> > >>      select CLKSRC_MMIO
> > >
> > >
> > > config CLKSRC_STM32
> > >         bool
> > >         select CLKSRC_MMIO
> > >
> > > and in the arch/arm/mach-stm32/Kconfig add select CLKSRC_STM32
> > 
> > Ok, I will send a patch for arch/arm/Kconfig, as Arnd already applied
> > the one intruducing ARCH_STM32.
> 
> Folks, can you please trim your replies. It's a PITA to scroll down
> through several pages to find a single line of content.

Absolutely right.  Too many people do not do this, and it's a waste of
readers time.  Maybe we should start ignoring emails which contain only
quoted mail in the first 50 lines.

Also, cutting the rest of the email below the point which you've finished
replying is good etiquette as well.  I've seen a number of posts where
people have added their signature mid-mail and left a huge chunk of quoted
mail below.  That's just not on either.
Maxime Coquelin May 19, 2015, 1:17 p.m. UTC | #13
2015-05-19 15:00 GMT+02:00 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Tue, May 19, 2015 at 02:56:43PM +0200, Thomas Gleixner wrote:
>> Folks, can you please trim your replies. It's a PITA to scroll down
>> through several pages to find a single line of content.
>
> Absolutely right.  Too many people do not do this, and it's a waste of
> readers time.  Maybe we should start ignoring emails which contain only
> quoted mail in the first 50 lines.
>
> Also, cutting the rest of the email below the point which you've finished
> replying is good etiquette as well.  I've seen a number of posts where
> people have added their signature mid-mail and left a huge chunk of quoted
> mail below.  That's just not on either.
>

Ok, I will take care of that.
Sorry for the inconvenience,

Maxime
Maxime Coquelin May 19, 2015, 1:42 p.m. UTC | #14
2015-05-19 12:55 GMT+02:00 Arnd Bergmann <arnd@arndb.de>:
> Please also make it possible to select this driver on other architectures
> with COMPILE_TEST, so we get coverage from all the x86 test infrastructure.

You proposal is to revert back to the original patch, except that
ARCH_STM32 should select CLKSRC_STM32, right?

Daniel, what do you think?

Regards,
Maxime
Daniel Lezcano May 19, 2015, 1:49 p.m. UTC | #15
On 05/19/2015 03:42 PM, Maxime Coquelin wrote:
> 2015-05-19 12:55 GMT+02:00 Arnd Bergmann <arnd@arndb.de>:
>> Please also make it possible to select this driver on other architectures
>> with COMPILE_TEST, so we get coverage from all the x86 test infrastructure.
>
> You proposal is to revert back to the original patch, except that
> ARCH_STM32 should select CLKSRC_STM32, right?
>
> Daniel, what do you think?

It is not exactly as the initial patch.

I think Arnd is proposing:

config CLKSRC_STM32
	bool "Clocksource for STM32 SoCs" if COMPILE_TEST
	select CLKSRC_MMIO
Arnd Bergmann May 19, 2015, 2:05 p.m. UTC | #16
On Tuesday 19 May 2015 15:49:52 Daniel Lezcano wrote:
> On 05/19/2015 03:42 PM, Maxime Coquelin wrote:
> > 2015-05-19 12:55 GMT+02:00 Arnd Bergmann <arnd@arndb.de>:
> >> Please also make it possible to select this driver on other architectures
> >> with COMPILE_TEST, so we get coverage from all the x86 test infrastructure.
> >
> > You proposal is to revert back to the original patch, except that
> > ARCH_STM32 should select CLKSRC_STM32, right?
> >
> > Daniel, what do you think?
> 
> It is not exactly as the initial patch.
> 
> I think Arnd is proposing:
> 
> config CLKSRC_STM32
>         bool "Clocksource for STM32 SoCs" if COMPILE_TEST
>         select CLKSRC_MMIO

Yes, that's fine, although we might also need a 'depends on OF'
line, not sure.

	Arnd
Maxime Coquelin May 19, 2015, 2:41 p.m. UTC | #17
2015-05-19 15:49 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
>
>
> It is not exactly as the initial patch.
>
> I think Arnd is proposing:
>
> config CLKSRC_STM32
>         bool "Clocksource for STM32 SoCs" if COMPILE_TEST
>         select CLKSRC_MMIO
>

Isn't "depends on OF" missing for COMPILE_TEST case?

Other than that, I'm fine with the proposal.
Daniel, is it the case for you too?

Regards,
Maxime
Russell King - ARM Linux May 19, 2015, 2:50 p.m. UTC | #18
On Tue, May 19, 2015 at 04:41:58PM +0200, Maxime Coquelin wrote:
> 2015-05-19 15:49 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
> >
> >
> > It is not exactly as the initial patch.
> >
> > I think Arnd is proposing:
> >
> > config CLKSRC_STM32
> >         bool "Clocksource for STM32 SoCs" if COMPILE_TEST
> >         select CLKSRC_MMIO
> >
> 
> Isn't "depends on OF" missing for COMPILE_TEST case?

config CLKSRC_STM32
	bool "Clocksource for STM32 SoCs" if COMPILE_TEST
	depends on OF
	select CLKSRC_MMIO

This permits CLKSRC_STM32 to be selected by STM32 (provided OF is enabled,
it's always going to be for that case, right?) while allowing the option
to be visible when both OF!=n and COMPILE_TEST!=n.

Remember,

	bool "string" if <condition-affects-visibility-of-string>
	depends on <condition-affects-config-symbol-availability>

The former merely hides the option from the user _if_ the condition fails.
The latter _disables_ the option completely (except if you try and select
it, at which point you end up with a Kconfig warning about that.)
Daniel Lezcano May 19, 2015, 3:03 p.m. UTC | #19
On 05/19/2015 04:41 PM, Maxime Coquelin wrote:
> 2015-05-19 15:49 GMT+02:00 Daniel Lezcano <daniel.lezcano@linaro.org>:
>>
>>
>> It is not exactly as the initial patch.
>>
>> I think Arnd is proposing:
>>
>> config CLKSRC_STM32
>>          bool "Clocksource for STM32 SoCs" if COMPILE_TEST
>>          select CLKSRC_MMIO
>>
>
> Isn't "depends on OF" missing for COMPILE_TEST case?

Hmm, yes. Probably.

> Other than that, I'm fine with the proposal.
> Daniel, is it the case for you too?

Yep.
Maxime Coquelin May 19, 2015, 3:34 p.m. UTC | #20
2015-05-19 16:50 GMT+02:00 Russell King - ARM Linux <linux@arm.linux.org.uk>:
>
> config CLKSRC_STM32
>         bool "Clocksource for STM32 SoCs" if COMPILE_TEST
>         depends on OF
>         select CLKSRC_MMIO
>
> This permits CLKSRC_STM32 to be selected by STM32 (provided OF is enabled,
> it's always going to be for that case, right?) while allowing the option
> to be visible when both OF!=n and COMPILE_TEST!=n.

Yes OF is always enabled when STM32.

So it fits our needs, as we only want the option to be visible when
both OF!=n and COMPILE_TEST!=n.

>
> Remember,
>
>         bool "string" if <condition-affects-visibility-of-string>
>         depends on <condition-affects-config-symbol-availability>
>
> The former merely hides the option from the user _if_ the condition fails.
> The latter _disables_ the option completely (except if you try and select
> it, at which point you end up with a Kconfig warning about that.)

Thanks for the reminder!
Maxime
diff mbox

Patch

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index bf9364c..2443520 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -106,6 +106,14 @@  config CLKSRC_EFM32
 	  Support to use the timers of EFM32 SoCs as clock source and clock
 	  event device.
 
+config CLKSRC_STM32
+	bool "Clocksource for STM32 SoCs" if !ARCH_STM32
+	depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST)
+	select CLKSRC_MMIO
+	default ARCH_STM32
+	help
+	  Support to use the timers of STM32 SoCs as clock event device.
+
 config ARM_ARCH_TIMER
 	bool
 	select CLKSRC_OF if OF
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index d510c54..888a7df 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -36,6 +36,7 @@  obj-$(CONFIG_ARCH_NSPIRE)	+= zevio-timer.o
 obj-$(CONFIG_ARCH_BCM_MOBILE)	+= bcm_kona_timer.o
 obj-$(CONFIG_CADENCE_TTC_TIMER)	+= cadence_ttc_timer.o
 obj-$(CONFIG_CLKSRC_EFM32)	+= time-efm32.o
+obj-$(CONFIG_CLKSRC_STM32)	+= timer-stm32.o
 obj-$(CONFIG_CLKSRC_EXYNOS_MCT)	+= exynos_mct.o
 obj-$(CONFIG_CLKSRC_SAMSUNG_PWM)	+= samsung_pwm_timer.o
 obj-$(CONFIG_FSL_FTM_TIMER)	+= fsl_ftm_timer.o
diff --git a/drivers/clocksource/timer-stm32.c b/drivers/clocksource/timer-stm32.c
new file mode 100644
index 0000000..fad2e2e
--- /dev/null
+++ b/drivers/clocksource/timer-stm32.c
@@ -0,0 +1,184 @@ 
+/*
+ * Copyright (C) Maxime Coquelin 2015
+ * Author:  Maxime Coquelin <mcoquelin.stm32@gmail.com>
+ * License terms:  GNU General Public License (GPL), version 2
+ *
+ * Inspired by time-efm32.c from Uwe Kleine-Koenig
+ */
+
+#include <linux/kernel.h>
+#include <linux/clocksource.h>
+#include <linux/clockchips.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/clk.h>
+#include <linux/reset.h>
+
+#define TIM_CR1		0x00
+#define TIM_DIER	0x0c
+#define TIM_SR		0x10
+#define TIM_EGR		0x14
+#define TIM_PSC		0x28
+#define TIM_ARR		0x2c
+
+#define TIM_CR1_CEN	BIT(0)
+#define TIM_CR1_OPM	BIT(3)
+#define TIM_CR1_ARPE	BIT(7)
+
+#define TIM_DIER_UIE	BIT(0)
+
+#define TIM_SR_UIF	BIT(0)
+
+#define TIM_EGR_UG	BIT(0)
+
+struct stm32_clock_event_ddata {
+	struct clock_event_device evtdev;
+	unsigned periodic_top;
+	void __iomem *base;
+};
+
+static void stm32_clock_event_set_mode(enum clock_event_mode mode,
+				       struct clock_event_device *evtdev)
+{
+	struct stm32_clock_event_ddata *data =
+		container_of(evtdev, struct stm32_clock_event_ddata, evtdev);
+	void *base = data->base;
+
+	switch (mode) {
+	case CLOCK_EVT_MODE_PERIODIC:
+		writel_relaxed(data->periodic_top, base + TIM_ARR);
+		writel_relaxed(TIM_CR1_ARPE | TIM_CR1_CEN, base + TIM_CR1);
+		break;
+
+	case CLOCK_EVT_MODE_ONESHOT:
+	default:
+		writel_relaxed(0, base + TIM_CR1);
+		break;
+	}
+}
+
+static int stm32_clock_event_set_next_event(unsigned long evt,
+					    struct clock_event_device *evtdev)
+{
+	struct stm32_clock_event_ddata *data =
+		container_of(evtdev, struct stm32_clock_event_ddata, evtdev);
+
+	writel_relaxed(evt, data->base + TIM_ARR);
+	writel_relaxed(TIM_CR1_ARPE | TIM_CR1_OPM | TIM_CR1_CEN,
+		       data->base + TIM_CR1);
+
+	return 0;
+}
+
+static irqreturn_t stm32_clock_event_handler(int irq, void *dev_id)
+{
+	struct stm32_clock_event_ddata *data = dev_id;
+
+	writel_relaxed(0, data->base + TIM_SR);
+
+	data->evtdev.event_handler(&data->evtdev);
+
+	return IRQ_HANDLED;
+}
+
+static struct stm32_clock_event_ddata clock_event_ddata = {
+	.evtdev = {
+		.name = "stm32 clockevent",
+		.features = CLOCK_EVT_FEAT_ONESHOT | CLOCK_EVT_FEAT_PERIODIC,
+		.set_mode = stm32_clock_event_set_mode,
+		.set_next_event = stm32_clock_event_set_next_event,
+		.rating = 200,
+	},
+};
+
+static void __init stm32_clockevent_init(struct device_node *np)
+{
+	struct stm32_clock_event_ddata *data = &clock_event_ddata;
+	struct clk *clk;
+	struct reset_control *rstc;
+	unsigned long rate, max_delta;
+	int irq, ret, bits, prescaler = 1;
+
+	clk = of_clk_get(np, 0);
+	if (IS_ERR(clk)) {
+		ret = PTR_ERR(clk);
+		pr_err("failed to get clock for clockevent (%d)\n", ret);
+		goto err_clk_get;
+	}
+
+	ret = clk_prepare_enable(clk);
+	if (ret) {
+		pr_err("failed to enable timer clock for clockevent (%d)\n",
+		       ret);
+		goto err_clk_enable;
+	}
+
+	rate = clk_get_rate(clk);
+
+	rstc = of_reset_control_get(np, NULL);
+	if (!IS_ERR(rstc)) {
+		reset_control_assert(rstc);
+		reset_control_deassert(rstc);
+	}
+
+	data->base = of_iomap(np, 0);
+	if (!data->base) {
+		pr_err("failed to map registers for clockevent\n");
+		goto err_iomap;
+	}
+
+	irq = irq_of_parse_and_map(np, 0);
+	if (!irq) {
+		pr_err("%s: failed to get irq.\n", np->full_name);
+		goto err_get_irq;
+	}
+
+	/* Detect whether the timer is 16 or 32 bits */
+	writel_relaxed(~0UL, data->base + TIM_ARR);
+	max_delta = readl_relaxed(data->base + TIM_ARR);
+	if (max_delta == ~0UL) {
+		prescaler = 1;
+		bits = 32;
+	} else {
+		prescaler = 1024;
+		bits = 16;
+	}
+	writel_relaxed(0, data->base + TIM_ARR);
+
+	writel_relaxed(prescaler - 1, data->base + TIM_PSC);
+	writel_relaxed(TIM_EGR_UG, data->base + TIM_EGR);
+	writel_relaxed(TIM_DIER_UIE, data->base + TIM_DIER);
+	writel_relaxed(0, data->base + TIM_SR);
+
+	data->periodic_top = DIV_ROUND_CLOSEST(rate, prescaler * HZ);
+
+	clockevents_config_and_register(&data->evtdev,
+					DIV_ROUND_CLOSEST(rate, prescaler),
+					0x1, max_delta);
+
+	ret = request_irq(irq, stm32_clock_event_handler, IRQF_TIMER,
+			"stm32 clockevent", data);
+	if (ret) {
+		pr_err("%s: failed to request irq.\n", np->full_name);
+		goto err_get_irq;
+	}
+
+	pr_info("%s: STM32 clockevent driver initialized (%d bits)\n",
+			np->full_name, bits);
+
+	return;
+
+err_get_irq:
+	iounmap(data->base);
+err_iomap:
+	clk_disable_unprepare(clk);
+err_clk_enable:
+	clk_put(clk);
+err_clk_get:
+	return;
+}
+
+CLOCKSOURCE_OF_DECLARE(stm32, "st,stm32-timer", stm32_clockevent_init);