[PATCH v16 05/10] iio: imu: inv_icm42607: Add SPI For icm42607

[Andy Shevchenko]

On Mon, Jul 13, 2026 at 04:58:35PM -0500, Chris Morgan wrote:

> Add SPI driver support for InvenSense ICM-42607 devices.

A few nit-picks below, otherwise
Reviewed-by: Andy Shevchenko <andriy.shevchenko at intel.com>

...

> +static int inv_icm42607_spi_bus_setup(struct inv_icm42607_state *st)
> +{
> +	unsigned int val;
> +	int ret;
> +
> +	/* Only support 4-wire mode for now. */
> +	ret = regmap_set_bits(st->map, INV_ICM42607_REG_DEVICE_CONFIG,
> +				      INV_ICM42607_DEVICE_CONFIG_SPI_AP_4WIRE);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_clear_bits(st->map, INV_ICM42607_REG_INTF_CONFIG1,
> +				INV_ICM42607_INTF_CONFIG1_I3C_DDR_EN |
> +				INV_ICM42607_INTF_CONFIG1_I3C_SDR_EN);
> +	if (ret)
> +		return ret;
> +
> +	val = FIELD_PREP(INV_ICM42607_DRIVE_CONFIG3_SPI_MASK,
> +			 INV_ICM42607_SLEW_RATE_2NS);
> +	ret = regmap_update_bits(st->map, INV_ICM42607_REG_DRIVE_CONFIG3,
> +				 INV_ICM42607_DRIVE_CONFIG3_SPI_MASK, val);
> +	if (ret)
> +		return ret;
> +
> +	val = FIELD_PREP(INV_ICM42607_INTF_CONFIG0_UI_SIFS_CFG_MASK,
> +			 INV_ICM42607_INTF_CONFIG0_UI_SIFS_CFG_I2C_DIS);

+ blank line?
(Not sure, this is an interesting case and I have no strong opinion on
 either style.)

> +	return regmap_update_bits(st->map, INV_ICM42607_REG_INTF_CONFIG0,

> +				  INV_ICM42607_INTF_CONFIG0_UI_SIFS_CFG_MASK,
> +				  val);

This can be a single (a bit longer) line.

> +}

...

> +static int inv_icm42607_probe(struct spi_device *spi)
> +{
> +	const struct inv_icm42607_hw *hw;
> +	struct device *dev = &spi->dev;
> +	struct regmap *regmap;
> +
> +	hw = spi_get_device_match_data(spi);
> +	if (!hw)
> +		return dev_err_probe(dev, -ENODEV, "Failed to get SPI data\n");
> +
> +	if (spi->mode & SPI_3WIRE)
> +		return dev_err_probe(dev, -ENODEV, "SPI 3-wire mode not supported\n");

Hmm... I would distinguish these two by the error code, perhaps -ENODATA for no
match data?

> +	regmap = devm_regmap_init_spi(spi, &inv_icm42607_regmap_config);
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(dev, PTR_ERR(regmap),
> +				     "Failed to register spi regmap\n");

> +	return inv_icm42607_core_probe(regmap, hw,
> +				       inv_icm42607_spi_bus_setup);

This is perfectly a single line.

> +}

-- 
With Best Regards,
Andy Shevchenko