* [PATCH 0/3] Add support for DLH pressure sensors @ 2019-11-14 10:09 tomislav.denis 2019-11-14 10:09 ` [PATCH 1/3] iio: pressure: Add driver " tomislav.denis ` (3 more replies) 0 siblings, 4 replies; 16+ messages in thread From: tomislav.denis @ 2019-11-14 10:09 UTC (permalink / raw) To: jic23; +Cc: linux-iio, devicetree, tomislav.denis From: Tomislav Denis <tomislav.denis@avl.com> This patchset adds support for All Sensors DLH series low voltage digital pressure sensors. Datasheet: https://www.allsensors.com/cad/DS-0355_Rev_B.PDF Tomislav Denis (3): iio: pressure: Add driver for DLH pressure sensors dt-bindings: Add asc vendor bindings: iio: pressure: Add dlh-i2c documentation .../devicetree/bindings/iio/pressure/dlh-i2c.yaml | 43 +++ .../devicetree/bindings/vendor-prefixes.yaml | 2 + MAINTAINERS | 8 + drivers/iio/pressure/Kconfig | 12 + drivers/iio/pressure/Makefile | 1 + drivers/iio/pressure/dlh-i2c.c | 322 +++++++++++++++++++++ 6 files changed, 388 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/pressure/dlh-i2c.yaml create mode 100644 drivers/iio/pressure/dlh-i2c.c -- 2.7.4 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] iio: pressure: Add driver for DLH pressure sensors 2019-11-14 10:09 [PATCH 0/3] Add support for DLH pressure sensors tomislav.denis @ 2019-11-14 10:09 ` tomislav.denis 2019-11-14 14:35 ` Jonathan Cameron 2019-11-14 10:09 ` [PATCH 2/3] dt-bindings: Add asc vendor tomislav.denis ` (2 subsequent siblings) 3 siblings, 1 reply; 16+ messages in thread From: tomislav.denis @ 2019-11-14 10:09 UTC (permalink / raw) To: jic23; +Cc: linux-iio, devicetree, tomislav.denis From: Tomislav Denis <tomislav.denis@avl.com> All Sensors DLH is series of low voltage digital pressure sensors. Additionally to pressure value sensors deliver a temperature value. Sensors can be accessed over I2C and SPI, this driver supports only I2C access. Signed-off-by: Tomislav Denis <tomislav.denis@avl.com> --- MAINTAINERS | 7 + drivers/iio/pressure/Kconfig | 12 ++ drivers/iio/pressure/Makefile | 1 + drivers/iio/pressure/dlh-i2c.c | 322 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 342 insertions(+) create mode 100644 drivers/iio/pressure/dlh-i2c.c diff --git a/MAINTAINERS b/MAINTAINERS index 8323258..2a08923 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -668,6 +668,13 @@ S: Maintained F: Documentation/i2c/busses/i2c-ali1563.rst F: drivers/i2c/busses/i2c-ali1563.c +ALL SENSORS DLH SERIES PRESSURE SENSORS DRIVER +M: Tomislav Denis <tomislav.denis@avl.com> +W: http://www.allsensors.com/cad/DS-0355_Rev_B.PDF +S: Maintained +L: linux-iio@vger.kernel.org +F: drivers/iio/pressure/dlh-i2c.c + ALLEGRO DVT VIDEO IP CORE DRIVER M: Michael Tretter <m.tretter@pengutronix.de> R: Pengutronix Kernel Team <kernel@pengutronix.de> diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig index ba420e4..504de3e 100644 --- a/drivers/iio/pressure/Kconfig +++ b/drivers/iio/pressure/Kconfig @@ -53,6 +53,18 @@ config IIO_CROS_EC_BARO To compile this driver as a module, choose M here: the module will be called cros_ec_baro. +config DLH_I2C + tristate "All Sensors DLH series low voltage digital pressure sensors" + depends on I2C + select IIO_BUFFER + select IIO_TRIGGERED_BUFFER + help + Say yes here to build support for the All Sensors DLH series + pressure sensors driver. + + To compile this driver as a module, choose M here: the module + will be called dlh-i2c. + config DPS310 tristate "Infineon DPS310 pressure and temperature sensor" depends on I2C diff --git a/drivers/iio/pressure/Makefile b/drivers/iio/pressure/Makefile index d8f5ace..1851a36 100644 --- a/drivers/iio/pressure/Makefile +++ b/drivers/iio/pressure/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_BMP280) += bmp280.o bmp280-objs := bmp280-core.o bmp280-regmap.o obj-$(CONFIG_BMP280_I2C) += bmp280-i2c.o obj-$(CONFIG_BMP280_SPI) += bmp280-spi.o +obj-$(CONFIG_DLH_I2C) += dlh-i2c.o obj-$(CONFIG_DPS310) += dps310.o obj-$(CONFIG_IIO_CROS_EC_BARO) += cros_ec_baro.o obj-$(CONFIG_HID_SENSOR_PRESS) += hid-sensor-press.o diff --git a/drivers/iio/pressure/dlh-i2c.c b/drivers/iio/pressure/dlh-i2c.c new file mode 100644 index 0000000..4ef13c2 --- /dev/null +++ b/drivers/iio/pressure/dlh-i2c.c @@ -0,0 +1,322 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * All Sensors DLH series low voltage digital pressure sensors + * + * Copyright (c) 2019 AVL DiTEST GmbH + * Tomislav Denis <tomislav.denis@avl.com> + * + * Datasheet: http://www.allsensors.com/cad/DS-0355_Rev_B.PDF + */ + +#include <linux/module.h> +#include <linux/delay.h> +#include <linux/i2c.h> +#include <linux/iio/iio.h> +#include <linux/iio/buffer.h> +#include <linux/iio/trigger_consumer.h> +#include <linux/iio/triggered_buffer.h> + +/* Commands */ +#define DLH_START_SINGLE 0xAA + +/* Status bits */ +#define DLH_STATUS_OK 0x40 + +/* DLH data format */ +#define DLH_NUM_READ_BYTES 7 +#define DLH_NUM_DATA_BYTES 3 +#define DLH_NUM_PR_BITS 24 +#define DLH_NUM_TEMP_BITS 24 + +/* DLH timings */ +#define DLH_SINGLE_DUT_MS 5 + +enum dhl_ids { + dlhl60d, + dlhl60g, +}; + +struct dlh_info { + u8 osdig; /* digital offset factor */ + unsigned int fss; /* full scale span (inch H2O) */ +}; + +struct dlh_state { + struct i2c_client *client; + struct dlh_info info; + u8 rx_buf[DLH_NUM_READ_BYTES] ____cacheline_aligned; +}; + +static struct dlh_info dlh_info_tbl[] = { + [dlhl60d] = { + .osdig = 2, + .fss = 120, + }, + [dlhl60g] = { + .osdig = 10, + .fss = 60, + }, +}; + +static int dlh_i2c_read_direct(struct dlh_state *st, + unsigned int *pressure, unsigned int *temperature) +{ + int ret; + + ret = i2c_smbus_write_byte(st->client, DLH_START_SINGLE); + if (ret) { + dev_err(&st->client->dev, + "%s: I2C write byte failed\n", __func__); + return ret; + } + + mdelay(DLH_SINGLE_DUT_MS); + + ret = i2c_master_recv(st->client, st->rx_buf, DLH_NUM_READ_BYTES); + if (ret < 0) { + dev_err(&st->client->dev, + "%s: I2C read block failed\n", __func__); + return ret; + } + + if (st->rx_buf[0] != DLH_STATUS_OK) { + dev_err(&st->client->dev, + "%s: invalid status 0x%02x\n", __func__, st->rx_buf[0]); + return -EBUSY; + } + + *pressure = be32_to_cpup((u32 *)&st->rx_buf[1]) >> 8; + *temperature = be32_to_cpup((u32 *)&st->rx_buf[3]) & + GENMASK(DLH_NUM_TEMP_BITS - 1, 0); + + return 0; +} + +static int dlh_i2c_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *channel, int *value, + int *value2, long mask) +{ + struct dlh_state *st = iio_priv(indio_dev); + unsigned int pressure, temperature; + int ret; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + if (iio_buffer_enabled(indio_dev)) + return -EBUSY; + + ret = iio_device_claim_direct_mode(indio_dev); + if (ret) + return ret; + + ret = dlh_i2c_read_direct(st, &pressure, &temperature); + iio_device_release_direct_mode(indio_dev); + if (ret) + return ret; + + switch (channel->type) { + case IIO_PRESSURE: /* inch H2O */ + *value = pressure; + return IIO_VAL_INT; + + case IIO_TEMP: /* degrees Celsius */ + *value = temperature; + return IIO_VAL_INT; + + default: + return -EINVAL; + } + case IIO_CHAN_INFO_SCALE: + switch (channel->type) { + case IIO_PRESSURE: + *value = 125 * st->info.fss; + *value2 = 100 * (1 << DLH_NUM_PR_BITS); + return IIO_VAL_FRACTIONAL; + + case IIO_TEMP: + *value = 125; + *value2 = DLH_NUM_TEMP_BITS; + return IIO_VAL_FRACTIONAL_LOG2; + + default: + return -EINVAL; + } + case IIO_CHAN_INFO_OFFSET: + switch (channel->type) { + case IIO_PRESSURE: + *value = -125 * st->info.fss; + *value2 = 100 * st->info.osdig; + return IIO_VAL_FRACTIONAL; + + case IIO_TEMP: + *value = -40; + return IIO_VAL_INT; + + default: + return -EINVAL; + } + } + + return -EINVAL; +} + +static const struct iio_info dlh_i2c_info = { + .read_raw = dlh_i2c_read_raw, +}; + +static const struct iio_chan_spec dlh_i2c_channels[] = { + { + .type = IIO_PRESSURE, + .indexed = 1, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), + .info_mask_shared_by_type = + BIT(IIO_CHAN_INFO_SCALE) | + BIT(IIO_CHAN_INFO_OFFSET), + .scan_index = 0, + .scan_type = { + .sign = 'u', + .realbits = DLH_NUM_PR_BITS, + .storagebits = 32, + .shift = 8, + .endianness = IIO_BE, + }, + }, { + .type = IIO_TEMP, + .indexed = 1, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), + .info_mask_shared_by_type = + BIT(IIO_CHAN_INFO_SCALE) | + BIT(IIO_CHAN_INFO_OFFSET), + .scan_index = 1, + .scan_type = { + .sign = 'u', + .realbits = DLH_NUM_TEMP_BITS, + .storagebits = 32, + .shift = 8, + .endianness = IIO_BE, + }, + } +}; + +static irqreturn_t dlh_i2c_trigger_handler(int irq, void *private) +{ + struct iio_poll_func *pf = (struct iio_poll_func *)private; + struct iio_dev *indio_dev = pf->indio_dev; + struct dlh_state *st = iio_priv(indio_dev); + int ret; + unsigned int chn, i = 0; + __be32 tmp_buf[2]; + + ret = i2c_master_recv(st->client, st->rx_buf, DLH_NUM_READ_BYTES); + if (ret < 0) { + dev_err(&st->client->dev, + "%s: I2C read block failed\n", __func__); + goto out; + } + + if (st->rx_buf[0] != DLH_STATUS_OK) { + dev_err(&st->client->dev, + "%s: invalid status 0x%02x\n", __func__, st->rx_buf[0]); + goto out; + } + + ret = i2c_smbus_write_byte(st->client, DLH_START_SINGLE); + if (ret) { + dev_err(&st->client->dev, + "%s: I2C write byte failed\n", __func__); + goto out; + } + + for_each_set_bit(chn, indio_dev->active_scan_mask, + indio_dev->masklength) { + memcpy(tmp_buf + i, + &st->rx_buf[1] + chn * DLH_NUM_DATA_BYTES, + DLH_NUM_DATA_BYTES); + i++; + } + + iio_push_to_buffers(indio_dev, tmp_buf); + +out: + iio_trigger_notify_done(indio_dev->trig); + + return IRQ_HANDLED; +} + +static int dlh_i2c_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct dlh_state *st; + struct iio_dev *indio_dev; + int ret; + + if (!i2c_check_functionality(client->adapter, + I2C_FUNC_I2C | I2C_FUNC_SMBUS_WRITE_BYTE)) { + dev_err(&client->dev, + "adapter doesn't support required i2c functionality\n"); + return -EOPNOTSUPP; + } + + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*st)); + if (!indio_dev) { + dev_err(&client->dev, "failed to allocate iio device\n"); + return -ENOMEM; + } + + i2c_set_clientdata(client, indio_dev); + + st = iio_priv(indio_dev); + st->info = dlh_info_tbl[id->driver_data]; + st->client = client; + + indio_dev->name = id->name; + indio_dev->dev.parent = &client->dev; + indio_dev->dev.of_node = client->dev.of_node; + indio_dev->info = &dlh_i2c_info; + indio_dev->modes = INDIO_DIRECT_MODE; + indio_dev->channels = dlh_i2c_channels; + indio_dev->num_channels = ARRAY_SIZE(dlh_i2c_channels); + + ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev, + &iio_pollfunc_store_time, &dlh_i2c_trigger_handler, NULL); + if (ret) { + dev_err(&client->dev, "failed to setup iio buffer\n"); + return ret; + } + + ret = devm_iio_device_register(&client->dev, indio_dev); + if (ret) { + dev_err(&client->dev, "failed to register iio device\n"); + return ret; + } + + return 0; +} + +static const struct of_device_id dlh_i2c_of_match[] = { + { .compatible = "asc,dlhl60d" }, + { .compatible = "asc,dlhl60g" }, + {} +}; +MODULE_DEVICE_TABLE(of, dlh_i2c_of_match); + +static const struct i2c_device_id dlh_i2c_id[] = { + { "dlhl60d", dlhl60d }, + { "dlhl60g", dlhl60g }, + {} +}; +MODULE_DEVICE_TABLE(i2c, dlh_i2c_id); + +static struct i2c_driver dlh_i2c_driver = { + .driver = { + .name = "dlh_i2c", + .of_match_table = dlh_i2c_of_match, + }, + .probe = dlh_i2c_probe, + .id_table = dlh_i2c_id, +}; +module_i2c_driver(dlh_i2c_driver); + +MODULE_AUTHOR("Tomislav Denis <tomislav.denis@avl.com>"); +MODULE_DESCRIPTION("Driver for All Sensors DLH series pressure sensors"); +MODULE_LICENSE("GPL v2"); -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] iio: pressure: Add driver for DLH pressure sensors 2019-11-14 10:09 ` [PATCH 1/3] iio: pressure: Add driver " tomislav.denis @ 2019-11-14 14:35 ` Jonathan Cameron 2019-11-15 7:09 ` Denis, Tomislav AVL DiTEST 0 siblings, 1 reply; 16+ messages in thread From: Jonathan Cameron @ 2019-11-14 14:35 UTC (permalink / raw) To: tomislav.denis; +Cc: jic23, linux-iio, devicetree On Thu, 14 Nov 2019 11:09:06 +0100 <tomislav.denis@avl.com> wrote: > From: Tomislav Denis <tomislav.denis@avl.com> > > All Sensors DLH is series of low voltage digital pressure sensors. > Additionally to pressure value sensors deliver a temperature value. > Sensors can be accessed over I2C and SPI, this driver supports > only I2C access. > > Signed-off-by: Tomislav Denis <tomislav.denis@avl.com> Hi Tomislav, A few comments inline. Please check the units of the output against the IIO ABI docs. Some IIO ABI units are non obvious unfortunately! Thanks, Jonathan > --- > MAINTAINERS | 7 + > drivers/iio/pressure/Kconfig | 12 ++ > drivers/iio/pressure/Makefile | 1 + > drivers/iio/pressure/dlh-i2c.c | 322 +++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 342 insertions(+) > create mode 100644 drivers/iio/pressure/dlh-i2c.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 8323258..2a08923 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -668,6 +668,13 @@ S: Maintained > F: Documentation/i2c/busses/i2c-ali1563.rst > F: drivers/i2c/busses/i2c-ali1563.c > > +ALL SENSORS DLH SERIES PRESSURE SENSORS DRIVER > +M: Tomislav Denis <tomislav.denis@avl.com> > +W: http://www.allsensors.com/cad/DS-0355_Rev_B.PDF The specific path is likely to bit rot. So either drop the entry entirely or perhaps W: http://www.allsensors.com/ > +S: Maintained > +L: linux-iio@vger.kernel.org > +F: drivers/iio/pressure/dlh-i2c.c > + > ALLEGRO DVT VIDEO IP CORE DRIVER > M: Michael Tretter <m.tretter@pengutronix.de> > R: Pengutronix Kernel Team <kernel@pengutronix.de> > diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig > index ba420e4..504de3e 100644 > --- a/drivers/iio/pressure/Kconfig > +++ b/drivers/iio/pressure/Kconfig > @@ -53,6 +53,18 @@ config IIO_CROS_EC_BARO > To compile this driver as a module, choose M here: the module > will be called cros_ec_baro. > > +config DLH_I2C > + tristate "All Sensors DLH series low voltage digital pressure sensors" > + depends on I2C > + select IIO_BUFFER > + select IIO_TRIGGERED_BUFFER > + help > + Say yes here to build support for the All Sensors DLH series > + pressure sensors driver. > + > + To compile this driver as a module, choose M here: the module > + will be called dlh-i2c. > + > config DPS310 > tristate "Infineon DPS310 pressure and temperature sensor" > depends on I2C > diff --git a/drivers/iio/pressure/Makefile b/drivers/iio/pressure/Makefile > index d8f5ace..1851a36 100644 > --- a/drivers/iio/pressure/Makefile > +++ b/drivers/iio/pressure/Makefile > @@ -9,6 +9,7 @@ obj-$(CONFIG_BMP280) += bmp280.o > bmp280-objs := bmp280-core.o bmp280-regmap.o > obj-$(CONFIG_BMP280_I2C) += bmp280-i2c.o > obj-$(CONFIG_BMP280_SPI) += bmp280-spi.o > +obj-$(CONFIG_DLH_I2C) += dlh-i2c.o > obj-$(CONFIG_DPS310) += dps310.o > obj-$(CONFIG_IIO_CROS_EC_BARO) += cros_ec_baro.o > obj-$(CONFIG_HID_SENSOR_PRESS) += hid-sensor-press.o > diff --git a/drivers/iio/pressure/dlh-i2c.c b/drivers/iio/pressure/dlh-i2c.c > new file mode 100644 > index 0000000..4ef13c2 > --- /dev/null > +++ b/drivers/iio/pressure/dlh-i2c.c > @@ -0,0 +1,322 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * All Sensors DLH series low voltage digital pressure sensors > + * > + * Copyright (c) 2019 AVL DiTEST GmbH > + * Tomislav Denis <tomislav.denis@avl.com> > + * > + * Datasheet: http://www.allsensors.com/cad/DS-0355_Rev_B.PDF > + */ > + > +#include <linux/module.h> > +#include <linux/delay.h> > +#include <linux/i2c.h> > +#include <linux/iio/iio.h> > +#include <linux/iio/buffer.h> > +#include <linux/iio/trigger_consumer.h> > +#include <linux/iio/triggered_buffer.h> > + > +/* Commands */ > +#define DLH_START_SINGLE 0xAA > + > +/* Status bits */ > +#define DLH_STATUS_OK 0x40 > + > +/* DLH data format */ > +#define DLH_NUM_READ_BYTES 7 > +#define DLH_NUM_DATA_BYTES 3 > +#define DLH_NUM_PR_BITS 24 > +#define DLH_NUM_TEMP_BITS 24 > + > +/* DLH timings */ > +#define DLH_SINGLE_DUT_MS 5 > + > +enum dhl_ids { > + dlhl60d, > + dlhl60g, > +}; > + > +struct dlh_info { > + u8 osdig; /* digital offset factor */ > + unsigned int fss; /* full scale span (inch H2O) */ > +}; > + > +struct dlh_state { > + struct i2c_client *client; > + struct dlh_info info; > + u8 rx_buf[DLH_NUM_READ_BYTES] ____cacheline_aligned; > +}; > + > +static struct dlh_info dlh_info_tbl[] = { > + [dlhl60d] = { > + .osdig = 2, > + .fss = 120, > + }, > + [dlhl60g] = { > + .osdig = 10, > + .fss = 60, > + }, > +}; > + > +static int dlh_i2c_read_direct(struct dlh_state *st, > + unsigned int *pressure, unsigned int *temperature) > +{ > + int ret; > + > + ret = i2c_smbus_write_byte(st->client, DLH_START_SINGLE); > + if (ret) { > + dev_err(&st->client->dev, > + "%s: I2C write byte failed\n", __func__); > + return ret; > + } > + > + mdelay(DLH_SINGLE_DUT_MS); > + > + ret = i2c_master_recv(st->client, st->rx_buf, DLH_NUM_READ_BYTES); > + if (ret < 0) { > + dev_err(&st->client->dev, > + "%s: I2C read block failed\n", __func__); > + return ret; > + } > + > + if (st->rx_buf[0] != DLH_STATUS_OK) { > + dev_err(&st->client->dev, > + "%s: invalid status 0x%02x\n", __func__, st->rx_buf[0]); > + return -EBUSY; > + } > + > + *pressure = be32_to_cpup((u32 *)&st->rx_buf[1]) >> 8; > + *temperature = be32_to_cpup((u32 *)&st->rx_buf[3]) & > + GENMASK(DLH_NUM_TEMP_BITS - 1, 0); > + > + return 0; > +} > + > +static int dlh_i2c_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *channel, int *value, > + int *value2, long mask) > +{ > + struct dlh_state *st = iio_priv(indio_dev); > + unsigned int pressure, temperature; > + int ret; > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + if (iio_buffer_enabled(indio_dev)) > + return -EBUSY; > + > + ret = iio_device_claim_direct_mode(indio_dev); > + if (ret) > + return ret; > + > + ret = dlh_i2c_read_direct(st, &pressure, &temperature); > + iio_device_release_direct_mode(indio_dev); > + if (ret) > + return ret; > + > + switch (channel->type) { > + case IIO_PRESSURE: /* inch H2O */ I'm guessing the scale converts that to kilopascals as per Documentation/ABI/testing/sysfs-bus-iio? > + *value = pressure; > + return IIO_VAL_INT; > + > + case IIO_TEMP: /* degrees Celsius */ Base units in IIO for temperature are milli degress Celsius. > + *value = temperature; > + return IIO_VAL_INT; > + > + default: > + return -EINVAL; > + } > + case IIO_CHAN_INFO_SCALE: > + switch (channel->type) { > + case IIO_PRESSURE: > + *value = 125 * st->info.fss; > + *value2 = 100 * (1 << DLH_NUM_PR_BITS); > + return IIO_VAL_FRACTIONAL; > + > + case IIO_TEMP: > + *value = 125; > + *value2 = DLH_NUM_TEMP_BITS; > + return IIO_VAL_FRACTIONAL_LOG2; > + > + default: > + return -EINVAL; > + } > + case IIO_CHAN_INFO_OFFSET: > + switch (channel->type) { > + case IIO_PRESSURE: > + *value = -125 * st->info.fss; > + *value2 = 100 * st->info.osdig; > + return IIO_VAL_FRACTIONAL; > + > + case IIO_TEMP: > + *value = -40; > + return IIO_VAL_INT; > + > + default: > + return -EINVAL; > + } default: return -EINVAL; > + } > + Drop the following. One of the standard build bots complains about this and it is sensible to ensure it was deliberate that not all cases were handled in that switch statement. > + return -EINVAL; > +} > + > +static const struct iio_info dlh_i2c_info = { > + .read_raw = dlh_i2c_read_raw, > +}; > + > +static const struct iio_chan_spec dlh_i2c_channels[] = { > + { > + .type = IIO_PRESSURE, > + .indexed = 1, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > + .info_mask_shared_by_type = > + BIT(IIO_CHAN_INFO_SCALE) | > + BIT(IIO_CHAN_INFO_OFFSET), > + .scan_index = 0, > + .scan_type = { > + .sign = 'u', > + .realbits = DLH_NUM_PR_BITS, > + .storagebits = 32, > + .shift = 8, > + .endianness = IIO_BE, > + }, > + }, { > + .type = IIO_TEMP, > + .indexed = 1, If setting indexed, even though the default is 0, should probably provide the .channel = 0, to make that explicit. > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > + .info_mask_shared_by_type = > + BIT(IIO_CHAN_INFO_SCALE) | > + BIT(IIO_CHAN_INFO_OFFSET), > + .scan_index = 1, > + .scan_type = { > + .sign = 'u', > + .realbits = DLH_NUM_TEMP_BITS, > + .storagebits = 32, > + .shift = 8, > + .endianness = IIO_BE, > + }, > + } > +}; > + > +static irqreturn_t dlh_i2c_trigger_handler(int irq, void *private) > +{ > + struct iio_poll_func *pf = (struct iio_poll_func *)private; No need to add explicit cast for pointers of type void *. The c spec allows simply struct iio_poll_func *pf = private; > + struct iio_dev *indio_dev = pf->indio_dev; > + struct dlh_state *st = iio_priv(indio_dev); > + int ret; > + unsigned int chn, i = 0; > + __be32 tmp_buf[2]; > + > + ret = i2c_master_recv(st->client, st->rx_buf, DLH_NUM_READ_BYTES); > + if (ret < 0) { > + dev_err(&st->client->dev, > + "%s: I2C read block failed\n", __func__); > + goto out; > + } > + > + if (st->rx_buf[0] != DLH_STATUS_OK) { > + dev_err(&st->client->dev, > + "%s: invalid status 0x%02x\n", __func__, st->rx_buf[0]); > + goto out; > + } > + > + ret = i2c_smbus_write_byte(st->client, DLH_START_SINGLE); > + if (ret) { > + dev_err(&st->client->dev, > + "%s: I2C write byte failed\n", __func__); > + goto out; > + } If I understand the logic here correctly, you are triggering the next capture as part of the previous one. This doesn't sound right as we are using an external trigger. Imagine. Trigger1 - reads whatever is currently in buffer and starts next sample. Wait 5 minutes Trigger2 - reads whatever was captured just after trigger1 not fresh data as we might expect. In particular I suspect you get random stale data for the first read after starting the buffer. The flow should be. Trigger1. Send the DLH_START_SINGLE; poll or wait for interrupt to signal completion of this reading. Read data. Wait for 5 minutes Trigger 2. Send the DLH_START_SINGLE; poll or wait for interrupt to signal completion of this reading. Read data. That should guarantee fresh data whatever the spacing of triggers. If you want to basically get data as quick as possible, the loop trigger will ensure that you call the start asap after the prevous read. > + > + for_each_set_bit(chn, indio_dev->active_scan_mask, > + indio_dev->masklength) { > + memcpy(tmp_buf + i, > + &st->rx_buf[1] + chn * DLH_NUM_DATA_BYTES, > + DLH_NUM_DATA_BYTES); > + i++; > + } > + > + iio_push_to_buffers(indio_dev, tmp_buf); > + > +out: > + iio_trigger_notify_done(indio_dev->trig); > + > + return IRQ_HANDLED; > +} > + > +static int dlh_i2c_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct dlh_state *st; > + struct iio_dev *indio_dev; > + int ret; > + > + if (!i2c_check_functionality(client->adapter, > + I2C_FUNC_I2C | I2C_FUNC_SMBUS_WRITE_BYTE)) { > + dev_err(&client->dev, > + "adapter doesn't support required i2c functionality\n"); > + return -EOPNOTSUPP; > + } > + > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*st)); > + if (!indio_dev) { > + dev_err(&client->dev, "failed to allocate iio device\n"); > + return -ENOMEM; > + } > + > + i2c_set_clientdata(client, indio_dev); > + > + st = iio_priv(indio_dev); > + st->info = dlh_info_tbl[id->driver_data]; > + st->client = client; > + > + indio_dev->name = id->name; > + indio_dev->dev.parent = &client->dev; > + indio_dev->dev.of_node = client->dev.of_node; > + indio_dev->info = &dlh_i2c_info; > + indio_dev->modes = INDIO_DIRECT_MODE; > + indio_dev->channels = dlh_i2c_channels; > + indio_dev->num_channels = ARRAY_SIZE(dlh_i2c_channels); > + > + ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev, > + &iio_pollfunc_store_time, &dlh_i2c_trigger_handler, NULL); > + if (ret) { > + dev_err(&client->dev, "failed to setup iio buffer\n"); > + return ret; > + } > + > + ret = devm_iio_device_register(&client->dev, indio_dev); > + if (ret) { > + dev_err(&client->dev, "failed to register iio device\n"); > + return ret; Drop the return ret; out of the brackets as then we don't need the return 0. if (ret) dev_err(...) return ret; } > + } > + > + return 0; > +} > + > +static const struct of_device_id dlh_i2c_of_match[] = { > + { .compatible = "asc,dlhl60d" }, > + { .compatible = "asc,dlhl60g" }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, dlh_i2c_of_match); > + > +static const struct i2c_device_id dlh_i2c_id[] = { > + { "dlhl60d", dlhl60d }, > + { "dlhl60g", dlhl60g }, > + {} > +}; > +MODULE_DEVICE_TABLE(i2c, dlh_i2c_id); > + > +static struct i2c_driver dlh_i2c_driver = { > + .driver = { > + .name = "dlh_i2c", > + .of_match_table = dlh_i2c_of_match, > + }, > + .probe = dlh_i2c_probe, > + .id_table = dlh_i2c_id, > +}; > +module_i2c_driver(dlh_i2c_driver); > + > +MODULE_AUTHOR("Tomislav Denis <tomislav.denis@avl.com>"); > +MODULE_DESCRIPTION("Driver for All Sensors DLH series pressure sensors"); > +MODULE_LICENSE("GPL v2"); ^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH 1/3] iio: pressure: Add driver for DLH pressure sensors 2019-11-14 14:35 ` Jonathan Cameron @ 2019-11-15 7:09 ` Denis, Tomislav AVL DiTEST 0 siblings, 0 replies; 16+ messages in thread From: Denis, Tomislav AVL DiTEST @ 2019-11-15 7:09 UTC (permalink / raw) To: Jonathan Cameron; +Cc: jic23, linux-iio, devicetree, Denis, Tomislav AVL DiTEST Hi Jonathan, Thanks for the fast review. V2 patches coming soon. Tomislav > On Thu, 14 Nov 2019 11:09:06 +0100 > <tomislav.denis@avl.com> wrote: > > > From: Tomislav Denis <tomislav.denis@avl.com> > > > > All Sensors DLH is series of low voltage digital pressure sensors. > > Additionally to pressure value sensors deliver a temperature value. > > Sensors can be accessed over I2C and SPI, this driver supports only > > I2C access. > > > > Signed-off-by: Tomislav Denis <tomislav.denis@avl.com> > Hi Tomislav, > > A few comments inline. Please check the units of the output against the IIO > ABI docs. Some IIO ABI units are non obvious unfortunately! > > Thanks, > > Jonathan > > > --- > > MAINTAINERS | 7 + > > drivers/iio/pressure/Kconfig | 12 ++ > > drivers/iio/pressure/Makefile | 1 + > > drivers/iio/pressure/dlh-i2c.c | 322 > > +++++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 342 insertions(+) > > create mode 100644 drivers/iio/pressure/dlh-i2c.c > > > > diff --git a/MAINTAINERS b/MAINTAINERS index 8323258..2a08923 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -668,6 +668,13 @@ S: Maintained > > F: Documentation/i2c/busses/i2c-ali1563.rst > > F: drivers/i2c/busses/i2c-ali1563.c > > > > +ALL SENSORS DLH SERIES PRESSURE SENSORS DRIVER > > +M: Tomislav Denis <tomislav.denis@avl.com> > > +W: http://www.allsensors.com/cad/DS-0355_Rev_B.PDF > The specific path is likely to bit rot. > So either drop the entry entirely or perhaps > W http://www.allsensors.com/cad/DS-0355_Rev_B.PDF > > +S: Maintained > > +L: linux-iio@vger.kernel.org > > +F: drivers/iio/pressure/dlh-i2c.c > > + > > ALLEGRO DVT VIDEO IP CORE DRIVER > > M: Michael Tretter <m.tretter@pengutronix.de> > > R: Pengutronix Kernel Team <kernel@pengutronix.de> > > diff --git a/drivers/iio/pressure/Kconfig > > b/drivers/iio/pressure/Kconfig index ba420e4..504de3e 100644 > > --- a/drivers/iio/pressure/Kconfig > > +++ b/drivers/iio/pressure/Kconfig > > @@ -53,6 +53,18 @@ config IIO_CROS_EC_BARO > > To compile this driver as a module, choose M here: the module > > will be called cros_ec_baro. > > > > +config DLH_I2C > > + tristate "All Sensors DLH series low voltage digital pressure sensors" > > + depends on I2C > > + select IIO_BUFFER > > + select IIO_TRIGGERED_BUFFER > > + help > > + Say yes here to build support for the All Sensors DLH series > > + pressure sensors driver. > > + > > + To compile this driver as a module, choose M here: the module > > + will be called dlh-i2c. > > + > > config DPS310 > > tristate "Infineon DPS310 pressure and temperature sensor" > > depends on I2C > > diff --git a/drivers/iio/pressure/Makefile > > b/drivers/iio/pressure/Makefile index d8f5ace..1851a36 100644 > > --- a/drivers/iio/pressure/Makefile > > +++ b/drivers/iio/pressure/Makefile > > @@ -9,6 +9,7 @@ obj-$(CONFIG_BMP280) += bmp280.o bmp280-objs := > > bmp280-core.o bmp280-regmap.o > > obj-$(CONFIG_BMP280_I2C) += bmp280-i2c.o > > obj-$(CONFIG_BMP280_SPI) += bmp280-spi.o > > +obj-$(CONFIG_DLH_I2C) += dlh-i2c.o > > obj-$(CONFIG_DPS310) += dps310.o > > obj-$(CONFIG_IIO_CROS_EC_BARO) += cros_ec_baro.o > > obj-$(CONFIG_HID_SENSOR_PRESS) += hid-sensor-press.o > > diff --git a/drivers/iio/pressure/dlh-i2c.c > > b/drivers/iio/pressure/dlh-i2c.c new file mode 100644 index > > 0000000..4ef13c2 > > --- /dev/null > > +++ b/drivers/iio/pressure/dlh-i2c.c > > @@ -0,0 +1,322 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * All Sensors DLH series low voltage digital pressure sensors > > + * > > + * Copyright (c) 2019 AVL DiTEST GmbH > > + * Tomislav Denis <tomislav.denis@avl.com> > > + * > > + * Datasheet: http://www.allsensors.com/cad/DS-0355_Rev_B.PDF > > + */ > > + > > +#include <linux/module.h> > > +#include <linux/delay.h> > > +#include <linux/i2c.h> > > +#include <linux/iio/iio.h> > > +#include <linux/iio/buffer.h> > > +#include <linux/iio/trigger_consumer.h> #include > > +<linux/iio/triggered_buffer.h> > > + > > +/* Commands */ > > +#define DLH_START_SINGLE 0xAA > > + > > +/* Status bits */ > > +#define DLH_STATUS_OK 0x40 > > + > > +/* DLH data format */ > > +#define DLH_NUM_READ_BYTES 7 > > +#define DLH_NUM_DATA_BYTES 3 > > +#define DLH_NUM_PR_BITS 24 > > +#define DLH_NUM_TEMP_BITS 24 > > + > > +/* DLH timings */ > > +#define DLH_SINGLE_DUT_MS 5 > > + > > +enum dhl_ids { > > + dlhl60d, > > + dlhl60g, > > +}; > > + > > +struct dlh_info { > > + u8 osdig; /* digital offset factor */ > > + unsigned int fss; /* full scale span (inch H2O) */ > > +}; > > + > > +struct dlh_state { > > + struct i2c_client *client; > > + struct dlh_info info; > > + u8 rx_buf[DLH_NUM_READ_BYTES] ____cacheline_aligned; }; > > + > > +static struct dlh_info dlh_info_tbl[] = { > > + [dlhl60d] = { > > + .osdig = 2, > > + .fss = 120, > > + }, > > + [dlhl60g] = { > > + .osdig = 10, > > + .fss = 60, > > + }, > > +}; > > + > > +static int dlh_i2c_read_direct(struct dlh_state *st, > > + unsigned int *pressure, unsigned int *temperature) { > > + int ret; > > + > > + ret = i2c_smbus_write_byte(st->client, DLH_START_SINGLE); > > + if (ret) { > > + dev_err(&st->client->dev, > > + "%s: I2C write byte failed\n", __func__); > > + return ret; > > + } > > + > > + mdelay(DLH_SINGLE_DUT_MS); > > + > > + ret = i2c_master_recv(st->client, st->rx_buf, > DLH_NUM_READ_BYTES); > > + if (ret < 0) { > > + dev_err(&st->client->dev, > > + "%s: I2C read block failed\n", __func__); > > + return ret; > > + } > > + > > + if (st->rx_buf[0] != DLH_STATUS_OK) { > > + dev_err(&st->client->dev, > > + "%s: invalid status 0x%02x\n", __func__, st- > >rx_buf[0]); > > + return -EBUSY; > > + } > > + > > + *pressure = be32_to_cpup((u32 *)&st->rx_buf[1]) >> 8; > > + *temperature = be32_to_cpup((u32 *)&st->rx_buf[3]) & > > + GENMASK(DLH_NUM_TEMP_BITS - 1, 0); > > + > > + return 0; > > +} > > + > > +static int dlh_i2c_read_raw(struct iio_dev *indio_dev, > > + struct iio_chan_spec const *channel, int *value, > > + int *value2, long mask) > > +{ > > + struct dlh_state *st = iio_priv(indio_dev); > > + unsigned int pressure, temperature; > > + int ret; > > + > > + switch (mask) { > > + case IIO_CHAN_INFO_RAW: > > + if (iio_buffer_enabled(indio_dev)) > > + return -EBUSY; > > + > > + ret = iio_device_claim_direct_mode(indio_dev); > > + if (ret) > > + return ret; > > + > > + ret = dlh_i2c_read_direct(st, &pressure, &temperature); > > + iio_device_release_direct_mode(indio_dev); > > + if (ret) > > + return ret; > > + > > + switch (channel->type) { > > + case IIO_PRESSURE: /* inch H2O */ > > I'm guessing the scale converts that to kilopascals as per > Documentation/ABI/testing/sysfs-bus-iio? > > > + *value = pressure; > > + return IIO_VAL_INT; > > + > > + case IIO_TEMP: /* degrees Celsius */ > > Base units in IIO for temperature are milli degress Celsius. > > > + *value = temperature; > > + return IIO_VAL_INT; > > + > > + default: > > + return -EINVAL; > > + } > > + case IIO_CHAN_INFO_SCALE: > > + switch (channel->type) { > > + case IIO_PRESSURE: > > + *value = 125 * st->info.fss; > > + *value2 = 100 * (1 << DLH_NUM_PR_BITS); > > + return IIO_VAL_FRACTIONAL; > > + > > + case IIO_TEMP: > > + *value = 125; > > + *value2 = DLH_NUM_TEMP_BITS; > > + return IIO_VAL_FRACTIONAL_LOG2; > > + > > + default: > > + return -EINVAL; > > + } > > + case IIO_CHAN_INFO_OFFSET: > > + switch (channel->type) { > > + case IIO_PRESSURE: > > + *value = -125 * st->info.fss; > > + *value2 = 100 * st->info.osdig; > > + return IIO_VAL_FRACTIONAL; > > + > > + case IIO_TEMP: > > + *value = -40; > > + return IIO_VAL_INT; > > + > > + default: > > + return -EINVAL; > > + } > default: > return -EINVAL; > > > + } > > + > Drop the following. One of the standard build bots complains about this and > it is sensible to ensure it was deliberate that not all cases were handled in > that switch statement. > > + return -EINVAL; > > +} > > + > > +static const struct iio_info dlh_i2c_info = { > > + .read_raw = dlh_i2c_read_raw, > > +}; > > + > > +static const struct iio_chan_spec dlh_i2c_channels[] = { > > + { > > + .type = IIO_PRESSURE, > > + .indexed = 1, > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > > + .info_mask_shared_by_type = > > + BIT(IIO_CHAN_INFO_SCALE) | > > + BIT(IIO_CHAN_INFO_OFFSET), > > + .scan_index = 0, > > + .scan_type = { > > + .sign = 'u', > > + .realbits = DLH_NUM_PR_BITS, > > + .storagebits = 32, > > + .shift = 8, > > + .endianness = IIO_BE, > > + }, > > + }, { > > + .type = IIO_TEMP, > > + .indexed = 1, > > If setting indexed, even though the default is 0, should probably provide the > .channel = 0, to make that explicit. > > > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > > + .info_mask_shared_by_type = > > + BIT(IIO_CHAN_INFO_SCALE) | > > + BIT(IIO_CHAN_INFO_OFFSET), > > + .scan_index = 1, > > + .scan_type = { > > + .sign = 'u', > > + .realbits = DLH_NUM_TEMP_BITS, > > + .storagebits = 32, > > + .shift = 8, > > + .endianness = IIO_BE, > > + }, > > + } > > +}; > > + > > +static irqreturn_t dlh_i2c_trigger_handler(int irq, void *private) { > > + struct iio_poll_func *pf = (struct iio_poll_func *)private; > No need to add explicit cast for pointers of type void *. The c spec allows > simply > > struct iio_poll_func *pf = private; > > > + struct iio_dev *indio_dev = pf->indio_dev; > > + struct dlh_state *st = iio_priv(indio_dev); > > + int ret; > > + unsigned int chn, i = 0; > > + __be32 tmp_buf[2]; > > + > > + ret = i2c_master_recv(st->client, st->rx_buf, > DLH_NUM_READ_BYTES); > > + if (ret < 0) { > > + dev_err(&st->client->dev, > > + "%s: I2C read block failed\n", __func__); > > + goto out; > > + } > > + > > + if (st->rx_buf[0] != DLH_STATUS_OK) { > > + dev_err(&st->client->dev, > > + "%s: invalid status 0x%02x\n", __func__, st- > >rx_buf[0]); > > + goto out; > > + } > > + > > + ret = i2c_smbus_write_byte(st->client, DLH_START_SINGLE); > > + if (ret) { > > + dev_err(&st->client->dev, > > + "%s: I2C write byte failed\n", __func__); > > + goto out; > > + } > > If I understand the logic here correctly, you are triggering the next capture as > part of the previous one. This doesn't sound right as we are using an > external trigger. Imagine. > > Trigger1 - reads whatever is currently in buffer and starts next sample. > Wait 5 minutes > Trigger2 - reads whatever was captured just after trigger1 not fresh data as > we might expect. > > In particular I suspect you get random stale data for the first read after > starting the buffer. > > The flow should be. > > Trigger1. Send the DLH_START_SINGLE; > poll or wait for interrupt to signal completion of this reading. > Read data. > > Wait for 5 minutes > Trigger 2. Send the DLH_START_SINGLE; > poll or wait for interrupt to signal completion of this reading. > Read data. > > That should guarantee fresh data whatever the spacing of triggers. > > If you want to basically get data as quick as possible, the loop trigger will > ensure that you call the start asap after the prevous read. > > > + > > + for_each_set_bit(chn, indio_dev->active_scan_mask, > > + indio_dev->masklength) { > > + memcpy(tmp_buf + i, > > + &st->rx_buf[1] + chn * DLH_NUM_DATA_BYTES, > > + DLH_NUM_DATA_BYTES); > > + i++; > > + } > > + > > + iio_push_to_buffers(indio_dev, tmp_buf); > > + > > +out: > > + iio_trigger_notify_done(indio_dev->trig); > > + > > + return IRQ_HANDLED; > > +} > > + > > +static int dlh_i2c_probe(struct i2c_client *client, > > + const struct i2c_device_id *id) > > +{ > > + struct dlh_state *st; > > + struct iio_dev *indio_dev; > > + int ret; > > + > > + if (!i2c_check_functionality(client->adapter, > > + I2C_FUNC_I2C | I2C_FUNC_SMBUS_WRITE_BYTE)) { > > + dev_err(&client->dev, > > + "adapter doesn't support required i2c > functionality\n"); > > + return -EOPNOTSUPP; > > + } > > + > > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*st)); > > + if (!indio_dev) { > > + dev_err(&client->dev, "failed to allocate iio device\n"); > > + return -ENOMEM; > > + } > > + > > + i2c_set_clientdata(client, indio_dev); > > + > > + st = iio_priv(indio_dev); > > + st->info = dlh_info_tbl[id->driver_data]; > > + st->client = client; > > + > > + indio_dev->name = id->name; > > + indio_dev->dev.parent = &client->dev; > > + indio_dev->dev.of_node = client->dev.of_node; > > + indio_dev->info = &dlh_i2c_info; > > + indio_dev->modes = INDIO_DIRECT_MODE; > > + indio_dev->channels = dlh_i2c_channels; > > + indio_dev->num_channels = ARRAY_SIZE(dlh_i2c_channels); > > + > > + ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev, > > + &iio_pollfunc_store_time, &dlh_i2c_trigger_handler, NULL); > > + if (ret) { > > + dev_err(&client->dev, "failed to setup iio buffer\n"); > > + return ret; > > + } > > + > > + ret = devm_iio_device_register(&client->dev, indio_dev); > > + if (ret) { > > + dev_err(&client->dev, "failed to register iio device\n"); > > + return ret; > Drop the return ret; out of the brackets as then we don't need the return 0. > > if (ret) > dev_err(...) > > return ret; > } > > + } > > + > > + return 0; > > +} > > + > > +static const struct of_device_id dlh_i2c_of_match[] = { > > + { .compatible = "asc,dlhl60d" }, > > + { .compatible = "asc,dlhl60g" }, > > + {} > > +}; > > +MODULE_DEVICE_TABLE(of, dlh_i2c_of_match); > > + > > +static const struct i2c_device_id dlh_i2c_id[] = { > > + { "dlhl60d", dlhl60d }, > > + { "dlhl60g", dlhl60g }, > > + {} > > +}; > > +MODULE_DEVICE_TABLE(i2c, dlh_i2c_id); > > + > > +static struct i2c_driver dlh_i2c_driver = { > > + .driver = { > > + .name = "dlh_i2c", > > + .of_match_table = dlh_i2c_of_match, > > + }, > > + .probe = dlh_i2c_probe, > > + .id_table = dlh_i2c_id, > > +}; > > +module_i2c_driver(dlh_i2c_driver); > > + > > +MODULE_AUTHOR("Tomislav Denis <tomislav.denis@avl.com>"); > > +MODULE_DESCRIPTION("Driver for All Sensors DLH series pressure > > +sensors"); MODULE_LICENSE("GPL v2"); > ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] dt-bindings: Add asc vendor 2019-11-14 10:09 [PATCH 0/3] Add support for DLH pressure sensors tomislav.denis 2019-11-14 10:09 ` [PATCH 1/3] iio: pressure: Add driver " tomislav.denis @ 2019-11-14 10:09 ` tomislav.denis 2019-11-18 22:16 ` Rob Herring 2019-11-14 10:09 ` [PATCH 3/3] bindings: iio: pressure: Add dlh-i2c documentation tomislav.denis 2019-11-29 7:34 ` [PATCH v2 0/3] Add support for DLH pressure sensors tomislav.denis 3 siblings, 1 reply; 16+ messages in thread From: tomislav.denis @ 2019-11-14 10:09 UTC (permalink / raw) To: jic23; +Cc: linux-iio, devicetree, tomislav.denis From: Tomislav Denis <tomislav.denis@avl.com> All Sensors Corporation is a manufacturer of MEMS piezoresitive ultra low pressure sensors and pressure transducers. Signed-off-by: Tomislav Denis <tomislav.denis@avl.com> --- Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index 967e78c..88247b3 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -109,6 +109,8 @@ patternProperties: description: Artesyn Embedded Technologies Inc. "^asahi-kasei,.*": description: Asahi Kasei Corp. + "^asc,.*": + description: All Sensors Corporation "^aspeed,.*": description: ASPEED Technology Inc. "^asus,.*": -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] dt-bindings: Add asc vendor 2019-11-14 10:09 ` [PATCH 2/3] dt-bindings: Add asc vendor tomislav.denis @ 2019-11-18 22:16 ` Rob Herring 0 siblings, 0 replies; 16+ messages in thread From: Rob Herring @ 2019-11-18 22:16 UTC (permalink / raw) To: tomislav.denis; +Cc: jic23, linux-iio, devicetree, tomislav.denis On Thu, 14 Nov 2019 11:09:07 +0100, <tomislav.denis@avl.com> wrote: > From: Tomislav Denis <tomislav.denis@avl.com> > > All Sensors Corporation is a manufacturer of MEMS piezoresitive > ultra low pressure sensors and pressure transducers. > > Signed-off-by: Tomislav Denis <tomislav.denis@avl.com> > --- > Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ > 1 file changed, 2 insertions(+) > Acked-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 3/3] bindings: iio: pressure: Add dlh-i2c documentation 2019-11-14 10:09 [PATCH 0/3] Add support for DLH pressure sensors tomislav.denis 2019-11-14 10:09 ` [PATCH 1/3] iio: pressure: Add driver " tomislav.denis 2019-11-14 10:09 ` [PATCH 2/3] dt-bindings: Add asc vendor tomislav.denis @ 2019-11-14 10:09 ` tomislav.denis 2019-11-14 14:12 ` Jonathan Cameron 2019-11-29 7:34 ` [PATCH v2 0/3] Add support for DLH pressure sensors tomislav.denis 3 siblings, 1 reply; 16+ messages in thread From: tomislav.denis @ 2019-11-14 10:09 UTC (permalink / raw) To: jic23; +Cc: linux-iio, devicetree, tomislav.denis From: Tomislav Denis <tomislav.denis@avl.com> Add a device tree binding documentation for DLH series pressure sensors. Signed-off-by: Tomislav Denis <tomislav.denis@avl.com> --- .../devicetree/bindings/iio/pressure/dlh-i2c.yaml | 43 ++++++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 44 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/pressure/dlh-i2c.yaml diff --git a/Documentation/devicetree/bindings/iio/pressure/dlh-i2c.yaml b/Documentation/devicetree/bindings/iio/pressure/dlh-i2c.yaml new file mode 100644 index 0000000..43539ba --- /dev/null +++ b/Documentation/devicetree/bindings/iio/pressure/dlh-i2c.yaml @@ -0,0 +1,43 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/pressure/dlh-i2c.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: All Sensors DLH series low voltage digital pressure sensors + +maintainers: + - Tomislav Denis <tomislav.denis@avl.com> + +description: | + Bindings for the All Sensors DLH series pressure sensors. + + Specifications about the sensors can be found at: + http://www.allsensors.com/cad/DS-0355_Rev_B.PDF + +properties: + compatible: + enum: + - asc,dlhl60d + - asc,dlhl60g + + reg: + description: I2C device address + maxItems: 1 + +required: + - compatible + - reg + +examples: + - | + i2c0 { + #address-cells = <1>; + #size-cells = <0>; + + pressure@29 { + compatible = "asc,dlhl60d"; + reg = <0x29>; + }; + }; +... diff --git a/MAINTAINERS b/MAINTAINERS index 2a08923..b45081d 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -674,6 +674,7 @@ W: http://www.allsensors.com/cad/DS-0355_Rev_B.PDF S: Maintained L: linux-iio@vger.kernel.org F: drivers/iio/pressure/dlh-i2c.c +F: Documentation/devicetree/bindings/iio/pressure/dlh-i2c.yaml ALLEGRO DVT VIDEO IP CORE DRIVER M: Michael Tretter <m.tretter@pengutronix.de> -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 3/3] bindings: iio: pressure: Add dlh-i2c documentation 2019-11-14 10:09 ` [PATCH 3/3] bindings: iio: pressure: Add dlh-i2c documentation tomislav.denis @ 2019-11-14 14:12 ` Jonathan Cameron 0 siblings, 0 replies; 16+ messages in thread From: Jonathan Cameron @ 2019-11-14 14:12 UTC (permalink / raw) To: tomislav.denis; +Cc: jic23, linux-iio, devicetree On Thu, 14 Nov 2019 11:09:08 +0100 <tomislav.denis@avl.com> wrote: > From: Tomislav Denis <tomislav.denis@avl.com> > > Add a device tree binding documentation for DLH series pressure > sensors. > > Signed-off-by: Tomislav Denis <tomislav.denis@avl.com> Hi Tomislav, A few little comments inline Thanks, Jonathan > --- > .../devicetree/bindings/iio/pressure/dlh-i2c.yaml | 43 ++++++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 44 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/pressure/dlh-i2c.yaml > > diff --git a/Documentation/devicetree/bindings/iio/pressure/dlh-i2c.yaml b/Documentation/devicetree/bindings/iio/pressure/dlh-i2c.yaml > new file mode 100644 > index 0000000..43539ba > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/pressure/dlh-i2c.yaml prefix the filename as per the compatible - so asc,slh-i2c.yaml > @@ -0,0 +1,43 @@ > +# SPDX-License-Identifier: GPL-2.0 I'm guessing you own the copyright on this binding. Where possible the DT maintainers are requesting that bindings are dual licensed as (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/pressure/dlh-i2c.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: All Sensors DLH series low voltage digital pressure sensors > + > +maintainers: > + - Tomislav Denis <tomislav.denis@avl.com> > + > +description: | > + Bindings for the All Sensors DLH series pressure sensors. > + > + Specifications about the sensors can be found at: > + http://www.allsensors.com/cad/DS-0355_Rev_B.PDF I took a quick look at the datasheet. Whilst I guess you don't have it wired up, there is an EOC line which should be here as the doc should cover the hardware, rather than what we currently use in the driver. The EOC line looks like a data ready interrupt from my quick read. > + > +properties: > + compatible: > + enum: > + - asc,dlhl60d > + - asc,dlhl60g > + > + reg: > + description: I2C device address > + maxItems: 1 > + > +required: > + - compatible > + - reg > + > +examples: > + - | > + i2c0 { > + #address-cells = <1>; > + #size-cells = <0>; > + > + pressure@29 { > + compatible = "asc,dlhl60d"; > + reg = <0x29>; > + }; > + }; > +... > diff --git a/MAINTAINERS b/MAINTAINERS > index 2a08923..b45081d 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -674,6 +674,7 @@ W: http://www.allsensors.com/cad/DS-0355_Rev_B.PDF > S: Maintained > L: linux-iio@vger.kernel.org > F: drivers/iio/pressure/dlh-i2c.c > +F: Documentation/devicetree/bindings/iio/pressure/dlh-i2c.yaml > > ALLEGRO DVT VIDEO IP CORE DRIVER > M: Michael Tretter <m.tretter@pengutronix.de> ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 0/3] Add support for DLH pressure sensors 2019-11-14 10:09 [PATCH 0/3] Add support for DLH pressure sensors tomislav.denis ` (2 preceding siblings ...) 2019-11-14 10:09 ` [PATCH 3/3] bindings: iio: pressure: Add dlh-i2c documentation tomislav.denis @ 2019-11-29 7:34 ` tomislav.denis 2019-11-29 7:34 ` [PATCH v2 1/3] iio: pressure: Add driver " tomislav.denis ` (3 more replies) 3 siblings, 4 replies; 16+ messages in thread From: tomislav.denis @ 2019-11-29 7:34 UTC (permalink / raw) To: jic23; +Cc: linux-iio, devicetree, tomislav.denis From: Tomislav Denis <tomislav.denis@avl.com> This patchset adds support for All Sensors DLH series low voltage digital pressure sensors. Datasheet: https://www.allsensors.com/cad/DS-0355_Rev_B.PDF Changes in v2: - web page link in the MAINTAINERS file fixed - adjust the units of the output to the IIO ABI - unneceseary default case removed - define the channel member of the iio_chan_spec struct for channels specification - remove explicit cast for pointers of type void * - add support for the EOC(data ready) pin - drop the unneceseary return ret; - rename dlh-i2c.yaml to asc,dlh-i2c.yaml - change the bindings copyright to GPL-2.0-only OR BSD-2-Clause - document EOC(data ready) pin Tomislav Denis (3): iio: pressure: Add driver for DLH pressure sensors dt-bindings: Add asc vendor bindings: iio: pressure: Add dlh-i2c documentation .../bindings/iio/pressure/asc,dlh-i2c.yaml | 51 +++ .../devicetree/bindings/vendor-prefixes.yaml | 2 + MAINTAINERS | 8 + drivers/iio/pressure/Kconfig | 12 + drivers/iio/pressure/Makefile | 1 + drivers/iio/pressure/dlh-i2c.c | 429 +++++++++++++++++++++ 6 files changed, 503 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/pressure/asc,dlh-i2c.yaml create mode 100644 drivers/iio/pressure/dlh-i2c.c -- 2.7.4 ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 1/3] iio: pressure: Add driver for DLH pressure sensors 2019-11-29 7:34 ` [PATCH v2 0/3] Add support for DLH pressure sensors tomislav.denis @ 2019-11-29 7:34 ` tomislav.denis 2019-12-01 10:57 ` Jonathan Cameron 2019-11-29 7:34 ` [PATCH v2 2/3] dt-bindings: Add asc vendor tomislav.denis ` (2 subsequent siblings) 3 siblings, 1 reply; 16+ messages in thread From: tomislav.denis @ 2019-11-29 7:34 UTC (permalink / raw) To: jic23; +Cc: linux-iio, devicetree, tomislav.denis From: Tomislav Denis <tomislav.denis@avl.com> All Sensors DLH is series of low voltage digital pressure sensors. Additionally to pressure value sensors deliver a temperature value. Sensors can be accessed over I2C and SPI, this driver supports only I2C access. Signed-off-by: Tomislav Denis <tomislav.denis@avl.com> --- MAINTAINERS | 7 + drivers/iio/pressure/Kconfig | 12 ++ drivers/iio/pressure/Makefile | 1 + drivers/iio/pressure/dlh-i2c.c | 429 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 449 insertions(+) create mode 100644 drivers/iio/pressure/dlh-i2c.c diff --git a/MAINTAINERS b/MAINTAINERS index d5ea4e4..39d6f0f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -668,6 +668,13 @@ S: Maintained F: Documentation/i2c/busses/i2c-ali1563.rst F: drivers/i2c/busses/i2c-ali1563.c +ALL SENSORS DLH SERIES PRESSURE SENSORS DRIVER +M: Tomislav Denis <tomislav.denis@avl.com> +W: http://www.allsensors.com/ +S: Maintained +L: linux-iio@vger.kernel.org +F: drivers/iio/pressure/dlh-i2c.c + ALLEGRO DVT VIDEO IP CORE DRIVER M: Michael Tretter <m.tretter@pengutronix.de> R: Pengutronix Kernel Team <kernel@pengutronix.de> diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig index ba420e4..504de3e 100644 --- a/drivers/iio/pressure/Kconfig +++ b/drivers/iio/pressure/Kconfig @@ -53,6 +53,18 @@ config IIO_CROS_EC_BARO To compile this driver as a module, choose M here: the module will be called cros_ec_baro. +config DLH_I2C + tristate "All Sensors DLH series low voltage digital pressure sensors" + depends on I2C + select IIO_BUFFER + select IIO_TRIGGERED_BUFFER + help + Say yes here to build support for the All Sensors DLH series + pressure sensors driver. + + To compile this driver as a module, choose M here: the module + will be called dlh-i2c. + config DPS310 tristate "Infineon DPS310 pressure and temperature sensor" depends on I2C diff --git a/drivers/iio/pressure/Makefile b/drivers/iio/pressure/Makefile index d8f5ace..1851a36 100644 --- a/drivers/iio/pressure/Makefile +++ b/drivers/iio/pressure/Makefile @@ -9,6 +9,7 @@ obj-$(CONFIG_BMP280) += bmp280.o bmp280-objs := bmp280-core.o bmp280-regmap.o obj-$(CONFIG_BMP280_I2C) += bmp280-i2c.o obj-$(CONFIG_BMP280_SPI) += bmp280-spi.o +obj-$(CONFIG_DLH_I2C) += dlh-i2c.o obj-$(CONFIG_DPS310) += dps310.o obj-$(CONFIG_IIO_CROS_EC_BARO) += cros_ec_baro.o obj-$(CONFIG_HID_SENSOR_PRESS) += hid-sensor-press.o diff --git a/drivers/iio/pressure/dlh-i2c.c b/drivers/iio/pressure/dlh-i2c.c new file mode 100644 index 0000000..d980f7e --- /dev/null +++ b/drivers/iio/pressure/dlh-i2c.c @@ -0,0 +1,429 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * All Sensors DLH series low voltage digital pressure sensors + * + * Copyright (c) 2019 AVL DiTEST GmbH + * Tomislav Denis <tomislav.denis@avl.com> + * + * Datasheet: http://www.allsensors.com/cad/DS-0355_Rev_B.PDF + */ + +#include <linux/module.h> +#include <linux/delay.h> +#include <linux/i2c.h> +#include <linux/iio/iio.h> +#include <linux/iio/buffer.h> +#include <linux/iio/trigger.h> +#include <linux/iio/trigger_consumer.h> +#include <linux/iio/triggered_buffer.h> + +/* Commands */ +#define DLH_START_SINGLE 0xAA + +/* Status bits */ +#define DLH_STATUS_OK 0x40 + +/* DLH data format */ +#define DLH_NUM_READ_BYTES 7 +#define DLH_NUM_DATA_BYTES 3 +#define DLH_NUM_PR_BITS 24 +#define DLH_NUM_TEMP_BITS 24 + +/* DLH timings */ +#define DLH_SINGLE_DUT_MS 5 + +enum dhl_ids { + dlhl60d, + dlhl60g, +}; + +struct dlh_info { + u8 osdig; /* digital offset factor */ + unsigned int fss; /* full scale span (inch H2O) */ +}; + +struct dlh_state { + struct i2c_client *client; + struct dlh_info info; + struct completion completion; + struct iio_trigger *dready_trig; + bool dready_trig_on; + u8 rx_buf[DLH_NUM_READ_BYTES] ____cacheline_aligned; +}; + +static struct dlh_info dlh_info_tbl[] = { + [dlhl60d] = { + .osdig = 2, + .fss = 120, + }, + [dlhl60g] = { + .osdig = 10, + .fss = 60, + }, +}; + + +static int dlh_i2c_cmd_start_single(struct dlh_state *st) +{ + int ret; + + ret = i2c_smbus_write_byte(st->client, DLH_START_SINGLE); + if (ret) + dev_err(&st->client->dev, + "%s: I2C write byte failed\n", __func__); + + return ret; +} + +static int dlh_i2c_cmd_read_data(struct dlh_state *st) +{ + int ret; + + ret = i2c_master_recv(st->client, st->rx_buf, DLH_NUM_READ_BYTES); + if (ret < 0) { + dev_err(&st->client->dev, + "%s: I2C read block failed\n", __func__); + return ret; + } + + if (st->rx_buf[0] != DLH_STATUS_OK) { + dev_err(&st->client->dev, + "%s: invalid status 0x%02x\n", __func__, st->rx_buf[0]); + return -EBUSY; + } + + return 0; +} + +static int dlh_i2c_read_direct(struct dlh_state *st, + unsigned int *pressure, unsigned int *temperature) +{ + int ret; + + if (st->dready_trig) + reinit_completion(&st->completion); + + ret = dlh_i2c_cmd_start_single(st); + if (ret) + return ret; + + if (st->dready_trig) { + ret = wait_for_completion_timeout(&st->completion, + msecs_to_jiffies(DLH_SINGLE_DUT_MS)); + if (!ret) + return -ETIMEDOUT; + } else { + mdelay(DLH_SINGLE_DUT_MS); + } + + ret = dlh_i2c_cmd_read_data(st); + if (ret) + return ret; + + *pressure = be32_to_cpup((u32 *)&st->rx_buf[1]) >> 8; + *temperature = be32_to_cpup((u32 *)&st->rx_buf[3]) & + GENMASK(DLH_NUM_TEMP_BITS - 1, 0); + + return 0; +} + +static int dlh_i2c_read_raw(struct iio_dev *indio_dev, + struct iio_chan_spec const *channel, int *value, + int *value2, long mask) +{ + struct dlh_state *st = iio_priv(indio_dev); + unsigned int pressure, temperature; + int ret; + s64 tmp; + s32 rem; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + if (iio_buffer_enabled(indio_dev)) + return -EBUSY; + + ret = iio_device_claim_direct_mode(indio_dev); + if (ret) + return ret; + + ret = dlh_i2c_read_direct(st, &pressure, &temperature); + iio_device_release_direct_mode(indio_dev); + if (ret) + return ret; + + switch (channel->type) { + case IIO_PRESSURE: + *value = pressure; + return IIO_VAL_INT; + + case IIO_TEMP: + *value = temperature; + return IIO_VAL_INT; + + default: + return -EINVAL; + } + case IIO_CHAN_INFO_SCALE: + switch (channel->type) { + case IIO_PRESSURE: + tmp = div_s64(125LL * st->info.fss * 24909 * 100, + 1 << DLH_NUM_PR_BITS); + tmp = div_s64_rem(tmp, 1000000000LL, &rem); + *value = tmp; + *value2 = rem; + return IIO_VAL_INT_PLUS_NANO; + + case IIO_TEMP: + *value = 125 * 1000; + *value2 = DLH_NUM_TEMP_BITS; + return IIO_VAL_FRACTIONAL_LOG2; + + default: + return -EINVAL; + } + case IIO_CHAN_INFO_OFFSET: + switch (channel->type) { + case IIO_PRESSURE: + *value = -125 * st->info.fss * 24909; + *value2 = 100 * st->info.osdig * 100000; + return IIO_VAL_FRACTIONAL; + + case IIO_TEMP: + *value = -40 * 1000; + return IIO_VAL_INT; + + default: + return -EINVAL; + } + } + + return -EINVAL; +} + +static int dlh_i2c_validate_trigger(struct iio_dev *indio_dev, + struct iio_trigger *trig) +{ + struct dlh_state *st = iio_priv(indio_dev); + + if (st->dready_trig != trig) + return -EINVAL; + + return 0; +} + +static const struct iio_info dlh_i2c_info = { + .read_raw = dlh_i2c_read_raw, + .validate_trigger = dlh_i2c_validate_trigger, +}; + +static const struct iio_chan_spec dlh_i2c_channels[] = { + { + .type = IIO_PRESSURE, + .indexed = 1, + .channel = 0, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), + .info_mask_shared_by_type = + BIT(IIO_CHAN_INFO_SCALE) | + BIT(IIO_CHAN_INFO_OFFSET), + .scan_index = 0, + .scan_type = { + .sign = 'u', + .realbits = DLH_NUM_PR_BITS, + .storagebits = 32, + .shift = 8, + .endianness = IIO_BE, + }, + }, { + .type = IIO_TEMP, + .indexed = 1, + .channel = 0, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), + .info_mask_shared_by_type = + BIT(IIO_CHAN_INFO_SCALE) | + BIT(IIO_CHAN_INFO_OFFSET), + .scan_index = 1, + .scan_type = { + .sign = 'u', + .realbits = DLH_NUM_TEMP_BITS, + .storagebits = 32, + .shift = 8, + .endianness = IIO_BE, + }, + } +}; + +static irqreturn_t dlh_i2c_trigger_handler(int irq, void *private) +{ + struct iio_poll_func *pf = private; + struct iio_dev *indio_dev = pf->indio_dev; + struct dlh_state *st = iio_priv(indio_dev); + int ret; + unsigned int chn, i = 0; + __be32 tmp_buf[2]; + + ret = dlh_i2c_cmd_read_data(st); + if (ret) + goto out; + + ret = dlh_i2c_cmd_start_single(st); + if (ret) + goto out; + + for_each_set_bit(chn, indio_dev->active_scan_mask, + indio_dev->masklength) { + memcpy(tmp_buf + i, + &st->rx_buf[1] + chn * DLH_NUM_DATA_BYTES, + DLH_NUM_DATA_BYTES); + i++; + } + + iio_push_to_buffers(indio_dev, tmp_buf); + +out: + iio_trigger_notify_done(indio_dev->trig); + + return IRQ_HANDLED; +} + +static irqreturn_t dlh_i2c_interrupt(int irq, void *private) +{ + struct iio_dev *indio_dev = private; + struct dlh_state *st = iio_priv(indio_dev); + + if (iio_buffer_enabled(indio_dev) && st->dready_trig_on) + iio_trigger_poll(st->dready_trig); + else + complete(&st->completion); + + return IRQ_HANDLED; +}; + +static int dlh_i2c_set_trigger_state(struct iio_trigger *trig, bool state) +{ + struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig); + struct dlh_state *st = iio_priv(indio_dev); + int ret = 0; + + st->dready_trig_on = state; + + if (state) + ret = dlh_i2c_cmd_start_single(st); + + return ret; +} + +static const struct iio_trigger_ops dlh_i2c_trigger_ops = { + .set_trigger_state = dlh_i2c_set_trigger_state, +}; + +static int dlh_i2c_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct dlh_state *st; + struct iio_dev *indio_dev; + int ret; + + if (!i2c_check_functionality(client->adapter, + I2C_FUNC_I2C | I2C_FUNC_SMBUS_WRITE_BYTE)) { + dev_err(&client->dev, + "adapter doesn't support required i2c functionality\n"); + return -EOPNOTSUPP; + } + + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*st)); + if (!indio_dev) { + dev_err(&client->dev, "failed to allocate iio device\n"); + return -ENOMEM; + } + + i2c_set_clientdata(client, indio_dev); + + st = iio_priv(indio_dev); + st->info = dlh_info_tbl[id->driver_data]; + st->client = client; + st->dready_trig = NULL; + + indio_dev->name = id->name; + indio_dev->dev.parent = &client->dev; + indio_dev->dev.of_node = client->dev.of_node; + indio_dev->info = &dlh_i2c_info; + indio_dev->modes = INDIO_DIRECT_MODE; + indio_dev->channels = dlh_i2c_channels; + indio_dev->num_channels = ARRAY_SIZE(dlh_i2c_channels); + + if (client->irq > 0) { + st->dready_trig = devm_iio_trigger_alloc(&client->dev, + "%s-dev%d", indio_dev->name, indio_dev->id); + if (!st->dready_trig) { + dev_err(&client->dev, "failed to allocate trigger"); + return -ENOMEM; + } + + ret = devm_request_threaded_irq(&client->dev, client->irq, + dlh_i2c_interrupt, NULL, + IRQF_TRIGGER_RISING | IRQF_ONESHOT, + id->name, indio_dev); + if (ret) { + dev_err(&client->dev, "failed to allocate threaded irq"); + return ret; + } + + st->dready_trig->dev.parent = &client->dev; + st->dready_trig->ops = &dlh_i2c_trigger_ops; + iio_trigger_set_drvdata(st->dready_trig, indio_dev); + indio_dev->trig = iio_trigger_get(st->dready_trig); + + init_completion(&st->completion); + st->dready_trig_on = false; + + ret = devm_iio_trigger_register(&client->dev, st->dready_trig); + if (ret) { + dev_err(&client->dev, "failed to register trigger\n"); + return ret; + } + } else { + dev_info(&client->dev, + "no irq, falling back to polling in direct mode\n"); + } + + ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev, + NULL, &dlh_i2c_trigger_handler, NULL); + if (ret) { + dev_err(&client->dev, "failed to setup iio buffer\n"); + return ret; + } + + ret = devm_iio_device_register(&client->dev, indio_dev); + if (ret) + dev_err(&client->dev, "failed to register iio device\n"); + + return ret; +} + +static const struct of_device_id dlh_i2c_of_match[] = { + { .compatible = "asc,dlhl60d" }, + { .compatible = "asc,dlhl60g" }, + {} +}; +MODULE_DEVICE_TABLE(of, dlh_i2c_of_match); + +static const struct i2c_device_id dlh_i2c_id[] = { + { "dlhl60d", dlhl60d }, + { "dlhl60g", dlhl60g }, + {} +}; +MODULE_DEVICE_TABLE(i2c, dlh_i2c_id); + +static struct i2c_driver dlh_i2c_driver = { + .driver = { + .name = "dlh_i2c", + .of_match_table = dlh_i2c_of_match, + }, + .probe = dlh_i2c_probe, + .id_table = dlh_i2c_id, +}; +module_i2c_driver(dlh_i2c_driver); + +MODULE_AUTHOR("Tomislav Denis <tomislav.denis@avl.com>"); +MODULE_DESCRIPTION("Driver for All Sensors DLH series pressure sensors"); +MODULE_LICENSE("GPL v2"); -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/3] iio: pressure: Add driver for DLH pressure sensors 2019-11-29 7:34 ` [PATCH v2 1/3] iio: pressure: Add driver " tomislav.denis @ 2019-12-01 10:57 ` Jonathan Cameron 0 siblings, 0 replies; 16+ messages in thread From: Jonathan Cameron @ 2019-12-01 10:57 UTC (permalink / raw) To: tomislav.denis; +Cc: linux-iio, devicetree On Fri, 29 Nov 2019 08:34:18 +0100 <tomislav.denis@avl.com> wrote: > From: Tomislav Denis <tomislav.denis@avl.com> > > All Sensors DLH is series of low voltage digital pressure sensors. > Additionally to pressure value sensors deliver a temperature value. > Sensors can be accessed over I2C and SPI, this driver supports > only I2C access. > > Signed-off-by: Tomislav Denis <tomislav.denis@avl.com> I'm afraid I misunderstood the nature of the trigger events previously. Comments inline, but in a general sense I don't think this device provides anything that should really be considered a trigger in IIO as it isn't 'self clocking'. For the usecase you have here we have the iio_loop trigger. It's been a while but from what I recall that should cause continuous reads, limited in speed by how long it takes the completion events in the driver to occur. Currently you can't use this device with a high resolution timer or similar trigger, which users may well want to do. Moving to a model with: 1) Trigger 2) Start Capture 3) Wait for completion from interrupt 4) Push data to buffer 5) Say done so trigger can start again. Would allow it to work with any trigger. Jonathan > --- > MAINTAINERS | 7 + > drivers/iio/pressure/Kconfig | 12 ++ > drivers/iio/pressure/Makefile | 1 + > drivers/iio/pressure/dlh-i2c.c | 429 +++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 449 insertions(+) > create mode 100644 drivers/iio/pressure/dlh-i2c.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index d5ea4e4..39d6f0f 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -668,6 +668,13 @@ S: Maintained > F: Documentation/i2c/busses/i2c-ali1563.rst > F: drivers/i2c/busses/i2c-ali1563.c > > +ALL SENSORS DLH SERIES PRESSURE SENSORS DRIVER > +M: Tomislav Denis <tomislav.denis@avl.com> > +W: http://www.allsensors.com/ > +S: Maintained > +L: linux-iio@vger.kernel.org > +F: drivers/iio/pressure/dlh-i2c.c > + > ALLEGRO DVT VIDEO IP CORE DRIVER > M: Michael Tretter <m.tretter@pengutronix.de> > R: Pengutronix Kernel Team <kernel@pengutronix.de> > diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig > index ba420e4..504de3e 100644 > --- a/drivers/iio/pressure/Kconfig > +++ b/drivers/iio/pressure/Kconfig > @@ -53,6 +53,18 @@ config IIO_CROS_EC_BARO > To compile this driver as a module, choose M here: the module > will be called cros_ec_baro. > > +config DLH_I2C > + tristate "All Sensors DLH series low voltage digital pressure sensors" > + depends on I2C > + select IIO_BUFFER > + select IIO_TRIGGERED_BUFFER > + help > + Say yes here to build support for the All Sensors DLH series > + pressure sensors driver. > + > + To compile this driver as a module, choose M here: the module > + will be called dlh-i2c. > + > config DPS310 > tristate "Infineon DPS310 pressure and temperature sensor" > depends on I2C > diff --git a/drivers/iio/pressure/Makefile b/drivers/iio/pressure/Makefile > index d8f5ace..1851a36 100644 > --- a/drivers/iio/pressure/Makefile > +++ b/drivers/iio/pressure/Makefile > @@ -9,6 +9,7 @@ obj-$(CONFIG_BMP280) += bmp280.o > bmp280-objs := bmp280-core.o bmp280-regmap.o > obj-$(CONFIG_BMP280_I2C) += bmp280-i2c.o > obj-$(CONFIG_BMP280_SPI) += bmp280-spi.o > +obj-$(CONFIG_DLH_I2C) += dlh-i2c.o > obj-$(CONFIG_DPS310) += dps310.o > obj-$(CONFIG_IIO_CROS_EC_BARO) += cros_ec_baro.o > obj-$(CONFIG_HID_SENSOR_PRESS) += hid-sensor-press.o > diff --git a/drivers/iio/pressure/dlh-i2c.c b/drivers/iio/pressure/dlh-i2c.c > new file mode 100644 > index 0000000..d980f7e > --- /dev/null > +++ b/drivers/iio/pressure/dlh-i2c.c > @@ -0,0 +1,429 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * All Sensors DLH series low voltage digital pressure sensors > + * > + * Copyright (c) 2019 AVL DiTEST GmbH > + * Tomislav Denis <tomislav.denis@avl.com> > + * > + * Datasheet: http://www.allsensors.com/cad/DS-0355_Rev_B.PDF > + */ > + > +#include <linux/module.h> > +#include <linux/delay.h> > +#include <linux/i2c.h> > +#include <linux/iio/iio.h> > +#include <linux/iio/buffer.h> > +#include <linux/iio/trigger.h> > +#include <linux/iio/trigger_consumer.h> > +#include <linux/iio/triggered_buffer.h> > + > +/* Commands */ > +#define DLH_START_SINGLE 0xAA > + > +/* Status bits */ > +#define DLH_STATUS_OK 0x40 > + > +/* DLH data format */ > +#define DLH_NUM_READ_BYTES 7 > +#define DLH_NUM_DATA_BYTES 3 > +#define DLH_NUM_PR_BITS 24 > +#define DLH_NUM_TEMP_BITS 24 > + > +/* DLH timings */ > +#define DLH_SINGLE_DUT_MS 5 > + > +enum dhl_ids { > + dlhl60d, > + dlhl60g, > +}; > + > +struct dlh_info { > + u8 osdig; /* digital offset factor */ > + unsigned int fss; /* full scale span (inch H2O) */ > +}; > + > +struct dlh_state { > + struct i2c_client *client; > + struct dlh_info info; > + struct completion completion; > + struct iio_trigger *dready_trig; > + bool dready_trig_on; > + u8 rx_buf[DLH_NUM_READ_BYTES] ____cacheline_aligned; > +}; > + > +static struct dlh_info dlh_info_tbl[] = { > + [dlhl60d] = { > + .osdig = 2, > + .fss = 120, > + }, > + [dlhl60g] = { > + .osdig = 10, > + .fss = 60, > + }, > +}; > + > + > +static int dlh_i2c_cmd_start_single(struct dlh_state *st) > +{ > + int ret; > + > + ret = i2c_smbus_write_byte(st->client, DLH_START_SINGLE); > + if (ret) > + dev_err(&st->client->dev, > + "%s: I2C write byte failed\n", __func__); > + > + return ret; > +} > + > +static int dlh_i2c_cmd_read_data(struct dlh_state *st) > +{ > + int ret; > + > + ret = i2c_master_recv(st->client, st->rx_buf, DLH_NUM_READ_BYTES); > + if (ret < 0) { > + dev_err(&st->client->dev, > + "%s: I2C read block failed\n", __func__); > + return ret; > + } > + > + if (st->rx_buf[0] != DLH_STATUS_OK) { > + dev_err(&st->client->dev, > + "%s: invalid status 0x%02x\n", __func__, st->rx_buf[0]); > + return -EBUSY; > + } > + > + return 0; > +} > + > +static int dlh_i2c_read_direct(struct dlh_state *st, > + unsigned int *pressure, unsigned int *temperature) > +{ > + int ret; > + > + if (st->dready_trig) > + reinit_completion(&st->completion); > + > + ret = dlh_i2c_cmd_start_single(st); > + if (ret) > + return ret; > + > + if (st->dready_trig) { > + ret = wait_for_completion_timeout(&st->completion, > + msecs_to_jiffies(DLH_SINGLE_DUT_MS)); > + if (!ret) > + return -ETIMEDOUT; > + } else { > + mdelay(DLH_SINGLE_DUT_MS); > + } > + > + ret = dlh_i2c_cmd_read_data(st); > + if (ret) > + return ret; > + > + *pressure = be32_to_cpup((u32 *)&st->rx_buf[1]) >> 8; > + *temperature = be32_to_cpup((u32 *)&st->rx_buf[3]) & > + GENMASK(DLH_NUM_TEMP_BITS - 1, 0); > + > + return 0; > +} > + > +static int dlh_i2c_read_raw(struct iio_dev *indio_dev, > + struct iio_chan_spec const *channel, int *value, > + int *value2, long mask) > +{ > + struct dlh_state *st = iio_priv(indio_dev); > + unsigned int pressure, temperature; > + int ret; > + s64 tmp; > + s32 rem; > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + if (iio_buffer_enabled(indio_dev)) > + return -EBUSY; claim_direct already returns -EBUSY if the buffer is enabled, so this prior check shouldn't be needed. > + > + ret = iio_device_claim_direct_mode(indio_dev); > + if (ret) > + return ret; > + > + ret = dlh_i2c_read_direct(st, &pressure, &temperature); > + iio_device_release_direct_mode(indio_dev); > + if (ret) > + return ret; > + > + switch (channel->type) { > + case IIO_PRESSURE: > + *value = pressure; > + return IIO_VAL_INT; > + > + case IIO_TEMP: > + *value = temperature; > + return IIO_VAL_INT; > + > + default: > + return -EINVAL; > + } > + case IIO_CHAN_INFO_SCALE: > + switch (channel->type) { > + case IIO_PRESSURE: > + tmp = div_s64(125LL * st->info.fss * 24909 * 100, > + 1 << DLH_NUM_PR_BITS); > + tmp = div_s64_rem(tmp, 1000000000LL, &rem); > + *value = tmp; > + *value2 = rem; > + return IIO_VAL_INT_PLUS_NANO; > + > + case IIO_TEMP: > + *value = 125 * 1000; > + *value2 = DLH_NUM_TEMP_BITS; > + return IIO_VAL_FRACTIONAL_LOG2; > + > + default: > + return -EINVAL; > + } > + case IIO_CHAN_INFO_OFFSET: > + switch (channel->type) { > + case IIO_PRESSURE: > + *value = -125 * st->info.fss * 24909; > + *value2 = 100 * st->info.osdig * 100000; > + return IIO_VAL_FRACTIONAL; > + > + case IIO_TEMP: > + *value = -40 * 1000; > + return IIO_VAL_INT; > + > + default: > + return -EINVAL; > + } > + } > + > + return -EINVAL; > +} > + > +static int dlh_i2c_validate_trigger(struct iio_dev *indio_dev, > + struct iio_trigger *trig) > +{ > + struct dlh_state *st = iio_priv(indio_dev); > + > + if (st->dready_trig != trig) > + return -EINVAL; > + > + return 0; > +} > + > +static const struct iio_info dlh_i2c_info = { > + .read_raw = dlh_i2c_read_raw, > + .validate_trigger = dlh_i2c_validate_trigger, > +}; > + > +static const struct iio_chan_spec dlh_i2c_channels[] = { > + { > + .type = IIO_PRESSURE, > + .indexed = 1, > + .channel = 0, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > + .info_mask_shared_by_type = > + BIT(IIO_CHAN_INFO_SCALE) | > + BIT(IIO_CHAN_INFO_OFFSET), > + .scan_index = 0, > + .scan_type = { > + .sign = 'u', > + .realbits = DLH_NUM_PR_BITS, > + .storagebits = 32, > + .shift = 8, > + .endianness = IIO_BE, > + }, > + }, { > + .type = IIO_TEMP, > + .indexed = 1, > + .channel = 0, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > + .info_mask_shared_by_type = > + BIT(IIO_CHAN_INFO_SCALE) | > + BIT(IIO_CHAN_INFO_OFFSET), > + .scan_index = 1, > + .scan_type = { > + .sign = 'u', > + .realbits = DLH_NUM_TEMP_BITS, > + .storagebits = 32, > + .shift = 8, > + .endianness = IIO_BE, > + }, > + } > +}; > + > +static irqreturn_t dlh_i2c_trigger_handler(int irq, void *private) > +{ > + struct iio_poll_func *pf = private; > + struct iio_dev *indio_dev = pf->indio_dev; > + struct dlh_state *st = iio_priv(indio_dev); > + int ret; > + unsigned int chn, i = 0; > + __be32 tmp_buf[2]; > + > + ret = dlh_i2c_cmd_read_data(st); > + if (ret) > + goto out; > + > + ret = dlh_i2c_cmd_start_single(st); > + if (ret) > + goto out; Is this repriming the sensor to grab another sample? If so it belongs in the try_renenable callback of the trigger. If we were using this trigger for another sensor, we only want to re enable the trigger once that other sensor is ready. This becomes less critical if we provide validation functions to limit the usefulness of this trigger as mentioned below. I'm wondering if this sensor isn't actually self clocking, but rather is a trigger of demand type of sensor with a completion interrupt? A self clocking or sequencing device would grab data at even samples whether or not we had acknowledged the previous one (sometimes into a fifo, sometimes just overwriting the previous data, sometimes reporting an overflow if data hasn't been read). If taking that view, it shouldn't provide a trigger at all. The same result would be achieved by using an iio_loop trigger. The loop trigger will result in data capture being initialised just as soon as the previous data capture finishes. > + > + for_each_set_bit(chn, indio_dev->active_scan_mask, > + indio_dev->masklength) { > + memcpy(tmp_buf + i, > + &st->rx_buf[1] + chn * DLH_NUM_DATA_BYTES, > + DLH_NUM_DATA_BYTES); > + i++; > + } > + > + iio_push_to_buffers(indio_dev, tmp_buf); > + > +out: > + iio_trigger_notify_done(indio_dev->trig); > + > + return IRQ_HANDLED; > +} > + > +static irqreturn_t dlh_i2c_interrupt(int irq, void *private) > +{ > + struct iio_dev *indio_dev = private; > + struct dlh_state *st = iio_priv(indio_dev); > + > + if (iio_buffer_enabled(indio_dev) && st->dready_trig_on) You have validation that prevents this device using any other trigger so I don't think you can get here with only one of the two conditions being true. However nothing is stopping another device using this trigger even if this device is not. That device will wait for ever. As suggested above, I'm not sure having a trigger here at all makes sense but if it doesn't you probably also want the validate_device callback set for the trigger. > + iio_trigger_poll(st->dready_trig); > + else > + complete(&st->completion); > + > + return IRQ_HANDLED; > +}; > + > +static int dlh_i2c_set_trigger_state(struct iio_trigger *trig, bool state) > +{ > + struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig); > + struct dlh_state *st = iio_priv(indio_dev); > + int ret = 0; > + > + st->dready_trig_on = state; > + > + if (state) > + ret = dlh_i2c_cmd_start_single(st); > + > + return ret; Flip the logic and this ends up a tiny bit simpler if (!state) return 0; return dlh_i2c_cmd_start_single(st); > +} > + > +static const struct iio_trigger_ops dlh_i2c_trigger_ops = { > + .set_trigger_state = dlh_i2c_set_trigger_state, > +}; > + > +static int dlh_i2c_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct dlh_state *st; > + struct iio_dev *indio_dev; > + int ret; > + > + if (!i2c_check_functionality(client->adapter, > + I2C_FUNC_I2C | I2C_FUNC_SMBUS_WRITE_BYTE)) { > + dev_err(&client->dev, > + "adapter doesn't support required i2c functionality\n"); > + return -EOPNOTSUPP; > + } > + > + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*st)); > + if (!indio_dev) { > + dev_err(&client->dev, "failed to allocate iio device\n"); > + return -ENOMEM; > + } > + > + i2c_set_clientdata(client, indio_dev); > + > + st = iio_priv(indio_dev); > + st->info = dlh_info_tbl[id->driver_data]; > + st->client = client; > + st->dready_trig = NULL; > + > + indio_dev->name = id->name; > + indio_dev->dev.parent = &client->dev; > + indio_dev->dev.of_node = client->dev.of_node; > + indio_dev->info = &dlh_i2c_info; > + indio_dev->modes = INDIO_DIRECT_MODE; > + indio_dev->channels = dlh_i2c_channels; > + indio_dev->num_channels = ARRAY_SIZE(dlh_i2c_channels); > + > + if (client->irq > 0) { > + st->dready_trig = devm_iio_trigger_alloc(&client->dev, > + "%s-dev%d", indio_dev->name, indio_dev->id); > + if (!st->dready_trig) { > + dev_err(&client->dev, "failed to allocate trigger"); > + return -ENOMEM; > + } > + > + ret = devm_request_threaded_irq(&client->dev, client->irq, > + dlh_i2c_interrupt, NULL, > + IRQF_TRIGGER_RISING | IRQF_ONESHOT, > + id->name, indio_dev); > + if (ret) { > + dev_err(&client->dev, "failed to allocate threaded irq"); > + return ret; > + } > + > + st->dready_trig->dev.parent = &client->dev; > + st->dready_trig->ops = &dlh_i2c_trigger_ops; > + iio_trigger_set_drvdata(st->dready_trig, indio_dev); > + indio_dev->trig = iio_trigger_get(st->dready_trig); > + > + init_completion(&st->completion); > + st->dready_trig_on = false; > + > + ret = devm_iio_trigger_register(&client->dev, st->dready_trig); > + if (ret) { > + dev_err(&client->dev, "failed to register trigger\n"); > + return ret; > + } > + } else { > + dev_info(&client->dev, > + "no irq, falling back to polling in direct mode\n"); > + } > + > + ret = devm_iio_triggered_buffer_setup(&client->dev, indio_dev, > + NULL, &dlh_i2c_trigger_handler, NULL); > + if (ret) { > + dev_err(&client->dev, "failed to setup iio buffer\n"); > + return ret; > + } > + > + ret = devm_iio_device_register(&client->dev, indio_dev); > + if (ret) > + dev_err(&client->dev, "failed to register iio device\n"); > + > + return ret; > +} > + > +static const struct of_device_id dlh_i2c_of_match[] = { > + { .compatible = "asc,dlhl60d" }, > + { .compatible = "asc,dlhl60g" }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, dlh_i2c_of_match); > + > +static const struct i2c_device_id dlh_i2c_id[] = { > + { "dlhl60d", dlhl60d }, > + { "dlhl60g", dlhl60g }, > + {} > +}; > +MODULE_DEVICE_TABLE(i2c, dlh_i2c_id); > + > +static struct i2c_driver dlh_i2c_driver = { > + .driver = { > + .name = "dlh_i2c", > + .of_match_table = dlh_i2c_of_match, > + }, > + .probe = dlh_i2c_probe, > + .id_table = dlh_i2c_id, > +}; > +module_i2c_driver(dlh_i2c_driver); > + > +MODULE_AUTHOR("Tomislav Denis <tomislav.denis@avl.com>"); > +MODULE_DESCRIPTION("Driver for All Sensors DLH series pressure sensors"); > +MODULE_LICENSE("GPL v2"); ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 2/3] dt-bindings: Add asc vendor 2019-11-29 7:34 ` [PATCH v2 0/3] Add support for DLH pressure sensors tomislav.denis 2019-11-29 7:34 ` [PATCH v2 1/3] iio: pressure: Add driver " tomislav.denis @ 2019-11-29 7:34 ` tomislav.denis 2019-12-01 10:58 ` Jonathan Cameron 2019-11-29 7:34 ` [PATCH v2 3/3] bindings: iio: pressure: Add dlh-i2c documentation tomislav.denis 2019-12-01 10:27 ` [PATCH v2 0/3] Add support for DLH pressure sensors Jonathan Cameron 3 siblings, 1 reply; 16+ messages in thread From: tomislav.denis @ 2019-11-29 7:34 UTC (permalink / raw) To: jic23; +Cc: linux-iio, devicetree, tomislav.denis From: Tomislav Denis <tomislav.denis@avl.com> All Sensors Corporation is a manufacturer of MEMS piezoresitive ultra low pressure sensors and pressure transducers. Signed-off-by: Tomislav Denis <tomislav.denis@avl.com> --- Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml index 967e78c..88247b3 100644 --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml @@ -109,6 +109,8 @@ patternProperties: description: Artesyn Embedded Technologies Inc. "^asahi-kasei,.*": description: Asahi Kasei Corp. + "^asc,.*": + description: All Sensors Corporation "^aspeed,.*": description: ASPEED Technology Inc. "^asus,.*": -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/3] dt-bindings: Add asc vendor 2019-11-29 7:34 ` [PATCH v2 2/3] dt-bindings: Add asc vendor tomislav.denis @ 2019-12-01 10:58 ` Jonathan Cameron 0 siblings, 0 replies; 16+ messages in thread From: Jonathan Cameron @ 2019-12-01 10:58 UTC (permalink / raw) To: tomislav.denis; +Cc: linux-iio, devicetree On Fri, 29 Nov 2019 08:34:19 +0100 <tomislav.denis@avl.com> wrote: > From: Tomislav Denis <tomislav.denis@avl.com> > > All Sensors Corporation is a manufacturer of MEMS piezoresitive > ultra low pressure sensors and pressure transducers. > > Signed-off-by: Tomislav Denis <tomislav.denis@avl.com> Please make sure to pick up an Acks from earlier versions. Only drop them if you make substantial changes. Rob acked this one. Thanks, Jonathan > --- > Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml > index 967e78c..88247b3 100644 > --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml > +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml > @@ -109,6 +109,8 @@ patternProperties: > description: Artesyn Embedded Technologies Inc. > "^asahi-kasei,.*": > description: Asahi Kasei Corp. > + "^asc,.*": > + description: All Sensors Corporation > "^aspeed,.*": > description: ASPEED Technology Inc. > "^asus,.*": ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 3/3] bindings: iio: pressure: Add dlh-i2c documentation 2019-11-29 7:34 ` [PATCH v2 0/3] Add support for DLH pressure sensors tomislav.denis 2019-11-29 7:34 ` [PATCH v2 1/3] iio: pressure: Add driver " tomislav.denis 2019-11-29 7:34 ` [PATCH v2 2/3] dt-bindings: Add asc vendor tomislav.denis @ 2019-11-29 7:34 ` tomislav.denis 2019-12-01 10:58 ` Jonathan Cameron 2019-12-01 10:27 ` [PATCH v2 0/3] Add support for DLH pressure sensors Jonathan Cameron 3 siblings, 1 reply; 16+ messages in thread From: tomislav.denis @ 2019-11-29 7:34 UTC (permalink / raw) To: jic23; +Cc: linux-iio, devicetree, tomislav.denis From: Tomislav Denis <tomislav.denis@avl.com> Add a device tree binding documentation for DLH series pressure sensors. Signed-off-by: Tomislav Denis <tomislav.denis@avl.com> --- .../bindings/iio/pressure/asc,dlh-i2c.yaml | 51 ++++++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 52 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/pressure/asc,dlh-i2c.yaml diff --git a/Documentation/devicetree/bindings/iio/pressure/asc,dlh-i2c.yaml b/Documentation/devicetree/bindings/iio/pressure/asc,dlh-i2c.yaml new file mode 100644 index 0000000..5de2277 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/pressure/asc,dlh-i2c.yaml @@ -0,0 +1,51 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/pressure/dlh-i2c.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: All Sensors DLH series low voltage digital pressure sensors + +maintainers: + - Tomislav Denis <tomislav.denis@avl.com> + +description: | + Bindings for the All Sensors DLH series pressure sensors. + + Specifications about the sensors can be found at: + http://www.allsensors.com/cad/DS-0355_Rev_B.PDF + +properties: + compatible: + enum: + - asc,dlhl60d + - asc,dlhl60g + + reg: + description: I2C device address + maxItems: 1 + + interrupts: + description: interrupt mapping for EOC(data ready) pin + maxItems: 1 + +required: + - compatible + - reg + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + + i2c0 { + #address-cells = <1>; + #size-cells = <0>; + + pressure@29 { + compatible = "asc,dlhl60d"; + reg = <0x29>; + interrupt-parent = <&gpio0>; + interrupts = <10 IRQ_TYPE_EDGE_RISING>; + }; + }; +... diff --git a/MAINTAINERS b/MAINTAINERS index 39d6f0f..8f0eab0 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -674,6 +674,7 @@ W: http://www.allsensors.com/ S: Maintained L: linux-iio@vger.kernel.org F: drivers/iio/pressure/dlh-i2c.c +F: Documentation/devicetree/bindings/iio/pressure/dlh-i2c.yaml ALLEGRO DVT VIDEO IP CORE DRIVER M: Michael Tretter <m.tretter@pengutronix.de> -- 2.7.4 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/3] bindings: iio: pressure: Add dlh-i2c documentation 2019-11-29 7:34 ` [PATCH v2 3/3] bindings: iio: pressure: Add dlh-i2c documentation tomislav.denis @ 2019-12-01 10:58 ` Jonathan Cameron 0 siblings, 0 replies; 16+ messages in thread From: Jonathan Cameron @ 2019-12-01 10:58 UTC (permalink / raw) To: tomislav.denis; +Cc: linux-iio, devicetree On Fri, 29 Nov 2019 08:34:20 +0100 <tomislav.denis@avl.com> wrote: > From: Tomislav Denis <tomislav.denis@avl.com> > > Add a device tree binding documentation for DLH series pressure > sensors. > > Signed-off-by: Tomislav Denis <tomislav.denis@avl.com> Looks good to me, but I'll leave some time for others to take a look. > --- > .../bindings/iio/pressure/asc,dlh-i2c.yaml | 51 ++++++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 52 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/pressure/asc,dlh-i2c.yaml > > diff --git a/Documentation/devicetree/bindings/iio/pressure/asc,dlh-i2c.yaml b/Documentation/devicetree/bindings/iio/pressure/asc,dlh-i2c.yaml > new file mode 100644 > index 0000000..5de2277 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/pressure/asc,dlh-i2c.yaml > @@ -0,0 +1,51 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/pressure/dlh-i2c.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: All Sensors DLH series low voltage digital pressure sensors > + > +maintainers: > + - Tomislav Denis <tomislav.denis@avl.com> > + > +description: | > + Bindings for the All Sensors DLH series pressure sensors. > + > + Specifications about the sensors can be found at: > + http://www.allsensors.com/cad/DS-0355_Rev_B.PDF > + > +properties: > + compatible: > + enum: > + - asc,dlhl60d > + - asc,dlhl60g > + > + reg: > + description: I2C device address > + maxItems: 1 > + > + interrupts: > + description: interrupt mapping for EOC(data ready) pin > + maxItems: 1 > + > +required: > + - compatible > + - reg > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/irq.h> > + > + i2c0 { > + #address-cells = <1>; > + #size-cells = <0>; > + > + pressure@29 { > + compatible = "asc,dlhl60d"; > + reg = <0x29>; > + interrupt-parent = <&gpio0>; > + interrupts = <10 IRQ_TYPE_EDGE_RISING>; > + }; > + }; > +... > diff --git a/MAINTAINERS b/MAINTAINERS > index 39d6f0f..8f0eab0 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -674,6 +674,7 @@ W: http://www.allsensors.com/ > S: Maintained > L: linux-iio@vger.kernel.org > F: drivers/iio/pressure/dlh-i2c.c > +F: Documentation/devicetree/bindings/iio/pressure/dlh-i2c.yaml > > ALLEGRO DVT VIDEO IP CORE DRIVER > M: Michael Tretter <m.tretter@pengutronix.de> ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 0/3] Add support for DLH pressure sensors 2019-11-29 7:34 ` [PATCH v2 0/3] Add support for DLH pressure sensors tomislav.denis ` (2 preceding siblings ...) 2019-11-29 7:34 ` [PATCH v2 3/3] bindings: iio: pressure: Add dlh-i2c documentation tomislav.denis @ 2019-12-01 10:27 ` Jonathan Cameron 3 siblings, 0 replies; 16+ messages in thread From: Jonathan Cameron @ 2019-12-01 10:27 UTC (permalink / raw) To: tomislav.denis; +Cc: linux-iio, devicetree On Fri, 29 Nov 2019 08:34:17 +0100 <tomislav.denis@avl.com> wrote: > From: Tomislav Denis <tomislav.denis@avl.com> > > This patchset adds support for All Sensors DLH series low > voltage digital pressure sensors. > > Datasheet: https://www.allsensors.com/cad/DS-0355_Rev_B.PDF One generic comment before I look at the code. Mostly (there may be exceptions) it is better to post a new version as a separate email thread. This avoid deep and extremely confusing threads when we run into many versions. Also, a lot of maintainers have limited time so start at their most recent emails on the basis that earlier discussions have often already been resolved by other reviewers / contributors. If you send in a reply to an older thread you reduce the chance of anyone looking at it ;) Thanks, Jonathan > > Changes in v2: > - web page link in the MAINTAINERS file fixed > - adjust the units of the output to the IIO ABI > - unneceseary default case removed > - define the channel member of the iio_chan_spec > struct for channels specification > - remove explicit cast for pointers of type void * > - add support for the EOC(data ready) pin > - drop the unneceseary return ret; > - rename dlh-i2c.yaml to asc,dlh-i2c.yaml > - change the bindings copyright to GPL-2.0-only OR BSD-2-Clause > - document EOC(data ready) pin > > Tomislav Denis (3): > iio: pressure: Add driver for DLH pressure sensors > dt-bindings: Add asc vendor > bindings: iio: pressure: Add dlh-i2c documentation > > .../bindings/iio/pressure/asc,dlh-i2c.yaml | 51 +++ > .../devicetree/bindings/vendor-prefixes.yaml | 2 + > MAINTAINERS | 8 + > drivers/iio/pressure/Kconfig | 12 + > drivers/iio/pressure/Makefile | 1 + > drivers/iio/pressure/dlh-i2c.c | 429 +++++++++++++++++++++ > 6 files changed, 503 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/pressure/asc,dlh-i2c.yaml > create mode 100644 drivers/iio/pressure/dlh-i2c.c > ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-12-01 10:58 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-14 10:09 [PATCH 0/3] Add support for DLH pressure sensors tomislav.denis 2019-11-14 10:09 ` [PATCH 1/3] iio: pressure: Add driver " tomislav.denis 2019-11-14 14:35 ` Jonathan Cameron 2019-11-15 7:09 ` Denis, Tomislav AVL DiTEST 2019-11-14 10:09 ` [PATCH 2/3] dt-bindings: Add asc vendor tomislav.denis 2019-11-18 22:16 ` Rob Herring 2019-11-14 10:09 ` [PATCH 3/3] bindings: iio: pressure: Add dlh-i2c documentation tomislav.denis 2019-11-14 14:12 ` Jonathan Cameron 2019-11-29 7:34 ` [PATCH v2 0/3] Add support for DLH pressure sensors tomislav.denis 2019-11-29 7:34 ` [PATCH v2 1/3] iio: pressure: Add driver " tomislav.denis 2019-12-01 10:57 ` Jonathan Cameron 2019-11-29 7:34 ` [PATCH v2 2/3] dt-bindings: Add asc vendor tomislav.denis 2019-12-01 10:58 ` Jonathan Cameron 2019-11-29 7:34 ` [PATCH v2 3/3] bindings: iio: pressure: Add dlh-i2c documentation tomislav.denis 2019-12-01 10:58 ` Jonathan Cameron 2019-12-01 10:27 ` [PATCH v2 0/3] Add support for DLH pressure sensors Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).