On 17/07/14 01:42, Srinivas Pandruvada wrote:
This chip can support 3 different ranges. Allowing range specification. Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>
A few minor suggestions. Basically fine though.
--- drivers/iio/accel/kxcjk-1013.c | 80 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 79 insertions(+), 1 deletion(-) diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c index 4912143..975f8a6 100644 --- a/drivers/iio/accel/kxcjk-1013.c +++ b/drivers/iio/accel/kxcjk-1013.c @@ -70,6 +70,10 @@ #define KXCJK1013_REG_INT_REG1_BIT_IEA BIT(4) #define KXCJK1013_REG_INT_REG1_BIT_IEN BIT(5) +#define KXCJK1013_RANGE_2G 0x00 +#define KXCJK1013_RANGE_4G 0x01 +#define KXCJK1013_RANGE_8G 0x02
These are effectively used as an enum? Hence I'd make them one.
+ #define KXCJK1013_DATA_MASK_12_BIT 0x0FFF #define KXCJK1013_MAX_STARTUP_TIME_US 100000 @@ -86,6 +90,7 @@ struct kxcjk1013_data { struct mutex mutex; s16 buffer[8]; u8 odr_bits; + u8 range;
with an enum above this becomes a little more obvious too.
bool active_high_intr; bool trigger_on; }; @@ -120,6 +125,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;
Could use a bitfield to make it clear these are single bits..
+} KXCJK1013_scale_table[] = { {9582, 0, 0}, + {19163, 0, 1}, + {38326, 1, 0} }; +
Once the _4G etc defines are an enum, you can explicitly set the values in the table using [KXCJK1013_RANGE_2G] = etc
static int kxcjk1013_set_mode(struct kxcjk1013_data *data, enum kxcjk1013_mode mode) { @@ -190,6 +203,7 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data) /* Setting range to 4G */ ret |= KXCJK1013_REG_CTRL1_BIT_GSEL0; ret &= ~KXCJK1013_REG_CTRL1_BIT_GSEL1; + data->range = KXCJK1013_RANGE_4G;
I'd almost be tempted to call the set function here instead of hand coding this case. I know it will involve more hardware writes, but it will give slightly more obviously correct code (one path to set it rather than two).
/* Set 12 bit mode */ ret |= KXCJK1013_REG_CTRL1_BIT_RES; @@ -422,6 +436,59 @@ static int kxcjk1013_get_startup_times(struct kxcjk1013_data *data) return KXCJK1013_MAX_STARTUP_TIME_US; } +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 = 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[i].gsel_0 << 3); + ret |= (KXCJK1013_scale_table[i].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 = i; + dev_dbg(&data->client->dev, "New Scale range %d\n", i); + + 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) @@ -458,7 +525,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: @@ -485,6 +552,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; } @@ -506,8 +581,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, };
-- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html