Re: [PATCH v3 1/3] iio: adc: add support for Berlin

From: Antoine Tenart
Date: Tue May 05 2015 - 05:16:52 EST


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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/