[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2024101316-designing-renegade-6f00@gregkh>
Date: Sun, 13 Oct 2024 17:02:43 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Zijun Hu <zijun_hu@...oud.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, linux-kernel@...r.kernel.org,
Zijun Hu <quic_zijuhu@...cinc.com>
Subject: Re: [PATCH v2] driver core: bus: Remove an impossible error handling
path in bus_add_driver()
On Tue, Sep 17, 2024 at 02:53:32PM +0800, Zijun Hu wrote:
> On 2024/9/17 14:49, Zijun Hu wrote:
> > From: Zijun Hu <quic_zijuhu@...cinc.com>
> >
> > For the following function call chain:
> > API driver_register() -> bus_add_driver() -> driver_attach()
> >
> > There is an error handling path for driver_attach() returning non-zero
> > or failure in bus_add_driver(), remove it with below reasons:
> >
> > - It is impossible for driver_attach() to have failure in bus_add_driver()
> > For int driver_attach(const struct device_driver *drv), the only factor
> > which makes it failed is that bus_to_subsys(@drv->bus) is NULL, but
> > the factor has been excluded by bus_add_driver() before calling it.
> >
> > - driver_attach() is irrelevant with driver_register(), so the former's
> > result should not also have an impact on the later.
> >
> > Signed-off-by: Zijun Hu <quic_zijuhu@...cinc.com>
> > ---
> > Changes in v2:
> > - Remove the error handling path instead of WARN_ON() it.
> > - Correct title and commit message
> > - Link to v1: https://lore.kernel.org/r/20240915-bus_add_driver_fix-v1-1-ce5cf1f66601@quicinc.com
> > ---
> > drivers/base/bus.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> > index 657c93c38b0d..54ff92aece92 100644
> > --- a/drivers/base/bus.c
> > +++ b/drivers/base/bus.c
> > @@ -674,7 +674,8 @@ int bus_add_driver(struct device_driver *drv)
> > if (sp->drivers_autoprobe) {
> > error = driver_attach(drv);
> > if (error)
> > - goto out_del_list;
> > + pr_warn("%s: failed to attach driver '%s' to bus '%s'\n",
> > + __func__, drv->name, sp->bus->name);
>
> driver_attach() has __must_check attribute and this error may be
> inconsequential for driver_register(), so give pr_warn() here
Yes, but you now ignore the error, so someone will come back and add
that error handling in. I'd just leave it as-is.
thanks,
greg k-h
Powered by blists - more mailing lists