[PATCH 0/3 3/3] auxdisplay: max6959: fix work initialization race and convert to devm_linedisp_register()

kr494167@gmail.com posted 3 patches 1 month, 1 week ago
Only 0 patches received!
drivers/auxdisplay/max6959.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
[PATCH 0/3 3/3] auxdisplay: max6959: fix work initialization race and convert to devm_linedisp_register()
Posted by kr494167@gmail.com 1 month, 1 week ago
From: Surendra Singh Chouhan <kr494167@gmail.com>

INIT_DELAYED_WORK(&priv->work, max6959_disp_update) was previously called
inside max6959_linedisp_get_map_type(), which is invoked during/after
linedisp_register(). Initializing a delayed_work structure inside a map
query callback can re-initialize an active work item or race with
max6959_linedisp_update().

In addition, max6959_i2c_remove() called cancel_delayed_work_sync() before
linedisp_unregister(&priv->linedisp), allowing sysfs updates to
reschedule work after cancel_delayed_work_sync() completed.

Fix these by moving INIT_DELAYED_WORK() to probe(), using
devm_add_action_or_reset() for work cancellation, and converting to
devm_linedisp_register(). Registering devm_linedisp_register() after
work cancellation action ensures proper LIFO teardown order, allowing
max6959_i2c_remove() to be removed entirely.

Fixes: a9bcd02fa422 ("auxdisplay: Add driver for MAX695x 7-segment LED controllers")
Signed-off-by: Surendra Singh Chouhan <kr494167@gmail.com>
---
 drivers/auxdisplay/max6959.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/auxdisplay/max6959.c b/drivers/auxdisplay/max6959.c
index 888788a1ff08..795bdac9af3f 100644
--- a/drivers/auxdisplay/max6959.c
+++ b/drivers/auxdisplay/max6959.c
@@ -63,11 +63,15 @@ static void max6959_disp_update(struct work_struct *work)
 	regmap_bulk_write(priv->regmap, REG_DIGIT(0), buf, ARRAY_SIZE(buf));
 }
 
-static int max6959_linedisp_get_map_type(struct linedisp *linedisp)
+static void max6959_cancel_work(void *data)
 {
-	struct max6959_priv *priv = container_of(linedisp, struct max6959_priv, linedisp);
+	struct delayed_work *work = data;
 
-	INIT_DELAYED_WORK(&priv->work, max6959_disp_update);
+	cancel_delayed_work_sync(work);
+}
+
+static int max6959_linedisp_get_map_type(struct linedisp *linedisp)
+{
 	return LINEDISP_MAP_SEG7;
 }
 
@@ -123,6 +127,11 @@ static int max6959_i2c_probe(struct i2c_client *client)
 	if (!priv)
 		return -ENOMEM;
 
+	INIT_DELAYED_WORK(&priv->work, max6959_disp_update);
+	ret = devm_add_action_or_reset(dev, max6959_cancel_work, &priv->work);
+	if (ret)
+		return ret;
+
 	priv->regmap = devm_regmap_init_i2c(client, &max6959_regmap_config);
 	if (IS_ERR(priv->regmap))
 		return PTR_ERR(priv->regmap);
@@ -131,7 +140,7 @@ static int max6959_i2c_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
-	ret = linedisp_register(&priv->linedisp, dev, 4, &max6959_linedisp_ops);
+	ret = devm_linedisp_register(dev, &priv->linedisp, 4, &max6959_linedisp_ops);
 	if (ret)
 		return ret;
 
@@ -140,14 +149,6 @@ static int max6959_i2c_probe(struct i2c_client *client)
 	return 0;
 }
 
-static void max6959_i2c_remove(struct i2c_client *client)
-{
-	struct max6959_priv *priv = i2c_get_clientdata(client);
-
-	cancel_delayed_work_sync(&priv->work);
-	linedisp_unregister(&priv->linedisp);
-}
-
 static int max6959_suspend(struct device *dev)
 {
 	return max6959_enable(dev_get_drvdata(dev), false);
@@ -179,7 +180,6 @@ static struct i2c_driver max6959_i2c_driver = {
 		.of_match_table = max6959_of_table,
 	},
 	.probe = max6959_i2c_probe,
-	.remove = max6959_i2c_remove,
 	.id_table = max6959_i2c_id,
 };
 module_i2c_driver(max6959_i2c_driver);
-- 
2.55.0
Re: [PATCH 0/3 3/3] auxdisplay: max6959: fix work initialization race and convert to devm_linedisp_register()
Posted by Andy Shevchenko 1 month, 1 week ago
On Wed, Aug 19, 2026 at 08:15:56AM +0530, kr494167@gmail.com wrote:

> INIT_DELAYED_WORK(&priv->work, max6959_disp_update) was previously called
> inside max6959_linedisp_get_map_type(), which is invoked during/after
> linedisp_register(). Initializing a delayed_work structure inside a map
> query callback can re-initialize an active work item or race with
> max6959_linedisp_update().
> 
> In addition, max6959_i2c_remove() called cancel_delayed_work_sync() before
> linedisp_unregister(&priv->linedisp), allowing sysfs updates to
> reschedule work after cancel_delayed_work_sync() completed.
> 
> Fix these by moving INIT_DELAYED_WORK() to probe(), using
> devm_add_action_or_reset() for work cancellation, and converting to
> devm_linedisp_register(). Registering devm_linedisp_register() after
> work cancellation action ensures proper LIFO teardown order, allowing
> max6959_i2c_remove() to be removed entirely.

Use devm_delayed_work_autocancel() from devm-helpers.h.

Also try to squeeze this most likely AI-assisted commit message to the point.

...

Same comment to the other fix-patch.

-- 
With Best Regards,
Andy Shevchenko