Re: [PATCH 0/2] DS1374 Watchdog fixes

[Guenter Roeck <linux@roeck-us.net>]

On Tue, Apr 25, 2017 at 12:58:36PM -0700, Moritz Fischer wrote:
> On Tue, Apr 25, 2017 at 09:58:24AM -0700, Guenter Roeck wrote:
> 
> > Ah, I missed the "n" in various #ifndef statements.
> >
> > I can't really comment on how to solve that; I simply don't know.
> > Also, even with a dt property, it still would be necessary to have
> > a non-DT means to configure one or the other. Making whatever solution
> > backward compatible also seems tricky; I don't have a solution for that
> > problem either.
> 
> How does one do these things in a non-dt context? Platform data? I'd let

Platform data is out of favor. You'd probably want to use device properties
(see drivers/base/property.c). Question though is if this is considered
configuration, hardware description, or both. Presumably the watchdog
only makes sense if the reset signal is wired, and the alarm only makes
sense if the interrupt is wired, but what if both are wired ?

> the MFD select the 'mode'. Maybe being backwards compatible isn't
> possible in any case. Is there a rule somewhere that we guarantee you'll
> never have to change your CONFIG_FOO options?
> 

That would be nice, but no, there is no such rule. You can probably argue
that no published kernel configuration enables the watchdog flag,
so there is nothing to be concerned about.

Guenter

> >
> > > > > The idea was to fix what's broken currently (this patchset) and then refactor.
> > > > > But if you prefer I can do all in one go instead.
> > > > >
> > > >
> > > > It just seemed a waste to me to change/fix a function which is going to
> > > > be removed in a subsequent patch (I seem to recall that there was a fix
> > > > to the ioctl function).
> > > >
> > >
> > > I'd say that it depends on whether you want to backport the fixes to the
> > > stable kernels. Backporting the full rework is probably riskier.
> 
> I suck at communicating these days. But yeah. That was basically my
> concern when I split it up into 'Fixes' and 'Rework'.
> 
> Mostly since the rework might take a couple of rounds of review, while the
> fix can unbrick stuff (might still need review of course)
> 
> Cheers,
> 
> Moritz