linux-kernel - Re: [PATCH v2 2/6] iio: adc: ad4030: add driver for ad4030-24

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] [day] [month] [year] [list]

Message-Id: <D6YGYCYXJSOF.3BIXI0UPGNNZW@baylibre.com>
Date: Fri, 10 Jan 2025 15:39:29 +0100
From: "Esteban Blanc" <eblanc@...libre.com>
To: "Jonathan Cameron" <jic23@...nel.org>, "Marcelo Schmitt"
 <marcelo.schmitt1@...il.com>
Cc: "Lars-Peter Clausen" <lars@...afoo.de>, "Michael Hennerich"
 <Michael.Hennerich@...log.com>, Nuno Sá
 <nuno.sa@...log.com>, "Rob Herring" <robh@...nel.org>, "Krzysztof
 Kozlowski" <krzk+dt@...nel.org>, "Conor Dooley" <conor+dt@...nel.org>,
 "Jonathan Corbet" <corbet@....net>, <linux-iio@...r.kernel.org>,
 <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
 <linux-doc@...r.kernel.org>
Subject: Re: [PATCH v2 2/6] iio: adc: ad4030: add driver for ad4030-24

On Mon Dec 23, 2024 at 1:04 PM CET, Jonathan Cameron wrote:
>
> Just commenting on one particular bit feedback. Makes sure to address the
> rest!
>
> > > +
> > > +static int ad4030_get_chan_calibscale(struct iio_dev *indio_dev,
> > > +				      struct iio_chan_spec const *chan,
> > > +				      int *val,
> > > +				      int *val2)
> > > +{
> > > +	struct ad4030_state *st = iio_priv(indio_dev);
> > > +	u16 gain;
> > > +	int ret;
> > > +
> > > +	ret = regmap_bulk_read(st->regmap, AD4030_REG_GAIN_CHAN(chan->address),
> > > +			       st->rx_data.raw, AD4030_REG_GAIN_BYTES_NB);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	gain = get_unaligned_be16(st->rx_data.raw);  
> > My impression is that it is a bit odd to handle endianness after/before
> > calling regmap_read/write(). Can you try set
> > .val_format_endian_default = REGMAP_ENDIAN_BIG, in ad4030_regmap_bus?
> > If that doesn't help, what about doing the get/set_unaligned stuff within
> > the bus read/write calls?
>
> Unless these are all 16 bit registers (in which case push it into the
> regmap side of things), then a bulk read is not implying the registers
> read are one value. They could just be a load of registers next to each other.
> As such the regmap core won't touch them and endian conversion here, at the
> layer where we know they are related is the right thing to do.
>
> It's  not worth dual regmap stuff just because we have a few pairs of
> registers. 

In fact I've already set `.val_format_endian_default = REGMAP_ENDIAN_BIG`
but it has any effect as registers are 8bits long.

Most of the time registers are not related to each other. It's only for
offset and scale that multiple registers form one number

-- 
Esteban Blanc
BayLibre