Re: [PATCH v4 2/2] gpiolib: append SFI helpers for GPIO API

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Mon, 2013-12-09 at 14:27 +0100, Linus Walleij wrote:
> On Mon, Dec 9, 2013 at 2:02 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Mon, 2013-12-09 at 15:04 +0200, Mika Westerberg wrote:
> >> On Mon, Dec 09, 2013 at 01:50:29PM +0200, Andy Shevchenko wrote:
> 
> >> > +struct gpio_desc *sfi_get_gpiod_by_name(const char *name)
> >> > +{
> >> > +   struct sfi_gpio_table_entry *pentry;
> >> > +
> >> > +   pentry = sfi_gpio_get_entry_by_name(name);
> >> > +   if (IS_ERR(pentry))
> >> > +           return pentry;

Here should be return ERR_CAST(pentry);

> >> > +
> >> > +   return gpio_to_desc(pentry->pin_no);
> >> > +}
> >>
> >> Do you really need add a new file just to have the above supported? I mean
> >> that if you place that function to drivers/gpio/gpiolib.c with the
> >> appropriate:
> (...)
> >
> > I won't have linux/sfi.h in the gpiolib.c. If Linus has no objections, I
> > can do that of course.
> 
> #ifdef:s are ugly according to Linus (Documentation/CodingStyle)
> and according to this instance of Linus as well. So I actually prefer
> so have a separate file.
> 
> But what is happening with this series? Should it be merged
> through the GPIO tree? I need the SFI maintainers (Len Brown)
> ACK for that.

I think Len is to busy for that old stuff. Anyway, he is in Cc list
here.

Len, would you mind to give your Ack for the first patch in the series?

Linus, by the way, there is a minor issue in code (see above comment). I
could fix by myself if you want me to do.

> 
> I really like the way this patch set developed to abstract away
> the SFI internals into this gpiolib sidecar, thanks!
> 
> Yours,
> Linus Walleij

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy