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

[Saravana Kannan]

On Thu, Apr 9, 2020 at 11:52 PM Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > On Thu, Apr 09, 2020 at 11:45:39PM -0400, Sasha Levin wrote:
> > > > From: Saravana Kannan <saravanak@xxxxxxxxxx>
> > > >
> > > > [ 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@xxxxxxxxxx>
> > > > Link: https://lore.kernel.org/r/20200222014038.180923-2-saravanak@xxxxxxxxxx
> > > > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > > > Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
> > > > ---
> > > >  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?

Yes, please do.

-Saravana