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: c2c724c868c4 ("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>

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 5933852df83749baed87139a450bc0139f897f8a..763e17e9f1483d1e38c864dd319bb058c8cf0f1c 100644 (file)
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -182,7 +182,7 @@ void fw_devlink_purge_absent_suppliers(struct fwnode_handle *fwnode)
        if (fwnode->dev)
                return;
 
-       fwnode->flags |= FWNODE_FLAG_NOT_DEVICE;
+       fwnode_set_flag(fwnode, FWNODE_FLAG_NOT_DEVICE);
        fwnode_links_purge_consumers(fwnode);
 
        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)
                return;
 
-       fwnode->flags |= FWNODE_FLAG_NOT_DEVICE;
+       fwnode_set_flag(fwnode, FWNODE_FLAG_NOT_DEVICE);
        __fwnode_links_move_consumers(fwnode, new_sup);
 
        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)
 {
        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(
@@ -1723,11 +1723,11 @@ bool fw_devlink_is_strict(void)
 
 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;
 
        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)
@@ -1885,7 +1885,7 @@ static bool fwnode_init_without_drv(struct fwnode_handle *fwnode)
        struct device *dev;
        bool ret;
 
-       if (!(fwnode->flags & FWNODE_FLAG_INITIALIZED))
+       if (!fwnode_test_flag(fwnode, FWNODE_FLAG_INITIALIZED))
                return false;
 
        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
         * sup_handle.
         */
-       if (sup_handle->flags & FWNODE_FLAG_VISITED)
+       if (fwnode_test_flag(sup_handle, FWNODE_FLAG_VISITED))
                return false;
 
-       sup_handle->flags |= FWNODE_FLAG_VISITED;
+       fwnode_set_flag(sup_handle, FWNODE_FLAG_VISITED);
 
        /* Termination condition. */
        if (sup_handle == con_handle) {
@@ -2074,7 +2074,7 @@ static bool __fw_devlink_relax_cycles(struct fwnode_handle *con_handle,
        }
 
 out:
-       sup_handle->flags &= ~FWNODE_FLAG_VISITED;
+       fwnode_clear_flag(sup_handle, FWNODE_FLAG_VISITED);
        put_device(sup_dev);
        put_device(con_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
         * 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))
                return -EINVAL;
 
@@ -2150,7 +2150,7 @@ static int fw_devlink_create_devlink(struct device *con,
        else
                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);
        else
                sup_dev = get_dev_from_fwnode(sup_handle);
@@ -2162,7 +2162,7 @@ static int fw_devlink_create_devlink(struct device *con,
                 * supplier device indefinitely.
                 */
                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,
                                "Not linking %pfwf - dev might never probe\n",
                                sup_handle);

diff --git a/drivers/bus/imx-weim.c b/drivers/bus/imx-weim.c
index 83d623d97f5f28e7c88f634c28c16510c74ae463..f735e0462c55ee9ab877b0f9c7a85330892b1be4 100644 (file)
--- a/drivers/bus/imx-weim.c
+++ b/drivers/bus/imx-weim.c
@@ -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
                         * 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)) {
                                dev_err(&pdev->dev,
                                        "Failed to create child device '%pOF'\n",

diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
index eb7fb202355f876420fa03ec4724ccd7ba6edeef..354a88d0599e3e983bb24d188b545f8441aec8c2 100644 (file)
--- a/drivers/i2c/i2c-core-of.c
+++ b/drivers/i2c/i2c-core-of.c
@@ -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
                 * 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);
                if (IS_ERR(client)) {
                        dev_err(&adap->dev, "failed to create client for '%pOF'\n",

diff --git a/drivers/net/phy/mdio_bus_provider.c b/drivers/net/phy/mdio_bus_provider.c
index 4b0637405740e789d7251f201eef93fe20db1011..fd691c5424ea1970ce95165b7388fa1b064da42a 100644 (file)
--- a/drivers/net/phy/mdio_bus_provider.c
+++ b/drivers/net/phy/mdio_bus_provider.c
@@ -294,8 +294,8 @@ int __mdiobus_register(struct mii_bus *bus, struct module *owner)
                return -EINVAL;
 
        if (bus->parent && bus->parent->of_node)
-               bus->parent->of_node->fwnode.flags |=
-                                       FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD;
+               fwnode_set_flag(&bus->parent->of_node->fwnode,
+                               FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD);
 
        WARN(bus->state != MDIOBUS_ALLOCATED &&
             bus->state != MDIOBUS_UNREGISTERED,

diff --git a/drivers/of/base.c b/drivers/of/base.c
index bf4a51887d7422f5b8bfc63c7b20674bf03d800e..180dbce65b98efd2f79910d8b7bb091af296b0d9 100644 (file)
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1943,7 +1943,7 @@ void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align))
                if (name)
                        of_stdout = of_find_node_opts_by_path(name, &of_stdout_options);
                if (of_stdout)
-                       of_stdout->fwnode.flags |= FWNODE_FLAG_BEST_EFFORT;
+                       fwnode_set_flag(&of_stdout->fwnode, FWNODE_FLAG_BEST_EFFORT);
        }
 
        if (!of_aliases)

diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 1a06175def3773c79c8574e3e22a79764561f27e..ade288372101bb9d24873a97f96bc52282a81e72 100644 (file)
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -225,7 +225,7 @@ static void __of_attach_node(struct device_node *np)
        np->sibling = np->parent->child;
        np->parent->child = np;
        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);
 

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index ba591fbceb56954bd8301e8ab479af6a2ba28ecc..7eeaf8e27b5b210d93eb275c1cb3f061ba4bc441 100644 (file)
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -742,7 +742,7 @@ static int of_platform_notify(struct notifier_block *nb,
                 * Clear the flag before adding the device so that fw_devlink
                 * 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 = of_find_device_by_node(parent);
                pdev = of_platform_device_create(rd->dn, NULL,

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 53dee314d76ae91f37f2f137fcf7bc2fad69a381..e686ecaf3dc897bcc33023d5e7bb8c5e82ac2068 100644 (file)
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -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
                 * 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);
                put_device(&ctlr->dev);
 

diff --git a/include/linux/fwnode.h b/include/linux/fwnode.h
index 097be89487bf5c5a96f01d569c1691088db4f0ca..80b38fbf2121c615cb216245bffa878aa5a531ca 100644 (file)
--- a/include/linux/fwnode.h
+++ b/include/linux/fwnode.h
@@ -15,6 +15,7 @@
 #define _LINUX_FWNODE_H_
 
 #include <linux/bits.h>
+#include <linux/bitops.h>
 #include <linux/err.h>
 #include <linux/list.h>
 #include <linux/types.h>
@@ -42,12 +43,12 @@ struct device;
  *             suppliers. Only enforce ordering with suppliers that have
  *             drivers.
  */
-#define FWNODE_FLAG_LINKS_ADDED                        BIT(0)
-#define FWNODE_FLAG_NOT_DEVICE                 BIT(1)
-#define FWNODE_FLAG_INITIALIZED                        BIT(2)
-#define FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD   BIT(3)
-#define FWNODE_FLAG_BEST_EFFORT                        BIT(4)
-#define FWNODE_FLAG_VISITED                    BIT(5)
+#define FWNODE_FLAG_LINKS_ADDED                        0
+#define FWNODE_FLAG_NOT_DEVICE                 1
+#define FWNODE_FLAG_INITIALIZED                        2
+#define FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD   3
+#define FWNODE_FLAG_BEST_EFFORT                        4
+#define FWNODE_FLAG_VISITED                    5
 
 struct fwnode_handle {
        struct fwnode_handle *secondary;
@@ -57,7 +58,7 @@ struct fwnode_handle {
        struct device *dev;
        struct list_head suppliers;
        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);
 }
 
+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,
                                          bool initialized)
 {
        if (IS_ERR_OR_NULL(fwnode))
                return;
 
-       if (initialized)
-               fwnode->flags |= FWNODE_FLAG_INITIALIZED;
-       else
-               fwnode->flags &= ~FWNODE_FLAG_INITIALIZED;
+       fwnode_assign_flag(fwnode, FWNODE_FLAG_INITIALIZED, initialized);
 }
 
 int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup,