device property: Make modifications of fwnode "flags" thread safe

In various places in the kernel, we modify the fwnode "flags" member
by doing either:
  fwnode->flags |= SOME_FLAG;
  fwnode->flags &= ~SOME_FLAG;

This type of modification is not thread-safe. If two threads are both
mucking with the flags at the same time then one can clobber the
other.

While flags are often modified while under the "fwnode_link_lock",
this is not universally true.

Create some accessor functions for setting, clearing, and testing the
FWNODE flags and move all users to these accessor functions. New
accessor functions use set_bit() and clear_bit(), which are
thread-safe.

Cc: stable@vger.kernel.org
Fixes: c2c724c868 ("driver core: Add fw_devlink_parse_fwtree()")
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>
Reviewed-by: Saravana Kannan <saravanak@kernel.org>
Link: https://patch.msgid.link/20260317090112.v2.1.I0a4d03104ecd5103df3d76f66c8d21b1d15a2e38@changeid
[ Fix fwnode_clear_flag() argument alignment, restore dropped blank
  line in fwnode_dev_initialized(), and remove unnecessary parentheses
  around fwnode_test_flag() calls. - Danilo ]
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
This commit is contained in:
Douglas Anderson 2026-03-17 09:01:20 -07:00 committed by Danilo Krummrich
parent 14cf406e08
commit f72e77c33e
9 changed files with 53 additions and 31 deletions

View File

@ -182,7 +182,7 @@ void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode)
if (fwnode->dev) if (fwnode->dev)
return; return;
fwnode->flags |= FWNODE_FLAG_NOT_DEVICE; fwnode_set_flag(fwnode, FWNODE_FLAG_NOT_DEVICE);
fwnode_links_purge_consumers(fwnode); fwnode_links_purge_consumers(fwnode);
fwnode_for_each_available_child_node(fwnode, child) fwnode_for_each_available_child_node(fwnode, child)
@ -228,7 +228,7 @@ static void __fw_devlink_pickup_dangling_consumers(struct fwnode_handle *fwnode,
if (fwnode->dev && fwnode->dev->bus) if (fwnode->dev && fwnode->dev->bus)
return; return;
fwnode->flags |= FWNODE_FLAG_NOT_DEVICE; fwnode_set_flag(fwnode, FWNODE_FLAG_NOT_DEVICE);
__fwnode_links_move_consumers(fwnode, new_sup); __fwnode_links_move_consumers(fwnode, new_sup);
fwnode_for_each_available_child_node(fwnode, child) fwnode_for_each_available_child_node(fwnode, child)
@ -1012,7 +1012,7 @@ static void device_links_missing_supplier(struct device *dev)
static bool dev_is_best_effort(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 && dev->can_match) ||
(dev->fwnode && (dev->fwnode->flags & FWNODE_FLAG_BEST_EFFORT)); (dev->fwnode && fwnode_test_flag(dev->fwnode, FWNODE_FLAG_BEST_EFFORT));
} }
static struct fwnode_handle *fwnode_links_check_suppliers( static struct fwnode_handle *fwnode_links_check_suppliers(
@ -1723,11 +1723,11 @@ bool fw_devlink_is_strict(void)
static void fw_devlink_parse_fwnode(struct fwnode_handle *fwnode) static void fw_devlink_parse_fwnode(struct fwnode_handle *fwnode)
{ {
if (fwnode->flags & FWNODE_FLAG_LINKS_ADDED) if (fwnode_test_flag(fwnode, FWNODE_FLAG_LINKS_ADDED))
return; return;
fwnode_call_int_op(fwnode, add_links); fwnode_call_int_op(fwnode, add_links);
fwnode->flags |= FWNODE_FLAG_LINKS_ADDED; fwnode_set_flag(fwnode, FWNODE_FLAG_LINKS_ADDED);
} }
static void fw_devlink_parse_fwtree(struct fwnode_handle *fwnode) static void fw_devlink_parse_fwtree(struct fwnode_handle *fwnode)
@ -1885,7 +1885,7 @@ static bool fwnode_init_without_drv(struct fwnode_handle *fwnode)
struct device *dev; struct device *dev;
bool ret; bool ret;
if (!(fwnode->flags & FWNODE_FLAG_INITIALIZED)) if (!fwnode_test_flag(fwnode, FWNODE_FLAG_INITIALIZED))
return false; return false;
dev = get_dev_from_fwnode(fwnode); dev = get_dev_from_fwnode(fwnode);
@ -2001,10 +2001,10 @@ static bool __fw_devlink_relax_cycles(struct fwnode_handle *con_handle,
* We aren't trying to find all cycles. Just a cycle between con and * We aren't trying to find all cycles. Just a cycle between con and
* sup_handle. * sup_handle.
*/ */
if (sup_handle->flags & FWNODE_FLAG_VISITED) if (fwnode_test_flag(sup_handle, FWNODE_FLAG_VISITED))
return false; return false;
sup_handle->flags |= FWNODE_FLAG_VISITED; fwnode_set_flag(sup_handle, FWNODE_FLAG_VISITED);
/* Termination condition. */ /* Termination condition. */
if (sup_handle == con_handle) { if (sup_handle == con_handle) {
@ -2074,7 +2074,7 @@ static bool __fw_devlink_relax_cycles(struct fwnode_handle *con_handle,
} }
out: out:
sup_handle->flags &= ~FWNODE_FLAG_VISITED; fwnode_clear_flag(sup_handle, FWNODE_FLAG_VISITED);
put_device(sup_dev); put_device(sup_dev);
put_device(con_dev); put_device(con_dev);
put_device(par_dev); put_device(par_dev);
@ -2127,7 +2127,7 @@ static int fw_devlink_create_devlink(struct device *con,
* When such a flag is set, we can't create device links where P is the * When such a flag is set, we can't create device links where P is the
* supplier of C as that would delay the probe of C. * supplier of C as that would delay the probe of C.
*/ */
if (sup_handle->flags & FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD && if (fwnode_test_flag(sup_handle, FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD) &&
fwnode_is_ancestor_of(sup_handle, con->fwnode)) fwnode_is_ancestor_of(sup_handle, con->fwnode))
return -EINVAL; return -EINVAL;
@ -2150,7 +2150,7 @@ static int fw_devlink_create_devlink(struct device *con,
else else
flags = FW_DEVLINK_FLAGS_PERMISSIVE; flags = FW_DEVLINK_FLAGS_PERMISSIVE;
if (sup_handle->flags & FWNODE_FLAG_NOT_DEVICE) if (fwnode_test_flag(sup_handle, FWNODE_FLAG_NOT_DEVICE))
sup_dev = fwnode_get_next_parent_dev(sup_handle); sup_dev = fwnode_get_next_parent_dev(sup_handle);
else else
sup_dev = get_dev_from_fwnode(sup_handle); sup_dev = get_dev_from_fwnode(sup_handle);
@ -2162,7 +2162,7 @@ static int fw_devlink_create_devlink(struct device *con,
* supplier device indefinitely. * supplier device indefinitely.
*/ */
if (sup_dev->links.status == DL_DEV_NO_DRIVER && if (sup_dev->links.status == DL_DEV_NO_DRIVER &&
sup_handle->flags & FWNODE_FLAG_INITIALIZED) { fwnode_test_flag(sup_handle, FWNODE_FLAG_INITIALIZED)) {
dev_dbg(con, dev_dbg(con,
"Not linking %pfwf - dev might never probe\n", "Not linking %pfwf - dev might never probe\n",
sup_handle); sup_handle);

View File

@ -332,7 +332,7 @@ static int of_weim_notify(struct notifier_block *nb, unsigned long action,
* fw_devlink doesn't skip adding consumers to this * fw_devlink doesn't skip adding consumers to this
* device. * device.
*/ */
rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE; fwnode_clear_flag(&rd->dn->fwnode, FWNODE_FLAG_NOT_DEVICE);
if (!of_platform_device_create(rd->dn, NULL, &pdev->dev)) { if (!of_platform_device_create(rd->dn, NULL, &pdev->dev)) {
dev_err(&pdev->dev, dev_err(&pdev->dev,
"Failed to create child device '%pOF'\n", "Failed to create child device '%pOF'\n",

View File

@ -180,7 +180,7 @@ static int of_i2c_notify(struct notifier_block *nb, unsigned long action,
* Clear the flag before adding the device so that fw_devlink * Clear the flag before adding the device so that fw_devlink
* doesn't skip adding consumers to this device. * doesn't skip adding consumers to this device.
*/ */
rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE; fwnode_clear_flag(&rd->dn->fwnode, FWNODE_FLAG_NOT_DEVICE);
client = of_i2c_register_device(adap, rd->dn); client = of_i2c_register_device(adap, rd->dn);
if (IS_ERR(client)) { if (IS_ERR(client)) {
dev_err(&adap->dev, "failed to create client for '%pOF'\n", dev_err(&adap->dev, "failed to create client for '%pOF'\n",

View File

@ -294,8 +294,8 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
return -EINVAL; return -EINVAL;
if (bus->parent && bus->parent->of_node) if (bus->parent && bus->parent->of_node)
bus->parent->of_node->fwnode.flags |= fwnode_set_flag(&bus->parent->of_node->fwnode,
FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD; FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD);
WARN(bus->state != MDIOBUS_ALLOCATED && WARN(bus->state != MDIOBUS_ALLOCATED &&
bus->state != MDIOBUS_UNREGISTERED, bus->state != MDIOBUS_UNREGISTERED,

View File

@ -1943,7 +1943,7 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align))
if (name) if (name)
of_stdout = of_find_node_opts_by_path(name, &of_stdout_options); of_stdout = of_find_node_opts_by_path(name, &of_stdout_options);
if (of_stdout) if (of_stdout)
of_stdout->fwnode.flags |= FWNODE_FLAG_BEST_EFFORT; fwnode_set_flag(&of_stdout->fwnode, FWNODE_FLAG_BEST_EFFORT);
} }
if (!of_aliases) if (!of_aliases)

View File

@ -225,7 +225,7 @@ static void __of_attach_node(struct device_node *np)
np->sibling = np->parent->child; np->sibling = np->parent->child;
np->parent->child = np; np->parent->child = np;
of_node_clear_flag(np, OF_DETACHED); of_node_clear_flag(np, OF_DETACHED);
np->fwnode.flags |= FWNODE_FLAG_NOT_DEVICE; fwnode_set_flag(&np->fwnode, FWNODE_FLAG_NOT_DEVICE);
raw_spin_unlock_irqrestore(&devtree_lock, flags); raw_spin_unlock_irqrestore(&devtree_lock, flags);

View File

@ -742,7 +742,7 @@ static int of_platform_notify(struct notifier_block *nb,
* Clear the flag before adding the device so that fw_devlink * Clear the flag before adding the device so that fw_devlink
* doesn't skip adding consumers to this device. * doesn't skip adding consumers to this device.
*/ */
rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE; fwnode_clear_flag(&rd->dn->fwnode, FWNODE_FLAG_NOT_DEVICE);
/* pdev_parent may be NULL when no bus platform device */ /* pdev_parent may be NULL when no bus platform device */
pdev_parent = of_find_device_by_node(parent); pdev_parent = of_find_device_by_node(parent);
pdev = of_platform_device_create(rd->dn, NULL, pdev = of_platform_device_create(rd->dn, NULL,

View File

@ -4937,7 +4937,7 @@ static int of_spi_notify(struct notifier_block *nb, unsigned long action,
* Clear the flag before adding the device so that fw_devlink * Clear the flag before adding the device so that fw_devlink
* doesn't skip adding consumers to this device. * doesn't skip adding consumers to this device.
*/ */
rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE; fwnode_clear_flag(&rd->dn->fwnode, FWNODE_FLAG_NOT_DEVICE);
spi = of_register_spi_device(ctlr, rd->dn); spi = of_register_spi_device(ctlr, rd->dn);
put_device(&ctlr->dev); put_device(&ctlr->dev);

View File

@ -15,6 +15,7 @@
#define _LINUX_FWNODE_H_ #define _LINUX_FWNODE_H_
#include <linux/bits.h> #include <linux/bits.h>
#include <linux/bitops.h>
#include <linux/err.h> #include <linux/err.h>
#include <linux/list.h> #include <linux/list.h>
#include <linux/types.h> #include <linux/types.h>
@ -42,12 +43,12 @@ struct device;
* suppliers. Only enforce ordering with suppliers that have * suppliers. Only enforce ordering with suppliers that have
* drivers. * drivers.
*/ */
#define FWNODE_FLAG_LINKS_ADDED BIT(0) #define FWNODE_FLAG_LINKS_ADDED 0
#define FWNODE_FLAG_NOT_DEVICE BIT(1) #define FWNODE_FLAG_NOT_DEVICE 1
#define FWNODE_FLAG_INITIALIZED BIT(2) #define FWNODE_FLAG_INITIALIZED 2
#define FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD BIT(3) #define FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD 3
#define FWNODE_FLAG_BEST_EFFORT BIT(4) #define FWNODE_FLAG_BEST_EFFORT 4
#define FWNODE_FLAG_VISITED BIT(5) #define FWNODE_FLAG_VISITED 5
struct fwnode_handle { struct fwnode_handle {
struct fwnode_handle *secondary; struct fwnode_handle *secondary;
@ -57,7 +58,7 @@ struct fwnode_handle {
struct device *dev; struct device *dev;
struct list_head suppliers; struct list_head suppliers;
struct list_head consumers; struct list_head consumers;
u8 flags; unsigned long flags;
}; };
/* /*
@ -212,16 +213,37 @@ static inline void fwnode_init(struct fwnode_handle *fwnode,
INIT_LIST_HEAD(&fwnode->suppliers); INIT_LIST_HEAD(&fwnode->suppliers);
} }
static inline void fwnode_set_flag(struct fwnode_handle *fwnode,
unsigned int bit)
{
set_bit(bit, &fwnode->flags);
}
static inline void fwnode_clear_flag(struct fwnode_handle *fwnode,
unsigned int bit)
{
clear_bit(bit, &fwnode->flags);
}
static inline void fwnode_assign_flag(struct fwnode_handle *fwnode,
unsigned int bit, bool value)
{
assign_bit(bit, &fwnode->flags, value);
}
static inline bool fwnode_test_flag(struct fwnode_handle *fwnode,
unsigned int bit)
{
return test_bit(bit, &fwnode->flags);
}
static inline void fwnode_dev_initialized(struct fwnode_handle *fwnode, static inline void fwnode_dev_initialized(struct fwnode_handle *fwnode,
bool initialized) bool initialized)
{ {
if (IS_ERR_OR_NULL(fwnode)) if (IS_ERR_OR_NULL(fwnode))
return; return;
if (initialized) fwnode_assign_flag(fwnode, FWNODE_FLAG_INITIALIZED, initialized);
fwnode->flags |= FWNODE_FLAG_INITIALIZED;
else
fwnode->flags &= ~FWNODE_FLAG_INITIALIZED;
} }
int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup, int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup,