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

[Wolfgang Denk <wd@denx.de>]

Dear Rasmus,

In message <3d48015a-07d3-e296-b9ba-a1edd455ce9e@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.

> > 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.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
To the systems programmer,  users  and  applications  serve  only  to
provide a test load.