On Sun, 23 May 2021 at 19:24, Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> From: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
>
> I can't see why we shouldn't sleep in the system resume path to allow
> the device firmware to fully wakeup. Having done that, the runtime
> system functions are identical (down to an error print) so use
> pm_runtime_force_suspend() and pm_runtime_force_resume() to reduce
> repitition.
>
> General preference in IIO is now to mark these functions __maybe_unused
> instead of using ifdefs as it is easy to get them wrong.
> Here they appear correct, but provide a less than desirable example
> to copy into other drivers.
>
Reviewed-by: Alexandru Ardelean <aardelean@xxxxxxxxxxx>
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
> ---
> drivers/iio/accel/mma9551.c | 37 ++++--------------------------------
> drivers/iio/accel/mma9553.c | 38 ++++---------------------------------
> 2 files changed, 8 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/iio/accel/mma9551.c b/drivers/iio/accel/mma9551.c
> index 2b74f67536a3..1b4a8b27f14a 100644
> --- a/drivers/iio/accel/mma9551.c
> +++ b/drivers/iio/accel/mma9551.c
> @@ -510,8 +510,7 @@ static int mma9551_remove(struct i2c_client *client)
> return 0;
> }
>
> -#ifdef CONFIG_PM
> -static int mma9551_runtime_suspend(struct device *dev)
> +static __maybe_unused int mma9551_runtime_suspend(struct device *dev)
> {
> struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> struct mma9551_data *data = iio_priv(indio_dev);
> @@ -522,13 +521,13 @@ static int mma9551_runtime_suspend(struct device *dev)
> mutex_unlock(&data->mutex);
> if (ret < 0) {
> dev_err(&data->client->dev, "powering off device failed\n");
> - return -EAGAIN;
> + return ret;
> }
>
> return 0;
> }
>
> -static int mma9551_runtime_resume(struct device *dev)
> +static __maybe_unused int mma9551_runtime_resume(struct device *dev)
> {
> struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> struct mma9551_data *data = iio_priv(indio_dev);
> @@ -542,38 +541,10 @@ static int mma9551_runtime_resume(struct device *dev)
>
> return 0;
> }
> -#endif
> -
> -#ifdef CONFIG_PM_SLEEP
> -static int mma9551_suspend(struct device *dev)
> -{
> - struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> - struct mma9551_data *data = iio_priv(indio_dev);
> - int ret;
> -
> - mutex_lock(&data->mutex);
> - ret = mma9551_set_device_state(data->client, false);
> - mutex_unlock(&data->mutex);
> -
> - return ret;
> -}
> -
> -static int mma9551_resume(struct device *dev)
> -{
> - struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> - struct mma9551_data *data = iio_priv(indio_dev);
> - int ret;
>
> - mutex_lock(&data->mutex);
> - ret = mma9551_set_device_state(data->client, true);
> - mutex_unlock(&data->mutex);
> -
> - return ret;
> -}
> -#endif
>
> static const struct dev_pm_ops mma9551_pm_ops = {
> - SET_SYSTEM_SLEEP_PM_OPS(mma9551_suspend, mma9551_resume)
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
> SET_RUNTIME_PM_OPS(mma9551_runtime_suspend,
> mma9551_runtime_resume, NULL)
> };
> diff --git a/drivers/iio/accel/mma9553.c b/drivers/iio/accel/mma9553.c
> index 32c9a79ebfec..dc2a3316c1a3 100644
> --- a/drivers/iio/accel/mma9553.c
> +++ b/drivers/iio/accel/mma9553.c
> @@ -1149,8 +1149,7 @@ static int mma9553_remove(struct i2c_client *client)
> return 0;
> }
>
> -#ifdef CONFIG_PM
> -static int mma9553_runtime_suspend(struct device *dev)
> +static __maybe_unused int mma9553_runtime_suspend(struct device *dev)
> {
> struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> struct mma9553_data *data = iio_priv(indio_dev);
> @@ -1161,13 +1160,13 @@ static int mma9553_runtime_suspend(struct device *dev)
> mutex_unlock(&data->mutex);
> if (ret < 0) {
> dev_err(&data->client->dev, "powering off device failed\n");
> - return -EAGAIN;
> + return ret;
> }
>
> return 0;
> }
>
> -static int mma9553_runtime_resume(struct device *dev)
> +static __maybe_unused int mma9553_runtime_resume(struct device *dev)
> {
> struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> struct mma9553_data *data = iio_priv(indio_dev);
> @@ -1181,38 +1180,9 @@ static int mma9553_runtime_resume(struct device *dev)
>
> return 0;
> }
> -#endif
> -
> -#ifdef CONFIG_PM_SLEEP
> -static int mma9553_suspend(struct device *dev)
> -{
> - struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> - struct mma9553_data *data = iio_priv(indio_dev);
> - int ret;
> -
> - mutex_lock(&data->mutex);
> - ret = mma9551_set_device_state(data->client, false);
> - mutex_unlock(&data->mutex);
> -
> - return ret;
> -}
> -
> -static int mma9553_resume(struct device *dev)
> -{
> - struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> - struct mma9553_data *data = iio_priv(indio_dev);
> - int ret;
> -
> - mutex_lock(&data->mutex);
> - ret = mma9551_set_device_state(data->client, true);
> - mutex_unlock(&data->mutex);
> -
> - return ret;
> -}
> -#endif
>
> static const struct dev_pm_ops mma9553_pm_ops = {
> - SET_SYSTEM_SLEEP_PM_OPS(mma9553_suspend, mma9553_resume)
> + SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
> SET_RUNTIME_PM_OPS(mma9553_runtime_suspend,
> mma9553_runtime_resume, NULL)
> };
> --
> 2.31.1
>
