diff mbox series

[v2] clocksource/drivers/timer-ti-dm: fix regression from errata i940 fix

Message ID 20220204053503.1409162-1-dfustini@baylibre.com
State New
Headers show
Series [v2] clocksource/drivers/timer-ti-dm: fix regression from errata i940 fix | expand

Commit Message

Drew Fustini Feb. 4, 2022, 5:35 a.m. UTC
The existing fix for errata i940 causes a conflict for IPU2 which is
using timer 3 and 4. From arch/arm/boot/dts/dra7-ipu-dsp-common.dtsi:

  &ipu2 {
          mboxes = <&mailbox6 &mbox_ipu2_ipc3x>;
          ti,timers = <&timer3>;
          ti,watchdog-timers = <&timer4>, <&timer9>;
  };

The conflict was noticed when booting mainline on the BeagleBoard X15
which has a TI AM5728 SoC:

  remoteproc remoteproc1: 55020000.ipu is available
  remoteproc remoteproc1: powering up 55020000.ipu
  remoteproc remoteproc1: Booting fw image dra7-ipu2-fw.xem4
  omap-rproc 55020000.ipu: could not get timer platform device
  omap-rproc 55020000.ipu: omap_rproc_enable_timers failed: -19
  remoteproc remoteproc1: can't start rproc 55020000.ipu: -19

This change modifies the errata fix to instead use timer 15 and 16 which
resolves the timer conflict.

It does not appear to introduce any latency regression. Results from
cyclictest with original errata fix using dmtimer 3 and 4:

  # cyclictest --mlockall --smp --priority=80 --interval=200 --distance=0
  policy: fifo: loadavg: 0.02 0.03 0.05

  T: 0 ( 1449) P:80 I:200 C: 800368 Min:   0 Act:   32 Avg:   22 Max:  128
  T: 1 ( 1450) P:80 I:200 C: 800301 Min:   0 Act:   12 Avg:   23 Max:   70

The results after the change to dmtimer 15 and 16:

  # cyclictest --mlockall --smp --priority=80 --interval=200 --distance=0
  policy: fifo: loadavg: 0.36 0.19 0.07

  T: 0 ( 1711) P:80 I:200 C: 759599 Min:   0 Act:    6 Avg:   22 Max:  108
  T: 1 ( 1712) P:80 I:200 C: 759539 Min:   0 Act:   19 Avg:   23 Max:   79

Fixes: 25de4ce5ed02 ("clocksource/drivers/timer-ti-dm: Handle dra7 timer wrap errata i940")
Link: https://lore.kernel.org/linux-omap/YfWsG0p6to3IJuvE@x1/
Suggested-by: Suman Anna <s-anna@ti.com>
Reviewed-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Drew Fustini <dfustini@baylibre.com>
---
v2 changes:
- add cyclictest results
- use lowercase letter in hex literals

 arch/arm/boot/dts/dra7-l4.dtsi             | 5 ++---
 arch/arm/boot/dts/dra7.dtsi                | 8 ++++----
 drivers/clocksource/timer-ti-dm-systimer.c | 4 ++--
 3 files changed, 8 insertions(+), 9 deletions(-)

Comments

Suman Anna Feb. 19, 2022, 12:03 a.m. UTC | #1
Hi Tony,

On 2/3/22 23:35, Drew Fustini wrote:
> The existing fix for errata i940 causes a conflict for IPU2 which is
> using timer 3 and 4. From arch/arm/boot/dts/dra7-ipu-dsp-common.dtsi:
> 
>   &ipu2 {
>           mboxes = <&mailbox6 &mbox_ipu2_ipc3x>;
>           ti,timers = <&timer3>;
>           ti,watchdog-timers = <&timer4>, <&timer9>;
>   };
> 
> The conflict was noticed when booting mainline on the BeagleBoard X15
> which has a TI AM5728 SoC:
> 
>   remoteproc remoteproc1: 55020000.ipu is available
>   remoteproc remoteproc1: powering up 55020000.ipu
>   remoteproc remoteproc1: Booting fw image dra7-ipu2-fw.xem4
>   omap-rproc 55020000.ipu: could not get timer platform device
>   omap-rproc 55020000.ipu: omap_rproc_enable_timers failed: -19
>   remoteproc remoteproc1: can't start rproc 55020000.ipu: -19
> 
> This change modifies the errata fix to instead use timer 15 and 16 which
> resolves the timer conflict.
> 
> It does not appear to introduce any latency regression. Results from
> cyclictest with original errata fix using dmtimer 3 and 4:
> 
>   # cyclictest --mlockall --smp --priority=80 --interval=200 --distance=0
>   policy: fifo: loadavg: 0.02 0.03 0.05
> 
>   T: 0 ( 1449) P:80 I:200 C: 800368 Min:   0 Act:   32 Avg:   22 Max:  128
>   T: 1 ( 1450) P:80 I:200 C: 800301 Min:   0 Act:   12 Avg:   23 Max:   70
> 
> The results after the change to dmtimer 15 and 16:
> 
>   # cyclictest --mlockall --smp --priority=80 --interval=200 --distance=0
>   policy: fifo: loadavg: 0.36 0.19 0.07
> 
>   T: 0 ( 1711) P:80 I:200 C: 759599 Min:   0 Act:    6 Avg:   22 Max:  108
>   T: 1 ( 1712) P:80 I:200 C: 759539 Min:   0 Act:   19 Avg:   23 Max:   79
> 

Gentle reminder, I don't see this in linux-next yet, was kinda expecting this
would be included in the fixes for 5.17.

Just want to make sure that the patch did not get lost in your mbox.

regards
Suman

> Fixes: 25de4ce5ed02 ("clocksource/drivers/timer-ti-dm: Handle dra7 timer wrap errata i940")
> Link: https://lore.kernel.org/linux-omap/YfWsG0p6to3IJuvE@x1/
> Suggested-by: Suman Anna <s-anna@ti.com>
> Reviewed-by: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Drew Fustini <dfustini@baylibre.com>
> ---
> v2 changes:
> - add cyclictest results
> - use lowercase letter in hex literals
> 
>  arch/arm/boot/dts/dra7-l4.dtsi             | 5 ++---
>  arch/arm/boot/dts/dra7.dtsi                | 8 ++++----
>  drivers/clocksource/timer-ti-dm-systimer.c | 4 ++--
>  3 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/dra7-l4.dtsi b/arch/arm/boot/dts/dra7-l4.dtsi
> index 956a26d52a4c..0a11bacffc1f 100644
> --- a/arch/arm/boot/dts/dra7-l4.dtsi
> +++ b/arch/arm/boot/dts/dra7-l4.dtsi
> @@ -3482,8 +3482,7 @@ timer14: timer@0 {
>  				ti,timer-pwm;
>  			};
>  		};
> -
> -		target-module@2c000 {			/* 0x4882c000, ap 17 02.0 */
> +		timer15_target: target-module@2c000 {	/* 0x4882c000, ap 17 02.0 */
>  			compatible = "ti,sysc-omap4-timer", "ti,sysc";
>  			reg = <0x2c000 0x4>,
>  			      <0x2c010 0x4>;
> @@ -3511,7 +3510,7 @@ timer15: timer@0 {
>  			};
>  		};
>  
> -		target-module@2e000 {			/* 0x4882e000, ap 19 14.0 */
> +		timer16_target: target-module@2e000 {	/* 0x4882e000, ap 19 14.0 */
>  			compatible = "ti,sysc-omap4-timer", "ti,sysc";
>  			reg = <0x2e000 0x4>,
>  			      <0x2e010 0x4>;
> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
> index 6b485cbed8d5..8f7ffe2f66e9 100644
> --- a/arch/arm/boot/dts/dra7.dtsi
> +++ b/arch/arm/boot/dts/dra7.dtsi
> @@ -1339,20 +1339,20 @@ timer@0 {
>  };
>  
>  /* Local timers, see ARM architected timer wrap erratum i940 */
> -&timer3_target {
> +&timer15_target {
>  	ti,no-reset-on-init;
>  	ti,no-idle;
>  	timer@0 {
> -		assigned-clocks = <&l4per_clkctrl DRA7_L4PER_TIMER3_CLKCTRL 24>;
> +		assigned-clocks = <&l4per3_clkctrl DRA7_L4PER3_TIMER15_CLKCTRL 24>;
>  		assigned-clock-parents = <&timer_sys_clk_div>;
>  	};
>  };
>  
> -&timer4_target {
> +&timer16_target {
>  	ti,no-reset-on-init;
>  	ti,no-idle;
>  	timer@0 {
> -		assigned-clocks = <&l4per_clkctrl DRA7_L4PER_TIMER4_CLKCTRL 24>;
> +		assigned-clocks = <&l4per3_clkctrl DRA7_L4PER3_TIMER16_CLKCTRL 24>;
>  		assigned-clock-parents = <&timer_sys_clk_div>;
>  	};
>  };
> diff --git a/drivers/clocksource/timer-ti-dm-systimer.c b/drivers/clocksource/timer-ti-dm-systimer.c
> index b6f97960d8ee..f52bf81dc1dd 100644
> --- a/drivers/clocksource/timer-ti-dm-systimer.c
> +++ b/drivers/clocksource/timer-ti-dm-systimer.c
> @@ -695,9 +695,9 @@ static int __init dmtimer_percpu_quirk_init(struct device_node *np, u32 pa)
>  		return 0;
>  	}
>  
> -	if (pa == 0x48034000)		/* dra7 dmtimer3 */
> +	if (pa == 0x4882c000)           /* dra7 dmtimer15 */
>  		return dmtimer_percpu_timer_init(np, 0);
> -	else if (pa == 0x48036000)	/* dra7 dmtimer4 */
> +	else if (pa == 0x4882e000)      /* dra7 dmtimer16 */
>  		return dmtimer_percpu_timer_init(np, 1);
>  
>  	return 0;
Daniel Lezcano Feb. 25, 2022, 10:25 p.m. UTC | #2
On 19/02/2022 01:03, Suman Anna wrote:
> Hi Tony,
> 
> On 2/3/22 23:35, Drew Fustini wrote:
>> The existing fix for errata i940 causes a conflict for IPU2 which is
>> using timer 3 and 4. From arch/arm/boot/dts/dra7-ipu-dsp-common.dtsi:
>>
>>    &ipu2 {
>>            mboxes = <&mailbox6 &mbox_ipu2_ipc3x>;
>>            ti,timers = <&timer3>;
>>            ti,watchdog-timers = <&timer4>, <&timer9>;
>>    };
>>
>> The conflict was noticed when booting mainline on the BeagleBoard X15
>> which has a TI AM5728 SoC:
>>
>>    remoteproc remoteproc1: 55020000.ipu is available
>>    remoteproc remoteproc1: powering up 55020000.ipu
>>    remoteproc remoteproc1: Booting fw image dra7-ipu2-fw.xem4
>>    omap-rproc 55020000.ipu: could not get timer platform device
>>    omap-rproc 55020000.ipu: omap_rproc_enable_timers failed: -19
>>    remoteproc remoteproc1: can't start rproc 55020000.ipu: -19
>>
>> This change modifies the errata fix to instead use timer 15 and 16 which
>> resolves the timer conflict.
>>
>> It does not appear to introduce any latency regression. Results from
>> cyclictest with original errata fix using dmtimer 3 and 4:
>>
>>    # cyclictest --mlockall --smp --priority=80 --interval=200 --distance=0
>>    policy: fifo: loadavg: 0.02 0.03 0.05
>>
>>    T: 0 ( 1449) P:80 I:200 C: 800368 Min:   0 Act:   32 Avg:   22 Max:  128
>>    T: 1 ( 1450) P:80 I:200 C: 800301 Min:   0 Act:   12 Avg:   23 Max:   70
>>
>> The results after the change to dmtimer 15 and 16:
>>
>>    # cyclictest --mlockall --smp --priority=80 --interval=200 --distance=0
>>    policy: fifo: loadavg: 0.36 0.19 0.07
>>
>>    T: 0 ( 1711) P:80 I:200 C: 759599 Min:   0 Act:    6 Avg:   22 Max:  108
>>    T: 1 ( 1712) P:80 I:200 C: 759539 Min:   0 Act:   19 Avg:   23 Max:   79
>>
> 
> Gentle reminder, I don't see this in linux-next yet, was kinda expecting this
> would be included in the fixes for 5.17.
> 
> Just want to make sure that the patch did not get lost in your mbox.

Applied, thanks
diff mbox series

Patch

diff --git a/arch/arm/boot/dts/dra7-l4.dtsi b/arch/arm/boot/dts/dra7-l4.dtsi
index 956a26d52a4c..0a11bacffc1f 100644
--- a/arch/arm/boot/dts/dra7-l4.dtsi
+++ b/arch/arm/boot/dts/dra7-l4.dtsi
@@ -3482,8 +3482,7 @@  timer14: timer@0 {
 				ti,timer-pwm;
 			};
 		};
-
-		target-module@2c000 {			/* 0x4882c000, ap 17 02.0 */
+		timer15_target: target-module@2c000 {	/* 0x4882c000, ap 17 02.0 */
 			compatible = "ti,sysc-omap4-timer", "ti,sysc";
 			reg = <0x2c000 0x4>,
 			      <0x2c010 0x4>;
@@ -3511,7 +3510,7 @@  timer15: timer@0 {
 			};
 		};
 
-		target-module@2e000 {			/* 0x4882e000, ap 19 14.0 */
+		timer16_target: target-module@2e000 {	/* 0x4882e000, ap 19 14.0 */
 			compatible = "ti,sysc-omap4-timer", "ti,sysc";
 			reg = <0x2e000 0x4>,
 			      <0x2e010 0x4>;
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index 6b485cbed8d5..8f7ffe2f66e9 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -1339,20 +1339,20 @@  timer@0 {
 };
 
 /* Local timers, see ARM architected timer wrap erratum i940 */
-&timer3_target {
+&timer15_target {
 	ti,no-reset-on-init;
 	ti,no-idle;
 	timer@0 {
-		assigned-clocks = <&l4per_clkctrl DRA7_L4PER_TIMER3_CLKCTRL 24>;
+		assigned-clocks = <&l4per3_clkctrl DRA7_L4PER3_TIMER15_CLKCTRL 24>;
 		assigned-clock-parents = <&timer_sys_clk_div>;
 	};
 };
 
-&timer4_target {
+&timer16_target {
 	ti,no-reset-on-init;
 	ti,no-idle;
 	timer@0 {
-		assigned-clocks = <&l4per_clkctrl DRA7_L4PER_TIMER4_CLKCTRL 24>;
+		assigned-clocks = <&l4per3_clkctrl DRA7_L4PER3_TIMER16_CLKCTRL 24>;
 		assigned-clock-parents = <&timer_sys_clk_div>;
 	};
 };
diff --git a/drivers/clocksource/timer-ti-dm-systimer.c b/drivers/clocksource/timer-ti-dm-systimer.c
index b6f97960d8ee..f52bf81dc1dd 100644
--- a/drivers/clocksource/timer-ti-dm-systimer.c
+++ b/drivers/clocksource/timer-ti-dm-systimer.c
@@ -695,9 +695,9 @@  static int __init dmtimer_percpu_quirk_init(struct device_node *np, u32 pa)
 		return 0;
 	}
 
-	if (pa == 0x48034000)		/* dra7 dmtimer3 */
+	if (pa == 0x4882c000)           /* dra7 dmtimer15 */
 		return dmtimer_percpu_timer_init(np, 0);
-	else if (pa == 0x48036000)	/* dra7 dmtimer4 */
+	else if (pa == 0x4882e000)      /* dra7 dmtimer16 */
 		return dmtimer_percpu_timer_init(np, 1);
 
 	return 0;