Re: [PATCH v5 1/8] iio: imu: adis: Fix custom reset dispatch in adislib

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] iio: imu: adis: incorrect Fixes tag and stale changelog in reset dispatch fix
--

commit 5c3f86b0c40a0ca82129c2f9586aae278b0f7d2a
Author: Radu Sabau <radu.sabau@analog.com>

iio: imu: adis: Fix custom reset dispatch in adislib

This commit updates __adis_initial_startup() and adis_reset() to dispatch
software resets through adis->ops->reset() rather than calling
__adis_reset() directly, and moves the sw_reset_ms settle delay out of
__adis_reset() into both callers.

>     iio: imu: adis: Fix custom reset dispatch in adislib
>     
>     __adis_initial_startup()'s no-reset-gpio path called __adis_reset()
>     directly instead of adis->ops->reset(), silently ignoring any driver's
>     custom reset implementation. Invisible until now since
>     adis_default_ops.reset is __adis_reset() itself. Dispatch through
>     adis->ops->reset() instead - the same bug existed in the public
>     adis_reset() wrapper, fixed here too.
>     
>     Move the settle-time sleep out of __adis_reset() and into both call
>     sites instead, so every reset op (default or custom) is treated
>     uniformly: adis->ops->reset() is expected to just do the write, and
>     the library always sleeps the settle time afterwards. This removes
>     the previous asymmetric "sleep here only for custom ops" special
>     case entirely.
>     
>     Fixes: ecb010d44108 ("iio: imu: adis: Refactor adis_initial_startup")
>     Reviewed-by: Nuno Sá <nuno.sa@analog.com>
>     Signed-off-by: Radu Sabau <radu.sabau@analog.com>

[Severity: Low]
Should the Fixes tag reference the commit that introduced the custom reset
callback instead?

Commit ecb010d44108 ("iio: imu: adis: Refactor adis_initial_startup")
predates struct adis_ops (added in commit 3b29bcee8f6f ("iio: imu: adis:
Add custom ops struct")). The custom reset callback adis->ops->reset was
introduced in commit 7f15d7a7d12d ("iio: imu: adis: Add reset to custom
ops"), which added reset to struct adis_ops without updating
__adis_initial_startup() or adis_reset() to call adis->ops->reset(). Should
this use:

  Fixes: 7f15d7a7d12d ("iio: imu: adis: Add reset to custom ops")

Also, is the commit message description accurate for the current baseline
tree?

The first paragraph mentions that the issue was invisible until now, but
drivers/iio/imu/adis16550.c already defines a custom reset callback in
adis16550_ops:

static const struct adis_ops adis16550_ops = {
	.write = adis16550_spi_write,
	.read = adis16550_spi_read,
	.reset = adis16550_reset,
};

In addition, the second paragraph states that this change removes a
previous asymmetric "sleep here only for custom ops" special case, which
does not exist in the baseline code prior to this patch. Could the commit
message be updated to reflect the current tree state?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-adis16607-v5-0-4f98b0d271ac@analog.com?part=1