Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753431AbbHOOnG (ORCPT ); Sat, 15 Aug 2015 10:43:06 -0400 Received: from saturn.retrosnub.co.uk ([178.18.118.26]:47459 "EHLO saturn.retrosnub.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752174AbbHOOnD (ORCPT ); Sat, 15 Aug 2015 10:43:03 -0400 Subject: Re: [PATCH v3 1/6] iio: bmg160: Use i2c regmap instead of direct i2c access To: Srinivas Pandruvada , Markus Pargmann References: <1439391009-6051-1-git-send-email-mpa@pengutronix.de> <1439391009-6051-2-git-send-email-mpa@pengutronix.de> <1439395274.2244.34.camel@spandruv-desk3.jf.intel.com> <55CF4F94.1090903@kernel.org> Cc: Vlad Dogaru , Paul Bolle , linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, kernel@pengutronix.de From: Jonathan Cameron Message-ID: <55CF4FF6.8060204@kernel.org> Date: Sat, 15 Aug 2015 15:43:02 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <55CF4F94.1090903@kernel.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 17950 Lines: 490 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 >> Reviewed-by: Srinivas Pandruvada > 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 >>> #include >>> #include >>> +#include >>> >>> #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@vger.kernel.org > 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/