[PATCH] watchdog: iTCO_wdt: Fix detection of SMI-off case

[PATCH] watchdog: iTCO_wdt: Fix detection of SMI-off case

[Jan Kiszka]

From: Jan Kiszka <jan.kiszka@siemens.com>

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 <grawity@gmail.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 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

Re: [PATCH] watchdog: iTCO_wdt: Fix detection of SMI-off case

[Andy Shevchenko]

On Mon, Jul 26, 2021 at 2:46 PM Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> 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 <grawity@gmail.com>

> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

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

Re: [PATCH] watchdog: iTCO_wdt: Fix detection of SMI-off case

[Paolo Bonzini]

On 26/07/21 13:46, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> 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 <grawity@gmail.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>   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 <pbonzini@redhat.com>
Cc: stable@vger.kernel.org

(the latter because cb011044e34c has been picked up by stable kernels 
already).

Paolo

Re: [PATCH] watchdog: iTCO_wdt: Fix detection of SMI-off case

[Jan Kiszka]

On 26.07.21 14:01, Andy Shevchenko wrote:
> On Mon, Jul 26, 2021 at 2:46 PM Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> 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 <grawity@gmail.com>
> 
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> 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

Re: [PATCH] watchdog: iTCO_wdt: Fix detection of SMI-off case

[Jan Kiszka]

On 26.07.21 14:03, Paolo Bonzini wrote:
> On 26/07/21 13:46, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> 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 <grawity@gmail.com>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>   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 <pbonzini@redhat.com>
> Cc: stable@vger.kernel.org
> 
> (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

Re: [PATCH] watchdog: iTCO_wdt: Fix detection of SMI-off case

[Andy Shevchenko]

On Mon, Jul 26, 2021 at 3:04 PM Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> On 26.07.21 14:01, Andy Shevchenko wrote:
> > On Mon, Jul 26, 2021 at 2:46 PM Jan Kiszka <jan.kiszka@siemens.com> wrote:
> >>
> >> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>
> >> 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 <grawity@gmail.com>
> >
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >
> > 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

Re: [PATCH] watchdog: iTCO_wdt: Fix detection of SMI-off case

[Guenter Roeck]

On 7/26/21 4:46 AM, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> 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 <grawity@gmail.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

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

> ---
>   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: */
> 

Re: [PATCH] watchdog: iTCO_wdt: Fix detection of SMI-off case

[Guenter Roeck]

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 <jan.kiszka@siemens.com>
>>>
>>> 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 <grawity@gmail.com>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>>    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 <pbonzini@redhat.com>
>> Cc: stable@vger.kernel.org
>>
>> (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

Re: [PATCH] watchdog: iTCO_wdt: Fix detection of SMI-off case

[Guenter Roeck]

On 7/26/21 6:40 AM, Andy Shevchenko wrote:
> On Mon, Jul 26, 2021 at 3:04 PM Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>
>> On 26.07.21 14:01, Andy Shevchenko wrote:
>>> On Mon, Jul 26, 2021 at 2:46 PM Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>
>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>
>>>> 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 <grawity@gmail.com>
>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> 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

Re: [PATCH] watchdog: iTCO_wdt: Fix detection of SMI-off case

[Jan Kiszka]

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 <jan.kiszka@siemens.com>
>> wrote:
>>>
>>> On 26.07.21 14:01, Andy Shevchenko wrote:
>>>> On Mon, Jul 26, 2021 at 2:46 PM Jan Kiszka <jan.kiszka@siemens.com>
>>>> wrote:
>>>>>
>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>
>>>>> 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 <grawity@gmail.com>
>>>>
>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>
>>>> 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

Re: [PATCH] watchdog: iTCO_wdt: Fix detection of SMI-off case

[Andy Shevchenko]

On Mon, Jul 26, 2021 at 5:05 PM Jan Kiszka <jan.kiszka@siemens.com> 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 <jan.kiszka@siemens.com>
> >> wrote:
> >>>
> >>> On 26.07.21 14:01, Andy Shevchenko wrote:
> >>>> On Mon, Jul 26, 2021 at 2:46 PM Jan Kiszka <jan.kiszka@siemens.com>
> >>>> wrote:
> >>>>>
> >>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>>>>
> >>>>> 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 <grawity@gmail.com>
> >>>>
> >>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>>>
> >>>> 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/d84f8e06-f646-8b43-d063-fb11f4827044@siemens.com/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

Re: [PATCH] watchdog: iTCO_wdt: Fix detection of SMI-off case

[Mantas Mikulėnas]

On Mon, Jul 26, 2021 at 2:46 PM Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> 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 <grawity@gmail.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  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 <grawity@gmail.com>

-- 
Mantas Mikulėnas

Re: [PATCH] watchdog: iTCO_wdt: Fix detection of SMI-off case

[Jan Kiszka]

On 26.07.21 16:51, Andy Shevchenko wrote:
> On Mon, Jul 26, 2021 at 5:05 PM Jan Kiszka <jan.kiszka@siemens.com> 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 <jan.kiszka@siemens.com>
>>>> wrote:
>>>>>
>>>>> On 26.07.21 14:01, Andy Shevchenko wrote:
>>>>>> On Mon, Jul 26, 2021 at 2:46 PM Jan Kiszka <jan.kiszka@siemens.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>>
>>>>>>> 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 <grawity@gmail.com>
>>>>>>
>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>
>>>>>> 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/d84f8e06-f646-8b43-d063-fb11f4827044@siemens.com/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

Re: [PATCH] watchdog: iTCO_wdt: Fix detection of SMI-off case

[Andy Shevchenko]

On Mon, Jul 26, 2021 at 8:10 PM Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 26.07.21 16:51, Andy Shevchenko wrote:
> > On Mon, Jul 26, 2021 at 5:05 PM Jan Kiszka <jan.kiszka@siemens.com> 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

Re: [PATCH] watchdog: iTCO_wdt: Fix detection of SMI-off case

[Jan Kiszka]

On 26.07.21 13:46, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> 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 <grawity@gmail.com>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  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

Re: [PATCH] watchdog: iTCO_wdt: Fix detection of SMI-off case

[Jan Kiszka]

On 20.08.21 15:45, Jan Kiszka wrote:
> On 26.07.21 13:46, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> 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 <grawity@gmail.com>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  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

Re: [PATCH] watchdog: iTCO_wdt: Fix detection of SMI-off case

[Guenter Roeck]

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 <jan.kiszka@siemens.com>
>>>
>>> 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 <grawity@gmail.com>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>>   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

Re: [PATCH] watchdog: iTCO_wdt: Fix detection of SMI-off case

[Wim Van Sebroeck]

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 <jan.kiszka@siemens.com>
> >>>
> >>>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 <grawity@gmail.com>
> >>>Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>>---
> >>>  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.