[PATCH 1/5] net: rfkill: gpio: Configurable GPIO idx

[PATCH 1/5] net: rfkill: gpio: Configurable GPIO idx

[Loic Poulain]

Some devices don't match the current static gpio mapping:
-BCM4752 (GPS) uses gpio res 0 as host wake irq.
-OBDA8723 (Bluetooth) uses gpio res 0 as controller wake gpio
and res 2 as host wake irq.

To allow more flexibility, this patch introduces an index
map description.
By default, legacy config still used (reset: idx 0;
shutdown: idx 1).

Signed-off-by: Loic Poulain <loic.poulain@intel.com>
---
 net/rfkill/rfkill-gpio.c | 125 +++++++++++++++++++++++++++++++++--------------
 1 file changed, 88 insertions(+), 37 deletions(-)

diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 0f62326..7982841 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -41,6 +41,13 @@ struct rfkill_gpio_data {
 	bool			clk_enabled;
 };
 
+struct rfkill_gpio_desc {
+	enum rfkill_type	type;
+
+	int			reset_idx;
+	int			shutdown_idx;
+};
+
 static int rfkill_gpio_set_power(void *data, bool blocked)
 {
 	struct rfkill_gpio_data *rfkill = data;
@@ -63,32 +70,89 @@ static const struct rfkill_ops rfkill_gpio_ops = {
 	.set_block = rfkill_gpio_set_power,
 };
 
+static int rfkill_gpio_init(struct device *dev, struct rfkill_gpio_desc *desc)
+{
+	int ret;
+	struct gpio_desc *gpio;
+	struct rfkill_gpio_data *rfkill = dev_get_drvdata(dev);
+
+	if (!rfkill->name) {
+		dev_err(dev, "invalid platform data\n");
+		return -EINVAL;
+	}
+
+	rfkill->clk = devm_clk_get(dev, NULL);
+
+	if (!desc)
+		gpio = devm_gpiod_get_index(dev, "reset", 0);
+	else if (desc->reset_idx >= 0)
+		gpio = devm_gpiod_get_index(dev, "reset", desc->reset_idx);
+	else
+		gpio = NULL;
+
+	if (gpio && !IS_ERR(gpio)) {
+		ret = gpiod_direction_output(gpio, 0);
+		if (ret)
+			return ret;
+		rfkill->reset_gpio = gpio;
+	}
+
+	if (!desc)
+		gpio = devm_gpiod_get_index(dev, "shutdown", 1);
+	else if (desc->shutdown_idx >= 0)
+		gpio = devm_gpiod_get_index(dev, "shutdown",
+					    desc->shutdown_idx);
+	else
+		gpio = NULL;
+
+	if (gpio && !IS_ERR(gpio)) {
+		ret = gpiod_direction_output(gpio, 0);
+		if (ret)
+			return ret;
+		rfkill->shutdown_gpio = gpio;
+	}
+
+	/* Make sure at-least one of the GPIO is defined */
+	if (!rfkill->reset_gpio && !rfkill->shutdown_gpio) {
+		dev_err(dev, "invalid platform data\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int rfkill_gpio_acpi_probe(struct device *dev,
 				  struct rfkill_gpio_data *rfkill)
 {
 	const struct acpi_device_id *id;
+	struct rfkill_gpio_desc *desc;
 
 	id = acpi_match_device(dev->driver->acpi_match_table, dev);
 	if (!id)
 		return -ENODEV;
 
+	desc = (struct rfkill_gpio_desc *)id->driver_data;
+	if (!desc)
+		return -ENODEV;
+
 	rfkill->name = dev_name(dev);
-	rfkill->type = (unsigned)id->driver_data;
+	rfkill->type = desc->type;
 
-	return 0;
+	return rfkill_gpio_init(dev, desc);
 }
 
 static int rfkill_gpio_probe(struct platform_device *pdev)
 {
 	struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
 	struct rfkill_gpio_data *rfkill;
-	struct gpio_desc *gpio;
 	int ret;
 
 	rfkill = devm_kzalloc(&pdev->dev, sizeof(*rfkill), GFP_KERNEL);
 	if (!rfkill)
 		return -ENOMEM;
 
+	platform_set_drvdata(pdev, rfkill);
+
 	if (ACPI_HANDLE(&pdev->dev)) {
 		ret = rfkill_gpio_acpi_probe(&pdev->dev, rfkill);
 		if (ret)
@@ -96,34 +160,11 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 	} else if (pdata) {
 		rfkill->name = pdata->name;
 		rfkill->type = pdata->type;
-	} else {
-		return -ENODEV;
-	}
-
-	rfkill->clk = devm_clk_get(&pdev->dev, NULL);
-
-	gpio = devm_gpiod_get_index(&pdev->dev, "reset", 0);
-	if (!IS_ERR(gpio)) {
-		ret = gpiod_direction_output(gpio, 0);
+		ret = rfkill_gpio_init(&pdev->dev, NULL);
 		if (ret)
 			return ret;
-		rfkill->reset_gpio = gpio;
-	}
-
-	gpio = devm_gpiod_get_index(&pdev->dev, "shutdown", 1);
-	if (!IS_ERR(gpio)) {
-		ret = gpiod_direction_output(gpio, 0);
-		if (ret)
-			return ret;
-		rfkill->shutdown_gpio = gpio;
-	}
-
-	/* 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;
+	} else {
+		return -ENODEV;
 	}
 
 	rfkill->rfkill_dev = rfkill_alloc(rfkill->name, &pdev->dev,
@@ -136,8 +177,6 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
-	platform_set_drvdata(pdev, rfkill);
-
 	dev_info(&pdev->dev, "%s device registered.\n", rfkill->name);
 
 	return 0;
@@ -154,13 +193,25 @@ static int rfkill_gpio_remove(struct platform_device *pdev)
 }
 
 #ifdef CONFIG_ACPI
+static struct rfkill_gpio_desc acpi_default_bluetooth = {
+	.type = RFKILL_TYPE_BLUETOOTH,
+	.reset_idx = 0,
+	.shutdown_idx = 1,
+};
+
+static struct rfkill_gpio_desc acpi_default_gps = {
+	.type = RFKILL_TYPE_GPS,
+	.reset_idx = 0,
+	.shutdown_idx = 1,
+};
+
 static const struct acpi_device_id rfkill_acpi_match[] = {
-	{ "BCM2E1A", RFKILL_TYPE_BLUETOOTH },
-	{ "BCM2E39", RFKILL_TYPE_BLUETOOTH },
-	{ "BCM2E3D", RFKILL_TYPE_BLUETOOTH },
-	{ "BCM2E64", RFKILL_TYPE_BLUETOOTH },
-	{ "BCM4752", RFKILL_TYPE_GPS },
-	{ "LNV4752", RFKILL_TYPE_GPS },
+	{ "BCM2E1A", (kernel_ulong_t)&acpi_default_bluetooth },
+	{ "BCM2E39", (kernel_ulong_t)&acpi_default_bluetooth },
+	{ "BCM2E3D", (kernel_ulong_t)&acpi_default_bluetooth },
+	{ "BCM2E64", (kernel_ulong_t)&acpi_default_bluetooth },
+	{ "BCM4752", (kernel_ulong_t)&acpi_default_gps },
+	{ "LNV4752", (kernel_ulong_t)&acpi_default_gps },
 	{ },
 };
 MODULE_DEVICE_TABLE(acpi, rfkill_acpi_match);
-- 
1.8.3.2

[PATCH 2/5] net: rfkill: gpio: Implement PM hooks

[Loic Poulain]

GPIO based rfkill devices should also be put to sleep mode when the
system enters suspend. This patch introduces a wake gpio.

Signed-off-by: Loic Poulain <loic.poulain@intel.com>
---
 net/rfkill/rfkill-gpio.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 7982841..29369d6 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -34,6 +34,7 @@ struct rfkill_gpio_data {
 	enum rfkill_type	type;
 	struct gpio_desc	*reset_gpio;
 	struct gpio_desc	*shutdown_gpio;
+	struct gpio_desc	*wake_gpio;
 
 	struct rfkill		*rfkill_dev;
 	struct clk		*clk;
@@ -46,6 +47,7 @@ struct rfkill_gpio_desc {
 
 	int			reset_idx;
 	int			shutdown_idx;
+	int			wake_idx;
 };
 
 static int rfkill_gpio_set_power(void *data, bool blocked)
@@ -57,6 +59,7 @@ static int rfkill_gpio_set_power(void *data, bool blocked)
 
 	gpiod_set_value_cansleep(rfkill->shutdown_gpio, !blocked);
 	gpiod_set_value_cansleep(rfkill->reset_gpio, !blocked);
+	gpiod_set_value_cansleep(rfkill->wake_gpio, !blocked);
 
 	if (blocked && !IS_ERR(rfkill->clk) && rfkill->clk_enabled)
 		clk_disable(rfkill->clk);
@@ -112,6 +115,16 @@ static int rfkill_gpio_init(struct device *dev, struct rfkill_gpio_desc *desc)
 		rfkill->shutdown_gpio = gpio;
 	}
 
+	if (desc && (desc->wake_idx >= 0)) {
+		gpio = devm_gpiod_get_index(dev, "wake", desc->wake_idx);
+		if (!IS_ERR(gpio)) {
+			ret = gpiod_direction_output(gpio, 0);
+			if (ret)
+				return ret;
+			rfkill->wake_gpio = gpio;
+		}
+	}
+
 	/* Make sure at-least one of the GPIO is defined */
 	if (!rfkill->reset_gpio && !rfkill->shutdown_gpio) {
 		dev_err(dev, "invalid platform data\n");
@@ -182,6 +195,38 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static int rfkill_gpio_suspend(struct device *dev)
+{
+	struct rfkill_gpio_data *rfkill = dev_get_drvdata(dev);
+
+	dev_dbg(dev, "Suspend\n");
+
+	if (!rfkill->clk_enabled)
+		return 0;
+
+	gpiod_set_value_cansleep(rfkill->wake_gpio, 0);
+
+	return 0;
+}
+
+static int rfkill_gpio_resume(struct device *dev)
+{
+	struct rfkill_gpio_data *rfkill = dev_get_drvdata(dev);
+
+	dev_dbg(dev, "Resume\n");
+
+	if (!rfkill->clk_enabled)
+		return 0;
+
+	gpiod_set_value_cansleep(rfkill->wake_gpio, 1);
+
+	return 0;
+}
+#endif
+static SIMPLE_DEV_PM_OPS(rfkill_gpio_pm, rfkill_gpio_suspend,
+			 rfkill_gpio_resume);
+
 static int rfkill_gpio_remove(struct platform_device *pdev)
 {
 	struct rfkill_gpio_data *rfkill = platform_get_drvdata(pdev);
@@ -197,12 +242,14 @@ static struct rfkill_gpio_desc acpi_default_bluetooth = {
 	.type = RFKILL_TYPE_BLUETOOTH,
 	.reset_idx = 0,
 	.shutdown_idx = 1,
+	.wake_idx = -1,
 };
 
 static struct rfkill_gpio_desc acpi_default_gps = {
 	.type = RFKILL_TYPE_GPS,
 	.reset_idx = 0,
 	.shutdown_idx = 1,
+	.wake_idx = -1,
 };
 
 static const struct acpi_device_id rfkill_acpi_match[] = {
@@ -223,6 +270,7 @@ static struct platform_driver rfkill_gpio_driver = {
 	.driver = {
 		.name = "rfkill_gpio",
 		.owner = THIS_MODULE,
+		.pm = &rfkill_gpio_pm,
 		.acpi_match_table = ACPI_PTR(rfkill_acpi_match),
 	},
 };
-- 
1.8.3.2

[PATCH 3/5] net: rfkill: gpio: Implement host wake up support

[Loic Poulain]

Some GPIO based rfkill devices can wake their host up from suspend by
toggling an input (from the host perspective) GPIO.
This patch adds a generic support for that feature by registering a
threaded interrupt routine and thus setting the corresponding GPIO as a
wake up source.

Signed-off-by: Loic Poulain <loic.poulain@intel.com>
---
 net/rfkill/rfkill-gpio.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 29369d6..2f6aad1 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -26,6 +26,7 @@
 #include <linux/slab.h>
 #include <linux/acpi.h>
 #include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
 
 #include <linux/rfkill-gpio.h>
 
@@ -36,6 +37,8 @@ struct rfkill_gpio_data {
 	struct gpio_desc	*shutdown_gpio;
 	struct gpio_desc	*wake_gpio;
 
+	int			host_wake_irq;
+
 	struct rfkill		*rfkill_dev;
 	struct clk		*clk;
 
@@ -48,6 +51,7 @@ struct rfkill_gpio_desc {
 	int			reset_idx;
 	int			shutdown_idx;
 	int			wake_idx;
+	int			host_wake_idx;
 };
 
 static int rfkill_gpio_set_power(void *data, bool blocked)
@@ -73,6 +77,15 @@ static const struct rfkill_ops rfkill_gpio_ops = {
 	.set_block = rfkill_gpio_set_power,
 };
 
+static irqreturn_t rfkill_gpio_host_wake(int irq, void *dev)
+{
+	struct rfkill_gpio_data *rfkill = dev;
+
+	pr_info("Host wake IRQ for %s\n", rfkill->name);
+
+	return IRQ_HANDLED;
+}
+
 static int rfkill_gpio_init(struct device *dev, struct rfkill_gpio_desc *desc)
 {
 	int ret;
@@ -125,6 +138,34 @@ static int rfkill_gpio_init(struct device *dev, struct rfkill_gpio_desc *desc)
 		}
 	}
 
+	rfkill->host_wake_irq = -1;
+
+	if (desc && (desc->host_wake_idx >= 0)) {
+		gpio = devm_gpiod_get_index(dev, "host_wake",
+					    desc->host_wake_idx);
+		if (!IS_ERR(gpio)) {
+			int irqf = IRQF_TRIGGER_RISING | IRQF_NO_SUSPEND |
+				   IRQF_ONESHOT;
+
+			ret = gpiod_direction_input(gpio);
+			if (ret)
+				return ret;
+
+			rfkill->host_wake_irq = gpiod_to_irq(gpio);
+			ret = devm_request_threaded_irq(dev,
+							rfkill->host_wake_irq,
+							NULL,
+							rfkill_gpio_host_wake,
+							irqf, "host_wake",
+							rfkill);
+			if (ret)
+				return ret;
+
+			/* Wakeup IRQ, only used in suspend */
+			disable_irq(rfkill->host_wake_irq);
+		}
+	}
+
 	/* Make sure at-least one of the GPIO is defined */
 	if (!rfkill->reset_gpio && !rfkill->shutdown_gpio) {
 		dev_err(dev, "invalid platform data\n");
@@ -205,6 +246,9 @@ static int rfkill_gpio_suspend(struct device *dev)
 	if (!rfkill->clk_enabled)
 		return 0;
 
+	if (rfkill->host_wake_irq >= 0)
+		enable_irq(rfkill->host_wake_irq);
+
 	gpiod_set_value_cansleep(rfkill->wake_gpio, 0);
 
 	return 0;
@@ -219,6 +263,9 @@ static int rfkill_gpio_resume(struct device *dev)
 	if (!rfkill->clk_enabled)
 		return 0;
 
+	if (rfkill->host_wake_irq >= 0)
+		disable_irq(rfkill->host_wake_irq);
+
 	gpiod_set_value_cansleep(rfkill->wake_gpio, 1);
 
 	return 0;
@@ -243,6 +290,7 @@ static struct rfkill_gpio_desc acpi_default_bluetooth = {
 	.reset_idx = 0,
 	.shutdown_idx = 1,
 	.wake_idx = -1,
+	.host_wake_idx = -1,
 };
 
 static struct rfkill_gpio_desc acpi_default_gps = {
@@ -250,6 +298,7 @@ static struct rfkill_gpio_desc acpi_default_gps = {
 	.reset_idx = 0,
 	.shutdown_idx = 1,
 	.wake_idx = -1,
+	.host_wake_idx = -1,
 };
 
 static const struct acpi_device_id rfkill_acpi_match[] = {
-- 
1.8.3.2

[PATCH 4/5] net: rfkill: gpio: Add OBDA8723 ACPI ID

[Loic Poulain]

Add OBDA8723 (rtl8723) ACPI id.
rtl8723 bluetooth chip has the following interface:
HOST <---UART---> CONTROLLER
HOST --- WAKE---> CONTROLLER (gpio res 0)
HOST ---ENABLE--> CONTROLLER (gpio res 1)
HOST <---WAKE---- CONTROLLER (gpio res 2)

Signed-off-by: Loic Poulain <loic.poulain@intel.com>
---
 net/rfkill/rfkill-gpio.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 2f6aad1..28d1cbf 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -301,11 +301,20 @@ static struct rfkill_gpio_desc acpi_default_gps = {
 	.host_wake_idx = -1,
 };
 
+static struct rfkill_gpio_desc acpi_bluetooth_wake = {
+	.type = RFKILL_TYPE_BLUETOOTH,
+	.reset_idx = -1,
+	.shutdown_idx = 1,
+	.wake_idx = 0,
+	.host_wake_idx = 2,
+};
+
 static const struct acpi_device_id rfkill_acpi_match[] = {
 	{ "BCM2E1A", (kernel_ulong_t)&acpi_default_bluetooth },
 	{ "BCM2E39", (kernel_ulong_t)&acpi_default_bluetooth },
 	{ "BCM2E3D", (kernel_ulong_t)&acpi_default_bluetooth },
 	{ "BCM2E64", (kernel_ulong_t)&acpi_default_bluetooth },
+	{ "OBDA8723", (kernel_ulong_t)&acpi_bluetooth_wake },
 	{ "BCM4752", (kernel_ulong_t)&acpi_default_gps },
 	{ "LNV4752", (kernel_ulong_t)&acpi_default_gps },
 	{ },
-- 
1.8.3.2

[PATCH 5/5] net: rfkill: gpio: Add BCM47521 ACPI ID

[Loic Poulain]

Add BCM47521 (GPS) to the ACPI table.
BCM47521 supports host wake.
HOST <---UART---> CONTROLLER
HOST <-- WAKE---- CONTROLLER (gpio res 0)
HOST ---ENABLE--> CONTROLLER (gpio res 1)

Signed-off-by: Loic Poulain <loic.poulain@intel.com>
---
 net/rfkill/rfkill-gpio.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 28d1cbf..da239ef 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -309,6 +309,14 @@ static struct rfkill_gpio_desc acpi_bluetooth_wake = {
 	.host_wake_idx = 2,
 };
 
+static struct rfkill_gpio_desc acpi_gps_wake = {
+	.type = RFKILL_TYPE_GPS,
+	.reset_idx = -1,
+	.shutdown_idx = 1,
+	.wake_idx = -1,
+	.host_wake_idx = 0,
+};
+
 static const struct acpi_device_id rfkill_acpi_match[] = {
 	{ "BCM2E1A", (kernel_ulong_t)&acpi_default_bluetooth },
 	{ "BCM2E39", (kernel_ulong_t)&acpi_default_bluetooth },
@@ -317,6 +325,7 @@ static const struct acpi_device_id rfkill_acpi_match[] = {
 	{ "OBDA8723", (kernel_ulong_t)&acpi_bluetooth_wake },
 	{ "BCM4752", (kernel_ulong_t)&acpi_default_gps },
 	{ "LNV4752", (kernel_ulong_t)&acpi_default_gps },
+	{ "BCM47521", (kernel_ulong_t)&acpi_gps_wake },
 	{ },
 };
 MODULE_DEVICE_TABLE(acpi, rfkill_acpi_match);
-- 
1.8.3.2

Re: [PATCH 1/5] net: rfkill: gpio: Configurable GPIO idx

[Johannes Berg]

On Wed, 2014-10-08 at 10:34 +0200, Loic Poulain wrote:

> +	{ "BCM2E1A", (kernel_ulong_t)&acpi_default_bluetooth },

Shouldn't that be uintptr_t?

johannes

Re: [PATCH 1/5] net: rfkill: gpio: Configurable GPIO idx

[Loic Poulain]

Hi Johannes,

acpi_device_id driver_data field is defined as a kernel_ulong_t in 
mod_devicetable.h.

Regards,
Loic

On 08/10/2014 10:53, Johannes Berg wrote:
> On Wed, 2014-10-08 at 10:34 +0200, Loic Poulain wrote:
>
>> +	{ "BCM2E1A", (kernel_ulong_t)&acpi_default_bluetooth },
> Shouldn't that be uintptr_t?
>
> johannes
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Intel Open Source Technology Center
http://oss.intel.com/

Re: [PATCH 1/5] net: rfkill: gpio: Configurable GPIO idx

[Heikki Krogerus]

On Wed, Oct 08, 2014 at 10:34:36AM +0200, Loic Poulain wrote:
> Some devices don't match the current static gpio mapping:
> -BCM4752 (GPS) uses gpio res 0 as host wake irq.
> -OBDA8723 (Bluetooth) uses gpio res 0 as controller wake gpio
> and res 2 as host wake irq.
> 
> To allow more flexibility, this patch introduces an index
> map description.
> By default, legacy config still used (reset: idx 0;
> shutdown: idx 1).
> 
> Signed-off-by: Loic Poulain <loic.poulain@intel.com>
> ---
>  net/rfkill/rfkill-gpio.c | 125 +++++++++++++++++++++++++++++++++--------------
>  1 file changed, 88 insertions(+), 37 deletions(-)

NAK

I'm afraid the order of the GPIOs is platform specific. This would
break the systems already out in the market.

You need to fix your firmware to generate ACPI DSDT according to ACPI
spec. v5.1 [1], which introduces the Device Specific Data (_DSD) and
dt like device properties. We need to be able to request the GPIO
based on the label, not the index, with all new platforms. Here is an
example of the type of_ASL you need to have for the device..

        ...
        Method (_CRS, 0, NotSerialized)  // _CRS: Current Resource Settings
        {
            Name (BBUF, ResourceTemplate ()
            {
                UartSerialBus (0x0001C200, DataBitsEight, StopBitsOne,
                    0xFC, LittleEndian, ParityTypeNone, FlowControlNone,
                    0x0020, 0x0020, "\\_SB.PCI0.URT1",
                    0x00, ResourceConsumer, ,
                    )
                GpioInt (Level, ActiveLow, Exclusive, PullNone, 0x0000,
                    "\\_SB.GPO3", 0x00, ResourceConsumer, ,
                    )
                    {   // Pin list
                        0x003C
                    }
                GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
                    "\\_SB.GPO3", 0x00, ResourceConsumer, ,
                    )
                    {   // Pin list
                        0x003E
                    }
                GpioIo (Exclusive, PullDefault, 0x0000, 0x0000, IoRestrictionOutputOnly,
                    "\\_SB.GPO3", 0x00, ResourceConsumer, ,
                    )
                    {   // Pin list
                        0x0040
                    }
            })
            Return (BBUF) /* \_SB_.PCI0.URT1.BTH1._CRS.BBUF */
        }

        Name (_DSD, Package ()
        {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () {
                Package () {"reset-gpio", Package () {^BTH1, 1, 0, 0}},
                Package () {"shutdown-gpio", Package () {^BTH1, 2, 0, 0}},
            }
        })
        ...

That will make sure you get the correct GPIO the moment we get support
for the ACPI properties [2], regardless of the index. So there is no
need for any changes to the driver.


[1] http://www.uefi.org/sites/default/files/resources/ACPI_5_1release.pdf
[2] https://lkml.org/lkml/2014/9/16/229


-- 
heikki

Re: [PATCH 3/5] net: rfkill: gpio: Implement host wake up support

[Heikki Krogerus]

On Wed, Oct 08, 2014 at 10:34:38AM +0200, Loic Poulain wrote:
> Some GPIO based rfkill devices can wake their host up from suspend by
> toggling an input (from the host perspective) GPIO.
> This patch adds a generic support for that feature by registering a
> threaded interrupt routine and thus setting the corresponding GPIO as a
> wake up source.
> 
> Signed-off-by: Loic Poulain <loic.poulain@intel.com>
> ---
>  net/rfkill/rfkill-gpio.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)

To continue my previous answer, for this you could have the following
_DSD..

        Name (_DSD, Package ()
        {
            ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
            Package () {
                Package () {"host_wake-gpio", Package () {^BTH1, 0, 0, 0}},
                Package () {"reset-gpio", Package () {^BTH1, 1, 0, 0}},
                Package () {"shutdown-gpio", Package () {^BTH1, 2, 0, 0}},
            }
        })

And in the driver you can then request it without caring about the
index..

        ...
        gpio = devm_gpiod_get_index(dev, "host_wake", 3);
        if (!IS_ERR(gpio)) {
		ret = gpiod_direction_input(gpio);
		if (ret)
			return ret;
		rfkill->irq = gpiod_to_irq(gpio);
        ...


Actually, we could just use devm_gpiod_get instead of
devm_gpiod_get_index in this case.


Cheers,

-- 
heikki

Re: [PATCH 3/5] net: rfkill: gpio: Implement host wake up support

[Loic Poulain]

Thanks, it sounds good to use a label instead of index.
However:
- DSD is only compatible with BIOS ACPI 5.1
- gpiod_get_index does not take care of con_id (label) parameter in case 
of ACPI (acpi_find_gpio).
- gpiod_get calls gpiod_get_index with index 0.

Any patch ongoing to support con_id parameter with ACPI?

Regards,
Loic

On 08/10/2014 13:38, Heikki Krogerus wrote:
> On Wed, Oct 08, 2014 at 10:34:38AM +0200, Loic Poulain wrote:
>> Some GPIO based rfkill devices can wake their host up from suspend by
>> toggling an input (from the host perspective) GPIO.
>> This patch adds a generic support for that feature by registering a
>> threaded interrupt routine and thus setting the corresponding GPIO as a
>> wake up source.
>>
>> Signed-off-by: Loic Poulain <loic.poulain@intel.com>
>> ---
>>   net/rfkill/rfkill-gpio.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 49 insertions(+)
> To continue my previous answer, for this you could have the following
> _DSD..
>
>          Name (_DSD, Package ()
>          {
>              ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>              Package () {
>                  Package () {"host_wake-gpio", Package () {^BTH1, 0, 0, 0}},
>                  Package () {"reset-gpio", Package () {^BTH1, 1, 0, 0}},
>                  Package () {"shutdown-gpio", Package () {^BTH1, 2, 0, 0}},
>              }
>          })
>
> And in the driver you can then request it without caring about the
> index..
>
>          ...
>          gpio = devm_gpiod_get_index(dev, "host_wake", 3);
>          if (!IS_ERR(gpio)) {
> 		ret = gpiod_direction_input(gpio);
> 		if (ret)
> 			return ret;
> 		rfkill->irq = gpiod_to_irq(gpio);
>          ...
>
>
> Actually, we could just use devm_gpiod_get instead of
> devm_gpiod_get_index in this case.
>
>
> Cheers,
>

-- 
Intel Open Source Technology Center
http://oss.intel.com/

Re: [PATCH 3/5] net: rfkill: gpio: Implement host wake up support

[Heikki Krogerus]

On Wed, Oct 08, 2014 at 04:09:50PM +0200, Loic Poulain wrote:
> Thanks, it sounds good to use a label instead of index.
> However:
> - DSD is only compatible with BIOS ACPI 5.1
> - gpiod_get_index does not take care of con_id (label) parameter in case of
> ACPI (acpi_find_gpio).
> - gpiod_get calls gpiod_get_index with index 0.
> 
> Any patch ongoing to support con_id parameter with ACPI?

Yes. This is the latest version being review as we speak:
https://lkml.org/lkml/2014/10/6/530

It is really important you get your firmware fixed. We simply can not
rely on the order of the resources like GPIOs in the resource
templates. They may be different on every second
board/BIOS/configuration/whatever. It's even possible for a ACPI
device object to have two (or more) different resource templates
describing the same GPIOs in different order which are then selected
based on some magic condition, for example user selection in BIOS setup.

The existing hardware works with the drivers we have now, and it's not
possible to change the BIOS/system firmware on those so we just have
to live with them and rely on being able to pick the correct resources
based on index. But everything new _must_ use _DSD properties, and
ACPI 5.1 in general.

So basically your BIOS must produce the ACPI tables according to ACPI
5.1. If you are worried about support for ACPI 5.1 in kernel, don't
worry about it, it's almost ready. And in the meantime, _DSD is just
something kernel will ignore.


Cheers,

-- 
heikki

Re: [PATCH 3/5] net: rfkill: gpio: Implement host wake up support

[Loic Poulain]

Thanks Heikki, I'll rework these patches accordingly.

Regards,
Loic

On 09/10/2014 09:38, Heikki Krogerus wrote:
> On Wed, Oct 08, 2014 at 04:09:50PM +0200, Loic Poulain wrote:
>> Thanks, it sounds good to use a label instead of index.
>> However:
>> - DSD is only compatible with BIOS ACPI 5.1
>> - gpiod_get_index does not take care of con_id (label) parameter in case of
>> ACPI (acpi_find_gpio).
>> - gpiod_get calls gpiod_get_index with index 0.
>>
>> Any patch ongoing to support con_id parameter with ACPI?
> Yes. This is the latest version being review as we speak:
> https://lkml.org/lkml/2014/10/6/530
>
> It is really important you get your firmware fixed. We simply can not
> rely on the order of the resources like GPIOs in the resource
> templates. They may be different on every second
> board/BIOS/configuration/whatever. It's even possible for a ACPI
> device object to have two (or more) different resource templates
> describing the same GPIOs in different order which are then selected
> based on some magic condition, for example user selection in BIOS setup.
>
> The existing hardware works with the drivers we have now, and it's not
> possible to change the BIOS/system firmware on those so we just have
> to live with them and rely on being able to pick the correct resources
> based on index. But everything new _must_ use _DSD properties, and
> ACPI 5.1 in general.
>
> So basically your BIOS must produce the ACPI tables according to ACPI
> 5.1. If you are worried about support for ACPI 5.1 in kernel, don't
> worry about it, it's almost ready. And in the meantime, _DSD is just
> something kernel will ignore.
>
>
> Cheers,
>

-- 
Intel Open Source Technology Center
http://oss.intel.com/