On Tue, Apr 16, 2019 at 02:20:46PM -0700, Guenter Roeck wrote: > On Tue, Apr 16, 2019 at 10:55:33PM +0200, Wolfram Sang wrote: > > > > > That is indeed a problem: the pointer will be NULL if there is no parent > > > device (such as in softdog.c). Otherwise it should never be NULL. > > > > Okay, this spoils my err_dev solution. So, we probably go this route > > then: > > > > pr_<errlvl>("watchdog%d: <err_msg>\n", wdd->id); > > > > I don't like it because it doesn't show the driver name, and watchdog%d > can change with each reboot. How about something like this ? > > static void pr_wdt_err(struct watchdog_device *wdd, char *text, int err) > { > if (wdd->parent) > dev_err(wdd->parent, "%s: %d\n", text, err); > else > pr_err("%s: %s: %d\n", wdd->info->identity, text, err); > } > > We could then use the same mechanism to generate error messages for > watchdog_register_device(). 'text' is a constant string then. Supporting a format string will make this much more complicated. Yet, printing out the wrong timeout is useful, I think. What about: dev_str = wdd->parent ? dev_name(wdd->parent) : wdd->info->identity; pr_<errlvl>("%s: <errstr>\n", dev_str, ...); ? This can be easily copied for watchdog_register_device, not much overhead there.
Attachment:
signature.asc
Description: PGP signature