| Message ID | 20220718194258.181738-3-ukleinek@debian.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2,1/3] iio: humidity: dht11: Don't warn on memory allocation failure | expand |
On Mon, Jul 18, 2022 at 9:51 PM Uwe Kleine-König <ukleinek@debian.org> wrote: > > All but one error path use dev_err_probe() to emit a message. Use the same > incarnation for the remaining exit point for consistency. ... > + if (dht11->irq < 0) > + return dev_err_probe(dev, -EINVAL, "GPIO %d has no interrupt\n", > + desc_to_gpio(dht11->gpiod)); Oh, what I missed is the error code shadowing. Can't we propagate the real error code? -EINVAL --> dht11->irq And to be honest I don't like this desc_to_gpio() usage. It's not for board files, it will bring confusing information to the user. What is important is the name of GPIO, i.o.w. "connection id".
On Mon, Jul 18, 2022 at 10:29 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Mon, Jul 18, 2022 at 9:51 PM Uwe Kleine-König <ukleinek@debian.org> wrote: ... > And to be honest I don't like this desc_to_gpio() usage. It's not for > board files, it will bring confusing information to the user. What is > important is the name of GPIO, i.o.w. "connection id". Yes, I have noticed that this is in the original code, just if you want to make it better at the same time. Not sure if Jonathan wants all these in the single patch again, however it will touch almost the same line in all (three ?).
On Mon, 18 Jul 2022 22:32:17 +0200 Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Mon, Jul 18, 2022 at 10:29 PM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > On Mon, Jul 18, 2022 at 9:51 PM Uwe Kleine-König <ukleinek@debian.org> wrote: > > ... > > > And to be honest I don't like this desc_to_gpio() usage. It's not for > > board files, it will bring confusing information to the user. What is > > important is the name of GPIO, i.o.w. "connection id". > > Yes, I have noticed that this is in the original code, just if you > want to make it better at the same time. Not sure if Jonathan wants > all these in the single patch again, however it will touch almost the > same line in all (three ?). > So ideal would be separate patches for the different change types, but meh, that's a pain as you say so given it's just a few lines I'm fine with whatever combining you want to do.
diff --git a/drivers/iio/humidity/dht11.c b/drivers/iio/humidity/dht11.c index 0db4f7471319..d8b2cb3ef81e 100644 --- a/drivers/iio/humidity/dht11.c +++ b/drivers/iio/humidity/dht11.c @@ -307,10 +307,9 @@ static int dht11_probe(struct platform_device *pdev) "Failed to acquire GPIO\n"); dht11->irq = gpiod_to_irq(dht11->gpiod); - if (dht11->irq < 0) { - dev_err(dev, "GPIO %d has no interrupt\n", desc_to_gpio(dht11->gpiod)); - return -EINVAL; - } + if (dht11->irq < 0) + return dev_err_probe(dev, -EINVAL, "GPIO %d has no interrupt\n", + desc_to_gpio(dht11->gpiod)); dht11->timestamp = ktime_get_boottime_ns() - DHT11_DATA_VALID_TIME - 1; dht11->num_edges = -1;
All but one error path use dev_err_probe() to emit a message. Use the same incarnation for the remaining exit point for consistency. Signed-off-by: Uwe Kleine-König <ukleinek@debian.org> --- No changes since (implicit) v1 drivers/iio/humidity/dht11.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)