This patchset solves readability issues in AD7150 code: use of FIELD_GET
to fashion improvement, make operation more succint and remove useless
comments.
Changes in v2:
- Remove noisy patch that reorganized registers definitions
- Remove else to improve i2c return operation.
Melissa Wen (3):
staging: iio: ad7150: use FIELD_GET and GENMASK
staging: iio: ad7150: simplify i2c SMBus return treatment
staging: iio: ad7150: clean up of comments
drivers/staging/iio/cdc/ad7150.c | 26 +++++++++-----------------
1 file changed, 9 insertions(+), 17 deletions(-)
--
2.20.1
Use the bitfield macro FIELD_GET, and GENMASK to do the shift and mask in
one go. This makes the code more readable than explicit masking followed
by a shift.
Signed-off-by: Melissa Wen <[email protected]>
---
drivers/staging/iio/cdc/ad7150.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index 8234da4b8c65..091aa33589d7 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -5,6 +5,7 @@
* Copyright 2010-2011 Analog Devices Inc.
*/
+#include <linux/bitfield.h>
#include <linux/interrupt.h>
#include <linux/device.h>
#include <linux/kernel.h>
@@ -45,6 +46,9 @@
#define AD7150_SN0 22
#define AD7150_ID 23
+/* AD7150 masks */
+#define AD7150_THRESHTYPE_MSK GENMASK(6, 5)
+
/**
* struct ad7150_chip_info - instance specific chip data
* @client: i2c client for this device
@@ -137,7 +141,7 @@ static int ad7150_read_event_config(struct iio_dev *indio_dev,
if (ret < 0)
return ret;
- threshtype = (ret >> 5) & 0x03;
+ threshtype = FIELD_GET(AD7150_THRESHTYPE_MSK, ret);
adaptive = !!(ret & 0x80);
switch (type) {
--
2.20.1
General cleaning of comments to remove useless information or improve
description.
Signed-off-by: Melissa Wen <[email protected]>
---
drivers/staging/iio/cdc/ad7150.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index 7d56f10a19ed..51d6b52bce8b 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -163,7 +163,8 @@ static int ad7150_read_event_config(struct iio_dev *indio_dev,
return -EINVAL;
}
-/* lock should be held */
+/* state_lock should be held to ensure consistent state*/
+
static int ad7150_write_event_params(struct iio_dev *indio_dev,
unsigned int chan,
enum iio_event_type type,
@@ -479,10 +480,6 @@ static const struct iio_chan_spec ad7150_channels[] = {
AD7150_CAPACITANCE_CHAN(1)
};
-/*
- * threshold events
- */
-
static irqreturn_t ad7150_event_handler(int irq, void *private)
{
struct iio_dev *indio_dev = private;
@@ -571,10 +568,6 @@ static const struct iio_info ad7150_info = {
.write_event_value = &ad7150_write_event_value,
};
-/*
- * device probe and remove
- */
-
static int ad7150_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
--
2.20.1
Since i2c_smbus_write_byte_data returns no-positive value, this commit
making the treatment of its return value less verbose.
Signed-off-by: Melissa Wen <[email protected]>
---
drivers/staging/iio/cdc/ad7150.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
index 091aa33589d7..7d56f10a19ed 100644
--- a/drivers/staging/iio/cdc/ad7150.c
+++ b/drivers/staging/iio/cdc/ad7150.c
@@ -202,16 +202,11 @@ static int ad7150_write_event_params(struct iio_dev *indio_dev,
ret = i2c_smbus_write_byte_data(chip->client,
ad7150_addresses[chan][4],
sens);
- if (ret < 0)
+ if (ret)
return ret;
-
- ret = i2c_smbus_write_byte_data(chip->client,
+ return i2c_smbus_write_byte_data(chip->client,
ad7150_addresses[chan][5],
timeout);
- if (ret < 0)
- return ret;
-
- return 0;
}
static int ad7150_write_event_config(struct iio_dev *indio_dev,
--
2.20.1
On Fri, 14 Jun 2019 13:32:21 -0300
Melissa Wen <[email protected]> wrote:
> Use the bitfield macro FIELD_GET, and GENMASK to do the shift and mask in
> one go. This makes the code more readable than explicit masking followed
> by a shift.
>
> Signed-off-by: Melissa Wen <[email protected]>
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to paly with it.
Thanks,
Jonathan
> ---
> drivers/staging/iio/cdc/ad7150.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
> index 8234da4b8c65..091aa33589d7 100644
> --- a/drivers/staging/iio/cdc/ad7150.c
> +++ b/drivers/staging/iio/cdc/ad7150.c
> @@ -5,6 +5,7 @@
> * Copyright 2010-2011 Analog Devices Inc.
> */
>
> +#include <linux/bitfield.h>
> #include <linux/interrupt.h>
> #include <linux/device.h>
> #include <linux/kernel.h>
> @@ -45,6 +46,9 @@
> #define AD7150_SN0 22
> #define AD7150_ID 23
>
> +/* AD7150 masks */
> +#define AD7150_THRESHTYPE_MSK GENMASK(6, 5)
> +
> /**
> * struct ad7150_chip_info - instance specific chip data
> * @client: i2c client for this device
> @@ -137,7 +141,7 @@ static int ad7150_read_event_config(struct iio_dev *indio_dev,
> if (ret < 0)
> return ret;
>
> - threshtype = (ret >> 5) & 0x03;
> + threshtype = FIELD_GET(AD7150_THRESHTYPE_MSK, ret);
> adaptive = !!(ret & 0x80);
>
> switch (type) {
On Fri, 14 Jun 2019 13:32:54 -0300
Melissa Wen <[email protected]> wrote:
> Since i2c_smbus_write_byte_data returns no-positive value, this commit
> making the treatment of its return value less verbose.
>
> Signed-off-by: Melissa Wen <[email protected]>
Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.
Thanks,
Jonathan
> ---
> drivers/staging/iio/cdc/ad7150.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
> index 091aa33589d7..7d56f10a19ed 100644
> --- a/drivers/staging/iio/cdc/ad7150.c
> +++ b/drivers/staging/iio/cdc/ad7150.c
> @@ -202,16 +202,11 @@ static int ad7150_write_event_params(struct iio_dev *indio_dev,
> ret = i2c_smbus_write_byte_data(chip->client,
> ad7150_addresses[chan][4],
> sens);
> - if (ret < 0)
> + if (ret)
> return ret;
> -
> - ret = i2c_smbus_write_byte_data(chip->client,
> + return i2c_smbus_write_byte_data(chip->client,
> ad7150_addresses[chan][5],
> timeout);
> - if (ret < 0)
> - return ret;
> -
> - return 0;
> }
>
> static int ad7150_write_event_config(struct iio_dev *indio_dev,
On Fri, 14 Jun 2019 13:33:19 -0300
Melissa Wen <[email protected]> wrote:
> General cleaning of comments to remove useless information or improve
> description.
>
> Signed-off-by: Melissa Wen <[email protected]>
Applied,
Thanks,
Jonathan
> ---
> drivers/staging/iio/cdc/ad7150.c | 11 ++---------
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/iio/cdc/ad7150.c b/drivers/staging/iio/cdc/ad7150.c
> index 7d56f10a19ed..51d6b52bce8b 100644
> --- a/drivers/staging/iio/cdc/ad7150.c
> +++ b/drivers/staging/iio/cdc/ad7150.c
> @@ -163,7 +163,8 @@ static int ad7150_read_event_config(struct iio_dev *indio_dev,
> return -EINVAL;
> }
>
> -/* lock should be held */
> +/* state_lock should be held to ensure consistent state*/
> +
> static int ad7150_write_event_params(struct iio_dev *indio_dev,
> unsigned int chan,
> enum iio_event_type type,
> @@ -479,10 +480,6 @@ static const struct iio_chan_spec ad7150_channels[] = {
> AD7150_CAPACITANCE_CHAN(1)
> };
>
> -/*
> - * threshold events
> - */
> -
> static irqreturn_t ad7150_event_handler(int irq, void *private)
> {
> struct iio_dev *indio_dev = private;
> @@ -571,10 +568,6 @@ static const struct iio_info ad7150_info = {
> .write_event_value = &ad7150_write_event_value,
> };
>
> -/*
> - * device probe and remove
> - */
> -
> static int ad7150_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {