[PATCH 0/4] simple generic timer infrastructure and stmmac example

[PATCH 0/4] simple generic timer infrastructure and stmmac example

[Peppe CAVALLARO]

Many devices targeted at the embedded market provide a number of
generic timers which are capable of generating interrupts at a
requested rate. These can then be used in the implementation of drivers
for other peripherals which require a timer interrupt, without having
to provide an additional timer as part of that peripheral.

A code provides a simple abstraction layer which allows a timer to be
registered, and for a driver to request a timer.
Currently this doesn't provide any of the additional information, such
as precision or position in clock framework which might be required
for a fully featured driver.

This patches also updates the stmmac Network device driver to use
the generic timer infrastructure The timer is used for mitigating
the number of the interrupts. This helps many people to save the
CPU on STM platforms with MAC10/100 and GMAC device without an
embedded timer.

Welcome comments and review.

Giuseppe Cavallaro (3):
  sh_timer: add the support to use the generic timer
  stmmac: switch to use the new generic timer interface
  stmmac: rework and improvement the stmmac timer

Stuart Menefy (1):
  clksource: Generic timer infrastructure

 drivers/clocksource/Makefile       |    1 +
 drivers/clocksource/generictimer.c |   60 +++++++++++++++++++++
 drivers/clocksource/sh_tmu.c       |   72 +++++++++++++++++++++++++
 drivers/net/stmmac/Kconfig         |   19 ++++---
 drivers/net/stmmac/Makefile        |    2 +-
 drivers/net/stmmac/common.h        |    6 ++
 drivers/net/stmmac/stmmac.h        |    5 +-
 drivers/net/stmmac/stmmac_main.c   |   66 ++++++++++++-----------
 drivers/net/stmmac/stmmac_timer.c  |  103 +++++++++++++++++++++---------------
 drivers/net/stmmac/stmmac_timer.h  |   12 ++---
 include/linux/generictimer.h       |   41 ++++++++++++++
 11 files changed, 295 insertions(+), 92 deletions(-)
 create mode 100644 drivers/clocksource/generictimer.c
 create mode 100644 include/linux/generictimer.h

-- 
1.7.4

[PATCH 0/4] simple generic timer infrastructure and stmmac example

[Peppe CAVALLARO]

Many devices targeted at the embedded market provide a number of
generic timers which are capable of generating interrupts at a
requested rate. These can then be used in the implementation of drivers
for other peripherals which require a timer interrupt, without having
to provide an additional timer as part of that peripheral.

A code provides a simple abstraction layer which allows a timer to be
registered, and for a driver to request a timer.
Currently this doesn't provide any of the additional information, such
as precision or position in clock framework which might be required
for a fully featured driver.

This patches also updates the stmmac Network device driver to use
the generic timer infrastructure The timer is used for mitigating
the number of the interrupts. This helps many people to save the
CPU on STM platforms with MAC10/100 and GMAC device without an
embedded timer.

Welcome comments and review.

Giuseppe Cavallaro (3):
  sh_timer: add the support to use the generic timer
  stmmac: switch to use the new generic timer interface
  stmmac: rework and improvement the stmmac timer

Stuart Menefy (1):
  clksource: Generic timer infrastructure

 drivers/clocksource/Makefile       |    1 +
 drivers/clocksource/generictimer.c |   60 +++++++++++++++++++++
 drivers/clocksource/sh_tmu.c       |   72 +++++++++++++++++++++++++
 drivers/net/stmmac/Kconfig         |   19 ++++---
 drivers/net/stmmac/Makefile        |    2 +-
 drivers/net/stmmac/common.h        |    6 ++
 drivers/net/stmmac/stmmac.h        |    5 +-
 drivers/net/stmmac/stmmac_main.c   |   66 ++++++++++++-----------
 drivers/net/stmmac/stmmac_timer.c  |  103 +++++++++++++++++++++---------------
 drivers/net/stmmac/stmmac_timer.h  |   12 ++---
 include/linux/generictimer.h       |   41 ++++++++++++++
 11 files changed, 295 insertions(+), 92 deletions(-)
 create mode 100644 drivers/clocksource/generictimer.c
 create mode 100644 include/linux/generictimer.h

-- 
1.7.4

[PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Peppe CAVALLARO]

From: Stuart Menefy <stuart.menefy@st.com>

Many devices targeted at the embedded market provide a number of
generic timers which are capable of generating interrupts at a
requested rate. These can then be used in the implementation of drivers
for other peripherals which require a timer interrupt, without having
to provide an additional timer as part of that peripheral.

A code provides a simple abstraction layer which allows a timer to be
registered, and for a driver to request a timer.

Currently this doesn't provide any of the additional information, such
as precision or position in clock framework which might be required
for a fully featured driver.

Signed-off-by: Stuart Menefy <stuart.menefy@st.com>
Hacked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/clocksource/Makefile       |    1 +
 drivers/clocksource/generictimer.c |   60 ++++++++++++++++++++++++++++++++++++
 include/linux/generictimer.h       |   41 ++++++++++++++++++++++++
 3 files changed, 102 insertions(+), 0 deletions(-)
 create mode 100644 drivers/clocksource/generictimer.c
 create mode 100644 include/linux/generictimer.h

diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index be61ece..b0be293 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_CS5535_CLOCK_EVENT_SRC)	+= cs5535-clockevt.o
 obj-$(CONFIG_SH_TIMER_CMT)	+= sh_cmt.o
 obj-$(CONFIG_SH_TIMER_MTU2)	+= sh_mtu2.o
 obj-$(CONFIG_SH_TIMER_TMU)	+= sh_tmu.o
+obj-y				+= generictimer.o
diff --git a/drivers/clocksource/generictimer.c b/drivers/clocksource/generictimer.c
new file mode 100644
index 0000000..a74a87a
--- /dev/null
+++ b/drivers/clocksource/generictimer.c
@@ -0,0 +1,60 @@
+/*
+ * Simple generic hardware timer interface
+ *
+ * Copyright (C) 2010 STMicroelectronics Limited
+ * Authors: Giuseppe Cavallaro <peppe.cavallaro@st.com>
+ *          Stuart Menefy <stuart.menefy@st.com>
+ *
+ * May be copied or modified under the terms of the GNU General Public
+ * License. See linux/COPYING for more information.
+ */
+
+#include <linux/kernel.h>
+#include <linux/mutex.h>
+#include <linux/list.h>
+#include <linux/generictimer.h>
+
+static DEFINE_MUTEX(gt_mutex);
+static LIST_HEAD(gt_list);
+
+void generic_timer_register_device(struct generic_timer *gt)
+{
+	mutex_lock(&gt_mutex);
+	list_add(&gt->list, &gt_list);
+	mutex_unlock(&gt_mutex);
+}
+
+struct generic_timer *generic_timer_claim(void (*handler) (void *), void *data)
+{
+	struct generic_timer *gt = NULL;
+
+	if (!handler) {
+		pr_err("%s: invalid handler\n", __func__);
+		return NULL;
+	}
+
+	mutex_lock(&gt_mutex);
+	if (!list_empty(&gt_list)) {
+		struct list_head *list = gt_list.next;
+		list_del(list);
+		gt = container_of(list, struct generic_timer, list);
+	}
+	mutex_unlock(&gt_mutex);
+
+	if (!gt)
+		return NULL;
+
+	/* Prepare the new handler */
+	gt->priv_handler = handler;
+	gt->data = data;
+
+	return gt;
+}
+
+void generic_timer_release(struct generic_timer *gt)
+{
+	/* Just in case... */
+	generic_timer_stop(gt);
+
+	generic_timer_register_device(gt);
+}
diff --git a/include/linux/generictimer.h b/include/linux/generictimer.h
new file mode 100644
index 0000000..87fb656
--- /dev/null
+++ b/include/linux/generictimer.h
@@ -0,0 +1,41 @@
+#ifndef __STM_GENERIC_TIMER_H
+#define __STM_GENERIC_TIMER_H
+
+#include <linux/list.h>
+
+/* Generic timer device intrface */
+
+struct generic_timer {
+	char *name;
+	struct list_head list;
+	void (*priv_handler)(void *data);
+	void *data;
+	void (*timer_start)(struct generic_timer *gt);
+	void (*timer_stop)(struct generic_timer *gt);
+	void (*set_rate)(struct generic_timer *gt, unsigned long rate);
+};
+
+void generic_timer_register_device(struct generic_timer *gt);
+
+/* Driver interface */
+
+struct generic_timer *generic_timer_claim(void (*handler)(void *), void *data);
+void generic_timer_release(struct generic_timer *gt);
+
+static inline void generic_timer_start(struct generic_timer *gt)
+{
+	gt->timer_start(gt);
+}
+
+static inline void generic_timer_stop(struct generic_timer *gt)
+{
+	gt->timer_stop(gt);
+}
+
+static inline void generic_timer_set_rate(struct generic_timer *gt,
+		unsigned long rate)
+{
+	gt->set_rate(gt, rate);
+}
+
+#endif /* __STM_GENERIC_TIMER_H */
-- 
1.7.4

[PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Peppe CAVALLARO]

From: Stuart Menefy <stuart.menefy@st.com>

Many devices targeted at the embedded market provide a number of
generic timers which are capable of generating interrupts at a
requested rate. These can then be used in the implementation of drivers
for other peripherals which require a timer interrupt, without having
to provide an additional timer as part of that peripheral.

A code provides a simple abstraction layer which allows a timer to be
registered, and for a driver to request a timer.

Currently this doesn't provide any of the additional information, such
as precision or position in clock framework which might be required
for a fully featured driver.

Signed-off-by: Stuart Menefy <stuart.menefy@st.com>
Hacked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/clocksource/Makefile       |    1 +
 drivers/clocksource/generictimer.c |   60 ++++++++++++++++++++++++++++++++++++
 include/linux/generictimer.h       |   41 ++++++++++++++++++++++++
 3 files changed, 102 insertions(+), 0 deletions(-)
 create mode 100644 drivers/clocksource/generictimer.c
 create mode 100644 include/linux/generictimer.h

diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index be61ece..b0be293 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_CS5535_CLOCK_EVENT_SRC)	+= cs5535-clockevt.o
 obj-$(CONFIG_SH_TIMER_CMT)	+= sh_cmt.o
 obj-$(CONFIG_SH_TIMER_MTU2)	+= sh_mtu2.o
 obj-$(CONFIG_SH_TIMER_TMU)	+= sh_tmu.o
+obj-y				+= generictimer.o
diff --git a/drivers/clocksource/generictimer.c b/drivers/clocksource/generictimer.c
new file mode 100644
index 0000000..a74a87a
--- /dev/null
+++ b/drivers/clocksource/generictimer.c
@@ -0,0 +1,60 @@
+/*
+ * Simple generic hardware timer interface
+ *
+ * Copyright (C) 2010 STMicroelectronics Limited
+ * Authors: Giuseppe Cavallaro <peppe.cavallaro@st.com>
+ *          Stuart Menefy <stuart.menefy@st.com>
+ *
+ * May be copied or modified under the terms of the GNU General Public
+ * License. See linux/COPYING for more information.
+ */
+
+#include <linux/kernel.h>
+#include <linux/mutex.h>
+#include <linux/list.h>
+#include <linux/generictimer.h>
+
+static DEFINE_MUTEX(gt_mutex);
+static LIST_HEAD(gt_list);
+
+void generic_timer_register_device(struct generic_timer *gt)
+{
+	mutex_lock(&gt_mutex);
+	list_add(&gt->list, &gt_list);
+	mutex_unlock(&gt_mutex);
+}
+
+struct generic_timer *generic_timer_claim(void (*handler) (void *), void *data)
+{
+	struct generic_timer *gt = NULL;
+
+	if (!handler) {
+		pr_err("%s: invalid handler\n", __func__);
+		return NULL;
+	}
+
+	mutex_lock(&gt_mutex);
+	if (!list_empty(&gt_list)) {
+		struct list_head *list = gt_list.next;
+		list_del(list);
+		gt = container_of(list, struct generic_timer, list);
+	}
+	mutex_unlock(&gt_mutex);
+
+	if (!gt)
+		return NULL;
+
+	/* Prepare the new handler */
+	gt->priv_handler = handler;
+	gt->data = data;
+
+	return gt;
+}
+
+void generic_timer_release(struct generic_timer *gt)
+{
+	/* Just in case... */
+	generic_timer_stop(gt);
+
+	generic_timer_register_device(gt);
+}
diff --git a/include/linux/generictimer.h b/include/linux/generictimer.h
new file mode 100644
index 0000000..87fb656
--- /dev/null
+++ b/include/linux/generictimer.h
@@ -0,0 +1,41 @@
+#ifndef __STM_GENERIC_TIMER_H
+#define __STM_GENERIC_TIMER_H
+
+#include <linux/list.h>
+
+/* Generic timer device intrface */
+
+struct generic_timer {
+	char *name;
+	struct list_head list;
+	void (*priv_handler)(void *data);
+	void *data;
+	void (*timer_start)(struct generic_timer *gt);
+	void (*timer_stop)(struct generic_timer *gt);
+	void (*set_rate)(struct generic_timer *gt, unsigned long rate);
+};
+
+void generic_timer_register_device(struct generic_timer *gt);
+
+/* Driver interface */
+
+struct generic_timer *generic_timer_claim(void (*handler)(void *), void *data);
+void generic_timer_release(struct generic_timer *gt);
+
+static inline void generic_timer_start(struct generic_timer *gt)
+{
+	gt->timer_start(gt);
+}
+
+static inline void generic_timer_stop(struct generic_timer *gt)
+{
+	gt->timer_stop(gt);
+}
+
+static inline void generic_timer_set_rate(struct generic_timer *gt,
+		unsigned long rate)
+{
+	gt->set_rate(gt, rate);
+}
+
+#endif /* __STM_GENERIC_TIMER_H */
-- 
1.7.4

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Arnd Bergmann]

On Tuesday 22 February 2011, Peppe CAVALLARO wrote:
> From: Stuart Menefy <stuart.menefy@st.com>
> 
> Many devices targeted at the embedded market provide a number of
> generic timers which are capable of generating interrupts at a
> requested rate. These can then be used in the implementation of drivers
> for other peripherals which require a timer interrupt, without having
> to provide an additional timer as part of that peripheral.
> 
> A code provides a simple abstraction layer which allows a timer to be
> registered, and for a driver to request a timer.
> 
> Currently this doesn't provide any of the additional information, such
> as precision or position in clock framework which might be required
> for a fully featured driver.

This code should probably be discussed on a more broader
platform than the netdev and linux-sh mailing lists,
as the scope is neither sh nor network specific.

You should at least add linux-kernel@vger.kernel.org, possibly
also linux-arch@vger.kernel.org.

Further, John and Thomas are responsible for the timekeeping
infrastructure, and they are probably interested in this
as well.

Why is this code useful to you? In the scenarios I've seen, the
board can always assign a timer to a specific device in a fixed
way that can be describe in a board file or device tree.

Also, what is the difference between this and clkdev?

> Signed-off-by: Stuart Menefy <stuart.menefy@st.com>
> Hacked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> ---
>  drivers/clocksource/Makefile       |    1 +
>  drivers/clocksource/generictimer.c |   60 ++++++++++++++++++++++++++++++++++++
>  include/linux/generictimer.h       |   41 ++++++++++++++++++++++++
>  3 files changed, 102 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/clocksource/generictimer.c
>  create mode 100644 include/linux/generictimer.h

I don't think it fits that well into the drivers/clocksource directory,
because you don't actually register a struct clock_event_device or
struct clocksource.

I don't know if this could also be merged with the clocksource infrastructure,
but your code currently doesn't do that.

> +struct generic_timer *generic_timer_claim(void (*handler) (void *), void *data)
> +{
> +	struct generic_timer *gt = NULL;
> +
> +	if (!handler) {
> +		pr_err("%s: invalid handler\n", __func__);
> +		return NULL;
> +	}
> +
> +	mutex_lock(&gt_mutex);
> +	if (!list_empty(&gt_list)) {
> +		struct list_head *list = gt_list.next;
> +		list_del(list);
> +		gt = container_of(list, struct generic_timer, list);
> +	}
> +	mutex_unlock(&gt_mutex);
> +
> +	if (!gt)
> +		return NULL;
> +
> +	/* Prepare the new handler */
> +	gt->priv_handler = handler;
> +	gt->data = data;
> +
> +	return gt;
> +}

This does not seem very generic. You put timers into the list and take
them out again, but don't have any way to deal with timers that match
specific purposes. It obviously works for your specific use case where
you register exactly one timer, and use that in exactly one driver.

If more drivers were converted to generic_timer, which is obviously
the intention, then you might have a situation with very different
timers (fixed/variable tick, high/low frequencies, accurate/inaccurate),
or you might have fewer timers than users.

> +static inline void generic_timer_set_rate(struct generic_timer *gt,
> +		unsigned long rate)
> +{
> +	gt->set_rate(gt, rate);
> +}
> +
> +#endif /* __STM_GENERIC_TIMER_H */

Shouldn't this one allow a return code?

	Arnd

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Arnd Bergmann]

On Tuesday 22 February 2011, Peppe CAVALLARO wrote:
> From: Stuart Menefy <stuart.menefy@st.com>
> 
> Many devices targeted at the embedded market provide a number of
> generic timers which are capable of generating interrupts at a
> requested rate. These can then be used in the implementation of drivers
> for other peripherals which require a timer interrupt, without having
> to provide an additional timer as part of that peripheral.
> 
> A code provides a simple abstraction layer which allows a timer to be
> registered, and for a driver to request a timer.
> 
> Currently this doesn't provide any of the additional information, such
> as precision or position in clock framework which might be required
> for a fully featured driver.

This code should probably be discussed on a more broader
platform than the netdev and linux-sh mailing lists,
as the scope is neither sh nor network specific.

You should at least add linux-kernel@vger.kernel.org, possibly
also linux-arch@vger.kernel.org.

Further, John and Thomas are responsible for the timekeeping
infrastructure, and they are probably interested in this
as well.

Why is this code useful to you? In the scenarios I've seen, the
board can always assign a timer to a specific device in a fixed
way that can be describe in a board file or device tree.

Also, what is the difference between this and clkdev?

> Signed-off-by: Stuart Menefy <stuart.menefy@st.com>
> Hacked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> ---
>  drivers/clocksource/Makefile       |    1 +
>  drivers/clocksource/generictimer.c |   60 ++++++++++++++++++++++++++++++++++++
>  include/linux/generictimer.h       |   41 ++++++++++++++++++++++++
>  3 files changed, 102 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/clocksource/generictimer.c
>  create mode 100644 include/linux/generictimer.h

I don't think it fits that well into the drivers/clocksource directory,
because you don't actually register a struct clock_event_device or
struct clocksource.

I don't know if this could also be merged with the clocksource infrastructure,
but your code currently doesn't do that.

> +struct generic_timer *generic_timer_claim(void (*handler) (void *), void *data)
> +{
> +	struct generic_timer *gt = NULL;
> +
> +	if (!handler) {
> +		pr_err("%s: invalid handler\n", __func__);
> +		return NULL;
> +	}
> +
> +	mutex_lock(&gt_mutex);
> +	if (!list_empty(&gt_list)) {
> +		struct list_head *list = gt_list.next;
> +		list_del(list);
> +		gt = container_of(list, struct generic_timer, list);
> +	}
> +	mutex_unlock(&gt_mutex);
> +
> +	if (!gt)
> +		return NULL;
> +
> +	/* Prepare the new handler */
> +	gt->priv_handler = handler;
> +	gt->data = data;
> +
> +	return gt;
> +}

This does not seem very generic. You put timers into the list and take
them out again, but don't have any way to deal with timers that match
specific purposes. It obviously works for your specific use case where
you register exactly one timer, and use that in exactly one driver.

If more drivers were converted to generic_timer, which is obviously
the intention, then you might have a situation with very different
timers (fixed/variable tick, high/low frequencies, accurate/inaccurate),
or you might have fewer timers than users.

> +static inline void generic_timer_set_rate(struct generic_timer *gt,
> +		unsigned long rate)
> +{
> +	gt->set_rate(gt, rate);
> +}
> +
> +#endif /* __STM_GENERIC_TIMER_H */

Shouldn't this one allow a return code?

	Arnd

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Stuart Menefy]

Hi Arnd

Thanks for the comments.

On 24/02/11 17:20, Arnd Bergmann wrote:
> On Tuesday 22 February 2011, Peppe CAVALLARO wrote:
>> From: Stuart Menefy <stuart.menefy@st.com>
>>
>> Many devices targeted at the embedded market provide a number of
>> generic timers which are capable of generating interrupts at a
>> requested rate. These can then be used in the implementation of drivers
>> for other peripherals which require a timer interrupt, without having
>> to provide an additional timer as part of that peripheral.
>>
>> A code provides a simple abstraction layer which allows a timer to be
>> registered, and for a driver to request a timer.
>>
>> Currently this doesn't provide any of the additional information, such
>> as precision or position in clock framework which might be required
>> for a fully featured driver.
> 
> This code should probably be discussed on a more broader
> platform than the netdev and linux-sh mailing lists,
> as the scope is neither sh nor network specific.
> 
> You should at least add linux-kernel@vger.kernel.org, possibly
> also linux-arch@vger.kernel.org.
> 
> Further, John and Thomas are responsible for the timekeeping
> infrastructure, and they are probably interested in this
> as well.
> 
> Why is this code useful to you? In the scenarios I've seen, the
> board can always assign a timer to a specific device in a fixed
> way that can be describe in a board file or device tree.

What we were trying to do was separate the code which actually manipulates
the timer hardware from the code which wants that timer service. In this
case it was a network device driver which is used on multiple SoC devices,
while the timer hardware tends to differ from device to device.

The other user of this code which we have is an OProfile driver, which
with this change can now be independent of the hardware it is running on,
while the previous version manipulated the timer hardware directly.

> Also, what is the difference between this and clkdev?

clkdev can be used to find a struct clk, which is fine if you just want to
read the time. In this instance we want to get interrupts from the timer
hardware, which isn't supported by the clk infrastructure.

If anything this duplicates clockevents. The main reason for not using
clockevents was that nobody else does! Currently clockevents are
used strictly for time keeping within the kernel, and most architectures
only register those which are intended to be used for this purpose.
We felt a bit nervous about adding code to register all the device's timers
as clockevents, and having the network device driver pick up one of those
for its own use.

>> Signed-off-by: Stuart Menefy <stuart.menefy@st.com>
>> Hacked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>> ---
>>  drivers/clocksource/Makefile       |    1 +
>>  drivers/clocksource/generictimer.c |   60 ++++++++++++++++++++++++++++++++++++
>>  include/linux/generictimer.h       |   41 ++++++++++++++++++++++++
>>  3 files changed, 102 insertions(+), 0 deletions(-)
>>  create mode 100644 drivers/clocksource/generictimer.c
>>  create mode 100644 include/linux/generictimer.h
> 
> I don't think it fits that well into the drivers/clocksource directory,
> because you don't actually register a struct clock_event_device or
> struct clocksource.

True. The intent was that this would be a third interface onto timer
hardware, alongside clocksources and clockevents.

> I don't know if this could also be merged with the clocksource infrastructure,
> but your code currently doesn't do that.

It would probably be clockevent rather than clocksource, but it could be if
people felt that was a better way to go.

>> +struct generic_timer *generic_timer_claim(void (*handler) (void *), void *data)
>> +{
>> +	struct generic_timer *gt = NULL;
>> +
>> +	if (!handler) {
>> +		pr_err("%s: invalid handler\n", __func__);
>> +		return NULL;
>> +	}
>> +
>> +	mutex_lock(&gt_mutex);
>> +	if (!list_empty(&gt_list)) {
>> +		struct list_head *list = gt_list.next;
>> +		list_del(list);
>> +		gt = container_of(list, struct generic_timer, list);
>> +	}
>> +	mutex_unlock(&gt_mutex);
>> +
>> +	if (!gt)
>> +		return NULL;
>> +
>> +	/* Prepare the new handler */
>> +	gt->priv_handler = handler;
>> +	gt->data = data;
>> +
>> +	return gt;
>> +}
> 
> This does not seem very generic. You put timers into the list and take
> them out again, but don't have any way to deal with timers that match
> specific purposes. It obviously works for your specific use case where
> you register exactly one timer, and use that in exactly one driver.
> 
> If more drivers were converted to generic_timer, which is obviously
> the intention, then you might have a situation with very different
> timers (fixed/variable tick, high/low frequencies, accurate/inaccurate),
> or you might have fewer timers than users.

Agreed, this was a first 'keep it simple' pass, maybe its too simple.

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Stuart Menefy]

Hi Arnd

Thanks for the comments.

On 24/02/11 17:20, Arnd Bergmann wrote:
> On Tuesday 22 February 2011, Peppe CAVALLARO wrote:
>> From: Stuart Menefy <stuart.menefy@st.com>
>>
>> Many devices targeted at the embedded market provide a number of
>> generic timers which are capable of generating interrupts at a
>> requested rate. These can then be used in the implementation of drivers
>> for other peripherals which require a timer interrupt, without having
>> to provide an additional timer as part of that peripheral.
>>
>> A code provides a simple abstraction layer which allows a timer to be
>> registered, and for a driver to request a timer.
>>
>> Currently this doesn't provide any of the additional information, such
>> as precision or position in clock framework which might be required
>> for a fully featured driver.
> 
> This code should probably be discussed on a more broader
> platform than the netdev and linux-sh mailing lists,
> as the scope is neither sh nor network specific.
> 
> You should at least add linux-kernel@vger.kernel.org, possibly
> also linux-arch@vger.kernel.org.
> 
> Further, John and Thomas are responsible for the timekeeping
> infrastructure, and they are probably interested in this
> as well.
> 
> Why is this code useful to you? In the scenarios I've seen, the
> board can always assign a timer to a specific device in a fixed
> way that can be describe in a board file or device tree.

What we were trying to do was separate the code which actually manipulates
the timer hardware from the code which wants that timer service. In this
case it was a network device driver which is used on multiple SoC devices,
while the timer hardware tends to differ from device to device.

The other user of this code which we have is an OProfile driver, which
with this change can now be independent of the hardware it is running on,
while the previous version manipulated the timer hardware directly.

> Also, what is the difference between this and clkdev?

clkdev can be used to find a struct clk, which is fine if you just want to
read the time. In this instance we want to get interrupts from the timer
hardware, which isn't supported by the clk infrastructure.

If anything this duplicates clockevents. The main reason for not using
clockevents was that nobody else does! Currently clockevents are
used strictly for time keeping within the kernel, and most architectures
only register those which are intended to be used for this purpose.
We felt a bit nervous about adding code to register all the device's timers
as clockevents, and having the network device driver pick up one of those
for its own use.

>> Signed-off-by: Stuart Menefy <stuart.menefy@st.com>
>> Hacked-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
>> ---
>>  drivers/clocksource/Makefile       |    1 +
>>  drivers/clocksource/generictimer.c |   60 ++++++++++++++++++++++++++++++++++++
>>  include/linux/generictimer.h       |   41 ++++++++++++++++++++++++
>>  3 files changed, 102 insertions(+), 0 deletions(-)
>>  create mode 100644 drivers/clocksource/generictimer.c
>>  create mode 100644 include/linux/generictimer.h
> 
> I don't think it fits that well into the drivers/clocksource directory,
> because you don't actually register a struct clock_event_device or
> struct clocksource.

True. The intent was that this would be a third interface onto timer
hardware, alongside clocksources and clockevents.

> I don't know if this could also be merged with the clocksource infrastructure,
> but your code currently doesn't do that.

It would probably be clockevent rather than clocksource, but it could be if
people felt that was a better way to go.

>> +struct generic_timer *generic_timer_claim(void (*handler) (void *), void *data)
>> +{
>> +	struct generic_timer *gt = NULL;
>> +
>> +	if (!handler) {
>> +		pr_err("%s: invalid handler\n", __func__);
>> +		return NULL;
>> +	}
>> +
>> +	mutex_lock(&gt_mutex);
>> +	if (!list_empty(&gt_list)) {
>> +		struct list_head *list = gt_list.next;
>> +		list_del(list);
>> +		gt = container_of(list, struct generic_timer, list);
>> +	}
>> +	mutex_unlock(&gt_mutex);
>> +
>> +	if (!gt)
>> +		return NULL;
>> +
>> +	/* Prepare the new handler */
>> +	gt->priv_handler = handler;
>> +	gt->data = data;
>> +
>> +	return gt;
>> +}
> 
> This does not seem very generic. You put timers into the list and take
> them out again, but don't have any way to deal with timers that match
> specific purposes. It obviously works for your specific use case where
> you register exactly one timer, and use that in exactly one driver.
> 
> If more drivers were converted to generic_timer, which is obviously
> the intention, then you might have a situation with very different
> timers (fixed/variable tick, high/low frequencies, accurate/inaccurate),
> or you might have fewer timers than users.

Agreed, this was a first 'keep it simple' pass, maybe its too simple.

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Arnd Bergmann]

On Tuesday 01 March 2011, Stuart Menefy wrote:
> On 24/02/11 17:20, Arnd Bergmann wrote:
>
> > Why is this code useful to you? In the scenarios I've seen, the
> > board can always assign a timer to a specific device in a fixed
> > way that can be describe in a board file or device tree.
> 
> What we were trying to do was separate the code which actually manipulates
> the timer hardware from the code which wants that timer service. In this
> case it was a network device driver which is used on multiple SoC devices,
> while the timer hardware tends to differ from device to device.

Right. It certainly makes sense to have an well-defined interface between
the user and the provider of a timer interrupt.

> The other user of this code which we have is an OProfile driver, which
> with this change can now be independent of the hardware it is running on,
> while the previous version manipulated the timer hardware directly.

Ok.

> > Also, what is the difference between this and clkdev?
> 
> clkdev can be used to find a struct clk, which is fine if you just want to
> read the time. In this instance we want to get interrupts from the timer
> hardware, which isn't supported by the clk infrastructure.

(adding Russell to Cc)

Is this something that could sensibly be added to clk/clkdev?

> If anything this duplicates clockevents. The main reason for not using
> clockevents was that nobody else does! Currently clockevents are
> used strictly for time keeping within the kernel, and most architectures
> only register those which are intended to be used for this purpose.
> We felt a bit nervous about adding code to register all the device's timers
> as clockevents, and having the network device driver pick up one of those
> for its own use.

I see. Using a clock_event_device for anything but the system timer tick
is currently not supported, so it certainly would not be straightforward.

I think you need a bit of both, clkdev and clockevent. I think the
options you have are:

1. copy the clkdev code to make it possible to associate a device with
a periodic timer.
2. extend the clkdev/clk code to handle periodic interrupts and reuse
the infrastructure there.
3. extend the clockevent code to make it possible for regular device
drivers to use a clockevent source.

I have no idea which makes the most sense (or if there are other ideas).
Maybe Russell, Thomas or John can comment.

	Arnd

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Arnd Bergmann]

On Tuesday 01 March 2011, Stuart Menefy wrote:
> On 24/02/11 17:20, Arnd Bergmann wrote:
>
> > Why is this code useful to you? In the scenarios I've seen, the
> > board can always assign a timer to a specific device in a fixed
> > way that can be describe in a board file or device tree.
> 
> What we were trying to do was separate the code which actually manipulates
> the timer hardware from the code which wants that timer service. In this
> case it was a network device driver which is used on multiple SoC devices,
> while the timer hardware tends to differ from device to device.

Right. It certainly makes sense to have an well-defined interface between
the user and the provider of a timer interrupt.

> The other user of this code which we have is an OProfile driver, which
> with this change can now be independent of the hardware it is running on,
> while the previous version manipulated the timer hardware directly.

Ok.

> > Also, what is the difference between this and clkdev?
> 
> clkdev can be used to find a struct clk, which is fine if you just want to
> read the time. In this instance we want to get interrupts from the timer
> hardware, which isn't supported by the clk infrastructure.

(adding Russell to Cc)

Is this something that could sensibly be added to clk/clkdev?

> If anything this duplicates clockevents. The main reason for not using
> clockevents was that nobody else does! Currently clockevents are
> used strictly for time keeping within the kernel, and most architectures
> only register those which are intended to be used for this purpose.
> We felt a bit nervous about adding code to register all the device's timers
> as clockevents, and having the network device driver pick up one of those
> for its own use.

I see. Using a clock_event_device for anything but the system timer tick
is currently not supported, so it certainly would not be straightforward.

I think you need a bit of both, clkdev and clockevent. I think the
options you have are:

1. copy the clkdev code to make it possible to associate a device with
a periodic timer.
2. extend the clkdev/clk code to handle periodic interrupts and reuse
the infrastructure there.
3. extend the clockevent code to make it possible for regular device
drivers to use a clockevent source.

I have no idea which makes the most sense (or if there are other ideas).
Maybe Russell, Thomas or John can comment.

	Arnd

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Russell King - ARM Linux]

On Tue, Mar 01, 2011 at 05:43:19PM +0100, Arnd Bergmann wrote:
> On Tuesday 01 March 2011, Stuart Menefy wrote:
> > On 24/02/11 17:20, Arnd Bergmann wrote:
> > > Also, what is the difference between this and clkdev?
> > 
> > clkdev can be used to find a struct clk, which is fine if you just want to
> > read the time. In this instance we want to get interrupts from the timer
> > hardware, which isn't supported by the clk infrastructure.
> 
> (adding Russell to Cc)
> 
> Is this something that could sensibly be added to clk/clkdev?

I don't understand - why would anyone want to use clk/clkdev for timers.
clk/clkdev is all about those signals on the SoC which wiggle at regular
intervals between logic 0 and logic 1.  It's not about things which count,
which seems to be an entirely separate problem, and hence why there's
nothing to deal with interrupts or setting timeouts etc.

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Russell King - ARM Linux]

On Tue, Mar 01, 2011 at 05:43:19PM +0100, Arnd Bergmann wrote:
> On Tuesday 01 March 2011, Stuart Menefy wrote:
> > On 24/02/11 17:20, Arnd Bergmann wrote:
> > > Also, what is the difference between this and clkdev?
> > 
> > clkdev can be used to find a struct clk, which is fine if you just want to
> > read the time. In this instance we want to get interrupts from the timer
> > hardware, which isn't supported by the clk infrastructure.
> 
> (adding Russell to Cc)
> 
> Is this something that could sensibly be added to clk/clkdev?

I don't understand - why would anyone want to use clk/clkdev for timers.
clk/clkdev is all about those signals on the SoC which wiggle at regular
intervals between logic 0 and logic 1.  It's not about things which count,
which seems to be an entirely separate problem, and hence why there's
nothing to deal with interrupts or setting timeouts etc.

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Arnd Bergmann]

On Tuesday 01 March 2011 21:26:11 Russell King - ARM Linux wrote:
> On Tue, Mar 01, 2011 at 05:43:19PM +0100, Arnd Bergmann wrote:
> > On Tuesday 01 March 2011, Stuart Menefy wrote:
> > > On 24/02/11 17:20, Arnd Bergmann wrote:
> > > > Also, what is the difference between this and clkdev?
> > > 
> > > clkdev can be used to find a struct clk, which is fine if you just want to
> > > read the time. In this instance we want to get interrupts from the timer
> > > hardware, which isn't supported by the clk infrastructure.
> > 
> > (adding Russell to Cc)
> > 
> > Is this something that could sensibly be added to clk/clkdev?
> 
> I don't understand - why would anyone want to use clk/clkdev for timers.
> clk/clkdev is all about those signals on the SoC which wiggle at regular
> intervals between logic 0 and logic 1.  It's not about things which count,
> which seems to be an entirely separate problem, and hence why there's
> nothing to deal with interrupts or setting timeouts etc.

Ok, I see. I had mixed the two concepts. The clkdev infrastructure
seemed like a nice way to connect a source and a consumer of a timer,
but you're right that it's an entirely separate thing, and as Thomas
said, it's probably not needed either.

	Arnd

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Arnd Bergmann]

On Tuesday 01 March 2011 21:26:11 Russell King - ARM Linux wrote:
> On Tue, Mar 01, 2011 at 05:43:19PM +0100, Arnd Bergmann wrote:
> > On Tuesday 01 March 2011, Stuart Menefy wrote:
> > > On 24/02/11 17:20, Arnd Bergmann wrote:
> > > > Also, what is the difference between this and clkdev?
> > > 
> > > clkdev can be used to find a struct clk, which is fine if you just want to
> > > read the time. In this instance we want to get interrupts from the timer
> > > hardware, which isn't supported by the clk infrastructure.
> > 
> > (adding Russell to Cc)
> > 
> > Is this something that could sensibly be added to clk/clkdev?
> 
> I don't understand - why would anyone want to use clk/clkdev for timers.
> clk/clkdev is all about those signals on the SoC which wiggle at regular
> intervals between logic 0 and logic 1.  It's not about things which count,
> which seems to be an entirely separate problem, and hence why there's
> nothing to deal with interrupts or setting timeouts etc.

Ok, I see. I had mixed the two concepts. The clkdev infrastructure
seemed like a nice way to connect a source and a consumer of a timer,
but you're right that it's an entirely separate thing, and as Thomas
said, it's probably not needed either.

	Arnd

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Thomas Gleixner]

On Tue, 1 Mar 2011, Stuart Menefy wrote:
> On 24/02/11 17:20, Arnd Bergmann wrote:
> > On Tuesday 22 February 2011, Peppe CAVALLARO wrote:
> >> From: Stuart Menefy <stuart.menefy@st.com>
> >>
> >> Many devices targeted at the embedded market provide a number of
> >> generic timers which are capable of generating interrupts at a
> >> requested rate. These can then be used in the implementation of drivers
> >> for other peripherals which require a timer interrupt, without having
> >> to provide an additional timer as part of that peripheral.

Why can't you just use an hrtimer and be done with it? Just because
there is some extra hardware in the chip?

> If anything this duplicates clockevents. The main reason for not using
> clockevents was that nobody else does! Currently clockevents are
> used strictly for time keeping within the kernel, and most architectures
> only register those which are intended to be used for this purpose.
> We felt a bit nervous about adding code to register all the device's timers
> as clockevents, and having the network device driver pick up one of those
> for its own use.

We had this discussion before and there was never an real outcome as
it turned out that hrtimers provide enough abstraction for this kind
of problems.
 
> True. The intent was that this would be a third interface onto timer
> hardware, alongside clocksources and clockevents.
> 
> > I don't know if this could also be merged with the clocksource infrastructure,
> > but your code currently doesn't do that.
> 
> It would probably be clockevent rather than clocksource, but it could be if
> people felt that was a better way to go.

If we get some reasonable explanation why an extra timer interrupt is
solving a specific problem better than hrtimers we can do that, but
that needs more thought than picking the first available clockevent
from a list. If we come to the conclusion, that we want/need this kind
of functionality then I really prefer not to create yet another piece
of infrastructure which deals with timer devices.

Thanks,

	tglx

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Thomas Gleixner]

On Tue, 1 Mar 2011, Stuart Menefy wrote:
> On 24/02/11 17:20, Arnd Bergmann wrote:
> > On Tuesday 22 February 2011, Peppe CAVALLARO wrote:
> >> From: Stuart Menefy <stuart.menefy@st.com>
> >>
> >> Many devices targeted at the embedded market provide a number of
> >> generic timers which are capable of generating interrupts at a
> >> requested rate. These can then be used in the implementation of drivers
> >> for other peripherals which require a timer interrupt, without having
> >> to provide an additional timer as part of that peripheral.

Why can't you just use an hrtimer and be done with it? Just because
there is some extra hardware in the chip?

> If anything this duplicates clockevents. The main reason for not using
> clockevents was that nobody else does! Currently clockevents are
> used strictly for time keeping within the kernel, and most architectures
> only register those which are intended to be used for this purpose.
> We felt a bit nervous about adding code to register all the device's timers
> as clockevents, and having the network device driver pick up one of those
> for its own use.

We had this discussion before and there was never an real outcome as
it turned out that hrtimers provide enough abstraction for this kind
of problems.
 
> True. The intent was that this would be a third interface onto timer
> hardware, alongside clocksources and clockevents.
> 
> > I don't know if this could also be merged with the clocksource infrastructure,
> > but your code currently doesn't do that.
> 
> It would probably be clockevent rather than clocksource, but it could be if
> people felt that was a better way to go.

If we get some reasonable explanation why an extra timer interrupt is
solving a specific problem better than hrtimers we can do that, but
that needs more thought than picking the first available clockevent
from a list. If we come to the conclusion, that we want/need this kind
of functionality then I really prefer not to create yet another piece
of infrastructure which deals with timer devices.

Thanks,

	tglx

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Peppe CAVALLARO]

Hello and many thanks to All for your feedback.

On 3/1/2011 5:48 PM, Thomas Gleixner wrote:
>
> On Tue, 1 Mar 2011, Stuart Menefy wrote:
> > On 24/02/11 17:20, Arnd Bergmann wrote:
> > > On Tuesday 22 February 2011, Peppe CAVALLARO wrote:
> > >> From: Stuart Menefy <stuart.menefy@st.com>
> > >>
> > >> Many devices targeted at the embedded market provide a number of
> > >> generic timers which are capable of generating interrupts at a
> > >> requested rate. These can then be used in the implementation of
> drivers
> > >> for other peripherals which require a timer interrupt, without having
> > >> to provide an additional timer as part of that peripheral.
>
> Why can't you just use an hrtimer and be done with it? Just because
> there is some extra hardware in the chip?
>
> > If anything this duplicates clockevents. The main reason for not using
> > clockevents was that nobody else does! Currently clockevents are
> > used strictly for time keeping within the kernel, and most architectures
> > only register those which are intended to be used for this purpose.
> > We felt a bit nervous about adding code to register all the device's
> timers
> > as clockevents, and having the network device driver pick up one of
> those
> > for its own use.
>
> We had this discussion before and there was never an real outcome as
> it turned out that hrtimers provide enough abstraction for this kind
> of problems.
>
> > True. The intent was that this would be a third interface onto timer
> > hardware, alongside clocksources and clockevents.
> >
> > > I don't know if this could also be merged with the clocksource
> infrastructure,
> > > but your code currently doesn't do that.
> >
> > It would probably be clockevent rather than clocksource, but it
> could be if
> > people felt that was a better way to go.
>
> If we get some reasonable explanation why an extra timer interrupt is
> solving a specific problem better than hrtimers we can do that, but
> that needs more thought than picking the first available clockevent
> from a list. If we come to the conclusion, that we want/need this kind
> of functionality then I really prefer not to create yet another piece
> of infrastructure which deals with timer devices.
>

The initial/main usage of this code was to provide
an interrupt timer to handle the rx/tx processes in
the stmmac Ethernet d.d.

Some Ethernet devices have embedded timer
used for mitigating the number of their DMA interrupts.
So, to be frankly, my first idea was to use an
extra HW (SH4 TMU channel 2), unused on our platforms,
because our MAC hadn't something like that.

I'm not sure if other Ethernet drivers use hrtimer for
handling the whole rx/tx processes (on CC the net-2.6 ML:
please correct me if I'm wrong) and if this could have
impact on the performances or reliability of the network
activity (pkt loss%).

At any rate, I am happy to use the stmmac as experimental
driver to do this kind tests.
Indeed, in the past, on old Kernel (IIRC 2.6.23), I tried to use
the kernel timers but I removed the code from it because
I had noticed packets loss and a strange phenomenon with cyclesoak
(that showed broken sysload % during the heavy network activities).

Let me know how to proceed:

1) experiment with stmmac and hrtimer for handling rx/tx?
2) rework the patches for the Generic Timer Infra?

Best Regards,
Peppe

> Thanks,
>
>         tglx
>

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Peppe CAVALLARO]

Hello and many thanks to All for your feedback.

On 3/1/2011 5:48 PM, Thomas Gleixner wrote:
>
> On Tue, 1 Mar 2011, Stuart Menefy wrote:
> > On 24/02/11 17:20, Arnd Bergmann wrote:
> > > On Tuesday 22 February 2011, Peppe CAVALLARO wrote:
> > >> From: Stuart Menefy <stuart.menefy@st.com>
> > >>
> > >> Many devices targeted at the embedded market provide a number of
> > >> generic timers which are capable of generating interrupts at a
> > >> requested rate. These can then be used in the implementation of
> drivers
> > >> for other peripherals which require a timer interrupt, without having
> > >> to provide an additional timer as part of that peripheral.
>
> Why can't you just use an hrtimer and be done with it? Just because
> there is some extra hardware in the chip?
>
> > If anything this duplicates clockevents. The main reason for not using
> > clockevents was that nobody else does! Currently clockevents are
> > used strictly for time keeping within the kernel, and most architectures
> > only register those which are intended to be used for this purpose.
> > We felt a bit nervous about adding code to register all the device's
> timers
> > as clockevents, and having the network device driver pick up one of
> those
> > for its own use.
>
> We had this discussion before and there was never an real outcome as
> it turned out that hrtimers provide enough abstraction for this kind
> of problems.
>
> > True. The intent was that this would be a third interface onto timer
> > hardware, alongside clocksources and clockevents.
> >
> > > I don't know if this could also be merged with the clocksource
> infrastructure,
> > > but your code currently doesn't do that.
> >
> > It would probably be clockevent rather than clocksource, but it
> could be if
> > people felt that was a better way to go.
>
> If we get some reasonable explanation why an extra timer interrupt is
> solving a specific problem better than hrtimers we can do that, but
> that needs more thought than picking the first available clockevent
> from a list. If we come to the conclusion, that we want/need this kind
> of functionality then I really prefer not to create yet another piece
> of infrastructure which deals with timer devices.
>

The initial/main usage of this code was to provide
an interrupt timer to handle the rx/tx processes in
the stmmac Ethernet d.d.

Some Ethernet devices have embedded timer
used for mitigating the number of their DMA interrupts.
So, to be frankly, my first idea was to use an
extra HW (SH4 TMU channel 2), unused on our platforms,
because our MAC hadn't something like that.

I'm not sure if other Ethernet drivers use hrtimer for
handling the whole rx/tx processes (on CC the net-2.6 ML:
please correct me if I'm wrong) and if this could have
impact on the performances or reliability of the network
activity (pkt loss%).

At any rate, I am happy to use the stmmac as experimental
driver to do this kind tests.
Indeed, in the past, on old Kernel (IIRC 2.6.23), I tried to use
the kernel timers but I removed the code from it because
I had noticed packets loss and a strange phenomenon with cyclesoak
(that showed broken sysload % during the heavy network activities).

Let me know how to proceed:

1) experiment with stmmac and hrtimer for handling rx/tx?
2) rework the patches for the Generic Timer Infra?

Best Regards,
Peppe

> Thanks,
>
>         tglx
>

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Arnd Bergmann]

On Wednesday 02 March 2011, Peppe CAVALLARO wrote:
> At any rate, I am happy to use the stmmac as experimental
> driver to do this kind tests.
> Indeed, in the past, on old Kernel (IIRC 2.6.23), I tried to use
> the kernel timers but I removed the code from it because
> I had noticed packets loss and a strange phenomenon with cyclesoak
> (that showed broken sysload % during the heavy network activities).
> 
> Let me know how to proceed:
> 
> 1) experiment with stmmac and hrtimer for handling rx/tx?
> 2) rework the patches for the Generic Timer Infra?

I'd suggest doing the first. I'm surprised that using an unrelated
timer for processing interrupts even helps you on stmmac.

The timers that you'd normally use for rx interrupt mitigation
are not periodic timers but are started when a packet arrives
from the outside.

Doing periodic wakeups for RX instead of just waiting for
packets to come in should have a significant impact on power
management on an otherwise idle system.

For tx resource reclaim, a relatively slow oneshot timer (not
even hrtimer) should be good enough, since it only needs to be
active when there is no other way to clean up. E.g. when you
are in napi polling mode (interrupt disabled), you know that
stmmac_poll gets called soon, and you can also do the reclaim
from stmmac_xmit() in order to prevent the timer from triggering
when you are constantly transmitting.

	Arnd

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Arnd Bergmann]

On Wednesday 02 March 2011, Peppe CAVALLARO wrote:
> At any rate, I am happy to use the stmmac as experimental
> driver to do this kind tests.
> Indeed, in the past, on old Kernel (IIRC 2.6.23), I tried to use
> the kernel timers but I removed the code from it because
> I had noticed packets loss and a strange phenomenon with cyclesoak
> (that showed broken sysload % during the heavy network activities).
> 
> Let me know how to proceed:
> 
> 1) experiment with stmmac and hrtimer for handling rx/tx?
> 2) rework the patches for the Generic Timer Infra?

I'd suggest doing the first. I'm surprised that using an unrelated
timer for processing interrupts even helps you on stmmac.

The timers that you'd normally use for rx interrupt mitigation
are not periodic timers but are started when a packet arrives
from the outside.

Doing periodic wakeups for RX instead of just waiting for
packets to come in should have a significant impact on power
management on an otherwise idle system.

For tx resource reclaim, a relatively slow oneshot timer (not
even hrtimer) should be good enough, since it only needs to be
active when there is no other way to clean up. E.g. when you
are in napi polling mode (interrupt disabled), you know that
stmmac_poll gets called soon, and you can also do the reclaim
from stmmac_xmit() in order to prevent the timer from triggering
when you are constantly transmitting.

	Arnd

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Peppe CAVALLARO]

Hi Arnd,

On 3/3/2011 9:45 AM, Arnd Bergmann wrote:
>
> On Wednesday 02 March 2011, Peppe CAVALLARO wrote:
> > At any rate, I am happy to use the stmmac as experimental
> > driver to do this kind tests.
> > Indeed, in the past, on old Kernel (IIRC 2.6.23), I tried to use
> > the kernel timers but I removed the code from it because
> > I had noticed packets loss and a strange phenomenon with cyclesoak
> > (that showed broken sysload % during the heavy network activities).
> >
> > Let me know how to proceed:
> >
> > 1) experiment with stmmac and hrtimer for handling rx/tx?
> > 2) rework the patches for the Generic Timer Infra?
>
> I'd suggest doing the first. I'm surprised that using an unrelated
> timer for processing interrupts even helps you on stmmac.
>

Indeed, this helped especially to save the cpu usage
on heavy IP traffic (but with Max Throughput and no pkt
loss).

> The timers that you'd normally use for rx interrupt mitigation
> are not periodic timers but are started when a packet arrives
> from the outside.
>
Yes you are right but unfortunately our mac devices have
not this kind of HW.
>
> Doing periodic wakeups for RX instead of just waiting for
> packets to come in should have a significant impact on power
> management on an otherwise idle system.
>
To "mitigate" this issue, the driver does a fast (and extra)
check in the rings and it does not start any rx processes
in case there are no incoming frames.

> For tx resource reclaim, a relatively slow oneshot timer (not
> even hrtimer) should be good enough, since it only needs to be
> active when there is no other way to clean up. E.g. when you
> are in napi polling mode (interrupt disabled), you know that
> stmmac_poll gets called soon, and you can also do the reclaim
> from stmmac_xmit() in order to prevent the timer from triggering
> when you are constantly transmitting.
>
This logic is already in the driver, indeed.
What I've seen on our embedded systems is that the
cost of RX interrupts is very hight and NAPI partially helps.
Typically, in an IP-STB, I receive a burst of UDP pkt
and this  means that many interrupts occur (~99% of CPU
usage on slow platforms).
With the ext timer I was able to reduce the CPU usage in
these kind of scenarios to ~50%.
When there is no net traffic, indeed, the timer periodically
"disturbs" the system but the impact on the CPU usage
actually is low.

Thanks
Peppe
>
>         Arnd
>

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Peppe CAVALLARO]

Hi Arnd,

On 3/3/2011 9:45 AM, Arnd Bergmann wrote:
>
> On Wednesday 02 March 2011, Peppe CAVALLARO wrote:
> > At any rate, I am happy to use the stmmac as experimental
> > driver to do this kind tests.
> > Indeed, in the past, on old Kernel (IIRC 2.6.23), I tried to use
> > the kernel timers but I removed the code from it because
> > I had noticed packets loss and a strange phenomenon with cyclesoak
> > (that showed broken sysload % during the heavy network activities).
> >
> > Let me know how to proceed:
> >
> > 1) experiment with stmmac and hrtimer for handling rx/tx?
> > 2) rework the patches for the Generic Timer Infra?
>
> I'd suggest doing the first. I'm surprised that using an unrelated
> timer for processing interrupts even helps you on stmmac.
>

Indeed, this helped especially to save the cpu usage
on heavy IP traffic (but with Max Throughput and no pkt
loss).

> The timers that you'd normally use for rx interrupt mitigation
> are not periodic timers but are started when a packet arrives
> from the outside.
>
Yes you are right but unfortunately our mac devices have
not this kind of HW.
>
> Doing periodic wakeups for RX instead of just waiting for
> packets to come in should have a significant impact on power
> management on an otherwise idle system.
>
To "mitigate" this issue, the driver does a fast (and extra)
check in the rings and it does not start any rx processes
in case there are no incoming frames.

> For tx resource reclaim, a relatively slow oneshot timer (not
> even hrtimer) should be good enough, since it only needs to be
> active when there is no other way to clean up. E.g. when you
> are in napi polling mode (interrupt disabled), you know that
> stmmac_poll gets called soon, and you can also do the reclaim
> from stmmac_xmit() in order to prevent the timer from triggering
> when you are constantly transmitting.
>
This logic is already in the driver, indeed.
What I've seen on our embedded systems is that the
cost of RX interrupts is very hight and NAPI partially helps.
Typically, in an IP-STB, I receive a burst of UDP pkt
and this  means that many interrupts occur (~99% of CPU
usage on slow platforms).
With the ext timer I was able to reduce the CPU usage in
these kind of scenarios to ~50%.
When there is no net traffic, indeed, the timer periodically
"disturbs" the system but the impact on the CPU usage
actually is low.

Thanks
Peppe
>
>         Arnd
>

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Arnd Bergmann]

On Thursday 03 March 2011, Peppe CAVALLARO wrote:
> This logic is already in the driver, indeed.
> What I've seen on our embedded systems is that the
> cost of RX interrupts is very hight and NAPI partially helps.
> Typically, in an IP-STB, I receive a burst of UDP pkt
> and this  means that many interrupts occur (~99% of CPU
> usage on slow platforms).
> With the ext timer I was able to reduce the CPU usage in
> these kind of scenarios to ~50%.

I don't understand. Shouldn't the interrupts be stopped as long
as the system is busy? This sounds like a bug in your NAPI
handling, or maybe you just need to use a lower NAPI_WEIGHT
so you stay in polling mode longer.

	Arnd

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Arnd Bergmann]

On Thursday 03 March 2011, Peppe CAVALLARO wrote:
> This logic is already in the driver, indeed.
> What I've seen on our embedded systems is that the
> cost of RX interrupts is very hight and NAPI partially helps.
> Typically, in an IP-STB, I receive a burst of UDP pkt
> and this  means that many interrupts occur (~99% of CPU
> usage on slow platforms).
> With the ext timer I was able to reduce the CPU usage in
> these kind of scenarios to ~50%.

I don't understand. Shouldn't the interrupts be stopped as long
as the system is busy? This sounds like a bug in your NAPI
handling, or maybe you just need to use a lower NAPI_WEIGHT
so you stay in polling mode longer.

	Arnd

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Peppe CAVALLARO]

On 3/3/2011 2:55 PM, Arnd Bergmann wrote:
>
> On Thursday 03 March 2011, Peppe CAVALLARO wrote:
> > This logic is already in the driver, indeed.
> > What I've seen on our embedded systems is that the
> > cost of RX interrupts is very hight and NAPI partially helps.
> > Typically, in an IP-STB, I receive a burst of UDP pkt
> > and this  means that many interrupts occur (~99% of CPU
> > usage on slow platforms).
> > With the ext timer I was able to reduce the CPU usage in
> > these kind of scenarios to ~50%.
>
> I don't understand. Shouldn't the interrupts be stopped as long
> as the system is busy? This sounds like a bug in your NAPI
> handling, or maybe you just need to use a lower NAPI_WEIGHT
> so you stay in polling mode longer.
>

Hi Arnd,
yes you are right, in fact, I do not receive an interrupt
for each frame received but the average of the frames
I found in the ring is very low.
At any rate, let me do some tests so i'll come back with
some numbers. In the meantime, I'm looking at the napi
support inside the driver and experiment with lower
NAPI_WEIGHT (default is 64).

Peppe
>
>
>         Arnd
>

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Peppe CAVALLARO]

On 3/3/2011 2:55 PM, Arnd Bergmann wrote:
>
> On Thursday 03 March 2011, Peppe CAVALLARO wrote:
> > This logic is already in the driver, indeed.
> > What I've seen on our embedded systems is that the
> > cost of RX interrupts is very hight and NAPI partially helps.
> > Typically, in an IP-STB, I receive a burst of UDP pkt
> > and this  means that many interrupts occur (~99% of CPU
> > usage on slow platforms).
> > With the ext timer I was able to reduce the CPU usage in
> > these kind of scenarios to ~50%.
>
> I don't understand. Shouldn't the interrupts be stopped as long
> as the system is busy? This sounds like a bug in your NAPI
> handling, or maybe you just need to use a lower NAPI_WEIGHT
> so you stay in polling mode longer.
>

Hi Arnd,
yes you are right, in fact, I do not receive an interrupt
for each frame received but the average of the frames
I found in the ring is very low.
At any rate, let me do some tests so i'll come back with
some numbers. In the meantime, I'm looking at the napi
support inside the driver and experiment with lower
NAPI_WEIGHT (default is 64).

Peppe
>
>
>         Arnd
>

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Paul Mundt]

On Tue, Mar 01, 2011 at 05:48:33PM +0100, Thomas Gleixner wrote:
> On Tue, 1 Mar 2011, Stuart Menefy wrote:
> > On 24/02/11 17:20, Arnd Bergmann wrote:
> > > On Tuesday 22 February 2011, Peppe CAVALLARO wrote:
> > >> From: Stuart Menefy <stuart.menefy@st.com>
> > >>
> > >> Many devices targeted at the embedded market provide a number of
> > >> generic timers which are capable of generating interrupts at a
> > >> requested rate. These can then be used in the implementation of drivers
> > >> for other peripherals which require a timer interrupt, without having
> > >> to provide an additional timer as part of that peripheral.
> 
> Why can't you just use an hrtimer and be done with it? Just because
> there is some extra hardware in the chip?
> 
..

> If we get some reasonable explanation why an extra timer interrupt is
> solving a specific problem better than hrtimers we can do that, but
> that needs more thought than picking the first available clockevent
> from a list. If we come to the conclusion, that we want/need this kind
> of functionality then I really prefer not to create yet another piece
> of infrastructure which deals with timer devices.
> 

I've run in to this issue again while working on local timer support on
SH, as we're presently dependent on broadcast and dummy tick devices in
the SMP case, while quite a few timer channels remain available for use.
(Ironically, while the aforementioned driver was able to solve the
problem with hrtimers, we have the same need to _provide_ hrtimers).

In the sh_tmu/cmt/mtu2 case all timer channels are handed off as platform
devices, and the block itself is global for all CPUs, though we can tweak
the IRQ affinity relative to the cpumask. My tentative plan is to deal
with the clockevent device as a percpu thing in the local timer case
which will require some additional side infrastructure, but some
additional work will need to be done in the clockevents code regardless.

The ARM approach is a bit backwards from what we're interested in
solving, as it uses its own local timer infrastructure and some
TWD-specific API for registering per-cpu timers and only then inserting
them in to the clockevents list. Whereas in our case we've got all of the
timer channels available at platform probe time (earlier for the early
timer case), and simply need a method for tracking unused channels as
well as having a facility for picking them up and reconfiguring them
later on when the secondary CPUs come up. I'm not at all interested in
registering the same timer multiple times with slightly different APIs,
having two different drivers fighting over the same register space is not
a thought that fills me with joy.

In any event, I'll hack on it a bit and see what falls out, patches
later.

Re: [PATCH (sh-2.6) 1/4] clksource: Generic timer infrastructure

[Paul Mundt]

On Tue, Mar 01, 2011 at 05:48:33PM +0100, Thomas Gleixner wrote:
> On Tue, 1 Mar 2011, Stuart Menefy wrote:
> > On 24/02/11 17:20, Arnd Bergmann wrote:
> > > On Tuesday 22 February 2011, Peppe CAVALLARO wrote:
> > >> From: Stuart Menefy <stuart.menefy@st.com>
> > >>
> > >> Many devices targeted at the embedded market provide a number of
> > >> generic timers which are capable of generating interrupts at a
> > >> requested rate. These can then be used in the implementation of drivers
> > >> for other peripherals which require a timer interrupt, without having
> > >> to provide an additional timer as part of that peripheral.
> 
> Why can't you just use an hrtimer and be done with it? Just because
> there is some extra hardware in the chip?
> 
..

> If we get some reasonable explanation why an extra timer interrupt is
> solving a specific problem better than hrtimers we can do that, but
> that needs more thought than picking the first available clockevent
> from a list. If we come to the conclusion, that we want/need this kind
> of functionality then I really prefer not to create yet another piece
> of infrastructure which deals with timer devices.
> 

I've run in to this issue again while working on local timer support on
SH, as we're presently dependent on broadcast and dummy tick devices in
the SMP case, while quite a few timer channels remain available for use.
(Ironically, while the aforementioned driver was able to solve the
problem with hrtimers, we have the same need to _provide_ hrtimers).

In the sh_tmu/cmt/mtu2 case all timer channels are handed off as platform
devices, and the block itself is global for all CPUs, though we can tweak
the IRQ affinity relative to the cpumask. My tentative plan is to deal
with the clockevent device as a percpu thing in the local timer case
which will require some additional side infrastructure, but some
additional work will need to be done in the clockevents code regardless.

The ARM approach is a bit backwards from what we're interested in
solving, as it uses its own local timer infrastructure and some
TWD-specific API for registering per-cpu timers and only then inserting
them in to the clockevents list. Whereas in our case we've got all of the
timer channels available at platform probe time (earlier for the early
timer case), and simply need a method for tracking unused channels as
well as having a facility for picking them up and reconfiguring them
later on when the secondary CPUs come up. I'm not at all interested in
registering the same timer multiple times with slightly different APIs,
having two different drivers fighting over the same register space is not
a thought that fills me with joy.

In any event, I'll hack on it a bit and see what falls out, patches
later.

[PATCH (sh-2.6) 2/4] sh_timer: add the support to use the generic

[Peppe CAVALLARO]

Add the support to register and use a TMU channel as
generic timer. For example the stmmac network device
driver, on STM platforms, uses the TMU channel 2 for
mitigating the number of interrupts.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Signed-off-by: Stuart Menefy <stuart.menefy@st.com>
---
 drivers/clocksource/sh_tmu.c |   72 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 72 insertions(+), 0 deletions(-)

diff --git a/drivers/clocksource/sh_tmu.c b/drivers/clocksource/sh_tmu.c
index 36aba99..8199556 100644
--- a/drivers/clocksource/sh_tmu.c
+++ b/drivers/clocksource/sh_tmu.c
@@ -29,6 +29,7 @@
 #include <linux/err.h>
 #include <linux/clocksource.h>
 #include <linux/clockchips.h>
+#include <linux/generictimer.h>
 #include <linux/sh_timer.h>
 #include <linux/slab.h>
 
@@ -41,6 +42,7 @@ struct sh_tmu_priv {
 	unsigned long periodic;
 	struct clock_event_device ced;
 	struct clocksource cs;
+	struct generic_timer gt;
 };
 
 static DEFINE_SPINLOCK(sh_tmu_lock);
@@ -184,6 +186,74 @@ static irqreturn_t sh_tmu_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static struct sh_tmu_priv *gt_to_sh_tmu(struct generic_timer *gt)
+{
+	return container_of(gt, struct sh_tmu_priv, gt);
+}
+
+static void sh_tmu_generic_timer_start(struct generic_timer *gt)
+{
+	struct sh_tmu_priv *p = gt_to_sh_tmu(gt);
+
+	sh_tmu_write(p, TCR, 0x0020);
+	sh_tmu_start_stop_ch(p, 1);
+}
+
+static void sh_tmu_generic_timer_stop(struct generic_timer *gt)
+{
+	struct sh_tmu_priv *p = gt_to_sh_tmu(gt);
+
+	sh_tmu_start_stop_ch(p, 0);
+	sh_tmu_write(p, TCR, 0x0000);
+}
+
+static void sh_tmu_generic_timer_set_rate(struct generic_timer *gt,
+					  unsigned long rate)
+{
+	struct sh_tmu_priv *p = gt_to_sh_tmu(gt);
+
+	sh_tmu_enable(p);
+	p->periodic = p->rate / rate;
+	sh_tmu_set_next(p, p->periodic, 1);
+}
+
+static irqreturn_t sh_tmu_generic_timer_interrupt(int irq, void *dev_id)
+{
+	struct sh_tmu_priv *p = dev_id;
+
+	sh_tmu_write(p, TCR, 0x0020);
+
+	p->gt.priv_handler(p->gt.data);
+	return IRQ_HANDLED;
+}
+
+static void sh_tmu_register_generic_timer(struct sh_tmu_priv *p, char *name)
+{
+	struct generic_timer *gt = &p->gt;
+	int ret;
+
+	gt->name = name;
+	gt->timer_start = sh_tmu_generic_timer_start;
+	gt->timer_stop = sh_tmu_generic_timer_stop;
+	gt->set_rate = sh_tmu_generic_timer_set_rate;
+
+	p->irqaction.handler = sh_tmu_generic_timer_interrupt;
+
+	ret = setup_irq(p->irqaction.irq, &p->irqaction);
+	if (unlikely(ret)) {
+		dev_err(&p->pdev->dev, "failed to request irq %d\n",
+			p->irqaction.irq);
+		return;
+	}
+
+	sh_tmu_generic_timer_stop(gt);
+	sh_tmu_write(p, TCOR, 0xffffffff);
+	sh_tmu_write(p, TCNT, 0xffffffff);
+
+	dev_info(&p->pdev->dev, "used for generic timer\n");
+	generic_timer_register_device(gt);
+}
+
 static struct sh_tmu_priv *cs_to_sh_tmu(struct clocksource *cs)
 {
 	return container_of(cs, struct sh_tmu_priv, cs);
@@ -342,6 +412,8 @@ static int sh_tmu_register(struct sh_tmu_priv *p, char *name,
 		sh_tmu_register_clockevent(p, name, clockevent_rating);
 	else if (clocksource_rating)
 		sh_tmu_register_clocksource(p, name, clocksource_rating);
+	else
+		sh_tmu_register_generic_timer(p, name);
 
 	return 0;
 }
-- 
1.7.4

[PATCH (sh-2.6) 2/4] sh_timer: add the support to use the generic timer

[Peppe CAVALLARO]

Add the support to register and use a TMU channel as
generic timer. For example the stmmac network device
driver, on STM platforms, uses the TMU channel 2 for
mitigating the number of interrupts.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Signed-off-by: Stuart Menefy <stuart.menefy@st.com>
---
 drivers/clocksource/sh_tmu.c |   72 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 72 insertions(+), 0 deletions(-)

diff --git a/drivers/clocksource/sh_tmu.c b/drivers/clocksource/sh_tmu.c
index 36aba99..8199556 100644
--- a/drivers/clocksource/sh_tmu.c
+++ b/drivers/clocksource/sh_tmu.c
@@ -29,6 +29,7 @@
 #include <linux/err.h>
 #include <linux/clocksource.h>
 #include <linux/clockchips.h>
+#include <linux/generictimer.h>
 #include <linux/sh_timer.h>
 #include <linux/slab.h>
 
@@ -41,6 +42,7 @@ struct sh_tmu_priv {
 	unsigned long periodic;
 	struct clock_event_device ced;
 	struct clocksource cs;
+	struct generic_timer gt;
 };
 
 static DEFINE_SPINLOCK(sh_tmu_lock);
@@ -184,6 +186,74 @@ static irqreturn_t sh_tmu_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static struct sh_tmu_priv *gt_to_sh_tmu(struct generic_timer *gt)
+{
+	return container_of(gt, struct sh_tmu_priv, gt);
+}
+
+static void sh_tmu_generic_timer_start(struct generic_timer *gt)
+{
+	struct sh_tmu_priv *p = gt_to_sh_tmu(gt);
+
+	sh_tmu_write(p, TCR, 0x0020);
+	sh_tmu_start_stop_ch(p, 1);
+}
+
+static void sh_tmu_generic_timer_stop(struct generic_timer *gt)
+{
+	struct sh_tmu_priv *p = gt_to_sh_tmu(gt);
+
+	sh_tmu_start_stop_ch(p, 0);
+	sh_tmu_write(p, TCR, 0x0000);
+}
+
+static void sh_tmu_generic_timer_set_rate(struct generic_timer *gt,
+					  unsigned long rate)
+{
+	struct sh_tmu_priv *p = gt_to_sh_tmu(gt);
+
+	sh_tmu_enable(p);
+	p->periodic = p->rate / rate;
+	sh_tmu_set_next(p, p->periodic, 1);
+}
+
+static irqreturn_t sh_tmu_generic_timer_interrupt(int irq, void *dev_id)
+{
+	struct sh_tmu_priv *p = dev_id;
+
+	sh_tmu_write(p, TCR, 0x0020);
+
+	p->gt.priv_handler(p->gt.data);
+	return IRQ_HANDLED;
+}
+
+static void sh_tmu_register_generic_timer(struct sh_tmu_priv *p, char *name)
+{
+	struct generic_timer *gt = &p->gt;
+	int ret;
+
+	gt->name = name;
+	gt->timer_start = sh_tmu_generic_timer_start;
+	gt->timer_stop = sh_tmu_generic_timer_stop;
+	gt->set_rate = sh_tmu_generic_timer_set_rate;
+
+	p->irqaction.handler = sh_tmu_generic_timer_interrupt;
+
+	ret = setup_irq(p->irqaction.irq, &p->irqaction);
+	if (unlikely(ret)) {
+		dev_err(&p->pdev->dev, "failed to request irq %d\n",
+			p->irqaction.irq);
+		return;
+	}
+
+	sh_tmu_generic_timer_stop(gt);
+	sh_tmu_write(p, TCOR, 0xffffffff);
+	sh_tmu_write(p, TCNT, 0xffffffff);
+
+	dev_info(&p->pdev->dev, "used for generic timer\n");
+	generic_timer_register_device(gt);
+}
+
 static struct sh_tmu_priv *cs_to_sh_tmu(struct clocksource *cs)
 {
 	return container_of(cs, struct sh_tmu_priv, cs);
@@ -342,6 +412,8 @@ static int sh_tmu_register(struct sh_tmu_priv *p, char *name,
 		sh_tmu_register_clockevent(p, name, clockevent_rating);
 	else if (clocksource_rating)
 		sh_tmu_register_clocksource(p, name, clocksource_rating);
+	else
+		sh_tmu_register_generic_timer(p, name);
 
 	return 0;
 }
-- 
1.7.4

[PATCH (net-2.6) 3/4] stmmac: switch to use the new generic timer

[Peppe CAVALLARO]

The stmmac can use an external timer for mitigating
the number of interrupts.
On SH4 platforms based this can be done by using the
TMU channel 2 interrupt.
This actually helps many people to save the CPU usage
on platforms with MAC10/100 and GMAC devices without
an embedded timer.

This patch removes the old code (obsoleted and only
valid for old STM Kernels) and uses the new generic
timer interface.

Signed-off-by: Stuart Menefy <stuart.menefy@st.com>
Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/stmmac/Kconfig        |    1 -
 drivers/net/stmmac/stmmac_main.c  |   21 ++++----
 drivers/net/stmmac/stmmac_timer.c |  101 +++++++++++++++++++++---------------
 drivers/net/stmmac/stmmac_timer.h |   11 ++---
 4 files changed, 74 insertions(+), 60 deletions(-)

diff --git a/drivers/net/stmmac/Kconfig b/drivers/net/stmmac/Kconfig
index 7df7df4..b74e79b 100644
--- a/drivers/net/stmmac/Kconfig
+++ b/drivers/net/stmmac/Kconfig
@@ -32,7 +32,6 @@ config STMMAC_DUAL_MAC
 config STMMAC_TIMER
 	bool "STMMAC Timer optimisation"
 	default n
-	depends on RTC_HCTOSYS_DEVICE
 	help
 	  Use an external timer for mitigating the number of network
 	  interrupts. Currently, for SH architectures, it is possible
diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c
index 34a0af3..26714b4 100644
--- a/drivers/net/stmmac/stmmac_main.c
+++ b/drivers/net/stmmac/stmmac_main.c
@@ -652,7 +652,7 @@ static inline void stmmac_enable_irq(struct stmmac_priv *priv)
 {
 #ifdef CONFIG_STMMAC_TIMER
 	if (likely(priv->tm->enable))
-		priv->tm->timer_start(tmrate);
+		priv->tm->timer_start(priv->tm->timer_callb, tmrate);
 	else
 #endif
 		priv->hw->dma->enable_dma_irq(priv->ioaddr);
@@ -662,7 +662,7 @@ static inline void stmmac_disable_irq(struct stmmac_priv *priv)
 {
 #ifdef CONFIG_STMMAC_TIMER
 	if (likely(priv->tm->enable))
-		priv->tm->timer_stop();
+		priv->tm->timer_stop(priv->tm->timer_callb);
 	else
 #endif
 		priv->hw->dma->disable_dma_irq(priv->ioaddr);
@@ -703,11 +703,11 @@ void stmmac_schedule(struct net_device *dev)
 	_stmmac_schedule(priv);
 }
 
-static void stmmac_no_timer_started(unsigned int x)
+static void stmmac_no_timer_started(void *t, unsigned int x)
 {;
 };
 
-static void stmmac_no_timer_stopped(void)
+static void stmmac_no_timer_stopped(void *t)
 {;
 };
 #endif
@@ -720,7 +720,6 @@ static void stmmac_no_timer_stopped(void)
  */
 static void stmmac_tx_err(struct stmmac_priv *priv)
 {
-
 	netif_stop_queue(priv->dev);
 
 	priv->hw->dma->stop_tx(priv->ioaddr);
@@ -807,7 +806,8 @@ static int stmmac_open(struct net_device *dev)
 	/* Test if the external timer can be actually used.
 	 * In case of failure continue without timer. */
 	if (unlikely((stmmac_open_ext_timer(dev, priv->tm)) < 0)) {
-		pr_warning("stmmaceth: cannot attach the external timer.\n");
+		pr_warning("stmmac (%s): cannot attach the external timer.\n",
+			   dev->name);
 		priv->tm->freq = 0;
 		priv->tm->timer_start = stmmac_no_timer_started;
 		priv->tm->timer_stop = stmmac_no_timer_stopped;
@@ -864,7 +864,8 @@ static int stmmac_open(struct net_device *dev)
 	priv->hw->dma->start_rx(priv->ioaddr);
 
 #ifdef CONFIG_STMMAC_TIMER
-	priv->tm->timer_start(tmrate);
+	if (likely(priv->tm->enable))
+		priv->tm->timer_start(priv->tm->timer_callb, tmrate);
 #endif
 	/* Dump DMA/MAC registers */
 	if (netif_msg_hw(priv)) {
@@ -902,7 +903,7 @@ static int stmmac_release(struct net_device *dev)
 
 #ifdef CONFIG_STMMAC_TIMER
 	/* Stop and release the timer */
-	stmmac_close_ext_timer();
+	stmmac_close_ext_timer(priv->tm->timer_callb);
 	if (priv->tm != NULL)
 		kfree(priv->tm);
 #endif
@@ -1815,7 +1816,7 @@ static int stmmac_suspend(struct device *dev)
 		phy_stop(priv->phydev);
 
 #ifdef CONFIG_STMMAC_TIMER
-	priv->tm->timer_stop();
+	priv->tm->timer_stop(priv->tm->timer_callb);
 	if (likely(priv->tm->enable))
 		dis_ic = 1;
 #endif
@@ -1866,7 +1867,7 @@ static int stmmac_resume(struct device *dev)
 
 #ifdef CONFIG_STMMAC_TIMER
 	if (likely(priv->tm->enable))
-		priv->tm->timer_start(tmrate);
+		priv->tm->timer_start(priv->tm->timer_callb, tmrate);
 #endif
 	napi_enable(&priv->napi);
 
diff --git a/drivers/net/stmmac/stmmac_timer.c b/drivers/net/stmmac/stmmac_timer.c
index 2a0e1ab..2481daa 100644
--- a/drivers/net/stmmac/stmmac_timer.c
+++ b/drivers/net/stmmac/stmmac_timer.c
@@ -33,102 +33,119 @@ static void stmmac_timer_handler(void *data)
 	stmmac_schedule(dev);
 }
 
-#define STMMAC_TIMER_MSG(timer, freq) \
-printk(KERN_INFO "stmmac_timer: %s Timer ON (freq %dHz)\n", timer, freq);
+#define STMMAC_TIMER_MSG(dev_name, timer, freq) \
+printk(KERN_INFO "stmmac_timer: (%s) %s Timer (freq %dHz)\n", \
+		 dev_name, timer, freq);
 
 #if defined(CONFIG_STMMAC_RTC_TIMER)
 #include <linux/rtc.h>
-static struct rtc_device *stmmac_rtc;
-static rtc_task_t stmmac_task;
 
-static void stmmac_rtc_start(unsigned int new_freq)
+static void stmmac_rtc_start(void *timer, unsigned int new_freq)
 {
-	rtc_irq_set_freq(stmmac_rtc, &stmmac_task, new_freq);
-	rtc_irq_set_state(stmmac_rtc, &stmmac_task, 1);
+	struct rtc_device *rtc = timer;
+
+	rtc_irq_set_freq(rtc, rtc->irq_task, new_freq);
+	rtc_irq_set_state(rtc, rtc->irq_task, 1);
+	return;
 }
 
-static void stmmac_rtc_stop(void)
+static void stmmac_rtc_stop(void *timer)
 {
-	rtc_irq_set_state(stmmac_rtc, &stmmac_task, 0);
+	struct rtc_device *rtc = timer;
+
+	rtc_irq_set_state(rtc, rtc->irq_task, 0);
+	return;
 }
 
 int stmmac_open_ext_timer(struct net_device *dev, struct stmmac_timer *tm)
 {
-	stmmac_task.private_data = dev;
-	stmmac_task.func = stmmac_timer_handler;
+	struct rtc_device *rtc;
+	rtc_task_t rtc_task;
+
+	rtc_task.private_data = dev;
+	rtc_task.func = stmmac_timer_handler;
 
-	stmmac_rtc = rtc_class_open(CONFIG_RTC_HCTOSYS_DEVICE);
-	if (stmmac_rtc = NULL) {
+	rtc = rtc_class_open(CONFIG_RTC_HCTOSYS_DEVICE);
+	if (rtc = NULL) {
 		pr_err("open rtc device failed\n");
 		return -ENODEV;
 	}
 
-	rtc_irq_register(stmmac_rtc, &stmmac_task);
+	rtc_irq_register(rtc, &rtc_task);
 
 	/* Periodic mode is not supported */
-	if ((rtc_irq_set_freq(stmmac_rtc, &stmmac_task, tm->freq) < 0)) {
+	if ((rtc_irq_set_freq(rtc, &rtc_task, tm->freq) < 0)) {
 		pr_err("set periodic failed\n");
-		rtc_irq_unregister(stmmac_rtc, &stmmac_task);
-		rtc_class_close(stmmac_rtc);
+		rtc_irq_unregister(rtc, &rtc_task);
+		rtc_class_close(rtc);
 		return -1;
 	}
 
-	STMMAC_TIMER_MSG(CONFIG_RTC_HCTOSYS_DEVICE, tm->freq);
+	STMMAC_TIMER_MSG(dev->name, CONFIG_RTC_HCTOSYS_DEVICE, tm->freq);
 
+	rtc->irq_task = &rtc_task;
+	tm->timer_callb = rtc;
 	tm->timer_start = stmmac_rtc_start;
 	tm->timer_stop = stmmac_rtc_stop;
 
 	return 0;
 }
 
-int stmmac_close_ext_timer(void)
+int stmmac_close_ext_timer(void *timer)
 {
-	rtc_irq_set_state(stmmac_rtc, &stmmac_task, 0);
-	rtc_irq_unregister(stmmac_rtc, &stmmac_task);
-	rtc_class_close(stmmac_rtc);
+	struct rtc_device *rtc = timer;
+
+	rtc_irq_set_state(rtc, rtc->irq_task, 0);
+	rtc_irq_unregister(rtc, rtc->irq_task);
+	rtc_class_close(rtc);
+
 	return 0;
 }
 
 #elif defined(CONFIG_STMMAC_TMU_TIMER)
-#include <linux/clk.h>
-#define TMU_CHANNEL "tmu2_clk"
-static struct clk *timer_clock;
+#include <linux/generictimer.h>
 
-static void stmmac_tmu_start(unsigned int new_freq)
+/* Set rate and start the timer */
+static void stmmac_tmu_set_rate(void *timer_callb, unsigned int new_freq)
 {
-	clk_set_rate(timer_clock, new_freq);
-	clk_enable(timer_clock);
+	struct generic_timer *timer = timer_callb;
+
+	generic_timer_start(timer);
+	generic_timer_set_rate(timer, new_freq);
+	return;
 }
 
-static void stmmac_tmu_stop(void)
+static void stmmac_tmu_stop(void *timer_callb)
 {
-	clk_disable(timer_clock);
+	struct generic_timer *timer = timer_callb;
+
+	generic_timer_stop(timer);
+	return;
 }
 
 int stmmac_open_ext_timer(struct net_device *dev, struct stmmac_timer *tm)
 {
-	timer_clock = clk_get(NULL, TMU_CHANNEL);
+	struct generic_timer *timer;
+	timer = generic_timer_claim(stmmac_timer_handler,  dev);
 
-	if (timer_clock = NULL)
+	if (timer = NULL)
 		return -1;
 
-	if (tmu2_register_user(stmmac_timer_handler, (void *)dev) < 0) {
-		timer_clock = NULL;
-		return -1;
-	}
+	STMMAC_TIMER_MSG(dev->name, "sh_tmu", tm->freq);
 
-	STMMAC_TIMER_MSG("TMU2", tm->freq);
-	tm->timer_start = stmmac_tmu_start;
+	tm->timer_callb = timer;
+	tm->timer_start = stmmac_tmu_set_rate;
 	tm->timer_stop = stmmac_tmu_stop;
 
 	return 0;
 }
 
-int stmmac_close_ext_timer(void)
+int stmmac_close_ext_timer(void *timer_callb)
 {
-	clk_disable(timer_clock);
-	tmu2_unregister_user();
-	clk_put(timer_clock);
+	struct generic_timer *timer = timer_callb;
+
+	stmmac_tmu_stop(timer_callb);
+	generic_timer_release(timer);
 	return 0;
 }
 #endif
diff --git a/drivers/net/stmmac/stmmac_timer.h b/drivers/net/stmmac/stmmac_timer.h
index 6863590..250f5cb 100644
--- a/drivers/net/stmmac/stmmac_timer.h
+++ b/drivers/net/stmmac/stmmac_timer.h
@@ -23,20 +23,17 @@
 *******************************************************************************/
 
 struct stmmac_timer {
-	void (*timer_start) (unsigned int new_freq);
-	void (*timer_stop) (void);
+	void (*timer_start) (void *timer, unsigned int new_freq);
+	void (*timer_stop) (void *timer);
 	unsigned int freq;
 	unsigned int enable;
+	void *timer_callb;
 };
 
 /* Open the HW timer device and return 0 in case of success */
 int stmmac_open_ext_timer(struct net_device *dev, struct stmmac_timer *tm);
 /* Stop the timer and release it */
-int stmmac_close_ext_timer(void);
+int stmmac_close_ext_timer(void *priv);
 /* Function used for scheduling task within the stmmac */
 void stmmac_schedule(struct net_device *dev);
 
-#if defined(CONFIG_STMMAC_TMU_TIMER)
-extern int tmu2_register_user(void *fnt, void *data);
-extern void tmu2_unregister_user(void);
-#endif
-- 
1.7.4

[PATCH (net-2.6) 3/4] stmmac: switch to use the new generic timer interface

[Peppe CAVALLARO]

The stmmac can use an external timer for mitigating
the number of interrupts.
On SH4 platforms based this can be done by using the
TMU channel 2 interrupt.
This actually helps many people to save the CPU usage
on platforms with MAC10/100 and GMAC devices without
an embedded timer.

This patch removes the old code (obsoleted and only
valid for old STM Kernels) and uses the new generic
timer interface.

Signed-off-by: Stuart Menefy <stuart.menefy@st.com>
Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/stmmac/Kconfig        |    1 -
 drivers/net/stmmac/stmmac_main.c  |   21 ++++----
 drivers/net/stmmac/stmmac_timer.c |  101 +++++++++++++++++++++---------------
 drivers/net/stmmac/stmmac_timer.h |   11 ++---
 4 files changed, 74 insertions(+), 60 deletions(-)

diff --git a/drivers/net/stmmac/Kconfig b/drivers/net/stmmac/Kconfig
index 7df7df4..b74e79b 100644
--- a/drivers/net/stmmac/Kconfig
+++ b/drivers/net/stmmac/Kconfig
@@ -32,7 +32,6 @@ config STMMAC_DUAL_MAC
 config STMMAC_TIMER
 	bool "STMMAC Timer optimisation"
 	default n
-	depends on RTC_HCTOSYS_DEVICE
 	help
 	  Use an external timer for mitigating the number of network
 	  interrupts. Currently, for SH architectures, it is possible
diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c
index 34a0af3..26714b4 100644
--- a/drivers/net/stmmac/stmmac_main.c
+++ b/drivers/net/stmmac/stmmac_main.c
@@ -652,7 +652,7 @@ static inline void stmmac_enable_irq(struct stmmac_priv *priv)
 {
 #ifdef CONFIG_STMMAC_TIMER
 	if (likely(priv->tm->enable))
-		priv->tm->timer_start(tmrate);
+		priv->tm->timer_start(priv->tm->timer_callb, tmrate);
 	else
 #endif
 		priv->hw->dma->enable_dma_irq(priv->ioaddr);
@@ -662,7 +662,7 @@ static inline void stmmac_disable_irq(struct stmmac_priv *priv)
 {
 #ifdef CONFIG_STMMAC_TIMER
 	if (likely(priv->tm->enable))
-		priv->tm->timer_stop();
+		priv->tm->timer_stop(priv->tm->timer_callb);
 	else
 #endif
 		priv->hw->dma->disable_dma_irq(priv->ioaddr);
@@ -703,11 +703,11 @@ void stmmac_schedule(struct net_device *dev)
 	_stmmac_schedule(priv);
 }
 
-static void stmmac_no_timer_started(unsigned int x)
+static void stmmac_no_timer_started(void *t, unsigned int x)
 {;
 };
 
-static void stmmac_no_timer_stopped(void)
+static void stmmac_no_timer_stopped(void *t)
 {;
 };
 #endif
@@ -720,7 +720,6 @@ static void stmmac_no_timer_stopped(void)
  */
 static void stmmac_tx_err(struct stmmac_priv *priv)
 {
-
 	netif_stop_queue(priv->dev);
 
 	priv->hw->dma->stop_tx(priv->ioaddr);
@@ -807,7 +806,8 @@ static int stmmac_open(struct net_device *dev)
 	/* Test if the external timer can be actually used.
 	 * In case of failure continue without timer. */
 	if (unlikely((stmmac_open_ext_timer(dev, priv->tm)) < 0)) {
-		pr_warning("stmmaceth: cannot attach the external timer.\n");
+		pr_warning("stmmac (%s): cannot attach the external timer.\n",
+			   dev->name);
 		priv->tm->freq = 0;
 		priv->tm->timer_start = stmmac_no_timer_started;
 		priv->tm->timer_stop = stmmac_no_timer_stopped;
@@ -864,7 +864,8 @@ static int stmmac_open(struct net_device *dev)
 	priv->hw->dma->start_rx(priv->ioaddr);
 
 #ifdef CONFIG_STMMAC_TIMER
-	priv->tm->timer_start(tmrate);
+	if (likely(priv->tm->enable))
+		priv->tm->timer_start(priv->tm->timer_callb, tmrate);
 #endif
 	/* Dump DMA/MAC registers */
 	if (netif_msg_hw(priv)) {
@@ -902,7 +903,7 @@ static int stmmac_release(struct net_device *dev)
 
 #ifdef CONFIG_STMMAC_TIMER
 	/* Stop and release the timer */
-	stmmac_close_ext_timer();
+	stmmac_close_ext_timer(priv->tm->timer_callb);
 	if (priv->tm != NULL)
 		kfree(priv->tm);
 #endif
@@ -1815,7 +1816,7 @@ static int stmmac_suspend(struct device *dev)
 		phy_stop(priv->phydev);
 
 #ifdef CONFIG_STMMAC_TIMER
-	priv->tm->timer_stop();
+	priv->tm->timer_stop(priv->tm->timer_callb);
 	if (likely(priv->tm->enable))
 		dis_ic = 1;
 #endif
@@ -1866,7 +1867,7 @@ static int stmmac_resume(struct device *dev)
 
 #ifdef CONFIG_STMMAC_TIMER
 	if (likely(priv->tm->enable))
-		priv->tm->timer_start(tmrate);
+		priv->tm->timer_start(priv->tm->timer_callb, tmrate);
 #endif
 	napi_enable(&priv->napi);
 
diff --git a/drivers/net/stmmac/stmmac_timer.c b/drivers/net/stmmac/stmmac_timer.c
index 2a0e1ab..2481daa 100644
--- a/drivers/net/stmmac/stmmac_timer.c
+++ b/drivers/net/stmmac/stmmac_timer.c
@@ -33,102 +33,119 @@ static void stmmac_timer_handler(void *data)
 	stmmac_schedule(dev);
 }
 
-#define STMMAC_TIMER_MSG(timer, freq) \
-printk(KERN_INFO "stmmac_timer: %s Timer ON (freq %dHz)\n", timer, freq);
+#define STMMAC_TIMER_MSG(dev_name, timer, freq) \
+printk(KERN_INFO "stmmac_timer: (%s) %s Timer (freq %dHz)\n", \
+		 dev_name, timer, freq);
 
 #if defined(CONFIG_STMMAC_RTC_TIMER)
 #include <linux/rtc.h>
-static struct rtc_device *stmmac_rtc;
-static rtc_task_t stmmac_task;
 
-static void stmmac_rtc_start(unsigned int new_freq)
+static void stmmac_rtc_start(void *timer, unsigned int new_freq)
 {
-	rtc_irq_set_freq(stmmac_rtc, &stmmac_task, new_freq);
-	rtc_irq_set_state(stmmac_rtc, &stmmac_task, 1);
+	struct rtc_device *rtc = timer;
+
+	rtc_irq_set_freq(rtc, rtc->irq_task, new_freq);
+	rtc_irq_set_state(rtc, rtc->irq_task, 1);
+	return;
 }
 
-static void stmmac_rtc_stop(void)
+static void stmmac_rtc_stop(void *timer)
 {
-	rtc_irq_set_state(stmmac_rtc, &stmmac_task, 0);
+	struct rtc_device *rtc = timer;
+
+	rtc_irq_set_state(rtc, rtc->irq_task, 0);
+	return;
 }
 
 int stmmac_open_ext_timer(struct net_device *dev, struct stmmac_timer *tm)
 {
-	stmmac_task.private_data = dev;
-	stmmac_task.func = stmmac_timer_handler;
+	struct rtc_device *rtc;
+	rtc_task_t rtc_task;
+
+	rtc_task.private_data = dev;
+	rtc_task.func = stmmac_timer_handler;
 
-	stmmac_rtc = rtc_class_open(CONFIG_RTC_HCTOSYS_DEVICE);
-	if (stmmac_rtc == NULL) {
+	rtc = rtc_class_open(CONFIG_RTC_HCTOSYS_DEVICE);
+	if (rtc == NULL) {
 		pr_err("open rtc device failed\n");
 		return -ENODEV;
 	}
 
-	rtc_irq_register(stmmac_rtc, &stmmac_task);
+	rtc_irq_register(rtc, &rtc_task);
 
 	/* Periodic mode is not supported */
-	if ((rtc_irq_set_freq(stmmac_rtc, &stmmac_task, tm->freq) < 0)) {
+	if ((rtc_irq_set_freq(rtc, &rtc_task, tm->freq) < 0)) {
 		pr_err("set periodic failed\n");
-		rtc_irq_unregister(stmmac_rtc, &stmmac_task);
-		rtc_class_close(stmmac_rtc);
+		rtc_irq_unregister(rtc, &rtc_task);
+		rtc_class_close(rtc);
 		return -1;
 	}
 
-	STMMAC_TIMER_MSG(CONFIG_RTC_HCTOSYS_DEVICE, tm->freq);
+	STMMAC_TIMER_MSG(dev->name, CONFIG_RTC_HCTOSYS_DEVICE, tm->freq);
 
+	rtc->irq_task = &rtc_task;
+	tm->timer_callb = rtc;
 	tm->timer_start = stmmac_rtc_start;
 	tm->timer_stop = stmmac_rtc_stop;
 
 	return 0;
 }
 
-int stmmac_close_ext_timer(void)
+int stmmac_close_ext_timer(void *timer)
 {
-	rtc_irq_set_state(stmmac_rtc, &stmmac_task, 0);
-	rtc_irq_unregister(stmmac_rtc, &stmmac_task);
-	rtc_class_close(stmmac_rtc);
+	struct rtc_device *rtc = timer;
+
+	rtc_irq_set_state(rtc, rtc->irq_task, 0);
+	rtc_irq_unregister(rtc, rtc->irq_task);
+	rtc_class_close(rtc);
+
 	return 0;
 }
 
 #elif defined(CONFIG_STMMAC_TMU_TIMER)
-#include <linux/clk.h>
-#define TMU_CHANNEL "tmu2_clk"
-static struct clk *timer_clock;
+#include <linux/generictimer.h>
 
-static void stmmac_tmu_start(unsigned int new_freq)
+/* Set rate and start the timer */
+static void stmmac_tmu_set_rate(void *timer_callb, unsigned int new_freq)
 {
-	clk_set_rate(timer_clock, new_freq);
-	clk_enable(timer_clock);
+	struct generic_timer *timer = timer_callb;
+
+	generic_timer_start(timer);
+	generic_timer_set_rate(timer, new_freq);
+	return;
 }
 
-static void stmmac_tmu_stop(void)
+static void stmmac_tmu_stop(void *timer_callb)
 {
-	clk_disable(timer_clock);
+	struct generic_timer *timer = timer_callb;
+
+	generic_timer_stop(timer);
+	return;
 }
 
 int stmmac_open_ext_timer(struct net_device *dev, struct stmmac_timer *tm)
 {
-	timer_clock = clk_get(NULL, TMU_CHANNEL);
+	struct generic_timer *timer;
+	timer = generic_timer_claim(stmmac_timer_handler,  dev);
 
-	if (timer_clock == NULL)
+	if (timer == NULL)
 		return -1;
 
-	if (tmu2_register_user(stmmac_timer_handler, (void *)dev) < 0) {
-		timer_clock = NULL;
-		return -1;
-	}
+	STMMAC_TIMER_MSG(dev->name, "sh_tmu", tm->freq);
 
-	STMMAC_TIMER_MSG("TMU2", tm->freq);
-	tm->timer_start = stmmac_tmu_start;
+	tm->timer_callb = timer;
+	tm->timer_start = stmmac_tmu_set_rate;
 	tm->timer_stop = stmmac_tmu_stop;
 
 	return 0;
 }
 
-int stmmac_close_ext_timer(void)
+int stmmac_close_ext_timer(void *timer_callb)
 {
-	clk_disable(timer_clock);
-	tmu2_unregister_user();
-	clk_put(timer_clock);
+	struct generic_timer *timer = timer_callb;
+
+	stmmac_tmu_stop(timer_callb);
+	generic_timer_release(timer);
 	return 0;
 }
 #endif
diff --git a/drivers/net/stmmac/stmmac_timer.h b/drivers/net/stmmac/stmmac_timer.h
index 6863590..250f5cb 100644
--- a/drivers/net/stmmac/stmmac_timer.h
+++ b/drivers/net/stmmac/stmmac_timer.h
@@ -23,20 +23,17 @@
 *******************************************************************************/
 
 struct stmmac_timer {
-	void (*timer_start) (unsigned int new_freq);
-	void (*timer_stop) (void);
+	void (*timer_start) (void *timer, unsigned int new_freq);
+	void (*timer_stop) (void *timer);
 	unsigned int freq;
 	unsigned int enable;
+	void *timer_callb;
 };
 
 /* Open the HW timer device and return 0 in case of success */
 int stmmac_open_ext_timer(struct net_device *dev, struct stmmac_timer *tm);
 /* Stop the timer and release it */
-int stmmac_close_ext_timer(void);
+int stmmac_close_ext_timer(void *priv);
 /* Function used for scheduling task within the stmmac */
 void stmmac_schedule(struct net_device *dev);
 
-#if defined(CONFIG_STMMAC_TMU_TIMER)
-extern int tmu2_register_user(void *fnt, void *data);
-extern void tmu2_unregister_user(void);
-#endif
-- 
1.7.4

[PATCH (net-2.6) 4/4] stmmac: rework and improvement the stmmac

[Peppe CAVALLARO]

This patch prepares the stmmac to use the
embedded watchdog timer included in the new
mac generations. The STMMAC_TIMER option becomes
STMMAC_EXT_TIMER. The patch also tidies-up the code
and improvements the Kconfig information.

The driver can use the external timer and in case
of problems, i.e. while registering the timer,
it'll continue to work with NAPI and interrupts.

Currently, the STMMAC_EXT_TIMER can be turned on
SUPERH only and it depends on SH_TIMER_TMU: I mean
on platforms where I performed all my tests.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/stmmac/Kconfig        |   18 ++++++++----
 drivers/net/stmmac/Makefile       |    2 +-
 drivers/net/stmmac/common.h       |    6 ++++
 drivers/net/stmmac/stmmac.h       |    5 ++-
 drivers/net/stmmac/stmmac_main.c  |   51 +++++++++++++++++++-----------------
 drivers/net/stmmac/stmmac_timer.c |    4 +-
 drivers/net/stmmac/stmmac_timer.h |    1 -
 7 files changed, 51 insertions(+), 36 deletions(-)

diff --git a/drivers/net/stmmac/Kconfig b/drivers/net/stmmac/Kconfig
index b74e79b..cdbe5a8 100644
--- a/drivers/net/stmmac/Kconfig
+++ b/drivers/net/stmmac/Kconfig
@@ -29,22 +29,28 @@ config STMMAC_DUAL_MAC
 	  Ethernet Controllers. This option turns on the second Ethernet
 	  device on this kind of platforms.
 
-config STMMAC_TIMER
+config STMMAC_EXT_TIMER
 	bool "STMMAC Timer optimisation"
 	default n
 	help
 	  Use an external timer for mitigating the number of network
 	  interrupts. Currently, for SH architectures, it is possible
-	  to use the TMU channel 2 and the SH-RTC device.
+	  to use the TMU channel 2 (via Generic Timer) and the SH-RTC
+	  device. If the timer registration fails during the interface
+	  initialisation then the driver will work without any
+	  mitigation schema.
 
 choice
         prompt "Select Timer device"
-        depends on STMMAC_TIMER
+        depends on STMMAC_EXT_TIMER
 
-config STMMAC_TMU_TIMER
-        bool "TMU channel 2"
-        depends on CPU_SH4
+config STMMAC_GEN_TIMER
+        bool "Generic External Timer"
+        depends on SH_TIMER_TMU
 	help
+	  Use the Generic timer for mitigating the interrupts.
+	  For example, in case of SUPERH the TMU channel 2
+	  is used for that.
 
 config STMMAC_RTC_TIMER
         bool "Real time clock"
diff --git a/drivers/net/stmmac/Makefile b/drivers/net/stmmac/Makefile
index 9691733..05d84b2 100644
--- a/drivers/net/stmmac/Makefile
+++ b/drivers/net/stmmac/Makefile
@@ -1,5 +1,5 @@
 obj-$(CONFIG_STMMAC_ETH) += stmmac.o
-stmmac-$(CONFIG_STMMAC_TIMER) += stmmac_timer.o
+stmmac-$(CONFIG_STMMAC_EXT_TIMER) += stmmac_timer.o
 stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o	\
 	      dwmac_lib.o dwmac1000_core.o  dwmac1000_dma.o	\
 	      dwmac100_core.o dwmac100_dma.o enh_desc.o  norm_desc.o $(stmmac-y)
diff --git a/drivers/net/stmmac/common.h b/drivers/net/stmmac/common.h
index 375ea19..4bd9d01 100644
--- a/drivers/net/stmmac/common.h
+++ b/drivers/net/stmmac/common.h
@@ -43,6 +43,12 @@
 #undef FRAME_FILTER_DEBUG
 /* #define FRAME_FILTER_DEBUG */
 
+enum mitigation_timer {
+	no_timer = 0,
+	external = 1,
+	embedded = 2,
+};
+
 struct stmmac_extra_stats {
 	/* Transmit errors */
 	unsigned long tx_underflow ____cacheline_aligned;
diff --git a/drivers/net/stmmac/stmmac.h b/drivers/net/stmmac/stmmac.h
index 5f06c47..1b76977 100644
--- a/drivers/net/stmmac/stmmac.h
+++ b/drivers/net/stmmac/stmmac.h
@@ -25,7 +25,7 @@
 #include <linux/stmmac.h>
 
 #include "common.h"
-#ifdef CONFIG_STMMAC_TIMER
+#ifdef CONFIG_STMMAC_EXT_TIMER
 #include "stmmac_timer.h"
 #endif
 
@@ -77,9 +77,10 @@ struct stmmac_priv {
 	spinlock_t lock;
 	int wolopts;
 	int wolenabled;
-#ifdef CONFIG_STMMAC_TIMER
+#ifdef CONFIG_STMMAC_EXT_TIMER
 	struct stmmac_timer *tm;
 #endif
+	unsigned int timer;
 #ifdef STMMAC_VLAN_TAG_USED
 	struct vlan_group *vlgrp;
 #endif
diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c
index 26714b4..53a7086 100644
--- a/drivers/net/stmmac/stmmac_main.c
+++ b/drivers/net/stmmac/stmmac_main.c
@@ -122,7 +122,7 @@ MODULE_PARM_DESC(tc, "DMA threshold control value");
 /* Pay attention to tune this parameter; take care of both
  * hardware capability and network stabitily/performance impact.
  * Many tests showed that ~4ms latency seems to be good enough. */
-#ifdef CONFIG_STMMAC_TIMER
+#ifdef CONFIG_STMMAC_EXT_TIMER
 #define DEFAULT_PERIODIC_RATE	256
 static int tmrate = DEFAULT_PERIODIC_RATE;
 module_param(tmrate, int, S_IRUGO | S_IWUSR);
@@ -415,9 +415,9 @@ static void init_dma_desc_rings(struct net_device *dev)
 	else
 		bfsize = DMA_BUFFER_SIZE;
 
-#ifdef CONFIG_STMMAC_TIMER
+#ifdef CONFIG_STMMAC_EXT_TIMER
 	/* Disable interrupts on completion for the reception if timer is on */
-	if (likely(priv->tm->enable))
+	if (likely(priv->timer = external))
 		dis_ic = 1;
 #endif
 	/* If the MTU exceeds 8k so use the second buffer in the chain */
@@ -650,8 +650,8 @@ static void stmmac_tx(struct stmmac_priv *priv)
 
 static inline void stmmac_enable_irq(struct stmmac_priv *priv)
 {
-#ifdef CONFIG_STMMAC_TIMER
-	if (likely(priv->tm->enable))
+#ifdef CONFIG_STMMAC_EXT_TIMER
+	if (likely(priv->timer = external))
 		priv->tm->timer_start(priv->tm->timer_callb, tmrate);
 	else
 #endif
@@ -660,8 +660,8 @@ static inline void stmmac_enable_irq(struct stmmac_priv *priv)
 
 static inline void stmmac_disable_irq(struct stmmac_priv *priv)
 {
-#ifdef CONFIG_STMMAC_TIMER
-	if (likely(priv->tm->enable))
+#ifdef CONFIG_STMMAC_EXT_TIMER
+	if (likely(priv->timer = external))
 		priv->tm->timer_stop(priv->tm->timer_callb);
 	else
 #endif
@@ -693,7 +693,7 @@ static inline void _stmmac_schedule(struct stmmac_priv *priv)
 	}
 }
 
-#ifdef CONFIG_STMMAC_TIMER
+#ifdef CONFIG_STMMAC_EXT_TIMER
 void stmmac_schedule(struct net_device *dev)
 {
 	struct stmmac_priv *priv = netdev_priv(dev);
@@ -795,7 +795,7 @@ static int stmmac_open(struct net_device *dev)
 		return ret;
 	}
 
-#ifdef CONFIG_STMMAC_TIMER
+#ifdef CONFIG_STMMAC_EXT_TIMER
 	priv->tm = kzalloc(sizeof(struct stmmac_timer *), GFP_KERNEL);
 	if (unlikely(priv->tm = NULL)) {
 		pr_err("%s: ERROR: timer memory alloc failed\n", __func__);
@@ -812,7 +812,7 @@ static int stmmac_open(struct net_device *dev)
 		priv->tm->timer_start = stmmac_no_timer_started;
 		priv->tm->timer_stop = stmmac_no_timer_stopped;
 	} else
-		priv->tm->enable = 1;
+		priv->timer = external;
 #endif
 
 	/* Create and initialize the TX/RX descriptors chains. */
@@ -863,8 +863,8 @@ static int stmmac_open(struct net_device *dev)
 	priv->hw->dma->start_tx(priv->ioaddr);
 	priv->hw->dma->start_rx(priv->ioaddr);
 
-#ifdef CONFIG_STMMAC_TIMER
-	if (likely(priv->tm->enable))
+#ifdef CONFIG_STMMAC_EXT_TIMER
+	if (likely(priv->timer = external))
 		priv->tm->timer_start(priv->tm->timer_callb, tmrate);
 #endif
 	/* Dump DMA/MAC registers */
@@ -901,11 +901,13 @@ static int stmmac_release(struct net_device *dev)
 
 	netif_stop_queue(dev);
 
-#ifdef CONFIG_STMMAC_TIMER
+#ifdef CONFIG_STMMAC_EXT_TIMER
 	/* Stop and release the timer */
-	stmmac_close_ext_timer(priv->tm->timer_callb);
-	if (priv->tm != NULL)
+	if (priv->tm != NULL) {
+		if (likely(priv->timer = external))
+			stmmac_close_ext_timer(priv->tm->timer_callb);
 		kfree(priv->tm);
+	}
 #endif
 	napi_disable(&priv->napi);
 	skb_queue_purge(&priv->rx_recycle);
@@ -1096,9 +1098,9 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 	/* Interrupt on completition only for the latest segment */
 	priv->hw->desc->close_tx_desc(desc);
 
-#ifdef CONFIG_STMMAC_TIMER
-	/* Clean IC while using timer */
-	if (likely(priv->tm->enable))
+#ifdef CONFIG_STMMAC_EXT_TIMER
+	/* Clean IC while using ext timer */
+	if (likely(priv->timer = external))
 		priv->hw->desc->clear_tx_ic(desc);
 #endif
 	/* To avoid raise condition */
@@ -1815,10 +1817,11 @@ static int stmmac_suspend(struct device *dev)
 	if (priv->phydev)
 		phy_stop(priv->phydev);
 
-#ifdef CONFIG_STMMAC_TIMER
-	priv->tm->timer_stop(priv->tm->timer_callb);
-	if (likely(priv->tm->enable))
+#ifdef CONFIG_STMMAC_EXT_TIMER
+	if (likely(priv->timer = external)) {
+		priv->tm->timer_stop(priv->tm->timer_callb);
 		dis_ic = 1;
+	}
 #endif
 	napi_disable(&priv->napi);
 
@@ -1865,8 +1868,8 @@ static int stmmac_resume(struct device *dev)
 	priv->hw->dma->start_tx(priv->ioaddr);
 	priv->hw->dma->start_rx(priv->ioaddr);
 
-#ifdef CONFIG_STMMAC_TIMER
-	if (likely(priv->tm->enable))
+#ifdef CONFIG_STMMAC_EXT_TIMER
+	if (likely(priv->timer = external))
 		priv->tm->timer_start(priv->tm->timer_callb, tmrate);
 #endif
 	napi_enable(&priv->napi);
@@ -1977,7 +1980,7 @@ static int __init stmmac_cmdline_opt(char *str)
 				       (unsigned long *)&flow_ctrl);
 		else if (!strncmp(opt, "pause:", 6))
 			strict_strtoul(opt + 6, 0, (unsigned long *)&pause);
-#ifdef CONFIG_STMMAC_TIMER
+#ifdef CONFIG_STMMAC_EXT_TIMER
 		else if (!strncmp(opt, "tmrate:", 7))
 			strict_strtoul(opt + 7, 0, (unsigned long *)&tmrate);
 #endif
diff --git a/drivers/net/stmmac/stmmac_timer.c b/drivers/net/stmmac/stmmac_timer.c
index 2481daa..e64db59 100644
--- a/drivers/net/stmmac/stmmac_timer.c
+++ b/drivers/net/stmmac/stmmac_timer.c
@@ -102,7 +102,7 @@ int stmmac_close_ext_timer(void *timer)
 	return 0;
 }
 
-#elif defined(CONFIG_STMMAC_TMU_TIMER)
+#elif defined(CONFIG_STMMAC_GEN_TIMER)
 #include <linux/generictimer.h>
 
 /* Set rate and start the timer */
@@ -131,7 +131,7 @@ int stmmac_open_ext_timer(struct net_device *dev, struct stmmac_timer *tm)
 	if (timer = NULL)
 		return -1;
 
-	STMMAC_TIMER_MSG(dev->name, "sh_tmu", tm->freq);
+	STMMAC_TIMER_MSG(dev->name, "Generic", tm->freq);
 
 	tm->timer_callb = timer;
 	tm->timer_start = stmmac_tmu_set_rate;
diff --git a/drivers/net/stmmac/stmmac_timer.h b/drivers/net/stmmac/stmmac_timer.h
index 250f5cb..d719bd5 100644
--- a/drivers/net/stmmac/stmmac_timer.h
+++ b/drivers/net/stmmac/stmmac_timer.h
@@ -26,7 +26,6 @@ struct stmmac_timer {
 	void (*timer_start) (void *timer, unsigned int new_freq);
 	void (*timer_stop) (void *timer);
 	unsigned int freq;
-	unsigned int enable;
 	void *timer_callb;
 };
 
-- 
1.7.4

[PATCH (net-2.6) 4/4] stmmac: rework and improvement the stmmac timer

[Peppe CAVALLARO]

This patch prepares the stmmac to use the
embedded watchdog timer included in the new
mac generations. The STMMAC_TIMER option becomes
STMMAC_EXT_TIMER. The patch also tidies-up the code
and improvements the Kconfig information.

The driver can use the external timer and in case
of problems, i.e. while registering the timer,
it'll continue to work with NAPI and interrupts.

Currently, the STMMAC_EXT_TIMER can be turned on
SUPERH only and it depends on SH_TIMER_TMU: I mean
on platforms where I performed all my tests.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 drivers/net/stmmac/Kconfig        |   18 ++++++++----
 drivers/net/stmmac/Makefile       |    2 +-
 drivers/net/stmmac/common.h       |    6 ++++
 drivers/net/stmmac/stmmac.h       |    5 ++-
 drivers/net/stmmac/stmmac_main.c  |   51 +++++++++++++++++++-----------------
 drivers/net/stmmac/stmmac_timer.c |    4 +-
 drivers/net/stmmac/stmmac_timer.h |    1 -
 7 files changed, 51 insertions(+), 36 deletions(-)

diff --git a/drivers/net/stmmac/Kconfig b/drivers/net/stmmac/Kconfig
index b74e79b..cdbe5a8 100644
--- a/drivers/net/stmmac/Kconfig
+++ b/drivers/net/stmmac/Kconfig
@@ -29,22 +29,28 @@ config STMMAC_DUAL_MAC
 	  Ethernet Controllers. This option turns on the second Ethernet
 	  device on this kind of platforms.
 
-config STMMAC_TIMER
+config STMMAC_EXT_TIMER
 	bool "STMMAC Timer optimisation"
 	default n
 	help
 	  Use an external timer for mitigating the number of network
 	  interrupts. Currently, for SH architectures, it is possible
-	  to use the TMU channel 2 and the SH-RTC device.
+	  to use the TMU channel 2 (via Generic Timer) and the SH-RTC
+	  device. If the timer registration fails during the interface
+	  initialisation then the driver will work without any
+	  mitigation schema.
 
 choice
         prompt "Select Timer device"
-        depends on STMMAC_TIMER
+        depends on STMMAC_EXT_TIMER
 
-config STMMAC_TMU_TIMER
-        bool "TMU channel 2"
-        depends on CPU_SH4
+config STMMAC_GEN_TIMER
+        bool "Generic External Timer"
+        depends on SH_TIMER_TMU
 	help
+	  Use the Generic timer for mitigating the interrupts.
+	  For example, in case of SUPERH the TMU channel 2
+	  is used for that.
 
 config STMMAC_RTC_TIMER
         bool "Real time clock"
diff --git a/drivers/net/stmmac/Makefile b/drivers/net/stmmac/Makefile
index 9691733..05d84b2 100644
--- a/drivers/net/stmmac/Makefile
+++ b/drivers/net/stmmac/Makefile
@@ -1,5 +1,5 @@
 obj-$(CONFIG_STMMAC_ETH) += stmmac.o
-stmmac-$(CONFIG_STMMAC_TIMER) += stmmac_timer.o
+stmmac-$(CONFIG_STMMAC_EXT_TIMER) += stmmac_timer.o
 stmmac-objs:= stmmac_main.o stmmac_ethtool.o stmmac_mdio.o	\
 	      dwmac_lib.o dwmac1000_core.o  dwmac1000_dma.o	\
 	      dwmac100_core.o dwmac100_dma.o enh_desc.o  norm_desc.o $(stmmac-y)
diff --git a/drivers/net/stmmac/common.h b/drivers/net/stmmac/common.h
index 375ea19..4bd9d01 100644
--- a/drivers/net/stmmac/common.h
+++ b/drivers/net/stmmac/common.h
@@ -43,6 +43,12 @@
 #undef FRAME_FILTER_DEBUG
 /* #define FRAME_FILTER_DEBUG */
 
+enum mitigation_timer {
+	no_timer = 0,
+	external = 1,
+	embedded = 2,
+};
+
 struct stmmac_extra_stats {
 	/* Transmit errors */
 	unsigned long tx_underflow ____cacheline_aligned;
diff --git a/drivers/net/stmmac/stmmac.h b/drivers/net/stmmac/stmmac.h
index 5f06c47..1b76977 100644
--- a/drivers/net/stmmac/stmmac.h
+++ b/drivers/net/stmmac/stmmac.h
@@ -25,7 +25,7 @@
 #include <linux/stmmac.h>
 
 #include "common.h"
-#ifdef CONFIG_STMMAC_TIMER
+#ifdef CONFIG_STMMAC_EXT_TIMER
 #include "stmmac_timer.h"
 #endif
 
@@ -77,9 +77,10 @@ struct stmmac_priv {
 	spinlock_t lock;
 	int wolopts;
 	int wolenabled;
-#ifdef CONFIG_STMMAC_TIMER
+#ifdef CONFIG_STMMAC_EXT_TIMER
 	struct stmmac_timer *tm;
 #endif
+	unsigned int timer;
 #ifdef STMMAC_VLAN_TAG_USED
 	struct vlan_group *vlgrp;
 #endif
diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c
index 26714b4..53a7086 100644
--- a/drivers/net/stmmac/stmmac_main.c
+++ b/drivers/net/stmmac/stmmac_main.c
@@ -122,7 +122,7 @@ MODULE_PARM_DESC(tc, "DMA threshold control value");
 /* Pay attention to tune this parameter; take care of both
  * hardware capability and network stabitily/performance impact.
  * Many tests showed that ~4ms latency seems to be good enough. */
-#ifdef CONFIG_STMMAC_TIMER
+#ifdef CONFIG_STMMAC_EXT_TIMER
 #define DEFAULT_PERIODIC_RATE	256
 static int tmrate = DEFAULT_PERIODIC_RATE;
 module_param(tmrate, int, S_IRUGO | S_IWUSR);
@@ -415,9 +415,9 @@ static void init_dma_desc_rings(struct net_device *dev)
 	else
 		bfsize = DMA_BUFFER_SIZE;
 
-#ifdef CONFIG_STMMAC_TIMER
+#ifdef CONFIG_STMMAC_EXT_TIMER
 	/* Disable interrupts on completion for the reception if timer is on */
-	if (likely(priv->tm->enable))
+	if (likely(priv->timer == external))
 		dis_ic = 1;
 #endif
 	/* If the MTU exceeds 8k so use the second buffer in the chain */
@@ -650,8 +650,8 @@ static void stmmac_tx(struct stmmac_priv *priv)
 
 static inline void stmmac_enable_irq(struct stmmac_priv *priv)
 {
-#ifdef CONFIG_STMMAC_TIMER
-	if (likely(priv->tm->enable))
+#ifdef CONFIG_STMMAC_EXT_TIMER
+	if (likely(priv->timer == external))
 		priv->tm->timer_start(priv->tm->timer_callb, tmrate);
 	else
 #endif
@@ -660,8 +660,8 @@ static inline void stmmac_enable_irq(struct stmmac_priv *priv)
 
 static inline void stmmac_disable_irq(struct stmmac_priv *priv)
 {
-#ifdef CONFIG_STMMAC_TIMER
-	if (likely(priv->tm->enable))
+#ifdef CONFIG_STMMAC_EXT_TIMER
+	if (likely(priv->timer == external))
 		priv->tm->timer_stop(priv->tm->timer_callb);
 	else
 #endif
@@ -693,7 +693,7 @@ static inline void _stmmac_schedule(struct stmmac_priv *priv)
 	}
 }
 
-#ifdef CONFIG_STMMAC_TIMER
+#ifdef CONFIG_STMMAC_EXT_TIMER
 void stmmac_schedule(struct net_device *dev)
 {
 	struct stmmac_priv *priv = netdev_priv(dev);
@@ -795,7 +795,7 @@ static int stmmac_open(struct net_device *dev)
 		return ret;
 	}
 
-#ifdef CONFIG_STMMAC_TIMER
+#ifdef CONFIG_STMMAC_EXT_TIMER
 	priv->tm = kzalloc(sizeof(struct stmmac_timer *), GFP_KERNEL);
 	if (unlikely(priv->tm == NULL)) {
 		pr_err("%s: ERROR: timer memory alloc failed\n", __func__);
@@ -812,7 +812,7 @@ static int stmmac_open(struct net_device *dev)
 		priv->tm->timer_start = stmmac_no_timer_started;
 		priv->tm->timer_stop = stmmac_no_timer_stopped;
 	} else
-		priv->tm->enable = 1;
+		priv->timer = external;
 #endif
 
 	/* Create and initialize the TX/RX descriptors chains. */
@@ -863,8 +863,8 @@ static int stmmac_open(struct net_device *dev)
 	priv->hw->dma->start_tx(priv->ioaddr);
 	priv->hw->dma->start_rx(priv->ioaddr);
 
-#ifdef CONFIG_STMMAC_TIMER
-	if (likely(priv->tm->enable))
+#ifdef CONFIG_STMMAC_EXT_TIMER
+	if (likely(priv->timer == external))
 		priv->tm->timer_start(priv->tm->timer_callb, tmrate);
 #endif
 	/* Dump DMA/MAC registers */
@@ -901,11 +901,13 @@ static int stmmac_release(struct net_device *dev)
 
 	netif_stop_queue(dev);
 
-#ifdef CONFIG_STMMAC_TIMER
+#ifdef CONFIG_STMMAC_EXT_TIMER
 	/* Stop and release the timer */
-	stmmac_close_ext_timer(priv->tm->timer_callb);
-	if (priv->tm != NULL)
+	if (priv->tm != NULL) {
+		if (likely(priv->timer == external))
+			stmmac_close_ext_timer(priv->tm->timer_callb);
 		kfree(priv->tm);
+	}
 #endif
 	napi_disable(&priv->napi);
 	skb_queue_purge(&priv->rx_recycle);
@@ -1096,9 +1098,9 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
 	/* Interrupt on completition only for the latest segment */
 	priv->hw->desc->close_tx_desc(desc);
 
-#ifdef CONFIG_STMMAC_TIMER
-	/* Clean IC while using timer */
-	if (likely(priv->tm->enable))
+#ifdef CONFIG_STMMAC_EXT_TIMER
+	/* Clean IC while using ext timer */
+	if (likely(priv->timer == external))
 		priv->hw->desc->clear_tx_ic(desc);
 #endif
 	/* To avoid raise condition */
@@ -1815,10 +1817,11 @@ static int stmmac_suspend(struct device *dev)
 	if (priv->phydev)
 		phy_stop(priv->phydev);
 
-#ifdef CONFIG_STMMAC_TIMER
-	priv->tm->timer_stop(priv->tm->timer_callb);
-	if (likely(priv->tm->enable))
+#ifdef CONFIG_STMMAC_EXT_TIMER
+	if (likely(priv->timer == external)) {
+		priv->tm->timer_stop(priv->tm->timer_callb);
 		dis_ic = 1;
+	}
 #endif
 	napi_disable(&priv->napi);
 
@@ -1865,8 +1868,8 @@ static int stmmac_resume(struct device *dev)
 	priv->hw->dma->start_tx(priv->ioaddr);
 	priv->hw->dma->start_rx(priv->ioaddr);
 
-#ifdef CONFIG_STMMAC_TIMER
-	if (likely(priv->tm->enable))
+#ifdef CONFIG_STMMAC_EXT_TIMER
+	if (likely(priv->timer == external))
 		priv->tm->timer_start(priv->tm->timer_callb, tmrate);
 #endif
 	napi_enable(&priv->napi);
@@ -1977,7 +1980,7 @@ static int __init stmmac_cmdline_opt(char *str)
 				       (unsigned long *)&flow_ctrl);
 		else if (!strncmp(opt, "pause:", 6))
 			strict_strtoul(opt + 6, 0, (unsigned long *)&pause);
-#ifdef CONFIG_STMMAC_TIMER
+#ifdef CONFIG_STMMAC_EXT_TIMER
 		else if (!strncmp(opt, "tmrate:", 7))
 			strict_strtoul(opt + 7, 0, (unsigned long *)&tmrate);
 #endif
diff --git a/drivers/net/stmmac/stmmac_timer.c b/drivers/net/stmmac/stmmac_timer.c
index 2481daa..e64db59 100644
--- a/drivers/net/stmmac/stmmac_timer.c
+++ b/drivers/net/stmmac/stmmac_timer.c
@@ -102,7 +102,7 @@ int stmmac_close_ext_timer(void *timer)
 	return 0;
 }
 
-#elif defined(CONFIG_STMMAC_TMU_TIMER)
+#elif defined(CONFIG_STMMAC_GEN_TIMER)
 #include <linux/generictimer.h>
 
 /* Set rate and start the timer */
@@ -131,7 +131,7 @@ int stmmac_open_ext_timer(struct net_device *dev, struct stmmac_timer *tm)
 	if (timer == NULL)
 		return -1;
 
-	STMMAC_TIMER_MSG(dev->name, "sh_tmu", tm->freq);
+	STMMAC_TIMER_MSG(dev->name, "Generic", tm->freq);
 
 	tm->timer_callb = timer;
 	tm->timer_start = stmmac_tmu_set_rate;
diff --git a/drivers/net/stmmac/stmmac_timer.h b/drivers/net/stmmac/stmmac_timer.h
index 250f5cb..d719bd5 100644
--- a/drivers/net/stmmac/stmmac_timer.h
+++ b/drivers/net/stmmac/stmmac_timer.h
@@ -26,7 +26,6 @@ struct stmmac_timer {
 	void (*timer_start) (void *timer, unsigned int new_freq);
 	void (*timer_stop) (void *timer);
 	unsigned int freq;
-	unsigned int enable;
 	void *timer_callb;
 };
 
-- 
1.7.4