Re: [PATCH v4 2/3] iio: imu: inv_icm42600: log whoami mismatch instead of failing probe
From: Andy Shevchenko
Date: Mon Aug 17 2026 - 04:29:56 EST
On Mon, Aug 17, 2026 at 03:18:59AM +0100, Jonathan Cameron wrote:
> On Mon, 10 Aug 2026 22:45:28 +0300
> Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> wrote:
> > On Mon, Aug 10, 2026 at 09:11:56PM +0200, Jorijn van der Graaf wrote:
...
> > > if (val != hw->whoami) {
> > > - dev_err(dev, "invalid whoami %#02x expected %#02x (%s)\n",
> > > - val, hw->whoami, hw->name);
> > > - return -ENODEV;
> > > + /*
> > > + * SPI interface has no ack mechanism.
> > > + * 0xFF or 0x00 whoami means no response from the device.
> > > + */
> > > + if (val == U8_MAX || val == 0)
> >
> > This is strange to be here. If hw->whoami happens to be 0x00 or 0xff (let's
> > assume some hypothetical future case), this check will become invalid for them.
> >
> > If it's guaranteed to be not the case for any HW (including the future variants)
> > this check should be outside, no?
>
> Hmm. This one has both worked in favour of finding connection trouble
> and bitten us in the past. I vaguely recall a device that indeed
> had a WHOAMI of 0 (though it might have been a hardware bug) and
> tripped on such a check.
>
> Personally I'm not that fixed either way on this. If the device isn't
> there we should know the moment we try to get any data anyway.
My point is that the nesting checks are not needed (assuming 0x00 and 0xff are
bad values for WHOAMI).
Something like
/* ...the comment... */
if (val == U8_MAX || val == 0)
...
if (val != hw->whoami) {
...
}
> > > + return dev_err_probe(dev, -ENODEV,
> > > + "invalid whoami %#04x expected %#04x (%s)\n",
> > > + val, hw->whoami, hw->name);
--
With Best Regards,
Andy Shevchenko