[PATCH v4 0/7] Clocksource changes for Pistachio CPUFreq.

[PATCH v4 0/7] Clocksource changes for Pistachio CPUFreq.

[Govindraj Raja]

From: Ezequiel Garcia <ezequiel.garcia@imgtec.com>

The purpose of this patchset is to support CPUFreq on Pistachio SoC.
However, given Pistachio uses the MIPS GIC clocksource and clockevent drivers
(clocked from the CPU), adding CPUFreq support needs some work.

This patchset changes the MIPS GIC clockevent driver to update the frequency of
the per-cpu clockevents using a clock notifier.

Then, we add a clocksource driver for IMG Pistachio SoC, based on the 
general purpose timers. The SoC only provides four timers, so we can't
use them to implement the four clockevents and the clocksource.

However, we can use one of these timers to provide a clocksource and a
sched clock. Given the general purpose timers are clocked from the peripheral
system clock tree, they are not affected by CPU rate changes.

Patches 1 to 3 are just style cleaning and preparation work.
Patch 4 adds the clockevent frequency update.
Patches 5 and 6 add the new clocksource driver.
Patch 7 introduces an option to enable the timer based clocksource on Pistachio.

For CPUFreq to really work, clk driver changes are needed to support MIPS PLL
clock rate change. Patches for this will be posted soon.

This series apply on v4.2-rc3. As always, comments and feedback are welcome!

Tested on Pistachio-Bring-up-Board.
Patch series based on 4.2-rc3.

Changes from v3:
---------------
No Changes from v2 re-posting the series again.

Changes from v2:
---------------
   * Fix spacing for consistency as pointed out by Sergei.

Changes since v1
----------------

Addressed review comments by Andrew:
   * Fix typo
   * Fix style issues
   * Use readl/writel accessors instead of raw variants
   * Drop spurious comment and of_device_id table
   * Add a pistachio_ prefix to clocksource functions


Ezequiel Garcia (7):
  clocksource: mips-gic: Enable the clock before using it
  clocksource: mips-gic: Add missing error returns checks
  clocksource: mips-gic: Split clocksource and clockevent initialization
  clocksource: mips-gic: Update clockevent frequency on clock rate
    changes
  clocksource: Add Pistachio SoC general purpose timer binding document
  clocksource: Add Pistachio clocksource-only driver
  mips: pistachio: Allow to enable the external timer based clocksource

 .../bindings/timer/img,pistachio-gptimer.txt       |  28 +++
 arch/mips/Kconfig                                  |   1 +
 arch/mips/pistachio/Kconfig                        |  13 ++
 drivers/clocksource/Kconfig                        |   4 +
 drivers/clocksource/Makefile                       |   1 +
 drivers/clocksource/mips-gic-timer.c               |  65 ++++++-
 drivers/clocksource/time-pistachio.c               | 194 +++++++++++++++++++++
 7 files changed, 297 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/timer/img,pistachio-gptimer.txt
 create mode 100644 arch/mips/pistachio/Kconfig
 create mode 100644 drivers/clocksource/time-pistachio.c

-- 
1.9.1

[PATCH v4 1/7] clocksource: mips-gic: Enable the clock before using it

[Govindraj Raja]

From: Ezequiel Garcia <ezequiel.garcia@imgtec.com>

For the clock to be used (e.g. get its rate through clk_get_rate)
it should be prepared and enabled first.

Also, while the clock is enabled the driver must hold a reference to it,
so let's remove the call to clk_put.

Reviewed-by: Andrew Bresticker <abrestic@chromium.org>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/clocksource/mips-gic-timer.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c
index b81ed1a..913585d 100644
--- a/drivers/clocksource/mips-gic-timer.c
+++ b/drivers/clocksource/mips-gic-timer.c
@@ -158,8 +158,13 @@ static void __init gic_clocksource_of_init(struct device_node *node)
 
 	clk = of_clk_get(node, 0);
 	if (!IS_ERR(clk)) {
+		if (clk_prepare_enable(clk) < 0) {
+			pr_err("GIC failed to enable clock\n");
+			clk_put(clk);
+			return;
+		}
+
 		gic_frequency = clk_get_rate(clk);
-		clk_put(clk);
 	} else if (of_property_read_u32(node, "clock-frequency",
 					&gic_frequency)) {
 		pr_err("GIC frequency not specified.\n");
-- 
1.9.1

Re: [PATCH v4 1/7] clocksource: mips-gic: Enable the clock before using it

[Daniel Lezcano]

On 07/27/2015 04:00 PM, Govindraj Raja wrote:
> From: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
>
> For the clock to be used (e.g. get its rate through clk_get_rate)
> it should be prepared and enabled first.
>
> Also, while the clock is enabled the driver must hold a reference to it,
> so let's remove the call to clk_put.
>
> Reviewed-by: Andrew Bresticker <abrestic@chromium.org>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>

Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>

> ---
>   drivers/clocksource/mips-gic-timer.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c
> index b81ed1a..913585d 100644
> --- a/drivers/clocksource/mips-gic-timer.c
> +++ b/drivers/clocksource/mips-gic-timer.c
> @@ -158,8 +158,13 @@ static void __init gic_clocksource_of_init(struct device_node *node)
>
>   	clk = of_clk_get(node, 0);
>   	if (!IS_ERR(clk)) {
> +		if (clk_prepare_enable(clk) < 0) {
> +			pr_err("GIC failed to enable clock\n");
> +			clk_put(clk);
> +			return;
> +		}
> +
>   		gic_frequency = clk_get_rate(clk);
> -		clk_put(clk);
>   	} else if (of_property_read_u32(node, "clock-frequency",
>   					&gic_frequency)) {
>   		pr_err("GIC frequency not specified.\n");
>


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

[PATCH v4 2/7] clocksource: mips-gic: Add missing error returns checks

[Govindraj Raja]

From: Ezequiel Garcia <ezequiel.garcia-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>

This commit adds the required checks on the functions that return
an error. Some of them are not critical, so only a warning is
printed.

Reviewed-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia-1AXoQHu6uovQT0dZR+AlfA@public.gmane.org>
---
 drivers/clocksource/mips-gic-timer.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c
index 913585d..c4352f0 100644
--- a/drivers/clocksource/mips-gic-timer.c
+++ b/drivers/clocksource/mips-gic-timer.c
@@ -100,12 +100,18 @@ static struct notifier_block gic_cpu_nb = {
 
 static int gic_clockevent_init(void)
 {
+	int ret;
+
 	if (!cpu_has_counter || !gic_frequency)
 		return -ENXIO;
 
-	setup_percpu_irq(gic_timer_irq, &gic_compare_irqaction);
+	ret = setup_percpu_irq(gic_timer_irq, &gic_compare_irqaction);
+	if (ret < 0)
+		return ret;
 
-	register_cpu_notifier(&gic_cpu_nb);
+	ret = register_cpu_notifier(&gic_cpu_nb);
+	if (ret < 0)
+		pr_warn("GIC: Unable to register CPU notifier\n");
 
 	gic_clockevent_cpu_init(this_cpu_ptr(&gic_clockevent_device));
 
@@ -125,13 +131,17 @@ static struct clocksource gic_clocksource = {
 
 static void __init __gic_clocksource_init(void)
 {
+	int ret;
+
 	/* Set clocksource mask. */
 	gic_clocksource.mask = CLOCKSOURCE_MASK(gic_get_count_width());
 
 	/* Calculate a somewhat reasonable rating value. */
 	gic_clocksource.rating = 200 + gic_frequency / 10000000;
 
-	clocksource_register_hz(&gic_clocksource, gic_frequency);
+	ret = clocksource_register_hz(&gic_clocksource, gic_frequency);
+	if (ret < 0)
+		pr_warn("GIC: Unable to register clocksource\n");
 
 	gic_clockevent_init();
 
-- 
1.9.1

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

[PATCH v4 3/7] clocksource: mips-gic: Split clocksource and clockevent initialization

[Govindraj Raja]

From: Ezequiel Garcia <ezequiel.garcia@imgtec.com>

This is preparation work for the introduction of clockevent frequency
update with a clock notifier. This is only possible when the device
is passed a clk struct, so let's split the legacy and devicetree
initialization.

Reviewed-by: Andrew Bresticker <abrestic@chromium.org>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/clocksource/mips-gic-timer.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c
index c4352f0..22a4daf 100644
--- a/drivers/clocksource/mips-gic-timer.c
+++ b/drivers/clocksource/mips-gic-timer.c
@@ -142,11 +142,6 @@ static void __init __gic_clocksource_init(void)
 	ret = clocksource_register_hz(&gic_clocksource, gic_frequency);
 	if (ret < 0)
 		pr_warn("GIC: Unable to register clocksource\n");
-
-	gic_clockevent_init();
-
-	/* And finally start the counter */
-	gic_start_count();
 }
 
 void __init gic_clocksource_init(unsigned int frequency)
@@ -156,6 +151,10 @@ void __init gic_clocksource_init(unsigned int frequency)
 		GIC_LOCAL_TO_HWIRQ(GIC_LOCAL_INT_COMPARE);
 
 	__gic_clocksource_init();
+	gic_clockevent_init();
+
+	/* And finally start the counter */
+	gic_start_count();
 }
 
 static void __init gic_clocksource_of_init(struct device_node *node)
@@ -187,6 +186,10 @@ static void __init gic_clocksource_of_init(struct device_node *node)
 	}
 
 	__gic_clocksource_init();
+	gic_clockevent_init();
+
+	/* And finally start the counter */
+	gic_start_count();
 }
 CLOCKSOURCE_OF_DECLARE(mips_gic_timer, "mti,gic-timer",
 		       gic_clocksource_of_init);
-- 
1.9.1

[PATCH v4 4/7] clocksource: mips-gic: Update clockevent frequency on clock rate changes

[Govindraj Raja]

From: Ezequiel Garcia <ezequiel.garcia@imgtec.com>

This commit introduces the clockevent frequency update, using
a clock notifier. It will be used to support CPUFreq on platforms
using MIPS GIC based clockevents.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/clocksource/mips-gic-timer.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c
index 22a4daf..a155bec 100644
--- a/drivers/clocksource/mips-gic-timer.c
+++ b/drivers/clocksource/mips-gic-timer.c
@@ -79,6 +79,13 @@ static void gic_clockevent_cpu_exit(struct clock_event_device *cd)
 	disable_percpu_irq(gic_timer_irq);
 }
 
+static void gic_update_frequency(void *data)
+{
+	unsigned long rate = (unsigned long)data;
+
+	clockevents_update_freq(this_cpu_ptr(&gic_clockevent_device), rate);
+}
+
 static int gic_cpu_notifier(struct notifier_block *nb, unsigned long action,
 				void *data)
 {
@@ -94,10 +101,26 @@ static int gic_cpu_notifier(struct notifier_block *nb, unsigned long action,
 	return NOTIFY_OK;
 }
 
+static int gic_clk_notifier(struct notifier_block *nb, unsigned long action,
+			    void *data)
+{
+	struct clk_notifier_data *cnd = data;
+
+	if (action == POST_RATE_CHANGE)
+		on_each_cpu(gic_update_frequency, (void *)cnd->new_rate, 1);
+
+	return NOTIFY_OK;
+}
+
+
 static struct notifier_block gic_cpu_nb = {
 	.notifier_call = gic_cpu_notifier,
 };
 
+static struct notifier_block gic_clk_nb = {
+	.notifier_call = gic_clk_notifier,
+};
+
 static int gic_clockevent_init(void)
 {
 	int ret;
@@ -160,6 +183,7 @@ void __init gic_clocksource_init(unsigned int frequency)
 static void __init gic_clocksource_of_init(struct device_node *node)
 {
 	struct clk *clk;
+	int ret;
 
 	if (WARN_ON(!gic_present || !node->parent ||
 		    !of_device_is_compatible(node->parent, "mti,gic")))
@@ -186,7 +210,12 @@ static void __init gic_clocksource_of_init(struct device_node *node)
 	}
 
 	__gic_clocksource_init();
-	gic_clockevent_init();
+
+	ret = gic_clockevent_init();
+	if (!ret && !IS_ERR(clk)) {
+		if (clk_notifier_register(clk, &gic_clk_nb) < 0)
+			pr_warn("GIC: Unable to register clock notifier\n");
+	}
 
 	/* And finally start the counter */
 	gic_start_count();
-- 
1.9.1

Re: [PATCH v4 0/7] Clocksource changes for Pistachio CPUFreq.

[Ralf Baechle]

Daniel,

On Mon, Jul 27, 2015 at 03:00:11PM +0100, Govindraj Raja wrote:

> From: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
> 
> The purpose of this patchset is to support CPUFreq on Pistachio SoC.
> However, given Pistachio uses the MIPS GIC clocksource and clockevent drivers
> (clocked from the CPU), adding CPUFreq support needs some work.
> 
> This patchset changes the MIPS GIC clockevent driver to update the frequency of
> the per-cpu clockevents using a clock notifier.
> 
> Then, we add a clocksource driver for IMG Pistachio SoC, based on the 
> general purpose timers. The SoC only provides four timers, so we can't
> use them to implement the four clockevents and the clocksource.
> 
> However, we can use one of these timers to provide a clocksource and a
> sched clock. Given the general purpose timers are clocked from the peripheral
> system clock tree, they are not affected by CPU rate changes.
> 
> Patches 1 to 3 are just style cleaning and preparation work.
> Patch 4 adds the clockevent frequency update.
> Patches 5 and 6 add the new clocksource driver.
> Patch 7 introduces an option to enable the timer based clocksource on Pistachio.

if you're happy with this series feel free to add my ack to patch 7/7
which is the only one that touches arch/mips.

Alternatively I can carry this in the MIPS tree which would have tbe
benefit of better testing.

  Ralf

Re: [PATCH v4 0/7] Clocksource changes for Pistachio CPUFreq.

[Daniel Lezcano]

On 07/28/2015 11:51 AM, Ralf Baechle wrote:
> Daniel,
>
> On Mon, Jul 27, 2015 at 03:00:11PM +0100, Govindraj Raja wrote:
>
>> From: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
>>
>> The purpose of this patchset is to support CPUFreq on Pistachio SoC.
>> However, given Pistachio uses the MIPS GIC clocksource and clockevent drivers
>> (clocked from the CPU), adding CPUFreq support needs some work.
>>
>> This patchset changes the MIPS GIC clockevent driver to update the frequency of
>> the per-cpu clockevents using a clock notifier.
>>
>> Then, we add a clocksource driver for IMG Pistachio SoC, based on the
>> general purpose timers. The SoC only provides four timers, so we can't
>> use them to implement the four clockevents and the clocksource.
>>
>> However, we can use one of these timers to provide a clocksource and a
>> sched clock. Given the general purpose timers are clocked from the peripheral
>> system clock tree, they are not affected by CPU rate changes.
>>
>> Patches 1 to 3 are just style cleaning and preparation work.
>> Patch 4 adds the clockevent frequency update.
>> Patches 5 and 6 add the new clocksource driver.
>> Patch 7 introduces an option to enable the timer based clocksource on Pistachio.
>
> if you're happy with this series feel free to add my ack to patch 7/7
> which is the only one that touches arch/mips.
>
> Alternatively I can carry this in the MIPS tree which would have tbe
> benefit of better testing.

Ok, go ahead.

For the series 1-4: Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>



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