Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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>

authored by

Douglas Anderson and committed by
Danilo Krummrich
f72e77c3 14cf406e

+53 -31
+12 -12
drivers/base/core.c
··· 182 182 if (fwnode->dev) 183 183 return; 184 184 185 - fwnode->flags |= FWNODE_FLAG_NOT_DEVICE; 185 + fwnode_set_flag(fwnode, FWNODE_FLAG_NOT_DEVICE); 186 186 fwnode_links_purge_consumers(fwnode); 187 187 188 188 fwnode_for_each_available_child_node(fwnode, child) ··· 228 228 if (fwnode->dev && fwnode->dev->bus) 229 229 return; 230 230 231 - fwnode->flags |= FWNODE_FLAG_NOT_DEVICE; 231 + fwnode_set_flag(fwnode, FWNODE_FLAG_NOT_DEVICE); 232 232 __fwnode_links_move_consumers(fwnode, new_sup); 233 233 234 234 fwnode_for_each_available_child_node(fwnode, child) ··· 1012 1012 static bool dev_is_best_effort(struct device *dev) 1013 1013 { 1014 1014 return (fw_devlink_best_effort && dev->can_match) || 1015 - (dev->fwnode && (dev->fwnode->flags & FWNODE_FLAG_BEST_EFFORT)); 1015 + (dev->fwnode && fwnode_test_flag(dev->fwnode, FWNODE_FLAG_BEST_EFFORT)); 1016 1016 } 1017 1017 1018 1018 static struct fwnode_handle *fwnode_links_check_suppliers( ··· 1723 1723 1724 1724 static void fw_devlink_parse_fwnode(struct fwnode_handle *fwnode) 1725 1725 { 1726 - if (fwnode->flags & FWNODE_FLAG_LINKS_ADDED) 1726 + if (fwnode_test_flag(fwnode, FWNODE_FLAG_LINKS_ADDED)) 1727 1727 return; 1728 1728 1729 1729 fwnode_call_int_op(fwnode, add_links); 1730 - fwnode->flags |= FWNODE_FLAG_LINKS_ADDED; 1730 + fwnode_set_flag(fwnode, FWNODE_FLAG_LINKS_ADDED); 1731 1731 } 1732 1732 1733 1733 static void fw_devlink_parse_fwtree(struct fwnode_handle *fwnode) ··· 1885 1885 struct device *dev; 1886 1886 bool ret; 1887 1887 1888 - if (!(fwnode->flags & FWNODE_FLAG_INITIALIZED)) 1888 + if (!fwnode_test_flag(fwnode, FWNODE_FLAG_INITIALIZED)) 1889 1889 return false; 1890 1890 1891 1891 dev = get_dev_from_fwnode(fwnode); ··· 2001 2001 * We aren't trying to find all cycles. Just a cycle between con and 2002 2002 * sup_handle. 2003 2003 */ 2004 - if (sup_handle->flags & FWNODE_FLAG_VISITED) 2004 + if (fwnode_test_flag(sup_handle, FWNODE_FLAG_VISITED)) 2005 2005 return false; 2006 2006 2007 - sup_handle->flags |= FWNODE_FLAG_VISITED; 2007 + fwnode_set_flag(sup_handle, FWNODE_FLAG_VISITED); 2008 2008 2009 2009 /* Termination condition. */ 2010 2010 if (sup_handle == con_handle) { ··· 2074 2074 } 2075 2075 2076 2076 out: 2077 - sup_handle->flags &= ~FWNODE_FLAG_VISITED; 2077 + fwnode_clear_flag(sup_handle, FWNODE_FLAG_VISITED); 2078 2078 put_device(sup_dev); 2079 2079 put_device(con_dev); 2080 2080 put_device(par_dev); ··· 2127 2127 * When such a flag is set, we can't create device links where P is the 2128 2128 * supplier of C as that would delay the probe of C. 2129 2129 */ 2130 - if (sup_handle->flags & FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD && 2130 + if (fwnode_test_flag(sup_handle, FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD) && 2131 2131 fwnode_is_ancestor_of(sup_handle, con->fwnode)) 2132 2132 return -EINVAL; 2133 2133 ··· 2150 2150 else 2151 2151 flags = FW_DEVLINK_FLAGS_PERMISSIVE; 2152 2152 2153 - if (sup_handle->flags & FWNODE_FLAG_NOT_DEVICE) 2153 + if (fwnode_test_flag(sup_handle, FWNODE_FLAG_NOT_DEVICE)) 2154 2154 sup_dev = fwnode_get_next_parent_dev(sup_handle); 2155 2155 else 2156 2156 sup_dev = get_dev_from_fwnode(sup_handle); ··· 2162 2162 * supplier device indefinitely. 2163 2163 */ 2164 2164 if (sup_dev->links.status == DL_DEV_NO_DRIVER && 2165 - sup_handle->flags & FWNODE_FLAG_INITIALIZED) { 2165 + fwnode_test_flag(sup_handle, FWNODE_FLAG_INITIALIZED)) { 2166 2166 dev_dbg(con, 2167 2167 "Not linking %pfwf - dev might never probe\n", 2168 2168 sup_handle);
+1 -1
drivers/bus/imx-weim.c
··· 332 332 * fw_devlink doesn't skip adding consumers to this 333 333 * device. 334 334 */ 335 - rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE; 335 + fwnode_clear_flag(&rd->dn->fwnode, FWNODE_FLAG_NOT_DEVICE); 336 336 if (!of_platform_device_create(rd->dn, NULL, &pdev->dev)) { 337 337 dev_err(&pdev->dev, 338 338 "Failed to create child device '%pOF'\n",
+1 -1
drivers/i2c/i2c-core-of.c
··· 180 180 * Clear the flag before adding the device so that fw_devlink 181 181 * doesn't skip adding consumers to this device. 182 182 */ 183 - rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE; 183 + fwnode_clear_flag(&rd->dn->fwnode, FWNODE_FLAG_NOT_DEVICE); 184 184 client = of_i2c_register_device(adap, rd->dn); 185 185 if (IS_ERR(client)) { 186 186 dev_err(&adap->dev, "failed to create client for '%pOF'\n",
+2 -2
drivers/net/phy/mdio_bus_provider.c
··· 294 294 return -EINVAL; 295 295 296 296 if (bus->parent && bus->parent->of_node) 297 - bus->parent->of_node->fwnode.flags |= 298 - FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD; 297 + fwnode_set_flag(&bus->parent->of_node->fwnode, 298 + FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD); 299 299 300 300 WARN(bus->state != MDIOBUS_ALLOCATED && 301 301 bus->state != MDIOBUS_UNREGISTERED,
+1 -1
drivers/of/base.c
··· 1943 1943 if (name) 1944 1944 of_stdout = of_find_node_opts_by_path(name, &of_stdout_options); 1945 1945 if (of_stdout) 1946 - of_stdout->fwnode.flags |= FWNODE_FLAG_BEST_EFFORT; 1946 + fwnode_set_flag(&of_stdout->fwnode, FWNODE_FLAG_BEST_EFFORT); 1947 1947 } 1948 1948 1949 1949 if (!of_aliases)
+1 -1
drivers/of/dynamic.c
··· 225 225 np->sibling = np->parent->child; 226 226 np->parent->child = np; 227 227 of_node_clear_flag(np, OF_DETACHED); 228 - np->fwnode.flags |= FWNODE_FLAG_NOT_DEVICE; 228 + fwnode_set_flag(&np->fwnode, FWNODE_FLAG_NOT_DEVICE); 229 229 230 230 raw_spin_unlock_irqrestore(&devtree_lock, flags); 231 231
+1 -1
drivers/of/platform.c
··· 742 742 * Clear the flag before adding the device so that fw_devlink 743 743 * doesn't skip adding consumers to this device. 744 744 */ 745 - rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE; 745 + fwnode_clear_flag(&rd->dn->fwnode, FWNODE_FLAG_NOT_DEVICE); 746 746 /* pdev_parent may be NULL when no bus platform device */ 747 747 pdev_parent = of_find_device_by_node(parent); 748 748 pdev = of_platform_device_create(rd->dn, NULL,
+1 -1
drivers/spi/spi.c
··· 4937 4937 * Clear the flag before adding the device so that fw_devlink 4938 4938 * doesn't skip adding consumers to this device. 4939 4939 */ 4940 - rd->dn->fwnode.flags &= ~FWNODE_FLAG_NOT_DEVICE; 4940 + fwnode_clear_flag(&rd->dn->fwnode, FWNODE_FLAG_NOT_DEVICE); 4941 4941 spi = of_register_spi_device(ctlr, rd->dn); 4942 4942 put_device(&ctlr->dev); 4943 4943
+33 -11
include/linux/fwnode.h
··· 15 15 #define _LINUX_FWNODE_H_ 16 16 17 17 #include <linux/bits.h> 18 + #include <linux/bitops.h> 18 19 #include <linux/err.h> 19 20 #include <linux/list.h> 20 21 #include <linux/types.h> ··· 43 42 * suppliers. Only enforce ordering with suppliers that have 44 43 * drivers. 45 44 */ 46 - #define FWNODE_FLAG_LINKS_ADDED BIT(0) 47 - #define FWNODE_FLAG_NOT_DEVICE BIT(1) 48 - #define FWNODE_FLAG_INITIALIZED BIT(2) 49 - #define FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD BIT(3) 50 - #define FWNODE_FLAG_BEST_EFFORT BIT(4) 51 - #define FWNODE_FLAG_VISITED BIT(5) 45 + #define FWNODE_FLAG_LINKS_ADDED 0 46 + #define FWNODE_FLAG_NOT_DEVICE 1 47 + #define FWNODE_FLAG_INITIALIZED 2 48 + #define FWNODE_FLAG_NEEDS_CHILD_BOUND_ON_ADD 3 49 + #define FWNODE_FLAG_BEST_EFFORT 4 50 + #define FWNODE_FLAG_VISITED 5 52 51 53 52 struct fwnode_handle { 54 53 struct fwnode_handle *secondary; ··· 58 57 struct device *dev; 59 58 struct list_head suppliers; 60 59 struct list_head consumers; 61 - u8 flags; 60 + unsigned long flags; 62 61 }; 63 62 64 63 /* ··· 213 212 INIT_LIST_HEAD(&fwnode->suppliers); 214 213 } 215 214 215 + static inline void fwnode_set_flag(struct fwnode_handle *fwnode, 216 + unsigned int bit) 217 + { 218 + set_bit(bit, &fwnode->flags); 219 + } 220 + 221 + static inline void fwnode_clear_flag(struct fwnode_handle *fwnode, 222 + unsigned int bit) 223 + { 224 + clear_bit(bit, &fwnode->flags); 225 + } 226 + 227 + static inline void fwnode_assign_flag(struct fwnode_handle *fwnode, 228 + unsigned int bit, bool value) 229 + { 230 + assign_bit(bit, &fwnode->flags, value); 231 + } 232 + 233 + static inline bool fwnode_test_flag(struct fwnode_handle *fwnode, 234 + unsigned int bit) 235 + { 236 + return test_bit(bit, &fwnode->flags); 237 + } 238 + 216 239 static inline void fwnode_dev_initialized(struct fwnode_handle *fwnode, 217 240 bool initialized) 218 241 { 219 242 if (IS_ERR_OR_NULL(fwnode)) 220 243 return; 221 244 222 - if (initialized) 223 - fwnode->flags |= FWNODE_FLAG_INITIALIZED; 224 - else 225 - fwnode->flags &= ~FWNODE_FLAG_INITIALIZED; 245 + fwnode_assign_flag(fwnode, FWNODE_FLAG_INITIALIZED, initialized); 226 246 } 227 247 228 248 int fwnode_link_add(struct fwnode_handle *con, struct fwnode_handle *sup,