On Sat, Mar 21, 2020 at 5:55 AM Saravana Kannan <saravanak@xxxxxxxxxx> wrote:
>
> Sometimes, more than one (generally two) device can point to the same
> fwnode. However, only one device is set as the fwnode's device
> (fwnode->dev) and can be looked up from the fwnode.
>
> Typically, only one of these devices actually have a driver and actually
> probe. If we create device links for all these devices, then the
> suppliers' of these devices (with the same fwnode) will never get a
> sync_state() call because one of their consumer devices will never probe
> (because they don't have a driver).
>
> So, create device links only for the device that is considered as the
> fwnode's device.
>
> One such example of this is the PCI bridge platform_device and the
> corresponding pci_bus device. Both these devices will have the same
> fwnode. It's the platform_device that is registered first and is set as
> the fwnode's device. Also the platform_device is the one that actually
> probes. Without this patch none of the suppliers of a PCI bridge
> platform_device would get a sync_state() callback.
For the record, I think that this is a PCI subsystem problem, but I
agree with the patch here.
> Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> Cc: linux-pci@xxxxxxxxxxxxxxx
> Signed-off-by: Saravana Kannan <saravanak@xxxxxxxxxx>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> ---
> 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 fc6a60998cd6..5e3cc1651c78 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -2404,6 +2404,7 @@ int device_add(struct device *dev)
> struct class_interface *class_intf;
> int error = -EINVAL, fw_ret;
> struct kobject *glue_dir = NULL;
> + bool is_fwnode_dev = false;
>
> dev = get_device(dev);
> if (!dev)
> @@ -2501,8 +2502,10 @@ int device_add(struct device *dev)
>
> kobject_uevent(&dev->kobj, KOBJ_ADD);
>
> - if (dev->fwnode && !dev->fwnode->dev)
> + if (dev->fwnode && !dev->fwnode->dev) {
> dev->fwnode->dev = dev;
> + is_fwnode_dev = true;
> + }
>
> /*
> * Check if any of the other devices (consumers) have been waiting for
> @@ -2518,7 +2521,8 @@ int device_add(struct device *dev)
> */
> device_link_add_missing_supplier_links();
>
> - if (fw_devlink_flags && fwnode_has_op(dev->fwnode, add_links)) {
> + if (fw_devlink_flags && is_fwnode_dev &&
> + fwnode_has_op(dev->fwnode, add_links)) {
> fw_ret = fwnode_call_int_op(dev->fwnode, add_links, dev);
> if (fw_ret == -ENODEV)
> device_link_wait_for_mandatory_supplier(dev);
> --
> 2.25.1.696.g5e7596f4ac-goog
>
