* 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.
^ 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.
_______________________________________________
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-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