* [PATCH next] iio: imu: inv_icm45600: Add a missing return statement in probe()
@ 2025-10-22 11:02 Dan Carpenter
2025-10-22 15:13 ` David Lechner
2025-10-27 14:23 ` Jonathan Cameron
0 siblings, 2 replies; 5+ messages in thread
From: Dan Carpenter @ 2025-10-22 11:02 UTC (permalink / raw)
To: Remi Buisson
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
linux-iio, linux-kernel, kernel-janitors
The intention here was clearly to return -ENODEV but the return statement
was missing. It would result in an off by one read in i3c_chip_info[] on
the next line. Add the return statement.
Fixes: 1bef24e9007e ("iio: imu: inv_icm45600: add I3C driver for inv_icm45600 driver")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
drivers/iio/imu/inv_icm45600/inv_icm45600_i3c.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/imu/inv_icm45600/inv_icm45600_i3c.c b/drivers/iio/imu/inv_icm45600/inv_icm45600_i3c.c
index b5df06b97d44..9247eae9b3e2 100644
--- a/drivers/iio/imu/inv_icm45600/inv_icm45600_i3c.c
+++ b/drivers/iio/imu/inv_icm45600/inv_icm45600_i3c.c
@@ -57,7 +57,8 @@ static int inv_icm45600_i3c_probe(struct i3c_device *i3cdev)
}
if (chip == nb_chip)
- dev_err_probe(&i3cdev->dev, -ENODEV, "Failed to match part id %d\n", whoami);
+ return dev_err_probe(&i3cdev->dev, -ENODEV,
+ "Failed to match part id %d\n", whoami);
return inv_icm45600_core_probe(regmap, i3c_chip_info[chip], false, NULL);
}
--
2.51.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH next] iio: imu: inv_icm45600: Add a missing return statement in probe() 2025-10-22 11:02 [PATCH next] iio: imu: inv_icm45600: Add a missing return statement in probe() Dan Carpenter @ 2025-10-22 15:13 ` David Lechner 2025-10-27 14:23 ` Jonathan Cameron 1 sibling, 0 replies; 5+ messages in thread From: David Lechner @ 2025-10-22 15:13 UTC (permalink / raw) To: Dan Carpenter, Remi Buisson Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, linux-iio, linux-kernel, kernel-janitors On 10/22/25 6:02 AM, Dan Carpenter wrote: > The intention here was clearly to return -ENODEV but the return statement > was missing. It would result in an off by one read in i3c_chip_info[] on > the next line. Add the return statement. > > Fixes: 1bef24e9007e ("iio: imu: inv_icm45600: add I3C driver for inv_icm45600 driver") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > --- Reviewed-by: David Lechner <dlechner@baylibre.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH next] iio: imu: inv_icm45600: Add a missing return statement in probe() 2025-10-22 11:02 [PATCH next] iio: imu: inv_icm45600: Add a missing return statement in probe() Dan Carpenter 2025-10-22 15:13 ` David Lechner @ 2025-10-27 14:23 ` Jonathan Cameron 2025-10-31 9:56 ` Remi Buisson 1 sibling, 1 reply; 5+ messages in thread From: Jonathan Cameron @ 2025-10-27 14:23 UTC (permalink / raw) To: Dan Carpenter Cc: Remi Buisson, David Lechner, Nuno Sá, Andy Shevchenko, linux-iio, linux-kernel, kernel-janitors On Wed, 22 Oct 2025 14:02:20 +0300 Dan Carpenter <dan.carpenter@linaro.org> wrote: > The intention here was clearly to return -ENODEV but the return statement > was missing. It would result in an off by one read in i3c_chip_info[] on > the next line. Add the return statement. > > Fixes: 1bef24e9007e ("iio: imu: inv_icm45600: add I3C driver for inv_icm45600 driver") > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > --- > drivers/iio/imu/inv_icm45600/inv_icm45600_i3c.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/iio/imu/inv_icm45600/inv_icm45600_i3c.c b/drivers/iio/imu/inv_icm45600/inv_icm45600_i3c.c > index b5df06b97d44..9247eae9b3e2 100644 > --- a/drivers/iio/imu/inv_icm45600/inv_icm45600_i3c.c > +++ b/drivers/iio/imu/inv_icm45600/inv_icm45600_i3c.c > @@ -57,7 +57,8 @@ static int inv_icm45600_i3c_probe(struct i3c_device *i3cdev) > } > > if (chip == nb_chip) > - dev_err_probe(&i3cdev->dev, -ENODEV, "Failed to match part id %d\n", whoami); > + return dev_err_probe(&i3cdev->dev, -ENODEV, > + "Failed to match part id %d\n", whoami); > > return inv_icm45600_core_probe(regmap, i3c_chip_info[chip], false, NULL); > } I'm going to apply this but the resulting code is still wrong (even if not a true bug after this fix). A hard ID match like this breaks use of dt fallback compatibles. What this should do is 'give it a go' on matching, but if there no match it should carry on as if the match was to whatever the compatible that was supplied was. When that happens a dev_info() is appropriate but not error out as this does. Remi, if possible could you look at adding such a patch on top of this? Thanks, Jonathan ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH next] iio: imu: inv_icm45600: Add a missing return statement in probe() 2025-10-27 14:23 ` Jonathan Cameron @ 2025-10-31 9:56 ` Remi Buisson 2025-11-02 12:15 ` Jonathan Cameron 0 siblings, 1 reply; 5+ messages in thread From: Remi Buisson @ 2025-10-31 9:56 UTC (permalink / raw) To: Jonathan Cameron, Dan Carpenter Cc: David Lechner, Nuno Sá, Andy Shevchenko, linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org > > >From: Jonathan Cameron <jic23@kernel.org> >Sent: Monday, October 27, 2025 3:24 PM >To: Dan Carpenter <dan.carpenter@linaro.org> >Cc: Remi Buisson <Remi.Buisson@tdk.com>; David Lechner <dlechner@baylibre.com>; Nuno Sá <nuno.sa@analog.com>; Andy Shevchenko <andy@kernel.org>; linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; kernel-janitors@vger.kernel.org >Subject: Re: [PATCH next] iio: imu: inv_icm45600: Add a missing return statement in probe() > >On Wed, 22 Oct 2025 14: 02: 20 +0300 Dan Carpenter <dan. carpenter@ linaro. org> wrote: > The intention here was clearly to return -ENODEV but the return statement > was missing. It would result in an off by one read in i3c_chip_info[] >On Wed, 22 Oct 2025 14:02:20 +0300 >Dan Carpenter <dan.carpenter@linaro.org> wrote: > >> The intention here was clearly to return -ENODEV but the return statement >> was missing. It would result in an off by one read in i3c_chip_info[] on >> the next line. Add the return statement. >> >> Fixes: 1bef24e9007e ("iio: imu: inv_icm45600: add I3C driver for inv_icm45600 driver") >> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> >> --- >> drivers/iio/imu/inv_icm45600/inv_icm45600_i3c.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/iio/imu/inv_icm45600/inv_icm45600_i3c.c b/drivers/iio/imu/inv_icm45600/inv_icm45600_i3c.c >> index b5df06b97d44..9247eae9b3e2 100644 >> --- a/drivers/iio/imu/inv_icm45600/inv_icm45600_i3c.c >> +++ b/drivers/iio/imu/inv_icm45600/inv_icm45600_i3c.c >> @@ -57,7 +57,8 @@ static int inv_icm45600_i3c_probe(struct i3c_device *i3cdev) >> } >> >> if (chip == nb_chip) >> - dev_err_probe(&i3cdev->dev, -ENODEV, "Failed to match part id %d\n", whoami); >> + return dev_err_probe(&i3cdev->dev, -ENODEV, >> + "Failed to match part id %d\n", whoami); >> >> return inv_icm45600_core_probe(regmap, i3c_chip_info[chip], false, NULL); >> } > >I'm going to apply this but the resulting code is still wrong (even if not >a true bug after this fix). > >A hard ID match like this breaks use of dt fallback compatibles. >What this should do is 'give it a go' on matching, but if there no match it should >carry on as if the match was to whatever the compatible that was supplied was. >When that happens a dev_info() is appropriate but not error out as this does. > >Remi, if possible could you look at adding such a patch on top of this? > >Thanks, > >Jonathan > Thanks Jonathan, the fix is correct. The problem is that I3C don't specify any device in the device tree, and these sensors cannot be identified by their I3C IDs neither. So, the driver cannot fallbacks to any compatible device, other than picking one, more or less, at random. Do you see any way to work around this limitation? ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH next] iio: imu: inv_icm45600: Add a missing return statement in probe() 2025-10-31 9:56 ` Remi Buisson @ 2025-11-02 12:15 ` Jonathan Cameron 0 siblings, 0 replies; 5+ messages in thread From: Jonathan Cameron @ 2025-11-02 12:15 UTC (permalink / raw) To: Remi Buisson Cc: Dan Carpenter, David Lechner, Nuno Sá, Andy Shevchenko, linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org On Fri, 31 Oct 2025 09:56:45 +0000 Remi Buisson <Remi.Buisson@tdk.com> wrote: > > > > > >From: Jonathan Cameron <jic23@kernel.org> > >Sent: Monday, October 27, 2025 3:24 PM > >To: Dan Carpenter <dan.carpenter@linaro.org> > >Cc: Remi Buisson <Remi.Buisson@tdk.com>; David Lechner <dlechner@baylibre.com>; Nuno Sá <nuno.sa@analog.com>; Andy Shevchenko <andy@kernel.org>; linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; kernel-janitors@vger.kernel.org > >Subject: Re: [PATCH next] iio: imu: inv_icm45600: Add a missing return statement in probe() > > > >On Wed, 22 Oct 2025 14: 02: 20 +0300 Dan Carpenter <dan. carpenter@ linaro. org> wrote: > The intention here was clearly to return -ENODEV but the return statement > was missing. It would result in an off by one read in i3c_chip_info[] > >On Wed, 22 Oct 2025 14:02:20 +0300 > >Dan Carpenter <dan.carpenter@linaro.org> wrote: > > > >> The intention here was clearly to return -ENODEV but the return statement > >> was missing. It would result in an off by one read in i3c_chip_info[] on > >> the next line. Add the return statement. > >> > >> Fixes: 1bef24e9007e ("iio: imu: inv_icm45600: add I3C driver for inv_icm45600 driver") > >> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> > >> --- > >> drivers/iio/imu/inv_icm45600/inv_icm45600_i3c.c | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/iio/imu/inv_icm45600/inv_icm45600_i3c.c b/drivers/iio/imu/inv_icm45600/inv_icm45600_i3c.c > >> index b5df06b97d44..9247eae9b3e2 100644 > >> --- a/drivers/iio/imu/inv_icm45600/inv_icm45600_i3c.c > >> +++ b/drivers/iio/imu/inv_icm45600/inv_icm45600_i3c.c > >> @@ -57,7 +57,8 @@ static int inv_icm45600_i3c_probe(struct i3c_device *i3cdev) > >> } > >> > >> if (chip == nb_chip) > >> - dev_err_probe(&i3cdev->dev, -ENODEV, "Failed to match part id %d\n", whoami); > >> + return dev_err_probe(&i3cdev->dev, -ENODEV, > >> + "Failed to match part id %d\n", whoami); > >> > >> return inv_icm45600_core_probe(regmap, i3c_chip_info[chip], false, NULL); > >> } > > > >I'm going to apply this but the resulting code is still wrong (even if not > >a true bug after this fix). > > > >A hard ID match like this breaks use of dt fallback compatibles. > >What this should do is 'give it a go' on matching, but if there no match it should > >carry on as if the match was to whatever the compatible that was supplied was. > >When that happens a dev_info() is appropriate but not error out as this does. > > > >Remi, if possible could you look at adding such a patch on top of this? > > > >Thanks, > > > >Jonathan > > > Thanks Jonathan, the fix is correct. > The problem is that I3C don't specify any device in the device tree, > and these sensors cannot be identified by their I3C IDs neither. > So, the driver cannot fallbacks to any compatible device, other than picking one, more or less, at random. > Do you see any way to work around this limitation? Ah. I'd missed that. Fair enough, sounds like nothing we can do if the driver doesn't know the ID. Jonathan ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-11-02 12:15 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-22 11:02 [PATCH next] iio: imu: inv_icm45600: Add a missing return statement in probe() Dan Carpenter 2025-10-22 15:13 ` David Lechner 2025-10-27 14:23 ` Jonathan Cameron 2025-10-31 9:56 ` Remi Buisson 2025-11-02 12:15 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox