Re: [PATCH v2 1/3] iio: Add driver for Infineon DPS310

[Eddie James]

On 5/11/19 4:22 AM, Jonathan Cameron wrote:
> On Wed,  8 May 2019 14:35:26 -0500
> Eddie James <eajames@xxxxxxxxxxxxx> wrote:
>
> > From: Joel Stanley <joel@xxxxxxxxx>
> >
> > The DPS310 is a temperature and pressure sensor. It can be accessed over
> > i2c and SPI.
> >
> > Signed-off-by: Eddie James <eajames@xxxxxxxxxxxxx>
> Hi Eddie,
>
> Ideally we'll get a sign off form Joel as well on this.
>
> A few comments inline.
>
> I 'think' this is probably fine without any locking to prevent simultaneous reads
> and /or writes to the registers because the few functions that do multiple reads
> and writes look fine.  Please do take another look at that though to confirm there
> are no corner cases.
>
> Otherwise there is a race in the remove path that needs fixing.
> Various minor bits and bobs inline.
>
> thanks,
>
> Jonathan
>
>
>
> > ---
> >   MAINTAINERS                   |   6 +
> >   drivers/iio/pressure/Kconfig  |  10 +
> >   drivers/iio/pressure/Makefile |   1 +
> >   drivers/iio/pressure/dps310.c | 429 ++++++++++++++++++++++++++++++++++++++++++
> >   4 files changed, 446 insertions(+)
> >   create mode 100644 drivers/iio/pressure/dps310.c

> > +};
> > +MODULE_DEVICE_TABLE(i2c, dps310_id);
> > +
> > +static const unsigned short normal_i2c[] = {
> > +	0x77, 0x76, I2C_CLIENT_END
> > +};
> > +
> > +static struct i2c_driver dps310_driver = {
> > +	.driver = {
> > +		.name = "dps310",
> > +	},
> > +	.probe = dps310_probe,
> > +	.remove = dps310_remove,
> > +	.address_list = normal_i2c,
> I'm fairly sure the address list is only used along with the detection
> infrastructure.  As such it doesn't actually provide any value unless
> you have a detect callback.  Please remove.
>
> I would like to see a DT and/or ACPI binding though as that is the
> means most people will use to find the device.


Somehow the device is already present in the witherspoon device tree 
where it's currently being used, so I don't have anything to add. 
arch/arm/boot/dts/aspeed-bmc-opp-witherspoon.dts

Thanks,

Eddie


> > +	.id_table = dps310_id,
> > +};
> > +module_i2c_driver(dps310_driver);
> > +
> > +MODULE_AUTHOR("Joel Stanley <joel@xxxxxxxxx>");
> > +MODULE_DESCRIPTION("Infineon DPS310 pressure and temperature sensor");
> > +MODULE_LICENSE("GPL v2");