Messages in this thread | | | Subject | Re: [PATCH v2 1/3] iio: Add driver for Infineon DPS310 | From | Eddie James <> | Date | Wed, 15 May 2019 13:46:46 -0500 |
| |
On 5/11/19 4:22 AM, Jonathan Cameron wrote: > On Wed, 8 May 2019 14:35:26 -0500 > Eddie James <eajames@linux.ibm.com> wrote: > >> From: Joel Stanley <joel@jms.id.au> >> >> The DPS310 is a temperature and pressure sensor. It can be accessed over >> i2c and SPI. >> >> Signed-off-by: Eddie James <eajames@linux.ibm.com> > 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@jms.id.au>"); >> +MODULE_DESCRIPTION("Infineon DPS310 pressure and temperature sensor"); >> +MODULE_LICENSE("GPL v2");
| |