Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1652960 > unrolled thread

[PATCH v2 6/7] gpio: mockup: improve the error message

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 6/7] gpio: mockup: improve the error message Bartosz Golaszewski <brgl@bgdev.pl> - 2017-05-30 11:00 +0200
    Re: [PATCH v2 6/7] gpio: mockup: improve the error message Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-05-30 21:00 +0200
      Re: [PATCH v2 6/7] gpio: mockup: improve the error message Bartosz Golaszewski <brgl@bgdev.pl> - 2017-05-31 13:00 +0200
        Re: [PATCH v2 6/7] gpio: mockup: improve the error message Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-05-31 17:10 +0200
          Re: [PATCH v2 6/7] gpio: mockup: improve the error message Bartosz Golaszewski <brgl@bgdev.pl> - 2017-05-31 17:30 +0200
            Re: [PATCH v2 6/7] gpio: mockup: improve the error message Bartosz Golaszewski <brgl@bgdev.pl> - 2017-06-01 09:20 +0200

#1652960 — [PATCH v2 6/7] gpio: mockup: improve the error message

Indicate the error number and make the message a bit more elaborate.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 drivers/gpio/gpio-mockup.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-mockup.c b/drivers/gpio/gpio-mockup.c
index ab2d38e..2f4fe41 100644
--- a/drivers/gpio/gpio-mockup.c
+++ b/drivers/gpio/gpio-mockup.c
@@ -373,8 +373,9 @@ static int gpio_mockup_probe(struct platform_device *pdev)
 		}
 
 		if (ret) {
-			dev_err(dev, "gpio<%d..%d> add failed\n",
-				base, base < 0 ? ngpio : base + ngpio);
+			dev_err(dev,
+				"adding gpiochip failed: %d (base: %d, ngpio: %d)\n",
+				ret, base, base < 0 ? ngpio : base + ngpio);
 
 			return ret;
 		}
-- 
2.9.3

[toc] | [next] | [standalone]

#1653493

On Tue, May 30, 2017 at 11:58 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> Indicate the error number and make the message a bit more elaborate.

> +                       dev_err(dev,
> +                               "adding gpiochip failed: %d (base: %d, ngpio: %d)\n",
> +                               ret, base, base < 0 ? ngpio : base + ngpio);

You may consider to use
'gpio_mockup_add' instead of 'adding gpiochip'. The latter points the
reader first to gpiochip_add family of functions while you run a
wrapper on top of it.

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1654124

2017-05-30 20:59 GMT+02:00 Andy Shevchenko <andy.shevchenko@gmail.com>:
> On Tue, May 30, 2017 at 11:58 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>> Indicate the error number and make the message a bit more elaborate.
>
>> +                       dev_err(dev,
>> +                               "adding gpiochip failed: %d (base: %d, ngpio: %d)\n",
>> +                               ret, base, base < 0 ? ngpio : base + ngpio);
>
> You may consider to use
> 'gpio_mockup_add' instead of 'adding gpiochip'. The latter points the
> reader first to gpiochip_add family of functions while you run a
> wrapper on top of it.
>

But this message can also be emitted if the module params are invalid,
in which case we don't even enter gpio_mockup_add().

Thanks,
Bartosz

[toc] | [prev] | [next] | [standalone]

#1654337

On Wed, May 31, 2017 at 1:54 PM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> 2017-05-30 20:59 GMT+02:00 Andy Shevchenko <andy.shevchenko@gmail.com>:
>> On Tue, May 30, 2017 at 11:58 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>> Indicate the error number and make the message a bit more elaborate.
>>
>>> +                       dev_err(dev,
>>> +                               "adding gpiochip failed: %d (base: %d, ngpio: %d)\n",
>>> +                               ret, base, base < 0 ? ngpio : base + ngpio);
>>
>> You may consider to use
>> 'gpio_mockup_add' instead of 'adding gpiochip'. The latter points the
>> reader first to gpiochip_add family of functions while you run a
>> wrapper on top of it.
>>
>
> But this message can also be emitted if the module params are invalid,
> in which case we don't even enter gpio_mockup_add().

...which unveils bad phrasing in the message. In that case "adding
gpiochip" is also misleading.

I dunno if it requires separate patch to fix the phrasing, though it
would be nice to make it more clear for both cases, or even split to
two cases.

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1654351

2017-05-31 17:00 GMT+02:00 Andy Shevchenko <andy.shevchenko@gmail.com>:
> On Wed, May 31, 2017 at 1:54 PM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>> 2017-05-30 20:59 GMT+02:00 Andy Shevchenko <andy.shevchenko@gmail.com>:
>>> On Tue, May 30, 2017 at 11:58 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>>> Indicate the error number and make the message a bit more elaborate.
>>>
>>>> +                       dev_err(dev,
>>>> +                               "adding gpiochip failed: %d (base: %d, ngpio: %d)\n",
>>>> +                               ret, base, base < 0 ? ngpio : base + ngpio);
>>>
>>> You may consider to use
>>> 'gpio_mockup_add' instead of 'adding gpiochip'. The latter points the
>>> reader first to gpiochip_add family of functions while you run a
>>> wrapper on top of it.
>>>
>>
>> But this message can also be emitted if the module params are invalid,
>> in which case we don't even enter gpio_mockup_add().
>
> ...which unveils bad phrasing in the message. In that case "adding
> gpiochip" is also misleading.
>

Not really. You can pass an invalid value later in the list which will
only become apparent when it's reached. In that case previous
gpiochips will be added correctly but probe will fail with -EINVAL
after reaching the bad one in which case the message is right. I hope
I'm being clear.

Thanks,
Bartosz

[toc] | [prev] | [next] | [standalone]

#1654881

2017-05-31 17:26 GMT+02:00 Bartosz Golaszewski <brgl@bgdev.pl>:
> 2017-05-31 17:00 GMT+02:00 Andy Shevchenko <andy.shevchenko@gmail.com>:
>> On Wed, May 31, 2017 at 1:54 PM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>> 2017-05-30 20:59 GMT+02:00 Andy Shevchenko <andy.shevchenko@gmail.com>:
>>>> On Tue, May 30, 2017 at 11:58 AM, Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>>>>> Indicate the error number and make the message a bit more elaborate.
>>>>
>>>>> +                       dev_err(dev,
>>>>> +                               "adding gpiochip failed: %d (base: %d, ngpio: %d)\n",
>>>>> +                               ret, base, base < 0 ? ngpio : base + ngpio);
>>>>
>>>> You may consider to use
>>>> 'gpio_mockup_add' instead of 'adding gpiochip'. The latter points the
>>>> reader first to gpiochip_add family of functions while you run a
>>>> wrapper on top of it.
>>>>
>>>
>>> But this message can also be emitted if the module params are invalid,
>>> in which case we don't even enter gpio_mockup_add().
>>
>> ...which unveils bad phrasing in the message. In that case "adding
>> gpiochip" is also misleading.
>>
>
> Not really. You can pass an invalid value later in the list which will
> only become apparent when it's reached. In that case previous
> gpiochips will be added correctly but probe will fail with -EINVAL
> after reaching the bad one in which case the message is right. I hope
> I'm being clear.
>

Which made me think: maybe the next step would be to parse the
arguments in the module init function and probe each dummy gpiochip
separately...

Best regards,
Bartosz Golaszewski

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web