From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: References: <20190313184002.2501-1-rui.silva@linaro.org> <20190313184002.2501-3-rui.silva@linaro.org> <20190316162959.226bf30f@archlinux> From: Rui Miguel Silva Subject: Re: [PATCH v3 2/6] iio: gyro: add core driver for fxas21002c In-reply-to: <20190316162959.226bf30f@archlinux> Date: Mon, 18 Mar 2019 08:29:41 +0000 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; format=flowed To: Jonathan Cameron Cc: Hartmut Knaack , Lars-Peter Clausen , Peter Meerwald-Stadler , Shawn Guo , Rob Herring , Fabio Estevam , linux-iio@vger.kernel.org, devicetree@vger.kernel.org List-ID: Hi Jonathan, On Sat 16 Mar 2019 at 16:29, Jonathan Cameron wrote: > On Wed, 13 Mar 2019 18:39:58 +0000 > Rui Miguel Silva wrote: > >> Add core support for the NXP fxas21002c Tri-axis gyroscope, >> using the iio subsystem. It supports PM operations, axis >> reading, >> temperature, scale factor of the axis, high pass and low pass >> filtering, and sampling frequency selection. >> >> It will have extras modules to support the communication over >> i2c and >> spi. >> >> Signed-off-by: Rui Miguel Silva > A few additional minor comments, but basically good. Great, thanks for the review, I will send v4 later. --- Cheers, Rui > >> --- >> drivers/iio/gyro/Kconfig | 11 + >> drivers/iio/gyro/Makefile | 1 + >> drivers/iio/gyro/fxas21002c.h | 151 +++++ >> drivers/iio/gyro/fxas21002c_core.c | 989 >> +++++++++++++++++++++++++++++ >> 4 files changed, 1152 insertions(+) >> create mode 100644 drivers/iio/gyro/fxas21002c.h >> create mode 100644 drivers/iio/gyro/fxas21002c_core.c >> >> diff --git a/drivers/iio/gyro/Kconfig >> b/drivers/iio/gyro/Kconfig >> index 3126cf05e6b9..cfd2cf44bac8 100644 >> --- a/drivers/iio/gyro/Kconfig >> +++ b/drivers/iio/gyro/Kconfig >> @@ -73,6 +73,17 @@ config BMG160_SPI >> tristate >> select REGMAP_SPI >> >> +config FXAS21002C >> + tristate "NXP FXAS21002C Gyro Sensor" >> + select IIO_BUFFER >> + select IIO_TRIGGERED_BUFFER >> + help >> + Say yes here to build support for NXP FXAS21002C >> Tri-axis Gyro >> + Sensor driver connected via I2C or SPI. >> + >> + This driver can also be built as a module. If so, >> the module >> + will be called fxas21002c_i2c or fxas21002c_spi. >> + >> config HID_SENSOR_GYRO_3D >> depends on HID_SENSOR_HUB >> select IIO_BUFFER >> diff --git a/drivers/iio/gyro/Makefile >> b/drivers/iio/gyro/Makefile >> index 295ec780c4eb..247dc600a602 100644 >> --- a/drivers/iio/gyro/Makefile >> +++ b/drivers/iio/gyro/Makefile >> @@ -12,6 +12,7 @@ obj-$(CONFIG_ADXRS450) += adxrs450.o >> obj-$(CONFIG_BMG160) += bmg160_core.o >> obj-$(CONFIG_BMG160_I2C) += bmg160_i2c.o >> obj-$(CONFIG_BMG160_SPI) += bmg160_spi.o >> +obj-$(CONFIG_FXAS21002C) += fxas21002c_core.o >> >> obj-$(CONFIG_HID_SENSOR_GYRO_3D) += hid-sensor-gyro-3d.o >> >> diff --git a/drivers/iio/gyro/fxas21002c.h >> b/drivers/iio/gyro/fxas21002c.h >> new file mode 100644 >> index 000000000000..e21fd410950c >> --- /dev/null >> +++ b/drivers/iio/gyro/fxas21002c.h >> @@ -0,0 +1,151 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> +/* >> + * Driver for NXP FXAS21002C Gyroscope - Header >> + * >> + * Copyright (C) 2019 Linaro Ltd. >> + * >> + */ >> + >> +#ifndef FXAS21002C_H_ >> +#define FXAS21002C_H_ >> + >> +#include >> + >> +#define FXAS21002C_REG_STATUS 0x00 >> +#define FXAS21002C_REG_OUT_X_MSB 0x01 >> +#define FXAS21002C_REG_OUT_X_LSB 0x02 >> +#define FXAS21002C_REG_OUT_Y_MSB 0x03 >> +#define FXAS21002C_REG_OUT_Y_LSB 0x04 >> +#define FXAS21002C_REG_OUT_Z_MSB 0x05 >> +#define FXAS21002C_REG_OUT_Z_LSB 0x06 >> +#define FXAS21002C_REG_DR_STATUS 0x07 >> +#define FXAS21002C_REG_F_STATUS 0x08 >> +#define FXAS21002C_REG_F_SETUP 0x09 >> +#define FXAS21002C_REG_F_EVENT 0x0A >> +#define FXAS21002C_REG_INT_SRC_FLAG 0x0B >> +#define FXAS21002C_REG_WHO_AM_I 0x0C >> +#define FXAS21002C_REG_CTRL0 0x0D >> +#define FXAS21002C_REG_RT_CFG 0x0E >> +#define FXAS21002C_REG_RT_SRC 0x0F >> +#define FXAS21002C_REG_RT_THS 0x10 >> +#define FXAS21002C_REG_RT_COUNT 0x11 >> +#define FXAS21002C_REG_TEMP 0x12 >> +#define FXAS21002C_REG_CTRL1 0x13 >> +#define FXAS21002C_REG_CTRL2 0x14 >> +#define FXAS21002C_REG_CTRL3 0x15 >> + >> +enum fxas21002c_fields { >> + F_DR_STATUS, >> + F_OUT_X_MSB, >> + F_OUT_X_LSB, >> + F_OUT_Y_MSB, >> + F_OUT_Y_LSB, >> + F_OUT_Z_MSB, >> + F_OUT_Z_LSB, >> + /* DR_STATUS */ >> + F_ZYX_OW, F_Z_OW, F_Y_OW, F_X_OW, F_ZYX_DR, F_Z_DR, >> F_Y_DR, F_X_DR, >> + /* F_STATUS */ >> + F_OVF, F_WMKF, F_CNT, >> + /* F_SETUP */ >> + F_MODE, F_WMRK, >> + /* F_EVENT */ >> + F_EVENT, FE_TIME, >> + /* INT_SOURCE_FLAG */ >> + F_BOOTEND, F_SRC_FIFO, F_SRC_RT, F_SRC_DRDY, >> + /* WHO_AM_I */ >> + F_WHO_AM_I, >> + /* CTRL_REG0 */ >> + F_BW, F_SPIW, F_SEL, F_HPF_EN, F_FS, >> + /* RT_CFG */ >> + F_ELE, F_ZTEFE, F_YTEFE, F_XTEFE, >> + /* RT_SRC */ >> + F_EA, F_ZRT, F_ZRT_POL, F_YRT, F_YRT_POL, F_XRT, >> F_XRT_POL, >> + /* RT_THS */ >> + F_DBCNTM, F_THS, >> + /* RT_COUNT */ >> + F_RT_COUNT, >> + /* TEMP */ >> + F_TEMP, >> + /* CTRL_REG1 */ >> + F_RST, F_ST, F_DR, F_ACTIVE, F_READY, >> + /* CTRL_REG2 */ >> + F_INT_CFG_FIFO, F_INT_EN_FIFO, F_INT_CFG_RT, F_INT_EN_RT, >> + F_INT_CFG_DRDY, F_INT_EN_DRDY, F_IPOL, F_PP_OD, >> + /* CTRL_REG3 */ >> + F_WRAPTOONE, F_EXTCTRLEN, F_FS_DOUBLE, >> + /* MAX FIELDS */ >> + F_MAX_FIELDS, >> +}; >> + >> +static const struct reg_field fxas21002c_reg_fields[] = { >> + [F_DR_STATUS] = REG_FIELD(FXAS21002C_REG_STATUS, >> 0, 7), >> + [F_OUT_X_MSB] = >> REG_FIELD(FXAS21002C_REG_OUT_X_MSB, 0, 7), >> + [F_OUT_X_LSB] = >> REG_FIELD(FXAS21002C_REG_OUT_X_LSB, 0, 7), >> + [F_OUT_Y_MSB] = >> REG_FIELD(FXAS21002C_REG_OUT_Y_MSB, 0, 7), >> + [F_OUT_Y_LSB] = >> REG_FIELD(FXAS21002C_REG_OUT_Y_LSB, 0, 7), >> + [F_OUT_Z_MSB] = >> REG_FIELD(FXAS21002C_REG_OUT_Z_MSB, 0, 7), >> + [F_OUT_Z_LSB] = >> REG_FIELD(FXAS21002C_REG_OUT_Z_LSB, 0, 7), >> + [F_ZYX_OW] = >> REG_FIELD(FXAS21002C_REG_DR_STATUS, 7, 7), >> + [F_Z_OW] = >> REG_FIELD(FXAS21002C_REG_DR_STATUS, 6, 6), >> + [F_Y_OW] = >> REG_FIELD(FXAS21002C_REG_DR_STATUS, 5, 5), >> + [F_X_OW] = >> REG_FIELD(FXAS21002C_REG_DR_STATUS, 4, 4), >> + [F_ZYX_DR] = >> REG_FIELD(FXAS21002C_REG_DR_STATUS, 3, 3), >> + [F_Z_DR] = >> REG_FIELD(FXAS21002C_REG_DR_STATUS, 2, 2), >> + [F_Y_DR] = >> REG_FIELD(FXAS21002C_REG_DR_STATUS, 1, 1), >> + [F_X_DR] = >> REG_FIELD(FXAS21002C_REG_DR_STATUS, 0, 0), >> + [F_OVF] = >> REG_FIELD(FXAS21002C_REG_F_STATUS, 7, 7), >> + [F_WMKF] = >> REG_FIELD(FXAS21002C_REG_F_STATUS, 6, 6), >> + [F_CNT] = >> REG_FIELD(FXAS21002C_REG_F_STATUS, 0, 5), >> + [F_MODE] = >> REG_FIELD(FXAS21002C_REG_F_SETUP, 6, 7), >> + [F_WMRK] = >> REG_FIELD(FXAS21002C_REG_F_SETUP, 0, 5), >> + [F_EVENT] = >> REG_FIELD(FXAS21002C_REG_F_EVENT, 5, 5), >> + [FE_TIME] = >> REG_FIELD(FXAS21002C_REG_F_EVENT, 0, 4), >> + [F_BOOTEND] = >> REG_FIELD(FXAS21002C_REG_INT_SRC_FLAG, 3, 3), >> + [F_SRC_FIFO] = >> REG_FIELD(FXAS21002C_REG_INT_SRC_FLAG, 2, 2), >> + [F_SRC_RT] = >> REG_FIELD(FXAS21002C_REG_INT_SRC_FLAG, 1, 1), >> + [F_SRC_DRDY] = >> REG_FIELD(FXAS21002C_REG_INT_SRC_FLAG, 0, 0), >> + [F_WHO_AM_I] = >> REG_FIELD(FXAS21002C_REG_WHO_AM_I, 0, 7), >> + [F_BW] = REG_FIELD(FXAS21002C_REG_CTRL0, >> 6, 7), >> + [F_SPIW] = REG_FIELD(FXAS21002C_REG_CTRL0, >> 5, 5), >> + [F_SEL] = REG_FIELD(FXAS21002C_REG_CTRL0, >> 3, 4), >> + [F_HPF_EN] = REG_FIELD(FXAS21002C_REG_CTRL0, >> 2, 2), >> + [F_FS] = REG_FIELD(FXAS21002C_REG_CTRL0, >> 0, 1), >> + [F_ELE] = REG_FIELD(FXAS21002C_REG_RT_CFG, >> 3, 3), >> + [F_ZTEFE] = REG_FIELD(FXAS21002C_REG_RT_CFG, >> 2, 2), >> + [F_YTEFE] = REG_FIELD(FXAS21002C_REG_RT_CFG, >> 1, 1), >> + [F_XTEFE] = REG_FIELD(FXAS21002C_REG_RT_CFG, >> 0, 0), >> + [F_EA] = REG_FIELD(FXAS21002C_REG_RT_SRC, >> 6, 6), >> + [F_ZRT] = REG_FIELD(FXAS21002C_REG_RT_SRC, >> 5, 5), >> + [F_ZRT_POL] = REG_FIELD(FXAS21002C_REG_RT_SRC, >> 4, 4), >> + [F_YRT] = REG_FIELD(FXAS21002C_REG_RT_SRC, >> 3, 3), >> + [F_YRT_POL] = REG_FIELD(FXAS21002C_REG_RT_SRC, >> 2, 2), >> + [F_XRT] = REG_FIELD(FXAS21002C_REG_RT_SRC, >> 1, 1), >> + [F_XRT_POL] = REG_FIELD(FXAS21002C_REG_RT_SRC, >> 0, 0), >> + [F_DBCNTM] = REG_FIELD(FXAS21002C_REG_RT_THS, >> 7, 7), >> + [F_THS] = REG_FIELD(FXAS21002C_REG_RT_SRC, >> 0, 6), >> + [F_RT_COUNT] = >> REG_FIELD(FXAS21002C_REG_RT_COUNT, 0, 7), >> + [F_TEMP] = REG_FIELD(FXAS21002C_REG_TEMP, >> 0, 7), >> + [F_RST] = REG_FIELD(FXAS21002C_REG_CTRL1, >> 6, 6), >> + [F_ST] = REG_FIELD(FXAS21002C_REG_CTRL1, >> 5, 5), >> + [F_DR] = REG_FIELD(FXAS21002C_REG_CTRL1, >> 2, 4), >> + [F_ACTIVE] = REG_FIELD(FXAS21002C_REG_CTRL1, >> 1, 1), >> + [F_READY] = REG_FIELD(FXAS21002C_REG_CTRL1, >> 0, 0), >> + [F_INT_CFG_FIFO] = REG_FIELD(FXAS21002C_REG_CTRL2, >> 7, 7), >> + [F_INT_EN_FIFO] = REG_FIELD(FXAS21002C_REG_CTRL2, >> 6, 6), >> + [F_INT_CFG_RT] = REG_FIELD(FXAS21002C_REG_CTRL2, >> 5, 5), >> + [F_INT_EN_RT] = REG_FIELD(FXAS21002C_REG_CTRL2, >> 4, 4), >> + [F_INT_CFG_DRDY] = REG_FIELD(FXAS21002C_REG_CTRL2, >> 3, 3), >> + [F_INT_EN_DRDY] = REG_FIELD(FXAS21002C_REG_CTRL2, >> 2, 2), >> + [F_IPOL] = REG_FIELD(FXAS21002C_REG_CTRL2, >> 1, 1), >> + [F_PP_OD] = REG_FIELD(FXAS21002C_REG_CTRL2, >> 0, 0), >> + [F_WRAPTOONE] = REG_FIELD(FXAS21002C_REG_CTRL3, >> 3, 3), >> + [F_EXTCTRLEN] = REG_FIELD(FXAS21002C_REG_CTRL3, >> 2, 2), >> + [F_FS_DOUBLE] = REG_FIELD(FXAS21002C_REG_CTRL3, >> 0, 0), >> +}; >> + >> +extern const struct dev_pm_ops fxas21002c_pm_ops; >> + >> +int fxas21002c_core_probe(struct device *dev, struct regmap >> *regmap, int irq, >> + const char *name); >> +void fxas21002c_core_remove(struct device *dev); >> +#endif >> diff --git a/drivers/iio/gyro/fxas21002c_core.c >> b/drivers/iio/gyro/fxas21002c_core.c >> new file mode 100644 >> index 000000000000..dccded203dd5 >> --- /dev/null >> +++ b/drivers/iio/gyro/fxas21002c_core.c >> @@ -0,0 +1,989 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Driver for NXP FXAS21002C Gyroscope - Core >> + * >> + * Copyright (C) 2019 Linaro Ltd. >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#include "fxas21002c.h" >> + >> +#define FXAS21002C_CHIP_ID_1 0xD6 >> +#define FXAS21002C_CHIP_ID_2 0xD7 >> + >> +enum fxas21002c_mode_state { >> + FXAS21002C_MODE_STANDBY, >> + FXAS21002C_MODE_READY, >> + FXAS21002C_MODE_ACTIVE, >> +}; >> + >> +#define FXAS21002C_STANDBY_ACTIVE_TIME_MS 62 >> +#define FXAS21002C_READY_ACTIVE_TIME_MS 7 >> + >> +#define FXAS21002C_ODR_LIST_MAX 10 >> + >> +#define FXAS21002C_SCALE_FRACTIONAL 32 >> +#define FXAS21002C_RANGE_LIMIT_DOUBLE 2000 >> + >> +#define FXAS21002C_AXIS_TO_REG(axis) (FXAS21002C_REG_OUT_X_MSB >> + ((axis) * 2)) >> + >> +static const int fxas21002c_odr_values[] = { >> + 800, 400, 200, 100, 50, 25, 12, 12 >> +}; >> + >> +/* >> + * These values are taken from the low-pass filter cutoff >> frequency calculated >> + * ODR * 0.lpf_values. So, for ODR = 800Hz with a lpf value = >> 0.32 >> + * => LPF cutoff frequency = 800 * 0.32 = 256 Hz >> + */ >> +static const int fxas21002c_lpf_values[] = { >> + 32, 16, 8 >> +}; >> + >> +/* >> + * These values are taken from the high-pass filter cutoff >> frequency calculated >> + * ODR * 0.0hpf_values. So, for ODR = 800Hz with a hpf value = >> 0.018750 >> + * => HPF cutoff frequency = 800 * 0.018750 = 15 Hz >> + */ >> +static const int fxas21002c_hpf_values[] = { >> + 18750, 9625, 4875, 2475 >> +}; >> + >> +static const int fxas21002c_range_values[] = { >> + 4000, 2000, 1000, 500, 250 >> +}; >> + >> +struct fxas21002c_data { >> + u8 chip_id; >> + enum fxas21002c_mode_state mode; >> + enum fxas21002c_mode_state prev_mode; >> + >> + struct mutex lock; /* serialize data access >> */ >> + struct regmap *regmap; >> + struct regmap_field *regmap_fields[F_MAX_FIELDS]; >> + struct iio_trigger *dready_trig; >> + int irq; >> + >> + struct regulator *vdd; >> + struct regulator *vddio; >> + >> + /* >> + * DMA (thus cache coherency maintenance) requires the >> + * transfer buffers to live in their own cache lines. >> + */ >> + s16 buffer[8] ____cacheline_aligned; >> +}; >> + >> +enum fxas21002c_channel_index { >> + CHANNEL_SCAN_INDEX_X, >> + CHANNEL_SCAN_INDEX_Y, >> + CHANNEL_SCAN_INDEX_Z, >> + CHANNEL_SCAN_MAX, >> +}; >> + >> +static int fxas21002c_odr_hz_from_value(struct fxas21002c_data >> *data, u8 value) >> +{ >> + int odr_value_max = ARRAY_SIZE(fxas21002c_odr_values) - 1; >> + >> + value = min_t(u8, value, odr_value_max); >> + >> + return fxas21002c_odr_values[value]; >> +} >> + >> +static int fxas21002c_odr_value_from_hz(struct fxas21002c_data >> *data, >> + unsigned int hz) >> +{ >> + int odr_table_size = ARRAY_SIZE(fxas21002c_odr_values); >> + int i; >> + >> + for (i = 0; i < odr_table_size; i++) >> + if (fxas21002c_odr_values[i] == hz) >> + return i; >> + >> + return -EINVAL; >> +} >> + >> +static int fxas21002c_lpf_bw_from_value(struct fxas21002c_data >> *data, u8 value) >> +{ >> + int lpf_value_max = ARRAY_SIZE(fxas21002c_lpf_values) - 1; >> + >> + value = min_t(u8, value, lpf_value_max); >> + >> + return fxas21002c_lpf_values[value]; >> +} >> + >> +static int fxas21002c_lpf_value_from_bw(struct fxas21002c_data >> *data, >> + unsigned int hz) >> +{ >> + int lpf_table_size = ARRAY_SIZE(fxas21002c_lpf_values); >> + int i; >> + >> + for (i = 0; i < lpf_table_size; i++) >> + if (fxas21002c_lpf_values[i] == hz) >> + return i; >> + >> + return -EINVAL; >> +} >> + >> +static int fxas21002c_hpf_sel_from_value(struct >> fxas21002c_data *data, u8 value) >> +{ >> + int hpf_value_max = ARRAY_SIZE(fxas21002c_hpf_values) - 1; >> + >> + value = min_t(u8, value, hpf_value_max); >> + >> + return fxas21002c_hpf_values[value]; >> +} >> + >> +static int fxas21002c_hpf_value_from_sel(struct >> fxas21002c_data *data, >> + unsigned int hz) >> +{ >> + int hpf_table_size = ARRAY_SIZE(fxas21002c_hpf_values); >> + int i; >> + >> + for (i = 0; i < hpf_table_size; i++) >> + if (fxas21002c_hpf_values[i] == hz) >> + return i; >> + >> + return -EINVAL; >> +} >> + >> +static int fxas21002c_range_fs_from_value(struct >> fxas21002c_data *data, >> + u8 value) >> +{ >> + int range_value_max = ARRAY_SIZE(fxas21002c_range_values) >> - 1; >> + unsigned int fs_double; >> + int ret; >> + >> + /* We need to check if FS_DOUBLE is enabled to offset the >> value */ >> + ret = regmap_field_read(data->regmap_fields[F_FS_DOUBLE], >> &fs_double); >> + if (ret < 0) >> + return ret; >> + >> + if (!fs_double) >> + value += 1; >> + >> + value = min_t(u8, value, range_value_max); >> + >> + return fxas21002c_range_values[value]; >> +} >> + >> +static int fxas21002c_range_value_from_fs(struct >> fxas21002c_data *data, >> + unsigned int range) >> +{ >> + int range_table_size = >> ARRAY_SIZE(fxas21002c_range_values); >> + bool found = false; >> + int fs_double = 0; >> + int ret; >> + int i; >> + >> + for (i = 0; i < range_table_size; i++) >> + if (fxas21002c_range_values[i] == range) { >> + found = true; >> + break; >> + } >> + >> + if (!found) >> + return -EINVAL; >> + >> + if (range > FXAS21002C_RANGE_LIMIT_DOUBLE) >> + fs_double = 1; >> + >> + ret = regmap_field_write(data->regmap_fields[F_FS_DOUBLE], >> fs_double); >> + if (ret < 0) >> + return ret; >> + >> + return i; >> +} >> + >> +static int fxas21002c_mode_get(struct fxas21002c_data *data) >> +{ >> + unsigned int active; >> + unsigned int ready; >> + int ret; >> + >> + ret = regmap_field_read(data->regmap_fields[F_ACTIVE], >> &active); >> + if (ret < 0) >> + return ret; >> + if (active) >> + return FXAS21002C_MODE_ACTIVE; >> + >> + ret = regmap_field_read(data->regmap_fields[F_READY], >> &ready); >> + if (ret < 0) >> + return ret; >> + if (ready) >> + return FXAS21002C_MODE_READY; >> + >> + return FXAS21002C_MODE_STANDBY; >> +} >> + >> +static int fxas21002c_mode_set(struct fxas21002c_data *data, >> + enum fxas21002c_mode_state mode) >> +{ >> + int ret; >> + >> + if (mode == data->mode) >> + return 0; >> + >> + if (mode == FXAS21002C_MODE_READY) >> + ret = >> regmap_field_write(data->regmap_fields[F_READY], 1); >> + else >> + ret = >> regmap_field_write(data->regmap_fields[F_READY], 0); >> + if (ret < 0) >> + return ret; >> + >> + if (mode == FXAS21002C_MODE_ACTIVE) >> + ret = >> regmap_field_write(data->regmap_fields[F_ACTIVE], 1); >> + else >> + ret = >> regmap_field_write(data->regmap_fields[F_ACTIVE], 0); >> + if (ret < 0) >> + return ret; >> + >> + /* if going to active wait the setup times */ >> + if (mode == FXAS21002C_MODE_ACTIVE && >> + data->mode == FXAS21002C_MODE_STANDBY) >> + >> msleep_interruptible(FXAS21002C_STANDBY_ACTIVE_TIME_MS); >> + >> + if (data->mode == FXAS21002C_MODE_READY) >> + >> msleep_interruptible(FXAS21002C_READY_ACTIVE_TIME_MS); >> + >> + data->prev_mode = data->mode; >> + data->mode = mode; >> + >> + return ret; >> +} >> + >> +static int fxas21002c_write(struct fxas21002c_data *data, >> + enum fxas21002c_fields field, int >> bits) >> +{ >> + int actual_mode; >> + int ret; >> + >> + mutex_lock(&data->lock); >> + >> + actual_mode = fxas21002c_mode_get(data); >> + if (actual_mode < 0) { >> + ret = actual_mode; >> + goto out_unlock; >> + } >> + >> + ret = fxas21002c_mode_set(data, FXAS21002C_MODE_READY); >> + if (ret < 0) >> + goto out_unlock; >> + >> + ret = regmap_field_write(data->regmap_fields[field], >> bits); >> + if (ret < 0) >> + goto out_unlock; >> + >> + ret = fxas21002c_mode_set(data, data->prev_mode); >> + >> +out_unlock: >> + mutex_unlock(&data->lock); >> + >> + return ret; >> +} >> + >> +static int fxas21002c_pm_get(struct fxas21002c_data *data) >> +{ >> + struct device *dev = regmap_get_device(data->regmap); >> + int ret; >> + >> + ret = pm_runtime_get_sync(dev); >> + if (ret < 0) >> + pm_runtime_put_noidle(dev); >> + >> + return ret; >> +} >> + >> +static int fxas21002c_pm_put(struct fxas21002c_data *data) >> +{ >> + struct device *dev = regmap_get_device(data->regmap); >> + >> + pm_runtime_mark_last_busy(dev); >> + >> + return pm_runtime_put_autosuspend(dev); >> +} >> + >> +static int fxas21002c_temp_get(struct fxas21002c_data *data, >> int *val) >> +{ >> + struct device *dev = regmap_get_device(data->regmap); >> + unsigned int temp; >> + int ret; >> + >> + mutex_lock(&data->lock); >> + ret = fxas21002c_pm_get(data); >> + if (ret < 0) >> + goto data_unlock; >> + >> + ret = regmap_field_read(data->regmap_fields[F_TEMP], >> &temp); >> + if (ret < 0) { >> + dev_err(dev, "failed to read temp: %d\n", ret); >> + goto data_unlock; >> + } >> + >> + *val = sign_extend32(temp, 7); >> + >> + ret = fxas21002c_pm_put(data); >> + if (ret < 0) >> + goto data_unlock; >> + >> + ret = IIO_VAL_INT; >> + >> +data_unlock: >> + mutex_unlock(&data->lock); >> + >> + return ret; >> +} >> + >> +static int fxas21002c_axis_get(struct fxas21002c_data *data, >> + int index, int *val) >> +{ >> + struct device *dev = regmap_get_device(data->regmap); >> + __be16 axis_be; >> + int ret; >> + >> + mutex_lock(&data->lock); >> + ret = fxas21002c_pm_get(data); >> + if (ret < 0) >> + goto data_unlock; >> + >> + ret = regmap_bulk_read(data->regmap, >> FXAS21002C_AXIS_TO_REG(index), >> + &axis_be, sizeof(axis_be)); >> + if (ret < 0) { >> + dev_err(dev, "failed to read axis: %d: %d\n", >> index, ret); >> + goto data_unlock; >> + } >> + >> + *val = sign_extend32(be16_to_cpu(axis_be), 15); >> + >> + ret = fxas21002c_pm_put(data); >> + if (ret < 0) >> + goto data_unlock; >> + >> + ret = IIO_VAL_INT; >> + >> +data_unlock: >> + mutex_unlock(&data->lock); >> + >> + return ret; >> +} >> + >> +static int fxas21002c_odr_get(struct fxas21002c_data *data, >> int *odr) >> +{ >> + unsigned int odr_bits; >> + int ret; >> + >> + mutex_lock(&data->lock); >> + ret = regmap_field_read(data->regmap_fields[F_DR], >> &odr_bits); >> + if (ret < 0) >> + goto data_unlock; >> + >> + *odr = fxas21002c_odr_hz_from_value(data, odr_bits); >> + >> + ret = IIO_VAL_INT; >> + >> +data_unlock: >> + mutex_unlock(&data->lock); >> + >> + return ret; >> +} >> + >> +static int fxas21002c_odr_set(struct fxas21002c_data *data, >> int odr) >> +{ >> + int odr_bits; >> + >> + odr_bits = fxas21002c_odr_value_from_hz(data, odr); >> + if (odr_bits < 0) >> + return odr_bits; >> + >> + return fxas21002c_write(data, F_DR, odr_bits); >> +} >> + >> +static int fxas21002c_lpf_get(struct fxas21002c_data *data, >> int *val2) >> +{ >> + unsigned int bw_bits; >> + int ret; >> + >> + mutex_lock(&data->lock); >> + ret = regmap_field_read(data->regmap_fields[F_BW], >> &bw_bits); >> + if (ret < 0) >> + goto data_unlock; >> + >> + *val2 = fxas21002c_lpf_bw_from_value(data, bw_bits) * >> 10000; >> + >> + ret = IIO_VAL_INT_PLUS_MICRO; >> + >> +data_unlock: >> + mutex_unlock(&data->lock); >> + >> + return ret; >> +} >> + >> +static int fxas21002c_lpf_set(struct fxas21002c_data *data, >> int bw) >> +{ >> + int bw_bits; >> + int odr; >> + int ret; >> + >> + bw_bits = fxas21002c_lpf_value_from_bw(data, bw); >> + if (bw_bits < 0) >> + return bw_bits; >> + >> + /* >> + * From table 33 of the device spec, for ODR = 25Hz and >> 12.5 value 0.08 >> + * is not allowed and for ODR = 12.5 value 0.16 is also >> not allowed >> + */ >> + ret = fxas21002c_odr_get(data, &odr); >> + if (ret < 0) >> + return -EINVAL; >> + >> + if ((odr == 25 && bw_bits > 0x01) || (odr == 12 && bw_bits >> > 0)) >> + return -EINVAL; >> + >> + return fxas21002c_write(data, F_BW, bw_bits); >> +} >> + >> +static int fxas21002c_hpf_get(struct fxas21002c_data *data, >> int *val2) >> +{ >> + unsigned int sel_bits; >> + int ret; >> + >> + mutex_lock(&data->lock); >> + ret = regmap_field_read(data->regmap_fields[F_SEL], >> &sel_bits); >> + if (ret < 0) >> + goto data_unlock; >> + >> + *val2 = fxas21002c_hpf_sel_from_value(data, sel_bits); >> + >> + ret = IIO_VAL_INT_PLUS_MICRO; >> + >> +data_unlock: >> + mutex_unlock(&data->lock); >> + >> + return ret; >> +} >> + >> +static int fxas21002c_hpf_set(struct fxas21002c_data *data, >> int sel) >> +{ >> + int sel_bits; >> + >> + sel_bits = fxas21002c_hpf_value_from_sel(data, sel); >> + if (sel_bits < 0) >> + return sel_bits; >> + >> + return fxas21002c_write(data, F_SEL, sel_bits); >> +} >> + >> +static int fxas21002c_scale_get(struct fxas21002c_data *data, >> int *val) >> +{ >> + int fs_bits; >> + int scale; >> + int ret = 0; >> + >> + mutex_lock(&data->lock); >> + ret = regmap_field_read(data->regmap_fields[F_FS], >> &fs_bits); >> + if (ret < 0) >> + goto data_unlock; >> + >> + scale = fxas21002c_range_fs_from_value(data, fs_bits); >> + if (scale < 0) { >> + ret = scale; >> + goto data_unlock; >> + } >> + >> + *val = scale; >> + >> +data_unlock: >> + mutex_unlock(&data->lock); >> + >> + return ret; >> +} >> + >> +static int fxas21002c_scale_set(struct fxas21002c_data *data, >> int range) >> +{ >> + int fs_bits; >> + >> + fs_bits = fxas21002c_range_value_from_fs(data, range); >> + if (fs_bits < 0) >> + return fs_bits; >> + >> + return fxas21002c_write(data, F_FS, fs_bits); >> +} >> + >> +static int fxas21002c_read_raw(struct iio_dev *indio_dev, >> + struct iio_chan_spec const *chan, >> int *val, >> + int *val2, long mask) >> +{ >> + struct fxas21002c_data *data = iio_priv(indio_dev); >> + int ret; >> + >> + switch (mask) { >> + case IIO_CHAN_INFO_RAW: >> + switch (chan->type) { >> + case IIO_TEMP: >> + return fxas21002c_temp_get(data, val); >> + case IIO_ANGL_VEL: >> + return fxas21002c_axis_get(data, >> chan->scan_index, val); >> + default: >> + return -EINVAL; >> + } >> + case IIO_CHAN_INFO_SCALE: >> + switch (chan->type) { >> + case IIO_ANGL_VEL: >> + *val2 = FXAS21002C_SCALE_FRACTIONAL; >> + ret = fxas21002c_scale_get(data, val); >> + if (ret < 0) >> + return ret; >> + >> + return IIO_VAL_FRACTIONAL; >> + default: >> + return -EINVAL; >> + } >> + case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY: >> + *val = 0; >> + return fxas21002c_lpf_get(data, val2); >> + case IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY: >> + *val = 0; >> + return fxas21002c_hpf_get(data, val2); >> + case IIO_CHAN_INFO_SAMP_FREQ: >> + *val2 = 0; >> + return fxas21002c_odr_get(data, val); >> + default: >> + return -EINVAL; >> + } >> +} >> + >> +static int fxas21002c_write_raw(struct iio_dev *indio_dev, >> + struct iio_chan_spec const *chan, >> int val, >> + int val2, long mask) >> +{ >> + struct fxas21002c_data *data = iio_priv(indio_dev); >> + int range; >> + >> + switch (mask) { >> + case IIO_CHAN_INFO_SAMP_FREQ: >> + if (val2) >> + return -EINVAL; >> + >> + return fxas21002c_odr_set(data, val); >> + case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY: >> + if (val) >> + return -EINVAL; >> + >> + val2 = val2 / 10000; >> + return fxas21002c_lpf_set(data, val2); >> + case IIO_CHAN_INFO_SCALE: >> + switch (chan->type) { >> + case IIO_ANGL_VEL: >> + range = (((val * 1000 + val2 / 1000) * >> + FXAS21002C_SCALE_FRACTIONAL) / >> 1000); >> + return fxas21002c_scale_set(data, range); >> + default: >> + return -EINVAL; >> + } >> + case IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY: >> + return fxas21002c_hpf_set(data, val2); >> + default: >> + return -EINVAL; >> + } >> +} >> + >> +static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("12.5 25 50 100 200 400 >> 800"); >> + >> +static >> IIO_CONST_ATTR(in_anglvel_filter_low_pass_3db_frequency_available, >> + "0.32 0.16 0.08"); >> + >> +static >> IIO_CONST_ATTR(in_anglvel_filter_high_pass_3db_frequency_available, >> + "0.018750 0.009625 0.004875 0.002475"); >> + >> +static IIO_CONST_ATTR(in_anglvel_scale_available, >> + "125.0 62.5 31.25 15.625 7.8125"); >> + >> +static struct attribute *fxas21002c_attributes[] = { >> + >> &iio_const_attr_sampling_frequency_available.dev_attr.attr, >> + >> &iio_const_attr_in_anglvel_filter_low_pass_3db_frequency_available.dev_attr.attr, >> + >> &iio_const_attr_in_anglvel_filter_high_pass_3db_frequency_available.dev_attr.attr, >> + &iio_const_attr_in_anglvel_scale_available.dev_attr.attr, >> + NULL, >> +}; >> + >> +static const struct attribute_group fxas21002c_attrs_group = { >> + .attrs = fxas21002c_attributes, >> +}; >> + >> +#define FXAS21002C_CHANNEL(_axis) { >> \ >> + .type = IIO_ANGL_VEL, >> \ >> + .modified = 1, >> \ >> + .channel2 = IIO_MOD_##_axis, >> \ >> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), >> \ >> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) | >> \ >> + BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY) | >> \ >> + BIT(IIO_CHAN_INFO_HIGH_PASS_FILTER_3DB_FREQUENCY) >> | \ >> + BIT(IIO_CHAN_INFO_SAMP_FREQ), >> \ >> + .scan_index = CHANNEL_SCAN_INDEX_##_axis, >> \ >> + .scan_type = { >> \ >> + .sign = 's', >> \ >> + .realbits = 16, >> \ >> + .storagebits = 16, >> \ >> + .endianness = IIO_BE, >> \ >> + }, >> \ >> +} >> + >> +static const struct iio_chan_spec fxas21002c_channels[] = { >> + { >> + .type = IIO_TEMP, >> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), >> + .scan_index = -1, >> + }, >> + FXAS21002C_CHANNEL(X), >> + FXAS21002C_CHANNEL(Y), >> + FXAS21002C_CHANNEL(Z), >> +}; >> + >> +static const struct iio_info fxas21002c_info = { >> + .attrs = &fxas21002c_attrs_group, >> + .read_raw = &fxas21002c_read_raw, >> + .write_raw = &fxas21002c_write_raw, >> +}; >> + >> +static irqreturn_t fxas21002c_trigger_handler(int irq, void >> *p) >> +{ >> + struct iio_poll_func *pf = p; >> + struct iio_dev *indio_dev = pf->indio_dev; >> + struct fxas21002c_data *data = iio_priv(indio_dev); >> + int ret; >> + >> + mutex_lock(&data->lock); >> + ret = regmap_bulk_read(data->regmap, >> FXAS21002C_REG_OUT_X_MSB, >> + data->buffer, CHANNEL_SCAN_MAX * >> sizeof(s16)); >> + mutex_unlock(&data->lock); >> + if (ret < 0) >> + goto notify_done; >> + >> + iio_push_to_buffers_with_timestamp(indio_dev, >> data->buffer, >> + pf->timestamp); >> + >> +notify_done: >> + iio_trigger_notify_done(indio_dev->trig); >> + >> + return IRQ_HANDLED; >> +} >> + >> +static int fxas21002c_chip_init(struct fxas21002c_data *data) >> +{ >> + struct device *dev = regmap_get_device(data->regmap); >> + unsigned int chip_id; >> + int ret; >> + >> + ret = regmap_field_read(data->regmap_fields[F_WHO_AM_I], >> &chip_id); >> + if (ret < 0) >> + return ret; >> + >> + if (chip_id != FXAS21002C_CHIP_ID_1 && >> + chip_id != FXAS21002C_CHIP_ID_2) { >> + dev_err(dev, "chip id 0x%02x is not supported\n", >> chip_id); >> + return -EINVAL; >> + } >> + >> + data->chip_id = chip_id; >> + >> + ret = fxas21002c_mode_set(data, FXAS21002C_MODE_STANDBY); >> + if (ret < 0) >> + return ret; >> + >> + /* Set ODR to 200HZ as default */ >> + ret = fxas21002c_odr_set(data, 200); >> + if (ret < 0) >> + dev_err(dev, "failed to set ODR: %d\n", ret); >> + >> + return ret; >> +} >> + >> +static int fxas21002c_data_rdy_trigger_set_state(struct >> iio_trigger *trig, >> + bool state) >> +{ >> + struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig); >> + struct fxas21002c_data *data = iio_priv(indio_dev); >> + >> + return >> regmap_field_write(data->regmap_fields[F_INT_EN_DRDY], state); >> +} >> + >> +static const struct iio_trigger_ops fxas21002c_trigger_ops = { >> + .set_trigger_state = >> &fxas21002c_data_rdy_trigger_set_state, >> +}; >> + >> +static irqreturn_t fxas21002c_data_rdy_trig_poll(int irq, void >> *private) >> +{ >> + struct iio_dev *indio_dev = private; >> + struct fxas21002c_data *data = iio_priv(indio_dev); >> + unsigned int data_ready; >> + int ret; >> + >> + ret = regmap_field_read(data->regmap_fields[F_SRC_DRDY], >> &data_ready); >> + if (ret < 0) >> + return IRQ_NONE; >> + >> + if (!data_ready) >> + return IRQ_NONE; >> + >> + iio_trigger_poll(data->dready_trig); >> + >> + return IRQ_HANDLED; >> +} >> + >> +static int fxas21002c_trigger_probe(struct fxas21002c_data >> *data) >> +{ >> + struct device *dev = regmap_get_device(data->regmap); >> + struct iio_dev *indio_dev = dev_get_drvdata(dev); >> + struct device_node *np = indio_dev->dev.of_node; >> + unsigned long irq_trig; >> + bool irq_open_drain; >> + int irq1; >> + int ret; >> + >> + if (!data->irq) >> + return 0; >> + >> + irq1 = of_irq_get_byname(np, "INT1"); >> + >> + if (irq1 == data->irq) { >> + dev_info(dev, "using interrupt line INT1\n"); >> + ret = >> regmap_field_write(data->regmap_fields[F_INT_CFG_DRDY], >> + 1); >> + if (ret < 0) >> + return ret; >> + } >> + >> + dev_info(dev, "using interrupt line INT2\n"); >> + >> + irq_open_drain = of_property_read_bool(np, >> "drive-open-drain"); >> + >> + data->dready_trig = devm_iio_trigger_alloc(dev, >> "%s-dev%d", >> + >> indio_dev->name, >> + indio_dev->id); >> + if (!data->dready_trig) >> + return -ENOMEM; >> + >> + irq_trig = IRQF_TRIGGER_RISING; >> + if (irq_open_drain) >> + irq_trig |= IRQF_SHARED; >> + >> + ret = devm_request_irq(dev, data->irq, >> fxas21002c_data_rdy_trig_poll, >> + irq_trig, "fxas21002c_data_ready", >> + data->dready_trig); >> + if (ret < 0) >> + return ret; >> + >> + data->dready_trig->dev.parent = dev; >> + data->dready_trig->ops = &fxas21002c_trigger_ops; >> + iio_trigger_set_drvdata(data->dready_trig, indio_dev); >> + >> + return devm_iio_trigger_register(dev, data->dready_trig); >> +} >> + >> +static int fxas21002c_power_enable(struct fxas21002c_data >> *data) >> +{ >> + int ret; >> + >> + ret = regulator_enable(data->vdd); >> + if (ret < 0) >> + return ret; >> + >> + return regulator_enable(data->vddio); > If this fails on this second call, then it should turn the first > regulator off. A function should leave no side effects if it > fails. Makes for a much easier code flow to review. > >> +} >> + >> +static void fxas21002c_power_disable(struct fxas21002c_data >> *data) >> +{ >> + regulator_disable(data->vdd); >> + regulator_disable(data->vddio); >> +} >> + >> +static void fxas21002c_power_disable_action(void *_data) >> +{ >> + struct fxas21002c_data *data = _data; >> + >> + fxas21002c_power_disable(data); >> +} >> + >> +static int fxas21002c_regulators_get(struct fxas21002c_data >> *data) >> +{ >> + struct device *dev = regmap_get_device(data->regmap); >> + >> + data->vdd = devm_regulator_get(dev->parent, "vdd"); >> + if (IS_ERR(data->vdd)) >> + return PTR_ERR(data->vdd); >> + >> + data->vddio = devm_regulator_get(dev->parent, "vddio"); >> + if (IS_ERR(data->vddio)) >> + return PTR_ERR(data->vddio); >> + >> + return 0; >> +} >> + >> +int fxas21002c_core_probe(struct device *dev, struct regmap >> *regmap, int irq, >> + const char *name) >> +{ >> + struct fxas21002c_data *data; >> + struct iio_dev *indio_dev; >> + struct regmap_field *f; >> + int i; >> + int ret; >> + >> + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); >> + if (!indio_dev) >> + return -ENOMEM; >> + >> + data = iio_priv(indio_dev); >> + dev_set_drvdata(dev, indio_dev); >> + data->irq = irq; >> + data->regmap = regmap; >> + >> + for (i = 0; i < F_MAX_FIELDS; i++) { >> + f = devm_regmap_field_alloc(dev, data->regmap, >> + >> fxas21002c_reg_fields[i]); >> + if (IS_ERR(f)) >> + return PTR_ERR(f); >> + >> + data->regmap_fields[i] = f; >> + } >> + >> + mutex_init(&data->lock); >> + >> + ret = fxas21002c_regulators_get(data); >> + if (ret < 0) >> + return ret; >> + >> + ret = devm_add_action(dev, >> fxas21002c_power_disable_action, data); > This is a bit odd, normally you would have a call > to devm_add_action_or_reset after the thing you are unwinding > rather > than set up the unwind before the call (power_enable) and hence > potentially > end up disabling something that was never successfully enabled. > > The or_reset deals with the devm_add_action itself failing. > >> + if (ret < 0) >> + return ret; >> + >> + ret = fxas21002c_power_enable(data); >> + if (ret < 0) >> + return ret; >> + >> + ret = fxas21002c_chip_init(data); >> + if (ret < 0) >> + return ret; >> + >> + indio_dev->dev.parent = dev; >> + indio_dev->channels = fxas21002c_channels; >> + indio_dev->num_channels = ARRAY_SIZE(fxas21002c_channels); >> + indio_dev->name = name; >> + indio_dev->modes = INDIO_DIRECT_MODE; >> + indio_dev->info = &fxas21002c_info; >> + >> + ret = fxas21002c_trigger_probe(data); >> + if (ret < 0) >> + return ret; >> + >> + ret = devm_iio_triggered_buffer_setup(dev, indio_dev, >> + >> iio_pollfunc_store_time, >> + >> fxas21002c_trigger_handler, NULL); >> + if (ret < 0) >> + return ret; >> + >> + ret = pm_runtime_set_active(dev); >> + if (ret) >> + return ret; >> + >> + pm_runtime_enable(dev); >> + pm_runtime_set_autosuspend_delay(dev, 2000); >> + pm_runtime_use_autosuspend(dev); >> + >> + ret = devm_iio_device_register(dev, indio_dev); >> + if (ret < 0) >> + return ret; >> + >> + dev_info(dev, "fxas21002c iio device ready\n"); > > I may have mentioned this before, but this is noise. There > are lots of trivial ways to find out if this succeeded and > in many ways the kernel log is one of the harder ways to > find out. Please drop. > >> + >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(fxas21002c_core_probe); >> + >> +void fxas21002c_core_remove(struct device *dev) >> +{ >> + struct fxas21002c_data *data = >> iio_priv(dev_get_drvdata(dev)); >> + >> + pm_runtime_disable(dev); >> + pm_runtime_set_suspended(dev); >> + pm_runtime_put_noidle(dev); >> + >> + fxas21002c_mode_set(data, FXAS21002C_MODE_STANDBY); >> + fxas21002c_power_disable(data); >> +} >> +EXPORT_SYMBOL_GPL(fxas21002c_core_remove); >> + >> +static int __maybe_unused fxas21002c_suspend(struct device >> *dev) >> +{ >> + struct fxas21002c_data *data = >> iio_priv(dev_get_drvdata(dev)); >> + >> + fxas21002c_mode_set(data, FXAS21002C_MODE_STANDBY); >> + fxas21002c_power_disable(data); >> + >> + return 0; >> +} >> + >> +static int __maybe_unused fxas21002c_resume(struct device >> *dev) >> +{ >> + struct fxas21002c_data *data = >> iio_priv(dev_get_drvdata(dev)); >> + int ret; >> + >> + ret = fxas21002c_power_enable(data); >> + if (ret < 0) >> + return ret; >> + >> + ret = fxas21002c_mode_set(data, data->prev_mode); >> + if (ret < 0) >> + return ret; > As below, _mode_set only returns 0 or negative so obvious > simplification here. >> + >> + return 0; >> +} >> + >> +static int __maybe_unused fxas21002c_runtime_suspend(struct >> device *dev) >> +{ >> + struct fxas21002c_data *data = >> iio_priv(dev_get_drvdata(dev)); >> + int ret; >> + >> + ret = fxas21002c_mode_set(data, FXAS21002C_MODE_READY); >> + if (ret < 0) > I'm not actually convinced this makes sense. If we have an error > from > mode set, it probably indicates a communications problem. > Frankly > at that point there is little point in coming back again. > So why this return? > >> + return -EAGAIN; >> + >> + return 0; >> +} >> + >> +static int __maybe_unused fxas21002c_runtime_resume(struct >> device *dev) >> +{ >> + struct fxas21002c_data *data = >> iio_priv(dev_get_drvdata(dev)); >> + int ret; >> + >> + ret = fxas21002c_mode_set(data, FXAS21002C_MODE_ACTIVE); >> + if (ret < 0) >> + return ret; > Could be simplified to a simple > > return fxsa21002c_mode_set(..) > > As that only returns negative or 0 (I think...) > >> + >> + return 0; >> +} >> + >> +const struct dev_pm_ops fxas21002c_pm_ops = { >> + SET_SYSTEM_SLEEP_PM_OPS(fxas21002c_suspend, >> fxas21002c_resume) >> + SET_RUNTIME_PM_OPS(fxas21002c_runtime_suspend, >> + fxas21002c_runtime_resume, NULL) >> +}; >> +EXPORT_SYMBOL_GPL(fxas21002c_pm_ops); >> + >> +MODULE_AUTHOR("Rui Miguel Silva "); >> +MODULE_LICENSE("GPL v2"); >> +MODULE_DESCRIPTION("FXAS21002C Gyro driver");