On March 17, 2014 3:40:37 PM GMT+00:00, Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote: >On 03/15/2014 09:01 AM, Jonathan Cameron wrote: >> On 11/03/14 21:04, Srinivas Pandruvada wrote: >>> This chip has two modes to control secondary sensor attached to it. >>> One is master mode and another is bypass mode. In master mode >>> MPU6500 will directly communicates to the secondary sensor device >>> attached to it. This can support very few secondary devices in this >>> mode. >>> But when configured in bypass mode the i2c lines are directly >connected >>> to host i2c bus controller. >>> Since in master mode it can only support few devices and they are >not >>> implemented in this driver, set the default mode to bypass mode. >>> >>> Signed-off-by: Srinivas Pandruvada ><srinivas.pandruvada@xxxxxxxxxxxxxxx> >> This has been proposed before by Manuel Stahl, >> >> Please read the thread from >> >> http://www.spinics.net/lists/linux-iio/msg11985.html >> >> In short, the best way we have come up with for handling this cleanly >> (avoiding issues with the device in between going into sleep etc) was >> to implement an i2c bus multiplexer with 1 input and 1 output as part >of >> the driver. >> >> Manuel, any progress on your patches? >> > >I think the mux is the right solution for using master mode of MPU60X0. >But I feel that when this driver is initialized then we should disable >the master mode at init >as currently it doesn't have any implementation for slave devices as >Invensense typically do for Android devices. Agreed that disabling effectively unused master mode makes sense. >Once a client want to use multiplexer then it can be turned on. Yes turn on bypass as needed... Or is it a case of one control for master or bypass? If so then seems like bypass will do nothing unless the MUX is enabled so makes sense to init in bypass mode. >Invensense engineer (author of this driver) recommends not to use >master >mode. > >So my patch disables at init. I am calling this function from init. I >can remove export of symbol. > >What do you think about this? All sounds sensible to me. > >Thanks, >Srinivas > > >> Jonathan >>> --- >>> drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 20 >+++++++++++++++++++- >>> drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h | 4 ++++ >>> 2 files changed, 23 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c >>> b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c >>> index 4a76697..6ba565a 100644 >>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c >>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c >>> @@ -61,6 +61,7 @@ static const struct inv_mpu6050_reg_map >>> reg_set_6050 = { >>> .int_enable = INV_MPU6050_REG_INT_ENABLE, >>> .pwr_mgmt_1 = INV_MPU6050_REG_PWR_MGMT_1, >>> .pwr_mgmt_2 = INV_MPU6050_REG_PWR_MGMT_2, >>> + .int_pin_cfg = INV_MPU6050_REG_INT_PIN_CFG, >>> }; >>> >>> static const struct inv_mpu6050_chip_config chip_config_6050 = { >>> @@ -616,6 +617,21 @@ static const struct iio_info mpu_info = { >>> .validate_trigger = inv_mpu6050_validate_trigger, >>> }; >>> >>> +int inv_set_bypass_status(struct inv_mpu6050_state *st, bool >enable) >>> +{ >>> + int ret; >>> + >>> + if (enable) >>> + ret = inv_mpu6050_write_reg(st, st->reg->int_pin_cfg, >>> + st->client->irq | >>> + INV_MPU6050_BIT_BYPASS_EN); >>> + else >>> + ret = inv_mpu6050_write_reg(st, st->reg->int_pin_cfg, >>> + st->client->irq); >>> + return ret; >>> +} >>> +EXPORT_SYMBOL_GPL(inv_set_bypass_status); >>> + >>> /** >>> * inv_check_and_setup_chip() - check and setup chip. >>> */ >>> @@ -654,7 +670,9 @@ static int inv_check_and_setup_chip(struct >>> inv_mpu6050_state *st, >>> if (result) >>> return result; >>> >>> - return 0; >>> + result = inv_set_bypass_status(st, true); >>> + >>> + return result; >>> } >>> >>> /** >>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h >>> b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h >>> index f383955..de5aa22 100644 >>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h >>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h >>> @@ -54,6 +54,7 @@ struct inv_mpu6050_reg_map { >>> u8 int_enable; >>> u8 pwr_mgmt_1; >>> u8 pwr_mgmt_2; >>> + u8 int_pin_cfg; >>> }; >>> >>> /*device enum */ >>> @@ -185,6 +186,9 @@ struct inv_mpu6050_state { >>> #define INV_MPU6050_MIN_FIFO_RATE 4 >>> #define INV_MPU6050_ONE_K_HZ 1000 >>> >>> +#define INV_MPU6050_REG_INT_PIN_CFG 0x37 >>> +#define INV_MPU6050_BIT_BYPASS_EN 0x2 >>> + >>> /* scan element definition */ >>> enum inv_mpu6050_scan { >>> INV_MPU6050_SCAN_ACCL_X, >>> >> >> > >-- >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 -- Sent from my Android phone with K-9 Mail. Please excuse my brevity. -- 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