Re: [PATCH AUTOSEL 5.6 14/68] driver core: Reevaluate dev->links.need_for_probe as suppliers are added

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Thu, Apr 09, 2020 at 11:39:55PM -0700, Saravana Kannan wrote:
> On Thu, Apr 9, 2020 at 11:29 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Thu, Apr 09, 2020 at 11:45:39PM -0400, Sasha Levin wrote:
> > > From: Saravana Kannan <saravanak@google.com>
> > >
> > > [ Upstream commit 1745d299af5b373abad08fa29bff0d31dc6aff21 ]
> > >
> > > A previous patch 03324507e66c ("driver core: Allow
> > > fwnode_operations.add_links to differentiate errors") forgot to update
> > > all call sites to fwnode_operations.add_links. This patch fixes that.
> > >
> > > Legend:
> > > -> Denotes RHS is an optional/potential supplier for LHS
> > > => Denotes RHS is a mandatory supplier for LHS
> > >
> > > Example:
> > >
> > > Device A => Device X
> > > Device A -> Device Y
> > >
> > > Before this patch:
> > > 1. Device A is added.
> > > 2. Device A is marked as waiting for mandatory suppliers
> > > 3. Device X is added
> > > 4. Device A is left marked as waiting for mandatory suppliers
> > >
> > > Step 4 is wrong since all mandatory suppliers of Device A have been
> > > added.
> > >
> > > After this patch:
> > > 1. Device A is added.
> > > 2. Device A is marked as waiting for mandatory suppliers
> > > 3. Device X is added
> > > 4. Device A is no longer considered as waiting for mandatory suppliers
> > >
> > > This is the correct behavior.
> > >
> > > Fixes: 03324507e66c ("driver core: Allow fwnode_operations.add_links to differentiate errors")
> > > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > > Link: https://lore.kernel.org/r/20200222014038.180923-2-saravanak@google.com
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Signed-off-by: Sasha Levin <sashal@kernel.org>
> > > ---
> > >  drivers/base/core.c | 8 ++++++--
> > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > > index dbb0f9130f42d..d32a3aefff32f 100644
> > > --- a/drivers/base/core.c
> > > +++ b/drivers/base/core.c
> > > @@ -523,9 +523,13 @@ static void device_link_add_missing_supplier_links(void)
> > >
> > >       mutex_lock(&wfs_lock);
> > >       list_for_each_entry_safe(dev, tmp, &wait_for_suppliers,
> > > -                              links.needs_suppliers)
> > > -             if (!fwnode_call_int_op(dev->fwnode, add_links, dev))
> > > +                              links.needs_suppliers) {
> > > +             int ret = fwnode_call_int_op(dev->fwnode, add_links, dev);
> > > +             if (!ret)
> > >                       list_del_init(&dev->links.needs_suppliers);
> > > +             else if (ret != -ENODEV)
> > > +                     dev->links.need_for_probe = false;
> > > +     }
> > >       mutex_unlock(&wfs_lock);
> > >  }
> >
> > For some reason this wasn't for stable kernels, but I can't remember.
> 
> It *is* for stable kernels too. It is an actual bug that's fixable in
> stable kernels. I think this might have been the one patch that I
> bundled into an unrelated series, but called it out as an unrelated
> bug. Maybe my wording in that email threw you off?

I think it did, sorry.  So no problem adding this to the stable trees so
I can go add it right now?

thanks,

greg k-h