On Mon, Aug 26, 2019 at 08:36:04PM +0200, Jacek Anaszewski wrote:
> On 8/26/19 11:57 AM, Andy Shevchenko wrote:
> > On Sat, Aug 24, 2019 at 06:47:54PM +0200, Jacek Anaszewski wrote:
> >> On 8/21/19 7:17 PM, Andy Shevchenko wrote:
> >>> Allow all valid GPIOs to be used in the driver.
> >>>
> >>> Fixes: 17354bfe8527 ("leds: Add gpio-led trigger")
> >>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> >
> >>> - if (!gpio) {
> >>> - if (gpio_data->gpio != 0)
> >>> + if (!gpio_is_valid(gpio)) {
> >>> + if (gpio_is_valid(gpio_data->gpio))
> >>> free_irq(gpio_to_irq(gpio_data->gpio), led);
> >>> - gpio_data->gpio = 0;
> >>> + gpio_data->gpio = gpio;
> >>
> >> It looks odd to me. I'd just assign invalid constant gpio number
> >> e.g. -1.
> >
> > Current ABI (unsigned) doesn't allow us to do this. Internally we can redefine
>
> Ah, right, missed that.
>
> > invalid GPIO line number to -1 or so, but does it worth it? And actually I
> > would prefer -EINVAL or -ENOENT in such cases.
>
> OK, we can keep your "= gpio" assignment in view of the above, but need
> to return error instead of "n".
Then we will break an ABI, where user expects no error it suddenly will be one.
--
With Best Regards,
Andy Shevchenko
