From: Jan Kiszka <[email protected]>
Obviously, the test needs to run against the register content, not its
address.
Fixes: cb011044e34c ("watchdog: iTCO_wdt: Account for rebooting on second timeout")
Reported-by: Mantas Mikulėnas <[email protected]>
Signed-off-by: Jan Kiszka <[email protected]>
---
drivers/watchdog/iTCO_wdt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
index b3f604669e2c..643c6c2d0b72 100644
--- a/drivers/watchdog/iTCO_wdt.c
+++ b/drivers/watchdog/iTCO_wdt.c
@@ -362,7 +362,7 @@ static int iTCO_wdt_set_timeout(struct watchdog_device *wd_dev, unsigned int t)
* Otherwise, the BIOS generally reboots when the SMI triggers.
*/
if (p->smi_res &&
- (SMI_EN(p) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
+ (inl(SMI_EN(p)) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
tmrval /= 2;
/* from the specs: */
--
2.26.2
On Mon, Jul 26, 2021 at 2:46 PM Jan Kiszka <[email protected]> wrote:
>
> From: Jan Kiszka <[email protected]>
>
> Obviously, the test needs to run against the register content, not its
> address.
>
> Fixes: cb011044e34c ("watchdog: iTCO_wdt: Account for rebooting on second timeout")
> Reported-by: Mantas Mikulėnas <[email protected]>
> Signed-off-by: Jan Kiszka <[email protected]>
Missed SoB of the submitter (hint: configure your Git to make sure
that submitter and author are the same in terms of name-email).
...
> if (p->smi_res &&
> - (SMI_EN(p) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
> + (inl(SMI_EN(p)) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
> tmrval /= 2;
There are so many parentheses, perhaps
#define TCO_GBL_SMI_EN (TCO_EN | GBL_SMI_EN)
...
if (p->smi_res &&
(inl(SMI_EN(p)) & TCO_GBL_SMI_EN) != TCO_GBL_SMI_EN)
tmrval /= 2;
?
--
With Best Regards,
Andy Shevchenko
On 26.07.21 14:01, Andy Shevchenko wrote:
> On Mon, Jul 26, 2021 at 2:46 PM Jan Kiszka <[email protected]> wrote:
>>
>> From: Jan Kiszka <[email protected]>
>>
>> Obviously, the test needs to run against the register content, not its
>> address.
>>
>> Fixes: cb011044e34c ("watchdog: iTCO_wdt: Account for rebooting on second timeout")
>> Reported-by: Mantas Mikulėnas <[email protected]>
>
>> Signed-off-by: Jan Kiszka <[email protected]>
>
> Missed SoB of the submitter (hint: configure your Git to make sure
> that submitter and author are the same in terms of name-email).
The signed off is there. Not sure what you are referring to.
>
> ...
>
>> if (p->smi_res &&
>> - (SMI_EN(p) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
>> + (inl(SMI_EN(p)) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
>> tmrval /= 2;
>
> There are so many parentheses, perhaps
>
> #define TCO_GBL_SMI_EN (TCO_EN | GBL_SMI_EN)
> ...
> if (p->smi_res &&
> (inl(SMI_EN(p)) & TCO_GBL_SMI_EN) != TCO_GBL_SMI_EN)
> tmrval /= 2;
>
> ?
>
Let's focus on the regression fix (and you could have mentioned that on
the original patch already).
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
On 26.07.21 14:03, Paolo Bonzini wrote:
> On 26/07/21 13:46, Jan Kiszka wrote:
>> From: Jan Kiszka <[email protected]>
>>
>> Obviously, the test needs to run against the register content, not its
>> address.
>>
>> Fixes: cb011044e34c ("watchdog: iTCO_wdt: Account for rebooting on
>> second timeout")
>> Reported-by: Mantas Mikulėnas <[email protected]>
>> Signed-off-by: Jan Kiszka <[email protected]>
>> ---
>> drivers/watchdog/iTCO_wdt.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
>> index b3f604669e2c..643c6c2d0b72 100644
>> --- a/drivers/watchdog/iTCO_wdt.c
>> +++ b/drivers/watchdog/iTCO_wdt.c
>> @@ -362,7 +362,7 @@ static int iTCO_wdt_set_timeout(struct
>> watchdog_device *wd_dev, unsigned int t)
>> * Otherwise, the BIOS generally reboots when the SMI triggers.
>> */
>> if (p->smi_res &&
>> - (SMI_EN(p) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
>> + (inl(SMI_EN(p)) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN |
>> GBL_SMI_EN))
>> tmrval /= 2;
>> /* from the specs: */
>>
>
> Reviewed-by: Paolo Bonzini <[email protected]>
> Cc: [email protected]
>
> (the latter because cb011044e34c has been picked up by stable kernels
> already).
>
Thanks. Originally wanted to add stable myself, but I'm still unsure
whether this is the privilege of the subsystem maintainer or should also
be done by contributors.
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
On Mon, Jul 26, 2021 at 3:04 PM Jan Kiszka <[email protected]> wrote:
>
> On 26.07.21 14:01, Andy Shevchenko wrote:
> > On Mon, Jul 26, 2021 at 2:46 PM Jan Kiszka <[email protected]> wrote:
> >>
> >> From: Jan Kiszka <[email protected]>
> >>
> >> Obviously, the test needs to run against the register content, not its
> >> address.
> >>
> >> Fixes: cb011044e34c ("watchdog: iTCO_wdt: Account for rebooting on second timeout")
> >> Reported-by: Mantas Mikulėnas <[email protected]>
> >
> >> Signed-off-by: Jan Kiszka <[email protected]>
> >
> > Missed SoB of the submitter (hint: configure your Git to make sure
> > that submitter and author are the same in terms of name-email).
>
> The signed off is there. Not sure what you are referring to.
Nope. It's not. The sign of that is the From: line in the body of the
email. It happens when the submitter != author. And SoB of the former
one is absent. But what is strange is that reading them here I haven't
found the difference. Maybe one is in UTF-8 while the other is not and
a unicode character degraded to Latin-1 or so?
--
With Best Regards,
Andy Shevchenko
On 7/26/21 4:46 AM, Jan Kiszka wrote:
> From: Jan Kiszka <[email protected]>
>
> Obviously, the test needs to run against the register content, not its
> address.
>
> Fixes: cb011044e34c ("watchdog: iTCO_wdt: Account for rebooting on second timeout")
> Reported-by: Mantas Mikulėnas <[email protected]>
> Signed-off-by: Jan Kiszka <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> drivers/watchdog/iTCO_wdt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> index b3f604669e2c..643c6c2d0b72 100644
> --- a/drivers/watchdog/iTCO_wdt.c
> +++ b/drivers/watchdog/iTCO_wdt.c
> @@ -362,7 +362,7 @@ static int iTCO_wdt_set_timeout(struct watchdog_device *wd_dev, unsigned int t)
> * Otherwise, the BIOS generally reboots when the SMI triggers.
> */
> if (p->smi_res &&
> - (SMI_EN(p) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
> + (inl(SMI_EN(p)) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
> tmrval /= 2;
>
> /* from the specs: */
>
On 7/26/21 5:05 AM, Jan Kiszka wrote:
> On 26.07.21 14:03, Paolo Bonzini wrote:
>> On 26/07/21 13:46, Jan Kiszka wrote:
>>> From: Jan Kiszka <[email protected]>
>>>
>>> Obviously, the test needs to run against the register content, not its
>>> address.
>>>
>>> Fixes: cb011044e34c ("watchdog: iTCO_wdt: Account for rebooting on
>>> second timeout")
>>> Reported-by: Mantas Mikulėnas <[email protected]>
>>> Signed-off-by: Jan Kiszka <[email protected]>
>>> ---
>>> drivers/watchdog/iTCO_wdt.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
>>> index b3f604669e2c..643c6c2d0b72 100644
>>> --- a/drivers/watchdog/iTCO_wdt.c
>>> +++ b/drivers/watchdog/iTCO_wdt.c
>>> @@ -362,7 +362,7 @@ static int iTCO_wdt_set_timeout(struct
>>> watchdog_device *wd_dev, unsigned int t)
>>> * Otherwise, the BIOS generally reboots when the SMI triggers.
>>> */
>>> if (p->smi_res &&
>>> - (SMI_EN(p) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
>>> + (inl(SMI_EN(p)) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN |
>>> GBL_SMI_EN))
>>> tmrval /= 2;
>>> /* from the specs: */
>>>
>>
>> Reviewed-by: Paolo Bonzini <[email protected]>
>> Cc: [email protected]
>>
>> (the latter because cb011044e34c has been picked up by stable kernels
>> already).
>>
>
> Thanks. Originally wanted to add stable myself, but I'm still unsure
> whether this is the privilege of the subsystem maintainer or should also
> be done by contributors.
>
Normally it is done by maintainers.
Guenter
On 7/26/21 6:40 AM, Andy Shevchenko wrote:
> On Mon, Jul 26, 2021 at 3:04 PM Jan Kiszka <[email protected]> wrote:
>>
>> On 26.07.21 14:01, Andy Shevchenko wrote:
>>> On Mon, Jul 26, 2021 at 2:46 PM Jan Kiszka <[email protected]> wrote:
>>>>
>>>> From: Jan Kiszka <[email protected]>
>>>>
>>>> Obviously, the test needs to run against the register content, not its
>>>> address.
>>>>
>>>> Fixes: cb011044e34c ("watchdog: iTCO_wdt: Account for rebooting on second timeout")
>>>> Reported-by: Mantas Mikulėnas <[email protected]>
>>>
>>>> Signed-off-by: Jan Kiszka <[email protected]>
>>>
>>> Missed SoB of the submitter (hint: configure your Git to make sure
>>> that submitter and author are the same in terms of name-email).
>>
>> The signed off is there. Not sure what you are referring to.
>
> Nope. It's not. The sign of that is the From: line in the body of the
> email. It happens when the submitter != author. And SoB of the former
> one is absent. But what is strange is that reading them here I haven't
> found the difference. Maybe one is in UTF-8 while the other is not and
> a unicode character degraded to Latin-1 or so?
>
I have no idea why there is an additional From:, but both From:
tags in the e-mail source are exact matches, and both match the
name and e-mail address in Signed-off-by:. I agree with Jan,
the SoB is there.
Guenter
On 26/07/21 13:46, Jan Kiszka wrote:
> From: Jan Kiszka <[email protected]>
>
> Obviously, the test needs to run against the register content, not its
> address.
>
> Fixes: cb011044e34c ("watchdog: iTCO_wdt: Account for rebooting on second timeout")
> Reported-by: Mantas Mikulėnas <[email protected]>
> Signed-off-by: Jan Kiszka <[email protected]>
> ---
> drivers/watchdog/iTCO_wdt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> index b3f604669e2c..643c6c2d0b72 100644
> --- a/drivers/watchdog/iTCO_wdt.c
> +++ b/drivers/watchdog/iTCO_wdt.c
> @@ -362,7 +362,7 @@ static int iTCO_wdt_set_timeout(struct watchdog_device *wd_dev, unsigned int t)
> * Otherwise, the BIOS generally reboots when the SMI triggers.
> */
> if (p->smi_res &&
> - (SMI_EN(p) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
> + (inl(SMI_EN(p)) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
> tmrval /= 2;
>
> /* from the specs: */
>
Reviewed-by: Paolo Bonzini <[email protected]>
Cc: [email protected]
(the latter because cb011044e34c has been picked up by stable kernels
already).
Paolo
On 26.07.21 15:59, Guenter Roeck wrote:
> On 7/26/21 6:40 AM, Andy Shevchenko wrote:
>> On Mon, Jul 26, 2021 at 3:04 PM Jan Kiszka <[email protected]>
>> wrote:
>>>
>>> On 26.07.21 14:01, Andy Shevchenko wrote:
>>>> On Mon, Jul 26, 2021 at 2:46 PM Jan Kiszka <[email protected]>
>>>> wrote:
>>>>>
>>>>> From: Jan Kiszka <[email protected]>
>>>>>
>>>>> Obviously, the test needs to run against the register content, not its
>>>>> address.
>>>>>
>>>>> Fixes: cb011044e34c ("watchdog: iTCO_wdt: Account for rebooting on
>>>>> second timeout")
>>>>> Reported-by: Mantas Mikulėnas <[email protected]>
>>>>
>>>>> Signed-off-by: Jan Kiszka <[email protected]>
>>>>
>>>> Missed SoB of the submitter (hint: configure your Git to make sure
>>>> that submitter and author are the same in terms of name-email).
>>>
>>> The signed off is there. Not sure what you are referring to.
>>
>> Nope. It's not. The sign of that is the From: line in the body of the
>> email. It happens when the submitter != author. And SoB of the former
>> one is absent. But what is strange is that reading them here I haven't
>> found the difference. Maybe one is in UTF-8 while the other is not and
>> a unicode character degraded to Latin-1 or so?
>>
>
> I have no idea why there is an additional From:, but both From:
> tags in the e-mail source are exact matches, and both match the
> name and e-mail address in Signed-off-by:. I agree with Jan,
> the SoB is there.
There is one unknown in this equation, and that is the anti-email system
operated by a our IT and some company in Redmond. But I haven't received
any complaints that my outgoing emails are negatively affected by it
(incoming are, but that's a different story...). If you received
something mangled, Andy, please share the source of that email. I'm
happy to escalate internally - and externally.
For the potential case they were mangled or in case I'm submitting via a
real email provider, my scripts always add a "From:" to the body of my
patches. Outgoing, that From matched my Signed-off.
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
On Mon, Jul 26, 2021 at 5:05 PM Jan Kiszka <[email protected]> wrote:
>
> On 26.07.21 15:59, Guenter Roeck wrote:
> > On 7/26/21 6:40 AM, Andy Shevchenko wrote:
> >> On Mon, Jul 26, 2021 at 3:04 PM Jan Kiszka <[email protected]>
> >> wrote:
> >>>
> >>> On 26.07.21 14:01, Andy Shevchenko wrote:
> >>>> On Mon, Jul 26, 2021 at 2:46 PM Jan Kiszka <[email protected]>
> >>>> wrote:
> >>>>>
> >>>>> From: Jan Kiszka <[email protected]>
> >>>>>
> >>>>> Obviously, the test needs to run against the register content, not its
> >>>>> address.
> >>>>>
> >>>>> Fixes: cb011044e34c ("watchdog: iTCO_wdt: Account for rebooting on
> >>>>> second timeout")
> >>>>> Reported-by: Mantas Mikulėnas <[email protected]>
> >>>>
> >>>>> Signed-off-by: Jan Kiszka <[email protected]>
> >>>>
> >>>> Missed SoB of the submitter (hint: configure your Git to make sure
> >>>> that submitter and author are the same in terms of name-email).
> >>>
> >>> The signed off is there. Not sure what you are referring to.
> >>
> >> Nope. It's not. The sign of that is the From: line in the body of the
> >> email. It happens when the submitter != author. And SoB of the former
> >> one is absent. But what is strange is that reading them here I haven't
> >> found the difference. Maybe one is in UTF-8 while the other is not and
> >> a unicode character degraded to Latin-1 or so?
> >>
> >
> > I have no idea why there is an additional From:, but both From:
> > tags in the e-mail source are exact matches, and both match the
> > name and e-mail address in Signed-off-by:. I agree with Jan,
> > the SoB is there.
>
> There is one unknown in this equation, and that is the anti-email system
> operated by a our IT and some company in Redmond.
Hmm... The From: in the body is the result of the `git format-patch` I believe.
So, two (or more?) possibilities here:
1) your configuration enforces it to always put From: (something new to me);
2) the submitter and author are not the same (see also:
https://github.com/git/git/commit/a90804752f6ab2b911882d47fafb6c2b78f447c3);
3) ...anything else...?
> But I haven't received
> any complaints that my outgoing emails are negatively affected by it
> (incoming are, but that's a different story...). If you received
> something mangled, Andy, please share the source of that email. I'm
> happy to escalate internally - and externally.
I believe I see it in the same way as lore, i.e.
https://lore.kernel.org/linux-watchdog/[email protected]/raw
> For the potential case they were mangled or in case I'm submitting via a
> real email provider, my scripts always add a "From:" to the body of my
> patches. Outgoing, that From matched my Signed-off.
--
With Best Regards,
Andy Shevchenko
On 26.07.21 16:51, Andy Shevchenko wrote:
> On Mon, Jul 26, 2021 at 5:05 PM Jan Kiszka <[email protected]> wrote:
>>
>> On 26.07.21 15:59, Guenter Roeck wrote:
>>> On 7/26/21 6:40 AM, Andy Shevchenko wrote:
>>>> On Mon, Jul 26, 2021 at 3:04 PM Jan Kiszka <[email protected]>
>>>> wrote:
>>>>>
>>>>> On 26.07.21 14:01, Andy Shevchenko wrote:
>>>>>> On Mon, Jul 26, 2021 at 2:46 PM Jan Kiszka <[email protected]>
>>>>>> wrote:
>>>>>>>
>>>>>>> From: Jan Kiszka <[email protected]>
>>>>>>>
>>>>>>> Obviously, the test needs to run against the register content, not its
>>>>>>> address.
>>>>>>>
>>>>>>> Fixes: cb011044e34c ("watchdog: iTCO_wdt: Account for rebooting on
>>>>>>> second timeout")
>>>>>>> Reported-by: Mantas Mikulėnas <[email protected]>
>>>>>>
>>>>>>> Signed-off-by: Jan Kiszka <[email protected]>
>>>>>>
>>>>>> Missed SoB of the submitter (hint: configure your Git to make sure
>>>>>> that submitter and author are the same in terms of name-email).
>>>>>
>>>>> The signed off is there. Not sure what you are referring to.
>>>>
>>>> Nope. It's not. The sign of that is the From: line in the body of the
>>>> email. It happens when the submitter != author. And SoB of the former
>>>> one is absent. But what is strange is that reading them here I haven't
>>>> found the difference. Maybe one is in UTF-8 while the other is not and
>>>> a unicode character degraded to Latin-1 or so?
>>>>
>>>
>>> I have no idea why there is an additional From:, but both From:
>>> tags in the e-mail source are exact matches, and both match the
>>> name and e-mail address in Signed-off-by:. I agree with Jan,
>>> the SoB is there.
>>
>> There is one unknown in this equation, and that is the anti-email system
>> operated by a our IT and some company in Redmond.
>
> Hmm... The From: in the body is the result of the `git format-patch` I believe.
> So, two (or more?) possibilities here:
> 1) your configuration enforces it to always put From: (something new to me);
Yes, it does, as I explained in my other reply. That's a safety net
because you never have full control over what some mail servers do to
the first From.
> 2) the submitter and author are not the same (see also:
> https://github.com/git/git/commit/a90804752f6ab2b911882d47fafb6c2b78f447c3);
> 3) ...anything else...?
>
>> But I haven't received
>> any complaints that my outgoing emails are negatively affected by it
>> (incoming are, but that's a different story...). If you received
>> something mangled, Andy, please share the source of that email. I'm
>> happy to escalate internally - and externally.
>
> I believe I see it in the same way as lore, i.e.
> https://lore.kernel.org/linux-watchdog/[email protected]/raw
Perfect, then all is fine as it should be (and no time for O365 bashing,
today).
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
On Mon, Jul 26, 2021 at 2:46 PM Jan Kiszka <[email protected]> wrote:
>
> From: Jan Kiszka <[email protected]>
>
> Obviously, the test needs to run against the register content, not its
> address.
>
> Fixes: cb011044e34c ("watchdog: iTCO_wdt: Account for rebooting on second timeout")
> Reported-by: Mantas Mikulėnas <[email protected]>
> Signed-off-by: Jan Kiszka <[email protected]>
> ---
> drivers/watchdog/iTCO_wdt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> index b3f604669e2c..643c6c2d0b72 100644
> --- a/drivers/watchdog/iTCO_wdt.c
> +++ b/drivers/watchdog/iTCO_wdt.c
> @@ -362,7 +362,7 @@ static int iTCO_wdt_set_timeout(struct watchdog_device *wd_dev, unsigned int t)
> * Otherwise, the BIOS generally reboots when the SMI triggers.
> */
> if (p->smi_res &&
> - (SMI_EN(p) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
> + (inl(SMI_EN(p)) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
> tmrval /= 2;
>
> /* from the specs: */
> --
> 2.26.2
Tested-by: Mantas Mikulėnas <[email protected]>
--
Mantas Mikulėnas
On Mon, Jul 26, 2021 at 8:10 PM Jan Kiszka <[email protected]> wrote:
> On 26.07.21 16:51, Andy Shevchenko wrote:
> > On Mon, Jul 26, 2021 at 5:05 PM Jan Kiszka <[email protected]> wrote:
> >> On 26.07.21 15:59, Guenter Roeck wrote:
> >>> On 7/26/21 6:40 AM, Andy Shevchenko wrote:
...
> >>> I have no idea why there is an additional From:, but both From:
> >>> tags in the e-mail source are exact matches, and both match the
> >>> name and e-mail address in Signed-off-by:. I agree with Jan,
> >>> the SoB is there.
> >>
> >> There is one unknown in this equation, and that is the anti-email system
> >> operated by a our IT and some company in Redmond.
> >
> > Hmm... The From: in the body is the result of the `git format-patch` I believe.
> > So, two (or more?) possibilities here:
> > 1) your configuration enforces it to always put From: (something new to me);
>
> Yes, it does, as I explained in my other reply. That's a safety net
> because you never have full control over what some mail servers do to
> the first From.
I see now. Thanks for your patience and elaboration, it's all good then!
> > 2) the submitter and author are not the same (see also:
> > https://github.com/git/git/commit/a90804752f6ab2b911882d47fafb6c2b78f447c3);
> > 3) ...anything else...?
--
With Best Regards,
Andy Shevchenko
On 26.07.21 13:46, Jan Kiszka wrote:
> From: Jan Kiszka <[email protected]>
>
> Obviously, the test needs to run against the register content, not its
> address.
>
> Fixes: cb011044e34c ("watchdog: iTCO_wdt: Account for rebooting on second timeout")
> Reported-by: Mantas Mikulėnas <[email protected]>
> Signed-off-by: Jan Kiszka <[email protected]>
> ---
> drivers/watchdog/iTCO_wdt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> index b3f604669e2c..643c6c2d0b72 100644
> --- a/drivers/watchdog/iTCO_wdt.c
> +++ b/drivers/watchdog/iTCO_wdt.c
> @@ -362,7 +362,7 @@ static int iTCO_wdt_set_timeout(struct watchdog_device *wd_dev, unsigned int t)
> * Otherwise, the BIOS generally reboots when the SMI triggers.
> */
> if (p->smi_res &&
> - (SMI_EN(p) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
> + (inl(SMI_EN(p)) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
> tmrval /= 2;
>
> /* from the specs: */
>
Ping, this is still missing in master. Stable kernels had the revert,
but 5.14 will need this.
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
On 20.08.21 15:45, Jan Kiszka wrote:
> On 26.07.21 13:46, Jan Kiszka wrote:
>> From: Jan Kiszka <[email protected]>
>>
>> Obviously, the test needs to run against the register content, not its
>> address.
>>
>> Fixes: cb011044e34c ("watchdog: iTCO_wdt: Account for rebooting on second timeout")
>> Reported-by: Mantas Mikulėnas <[email protected]>
>> Signed-off-by: Jan Kiszka <[email protected]>
>> ---
>> drivers/watchdog/iTCO_wdt.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
>> index b3f604669e2c..643c6c2d0b72 100644
>> --- a/drivers/watchdog/iTCO_wdt.c
>> +++ b/drivers/watchdog/iTCO_wdt.c
>> @@ -362,7 +362,7 @@ static int iTCO_wdt_set_timeout(struct watchdog_device *wd_dev, unsigned int t)
>> * Otherwise, the BIOS generally reboots when the SMI triggers.
>> */
>> if (p->smi_res &&
>> - (SMI_EN(p) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
>> + (inl(SMI_EN(p)) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
>> tmrval /= 2;
>>
>> /* from the specs: */
>>
>
> Ping, this is still missing in master. Stable kernels had the revert,
> but 5.14 will need this.
>
Second reminder: 5.14 is out and now broken. Is the patch queued
somewhere? I do not see it in the watchdog staging branch.
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
On 8/30/21 12:47 PM, Jan Kiszka wrote:
> On 20.08.21 15:45, Jan Kiszka wrote:
>> On 26.07.21 13:46, Jan Kiszka wrote:
>>> From: Jan Kiszka <[email protected]>
>>>
>>> Obviously, the test needs to run against the register content, not its
>>> address.
>>>
>>> Fixes: cb011044e34c ("watchdog: iTCO_wdt: Account for rebooting on second timeout")
>>> Reported-by: Mantas Mikulėnas <[email protected]>
>>> Signed-off-by: Jan Kiszka <[email protected]>
>>> ---
>>> drivers/watchdog/iTCO_wdt.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
>>> index b3f604669e2c..643c6c2d0b72 100644
>>> --- a/drivers/watchdog/iTCO_wdt.c
>>> +++ b/drivers/watchdog/iTCO_wdt.c
>>> @@ -362,7 +362,7 @@ static int iTCO_wdt_set_timeout(struct watchdog_device *wd_dev, unsigned int t)
>>> * Otherwise, the BIOS generally reboots when the SMI triggers.
>>> */
>>> if (p->smi_res &&
>>> - (SMI_EN(p) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
>>> + (inl(SMI_EN(p)) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
>>> tmrval /= 2;
>>>
>>> /* from the specs: */
>>>
>>
>> Ping, this is still missing in master. Stable kernels had the revert,
>> but 5.14 will need this.
>>
>
> Second reminder: 5.14 is out and now broken. Is the patch queued
> somewhere? I do not see it in the watchdog staging branch.
>
I had it in my own watchdog-next branch for about a month.
Usually Wim picks it up from there or from the mainling list;
he handles all upstreaming. Wim ?
Guenter
Hi All,
> On 8/30/21 12:47 PM, Jan Kiszka wrote:
> >On 20.08.21 15:45, Jan Kiszka wrote:
> >>On 26.07.21 13:46, Jan Kiszka wrote:
> >>>From: Jan Kiszka <[email protected]>
> >>>
> >>>Obviously, the test needs to run against the register content, not its
> >>>address.
> >>>
> >>>Fixes: cb011044e34c ("watchdog: iTCO_wdt: Account for rebooting on second timeout")
> >>>Reported-by: Mantas Mikulėnas <[email protected]>
> >>>Signed-off-by: Jan Kiszka <[email protected]>
> >>>---
> >>> drivers/watchdog/iTCO_wdt.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>>diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> >>>index b3f604669e2c..643c6c2d0b72 100644
> >>>--- a/drivers/watchdog/iTCO_wdt.c
> >>>+++ b/drivers/watchdog/iTCO_wdt.c
> >>>@@ -362,7 +362,7 @@ static int iTCO_wdt_set_timeout(struct watchdog_device *wd_dev, unsigned int t)
> >>> * Otherwise, the BIOS generally reboots when the SMI triggers.
> >>> */
> >>> if (p->smi_res &&
> >>>- (SMI_EN(p) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
> >>>+ (inl(SMI_EN(p)) & (TCO_EN | GBL_SMI_EN)) != (TCO_EN | GBL_SMI_EN))
> >>> tmrval /= 2;
> >>> /* from the specs: */
> >>>
> >>
> >>Ping, this is still missing in master. Stable kernels had the revert,
> >>but 5.14 will need this.
> >>
> >
> >Second reminder: 5.14 is out and now broken. Is the patch queued
> >somewhere? I do not see it in the watchdog staging branch.
> >
>
> I had it in my own watchdog-next branch for about a month.
> Usually Wim picks it up from there or from the mainling list;
> he handles all upstreaming. Wim ?
This one is in linux-watchdog-next since 22 Aug.
Working on getting it upstream now.
Kind regards,
Wim.