linux-kernel - Re: [PATCH 2/2] iio: pressure: adp810: Add driver for adp810 sensor

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: <20251025-5556-944955@bhairav-test.ee.iitb.ac.in>
Date: Sat, 25 Oct 2025 11:25:06 +0530
From: Akhilesh Patil <akhilesh@...iitb.ac.in>
To: Andy Shevchenko <andriy.shevchenko@...el.com>
Cc: Andy Shevchenko <andy.shevchenko@...il.com>, jic23@...nel.org,
	dlechner@...libre.com, robh@...nel.org, krzk+dt@...nel.org,
	conor+dt@...nel.org, nuno.sa@...log.com, andy@...nel.org,
	marcelo.schmitt1@...il.com, vassilisamir@...il.com,
	salah.triki@...il.com, skhan@...uxfoundation.org,
	linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org, akhileshpatilvnit@...il.com
Subject: Re: [PATCH 2/2] iio: pressure: adp810: Add driver for adp810 sensor

On Thu, Oct 23, 2025 at 09:58:45PM +0300, Andy Shevchenko wrote:
> On Mon, Oct 13, 2025 at 08:16:14PM +0530, Akhilesh Patil wrote:
> > On Sat, Oct 11, 2025 at 05:10:58PM +0300, Andy Shevchenko wrote:
> > > On Sat, Oct 11, 2025 at 3:25 PM Akhilesh Patil <akhilesh@...iitb.ac.in> wrote:
> 
> ...
> 
> > > > +struct adp810_read_buf {
> > > > +       u8 dp_msb;
> > > > +       u8 dp_lsb;
> > > > +       u8 dp_crc;
> > > > +       u8 tmp_msb;
> > > > +       u8 tmp_lsb;
> > > > +       u8 tmp_crc;
> > > > +       u8 sf_msb;
> > > > +       u8 sf_lsb;
> > > > +       u8 sf_crc;
> > > > +} __packed;
> > > 
> > > Why __packed?
> > 
> > Yes. This is the structure used as a buffer to store sensor values read.
> > Each entry in this structure should be contiguous in the memory because
> > reference of this structure will be passed to i2c_master_recv() to
> > receive and fill the data.
> > __packed will avoid any compiler generated paddings in the structure to
> > force alignments on certain architectures. We do not want these paddings
> > and want our struct members to be sequentially ordered as shown, with
> > no padding and size of the struct should also be 9 bytes as only 9 bytes of
> > data should be read from the sensor as per the specification.
> > 
> > I could have used array here. But I preferred strcture for better
> > readability of the code as one can easily see what values are expected
> > from sensor while reading and in which order.
> 
> Right, but in this form packed only affects the last member size (due to
> alignment), in any case since it's HW mandated requirement, perhaps add a
> comment. (Since we also going to use __be16 types, the __packed is required
> for that to be properly placed in the memory.)

Sure. Added comment explaning this in v4.

Regards,
Akhilesh

> 
> 
> ...
> 
> > > > +struct adp810_data {
> > > > +       struct i2c_client *client;
> > > > +       /* Use lock to synchronize access to device during read sequence */
> > > > +       struct mutex lock;
> > > > +};
> > > 
> > > Is there a deliberate choice to not use regmap I²C APIs?
> > 
> > Yes. I explored that possibility. However this sensor follows simple I2C
> > client protocol. It does not expose the concept of I2C registers. It
> > does not follow smbus. Specifically, while reading the measurement from
> > the sensor, we need to only send the device address with read bit on the bus,
> > and start reading 9 bytes following that. That is, no register address
> > should be sent. I am not sure if regmap API has some hack to achive
> > similar because these APIs expect register addresses to read/write which
> > this sensor does not follow. Hence using raw i2c functions. I also
> > thought regmap abstraction is not needed here as this sensor has very
> > limited commands to send and not many command/configurations.
> 
> Ah, makes sense.
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>