From 2281f0473fa257d19d59c9e430e91b5a76f418bc Mon Sep 17 00:00:00 2001 From: Josef Bacik Date: Sun, 4 Oct 2026 22:15:41 +0000 Subject: [PATCH] hwmon: (ftsteutates) serialise the watchdog and sensors The registers of the chip are paged: fts_read_byte() and fts_write_byte() select the page and then access the register. access_lock used to make the two steps atomic. It has been removed because the hwmon core serialises the sensor callbacks, but the watchdog code uses the same two functions and is not called by the hwmon core. So the sensors can now be read, or an alarm be cleared, while the watchdog is being started or stopped. If one of them selects its page between the two steps of the other, the second step hits a register of the wrong page. Take the lock of the hwmon device in the watchdog callbacks, and for the accesses of fts_watchdog_init(), which runs when the hwmon device has already been registered. These are moved to a function of their own, so that the lock is not held while the watchdog is being registered. devres unregisters the watchdog before the hwmon device. Fixes: fa035d98dbae ("hwmon: (ftsteutates) Rely on subsystem locking") Assisted-by: LLM Signed-off-by: Josef Bacik Signed-off-by: Guenter Roeck --- drivers/hwmon/ftsteutates.c | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/drivers/hwmon/ftsteutates.c b/drivers/hwmon/ftsteutates.c index 06be120a349d..10d786960ed6 100644 --- a/drivers/hwmon/ftsteutates.c +++ b/drivers/hwmon/ftsteutates.c @@ -61,6 +61,7 @@ enum WATCHDOG_RESOLUTION { struct fts_data { struct i2c_client *client; + struct device *hwmon_dev; unsigned long last_updated; /* in jiffies */ struct watchdog_device wdd; enum WATCHDOG_RESOLUTION resolution; @@ -240,6 +241,8 @@ static int fts_wd_set_timeout(struct watchdog_device *wdd, unsigned int timeout) resolution = minutes; } + guard(hwmon_lock)(data->hwmon_dev); + ret = fts_wd_set_resolution(data, resolution); if (ret < 0) return ret; @@ -252,6 +255,8 @@ static int fts_wd_start(struct watchdog_device *wdd) { struct fts_data *data = watchdog_get_drvdata(wdd); + guard(hwmon_lock)(data->hwmon_dev); + return fts_write_byte(data->client, FTS_WATCHDOG_TIME_PRESET, wdd->timeout / (u8)data->resolution); } @@ -261,6 +266,9 @@ static int fts_wd_stop(struct watchdog_device *wdd) struct fts_data *data; data = watchdog_get_drvdata(wdd); + + guard(hwmon_lock)(data->hwmon_dev); + return fts_write_byte(data->client, FTS_WATCHDOG_TIME_PRESET, 0); } @@ -276,11 +284,11 @@ static const struct watchdog_ops fts_wd_ops = { .set_timeout = fts_wd_set_timeout, }; -static int fts_watchdog_init(struct fts_data *data) +static int fts_watchdog_read_state(struct fts_data *data) { int timeout, ret; - watchdog_set_drvdata(&data->wdd, data); + guard(hwmon_lock)(data->hwmon_dev); timeout = fts_read_byte(data->client, FTS_WATCHDOG_TIME_PRESET); if (timeout < 0) @@ -302,6 +310,19 @@ static int fts_watchdog_init(struct fts_data *data) set_bit(WDOG_HW_RUNNING, &data->wdd.status); } + return 0; +} + +static int fts_watchdog_init(struct fts_data *data) +{ + int ret; + + watchdog_set_drvdata(&data->wdd, data); + + ret = fts_watchdog_read_state(data); + if (ret < 0) + return ret; + /* Register our watchdog part */ data->wdd.info = &fts_wd_info; data->wdd.ops = &fts_wd_ops; @@ -629,6 +650,9 @@ static int fts_probe(struct i2c_client *client) if (IS_ERR(hwmon_dev)) return PTR_ERR(hwmon_dev); + /* The watchdog code takes its lock */ + data->hwmon_dev = hwmon_dev; + err = fts_watchdog_init(data); if (err) return err;