Re: [PATCH v3 1/6] iio: bmg160: Use i2c regmap instead of direct i2c access
From: Jonathan Cameron
Date: Sat Aug 15 2015 - 10:43:09 EST
On 15/08/15 15:41, Jonathan Cameron wrote:
> On 12/08/15 17:01, Srinivas Pandruvada wrote:
>> On Wed, 2015-08-12 at 16:50 +0200, Markus Pargmann wrote:
>>> This patch introduces regmap usage into the driver. This is done to
>>> later easily support the SPI interface of this chip.
>>>
>>> Signed-off-by: Markus Pargmann <mpa@xxxxxxxxxxxxxx>
>> Reviewed-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>
> Applied to the togreg branch of iio.git. Initially pushed out as testing.
> Unfortunately these have probably just missed the merge window for this
> cycle. I'll get them to Greg and hence into linux-next in about 3-4 weeks
> time.
>
> Jonathan
One slight quirk from build test. I've added a static below.
>>> ---
>>> drivers/iio/gyro/Kconfig | 1 +
>>> drivers/iio/gyro/bmg160.c | 198 ++++++++++++++++++++--------------------------
>>> 2 files changed, 88 insertions(+), 111 deletions(-)
>>>
>>> diff --git a/drivers/iio/gyro/Kconfig b/drivers/iio/gyro/Kconfig
>>> index 8d2439345673..cf82d74139a2 100644
>>> --- a/drivers/iio/gyro/Kconfig
>>> +++ b/drivers/iio/gyro/Kconfig
>>> @@ -55,6 +55,7 @@ config BMG160
>>> depends on I2C
>>> select IIO_BUFFER
>>> select IIO_TRIGGERED_BUFFER
>>> + select REGMAP_I2C
>>> help
>>> Say yes here to build support for Bosch BMG160 Tri-axis Gyro Sensor
>>> driver. This driver also supports BMI055 gyroscope.
>>> diff --git a/drivers/iio/gyro/bmg160.c b/drivers/iio/gyro/bmg160.c
>>> index 460bf715d541..bbe02053e98a 100644
>>> --- a/drivers/iio/gyro/bmg160.c
>>> +++ b/drivers/iio/gyro/bmg160.c
>>> @@ -28,6 +28,7 @@
>>> #include <linux/iio/events.h>
>>> #include <linux/iio/trigger_consumer.h>
>>> #include <linux/iio/triggered_buffer.h>
>>> +#include <linux/regmap.h>
>>>
>>> #define BMG160_DRV_NAME "bmg160"
>>> #define BMG160_IRQ_NAME "bmg160_event"
>>> @@ -98,6 +99,7 @@
>>>
>>> struct bmg160_data {
>>> struct i2c_client *client;
>>> + struct regmap *regmap;
>>> struct iio_trigger *dready_trig;
>>> struct iio_trigger *motion_trig;
>>> struct mutex mutex;
>>> @@ -134,12 +136,17 @@ static const struct {
>>> { 133, BMG160_RANGE_250DPS},
>>> { 66, BMG160_RANGE_125DPS} };
>>>
>>> +struct regmap_config bmg160_regmap_i2c_conf = {
Should be static.
>>> + .reg_bits = 8,
>>> + .val_bits = 8,
>>> + .max_register = 0x3f
>>> +};
>>> +
>>> static int bmg160_set_mode(struct bmg160_data *data, u8 mode)
>>> {
>>> int ret;
>>>
>>> - ret = i2c_smbus_write_byte_data(data->client,
>>> - BMG160_REG_PMU_LPW, mode);
>>> + ret = regmap_write(data->regmap, BMG160_REG_PMU_LPW, mode);
>>> if (ret < 0) {
>>> dev_err(&data->client->dev, "Error writing reg_pmu_lpw\n");
>>> return ret;
>>> @@ -169,8 +176,7 @@ static int bmg160_set_bw(struct bmg160_data *data, int val)
>>> if (bw_bits < 0)
>>> return bw_bits;
>>>
>>> - ret = i2c_smbus_write_byte_data(data->client, BMG160_REG_PMU_BW,
>>> - bw_bits);
>>> + ret = regmap_write(data->regmap, BMG160_REG_PMU_BW, bw_bits);
>>> if (ret < 0) {
>>> dev_err(&data->client->dev, "Error writing reg_pmu_bw\n");
>>> return ret;
>>> @@ -184,16 +190,17 @@ static int bmg160_set_bw(struct bmg160_data *data, int val)
>>> static int bmg160_chip_init(struct bmg160_data *data)
>>> {
>>> int ret;
>>> + unsigned int val;
>>>
>>> - ret = i2c_smbus_read_byte_data(data->client, BMG160_REG_CHIP_ID);
>>> + ret = regmap_read(data->regmap, BMG160_REG_CHIP_ID, &val);
>>> if (ret < 0) {
>>> dev_err(&data->client->dev, "Error reading reg_chip_id\n");
>>> return ret;
>>> }
>>>
>>> - dev_dbg(&data->client->dev, "Chip Id %x\n", ret);
>>> - if (ret != BMG160_CHIP_ID_VAL) {
>>> - dev_err(&data->client->dev, "invalid chip %x\n", ret);
>>> + dev_dbg(&data->client->dev, "Chip Id %x\n", val);
>>> + if (val != BMG160_CHIP_ID_VAL) {
>>> + dev_err(&data->client->dev, "invalid chip %x\n", val);
>>> return -ENODEV;
>>> }
>>>
>>> @@ -210,40 +217,31 @@ static int bmg160_chip_init(struct bmg160_data *data)
>>> return ret;
>>>
>>> /* Set Default Range */
>>> - ret = i2c_smbus_write_byte_data(data->client,
>>> - BMG160_REG_RANGE,
>>> - BMG160_RANGE_500DPS);
>>> + ret = regmap_write(data->regmap, BMG160_REG_RANGE, BMG160_RANGE_500DPS);
>>> if (ret < 0) {
>>> dev_err(&data->client->dev, "Error writing reg_range\n");
>>> return ret;
>>> }
>>> data->dps_range = BMG160_RANGE_500DPS;
>>>
>>> - ret = i2c_smbus_read_byte_data(data->client, BMG160_REG_SLOPE_THRES);
>>> + ret = regmap_read(data->regmap, BMG160_REG_SLOPE_THRES, &val);
>>> if (ret < 0) {
>>> dev_err(&data->client->dev, "Error reading reg_slope_thres\n");
>>> return ret;
>>> }
>>> - data->slope_thres = ret;
>>> + data->slope_thres = val;
>>>
>>> /* Set default interrupt mode */
>>> - ret = i2c_smbus_read_byte_data(data->client, BMG160_REG_INT_EN_1);
>>> - if (ret < 0) {
>>> - dev_err(&data->client->dev, "Error reading reg_int_en_1\n");
>>> - return ret;
>>> - }
>>> - ret &= ~BMG160_INT1_BIT_OD;
>>> - ret = i2c_smbus_write_byte_data(data->client,
>>> - BMG160_REG_INT_EN_1, ret);
>>> + ret = regmap_update_bits(data->regmap, BMG160_REG_INT_EN_1,
>>> + BMG160_INT1_BIT_OD, 0);
>>> if (ret < 0) {
>>> - dev_err(&data->client->dev, "Error writing reg_int_en_1\n");
>>> + dev_err(&data->client->dev, "Error updating bits in reg_int_en_1\n");
>>> return ret;
>>> }
>>>
>>> - ret = i2c_smbus_write_byte_data(data->client,
>>> - BMG160_REG_INT_RST_LATCH,
>>> - BMG160_INT_MODE_LATCH_INT |
>>> - BMG160_INT_MODE_LATCH_RESET);
>>> + ret = regmap_write(data->regmap, BMG160_REG_INT_RST_LATCH,
>>> + BMG160_INT_MODE_LATCH_INT |
>>> + BMG160_INT_MODE_LATCH_RESET);
>>> if (ret < 0) {
>>> dev_err(&data->client->dev,
>>> "Error writing reg_motion_intr\n");
>>> @@ -284,41 +282,28 @@ static int bmg160_setup_any_motion_interrupt(struct bmg160_data *data,
>>> int ret;
>>>
>>> /* Enable/Disable INT_MAP0 mapping */
>>> - ret = i2c_smbus_read_byte_data(data->client, BMG160_REG_INT_MAP_0);
>>> - if (ret < 0) {
>>> - dev_err(&data->client->dev, "Error reading reg_int_map0\n");
>>> - return ret;
>>> - }
>>> - if (status)
>>> - ret |= BMG160_INT_MAP_0_BIT_ANY;
>>> - else
>>> - ret &= ~BMG160_INT_MAP_0_BIT_ANY;
>>> -
>>> - ret = i2c_smbus_write_byte_data(data->client,
>>> - BMG160_REG_INT_MAP_0,
>>> - ret);
>>> + ret = regmap_update_bits(data->regmap, BMG160_REG_INT_MAP_0,
>>> + BMG160_INT_MAP_0_BIT_ANY,
>>> + (status ? BMG160_INT_MAP_0_BIT_ANY : 0));
>>> if (ret < 0) {
>>> - dev_err(&data->client->dev, "Error writing reg_int_map0\n");
>>> + dev_err(&data->client->dev, "Error updating bits reg_int_map0\n");
>>> return ret;
>>> }
>>>
>>> /* Enable/Disable slope interrupts */
>>> if (status) {
>>> /* Update slope thres */
>>> - ret = i2c_smbus_write_byte_data(data->client,
>>> - BMG160_REG_SLOPE_THRES,
>>> - data->slope_thres);
>>> + ret = regmap_write(data->regmap, BMG160_REG_SLOPE_THRES,
>>> + data->slope_thres);
>>> if (ret < 0) {
>>> dev_err(&data->client->dev,
>>> "Error writing reg_slope_thres\n");
>>> return ret;
>>> }
>>>
>>> - ret = i2c_smbus_write_byte_data(data->client,
>>> - BMG160_REG_MOTION_INTR,
>>> - BMG160_INT_MOTION_X |
>>> - BMG160_INT_MOTION_Y |
>>> - BMG160_INT_MOTION_Z);
>>> + ret = regmap_write(data->regmap, BMG160_REG_MOTION_INTR,
>>> + BMG160_INT_MOTION_X | BMG160_INT_MOTION_Y |
>>> + BMG160_INT_MOTION_Z);
>>> if (ret < 0) {
>>> dev_err(&data->client->dev,
>>> "Error writing reg_motion_intr\n");
>>> @@ -331,10 +316,10 @@ static int bmg160_setup_any_motion_interrupt(struct bmg160_data *data,
>>> * to set latched mode, we will be flooded anyway with INTR
>>> */
>>> if (!data->dready_trigger_on) {
>>> - ret = i2c_smbus_write_byte_data(data->client,
>>> - BMG160_REG_INT_RST_LATCH,
>>> - BMG160_INT_MODE_LATCH_INT |
>>> - BMG160_INT_MODE_LATCH_RESET);
>>> + ret = regmap_write(data->regmap,
>>> + BMG160_REG_INT_RST_LATCH,
>>> + BMG160_INT_MODE_LATCH_INT |
>>> + BMG160_INT_MODE_LATCH_RESET);
>>> if (ret < 0) {
>>> dev_err(&data->client->dev,
>>> "Error writing reg_rst_latch\n");
>>> @@ -342,14 +327,12 @@ static int bmg160_setup_any_motion_interrupt(struct bmg160_data *data,
>>> }
>>> }
>>>
>>> - ret = i2c_smbus_write_byte_data(data->client,
>>> - BMG160_REG_INT_EN_0,
>>> - BMG160_DATA_ENABLE_INT);
>>> + ret = regmap_write(data->regmap, BMG160_REG_INT_EN_0,
>>> + BMG160_DATA_ENABLE_INT);
>>>
>>> - } else
>>> - ret = i2c_smbus_write_byte_data(data->client,
>>> - BMG160_REG_INT_EN_0,
>>> - 0);
>>> + } else {
>>> + ret = regmap_write(data->regmap, BMG160_REG_INT_EN_0, 0);
>>> + }
>>>
>>> if (ret < 0) {
>>> dev_err(&data->client->dev, "Error writing reg_int_en0\n");
>>> @@ -365,55 +348,39 @@ static int bmg160_setup_new_data_interrupt(struct bmg160_data *data,
>>> int ret;
>>>
>>> /* Enable/Disable INT_MAP1 mapping */
>>> - ret = i2c_smbus_read_byte_data(data->client, BMG160_REG_INT_MAP_1);
>>> - if (ret < 0) {
>>> - dev_err(&data->client->dev, "Error reading reg_int_map1\n");
>>> - return ret;
>>> - }
>>> -
>>> - if (status)
>>> - ret |= BMG160_INT_MAP_1_BIT_NEW_DATA;
>>> - else
>>> - ret &= ~BMG160_INT_MAP_1_BIT_NEW_DATA;
>>> -
>>> - ret = i2c_smbus_write_byte_data(data->client,
>>> - BMG160_REG_INT_MAP_1,
>>> - ret);
>>> + ret = regmap_update_bits(data->regmap, BMG160_REG_INT_MAP_1,
>>> + BMG160_INT_MAP_1_BIT_NEW_DATA,
>>> + (status ? BMG160_INT_MAP_1_BIT_NEW_DATA : 0));
>>> if (ret < 0) {
>>> - dev_err(&data->client->dev, "Error writing reg_int_map1\n");
>>> + dev_err(&data->client->dev, "Error updating bits in reg_int_map1\n");
>>> return ret;
>>> }
>>>
>>> if (status) {
>>> - ret = i2c_smbus_write_byte_data(data->client,
>>> - BMG160_REG_INT_RST_LATCH,
>>> - BMG160_INT_MODE_NON_LATCH_INT |
>>> - BMG160_INT_MODE_LATCH_RESET);
>>> + ret = regmap_write(data->regmap, BMG160_REG_INT_RST_LATCH,
>>> + BMG160_INT_MODE_NON_LATCH_INT |
>>> + BMG160_INT_MODE_LATCH_RESET);
>>> if (ret < 0) {
>>> dev_err(&data->client->dev,
>>> "Error writing reg_rst_latch\n");
>>> return ret;
>>> }
>>>
>>> - ret = i2c_smbus_write_byte_data(data->client,
>>> - BMG160_REG_INT_EN_0,
>>> - BMG160_DATA_ENABLE_INT);
>>> + ret = regmap_write(data->regmap, BMG160_REG_INT_EN_0,
>>> + BMG160_DATA_ENABLE_INT);
>>>
>>> } else {
>>> /* Restore interrupt mode */
>>> - ret = i2c_smbus_write_byte_data(data->client,
>>> - BMG160_REG_INT_RST_LATCH,
>>> - BMG160_INT_MODE_LATCH_INT |
>>> - BMG160_INT_MODE_LATCH_RESET);
>>> + ret = regmap_write(data->regmap, BMG160_REG_INT_RST_LATCH,
>>> + BMG160_INT_MODE_LATCH_INT |
>>> + BMG160_INT_MODE_LATCH_RESET);
>>> if (ret < 0) {
>>> dev_err(&data->client->dev,
>>> "Error writing reg_rst_latch\n");
>>> return ret;
>>> }
>>>
>>> - ret = i2c_smbus_write_byte_data(data->client,
>>> - BMG160_REG_INT_EN_0,
>>> - 0);
>>> + ret = regmap_write(data->regmap, BMG160_REG_INT_EN_0, 0);
>>> }
>>>
>>> if (ret < 0) {
>>> @@ -444,10 +411,8 @@ static int bmg160_set_scale(struct bmg160_data *data, int val)
>>>
>>> for (i = 0; i < ARRAY_SIZE(bmg160_scale_table); ++i) {
>>> if (bmg160_scale_table[i].scale == val) {
>>> - ret = i2c_smbus_write_byte_data(
>>> - data->client,
>>> - BMG160_REG_RANGE,
>>> - bmg160_scale_table[i].dps_range);
>>> + ret = regmap_write(data->regmap, BMG160_REG_RANGE,
>>> + bmg160_scale_table[i].dps_range);
>>> if (ret < 0) {
>>> dev_err(&data->client->dev,
>>> "Error writing reg_range\n");
>>> @@ -464,6 +429,7 @@ static int bmg160_set_scale(struct bmg160_data *data, int val)
>>> static int bmg160_get_temp(struct bmg160_data *data, int *val)
>>> {
>>> int ret;
>>> + unsigned int raw_val;
>>>
>>> mutex_lock(&data->mutex);
>>> ret = bmg160_set_power_state(data, true);
>>> @@ -472,7 +438,7 @@ static int bmg160_get_temp(struct bmg160_data *data, int *val)
>>> return ret;
>>> }
>>>
>>> - ret = i2c_smbus_read_byte_data(data->client, BMG160_REG_TEMP);
>>> + ret = regmap_read(data->regmap, BMG160_REG_TEMP, &raw_val);
>>> if (ret < 0) {
>>> dev_err(&data->client->dev, "Error reading reg_temp\n");
>>> bmg160_set_power_state(data, false);
>>> @@ -480,7 +446,7 @@ static int bmg160_get_temp(struct bmg160_data *data, int *val)
>>> return ret;
>>> }
>>>
>>> - *val = sign_extend32(ret, 7);
>>> + *val = sign_extend32(raw_val, 7);
>>> ret = bmg160_set_power_state(data, false);
>>> mutex_unlock(&data->mutex);
>>> if (ret < 0)
>>> @@ -492,6 +458,7 @@ static int bmg160_get_temp(struct bmg160_data *data, int *val)
>>> static int bmg160_get_axis(struct bmg160_data *data, int axis, int *val)
>>> {
>>> int ret;
>>> + unsigned int raw_val;
>>>
>>> mutex_lock(&data->mutex);
>>> ret = bmg160_set_power_state(data, true);
>>> @@ -500,7 +467,8 @@ static int bmg160_get_axis(struct bmg160_data *data, int axis, int *val)
>>> return ret;
>>> }
>>>
>>> - ret = i2c_smbus_read_word_data(data->client, BMG160_AXIS_TO_REG(axis));
>>> + ret = regmap_bulk_read(data->regmap, BMG160_AXIS_TO_REG(axis), &raw_val,
>>> + 2);
>>> if (ret < 0) {
>>> dev_err(&data->client->dev, "Error reading axis %d\n", axis);
>>> bmg160_set_power_state(data, false);
>>> @@ -508,7 +476,7 @@ static int bmg160_get_axis(struct bmg160_data *data, int axis, int *val)
>>> return ret;
>>> }
>>>
>>> - *val = sign_extend32(ret, 15);
>>> + *val = sign_extend32(raw_val, 15);
>>> ret = bmg160_set_power_state(data, false);
>>> mutex_unlock(&data->mutex);
>>> if (ret < 0)
>>> @@ -807,12 +775,13 @@ static irqreturn_t bmg160_trigger_handler(int irq, void *p)
>>> struct iio_dev *indio_dev = pf->indio_dev;
>>> struct bmg160_data *data = iio_priv(indio_dev);
>>> int bit, ret, i = 0;
>>> + unsigned int val;
>>>
>>> mutex_lock(&data->mutex);
>>> for_each_set_bit(bit, indio_dev->active_scan_mask,
>>> indio_dev->masklength) {
>>> - ret = i2c_smbus_read_word_data(data->client,
>>> - BMG160_AXIS_TO_REG(bit));
>>> + ret = regmap_bulk_read(data->regmap, BMG160_AXIS_TO_REG(bit),
>>> + &val, 2);
>>> if (ret < 0) {
>>> mutex_unlock(&data->mutex);
>>> goto err;
>>> @@ -840,10 +809,9 @@ static int bmg160_trig_try_reen(struct iio_trigger *trig)
>>> return 0;
>>>
>>> /* Set latched mode interrupt and clear any latched interrupt */
>>> - ret = i2c_smbus_write_byte_data(data->client,
>>> - BMG160_REG_INT_RST_LATCH,
>>> - BMG160_INT_MODE_LATCH_INT |
>>> - BMG160_INT_MODE_LATCH_RESET);
>>> + ret = regmap_write(data->regmap, BMG160_REG_INT_RST_LATCH,
>>> + BMG160_INT_MODE_LATCH_INT |
>>> + BMG160_INT_MODE_LATCH_RESET);
>>> if (ret < 0) {
>>> dev_err(&data->client->dev, "Error writing reg_rst_latch\n");
>>> return ret;
>>> @@ -907,33 +875,34 @@ static irqreturn_t bmg160_event_handler(int irq, void *private)
>>> struct bmg160_data *data = iio_priv(indio_dev);
>>> int ret;
>>> int dir;
>>> + unsigned int val;
>>>
>>> - ret = i2c_smbus_read_byte_data(data->client, BMG160_REG_INT_STATUS_2);
>>> + ret = regmap_read(data->regmap, BMG160_REG_INT_STATUS_2, &val);
>>> if (ret < 0) {
>>> dev_err(&data->client->dev, "Error reading reg_int_status2\n");
>>> goto ack_intr_status;
>>> }
>>>
>>> - if (ret & 0x08)
>>> + if (val & 0x08)
>>> dir = IIO_EV_DIR_RISING;
>>> else
>>> dir = IIO_EV_DIR_FALLING;
>>>
>>> - if (ret & BMG160_ANY_MOTION_BIT_X)
>>> + if (val & BMG160_ANY_MOTION_BIT_X)
>>> iio_push_event(indio_dev, IIO_MOD_EVENT_CODE(IIO_ANGL_VEL,
>>> 0,
>>> IIO_MOD_X,
>>> IIO_EV_TYPE_ROC,
>>> dir),
>>> iio_get_time_ns());
>>> - if (ret & BMG160_ANY_MOTION_BIT_Y)
>>> + if (val & BMG160_ANY_MOTION_BIT_Y)
>>> iio_push_event(indio_dev, IIO_MOD_EVENT_CODE(IIO_ANGL_VEL,
>>> 0,
>>> IIO_MOD_Y,
>>> IIO_EV_TYPE_ROC,
>>> dir),
>>> iio_get_time_ns());
>>> - if (ret & BMG160_ANY_MOTION_BIT_Z)
>>> + if (val & BMG160_ANY_MOTION_BIT_Z)
>>> iio_push_event(indio_dev, IIO_MOD_EVENT_CODE(IIO_ANGL_VEL,
>>> 0,
>>> IIO_MOD_Z,
>>> @@ -943,10 +912,9 @@ static irqreturn_t bmg160_event_handler(int irq, void *private)
>>>
>>> ack_intr_status:
>>> if (!data->dready_trigger_on) {
>>> - ret = i2c_smbus_write_byte_data(data->client,
>>> - BMG160_REG_INT_RST_LATCH,
>>> - BMG160_INT_MODE_LATCH_INT |
>>> - BMG160_INT_MODE_LATCH_RESET);
>>> + ret = regmap_write(data->regmap, BMG160_REG_INT_RST_LATCH,
>>> + BMG160_INT_MODE_LATCH_INT |
>>> + BMG160_INT_MODE_LATCH_RESET);
>>> if (ret < 0)
>>> dev_err(&data->client->dev,
>>> "Error writing reg_rst_latch\n");
>>> @@ -1038,6 +1006,14 @@ static int bmg160_probe(struct i2c_client *client,
>>> struct iio_dev *indio_dev;
>>> int ret;
>>> const char *name = NULL;
>>> + struct regmap *regmap;
>>> +
>>> + regmap = devm_regmap_init_i2c(client, &bmg160_regmap_i2c_conf);
>>> + if (IS_ERR(regmap)) {
>>> + dev_err(&client->dev, "Failed to register i2c regmap %d\n",
>>> + (int)PTR_ERR(regmap));
>>> + return PTR_ERR(regmap);
>>> + }
>>>
>>> indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
>>> if (!indio_dev)
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/