All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Lucas Oshiro <lucasseikioshiro@gmail.com>
Cc: Joe Perches <joe@perches.com>,
	knaack.h@gmx.de, lars@metafoo.de, pmeerw@pmeerw.net,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
	kernel-usp@googlegroups.com,
	Anderson Reis <andersonreisrosa@gmail.com>
Subject: Re: [PATCH 5/5] iio:potentiostat:lmp91000: add '\n' on dev_err
Date: Sun, 3 Mar 2019 16:49:18 +0000	[thread overview]
Message-ID: <20190303164918.0bb5bd72@archlinux> (raw)
In-Reply-To: <f21c5ea1-2775-91e4-4df6-3f15eec4fb81@gmail.com>

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
> >   


  reply	other threads:[~2019-03-03 16:49 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2019-02-20  9:41 ` [PATCH 0/5] iio:potentiostat:lmp91000: Adjust codestyle, and minor cleanup changes Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190303164918.0bb5bd72@archlinux \
    --to=jic23@kernel.org \
    --cc=andersonreisrosa@gmail.com \
    --cc=joe@perches.com \
    --cc=kernel-usp@googlegroups.com \
    --cc=knaack.h@gmx.de \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lucasseikioshiro@gmail.com \
    --cc=pmeerw@pmeerw.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.