* [PATCH] iio: humidity: dht11: Improve error reporting in .probe()
@ 2022-06-30 6:35 Uwe Kleine-König
2022-06-30 7:17 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Uwe Kleine-König @ 2022-06-30 6:35 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Lars-Peter Clausen, Andy Shevchenko, linux-iio, Yves-Alexis Perez
Don't emit a message for -ENOMEM, the kernel is already loud enough in
this case. Add a message if getting the GPIO or registering the iio
device fails and use dev_err_probe for improved behaviour on
-EPROBE_DEFER.
Signed-off-by: Uwe Kleine-König <ukleinek@debian.org>
---
drivers/iio/humidity/dht11.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/humidity/dht11.c b/drivers/iio/humidity/dht11.c
index c97e25448772..d8b2cb3ef81e 100644
--- a/drivers/iio/humidity/dht11.c
+++ b/drivers/iio/humidity/dht11.c
@@ -293,24 +293,23 @@ static int dht11_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct dht11 *dht11;
struct iio_dev *iio;
+ int ret;
iio = devm_iio_device_alloc(dev, sizeof(*dht11));
- if (!iio) {
- dev_err(dev, "Failed to allocate IIO device\n");
+ if (!iio)
return -ENOMEM;
- }
dht11 = iio_priv(iio);
dht11->dev = dev;
dht11->gpiod = devm_gpiod_get(dev, NULL, GPIOD_IN);
if (IS_ERR(dht11->gpiod))
- return PTR_ERR(dht11->gpiod);
+ return dev_err_probe(dev, PTR_ERR(dht11->gpiod),
+ "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;
@@ -325,7 +324,11 @@ static int dht11_probe(struct platform_device *pdev)
iio->channels = dht11_chan_spec;
iio->num_channels = ARRAY_SIZE(dht11_chan_spec);
- return devm_iio_device_register(dev, iio);
+ ret = devm_iio_device_register(dev, iio);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "Failed to register iio device\n");
+
+ return 0;
}
static struct platform_driver dht11_driver = {
base-commit: f2906aa863381afb0015a9eb7fefad885d4e5a56
--
2.36.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: humidity: dht11: Improve error reporting in .probe()
2022-06-30 6:35 [PATCH] iio: humidity: dht11: Improve error reporting in .probe() Uwe Kleine-König
@ 2022-06-30 7:17 ` Andy Shevchenko
2022-06-30 10:14 ` Uwe Kleine-König
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2022-06-30 7:17 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jonathan Cameron, Lars-Peter Clausen, Andy Shevchenko, linux-iio,
Yves-Alexis Perez
On Thu, Jun 30, 2022 at 8:58 AM Uwe Kleine-König <ukleinek@debian.org> wrote:
>
> Don't emit a message for -ENOMEM, the kernel is already loud enough in
> this case. Add a message if getting the GPIO or registering the iio
> device fails and use dev_err_probe for improved behaviour on
> -EPROBE_DEFER.
Why do we need additional messages?
They should be split in a separate patch, perhaps, with the explanation.
Actually the rest I would split to two: converting to dev_err_probe()
in the case where it's not right now, and dropping ENOMEM message.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: humidity: dht11: Improve error reporting in .probe()
2022-06-30 7:17 ` Andy Shevchenko
@ 2022-06-30 10:14 ` Uwe Kleine-König
2022-06-30 10:26 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Uwe Kleine-König @ 2022-06-30 10:14 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Cameron, Lars-Peter Clausen, Andy Shevchenko, linux-iio,
Yves-Alexis Perez
[-- Attachment #1.1: Type: text/plain, Size: 1233 bytes --]
On 6/30/22 09:17, Andy Shevchenko wrote:
> On Thu, Jun 30, 2022 at 8:58 AM Uwe Kleine-König <ukleinek@debian.org> wrote:
>>
>> Don't emit a message for -ENOMEM, the kernel is already loud enough in
>> this case. Add a message if getting the GPIO or registering the iio
>> device fails and use dev_err_probe for improved behaviour on
>> -EPROBE_DEFER.
>
> Why do we need additional messages?
I don't understand the question. Do you really wonder why there is a
benefit of an error message if a resource allocation in probe fails?
Current state is that for Yves-Alexis (on Cc:) the driver is silent but
unbound. I guess that's because gpiod_get fails, but seeing a
confirmation in the kernel log would be nice.
> They should be split in a separate patch, perhaps, with the explanation.
>
> Actually the rest I would split to two: converting to dev_err_probe()
> in the case where it's not right now, and dropping ENOMEM message.
So we're at three patches:
- drop ENOMEM message
- convert existing messages to dev_err_probe()
- introduce errors for devm_gpiod_get() and devm_iio_device_register()
I can rework accordingly, but for me this looks a bit over engineered.
Best regards
Uwe
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: humidity: dht11: Improve error reporting in .probe()
2022-06-30 10:14 ` Uwe Kleine-König
@ 2022-06-30 10:26 ` Andy Shevchenko
2022-06-30 14:15 ` Uwe Kleine-König
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2022-06-30 10:26 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jonathan Cameron, Lars-Peter Clausen, Andy Shevchenko, linux-iio,
Yves-Alexis Perez
On Thu, Jun 30, 2022 at 12:25 PM Uwe Kleine-König <ukleinek@debian.org> wrote:
> On 6/30/22 09:17, Andy Shevchenko wrote:
> > On Thu, Jun 30, 2022 at 8:58 AM Uwe Kleine-König <ukleinek@debian.org> wrote:
> >>
> >> Don't emit a message for -ENOMEM, the kernel is already loud enough in
> >> this case. Add a message if getting the GPIO or registering the iio
> >> device fails and use dev_err_probe for improved behaviour on
> >> -EPROBE_DEFER.
> >
> > Why do we need additional messages?
>
> I don't understand the question. Do you really wonder why there is a
> benefit of an error message if a resource allocation in probe fails?
>
> Current state is that for Yves-Alexis (on Cc:) the driver is silent but
> unbound. I guess that's because gpiod_get fails, but seeing a
> confirmation in the kernel log would be nice.
>
> > They should be split in a separate patch, perhaps, with the explanation.
> >
> > Actually the rest I would split to two: converting to dev_err_probe()
> > in the case where it's not right now, and dropping ENOMEM message.
>
> So we're at three patches:
>
> - drop ENOMEM message
> - convert existing messages to dev_err_probe()
> - introduce errors for devm_gpiod_get() and devm_iio_device_register()
>
> I can rework accordingly, but for me this looks a bit over engineered.
Perhaps, but they three are about different stuff.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: humidity: dht11: Improve error reporting in .probe()
2022-06-30 10:26 ` Andy Shevchenko
@ 2022-06-30 14:15 ` Uwe Kleine-König
2022-07-16 16:51 ` Jonathan Cameron
0 siblings, 1 reply; 6+ messages in thread
From: Uwe Kleine-König @ 2022-06-30 14:15 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Jonathan Cameron, Lars-Peter Clausen, Andy Shevchenko, linux-iio,
Yves-Alexis Perez
[-- Attachment #1.1: Type: text/plain, Size: 2001 bytes --]
On 6/30/22 12:26, Andy Shevchenko wrote:
> On Thu, Jun 30, 2022 at 12:25 PM Uwe Kleine-König <ukleinek@debian.org> wrote:
>> On 6/30/22 09:17, Andy Shevchenko wrote:
>>> On Thu, Jun 30, 2022 at 8:58 AM Uwe Kleine-König <ukleinek@debian.org> wrote:
>>>>
>>>> Don't emit a message for -ENOMEM, the kernel is already loud enough in
>>>> this case. Add a message if getting the GPIO or registering the iio
>>>> device fails and use dev_err_probe for improved behaviour on
>>>> -EPROBE_DEFER.
>>>
>>> Why do we need additional messages?
>>
>> I don't understand the question. Do you really wonder why there is a
>> benefit of an error message if a resource allocation in probe fails?
>>
>> Current state is that for Yves-Alexis (on Cc:) the driver is silent but
>> unbound. I guess that's because gpiod_get fails, but seeing a
>> confirmation in the kernel log would be nice.
>>
>>> They should be split in a separate patch, perhaps, with the explanation.
>>>
>>> Actually the rest I would split to two: converting to dev_err_probe()
>>> in the case where it's not right now, and dropping ENOMEM message.
>>
>> So we're at three patches:
>>
>> - drop ENOMEM message
>> - convert existing messages to dev_err_probe()
>> - introduce errors for devm_gpiod_get() and devm_iio_device_register()
>>
>> I can rework accordingly, but for me this looks a bit over engineered.
>
> Perhaps, but they three are about different stuff.
The different things could be named as follows:
- Improve error reporting in .probe() for devm_iio_device_alloc()
- Improve error reporting in .probe() for devm_gpiod_get()
- Improve error reporting in .probe() for gpiod_to_irq()
- Improve error reporting in .probe() for devm_iio_device_register()
Even after the discussion I think it's ok to summarize that to a single
patch "Improve error reporting in .probe()". I'd wait for Jonathan's
feedback with his opinion before reworking the patch.
Best regards
Uwe
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iio: humidity: dht11: Improve error reporting in .probe()
2022-06-30 14:15 ` Uwe Kleine-König
@ 2022-07-16 16:51 ` Jonathan Cameron
0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2022-07-16 16:51 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Andy Shevchenko, Lars-Peter Clausen, Andy Shevchenko, linux-iio,
Yves-Alexis Perez
On Thu, 30 Jun 2022 16:15:05 +0200
Uwe Kleine-König <ukleinek@debian.org> wrote:
> On 6/30/22 12:26, Andy Shevchenko wrote:
> > On Thu, Jun 30, 2022 at 12:25 PM Uwe Kleine-König <ukleinek@debian.org> wrote:
> >> On 6/30/22 09:17, Andy Shevchenko wrote:
> >>> On Thu, Jun 30, 2022 at 8:58 AM Uwe Kleine-König <ukleinek@debian.org> wrote:
> >>>>
> >>>> Don't emit a message for -ENOMEM, the kernel is already loud enough in
> >>>> this case. Add a message if getting the GPIO or registering the iio
> >>>> device fails and use dev_err_probe for improved behaviour on
> >>>> -EPROBE_DEFER.
> >>>
> >>> Why do we need additional messages?
> >>
> >> I don't understand the question. Do you really wonder why there is a
> >> benefit of an error message if a resource allocation in probe fails?
> >>
> >> Current state is that for Yves-Alexis (on Cc:) the driver is silent but
> >> unbound. I guess that's because gpiod_get fails, but seeing a
> >> confirmation in the kernel log would be nice.
Giving that example in the patch description makes for a very strong
reasoning. Mostly I want this captured so that others who might consider
similar patches and see this, understand the reasoning. Messages for things
that can happen in real life are definitely a good thing!
> >>
> >>> They should be split in a separate patch, perhaps, with the explanation.
> >>>
> >>> Actually the rest I would split to two: converting to dev_err_probe()
> >>> in the case where it's not right now, and dropping ENOMEM message.
> >>
> >> So we're at three patches:
> >>
> >> - drop ENOMEM message
> >> - convert existing messages to dev_err_probe()
> >> - introduce errors for devm_gpiod_get() and devm_iio_device_register()
That lines up with how I'd prefer it split. If I'd not wanted the patch description
additions above anyway I 'might' have just picked it up. Given we are going
around again, let's set a nice example. In particularly it will make the
description easier to read as separates -ENOMEM case from the new additions.
Jonathan
> >>
> >> I can rework accordingly, but for me this looks a bit over engineered.
> >
> > Perhaps, but they three are about different stuff.
>
> The different things could be named as follows:
>
> - Improve error reporting in .probe() for devm_iio_device_alloc()
> - Improve error reporting in .probe() for devm_gpiod_get()
> - Improve error reporting in .probe() for gpiod_to_irq()
> - Improve error reporting in .probe() for devm_iio_device_register()
>
> Even after the discussion I think it's ok to summarize that to a single
> patch "Improve error reporting in .probe()". I'd wait for Jonathan's
> feedback with his opinion before reworking the patch.
>
> Best regards
> Uwe
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-07-16 16:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-30 6:35 [PATCH] iio: humidity: dht11: Improve error reporting in .probe() Uwe Kleine-König
2022-06-30 7:17 ` Andy Shevchenko
2022-06-30 10:14 ` Uwe Kleine-König
2022-06-30 10:26 ` Andy Shevchenko
2022-06-30 14:15 ` Uwe Kleine-König
2022-07-16 16:51 ` Jonathan Cameron
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.