Re: [Watchdog drivers] - WDIOC_GETSUPPORT clarification

Re: [Watchdog drivers] - WDIOC_GETSUPPORT clarification

[Hans de Goede]

Hi,

On 8/18/21 11:57 AM, Jose Noguera wrote:
> Hello all!
> 
> I’m Jose, working in Red Hat on Fedora IoT and RHEL For Edge related projects.
> 
> While trying to add the feature of figuring out whether the current boot was triggered or not by a hardware watchdog using wdctl, we’ve found an odd behaviour in the drivers definition that we would like to check with you.
> 
> Looking specifically for the flag WDIOF_CARDRESET, we can find 40 files that contain it:
> 
> $ grep -rl CARDRESET drivers/watchdog/ | wc -l
> 40
> 
> but only 19 of them have the flag advertised in the options field of the watchdog_info struct returned by the WDIOC_GETSUPPORT ioctl.
> 
> This leads to wdctl not showing WDIOF_CARDRESET for drivers like i6300esb, even when the board had been reset this way:
> 
> $ sudo wdctl
> Device: /dev/watchdog0
> Identity: i6300ESB timer [version 0]
> Timeout: 30 seconds
> Pre-timeout: 0 seconds
> 
> FLAG DESCRIPTION STATUS BOOT-STATUS
> KEEPALIVEPING Keep alive ping reply 1 0
> MAGICCLOSE Supports magic close char 0 0
> SETTIMEOUT Set timeout (in seconds) 0 0
> 
> Working with Hans (in CC), we added a little patch (BugZilla ticket: 1993983) on wdctl

To be clear, Jose is talking about this bugzilla:
https://bugzilla.redhat.com/show_bug.cgi?id=1993983

And this is the util-linux commit with the little workaround-patch:
https://github.com/karelzak/util-linux/commit/b1b0259fe42aad1bf0997ce1c03a020ce59e38ab

> and this would be the output of the command when it was a card reset triggered boot:
> 
> $ sudo wdctl
> Device: /dev/watchdog0
> Identity: i6300ESB timer [version 0]
> Timeout: 30 seconds
> Pre-timeout: 0 seconds
> 
> FLAG DESCRIPTION STATUS BOOT-STATUS
> CARDRESET Card previously reset the CPU 1 1
> KEEPALIVEPING Keep alive ping reply 1 0
> MAGICCLOSE Supports magic close char 0 0
> SETTIMEOUT Set timeout (in seconds) 0 0
> 
> So our question is, may we know what is intended to be present in ident.options? What should the API call WDIOC_GETSUPPORT return in the options field?

Regards,

Hans

Re: [Watchdog drivers] - WDIOC_GETSUPPORT clarification

[Guenter Roeck]

On 8/18/21 2:57 AM, Jose Noguera wrote:
> Hello all!
> 
> I’m Jose, working in Red Hat on Fedora IoT and RHEL For Edge related projects.
> 
> While trying to add the feature of figuring out whether the current boot was triggered or not by a hardware watchdog using wdctl, we’ve found an odd behaviour in the drivers definition that we would like to check with you.
> 
> Looking specifically for the flag WDIOF_CARDRESET, we can find 40 files that contain it:
> 
> $ grep -rl CARDRESET drivers/watchdog/ | wc -l
> 40
> 
> but only 19 of them have the flag advertised in the options field of the watchdog_info struct returned by the WDIOC_GETSUPPORT ioctl.
> 
> This leads to wdctl not showing WDIOF_CARDRESET for drivers like i6300esb, even when the board had been reset this way:
> 
> $ sudo wdctl
> Device: /dev/watchdog0
> Identity: i6300ESB timer [version 0]
> Timeout: 30 seconds
> Pre-timeout: 0 seconds
> 
> FLAG DESCRIPTION STATUS BOOT-STATUS
> KEEPALIVEPING Keep alive ping reply 1 0
> MAGICCLOSE Supports magic close char 0 0
> SETTIMEOUT Set timeout (in seconds) 0 0
> 
> Working with Hans (in CC), we added a little patch (BugZilla ticket: 1993983) on wdctl and this would be the output of the command when it was a card reset triggered boot:
> 
> $ sudo wdctl
> Device: /dev/watchdog0
> Identity: i6300ESB timer [version 0]
> Thank you all for your time,
> 
> Jose

> Timeout: 30 seconds
> Pre-timeout: 0 seconds
> 
> FLAG DESCRIPTION STATUS BOOT-STATUS
> CARDRESET Card previously reset the CPU 1 1
> KEEPALIVEPING Keep alive ping reply 1 0
> MAGICCLOSE Supports magic close char 0 0
> SETTIMEOUT Set timeout (in seconds) 0 0
> 
> So our question is, may we know what is intended to be present in ident.options? What should the API call WDIOC_GETSUPPORT return in the options field?
> 

Search for WDIOC_GETSUPPORT in Documentation/watchdog/watchdog-api.rst.
I don't see any ambiguity there. Patches welcome.

Guenter

Re: [Watchdog drivers] - WDIOC_GETSUPPORT clarification

[Hans de Goede]

Hi,

On 8/18/21 4:13 PM, Guenter Roeck wrote:
> On 8/18/21 2:57 AM, Jose Noguera wrote:
>> Hello all!
>>
>> I’m Jose, working in Red Hat on Fedora IoT and RHEL For Edge related projects.
>>
>> While trying to add the feature of figuring out whether the current boot was triggered or not by a hardware watchdog using wdctl, we’ve found an odd behaviour in the drivers definition that we would like to check with you.
>>
>> Looking specifically for the flag WDIOF_CARDRESET, we can find 40 files that contain it:
>>
>> $ grep -rl CARDRESET drivers/watchdog/ | wc -l
>> 40
>>
>> but only 19 of them have the flag advertised in the options field of the watchdog_info struct returned by the WDIOC_GETSUPPORT ioctl.
>>
>> This leads to wdctl not showing WDIOF_CARDRESET for drivers like i6300esb, even when the board had been reset this way:
>>
>> $ sudo wdctl
>> Device: /dev/watchdog0
>> Identity: i6300ESB timer [version 0]
>> Timeout: 30 seconds
>> Pre-timeout: 0 seconds
>>
>> FLAG DESCRIPTION STATUS BOOT-STATUS
>> KEEPALIVEPING Keep alive ping reply 1 0
>> MAGICCLOSE Supports magic close char 0 0
>> SETTIMEOUT Set timeout (in seconds) 0 0
>>
>> Working with Hans (in CC), we added a little patch (BugZilla ticket: 1993983) on wdctl and this would be the output of the command when it was a card reset triggered boot:
>>
>> $ sudo wdctl
>> Device: /dev/watchdog0
>> Identity: i6300ESB timer [version 0]
>> Thank you all for your time,
>>
>> Jose
> 
>> Timeout: 30 seconds
>> Pre-timeout: 0 seconds
>>
>> FLAG DESCRIPTION STATUS BOOT-STATUS
>> CARDRESET Card previously reset the CPU 1 1
>> KEEPALIVEPING Keep alive ping reply 1 0
>> MAGICCLOSE Supports magic close char 0 0
>> SETTIMEOUT Set timeout (in seconds) 0 0
>>
>> So our question is, may we know what is intended to be present in ident.options? What should the API call WDIOC_GETSUPPORT return in the options field?
>>
> 
> Search for WDIOC_GETSUPPORT in Documentation/watchdog/watchdog-api.rst.
> I don't see any ambiguity there. Patches welcome.

Ok, so drivers which may set CARDRESET in their GETBOOTSTATUS reply, but
don't advertise this in their GETSUPPORT watchdog_info.options reply
are buggy and should be fixed, got it. Thanks.

I've made a note about fixing this in a possible-kernel-projects document
which I keep for when people who are interested in kernel development
ask me for projects.

Regards,

Hans

Re: [Watchdog drivers] - WDIOC_GETSUPPORT clarification

[Guenter Roeck]

On 8/18/21 7:49 AM, Hans de Goede wrote:
> Hi,
> 
> On 8/18/21 4:13 PM, Guenter Roeck wrote:
>> On 8/18/21 2:57 AM, Jose Noguera wrote:
>>> Hello all!
>>>
>>> I’m Jose, working in Red Hat on Fedora IoT and RHEL For Edge related projects.
>>>
>>> While trying to add the feature of figuring out whether the current boot was triggered or not by a hardware watchdog using wdctl, we’ve found an odd behaviour in the drivers definition that we would like to check with you.
>>>
>>> Looking specifically for the flag WDIOF_CARDRESET, we can find 40 files that contain it:
>>>
>>> $ grep -rl CARDRESET drivers/watchdog/ | wc -l
>>> 40
>>>
>>> but only 19 of them have the flag advertised in the options field of the watchdog_info struct returned by the WDIOC_GETSUPPORT ioctl.
>>>
>>> This leads to wdctl not showing WDIOF_CARDRESET for drivers like i6300esb, even when the board had been reset this way:
>>>
>>> $ sudo wdctl
>>> Device: /dev/watchdog0
>>> Identity: i6300ESB timer [version 0]
>>> Timeout: 30 seconds
>>> Pre-timeout: 0 seconds
>>>
>>> FLAG DESCRIPTION STATUS BOOT-STATUS
>>> KEEPALIVEPING Keep alive ping reply 1 0
>>> MAGICCLOSE Supports magic close char 0 0
>>> SETTIMEOUT Set timeout (in seconds) 0 0
>>>
>>> Working with Hans (in CC), we added a little patch (BugZilla ticket: 1993983) on wdctl and this would be the output of the command when it was a card reset triggered boot:
>>>
>>> $ sudo wdctl
>>> Device: /dev/watchdog0
>>> Identity: i6300ESB timer [version 0]
>>> Thank you all for your time,
>>>
>>> Jose
>>
>>> Timeout: 30 seconds
>>> Pre-timeout: 0 seconds
>>>
>>> FLAG DESCRIPTION STATUS BOOT-STATUS
>>> CARDRESET Card previously reset the CPU 1 1
>>> KEEPALIVEPING Keep alive ping reply 1 0
>>> MAGICCLOSE Supports magic close char 0 0
>>> SETTIMEOUT Set timeout (in seconds) 0 0
>>>
>>> So our question is, may we know what is intended to be present in ident.options? What should the API call WDIOC_GETSUPPORT return in the options field?
>>>
>>
>> Search for WDIOC_GETSUPPORT in Documentation/watchdog/watchdog-api.rst.
>> I don't see any ambiguity there. Patches welcome.
> 
> Ok, so drivers which may set CARDRESET in their GETBOOTSTATUS reply, but
> don't advertise this in their GETSUPPORT watchdog_info.options reply
> are buggy and should be fixed, got it. Thanks.
> 
> I've made a note about fixing this in a possible-kernel-projects document
> which I keep for when people who are interested in kernel development
> ask me for projects.
> 

With an add-on: If the driver in question is an old-style driver, anyone
affected by the problem should really convert the driver to support
the watchdog subsystem.

Thanks,
Guenter

> Regards,
> 
> Hans
> 

Re: [Watchdog drivers] - WDIOC_GETSUPPORT clarification

[Peter Robinson]

On Wed, 2021-08-18 at 07:53 -0700, Guenter Roeck wrote:
> On 8/18/21 7:49 AM, Hans de Goede wrote:
> > Hi,
> > 
> > On 8/18/21 4:13 PM, Guenter Roeck wrote:
> > > On 8/18/21 2:57 AM, Jose Noguera wrote:
> > > > Hello all!
> > > > 
> > > > I’m Jose, working in Red Hat on Fedora IoT and RHEL For Edge
> > > > related projects.
> > > > 
> > > > While trying to add the feature of figuring out whether the
> > > > current boot was triggered or not by a hardware watchdog using
> > > > wdctl, we’ve found an odd behaviour in the drivers definition
> > > > that we would like to check with you.
> > > > 
> > > > Looking specifically for the flag WDIOF_CARDRESET, we can find
> > > > 40 files that contain it:
> > > > 
> > > > $ grep -rl CARDRESET drivers/watchdog/ | wc -l
> > > > 40
> > > > 
> > > > but only 19 of them have the flag advertised in the options
> > > > field of the watchdog_info struct returned by the
> > > > WDIOC_GETSUPPORT ioctl.
> > > > 
> > > > This leads to wdctl not showing WDIOF_CARDRESET for drivers
> > > > like i6300esb, even when the board had been reset this way:
> > > > 
> > > > $ sudo wdctl
> > > > Device: /dev/watchdog0
> > > > Identity: i6300ESB timer [version 0]
> > > > Timeout: 30 seconds
> > > > Pre-timeout: 0 seconds
> > > > 
> > > > FLAG DESCRIPTION STATUS BOOT-STATUS
> > > > KEEPALIVEPING Keep alive ping reply 1 0
> > > > MAGICCLOSE Supports magic close char 0 0
> > > > SETTIMEOUT Set timeout (in seconds) 0 0
> > > > 
> > > > Working with Hans (in CC), we added a little patch (BugZilla
> > > > ticket: 1993983) on wdctl and this would be the output of the
> > > > command when it was a card reset triggered boot:
> > > > 
> > > > $ sudo wdctl
> > > > Device: /dev/watchdog0
> > > > Identity: i6300ESB timer [version 0]
> > > > Thank you all for your time,
> > > > 
> > > > Jose
> > > 
> > > > Timeout: 30 seconds
> > > > Pre-timeout: 0 seconds
> > > > 
> > > > FLAG DESCRIPTION STATUS BOOT-STATUS
> > > > CARDRESET Card previously reset the CPU 1 1
> > > > KEEPALIVEPING Keep alive ping reply 1 0
> > > > MAGICCLOSE Supports magic close char 0 0
> > > > SETTIMEOUT Set timeout (in seconds) 0 0
> > > > 
> > > > So our question is, may we know what is intended to be present
> > > > in ident.options? What should the API call WDIOC_GETSUPPORT
> > > > return in the options field?
> > > > 
> > > 
> > > Search for WDIOC_GETSUPPORT in Documentation/watchdog/watchdog-
> > > api.rst.
> > > I don't see any ambiguity there. Patches welcome.
> > 
> > Ok, so drivers which may set CARDRESET in their GETBOOTSTATUS
> > reply, but
> > don't advertise this in their GETSUPPORT watchdog_info.options
> > reply
> > are buggy and should be fixed, got it. Thanks.
> > 
> > I've made a note about fixing this in a possible-kernel-projects
> > document
> > which I keep for when people who are interested in kernel
> > development
> > ask me for projects.
> > 
> 
> With an add-on: If the driver in question is an old-style driver,
> anyone
> affected by the problem should really convert the driver to support
> the watchdog subsystem.

Would you have a link to any docs/posts outlining what needs to be done
for conversion to the watchdog subsystem?

Regards,
Peter

Re: [Watchdog drivers] - WDIOC_GETSUPPORT clarification

[Guenter Roeck]

On Wed, Sep 01, 2021 at 12:15:23PM +0100, Peter Robinson wrote:
> On Wed, 2021-08-18 at 07:53 -0700, Guenter Roeck wrote:
> > On 8/18/21 7:49 AM, Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 8/18/21 4:13 PM, Guenter Roeck wrote:
> > > > On 8/18/21 2:57 AM, Jose Noguera wrote:
> > > > > Hello all!
> > > > > 
> > > > > I’m Jose, working in Red Hat on Fedora IoT and RHEL For Edge
> > > > > related projects.
> > > > > 
> > > > > While trying to add the feature of figuring out whether the
> > > > > current boot was triggered or not by a hardware watchdog using
> > > > > wdctl, we’ve found an odd behaviour in the drivers definition
> > > > > that we would like to check with you.
> > > > > 
> > > > > Looking specifically for the flag WDIOF_CARDRESET, we can find
> > > > > 40 files that contain it:
> > > > > 
> > > > > $ grep -rl CARDRESET drivers/watchdog/ | wc -l
> > > > > 40
> > > > > 
> > > > > but only 19 of them have the flag advertised in the options
> > > > > field of the watchdog_info struct returned by the
> > > > > WDIOC_GETSUPPORT ioctl.
> > > > > 
> > > > > This leads to wdctl not showing WDIOF_CARDRESET for drivers
> > > > > like i6300esb, even when the board had been reset this way:
> > > > > 
> > > > > $ sudo wdctl
> > > > > Device: /dev/watchdog0
> > > > > Identity: i6300ESB timer [version 0]
> > > > > Timeout: 30 seconds
> > > > > Pre-timeout: 0 seconds
> > > > > 
> > > > > FLAG DESCRIPTION STATUS BOOT-STATUS
> > > > > KEEPALIVEPING Keep alive ping reply 1 0
> > > > > MAGICCLOSE Supports magic close char 0 0
> > > > > SETTIMEOUT Set timeout (in seconds) 0 0
> > > > > 
> > > > > Working with Hans (in CC), we added a little patch (BugZilla
> > > > > ticket: 1993983) on wdctl and this would be the output of the
> > > > > command when it was a card reset triggered boot:
> > > > > 
> > > > > $ sudo wdctl
> > > > > Device: /dev/watchdog0
> > > > > Identity: i6300ESB timer [version 0]
> > > > > Thank you all for your time,
> > > > > 
> > > > > Jose
> > > > 
> > > > > Timeout: 30 seconds
> > > > > Pre-timeout: 0 seconds
> > > > > 
> > > > > FLAG DESCRIPTION STATUS BOOT-STATUS
> > > > > CARDRESET Card previously reset the CPU 1 1
> > > > > KEEPALIVEPING Keep alive ping reply 1 0
> > > > > MAGICCLOSE Supports magic close char 0 0
> > > > > SETTIMEOUT Set timeout (in seconds) 0 0
> > > > > 
> > > > > So our question is, may we know what is intended to be present
> > > > > in ident.options? What should the API call WDIOC_GETSUPPORT
> > > > > return in the options field?
> > > > > 
> > > > 
> > > > Search for WDIOC_GETSUPPORT in Documentation/watchdog/watchdog-
> > > > api.rst.
> > > > I don't see any ambiguity there. Patches welcome.
> > > 
> > > Ok, so drivers which may set CARDRESET in their GETBOOTSTATUS
> > > reply, but
> > > don't advertise this in their GETSUPPORT watchdog_info.options
> > > reply
> > > are buggy and should be fixed, got it. Thanks.
> > > 
> > > I've made a note about fixing this in a possible-kernel-projects
> > > document
> > > which I keep for when people who are interested in kernel
> > > development
> > > ask me for projects.
> > > 
> > 
> > With an add-on: If the driver in question is an old-style driver,
> > anyone
> > affected by the problem should really convert the driver to support
> > the watchdog subsystem.
> 
> Would you have a link to any docs/posts outlining what needs to be done
> for conversion to the watchdog subsystem?

Documentation/watchdog/convert_drivers_to_kernel_api.rst should help.

Guenter