Re: [PATCH v6 1/2] iio:dac:ad7293: add support for AD7293

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 3 Dec 2021 10:22:04 +0800
Cai Huoqing <cai.huoqing@linux.dev> wrote:

> On 02 12月 21 17:08:18, Antoniu Miclaus wrote:
> > The AD7293 is a Power Amplifier drain current controller
> > containing functionality for general-purpose monitoring
> > and control of current, voltage, and temperature, integrated
> > into a single chip solution with an SPI-compatible interface.
> > 
> > Datasheet:
> > https://www.analog.com/media/en/technical-documentation/data-sheets/AD7293.pdf
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>

Hi Antoniu

I've made a bunch of white space tweaks whilst applying this.
A few things to keep in mind for future submissions.

1) Whilst the rule is 100 chars per line, 80 chars is preferred where it
   doesn't hurt readability.
2) Don't do this pattern.
	betty(x, y, z, fred +
	      wilma, barny)

If you have to break fred + wilma, then

	betty(x, y, z,
	      fred +
	      wilma,
	      barny);

is more readable in my view.


Otherwise endian issue in your debugfs functions.  Don't cast pointers like
that...

Please sanity check the changes I made and shout if I messed anything up.

Applied to the togreg branch of iio.git but pushed out as testing first to see
if 0-day finds anything we missed.




> > ---


> > +
> > +static int ad7293_reg_access(struct iio_dev *indio_dev,
> > +			     unsigned int reg,
> > +			     unsigned int write_val,
> > +			     unsigned int *read_val)
> > +{
> > +	struct ad7293_state *st = iio_priv(indio_dev);
> > +	int ret;
> > +
> > +	if (read_val)
> > +		ret = ad7293_spi_read(st, reg, (u16 *)read_val);

This only works on little endian platforms...
Think about where the data ends up on a big endian platform.

I've added a local variable to fix this.

> > +	else
> > +		ret = ad7293_spi_write(st, reg, (u16)write_val);


Jonathan