Hi Arnd,

Great thanks,  I am using platform_get_irq() right now, and for ws0 routine, I am using panic()

On 4 May 2015 at 18:22, Arnd Bergmann <arnd@arndb.de> wrote:

On Monday 04 May 2015 17:02:28 Fu Wei wrote:
> On 1 May 2015 at 19:26, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Friday 01 May 2015 16:22:08 Fu Wei wrote:
> > >
> > > now , we have tree option:
> > > (1) just a warning to tell user space : you need to feed the dog ( but
> > what
> > > is the best way to sent this message? by signal ?)
> >
> > This is very unlikely to ever be seen by a real user, and the message will
> > otherwise just be lost.
> >
> > > (2) feed the dog in the driver, this make sure kernel is alive, if not,
> > WS1
> > > will be triggered
> >
> > This seems counterproductive, because it prevents the watchdog hardware to
> > do its job of rebooting the system when user space is unresponsive.
> >
>
> the purpose to do this way is : if kernel can still feed the dog , that
> means kernel is still alive, app is dead. so we can just kill the app, and
> keep kernel running for a while. but user need to be warned
> do you think it is make sense ?

No, that's not how the watchdog subsystem works. The idea is that the
watchdog tries to probe all kinds of actions periodically (possibly
network, disk, or some IPC) and only feeds the watchdog if all of them
are functioning correctly. If the user space fails to notify the kernel,
the assumption is that the system is not in a working state and needs to
be reset. A bug in the watchdog program however is not an unlikely error
scenario we need to worry about: even if that happens, the safe action
to do is still to reboot the system because we can no longer tell if the
system is working reliably.

> > > (3)panic() (if we have kdump, we can tell what is wrong with kernel or
> > > app), backup and reboot. by this way , we have time to backup important
> > > date and debug instead of reset directly like normal WDT.
> >
> > I can see value of this one, but if the other two are not needed,
> > we can avoid adding an interface to configure the behavior and always
> > do it this way if the pretimeout is set, or default to doing nothing
> > when it is not set.
> >
>
> Good idea , let's do this way first ?

Ok.

> > > > Use platform_get_irq() here to allow deferred probing of the irqchip.
> > > >
> > >
> > > I tried to used that API before, but I found out I need not only a irq
> > > number, but also flags, so I turn to use resource.
> > > any thing we can do to improve this ? any thought?
> >
> > I don't see where in your driver you actually use the flags, but I also
> > don't fully understand the ACPI portion. It seems that the probe function
> > just sets a variable in the watchdog specific structure, but that one
> > is never used.
> >
>
> yes, for now , It's just for ACPI, but maybe we need the flags to config
> GIC?
> so maybe I do this way first, then if the flags are unnecessary in the
> future, I will turn to use platform_get_irq() .

platform_get_irq() guarantees that the flags are set up correctly in the GIC,
and that should be the case for both ACPI and DT, a driver should never
set that up manually. Just use it now.

        Arnd

--

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