Re: [PATCH v1] iio: adc: intel_mrfld_adc: Add Basin Cove ADC driver

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Tue, Mar 19, 2019 at 08:02:15PM +0100, Tomasz Duszynski wrote:
> On Tue, Mar 19, 2019 at 12:37:28AM +0200, Andy Shevchenko wrote:
> > On Mon, Mar 18, 2019 at 09:52:42PM +0100, Tomasz Duszynski wrote:
> > > On Mon, Mar 18, 2019 at 12:51:03PM +0300, Andy Shevchenko wrote:

> > > > +#include <linux/iio/iio.h>
> > > > +#include <linux/iio/driver.h>
> > > > +#include <linux/iio/machine.h>

> > > Alphabetical order is preferred.
> >
> > Ah, you meant iio.h vs. driver.h?
> >
> 
> Yes.

It won't compile in alphabetical order.

> > > > +	u8 buf[2];
> > > > +	int ret;
> >
> > > > +	ret = regmap_bulk_read(regmap, chan->address, buf, 2);
> > > > +	if (ret)
> > > > +		goto done;
> > > > +
> > > > +	*result = (buf[0] << 8) | buf[1];
> > >
> > > be/le16_to_cpu() will do it for you.
> >
> > I think get_unaligned_le16() will be better here. Otherwise we need to define
> > __le16 variable and cast around it.
> >
> 
> I was thinking about be16_to_cpu(*(__be16 *)buf).
> Given this is local array and you do not do any pointer arithmetic
> before casting I would be surprised if that caused unaligned access.

Explicit casting to bitwise types seems weird to me, that's why get_unaligned looks better.
And no, there is no unaligned access per se.

-- 
With Best Regards,
Andy Shevchenko