Re: [PATCH v5 09/12] watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset()

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 1806 bytes --]

On Thu, Aug 19, 2021 at 01:10:54PM +0200, Wolfgang Denk wrote:
> Dear Rasmus,
> 
> In message <4798abb5-07d9-fa88-931f-dbaff951e3fb@prevas.dk> you wrote:
> > >>
> > >> +	ret = uclass_get(UCLASS_WDT, &uc);
> > >> +	if (ret) {
> > >> +		log_debug("Error getting UCLASS_WDT: %d\n", ret);
> > >> +		return 0;
> > >> +	}
> > > 
> > > Here the error goes silent, so we should fix the callers to report
> > > it.
> >
> > The caller (singular) is the initr sequence, so returning an error is
> > effectively the same as halting the boot process, and as I've already
> > explained, I'm not going to change the semantics of initr_watchdog in
> > this regard.
> 
> In this case you must print an error message here.
> 
> > Feel free to submit a patch if you feel a change in this area is in
> > order. That's completely unrelated to what these patches are trying to
> > achieve.
> 
> You add new code here, so please make sure not to add known issues.
> 
> > >> +	uclass_foreach_dev(dev, uc) {
> > >> +		ret = device_probe(dev);
> > >> +		if (ret) {
> > >> +			log_debug("Error probing %s: %d\n", dev->name, ret);
> > >> +			continue;
> > >>  		}
> > > 
> > > Here the situation is different.  The probing error is never
> > > reported anywhere.  Is it really a normal condition that a
> > > device_probe() fails here?
> >
> > No, it is not a normal condition. I added the log_debug() after a
> > request from Simon.
> 
> But log_debug() is nothing any user will see in the field.  We need
> an error message here, too.

Wolfgang,

If you would like to come along afterwards with additional logging
changes, please do so.  As Rasmus has pointed out numerous times at this
point, what he's doing is consistent with everything else in these
areas.  Thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]