Re: [PATCH v2 2/4] iio: imu: inv_icm42600: warn on whoami mismatch instead of failing probe

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] Demoting the WHOAMI check to a warning allows the driver to successfully probe on a disconnected or floating SPI bus, allocating resources for a phantom device.

Pre-existing issues:
- [High] The VDDIO regulator enable count is leaked on probe error paths because the devres cleanup function checks the wrong runtime PM state.
- [High] Devres teardown ordering is incorrect because IIO devices are registered before Runtime PM is initialized, creating a race condition on driver unbind.
--

--- Patch [2]: [PATCH v2 2/4] iio: imu: inv_icm42600: warn on whoami mismatch instead of failing probe ---
commit 8842cf41876131adceea9740e2e506d7567dcc89
Author: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>

iio: imu: inv_icm42600: warn on whoami mismatch instead of failing probe

This changes the inv_icm42600 driver to warn instead of failing the probe
when a WHOAMI mismatch occurs. This allows register-compatible parts 
described with fallback compatibles to probe successfully. The driver now 
relies on the devicetree compatible as the authoritative source for the 
fitted part.

Link: https://lore.kernel.org/all/20260722-creature-volley-0f083b904c1d@spud/

> diff --git a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> index dc97d8a274e36..f306e3ffca486 100644
> --- a/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> +++ b/drivers/iio/imu/inv_icm42600/inv_icm42600_core.c
> @@ -509,11 +509,9 @@ static int inv_icm42600_setup(struct inv_icm42600_state *st,
>  	ret = regmap_read(st->map, INV_ICM42600_REG_WHOAMI, &val);
>  	if (ret)
>  		return ret;
> -	if (val != hw->whoami) {
> -		dev_err(dev, "invalid whoami %#02x expected %#02x (%s)\n",
> -			val, hw->whoami, hw->name);
> -		return -ENODEV;
> -	}
> +	if (val != hw->whoami)
> +		dev_warn(dev, "unexpected whoami %#02x, expected %#02x (%s)\n",
> +			 val, hw->whoami, hw->name);

[Severity: Medium]
Will this allow a disconnected SPI bus to successfully probe a phantom device?

If the SPI bus is disconnected, a floating MISO often returns 0xFF for all
reads. With this check demoted to a warning, the driver proceeds to read
the reset status:

drivers/iio/imu/inv_icm42600/inv_icm42600_core.c:inv_icm42600_setup() {
    ...
    ret = regmap_read(st->map, INV_ICM42600_REG_INT_STATUS, &val);
    ...
    if (!(val & INV_ICM42600_INT_STATUS_RESET_DONE)) {
        dev_err(dev, "reset error, reset done bit not set\n");
        return -ENODEV;
    }
...

Since 0xFF & INV_ICM42600_INT_STATUS_RESET_DONE evaluates to true, the reset
check passes and the driver allocates IIO devices, IRQs, and memory for a
non-existent device. Could extreme values like 0xFF or 0x00 still be treated
as errors to abort the probe?

[Severity: High]
This is a pre-existing issue, but does a probe failure leak the VDDIO
regulator enable count?

If the probe fails before devm_pm_runtime_set_active_enabled() is called 
(for instance, during inv_icm42600_setup), devres executes the cleanup 
function:

drivers/iio/imu/inv_icm42600/inv_icm42600_core.c:inv_icm42600_disable_vddio_reg() {
    if (pm_runtime_status_suspended(dev))
        return;

    regulator_disable(st->vddio_supply);
}

Because the device's default PM status is RPM_SUSPENDED before runtime PM is
enabled, won't this incorrectly return early and leave the regulator
permanently enabled?

[Severity: High]
This isn't a bug introduced by this patch, but could there be a race condition
on driver unbind due to devres teardown ordering?

In inv_icm42600_core_probe(), IIO devices are registered before runtime PM is
initialized:

drivers/iio/imu/inv_icm42600/inv_icm42600_core.c:inv_icm42600_core_probe() {
    ...
    st->indio_gyro = inv_icm42600_gyro_init(st);
    ...
    ret = devm_pm_runtime_set_active_enabled(dev);
    ...
}

On teardown, runtime PM is disabled before the IIO devices are unregistered.
If the device was runtime suspended (VDDIO off) and userspace reads a sysfs
attribute during this window, we can hit this path:

drivers/iio/imu/inv_icm42600/inv_icm42600_gyro.c:inv_icm42600_gyro_read_sensor() {
    ...
    pm_runtime_get_sync(dev);
    mutex_lock(&st->lock);
    ...
    ret = inv_icm42600_set_gyro_conf(st, &conf, NULL);
    ...
}

Since runtime PM is disabled, pm_runtime_get_sync() fails, but its return
value is ignored. Doesn't this cause the driver to access the regmap while
VDDIO is powered off, potentially leading to bus timeouts or lockups?

>  	st->name = hw->name;
>  
>  	/* reset to make sure previous state are not there */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724202708.128267-1-jorijnvdgraaf@catcrafts.net?part=2