linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: antoine.tenart@free-electrons.com (Antoine Tenart)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 1/3] iio: adc: add support for Berlin
Date: Tue, 5 May 2015 11:16:35 +0200	[thread overview]
Message-ID: <20150505091635.GF11312@kwain> (raw)
In-Reply-To: <55479C77.4070305@kernel.org>

Jonathan,

On Mon, May 04, 2015 at 05:21:11PM +0100, Jonathan Cameron wrote:
> On 04/05/15 10:06, Antoine Tenart wrote:

[...]

> Also, I'm travelling and without internet so can't look it up directly.
> So.. On the basis I'll forget to check later..  What is the status
> of the two series you say this is based upon?

These two series haven't been taken yet, but they should be merged in
the next merge window (at least that's what we aim). Sebastian may have
more information on this matter.

> 
> There are also some overly long lines where spitting them would not
> significantly hurt readability so clean those up as well to avoid
> the inevitable code style fix patch from someone with too much time on
> their hands!

I'll have a look.

> > diff --git a/drivers/iio/adc/berlin2-adc.c b/drivers/iio/adc/berlin2-adc.c
> > new file mode 100644
> > index 000000000000..0adfef035d79
> > --- /dev/null
> > +++ b/drivers/iio/adc/berlin2-adc.c

[...]

> > +
> > +struct berlin2_adc_priv {
> > +	struct regmap		*regmap;
> > +	struct mutex		lock;
> > +	wait_queue_head_t	wq;
> > +	int			irq;
> > +	int			tsen_irq;
> tsen_irq and irq are only used in one function I think. If so just use local
> variables rather than cluttering up this structure.

Sure.

[...]

> > +
> > +	regmap_update_bits(priv->regmap, BERLIN2_SM_CTRL,
> > +			BERLIN2_SM_CTRL_ADC_START, 0);
> > +
> > +	data = priv->data;
> > +	priv->data = -1;
> Is setting data = -1 not overkill with data_available there as well?

It is, we can remove this line.

[...]

> > +
> > +		temp = berlin2_adc_tsen_read(indio_dev);
> > +		if (temp < 0)
> > +			return temp;
> > +
> > +		if (temp > 2047)
> > +			temp = -(4096 - temp);
> > +
> > +		/* Convert to Celsius */
> Just to check... Celsius or milli degrees celsius?
> (which is the ABI units - matched with hwmon)

Celsuis, I'll update to return milli Celsius.

Thanks!

Antoine

-- 
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

  reply	other threads:[~2015-05-05  9:16 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-04  9:06 [PATCH v3 0/3] ARM: berlin: ADC support Antoine Tenart
2015-05-04  9:06 ` [PATCH v3 1/3] iio: adc: add support for Berlin Antoine Tenart
2015-05-04 16:21   ` Jonathan Cameron
2015-05-05  9:16     ` Antoine Tenart [this message]
2015-05-05 10:16       ` Sebastian Hesselbarth
2015-05-04  9:06 ` [PATCH v3 2/3] Documentation: bindings: document the Berlin ADC driver Antoine Tenart
2015-05-04  9:06 ` [PATCH v3 3/3] ARM: berlin: add an ADC node for the BG2Q Antoine Tenart

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=20150505091635.GF11312@kwain \
    --to=antoine.tenart@free-electrons.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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 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).