[PATCH v3 0/6] gpio / ACPI: convert users to gpiod_* and drop acpi_gpio.h

[PATCH v3 0/6] gpio / ACPI: convert users to gpiod_* and drop acpi_gpio.h

[Mika Westerberg]

Hi all,

Now that the mainline kernel has full ACPI support for the GPIO descriptor
interface we can get rid of ACPI specific GPIO functions in favor of GPIO
descriptor (gpiod_*) interfaces.

This series first converts the existing two users to this interface and
then modifies gpiolib and gpiolib-acpi so that the ACPI functions are only
called internally in drivers/gpio. We then remove the acpi_gpio.h and
require all users to use gpiod_* interfaces.

This is third version of the series. Changes to the previous version [1]:

 * Instead of adding temporary conversion from GPIO descriptors to numbers
   in rfkill-gpio.c we now first introduce a lookup table for paz00 and
   in the next patch convert the driver to use only GPIO descriptors.

 * Corrected a typo in subject of patch [3/6] "covert" -> "convert".

 * Corrected a typo in changelog of patch [5/6] "user" -> "use".

 * Dropped EXPORT_SYMBOL_GPL(acpi_get_gpiod_by_index) from patch [5/6] that
   was left there accidentally.

Since the patches in the series depend on each other I would propose this
to be merged via GPIO tree.

[1] http://www.spinics.net/lists/linux-acpi/msg47472.html (v2 of the series)

Heikki Krogerus (2):
  ARM: tegra: add gpiod_lookup table for paz00
  net: rfkill: gpio: convert to descriptor-based GPIO interface

Mika Westerberg (4):
  mmc: sdhci-acpi: convert to use GPIO descriptor API
  gpio / ACPI: register to ACPI events automatically
  gpio / ACPI: get rid of acpi_gpio.h
  Documentation / ACPI: update to GPIO descriptor API

 Documentation/acpi/enumeration.txt | 36 ++++--------------
 arch/arm/mach-tegra/board-paz00.c  |  7 ++++
 drivers/gpio/gpiolib-acpi.c        | 21 ++++++++---
 drivers/gpio/gpiolib.c             |  5 ++-
 drivers/gpio/gpiolib.h             | 46 +++++++++++++++++++++++
 drivers/mmc/host/sdhci-acpi.c      | 26 ++++++-------
 drivers/pinctrl/pinctrl-baytrail.c |  4 --
 include/linux/acpi_gpio.h          | 51 -------------------------
 net/rfkill/rfkill-gpio.c           | 77 +++++++++++++++++---------------------
 9 files changed, 125 insertions(+), 148 deletions(-)
 create mode 100644 drivers/gpio/gpiolib.h
 delete mode 100644 include/linux/acpi_gpio.h

-- 
1.8.4.3

[PATCH v3 1/6] ARM: tegra: add gpiod_lookup table for paz00

[Mika Westerberg]

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

This makes it possible to request the gpio descriptors in
rfkill_gpio driver regardless of the platform.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Tested-by: Stephen Warren <swarren@nvidia.com>
---
 arch/arm/mach-tegra/board-paz00.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c
index 06f024070dab..a309795da665 100644
--- a/arch/arm/mach-tegra/board-paz00.c
+++ b/arch/arm/mach-tegra/board-paz00.c
@@ -18,6 +18,7 @@
  */
 
 #include <linux/platform_device.h>
+#include <linux/gpio/driver.h>
 #include <linux/rfkill-gpio.h>
 #include "board.h"
 
@@ -36,7 +37,13 @@ static struct platform_device wifi_rfkill_device = {
 	},
 };
 
+static struct gpiod_lookup wifi_gpio_lookup[] = {
+	GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio", NULL, 0, 0),
+	GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio", NULL, 1, 0),
+};
+
 void __init tegra_paz00_wifikill_init(void)
 {
+	gpiod_add_table(wifi_gpio_lookup, ARRAY_SIZE(wifi_gpio_lookup));
 	platform_device_register(&wifi_rfkill_device);
 }
-- 
1.8.4.3

Re: [PATCH v3 1/6] ARM: tegra: add gpiod_lookup table for paz00

[Stephen Warren]

On 11/26/2013 03:05 AM, Mika Westerberg wrote:
> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> 
> This makes it possible to request the gpio descriptors in
> rfkill_gpio driver regardless of the platform.

V2 of the series did the following:

 static struct rfkill_gpio_platform_data wifi_rfkill_platform_data = {
 	.name		= "wifi_rfkill",
-	.reset_gpio	= 25, /* PD1 */
-	.shutdown_gpio	= 85, /* PK5 */
 	.type	= RFKILL_TYPE_WLAN,
 };

I assume that will happen sometime, along with removing those two fields
from the struct definition.

Re: [PATCH v3 1/6] ARM: tegra: add gpiod_lookup table for paz00

[Alex Courbot]

On 11/27/2013 05:33 AM, Stephen Warren wrote:
> On 11/26/2013 03:05 AM, Mika Westerberg wrote:
>> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>>
>> This makes it possible to request the gpio descriptors in
>> rfkill_gpio driver regardless of the platform.
>
> V2 of the series did the following:
>
>   static struct rfkill_gpio_platform_data wifi_rfkill_platform_data = {
>   	.name		= "wifi_rfkill",
> -	.reset_gpio	= 25, /* PD1 */
> -	.shutdown_gpio	= 85, /* PK5 */
>   	.type	= RFKILL_TYPE_WLAN,
>   };
>
> I assume that will happen sometime, along with removing those two fields
> from the struct definition.

This needs to happen after patch 2 of this series, but would be nice to 
have indeed. Would suggest to do it in a separate patch if this series 
needs no more revision.

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

Re: [PATCH v3 1/6] ARM: tegra: add gpiod_lookup table for paz00

[Rhyland Klein]

On 11/26/2013 5:05 AM, Mika Westerberg wrote:
> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> 
> This makes it possible to request the gpio descriptors in
> rfkill_gpio driver regardless of the platform.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Tested-by: Stephen Warren <swarren@nvidia.com>
> ---
>  arch/arm/mach-tegra/board-paz00.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c
> index 06f024070dab..a309795da665 100644
> --- a/arch/arm/mach-tegra/board-paz00.c
> +++ b/arch/arm/mach-tegra/board-paz00.c
> @@ -18,6 +18,7 @@
>   */
>  
>  #include <linux/platform_device.h>
> +#include <linux/gpio/driver.h>
>  #include <linux/rfkill-gpio.h>
>  #include "board.h"
>  
> @@ -36,7 +37,13 @@ static struct platform_device wifi_rfkill_device = {
>  	},
>  };
>  
> +static struct gpiod_lookup wifi_gpio_lookup[] = {
> +	GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio", NULL, 0, 0),
> +	GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio", NULL, 1, 0),
> +};

I wouldn't think this table would match for the gpios as the driver
currently is. From what I see, the driver calls into gpiod_get_index,
which will try 1 of 3 ways of getting the gpios:

of-enabled: of_find_gpio
	- which I believe wouldn't work for paz00, since rfkill
	  doesn't support dt?
acpi: acpi_find_gpio
	- I assume this does work, but I didn't dive into it
gpiod lookup table: gpiod_find
	- I think this is the path we expect to be taken, given the addition of
the lookup table here, but I don't think it would actually match.
Looking at the code for gpiod_find, it seems like it would try to match
the con_id, but would fail. Patch 2/6 is passing the reset_name and
shutdown_name for the con_ids, which isn't what is registered in this
table.

Shouldn't it look more like this?

+static struct gpiod_lookup wifi_gpio_lookup[] = {
+	GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio_reset", NULL, 0, 0),
+	GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio_shutdown", NULL, 1, 0),
+};

Sorry if I am missing something...

-rhyland

-- 
nvpublic

Re: [PATCH v3 1/6] ARM: tegra: add gpiod_lookup table for paz00

[Alexandre Courbot]

On Thu, Nov 28, 2013 at 1:47 AM, Rhyland Klein <rklein@nvidia.com> wrote:
> On 11/26/2013 5:05 AM, Mika Westerberg wrote:
>> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>>
>> This makes it possible to request the gpio descriptors in
>> rfkill_gpio driver regardless of the platform.
>>
>> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>> Tested-by: Stephen Warren <swarren@nvidia.com>
>> ---
>>  arch/arm/mach-tegra/board-paz00.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c
>> index 06f024070dab..a309795da665 100644
>> --- a/arch/arm/mach-tegra/board-paz00.c
>> +++ b/arch/arm/mach-tegra/board-paz00.c
>> @@ -18,6 +18,7 @@
>>   */
>>
>>  #include <linux/platform_device.h>
>> +#include <linux/gpio/driver.h>
>>  #include <linux/rfkill-gpio.h>
>>  #include "board.h"
>>
>> @@ -36,7 +37,13 @@ static struct platform_device wifi_rfkill_device = {
>>       },
>>  };
>>
>> +static struct gpiod_lookup wifi_gpio_lookup[] = {
>> +     GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio", NULL, 0, 0),
>> +     GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio", NULL, 1, 0),
>> +};
>
> I wouldn't think this table would match for the gpios as the driver
> currently is. From what I see, the driver calls into gpiod_get_index,
> which will try 1 of 3 ways of getting the gpios:
>
> of-enabled: of_find_gpio
>         - which I believe wouldn't work for paz00, since rfkill
>           doesn't support dt?
> acpi: acpi_find_gpio
>         - I assume this does work, but I didn't dive into it
> gpiod lookup table: gpiod_find
>         - I think this is the path we expect to be taken, given the addition of
> the lookup table here, but I don't think it would actually match.
> Looking at the code for gpiod_find, it seems like it would try to match
> the con_id, but would fail. Patch 2/6 is passing the reset_name and
> shutdown_name for the con_ids, which isn't what is registered in this
> table.
>
> Shouldn't it look more like this?
>
> +static struct gpiod_lookup wifi_gpio_lookup[] = {
> +       GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio_reset", NULL, 0, 0),
> +       GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio_shutdown", NULL, 1, 0),
> +};

The original GPIO lookup table specifies the device name
("rfkill_gpio"), no con_id, and an index. gpiod_find() will ensure
that the device names match, skip the con_id (as it is null in the
table), and again require that the indexes are the same. So provided
the instanciated device's dev_name() is "rfkill_gpio" (which it seems
to be according to the driver - not sure if it could become
"rfkill_gpio.0"), then I'd say the GPIOs will match. The con_id passed
to gpiod_get() will only be used as a label for debugfs. I am not sure
where the "rfkill_gpio_reset" you mention comes from?

One could wonder why the names of the GPIO are not hardcoded in the
driver instead of being defined as platform data, but that point could
be improved in a future patch.

It is true however that the platform GPIO lookup mechanism is
confusing at best, on top of being inefficient (one big linked list).
I have a patch in the pipe that should improve both points by making
GPIO lookup tables defined per-device - don't hesitate to merge this
series first if it works though.

Alex.

Re: [PATCH v3 1/6] ARM: tegra: add gpiod_lookup table for paz00

[Marc Dietrich]

Hi,

Am Donnerstag, 28. November 2013, 11:47:34 schrieb Alexandre Courbot:
> On Thu, Nov 28, 2013 at 1:47 AM, Rhyland Klein <rklein@nvidia.com> wrote:
> > On 11/26/2013 5:05 AM, Mika Westerberg wrote:
> >> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> >> 
> >> This makes it possible to request the gpio descriptors in
> >> rfkill_gpio driver regardless of the platform.
> >> 
> >> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> >> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> >> Tested-by: Stephen Warren <swarren@nvidia.com>
> >> ---
> >> 
> >>  arch/arm/mach-tegra/board-paz00.c | 7 +++++++
> >>  1 file changed, 7 insertions(+)
> >> 
> >> diff --git a/arch/arm/mach-tegra/board-paz00.c
> >> b/arch/arm/mach-tegra/board-paz00.c index 06f024070dab..a309795da665
> >> 100644
> >> --- a/arch/arm/mach-tegra/board-paz00.c
> >> +++ b/arch/arm/mach-tegra/board-paz00.c
> >> @@ -18,6 +18,7 @@
> >> 
> >>   */
> >>  
> >>  #include <linux/platform_device.h>
> >> 
> >> +#include <linux/gpio/driver.h>
> >> 
> >>  #include <linux/rfkill-gpio.h>
> >>  #include "board.h"
> >> 
> >> @@ -36,7 +37,13 @@ static struct platform_device wifi_rfkill_device = {
> >> 
> >>       },
> >>  
> >>  };
> >> 
> >> +static struct gpiod_lookup wifi_gpio_lookup[] = {
> >> +     GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio", NULL, 0, 0),
> >> +     GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio", NULL, 1, 0),
> >> +};
> > 
> > I wouldn't think this table would match for the gpios as the driver
> > currently is. From what I see, the driver calls into gpiod_get_index,
> > which will try 1 of 3 ways of getting the gpios:
> > 
> > of-enabled: of_find_gpio
> > 
> >         - which I believe wouldn't work for paz00, since rfkill
> >         
> >           doesn't support dt?
> > 
> > acpi: acpi_find_gpio
> > 
> >         - I assume this does work, but I didn't dive into it
> > 
> > gpiod lookup table: gpiod_find
> > 
> >         - I think this is the path we expect to be taken, given the
> >         addition of
> > 
> > the lookup table here, but I don't think it would actually match.
> > Looking at the code for gpiod_find, it seems like it would try to match
> > the con_id, but would fail. Patch 2/6 is passing the reset_name and
> > shutdown_name for the con_ids, which isn't what is registered in this
> > table.
> > 
> > Shouldn't it look more like this?
> > 
> > +static struct gpiod_lookup wifi_gpio_lookup[] = {
> > +       GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio_reset", NULL, 0,
> > 0),
> > +       GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio_shutdown", NULL, 1,
> > 0), +};
> 
> The original GPIO lookup table specifies the device name
> ("rfkill_gpio"), no con_id, and an index. gpiod_find() will ensure
> that the device names match, skip the con_id (as it is null in the
> table), and again require that the indexes are the same. So provided
> the instanciated device's dev_name() is "rfkill_gpio" (which it seems
> to be according to the driver - not sure if it could become
> "rfkill_gpio.0"), then I'd say the GPIOs will match. The con_id passed
> to gpiod_get() will only be used as a label for debugfs. I am not sure
> where the "rfkill_gpio_reset" you mention comes from?
> 
> One could wonder why the names of the GPIO are not hardcoded in the
> driver instead of being defined as platform data, but that point could
> be improved in a future patch.
> 
> It is true however that the platform GPIO lookup mechanism is
> confusing at best, on top of being inefficient (one big linked list).
> I have a patch in the pipe that should improve both points by making
> GPIO lookup tables defined per-device - don't hesitate to merge this
> series first if it works though.

I'll try to apply the patches and check on the actual hw. rfkill userspace 
command should be able to enable/disable the gpio. No idea how it finds the 
required gpio names.

The real problem with the rfkill driver is that it does not support DT. A 
naive attempt to convert it was made some year ago, but got rejected because 
rfkill wasn't seen as isolated device which can be represented in the device-
tree. Also it can not be added under some existing device node (e.g. the wifi 
driver) because those devices sit normally on an "enumeratable" bus (e.g. usb, 
pci), which is not listed in the device tree at all. This is why it still 
requires a board file and platform_data. I wish we could find a solution for 
this.

Marc

Re: [PATCH v3 1/6] ARM: tegra: add gpiod_lookup table for paz00

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 5601 bytes --]

On Thu, Nov 28, 2013 at 10:09:14AM +0100, Marc Dietrich wrote:
> Hi,
> 
> Am Donnerstag, 28. November 2013, 11:47:34 schrieb Alexandre Courbot:
> > On Thu, Nov 28, 2013 at 1:47 AM, Rhyland Klein <rklein@nvidia.com> wrote:
> > > On 11/26/2013 5:05 AM, Mika Westerberg wrote:
> > >> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > >> 
> > >> This makes it possible to request the gpio descriptors in
> > >> rfkill_gpio driver regardless of the platform.
> > >> 
> > >> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > >> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > >> Tested-by: Stephen Warren <swarren@nvidia.com>
> > >> ---
> > >> 
> > >>  arch/arm/mach-tegra/board-paz00.c | 7 +++++++
> > >>  1 file changed, 7 insertions(+)
> > >> 
> > >> diff --git a/arch/arm/mach-tegra/board-paz00.c
> > >> b/arch/arm/mach-tegra/board-paz00.c index 06f024070dab..a309795da665
> > >> 100644
> > >> --- a/arch/arm/mach-tegra/board-paz00.c
> > >> +++ b/arch/arm/mach-tegra/board-paz00.c
> > >> @@ -18,6 +18,7 @@
> > >> 
> > >>   */
> > >>  
> > >>  #include <linux/platform_device.h>
> > >> 
> > >> +#include <linux/gpio/driver.h>
> > >> 
> > >>  #include <linux/rfkill-gpio.h>
> > >>  #include "board.h"
> > >> 
> > >> @@ -36,7 +37,13 @@ static struct platform_device wifi_rfkill_device = {
> > >> 
> > >>       },
> > >>  
> > >>  };
> > >> 
> > >> +static struct gpiod_lookup wifi_gpio_lookup[] = {
> > >> +     GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio", NULL, 0, 0),
> > >> +     GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio", NULL, 1, 0),
> > >> +};
> > > 
> > > I wouldn't think this table would match for the gpios as the driver
> > > currently is. From what I see, the driver calls into gpiod_get_index,
> > > which will try 1 of 3 ways of getting the gpios:
> > > 
> > > of-enabled: of_find_gpio
> > > 
> > >         - which I believe wouldn't work for paz00, since rfkill
> > >         
> > >           doesn't support dt?
> > > 
> > > acpi: acpi_find_gpio
> > > 
> > >         - I assume this does work, but I didn't dive into it
> > > 
> > > gpiod lookup table: gpiod_find
> > > 
> > >         - I think this is the path we expect to be taken, given the
> > >         addition of
> > > 
> > > the lookup table here, but I don't think it would actually match.
> > > Looking at the code for gpiod_find, it seems like it would try to match
> > > the con_id, but would fail. Patch 2/6 is passing the reset_name and
> > > shutdown_name for the con_ids, which isn't what is registered in this
> > > table.
> > > 
> > > Shouldn't it look more like this?
> > > 
> > > +static struct gpiod_lookup wifi_gpio_lookup[] = {
> > > +       GPIO_LOOKUP_IDX("tegra-gpio", 25, "rfkill_gpio_reset", NULL, 0,
> > > 0),
> > > +       GPIO_LOOKUP_IDX("tegra-gpio", 85, "rfkill_gpio_shutdown", NULL, 1,
> > > 0), +};
> > 
> > The original GPIO lookup table specifies the device name
> > ("rfkill_gpio"), no con_id, and an index. gpiod_find() will ensure
> > that the device names match, skip the con_id (as it is null in the
> > table), and again require that the indexes are the same. So provided
> > the instanciated device's dev_name() is "rfkill_gpio" (which it seems
> > to be according to the driver - not sure if it could become
> > "rfkill_gpio.0"), then I'd say the GPIOs will match. The con_id passed
> > to gpiod_get() will only be used as a label for debugfs. I am not sure
> > where the "rfkill_gpio_reset" you mention comes from?
> > 
> > One could wonder why the names of the GPIO are not hardcoded in the
> > driver instead of being defined as platform data, but that point could
> > be improved in a future patch.
> > 
> > It is true however that the platform GPIO lookup mechanism is
> > confusing at best, on top of being inefficient (one big linked list).
> > I have a patch in the pipe that should improve both points by making
> > GPIO lookup tables defined per-device - don't hesitate to merge this
> > series first if it works though.
> 
> I'll try to apply the patches and check on the actual hw. rfkill userspace 
> command should be able to enable/disable the gpio. No idea how it finds the 
> required gpio names.
> 
> The real problem with the rfkill driver is that it does not support DT. A 
> naive attempt to convert it was made some year ago, but got rejected because 
> rfkill wasn't seen as isolated device which can be represented in the device-
> tree. Also it can not be added under some existing device node (e.g. the wifi 
> driver) because those devices sit normally on an "enumeratable" bus (e.g. usb, 
> pci), which is not listed in the device tree at all. This is why it still 
> requires a board file and platform_data. I wish we could find a solution for 
> this.

There is a solution at least for PCI. You can list PCI devices within
the device tree, which is really handy (required even) if for instance
one of the PCI devices is an SPI or I2C controller, each providing a bus
that cannot be probed. What you usually do is describe the PCI hierarchy
at least up to the controller and then list slaves as child nodes.

I'm not aware of anything similar for USB, but it should certainly be
possible to come up with a standard binding for the USB bus. It has a
topology that's similar enough to that of PCI so that the same general
rules could be applied.

If that's really the only thing that keeps rfkill from gaining DT
support then it's something worth tackling in my opinion.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 1/6] ARM: tegra: add gpiod_lookup table for paz00

[Marc Dietrich]

Am Donnerstag, 28. November 2013, 10:32:41 schrieb Thierry Reding:
> On Thu, Nov 28, 2013 at 10:09:14AM +0100, Marc Dietrich wrote:
> > The real problem with the rfkill driver is that it does not support DT. A
> > naive attempt to convert it was made some year ago, but got rejected
> > because rfkill wasn't seen as isolated device which can be represented in
> > the device- tree. Also it can not be added under some existing device
> > node (e.g. the wifi driver) because those devices sit normally on an
> > "enumeratable" bus (e.g. usb, pci), which is not listed in the device
> > tree at all. This is why it still requires a board file and
> > platform_data. I wish we could find a solution for this.
> 
> There is a solution at least for PCI. You can list PCI devices within
> the device tree, which is really handy (required even) if for instance
> one of the PCI devices is an SPI or I2C controller, each providing a bus
> that cannot be probed. What you usually do is describe the PCI hierarchy
> at least up to the controller and then list slaves as child nodes.
> 
> I'm not aware of anything similar for USB, but it should certainly be
> possible to come up with a standard binding for the USB bus. It has a
> topology that's similar enough to that of PCI so that the same general
> rules could be applied.
> 
> If that's really the only thing that keeps rfkill from gaining DT
> support then it's something worth tackling in my opinion.

it's not so simple I fear. The wifi driver needs to learn about the rfkill 
"device". As mentioned above, it's not really a device so the question is what 
needs to be added and where? The wifi driver just polls his own gpio lines to 
check the status of rfkill. Be we want to modify the "other side", so maybe 
this isn't related to the wifi driver at all. It's more a "virtual rfkill 
device". No idea if something like this exists already in device tree.

Marc

Re: [PATCH v3 1/6] ARM: tegra: add gpiod_lookup table for paz00

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3497 bytes --]

On Thu, Nov 28, 2013 at 11:20:19AM +0100, Marc Dietrich wrote:
> Am Donnerstag, 28. November 2013, 10:32:41 schrieb Thierry Reding:
> > On Thu, Nov 28, 2013 at 10:09:14AM +0100, Marc Dietrich wrote:
> > > The real problem with the rfkill driver is that it does not support DT. A
> > > naive attempt to convert it was made some year ago, but got rejected
> > > because rfkill wasn't seen as isolated device which can be represented in
> > > the device- tree. Also it can not be added under some existing device
> > > node (e.g. the wifi driver) because those devices sit normally on an
> > > "enumeratable" bus (e.g. usb, pci), which is not listed in the device
> > > tree at all. This is why it still requires a board file and
> > > platform_data. I wish we could find a solution for this.
> > 
> > There is a solution at least for PCI. You can list PCI devices within
> > the device tree, which is really handy (required even) if for instance
> > one of the PCI devices is an SPI or I2C controller, each providing a bus
> > that cannot be probed. What you usually do is describe the PCI hierarchy
> > at least up to the controller and then list slaves as child nodes.
> > 
> > I'm not aware of anything similar for USB, but it should certainly be
> > possible to come up with a standard binding for the USB bus. It has a
> > topology that's similar enough to that of PCI so that the same general
> > rules could be applied.
> > 
> > If that's really the only thing that keeps rfkill from gaining DT
> > support then it's something worth tackling in my opinion.
> 
> it's not so simple I fear. The wifi driver needs to learn about the rfkill 
> "device".

Why does the WiFi driver need to know about it? You say below that the
WiFi driver polls a separate set of GPIO lines (internal to the WiFi
module I assume?). In that case rfkill is merely a way to export control
of that functionality so that it can be used from some other part of the
kernel or from userspace.

That's very similar to things such as backlight control or fan control.
Still we manage to describe those in DT as well.

> As mentioned above, it's not really a device so the question is what 
> needs to be added and where? The wifi driver just polls his own gpio lines to 
> check the status of rfkill. Be we want to modify the "other side", so maybe 
> this isn't related to the wifi driver at all. It's more a "virtual rfkill 
> device". No idea if something like this exists already in device tree.

But it's a part of some other device. Or rather, it's always associated
with another device, right? So I don't see anything particularily wrong
with something like this:

	usb-controller {
		/* USB hub */
		usb@0,0 {
			...

			/* USB device */
			usb@1,0 {
				...

				rfkill {
					shutdown-gpio = <&gpio ...>;
					reset-gpio = <&gpio ...>;
				};

				...
			};

			...
		};
	};

You said that the main objection was that it needed to be represented as
a "subdevice" of whatever device it controls. If the only reason is that
we have no means to represent those devices because they are on a bus
that can be enumerated and therefore can't be listed in DT, then my
suggestion is to fix things so that we can describe those devices in DT.

The goal is to get rid of board files, right? board-paz00.c is the only
one left for Tegra, and rfkill is an actual hardware component, so there
is no reason why it can't be described in DT.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3 1/6] ARM: tegra: add gpiod_lookup table for paz00

[Marc Dietrich]

Am Donnerstag, 28. November 2013, 12:06:37 schrieb Thierry Reding:
> On Thu, Nov 28, 2013 at 11:20:19AM +0100, Marc Dietrich wrote:
> > Am Donnerstag, 28. November 2013, 10:32:41 schrieb Thierry Reding:
> > > On Thu, Nov 28, 2013 at 10:09:14AM +0100, Marc Dietrich wrote:
> > > > The real problem with the rfkill driver is that it does not support
> > > > DT. A
> > > > naive attempt to convert it was made some year ago, but got rejected
> > > > because rfkill wasn't seen as isolated device which can be represented
> > > > in
> > > > the device- tree. Also it can not be added under some existing device
> > > > node (e.g. the wifi driver) because those devices sit normally on an
> > > > "enumeratable" bus (e.g. usb, pci), which is not listed in the device
> > > > tree at all. This is why it still requires a board file and
> > > > platform_data. I wish we could find a solution for this.
> > > 
> > > There is a solution at least for PCI. You can list PCI devices within
> > > the device tree, which is really handy (required even) if for instance
> > > one of the PCI devices is an SPI or I2C controller, each providing a bus
> > > that cannot be probed. What you usually do is describe the PCI hierarchy
> > > at least up to the controller and then list slaves as child nodes.
> > > 
> > > I'm not aware of anything similar for USB, but it should certainly be
> > > possible to come up with a standard binding for the USB bus. It has a
> > > topology that's similar enough to that of PCI so that the same general
> > > rules could be applied.
> > > 
> > > If that's really the only thing that keeps rfkill from gaining DT
> > > support then it's something worth tackling in my opinion.
> > 
> > it's not so simple I fear. The wifi driver needs to learn about the rfkill
> > "device".
> 
> Why does the WiFi driver need to know about it? You say below that the
> WiFi driver polls a separate set of GPIO lines (internal to the WiFi
> module I assume?). In that case rfkill is merely a way to export control
> of that functionality so that it can be used from some other part of the
> kernel or from userspace.

You are right. We just need some device to bind this driver to. It doesn't 
need to be the wifi driver.

> That's very similar to things such as backlight control or fan control.
> Still we manage to describe those in DT as well.

Yes, rfkill is just an interface for userspace to able to control the gpio. 
E.g. backlight of medcom-wide seems to be related to the pwm controller, but 
is not a subnode of it. Instead it is a device of its own without parent. 
Hence, we may include rfkill in a similar, "free-standing" node. But this 
approch was rejected in the past. Maybe this changed now.

> > As mentioned above, it's not really a device so the question is what
> > needs to be added and where? The wifi driver just polls his own gpio lines
> > to check the status of rfkill. Be we want to modify the "other side", so
> > maybe this isn't related to the wifi driver at all. It's more a "virtual
> > rfkill device". No idea if something like this exists already in device
> > tree.
> But it's a part of some other device. Or rather, it's always associated
> with another device, right? So I don't see anything particularily wrong
> with something like this:
> 
> 	usb-controller {
> 		/* USB hub */
> 		usb@0,0 {
> 			...
> 
> 			/* USB device */
> 			usb@1,0 {
> 				...
> 
> 				rfkill {
> 					shutdown-gpio = <&gpio ...>;
> 					reset-gpio = <&gpio ...>;
> 				};
> 
> 				...
> 			};
> 
> 			...
> 		};
> 	};
> 
> You said that the main objection was that it needed to be represented as
> a "subdevice" of whatever device it controls. If the only reason is that
> we have no means to represent those devices because they are on a bus
> that can be enumerated and therefore can't be listed in DT, then my
> suggestion is to fix things so that we can describe those devices in DT.
>
> The goal is to get rid of board files, right? board-paz00.c is the only
> one left for Tegra, and rfkill is an actual hardware component, so there
> is no reason why it can't be described in DT.

Thinking a bit more about it, rfkill is neither a hw block nor a function of 
the wifi driver, so I guess it cannot be added to the usb controller (or a usb 
device).

Anyway, I think this discussion deserves a new mail thread, but I don't have 
enough background information to start one. We can bring this topic back when 
all turkeys who deserve it, are dead.

Marc

Re: [PATCH v3 1/6] ARM: tegra: add gpiod_lookup table for paz00

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1234 bytes --]

On Thu, Nov 28, 2013 at 01:54:10PM +0100, Marc Dietrich wrote:
[...]
> Yes, rfkill is just an interface for userspace to able to control the gpio. 
> E.g. backlight of medcom-wide seems to be related to the pwm controller, but 
> is not a subnode of it. Instead it is a device of its own without parent. 
> Hence, we may include rfkill in a similar, "free-standing" node. But this 
> approch was rejected in the past. Maybe this changed now.

Indeed. I think we really do need ways to represent this kind of device.
If we can't then how are we supposed to get rid of board files?

> Thinking a bit more about it, rfkill is neither a hw block nor a function of 
> the wifi driver, so I guess it cannot be added to the usb controller (or a usb 
> device).

I guess it depends a bit. Some rfkill-type devices may control more than
just WiFi (bluetooth, NFC, ...). If it really only controls WiFi, I
think it would still be reasonable to represent it as a child of the
WiFi device. It is, after all, a mechanism to control the WiFi hardware.

Alternatively I suppose we wouldn't really need to have an explicit
node. The WiFi driver could simply instantiate an rfkill device based on
its own properties.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

[PATCHv4] ARM: tegra: add gpiod_lookup table for paz00

[Heikki Krogerus]

This makes it possible to request the gpio descriptors in
rfkill_gpio driver regardless of the platform.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 arch/arm/mach-tegra/board-paz00.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Changes since v3:
-----------------
Rebased on top of Alexandre's patch "gpio: better lookup method for
platform GPIOs".

diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c
index 06f0240..e4dec9f 100644
--- a/arch/arm/mach-tegra/board-paz00.c
+++ b/arch/arm/mach-tegra/board-paz00.c
@@ -18,6 +18,7 @@
  */
 
 #include <linux/platform_device.h>
+#include <linux/gpio/driver.h>
 #include <linux/rfkill-gpio.h>
 #include "board.h"
 
@@ -36,7 +37,17 @@ static struct platform_device wifi_rfkill_device = {
 	},
 };
 
+static struct gpiod_lookup_table wifi_gpio_lookup = {
+	.dev_id = "rfkill_gpio",
+	.table = {
+		GPIO_LOOKUP_IDX("tegra-gpio", 25, NULL, 0, 0),
+		GPIO_LOOKUP_IDX("tegra-gpio", 85, NULL, 1, 0),
+		{ },
+	},
+};
+
 void __init tegra_paz00_wifikill_init(void)
 {
+	gpiod_add_lookup_table(&wifi_gpio_lookup);
 	platform_device_register(&wifi_rfkill_device);
 }
-- 
1.8.4.4

Re: [PATCHv4] ARM: tegra: add gpiod_lookup table for paz00

[Mika Westerberg]

On Tue, Dec 03, 2013 at 02:49:58PM +0200, Heikki Krogerus wrote:
> This makes it possible to request the gpio descriptors in
> rfkill_gpio driver regardless of the platform.
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  arch/arm/mach-tegra/board-paz00.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> Changes since v3:
> -----------------
> Rebased on top of Alexandre's patch "gpio: better lookup method for
> platform GPIOs".

Thanks Heikki.

Linus, are you OK with picking patches 2-6 from this series and replacing
patch 1 with this v4 from Heikki or should I resend the whole series?

Re: [PATCHv4] ARM: tegra: add gpiod_lookup table for paz00

[Stephen Warren]

On 12/03/2013 05:49 AM, Heikki Krogerus wrote:
> This makes it possible to request the gpio descriptors in
> rfkill_gpio driver regardless of the platform.

Acked-by: Stephen Warren <swarren@nvidia.com>

(I haven't retested V4. The previous versions worked fine...)

Re: [PATCHv4] ARM: tegra: add gpiod_lookup table for paz00

[Alex Courbot]

On 12/03/2013 09:49 PM, Heikki Krogerus wrote:
> This makes it possible to request the gpio descriptors in
> rfkill_gpio driver regardless of the platform.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>   arch/arm/mach-tegra/board-paz00.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
>
> Changes since v3:
> -----------------
> Rebased on top of Alexandre's patch "gpio: better lookup method for
> platform GPIOs".
>
> diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c
> index 06f0240..e4dec9f 100644
> --- a/arch/arm/mach-tegra/board-paz00.c
> +++ b/arch/arm/mach-tegra/board-paz00.c
> @@ -18,6 +18,7 @@
>    */
>
>   #include <linux/platform_device.h>
> +#include <linux/gpio/driver.h>
>   #include <linux/rfkill-gpio.h>
>   #include "board.h"
>
> @@ -36,7 +37,17 @@ static struct platform_device wifi_rfkill_device = {
>   	},
>   };
>
> +static struct gpiod_lookup_table wifi_gpio_lookup = {
> +	.dev_id = "rfkill_gpio",
> +	.table = {
> +		GPIO_LOOKUP_IDX("tegra-gpio", 25, NULL, 0, 0),
> +		GPIO_LOOKUP_IDX("tegra-gpio", 85, NULL, 1, 0),

nit: the flags could be changed to GPIO_ACTIVE_HIGH (which evaluates to 
0) to be more explicit.

Re: [PATCHv4] ARM: tegra: add gpiod_lookup table for paz00

[Linus Walleij]

On Tue, Dec 3, 2013 at 1:49 PM, Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:

> This makes it possible to request the gpio descriptors in
> rfkill_gpio driver regardless of the platform.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

Applied this v4 version to the devel branch in the GPIO tree with
Stephen's ACK, thanks!

Yours,
Linus Walleij

[PATCH v3 2/6] net: rfkill: gpio: convert to descriptor-based GPIO interface

[Mika Westerberg]

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

Convert to the safer gpiod_* family of API functions.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Tested-by: Stephen Warren <swarren@nvidia.com>
---
 net/rfkill/rfkill-gpio.c | 77 +++++++++++++++++++++---------------------------
 1 file changed, 34 insertions(+), 43 deletions(-)

diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 5620d3c07479..bd2a5b90400c 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -25,15 +25,15 @@
 #include <linux/clk.h>
 #include <linux/slab.h>
 #include <linux/acpi.h>
-#include <linux/acpi_gpio.h>
+#include <linux/gpio/consumer.h>
 
 #include <linux/rfkill-gpio.h>
 
 struct rfkill_gpio_data {
 	const char		*name;
 	enum rfkill_type	type;
-	int			reset_gpio;
-	int			shutdown_gpio;
+	struct gpio_desc	*reset_gpio;
+	struct gpio_desc	*shutdown_gpio;
 
 	struct rfkill		*rfkill_dev;
 	char			*reset_name;
@@ -48,19 +48,15 @@ static int rfkill_gpio_set_power(void *data, bool blocked)
 	struct rfkill_gpio_data *rfkill = data;
 
 	if (blocked) {
-		if (gpio_is_valid(rfkill->shutdown_gpio))
-			gpio_set_value(rfkill->shutdown_gpio, 0);
-		if (gpio_is_valid(rfkill->reset_gpio))
-			gpio_set_value(rfkill->reset_gpio, 0);
+		gpiod_set_value(rfkill->shutdown_gpio, 0);
+		gpiod_set_value(rfkill->reset_gpio, 0);
 		if (!IS_ERR(rfkill->clk) && rfkill->clk_enabled)
 			clk_disable(rfkill->clk);
 	} else {
 		if (!IS_ERR(rfkill->clk) && !rfkill->clk_enabled)
 			clk_enable(rfkill->clk);
-		if (gpio_is_valid(rfkill->reset_gpio))
-			gpio_set_value(rfkill->reset_gpio, 1);
-		if (gpio_is_valid(rfkill->shutdown_gpio))
-			gpio_set_value(rfkill->shutdown_gpio, 1);
+		gpiod_set_value(rfkill->reset_gpio, 1);
+		gpiod_set_value(rfkill->shutdown_gpio, 1);
 	}
 
 	rfkill->clk_enabled = blocked;
@@ -83,8 +79,6 @@ static int rfkill_gpio_acpi_probe(struct device *dev,
 
 	rfkill->name = dev_name(dev);
 	rfkill->type = (unsigned)id->driver_data;
-	rfkill->reset_gpio = acpi_get_gpio_by_index(dev, 0, NULL);
-	rfkill->shutdown_gpio = acpi_get_gpio_by_index(dev, 1, NULL);
 
 	return 0;
 }
@@ -94,8 +88,9 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 	struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
 	struct rfkill_gpio_data *rfkill;
 	const char *clk_name = NULL;
-	int ret = 0;
-	int len = 0;
+	struct gpio_desc *gpio;
+	int ret;
+	int len;
 
 	rfkill = devm_kzalloc(&pdev->dev, sizeof(*rfkill), GFP_KERNEL);
 	if (!rfkill)
@@ -109,28 +104,10 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 		clk_name = pdata->power_clk_name;
 		rfkill->name = pdata->name;
 		rfkill->type = pdata->type;
-		rfkill->reset_gpio = pdata->reset_gpio;
-		rfkill->shutdown_gpio = pdata->shutdown_gpio;
 	} else {
 		return -ENODEV;
 	}
 
-	/* make sure at-least one of the GPIO is defined and that
-	 * a name is specified for this instance */
-	if ((!gpio_is_valid(rfkill->reset_gpio) &&
-	     !gpio_is_valid(rfkill->shutdown_gpio)) || !rfkill->name) {
-		pr_warn("%s: invalid platform data\n", __func__);
-		return -EINVAL;
-	}
-
-	if (pdata && pdata->gpio_runtime_setup) {
-		ret = pdata->gpio_runtime_setup(pdev);
-		if (ret) {
-			pr_warn("%s: can't set up gpio\n", __func__);
-			return ret;
-		}
-	}
-
 	len = strlen(rfkill->name);
 	rfkill->reset_name = devm_kzalloc(&pdev->dev, len + 7, GFP_KERNEL);
 	if (!rfkill->reset_name)
@@ -145,20 +122,34 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 
 	rfkill->clk = devm_clk_get(&pdev->dev, clk_name);
 
-	if (gpio_is_valid(rfkill->reset_gpio)) {
-		ret = devm_gpio_request_one(&pdev->dev, rfkill->reset_gpio,
-					    0, rfkill->reset_name);
-		if (ret) {
-			pr_warn("%s: failed to get reset gpio.\n", __func__);
+	gpio = devm_gpiod_get_index(&pdev->dev, rfkill->reset_name, 0);
+	if (!IS_ERR(gpio)) {
+		ret = gpiod_direction_output(gpio, 0);
+		if (ret)
 			return ret;
-		}
+		rfkill->reset_gpio = gpio;
+	}
+
+	gpio = devm_gpiod_get_index(&pdev->dev, rfkill->shutdown_name, 1);
+	if (!IS_ERR(gpio)) {
+		ret = gpiod_direction_output(gpio, 0);
+		if (ret)
+			return ret;
+		rfkill->shutdown_gpio = gpio;
 	}
 
-	if (gpio_is_valid(rfkill->shutdown_gpio)) {
-		ret = devm_gpio_request_one(&pdev->dev, rfkill->shutdown_gpio,
-					    0, rfkill->shutdown_name);
+	/* Make sure at-least one of the GPIO is defined and that
+	 * a name is specified for this instance
+	 */
+	if ((!rfkill->reset_gpio && !rfkill->shutdown_gpio) || !rfkill->name) {
+		dev_err(&pdev->dev, "invalid platform data\n");
+		return -EINVAL;
+	}
+
+	if (pdata && pdata->gpio_runtime_setup) {
+		ret = pdata->gpio_runtime_setup(pdev);
 		if (ret) {
-			pr_warn("%s: failed to get shutdown gpio.\n", __func__);
+			dev_err(&pdev->dev, "can't set up gpio\n");
 			return ret;
 		}
 	}
-- 
1.8.4.3

Re: [PATCH v3 2/6] net: rfkill: gpio: convert to descriptor-based GPIO interface

[Alex Courbot]

On 11/26/2013 07:05 PM, Mika Westerberg wrote:
> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>
> Convert to the safer gpiod_* family of API functions.

Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>

Re: [PATCH v3 2/6] net: rfkill: gpio: convert to descriptor-based GPIO interface

[Linus Walleij]

On Tue, Nov 26, 2013 at 11:05 AM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>
> Convert to the safer gpiod_* family of API functions.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Tested-by: Stephen Warren <swarren@nvidia.com>
> ---
>  net/rfkill/rfkill-gpio.c | 77 +++++++++++++++++++++---------------------------
>  1 file changed, 34 insertions(+), 43 deletions(-)
>
> diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
> index 5620d3c07479..bd2a5b90400c 100644
> --- a/net/rfkill/rfkill-gpio.c
> +++ b/net/rfkill/rfkill-gpio.c
> @@ -25,15 +25,15 @@
>  #include <linux/clk.h>
>  #include <linux/slab.h>
>  #include <linux/acpi.h>
> -#include <linux/acpi_gpio.h>
> +#include <linux/gpio/consumer.h>
>
>  #include <linux/rfkill-gpio.h>
>
>  struct rfkill_gpio_data {
>         const char              *name;
>         enum rfkill_type        type;
> -       int                     reset_gpio;
> -       int                     shutdown_gpio;
> +       struct gpio_desc        *reset_gpio;
> +       struct gpio_desc        *shutdown_gpio;
>
>         struct rfkill           *rfkill_dev;
>         char                    *reset_name;
> @@ -48,19 +48,15 @@ static int rfkill_gpio_set_power(void *data, bool blocked)
>         struct rfkill_gpio_data *rfkill = data;
>
>         if (blocked) {
> -               if (gpio_is_valid(rfkill->shutdown_gpio))
> -                       gpio_set_value(rfkill->shutdown_gpio, 0);
> -               if (gpio_is_valid(rfkill->reset_gpio))
> -                       gpio_set_value(rfkill->reset_gpio, 0);
> +               gpiod_set_value(rfkill->shutdown_gpio, 0);
> +               gpiod_set_value(rfkill->reset_gpio, 0);
>                 if (!IS_ERR(rfkill->clk) && rfkill->clk_enabled)
>                         clk_disable(rfkill->clk);
>         } else {
>                 if (!IS_ERR(rfkill->clk) && !rfkill->clk_enabled)
>                         clk_enable(rfkill->clk);
> -               if (gpio_is_valid(rfkill->reset_gpio))
> -                       gpio_set_value(rfkill->reset_gpio, 1);
> -               if (gpio_is_valid(rfkill->shutdown_gpio))
> -                       gpio_set_value(rfkill->shutdown_gpio, 1);
> +               gpiod_set_value(rfkill->reset_gpio, 1);
> +               gpiod_set_value(rfkill->shutdown_gpio, 1);
>         }
>
>         rfkill->clk_enabled = blocked;
> @@ -83,8 +79,6 @@ static int rfkill_gpio_acpi_probe(struct device *dev,
>
>         rfkill->name = dev_name(dev);
>         rfkill->type = (unsigned)id->driver_data;
> -       rfkill->reset_gpio = acpi_get_gpio_by_index(dev, 0, NULL);
> -       rfkill->shutdown_gpio = acpi_get_gpio_by_index(dev, 1, NULL);
>
>         return 0;
>  }
> @@ -94,8 +88,9 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
>         struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
>         struct rfkill_gpio_data *rfkill;
>         const char *clk_name = NULL;
> -       int ret = 0;
> -       int len = 0;
> +       struct gpio_desc *gpio;
> +       int ret;
> +       int len;
>
>         rfkill = devm_kzalloc(&pdev->dev, sizeof(*rfkill), GFP_KERNEL);
>         if (!rfkill)
> @@ -109,28 +104,10 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
>                 clk_name = pdata->power_clk_name;
>                 rfkill->name = pdata->name;
>                 rfkill->type = pdata->type;
> -               rfkill->reset_gpio = pdata->reset_gpio;
> -               rfkill->shutdown_gpio = pdata->shutdown_gpio;
>         } else {
>                 return -ENODEV;
>         }
>
> -       /* make sure at-least one of the GPIO is defined and that
> -        * a name is specified for this instance */
> -       if ((!gpio_is_valid(rfkill->reset_gpio) &&
> -            !gpio_is_valid(rfkill->shutdown_gpio)) || !rfkill->name) {
> -               pr_warn("%s: invalid platform data\n", __func__);
> -               return -EINVAL;
> -       }
> -
> -       if (pdata && pdata->gpio_runtime_setup) {
> -               ret = pdata->gpio_runtime_setup(pdev);
> -               if (ret) {
> -                       pr_warn("%s: can't set up gpio\n", __func__);
> -                       return ret;
> -               }
> -       }
> -
>         len = strlen(rfkill->name);
>         rfkill->reset_name = devm_kzalloc(&pdev->dev, len + 7, GFP_KERNEL);
>         if (!rfkill->reset_name)
> @@ -145,20 +122,34 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
>
>         rfkill->clk = devm_clk_get(&pdev->dev, clk_name);
>
> -       if (gpio_is_valid(rfkill->reset_gpio)) {
> -               ret = devm_gpio_request_one(&pdev->dev, rfkill->reset_gpio,
> -                                           0, rfkill->reset_name);
> -               if (ret) {
> -                       pr_warn("%s: failed to get reset gpio.\n", __func__);
> +       gpio = devm_gpiod_get_index(&pdev->dev, rfkill->reset_name, 0);
> +       if (!IS_ERR(gpio)) {
> +               ret = gpiod_direction_output(gpio, 0);
> +               if (ret)
>                         return ret;
> -               }
> +               rfkill->reset_gpio = gpio;
> +       }
> +
> +       gpio = devm_gpiod_get_index(&pdev->dev, rfkill->shutdown_name, 1);
> +       if (!IS_ERR(gpio)) {
> +               ret = gpiod_direction_output(gpio, 0);
> +               if (ret)
> +                       return ret;
> +               rfkill->shutdown_gpio = gpio;
>         }
>
> -       if (gpio_is_valid(rfkill->shutdown_gpio)) {
> -               ret = devm_gpio_request_one(&pdev->dev, rfkill->shutdown_gpio,
> -                                           0, rfkill->shutdown_name);
> +       /* Make sure at-least one of the GPIO is defined and that
> +        * a name is specified for this instance
> +        */
> +       if ((!rfkill->reset_gpio && !rfkill->shutdown_gpio) || !rfkill->name) {
> +               dev_err(&pdev->dev, "invalid platform data\n");
> +               return -EINVAL;
> +       }
> +
> +       if (pdata && pdata->gpio_runtime_setup) {
> +               ret = pdata->gpio_runtime_setup(pdev);
>                 if (ret) {
> -                       pr_warn("%s: failed to get shutdown gpio.\n", __func__);
> +                       dev_err(&pdev->dev, "can't set up gpio\n");
>                         return ret;
>                 }
>         }

DavidM: can I have an ACK from you to apply this patch through the GPIO
tree?

Yours,
Linus Walleij

Re: [PATCH v3 2/6] net: rfkill: gpio: convert to descriptor-based GPIO interface

[Mika Westerberg]

On Wed, Dec 11, 2013 at 01:00:18PM +0100, Linus Walleij wrote:
> On Tue, Nov 26, 2013 at 11:05 AM, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> 
> > From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> >
> > Convert to the safer gpiod_* family of API functions.
> >
> > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Tested-by: Stephen Warren <swarren@nvidia.com>

Johannes B, David M,

Are you fine with this patch? If yes, could you ack it so that we could get
it merged for 3.14.

Thanks!

Here's a link to this series:

https://lkml.org/lkml/2013/11/26/104

Re: [PATCH v3 2/6] net: rfkill: gpio: convert to descriptor-based GPIO interface

[Johannes Berg]

On Mon, 2013-12-23 at 12:54 +0200, Mika Westerberg wrote:
> On Wed, Dec 11, 2013 at 01:00:18PM +0100, Linus Walleij wrote:
> > On Tue, Nov 26, 2013 at 11:05 AM, Mika Westerberg
> > <mika.westerberg@linux.intel.com> wrote:
> > 
> > > From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > >
> > > Convert to the safer gpiod_* family of API functions.
> > >
> > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > > Tested-by: Stephen Warren <swarren@nvidia.com>
> 
> Johannes B, David M,
> 
> Are you fine with this patch? If yes, could you ack it so that we could get
> it merged for 3.14.

I have no objection to this particular patch. I can't really comment on
it, but I never really delved too deeply into the rfkill-gpio thing - I
guess it works for whoever needed it :)

johannes

Re: [PATCH v3 2/6] net: rfkill: gpio: convert to descriptor-based GPIO interface

[Linus Walleij]

On Mon, Dec 23, 2013 at 10:14 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Mon, 2013-12-23 at 12:54 +0200, Mika Westerberg wrote:
>> On Wed, Dec 11, 2013 at 01:00:18PM +0100, Linus Walleij wrote:
>> > On Tue, Nov 26, 2013 at 11:05 AM, Mika Westerberg
>> > <mika.westerberg@linux.intel.com> wrote:
>> >
>> > > From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> > >
>> > > Convert to the safer gpiod_* family of API functions.
>> > >
>> > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>> > > Tested-by: Stephen Warren <swarren@nvidia.com>
>>
>> Johannes B, David M,
>>
>> Are you fine with this patch? If yes, could you ack it so that we could get
>> it merged for 3.14.
>
> I have no objection to this particular patch. I can't really comment on
> it, but I never really delved too deeply into the rfkill-gpio thing - I
> guess it works for whoever needed it :)

I take that as an ACK ;-)

Patch applied.

Yours,
Linus Walleij

[PATCH v3 3/6] mmc: sdhci-acpi: convert to use GPIO descriptor API

[Mika Westerberg]

The new descriptor based GPIO interface is now the recommended and safer
way of using GPIOs from device drivers. Convert the ACPI SDHCI driver to
use that interface.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/mmc/host/sdhci-acpi.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index ef19874fcd1f..5c86550f83ad 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -31,10 +31,9 @@
 #include <linux/bitops.h>
 #include <linux/types.h>
 #include <linux/err.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/interrupt.h>
 #include <linux/acpi.h>
-#include <linux/acpi_gpio.h>
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/delay.h>
@@ -199,22 +198,23 @@ static irqreturn_t sdhci_acpi_sd_cd(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static int sdhci_acpi_add_own_cd(struct device *dev, int gpio,
-				 struct mmc_host *mmc)
+static int sdhci_acpi_add_own_cd(struct device *dev, struct mmc_host *mmc)
 {
+	struct gpio_desc *desc;
 	unsigned long flags;
 	int err, irq;
 
-	if (gpio < 0) {
-		err = gpio;
+	desc = devm_gpiod_get_index(dev, "sd_cd", 0);
+	if (IS_ERR(desc)) {
+		err = PTR_ERR(desc);
 		goto out;
 	}
 
-	err = devm_gpio_request_one(dev, gpio, GPIOF_DIR_IN, "sd_cd");
+	err = gpiod_direction_input(desc);
 	if (err)
-		goto out;
+		goto out_free;
 
-	irq = gpio_to_irq(gpio);
+	irq = gpiod_to_irq(desc);
 	if (irq < 0) {
 		err = irq;
 		goto out_free;
@@ -228,7 +228,7 @@ static int sdhci_acpi_add_own_cd(struct device *dev, int gpio,
 	return 0;
 
 out_free:
-	devm_gpio_free(dev, gpio);
+	devm_gpiod_put(dev, desc);
 out:
 	dev_warn(dev, "failed to setup card detect wake up\n");
 	return err;
@@ -254,7 +254,7 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
 	struct resource *iomem;
 	resource_size_t len;
 	const char *hid;
-	int err, gpio;
+	int err;
 
 	if (acpi_bus_get_device(handle, &device))
 		return -ENODEV;
@@ -279,8 +279,6 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
 	if (IS_ERR(host))
 		return PTR_ERR(host);
 
-	gpio = acpi_get_gpio_by_index(dev, 0, NULL);
-
 	c = sdhci_priv(host);
 	c->host = host;
 	c->slot = sdhci_acpi_get_slot(handle, hid);
@@ -338,7 +336,7 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
 		goto err_free;
 
 	if (sdhci_acpi_flag(c, SDHCI_ACPI_SD_CD)) {
-		if (sdhci_acpi_add_own_cd(dev, gpio, host->mmc))
+		if (sdhci_acpi_add_own_cd(dev, host->mmc))
 			c->use_runtime_pm = false;
 	}
 
-- 
1.8.4.3

Re: [PATCH v3 3/6] mmc: sdhci-acpi: convert to use GPIO descriptor API

[Linus Walleij]

On Tue, Nov 26, 2013 at 11:05 AM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> The new descriptor based GPIO interface is now the recommended and safer
> way of using GPIOs from device drivers. Convert the ACPI SDHCI driver to
> use that interface.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> Acked-by: Alexandre Courbot <acourbot@nvidia.com>

Chris, can I have your ACK on this patch?

Yours,
Linus Walleij

[PATCH v3 4/6] gpio / ACPI: register to ACPI events automatically

[Mika Westerberg]

Instead of asking each driver to register to ACPI events we can just call
acpi_gpiochip_register_interrupts() for each chip that has an ACPI handle.
The function checks chip->to_irq and if it is set to NULL (a GPIO driver
that doesn't do interrupts) the function does nothing.

We also add the a new header drivers/gpio/gpiolib.h that is used for
functions internal to gpiolib and add ACPI GPIO chip registering functions
to that header.

Once that is done we can remove call to acpi_gpiochip_register_interrupts()
from its only user, pinctrl-baytrail.c

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/gpio/gpiolib-acpi.c        | 16 ++++++++++++----
 drivers/gpio/gpiolib.c             |  4 ++++
 drivers/gpio/gpiolib.h             | 23 +++++++++++++++++++++++
 drivers/pinctrl/pinctrl-baytrail.c |  4 ----
 include/linux/acpi_gpio.h          |  6 ------
 5 files changed, 39 insertions(+), 14 deletions(-)
 create mode 100644 drivers/gpio/gpiolib.h

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index ae0ffdce8bd5..cb2da66fbbfe 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -94,7 +94,7 @@ static void acpi_gpio_evt_dh(acpi_handle handle, void *data)
  * gpio pins have acpi event methods and assigns interrupt handlers that calls
  * the acpi event methods for those pins.
  */
-void acpi_gpiochip_request_interrupts(struct gpio_chip *chip)
+static void acpi_gpiochip_request_interrupts(struct gpio_chip *chip)
 {
 	struct acpi_buffer buf = {ACPI_ALLOCATE_BUFFER, NULL};
 	struct acpi_resource *res;
@@ -192,7 +192,6 @@ void acpi_gpiochip_request_interrupts(struct gpio_chip *chip)
 				irq);
 	}
 }
-EXPORT_SYMBOL(acpi_gpiochip_request_interrupts);
 
 /**
  * acpi_gpiochip_free_interrupts() - Free GPIO _EVT ACPI event interrupts.
@@ -203,7 +202,7 @@ EXPORT_SYMBOL(acpi_gpiochip_request_interrupts);
  * The remaining ACPI event interrupts associated with the chip are freed
  * automatically.
  */
-void acpi_gpiochip_free_interrupts(struct gpio_chip *chip)
+static void acpi_gpiochip_free_interrupts(struct gpio_chip *chip)
 {
 	acpi_handle handle;
 	acpi_status status;
@@ -230,7 +229,6 @@ void acpi_gpiochip_free_interrupts(struct gpio_chip *chip)
 	acpi_detach_data(handle, acpi_gpio_evt_dh);
 	kfree(evt_pins);
 }
-EXPORT_SYMBOL(acpi_gpiochip_free_interrupts);
 
 struct acpi_gpio_lookup {
 	struct acpi_gpio_info info;
@@ -310,3 +308,13 @@ struct gpio_desc *acpi_get_gpiod_by_index(struct device *dev, int index,
 	return lookup.desc ? lookup.desc : ERR_PTR(-ENODEV);
 }
 EXPORT_SYMBOL_GPL(acpi_get_gpiod_by_index);
+
+void acpi_gpiochip_add(struct gpio_chip *chip)
+{
+	acpi_gpiochip_request_interrupts(chip);
+}
+
+void acpi_gpiochip_remove(struct gpio_chip *chip)
+{
+	acpi_gpiochip_free_interrupts(chip);
+}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 4e10b10d3ddd..70abccf0d144 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -15,6 +15,8 @@
 #include <linux/slab.h>
 #include <linux/acpi.h>
 
+#include "gpiolib.h"
+
 #define CREATE_TRACE_POINTS
 #include <trace/events/gpio.h>
 
@@ -1212,6 +1214,7 @@ int gpiochip_add(struct gpio_chip *chip)
 #endif
 
 	of_gpiochip_add(chip);
+	acpi_gpiochip_add(chip);
 
 	if (status)
 		goto fail;
@@ -1253,6 +1256,7 @@ int gpiochip_remove(struct gpio_chip *chip)
 
 	gpiochip_remove_pin_ranges(chip);
 	of_gpiochip_remove(chip);
+	acpi_gpiochip_remove(chip);
 
 	for (id = 0; id < chip->ngpio; id++) {
 		if (test_bit(FLAG_REQUESTED, &chip->desc[id].flags)) {
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
new file mode 100644
index 000000000000..2ed23ab8298c
--- /dev/null
+++ b/drivers/gpio/gpiolib.h
@@ -0,0 +1,23 @@
+/*
+ * Internal GPIO functions.
+ *
+ * Copyright (C) 2013, Intel Corporation
+ * Author: Mika Westerberg <mika.westerberg@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.
+ */
+
+#ifndef GPIOLIB_H
+#define GPIOLIB_H
+
+#ifdef CONFIG_ACPI
+void acpi_gpiochip_add(struct gpio_chip *chip);
+void acpi_gpiochip_remove(struct gpio_chip *chip);
+#else
+static inline void acpi_gpiochip_add(struct gpio_chip *chip) { }
+static inline void acpi_gpiochip_remove(struct gpio_chip *chip) { }
+#endif
+
+#endif /* GPIOLIB_H */
diff --git a/drivers/pinctrl/pinctrl-baytrail.c b/drivers/pinctrl/pinctrl-baytrail.c
index 2832576d8b12..98b87a59df2d 100644
--- a/drivers/pinctrl/pinctrl-baytrail.c
+++ b/drivers/pinctrl/pinctrl-baytrail.c
@@ -29,7 +29,6 @@
 #include <linux/gpio.h>
 #include <linux/irqdomain.h>
 #include <linux/acpi.h>
-#include <linux/acpi_gpio.h>
 #include <linux/platform_device.h>
 #include <linux/seq_file.h>
 #include <linux/io.h>
@@ -485,9 +484,6 @@ static int byt_gpio_probe(struct platform_device *pdev)
 
 		irq_set_handler_data(hwirq, vg);
 		irq_set_chained_handler(hwirq, byt_gpio_irq_handler);
-
-		/* Register interrupt handlers for gpio signaled acpi events */
-		acpi_gpiochip_request_interrupts(gc);
 	}
 
 	pm_runtime_enable(dev);
diff --git a/include/linux/acpi_gpio.h b/include/linux/acpi_gpio.h
index d875bc3dba3c..af96a0d452f6 100644
--- a/include/linux/acpi_gpio.h
+++ b/include/linux/acpi_gpio.h
@@ -21,9 +21,6 @@ struct acpi_gpio_info {
 
 struct gpio_desc *acpi_get_gpiod_by_index(struct device *dev, int index,
 					  struct acpi_gpio_info *info);
-void acpi_gpiochip_request_interrupts(struct gpio_chip *chip);
-void acpi_gpiochip_free_interrupts(struct gpio_chip *chip);
-
 #else /* CONFIG_GPIO_ACPI */
 
 static inline struct gpio_desc *
@@ -33,9 +30,6 @@ acpi_get_gpiod_by_index(struct device *dev, int index,
 	return ERR_PTR(-ENOSYS);
 }
 
-static inline void acpi_gpiochip_request_interrupts(struct gpio_chip *chip) { }
-static inline void acpi_gpiochip_free_interrupts(struct gpio_chip *chip) { }
-
 #endif /* CONFIG_GPIO_ACPI */
 
 static inline int acpi_get_gpio_by_index(struct device *dev, int index,
-- 
1.8.4.3

Re: [PATCH v3 4/6] gpio / ACPI: register to ACPI events automatically

[Linus Walleij]

On Tue, Nov 26, 2013 at 11:05 AM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> Instead of asking each driver to register to ACPI events we can just call
> acpi_gpiochip_register_interrupts() for each chip that has an ACPI handle.
> The function checks chip->to_irq and if it is set to NULL (a GPIO driver
> that doesn't do interrupts) the function does nothing.
>
> We also add the a new header drivers/gpio/gpiolib.h that is used for
> functions internal to gpiolib and add ACPI GPIO chip registering functions
> to that header.
>
> Once that is done we can remove call to acpi_gpiochip_register_interrupts()
> from its only user, pinctrl-baytrail.c
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Two questions:

- Can you please rebase patches 3-6 on my GPIO tree "devel" branch?

- Can this patch be placed first? It does not seem to depend on the
  others, to to push dependent patches to the end of the series.

Yours,
Linus Walleij

Re: [PATCH v3 4/6] gpio / ACPI: register to ACPI events automatically

[Mika Westerberg]

On Tue, Jan 07, 2014 at 06:50:00PM +0100, Linus Walleij wrote:
> On Tue, Nov 26, 2013 at 11:05 AM, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> 
> > Instead of asking each driver to register to ACPI events we can just call
> > acpi_gpiochip_register_interrupts() for each chip that has an ACPI handle.
> > The function checks chip->to_irq and if it is set to NULL (a GPIO driver
> > that doesn't do interrupts) the function does nothing.
> >
> > We also add the a new header drivers/gpio/gpiolib.h that is used for
> > functions internal to gpiolib and add ACPI GPIO chip registering functions
> > to that header.
> >
> > Once that is done we can remove call to acpi_gpiochip_register_interrupts()
> > from its only user, pinctrl-baytrail.c
> >
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Two questions:
> 
> - Can you please rebase patches 3-6 on my GPIO tree "devel" branch?

Yes.

> - Can this patch be placed first? It does not seem to depend on the
>   others, to to push dependent patches to the end of the series.

Sure - I'll post a rebased version in a moment.

[PATCH v3 5/6] gpio / ACPI: get rid of acpi_gpio.h

[Mika Westerberg]

Now that all users of acpi_gpio.h have been moved to use either the GPIO
descriptor interface or to the internal gpiolib.h we can get rid of
acpi_gpio.h entirely.

Once this is done the only interface to get GPIOs to drivers enumerated
from ACPI namespace is the descriptor based interface.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpio/gpiolib-acpi.c |  5 +++--
 drivers/gpio/gpiolib.c      |  1 -
 drivers/gpio/gpiolib.h      | 23 +++++++++++++++++++++++
 include/linux/acpi_gpio.h   | 45 ---------------------------------------------
 4 files changed, 26 insertions(+), 48 deletions(-)
 delete mode 100644 include/linux/acpi_gpio.h

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index cb2da66fbbfe..8506e4ce41f7 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -12,11 +12,13 @@
 
 #include <linux/errno.h>
 #include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
 #include <linux/export.h>
-#include <linux/acpi_gpio.h>
 #include <linux/acpi.h>
 #include <linux/interrupt.h>
 
+#include "gpiolib.h"
+
 struct acpi_gpio_evt_pin {
 	struct list_head node;
 	acpi_handle *evt_handle;
@@ -307,7 +309,6 @@ struct gpio_desc *acpi_get_gpiod_by_index(struct device *dev, int index,
 
 	return lookup.desc ? lookup.desc : ERR_PTR(-ENODEV);
 }
-EXPORT_SYMBOL_GPL(acpi_get_gpiod_by_index);
 
 void acpi_gpiochip_add(struct gpio_chip *chip)
 {
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 70abccf0d144..a6b82413c290 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -10,7 +10,6 @@
 #include <linux/seq_file.h>
 #include <linux/gpio.h>
 #include <linux/of_gpio.h>
-#include <linux/acpi_gpio.h>
 #include <linux/idr.h>
 #include <linux/slab.h>
 #include <linux/acpi.h>
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index 2ed23ab8298c..82be586c1f90 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -12,12 +12,35 @@
 #ifndef GPIOLIB_H
 #define GPIOLIB_H
 
+#include <linux/err.h>
+#include <linux/device.h>
+
+/**
+ * struct acpi_gpio_info - ACPI GPIO specific information
+ * @gpioint: if %true this GPIO is of type GpioInt otherwise type is GpioIo
+ * @active_low: in case of @gpioint, the pin is active low
+ */
+struct acpi_gpio_info {
+	bool gpioint;
+	bool active_low;
+};
+
 #ifdef CONFIG_ACPI
 void acpi_gpiochip_add(struct gpio_chip *chip);
 void acpi_gpiochip_remove(struct gpio_chip *chip);
+
+struct gpio_desc *acpi_get_gpiod_by_index(struct device *dev, int index,
+					  struct acpi_gpio_info *info);
 #else
 static inline void acpi_gpiochip_add(struct gpio_chip *chip) { }
 static inline void acpi_gpiochip_remove(struct gpio_chip *chip) { }
+
+static inline struct gpio_desc *
+acpi_get_gpiod_by_index(struct device *dev, int index,
+			struct acpi_gpio_info *info)
+{
+	return ERR_PTR(-ENOSYS);
+}
 #endif
 
 #endif /* GPIOLIB_H */
diff --git a/include/linux/acpi_gpio.h b/include/linux/acpi_gpio.h
deleted file mode 100644
index af96a0d452f6..000000000000
--- a/include/linux/acpi_gpio.h
+++ /dev/null
@@ -1,45 +0,0 @@
-#ifndef _LINUX_ACPI_GPIO_H_
-#define _LINUX_ACPI_GPIO_H_
-
-#include <linux/device.h>
-#include <linux/err.h>
-#include <linux/errno.h>
-#include <linux/gpio.h>
-#include <linux/gpio/consumer.h>
-
-/**
- * struct acpi_gpio_info - ACPI GPIO specific information
- * @gpioint: if %true this GPIO is of type GpioInt otherwise type is GpioIo
- * @active_low: in case of @gpioint, the pin is active low
- */
-struct acpi_gpio_info {
-	bool gpioint;
-	bool active_low;
-};
-
-#ifdef CONFIG_GPIO_ACPI
-
-struct gpio_desc *acpi_get_gpiod_by_index(struct device *dev, int index,
-					  struct acpi_gpio_info *info);
-#else /* CONFIG_GPIO_ACPI */
-
-static inline struct gpio_desc *
-acpi_get_gpiod_by_index(struct device *dev, int index,
-			struct acpi_gpio_info *info)
-{
-	return ERR_PTR(-ENOSYS);
-}
-
-#endif /* CONFIG_GPIO_ACPI */
-
-static inline int acpi_get_gpio_by_index(struct device *dev, int index,
-					 struct acpi_gpio_info *info)
-{
-	struct gpio_desc *desc = acpi_get_gpiod_by_index(dev, index, info);
-
-	if (IS_ERR(desc))
-		return PTR_ERR(desc);
-	return desc_to_gpio(desc);
-}
-
-#endif /* _LINUX_ACPI_GPIO_H_ */
-- 
1.8.4.3

Re: [PATCH v3 5/6] gpio / ACPI: get rid of acpi_gpio.h

[Linus Walleij]

On Tue, Nov 26, 2013 at 11:05 AM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> Now that all users of acpi_gpio.h have been moved to use either the GPIO
> descriptor interface or to the internal gpiolib.h we can get rid of
> acpi_gpio.h entirely.
>
> Once this is done the only interface to get GPIOs to drivers enumerated
> from ACPI namespace is the descriptor based interface.
>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Acked-by: Alexandre Courbot <acourbot@nvidia.com>

I must say I really like this patch. This is exactly how we should
proceed with everything also for device tree, getting rid of
<linux/of_gpio.h> - OK maybe some time will be required but
surely we can get there.

I'm giving subsystem maintainers some grace time but this is a
real important series for v3.14.

Yours,
Linus Walleij

[PATCH v3 6/6] Documentation / ACPI: update to GPIO descriptor API

[Mika Westerberg]

Update the documentation also to reflect the fact that there are no ACPI
specific GPIO interfaces anymore but drivers should instead use the
descriptor based GPIO APIs.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 Documentation/acpi/enumeration.txt | 36 +++++++-----------------------------
 1 file changed, 7 insertions(+), 29 deletions(-)

diff --git a/Documentation/acpi/enumeration.txt b/Documentation/acpi/enumeration.txt
index b994bcb32b92..2a1519b87177 100644
--- a/Documentation/acpi/enumeration.txt
+++ b/Documentation/acpi/enumeration.txt
@@ -293,36 +293,13 @@ the device to the driver. For example:
 
 These GPIO numbers are controller relative and path "\\_SB.PCI0.GPI0"
 specifies the path to the controller. In order to use these GPIOs in Linux
-we need to translate them to the Linux GPIO numbers.
+we need to translate them to the corresponding Linux GPIO descriptors.
 
-In a simple case of just getting the Linux GPIO number from device
-resources one can use acpi_get_gpio_by_index() helper function. It takes
-pointer to the device and index of the GpioIo/GpioInt descriptor in the
-device resources list. For example:
+There is a standard GPIO API for that and is documented in
+Documentation/gpio.txt.
 
-	int gpio_irq, gpio_power;
-	int ret;
-
-	gpio_irq = acpi_get_gpio_by_index(dev, 1, NULL);
-	if (gpio_irq < 0)
-		/* handle error */
-
-	gpio_power = acpi_get_gpio_by_index(dev, 0, NULL);
-	if (gpio_power < 0)
-		/* handle error */
-
-	/* Now we can use the GPIO numbers */
-
-Other GpioIo parameters must be converted first by the driver to be
-suitable to the gpiolib before passing them.
-
-In case of GpioInt resource an additional call to gpio_to_irq() must be
-done before calling request_irq().
-
-Note that the above API is ACPI specific and not recommended for drivers
-that need to support non-ACPI systems. The recommended way is to use
-the descriptor based GPIO interfaces. The above example looks like this
-when converted to the GPIO desc:
+In the above example we can get the corresponding two GPIO descriptors with
+a code like this:
 
 	#include <linux/gpio/consumer.h>
 	...
@@ -339,4 +316,5 @@ when converted to the GPIO desc:
 
 	/* Now we can use the GPIO descriptors */
 
-See also Documentation/gpio.txt.
+There are also devm_* versions of these functions which release the
+descriptors once the device is released.
-- 
1.8.4.3

Re: [PATCH v3 0/6] gpio / ACPI: convert users to gpiod_* and drop acpi_gpio.h

[Linus Walleij]

On Tue, Nov 26, 2013 at 11:05 AM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> Since the patches in the series depend on each other I would propose this
> to be merged via GPIO tree.

Sure! Can you hunt the net/mmc/Tegra subsystem maintainers
for ACKs please?

(Haven't looked close at v3 yet, bear with me if you already have
some ACKs...)

Yours,
Linus Walleij

Re: [PATCH v3 0/6] gpio / ACPI: convert users to gpiod_* and drop acpi_gpio.h

[Mika Westerberg]

On Thu, Nov 28, 2013 at 03:36:40PM +0100, Linus Walleij wrote:
> On Tue, Nov 26, 2013 at 11:05 AM, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> 
> > Since the patches in the series depend on each other I would propose this
> > to be merged via GPIO tree.
> 
> Sure! Can you hunt the net/mmc/Tegra subsystem maintainers
> for ACKs please?

I think Stephen already tested the tegra parts and his Tested-by is added
to the patches.

Chris,

Do you have any comments on the series? Could you give your ACK for the mmc
part if you are OK with that?

Thanks.