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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20211204162252.66e62306@jic23-huawei>
Date:   Sat, 4 Dec 2021 16:22:52 +0000
From:   Jonathan Cameron <jic23@...nel.org>
To:     Cai Huoqing <cai.huoqing@...ux.dev>
Cc:     Antoniu Miclaus <antoniu.miclaus@...log.com>, robh+dt@...nel.org,
        linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 1/2] iio:dac:ad7293: add support for AD7293

On Fri, 3 Dec 2021 10:22:04 +0800
Cai Huoqing <cai.huoqing@...ux.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@...log.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