Re: [PATCH v2] iio: imu: st_lsm6dsx: deselect shub page before reading whoami

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3349 bytes --]

> As part of driver initialization, e.g. st_lsm6dsx_init_shub() selects
> the shub register page using st_lsm6dsx_set_page(). Selecting the shub
> register page shadows the regular register space so whoami, among other
> registers, is no longer accessible.
> 
> In applications where the IMU is permanently powered separately from the
> processor, there is a window where a reset of the CPU leaves the IMU in
> the shub register page. Once this occurs, any subsequent probe attempt
> fails because of the register shadowing.
> 
> Using the ism330dlc, the error typically looks like
> 
>     st_lsm6dsx_i2c 3-006a: unsupported whoami [10]
> 
> with the unknown whoami read from a reserved register in the shub page.
> 
> The reset register is also shadowed by the page select, preventing a
> reset from recovering the chip.
> 
> Unconditionally clear the shub page before the whoami readout to ensure
> normal register access and allow the initialization to proceed.
> 
> Place the fix in st_lsm6dsx_check_whoami() before the whoami check
> because hw->settings, which st_lsm6dsx_set_page() relies on, is first
> assigned in that function.
> 
> Placing the fix in a more logical place than the whoami check would
> require a bigger restructuring of the code.
> 
> Signed-off-by: Andreas Kempe <andreas.kempe@actia.se>
> ---
> 
>  Changes in v2:
>    - Drop st_lsm6dsx_get_page() and unconditionally clear the shub page.
>    - Make the code comment clearer.
>    - Document the placement rationale in the commit message.
>    - Link to v1: https://lore.kernel.org/linux-iio/20260604132646.1099072-1-andreas.kempe@actia.se/
> 
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 21 +++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)

Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>

> 
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> index 630e2cae6f19..f4edcb73ec8c 100644
> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -1712,6 +1712,26 @@ static int st_lsm6dsx_check_whoami(struct st_lsm6dsx_hw *hw, int id,
>  		return -ENODEV;
>  	}
>  
> +	hw->settings = &st_lsm6dsx_sensor_settings[i];
> +
> +	if (hw->settings->shub_settings.page_mux.addr) {
> +		/*
> +		 * If the IMU has the shub page selected on init, for example
> +		 * after a CPU watchdog reset while the page is selected, the
> +		 * regular register space is shadowed. While the regular
> +		 * register space is shadowed, the registers needed for
> +		 * initializing the IMU are not available.
> +		 *
> +		 * Unconditionally clear the shub page selection to ensure
> +		 * normal register access.
> +		 */
> +		err = st_lsm6dsx_set_page(hw, false);
> +		if (err < 0) {
> +			dev_err(hw->dev, "failed to clear shub page\n");
> +			return err;
> +		}
> +	}
> +
>  	err = regmap_read(hw->regmap, ST_LSM6DSX_REG_WHOAMI_ADDR, &data);
>  	if (err < 0) {
>  		dev_err(hw->dev, "failed to read whoami register\n");
> @@ -1724,7 +1744,6 @@ static int st_lsm6dsx_check_whoami(struct st_lsm6dsx_hw *hw, int id,
>  	}
>  
>  	*name = st_lsm6dsx_sensor_settings[i].id[j].name;
> -	hw->settings = &st_lsm6dsx_sensor_settings[i];
>  
>  	return 0;
>  }
> -- 
> 2.53.0

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]