On 5/11/19 4:22 AM, Jonathan Cameron wrote:
On Wed,  8 May 2019 14:35:26 -0500
Eddie James <eaja...@linux.ibm.com> wrote:

From: Joel Stanley <j...@jms.id.au>

The DPS310 is a temperature and pressure sensor. It can be accessed over
i2c and SPI.

Signed-off-by: Eddie James <eaja...@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 
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 
are no corner cases.

Otherwise there is a race in the remove path that needs fixing.
Various minor bits and bobs inline.



  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



+       .id_table = dps310_id,
+MODULE_AUTHOR("Joel Stanley <j...@jms.id.au>");
+MODULE_DESCRIPTION("Infineon DPS310 pressure and temperature sensor");

Reply via email to