Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1259314 > unrolled thread

[PATCH] clocksource: dw_apb_timer_of: support timer-based delay

Back to article view | Back to linux.kernel

Contents

  [PATCH] clocksource: dw_apb_timer_of: support timer-based delay Jisheng Zhang <jszhang@marvell.com> - 2015-10-30 09:40 +0100
    Re: [PATCH] clocksource: dw_apb_timer_of: support timer-based delay Jisheng Zhang <jszhang@marvell.com> - 2015-10-30 11:30 +0100
    Re: [PATCH] clocksource: dw_apb_timer_of: support timer-based delay Daniel Lezcano <daniel.lezcano@linaro.org> - 2015-10-30 11:50 +0100
      Re: [PATCH] clocksource: dw_apb_timer_of: support timer-based delay Jisheng Zhang <jszhang@marvell.com> - 2015-10-30 12:20 +0100
        Re: [PATCH] clocksource: dw_apb_timer_of: support timer-based delay Daniel Lezcano <daniel.lezcano@linaro.org> - 2015-10-30 13:40 +0100
        Re: [PATCH] clocksource: dw_apb_timer_of: support timer-based delay Arnd Bergmann <arnd@arndb.de> - 2015-10-30 13:50 +0100

#1259314 — [PATCH] clocksource: dw_apb_timer_of: support timer-based delay

Implement an ARM delay timer to be used for udelay(). This allows us to
skip the delay loop calibration at boot on Marvell BG2, BG2Q, BG2CD
platforms. And after this patch, udelay() will be unaffected by CPU
frequency changes.

Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
---
 drivers/clocksource/Kconfig           | 10 ++++++++++
 drivers/clocksource/dw_apb_timer_of.c | 16 ++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index a7726db..7b081805 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -29,6 +29,16 @@ config DW_APB_TIMER_OF
 	select DW_APB_TIMER
 	select CLKSRC_OF
 
+config DW_APB_TIMER_BASED_DELAY
+	bool "DW APB timer based delay"
+	depends on ARM && DW_APB_TIMER_OF
+	default n
+	help
+	  This option enables support for using the DW APB timer to
+	  implement timer-based delay. It is useful for skiping the
+	  delay loop calibration at boot on some platforms. And the
+	  udelay() will be unaffected by CPU frequency changes.
+
 config ROCKCHIP_TIMER
 	bool
 	select CLKSRC_OF
diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c
index a19a3f6..4bab048 100644
--- a/drivers/clocksource/dw_apb_timer_of.c
+++ b/drivers/clocksource/dw_apb_timer_of.c
@@ -16,6 +16,7 @@
  * You should have received a copy of the GNU General Public License
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
+#include <linux/delay.h>
 #include <linux/dw_apb_timer.h>
 #include <linux/of.h>
 #include <linux/of_address.h>
@@ -130,6 +131,17 @@ static void __init init_sched_clock(void)
 	sched_clock_register(read_sched_clock, 32, sched_rate);
 }
 
+#ifdef CONFIG_DW_APB_TIMER_BASED_DELAY
+static unsigned long dw_apb_delay_timer_read(void)
+{
+	return ~readl_relaxed(sched_io_base);
+}
+
+static struct delay_timer dw_apb_delay_timer = {
+	.read_current_timer = dw_apb_delay_timer_read,
+};
+#endif
+
 static int num_called;
 static void __init dw_apb_timer_init(struct device_node *timer)
 {
@@ -142,6 +154,10 @@ static void __init dw_apb_timer_init(struct device_node *timer)
 		pr_debug("%s: found clocksource timer\n", __func__);
 		add_clocksource(timer);
 		init_sched_clock();
+#ifdef CONFIG_DW_APB_TIMER_BASED_DELAY
+		dw_apb_delay_timer.freq = sched_rate;
+		register_current_timer_delay(&dw_apb_delay_timer);
+#endif
 		break;
 	default:
 		break;
-- 
2.6.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1259372

Dear Daniel,

On Fri, 30 Oct 2015 16:27:39 +0800
Jisheng Zhang <jszhang@marvell.com> wrote:

> Implement an ARM delay timer to be used for udelay(). This allows us to
> skip the delay loop calibration at boot on Marvell BG2, BG2Q, BG2CD
> platforms. And after this patch, udelay() will be unaffected by CPU
> frequency changes.

The commit msg doesn't follow "clocksource/drivers/...: Dnnn.."
But I guess I'll need to post v2, v3 ..., I'll change the msg style in v2.

Thanks,
Jisheng

> 
> Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
> ---
>  drivers/clocksource/Kconfig           | 10 ++++++++++
>  drivers/clocksource/dw_apb_timer_of.c | 16 ++++++++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index a7726db..7b081805 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -29,6 +29,16 @@ config DW_APB_TIMER_OF
>  	select DW_APB_TIMER
>  	select CLKSRC_OF
>  
> +config DW_APB_TIMER_BASED_DELAY
> +	bool "DW APB timer based delay"
> +	depends on ARM && DW_APB_TIMER_OF
> +	default n
> +	help
> +	  This option enables support for using the DW APB timer to
> +	  implement timer-based delay. It is useful for skiping the
> +	  delay loop calibration at boot on some platforms. And the
> +	  udelay() will be unaffected by CPU frequency changes.
> +
>  config ROCKCHIP_TIMER
>  	bool
>  	select CLKSRC_OF
> diff --git a/drivers/clocksource/dw_apb_timer_of.c b/drivers/clocksource/dw_apb_timer_of.c
> index a19a3f6..4bab048 100644
> --- a/drivers/clocksource/dw_apb_timer_of.c
> +++ b/drivers/clocksource/dw_apb_timer_of.c
> @@ -16,6 +16,7 @@
>   * You should have received a copy of the GNU General Public License
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
> +#include <linux/delay.h>
>  #include <linux/dw_apb_timer.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
> @@ -130,6 +131,17 @@ static void __init init_sched_clock(void)
>  	sched_clock_register(read_sched_clock, 32, sched_rate);
>  }
>  
> +#ifdef CONFIG_DW_APB_TIMER_BASED_DELAY
> +static unsigned long dw_apb_delay_timer_read(void)
> +{
> +	return ~readl_relaxed(sched_io_base);
> +}
> +
> +static struct delay_timer dw_apb_delay_timer = {
> +	.read_current_timer = dw_apb_delay_timer_read,
> +};
> +#endif
> +
>  static int num_called;
>  static void __init dw_apb_timer_init(struct device_node *timer)
>  {
> @@ -142,6 +154,10 @@ static void __init dw_apb_timer_init(struct device_node *timer)
>  		pr_debug("%s: found clocksource timer\n", __func__);
>  		add_clocksource(timer);
>  		init_sched_clock();
> +#ifdef CONFIG_DW_APB_TIMER_BASED_DELAY
> +		dw_apb_delay_timer.freq = sched_rate;
> +		register_current_timer_delay(&dw_apb_delay_timer);
> +#endif
>  		break;
>  	default:
>  		break;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1259387

On 10/30/2015 09:27 AM, Jisheng Zhang wrote:
> Implement an ARM delay timer to be used for udelay(). This allows us to
> skip the delay loop calibration at boot on Marvell BG2, BG2Q, BG2CD
> platforms. And after this patch, udelay() will be unaffected by CPU
> frequency changes.
>
> Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
> ---
>   drivers/clocksource/Kconfig           | 10 ++++++++++
>   drivers/clocksource/dw_apb_timer_of.c | 16 ++++++++++++++++
>   2 files changed, 26 insertions(+)
>
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index a7726db..7b081805 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -29,6 +29,16 @@ config DW_APB_TIMER_OF
>   	select DW_APB_TIMER
>   	select CLKSRC_OF
>
> +config DW_APB_TIMER_BASED_DELAY
> +	bool "DW APB timer based delay"
> +	depends on ARM && DW_APB_TIMER_OF
> +	default n
> +	help
> +	  This option enables support for using the DW APB timer to
> +	  implement timer-based delay. It is useful for skiping the
> +	  delay loop calibration at boot on some platforms. And the
> +	  udelay() will be unaffected by CPU frequency changes.
> +

Why do you want it to be optional ?

-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1259406

Dear Daniel,

On Fri, 30 Oct 2015 11:44:46 +0100
Daniel Lezcano <daniel.lezcano@....> wrote:

> On 10/30/2015 09:27 AM, Jisheng Zhang wrote:
> > Implement an ARM delay timer to be used for udelay(). This allows us to
> > skip the delay loop calibration at boot on Marvell BG2, BG2Q, BG2CD
> > platforms. And after this patch, udelay() will be unaffected by CPU
> > frequency changes.
> >
> > Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
> > ---
> >   drivers/clocksource/Kconfig           | 10 ++++++++++
> >   drivers/clocksource/dw_apb_timer_of.c | 16 ++++++++++++++++
> >   2 files changed, 26 insertions(+)
> >
> > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > index a7726db..7b081805 100644
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -29,6 +29,16 @@ config DW_APB_TIMER_OF
> >   	select DW_APB_TIMER
> >   	select CLKSRC_OF
> >
> > +config DW_APB_TIMER_BASED_DELAY
> > +	bool "DW APB timer based delay"
> > +	depends on ARM && DW_APB_TIMER_OF
> > +	default n
> > +	help
> > +	  This option enables support for using the DW APB timer to
> > +	  implement timer-based delay. It is useful for skiping the
> > +	  delay loop calibration at boot on some platforms. And the
> > +	  udelay() will be unaffected by CPU frequency changes.
> > +  
> 
> Why do you want it to be optional ?
> 

Because in some platforms which has arm arch timer, this dw apb timer
delay isn't needed, the arch timer is better. So we want it be optional
so that the platforms which need this feature select it manually when config
the kernel.

Thanks,
Jisheng
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1259470

On 10/30/2015 12:09 PM, Jisheng Zhang wrote:
> Dear Daniel,
>
> On Fri, 30 Oct 2015 11:44:46 +0100
> Daniel Lezcano <daniel.lezcano@....> wrote:
>
>> On 10/30/2015 09:27 AM, Jisheng Zhang wrote:
>>> Implement an ARM delay timer to be used for udelay(). This allows us to
>>> skip the delay loop calibration at boot on Marvell BG2, BG2Q, BG2CD
>>> platforms. And after this patch, udelay() will be unaffected by CPU
>>> frequency changes.
>>>
>>> Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
>>> ---
>>>    drivers/clocksource/Kconfig           | 10 ++++++++++
>>>    drivers/clocksource/dw_apb_timer_of.c | 16 ++++++++++++++++
>>>    2 files changed, 26 insertions(+)
>>>
>>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>>> index a7726db..7b081805 100644
>>> --- a/drivers/clocksource/Kconfig
>>> +++ b/drivers/clocksource/Kconfig
>>> @@ -29,6 +29,16 @@ config DW_APB_TIMER_OF
>>>    	select DW_APB_TIMER
>>>    	select CLKSRC_OF
>>>
>>> +config DW_APB_TIMER_BASED_DELAY
>>> +	bool "DW APB timer based delay"
>>> +	depends on ARM && DW_APB_TIMER_OF
>>> +	default n
>>> +	help
>>> +	  This option enables support for using the DW APB timer to
>>> +	  implement timer-based delay. It is useful for skiping the
>>> +	  delay loop calibration at boot on some platforms. And the
>>> +	  udelay() will be unaffected by CPU frequency changes.
>>> +
>>
>> Why do you want it to be optional ?
>>
>
> Because in some platforms which has arm arch timer, this dw apb timer
> delay isn't needed, the arch timer is better. So we want it be optional
> so that the platforms which need this feature select it manually when config
> the kernel.

Correct me if I am wrong. If you have the arch timer, you don't need the 
dw apb timer at all, no ? So the selection would be arch arm timer *or* 
dw_apb_timer ? not arch_arm_timer for delay and dw_apb_timer for 
clockevents, right ?


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1259479

On Friday 30 October 2015 19:09:29 Jisheng Zhang wrote:
> > > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > > index a7726db..7b081805 100644
> > > --- a/drivers/clocksource/Kconfig
> > > +++ b/drivers/clocksource/Kconfig
> > > @@ -29,6 +29,16 @@ config DW_APB_TIMER_OF
> > >     select DW_APB_TIMER
> > >     select CLKSRC_OF
> > >
> > > +config DW_APB_TIMER_BASED_DELAY
> > > +   bool "DW APB timer based delay"
> > > +   depends on ARM && DW_APB_TIMER_OF
> > > +   default n
> > > +   help
> > > +     This option enables support for using the DW APB timer to
> > > +     implement timer-based delay. It is useful for skiping the
> > > +     delay loop calibration at boot on some platforms. And the
> > > +     udelay() will be unaffected by CPU frequency changes.
> > > +  
> > 
> > Why do you want it to be optional ?
> > 
> 
> Because in some platforms which has arm arch timer, this dw apb timer
> delay isn't needed, the arch timer is better. So we want it be optional
> so that the platforms which need this feature select it manually when config
> the kernel.
> 

This is not ideal from an overall maintenance perspective. We want to
be able to have a kernel with all drivers enabled that gives us the
best behavior on all platforms.

The current behavior appears to be that we override all previous
registrations as long as the new one is higher resolution. Is that
the case here? I.e. does the arch timer have a lower resultion than
the dw-apb timer but have some other advantages?

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web