stk8312 fixes and cleanup
cover stk8312 fixes and cleanup
Fixing some issues I spotted during review.
Hartmut Knaack (7):
iio:accel:stk8312: add triggered buffer dependency iio:accel:stk8312: check for invalid value iio:accel:stk8312: improve error handling iio:accel:stk8312: rework macro definitions iio:accel:stk8312: use appropriate variable types iio:accel:stk8312: code style cleanup iio:accel:stk8312: drop local buffer
drivers/iio/accel/Kconfig | 2 ++ drivers/iio/accel/stk8312.c | 85 +++++++++++++++++++++++++-------------------- 2 files changed, 49 insertions(+), 38 deletions(-)
Signature · 1 line
2.4.6
PATCH 1/7 iio:accel:stk8312: add triggered buffer dependency
Add the still missing dependencies for triggered buffer support.
--- drivers/iio/accel/Kconfig | 2 ++ 1 file changed, 2 insertions(+)
drivers/iio/accel/Kconfig +2 -0
@@ -140,6 +140,8 @@ config MMA9553 config STK8312 tristate "Sensortek STK8312 3-Axis Accelerometer Driver" depends on I2C + select IIO_BUFFER + select IIO_TRIGGERED_BUFFER help Say yes here to get support for the Sensortek STK8312 3-axis accelerometer.
Signature · 1 line
2.4.6
RE: [PATCH 1/7] iio:accel:stk8312: add triggered buffer dependency
Quoted 8 lines
-----Original Message-----
From: Hartmut Knaack [mailto:[email protected]]
Sent: Tuesday, July 28, 2015 1:49 AM
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Breana, Tiberiu A
Subject: [PATCH 1/7] iio:accel:stk8312: add triggered buffer dependency
Quoted 5 lines
Add the still missing dependencies for triggered buffer support.
Fixes 95c12bba51c37 ("iio: accel: Add buffer mode for Sensortek STK8312")Signed-off-by Hartmut Knaack <[email protected]>
+1
Quoted 19 lines
--- drivers/iio/accel/Kconfig | 2 ++ 1 file changed, 2 insertions(+)drivers/iio/accel/Kconfig index renamed from drivers/iio/accel/Kconfig +0
-0
00e7bcbdbe24..c99d8e242218100644drivers/iio/accel/Kconfig +2
-1@@ -140,6 +140,8 @@ config MMA9553 config STK8312 tristate "Sensortek STK8312 3-Axis Accelerometer Driver" depends on I2C + select IIO_BUFFER + select IIO_TRIGGERED_BUFFER help Say yes here to get support for the Sensortek STK8312 3-axis accelerometer. --2.4.6
Re: [PATCH 1/7] iio:accel:stk8312: add triggered buffer dependency
Quoted 10 lines · Tiberiu A
Quoted 10 lines
-----Original Message-----
From: Hartmut Knaack [mailto:[email protected]]
Sent: Tuesday, July 28, 2015 1:49 AM
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Breana, Tiberiu A
Subject: [PATCH 1/7] iio:accel:stk8312: add triggered buffer dependencyAdd the still missing dependencies for triggered buffer support.
Quoted 7 lines
Quoted 3 lines
Fixes 95c12bba51c37 ("iio: accel: Add buffer mode for Sensortek STK8312")Signed-off-by Hartmut Knaack <[email protected]>+1
Reviewed-by Tiberiu Breana <[email protected]>
One day I'll remember to actually check all the dependencies when reviewing! Thanks Hartmut.
Applied to the togreg branch of iio.git.
Jonathan
Quoted 26 lines
Quoted 19 lines
--- drivers/iio/accel/Kconfig | 2 ++ 1 file changed, 2 insertions(+)drivers/iio/accel/Kconfig index renamed from drivers/iio/accel/Kconfig +0
-0
00e7bcbdbe24..c99d8e242218100644drivers/iio/accel/Kconfig +2
-1@@ -140,6 +140,8 @@ config MMA9553 config STK8312 tristate "Sensortek STK8312 3-Axis Accelerometer Driver" depends on I2C + select IIO_BUFFER + select IIO_TRIGGERED_BUFFER help Say yes here to get support for the Sensortek STK8312 3-axis accelerometer. --Quoted 1 line
2.4.6
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 2/7 iio:accel:stk8312: check for invalid value
In reply to Hartmut KnaackRevision 1.2 of the datasheet recommends on page 22 to only write non-zero values read from OTP register 0x70 into AFECTRL register.
--- drivers/iio/accel/stk8312.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/iio/accel/stk8312.c +2 -0
@@ -150,6 +150,8 @@ static int stk8312_otp_init(struct stk8312_data *data) goto exit_err; ret =i2c_smbus_read_byte_data (client,STK8312_REG_OTPDATA ); +if (ret ==0 ) + ret = -EINVAL ;if (ret <0 ) goto exit_err;
Signature · 1 line
2.4.6
RE: [PATCH 2/7] iio:accel:stk8312: check for invalid value
Quoted 9 lines
-----Original Message-----
From: [email protected] [mailto:linux-iio- [email protected]] On Behalf Of Hartmut Knaack
Sent: Tuesday, July 28, 2015 1:49 AM
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Breana, Tiberiu A
Subject: [PATCH 2/7] iio:accel:stk8312: check for invalid value
Quoted 4 lines
Revision 1.2 of the datasheet recommends on page 22 to only write non-zero values read from OTP register
0x70into AFECTRL register.Signed-off-by Hartmut Knaack <[email protected]>
+1
Quoted 24 lines
--- drivers/iio/accel/stk8312.c | 2 ++ 1 file changed, 2 insertions(+)drivers/iio/accel/stk8312.c index renamed from drivers/iio/accel/stk8312.c +0
-0
c2bd1444d6da..6592be8e6377100644drivers/iio/accel/stk8312.c +2
-1@@ -150,6 +150,8 @@ static int stk8312_otp_init(struct stk8312_data *data) goto exit_err; ret =i2c_smbus_read_byte_data (client,STK8312_REG_OTPDATA ); +if (ret ==0 ) + ret = -EINVAL ;if (ret <0 ) goto exit_err; --2.4.6
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected] More majordomo info at
http://vger.kernel.org/majordomo-info.html
PATCH 3/7 iio:accel:stk8312: improve error handling
In reply to Hartmut KnaackImprove error handling in the following ways:
- set return value on error condition to an appropriate error code
- return error code immediately in case of an error (slightly changes code structure)
- pass up real error code
- add missing error handling
- return 0 when error have been caught already
--- drivers/iio/accel/stk8312.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-)
drivers/iio/accel/stk8312.c +25 -14
@@ -146,8 +146,10 @@ static int stk8312_otp_init(struct stk8312_data *data) count--; }while (!(ret &0x80 ) && count >0 ); -if (count ==0 ) +if (count ==0 ) { + ret = -ETIMEDOUT ; goto exit_err; + } ret =i2c_smbus_read_byte_data (client,STK8312_REG_OTPDATA );if (ret ==0 )
@@ -161,7 +163,7 @@ static int stk8312_otp_init(struct stk8312_data *data) goto exit_err;msleep (150 ); -return ret; +return 0 ; exit_err:dev_err (&client->dev,"failed to initialize sensor\n" );
@@ -205,8 +207,10 @@ static int stk8312_set_interrupts(struct stk8312_data *data, u8 int_mask)return ret; ret =i2c_smbus_write_byte_data (client,STK8312_REG_INTSU , int_mask); -if (ret <0 ) +if (ret <0 ) {dev_err (&client->dev,"failed to set interrupts\n" ); +return ret; + }return stk8312_set_mode (data, mode); }
@@ -230,7 +234,7 @@ static int stk8312_data_rdy_trigger_set_state(struct iio_trigger *trig, data->dready_trigger_on = state; -return ret; +return 0 ; }static const struct iio_trigger_ops stk8312_trigger_ops = {
@@ -263,10 +267,12 @@ static int stk8312_set_sample_rate(struct stk8312_data *data, int rate) masked_reg = (ret & (~STK8312_SR_MASK )) | rate; ret =i2c_smbus_write_byte_data (client,STK8312_REG_SR , masked_reg); -if (ret <0 ) +if (ret <0 ) {dev_err (&client->dev,"failed to set sampling rate\n" ); -else - data->sample_rate_idx = rate; +return ret; + } + + data->sample_rate_idx = rate;return stk8312_set_mode (data, mode); }
@@ -299,10 +305,12 @@ static int stk8312_set_range(struct stk8312_data *data, u8 range) masked_reg |= range <<STK8312_RNG_SHIFT ; ret =i2c_smbus_write_byte_data (client,STK8312_REG_STH , masked_reg); -if (ret <0 ) +if (ret <0 ) {dev_err (&client->dev,"failed to change sensor range\n" ); -else - data->range = range; +return ret; + } + + data->range = range;return stk8312_set_mode (data, mode); }
@@ -337,18 +345,21 @@ static int stk8312_read_raw(struct iio_dev *indio_dev, ret =stk8312_set_mode (data, data->mode |STK8312_MODE_ACTIVE );if (ret <0 ) {mutex_unlock (&data->lock); -return -EINVAL ; +return ret; } ret =stk8312_read_accel (data, chan->address);if (ret <0 ) {stk8312_set_mode (data, data->mode & (~STK8312_MODE_ACTIVE ));mutex_unlock (&data->lock); -return -EINVAL ; +return ret; } *val =sign_extend32 (ret,7 ); -stk8312_set_mode (data, data->mode & (~STK8312_MODE_ACTIVE )); + ret =stk8312_set_mode (data, + data->mode & (~STK8312_MODE_ACTIVE ));mutex_unlock (&data->lock); +if (ret <0 ) +return ret;return IIO_VAL_INT ;case IIO_CHAN_INFO_SCALE : *val = stk8312_scale_table[data->range -1 ][0 ];
@@ -608,7 +619,7 @@ static int stk8312_probe(struct i2c_client *client, goto err_buffer_cleanup; } -return ret; +return 0 ; err_buffer_cleanup:iio_triggered_buffer_cleanup (indio_dev);
Signature · 1 line
2.4.6
RE: [PATCH 3/7] iio:accel:stk8312: improve error handling
Quoted 16 lines
-----Original Message-----
From: Hartmut Knaack [mailto:[email protected]]
Sent: Tuesday, July 28, 2015 1:49 AM
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Breana, Tiberiu A
Subject: [PATCH 3/7] iio:accel:stk8312: improve error handlingImprove error handling in the following ways:
- set return value on error condition to an appropriate error code
- return error code immediately in case of an error (slightly changes code structure)
- pass up real error code
- add missing error handling
- return 0 when error have been caught already
Quoted 1 line
Signed-off-by Hartmut Knaack <[email protected]>
Comments inline.
Quoted 60 lines
--- drivers/iio/accel/stk8312.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-)drivers/iio/accel/stk8312.c index renamed from drivers/iio/accel/stk8312.c +0
-0
6592be8e6377..50a102ca44d3100644drivers/iio/accel/stk8312.c +3
-1@@ -146,8 +146,10 @@ static int stk8312_otp_init(struct stk8312_data *data) count--; }while (!(ret &0x80 ) && count >0 ); -if (count ==0 ) +if (count ==0 ) { + ret = -ETIMEDOUT ; goto exit_err; + } ret =i2c_smbus_read_byte_data (client,STK8312_REG_OTPDATA );drivers/iio/accel/stk8312.c +2
-1if (ret ==0 )@@ -161,7 +163,7 @@ static int stk8312_otp_init(struct stk8312_data *data) goto exit_err;msleep (150 ); -return ret; +return 0 ; exit_err: dev_err(&client->dev, "failed to initialize sensor\n"); @@ -205,8 +207,10 @@ static int stk8312_set_interrupts(struct stk8312_data *data, u8
int_mask)drivers/iio/accel/stk8312.c +0
-0return ret; ret = i2c_smbus_write_byte_data(client, STK8312_REG_INTSU,
int_mask);drivers/iio/accel/stk8312.c +4
-2-if (ret <0 ) +if (ret <0 ) {dev_err (&client->dev,"failed to set interrupts\n" ); +return ret; + }return stk8312_set_mode (data, mode); }@@ -230,7 +234,7 @@ static int stk8312_data_rdy_trigger_set_state(struct iio_trigger *trig, data->dready_trigger_on = state; -return ret; +return 0 ; }static const struct
iio_trigger_opsstk8312_trigger_ops= { @@ -263,10
+267,12 @@ static intstk8312_set_sample_rate(struct stk8312_data *data, int rate)drivers/iio/accel/stk8312.c +0
-0masked_reg = (ret & (~STK8312_SR_MASK )) | rate; ret = i2c_smbus_write_byte_data(client, STK8312_REG_SR,
masked_reg);
drivers/iio/accel/stk8312.c static int stk8312_set_sample_rate(struct stk8312_data *data, int rate) · line 272
Quoted 6 lines
drivers/iio/accel/stk8312.c +2
-3-if (ret <0 ) +if (ret <0 ) {dev_err (&client->dev,"failed to set sampling rate\n" ); -else - data->sample_rate_idx = rate; +return ret;
We can't just return here. Bear in mind that earlier in the function we set the sensor's operation mode to standby. Return to active mode before returning. This should be added @258 as well.
Quoted 48 lines
drivers/iio/accel/stk8312.c +8
-3+ } + + data->sample_rate_idx = rate;return stk8312_set_mode (data, mode); }@@ -299,10 +305,12 @@ static int stk8312_set_range(struct stk8312_data *data, u8 range) masked_reg |= range <<STK8312_RNG_SHIFT ; ret =i2c_smbus_write_byte_data (client,STK8312_REG_STH , masked_reg); -if (ret <0 ) +if (ret <0 ) {dev_err (&client->dev,"failed to change sensor range\n" ); -else - data->range = range; +return ret; + } + + data->range = range;drivers/iio/accel/stk8312.c +4
-3return stk8312_set_mode (data, mode); }@@ -337,18 +345,21 @@ static int stk8312_read_raw(struct iio_dev *indio_dev, ret =stk8312_set_mode (data, data->mode |STK8312_MODE_ACTIVE );if (ret <0 ) {mutex_unlock (&data->lock); -return -EINVAL ; +return ret; } ret =stk8312_read_accel (data, chan->address);if (ret <0 ) {stk8312_set_mode (data, data->mode & (~STK8312_MODE_ACTIVE ));mutex_unlock (&data->lock); -return -EINVAL ; +return ret; } *val =sign_extend32 (ret,7 ); -stk8312_set_mode (data, data->mode & (~STK8312_MODE_ACTIVE)); + ret = stk8312_set_mode(data, + data->mode &(~STK8312_MODE_ACTIVE));
drivers/iio/accel/stk8312.c static int stk8312_read_raw(struct iio_dev *indio_dev, · line 362
Quoted 3 lines
drivers/iio/accel/stk8312.c +2
-0mutex_unlock (&data->lock); +if (ret <0 ) +return ret;
I don't agree with this. We've succesfully read the data, we shouldn't return an error if the sensor couldn't be put back to standby. I suggest leaving it as it is.
Quoted 14 lines
drivers/iio/accel/stk8312.c +2
-2return IIO_VAL_INT ;case IIO_CHAN_INFO_SCALE : *val = stk8312_scale_table[data->range - 1][0]; @@ -608,7 +619,7 @@ static int stk8312_probe(struct i2c_client *client, goto err_buffer_cleanup; } -return ret; +return 0 ; err_buffer_cleanup:iio_triggered_buffer_cleanup (indio_dev); --2.4.6
Re: [PATCH 3/7] iio:accel:stk8312: improve error handling
Breana, Tiberiu A schrieb am 28.07.2015 um 15:38:
Quoted 16 lines
Quoted 16 lines
-----Original Message-----
From: Hartmut Knaack [mailto:[email protected]]
Sent: Tuesday, July 28, 2015 1:49 AM
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Breana, Tiberiu A
Subject: [PATCH 3/7] iio:accel:stk8312: improve error handlingImprove error handling in the following ways:
- set return value on error condition to an appropriate error code
- return error code immediately in case of an error (slightly changes code structure)
- pass up real error code
- add missing error handling
- return 0 when error have been caught already
Quoted 4 lines
Quoted 1 line
Signed-off-by Hartmut Knaack <[email protected]>Comments inline.
Hi,
<...>
Quoted 4 lines
Quoted 4 lines
+267,12 @@ static int
stk8312_set_sample_rate(struct stk8312_data *data, int rate)
masked_reg= (ret & (~STK8312_SR_MASK)) | rate;
Quoted 13 lines
drivers/iio/accel/stk8312.c static int stk8312_set_sample_rate(struct stk8312_data *data, int rate) · line 272
Quoted 8 lines
ret = i2c_smbus_write_byte_data(client,
STK8312_REG_SR,masked_reg);drivers/iio/accel/stk8312.c +2
-3-if (ret <0 ) +if (ret <0 ) {dev_err (&client->dev,"failed to set sampling rate\n" ); -else - data->sample_rate_idx = rate; +return ret;We can't just return here. Bear in mind that earlier in the function we set the sensor's operation mode to standby. Return to active mode before returning. This should be added @258 as well.
OK, I will respin with a common error path to put the device back in active mode.
Quoted 46 lines
Quoted 46 lines
drivers/iio/accel/stk8312.c +8
-3+ } + + data->sample_rate_idx = rate;return stk8312_set_mode (data, mode); }@@ -299,10 +305,12 @@ static int stk8312_set_range(struct stk8312_data *data, u8 range) masked_reg |= range <<STK8312_RNG_SHIFT ; ret =i2c_smbus_write_byte_data (client,STK8312_REG_STH , masked_reg); -if (ret <0 ) +if (ret <0 ) {dev_err (&client->dev,"failed to change sensor range\n" ); -else - data->range = range; +return ret; + } + + data->range = range;drivers/iio/accel/stk8312.c +3
-3return stk8312_set_mode (data, mode); }@@ -337,18 +345,21 @@ static int stk8312_read_raw(struct iio_dev *indio_dev, ret =stk8312_set_mode (data, data->mode |STK8312_MODE_ACTIVE );if (ret <0 ) {mutex_unlock (&data->lock); -return -EINVAL ; +return ret; } ret =stk8312_read_accel (data, chan->address);if (ret <0 ) {stk8312_set_mode (data, data->mode & (~STK8312_MODE_ACTIVE ));mutex_unlock (&data->lock); -return -EINVAL ; +return ret; } *val =sign_extend32 (ret,7 ); -stk8312_set_mode (data, data->mode & (~STK8312_MODE_ACTIVE)); + ret = stk8312_set_mode(data,
Quoted 9 lines
drivers/iio/accel/stk8312.c static int stk8312_read_raw(struct iio_dev *indio_dev, · line 362
Quoted 5 lines
(quoted diff) +1
-0+ data->mode &(~STK8312_MODE_ACTIVE));
drivers/iio/accel/stk8312.c +2
-0mutex_unlock (&data->lock); +if (ret <0 ) +return ret;I don't agree with this. We've succesfully read the data, we shouldn't return an error if the sensor couldn't be put back to standby. I suggest leaving it as it is.
Well, encountering an error should be an exceptional case, which should not happen in normal operation (and thus do no harm). But something goes wrong, then better report it, even a data sample has been read successfully.
Thanks,
Hartmut
Quoted 20 lines
Quoted 13 lines
drivers/iio/accel/stk8312.c +2
-2return IIO_VAL_INT ;case IIO_CHAN_INFO_SCALE : *val = stk8312_scale_table[data->range - 1][0]; @@ -608,7 +619,7 @@ static int stk8312_probe(struct i2c_client *client, goto err_buffer_cleanup; } -return ret; +return 0 ; err_buffer_cleanup:iio_triggered_buffer_cleanup (indio_dev); --Quoted 1 line
2.4.6
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 4/7 iio:accel:stk8312: rework macro definitions
In reply to Hartmut KnaackMake use of BIT to describe register bits, GENMASK for consecutive bitmasks, rename and sort existing definitions, replace magic value with an expressive definition, drop an unused definition.
--- drivers/iio/accel/stk8312.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-)
drivers/iio/accel/stk8312.c +13 -12
@@ -37,16 +37,16 @@#define STK8312_REG_OTPDATA 0x3E#define STK8312_REG_OTPCTRL 0x3F -#define STK8312_MODE_ACTIVE 0x01 +#define STK8312_MODE_ACTIVE BIT(0)#define STK8312_MODE_STANDBY 0x00 -#define STK8312_DREADY_BIT 0x10 -#define STK8312_INT_MODE 0xC0 -#define STK8312_RNG_MASK 0xC0 -#define STK8312_SR_MASK 0x07 -#define STK8312_SR_400HZ_IDX 0 +#define STK8312_MODE_INT_AH_PP 0xC0/* active-high, push-pull */ +#define STK8312_DREADY_BIT BIT(4) +#define STK8312_RNG_6G 1#define STK8312_RNG_SHIFT 6 -#define STK8312_READ_RETRIES 16 -#define STK8312_ALL_CHANNEL_MASK 7 +#define STK8312_RNG_MASK GENMASK(7, 6) +#define STK8312_SR_MASK GENMASK(2, 0) +#define STK8312_SR_400HZ_IDX 0 +#define STK8312_ALL_CHANNEL_MASK GENMASK(2, 0)#define STK8312_ALL_CHANNEL_SIZE 3#define STK8312_DRIVER_NAME "stk8312"
@@ -144,7 +144,7 @@ static int stk8312_otp_init(struct stk8312_data *data)if (ret <0 ) goto exit_err; count--; - }while (!(ret &0x80 ) && count >0 ); + }while (!(ret &BIT (7 )) && count >0 );if (count ==0 ) { ret = -ETIMEDOUT ;
@@ -560,11 +560,12 @@ static int stk8312_probe(struct i2c_client *client,return ret; } data->sample_rate_idx =STK8312_SR_400HZ_IDX ; - ret =stk8312_set_range (data,1 ); + ret =stk8312_set_range (data,STK8312_RNG_6G );if (ret <0 )return ret; - ret =stk8312_set_mode (data,STK8312_INT_MODE |STK8312_MODE_ACTIVE ); + ret =stk8312_set_mode (data, +STK8312_MODE_INT_AH_PP |STK8312_MODE_ACTIVE );if (ret <0 )return ret;
@@ -685,7 +686,7 @@ MODULE_DEVICE_TABLE(acpi, stk8312_acpi_id);static struct i2c_driver stk8312_driver = { .driver = { - .name ="stk8312" , + .name =STK8312_DRIVER_NAME , .pm =STK8312_PM_OPS , .acpi_match_table =ACPI_PTR (stk8312_acpi_id), },
Signature · 1 line
2.4.6
RE: [PATCH 4/7] iio:accel:stk8312: rework macro definitions
Quoted 8 lines
-----Original Message-----
From: Hartmut Knaack [mailto:[email protected]]
Sent: Tuesday, July 28, 2015 1:49 AM
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Breana, Tiberiu A
Subject: [PATCH 4/7] iio:accel:stk8312: rework macro definitions
Quoted 5 lines
Make use of BIT to describe register bits, GENMASK for consecutive bitmasks, rename and sort existing definitions, replace magic value with an expressive definition, drop an unused definition.
Signed-off-by Hartmut Knaack <[email protected]>
+1
Quoted 71 lines
--- drivers/iio/accel/stk8312.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-)drivers/iio/accel/stk8312.c index renamed from drivers/iio/accel/stk8312.c +0
-0
50a102ca44d3..8783f0bf7c53100644drivers/iio/accel/stk8312.c +2
-6@@ -37,16 +37,16 @@#define STK8312_REG_OTPDATA 0x3E#define STK8312_REG_OTPCTRL 0x3F -#define STK8312_MODE_ACTIVE 0x01 +#define STK8312_MODE_ACTIVE BIT(0)#define STK8312_MODE_STANDBY 0x00 -#define STK8312_DREADY_BIT 0x10 -#define STK8312_INT_MODE 0xC0 -#define STK8312_RNG_MASK 0xC0 -#define STK8312_SR_MASK 0x07 -#define STK8312_SR_400HZ_IDX 0 +#define STK8312_MODE_INT_AH_PP 0xC0/* active-high, push- pull */
drivers/iio/accel/stk8312.c +10
-5+#define STK8312_DREADY_BIT BIT(4) +#define STK8312_RNG_6G 1#define STK8312_RNG_SHIFT 6 -#define STK8312_READ_RETRIES 16 -#define STK8312_ALL_CHANNEL_MASK 7 +#define STK8312_RNG_MASK GENMASK(7, 6) +#define STK8312_SR_MASK GENMASK(2, 0) +#define STK8312_SR_400HZ_IDX 0 +#define STK8312_ALL_CHANNEL_MASK GENMASK(2, 0)#define STK8312_ALL_CHANNEL_SIZE 3#define STK8312_DRIVER_NAME "stk8312"@@ -144,7 +144,7 @@ static int stk8312_otp_init(struct stk8312_data *data)if (ret <0 ) goto exit_err; count--; - }while (!(ret &0x80 ) && count >0 ); + }while (!(ret &BIT (7 )) && count >0 );if (count ==0 ) { ret = -ETIMEDOUT ;@@ -560,11 +560,12 @@ static int stk8312_probe(struct i2c_client *client,return ret; } data->sample_rate_idx =STK8312_SR_400HZ_IDX ; - ret =stk8312_set_range (data,1 ); + ret =stk8312_set_range (data,STK8312_RNG_6G );if (ret <0 )return ret; - ret =stk8312_set_mode (data,STK8312_INT_MODE | STK8312_MODE_ACTIVE); + ret =stk8312_set_mode (data, +STK8312_MODE_INT_AH_PP |STK8312_MODE_ACTIVE );if (ret <0 )drivers/iio/accel/stk8312.c +1
-2return ret;@@ -685,7 +686,7 @@ MODULE_DEVICE_TABLE(acpi, stk8312_acpi_id);static struct i2c_driver stk8312_driver = { .driver = { - .name ="stk8312" , + .name =STK8312_DRIVER_NAME , .pm =STK8312_PM_OPS , .acpi_match_table =ACPI_PTR (stk8312_acpi_id), }, --2.4.6
Jonathan Cameron
maintainerRe: [PATCH 4/7] iio:accel:stk8312: rework macro definitions
Quoted 12 lines · Tiberiu A
Quoted 12 lines
-----Original Message-----
From: Hartmut Knaack [mailto:[email protected]]
Sent: Tuesday, July 28, 2015 1:49 AM
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Breana, Tiberiu A
Subject: [PATCH 4/7] iio:accel:stk8312: rework macro definitionsMake use of BIT to describe register bits, GENMASK for consecutive bitmasks, rename and sort existing definitions, replace magic value with an expressive definition, drop an unused definition.
Quoted 5 lines
Quoted 1 line
Signed-off-by Hartmut Knaack <[email protected]>+1
Reviewed-by Tiberiu Breana <[email protected]>
Start getting fuzz over the earlier patch that you are rerolling. Hence will stop here until you have rerolled.
Jonathan
Quoted 78 lines
Quoted 71 lines
--- drivers/iio/accel/stk8312.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-)drivers/iio/accel/stk8312.c index renamed from drivers/iio/accel/stk8312.c +0
-0
50a102ca44d3..8783f0bf7c53100644drivers/iio/accel/stk8312.c +2
-6@@ -37,16 +37,16 @@#define STK8312_REG_OTPDATA 0x3E#define STK8312_REG_OTPCTRL 0x3F -#define STK8312_MODE_ACTIVE 0x01 +#define STK8312_MODE_ACTIVE BIT(0)#define STK8312_MODE_STANDBY 0x00 -#define STK8312_DREADY_BIT 0x10 -#define STK8312_INT_MODE 0xC0 -#define STK8312_RNG_MASK 0xC0 -#define STK8312_SR_MASK 0x07 -#define STK8312_SR_400HZ_IDX 0 +#define STK8312_MODE_INT_AH_PP 0xC0/* active-high, push- pull */
drivers/iio/accel/stk8312.c +10
-5+#define STK8312_DREADY_BIT BIT(4) +#define STK8312_RNG_6G 1#define STK8312_RNG_SHIFT 6 -#define STK8312_READ_RETRIES 16 -#define STK8312_ALL_CHANNEL_MASK 7 +#define STK8312_RNG_MASK GENMASK(7, 6) +#define STK8312_SR_MASK GENMASK(2, 0) +#define STK8312_SR_400HZ_IDX 0 +#define STK8312_ALL_CHANNEL_MASK GENMASK(2, 0)#define STK8312_ALL_CHANNEL_SIZE 3#define STK8312_DRIVER_NAME "stk8312"@@ -144,7 +144,7 @@ static int stk8312_otp_init(struct stk8312_data *data)if (ret <0 ) goto exit_err; count--; - }while (!(ret &0x80 ) && count >0 ); + }while (!(ret &BIT (7 )) && count >0 );if (count ==0 ) { ret = -ETIMEDOUT ;@@ -560,11 +560,12 @@ static int stk8312_probe(struct i2c_client *client,return ret; } data->sample_rate_idx =STK8312_SR_400HZ_IDX ; - ret =stk8312_set_range (data,1 ); + ret =stk8312_set_range (data,STK8312_RNG_6G );if (ret <0 )return ret; - ret =stk8312_set_mode (data,STK8312_INT_MODE | STK8312_MODE_ACTIVE); + ret =stk8312_set_mode (data, +STK8312_MODE_INT_AH_PP |STK8312_MODE_ACTIVE );if (ret <0 )drivers/iio/accel/stk8312.c +1
-2return ret;@@ -685,7 +686,7 @@ MODULE_DEVICE_TABLE(acpi, stk8312_acpi_id);static struct i2c_driver stk8312_driver = { .driver = { - .name ="stk8312" , + .name =STK8312_DRIVER_NAME , .pm =STK8312_PM_OPS , .acpi_match_table =ACPI_PTR (stk8312_acpi_id), }, --Quoted 1 line
2.4.6
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 5/7 iio:accel:stk8312: use appropriate variable types
In reply to Hartmut KnaackAdapt some variable types to reduce unnecessary casting.
--- drivers/iio/accel/stk8312.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/iio/accel/stk8312.c +4 -4
@@ -69,8 +69,8 @@ static const int stk8312_scale_table[][2] = { };static const struct { -u16 val; -u32 val2; +int val; +int val2; } stk8312_samp_freq_table[] = { {400 ,0 }, {200 ,0 }, {100 ,0 }, {50 ,0 }, {25 ,0 }, {12 ,500000 }, {6 ,250000 }, {3 ,125000 }
@@ -103,7 +103,7 @@ static const struct iio_chan_spec stk8312_channels[] = {struct stk8312_data {struct i2c_client *client;struct mutex lock; -int range; +u8 range;u8 sample_rate_idx;u8 mode;struct iio_trigger *dready_trig;
@@ -242,7 +242,7 @@ static const struct iio_trigger_ops stk8312_trigger_ops = { .owner = THIS_MODULE, }; -static int stk8312_set_sample_rate (struct stk8312_data *data,int rate) +static int stk8312_set_sample_rate (struct stk8312_data *data,u8 rate) {int ret;u8 masked_reg;
Signature · 1 line
2.4.6
RE: [PATCH 5/7] iio:accel:stk8312: use appropriate variable types
Quoted 11 lines
-----Original Message-----
From: Hartmut Knaack [mailto:[email protected]]
Sent: Tuesday, July 28, 2015 1:49 AM
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Breana, Tiberiu A
Subject: [PATCH 5/7] iio:accel:stk8312: use appropriate variable typesAdapt some variable types to reduce unnecessary casting.
Signed-off-by Hartmut Knaack <[email protected]>
+1
Quoted 38 lines
--- drivers/iio/accel/stk8312.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)drivers/iio/accel/stk8312.c index renamed from drivers/iio/accel/stk8312.c +0
-0
8783f0bf7c53..e3624e2b3ec4100644drivers/iio/accel/stk8312.c +2
-2@@ -69,8 +69,8 @@ static const int stk8312_scale_table[][2] = { }; static const struct { - u16 val; - u32 val2; + int val; + int val2; } stk8312_samp_freq_table[] = { {400, 0}, {200, 0}, {100, 0}, {50, 0}, {25, 0}, {12, 500000}, {6, 250000}, {3, 125000} @@ -103,7 +103,7 @@ static const struct iio_chan_spec stk8312_channels[] = { struct stk8312_data {drivers/iio/accel/stk8312.c +2
-2struct i2c_client *client;struct mutex lock; -int range; +u8 range;u8 sample_rate_idx;u8 mode;struct iio_trigger *dready_trig;@@ -242,7 +242,7 @@ static const struct iio_trigger_ops stk8312_trigger_ops = { .owner =THIS_MODULE , }; -static int stk8312_set_sample_rate (struct stk8312_data *data,int rate) +static int stk8312_set_sample_rate (struct stk8312_data *data,u8 rate) {int ret;u8
masked_reg;
--
2.4.6
PATCH 6/7 iio:accel:stk8312: code style cleanup
In reply to Hartmut KnaackAdjust some indentation issues to make checkpatch.pl happy in strict mode.
--- drivers/iio/accel/stk8312.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/iio/accel/stk8312.c +2 -3
@@ -157,8 +157,7 @@ static int stk8312_otp_init(struct stk8312_data *data)if (ret <0 ) goto exit_err; - ret =i2c_smbus_write_byte_data (data->client, -STK8312_REG_AFECTRL , ret); + ret =i2c_smbus_write_byte_data (data->client,STK8312_REG_AFECTRL , ret);if (ret <0 ) goto exit_err;msleep (150 );
@@ -453,7 +452,7 @@ static irqreturn_t stk8312_trigger_handler(int irq, void *p) data->buffer[2 ] = buffer[2 ]; }else {for_each_set_bit (bit, indio_dev->active_scan_mask, - indio_dev->masklength) { + indio_dev->masklength) { ret =stk8312_read_accel (data, bit);if (ret <0 ) {mutex_unlock (&data->lock);
Signature · 1 line
2.4.6
RE: [PATCH 6/7] iio:accel:stk8312: code style cleanup
Quoted 11 lines
-----Original Message-----
From: Hartmut Knaack [mailto:[email protected]]
Sent: Tuesday, July 28, 2015 1:49 AM
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Breana, Tiberiu A
Subject: [PATCH 6/7] iio:accel:stk8312: code style cleanupAdjust some indentation issues to make checkpatch.pl happy in strict mode.
Signed-off-by Hartmut Knaack <[email protected]>
Comment inline.
Quoted 9 lines
--- drivers/iio/accel/stk8312.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)drivers/iio/accel/stk8312.c index renamed from drivers/iio/accel/stk8312.c +0
-0
e3624e2b3ec4..b059a6cabfbb100644
drivers/iio/accel/stk8312.c static int stk8312_otp_init(struct stk8312_data *data) · line 162
Quoted 8 lines
drivers/iio/accel/stk8312.c +2
-2@@ -157,8 +157,7 @@ static int stk8312_otp_init(struct stk8312_data *data)if (ret <0 ) goto exit_err; - ret =i2c_smbus_write_byte_data (data->client, -STK8312_REG_AFECTRL , ret); + ret =i2c_smbus_write_byte_data (data->client,STK8312_REG_AFECTRL , +ret);
On a single line, this write op would take up 81 chars. I think it's best to just add 2 tabs to the 2nd line.
Quoted 15 lines
drivers/iio/accel/stk8312.c +1
-1if (ret <0 ) goto exit_err;msleep (150 );@@ -453,7 +452,7 @@ static irqreturn_t stk8312_trigger_handler(int irq, void *p) data->buffer[2 ] = buffer[2 ]; }else {for_each_set_bit (bit, indio_dev->active_scan_mask, - indio_dev->masklength) { + indio_dev->masklength) { ret =stk8312_read_accel (data, bit);if (ret <0 ) {
mutex_unlock(&data->lock);
--
2.4.6
Re: [PATCH 6/7] iio:accel:stk8312: code style cleanup
Breana, Tiberiu A schrieb am 28.07.2015 um 15:45:
Quoted 26 lines
Quoted 11 lines
-----Original Message-----
From: Hartmut Knaack [mailto:[email protected]]
Sent: Tuesday, July 28, 2015 1:49 AM
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Breana, Tiberiu A
Subject: [PATCH 6/7] iio:accel:stk8312: code style cleanupAdjust some indentation issues to make checkpatch.pl happy in strict mode.
Signed-off-by Hartmut Knaack <[email protected]>Comment inline.
Quoted 12 lines
--- drivers/iio/accel/stk8312.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)drivers/iio/accel/stk8312.c index renamed from drivers/iio/accel/stk8312.c +0
-0
e3624e2b3ec4..b059a6cabfbb100644drivers/iio/accel/stk8312.c +0
-0@@ -157,8 +157,7 @@ static int stk8312_otp_init(struct stk8312_data *data)if (ret <0 ) goto exit_err;
Quoted 9 lines
drivers/iio/accel/stk8312.c static int stk8312_otp_init(struct stk8312_data *data) · line 162
Quoted 5 lines
drivers/iio/accel/stk8312.c +2
-2- ret =i2c_smbus_write_byte_data (data->client, -STK8312_REG_AFECTRL , ret); + ret =i2c_smbus_write_byte_data (data->client,STK8312_REG_AFECTRL , +ret);On a single line, this write op would take up 81 chars. I think it's best to just add 2 tabs to the 2nd line.
Don't worry, it is 80 chars sharp. I have double checked, even checkpatch.pl in strict mode doesn't complain on this one. Just on 2 other points, which I don't consider essential.
Thanks,
Hartmut
Quoted 21 lines
Quoted 12 lines
drivers/iio/accel/stk8312.c +1
-1if (ret <0 ) goto exit_err;msleep (150 );@@ -453,7 +452,7 @@ static irqreturn_t stk8312_trigger_handler(int irq, void *p) data->buffer[2 ] = buffer[2 ]; }else {for_each_set_bit (bit, indio_dev->active_scan_mask, - indio_dev->masklength) { + indio_dev->masklength) { ret =stk8312_read_accel (data, bit);if (ret <0 ) {Quoted 3 lines
mutex_unlock(&data->lock);
--
2.4.6--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 7/7 iio:accel:stk8312: drop local buffer
In reply to Hartmut KnaackDrop the local buffer in stk8312_trigger_handler() and use data-buffer instead for bulk reads.
--- drivers/iio/accel/stk8312.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
drivers/iio/accel/stk8312.c +1 -5
@@ -430,7 +430,6 @@ static irqreturn_t stk8312_trigger_handler(int irq, void *p)struct iio_dev *indio_dev = pf->indio_dev;struct stk8312_data *data =iio_priv (indio_dev);int bit, ret, i =0 ; -u8 buffer[STK8312_ALL_CHANNEL_SIZE ];mutex_lock (&data->lock);/*
@@ -441,15 +440,12 @@ static irqreturn_t stk8312_trigger_handler(int irq, void *p) ret =i2c_smbus_read_i2c_block_data (data->client,STK8312_REG_XOUT ,STK8312_ALL_CHANNEL_SIZE , - buffer); + data->buffer);if (ret <STK8312_ALL_CHANNEL_SIZE ) {dev_err (&data->client->dev,"register read failed\n" );mutex_unlock (&data->lock); goto err; } - data->buffer[0 ] = buffer[0 ]; - data->buffer[1 ] = buffer[1 ]; - data->buffer[2 ] = buffer[2 ]; }else {for_each_set_bit (bit, indio_dev->active_scan_mask, indio_dev->masklength) {
Signature · 1 line
2.4.6
RE: [PATCH 7/7] iio:accel:stk8312: drop local buffer
Quoted 9 lines
-----Original Message-----
From: [email protected] [mailto:linux-iio- [email protected]] On Behalf Of Hartmut Knaack
Sent: Tuesday, July 28, 2015 1:49 AM
To: [email protected]
Cc: Jonathan Cameron; Lars-Peter Clausen; Peter Meerwald; Breana, Tiberiu A
Subject: [PATCH 7/7] iio:accel:stk8312: drop local buffer
Quoted 4 lines
Drop the local buffer in
stk8312_trigger_handler()and use data-buffer instead for bulk reads.Signed-off-by Hartmut Knaack <[email protected]>
+1
Quoted 44 lines
--- drivers/iio/accel/stk8312.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)drivers/iio/accel/stk8312.c index renamed from drivers/iio/accel/stk8312.c +0
-0
b059a6cabfbb..a6b2bda7a63c100644drivers/iio/accel/stk8312.c +0
-1@@ -430,7 +430,6 @@ static irqreturn_t stk8312_trigger_handler(int irq, void *p)struct iio_dev *indio_dev = pf->indio_dev;struct stk8312_data *data =iio_priv (indio_dev);int bit, ret, i =0 ; -u8 buffer[STK8312_ALL_CHANNEL_SIZE ];mutex_lock (&data->lock);drivers/iio/accel/stk8312.c +1
-4/* @@ -441,15 +440,12 @@ static irqreturn_t stk8312_trigger_handler(int irq, void *p) ret =i2c_smbus_read_i2c_block_data (data->client,STK8312_REG_XOUT ,STK8312_ALL_CHANNEL_SIZE , - buffer); + data->buffer);if (ret <STK8312_ALL_CHANNEL_SIZE ) {dev_err (&data->client->dev,"register read failed\n");mutex_unlock (&data->lock); goto err; } - data->buffer[0 ] = buffer[0 ]; - data->buffer[1 ] = buffer[1 ]; - data->buffer[2 ] = buffer[2 ];drivers/iio/accel/stk8312.c +0
-1}else {for_each_set_bit (bit, indio_dev->active_scan_mask, indio_dev->masklength) { --2.4.6
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected] More majordomo info at
http://vger.kernel.org/majordomo-info.html