[PATCH 1/2] clocksource: imx-tpm: fix return -ETIME when delta exceeds INT_MAX

[PATCH 1/2] clocksource: imx-tpm: fix return -ETIME when delta exceeds INT_MAX

[Frank Li]

From: Jacky Bai <ping.bai@nxp.com>

In tpm_set_next_event(delta), return -ETIME by wrong cast to int when delta
is larger than INT_MAX.

For example:

tpm_set_next_event(delta = 0xffff_fffe)
{
        ...
        next = tpm_read_counter(); // assume next is 0x10
        next += delta; // next will 0xffff_fffe + 0x10 = 0x1_0000_000e
        now = tpm_read_counter();  // now is 0x10
        ...

        return (int)(next - now) <= 0 ? -ETIME : 0;
                     ^^^^^^^^^^
                     0x1_0000_000e - 0x10 = 0xffff_fffe, which is -2 when
                     cast to int. So return -ETIME.
}

To fix this, introduce a 'prev' variable and check if 'now - prev' is
larger than delta.

Cc: <stable@vger.kernel.org>
Fixes: 059ab7b82eec ("clocksource/drivers/imx-tpm: Add imx tpm timer support")
Signed-off-by: Jacky Bai <ping.bai@nxp.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Ye Li <ye.li@nxp.com>
Reviewed-by: Jason Liu <jason.hui.liu@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/clocksource/timer-imx-tpm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/clocksource/timer-imx-tpm.c b/drivers/clocksource/timer-imx-tpm.c
index bd64a8a8427f3..cd23caf1e5999 100644
--- a/drivers/clocksource/timer-imx-tpm.c
+++ b/drivers/clocksource/timer-imx-tpm.c
@@ -83,10 +83,10 @@ static u64 notrace tpm_read_sched_clock(void)
 static int tpm_set_next_event(unsigned long delta,
 				struct clock_event_device *evt)
 {
-	unsigned long next, now;
+	unsigned long next, prev, now;
 
-	next = tpm_read_counter();
-	next += delta;
+	prev = tpm_read_counter();
+	next = prev + delta;
 	writel(next, timer_base + TPM_C0V);
 	now = tpm_read_counter();
 
@@ -96,7 +96,7 @@ static int tpm_set_next_event(unsigned long delta,
 	 * of writing CNT registers which may cause the min_delta event got
 	 * missed, so we need add a ETIME check here in case it happened.
 	 */
-	return (int)(next - now) <= 0 ? -ETIME : 0;
+	return (now - prev) >= delta ? -ETIME : 0;
 }
 
 static int tpm_set_state_oneshot(struct clock_event_device *evt)
-- 
2.34.1

[PATCH 2/2] clocksource: imx-tpm: fix next event not taking effect sometime

[Frank Li]

From: Jacky Bai <ping.bai@nxp.com>

The value written into the TPM CnV can only be updated into the hardware
when the counter increases. Additional writes to the CnV write buffer are
ignored until the register has been updated. Therefore, we need to check
if the CnV has been updated before continuing. This may require waiting for
1 counter cycle in the worst case.

Cc: <stable@vger.kernel.org>
Fixes: 059ab7b82eec ("clocksource/drivers/imx-tpm: Add imx tpm timer support")
Signed-off-by: Jacky Bai <ping.bai@nxp.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Ye Li <ye.li@nxp.com>
Reviewed-by: Jason Liu <jason.hui.liu@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/clocksource/timer-imx-tpm.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/clocksource/timer-imx-tpm.c b/drivers/clocksource/timer-imx-tpm.c
index cd23caf1e5999..92c025b70eb62 100644
--- a/drivers/clocksource/timer-imx-tpm.c
+++ b/drivers/clocksource/timer-imx-tpm.c
@@ -90,6 +90,14 @@ static int tpm_set_next_event(unsigned long delta,
 	writel(next, timer_base + TPM_C0V);
 	now = tpm_read_counter();
 
+	/*
+	 * Need to wait CNT increase at least 1 cycle to make sure
+	 * the C0V has been updated into HW.
+	 */
+	if ((next & 0xffffffff) != readl(timer_base + TPM_C0V))
+		while (now == tpm_read_counter())
+			;
+
 	/*
 	 * NOTE: We observed in a very small probability, the bus fabric
 	 * contention between GPU and A7 may results a few cycles delay
-- 
2.34.1

[tip: timers/urgent] clocksource/drivers/imx-tpm: Fix next event not taking effect sometime

[tip-bot2 for Jacky Bai]

The following commit has been merged into the timers/urgent branch of tip:

Commit-ID:     3d5c2f8e75a55cfb11a85086c71996af0354a1fb
Gitweb:        https://git.kernel.org/tip/3d5c2f8e75a55cfb11a85086c71996af0354a1fb
Author:        Jacky Bai <ping.bai@nxp.com>
AuthorDate:    Thu, 25 Jul 2024 15:33:55 -04:00
Committer:     Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Mon, 02 Sep 2024 10:04:15 +02:00

clocksource/drivers/imx-tpm: Fix next event not taking effect sometime

The value written into the TPM CnV can only be updated into the hardware
when the counter increases. Additional writes to the CnV write buffer are
ignored until the register has been updated. Therefore, we need to check
if the CnV has been updated before continuing. This may require waiting for
1 counter cycle in the worst case.

Cc: stable@vger.kernel.org
Fixes: 059ab7b82eec ("clocksource/drivers/imx-tpm: Add imx tpm timer support")
Signed-off-by: Jacky Bai <ping.bai@nxp.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Ye Li <ye.li@nxp.com>
Reviewed-by: Jason Liu <jason.hui.liu@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Link: https://lore.kernel.org/r/20240725193355.1436005-2-Frank.Li@nxp.com
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/timer-imx-tpm.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/clocksource/timer-imx-tpm.c b/drivers/clocksource/timer-imx-tpm.c
index cd23caf..92c025b 100644
--- a/drivers/clocksource/timer-imx-tpm.c
+++ b/drivers/clocksource/timer-imx-tpm.c
@@ -91,6 +91,14 @@ static int tpm_set_next_event(unsigned long delta,
 	now = tpm_read_counter();
 
 	/*
+	 * Need to wait CNT increase at least 1 cycle to make sure
+	 * the C0V has been updated into HW.
+	 */
+	if ((next & 0xffffffff) != readl(timer_base + TPM_C0V))
+		while (now == tpm_read_counter())
+			;
+
+	/*
 	 * NOTE: We observed in a very small probability, the bus fabric
 	 * contention between GPU and A7 may results a few cycles delay
 	 * of writing CNT registers which may cause the min_delta event got

Re: [PATCH 1/2] clocksource: imx-tpm: fix return -ETIME when delta exceeds INT_MAX

[Frank Li]

On Thu, Jul 25, 2024 at 03:33:54PM -0400, Frank Li wrote:
> From: Jacky Bai <ping.bai@nxp.com>
>
> In tpm_set_next_event(delta), return -ETIME by wrong cast to int when delta
> is larger than INT_MAX.
>
> For example:
>
> tpm_set_next_event(delta = 0xffff_fffe)
> {
>         ...
>         next = tpm_read_counter(); // assume next is 0x10
>         next += delta; // next will 0xffff_fffe + 0x10 = 0x1_0000_000e
>         now = tpm_read_counter();  // now is 0x10
>         ...
>
>         return (int)(next - now) <= 0 ? -ETIME : 0;
>                      ^^^^^^^^^^
>                      0x1_0000_000e - 0x10 = 0xffff_fffe, which is -2 when
>                      cast to int. So return -ETIME.
> }
>
> To fix this, introduce a 'prev' variable and check if 'now - prev' is
> larger than delta.
>
> Cc: <stable@vger.kernel.org>
> Fixes: 059ab7b82eec ("clocksource/drivers/imx-tpm: Add imx tpm timer support")
> Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> Reviewed-by: Ye Li <ye.li@nxp.com>
> Reviewed-by: Jason Liu <jason.hui.liu@nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---

Ping?


>  drivers/clocksource/timer-imx-tpm.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clocksource/timer-imx-tpm.c b/drivers/clocksource/timer-imx-tpm.c
> index bd64a8a8427f3..cd23caf1e5999 100644
> --- a/drivers/clocksource/timer-imx-tpm.c
> +++ b/drivers/clocksource/timer-imx-tpm.c
> @@ -83,10 +83,10 @@ static u64 notrace tpm_read_sched_clock(void)
>  static int tpm_set_next_event(unsigned long delta,
>  				struct clock_event_device *evt)
>  {
> -	unsigned long next, now;
> +	unsigned long next, prev, now;
>
> -	next = tpm_read_counter();
> -	next += delta;
> +	prev = tpm_read_counter();
> +	next = prev + delta;
>  	writel(next, timer_base + TPM_C0V);
>  	now = tpm_read_counter();
>
> @@ -96,7 +96,7 @@ static int tpm_set_next_event(unsigned long delta,
>  	 * of writing CNT registers which may cause the min_delta event got
>  	 * missed, so we need add a ETIME check here in case it happened.
>  	 */
> -	return (int)(next - now) <= 0 ? -ETIME : 0;
> +	return (now - prev) >= delta ? -ETIME : 0;
>  }
>
>  static int tpm_set_state_oneshot(struct clock_event_device *evt)
> --
> 2.34.1
>

Re: [PATCH 1/2] clocksource: imx-tpm: fix return -ETIME when delta exceeds INT_MAX

[Daniel Lezcano]

On 25/07/2024 21:33, Frank Li wrote:
> From: Jacky Bai <ping.bai@nxp.com>
> 
> In tpm_set_next_event(delta), return -ETIME by wrong cast to int when delta
> is larger than INT_MAX.

Is it something you observed or from your understanding of the code ?

> For example:
> 
> tpm_set_next_event(delta = 0xffff_fffe)
> {
>          ...
>          next = tpm_read_counter(); // assume next is 0x10
>          next += delta; // next will 0xffff_fffe + 0x10 = 0x1_0000_000e
>          now = tpm_read_counter();  // now is 0x10
>          ...
> 
>          return (int)(next - now) <= 0 ? -ETIME : 0;
>                       ^^^^^^^^^^
>                       0x1_0000_000e - 0x10 = 0xffff_fffe, which is -2 when
>                       cast to int. So return -ETIME.
> }
> 
> To fix this, introduce a 'prev' variable and check if 'now - prev' is
> larger than delta.
> 
> Cc: <stable@vger.kernel.org>
> Fixes: 059ab7b82eec ("clocksource/drivers/imx-tpm: Add imx tpm timer support")
> Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> Reviewed-by: Ye Li <ye.li@nxp.com>
> Reviewed-by: Jason Liu <jason.hui.liu@nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>   drivers/clocksource/timer-imx-tpm.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clocksource/timer-imx-tpm.c b/drivers/clocksource/timer-imx-tpm.c
> index bd64a8a8427f3..cd23caf1e5999 100644
> --- a/drivers/clocksource/timer-imx-tpm.c
> +++ b/drivers/clocksource/timer-imx-tpm.c
> @@ -83,10 +83,10 @@ static u64 notrace tpm_read_sched_clock(void)
>   static int tpm_set_next_event(unsigned long delta,
>   				struct clock_event_device *evt)
>   {
> -	unsigned long next, now;
> +	unsigned long next, prev, now;
>   
> -	next = tpm_read_counter();
> -	next += delta;
> +	prev = tpm_read_counter();
> +	next = prev + delta;
>   	writel(next, timer_base + TPM_C0V);
>   	now = tpm_read_counter();
>   
> @@ -96,7 +96,7 @@ static int tpm_set_next_event(unsigned long delta,
>   	 * of writing CNT registers which may cause the min_delta event got
>   	 * missed, so we need add a ETIME check here in case it happened.
>   	 */
> -	return (int)(next - now) <= 0 ? -ETIME : 0;
> +	return (now - prev) >= delta ? -ETIME : 0;
>   }
>   
>   static int tpm_set_state_oneshot(struct clock_event_device *evt)


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

Re: [PATCH 1/2] clocksource: imx-tpm: fix return -ETIME when delta exceeds INT_MAX

[Frank Li]

On Mon, Aug 19, 2024 at 03:21:36PM +0200, Daniel Lezcano wrote:
> On 25/07/2024 21:33, Frank Li wrote:
> > From: Jacky Bai <ping.bai@nxp.com>
> >
> > In tpm_set_next_event(delta), return -ETIME by wrong cast to int when delta
> > is larger than INT_MAX.
>
> Is it something you observed or from your understanding of the code ?

System use this timer to wakeup from lower idle, which core and local timer
turn off. -ETIME may prevent system enter lower idle, or failure wakeup
from lower idle utill counter run overflow. I am not sure how to trigger
this problem.

Assume run at 24Mhz, 1/24us * 0x8000_0000 is around 89s. Problem trigger
only next event bigger 89s, which relative quite big number.

Anyways, the original code is wrong, it claim max delta is 32bit.

Jack bai: any additonal supplement for this?

Frank

>
> > For example:
> >
> > tpm_set_next_event(delta = 0xffff_fffe)
> > {
> >          ...
> >          next = tpm_read_counter(); // assume next is 0x10
> >          next += delta; // next will 0xffff_fffe + 0x10 = 0x1_0000_000e
> >          now = tpm_read_counter();  // now is 0x10
> >          ...
> >
> >          return (int)(next - now) <= 0 ? -ETIME : 0;
> >                       ^^^^^^^^^^
> >                       0x1_0000_000e - 0x10 = 0xffff_fffe, which is -2 when
> >                       cast to int. So return -ETIME.
> > }
> >
> > To fix this, introduce a 'prev' variable and check if 'now - prev' is
> > larger than delta.
> >
> > Cc: <stable@vger.kernel.org>
> > Fixes: 059ab7b82eec ("clocksource/drivers/imx-tpm: Add imx tpm timer support")
> > Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> > Reviewed-by: Peng Fan <peng.fan@nxp.com>
> > Reviewed-by: Ye Li <ye.li@nxp.com>
> > Reviewed-by: Jason Liu <jason.hui.liu@nxp.com>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >   drivers/clocksource/timer-imx-tpm.c | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/clocksource/timer-imx-tpm.c b/drivers/clocksource/timer-imx-tpm.c
> > index bd64a8a8427f3..cd23caf1e5999 100644
> > --- a/drivers/clocksource/timer-imx-tpm.c
> > +++ b/drivers/clocksource/timer-imx-tpm.c
> > @@ -83,10 +83,10 @@ static u64 notrace tpm_read_sched_clock(void)
> >   static int tpm_set_next_event(unsigned long delta,
> >   				struct clock_event_device *evt)
> >   {
> > -	unsigned long next, now;
> > +	unsigned long next, prev, now;
> > -	next = tpm_read_counter();
> > -	next += delta;
> > +	prev = tpm_read_counter();
> > +	next = prev + delta;
> >   	writel(next, timer_base + TPM_C0V);
> >   	now = tpm_read_counter();
> > @@ -96,7 +96,7 @@ static int tpm_set_next_event(unsigned long delta,
> >   	 * of writing CNT registers which may cause the min_delta event got
> >   	 * missed, so we need add a ETIME check here in case it happened.
> >   	 */
> > -	return (int)(next - now) <= 0 ? -ETIME : 0;
> > +	return (now - prev) >= delta ? -ETIME : 0;
> >   }
> >   static int tpm_set_state_oneshot(struct clock_event_device *evt)
>
>
> --
> <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

RE: [PATCH 1/2] clocksource: imx-tpm: fix return -ETIME when delta exceeds INT_MAX

[Jacky Bai (OSS)]

> Subject: Re: [PATCH 1/2] clocksource: imx-tpm: fix return -ETIME when delta
> exceeds INT_MAX
> 
> On Mon, Aug 19, 2024 at 03:21:36PM +0200, Daniel Lezcano wrote:
> > On 25/07/2024 21:33, Frank Li wrote:
> > > From: Jacky Bai <ping.bai@nxp.com>
> > >
> > > In tpm_set_next_event(delta), return -ETIME by wrong cast to int
> > > when delta is larger than INT_MAX.
> >
> > Is it something you observed or from your understanding of the code ?
> 
> System use this timer to wakeup from lower idle, which core and local timer turn
> off. -ETIME may prevent system enter lower idle, or failure wakeup from lower
> idle utill counter run overflow. I am not sure how to trigger this problem.
> 
> Assume run at 24Mhz, 1/24us * 0x8000_0000 is around 89s. Problem trigger
> only next event bigger 89s, which relative quite big number.
> 
> Anyways, the original code is wrong, it claim max delta is 32bit.
> 
> Jack bai: any additonal supplement for this?
>

The issue can be triggered in below function, it is possible max_delta will be assigned to delta,
So the original int case logic will report wrong '-ETIME' error, then 'clockevents_program_min_delta'
Will be called to reprogram the timer, due to the tpm limitation mentioned in patch 2/2, this min_delta
program will not take effect.

int clockevents_program_event(struct clock_event_device *dev, ktime_t expires,
                        bool force) {
	...
	delta = min(delta, (int64_t) dev->max_delta_ns);
	delta = max(delta, (int64_t) dev->min_delta_ns);

	clc = ((unsigned long long) delta * dev->mult) >> dev->shift;
	rc = dev->set_next_event((unsigned long) clc, dev);

	return (rc && force) ? clockevents_program_min_delta(dev) : rc;
}

BR
> Frank
> 
> >
> > > For example:
> > >
> > > tpm_set_next_event(delta = 0xffff_fffe) {
> > >          ...
> > >          next = tpm_read_counter(); // assume next is 0x10
> > >          next += delta; // next will 0xffff_fffe + 0x10 = 0x1_0000_000e
> > >          now = tpm_read_counter();  // now is 0x10
> > >          ...
> > >
> > >          return (int)(next - now) <= 0 ? -ETIME : 0;
> > >                       ^^^^^^^^^^
> > >                       0x1_0000_000e - 0x10 = 0xffff_fffe, which is -2
> when
> > >                       cast to int. So return -ETIME.
> > > }
> > >
> > > To fix this, introduce a 'prev' variable and check if 'now - prev'
> > > is larger than delta.
> > >
> > > Cc: <stable@vger.kernel.org>
> > > Fixes: 059ab7b82eec ("clocksource/drivers/imx-tpm: Add imx tpm timer
> > > support")
> > > Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> > > Reviewed-by: Peng Fan <peng.fan@nxp.com>
> > > Reviewed-by: Ye Li <ye.li@nxp.com>
> > > Reviewed-by: Jason Liu <jason.hui.liu@nxp.com>
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > >   drivers/clocksource/timer-imx-tpm.c | 8 ++++----
> > >   1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/clocksource/timer-imx-tpm.c
> > > b/drivers/clocksource/timer-imx-tpm.c
> > > index bd64a8a8427f3..cd23caf1e5999 100644
> > > --- a/drivers/clocksource/timer-imx-tpm.c
> > > +++ b/drivers/clocksource/timer-imx-tpm.c
> > > @@ -83,10 +83,10 @@ static u64 notrace tpm_read_sched_clock(void)
> > >   static int tpm_set_next_event(unsigned long delta,
> > >   				struct clock_event_device *evt)
> > >   {
> > > -	unsigned long next, now;
> > > +	unsigned long next, prev, now;
> > > -	next = tpm_read_counter();
> > > -	next += delta;
> > > +	prev = tpm_read_counter();
> > > +	next = prev + delta;
> > >   	writel(next, timer_base + TPM_C0V);
> > >   	now = tpm_read_counter();
> > > @@ -96,7 +96,7 @@ static int tpm_set_next_event(unsigned long delta,
> > >   	 * of writing CNT registers which may cause the min_delta event got
> > >   	 * missed, so we need add a ETIME check here in case it happened.
> > >   	 */
> > > -	return (int)(next - now) <= 0 ? -ETIME : 0;
> > > +	return (now - prev) >= delta ? -ETIME : 0;
> > >   }
> > >   static int tpm_set_state_oneshot(struct clock_event_device *evt)
> >
> >
> > --
> > <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

Re: [PATCH 1/2] clocksource: imx-tpm: fix return -ETIME when delta exceeds INT_MAX

[Daniel Lezcano]

On 25/07/2024 21:33, Frank Li wrote:
> From: Jacky Bai <ping.bai@nxp.com>
> 
> In tpm_set_next_event(delta), return -ETIME by wrong cast to int when delta
> is larger than INT_MAX.
> 
> For example:
> 
> tpm_set_next_event(delta = 0xffff_fffe)
> {
>          ...
>          next = tpm_read_counter(); // assume next is 0x10
>          next += delta; // next will 0xffff_fffe + 0x10 = 0x1_0000_000e
>          now = tpm_read_counter();  // now is 0x10
>          ...
> 
>          return (int)(next - now) <= 0 ? -ETIME : 0;
>                       ^^^^^^^^^^
>                       0x1_0000_000e - 0x10 = 0xffff_fffe, which is -2 when
>                       cast to int. So return -ETIME.
> }
> 
> To fix this, introduce a 'prev' variable and check if 'now - prev' is
> larger than delta.
> 
> Cc: <stable@vger.kernel.org>
> Fixes: 059ab7b82eec ("clocksource/drivers/imx-tpm: Add imx tpm timer support")
> Signed-off-by: Jacky Bai <ping.bai@nxp.com>
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> Reviewed-by: Ye Li <ye.li@nxp.com>
> Reviewed-by: Jason Liu <jason.hui.liu@nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---

Applied, thanks


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

[tip: timers/urgent] clocksource/drivers/imx-tpm: Fix return -ETIME when delta exceeds INT_MAX

[tip-bot2 for Jacky Bai]

The following commit has been merged into the timers/urgent branch of tip:

Commit-ID:     5b8843fcd49827813da80c0f590a17ae4ce93c5d
Gitweb:        https://git.kernel.org/tip/5b8843fcd49827813da80c0f590a17ae4ce93c5d
Author:        Jacky Bai <ping.bai@nxp.com>
AuthorDate:    Thu, 25 Jul 2024 15:33:54 -04:00
Committer:     Daniel Lezcano <daniel.lezcano@linaro.org>
CommitterDate: Mon, 02 Sep 2024 10:04:15 +02:00

clocksource/drivers/imx-tpm: Fix return -ETIME when delta exceeds INT_MAX

In tpm_set_next_event(delta), return -ETIME by wrong cast to int when delta
is larger than INT_MAX.

For example:

tpm_set_next_event(delta = 0xffff_fffe)
{
        ...
        next = tpm_read_counter(); // assume next is 0x10
        next += delta; // next will 0xffff_fffe + 0x10 = 0x1_0000_000e
        now = tpm_read_counter();  // now is 0x10
        ...

        return (int)(next - now) <= 0 ? -ETIME : 0;
                     ^^^^^^^^^^
                     0x1_0000_000e - 0x10 = 0xffff_fffe, which is -2 when
                     cast to int. So return -ETIME.
}

To fix this, introduce a 'prev' variable and check if 'now - prev' is
larger than delta.

Cc: stable@vger.kernel.org
Fixes: 059ab7b82eec ("clocksource/drivers/imx-tpm: Add imx tpm timer support")
Signed-off-by: Jacky Bai <ping.bai@nxp.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Ye Li <ye.li@nxp.com>
Reviewed-by: Jason Liu <jason.hui.liu@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Link: https://lore.kernel.org/r/20240725193355.1436005-1-Frank.Li@nxp.com
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/clocksource/timer-imx-tpm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/clocksource/timer-imx-tpm.c b/drivers/clocksource/timer-imx-tpm.c
index bd64a8a..cd23caf 100644
--- a/drivers/clocksource/timer-imx-tpm.c
+++ b/drivers/clocksource/timer-imx-tpm.c
@@ -83,10 +83,10 @@ static u64 notrace tpm_read_sched_clock(void)
 static int tpm_set_next_event(unsigned long delta,
 				struct clock_event_device *evt)
 {
-	unsigned long next, now;
+	unsigned long next, prev, now;
 
-	next = tpm_read_counter();
-	next += delta;
+	prev = tpm_read_counter();
+	next = prev + delta;
 	writel(next, timer_base + TPM_C0V);
 	now = tpm_read_counter();
 
@@ -96,7 +96,7 @@ static int tpm_set_next_event(unsigned long delta,
 	 * of writing CNT registers which may cause the min_delta event got
 	 * missed, so we need add a ETIME check here in case it happened.
 	 */
-	return (int)(next - now) <= 0 ? -ETIME : 0;
+	return (now - prev) >= delta ? -ETIME : 0;
 }
 
 static int tpm_set_state_oneshot(struct clock_event_device *evt)