All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] sbsa_gwdt: Calculate timeout with 64-bit math
@ 2023-09-21  9:02 ` Darren Hart
  0 siblings, 0 replies; 18+ messages in thread
From: Darren Hart @ 2023-09-21  9:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Vanshidhar Konda, Wim Van Sebroeck, Guenter Roeck,
	linux-watchdog, linux-arm-kernel, stable

Commit abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
introduced new timer math for watchdog revision 1 with the 48 bit offset
register.

The gwdt->clk and timeout are u32, but the argument being calculated is
u64. Without a cast, the compiler performs u32 operations, truncating
intermediate steps, resulting in incorrect values.

A watchdog revision 1 implementation with a gwdt->clk of 1GHz and a
timeout of 600s writes 3647256576 to the one shot watchdog instead of
300000000000, resulting in the watchdog firing in 3.6s instead of 600s.

Force u64 math by casting the first argument (gwdt->clk) as a u64. Make
the order of operations explicit with parenthesis.

Fixes: abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
Reported-by: Vanshidhar Konda <vanshikonda@os.amperecomputing.com>
Signed-off-by: Darren Hart <darren@os.amperecomputing.com>
Cc: Wim Van Sebroeck <wim@linux-watchdog.org>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: linux-watchdog@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: <stable@vger.kernel.org> # 5.14.x
---
 drivers/watchdog/sbsa_gwdt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c
index fd3cfdda4949..76527324b63c 100644
--- a/drivers/watchdog/sbsa_gwdt.c
+++ b/drivers/watchdog/sbsa_gwdt.c
@@ -153,14 +153,14 @@ static int sbsa_gwdt_set_timeout(struct watchdog_device *wdd,
 	timeout = clamp_t(unsigned int, timeout, 1, wdd->max_hw_heartbeat_ms / 1000);
 
 	if (action)
-		sbsa_gwdt_reg_write(gwdt->clk * timeout, gwdt);
+		sbsa_gwdt_reg_write((u64)gwdt->clk * timeout, gwdt);
 	else
 		/*
 		 * In the single stage mode, The first signal (WS0) is ignored,
 		 * the timeout is (WOR * 2), so the WOR should be configured
 		 * to half value of timeout.
 		 */
-		sbsa_gwdt_reg_write(gwdt->clk / 2 * timeout, gwdt);
+		sbsa_gwdt_reg_write(((u64)gwdt->clk / 2) * timeout, gwdt);
 
 	return 0;
 }
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH] sbsa_gwdt: Calculate timeout with 64-bit math
@ 2023-09-21  9:02 ` Darren Hart
  0 siblings, 0 replies; 18+ messages in thread
From: Darren Hart @ 2023-09-21  9:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: Vanshidhar Konda, Wim Van Sebroeck, Guenter Roeck,
	linux-watchdog, linux-arm-kernel, stable

Commit abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
introduced new timer math for watchdog revision 1 with the 48 bit offset
register.

The gwdt->clk and timeout are u32, but the argument being calculated is
u64. Without a cast, the compiler performs u32 operations, truncating
intermediate steps, resulting in incorrect values.

A watchdog revision 1 implementation with a gwdt->clk of 1GHz and a
timeout of 600s writes 3647256576 to the one shot watchdog instead of
300000000000, resulting in the watchdog firing in 3.6s instead of 600s.

Force u64 math by casting the first argument (gwdt->clk) as a u64. Make
the order of operations explicit with parenthesis.

Fixes: abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
Reported-by: Vanshidhar Konda <vanshikonda@os.amperecomputing.com>
Signed-off-by: Darren Hart <darren@os.amperecomputing.com>
Cc: Wim Van Sebroeck <wim@linux-watchdog.org>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: linux-watchdog@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: <stable@vger.kernel.org> # 5.14.x
---
 drivers/watchdog/sbsa_gwdt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c
index fd3cfdda4949..76527324b63c 100644
--- a/drivers/watchdog/sbsa_gwdt.c
+++ b/drivers/watchdog/sbsa_gwdt.c
@@ -153,14 +153,14 @@ static int sbsa_gwdt_set_timeout(struct watchdog_device *wdd,
 	timeout = clamp_t(unsigned int, timeout, 1, wdd->max_hw_heartbeat_ms / 1000);
 
 	if (action)
-		sbsa_gwdt_reg_write(gwdt->clk * timeout, gwdt);
+		sbsa_gwdt_reg_write((u64)gwdt->clk * timeout, gwdt);
 	else
 		/*
 		 * In the single stage mode, The first signal (WS0) is ignored,
 		 * the timeout is (WOR * 2), so the WOR should be configured
 		 * to half value of timeout.
 		 */
-		sbsa_gwdt_reg_write(gwdt->clk / 2 * timeout, gwdt);
+		sbsa_gwdt_reg_write(((u64)gwdt->clk / 2) * timeout, gwdt);
 
 	return 0;
 }
-- 
2.41.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] sbsa_gwdt: Calculate timeout with 64-bit math
  2023-09-21  9:02 ` Darren Hart
@ 2023-09-26 12:45   ` Guenter Roeck
  -1 siblings, 0 replies; 18+ messages in thread
From: Guenter Roeck @ 2023-09-26 12:45 UTC (permalink / raw)
  To: Darren Hart
  Cc: linux-kernel, Vanshidhar Konda, Wim Van Sebroeck, linux-watchdog,
	linux-arm-kernel, stable

On Thu, Sep 21, 2023 at 02:02:36AM -0700, Darren Hart wrote:
> Commit abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> introduced new timer math for watchdog revision 1 with the 48 bit offset
> register.
> 
> The gwdt->clk and timeout are u32, but the argument being calculated is
> u64. Without a cast, the compiler performs u32 operations, truncating
> intermediate steps, resulting in incorrect values.
> 
> A watchdog revision 1 implementation with a gwdt->clk of 1GHz and a
> timeout of 600s writes 3647256576 to the one shot watchdog instead of
> 300000000000, resulting in the watchdog firing in 3.6s instead of 600s.
> 
> Force u64 math by casting the first argument (gwdt->clk) as a u64. Make
> the order of operations explicit with parenthesis.
> 
> Fixes: abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> Reported-by: Vanshidhar Konda <vanshikonda@os.amperecomputing.com>
> Signed-off-by: Darren Hart <darren@os.amperecomputing.com>
> Cc: Wim Van Sebroeck <wim@linux-watchdog.org>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: linux-watchdog@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: <stable@vger.kernel.org> # 5.14.x

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/sbsa_gwdt.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c
> index fd3cfdda4949..76527324b63c 100644
> --- a/drivers/watchdog/sbsa_gwdt.c
> +++ b/drivers/watchdog/sbsa_gwdt.c
> @@ -153,14 +153,14 @@ static int sbsa_gwdt_set_timeout(struct watchdog_device *wdd,
>  	timeout = clamp_t(unsigned int, timeout, 1, wdd->max_hw_heartbeat_ms / 1000);
>  
>  	if (action)
> -		sbsa_gwdt_reg_write(gwdt->clk * timeout, gwdt);
> +		sbsa_gwdt_reg_write((u64)gwdt->clk * timeout, gwdt);
>  	else
>  		/*
>  		 * In the single stage mode, The first signal (WS0) is ignored,
>  		 * the timeout is (WOR * 2), so the WOR should be configured
>  		 * to half value of timeout.
>  		 */
> -		sbsa_gwdt_reg_write(gwdt->clk / 2 * timeout, gwdt);
> +		sbsa_gwdt_reg_write(((u64)gwdt->clk / 2) * timeout, gwdt);
>  
>  	return 0;
>  }
> -- 
> 2.41.0
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] sbsa_gwdt: Calculate timeout with 64-bit math
@ 2023-09-26 12:45   ` Guenter Roeck
  0 siblings, 0 replies; 18+ messages in thread
From: Guenter Roeck @ 2023-09-26 12:45 UTC (permalink / raw)
  To: Darren Hart
  Cc: linux-kernel, Vanshidhar Konda, Wim Van Sebroeck, linux-watchdog,
	linux-arm-kernel, stable

On Thu, Sep 21, 2023 at 02:02:36AM -0700, Darren Hart wrote:
> Commit abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> introduced new timer math for watchdog revision 1 with the 48 bit offset
> register.
> 
> The gwdt->clk and timeout are u32, but the argument being calculated is
> u64. Without a cast, the compiler performs u32 operations, truncating
> intermediate steps, resulting in incorrect values.
> 
> A watchdog revision 1 implementation with a gwdt->clk of 1GHz and a
> timeout of 600s writes 3647256576 to the one shot watchdog instead of
> 300000000000, resulting in the watchdog firing in 3.6s instead of 600s.
> 
> Force u64 math by casting the first argument (gwdt->clk) as a u64. Make
> the order of operations explicit with parenthesis.
> 
> Fixes: abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> Reported-by: Vanshidhar Konda <vanshikonda@os.amperecomputing.com>
> Signed-off-by: Darren Hart <darren@os.amperecomputing.com>
> Cc: Wim Van Sebroeck <wim@linux-watchdog.org>
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: linux-watchdog@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: <stable@vger.kernel.org> # 5.14.x

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/sbsa_gwdt.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c
> index fd3cfdda4949..76527324b63c 100644
> --- a/drivers/watchdog/sbsa_gwdt.c
> +++ b/drivers/watchdog/sbsa_gwdt.c
> @@ -153,14 +153,14 @@ static int sbsa_gwdt_set_timeout(struct watchdog_device *wdd,
>  	timeout = clamp_t(unsigned int, timeout, 1, wdd->max_hw_heartbeat_ms / 1000);
>  
>  	if (action)
> -		sbsa_gwdt_reg_write(gwdt->clk * timeout, gwdt);
> +		sbsa_gwdt_reg_write((u64)gwdt->clk * timeout, gwdt);
>  	else
>  		/*
>  		 * In the single stage mode, The first signal (WS0) is ignored,
>  		 * the timeout is (WOR * 2), so the WOR should be configured
>  		 * to half value of timeout.
>  		 */
> -		sbsa_gwdt_reg_write(gwdt->clk / 2 * timeout, gwdt);
> +		sbsa_gwdt_reg_write(((u64)gwdt->clk / 2) * timeout, gwdt);
>  
>  	return 0;
>  }
> -- 
> 2.41.0
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] sbsa_gwdt: Calculate timeout with 64-bit math
  2023-09-26 12:45   ` Guenter Roeck
@ 2023-10-14  9:12     ` Darren Hart
  -1 siblings, 0 replies; 18+ messages in thread
From: Darren Hart @ 2023-10-14  9:12 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-kernel, Vanshidhar Konda, Wim Van Sebroeck, linux-watchdog,
	linux-arm-kernel, stable

On Tue, Sep 26, 2023 at 05:45:13AM -0700, Guenter Roeck wrote:
> On Thu, Sep 21, 2023 at 02:02:36AM -0700, Darren Hart wrote:
> > Commit abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> > introduced new timer math for watchdog revision 1 with the 48 bit offset
> > register.
> > 
> > The gwdt->clk and timeout are u32, but the argument being calculated is
> > u64. Without a cast, the compiler performs u32 operations, truncating
> > intermediate steps, resulting in incorrect values.
> > 
> > A watchdog revision 1 implementation with a gwdt->clk of 1GHz and a
> > timeout of 600s writes 3647256576 to the one shot watchdog instead of
> > 300000000000, resulting in the watchdog firing in 3.6s instead of 600s.
> > 
> > Force u64 math by casting the first argument (gwdt->clk) as a u64. Make
> > the order of operations explicit with parenthesis.
> > 
> > Fixes: abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> > Reported-by: Vanshidhar Konda <vanshikonda@os.amperecomputing.com>
> > Signed-off-by: Darren Hart <darren@os.amperecomputing.com>
> > Cc: Wim Van Sebroeck <wim@linux-watchdog.org>
> > Cc: Guenter Roeck <linux@roeck-us.net>
> > Cc: linux-watchdog@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: <stable@vger.kernel.org> # 5.14.x
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Guenter or Wim, I haven't seen this land in the RCs or in next yet. Have
you already picked it up? Anything more needed from me?

Thanks,

-- 
Darren Hart
Ampere Computing / OS and Kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] sbsa_gwdt: Calculate timeout with 64-bit math
@ 2023-10-14  9:12     ` Darren Hart
  0 siblings, 0 replies; 18+ messages in thread
From: Darren Hart @ 2023-10-14  9:12 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-kernel, Vanshidhar Konda, Wim Van Sebroeck, linux-watchdog,
	linux-arm-kernel, stable

On Tue, Sep 26, 2023 at 05:45:13AM -0700, Guenter Roeck wrote:
> On Thu, Sep 21, 2023 at 02:02:36AM -0700, Darren Hart wrote:
> > Commit abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> > introduced new timer math for watchdog revision 1 with the 48 bit offset
> > register.
> > 
> > The gwdt->clk and timeout are u32, but the argument being calculated is
> > u64. Without a cast, the compiler performs u32 operations, truncating
> > intermediate steps, resulting in incorrect values.
> > 
> > A watchdog revision 1 implementation with a gwdt->clk of 1GHz and a
> > timeout of 600s writes 3647256576 to the one shot watchdog instead of
> > 300000000000, resulting in the watchdog firing in 3.6s instead of 600s.
> > 
> > Force u64 math by casting the first argument (gwdt->clk) as a u64. Make
> > the order of operations explicit with parenthesis.
> > 
> > Fixes: abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> > Reported-by: Vanshidhar Konda <vanshikonda@os.amperecomputing.com>
> > Signed-off-by: Darren Hart <darren@os.amperecomputing.com>
> > Cc: Wim Van Sebroeck <wim@linux-watchdog.org>
> > Cc: Guenter Roeck <linux@roeck-us.net>
> > Cc: linux-watchdog@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: <stable@vger.kernel.org> # 5.14.x
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Guenter or Wim, I haven't seen this land in the RCs or in next yet. Have
you already picked it up? Anything more needed from me?

Thanks,

-- 
Darren Hart
Ampere Computing / OS and Kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] sbsa_gwdt: Calculate timeout with 64-bit math
  2023-10-14  9:12     ` Darren Hart
@ 2023-10-22 16:58       ` Guenter Roeck
  -1 siblings, 0 replies; 18+ messages in thread
From: Guenter Roeck @ 2023-10-22 16:58 UTC (permalink / raw)
  To: Darren Hart
  Cc: linux-kernel, Vanshidhar Konda, Wim Van Sebroeck, linux-watchdog,
	linux-arm-kernel, stable

On 10/14/23 02:12, Darren Hart wrote:
> On Tue, Sep 26, 2023 at 05:45:13AM -0700, Guenter Roeck wrote:
>> On Thu, Sep 21, 2023 at 02:02:36AM -0700, Darren Hart wrote:
>>> Commit abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
>>> introduced new timer math for watchdog revision 1 with the 48 bit offset
>>> register.
>>>
>>> The gwdt->clk and timeout are u32, but the argument being calculated is
>>> u64. Without a cast, the compiler performs u32 operations, truncating
>>> intermediate steps, resulting in incorrect values.
>>>
>>> A watchdog revision 1 implementation with a gwdt->clk of 1GHz and a
>>> timeout of 600s writes 3647256576 to the one shot watchdog instead of
>>> 300000000000, resulting in the watchdog firing in 3.6s instead of 600s.
>>>
>>> Force u64 math by casting the first argument (gwdt->clk) as a u64. Make
>>> the order of operations explicit with parenthesis.
>>>
>>> Fixes: abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
>>> Reported-by: Vanshidhar Konda <vanshikonda@os.amperecomputing.com>
>>> Signed-off-by: Darren Hart <darren@os.amperecomputing.com>
>>> Cc: Wim Van Sebroeck <wim@linux-watchdog.org>
>>> Cc: Guenter Roeck <linux@roeck-us.net>
>>> Cc: linux-watchdog@vger.kernel.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: <stable@vger.kernel.org> # 5.14.x
>>
>> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> 
> Guenter or Wim, I haven't seen this land in the RCs or in next yet. Have
> you already picked it up? Anything more needed from me?
> 
> Thanks,
> 

Sorry, I am suffering from what I can only describe as a severe case of
maintainer/reviewer PTSD, and I have yet to find a way of dealing with that.

Guenter


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] sbsa_gwdt: Calculate timeout with 64-bit math
@ 2023-10-22 16:58       ` Guenter Roeck
  0 siblings, 0 replies; 18+ messages in thread
From: Guenter Roeck @ 2023-10-22 16:58 UTC (permalink / raw)
  To: Darren Hart
  Cc: linux-kernel, Vanshidhar Konda, Wim Van Sebroeck, linux-watchdog,
	linux-arm-kernel, stable

On 10/14/23 02:12, Darren Hart wrote:
> On Tue, Sep 26, 2023 at 05:45:13AM -0700, Guenter Roeck wrote:
>> On Thu, Sep 21, 2023 at 02:02:36AM -0700, Darren Hart wrote:
>>> Commit abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
>>> introduced new timer math for watchdog revision 1 with the 48 bit offset
>>> register.
>>>
>>> The gwdt->clk and timeout are u32, but the argument being calculated is
>>> u64. Without a cast, the compiler performs u32 operations, truncating
>>> intermediate steps, resulting in incorrect values.
>>>
>>> A watchdog revision 1 implementation with a gwdt->clk of 1GHz and a
>>> timeout of 600s writes 3647256576 to the one shot watchdog instead of
>>> 300000000000, resulting in the watchdog firing in 3.6s instead of 600s.
>>>
>>> Force u64 math by casting the first argument (gwdt->clk) as a u64. Make
>>> the order of operations explicit with parenthesis.
>>>
>>> Fixes: abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
>>> Reported-by: Vanshidhar Konda <vanshikonda@os.amperecomputing.com>
>>> Signed-off-by: Darren Hart <darren@os.amperecomputing.com>
>>> Cc: Wim Van Sebroeck <wim@linux-watchdog.org>
>>> Cc: Guenter Roeck <linux@roeck-us.net>
>>> Cc: linux-watchdog@vger.kernel.org
>>> Cc: linux-kernel@vger.kernel.org
>>> Cc: linux-arm-kernel@lists.infradead.org
>>> Cc: <stable@vger.kernel.org> # 5.14.x
>>
>> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> 
> Guenter or Wim, I haven't seen this land in the RCs or in next yet. Have
> you already picked it up? Anything more needed from me?
> 
> Thanks,
> 

Sorry, I am suffering from what I can only describe as a severe case of
maintainer/reviewer PTSD, and I have yet to find a way of dealing with that.

Guenter


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] sbsa_gwdt: Calculate timeout with 64-bit math
  2023-10-22 16:58       ` Guenter Roeck
@ 2023-10-26 17:17         ` Darren Hart
  -1 siblings, 0 replies; 18+ messages in thread
From: Darren Hart @ 2023-10-26 17:17 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-kernel, Vanshidhar Konda, Wim Van Sebroeck, linux-watchdog,
	linux-arm-kernel, stable

On Sun, Oct 22, 2023 at 09:58:26AM -0700, Guenter Roeck wrote:
> On 10/14/23 02:12, Darren Hart wrote:
> > On Tue, Sep 26, 2023 at 05:45:13AM -0700, Guenter Roeck wrote:
> > > On Thu, Sep 21, 2023 at 02:02:36AM -0700, Darren Hart wrote:
> > > > Commit abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> > > > introduced new timer math for watchdog revision 1 with the 48 bit offset
> > > > register.
> > > > 
> > > > The gwdt->clk and timeout are u32, but the argument being calculated is
> > > > u64. Without a cast, the compiler performs u32 operations, truncating
> > > > intermediate steps, resulting in incorrect values.
> > > > 
> > > > A watchdog revision 1 implementation with a gwdt->clk of 1GHz and a
> > > > timeout of 600s writes 3647256576 to the one shot watchdog instead of
> > > > 300000000000, resulting in the watchdog firing in 3.6s instead of 600s.
> > > > 
> > > > Force u64 math by casting the first argument (gwdt->clk) as a u64. Make
> > > > the order of operations explicit with parenthesis.
> > > > 
> > > > Fixes: abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> > > > Reported-by: Vanshidhar Konda <vanshikonda@os.amperecomputing.com>
> > > > Signed-off-by: Darren Hart <darren@os.amperecomputing.com>
> > > > Cc: Wim Van Sebroeck <wim@linux-watchdog.org>
> > > > Cc: Guenter Roeck <linux@roeck-us.net>
> > > > Cc: linux-watchdog@vger.kernel.org
> > > > Cc: linux-kernel@vger.kernel.org
> > > > Cc: linux-arm-kernel@lists.infradead.org
> > > > Cc: <stable@vger.kernel.org> # 5.14.x
> > > 
> > > Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> > 
> > Guenter or Wim, I haven't seen this land in the RCs or in next yet. Have
> > you already picked it up? Anything more needed from me?
> > 
> > Thanks,
> > 
> 
> Sorry, I am suffering from what I can only describe as a severe case of
> maintainer/reviewer PTSD, and I have yet to find a way of dealing with that.
> 

I'm sorry to hear it Guenter, it can be a thankless slog of a treadmill
sometimes. I found having a co-maintainer a huge help to even out the human
factors while maintaining the x86 platform drivers (in the before times).

In the short term, should I ask if one of the Arm maintainers would be willing
to pick this patch up?

Thanks,

-- 
Darren Hart
Ampere Computing / Linux Enabling

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] sbsa_gwdt: Calculate timeout with 64-bit math
@ 2023-10-26 17:17         ` Darren Hart
  0 siblings, 0 replies; 18+ messages in thread
From: Darren Hart @ 2023-10-26 17:17 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-kernel, Vanshidhar Konda, Wim Van Sebroeck, linux-watchdog,
	linux-arm-kernel, stable

On Sun, Oct 22, 2023 at 09:58:26AM -0700, Guenter Roeck wrote:
> On 10/14/23 02:12, Darren Hart wrote:
> > On Tue, Sep 26, 2023 at 05:45:13AM -0700, Guenter Roeck wrote:
> > > On Thu, Sep 21, 2023 at 02:02:36AM -0700, Darren Hart wrote:
> > > > Commit abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> > > > introduced new timer math for watchdog revision 1 with the 48 bit offset
> > > > register.
> > > > 
> > > > The gwdt->clk and timeout are u32, but the argument being calculated is
> > > > u64. Without a cast, the compiler performs u32 operations, truncating
> > > > intermediate steps, resulting in incorrect values.
> > > > 
> > > > A watchdog revision 1 implementation with a gwdt->clk of 1GHz and a
> > > > timeout of 600s writes 3647256576 to the one shot watchdog instead of
> > > > 300000000000, resulting in the watchdog firing in 3.6s instead of 600s.
> > > > 
> > > > Force u64 math by casting the first argument (gwdt->clk) as a u64. Make
> > > > the order of operations explicit with parenthesis.
> > > > 
> > > > Fixes: abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> > > > Reported-by: Vanshidhar Konda <vanshikonda@os.amperecomputing.com>
> > > > Signed-off-by: Darren Hart <darren@os.amperecomputing.com>
> > > > Cc: Wim Van Sebroeck <wim@linux-watchdog.org>
> > > > Cc: Guenter Roeck <linux@roeck-us.net>
> > > > Cc: linux-watchdog@vger.kernel.org
> > > > Cc: linux-kernel@vger.kernel.org
> > > > Cc: linux-arm-kernel@lists.infradead.org
> > > > Cc: <stable@vger.kernel.org> # 5.14.x
> > > 
> > > Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> > 
> > Guenter or Wim, I haven't seen this land in the RCs or in next yet. Have
> > you already picked it up? Anything more needed from me?
> > 
> > Thanks,
> > 
> 
> Sorry, I am suffering from what I can only describe as a severe case of
> maintainer/reviewer PTSD, and I have yet to find a way of dealing with that.
> 

I'm sorry to hear it Guenter, it can be a thankless slog of a treadmill
sometimes. I found having a co-maintainer a huge help to even out the human
factors while maintaining the x86 platform drivers (in the before times).

In the short term, should I ask if one of the Arm maintainers would be willing
to pick this patch up?

Thanks,

-- 
Darren Hart
Ampere Computing / Linux Enabling

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] sbsa_gwdt: Calculate timeout with 64-bit math
  2023-10-26 17:17         ` Darren Hart
@ 2023-10-29 15:53           ` Wim Van Sebroeck
  -1 siblings, 0 replies; 18+ messages in thread
From: Wim Van Sebroeck @ 2023-10-29 15:53 UTC (permalink / raw)
  To: Darren Hart
  Cc: Guenter Roeck, linux-kernel, Vanshidhar Konda, Wim Van Sebroeck,
	linux-watchdog, linux-arm-kernel, stable

Hi Darren,

> On Sun, Oct 22, 2023 at 09:58:26AM -0700, Guenter Roeck wrote:
> > On 10/14/23 02:12, Darren Hart wrote:
> > > On Tue, Sep 26, 2023 at 05:45:13AM -0700, Guenter Roeck wrote:
> > > > On Thu, Sep 21, 2023 at 02:02:36AM -0700, Darren Hart wrote:
> > > > > Commit abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> > > > > introduced new timer math for watchdog revision 1 with the 48 bit offset
> > > > > register.
> > > > > 
> > > > > The gwdt->clk and timeout are u32, but the argument being calculated is
> > > > > u64. Without a cast, the compiler performs u32 operations, truncating
> > > > > intermediate steps, resulting in incorrect values.
> > > > > 
> > > > > A watchdog revision 1 implementation with a gwdt->clk of 1GHz and a
> > > > > timeout of 600s writes 3647256576 to the one shot watchdog instead of
> > > > > 300000000000, resulting in the watchdog firing in 3.6s instead of 600s.
> > > > > 
> > > > > Force u64 math by casting the first argument (gwdt->clk) as a u64. Make
> > > > > the order of operations explicit with parenthesis.
> > > > > 
> > > > > Fixes: abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> > > > > Reported-by: Vanshidhar Konda <vanshikonda@os.amperecomputing.com>
> > > > > Signed-off-by: Darren Hart <darren@os.amperecomputing.com>
> > > > > Cc: Wim Van Sebroeck <wim@linux-watchdog.org>
> > > > > Cc: Guenter Roeck <linux@roeck-us.net>
> > > > > Cc: linux-watchdog@vger.kernel.org
> > > > > Cc: linux-kernel@vger.kernel.org
> > > > > Cc: linux-arm-kernel@lists.infradead.org
> > > > > Cc: <stable@vger.kernel.org> # 5.14.x
> > > > 
> > > > Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> > > 
> > > Guenter or Wim, I haven't seen this land in the RCs or in next yet. Have
> > > you already picked it up? Anything more needed from me?
> > > 
> > > Thanks,
> > > 
> > 
> > Sorry, I am suffering from what I can only describe as a severe case of
> > maintainer/reviewer PTSD, and I have yet to find a way of dealing with that.
> > 
> 
> I'm sorry to hear it Guenter, it can be a thankless slog of a treadmill
> sometimes. I found having a co-maintainer a huge help to even out the human
> factors while maintaining the x86 platform drivers (in the before times).
>
> In the short term, should I ask if one of the Arm maintainers would be willing
> to pick this patch up?

I'm picking this one up right now. So no need to ask it to the Arm maintainers.

Kind regards,
Wim.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] sbsa_gwdt: Calculate timeout with 64-bit math
@ 2023-10-29 15:53           ` Wim Van Sebroeck
  0 siblings, 0 replies; 18+ messages in thread
From: Wim Van Sebroeck @ 2023-10-29 15:53 UTC (permalink / raw)
  To: Darren Hart
  Cc: Guenter Roeck, linux-kernel, Vanshidhar Konda, Wim Van Sebroeck,
	linux-watchdog, linux-arm-kernel, stable

Hi Darren,

> On Sun, Oct 22, 2023 at 09:58:26AM -0700, Guenter Roeck wrote:
> > On 10/14/23 02:12, Darren Hart wrote:
> > > On Tue, Sep 26, 2023 at 05:45:13AM -0700, Guenter Roeck wrote:
> > > > On Thu, Sep 21, 2023 at 02:02:36AM -0700, Darren Hart wrote:
> > > > > Commit abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> > > > > introduced new timer math for watchdog revision 1 with the 48 bit offset
> > > > > register.
> > > > > 
> > > > > The gwdt->clk and timeout are u32, but the argument being calculated is
> > > > > u64. Without a cast, the compiler performs u32 operations, truncating
> > > > > intermediate steps, resulting in incorrect values.
> > > > > 
> > > > > A watchdog revision 1 implementation with a gwdt->clk of 1GHz and a
> > > > > timeout of 600s writes 3647256576 to the one shot watchdog instead of
> > > > > 300000000000, resulting in the watchdog firing in 3.6s instead of 600s.
> > > > > 
> > > > > Force u64 math by casting the first argument (gwdt->clk) as a u64. Make
> > > > > the order of operations explicit with parenthesis.
> > > > > 
> > > > > Fixes: abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> > > > > Reported-by: Vanshidhar Konda <vanshikonda@os.amperecomputing.com>
> > > > > Signed-off-by: Darren Hart <darren@os.amperecomputing.com>
> > > > > Cc: Wim Van Sebroeck <wim@linux-watchdog.org>
> > > > > Cc: Guenter Roeck <linux@roeck-us.net>
> > > > > Cc: linux-watchdog@vger.kernel.org
> > > > > Cc: linux-kernel@vger.kernel.org
> > > > > Cc: linux-arm-kernel@lists.infradead.org
> > > > > Cc: <stable@vger.kernel.org> # 5.14.x
> > > > 
> > > > Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> > > 
> > > Guenter or Wim, I haven't seen this land in the RCs or in next yet. Have
> > > you already picked it up? Anything more needed from me?
> > > 
> > > Thanks,
> > > 
> > 
> > Sorry, I am suffering from what I can only describe as a severe case of
> > maintainer/reviewer PTSD, and I have yet to find a way of dealing with that.
> > 
> 
> I'm sorry to hear it Guenter, it can be a thankless slog of a treadmill
> sometimes. I found having a co-maintainer a huge help to even out the human
> factors while maintaining the x86 platform drivers (in the before times).
>
> In the short term, should I ask if one of the Arm maintainers would be willing
> to pick this patch up?

I'm picking this one up right now. So no need to ask it to the Arm maintainers.

Kind regards,
Wim.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] sbsa_gwdt: Calculate timeout with 64-bit math
  2023-10-22 16:58       ` Guenter Roeck
@ 2023-10-29 16:10         ` Wim Van Sebroeck
  -1 siblings, 0 replies; 18+ messages in thread
From: Wim Van Sebroeck @ 2023-10-29 16:10 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Darren Hart, linux-kernel, Vanshidhar Konda, Wim Van Sebroeck,
	linux-watchdog, linux-arm-kernel, stable

Hi Guenter,

> On 10/14/23 02:12, Darren Hart wrote:
> >On Tue, Sep 26, 2023 at 05:45:13AM -0700, Guenter Roeck wrote:
> >>On Thu, Sep 21, 2023 at 02:02:36AM -0700, Darren Hart wrote:
> >>>Commit abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> >>>introduced new timer math for watchdog revision 1 with the 48 bit offset
> >>>register.
> >>>
> >>>The gwdt->clk and timeout are u32, but the argument being calculated is
> >>>u64. Without a cast, the compiler performs u32 operations, truncating
> >>>intermediate steps, resulting in incorrect values.
> >>>
> >>>A watchdog revision 1 implementation with a gwdt->clk of 1GHz and a
> >>>timeout of 600s writes 3647256576 to the one shot watchdog instead of
> >>>300000000000, resulting in the watchdog firing in 3.6s instead of 600s.
> >>>
> >>>Force u64 math by casting the first argument (gwdt->clk) as a u64. Make
> >>>the order of operations explicit with parenthesis.
> >>>
> >>>Fixes: abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> >>>Reported-by: Vanshidhar Konda <vanshikonda@os.amperecomputing.com>
> >>>Signed-off-by: Darren Hart <darren@os.amperecomputing.com>
> >>>Cc: Wim Van Sebroeck <wim@linux-watchdog.org>
> >>>Cc: Guenter Roeck <linux@roeck-us.net>
> >>>Cc: linux-watchdog@vger.kernel.org
> >>>Cc: linux-kernel@vger.kernel.org
> >>>Cc: linux-arm-kernel@lists.infradead.org
> >>>Cc: <stable@vger.kernel.org> # 5.14.x
> >>
> >>Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> >
> >Guenter or Wim, I haven't seen this land in the RCs or in next yet. Have
> >you already picked it up? Anything more needed from me?
> >
> >Thanks,
> >
> 
> Sorry, I am suffering from what I can only describe as a severe case of
> maintainer/reviewer PTSD, and I have yet to find a way of dealing with that.

I can imagine what it is like. And I do know that if you wouldn't have been there, 
that I would have allready stopped being a maintainer. So I hope you can find the 
right cooping mechanisms. I also had to work non-stop the last 4 to 5 weeks and it was hell.
So I wish you all the best.

PS: picking up all patches that have your review-by tag on it as we speack.

Kind regards,
Wim.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] sbsa_gwdt: Calculate timeout with 64-bit math
@ 2023-10-29 16:10         ` Wim Van Sebroeck
  0 siblings, 0 replies; 18+ messages in thread
From: Wim Van Sebroeck @ 2023-10-29 16:10 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Darren Hart, linux-kernel, Vanshidhar Konda, Wim Van Sebroeck,
	linux-watchdog, linux-arm-kernel, stable

Hi Guenter,

> On 10/14/23 02:12, Darren Hart wrote:
> >On Tue, Sep 26, 2023 at 05:45:13AM -0700, Guenter Roeck wrote:
> >>On Thu, Sep 21, 2023 at 02:02:36AM -0700, Darren Hart wrote:
> >>>Commit abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> >>>introduced new timer math for watchdog revision 1 with the 48 bit offset
> >>>register.
> >>>
> >>>The gwdt->clk and timeout are u32, but the argument being calculated is
> >>>u64. Without a cast, the compiler performs u32 operations, truncating
> >>>intermediate steps, resulting in incorrect values.
> >>>
> >>>A watchdog revision 1 implementation with a gwdt->clk of 1GHz and a
> >>>timeout of 600s writes 3647256576 to the one shot watchdog instead of
> >>>300000000000, resulting in the watchdog firing in 3.6s instead of 600s.
> >>>
> >>>Force u64 math by casting the first argument (gwdt->clk) as a u64. Make
> >>>the order of operations explicit with parenthesis.
> >>>
> >>>Fixes: abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> >>>Reported-by: Vanshidhar Konda <vanshikonda@os.amperecomputing.com>
> >>>Signed-off-by: Darren Hart <darren@os.amperecomputing.com>
> >>>Cc: Wim Van Sebroeck <wim@linux-watchdog.org>
> >>>Cc: Guenter Roeck <linux@roeck-us.net>
> >>>Cc: linux-watchdog@vger.kernel.org
> >>>Cc: linux-kernel@vger.kernel.org
> >>>Cc: linux-arm-kernel@lists.infradead.org
> >>>Cc: <stable@vger.kernel.org> # 5.14.x
> >>
> >>Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> >
> >Guenter or Wim, I haven't seen this land in the RCs or in next yet. Have
> >you already picked it up? Anything more needed from me?
> >
> >Thanks,
> >
> 
> Sorry, I am suffering from what I can only describe as a severe case of
> maintainer/reviewer PTSD, and I have yet to find a way of dealing with that.

I can imagine what it is like. And I do know that if you wouldn't have been there, 
that I would have allready stopped being a maintainer. So I hope you can find the 
right cooping mechanisms. I also had to work non-stop the last 4 to 5 weeks and it was hell.
So I wish you all the best.

PS: picking up all patches that have your review-by tag on it as we speack.

Kind regards,
Wim.


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] sbsa_gwdt: Calculate timeout with 64-bit math
  2023-10-29 16:10         ` Wim Van Sebroeck
@ 2023-10-29 17:00           ` Guenter Roeck
  -1 siblings, 0 replies; 18+ messages in thread
From: Guenter Roeck @ 2023-10-29 17:00 UTC (permalink / raw)
  To: Wim Van Sebroeck
  Cc: Darren Hart, linux-kernel, Vanshidhar Konda, linux-watchdog,
	linux-arm-kernel, stable

On 10/29/23 09:10, Wim Van Sebroeck wrote:
> Hi Guenter,
> 
>> On 10/14/23 02:12, Darren Hart wrote:
>>> On Tue, Sep 26, 2023 at 05:45:13AM -0700, Guenter Roeck wrote:
>>>> On Thu, Sep 21, 2023 at 02:02:36AM -0700, Darren Hart wrote:
>>>>> Commit abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
>>>>> introduced new timer math for watchdog revision 1 with the 48 bit offset
>>>>> register.
>>>>>
>>>>> The gwdt->clk and timeout are u32, but the argument being calculated is
>>>>> u64. Without a cast, the compiler performs u32 operations, truncating
>>>>> intermediate steps, resulting in incorrect values.
>>>>>
>>>>> A watchdog revision 1 implementation with a gwdt->clk of 1GHz and a
>>>>> timeout of 600s writes 3647256576 to the one shot watchdog instead of
>>>>> 300000000000, resulting in the watchdog firing in 3.6s instead of 600s.
>>>>>
>>>>> Force u64 math by casting the first argument (gwdt->clk) as a u64. Make
>>>>> the order of operations explicit with parenthesis.
>>>>>
>>>>> Fixes: abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
>>>>> Reported-by: Vanshidhar Konda <vanshikonda@os.amperecomputing.com>
>>>>> Signed-off-by: Darren Hart <darren@os.amperecomputing.com>
>>>>> Cc: Wim Van Sebroeck <wim@linux-watchdog.org>
>>>>> Cc: Guenter Roeck <linux@roeck-us.net>
>>>>> Cc: linux-watchdog@vger.kernel.org
>>>>> Cc: linux-kernel@vger.kernel.org
>>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>>> Cc: <stable@vger.kernel.org> # 5.14.x
>>>>
>>>> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>>>
>>> Guenter or Wim, I haven't seen this land in the RCs or in next yet. Have
>>> you already picked it up? Anything more needed from me?
>>>
>>> Thanks,
>>>
>>
>> Sorry, I am suffering from what I can only describe as a severe case of
>> maintainer/reviewer PTSD, and I have yet to find a way of dealing with that.
> 
> I can imagine what it is like. And I do know that if you wouldn't have been there,
> that I would have allready stopped being a maintainer. So I hope you can find the
> right cooping mechanisms. I also had to work non-stop the last 4 to 5 weeks and it was hell.
> So I wish you all the best.
> 
> PS: picking up all patches that have your review-by tag on it as we speack.
> 

Thanks,
Guenter


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] sbsa_gwdt: Calculate timeout with 64-bit math
@ 2023-10-29 17:00           ` Guenter Roeck
  0 siblings, 0 replies; 18+ messages in thread
From: Guenter Roeck @ 2023-10-29 17:00 UTC (permalink / raw)
  To: Wim Van Sebroeck
  Cc: Darren Hart, linux-kernel, Vanshidhar Konda, linux-watchdog,
	linux-arm-kernel, stable

On 10/29/23 09:10, Wim Van Sebroeck wrote:
> Hi Guenter,
> 
>> On 10/14/23 02:12, Darren Hart wrote:
>>> On Tue, Sep 26, 2023 at 05:45:13AM -0700, Guenter Roeck wrote:
>>>> On Thu, Sep 21, 2023 at 02:02:36AM -0700, Darren Hart wrote:
>>>>> Commit abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
>>>>> introduced new timer math for watchdog revision 1 with the 48 bit offset
>>>>> register.
>>>>>
>>>>> The gwdt->clk and timeout are u32, but the argument being calculated is
>>>>> u64. Without a cast, the compiler performs u32 operations, truncating
>>>>> intermediate steps, resulting in incorrect values.
>>>>>
>>>>> A watchdog revision 1 implementation with a gwdt->clk of 1GHz and a
>>>>> timeout of 600s writes 3647256576 to the one shot watchdog instead of
>>>>> 300000000000, resulting in the watchdog firing in 3.6s instead of 600s.
>>>>>
>>>>> Force u64 math by casting the first argument (gwdt->clk) as a u64. Make
>>>>> the order of operations explicit with parenthesis.
>>>>>
>>>>> Fixes: abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
>>>>> Reported-by: Vanshidhar Konda <vanshikonda@os.amperecomputing.com>
>>>>> Signed-off-by: Darren Hart <darren@os.amperecomputing.com>
>>>>> Cc: Wim Van Sebroeck <wim@linux-watchdog.org>
>>>>> Cc: Guenter Roeck <linux@roeck-us.net>
>>>>> Cc: linux-watchdog@vger.kernel.org
>>>>> Cc: linux-kernel@vger.kernel.org
>>>>> Cc: linux-arm-kernel@lists.infradead.org
>>>>> Cc: <stable@vger.kernel.org> # 5.14.x
>>>>
>>>> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>>>
>>> Guenter or Wim, I haven't seen this land in the RCs or in next yet. Have
>>> you already picked it up? Anything more needed from me?
>>>
>>> Thanks,
>>>
>>
>> Sorry, I am suffering from what I can only describe as a severe case of
>> maintainer/reviewer PTSD, and I have yet to find a way of dealing with that.
> 
> I can imagine what it is like. And I do know that if you wouldn't have been there,
> that I would have allready stopped being a maintainer. So I hope you can find the
> right cooping mechanisms. I also had to work non-stop the last 4 to 5 weeks and it was hell.
> So I wish you all the best.
> 
> PS: picking up all patches that have your review-by tag on it as we speack.
> 

Thanks,
Guenter


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] sbsa_gwdt: Calculate timeout with 64-bit math
  2023-10-29 17:00           ` Guenter Roeck
@ 2023-10-30 16:21             ` Darren Hart
  -1 siblings, 0 replies; 18+ messages in thread
From: Darren Hart @ 2023-10-30 16:21 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Wim Van Sebroeck, linux-kernel, Vanshidhar Konda, linux-watchdog,
	linux-arm-kernel, stable

On Sun, Oct 29, 2023 at 10:00:54AM -0700, Guenter Roeck wrote:
> On 10/29/23 09:10, Wim Van Sebroeck wrote:
> > Hi Guenter,
> > 
> > > On 10/14/23 02:12, Darren Hart wrote:
> > > > On Tue, Sep 26, 2023 at 05:45:13AM -0700, Guenter Roeck wrote:
> > > > > On Thu, Sep 21, 2023 at 02:02:36AM -0700, Darren Hart wrote:
> > > > > > Commit abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> > > > > > introduced new timer math for watchdog revision 1 with the 48 bit offset
> > > > > > register.
> > > > > > 
> > > > > > The gwdt->clk and timeout are u32, but the argument being calculated is
> > > > > > u64. Without a cast, the compiler performs u32 operations, truncating
> > > > > > intermediate steps, resulting in incorrect values.
> > > > > > 
> > > > > > A watchdog revision 1 implementation with a gwdt->clk of 1GHz and a
> > > > > > timeout of 600s writes 3647256576 to the one shot watchdog instead of
> > > > > > 300000000000, resulting in the watchdog firing in 3.6s instead of 600s.
> > > > > > 
> > > > > > Force u64 math by casting the first argument (gwdt->clk) as a u64. Make
> > > > > > the order of operations explicit with parenthesis.
> > > > > > 
> > > > > > Fixes: abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> > > > > > Reported-by: Vanshidhar Konda <vanshikonda@os.amperecomputing.com>
> > > > > > Signed-off-by: Darren Hart <darren@os.amperecomputing.com>
> > > > > > Cc: Wim Van Sebroeck <wim@linux-watchdog.org>
> > > > > > Cc: Guenter Roeck <linux@roeck-us.net>
> > > > > > Cc: linux-watchdog@vger.kernel.org
> > > > > > Cc: linux-kernel@vger.kernel.org
> > > > > > Cc: linux-arm-kernel@lists.infradead.org
> > > > > > Cc: <stable@vger.kernel.org> # 5.14.x
> > > > > 
> > > > > Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> > > > 
> > > > Guenter or Wim, I haven't seen this land in the RCs or in next yet. Have
> > > > you already picked it up? Anything more needed from me?
> > > > 
> > > > Thanks,
> > > > 
> > > 
> > > Sorry, I am suffering from what I can only describe as a severe case of
> > > maintainer/reviewer PTSD, and I have yet to find a way of dealing with that.
> > 
> > I can imagine what it is like. And I do know that if you wouldn't have been there,
> > that I would have allready stopped being a maintainer. So I hope you can find the
> > right cooping mechanisms. I also had to work non-stop the last 4 to 5 weeks and it was hell.
> > So I wish you all the best.
> > 
> > PS: picking up all patches that have your review-by tag on it as we speack.
> > 
> 
> Thanks,
> Guenter

Thanks for the support Wim and Guenter. Appreciate the work you do and the
pressures of maintainership.

-- 
Darren Hart
Ampere Computing / Linux Enabling

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] sbsa_gwdt: Calculate timeout with 64-bit math
@ 2023-10-30 16:21             ` Darren Hart
  0 siblings, 0 replies; 18+ messages in thread
From: Darren Hart @ 2023-10-30 16:21 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Wim Van Sebroeck, linux-kernel, Vanshidhar Konda, linux-watchdog,
	linux-arm-kernel, stable

On Sun, Oct 29, 2023 at 10:00:54AM -0700, Guenter Roeck wrote:
> On 10/29/23 09:10, Wim Van Sebroeck wrote:
> > Hi Guenter,
> > 
> > > On 10/14/23 02:12, Darren Hart wrote:
> > > > On Tue, Sep 26, 2023 at 05:45:13AM -0700, Guenter Roeck wrote:
> > > > > On Thu, Sep 21, 2023 at 02:02:36AM -0700, Darren Hart wrote:
> > > > > > Commit abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> > > > > > introduced new timer math for watchdog revision 1 with the 48 bit offset
> > > > > > register.
> > > > > > 
> > > > > > The gwdt->clk and timeout are u32, but the argument being calculated is
> > > > > > u64. Without a cast, the compiler performs u32 operations, truncating
> > > > > > intermediate steps, resulting in incorrect values.
> > > > > > 
> > > > > > A watchdog revision 1 implementation with a gwdt->clk of 1GHz and a
> > > > > > timeout of 600s writes 3647256576 to the one shot watchdog instead of
> > > > > > 300000000000, resulting in the watchdog firing in 3.6s instead of 600s.
> > > > > > 
> > > > > > Force u64 math by casting the first argument (gwdt->clk) as a u64. Make
> > > > > > the order of operations explicit with parenthesis.
> > > > > > 
> > > > > > Fixes: abd3ac7902fb ("watchdog: sbsa: Support architecture version 1")
> > > > > > Reported-by: Vanshidhar Konda <vanshikonda@os.amperecomputing.com>
> > > > > > Signed-off-by: Darren Hart <darren@os.amperecomputing.com>
> > > > > > Cc: Wim Van Sebroeck <wim@linux-watchdog.org>
> > > > > > Cc: Guenter Roeck <linux@roeck-us.net>
> > > > > > Cc: linux-watchdog@vger.kernel.org
> > > > > > Cc: linux-kernel@vger.kernel.org
> > > > > > Cc: linux-arm-kernel@lists.infradead.org
> > > > > > Cc: <stable@vger.kernel.org> # 5.14.x
> > > > > 
> > > > > Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> > > > 
> > > > Guenter or Wim, I haven't seen this land in the RCs or in next yet. Have
> > > > you already picked it up? Anything more needed from me?
> > > > 
> > > > Thanks,
> > > > 
> > > 
> > > Sorry, I am suffering from what I can only describe as a severe case of
> > > maintainer/reviewer PTSD, and I have yet to find a way of dealing with that.
> > 
> > I can imagine what it is like. And I do know that if you wouldn't have been there,
> > that I would have allready stopped being a maintainer. So I hope you can find the
> > right cooping mechanisms. I also had to work non-stop the last 4 to 5 weeks and it was hell.
> > So I wish you all the best.
> > 
> > PS: picking up all patches that have your review-by tag on it as we speack.
> > 
> 
> Thanks,
> Guenter

Thanks for the support Wim and Guenter. Appreciate the work you do and the
pressures of maintainership.

-- 
Darren Hart
Ampere Computing / Linux Enabling

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2023-10-30 16:21 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-21  9:02 [PATCH] sbsa_gwdt: Calculate timeout with 64-bit math Darren Hart
2023-09-21  9:02 ` Darren Hart
2023-09-26 12:45 ` Guenter Roeck
2023-09-26 12:45   ` Guenter Roeck
2023-10-14  9:12   ` Darren Hart
2023-10-14  9:12     ` Darren Hart
2023-10-22 16:58     ` Guenter Roeck
2023-10-22 16:58       ` Guenter Roeck
2023-10-26 17:17       ` Darren Hart
2023-10-26 17:17         ` Darren Hart
2023-10-29 15:53         ` Wim Van Sebroeck
2023-10-29 15:53           ` Wim Van Sebroeck
2023-10-29 16:10       ` Wim Van Sebroeck
2023-10-29 16:10         ` Wim Van Sebroeck
2023-10-29 17:00         ` Guenter Roeck
2023-10-29 17:00           ` Guenter Roeck
2023-10-30 16:21           ` Darren Hart
2023-10-30 16:21             ` Darren Hart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.