2019-03-08 19:48:08

by Anderson Reis

[permalink] [raw]
Subject: [PATCH v2 0/4] iio:potentiostat:lmp91000: Adjust codestyle, and minor cleanup changes

Hi,

We solved some checkpath.el CHECKs and WARNINGs. We also inverted the arms of
an if statement, in order to make the code smaller as the else statement was
supressed. We also changed a dev_err message as requested


Anderson Reis (3):
iio:potentiostat:lmp91000: reduce line width and remove blank line
iio:potentiostat:lmp91000: invert if statement
iio:potentiostat:lmp91000: change dev_err message

Lucas Oshiro (1):
iio:potentiostat:lmp91000: remove unnecessary parentheses

drivers/iio/potentiostat/lmp91000.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)

--
2.20.1



2019-03-08 19:48:25

by Anderson Reis

[permalink] [raw]
Subject: [PATCH v2 1/4] iio:potentiostat:lmp91000: remove unnecessary parentheses

From: Lucas Oshiro <[email protected]>

Remove unnecessary parentheses on line 116, and solve these checkpatch.pl
CHECKs:

- lmp91000.c:116: CHECK: Unnecessary parentheses around 'state != channel'
- lmp91000.c:116: CHECK: Unnecessary parentheses around 'channel == LMP91000_REG_MODECN_TEMP'

Signed-off-by: Lucas Oshiro <[email protected]>
Signed-off-by: Anderson Reis <[email protected]>
---
drivers/iio/potentiostat/lmp91000.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c
index 90e895adf997..03d277621861 100644
--- a/drivers/iio/potentiostat/lmp91000.c
+++ b/drivers/iio/potentiostat/lmp91000.c
@@ -113,7 +113,7 @@ static int lmp91000_read(struct lmp91000_data *data, int channel, int *val)
return -EINVAL;

/* delay till first temperature reading is complete */
- if ((state != channel) && (channel == LMP91000_REG_MODECN_TEMP))
+ if (state != channel && channel == LMP91000_REG_MODECN_TEMP)
usleep_range(3000, 4000);

data->chan_select = channel != LMP91000_REG_MODECN_3LEAD;
--
2.20.1


2019-03-08 19:48:35

by Anderson Reis

[permalink] [raw]
Subject: [PATCH v2 2/4] iio:potentiostat:lmp91000: reduce line width and remove blank line

Break the line 258 in order fit the line width on 80 characters. Remove
the blank line 279, as the line before is also a blank line. Solve these
checkpath.el WARNING and CHECK:

- lmp91000.c:258: WARNING: line over 80 characters
- lmp91000.c:279: CHECK: Please don't use multiple blank lines

Signed-off-by: Lucas Oshiro <[email protected]>
Signed-off-by: Anderson Reis <[email protected]>
---
drivers/iio/potentiostat/lmp91000.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c
index 03d277621861..c45cfb632649 100644
--- a/drivers/iio/potentiostat/lmp91000.c
+++ b/drivers/iio/potentiostat/lmp91000.c
@@ -255,8 +255,8 @@ static int lmp91000_read_config(struct lmp91000_data *data)

regmap_write(data->regmap, LMP91000_REG_LOCK, 0);
regmap_write(data->regmap, LMP91000_REG_TIACN, reg);
- regmap_write(data->regmap, LMP91000_REG_REFCN, LMP91000_REG_REFCN_EXT_REF
- | LMP91000_REG_REFCN_50_ZERO);
+ regmap_write(data->regmap, LMP91000_REG_REFCN,
+ LMP91000_REG_REFCN_EXT_REF | LMP91000_REG_REFCN_50_ZERO);
regmap_write(data->regmap, LMP91000_REG_LOCK, 1);

return 0;
@@ -276,7 +276,6 @@ static int lmp91000_buffer_cb(const void *val, void *private)
static const struct iio_trigger_ops lmp91000_trigger_ops = {
};

-
static int lmp91000_buffer_preenable(struct iio_dev *indio_dev)
{
struct lmp91000_data *data = iio_priv(indio_dev);
--
2.20.1


2019-03-08 19:48:54

by Anderson Reis

[permalink] [raw]
Subject: [PATCH v2 3/4] iio:potentiostat:lmp91000: invert if statement

Invert if statement arms in line 214, in order to make the code cleaner,
solve these checkpatch.pl CHECKs:

- lmp9100.c:214: CHECK: braces {} should be used on all arms of this statement
- lmp9100.c:216: CHECK: Unbalanced braces around else statement

Signed-off-by: Lucas Oshiro <[email protected]>
Signed-off-by: Anderson Reis <[email protected]>
---
drivers/iio/potentiostat/lmp91000.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c
index c45cfb632649..1de17924e154 100644
--- a/drivers/iio/potentiostat/lmp91000.c
+++ b/drivers/iio/potentiostat/lmp91000.c
@@ -211,12 +211,11 @@ static int lmp91000_read_config(struct lmp91000_data *data)

ret = of_property_read_u32(np, "ti,tia-gain-ohm", &val);
if (ret) {
- if (of_property_read_bool(np, "ti,external-tia-resistor"))
- val = 0;
- else {
+ if (!of_property_read_bool(np, "ti,external-tia-resistor"))
dev_err(dev, "no ti,tia-gain-ohm defined");
return ret;
}
+ val = 0;
}

ret = -EINVAL;
--
2.20.1


2019-03-08 19:49:09

by Anderson Reis

[permalink] [raw]
Subject: [PATCH v2 4/4] iio:potentiostat:lmp91000: change dev_err message

Change dev_err message on line 215 in order to inform that
tia-gain-ohm is not defined and an external resistor is not
specified.

Signed-off-by: Anderson Reis <[email protected]>
Signed-off-by: Lucas Oshiro <[email protected]>
---
drivers/iio/potentiostat/lmp91000.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c
index 1de17924e154..5ed9752008a5 100644
--- a/drivers/iio/potentiostat/lmp91000.c
+++ b/drivers/iio/potentiostat/lmp91000.c
@@ -212,7 +212,7 @@ static int lmp91000_read_config(struct lmp91000_data *data)
ret = of_property_read_u32(np, "ti,tia-gain-ohm", &val);
if (ret) {
if (!of_property_read_bool(np, "ti,external-tia-resistor"))
- dev_err(dev, "no ti,tia-gain-ohm defined");
+ dev_err(dev, "no ti,tia-gain-ohm defined and external resistor not specified\n");
return ret;
}
val = 0;
--
2.20.1


2019-03-08 21:06:56

by Tomasz Duszynski

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] iio:potentiostat:lmp91000: remove unnecessary parentheses

On Fri, Mar 08, 2019 at 04:46:52PM -0300, Anderson Reis wrote:
> From: Lucas Oshiro <[email protected]>
>
> Remove unnecessary parentheses on line 116, and solve these checkpatch.pl

No need to explicitly point finger at changed lines. This is what
context lines are for.

> CHECKs:
>
> - lmp91000.c:116: CHECK: Unnecessary parentheses around 'state != channel'
> - lmp91000.c:116: CHECK: Unnecessary parentheses around 'channel == LMP91000_REG_MODECN_TEMP'
>
> Signed-off-by: Lucas Oshiro <[email protected]>
> Signed-off-by: Anderson Reis <[email protected]>
> ---
> drivers/iio/potentiostat/lmp91000.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c
> index 90e895adf997..03d277621861 100644
> --- a/drivers/iio/potentiostat/lmp91000.c
> +++ b/drivers/iio/potentiostat/lmp91000.c
> @@ -113,7 +113,7 @@ static int lmp91000_read(struct lmp91000_data *data, int channel, int *val)
> return -EINVAL;
>
> /* delay till first temperature reading is complete */
> - if ((state != channel) && (channel == LMP91000_REG_MODECN_TEMP))
> + if (state != channel && channel == LMP91000_REG_MODECN_TEMP)
> usleep_range(3000, 4000);
>
> data->chan_select = channel != LMP91000_REG_MODECN_3LEAD;
> --
> 2.20.1
>

2019-03-09 18:45:25

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] iio:potentiostat:lmp91000: remove unnecessary parentheses

On Fri, 8 Mar 2019 19:26:16 -0300
Anderson Reis <[email protected]> wrote:

> Hi, Tomasz
>
> thanks for the guidance.
Applied with the description tweaked.

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan

>
>
>
> On Fri, Mar 8, 2019, 18:04 Tomasz Duszynski <[email protected]> wrote:
>
> > On Fri, Mar 08, 2019 at 04:46:52PM -0300, Anderson Reis wrote:
> > > From: Lucas Oshiro <[email protected]>
> > >
> > > Remove unnecessary parentheses on line 116, and solve these
> > checkpatch.pl
> >
> > No need to explicitly point finger at changed lines. This is what
> > context lines are for.
> >
> > > CHECKs:
> > >
> > > - lmp91000.c:116: CHECK: Unnecessary parentheses around 'state !=
> > channel'
> > > - lmp91000.c:116: CHECK: Unnecessary parentheses around 'channel ==
> > LMP91000_REG_MODECN_TEMP'
> > >
> > > Signed-off-by: Lucas Oshiro <[email protected]>
> > > Signed-off-by: Anderson Reis <[email protected]>
> > > ---
> > > drivers/iio/potentiostat/lmp91000.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/iio/potentiostat/lmp91000.c
> > b/drivers/iio/potentiostat/lmp91000.c
> > > index 90e895adf997..03d277621861 100644
> > > --- a/drivers/iio/potentiostat/lmp91000.c
> > > +++ b/drivers/iio/potentiostat/lmp91000.c
> > > @@ -113,7 +113,7 @@ static int lmp91000_read(struct lmp91000_data *data,
> > int channel, int *val)
> > > return -EINVAL;
> > >
> > > /* delay till first temperature reading is complete */
> > > - if ((state != channel) && (channel == LMP91000_REG_MODECN_TEMP))
> > > + if (state != channel && channel == LMP91000_REG_MODECN_TEMP)
> > > usleep_range(3000, 4000);
> > >
> > > data->chan_select = channel != LMP91000_REG_MODECN_3LEAD;
> > > --
> > > 2.20.1
> > >
> >


2019-03-09 18:46:15

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 2/4] iio:potentiostat:lmp91000: reduce line width and remove blank line

On Fri, 8 Mar 2019 16:46:53 -0300
Anderson Reis <[email protected]> wrote:

> Break the line 258 in order fit the line width on 80 characters. Remove
> the blank line 279, as the line before is also a blank line. Solve these
> checkpath.el WARNING and CHECK:
>
> - lmp91000.c:258: WARNING: line over 80 characters
> - lmp91000.c:279: CHECK: Please don't use multiple blank lines
Again, not really needed, but doesn't matter much

Applied.

Thanks,

Jonathan

>
> Signed-off-by: Lucas Oshiro <[email protected]>
> Signed-off-by: Anderson Reis <[email protected]>
> ---
> drivers/iio/potentiostat/lmp91000.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c
> index 03d277621861..c45cfb632649 100644
> --- a/drivers/iio/potentiostat/lmp91000.c
> +++ b/drivers/iio/potentiostat/lmp91000.c
> @@ -255,8 +255,8 @@ static int lmp91000_read_config(struct lmp91000_data *data)
>
> regmap_write(data->regmap, LMP91000_REG_LOCK, 0);
> regmap_write(data->regmap, LMP91000_REG_TIACN, reg);
> - regmap_write(data->regmap, LMP91000_REG_REFCN, LMP91000_REG_REFCN_EXT_REF
> - | LMP91000_REG_REFCN_50_ZERO);
> + regmap_write(data->regmap, LMP91000_REG_REFCN,
> + LMP91000_REG_REFCN_EXT_REF | LMP91000_REG_REFCN_50_ZERO);
> regmap_write(data->regmap, LMP91000_REG_LOCK, 1);
>
> return 0;
> @@ -276,7 +276,6 @@ static int lmp91000_buffer_cb(const void *val, void *private)
> static const struct iio_trigger_ops lmp91000_trigger_ops = {
> };
>
> -
> static int lmp91000_buffer_preenable(struct iio_dev *indio_dev)
> {
> struct lmp91000_data *data = iio_priv(indio_dev);


2019-03-09 18:47:31

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] iio:potentiostat:lmp91000: invert if statement

On Fri, 8 Mar 2019 16:46:54 -0300
Anderson Reis <[email protected]> wrote:

> Invert if statement arms in line 214, in order to make the code cleaner,
> solve these checkpatch.pl CHECKs:
>
> - lmp9100.c:214: CHECK: braces {} should be used on all arms of this statement
> - lmp9100.c:216: CHECK: Unbalanced braces around else statement
>
> Signed-off-by: Lucas Oshiro <[email protected]>
> Signed-off-by: Anderson Reis <[email protected]>
Applied.

Thanks,

Jonathan

> ---
> drivers/iio/potentiostat/lmp91000.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c
> index c45cfb632649..1de17924e154 100644
> --- a/drivers/iio/potentiostat/lmp91000.c
> +++ b/drivers/iio/potentiostat/lmp91000.c
> @@ -211,12 +211,11 @@ static int lmp91000_read_config(struct lmp91000_data *data)
>
> ret = of_property_read_u32(np, "ti,tia-gain-ohm", &val);
> if (ret) {
> - if (of_property_read_bool(np, "ti,external-tia-resistor"))
> - val = 0;
> - else {
> + if (!of_property_read_bool(np, "ti,external-tia-resistor"))
> dev_err(dev, "no ti,tia-gain-ohm defined");
> return ret;
> }
> + val = 0;
> }
>
> ret = -EINVAL;


2019-03-09 18:50:27

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 4/4] iio:potentiostat:lmp91000: change dev_err message

On Fri, 8 Mar 2019 16:46:55 -0300
Anderson Reis <[email protected]> wrote:

> Change dev_err message on line 215 in order to inform that
> tia-gain-ohm is not defined and an external resistor is not
> specified.
>
> Signed-off-by: Anderson Reis <[email protected]>
> Signed-off-by: Lucas Oshiro <[email protected]>
Applied.

Thanks,

Jonathan

> ---
> drivers/iio/potentiostat/lmp91000.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c
> index 1de17924e154..5ed9752008a5 100644
> --- a/drivers/iio/potentiostat/lmp91000.c
> +++ b/drivers/iio/potentiostat/lmp91000.c
> @@ -212,7 +212,7 @@ static int lmp91000_read_config(struct lmp91000_data *data)
> ret = of_property_read_u32(np, "ti,tia-gain-ohm", &val);
> if (ret) {
> if (!of_property_read_bool(np, "ti,external-tia-resistor"))
> - dev_err(dev, "no ti,tia-gain-ohm defined");
> + dev_err(dev, "no ti,tia-gain-ohm defined and external resistor not specified\n");
> return ret;
> }
> val = 0;


2019-03-16 16:16:59

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v2 3/4] iio:potentiostat:lmp91000: invert if statement

On Sat, 9 Mar 2019 18:46:15 +0000
Jonathan Cameron <[email protected]> wrote:

> On Fri, 8 Mar 2019 16:46:54 -0300
> Anderson Reis <[email protected]> wrote:
>
> > Invert if statement arms in line 214, in order to make the code cleaner,
> > solve these checkpatch.pl CHECKs:
> >
> > - lmp9100.c:214: CHECK: braces {} should be used on all arms of this statement
> > - lmp9100.c:216: CHECK: Unbalanced braces around else statement
> >
> > Signed-off-by: Lucas Oshiro <[email protected]>
> > Signed-off-by: Anderson Reis <[email protected]>
> Applied.
>
> Thanks,
>
> Jonathan
>
> > ---
> > drivers/iio/potentiostat/lmp91000.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c
> > index c45cfb632649..1de17924e154 100644
> > --- a/drivers/iio/potentiostat/lmp91000.c
> > +++ b/drivers/iio/potentiostat/lmp91000.c
> > @@ -211,12 +211,11 @@ static int lmp91000_read_config(struct lmp91000_data *data)
> >
> > ret = of_property_read_u32(np, "ti,tia-gain-ohm", &val);
> > if (ret) {
> > - if (of_property_read_bool(np, "ti,external-tia-resistor"))
> > - val = 0;
> > - else {
> > + if (!of_property_read_bool(np, "ti,external-tia-resistor"))
Spoke too soon. You didn't build test this. Missing {

Fixed up.

Jonathan

> > dev_err(dev, "no ti,tia-gain-ohm defined");
> > return ret;
> > }
> > + val = 0;
> > }
> >
> > ret = -EINVAL;
>