Re: [PATCH v3 2/9] driver core: Replace dev->can_match with DEV_FLAG_CAN_MATCH

[Rafael J. Wysocki]

On Fri, Apr 3, 2026 at 2:51 AM Douglas Anderson <dianders@xxxxxxxxxxxx> wrote:
>
> In C, bitfields are not necessarily safe to modify from multiple
> threads without locking. Switch "can_match" over to the "flags" field
> so modifications are safe.
>
> Cc: Saravana Kannan <saravanak@xxxxxxxxxx>
> Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> ---
> Not fixing any known bugs; problem is theoretical and found by code
> inspection. Change is done somewhat manually and only lightly tested
> (mostly compile-time tested).
>
> Changes in v3:
> - New
>
>  drivers/base/core.c    | 10 +++++-----
>  drivers/base/dd.c      |  8 ++++----
>  include/linux/device.h |  8 ++++----
>  3 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 8d4028a2165f..cbdfee887833 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -1011,7 +1011,7 @@ static void device_links_missing_supplier(struct device *dev)
>
>  static bool dev_is_best_effort(struct device *dev)
>  {
> -       return (fw_devlink_best_effort && dev->can_match) ||
> +       return (fw_devlink_best_effort && test_bit(DEV_FLAG_CAN_MATCH, &dev->flags)) ||

I'm wondering if these test_bit(DEV_FLAG_..., &dev->flags) can be put
behind a set of static inline functions, like for example in this case
dev_can_match() or device_can_mach().  The above would become

      return (fw_devlink_best_effort && dev_can_match(dev)) ||

(and analogously below).

That would at least make the code somewhat easier to follow.

>                 (dev->fwnode && (dev->fwnode->flags & FWNODE_FLAG_BEST_EFFORT));
>  }
>
> @@ -1079,7 +1079,7 @@ int device_links_check_suppliers(struct device *dev)
>
>                         if (dev_is_best_effort(dev) &&
>                             device_link_test(link, DL_FLAG_INFERRED) &&
> -                           !link->supplier->can_match) {
> +                           !test_bit(DEV_FLAG_CAN_MATCH, &link->supplier->flags)) {
>                                 ret = -EAGAIN;
>                                 continue;
>                         }
> @@ -1370,7 +1370,7 @@ void device_links_driver_bound(struct device *dev)
>                 } else if (dev_is_best_effort(dev) &&
>                            device_link_test(link, DL_FLAG_INFERRED) &&
>                            link->status != DL_STATE_CONSUMER_PROBE &&
> -                          !link->supplier->can_match) {
> +                          !test_bit(DEV_FLAG_CAN_MATCH, &link->supplier->flags)) {
>                         /*
>                          * When dev_is_best_effort() is true, we ignore device
>                          * links to suppliers that don't have a driver.  If the
> @@ -1758,7 +1758,7 @@ static int fw_devlink_no_driver(struct device *dev, void *data)
>  {
>         struct device_link *link = to_devlink(dev);
>
> -       if (!link->supplier->can_match)
> +       if (!test_bit(DEV_FLAG_CAN_MATCH, &link->supplier->flags))
>                 fw_devlink_relax_link(link);
>
>         return 0;
> @@ -3708,7 +3708,7 @@ int device_add(struct device *dev)
>          * match with any driver, don't block its consumers from probing in
>          * case the consumer device is able to operate without this supplier.
>          */
> -       if (dev->fwnode && fw_devlink_drv_reg_done && !dev->can_match)
> +       if (dev->fwnode && fw_devlink_drv_reg_done && !test_bit(DEV_FLAG_CAN_MATCH, &dev->flags))
>                 fw_devlink_unblock_consumers(dev);
>
>         if (parent)
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 3aead51173f8..66df31696349 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -132,7 +132,7 @@ static DECLARE_WORK(deferred_probe_work, deferred_probe_work_func);
>
>  void driver_deferred_probe_add(struct device *dev)
>  {
> -       if (!dev->can_match)
> +       if (!test_bit(DEV_FLAG_CAN_MATCH, &dev->flags))
>                 return;
>
>         mutex_lock(&deferred_probe_mutex);
> @@ -860,7 +860,7 @@ static int __driver_probe_device(const struct device_driver *drv, struct device
>         if (!test_bit(DEV_FLAG_READY_TO_PROBE, &dev->flags))
>                 return dev_err_probe(dev, -EPROBE_DEFER, "Device not ready to probe\n");
>
> -       dev->can_match = true;
> +       set_bit(DEV_FLAG_CAN_MATCH, &dev->flags);
>         dev_dbg(dev, "bus: '%s': %s: matched device with driver %s\n",
>                 drv->bus->name, __func__, drv->name);
>
> @@ -1006,7 +1006,7 @@ static int __device_attach_driver(struct device_driver *drv, void *_data)
>                 return 0;
>         } else if (ret == -EPROBE_DEFER) {
>                 dev_dbg(dev, "Device match requests probe deferral\n");
> -               dev->can_match = true;
> +               set_bit(DEV_FLAG_CAN_MATCH, &dev->flags);
>                 driver_deferred_probe_add(dev);
>                 /*
>                  * Device can't match with a driver right now, so don't attempt
> @@ -1258,7 +1258,7 @@ static int __driver_attach(struct device *dev, void *data)
>                 return 0;
>         } else if (ret == -EPROBE_DEFER) {
>                 dev_dbg(dev, "Device match requests probe deferral\n");
> -               dev->can_match = true;
> +               set_bit(DEV_FLAG_CAN_MATCH, &dev->flags);
>                 driver_deferred_probe_add(dev);
>                 /*
>                  * Driver could not match with device, but may match with
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 5b7ace5921aa..7e5737c6d7d1 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -465,9 +465,13 @@ struct device_physical_location {
>   *
>   * @DEV_FLAG_READY_TO_PROBE: If set then device_add() has finished enough
>   *             initialization that probe could be called.
> + * @DEV_FLAG_CAN_MATCH: The device has matched with a driver at least once or it
> + *             is in a bus (like AMBA) which can't check for matching drivers
> + *             until other devices probe successfully.
>   */
>  enum struct_device_flags {
>         DEV_FLAG_READY_TO_PROBE,
> +       DEV_FLAG_CAN_MATCH,
>  };
>
>  /**
> @@ -552,9 +556,6 @@ enum struct_device_flags {
>   * @state_synced: The hardware state of this device has been synced to match
>   *               the software state of this device by calling the driver/bus
>   *               sync_state() callback.
> - * @can_match: The device has matched with a driver at least once or it is in
> - *             a bus (like AMBA) which can't check for matching drivers until
> - *             other devices probe successfully.
>   * @dma_coherent: this particular device is dma coherent, even if the
>   *             architecture supports non-coherent devices.
>   * @dma_ops_bypass: If set to %true then the dma_ops are bypassed for the
> @@ -673,7 +674,6 @@ struct device {
>         bool                    offline:1;
>         bool                    of_node_reused:1;
>         bool                    state_synced:1;
> -       bool                    can_match:1;
>  #if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \
>      defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU) || \
>      defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL)
> --
> 2.53.0.1213.gd9a14994de-goog
>
>