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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Fri, Oct 09, 2026 at 04:12:08PM +0200, 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 for
> adding 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.

...

> static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,

>  	s64 disable_wait;
>  	int ret;
>  
> +	/* Disable gyro if has_gyro is not set */
> +	if (!st->hw->has_gyro)
> +		gyro = INV_ICM42607_SENSOR_MODE_OFF;

This doesn't sound as a good style. Instead, I would see the wrapper that calls
this with different settings depending on the has_gyro:


	if (st->hw->has_gyro)
		return inv_icm42607_set_pwr_mgmt0(st, gyro, accel);

	return inv_icm42607_set_pwr_mgmt0(st, INV_ICM42607_SENSOR_MODE_OFF, accel);

...

Side note.

In this function and maybe somewhere else the all *implicit* checks against 0
has to be made explicit, for instance

	if (!gyro && oldgyro) {

has to be

	if (gyro == INV_ICM42607_SENSOR_MODE_OFF && oldgyro != gyro) {

otherwise the choice of the enum type makes less sense.

...

All the same for the 'accel'.

-- 
With Best Regards,
Andy Shevchenko