From: "Rafael J. Wysocki" <rafael@kernel.org>
To: Saravana Kannan <saravanak@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"Rafael J. Wysocki" <rafael@kernel.org>,
Marek Szyprowski <m.szyprowski@samsung.com>,
Geert Uytterhoeven <geert@linux-m68k.org>,
Marc Zyngier <maz@kernel.org>,
Tudor Ambarus <Tudor.Ambarus@microchip.com>,
Linus Walleij <linus.walleij@linaro.org>,
Bartosz Golaszewski <bgolaszewski@baylibre.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
"Cc: Android Kernel" <kernel-team@android.com>
Subject: Re: [PATCH v1 1/2] driver core: fw_devlink: Detect supplier devices that will never be added
Date: Mon, 1 Feb 2021 15:29:52 +0100 [thread overview]
Message-ID: <CAJZ5v0i+c+vQ0i3UUzPT9s89OVbVvMQW1ZZ8=cCUYHXq936vzA@mail.gmail.com> (raw)
In-Reply-To: <20210130040344.2807439-2-saravanak@google.com>
On Sat, Jan 30, 2021 at 5:03 AM Saravana Kannan <saravanak@google.com> wrote:
>
> During the initial parsing of firmware by fw_devlink, fw_devlink might
> infer that some supplier firmware nodes would get populated as devices.
> But the inference is not always correct. This patch tries to logically
> detect and fix such mistakes as boot progresses or more devices probe.
>
> fw_devlink makes a fundamental assumption that once a device binds to a
> driver, it will populate (i.e: add as struct devices) all the child
> firmware nodes that could be populated as devices.
That is unless they have been populated earlier.
> So, whenever a device probes, we check all its child firmware nodes. If
> a child firmware node has a corresponding device populated, we don't
> modify the child node or its descendants. However, if a child firmware
> node has not been populated as a device, we go an delete all the fwnode
> links where the child node or its descendants are suppliers. This
> ensures that no other device is blocked on a firmware node that will
> never be populated as a device.
>
> Fixes: e590474768f1 ("driver core: Set fw_devlink=on by default")
> Signed-off-by: Saravana Kannan <saravanak@google.com>
Looks reasonable to me:
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> drivers/base/core.c | 30 +++++++++++++++++++++++++++---
> 1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 484a942884ba..f380133df63b 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -148,6 +148,20 @@ void fwnode_links_purge(struct fwnode_handle *fwnode)
> fwnode_links_purge_consumers(fwnode);
> }
>
> +static void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode)
> +{
> + struct fwnode_handle *child;
> +
> + /* Don't purge consumer links of an added child */
> + if (fwnode->dev)
> + return;
> +
> + fwnode_links_purge_consumers(fwnode);
> +
> + fwnode_for_each_available_child_node(fwnode, child)
> + fw_devlink_purge_absent_suppliers(child);
> +}
> +
> #ifdef CONFIG_SRCU
> static DEFINE_MUTEX(device_links_lock);
> DEFINE_STATIC_SRCU(device_links_srcu);
> @@ -1154,12 +1168,22 @@ void device_links_driver_bound(struct device *dev)
> LIST_HEAD(sync_list);
>
> /*
> - * If a device probes successfully, it's expected to have created all
> + * If a device binds successfully, it's expected to have created all
> * the device links it needs to or make new device links as it needs
> - * them. So, it no longer needs to wait on any suppliers.
> + * them. So, fw_devlink no longer needs to create device links to any
> + * of the device's suppliers.
> + *
> + * Also, if a child firmware node of this bound device is not added as
> + * a device by now, assume it is never going to be added and make sure
> + * other devices don't defer probe indefinitely by waiting for such a
> + * child device.
> */
> - if (dev->fwnode && dev->fwnode->dev == dev)
> + if (dev->fwnode && dev->fwnode->dev == dev) {
> + struct fwnode_handle *child;
> fwnode_links_purge_suppliers(dev->fwnode);
> + fwnode_for_each_available_child_node(dev->fwnode, child)
> + fw_devlink_purge_absent_suppliers(child);
> + }
> device_remove_file(dev, &dev_attr_waiting_for_supplier);
>
> device_links_write_lock();
> --
> 2.30.0.365.g02bc693789-goog
>
next prev parent reply other threads:[~2021-02-01 14:30 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-30 4:03 [PATCH v1 0/2] Make fw_devlink=on more forgiving Saravana Kannan
2021-01-30 4:03 ` [PATCH v1 1/2] driver core: fw_devlink: Detect supplier devices that will never be added Saravana Kannan
2021-02-01 14:29 ` Rafael J. Wysocki [this message]
2021-01-30 4:03 ` [PATCH v1 2/2] driver core: fw_devlink: Handle missing drivers for optional suppliers Saravana Kannan
2021-02-01 10:31 ` Geert Uytterhoeven
2021-02-01 20:48 ` Saravana Kannan
2021-02-02 8:49 ` Geert Uytterhoeven
2021-02-02 19:33 ` Saravana Kannan
2021-03-10 2:08 ` [gpiolib] 4731210c09: BUG:kernel_NULL_pointer_dereference,address kernel test robot
2021-01-30 4:08 ` [PATCH v1 0/2] Make fw_devlink=on more forgiving Saravana Kannan
2021-02-01 8:05 ` Marek Szyprowski
2021-02-01 9:02 ` Saravana Kannan
2021-02-02 8:12 ` Marek Szyprowski
2021-02-02 8:33 ` Saravana Kannan
2021-02-01 10:39 ` Geert Uytterhoeven
2021-02-02 3:00 ` Saravana Kannan
2021-02-02 7:55 ` Geert Uytterhoeven
2021-02-02 8:20 ` Saravana Kannan
2021-01-31 21:37 ` Saravana Kannan
2021-02-01 14:42 ` Marc Zyngier
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAJZ5v0i+c+vQ0i3UUzPT9s89OVbVvMQW1ZZ8=cCUYHXq936vzA@mail.gmail.com' \
--to=rafael@kernel.org \
--cc=Tudor.Ambarus@microchip.com \
--cc=bgolaszewski@baylibre.com \
--cc=geert@linux-m68k.org \
--cc=gregkh@linuxfoundation.org \
--cc=kernel-team@android.com \
--cc=linus.walleij@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=maz@kernel.org \
--cc=saravanak@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).