2015-07-27 14:01:25

by Govindraj Raja

[permalink] [raw]
Subject: [PATCH v4 0/7] Clocksource changes for Pistachio CPUFreq.

From: Ezequiel Garcia <[email protected]>

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


2015-07-27 14:02:17

by Govindraj Raja

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

From: Ezequiel Garcia <[email protected]>

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 <[email protected]>
Signed-off-by: Ezequiel Garcia <[email protected]>
---
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

2015-07-27 14:01:29

by Govindraj Raja

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

From: Ezequiel Garcia <[email protected]>

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 <[email protected]>
Signed-off-by: Ezequiel Garcia <[email protected]>
---
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

2015-07-27 14:01:31

by Govindraj Raja

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

From: Ezequiel Garcia <[email protected]>

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 <[email protected]>
Signed-off-by: Ezequiel Garcia <[email protected]>
---
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

2015-07-27 14:01:33

by Govindraj Raja

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

From: Ezequiel Garcia <[email protected]>

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 <[email protected]>
---
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

2015-07-28 09:51:39

by Ralf Baechle

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

Daniel,

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

> From: Ezequiel Garcia <[email protected]>
>
> 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

2015-08-04 09:24:06

by Daniel Lezcano

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

On 07/27/2015 04:00 PM, Govindraj Raja wrote:
> From: Ezequiel Garcia <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Ezequiel Garcia <[email protected]>

Acked-by: Daniel Lezcano <[email protected]>

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

2015-08-04 09:48:41

by Daniel Lezcano

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

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 <[email protected]>
>>
>> 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 <[email protected]>



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