| Message ID | 1496134720-5363-7-git-send-email-brgl@bgdev.pl |
|---|---|
| State | New |
| Headers | show |
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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; }
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(-)