[PATCH v2 02/17] iio: accel: kxcjk-1013: Discard pm_runtime_put_autosuspend() return values
From: Brian Norris
Date: Wed Oct 21 2026 - 20:01:45 EST
This driver propagates the return value of pm_runtime_put_autosuspend(),
which is not particularly useful.
Returning an error code from pm_runtime_put_autosuspend() merely means
that it has not queued up a timer or work item to check whether or not
the device can be suspended and there are many perfectly valid
situations in which that can happen, like after writing "on" to the
device's runtime PM "control" attribute in sysfs for one example.
For this reason, modify this driver to discard the
pm_runtime_put_autosuspend() return value, and inline these pm_runtime
calls for simplicity.
While doing this, it becomes obvious that kxcjk1013_write_event_config()
and kxcjk1013_data_rdy_trigger_set_state() are intentionally mismanaging
runtime PM reference counts and instead are relying on (racy)
autosuspend delay for correct ordering. Undo this choice, and instead
properly drop the reference count only after we're done touching the
hardware.
Use guard() for managing the mutex in kxcjk1013_write_event_config(),
because it helps simplify the above refactoring, making it less
error-prone.
This will facilitate a planned change of the
pm_runtime_put_autosuspend() return type to void in the future, similar
to commit 3afd8df02433 ("PM: runtime: Change pm_runtime_put() return
type to void").
Signed-off-by: Brian Norris <briannorris@xxxxxxxxxxxx>
---
Changes in v2:
* Split iio patch into per-driver patches
* Inlining
* Refcount ordering fixes
drivers/iio/accel/kxcjk-1013.c | 118 ++++++++++++++++-----------------
1 file changed, 57 insertions(+), 61 deletions(-)
diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index b3d753bd9476..3b1a8e67b266 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -13,6 +13,7 @@
#include <linux/string.h>
#include <linux/types.h>
#include <linux/acpi.h>
+#include <linux/cleanup.h>
#include <linux/pm.h>
#include <linux/pm_runtime.h>
#include <linux/regulator/consumer.h>
@@ -628,25 +629,6 @@ static int kxcjk1013_get_startup_times(struct kxcjk1013_data *data)
return KXCJK1013_MAX_STARTUP_TIME_US;
}
-static int kxcjk1013_set_power_state(struct kxcjk1013_data *data, bool on)
-{
-#ifdef CONFIG_PM
- int ret;
-
- if (on)
- ret = pm_runtime_resume_and_get(&data->client->dev);
- else
- ret = pm_runtime_put_autosuspend(&data->client->dev);
- if (ret < 0) {
- dev_err(&data->client->dev,
- "Failed: %s for %d\n", __func__, on);
- return ret;
- }
-#endif
-
- return 0;
-}
-
static int kxcjk1013_chip_update_thresholds(struct kxcjk1013_data *data)
{
const struct kx_chipset_regs *regs = data->info->regs;
@@ -895,20 +877,22 @@ static int kxcjk1013_read_raw(struct iio_dev *indio_dev,
if (iio_buffer_enabled(indio_dev))
ret = -EBUSY;
else {
- ret = kxcjk1013_set_power_state(data, true);
+ ret = pm_runtime_resume_and_get(&data->client->dev);
if (ret < 0) {
+ dev_err(&data->client->dev,
+ "failed to resume: %d\n", ret);
mutex_unlock(&data->mutex);
return ret;
}
ret = kxcjk1013_get_acc_reg(data, chan->scan_index);
if (ret < 0) {
- kxcjk1013_set_power_state(data, false);
+ pm_runtime_put_autosuspend(&data->client->dev);
mutex_unlock(&data->mutex);
return ret;
}
*val = sign_extend32(ret >> chan->scan_type.shift,
chan->scan_type.realbits - 1);
- ret = kxcjk1013_set_power_state(data, false);
+ pm_runtime_put_autosuspend(&data->client->dev);
}
mutex_unlock(&data->mutex);
@@ -1033,39 +1017,35 @@ static int kxcjk1013_write_event_config(struct iio_dev *indio_dev,
if (state == data->ev_enable_state)
return 0;
- mutex_lock(&data->mutex);
+ guard(mutex)(&data->mutex);
if (!state && data->motion_trigger_on) {
data->ev_enable_state = 0;
- mutex_unlock(&data->mutex);
return 0;
}
- /*
- * We will expect the enable and disable to do operation in
- * reverse order. This will happen here anyway as our
- * resume operation uses sync mode runtime pm calls, the
- * suspend operation will be delayed by autosuspend delay
- * So the disable operation will still happen in reverse of
- * enable operation. When runtime pm is disabled the mode
- * is always on so sequence doesn't matter
- */
- ret = kxcjk1013_set_power_state(data, state);
- if (ret < 0) {
- mutex_unlock(&data->mutex);
- return ret;
- }
+ if (state) {
+ ret = pm_runtime_resume_and_get(&data->client->dev);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "failed to resume: %d\n",
+ ret);
+ return ret;
+ }
- ret = kxcjk1013_setup_interrupt(data, state, false);
- if (ret < 0) {
- kxcjk1013_set_power_state(data, false);
- data->ev_enable_state = 0;
- mutex_unlock(&data->mutex);
- return ret;
+ ret = kxcjk1013_setup_interrupt(data, true, false);
+ if (ret < 0) {
+ pm_runtime_put_autosuspend(&data->client->dev);
+ return ret;
+ }
+ } else {
+ ret = kxcjk1013_setup_interrupt(data, false, false);
+ if (ret < 0)
+ return ret;
+
+ pm_runtime_put_autosuspend(&data->client->dev);
}
data->ev_enable_state = state;
- mutex_unlock(&data->mutex);
return 0;
}
@@ -1073,15 +1053,22 @@ static int kxcjk1013_write_event_config(struct iio_dev *indio_dev,
static int kxcjk1013_buffer_preenable(struct iio_dev *indio_dev)
{
struct kxcjk1013_data *data = iio_priv(indio_dev);
+ int ret;
- return kxcjk1013_set_power_state(data, true);
+ ret = pm_runtime_resume_and_get(&data->client->dev);
+ if (ret < 0)
+ dev_err(&data->client->dev, "failed to resume: %d\n", ret);
+
+ return ret;
}
static int kxcjk1013_buffer_postdisable(struct iio_dev *indio_dev)
{
struct kxcjk1013_data *data = iio_priv(indio_dev);
- return kxcjk1013_set_power_state(data, false);
+ pm_runtime_put_autosuspend(&data->client->dev);
+
+ return 0;
}
static ssize_t kxcjk1013_get_samp_freq_avail(struct device *dev,
@@ -1224,32 +1211,41 @@ static int kxcjk1013_data_rdy_trigger_set_state(struct iio_trigger *trig,
struct kxcjk1013_data *data = iio_priv(indio_dev);
int ret;
- mutex_lock(&data->mutex);
+ guard(mutex)(&data->mutex);
if (!state && data->ev_enable_state && data->motion_trigger_on) {
data->motion_trigger_on = false;
- mutex_unlock(&data->mutex);
return 0;
}
- ret = kxcjk1013_set_power_state(data, state);
- if (ret < 0) {
- mutex_unlock(&data->mutex);
- return ret;
- }
- ret = kxcjk1013_setup_interrupt(data, state, data->motion_trig != trig);
- if (ret < 0) {
- kxcjk1013_set_power_state(data, false);
- mutex_unlock(&data->mutex);
- return ret;
+ if (state) {
+ ret = pm_runtime_resume_and_get(&data->client->dev);
+ if (ret < 0) {
+ dev_err(&data->client->dev, "failed to resume: %d\n",
+ ret);
+ return ret;
+ }
+
+ ret = kxcjk1013_setup_interrupt(data, true,
+ data->motion_trig != trig);
+ if (ret < 0) {
+ pm_runtime_put_autosuspend(&data->client->dev);
+ return ret;
+ }
+ } else {
+ ret = kxcjk1013_setup_interrupt(data, false,
+ data->motion_trig != trig);
+ if (ret < 0)
+ return ret;
+
+ pm_runtime_put_autosuspend(&data->client->dev);
}
+
if (data->motion_trig == trig)
data->motion_trigger_on = state;
else
data->dready_trigger_on = state;
- mutex_unlock(&data->mutex);
-
return 0;
}
--
2.56.0.218.g1ec904b173-goog