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

Akhilesh Patil
2025-10-25T05:55:19Z

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@ee.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
>
>