[PATCH v4 07/10] watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset()

[Tom Rini]

On Wed, Aug 11, 2021 at 02:29:12PM +0200, Wolfgang Denk wrote:
> Dear Rasmus,
> 
> In message <3d48015a-07d3-e296-b9ba-a1edd455ce9e at prevas.dk> you wrote:
> >
> > >> +       if (ret) {
> > >> +               log_debug("Error getting UCLASS_WDT: %d\n", ret);
> > > 
> > > Perhaps log_err()?
> >
> > No, we've already been over this in earlier discussions (it's the exact
> > same pattern and reasoning as initr_watchdog). If I made it log_err(),
> > it would cost .text for something that never-ever happens in practice,
> > while log_debug() is usually a no-op, but can be compiled in if
> > something truly fishy seems to be going on.
> 
> This argument fits on all types or effors: they are supposed to
> never ever happen - at least in theory; in reality they do, and more
> often than we like.
> 
> And a proper error message is mandatory for correct error handling.

Error messages are a fine line to walk.  We've got to have been very
badly corrupted to go down this error path.  There's going to be lots of
other error messages popping out.  Saving a bit of .text is good.  It
makes it easier to justify spending a little .text later.

> > > Looks good, thanks for quickly working on this. Not sure, if this new
> > > function should be "void" or better "int" so that the error can be
> > > returned.
> >
> > That's why I included my tentative commit log, so you could see my
> > explanation for why I made it void. Until some user shows up that
> > _wants_ a return value, there's no point making it return int. When that
> > user shows up, we can discuss which int (return early on failure?
> > remember that an error was seen but still call wdt_stop on remaining
> > devices? etc. etc.).
> 
> Returning an error code is always a good ide, no matter if
> current users check it or not.

And here I agree, catch an error code, pass the error code back to the
caller.  That's far more important than making sure that every error
code we catch logs a message by default every time.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210811/0b7919d6/attachment.sig>