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: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?

-Saravana