Additional patches in my watchdog-next branch

Additional patches in my watchdog-next branch

[Guenter Roeck]

Hi Wim,

I have a number of patches pending in my watchdog-next branch which are not
in your watchdog-next repository. Below is a summary, rebased onto your
repository. This does not include the patches I had submitted.

Please consider those patches for v4.5.

Thanks,
Guenter

---

The following changes since commit 62e21f5d90f162208e43b8a248b01e1a4a24ecbd:

  watchdog: cadence_wdt: use to_platform_device() (2015-12-27 21:15:34 +0100)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git 

for you to fetch changes up to dffb1339276a535f54497a4d1520118a5e04fbc6:

  watchdog: atlas7: add watchdog driver of CSRatlas7 (2015-12-27 15:08:50 -0800)

----------------------------------------------------------------
Carlo Caione (2):
      watchdog: meson: Enable meson SoC specific data
      watchdog: meson: Add meson8b SoC specific data

Damien Riegel (1):
      watchdog: ts4800: add driver for TS-4800 watchdog

Guo Zeng (1):
      watchdog: atlas7: add watchdog driver of CSRatlas7

Mans Rullgard (1):
      watchdog: add support for Sigma Designs SMP86xx/SMP87xx

Martyn Welch (2):
      Add binding documentation for Zodiac Watchdog Timer
      watchdog: Zodiac Aerospace RAVE Switch Watchdog Processor Driver

Måns Rullgård (1):
      devicetree: watchdog: add binding for Sigma Designs SMP8642 watchdog

Oleksij Rempel (2):
      watchdog: add Alphascale asm9260-wdt driver
      DT: watchdog: add Alphascale asm9260 watchdog binding documentation.

 .../bindings/watchdog/alphascale-asm9260.txt       |  35 ++
 .../bindings/watchdog/sigma,smp8642-wdt.txt        |  18 +
 .../devicetree/bindings/watchdog/ts4800-wdt.txt    |  25 ++
 .../devicetree/bindings/watchdog/ziirave-wdt.txt   |  19 +
 drivers/watchdog/Kconfig                           |  51 +++
 drivers/watchdog/Makefile                          |   5 +
 drivers/watchdog/asm9260_wdt.c                     | 403 +++++++++++++++++++++
 drivers/watchdog/atlas7_wdt.c                      | 242 +++++++++++++
 drivers/watchdog/meson_wdt.c                       |  65 +++-
 drivers/watchdog/tangox_wdt.c                      | 225 ++++++++++++
 drivers/watchdog/ts4800_wdt.c                      | 215 +++++++++++
 drivers/watchdog/ziirave_wdt.c                     | 381 +++++++++++++++++++
 12 files changed, 1667 insertions(+), 17 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/watchdog/alphascale-asm9260.txt
 create mode 100644 Documentation/devicetree/bindings/watchdog/sigma,smp8642-wdt.txt
 create mode 100644 Documentation/devicetree/bindings/watchdog/ts4800-wdt.txt
 create mode 100644 Documentation/devicetree/bindings/watchdog/ziirave-wdt.txt
 create mode 100644 drivers/watchdog/asm9260_wdt.c
 create mode 100644 drivers/watchdog/atlas7_wdt.c
 create mode 100644 drivers/watchdog/tangox_wdt.c
 create mode 100644 drivers/watchdog/ts4800_wdt.c
 create mode 100644 drivers/watchdog/ziirave_wdt.c
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Additional patches in my watchdog-next branch

[Wim Van Sebroeck]

Hi Guneter,

> I have a number of patches pending in my watchdog-next branch which are not
> in your watchdog-next repository. Below is a summary, rebased onto your
> repository. This does not include the patches I had submitted.
> 
> Please consider those patches for v4.5.

I'm still working on those. Will be for this evening. I left those because they are all new drivers. I wanted to have all fixes and other patches first.
So not finished yet :-) I will also add your dev fixes (V2 patchset containing 5 patches).

Kind regards,
Wim.

Re: Additional patches in my watchdog-next branch

[Guenter Roeck]

On Mon, Dec 28, 2015 at 04:27:11PM +0100, Wim Van Sebroeck wrote:
> Hi Guneter,
> 
> > I have a number of patches pending in my watchdog-next branch which are not
> > in your watchdog-next repository. Below is a summary, rebased onto your
> > repository. This does not include the patches I had submitted.
> > 
> > Please consider those patches for v4.5.
> 
> I'm still working on those. Will be for this evening. I left those because they are all new drivers. I wanted to have all fixes and other patches first.
> So not finished yet :-) I will also add your dev fixes (V2 patchset containing 5 patches).
> 
Thanks!

Guenter

Re: Additional patches in my watchdog-next branch

[Wim Van Sebroeck]

Hi Guenter,

> On Mon, Dec 28, 2015 at 04:27:11PM +0100, Wim Van Sebroeck wrote:
> > Hi Guneter,
> > 
> > > I have a number of patches pending in my watchdog-next branch which are not
> > > in your watchdog-next repository. Below is a summary, rebased onto your
> > > repository. This does not include the patches I had submitted.
> > > 
> > > Please consider those patches for v4.5.
> > 
> > I'm still working on those. Will be for this evening. I left those because they are all new drivers. I wanted to have all fixes and other patches first.
> > So not finished yet :-) I will also add your dev fixes (V2 patchset containing 5 patches).
> > 
> Thanks!

No problem. I think we now have all patches that should go in for v4.5 .

There is off course still other stuff that I need to look at:
SBSA, pretimeout, bcm63xx, stmp3xxx GETBOOTSTATUS (although I have the impression that this patch is a dead end because you cannot really tell what the reboot reason is with the patch), soft timer's moving into the core, mei watchdog code, ...

Kind regards,
Wim.

Re: Additional patches in my watchdog-next branch

[Vladimir Zapolskiy]

Hi Wim,

On 29.12.2015 00:54, Wim Van Sebroeck wrote:
> Hi Guenter,
> 
>> On Mon, Dec 28, 2015 at 04:27:11PM +0100, Wim Van Sebroeck wrote:
>>> Hi Guneter,
>>>
>>>> I have a number of patches pending in my watchdog-next branch which are not
>>>> in your watchdog-next repository. Below is a summary, rebased onto your
>>>> repository. This does not include the patches I had submitted.
>>>>
>>>> Please consider those patches for v4.5.
>>>
>>> I'm still working on those. Will be for this evening. I left those because they are all new drivers. I wanted to have all fixes and other patches first.
>>> So not finished yet :-) I will also add your dev fixes (V2 patchset containing 5 patches).
>>>
>> Thanks!
> 
> No problem. I think we now have all patches that should go in for v4.5 .
> 
> There is off course still other stuff that I need to look at:
> SBSA, pretimeout, bcm63xx, stmp3xxx GETBOOTSTATUS (although I have the impression that this patch is a dead end because you cannot really tell what the reboot reason is with the patch), soft timer's moving into the core, mei watchdog code, ...
> 

please let me know, if you want to see the proposed pretimeout change rebased earlier, at the moment it may have merge conflicts with your watchdog/master branch. In any case I would appreciate to get your review comments :)

--
With best wishes,
Vladimir

Re: Additional patches in my watchdog-next branch

[Guenter Roeck]

On Mon, Dec 28, 2015 at 11:54:27PM +0100, Wim Van Sebroeck wrote:
> Hi Guenter,
> 
> > On Mon, Dec 28, 2015 at 04:27:11PM +0100, Wim Van Sebroeck wrote:
> > > Hi Guneter,
> > > 
> > > > I have a number of patches pending in my watchdog-next branch which are not
> > > > in your watchdog-next repository. Below is a summary, rebased onto your
> > > > repository. This does not include the patches I had submitted.
> > > > 
> > > > Please consider those patches for v4.5.
> > > 
> > > I'm still working on those. Will be for this evening. I left those because they are all new drivers. I wanted to have all fixes and other patches first.
> > > So not finished yet :-) I will also add your dev fixes (V2 patchset containing 5 patches).
> > > 
> > Thanks!
> 
> No problem. I think we now have all patches that should go in for v4.5 .
> 
> There is off course still other stuff that I need to look at:
> SBSA, pretimeout, bcm63xx, stmp3xxx GETBOOTSTATUS (although I have the impression that this patch is a dead end because you cannot really tell what the reboot reason is with the patch), soft timer's moving into the core, mei watchdog code, ...
Hi Wim,

I'll have to look into those again as well. Most will now conflict
with the new watchdog core code.

Any thoughts on the other patch series I sent out, to create the watchdog
device in watchdog_dev.c, and to handle reference counting in the watchdog
core ? That would be a prerequisite for me to rebase (and rework) the soft
timer handling core. I had hoped that the changes to handle reference
counting in the core could make it into 4.5.

Thanks,
Guenter

Re: Additional patches in my watchdog-next branch

[Wim Van Sebroeck]

Hi Vladimir,

> Hi Wim,
> 
> On 29.12.2015 00:54, Wim Van Sebroeck wrote:
> > Hi Guenter,
> > 
> >> On Mon, Dec 28, 2015 at 04:27:11PM +0100, Wim Van Sebroeck wrote:
> >>> Hi Guneter,
> >>>
> >>>> I have a number of patches pending in my watchdog-next branch which are not
> >>>> in your watchdog-next repository. Below is a summary, rebased onto your
> >>>> repository. This does not include the patches I had submitted.
> >>>>
> >>>> Please consider those patches for v4.5.
> >>>
> >>> I'm still working on those. Will be for this evening. I left those because they are all new drivers. I wanted to have all fixes and other patches first.
> >>> So not finished yet :-) I will also add your dev fixes (V2 patchset containing 5 patches).
> >>>
> >> Thanks!
> > 
> > No problem. I think we now have all patches that should go in for v4.5 .
> > 
> > There is off course still other stuff that I need to look at:
> > SBSA, pretimeout, bcm63xx, stmp3xxx GETBOOTSTATUS (although I have the impression that this patch is a dead end because you cannot really tell what the reboot reason is with the patch), soft timer's moving into the core, mei watchdog code, ...
> > 
> 
> please let me know, if you want to see the proposed pretimeout change rebased earlier, at the moment it may have merge conflicts with your watchdog/master branch. In any case I would appreciate to get your review comments :)

Pretimeout will not go in during the next merge window. It's for 4.6 at the earliest.
But I will come back on this.

SBSA: What should be done is to create a correct driver without pretimeout and with just the normal watchdog (=reboot if we aren't kicked fast enough) behaviour and get that in.

Kind regards,
Wim.

Re: Additional patches in my watchdog-next branch

[Wim Van Sebroeck]

Hi Guenter,

> On Mon, Dec 28, 2015 at 11:54:27PM +0100, Wim Van Sebroeck wrote:
> > Hi Guenter,
> > 
> > > On Mon, Dec 28, 2015 at 04:27:11PM +0100, Wim Van Sebroeck wrote:
> > > > Hi Guneter,
> > > > 
> > > > > I have a number of patches pending in my watchdog-next branch which are not
> > > > > in your watchdog-next repository. Below is a summary, rebased onto your
> > > > > repository. This does not include the patches I had submitted.
> > > > > 
> > > > > Please consider those patches for v4.5.
> > > > 
> > > > I'm still working on those. Will be for this evening. I left those because they are all new drivers. I wanted to have all fixes and other patches first.
> > > > So not finished yet :-) I will also add your dev fixes (V2 patchset containing 5 patches).
> > > > 
> > > Thanks!
> > 
> > No problem. I think we now have all patches that should go in for v4.5 .
> > 
> > There is off course still other stuff that I need to look at:
> > SBSA, pretimeout, bcm63xx, stmp3xxx GETBOOTSTATUS (although I have the impression that this patch is a dead end because you cannot really tell what the reboot reason is with the patch), soft timer's moving into the core, mei watchdog code, ...
> Hi Wim,
> 
> I'll have to look into those again as well. Most will now conflict
> with the new watchdog core code.
> 
> Any thoughts on the other patch series I sent out, to create the watchdog
> device in watchdog_dev.c, and to handle reference counting in the watchdog
> core ? That would be a prerequisite for me to rebase (and rework) the soft
> timer handling core. I had hoped that the changes to handle reference
> counting in the core could make it into 4.5.

Need to review this 6-part patchset still. But probably still possible for 4.5 .
The soft-timer patches are also 4.6 at the earliest.

Kind regards,
Wim.

Re: Additional patches in my watchdog-next branch

[Guenter Roeck]

Hi Wim,

On Tue, Dec 29, 2015 at 08:21:08AM +0100, Wim Van Sebroeck wrote:
> Hi Guenter,
> 
> > On Mon, Dec 28, 2015 at 11:54:27PM +0100, Wim Van Sebroeck wrote:
> > > Hi Guenter,
> > > 
> > > > On Mon, Dec 28, 2015 at 04:27:11PM +0100, Wim Van Sebroeck wrote:
> > > > > Hi Guneter,
> > > > > 
> > > > > > I have a number of patches pending in my watchdog-next branch which are not
> > > > > > in your watchdog-next repository. Below is a summary, rebased onto your
> > > > > > repository. This does not include the patches I had submitted.
> > > > > > 
> > > > > > Please consider those patches for v4.5.
> > > > > 
> > > > > I'm still working on those. Will be for this evening. I left those because they are all new drivers. I wanted to have all fixes and other patches first.
> > > > > So not finished yet :-) I will also add your dev fixes (V2 patchset containing 5 patches).
> > > > > 
> > > > Thanks!
> > > 
> > > No problem. I think we now have all patches that should go in for v4.5 .
> > > 
> > > There is off course still other stuff that I need to look at:
> > > SBSA, pretimeout, bcm63xx, stmp3xxx GETBOOTSTATUS (although I have the impression that this patch is a dead end because you cannot really tell what the reboot reason is with the patch), soft timer's moving into the core, mei watchdog code, ...
> > Hi Wim,
> > 
> > I'll have to look into those again as well. Most will now conflict
> > with the new watchdog core code.
> > 
> > Any thoughts on the other patch series I sent out, to create the watchdog
> > device in watchdog_dev.c, and to handle reference counting in the watchdog
> > core ? That would be a prerequisite for me to rebase (and rework) the soft
> > timer handling core. I had hoped that the changes to handle reference
> > counting in the core could make it into 4.5.
> 
> Need to review this 6-part patchset still. But probably still possible for 4.5 .

That would be great.

> The soft-timer patches are also 4.6 at the earliest.
> 
Absolutely agree. I'll have to rebase and retest the code.

Thanks,
Guenter

> Kind regards,
> Wim.
> 

Re: Additional patches in my watchdog-next branch

[Guenter Roeck]

Hi Wim,

On Tue, Dec 29, 2015 at 08:17:32AM +0100, Wim Van Sebroeck wrote:
[ ... ]
> 
> SBSA: What should be done is to create a correct driver without pretimeout and with just the normal watchdog (=reboot if we aren't kicked fast enough) behaviour and get that in.
> 
Absoutely agree.

Thanks,
Guenter

Re: Additional patches in my watchdog-next branch

[Thomas Petazzoni]

Hello,

On Tue, 29 Dec 2015 08:04:11 -0800, Guenter Roeck wrote:
> Hi Wim,
> 
> On Tue, Dec 29, 2015 at 08:17:32AM +0100, Wim Van Sebroeck wrote:
> [ ... ]
> > 
> > SBSA: What should be done is to create a correct driver without pretimeout and with just the normal watchdog (=reboot if we aren't kicked fast enough) behaviour and get that in.
> > 
> Absoutely agree.

Sorry to get into the conversation, but I got interested into the SBSA
watchdog driver recently, and looked at the status of its merge in the
kernel.

I've looked at Fu Wei's v9 from November which I believe is the latest
version, and then looked at Vladimir's generic pretimeout patch set.

Vladimir, do you plan to rework the SBSA watchdog driver on top of your
pretimeout patch series ? Or do Fu Wei intends do it ? Alternatively,
as suggested by Wim and Guenter, submitting an initial version without
pretimeout support would be a good thing. Are you, or Fu, interested in
doing this, or should I work at updating Fu's patches in this
direction ?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: Additional patches in my watchdog-next branch

[Guenter Roeck]

On Tue, Dec 29, 2015 at 05:37:05PM +0100, Thomas Petazzoni wrote:
> Hello,
> 
> On Tue, 29 Dec 2015 08:04:11 -0800, Guenter Roeck wrote:
> > Hi Wim,
> > 
> > On Tue, Dec 29, 2015 at 08:17:32AM +0100, Wim Van Sebroeck wrote:
> > [ ... ]
> > > 
> > > SBSA: What should be done is to create a correct driver without pretimeout and with just the normal watchdog (=reboot if we aren't kicked fast enough) behaviour and get that in.
> > > 
> > Absoutely agree.
> 
> Sorry to get into the conversation, but I got interested into the SBSA
> watchdog driver recently, and looked at the status of its merge in the
> kernel.
> 
> I've looked at Fu Wei's v9 from November which I believe is the latest
> version, and then looked at Vladimir's generic pretimeout patch set.
> 
> Vladimir, do you plan to rework the SBSA watchdog driver on top of your
> pretimeout patch series ? Or do Fu Wei intends do it ? Alternatively,
> as suggested by Wim and Guenter, submitting an initial version without
> pretimeout support would be a good thing. Are you, or Fu, interested in
> doing this, or should I work at updating Fu's patches in this
> direction ?
> 
Probem with SBSA and pretimeout is two-fold. First, it adds a quite a bit of
complexity to the driver, second, it adds a lot of risk to the driver.
Reason is that this is not one specific watchdog, but a watchdog implementation
based on a standard which, in my opinion, is less than well thought out
and leaves a lot of ambiguity when it comes to implementation details.
The more functionality is used, the higher the risk that some implementation
won't work.

I think we should stick with the basic implemementation and not support
pretimeout at all with the SBSA driver. If the maximum timeout is insufficient,
there are two options: Use the soft-timeout handled by the watchdog core,
which will hopefully make it for 4.6, or live with it. 

As for pretimeout itself, we will have multiple proposals to consider,
as well as integration order. At this point I would prefer to get the other
infrastructure changes in first, and then pick an implementation which
doesn't add too much complexity. Current proposals are, from my
perspective, either too heavy-weigt or too light-weight. I think we should
try to find a middle ground.

Thanks,
Guenter

Re: Additional patches in my watchdog-next branch

[Wim Van Sebroeck]

Hi Guenter,

> Hi Wim,
> 
> On Tue, Dec 29, 2015 at 08:21:08AM +0100, Wim Van Sebroeck wrote:
> > Hi Guenter,
> > 
> > > On Mon, Dec 28, 2015 at 11:54:27PM +0100, Wim Van Sebroeck wrote:
> > > > Hi Guenter,
> > > > 
> > > > > On Mon, Dec 28, 2015 at 04:27:11PM +0100, Wim Van Sebroeck wrote:
> > > > > > Hi Guneter,
> > > > > > 
> > > > > > > I have a number of patches pending in my watchdog-next branch which are not
> > > > > > > in your watchdog-next repository. Below is a summary, rebased onto your
> > > > > > > repository. This does not include the patches I had submitted.
> > > > > > > 
> > > > > > > Please consider those patches for v4.5.
> > > > > > 
> > > > > > I'm still working on those. Will be for this evening. I left those because they are all new drivers. I wanted to have all fixes and other patches first.
> > > > > > So not finished yet :-) I will also add your dev fixes (V2 patchset containing 5 patches).
> > > > > > 
> > > > > Thanks!
> > > > 
> > > > No problem. I think we now have all patches that should go in for v4.5 .
> > > > 
> > > > There is off course still other stuff that I need to look at:
> > > > SBSA, pretimeout, bcm63xx, stmp3xxx GETBOOTSTATUS (although I have the impression that this patch is a dead end because you cannot really tell what the reboot reason is with the patch), soft timer's moving into the core, mei watchdog code, ...
> > > Hi Wim,
> > > 
> > > I'll have to look into those again as well. Most will now conflict
> > > with the new watchdog core code.
> > > 
> > > Any thoughts on the other patch series I sent out, to create the watchdog
> > > device in watchdog_dev.c, and to handle reference counting in the watchdog
> > > core ? That would be a prerequisite for me to rebase (and rework) the soft
> > > timer handling core. I had hoped that the changes to handle reference
> > > counting in the core could make it into 4.5.
> > 
> > Need to review this 6-part patchset still. But probably still possible for 4.5 .
> 
> That would be great.

It's reviewed and added. I'm actually glad to have a nicer way than the ref/unref
code that we had.

Kind regards,
Wim.

Re: Additional patches in my watchdog-next branch

[Guenter Roeck]

Hi Wim,

On Tue, Dec 29, 2015 at 09:01:58PM +0100, Wim Van Sebroeck wrote:
> Hi Guenter,
> 
> > > > 
> > > > Any thoughts on the other patch series I sent out, to create the watchdog
> > > > device in watchdog_dev.c, and to handle reference counting in the watchdog
> > > > core ? That would be a prerequisite for me to rebase (and rework) the soft
> > > > timer handling core. I had hoped that the changes to handle reference
> > > > counting in the core could make it into 4.5.
> > > 
> > > Need to review this 6-part patchset still. But probably still possible for 4.5 .
> > 
> > That would be great.
> 
> It's reviewed and added. I'm actually glad to have a nicer way than the ref/unref
> code that we had.
> 
Great - thanks a lot!

Guenter

Re: Additional patches in my watchdog-next branch

[Thomas Petazzoni]

Guenter,

On Tue, 29 Dec 2015 09:28:48 -0800, Guenter Roeck wrote:

> Probem with SBSA and pretimeout is two-fold. First, it adds a quite a bit of
> complexity to the driver, second, it adds a lot of risk to the driver.
> Reason is that this is not one specific watchdog, but a watchdog implementation
> based on a standard which, in my opinion, is less than well thought out
> and leaves a lot of ambiguity when it comes to implementation details.

I agree that calling the SBSA watchdog specification a "specification"
is a strong word. At least in the version I have, it's 4-5 pages of
somewhat random thoughts on what a standardized watchdog could look
like, but not a very well-defined specification that guarantees that
all implementations will behave in the same way.

> The more functionality is used, the higher the risk that some implementation
> won't work.

Maybe we should enforce that all users of the driver use two Device
Tree compatible strings:

	compatible = "<vendor>,sbsa-gwdt", "arm,sbsa-gwdt";

So that the SBSA watchdog driver can implement vendor specific quirks
if needed. Though it is true that the HW has some identification
registers that should allow to differentiate between the implementation.

> I think we should stick with the basic implemementation and not support
> pretimeout at all with the SBSA driver. If the maximum timeout is insufficient,
> there are two options: Use the soft-timeout handled by the watchdog core,
> which will hopefully make it for 4.6, or live with it. 

For now, I'm personally fine with a SBSA driver that does not support
the pretimeout. Is anyone working on submitting something like this ?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: Additional patches in my watchdog-next branch

[Wim Van Sebroeck]

Hi Guenter,

> On Tue, Dec 29, 2015 at 09:01:58PM +0100, Wim Van Sebroeck wrote:
> > Hi Guenter,
> > 
> > > > > 
> > > > > Any thoughts on the other patch series I sent out, to create the watchdog
> > > > > device in watchdog_dev.c, and to handle reference counting in the watchdog
> > > > > core ? That would be a prerequisite for me to rebase (and rework) the soft
> > > > > timer handling core. I had hoped that the changes to handle reference
> > > > > counting in the core could make it into 4.5.
> > > > 
> > > > Need to review this 6-part patchset still. But probably still possible for 4.5 .
> > > 
> > > That would be great.
> > 
> > It's reviewed and added. I'm actually glad to have a nicer way than the ref/unref
> > code that we had.
> > 
> Great - thanks a lot!

No problem. :-)

Just to clarify to all others: the ref/unref code was added because we needed to
address the issue of (let's call it) "hotswapping" of a watchdog device while it
was still running. That issue (off-course) remains but this code moves the complexity
in the watchdog core code instead of in the drivers. Which basically simplifies live
for everyone.

Kind regards,
Wim.

Re: Additional patches in my watchdog-next branch

[Fu Wei]

Hi Thomas,

Thanks for your email.
Add Al Stone in the cc list, he is also working on this.

On 30 December 2015 at 00:37, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Tue, 29 Dec 2015 08:04:11 -0800, Guenter Roeck wrote:
>> Hi Wim,
>>
>> On Tue, Dec 29, 2015 at 08:17:32AM +0100, Wim Van Sebroeck wrote:
>> [ ... ]
>> >
>> > SBSA: What should be done is to create a correct driver without pretimeout and with just the normal watchdog (=reboot if we aren't kicked fast enough) behaviour and get that in.
>> >
>> Absoutely agree.
>
> Sorry to get into the conversation, but I got interested into the SBSA
> watchdog driver recently, and looked at the status of its merge in the
> kernel.

Thanks :-)


>
> I've looked at Fu Wei's v9 from November which I believe is the latest
> version, and then looked at Vladimir's generic pretimeout patch set.

yes, v9 is the latest version for upstream, but the real latest
one(rebased to the latest mainline kernel) is here :

https://git.linaro.org/people/fu.wei/linux.git/shortlog/refs/heads/acpi-topic-sbsa-watchdog_upstream_v10_devel


>
> Vladimir, do you plan to rework the SBSA watchdog driver on top of your
> pretimeout patch series ? Or do Fu Wei intends do it ? Alternatively,

in my latest patchset , I was trying to implement pretimeout in the
watchdog framwork,
I thought the SBSA watchdog driver need this , and that makes sense to
be implemented this time.

But after I discussed with some colleagues,  I have to admit that my
design may has some problem.
we are still discussing about that,  but you know, it is holiday in US
now, so most of my colleagues are in vacation, maybe I will post v10
in the middle of January

> as suggested by Wim and Guenter, submitting an initial version without
> pretimeout support would be a good thing. Are you, or Fu, interested in
> doing this, or should I work at updating Fu's patches in this
> direction ?

I have tested and working on this for a while, If you are OK with my
pretimeout patch, we can work together on this, and upstream this as a
separated patch, or integrated into your patchset.
Please feel free to update my patch, you are welcome to work on this.
I am very happy with that. (Please let me know your update if you can,
we can work together or I can use your update :-)  )

But please note that: it doesn't mean my SBSA watchdog patch need this
function, I haven't decided if I still need this or not.

Thanks :-)

>
> Thanks,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch
Ph: +86 21 61221326(direct)
Ph: +86 186 2020 4684 (mobile)
Room 1512, Regus One Corporate Avenue,Level 15,
One Corporate Avenue,222 Hubin Road,Huangpu District,
Shanghai,China 200021

Re: Additional patches in my watchdog-next branch

[Fu Wei]

Hi Guenter,



On 30 December 2015 at 01:28, Guenter Roeck <linux@roeck-us.net> wrote:
> On Tue, Dec 29, 2015 at 05:37:05PM +0100, Thomas Petazzoni wrote:
>> Hello,
>>
>> On Tue, 29 Dec 2015 08:04:11 -0800, Guenter Roeck wrote:
>> > Hi Wim,
>> >
>> > On Tue, Dec 29, 2015 at 08:17:32AM +0100, Wim Van Sebroeck wrote:
>> > [ ... ]
>> > >
>> > > SBSA: What should be done is to create a correct driver without pretimeout and with just the normal watchdog (=reboot if we aren't kicked fast enough) behaviour and get that in.
>> > >
>> > Absoutely agree.
>>
>> Sorry to get into the conversation, but I got interested into the SBSA
>> watchdog driver recently, and looked at the status of its merge in the
>> kernel.
>>
>> I've looked at Fu Wei's v9 from November which I believe is the latest
>> version, and then looked at Vladimir's generic pretimeout patch set.
>>
>> Vladimir, do you plan to rework the SBSA watchdog driver on top of your
>> pretimeout patch series ? Or do Fu Wei intends do it ? Alternatively,
>> as suggested by Wim and Guenter, submitting an initial version without
>> pretimeout support would be a good thing. Are you, or Fu, interested in
>> doing this, or should I work at updating Fu's patches in this
>> direction ?
>>
> Probem with SBSA and pretimeout is two-fold. First, it adds a quite a bit of
> complexity to the driver, second, it adds a lot of risk to the driver.
> Reason is that this is not one specific watchdog, but a watchdog implementation
> based on a standard which, in my opinion, is less than well thought out
> and leaves a lot of ambiguity when it comes to implementation details.
> The more functionality is used, the higher the risk that some implementation
> won't work.

After discussing with my colleagues, I agree with you now.
So I am updating  my patchset now, will post ASAP.

Great thanks for your and Timur's review :-)  To be honest, now , I am
very appreciate that :-)

>
> I think we should stick with the basic implemementation and not support
> pretimeout at all with the SBSA driver. If the maximum timeout is insufficient,
> there are two options: Use the soft-timeout handled by the watchdog core,
> which will hopefully make it for 4.6, or live with it.

yes, I am tending to give up pretimeout in my driver, but I am still
thinking about that, please give me some time.

for the maximum timeout:
it is easy to solve the first stage,  but for the second stage , it is
impossible to extend the  maximum timeout  without writing the WCV in
IRQ handler.
So I am thinking that maybe I will give up to  extend the  maximum
timeout of  the second stage

>
> As for pretimeout itself, we will have multiple proposals to consider,
> as well as integration order. At this point I would prefer to get the other
> infrastructure changes in first, and then pick an implementation which
> doesn't add too much complexity. Current proposals are, from my
> perspective, either too heavy-weigt or too light-weight. I think we should
> try to find a middle ground.

For pretimeout itself,  could you provide some suggestion on my patch,
see if we can working on mine. or if you have better solution, I am
happy to try

Thank you very much!

>
> Thanks,
> Guenter



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch
Ph: +86 21 61221326(direct)
Ph: +86 186 2020 4684 (mobile)
Room 1512, Regus One Corporate Avenue,Level 15,
One Corporate Avenue,222 Hubin Road,Huangpu District,
Shanghai,China 200021

Re: Additional patches in my watchdog-next branch

[Fu Wei]

Hi Thomas,


On 30 December 2015 at 17:44, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Guenter,
>
> On Tue, 29 Dec 2015 09:28:48 -0800, Guenter Roeck wrote:
>
>> Probem with SBSA and pretimeout is two-fold. First, it adds a quite a bit of
>> complexity to the driver, second, it adds a lot of risk to the driver.
>> Reason is that this is not one specific watchdog, but a watchdog implementation
>> based on a standard which, in my opinion, is less than well thought out
>> and leaves a lot of ambiguity when it comes to implementation details.
>
> I agree that calling the SBSA watchdog specification a "specification"
> is a strong word. At least in the version I have, it's 4-5 pages of
> somewhat random thoughts on what a standardized watchdog could look
> like, but not a very well-defined specification that guarantees that
> all implementations will behave in the same way.

Although the basic function is described well in spec 2.3, but it do
need some improvement, in my opinion.

>
>> The more functionality is used, the higher the risk that some implementation
>> won't work.
>
> Maybe we should enforce that all users of the driver use two Device
> Tree compatible strings:
>
>         compatible = "<vendor>,sbsa-gwdt", "arm,sbsa-gwdt";
>
> So that the SBSA watchdog driver can implement vendor specific quirks
> if needed. Though it is true that the HW has some identification
> registers that should allow to differentiate between the implementation.

I think the implementation can be different, but all the SBSA watchdog
should be able to use the same driver.

>
>> I think we should stick with the basic implemementation and not support
>> pretimeout at all with the SBSA driver. If the maximum timeout is insufficient,
>> there are two options: Use the soft-timeout handled by the watchdog core,
>> which will hopefully make it for 4.6, or live with it.
>
> For now, I'm personally fine with a SBSA driver that does not support
> the pretimeout. Is anyone working on submitting something like this ?

I am working on a new non-pretimeout  SBSA driver.

There are two patchsets :
(1) Timur's v4 patch : http://www.spinics.net/lists/linux-watchdog/msg06567.html
(2) my previous non-pretimeout version : https://lkml.org/lkml/2015/6/10/766

>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch
Ph: +86 21 61221326(direct)
Ph: +86 186 2020 4684 (mobile)
Room 1512, Regus One Corporate Avenue,Level 15,
One Corporate Avenue,222 Hubin Road,Huangpu District,
Shanghai,China 200021

Re: Additional patches in my watchdog-next branch

[Thomas Petazzoni]

Hello,

On Wed, 30 Dec 2015 21:30:08 +0800, Fu Wei wrote:

> > Maybe we should enforce that all users of the driver use two Device
> > Tree compatible strings:
> >
> >         compatible = "<vendor>,sbsa-gwdt", "arm,sbsa-gwdt";
> >
> > So that the SBSA watchdog driver can implement vendor specific quirks
> > if needed. Though it is true that the HW has some identification
> > registers that should allow to differentiate between the implementation.
> 
> I think the implementation can be different, but all the SBSA watchdog
> should be able to use the same driver.

Yes, they *should*, but as Guenter said, since the specification is not
very clear, it is quite likely that some HW implementations will not
behave exactly like others, so the driver will likely need to have some
quirks to adapt to those subtle differences between implementations.

> > For now, I'm personally fine with a SBSA driver that does not support
> > the pretimeout. Is anyone working on submitting something like this ?
> 
> I am working on a new non-pretimeout  SBSA driver.

Ok, great. Can you Cc: me when you send this new version ?

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: Additional patches in my watchdog-next branch

[Fu Wei]

Hi Thomas,

On 30 December 2015 at 22:02, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Wed, 30 Dec 2015 21:30:08 +0800, Fu Wei wrote:
>
>> > Maybe we should enforce that all users of the driver use two Device
>> > Tree compatible strings:
>> >
>> >         compatible = "<vendor>,sbsa-gwdt", "arm,sbsa-gwdt";
>> >
>> > So that the SBSA watchdog driver can implement vendor specific quirks
>> > if needed. Though it is true that the HW has some identification
>> > registers that should allow to differentiate between the implementation.
>>
>> I think the implementation can be different, but all the SBSA watchdog
>> should be able to use the same driver.
>
> Yes, they *should*, but as Guenter said, since the specification is not
> very clear, it is quite likely that some HW implementations will not
> behave exactly like others, so the driver will likely need to have some
> quirks to adapt to those subtle differences between implementations.

As Guenter said, the specification is not very clear. Yes, agree, my
colleagues also have this feeling.
if there are some problems in the spec, maybe we can suggest ARM
engineer improving that

Because the SBSA spec defines the device, I think a "so-called"  SBSA
watchdog can be added more useful features,
but it should follow all the  description in the spec at least, then
the SBSA watchdog driver can be a common driver for all this devices.

For now what we can do is testing the common driver on the different
platform as much as we can. See if the driver can work,
I have tested my driver on:
    (1)ARM Foundation v8 model
    (2)AMD Seattle platform

maybe there are some other platform has this watchdog, If I can test
the driver on them, that would be great.


>
>> > For now, I'm personally fine with a SBSA driver that does not support
>> > the pretimeout. Is anyone working on submitting something like this ?
>>
>> I am working on a new non-pretimeout  SBSA driver.
>
> Ok, great. Can you Cc: me when you send this new version ?
>

yes, NP, will do definitely.  Great thanks for your feedback.

> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch
Ph: +86 21 61221326(direct)
Ph: +86 186 2020 4684 (mobile)
Room 1512, Regus One Corporate Avenue,Level 15,
One Corporate Avenue,222 Hubin Road,Huangpu District,
Shanghai,China 200021

Re: Additional patches in my watchdog-next branch

[Thomas Petazzoni]

Hello,

On Wed, 30 Dec 2015 23:01:09 +0800, Fu Wei wrote:

> For now what we can do is testing the common driver on the different
> platform as much as we can. See if the driver can work,
> I have tested my driver on:
>     (1)ARM Foundation v8 model
>     (2)AMD Seattle platform
> 
> maybe there are some other platform has this watchdog, If I can test
> the driver on them, that would be great.

I have another platform with this watchdog, and I'm currently testing
it with your driver. So far, it's not working that well, but I haven't
determined yet if it's a driver problem or a HW problem.

> > Ok, great. Can you Cc: me when you send this new version ?
> 
> yes, NP, will do definitely.  Great thanks for your feedback.

Good, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: Additional patches in my watchdog-next branch

[Fu Wei]

Hi Thomas,


On 30 December 2015 at 23:03, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Wed, 30 Dec 2015 23:01:09 +0800, Fu Wei wrote:
>
>> For now what we can do is testing the common driver on the different
>> platform as much as we can. See if the driver can work,
>> I have tested my driver on:
>>     (1)ARM Foundation v8 model
>>     (2)AMD Seattle platform
>>
>> maybe there are some other platform has this watchdog, If I can test
>> the driver on them, that would be great.
>
> I have another platform with this watchdog, and I'm currently testing
> it with your driver. So far, it's not working that well, but I haven't
> determined yet if it's a driver problem or a HW problem.

If you can share some info to me, I am glad to help, see if we can
figure out the problem together.

>
>> > Ok, great. Can you Cc: me when you send this new version ?
>>
>> yes, NP, will do definitely.  Great thanks for your feedback.
>
> Good, thanks.
>

 You are welcome! :-)

> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch
Ph: +86 21 61221326(direct)
Ph: +86 186 2020 4684 (mobile)
Room 1512, Regus One Corporate Avenue,Level 15,
One Corporate Avenue,222 Hubin Road,Huangpu District,
Shanghai,China 200021

Re: Additional patches in my watchdog-next branch

[Vladimir Zapolskiy]

Hello,

On 29.12.2015 18:37, Thomas Petazzoni wrote:
> Hello,
> 
> On Tue, 29 Dec 2015 08:04:11 -0800, Guenter Roeck wrote:
>> Hi Wim,
>>
>> On Tue, Dec 29, 2015 at 08:17:32AM +0100, Wim Van Sebroeck wrote:
>> [ ... ]
>>>
>>> SBSA: What should be done is to create a correct driver without pretimeout and with just the normal watchdog (=reboot if we aren't kicked fast enough) behaviour and get that in.
>>>
>> Absoutely agree.
> 
> Sorry to get into the conversation, but I got interested into the SBSA
> watchdog driver recently, and looked at the status of its merge in the
> kernel.
> 
> I've looked at Fu Wei's v9 from November which I believe is the latest
> version, and then looked at Vladimir's generic pretimeout patch set.
> 
> Vladimir, do you plan to rework the SBSA watchdog driver on top of your
> pretimeout patch series ?

I've looked at v9 ARM SBSA watchdog series and to remove any ambiguity
the proposed pretimeout framework allows to select a taken action on
pretimeout event got from a watchdog device (talking about v9 SBSA series
the hardcoded action is panic, which might be not what a user wants), it
does not add pretimeout handlers though (the kernel already has
WDIOC_SETPRETIMEOUT/WDIOC_GETPRETIMEOUT, and I suppose this mechanism is
going to be reused by the watchdog framework and drivers), so my
pretimeout framework is kind of orthogonal to Fu's changes.

Also for your information in v2 series I've excluded a watchdog
device/driver specific pretimeout handler found in v1:
https://www.spinics.net/lists/linux-watchdog/msg07878.html , if you think
that ARM SBSA watchdog driver may need this, please let me know.

Since there are some noticeable changes in the watchdog framework, I'll
rebase and resend the pretimeout framework for review after the merge
window, please feel free to review/comment on it.

> Or do Fu Wei intends do it ? Alternatively,
> as suggested by Wim and Guenter, submitting an initial version without
> pretimeout support would be a good thing. Are you, or Fu, interested in
> doing this, or should I work at updating Fu's patches in this
> direction ?
> 

--
Best wishes,
Vladimir