From: Wolfgang Denk <wd@denx.de>
To: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Cc: u-boot@lists.denx.de, Simon Glass <sjg@chromium.org>,
Stefan Roese <sr@denx.de>, Tom Rini <trini@konsulko.com>
Subject: Re: [PATCH v5 09/12] watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset()
Date: Thu, 19 Aug 2021 13:10:54 +0200 [thread overview]
Message-ID: <53686.1629371454@gemini.denx.de> (raw)
In-Reply-To: <4798abb5-07d9-fa88-931f-dbaff951e3fb@prevas.dk>
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.
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
If the odds are a million to one against something occuring, chances
are 50-50 it will.
next prev parent reply other threads:[~2021-08-19 11:11 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-11 12:47 [PATCH v5 00/12] handling all DM watchdogs in watchdog_reset() Rasmus Villemoes
2021-08-11 12:47 ` [PATCH v5 01/12] watchdog: wdt-uclass.c: use wdt_start() in wdt_expire_now() Rasmus Villemoes
2021-08-11 12:47 ` [PATCH v5 02/12] watchdog: wdt-uclass.c: introduce struct wdt_priv Rasmus Villemoes
2021-08-11 12:47 ` [PATCH v5 03/12] watchdog: wdt-uclass.c: neaten UCLASS_DRIVER definition Rasmus Villemoes
2021-08-11 12:47 ` [PATCH v5 04/12] watchdog: wdt-uclass.c: refactor initr_watchdog() Rasmus Villemoes
2021-08-11 12:47 ` [PATCH v5 05/12] watchdog: wdt-uclass.c: keep track of each device's running state Rasmus Villemoes
2021-08-11 12:47 ` [PATCH v5 06/12] sandbox: disable CONFIG_WATCHDOG_AUTOSTART Rasmus Villemoes
2021-08-11 12:47 ` [PATCH v5 07/12] watchdog: wdt-uclass.c: add wdt_stop_all() helper Rasmus Villemoes
2021-08-11 12:47 ` [PATCH v5 08/12] board: x530: switch to wdt_stop_all() Rasmus Villemoes
2021-08-11 12:47 ` [PATCH v5 09/12] watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset() Rasmus Villemoes
2021-08-12 6:50 ` Wolfgang Denk
2021-08-13 12:11 ` Rasmus Villemoes
2021-08-19 11:10 ` Wolfgang Denk [this message]
2021-08-19 12:32 ` Tom Rini
2021-08-11 12:47 ` [PATCH v5 10/12] watchdog: add gpio watchdog driver Rasmus Villemoes
2021-08-11 12:47 ` [PATCH v5 11/12] sandbox: add test of wdt_gpio driver Rasmus Villemoes
2021-08-11 12:48 ` [PATCH v5 12/12] sandbox: add test of wdt-uclass' watchdog_reset() Rasmus Villemoes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53686.1629371454@gemini.denx.de \
--to=wd@denx.de \
--cc=rasmus.villemoes@prevas.dk \
--cc=sjg@chromium.org \
--cc=sr@denx.de \
--cc=trini@konsulko.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox