On Fri, Mar 4, 2016 at 1:09 AM, Peter Rosin <peda@xxxxxxxxxxxxxx> wrote: > From: Peter Rosin <peda@xxxxxxxxxx> > > Hi Daniel, > > Daniel Baluta wrote: >>On Mon, Feb 22, 2016 at 1:17 AM, Wolfram Sang wrote: >>> On Thu, Feb 18, 2016 at 05:53:05PM +0200, Daniel Baluta wrote: >>>> Sending this as an RFC because I don't know if style fixes are appropriate >>>> for this driver and also not sure if deadlock fix is the best solution. > > *snip* > >>> We recently had a bigger patch series fixing locking problems related to >>> muxes. I sadly didn't have the time to review it. Can you have a look if >>> it helps your case? >>> >>> http://thread.gmane.org/gmane.linux.drivers.i2c/26169 >> >> Tested this and the deadlock is still there :(. > > I assume that when you tested v3 of my series, you did nothing > to actually make use of the new stuff available in the mux-core? > If you didn't do anything to make use of the new stuff, the > driver should behave as before. You are right. I used v3 as it is. > > So, please try this patch on top of my recently posted v4 of the > "i2c mux cleanup and locking update" series [1]. I have not tested > this patch at all, but I have the feeling it could do the trick... This patch fixes the problem! :P > Cheers, > Peter > > [1] https://marc.info/?l=linux-iio&m=145704469628330&w=3 > > diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c > index 642f22013d17..02b56e631973 100644 > --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c > +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c > @@ -79,35 +79,6 @@ int inv_mpu6050_write_reg(struct inv_mpu6050_state *st, int reg, u8 d) > return i2c_smbus_write_i2c_block_data(st->client, reg, 1, &d); > } > > -/* > - * The i2c read/write needs to happen in unlocked mode. As the parent > - * adapter is common. If we use locked versions, it will fail as > - * the mux adapter will lock the parent i2c adapter, while calling > - * select/deselect functions. > - */ > -static int inv_mpu6050_write_reg_unlocked(struct inv_mpu6050_state *st, > - u8 reg, u8 d) > -{ > - int ret; > - u8 buf[2]; > - struct i2c_msg msg[1] = { > - { > - .addr = st->client->addr, > - .flags = 0, > - .len = sizeof(buf), > - .buf = buf, > - } > - }; > - > - buf[0] = reg; > - buf[1] = d; > - ret = __i2c_transfer(st->client->adapter, msg, 1); > - if (ret != 1) > - return ret; > - > - return 0; > -} > - > static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id) > { > struct iio_dev *indio_dev = i2c_mux_priv(muxc); > @@ -117,8 +88,7 @@ static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id) > /* Use the same mutex which was used everywhere to protect power-op */ > mutex_lock(&indio_dev->mlock); > if (!st->powerup_count) { > - ret = inv_mpu6050_write_reg_unlocked(st, st->reg->pwr_mgmt_1, > - 0); > + ret = inv_mpu6050_write_reg(st, st->reg->pwr_mgmt_1, 0); > if (ret) > goto write_error; > > @@ -126,9 +96,9 @@ static int inv_mpu6050_select_bypass(struct i2c_mux_core *muxc, u32 chan_id) > } > if (!ret) { > st->powerup_count++; > - ret = inv_mpu6050_write_reg_unlocked(st, st->reg->int_pin_cfg, > - st->client->irq | > - INV_MPU6050_BIT_BYPASS_EN); > + ret = inv_mpu6050_write_reg(st, st->reg->int_pin_cfg, > + st->client->irq | > + INV_MPU6050_BIT_BYPASS_EN); > } > write_error: > mutex_unlock(&indio_dev->mlock); > @@ -143,12 +113,11 @@ static int inv_mpu6050_deselect_bypass(struct i2c_mux_core *muxc, u32 chan_id) > > mutex_lock(&indio_dev->mlock); > /* It doesn't really mattter, if any of the calls fails */ > - inv_mpu6050_write_reg_unlocked(st, st->reg->int_pin_cfg, > - st->client->irq); > + inv_mpu6050_write_reg(st, st->reg->int_pin_cfg, st->client->irq); > st->powerup_count--; > if (!st->powerup_count) > - inv_mpu6050_write_reg_unlocked(st, st->reg->pwr_mgmt_1, > - INV_MPU6050_BIT_SLEEP); > + inv_mpu6050_write_reg(st, st->reg->pwr_mgmt_1, > + INV_MPU6050_BIT_SLEEP); > mutex_unlock(&indio_dev->mlock); > > return 0; > @@ -839,8 +808,8 @@ static int inv_mpu_probe(struct i2c_client *client, > goto out_remove_trigger; > } > > - st->muxc = i2c_mux_one_adapter(client->adapter, &client->dev, 0, 0, > - 0, 0, 0, > + st->muxc = i2c_mux_one_adapter(client->adapter, &client->dev, 0, > + I2C_CONTROLLED_MUX, 0, 0, 0, > inv_mpu6050_select_bypass, > inv_mpu6050_deselect_bypass); > if (IS_ERR(st->muxc)) { > -- > 2.1.4 > > -- > 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-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
