[PATCH v2 0/2] kxcjk-1013 updates after last review

[PATCH v2 0/2] kxcjk-1013 updates after last review

[Srinivas Pandruvada]

This address review comments:

v2
Re: [PATCH 3/6] iio: accel: kxcjk-1013: support runtime pm: Date 07/27/14
Switch to always on mode if RUNTIME PM is not defined.

Re: [PATCH 4/6] iio: accel: kxcjk-1013: Set adjustable range: Date 07/27/14
Changes:
Fix range register for 4G and 8G.

v1
1.
Re: [PATCH 4/6] iio: accel: kxcjk-1013: Set adjustable range: Date 07/27/14
Changes:
- Changed defines to enums
- use set function for setting range in kxcjk1013_chip_init, instead of hard coding
2.
Re: [PATCH 3/6] iio: accel: kxcjk-1013: support runtime pm: Date 07/27/14
- Fix compile errors when RUNTIME PM is not defined
Other changes
- Remove module parameters and use autosuspend feature of runtime PM

Srinivas Pandruvada (2):
  iio: accel: kxcjk-1013: support runtime pm
  iio: accel: kxcjk-1013: Set adjustable range

 drivers/iio/accel/kxcjk-1013.c | 294 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 245 insertions(+), 49 deletions(-)

-- 
1.7.11.7

[PATCH v2 1/2] iio: accel: kxcjk-1013: support runtime pm

[Srinivas Pandruvada]

In an effort to improve raw read performance and at the same time enter
low power state at every possible chance.
For raw reads, it will keep the system powered on for a default or user
specified time, via autosuspend_delay attribute of device power.
This will help read multiple samples without power on/off sequence.
For triggers it will keep the system on till, requested to be turned
off by trigger state by utilizing run time PM usage counters.

When runtime pm is not enabled, then it keeps the chip in operation
mode always.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/iio/accel/kxcjk-1013.c | 206 +++++++++++++++++++++++++++++++----------
 1 file changed, 156 insertions(+), 50 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index 7941cf2..b32bdd1 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -21,6 +21,8 @@
 #include <linux/string.h>
 #include <linux/acpi.h>
 #include <linux/gpio/consumer.h>
+#include <linux/pm.h>
+#include <linux/pm_runtime.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
 #include <linux/iio/buffer.h>
@@ -71,15 +73,17 @@
 #define KXCJK1013_DATA_MASK_12_BIT	0x0FFF
 #define KXCJK1013_MAX_STARTUP_TIME_US	100000
 
+#define KXCJK1013_SLEEP_DELAY_MS	2000
+
 struct kxcjk1013_data {
 	struct i2c_client *client;
 	struct iio_trigger *trig;
 	bool trig_mode;
 	struct mutex mutex;
 	s16 buffer[8];
-	int power_state;
 	u8 odr_bits;
 	bool active_high_intr;
+	bool trigger_on;
 };
 
 enum kxcjk1013_axis {
@@ -138,6 +142,25 @@ static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
 	return 0;
 }
 
+static int kxcjk1013_get_mode(struct kxcjk1013_data *data,
+			      enum kxcjk1013_mode *mode)
+{
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_CTRL1);
+	if (ret < 0) {
+		dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
+		return ret;
+	}
+
+	if (ret & KXCJK1013_REG_CTRL1_BIT_PC1)
+		*mode = OPERATION;
+	else
+		*mode = STANDBY;
+
+	return 0;
+}
+
 static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 {
 	int ret;
@@ -201,6 +224,41 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 		return ret;
 	}
 
+	ret = kxcjk1013_set_mode(data, OPERATION);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static int kxcjk1013_get_startup_times(struct kxcjk1013_data *data)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(odr_start_up_times); ++i) {
+		if (odr_start_up_times[i].odr_bits == data->odr_bits)
+			return odr_start_up_times[i].usec;
+	}
+
+	return KXCJK1013_MAX_STARTUP_TIME_US;
+}
+
+static int kxcjk1013_set_power_state(struct kxcjk1013_data *data, bool on)
+{
+	int ret;
+
+	if (on)
+		ret = pm_runtime_get_sync(&data->client->dev);
+	else {
+		pm_runtime_mark_last_busy(&data->client->dev);
+		ret = pm_runtime_put_autosuspend(&data->client->dev);
+	}
+	if (ret < 0) {
+		dev_err(&data->client->dev,
+			"Failed: kxcjk1013_set_power_state for %d\n", on);
+		return ret;
+	}
+
 	return 0;
 }
 
@@ -208,6 +266,11 @@ static int kxcjk1013_chip_setup_interrupt(struct kxcjk1013_data *data,
 					  bool status)
 {
 	int ret;
+	enum kxcjk1013_mode store_mode;
+
+	ret = kxcjk1013_get_mode(data, &store_mode);
+	if (ret < 0)
+		return ret;
 
 	/* This is requirement by spec to change state to STANDBY */
 	ret = kxcjk1013_set_mode(data, STANDBY);
@@ -250,7 +313,13 @@ static int kxcjk1013_chip_setup_interrupt(struct kxcjk1013_data *data,
 		return ret;
 	}
 
-	return ret;
+	if (store_mode == OPERATION) {
+		ret = kxcjk1013_set_mode(data, OPERATION);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
 }
 
 static int kxcjk1013_convert_freq_to_bit(int val, int val2)
@@ -271,6 +340,11 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
 {
 	int ret;
 	int odr_bits;
+	enum kxcjk1013_mode store_mode;
+
+	ret = kxcjk1013_get_mode(data, &store_mode);
+	if (ret < 0)
+		return ret;
 
 	odr_bits = kxcjk1013_convert_freq_to_bit(val, val2);
 	if (odr_bits < 0)
@@ -290,9 +364,7 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
 
 	data->odr_bits = odr_bits;
 
-	/* Check, if the ODR is changed after data enable */
-	if (data->power_state) {
-		/* Set the state back to operation */
+	if (store_mode == OPERATION) {
 		ret = kxcjk1013_set_mode(data, OPERATION);
 		if (ret < 0)
 			return ret;
@@ -331,18 +403,6 @@ static int kxcjk1013_get_acc_reg(struct kxcjk1013_data *data, int axis)
 	return ret;
 }
 
-static int kxcjk1013_get_startup_times(struct kxcjk1013_data *data)
-{
-	int i;
-
-	for (i = 0; i < ARRAY_SIZE(odr_start_up_times); ++i) {
-		if (odr_start_up_times[i].odr_bits == data->odr_bits)
-			return odr_start_up_times[i].usec;
-	}
-
-	return KXCJK1013_MAX_STARTUP_TIME_US;
-}
-
 static int kxcjk1013_read_raw(struct iio_dev *indio_dev,
 			      struct iio_chan_spec const *chan, int *val,
 			      int *val2, long mask)
@@ -356,29 +416,25 @@ static int kxcjk1013_read_raw(struct iio_dev *indio_dev,
 		if (iio_buffer_enabled(indio_dev))
 			ret = -EBUSY;
 		else {
-			int sleep_val;
-
-			ret = kxcjk1013_set_mode(data, OPERATION);
+			ret = kxcjk1013_set_power_state(data, true);
 			if (ret < 0) {
 				mutex_unlock(&data->mutex);
 				return ret;
 			}
-			++data->power_state;
-			sleep_val = kxcjk1013_get_startup_times(data);
-			if (sleep_val < 20000)
-				usleep_range(sleep_val, 20000);
-			else
-				msleep_interruptible(sleep_val/1000);
 			ret = kxcjk1013_get_acc_reg(data, chan->scan_index);
-			if (--data->power_state == 0)
-				kxcjk1013_set_mode(data, STANDBY);
+			if (ret < 0) {
+				kxcjk1013_set_power_state(data, false);
+				mutex_unlock(&data->mutex);
+				return ret;
+			}
+			*val = sign_extend32(ret >> 4, 11);
+			ret = kxcjk1013_set_power_state(data, false);
 		}
 		mutex_unlock(&data->mutex);
 
 		if (ret < 0)
 			return ret;
 
-		*val = sign_extend32(ret >> 4, 11);
 		return IIO_VAL_INT;
 
 	case IIO_CHAN_INFO_SCALE:
@@ -520,20 +576,21 @@ static int kxcjk1013_data_rdy_trigger_set_state(struct iio_trigger *trig,
 {
 	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
+	int ret;
+
+	if (state && data->trigger_on)
+		return 0;
 
 	mutex_lock(&data->mutex);
-	if (state) {
-		kxcjk1013_chip_setup_interrupt(data, true);
-		kxcjk1013_set_mode(data, OPERATION);
-		++data->power_state;
-	} else {
-		if (--data->power_state) {
+	ret = kxcjk1013_chip_setup_interrupt(data, state);
+	if (!ret) {
+		ret = kxcjk1013_set_power_state(data, state);
+		if (ret < 0) {
 			mutex_unlock(&data->mutex);
-			return 0;
+			return ret;
 		}
-		kxcjk1013_chip_setup_interrupt(data, false);
-		kxcjk1013_set_mode(data, STANDBY);
 	}
+	data->trigger_on = state;
 	mutex_unlock(&data->mutex);
 
 	return 0;
@@ -661,14 +718,25 @@ static int kxcjk1013_probe(struct i2c_client *client,
 		}
 	}
 
-	ret = devm_iio_device_register(&client->dev, indio_dev);
+	ret = iio_device_register(indio_dev);
 	if (ret < 0) {
 		dev_err(&client->dev, "unable to register iio device\n");
 		goto err_buffer_cleanup;
 	}
 
+	ret = pm_runtime_set_active(&client->dev);
+	if (ret)
+		goto err_iio_unregister;
+
+	pm_runtime_enable(&client->dev);
+	pm_runtime_set_autosuspend_delay(&client->dev,
+					 KXCJK1013_SLEEP_DELAY_MS);
+	pm_runtime_use_autosuspend(&client->dev);
+
 	return 0;
 
+err_iio_unregister:
+	iio_device_unregister(indio_dev);
 err_buffer_cleanup:
 	if (data->trig_mode)
 		iio_triggered_buffer_cleanup(indio_dev);
@@ -687,6 +755,12 @@ static int kxcjk1013_remove(struct i2c_client *client)
 	struct iio_dev *indio_dev = i2c_get_clientdata(client);
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
 
+	pm_runtime_disable(&client->dev);
+	pm_runtime_set_suspended(&client->dev);
+	pm_runtime_put_noidle(&client->dev);
+
+	iio_device_unregister(indio_dev);
+
 	if (data->trig_mode) {
 		iio_triggered_buffer_cleanup(indio_dev);
 		iio_trigger_unregister(data->trig);
@@ -705,35 +779,67 @@ static int kxcjk1013_suspend(struct device *dev)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
+	int ret;
 
 	mutex_lock(&data->mutex);
-	kxcjk1013_set_mode(data, STANDBY);
+	ret = kxcjk1013_set_mode(data, STANDBY);
 	mutex_unlock(&data->mutex);
 
-	return 0;
+	return ret;
 }
 
 static int kxcjk1013_resume(struct device *dev)
 {
 	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
 	struct kxcjk1013_data *data = iio_priv(indio_dev);
+	int ret = 0;
 
 	mutex_lock(&data->mutex);
+	/* Check, if the suspend occured while active */
+	if (data->trigger_on)
+		ret = kxcjk1013_set_mode(data, OPERATION);
+	mutex_unlock(&data->mutex);
 
-	if (data->power_state)
-		kxcjk1013_set_mode(data, OPERATION);
+	return ret;
+}
+#endif
 
-	mutex_unlock(&data->mutex);
+#ifdef CONFIG_PM_RUNTIME
+static int kxcjk1013_runtime_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+	struct kxcjk1013_data *data = iio_priv(indio_dev);
 
-	return 0;
+	return kxcjk1013_set_mode(data, STANDBY);
 }
 
-static SIMPLE_DEV_PM_OPS(kxcjk1013_pm_ops, kxcjk1013_suspend, kxcjk1013_resume);
-#define KXCJK1013_PM_OPS (&kxcjk1013_pm_ops)
-#else
-#define KXCJK1013_PM_OPS NULL
+static int kxcjk1013_runtime_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
+	struct kxcjk1013_data *data = iio_priv(indio_dev);
+	int ret;
+	int sleep_val;
+
+	ret = kxcjk1013_set_mode(data, OPERATION);
+	if (ret < 0)
+		return ret;
+
+	sleep_val = kxcjk1013_get_startup_times(data);
+	if (sleep_val < 20000)
+		usleep_range(sleep_val, 20000);
+	else
+		msleep_interruptible(sleep_val/1000);
+
+	return 0;
+}
 #endif
 
+static const struct dev_pm_ops kxcjk1013_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(kxcjk1013_suspend, kxcjk1013_resume)
+	SET_RUNTIME_PM_OPS(kxcjk1013_runtime_suspend,
+			   kxcjk1013_runtime_resume, NULL)
+};
+
 static const struct acpi_device_id kx_acpi_match[] = {
 	{"KXCJ1013", 0},
 	{ },
@@ -751,7 +857,7 @@ static struct i2c_driver kxcjk1013_driver = {
 	.driver = {
 		.name	= KXCJK1013_DRV_NAME,
 		.acpi_match_table = ACPI_PTR(kx_acpi_match),
-		.pm	= KXCJK1013_PM_OPS,
+		.pm	= &kxcjk1013_pm_ops,
 	},
 	.probe		= kxcjk1013_probe,
 	.remove		= kxcjk1013_remove,
-- 
1.7.11.7

Re: [PATCH v2 1/2] iio: accel: kxcjk-1013: support runtime pm

[Jonathan Cameron]

On 05/08/14 22:58, Srinivas Pandruvada wrote:
> In an effort to improve raw read performance and at the same time enter
> low power state at every possible chance.
> For raw reads, it will keep the system powered on for a default or user
> specified time, via autosuspend_delay attribute of device power.
> This will help read multiple samples without power on/off sequence.
> For triggers it will keep the system on till, requested to be turned
> off by trigger state by utilizing run time PM usage counters.
> 
> When runtime pm is not enabled, then it keeps the chip in operation
> mode always.
> 
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Looks good.

Applied to the togreg branch of iio.git - initiallly pushed out as
testing.  Thanks.

J
> ---
>  drivers/iio/accel/kxcjk-1013.c | 206 +++++++++++++++++++++++++++++++----------
>  1 file changed, 156 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index 7941cf2..b32bdd1 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -21,6 +21,8 @@
>  #include <linux/string.h>
>  #include <linux/acpi.h>
>  #include <linux/gpio/consumer.h>
> +#include <linux/pm.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
>  #include <linux/iio/buffer.h>
> @@ -71,15 +73,17 @@
>  #define KXCJK1013_DATA_MASK_12_BIT	0x0FFF
>  #define KXCJK1013_MAX_STARTUP_TIME_US	100000
>  
> +#define KXCJK1013_SLEEP_DELAY_MS	2000
> +
>  struct kxcjk1013_data {
>  	struct i2c_client *client;
>  	struct iio_trigger *trig;
>  	bool trig_mode;
>  	struct mutex mutex;
>  	s16 buffer[8];
> -	int power_state;
>  	u8 odr_bits;
>  	bool active_high_intr;
> +	bool trigger_on;
>  };
>  
>  enum kxcjk1013_axis {
> @@ -138,6 +142,25 @@ static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
>  	return 0;
>  }
>  
> +static int kxcjk1013_get_mode(struct kxcjk1013_data *data,
> +			      enum kxcjk1013_mode *mode)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_CTRL1);
> +	if (ret < 0) {
> +		dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
> +		return ret;
> +	}
> +
> +	if (ret & KXCJK1013_REG_CTRL1_BIT_PC1)
> +		*mode = OPERATION;
> +	else
> +		*mode = STANDBY;
> +
> +	return 0;
> +}
> +
>  static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
>  {
>  	int ret;
> @@ -201,6 +224,41 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
>  		return ret;
>  	}
>  
> +	ret = kxcjk1013_set_mode(data, OPERATION);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int kxcjk1013_get_startup_times(struct kxcjk1013_data *data)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(odr_start_up_times); ++i) {
> +		if (odr_start_up_times[i].odr_bits == data->odr_bits)
> +			return odr_start_up_times[i].usec;
> +	}
> +
> +	return KXCJK1013_MAX_STARTUP_TIME_US;
> +}
> +
> +static int kxcjk1013_set_power_state(struct kxcjk1013_data *data, bool on)
> +{
> +	int ret;
> +
> +	if (on)
> +		ret = pm_runtime_get_sync(&data->client->dev);
> +	else {
> +		pm_runtime_mark_last_busy(&data->client->dev);
> +		ret = pm_runtime_put_autosuspend(&data->client->dev);
> +	}
> +	if (ret < 0) {
> +		dev_err(&data->client->dev,
> +			"Failed: kxcjk1013_set_power_state for %d\n", on);
> +		return ret;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -208,6 +266,11 @@ static int kxcjk1013_chip_setup_interrupt(struct kxcjk1013_data *data,
>  					  bool status)
>  {
>  	int ret;
> +	enum kxcjk1013_mode store_mode;
> +
> +	ret = kxcjk1013_get_mode(data, &store_mode);
> +	if (ret < 0)
> +		return ret;
>  
>  	/* This is requirement by spec to change state to STANDBY */
>  	ret = kxcjk1013_set_mode(data, STANDBY);
> @@ -250,7 +313,13 @@ static int kxcjk1013_chip_setup_interrupt(struct kxcjk1013_data *data,
>  		return ret;
>  	}
>  
> -	return ret;
> +	if (store_mode == OPERATION) {
> +		ret = kxcjk1013_set_mode(data, OPERATION);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return 0;
>  }
>  
>  static int kxcjk1013_convert_freq_to_bit(int val, int val2)
> @@ -271,6 +340,11 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
>  {
>  	int ret;
>  	int odr_bits;
> +	enum kxcjk1013_mode store_mode;
> +
> +	ret = kxcjk1013_get_mode(data, &store_mode);
> +	if (ret < 0)
> +		return ret;
>  
>  	odr_bits = kxcjk1013_convert_freq_to_bit(val, val2);
>  	if (odr_bits < 0)
> @@ -290,9 +364,7 @@ static int kxcjk1013_set_odr(struct kxcjk1013_data *data, int val, int val2)
>  
>  	data->odr_bits = odr_bits;
>  
> -	/* Check, if the ODR is changed after data enable */
> -	if (data->power_state) {
> -		/* Set the state back to operation */
> +	if (store_mode == OPERATION) {
>  		ret = kxcjk1013_set_mode(data, OPERATION);
>  		if (ret < 0)
>  			return ret;
> @@ -331,18 +403,6 @@ static int kxcjk1013_get_acc_reg(struct kxcjk1013_data *data, int axis)
>  	return ret;
>  }
>  
> -static int kxcjk1013_get_startup_times(struct kxcjk1013_data *data)
> -{
> -	int i;
> -
> -	for (i = 0; i < ARRAY_SIZE(odr_start_up_times); ++i) {
> -		if (odr_start_up_times[i].odr_bits == data->odr_bits)
> -			return odr_start_up_times[i].usec;
> -	}
> -
> -	return KXCJK1013_MAX_STARTUP_TIME_US;
> -}
> -
>  static int kxcjk1013_read_raw(struct iio_dev *indio_dev,
>  			      struct iio_chan_spec const *chan, int *val,
>  			      int *val2, long mask)
> @@ -356,29 +416,25 @@ static int kxcjk1013_read_raw(struct iio_dev *indio_dev,
>  		if (iio_buffer_enabled(indio_dev))
>  			ret = -EBUSY;
>  		else {
> -			int sleep_val;
> -
> -			ret = kxcjk1013_set_mode(data, OPERATION);
> +			ret = kxcjk1013_set_power_state(data, true);
>  			if (ret < 0) {
>  				mutex_unlock(&data->mutex);
>  				return ret;
>  			}
> -			++data->power_state;
> -			sleep_val = kxcjk1013_get_startup_times(data);
> -			if (sleep_val < 20000)
> -				usleep_range(sleep_val, 20000);
> -			else
> -				msleep_interruptible(sleep_val/1000);
>  			ret = kxcjk1013_get_acc_reg(data, chan->scan_index);
> -			if (--data->power_state == 0)
> -				kxcjk1013_set_mode(data, STANDBY);
> +			if (ret < 0) {
> +				kxcjk1013_set_power_state(data, false);
> +				mutex_unlock(&data->mutex);
> +				return ret;
> +			}
> +			*val = sign_extend32(ret >> 4, 11);
> +			ret = kxcjk1013_set_power_state(data, false);
>  		}
>  		mutex_unlock(&data->mutex);
>  
>  		if (ret < 0)
>  			return ret;
>  
> -		*val = sign_extend32(ret >> 4, 11);
>  		return IIO_VAL_INT;
>  
>  	case IIO_CHAN_INFO_SCALE:
> @@ -520,20 +576,21 @@ static int kxcjk1013_data_rdy_trigger_set_state(struct iio_trigger *trig,
>  {
>  	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
>  	struct kxcjk1013_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	if (state && data->trigger_on)
> +		return 0;
>  
>  	mutex_lock(&data->mutex);
> -	if (state) {
> -		kxcjk1013_chip_setup_interrupt(data, true);
> -		kxcjk1013_set_mode(data, OPERATION);
> -		++data->power_state;
> -	} else {
> -		if (--data->power_state) {
> +	ret = kxcjk1013_chip_setup_interrupt(data, state);
> +	if (!ret) {
> +		ret = kxcjk1013_set_power_state(data, state);
> +		if (ret < 0) {
>  			mutex_unlock(&data->mutex);
> -			return 0;
> +			return ret;
>  		}
> -		kxcjk1013_chip_setup_interrupt(data, false);
> -		kxcjk1013_set_mode(data, STANDBY);
>  	}
> +	data->trigger_on = state;
>  	mutex_unlock(&data->mutex);
>  
>  	return 0;
> @@ -661,14 +718,25 @@ static int kxcjk1013_probe(struct i2c_client *client,
>  		}
>  	}
>  
> -	ret = devm_iio_device_register(&client->dev, indio_dev);
> +	ret = iio_device_register(indio_dev);
>  	if (ret < 0) {
>  		dev_err(&client->dev, "unable to register iio device\n");
>  		goto err_buffer_cleanup;
>  	}
>  
> +	ret = pm_runtime_set_active(&client->dev);
> +	if (ret)
> +		goto err_iio_unregister;
> +
> +	pm_runtime_enable(&client->dev);
> +	pm_runtime_set_autosuspend_delay(&client->dev,
> +					 KXCJK1013_SLEEP_DELAY_MS);
> +	pm_runtime_use_autosuspend(&client->dev);
> +
>  	return 0;
>  
> +err_iio_unregister:
> +	iio_device_unregister(indio_dev);
>  err_buffer_cleanup:
>  	if (data->trig_mode)
>  		iio_triggered_buffer_cleanup(indio_dev);
> @@ -687,6 +755,12 @@ static int kxcjk1013_remove(struct i2c_client *client)
>  	struct iio_dev *indio_dev = i2c_get_clientdata(client);
>  	struct kxcjk1013_data *data = iio_priv(indio_dev);
>  
> +	pm_runtime_disable(&client->dev);
> +	pm_runtime_set_suspended(&client->dev);
> +	pm_runtime_put_noidle(&client->dev);
> +
> +	iio_device_unregister(indio_dev);
> +
>  	if (data->trig_mode) {
>  		iio_triggered_buffer_cleanup(indio_dev);
>  		iio_trigger_unregister(data->trig);
> @@ -705,35 +779,67 @@ static int kxcjk1013_suspend(struct device *dev)
>  {
>  	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
>  	struct kxcjk1013_data *data = iio_priv(indio_dev);
> +	int ret;
>  
>  	mutex_lock(&data->mutex);
> -	kxcjk1013_set_mode(data, STANDBY);
> +	ret = kxcjk1013_set_mode(data, STANDBY);
>  	mutex_unlock(&data->mutex);
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static int kxcjk1013_resume(struct device *dev)
>  {
>  	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
>  	struct kxcjk1013_data *data = iio_priv(indio_dev);
> +	int ret = 0;
>  
>  	mutex_lock(&data->mutex);
> +	/* Check, if the suspend occured while active */
> +	if (data->trigger_on)
> +		ret = kxcjk1013_set_mode(data, OPERATION);
> +	mutex_unlock(&data->mutex);
>  
> -	if (data->power_state)
> -		kxcjk1013_set_mode(data, OPERATION);
> +	return ret;
> +}
> +#endif
>  
> -	mutex_unlock(&data->mutex);
> +#ifdef CONFIG_PM_RUNTIME
> +static int kxcjk1013_runtime_suspend(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> +	struct kxcjk1013_data *data = iio_priv(indio_dev);
>  
> -	return 0;
> +	return kxcjk1013_set_mode(data, STANDBY);
>  }
>  
> -static SIMPLE_DEV_PM_OPS(kxcjk1013_pm_ops, kxcjk1013_suspend, kxcjk1013_resume);
> -#define KXCJK1013_PM_OPS (&kxcjk1013_pm_ops)
> -#else
> -#define KXCJK1013_PM_OPS NULL
> +static int kxcjk1013_runtime_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = i2c_get_clientdata(to_i2c_client(dev));
> +	struct kxcjk1013_data *data = iio_priv(indio_dev);
> +	int ret;
> +	int sleep_val;
> +
> +	ret = kxcjk1013_set_mode(data, OPERATION);
> +	if (ret < 0)
> +		return ret;
> +
> +	sleep_val = kxcjk1013_get_startup_times(data);
> +	if (sleep_val < 20000)
> +		usleep_range(sleep_val, 20000);
> +	else
> +		msleep_interruptible(sleep_val/1000);
> +
> +	return 0;
> +}
>  #endif
>  
> +static const struct dev_pm_ops kxcjk1013_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(kxcjk1013_suspend, kxcjk1013_resume)
> +	SET_RUNTIME_PM_OPS(kxcjk1013_runtime_suspend,
> +			   kxcjk1013_runtime_resume, NULL)
> +};
> +
>  static const struct acpi_device_id kx_acpi_match[] = {
>  	{"KXCJ1013", 0},
>  	{ },
> @@ -751,7 +857,7 @@ static struct i2c_driver kxcjk1013_driver = {
>  	.driver = {
>  		.name	= KXCJK1013_DRV_NAME,
>  		.acpi_match_table = ACPI_PTR(kx_acpi_match),
> -		.pm	= KXCJK1013_PM_OPS,
> +		.pm	= &kxcjk1013_pm_ops,
>  	},
>  	.probe		= kxcjk1013_probe,
>  	.remove		= kxcjk1013_remove,
> 

[PATCH v2 2/2] iio: accel: kxcjk-1013: Set adjustable range

[Srinivas Pandruvada]

This chip can support 3 different ranges. Allowing range specification.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/iio/accel/kxcjk-1013.c | 100 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 95 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
index b32bdd1..57c515b 100644
--- a/drivers/iio/accel/kxcjk-1013.c
+++ b/drivers/iio/accel/kxcjk-1013.c
@@ -82,6 +82,7 @@ struct kxcjk1013_data {
 	struct mutex mutex;
 	s16 buffer[8];
 	u8 odr_bits;
+	u8 range;
 	bool active_high_intr;
 	bool trigger_on;
 };
@@ -97,6 +98,12 @@ enum kxcjk1013_mode {
 	OPERATION,
 };
 
+enum kxcjk1013_range {
+	KXCJK1013_RANGE_2G,
+	KXCJK1013_RANGE_4G,
+	KXCJK1013_RANGE_8G,
+};
+
 static const struct {
 	int val;
 	int val2;
@@ -116,6 +123,14 @@ static const struct {
 			   {0x02, 21000}, {0x03, 11000}, {0x04, 6400},
 			   {0x05, 3900}, {0x06, 2700}, {0x07, 2100} };
 
+static const struct {
+	u16 scale;
+	u8 gsel_0;
+	u8 gsel_1;
+} KXCJK1013_scale_table[] = { {9582, 0, 0},
+			      {19163, 1, 0},
+			      {38326, 0, 1} };
+
 static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
 			      enum kxcjk1013_mode mode)
 {
@@ -161,6 +176,32 @@ static int kxcjk1013_get_mode(struct kxcjk1013_data *data,
 	return 0;
 }
 
+static int kxcjk1013_set_range(struct kxcjk1013_data *data, int range_index)
+{
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_CTRL1);
+	if (ret < 0) {
+		dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
+		return ret;
+	}
+
+	ret |= (KXCJK1013_scale_table[range_index].gsel_0 << 3);
+	ret |= (KXCJK1013_scale_table[range_index].gsel_1 << 4);
+
+	ret = i2c_smbus_write_byte_data(data->client,
+					KXCJK1013_REG_CTRL1,
+					ret);
+	if (ret < 0) {
+		dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
+		return ret;
+	}
+
+	data->range = range_index;
+
+	return 0;
+}
+
 static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 {
 	int ret;
@@ -183,10 +224,6 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 		return ret;
 	}
 
-	/* Setting range to 4G */
-	ret |= KXCJK1013_REG_CTRL1_BIT_GSEL0;
-	ret &= ~KXCJK1013_REG_CTRL1_BIT_GSEL1;
-
 	/* Set 12 bit mode */
 	ret |= KXCJK1013_REG_CTRL1_BIT_RES;
 
@@ -197,6 +234,14 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
 		return ret;
 	}
 
+	/* Setting range to 4G */
+	ret = kxcjk1013_set_range(data, KXCJK1013_RANGE_4G);
+	if (ret < 0)
+		return ret;
+
+	data->range = KXCJK1013_RANGE_4G;
+
+
 	ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_DATA_CTRL);
 	if (ret < 0) {
 		dev_err(&data->client->dev, "Error reading reg_data_ctrl\n");
@@ -403,6 +448,40 @@ static int kxcjk1013_get_acc_reg(struct kxcjk1013_data *data, int axis)
 	return ret;
 }
 
+static int kxcjk1013_set_scale(struct kxcjk1013_data *data, int val)
+{
+	int ret, i;
+	enum kxcjk1013_mode store_mode;
+
+
+	for (i = 0; i < ARRAY_SIZE(KXCJK1013_scale_table); ++i) {
+		if (KXCJK1013_scale_table[i].scale == val) {
+
+			ret = kxcjk1013_get_mode(data, &store_mode);
+			if (ret < 0)
+				return ret;
+
+			ret = kxcjk1013_set_mode(data, STANDBY);
+			if (ret < 0)
+				return ret;
+
+			ret = kxcjk1013_set_range(data, i);
+			if (ret < 0)
+				return ret;
+
+			if (store_mode == OPERATION) {
+				ret = kxcjk1013_set_mode(data, OPERATION);
+				if (ret)
+					return ret;
+			}
+
+			return 0;
+		}
+	}
+
+	return -EINVAL;
+}
+
 static int kxcjk1013_read_raw(struct iio_dev *indio_dev,
 			      struct iio_chan_spec const *chan, int *val,
 			      int *val2, long mask)
@@ -439,7 +518,7 @@ static int kxcjk1013_read_raw(struct iio_dev *indio_dev,
 
 	case IIO_CHAN_INFO_SCALE:
 		*val = 0;
-		*val2 = 19163; /* range +-4g (4/2047*9.806650) */
+		*val2 = KXCJK1013_scale_table[data->range].scale;
 		return IIO_VAL_INT_PLUS_MICRO;
 
 	case IIO_CHAN_INFO_SAMP_FREQ:
@@ -466,6 +545,14 @@ static int kxcjk1013_write_raw(struct iio_dev *indio_dev,
 		ret = kxcjk1013_set_odr(data, val, val2);
 		mutex_unlock(&data->mutex);
 		break;
+	case IIO_CHAN_INFO_SCALE:
+		if (val)
+			return -EINVAL;
+
+		mutex_lock(&data->mutex);
+		ret = kxcjk1013_set_scale(data, val2);
+		mutex_unlock(&data->mutex);
+		break;
 	default:
 		ret = -EINVAL;
 	}
@@ -487,8 +574,11 @@ static int kxcjk1013_validate_trigger(struct iio_dev *indio_dev,
 static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
 	"0.781000 1.563000 3.125000 6.250000 12.500000 25 50 100 200 400 800 1600");
 
+static IIO_CONST_ATTR(in_accel_scale_available, "0.009582 0.019163 0.038326");
+
 static struct attribute *kxcjk1013_attributes[] = {
 	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
+	&iio_const_attr_in_accel_scale_available.dev_attr.attr,
 	NULL,
 };
 
-- 
1.7.11.7

Re: [PATCH v2 2/2] iio: accel: kxcjk-1013: Set adjustable range

[Jonathan Cameron]

On 05/08/14 22:58, Srinivas Pandruvada wrote:
> This chip can support 3 different ranges. Allowing range specification.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Applied to the togreg branch of iio.git.  Pushed out as testing etc...

Thanks

Jonathan
> ---
>  drivers/iio/accel/kxcjk-1013.c | 100 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 95 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c
> index b32bdd1..57c515b 100644
> --- a/drivers/iio/accel/kxcjk-1013.c
> +++ b/drivers/iio/accel/kxcjk-1013.c
> @@ -82,6 +82,7 @@ struct kxcjk1013_data {
>  	struct mutex mutex;
>  	s16 buffer[8];
>  	u8 odr_bits;
> +	u8 range;
>  	bool active_high_intr;
>  	bool trigger_on;
>  };
> @@ -97,6 +98,12 @@ enum kxcjk1013_mode {
>  	OPERATION,
>  };
>
> +enum kxcjk1013_range {
> +	KXCJK1013_RANGE_2G,
> +	KXCJK1013_RANGE_4G,
> +	KXCJK1013_RANGE_8G,
> +};
> +
>  static const struct {
>  	int val;
>  	int val2;
> @@ -116,6 +123,14 @@ static const struct {
>  			   {0x02, 21000}, {0x03, 11000}, {0x04, 6400},
>  			   {0x05, 3900}, {0x06, 2700}, {0x07, 2100} };
>
> +static const struct {
> +	u16 scale;
> +	u8 gsel_0;
> +	u8 gsel_1;
> +} KXCJK1013_scale_table[] = { {9582, 0, 0},
> +			      {19163, 1, 0},
> +			      {38326, 0, 1} };
> +
>  static int kxcjk1013_set_mode(struct kxcjk1013_data *data,
>  			      enum kxcjk1013_mode mode)
>  {
> @@ -161,6 +176,32 @@ static int kxcjk1013_get_mode(struct kxcjk1013_data *data,
>  	return 0;
>  }
>
> +static int kxcjk1013_set_range(struct kxcjk1013_data *data, int range_index)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_CTRL1);
> +	if (ret < 0) {
> +		dev_err(&data->client->dev, "Error reading reg_ctrl1\n");
> +		return ret;
> +	}
> +
> +	ret |= (KXCJK1013_scale_table[range_index].gsel_0 << 3);
> +	ret |= (KXCJK1013_scale_table[range_index].gsel_1 << 4);
> +
> +	ret = i2c_smbus_write_byte_data(data->client,
> +					KXCJK1013_REG_CTRL1,
> +					ret);
> +	if (ret < 0) {
> +		dev_err(&data->client->dev, "Error writing reg_ctrl1\n");
> +		return ret;
> +	}
> +
> +	data->range = range_index;
> +
> +	return 0;
> +}
> +
>  static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
>  {
>  	int ret;
> @@ -183,10 +224,6 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
>  		return ret;
>  	}
>
> -	/* Setting range to 4G */
> -	ret |= KXCJK1013_REG_CTRL1_BIT_GSEL0;
> -	ret &= ~KXCJK1013_REG_CTRL1_BIT_GSEL1;
> -
>  	/* Set 12 bit mode */
>  	ret |= KXCJK1013_REG_CTRL1_BIT_RES;
>
> @@ -197,6 +234,14 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data)
>  		return ret;
>  	}
>
> +	/* Setting range to 4G */
> +	ret = kxcjk1013_set_range(data, KXCJK1013_RANGE_4G);
> +	if (ret < 0)
> +		return ret;
> +
> +	data->range = KXCJK1013_RANGE_4G;
> +
> +
>  	ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_DATA_CTRL);
>  	if (ret < 0) {
>  		dev_err(&data->client->dev, "Error reading reg_data_ctrl\n");
> @@ -403,6 +448,40 @@ static int kxcjk1013_get_acc_reg(struct kxcjk1013_data *data, int axis)
>  	return ret;
>  }
>
> +static int kxcjk1013_set_scale(struct kxcjk1013_data *data, int val)
> +{
> +	int ret, i;
> +	enum kxcjk1013_mode store_mode;
> +
> +
> +	for (i = 0; i < ARRAY_SIZE(KXCJK1013_scale_table); ++i) {
> +		if (KXCJK1013_scale_table[i].scale == val) {
> +
> +			ret = kxcjk1013_get_mode(data, &store_mode);
> +			if (ret < 0)
> +				return ret;
> +
> +			ret = kxcjk1013_set_mode(data, STANDBY);
> +			if (ret < 0)
> +				return ret;
> +
> +			ret = kxcjk1013_set_range(data, i);
> +			if (ret < 0)
> +				return ret;
> +
> +			if (store_mode == OPERATION) {
> +				ret = kxcjk1013_set_mode(data, OPERATION);
> +				if (ret)
> +					return ret;
> +			}
> +
> +			return 0;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
>  static int kxcjk1013_read_raw(struct iio_dev *indio_dev,
>  			      struct iio_chan_spec const *chan, int *val,
>  			      int *val2, long mask)
> @@ -439,7 +518,7 @@ static int kxcjk1013_read_raw(struct iio_dev *indio_dev,
>
>  	case IIO_CHAN_INFO_SCALE:
>  		*val = 0;
> -		*val2 = 19163; /* range +-4g (4/2047*9.806650) */
> +		*val2 = KXCJK1013_scale_table[data->range].scale;
>  		return IIO_VAL_INT_PLUS_MICRO;
>
>  	case IIO_CHAN_INFO_SAMP_FREQ:
> @@ -466,6 +545,14 @@ static int kxcjk1013_write_raw(struct iio_dev *indio_dev,
>  		ret = kxcjk1013_set_odr(data, val, val2);
>  		mutex_unlock(&data->mutex);
>  		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		if (val)
> +			return -EINVAL;
> +
> +		mutex_lock(&data->mutex);
> +		ret = kxcjk1013_set_scale(data, val2);
> +		mutex_unlock(&data->mutex);
> +		break;
>  	default:
>  		ret = -EINVAL;
>  	}
> @@ -487,8 +574,11 @@ static int kxcjk1013_validate_trigger(struct iio_dev *indio_dev,
>  static IIO_CONST_ATTR_SAMP_FREQ_AVAIL(
>  	"0.781000 1.563000 3.125000 6.250000 12.500000 25 50 100 200 400 800 1600");
>
> +static IIO_CONST_ATTR(in_accel_scale_available, "0.009582 0.019163 0.038326");
> +
>  static struct attribute *kxcjk1013_attributes[] = {
>  	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
> +	&iio_const_attr_in_accel_scale_available.dev_attr.attr,
>  	NULL,
>  };
>
>