Re: [PATCH v5 3/6] iio: imu: inv_icm42607: Initialize gyro based on chip_info

[Marcelo Schmitt]

On 10/02, Kanak Shilledar wrote:
> Update the chip_info struct with a new `has_gyro` property to support,
> devices which do not have gyro functionality. This is a precursor to the
> next commit which adds support for the Invensense, ICM-42370-P. It is
> similar to the existing device except it only has accelerometer. Check
> all operations related to gyro with the boolean.
> 
> Signed-off-by: Kanak Shilledar <kanak.shilledar@xxxxxxxx>
> ---
>  drivers/iio/imu/inv_icm42607/inv_icm42607.h      |  1 +
>  drivers/iio/imu/inv_icm42607/inv_icm42607_core.c | 41 +++++++++++++++---------
>  2 files changed, 26 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607.h b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> index 4d51b0da1aa16..e4075288247b8 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> @@ -130,6 +130,7 @@ struct inv_icm42607_hw {
>  	const char *name;
>  	const struct inv_icm42607_conf *conf;
>  	u8 whoami;
> +	bool has_gyro;
>  };
>  

Might not be an issue but, don't we need to check has_gyro in
inv_icm42607_set_pwr_mgmt0() and inv_icm42607_get_pwr_mgmt0() as well? 
If I'm not missing anything, a sample read could cause the device driver to
read/write a reserved pair of bits (reserved for ICM-42370-P on the next patch).
inv_icm42607_accel_read_raw() -> inv_icm42607_read_sensor() ->
	inv_icm42607_set_sensor_conf() -> inv_icm42607_get_pwr_mgmt0()

On the read side, a simple safeguard could be
	if (st->hw->has_gyro)
		*gyro = FIELD_GET(INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK, val);

With best regards,
Marcelo