All of lore.kernel.org
 help / color / mirror / Atom feed
From: maxime.coquelin@st.com (Maxime Coquelin)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] clocksource: arm_global_timer: Detect if gt is usable with CPU_FREQ
Date: Tue, 14 Apr 2015 10:06:18 +0200	[thread overview]
Message-ID: <552CCA7A.3090306@st.com> (raw)
In-Reply-To: <552CC490.4010002@linaro.org>

Hi Srini, Ola,

On 04/14/2015 09:41 AM, Srinivas Kandagatla wrote:
> +Adding Pete and Maxime
>
> Hi Ola,
> Thankyou for sending the patch,
>
> I like the Idea, but I have some specific concerns which would break 
> existing SOCs.

I like the idea too.

>
>
> On 13/04/15 18:37, Ola Jeppsson wrote:
>> Some Cortex A9 CPU:s (e.g. zynq) have the tick tied to the CPU
>> frequency. On those CPU:s we cannot use the global-timer as a reliable
>> clocksource with CPU frequency scaling enabled since this is not
>> currently taken into account by the driver.
>>
>> Add a "tied-to-cpu-freq" boolean to the global-timer dt node indicate
>> this condition.
>>
>> When the global-timer register function sees this property return
>> immediately and don't register the clocksource.
>>
>> Signed-off-by: Ola Jeppsson <ola@adapteva.com>
>> ---
>>   Documentation/devicetree/bindings/arm/global_timer.txt | 4 ++++
>>   drivers/clocksource/arm_global_timer.c                 | 7 +++++++
>>   2 files changed, 11 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/global_timer.txt 
>> b/Documentation/devicetree/bindings/arm/global_timer.txt
>> index bdae3a818793..465e02c17b5b 100644
>> --- a/Documentation/devicetree/bindings/arm/global_timer.txt
>> +++ b/Documentation/devicetree/bindings/arm/global_timer.txt
>> @@ -17,6 +17,10 @@
>>
>>   - clocks : Should be phandle to a clock.
>>
>> +** Timer node optional properties:
>> +
>> +- tied-to-cpu-freq : indicates that the timer scales with the CPU 
>> frequency.
>> +
>>   Example:
>>
>>       timer at 2c000600 {
>> diff --git a/drivers/clocksource/arm_global_timer.c 
>> b/drivers/clocksource/arm_global_timer.c
>> index e6833771a716..8913ebda3f09 100644
>> --- a/drivers/clocksource/arm_global_timer.c
>> +++ b/drivers/clocksource/arm_global_timer.c
>> @@ -268,6 +268,13 @@ static void __init 
>> global_timer_of_register(struct device_node *np)
>>           return;
>>       }
>>
>> +#ifdef CONFIG_CPU_FREQ
>> +    if (of_property_read_bool(np, "tied-to-cpu-freq")) {
>> +        pr_warn("global-timer: tied to cpu frequency, not supported 
>> with scaling\n");
>> +        return;
>> +    }
>> +#endif
>> +
>
> This patch would not let the SOC like STiH415/416 or zynq with 
> "tied-to-cpu-freq" property to boot with multi_v7_defconfig. Which is 
> not correct thing to do, as STi SOC's do not use cpufreq driver 
> however the tick is tied to this clocksource.
Yes, you are right, but I don't see any cleaner way to do this.

On STi, we have another timer we can use as a clocksource when doing CPU 
Freq, the ST LPC timer.
It is not upstreamed yet, but we will try to have it for next release.

I propose we set the "tied-to-cpu-freq" in GT node of STi family as soon 
as we enable the LPC timer one.
Doing that, the STi boot won't break in multi_v7 config.

Kind regards,
Maxime

>
>
>
> --srini
>
>>       gt_clk = of_clk_get(np, 0);
>>       if (!IS_ERR(gt_clk)) {
>>           err = clk_prepare_enable(gt_clk);
>>

WARNING: multiple messages have this Message-ID (diff)
From: Maxime Coquelin <maxime.coquelin-qxv4g6HH51o@public.gmane.org>
To: Srinivas Kandagatla
	<srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Ola Jeppsson <ola-hhg9azYwhpdWk0Htik3J/w@public.gmane.org>,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	"Arnd Bergmann" <arnd-r2nGTMty4D4@public.gmane.org>,
	"Srinivas Kandagatla"
	<srinivas.kandagatla-qxv4g6HH51o@public.gmane.org>,
	"Michal Simek"
	<michal.simek-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>,
	"Stuart Menefy" <stuart.menefy-qxv4g6HH51o@public.gmane.org>,
	"Olof Johansson" <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>,
	"Sören Brinkmann"
	<soren.brinkmann-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>,
	"Peter Griffin"
	<peter.griffin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Subject: Re: [PATCH] clocksource: arm_global_timer: Detect if gt is usable with CPU_FREQ
Date: Tue, 14 Apr 2015 10:06:18 +0200	[thread overview]
Message-ID: <552CCA7A.3090306@st.com> (raw)
In-Reply-To: <552CC490.4010002-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Hi Srini, Ola,

On 04/14/2015 09:41 AM, Srinivas Kandagatla wrote:
> +Adding Pete and Maxime
>
> Hi Ola,
> Thankyou for sending the patch,
>
> I like the Idea, but I have some specific concerns which would break 
> existing SOCs.

I like the idea too.

>
>
> On 13/04/15 18:37, Ola Jeppsson wrote:
>> Some Cortex A9 CPU:s (e.g. zynq) have the tick tied to the CPU
>> frequency. On those CPU:s we cannot use the global-timer as a reliable
>> clocksource with CPU frequency scaling enabled since this is not
>> currently taken into account by the driver.
>>
>> Add a "tied-to-cpu-freq" boolean to the global-timer dt node indicate
>> this condition.
>>
>> When the global-timer register function sees this property return
>> immediately and don't register the clocksource.
>>
>> Signed-off-by: Ola Jeppsson <ola-hhg9azYwhpdWk0Htik3J/w@public.gmane.org>
>> ---
>>   Documentation/devicetree/bindings/arm/global_timer.txt | 4 ++++
>>   drivers/clocksource/arm_global_timer.c                 | 7 +++++++
>>   2 files changed, 11 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/global_timer.txt 
>> b/Documentation/devicetree/bindings/arm/global_timer.txt
>> index bdae3a818793..465e02c17b5b 100644
>> --- a/Documentation/devicetree/bindings/arm/global_timer.txt
>> +++ b/Documentation/devicetree/bindings/arm/global_timer.txt
>> @@ -17,6 +17,10 @@
>>
>>   - clocks : Should be phandle to a clock.
>>
>> +** Timer node optional properties:
>> +
>> +- tied-to-cpu-freq : indicates that the timer scales with the CPU 
>> frequency.
>> +
>>   Example:
>>
>>       timer@2c000600 {
>> diff --git a/drivers/clocksource/arm_global_timer.c 
>> b/drivers/clocksource/arm_global_timer.c
>> index e6833771a716..8913ebda3f09 100644
>> --- a/drivers/clocksource/arm_global_timer.c
>> +++ b/drivers/clocksource/arm_global_timer.c
>> @@ -268,6 +268,13 @@ static void __init 
>> global_timer_of_register(struct device_node *np)
>>           return;
>>       }
>>
>> +#ifdef CONFIG_CPU_FREQ
>> +    if (of_property_read_bool(np, "tied-to-cpu-freq")) {
>> +        pr_warn("global-timer: tied to cpu frequency, not supported 
>> with scaling\n");
>> +        return;
>> +    }
>> +#endif
>> +
>
> This patch would not let the SOC like STiH415/416 or zynq with 
> "tied-to-cpu-freq" property to boot with multi_v7_defconfig. Which is 
> not correct thing to do, as STi SOC's do not use cpufreq driver 
> however the tick is tied to this clocksource.
Yes, you are right, but I don't see any cleaner way to do this.

On STi, we have another timer we can use as a clocksource when doing CPU 
Freq, the ST LPC timer.
It is not upstreamed yet, but we will try to have it for next release.

I propose we set the "tied-to-cpu-freq" in GT node of STi family as soon 
as we enable the LPC timer one.
Doing that, the STi boot won't break in multi_v7 config.

Kind regards,
Maxime

>
>
>
> --srini
>
>>       gt_clk = of_clk_get(np, 0);
>>       if (!IS_ERR(gt_clk)) {
>>           err = clk_prepare_enable(gt_clk);
>>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2015-04-14  8:06 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-13 17:37 [PATCH] clocksource: arm_global_timer: Detect if gt is usable with CPU_FREQ Ola Jeppsson
2015-04-13 17:37 ` Ola Jeppsson
2015-04-13 17:55 ` Mark Rutland
2015-04-13 17:55   ` Mark Rutland
2015-04-14  7:41 ` Srinivas Kandagatla
2015-04-14  7:41   ` Srinivas Kandagatla
2015-04-14  8:06   ` Maxime Coquelin [this message]
2015-04-14  8:06     ` Maxime Coquelin
2015-04-17 12:49     ` Peter Griffin
2015-04-17 12:49       ` Peter Griffin
2015-04-14 13:50   ` Sören Brinkmann
2015-04-14 13:50     ` Sören Brinkmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=552CCA7A.3090306@st.com \
    --to=maxime.coquelin@st.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.