* [PATCH 0/5] iio:potentiostat:lmp91000: Adjust codestyle, and minor cleanup changes
@ 2019-02-18 17:22 Lucas Oshiro
2019-02-18 17:22 ` [PATCH 1/5] iio:potentiostat:lmp91000: remove unnecessary parentheses Lucas Oshiro
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: Lucas Oshiro @ 2019-02-18 17:22 UTC (permalink / raw)
To: jic23, knaack.h, lars, pmeerw; +Cc: linux-iio, linux-kernel, kernel-usp
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 added a missing '\n' on a dev_err message.
Lucas Oshiro (5):
iio:potentiostat:lmp91000: remove unnecessary parentheses
iio:potentiostat:lmp91000: insert braces around if arms
iio:potentiostat:lmp91000: reduce line width and remove blank line
iio:potentiostat:lmp91000: invert if statement
iio:potentiostat:lmp91000: add '\n' on dev_err
drivers/iio/potentiostat/lmp91000.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/5] iio:potentiostat:lmp91000: remove unnecessary parentheses
2019-02-18 17:22 [PATCH 0/5] iio:potentiostat:lmp91000: Adjust codestyle, and minor cleanup changes Lucas Oshiro
@ 2019-02-18 17:22 ` Lucas Oshiro
2019-02-18 17:22 ` [PATCH 2/5] iio:potentiostat:lmp91000: insert braces around if arms Lucas Oshiro
` (4 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Lucas Oshiro @ 2019-02-18 17:22 UTC (permalink / raw)
To: jic23, knaack.h, lars, pmeerw
Cc: linux-iio, linux-kernel, kernel-usp, Anderson Reis
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 <lucasseikioshiro@gmail.com>
Signed-off-by: Anderson Reis <andersonreisrosa@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/5] iio:potentiostat:lmp91000: insert braces around if arms
2019-02-18 17:22 [PATCH 0/5] iio:potentiostat:lmp91000: Adjust codestyle, and minor cleanup changes Lucas Oshiro
2019-02-18 17:22 ` [PATCH 1/5] iio:potentiostat:lmp91000: remove unnecessary parentheses Lucas Oshiro
@ 2019-02-18 17:22 ` Lucas Oshiro
2019-02-18 17:22 ` [PATCH 3/5] iio:potentiostat:lmp91000: reduce line width and remove blank line Lucas Oshiro
` (3 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Lucas Oshiro @ 2019-02-18 17:22 UTC (permalink / raw)
To: jic23, knaack.h, lars, pmeerw
Cc: linux-iio, linux-kernel, kernel-usp, Anderson Reis
Insert braces around all arms of if statement starting on line 214,
and solve these checkpath.el CHECKs:
- lmp91000.c:214: CHECK: braces {} should be used on all arms of this statement
- lmp91000.c:216: CHECK: Unbalanced braces around else statement
Signed-off-by: Lucas Oshiro <lucasseikioshiro@gmail.com>
Signed-off-by: Anderson Reis <andersonreisrosa@gmail.com>
---
drivers/iio/potentiostat/lmp91000.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/potentiostat/lmp91000.c b/drivers/iio/potentiostat/lmp91000.c
index 03d277621861..ca31be510940 100644
--- a/drivers/iio/potentiostat/lmp91000.c
+++ b/drivers/iio/potentiostat/lmp91000.c
@@ -211,9 +211,9 @@ 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"))
+ if (of_property_read_bool(np, "ti,external-tia-resistor")) {
val = 0;
- else {
+ } else {
dev_err(dev, "no ti,tia-gain-ohm defined");
return ret;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/5] iio:potentiostat:lmp91000: reduce line width and remove blank line
2019-02-18 17:22 [PATCH 0/5] iio:potentiostat:lmp91000: Adjust codestyle, and minor cleanup changes Lucas Oshiro
2019-02-18 17:22 ` [PATCH 1/5] iio:potentiostat:lmp91000: remove unnecessary parentheses Lucas Oshiro
2019-02-18 17:22 ` [PATCH 2/5] iio:potentiostat:lmp91000: insert braces around if arms Lucas Oshiro
@ 2019-02-18 17:22 ` Lucas Oshiro
2019-02-18 17:22 ` [PATCH 4/5] iio:potentiostat:lmp91000: invert if statement Lucas Oshiro
` (2 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Lucas Oshiro @ 2019-02-18 17:22 UTC (permalink / raw)
To: jic23, knaack.h, lars, pmeerw
Cc: linux-iio, linux-kernel, kernel-usp, Anderson Reis
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 <lucasseikioshiro@gmail.com>
Signed-off-by: Anderson Reis <andersonreisrosa@gmail.com>
---
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 ca31be510940..6dba26121a62 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/5] iio:potentiostat:lmp91000: invert if statement
2019-02-18 17:22 [PATCH 0/5] iio:potentiostat:lmp91000: Adjust codestyle, and minor cleanup changes Lucas Oshiro
` (2 preceding siblings ...)
2019-02-18 17:22 ` [PATCH 3/5] iio:potentiostat:lmp91000: reduce line width and remove blank line Lucas Oshiro
@ 2019-02-18 17:22 ` Lucas Oshiro
2019-02-20 9:52 ` Jonathan Cameron
2019-02-18 17:22 ` [PATCH 5/5] iio:potentiostat:lmp91000: add '\n' on dev_err Lucas Oshiro
2019-02-20 9:41 ` [PATCH 0/5] iio:potentiostat:lmp91000: Adjust codestyle, and minor cleanup changes Jonathan Cameron
5 siblings, 1 reply; 13+ messages in thread
From: Lucas Oshiro @ 2019-02-18 17:22 UTC (permalink / raw)
To: jic23, knaack.h, lars, pmeerw
Cc: linux-iio, linux-kernel, kernel-usp, Anderson Reis
Invert if statement arms in line 214, in order to make the code cleaner
Signed-off-by: Lucas Oshiro <lucasseikioshiro@gmail.com>
Signed-off-by: Anderson Reis <andersonreisrosa@gmail.com>
---
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 6dba26121a62..7229ef59590a 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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5/5] iio:potentiostat:lmp91000: add '\n' on dev_err
2019-02-18 17:22 [PATCH 0/5] iio:potentiostat:lmp91000: Adjust codestyle, and minor cleanup changes Lucas Oshiro
` (3 preceding siblings ...)
2019-02-18 17:22 ` [PATCH 4/5] iio:potentiostat:lmp91000: invert if statement Lucas Oshiro
@ 2019-02-18 17:22 ` Lucas Oshiro
2019-02-18 21:01 ` Joe Perches
2019-02-20 9:41 ` [PATCH 0/5] iio:potentiostat:lmp91000: Adjust codestyle, and minor cleanup changes Jonathan Cameron
5 siblings, 1 reply; 13+ messages in thread
From: Lucas Oshiro @ 2019-02-18 17:22 UTC (permalink / raw)
To: jic23, knaack.h, lars, pmeerw
Cc: linux-iio, linux-kernel, kernel-usp, Anderson Reis
Add missing '\n' at the end of dev_err message on line 215.
Signed-off-by: Lucas Oshiro <lucasseikioshiro@gmail.com>
Signed-off-by: Anderson Reis <andersonreisrosa@gmail.com>
---
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 7229ef59590a..aecdda757586 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\n");
return ret;
}
val = 0;
--
2.20.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 5/5] iio:potentiostat:lmp91000: add '\n' on dev_err
2019-02-18 17:22 ` [PATCH 5/5] iio:potentiostat:lmp91000: add '\n' on dev_err Lucas Oshiro
@ 2019-02-18 21:01 ` Joe Perches
2019-02-20 9:49 ` Jonathan Cameron
0 siblings, 1 reply; 13+ messages in thread
From: Joe Perches @ 2019-02-18 21:01 UTC (permalink / raw)
To: Lucas Oshiro, jic23, knaack.h, lars, pmeerw
Cc: linux-iio, linux-kernel, kernel-usp, Anderson Reis
On Mon, 2019-02-18 at 14:22 -0300, Lucas Oshiro wrote:
> Add missing '\n' at the end of dev_err message on line 215.
[]
> diff --git 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\n");
Perhaps a copy/paste error as the test is for
external-tia-resistor and not tia-gain-ohm
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/5] iio:potentiostat:lmp91000: Adjust codestyle, and minor cleanup changes
2019-02-18 17:22 [PATCH 0/5] iio:potentiostat:lmp91000: Adjust codestyle, and minor cleanup changes Lucas Oshiro
` (4 preceding siblings ...)
2019-02-18 17:22 ` [PATCH 5/5] iio:potentiostat:lmp91000: add '\n' on dev_err Lucas Oshiro
@ 2019-02-20 9:41 ` Jonathan Cameron
5 siblings, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2019-02-20 9:41 UTC (permalink / raw)
To: Lucas Oshiro
Cc: knaack.h, lars, pmeerw, linux-iio, linux-kernel, kernel-usp,
matt.ranostay
On Mon, 18 Feb 2019 14:22:31 -0300
Lucas Oshiro <lucasseikioshiro@gmail.com> wrote:
> 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 added a missing '\n' on a dev_err message.
Please try to identify the original author. Matt is still definitely
about, and he is obviously likely to be able to provide the most detailed
review of changes to this driver.
Thanks,
Jonathan
>
> Lucas Oshiro (5):
> iio:potentiostat:lmp91000: remove unnecessary parentheses
> iio:potentiostat:lmp91000: insert braces around if arms
> iio:potentiostat:lmp91000: reduce line width and remove blank line
> iio:potentiostat:lmp91000: invert if statement
> iio:potentiostat:lmp91000: add '\n' on dev_err
>
> drivers/iio/potentiostat/lmp91000.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5/5] iio:potentiostat:lmp91000: add '\n' on dev_err
2019-02-18 21:01 ` Joe Perches
@ 2019-02-20 9:49 ` Jonathan Cameron
2019-02-20 22:22 ` Joe Perches
0 siblings, 1 reply; 13+ messages in thread
From: Jonathan Cameron @ 2019-02-20 9:49 UTC (permalink / raw)
To: Joe Perches
Cc: Lucas Oshiro, knaack.h, lars, pmeerw, linux-iio, linux-kernel,
kernel-usp, Anderson Reis
On Mon, 18 Feb 2019 13:01:23 -0800
Joe Perches <joe@perches.com> wrote:
> On Mon, 2019-02-18 at 14:22 -0300, Lucas Oshiro wrote:
> > Add missing '\n' at the end of dev_err message on line 215.
> []
> > diff --git 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\n");
>
> Perhaps a copy/paste error as the test is for
> external-tia-resistor and not tia-gain-ohm
>
It is an odd construct, but I think this is correct. What it is actually
saying is that, given that we don't have an external resistor, we care
that the tia-gain-ohm isn't set (otherwise it wouldn't matter).
From the docs
- ti,external-tia-resistor: if the property ti,tia-gain-ohm is not defined this
needs to be set to signal that an external resistor value is being used.
So, it might be ideal to say that tia-gain-ohm is not defined and we do
not have an external resistor specified.
So not wrong, but could be more informative! So perhaps a follow up patch
to tidy that up would be good.
Jonathan
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/5] iio:potentiostat:lmp91000: invert if statement
2019-02-18 17:22 ` [PATCH 4/5] iio:potentiostat:lmp91000: invert if statement Lucas Oshiro
@ 2019-02-20 9:52 ` Jonathan Cameron
0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2019-02-20 9:52 UTC (permalink / raw)
To: Lucas Oshiro
Cc: knaack.h, lars, pmeerw, linux-iio, linux-kernel, kernel-usp,
Anderson Reis
On Mon, 18 Feb 2019 14:22:35 -0300
Lucas Oshiro <lucasseikioshiro@gmail.com> wrote:
> Invert if statement arms in line 214, in order to make the code cleaner
>
> Signed-off-by: Lucas Oshiro <lucasseikioshiro@gmail.com>
> Signed-off-by: Anderson Reis <andersonreisrosa@gmail.com>
Given this undoes one of the earlier changes, please merge them.
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 6dba26121a62..7229ef59590a 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;
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5/5] iio:potentiostat:lmp91000: add '\n' on dev_err
2019-02-20 9:49 ` Jonathan Cameron
@ 2019-02-20 22:22 ` Joe Perches
2019-02-26 20:15 ` Lucas Oshiro
0 siblings, 1 reply; 13+ messages in thread
From: Joe Perches @ 2019-02-20 22:22 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lucas Oshiro, knaack.h, lars, pmeerw, linux-iio, linux-kernel,
kernel-usp, Anderson Reis
On Wed, 2019-02-20 at 09:49 +0000, Jonathan Cameron wrote:
> On Mon, 18 Feb 2019 13:01:23 -0800 Joe Perches <joe@perches.com> wrote:
> > On Mon, 2019-02-18 at 14:22 -0300, Lucas Oshiro wrote:
> > > Add missing '\n' at the end of dev_err message on line 215.
> > []
> > > diff --git 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\n");
> >
> > Perhaps a copy/paste error as the test is for
> > external-tia-resistor and not tia-gain-ohm
> >
> It is an odd construct, but I think this is correct. What it is actually
> saying is that, given that we don't have an external resistor, we care
> that the tia-gain-ohm isn't set (otherwise it wouldn't matter).
>
> From the docs
> - ti,external-tia-resistor: if the property ti,tia-gain-ohm is not defined this
> needs to be set to signal that an external resistor value is being used.
>
> So, it might be ideal to say that tia-gain-ohm is not defined and we do
> not have an external resistor specified.
>
> So not wrong, but could be more informative! So perhaps a follow up patch
> to tidy that up would be good.
Then thanks in advance for doing that.
cheers, Joe
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5/5] iio:potentiostat:lmp91000: add '\n' on dev_err
2019-02-20 22:22 ` Joe Perches
@ 2019-02-26 20:15 ` Lucas Oshiro
2019-03-03 16:49 ` Jonathan Cameron
0 siblings, 1 reply; 13+ messages in thread
From: Lucas Oshiro @ 2019-02-26 20:15 UTC (permalink / raw)
To: Joe Perches, Jonathan Cameron
Cc: knaack.h, lars, pmeerw, linux-iio, linux-kernel, kernel-usp,
Anderson Reis
Thanks for the review!
On 20/02/2019 19:22, Joe Perches wrote:
> On Wed, 2019-02-20 at 09:49 +0000, Jonathan Cameron wrote:
>> On Mon, 18 Feb 2019 13:01:23 -0800 Joe Perches <joe@perches.com> wrote:
>>> On Mon, 2019-02-18 at 14:22 -0300, Lucas Oshiro wrote:
>>>> Add missing '\n' at the end of dev_err message on line 215.
>>> []
>>>> diff --git 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\n");
>>>
>>> Perhaps a copy/paste error as the test is for
>>> external-tia-resistor and not tia-gain-ohm
>>>
>> It is an odd construct, but I think this is correct. What it is actually
>> saying is that, given that we don't have an external resistor, we care
>> that the tia-gain-ohm isn't set (otherwise it wouldn't matter).
>>
>> From the docs
>> - ti,external-tia-resistor: if the property ti,tia-gain-ohm is not defined this
>> needs to be set to signal that an external resistor value is being used.
>>
>> So, it might be ideal to say that tia-gain-ohm is not defined and we do
>> not have an external resistor specified.
>>
>> So not wrong, but could be more informative! So perhaps a follow up patch
>> to tidy that up would be good.
So, this means that it's a good idea to change the dev_err message to something
like "no ti,tia-gain-ohm defined and external resistor not specified"?
>
> Then thanks in advance for doing that.
> cheers, Joe
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5/5] iio:potentiostat:lmp91000: add '\n' on dev_err
2019-02-26 20:15 ` Lucas Oshiro
@ 2019-03-03 16:49 ` Jonathan Cameron
0 siblings, 0 replies; 13+ messages in thread
From: Jonathan Cameron @ 2019-03-03 16:49 UTC (permalink / raw)
To: Lucas Oshiro
Cc: Joe Perches, knaack.h, lars, pmeerw, linux-iio, linux-kernel,
kernel-usp, Anderson Reis
On Tue, 26 Feb 2019 17:15:52 -0300
Lucas Oshiro <lucasseikioshiro@gmail.com> wrote:
> Thanks for the review!
>
> On 20/02/2019 19:22, Joe Perches wrote:
> > On Wed, 2019-02-20 at 09:49 +0000, Jonathan Cameron wrote:
> >> On Mon, 18 Feb 2019 13:01:23 -0800 Joe Perches <joe@perches.com> wrote:
> >>> On Mon, 2019-02-18 at 14:22 -0300, Lucas Oshiro wrote:
> >>>> Add missing '\n' at the end of dev_err message on line 215.
> >>> []
> >>>> diff --git 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\n");
> >>>
> >>> Perhaps a copy/paste error as the test is for
> >>> external-tia-resistor and not tia-gain-ohm
> >>>
> >> It is an odd construct, but I think this is correct. What it is actually
> >> saying is that, given that we don't have an external resistor, we care
> >> that the tia-gain-ohm isn't set (otherwise it wouldn't matter).
> >>
> >> From the docs
> >> - ti,external-tia-resistor: if the property ti,tia-gain-ohm is not defined this
> >> needs to be set to signal that an external resistor value is being used.
> >>
> >> So, it might be ideal to say that tia-gain-ohm is not defined and we do
> >> not have an external resistor specified.
> >>
> >> So not wrong, but could be more informative! So perhaps a follow up patch
> >> to tidy that up would be good.
>
> So, this means that it's a good idea to change the dev_err message to something
> like "no ti,tia-gain-ohm defined and external resistor not specified"?
Exactly. Thanks,
Jonathan
>
> >
> > Then thanks in advance for doing that.
> > cheers, Joe
> >
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-03-03 16:49 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-18 17:22 [PATCH 0/5] iio:potentiostat:lmp91000: Adjust codestyle, and minor cleanup changes Lucas Oshiro
2019-02-18 17:22 ` [PATCH 1/5] iio:potentiostat:lmp91000: remove unnecessary parentheses Lucas Oshiro
2019-02-18 17:22 ` [PATCH 2/5] iio:potentiostat:lmp91000: insert braces around if arms Lucas Oshiro
2019-02-18 17:22 ` [PATCH 3/5] iio:potentiostat:lmp91000: reduce line width and remove blank line Lucas Oshiro
2019-02-18 17:22 ` [PATCH 4/5] iio:potentiostat:lmp91000: invert if statement Lucas Oshiro
2019-02-20 9:52 ` Jonathan Cameron
2019-02-18 17:22 ` [PATCH 5/5] iio:potentiostat:lmp91000: add '\n' on dev_err Lucas Oshiro
2019-02-18 21:01 ` Joe Perches
2019-02-20 9:49 ` Jonathan Cameron
2019-02-20 22:22 ` Joe Perches
2019-02-26 20:15 ` Lucas Oshiro
2019-03-03 16:49 ` Jonathan Cameron
2019-02-20 9:41 ` [PATCH 0/5] iio:potentiostat:lmp91000: Adjust codestyle, and minor cleanup changes Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).