[PATCH v4 0/2] gpiolib, sfi: introduce SFI GPIO helpers

[PATCH v4 0/2] gpiolib, sfi: introduce SFI GPIO helpers

[Andy Shevchenko]

Here is a fourth [1] version of the SFI GPIO helpers. This series contains SFI
GPIO helpers along with the update of GPIO library to lookup for SFI resources.

It has been tested on Medfield device on top of linux-next 20131209 +
Alexandre's GPIO descriptor API update [2], Mika's ACPI gpio patches [3] and my
GPIO library clean up [4].

Changes since v3:
 - push patch 1/3 separate from this series
 - address David's and Alexandre's comments

[1] https://lkml.org/lkml/2013/6/5/316
[2] http://www.spinics.net/lists/kernel/msg1645907.html
[3] http://www.spinics.net/lists/linux-acpi/msg47572.html
[4] https://www.mail-archive.com/linux-gpio@vger.kernel.org/msg01515.html

Andy Shevchenko (2):
  SFI: store GPIO table and export lookup function
  gpiolib: append SFI helpers for GPIO API

 drivers/gpio/Kconfig       |   4 ++
 drivers/gpio/Makefile      |   1 +
 drivers/gpio/gpiolib-sfi.c |  28 +++++++++++
 drivers/gpio/gpiolib.c     |   3 ++
 drivers/gpio/gpiolib.h     |  13 +++++
 drivers/sfi/Makefile       |   2 +-
 drivers/sfi/sfi_core.c     |   6 +++
 drivers/sfi/sfi_core.h     |   3 ++
 drivers/sfi/sfi_gpio.c     | 121 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/sfi.h        |  10 ++++
 10 files changed, 190 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpio/gpiolib-sfi.c
 create mode 100644 drivers/sfi/sfi_gpio.c

-- 
1.8.4.4

[PATCH v4 1/2] SFI: store GPIO table and export lookup function

[Andy Shevchenko]

We have to provide a mechanism to retrive GPIO information from SFI. For this
we store SFI GPIO table and provide the lookup function
sfi_gpio_get_entry_by_name() that will be used later in GPIO framework.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/sfi/Makefile   |   2 +-
 drivers/sfi/sfi_core.c |   6 +++
 drivers/sfi/sfi_core.h |   3 ++
 drivers/sfi/sfi_gpio.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/sfi.h    |  10 ++++
 5 files changed, 141 insertions(+), 1 deletion(-)
 create mode 100644 drivers/sfi/sfi_gpio.c

diff --git a/drivers/sfi/Makefile b/drivers/sfi/Makefile
index 2343732..dc011db 100644
--- a/drivers/sfi/Makefile
+++ b/drivers/sfi/Makefile
@@ -1,3 +1,3 @@
 obj-y	+= sfi_acpi.o
 obj-y	+= sfi_core.o
-
+obj-y	+= sfi_gpio.o
diff --git a/drivers/sfi/sfi_core.c b/drivers/sfi/sfi_core.c
index 296db7a..e9ff6f0 100644
--- a/drivers/sfi/sfi_core.c
+++ b/drivers/sfi/sfi_core.c
@@ -512,6 +512,12 @@ void __init sfi_init_late(void)
 	syst_va = sfi_map_memory(syst_pa, length);
 
 	sfi_acpi_init();
+
+	/*
+	 * Parsing GPIO table first, since the DEVS table will need this table
+	 * to map the pin name to the actual pin.
+	 */
+	sfi_gpio_init();
 }
 
 /*
diff --git a/drivers/sfi/sfi_core.h b/drivers/sfi/sfi_core.h
index 1d5cfe8..18c663d 100644
--- a/drivers/sfi/sfi_core.h
+++ b/drivers/sfi/sfi_core.h
@@ -79,3 +79,6 @@ struct sfi_table_header *sfi_get_table(struct sfi_table_key *key);
 extern void sfi_put_table(struct sfi_table_header *table);
 extern struct sfi_table_attr __init *sfi_sysfs_install_table(u64 pa);
 extern void __init sfi_acpi_sysfs_init(void);
+
+/* sfi_gpio.c */
+int sfi_gpio_init(void);
diff --git a/drivers/sfi/sfi_gpio.c b/drivers/sfi/sfi_gpio.c
new file mode 100644
index 0000000..22627a1
--- /dev/null
+++ b/drivers/sfi/sfi_gpio.c
@@ -0,0 +1,121 @@
+/* sfi_gpio.c Simple Firmware Interface - GPIO extensions */
+
+/*
+
+  This file is provided under a dual BSD/GPLv2 license.  When using or
+  redistributing this file, you may do so under either license.
+
+  GPL LICENSE SUMMARY
+
+  Copyright(c) 2013 Intel Corporation. All rights reserved.
+
+  This program is free software; you can redistribute it and/or modify
+  it under the terms of version 2 of the GNU General Public License as
+  published by the Free Software Foundation.
+
+  This program is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+  General Public License for more details.
+
+  You should have received a copy of the GNU General Public License
+  along with this program; if not, write to the Free Software
+  Foundation, Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+  The full GNU General Public License is included in this distribution
+  in the file called LICENSE.GPL.
+
+  BSD LICENSE
+
+  Copyright(c) 2013 Intel Corporation. All rights reserved.
+
+  Redistribution and use in source and binary forms, with or without
+  modification, are permitted provided that the following conditions
+  are met:
+
+    * Redistributions of source code must retain the above copyright
+      notice, this list of conditions and the following disclaimer.
+    * Redistributions in binary form must reproduce the above copyright
+      notice, this list of conditions and the following disclaimer in
+      the documentation and/or other materials provided with the
+      distribution.
+    * Neither the name of Intel Corporation nor the names of its
+      contributors may be used to endorse or promote products derived
+      from this software without specific prior written permission.
+
+  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+  "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+  LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+  A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+  OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+  SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+  LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+  DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+  THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+  (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+  OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+*/
+
+#define KMSG_COMPONENT "SFI"
+#define pr_fmt(fmt) KMSG_COMPONENT ": " fmt
+
+#include <linux/errno.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include <linux/slab.h>
+#include <linux/export.h>
+#include <linux/sfi.h>
+
+#include "sfi_core.h"
+
+static struct sfi_gpio_table_entry *sfi_gpio_table;
+static int sfi_gpio_num_entry;
+
+struct sfi_gpio_table_entry *sfi_gpio_get_entry_by_name(const char *name)
+{
+	struct sfi_gpio_table_entry *pentry = sfi_gpio_table;
+	int i;
+
+	if (!pentry)
+		return ERR_PTR(-EAGAIN);
+
+	for (i = 0; i < sfi_gpio_num_entry; i++, pentry++) {
+		if (!strncmp(name, pentry->pin_name, SFI_NAME_LEN))
+			return pentry;
+	}
+
+	return ERR_PTR(-ENOENT);
+}
+EXPORT_SYMBOL_GPL(sfi_gpio_get_entry_by_name);
+
+static int __init sfi_gpio_parse(struct sfi_table_header *table)
+{
+	struct sfi_table_simple *sb;
+	struct sfi_gpio_table_entry *pentry;
+	int num, i;
+
+	sb = container_of(table, struct sfi_table_simple, header);
+
+	num = SFI_GET_NUM_ENTRIES(sb, struct sfi_gpio_table_entry);
+	pentry = (struct sfi_gpio_table_entry *)sb->pentry;
+
+	sfi_gpio_table = kmemdup(pentry, num * sizeof(*pentry), GFP_KERNEL);
+	if (!sfi_gpio_table)
+		return -ENOMEM;
+
+	sfi_gpio_num_entry = num;
+
+	pr_debug("GPIO pin info:\n");
+	for (i = 0; i < num; i++, pentry++)
+		pr_debug("GPIO [%2d] chip = %16.16s, name = %16.16s, pin=%d\n",
+			 i, pentry->controller_name, pentry->pin_name,
+			 pentry->pin_no);
+
+	return 0;
+}
+
+int __init sfi_gpio_init(void)
+{
+	return sfi_table_parse(SFI_SIG_GPIO, NULL, NULL, sfi_gpio_parse);
+}
diff --git a/include/linux/sfi.h b/include/linux/sfi.h
index d9b436f..4a9432f 100644
--- a/include/linux/sfi.h
+++ b/include/linux/sfi.h
@@ -61,6 +61,8 @@
 
 #include <linux/init.h>
 #include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/err.h>
 
 /* Table signatures reserved by the SFI specification */
 #define SFI_SIG_SYST		"SYST"
@@ -185,6 +187,8 @@ static inline void disable_sfi(void)
 	sfi_disabled = 1;
 }
 
+struct sfi_gpio_table_entry *sfi_gpio_get_entry_by_name(const char *name);
+
 #else /* !CONFIG_SFI */
 
 static inline void sfi_init(void)
@@ -204,6 +208,12 @@ static inline int sfi_table_parse(char *signature, char *oem_id,
 	return -1;
 }
 
+static inline
+struct sfi_gpio_table_entry *sfi_gpio_get_entry_by_name(const char *name)
+{
+	return ERR_PTR(-ENODEV);
+}
+
 #endif /* !CONFIG_SFI */
 
 #endif /*_LINUX_SFI_H*/
-- 
1.8.4.4

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

[Andy Shevchenko]

To support some (legacy) firmwares and platforms let's make life easier for
their customers.

This patch provides a function which converts sfi_gpio_table_entry to
gpio_desc. The use of it is integrated into GPIO library to enable generic
access to the SFI GPIO resources.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/Kconfig       |  4 ++++
 drivers/gpio/Makefile      |  1 +
 drivers/gpio/gpiolib-sfi.c | 28 ++++++++++++++++++++++++++++
 drivers/gpio/gpiolib.c     |  3 +++
 drivers/gpio/gpiolib.h     | 13 +++++++++++++
 5 files changed, 49 insertions(+)
 create mode 100644 drivers/gpio/gpiolib-sfi.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 365ea61..34235c8 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -51,6 +51,10 @@ config OF_GPIO
 	def_bool y
 	depends on OF
 
+config GPIO_SFI
+	def_bool y
+	depends on SFI
+
 config GPIO_ACPI
 	def_bool y
 	depends on ACPI
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 95d7359..f823ff5 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -5,6 +5,7 @@ ccflags-$(CONFIG_DEBUG_GPIO)	+= -DDEBUG
 obj-$(CONFIG_GPIO_DEVRES)	+= devres.o
 obj-$(CONFIG_GPIOLIB)		+= gpiolib.o
 obj-$(CONFIG_OF_GPIO)		+= gpiolib-of.o
+obj-$(CONFIG_GPIO_SFI)		+= gpiolib-sfi.o
 obj-$(CONFIG_GPIO_ACPI)		+= gpiolib-acpi.o
 
 # Device drivers. Generally keep list sorted alphabetically
diff --git a/drivers/gpio/gpiolib-sfi.c b/drivers/gpio/gpiolib-sfi.c
new file mode 100644
index 0000000..43db749
--- /dev/null
+++ b/drivers/gpio/gpiolib-sfi.c
@@ -0,0 +1,28 @@
+/*
+ * Simple Firmware Interface (SFI) helpers for GPIO API
+ *
+ * Copyright (C) 2013, Intel Corporation
+ * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/sfi.h>
+#include <linux/errno.h>
+#include <linux/err.h>
+
+#include "gpiolib.h"
+
+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;
+
+	return gpio_to_desc(pentry->pin_no);
+}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index d2ffdfe..7575c51 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2452,6 +2452,9 @@ struct gpio_desc *__must_check gpiod_get_index(struct device *dev,
 	} else if (IS_ENABLED(CONFIG_ACPI) && dev && ACPI_HANDLE(dev)) {
 		dev_dbg(dev, "using ACPI for GPIO lookup\n");
 		desc = acpi_find_gpio(dev, con_id, idx, &flags);
+	} else if (IS_ENABLED(CONFIG_SFI)) {
+		dev_dbg(dev, "using SFI for GPIO lookup\n");
+		desc = sfi_get_gpiod_by_name(con_id);
 	}
 
 	/*
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 82be586..ca0615a 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -43,4 +43,17 @@ acpi_get_gpiod_by_index(struct device *dev, int index,
 }
 #endif
 
+#ifdef CONFIG_GPIO_SFI
+
+struct gpio_desc *sfi_get_gpiod_by_name(const char *name);
+
+#else /* !CONFIG_GPIO_SFI */
+
+static inline struct gpio_desc *sfi_get_gpiod_by_name(const char *name)
+{
+	return ERR_PTR(-ENODEV);
+}
+
+#endif /* !CONFIG_GPIO_SFI */
+
 #endif /* GPIOLIB_H */
-- 
1.8.4.4

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

[Andy Shevchenko]

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:
> > diff --git a/drivers/gpio/gpiolib-sfi.c b/drivers/gpio/gpiolib-sfi.c
> > new file mode 100644
> > index 0000000..43db749
> > --- /dev/null
> > +++ b/drivers/gpio/gpiolib-sfi.c
> > @@ -0,0 +1,28 @@
> > +/*
> > + * Simple Firmware Interface (SFI) helpers for GPIO API
> > + *
> > + * Copyright (C) 2013, Intel Corporation
> > + * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/sfi.h>
> > +#include <linux/errno.h>
> > +#include <linux/err.h>
> > +
> > +#include "gpiolib.h"
> > +
> > +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;
> > +
> > +	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:
> 
> #ifdef CONFIG_SFI
> #else
> #endif
> 
> you can get rid of that file (and Kconfig + Makefile changes) completely,
> no?

I won't have linux/sfi.h in the gpiolib.c. If Linus has no objections, I
can do that of course.


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

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

[Mika Westerberg]

On Mon, Dec 09, 2013 at 01:50:29PM +0200, Andy Shevchenko wrote:
> diff --git a/drivers/gpio/gpiolib-sfi.c b/drivers/gpio/gpiolib-sfi.c
> new file mode 100644
> index 0000000..43db749
> --- /dev/null
> +++ b/drivers/gpio/gpiolib-sfi.c
> @@ -0,0 +1,28 @@
> +/*
> + * Simple Firmware Interface (SFI) helpers for GPIO API
> + *
> + * Copyright (C) 2013, Intel Corporation
> + * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/sfi.h>
> +#include <linux/errno.h>
> +#include <linux/err.h>
> +
> +#include "gpiolib.h"
> +
> +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;
> +
> +	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:

#ifdef CONFIG_SFI
#else
#endif

you can get rid of that file (and Kconfig + Makefile changes) completely,
no?

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

[Linus Walleij]

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;
>> > +
>> > +   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 really like the way this patch set developed to abstract away
the SFI internals into this gpiolib sidecar, thanks!

Yours,
Linus Walleij

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

[Andy Shevchenko]

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

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

[Linus Walleij]

On Mon, Dec 9, 2013 at 2:32 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> 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.

Just respin it while we're waiting for Len's ACK...

Maybe he could use a co-maintainer for SFI?

Yours,
Linus Walleij

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

[Andy Shevchenko]

On Mon, 2013-12-09 at 15:15 +0100, Linus Walleij wrote:
> On Mon, Dec 9, 2013 at 2:32 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> 
> > 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.
> 
> Just respin it while we're waiting for Len's ACK...

Will send v5 soon.

> 
> Maybe he could use a co-maintainer for SFI?

Let's ask David. At least he is closer to that SFI stuff than me.
David, who is the best person to get an Ack for SFI stuff nowadays?

> 
> Yours,
> Linus Walleij

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

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

[Alex Courbot]

On 12/09/2013 10:27 PM, 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;
>>>> +
>>>> +   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.

Also agree with that. If anything, I'd like to see the DT/ACPI specific 
functions moved into a different file at some point.

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

Yes - that's a great followup to Mika's work on ACPI and it's nice to 
see this interface adopted (at least on the producer side).