Re: [PATCH] iio: imu: st_lsm6dsx: disable I3C support during device reset

[Jonathan Cameron <jic23@kernel.org>]

On Sun,  1 Mar 2020 20:28:03 +0100
Lorenzo Bianconi <lorenzo@kernel.org> wrote:

> Disable MIPI I3C during device reset in order to avoid
> possible races on interrupt line 1. If the first interrupt
> line is asserted during hw reset the device will work in
> I3C-only mode
> 
> Reported-by: Mario Tesi <mario.tesi@st.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

Fixes tag?

One trivial thing inline.

Is this something we should be looking to have applied to stable?

Jonathan

> ---
> I have not been able to test this patch on a I3C device, just SPI/I2C
> ---
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h      |  2 ++
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 30 ++++++++++++++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> index f2113a63721a..dfcbe7c42493 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -266,6 +266,7 @@ struct st_lsm6dsx_ext_dev_settings {
>   * @wai: Sensor WhoAmI default value.
>   * @reset: register address for reset.
>   * @boot: register address for boot.
> + * @i3c_disable:  register address for enabling/disabling I3C (addr + mask).
>   * @bdu: register address for Block Data Update.
>   * @max_fifo_size: Sensor max fifo length in FIFO words.
>   * @id: List of hw id/device name supported by the driver configuration.
> @@ -284,6 +285,7 @@ struct st_lsm6dsx_settings {
>  	u8 wai;
>  	struct st_lsm6dsx_reg reset;
>  	struct st_lsm6dsx_reg boot;
> +	struct st_lsm6dsx_reg i3c_disable;
>  	struct st_lsm6dsx_reg bdu;
>  	u16 max_fifo_size;
>  	struct {
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index 84d219ae6aee..b1435c5d2d6d 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -751,6 +751,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  			.addr = 0x12,
>  			.mask = BIT(7),
>  		},
> +		.i3c_disable = {
> +			.addr = 0x18,
> +			.mask = BIT(1),
> +		},
>  		.bdu = {
>  			.addr = 0x12,
>  			.mask = BIT(6),
> @@ -1128,6 +1132,10 @@ static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
>  			.addr = 0x12,
>  			.mask = BIT(7),
>  		},
> +		.i3c_disable = {
> +			.addr = 0x18,
> +			.mask = BIT(1),
> +		},
>  		.bdu = {
>  			.addr = 0x12,
>  			.mask = BIT(6),
> @@ -2041,6 +2049,19 @@ static int st_lsm6dsx_init_device(struct st_lsm6dsx_hw *hw)
>  	const struct st_lsm6dsx_reg *reg;
>  	int err;
>  
> +	/* disable MIPI I3C during device reset in order to avoid

comment syntax
/*
 * disable...

> +	 * possible races on interrupt line 1. If the first interrupt
> +	 * line is asserted during hw reset the device will work in
> +	 * I3C-only mode
> +	 */
> +	if (hw->settings->i3c_disable.addr) {
> +		reg = &hw->settings->i3c_disable;
> +		err = regmap_update_bits(hw->regmap, reg->addr, reg->mask,
> +					 ST_LSM6DSX_SHIFT_VAL(1, reg->mask));
> +		if (err < 0)
> +			return err;
> +	}
> +
>  	/* device sw reset */
>  	reg = &hw->settings->reset;
>  	err = regmap_update_bits(hw->regmap, reg->addr, reg->mask,
> @@ -2059,6 +2080,15 @@ static int st_lsm6dsx_init_device(struct st_lsm6dsx_hw *hw)
>  
>  	msleep(50);
>  
> +	/* enable MIPI I3C */
> +	if (hw->settings->i3c_disable.addr) {
> +		reg = &hw->settings->i3c_disable;
> +		err = regmap_update_bits(hw->regmap, reg->addr, reg->mask,
> +					 ST_LSM6DSX_SHIFT_VAL(0, reg->mask));
> +		if (err < 0)
> +			return err;
> +	}
> +
>  	/* enable Block Data Update */
>  	reg = &hw->settings->bdu;
>  	err = regmap_update_bits(hw->regmap, reg->addr, reg->mask,