On Thu, 16 Jun 2022 12:00:06 +0200 Jacopo Mondi <jacopo+renesas@xxxxxxxxxx> wrote: > From: Jacopo Mondi <jacopo@xxxxxxxxxx> > > Add support for CEVA BNO08x Sensor Hub. > > The BNO08X family (BNO080/85/86) is a System in Package (SiP) that > integrates a triaxial accelerometer, triaxial gyroscope, magnetometer > and a 32-bit ARM Cortex-M0+ microcontroller running CEVA's SH-2 > firmware. > > Datasheet: > https://www.ceva-dsp.com/wp-content/uploads/2019/10/BNO080_085-Datasheet.pdf > > Signed-off-by: Jacopo Mondi <jacopo@xxxxxxxxxx> Hi Jacopo Review for whoever takes this forwards in the future :) It's proving hard enough to get drivers that have been in staging for years out, don't want to add new ones! Jonathan > --- > MAINTAINERS | 7 + > drivers/staging/iio/Kconfig | 1 + > drivers/staging/iio/Makefile | 1 + > drivers/staging/iio/imu/bno08x/Kconfig | 11 + > drivers/staging/iio/imu/bno08x/Makefile | 3 + > drivers/staging/iio/imu/bno08x/bno08x.c | 618 ++++++++++++++++++++++++ > 6 files changed, 641 insertions(+) > create mode 100644 drivers/staging/iio/imu/bno08x/Kconfig > create mode 100644 drivers/staging/iio/imu/bno08x/Makefile > create mode 100644 drivers/staging/iio/imu/bno08x/bno08x.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index 96b8c3560117..4b709599c82b 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4314,6 +4314,13 @@ F: certs/ > F: scripts/extract-cert.c > F: scripts/sign-file.c > > +CEVA BNO08x SENSOR HUB > +ML Jacopo Mondi <jacopo@xxxxxxxxxx> > +L: linux-iio@xxxxxxxxxxxxxxx > +S: Odd Fixes > +F: Documentation/devicetree/bindings/staging/iio/imu/ceva,bno08x.yaml > +F drivers/staging/iio/imu/bno08x/ > + > CFAG12864B LCD DRIVER > M: Miguel Ojeda <ojeda@xxxxxxxxxx> > S: Maintained > diff --git a/drivers/staging/iio/Kconfig b/drivers/staging/iio/Kconfig > index a8e970db179d..bcecace954a5 100644 > --- a/drivers/staging/iio/Kconfig > +++ b/drivers/staging/iio/Kconfig > @@ -11,6 +11,7 @@ source "drivers/staging/iio/addac/Kconfig" > source "drivers/staging/iio/cdc/Kconfig" > source "drivers/staging/iio/frequency/Kconfig" > source "drivers/staging/iio/impedance-analyzer/Kconfig" > +source "drivers/staging/iio/imu/Kconfig" > source "drivers/staging/iio/meter/Kconfig" > source "drivers/staging/iio/resolver/Kconfig" > > diff --git a/drivers/staging/iio/Makefile b/drivers/staging/iio/Makefile > index b15904b99581..eafcc25b64f9 100644 > --- a/drivers/staging/iio/Makefile > +++ b/drivers/staging/iio/Makefile > @@ -9,5 +9,6 @@ obj-y += addac/ > obj-y += cdc/ > obj-y += frequency/ > obj-y += impedance-analyzer/ > +obj-y += imu/ > obj-y += meter/ > obj-y += resolver/ > diff --git a/drivers/staging/iio/imu/bno08x/Kconfig b/drivers/staging/iio/imu/bno08x/Kconfig > new file mode 100644 > index 000000000000..109ac11ed4f9 > --- /dev/null > +++ b/drivers/staging/iio/imu/bno08x/Kconfig > @@ -0,0 +1,11 @@ > +# SPDX-License-Identifier: GPL-2.0-only > + > +config CEVA_BNO08x_IMU > + tristate "CEVA BNO08x IMU driver" > + depends on I2C > + select REGMAP_I2C > + help > + Say yes here to build support for CEVA BNO08x IMU. > + > + To compile this driver as a module, choose M here: the module will be > + called bno08x. > diff --git a/drivers/staging/iio/imu/bno08x/Makefile b/drivers/staging/iio/imu/bno08x/Makefile > new file mode 100644 > index 000000000000..48bbbf12a366 > --- /dev/null > +++ b/drivers/staging/iio/imu/bno08x/Makefile > @@ -0,0 +1,3 @@ > +# SPDX-License-Identifier: GPL-2.0 > + > +obj-$(CONFIG_CEVA_BNO08x_IMU) += bno08x.o > diff --git a/drivers/staging/iio/imu/bno08x/bno08x.c b/drivers/staging/iio/imu/bno08x/bno08x.c > new file mode 100644 > index 000000000000..e07735a85b34 > --- /dev/null > +++ b/drivers/staging/iio/imu/bno08x/bno08x.c > @@ -0,0 +1,618 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * CEVA BNO08x IMU driver. > + * > + * Copyright (c) 2021 Jacopo Mondi <jacopo@xxxxxxxxxx> > + */ > + > +#include <linux/atomic.h> > +#include <linux/completion.h> > +#include <linux/delay.h> > +#include <linux/i2c.h> > +#include <linux/mutex.h> > +#include <linux/interrupt.h> > +#include <linux/jiffies.h> > +#include <linux/mod_devicetable.h> > +#include <linux/module.h> > + > +#include <linux/iio/iio.h> > +#include <linux/iio/triggered_buffer.h> > +#include <linux/iio/trigger_consumer.h> > +#include <linux/iio/buffer.h> > +#include <linux/iio/sysfs.h> > +#include <linux/iio/trigger.h> > + > +#define DRIVER_NAME "bno08x" > + > +#define BNO08x_SHTP_HDR_SIZE 0x4 > +#define BNO08x_SHTP_HDR_LEN_LSB 0x0 > +#define BNO08x_SHTP_HDR_LEN_MSB 0x1 > +#define BNO08x_SHTP_HDR_CHAN 0x2 > +#define BNO08x_SHTP_HDR_SEQ 0x3 > + > +#define BNO08x_SHTP_MAX_SIZE 128 > +#define BNO08x_SHTP_ADV_MAX_SIZE 512 > + > +#define BNO08x_SHTP_SOFT_RESET 0x01 > +#define BNO08x_SHTP_PROD_ID 0xf8 > +#define BNO08x_SHTP_PROD_ID_REQ 0xf9 > + > +#define BNO08x_SHTP_GET_FEATURE_RESP 0xfc > +#define BNO08x_SHTP_SET_FEATURE_CMD 0xfd > +#define BNO08x_SHTP_GET_FEATURE_REQ 0xfe > + > +#define BNO08x_ROT_SCAN_INDEX 0x01 > +#define BNO08x_REPORTID_ROTATION_VEC 0x05 > +#define BNO08x_REPORTID_TIMESTAMP_BASE 0xfb > + > +enum bno08x_shtp_channels { > + BNO08x_SHTP_COMMAND_CHAN, > + BNO08x_SHTP_EXECTUABLE_CHAN, > + BNO08x_SHTP_CONTROL_CHAN, > + BNO08x_SHTP_REPORTS_CHAN, > + BNO08x_SHTP_WAKE_REPORTS_CHAN, > + BNO08x_SHTP_GYRO_CHAN, > + BNO08x_NUM_CHANNELS, > +}; > + > +struct bno08x_dev { > + struct i2c_client *client; > + > + struct iio_trigger *trig; > + > + /* Completions for cargo transfer. */ > + struct completion cargo_ready; > + struct completion waiters_done; > + > + /* cargo sequence number per channel. */ > + u32 seq_num[BNO08x_NUM_CHANNELS]; > + > + /* Mask of the enabled report ids. */ > + u64 enabled_reports_mask; > + > +#define BNO08x_CARGO_BUFFER_SIZE 1024lu > + struct { > + u8 buffer[BNO08x_CARGO_BUFFER_SIZE]; > + size_t len; > + atomic_t waiters; > + > + /* Protects access to the cargo buffer content. */ > + struct mutex mutex; > + } cargo; > +}; > + > +static irqreturn_t bno08x_interrupt(int irq, void *dev_id) > +{ > + struct iio_dev *iio_dev = dev_id; > + > + if (iio_dev->trig) > + iio_trigger_poll((void *)iio_dev->trig); Trigger isn't guaranteed to be the one you think it is :) also with an interrupt doing other things, seems unlikely you can always assume trigger fired... > + > + return IRQ_WAKE_THREAD; > +} > + > +static irqreturn_t bno08x_dump_cargo(int irq, void *dev_id) > +{ > + struct iio_dev *iio_dev = dev_id; > + struct bno08x_dev *bno08x = iio_priv(iio_dev); > + struct i2c_client *client = bno08x->client; > + u8 *cargo = bno08x->cargo.buffer; > + size_t to_read; > + size_t len; > + int ret; > + > + mutex_lock(&bno08x->cargo.mutex); > + > + /* Read only the header first to know how many bytes we expect to receive. */ > + ret = i2c_master_recv(client, bno08x->cargo.buffer, BNO08x_SHTP_HDR_SIZE); > + if (ret != BNO08x_SHTP_HDR_SIZE) > + goto out_unlock; > + > + /* > + * Clear the top bit: it means a cargo is a continuation of the last one. > + * Ignore it for now. > + */ > + len = (cargo[BNO08x_SHTP_HDR_LEN_MSB] << 8 | > + cargo[BNO08x_SHTP_HDR_LEN_LSB]) & ~BIT(15); FIELD_GET() with appropriate mask and an unaligned_get_le16 probably (assuming the LSB is next to the MSB) > + > + if (len == 0) > + goto out_complete; > + > + if (len > BNO08x_CARGO_BUFFER_SIZE) > + dev_warn(&client->dev, > + "Cargo size exceeds buffer: content will be unusable\n"); > + > + /* > + * Read the full cargo now that we know its length. > + * > + * If the reported length exceeds the max transfer size, read the cargo > + * in chunks. Its content will be unusable though. > + */ > + to_read = len; > + len = min(len, BNO08x_CARGO_BUFFER_SIZE); > + while (to_read) { > + memset(bno08x->cargo.buffer, 0, len); > + > + ret = i2c_master_recv(client, bno08x->cargo.buffer, len); > + if (ret != len) { > + dev_err(&client->dev, > + "Failed to read cargo of size %lu: %d\n", > + len, ret); > + goto out_complete; > + } > + > + to_read -= len; > + } > + > + bno08x->cargo.len = len; > + > +out_complete: > + if (atomic_read(&bno08x->cargo.waiters) == 0) > + goto out_unlock; > + > + /* > + * Wake up all waiters and let them read the cargo buffer. > + * > + * Unlock the cargo mutex only after all waiters are done, to avoid > + * this interrupt handler re-writing the buffer content with the next > + * cargo before waiters have read it. > + */ > + complete_all(&bno08x->cargo_ready); > + > + wait_for_completion(&bno08x->waiters_done); > + reinit_completion(&bno08x->waiters_done); > + > +out_unlock: > + mutex_unlock(&bno08x->cargo.mutex); > + > + return IRQ_HANDLED; > +} > + > +static int bno08x_write_cargo(struct bno08x_dev *bno08x, > + enum bno08x_shtp_channels channel, > + u8 *cargo, u16 length) > +{ > + u16 cargo_length = length + BNO08x_SHTP_HDR_SIZE; > + struct i2c_client *client = bno08x->client; > + int ret; > + > + /* Assemble header */ > + cargo[BNO08x_SHTP_HDR_LEN_LSB] = cargo_length & 0xff; > + cargo[BNO08x_SHTP_HDR_LEN_MSB] = cargo_length >> 8; unaligned_put_le16() > + cargo[BNO08x_SHTP_HDR_CHAN] = channel; > + cargo[BNO08x_SHTP_HDR_SEQ] = bno08x->seq_num[channel]++; > + > + ret = i2c_master_send(client, cargo, cargo_length); > + if (ret < 0) { > + dev_err(&client->dev, "Failed to write cargo to channel %u: %d\n", > + channel, ret); > + return ret; > + } > + > + return 0; > +} > + > +static int bno08x_wait_for_cargo_timeout(struct bno08x_dev *bno08x, u8 *cargo, > + size_t len) Wait for seems a bit confusing to me as I read it as waiting for a timeout which is a little odd :) bn08x_read_cargo_timeout() maybe? That's not great either thogu. > +{ > + static const unsigned long complete_timeout = 500; /* msecs */ > + struct i2c_client *client = bno08x->client; > + int ret = 0; > + > + atomic_inc(&bno08x->cargo.waiters); > + > + if (!wait_for_completion_timeout(&bno08x->cargo_ready, > + msecs_to_jiffies(complete_timeout))) { > + dev_dbg(&client->dev, "Wait for chip interrupt timeout.\n"); > + ret = -EIO; > + goto out_unlock; > + } > + > + /* The caller is not interested in the data. */ > + if (!len) > + goto out_unlock; > + > + /* > + * Multiple readers might want to inspect the cargo content in response > + * to the complete_all(cargo_ready) signal. > + * > + * Copy data into the caller buffer to allow multiple threads to > + * access the cargo content independently. > + */ > + ret = min(len, bno08x->cargo.len); > + memcpy(cargo, bno08x->cargo.buffer, ret); > + > +out_unlock: > + /* The last waiter unlocks the cargo read routine. */ > + if (atomic_dec_return(&bno08x->cargo.waiters) == 0) { > + reinit_completion(&bno08x->cargo_ready); > + complete(&bno08x->waiters_done); > + } > + > + return ret; > +} > + > +static const struct iio_chan_spec bno08x_iio_channels[] = { > + { > + .type = IIO_ROT, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > + .scan_index = BNO08x_ROT_SCAN_INDEX, > + .scan_type = { > + .sign = 'u', > + .realbits = 16, > + .storagebits = 16, > + .shift = 0, > + .endianness = IIO_LE, > + }, > + }, > +}; > + > +static int bno08x_enable_feature_report(struct bno08x_dev *bno08x, u8 report_id) > +{ > + struct i2c_client *client = bno08x->client; > + /* Reporting interval hardcoded to 5 msec. */ > + unsigned int report_interval_usec = 5000; > + u8 cargo[BNO08x_SHTP_MAX_SIZE] = {}; > + u8 *data = cargo + BNO08x_SHTP_HDR_SIZE; > + unsigned int max_attempts = 50; > + int ret; > + > + if (bno08x->enabled_reports_mask & BIT(report_id)) > + return 0; > + > + /* > + * Enable reporting the requested feature. The full "feature set > + * request" package size is 17 bytes. > + */ > + data[0] = BNO08x_SHTP_SET_FEATURE_CMD; > + data[1] = report_id; > + > + data[5] = (report_interval_usec >> 0); > + data[6] = (report_interval_usec >> 8); > + data[7] = (report_interval_usec >> 16); > + data[8] = (report_interval_usec >> 24); put_unaligned_le32() > + > + ret = bno08x_write_cargo(bno08x, BNO08x_SHTP_CONTROL_CHAN, cargo, 17); > + if (ret) > + return ret; > + > + /* > + * Verify the requested feature is enabled inspecting the > + * 'feature request status" response cargo. > + */ > + memset(cargo, 0, BNO08x_SHTP_MAX_SIZE); > + data[0] = BNO08x_SHTP_GET_FEATURE_REQ; > + data[1] = report_id; > + > + ret = bno08x_write_cargo(bno08x, BNO08x_SHTP_CONTROL_CHAN, cargo, 2); > + if (ret) > + return ret; > + > + /* > + * Wait for a "get feature response". The datasheet says it will "arrive > + * some time in the future". Read up to 10 packets then give up. > + */ > + while (--max_attempts) { > + memset(cargo, 0, 5); > + ret = bno08x_wait_for_cargo_timeout(bno08x, cargo, 5); > + if (ret < 0) > + continue; > + > + if (ret < 5) > + continue; > + > + if (data[0] == BNO08x_SHTP_GET_FEATURE_RESP || > + data[1] == report_id) > + break; > + } > + if (!max_attempts) { > + dev_err(&client->dev, > + "Failed to parse GET_FEATURE_RESPONSE: bad header\n"); > + return -EINVAL; > + } > + > + bno08x->enabled_reports_mask |= BIT(report_id); > + dev_dbg(&client->dev, "Reporting of feature %x enabled!\n", report_id); > + > + return 0; > +} > + > +static int bno08x_read_raw_rotation(struct bno08x_dev *bno08x, int *vals, > + int *val_len, int max_len) > +{ > + struct i2c_client *client = bno08x->client; > + unsigned int max_attempts = 50; > + u8 cargo[24]; > + int ret; > + > + if (max_len < 3) > + return -EINVAL; > + > + ret = bno08x_enable_feature_report(bno08x, BNO08x_REPORTID_ROTATION_VEC); > + if (ret) > + return ret; > + > + /* > + * Read the rotation vector in I, J and K quaternions. > + * > + * We're going to receive several other cargos before an actual rotation > + * vector input report, so we parse the cargo fields and only proceed > + * with a cargo that refers to the rotation vector report id. > + * > + * Use a sentinel to avoid looping forever, unfortunately we can't > + * really know how many cargos we have to discard before receiving the > + * 'right' one. > + */ > + while (--max_attempts) { > + ret = bno08x_wait_for_cargo_timeout(bno08x, cargo, 24); > + if (ret < 0) > + continue; > + > + if (ret != 24) > + continue; > + > + /* > + * Parse the cargo content to make sure it's a well-formed input > + * report containing rotation vector information. > + * > + * The layout of an input report cargo is reported by Figure 5.2 > + * of "BNO080/85/86 Datasheet 1000-3927 v.1.11". > + * > + * The rotation vector input report is 24 bytes long. > + */ > + if (cargo[2] == BNO08x_SHTP_REPORTS_CHAN && > + cargo[4] == BNO08x_REPORTID_TIMESTAMP_BASE && > + cargo[9] == BNO08x_REPORTID_ROTATION_VEC) > + break; > + } > + if (!max_attempts) { > + dev_err(&client->dev, "Failed to receive the correct input report\n"); > + return -EINVAL; > + } > + > + /* Read the I, J, K quaternions. */ > + vals[0] = cargo[13] | (cargo[14] << 8); Use get_unaligned_le16() > + vals[1] = cargo[15] | (cargo[16] << 8); > + vals[2] = cargo[17] | (cargo[18] << 8); > + *val_len = 3; > + > + return IIO_VAL_INT_MULTIPLE; > +} > + > +static int bno08x_read_raw_multi(struct iio_dev *indio_dev, > + struct iio_chan_spec const *chan, int max_len, > + int *vals, int *val_len, long mask) > +{ > + struct bno08x_dev *bno08x = iio_priv(indio_dev); > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + return bno08x_read_raw_rotation(bno08x, vals, val_len, max_len); > + > + case IIO_CHAN_INFO_SCALE: > + vals[0] = 0; > + return IIO_VAL_INT; > + > + default: > + return -EINVAL; > + } > + > + return 0; > +} > + > +static const struct iio_info bno08x_iio_info = { > + .read_raw_multi = bno08x_read_raw_multi, > +}; > + > +static int bno08x_data_rdy_trigger_set_state(struct iio_trigger *trig, > + bool enable) > +{ > + struct iio_dev *iio_dev = iio_trigger_get_drvdata(trig); > + struct bno08x_dev *bno08x = iio_priv(iio_dev); > + int ret; > + > + if (!enable) > + return 0; > + > + ret = bno08x_enable_feature_report(bno08x, BNO08x_REPORTID_ROTATION_VEC); > + if (ret) > + return ret; > + > + return 0; return bn08x_enable... > +} > + > +static const struct iio_trigger_ops bno08x_trigger_ops = { > + .set_trigger_state = &bno08x_data_rdy_trigger_set_state, > +}; > + > +static irqreturn_t bno08x_trigger_handler(int irq, void *p) > +{ > + struct iio_poll_func *pf = p; > + struct iio_dev *iio_dev = pf->indio_dev; > + struct bno08x_dev *bno08x = iio_priv(iio_dev); > + u8 cargo[BNO08x_CARGO_BUFFER_SIZE]; > + int ret; > + > + ret = bno08x_wait_for_cargo_timeout(bno08x, cargo, BNO08x_CARGO_BUFFER_SIZE); > + if (ret < 0) > + goto done; > + I'm lazy - if it's less than 0 it's definitely less than 24 so no need for separate paths unless you add some dev_dbg/err or similar which might make sense here. > + if (ret < 24) > + goto done; > + > + /* Make sure the cargo matches the active scan channel. */ > + if (*iio_dev->active_scan_mask & BIT(BNO08x_ROT_SCAN_INDEX) && > + cargo[2] == BNO08x_SHTP_REPORTS_CHAN && > + cargo[4] == BNO08x_REPORTID_TIMESTAMP_BASE && > + cargo[9] == BNO08x_REPORTID_ROTATION_VEC) { > + u16 data; > + > + /* Unit quaternion I component. */ > + data = le16_to_cpu(cargo[13]); > + iio_push_to_buffers_with_timestamp(iio_dev, &data, pf->timestamp); If it's a single scan with multiple components, you need to build the entire scan up here, then push it to the buffer in one go. > + > + /* Unit quaternion J component. */ > + data = le16_to_cpu(cargo[15]); > + iio_push_to_buffers_with_timestamp(iio_dev, &data, pf->timestamp); > + > + /* Unit quaternion K component. */ > + data = le16_to_cpu(cargo[17]); > + iio_push_to_buffers_with_timestamp(iio_dev, &data, pf->timestamp); > + } > + > +done: > + iio_trigger_notify_done(iio_dev->trig); > + > + return IRQ_HANDLED; > +} > + > +static int bno08x_check_prod_id(struct bno08x_dev *bno08x) > +{ > + struct i2c_client *client = bno08x->client; > + u8 cargo[BNO08x_SHTP_MAX_SIZE] = {}; > + u8 *data = cargo + BNO08x_SHTP_HDR_SIZE; > + int ret; > + > + data[0] = BNO08x_SHTP_PROD_ID_REQ; > + ret = bno08x_write_cargo(bno08x, BNO08x_SHTP_CONTROL_CHAN, cargo, 2); > + if (ret) > + return ret; > + > + while (true) { > + ret = bno08x_wait_for_cargo_timeout(bno08x, cargo, 20); Why use MAX_SIZE for sizing then 20 here? > + if (ret < 0) { > + dev_dbg(&client->dev, "Failed to read PROD_ID: %d\n", ret); > + return ret; > + } > + > + if (ret >= 20 && data[0] == BNO08x_SHTP_PROD_ID) > + break; > + } > + > + dev_dbg(&client->dev, "FW version: 0x%x.%x\n", data[2], data[3]); This is sometimes useful info. I'd put it out with dev_info() but up to you. > + > + return 0; > +} > + > +static int bno08x_soft_reset(struct bno08x_dev *bno08x) > +{ > + struct i2c_client *client = bno08x->client; > + u8 cargo[20] = {}; > + int ret; > + > + /* Soft reset the device: write 0x01 to EXECUTABLE channel. */ > + cargo[BNO08x_SHTP_HDR_SIZE] = BNO08x_SHTP_SOFT_RESET; > + ret = bno08x_write_cargo(bno08x, BNO08x_SHTP_EXECTUABLE_CHAN, cargo, 1); > + if (ret) { > + dev_err(&client->dev, "Failed to soft-reset BNO08x\n"); > + return ret; > + } > + > + /* > + * Now read the all the channel advertisments packets. > + * We don't care about the content. > + */ > + do { > + ret = bno08x_wait_for_cargo_timeout(bno08x, cargo, 0); > + if (ret < 0) { > + dev_dbg(&client->dev, > + "Failed to read channel advertisments: %d\n", ret); Error path, I would go with dev_err() as it's fatal. > + return ret; > + } > + } while (ret != 0); > + > + /* > + * Give the chip some time to stabilize. There's no mention of any > + * delay required after startup in the manual, but this makes operation > + * stable through module load/unload. > + */ > + usleep_range(15000, 20000); > + > + return 0; > +} > + > +static int bno08x_probe(struct i2c_client *client) > +{ > + struct bno08x_dev *bno08x; > + struct iio_dev *iio_dev; > + int ret; > + > + iio_dev = devm_iio_device_alloc(&client->dev, sizeof(*bno08x)); > + if (!iio_dev) > + return -ENOMEM; > + > + dev_set_drvdata(&client->dev, iio_dev); I'm not immediately seeing a user - perhaps there isn't one and this should go away. > + > + bno08x = iio_priv(iio_dev); > + bno08x->client = client; > + > + mutex_init(&bno08x->cargo.mutex); > + init_completion(&bno08x->cargo_ready); > + init_completion(&bno08x->waiters_done); > + > + iio_dev->info = &bno08x_iio_info; > + iio_dev->name = DRIVER_NAME; > + iio_dev->channels = bno08x_iio_channels; > + iio_dev->num_channels = ARRAY_SIZE(bno08x_iio_channels); > + iio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_TRIGGERED; If triggered buffer is registered, this will be set by the core. Only DIRECT_MODE should be set in drivers. > + > + ret = devm_iio_triggered_buffer_setup(&client->dev, iio_dev, > + iio_pollfunc_store_time, > + bno08x_trigger_handler, NULL); > + if (ret) > + return ret; > + > + bno08x->trig = devm_iio_trigger_alloc(&client->dev, "%s-dev%d", > + DRIVER_NAME, > + iio_device_id(iio_dev)); > + if (!bno08x->trig) > + return -ENOMEM; > + > + if (client->irq > 0) { > + ret = devm_request_threaded_irq(&client->dev, client->irq, > + bno08x_interrupt, > + bno08x_dump_cargo, > + IRQF_TRIGGER_FALLING, Not in the driver, that's for firmware to specify. We have lots of old drivers doing this which we can't cleanly fix, but don't do it in a new one. > + iio_dev->name, iio_dev); Unusual to create/register trigger if no interrupts provided... Need some explanation. > + if (ret) > + return ret; > + } > + > + bno08x->trig->dev.parent = &client->dev; > + bno08x->trig->ops = &bno08x_trigger_ops; > + iio_trigger_set_drvdata(bno08x->trig, iio_dev); > + > + ret = devm_iio_trigger_register(&client->dev, bno08x->trig); > + if (ret) > + return ret; > + > + iio_dev->trig = iio_trigger_get(bno08x->trig); No validate callbacks, so I assume the device can use other triggers. If that's the case then don't set a default. Which one to use is a userspace policy decision. If it can only use this one then enforce it. > + > + ret = bno08x_soft_reset(bno08x); > + if (ret) > + return ret; > + > + bno08x_check_prod_id(bno08x); handle any errors returned. > + > + return devm_iio_device_register(&client->dev, iio_dev); > +} > + > +static const struct of_device_id bno08x_of_match[] = { > + { .compatible = "ceva,bno08x" }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, bno08x_of_match); > + > +static struct i2c_driver bno08x_driver = { > + .driver = { > + .name = DRIVER_NAME, > + .of_match_table = of_match_ptr(bno08x_of_match), > + }, > + .probe_new = bno08x_probe, > +}; > +module_i2c_driver(bno08x_driver); > + > +MODULE_AUTHOR("Jacopo Mondi <jacopo@xxxxxxxxxx>"); > +MODULE_DESCRIPTION("CEVA BNO08x IMU driver"); > +MODULE_LICENSE("GPL");
