Re: [PATCH v3 1/6] iio: imu: adis: Add optional self_test callback and fix custom reset dispatch

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] iio: imu: adis: undocumented addition of post_reset_setup callback
--

commit 6c20542e733281223a77056f3199ffa32200b12f
Author: Radu Sabau <radu.sabau@analog.com>

iio: imu: adis: Add optional self_test callback and fix custom reset dispatch

This commit adds an optional self_test callback for devices with custom
test sequences and fixes the no-reset-gpio startup path to use the driver
provided reset ops instead of directly calling __adis_reset().

> diff --git a/drivers/iio/imu/adis.c b/drivers/iio/imu/adis.c
> index a2bc1d14ed91d..448046fd2ada4 100644
> --- a/drivers/iio/imu/adis.c
> +++ b/drivers/iio/imu/adis.c
[ ... ]
> @@ -413,7 +422,20 @@ int __adis_initial_startup(struct adis *adis)
[ ... ]
> -		ret = __adis_reset(adis);
> +		ret = adis->ops->reset(adis);
> +		if (ret)
> +			return ret;
> +		/*
> +		 * __adis_reset() already sleeps the settle time internally.
> +		 * Custom reset ops are documented (struct adis_ops::reset)
> +		 * not to, so sleep here for them.
> +		 */
> +		if (adis->ops->reset != __adis_reset)
> +			msleep(timeouts->sw_reset_ms);
> +	}
> +
> +	if (adis->data->post_reset_setup) {
> +		ret = adis->data->post_reset_setup(adis);

[Severity: Low]
The commit message explicitly details the self_test callback addition and the
changes to the reset dispatch path, but it doesn't mention this new
post_reset_setup callback being added to the initialization sequence. Could
the commit message be updated to document this new API addition?

>  		if (ret)
>  			return ret;
>  	}
[ ... ]
> diff --git a/include/linux/iio/imu/adis.h b/include/linux/iio/imu/adis.h
> index bfb6df68e6c94..5d0baeded3c92 100644
> --- a/include/linux/iio/imu/adis.h
> +++ b/include/linux/iio/imu/adis.h
[ ... ]
> @@ -81,6 +94,8 @@ struct adis_data {
>  	unsigned int self_test_mask;
>  	unsigned int self_test_reg;
>  	bool self_test_no_autoclear;
> +	int (*post_reset_setup)(struct adis *adis);

[Severity: Low]
Similar to the above, should this struct adis_data API addition be mentioned
in the commit message so that developers are aware of the newly available
bring-up hook?

> +	int (*self_test)(struct adis *adis);
>  	const struct adis_timeout *timeouts;
>  
>  	const char * const *status_error_msgs;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-adis16607-v3-0-f972d4efee85@analog.com?part=1