| Message ID | 1422282573-18215-2-git-send-email-mika.westerberg@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
On Mon, Jan 26, 2015 at 02:29:33PM +0000, Mika Westerberg wrote: > The HID over I2C specification allows to have the interrupt for a HID > device to be GPIO instead of directly connected to the IO-APIC. > > Add support for this so that when the driver does not find proper interrupt > number from the I2C client structure we check if the device has property > named "gpios". This is then assumed to be the GPIO that serves as an > interrupt for the device. > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > --- > .../devicetree/bindings/hid/hid-over-i2c.txt | 5 +- > drivers/hid/i2c-hid/i2c-hid.c | 70 ++++++++++++++++------ > 2 files changed, 56 insertions(+), 19 deletions(-) > > diff --git a/Documentation/devicetree/bindings/hid/hid-over-i2c.txt b/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > index 488edcb264c4..8f4a99dad3b9 100644 > --- a/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > +++ b/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > @@ -15,7 +15,10 @@ Required properties: > - reg: i2c slave address > - hid-descr-addr: HID descriptor address > - interrupt-parent: the phandle for the interrupt controller > -- interrupts: interrupt line > +- interrupts: interrupt line if the device uses IO-APIC interrupts > + > +Optional properties: > +- gpios: GPIO used as an interrupt if the device uses GPIO interrupts Elsewhere we've said that for a GPIO acting as an interrupt line, GPIO controller should be marked as an interrupt-controller, and the GPIO described as an interrupt line. That also gets you the appropriate configuration for the GPIO as an interrupt. Does this GPIO serve any other purpose than an ersatz interrupt line? If not, it should probably be described as an interrupt. From the PoV of this device, it's just an interrupt controller hooked up to the interrupt pin. Thanks, Mark. > > Example: > > diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c > index 8f1dfc5c5d9c..d3c49ff4888e 100644 > --- a/drivers/hid/i2c-hid/i2c-hid.c > +++ b/drivers/hid/i2c-hid/i2c-hid.c > @@ -37,6 +37,7 @@ > #include <linux/mutex.h> > #include <linux/acpi.h> > #include <linux/of.h> > +#include <linux/gpio/consumer.h> > > #include <linux/i2c/i2c-hid.h> > > @@ -144,6 +145,8 @@ struct i2c_hid { > unsigned long flags; /* device flags */ > > wait_queue_head_t wait; /* For waiting the interrupt */ > + struct gpio_desc *desc; > + int irq; > > struct i2c_hid_platform_data pdata; > }; > @@ -782,16 +785,16 @@ static int i2c_hid_init_irq(struct i2c_client *client) > struct i2c_hid *ihid = i2c_get_clientdata(client); > int ret; > > - dev_dbg(&client->dev, "Requesting IRQ: %d\n", client->irq); > + dev_dbg(&client->dev, "Requesting IRQ: %d\n", ihid->irq); > > - ret = request_threaded_irq(client->irq, NULL, i2c_hid_irq, > + ret = request_threaded_irq(ihid->irq, NULL, i2c_hid_irq, > IRQF_TRIGGER_LOW | IRQF_ONESHOT, > client->name, ihid); > if (ret < 0) { > dev_warn(&client->dev, > "Could not register for %s interrupt, irq = %d," > " ret = %d\n", > - client->name, client->irq, ret); > + client->name, ihid->irq, ret); > > return ret; > } > @@ -838,6 +841,14 @@ static int i2c_hid_fetch_hid_descriptor(struct i2c_hid *ihid) > } > > #ifdef CONFIG_ACPI > + > +/* Default GPIO mapping */ > +static const struct acpi_gpio_params i2c_hid_irq_gpio = { 0, 0, true }; > +static const struct acpi_gpio_mapping i2c_hid_acpi_gpios[] = { > + { "gpios", &i2c_hid_irq_gpio, 1 }, > + { }, > +}; > + > static int i2c_hid_acpi_pdata(struct i2c_client *client, > struct i2c_hid_platform_data *pdata) > { > @@ -863,7 +874,7 @@ static int i2c_hid_acpi_pdata(struct i2c_client *client, > pdata->hid_descriptor_address = obj->integer.value; > ACPI_FREE(obj); > > - return 0; > + return acpi_dev_add_driver_gpios(adev, i2c_hid_acpi_gpios); > } > > static const struct acpi_device_id i2c_hid_acpi_match[] = { > @@ -927,12 +938,6 @@ static int i2c_hid_probe(struct i2c_client *client, > > dbg_hid("HID probe called for i2c 0x%02x\n", client->addr); > > - if (!client->irq) { > - dev_err(&client->dev, > - "HID over i2c has not been provided an Int IRQ\n"); > - return -EINVAL; > - } > - > ihid = kzalloc(sizeof(struct i2c_hid), GFP_KERNEL); > if (!ihid) > return -ENOMEM; > @@ -952,6 +957,25 @@ static int i2c_hid_probe(struct i2c_client *client, > ihid->pdata = *platform_data; > } > > + if (client->irq > 0) { > + ihid->irq = client->irq; > + } else { > + ihid->desc = gpiod_get(&client->dev, NULL); > + if (IS_ERR(ihid->desc)) { > + dev_err(&client->dev, "Failed to get GPIO interrupt\n"); > + return PTR_ERR(ihid->desc); > + } > + > + gpiod_direction_input(ihid->desc); > + > + ihid->irq = gpiod_to_irq(ihid->desc); > + if (ihid->irq < 0) { > + gpiod_put(ihid->desc); > + dev_err(&client->dev, "Failed to convert GPIO to IRQ\n"); > + return ihid->irq; > + } > + } > + > i2c_set_clientdata(client, ihid); > > ihid->client = client; > @@ -1014,13 +1038,16 @@ err_mem_free: > hid_destroy_device(hid); > > err_irq: > - free_irq(client->irq, ihid); > + free_irq(ihid->irq, ihid); > > err_pm: > pm_runtime_put_noidle(&client->dev); > pm_runtime_disable(&client->dev); > > err: > + if (ihid->desc) > + gpiod_put(ihid->desc); > + > i2c_hid_free_buffers(ihid); > kfree(ihid); > return ret; > @@ -1039,13 +1066,18 @@ static int i2c_hid_remove(struct i2c_client *client) > hid = ihid->hid; > hid_destroy_device(hid); > > - free_irq(client->irq, ihid); > + free_irq(ihid->irq, ihid); > > if (ihid->bufsize) > i2c_hid_free_buffers(ihid); > > + if (ihid->desc) > + gpiod_put(ihid->desc); > + > kfree(ihid); > > + acpi_dev_remove_driver_gpios(ACPI_COMPANION(&client->dev)); > + > return 0; > } > > @@ -1057,9 +1089,9 @@ static int i2c_hid_suspend(struct device *dev) > struct hid_device *hid = ihid->hid; > int ret = 0; > > - disable_irq(client->irq); > + disable_irq(ihid->irq); > if (device_may_wakeup(&client->dev)) > - enable_irq_wake(client->irq); > + enable_irq_wake(ihid->irq); > > if (hid->driver && hid->driver->suspend) > ret = hid->driver->suspend(hid, PMSG_SUSPEND); > @@ -1077,13 +1109,13 @@ static int i2c_hid_resume(struct device *dev) > struct i2c_hid *ihid = i2c_get_clientdata(client); > struct hid_device *hid = ihid->hid; > > - enable_irq(client->irq); > + enable_irq(ihid->irq); > ret = i2c_hid_hwreset(client); > if (ret) > return ret; > > if (device_may_wakeup(&client->dev)) > - disable_irq_wake(client->irq); > + disable_irq_wake(ihid->irq); > > if (hid->driver && hid->driver->reset_resume) { > ret = hid->driver->reset_resume(hid); > @@ -1098,17 +1130,19 @@ static int i2c_hid_resume(struct device *dev) > static int i2c_hid_runtime_suspend(struct device *dev) > { > struct i2c_client *client = to_i2c_client(dev); > + struct i2c_hid *ihid = i2c_get_clientdata(client); > > i2c_hid_set_power(client, I2C_HID_PWR_SLEEP); > - disable_irq(client->irq); > + disable_irq(ihid->irq); > return 0; > } > > static int i2c_hid_runtime_resume(struct device *dev) > { > struct i2c_client *client = to_i2c_client(dev); > + struct i2c_hid *ihid = i2c_get_clientdata(client); > > - enable_irq(client->irq); > + enable_irq(ihid->irq); > i2c_hid_set_power(client, I2C_HID_PWR_ON); > return 0; > } > -- > 2.1.4 > > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 26, 2015 at 02:37:24PM +0000, Mark Rutland wrote: > On Mon, Jan 26, 2015 at 02:29:33PM +0000, Mika Westerberg wrote: > > The HID over I2C specification allows to have the interrupt for a HID > > device to be GPIO instead of directly connected to the IO-APIC. > > > > Add support for this so that when the driver does not find proper interrupt > > number from the I2C client structure we check if the device has property > > named "gpios". This is then assumed to be the GPIO that serves as an > > interrupt for the device. > > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > --- > > .../devicetree/bindings/hid/hid-over-i2c.txt | 5 +- > > drivers/hid/i2c-hid/i2c-hid.c | 70 ++++++++++++++++------ > > 2 files changed, 56 insertions(+), 19 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/hid/hid-over-i2c.txt b/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > > index 488edcb264c4..8f4a99dad3b9 100644 > > --- a/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > > +++ b/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > > @@ -15,7 +15,10 @@ Required properties: > > - reg: i2c slave address > > - hid-descr-addr: HID descriptor address > > - interrupt-parent: the phandle for the interrupt controller > > -- interrupts: interrupt line > > +- interrupts: interrupt line if the device uses IO-APIC interrupts > > + > > +Optional properties: > > +- gpios: GPIO used as an interrupt if the device uses GPIO interrupts > > Elsewhere we've said that for a GPIO acting as an interrupt line, GPIO > controller should be marked as an interrupt-controller, and the GPIO > described as an interrupt line. That also gets you the appropriate > configuration for the GPIO as an interrupt. > > Does this GPIO serve any other purpose than an ersatz interrupt line? It is just an interrupt. > If not, it should probably be described as an interrupt. From the PoV of > this device, it's just an interrupt controller hooked up to the > interrupt pin. What I'm trying to do is to get a GPIO that is described in ACPI (as GpioInt() in _CRS) to be supported in this driver using gpiolib like this: desc = gpiod_get(&client->dev, NULL); This calls to find "gpios" property which ends up finding the GpioInt() in _CRS. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 26, 2015 at 02:47:29PM +0000, Mika Westerberg wrote: > On Mon, Jan 26, 2015 at 02:37:24PM +0000, Mark Rutland wrote: > > On Mon, Jan 26, 2015 at 02:29:33PM +0000, Mika Westerberg wrote: > > > The HID over I2C specification allows to have the interrupt for a HID > > > device to be GPIO instead of directly connected to the IO-APIC. > > > > > > Add support for this so that when the driver does not find proper interrupt > > > number from the I2C client structure we check if the device has property > > > named "gpios". This is then assumed to be the GPIO that serves as an > > > interrupt for the device. > > > > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > > --- > > > .../devicetree/bindings/hid/hid-over-i2c.txt | 5 +- > > > drivers/hid/i2c-hid/i2c-hid.c | 70 ++++++++++++++++------ > > > 2 files changed, 56 insertions(+), 19 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/hid/hid-over-i2c.txt b/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > > > index 488edcb264c4..8f4a99dad3b9 100644 > > > --- a/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > > > +++ b/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > > > @@ -15,7 +15,10 @@ Required properties: > > > - reg: i2c slave address > > > - hid-descr-addr: HID descriptor address > > > - interrupt-parent: the phandle for the interrupt controller > > > -- interrupts: interrupt line > > > +- interrupts: interrupt line if the device uses IO-APIC interrupts > > > + > > > +Optional properties: > > > +- gpios: GPIO used as an interrupt if the device uses GPIO interrupts > > > > Elsewhere we've said that for a GPIO acting as an interrupt line, GPIO > > controller should be marked as an interrupt-controller, and the GPIO > > described as an interrupt line. That also gets you the appropriate > > configuration for the GPIO as an interrupt. > > > > Does this GPIO serve any other purpose than an ersatz interrupt line? > > It is just an interrupt. > > > If not, it should probably be described as an interrupt. From the PoV of > > this device, it's just an interrupt controller hooked up to the > > interrupt pin. > > What I'm trying to do is to get a GPIO that is described in ACPI (as > GpioInt() in _CRS) to be supported in this driver using gpiolib like > this: > > desc = gpiod_get(&client->dev, NULL); > > This calls to find "gpios" property which ends up finding the GpioInt() > in _CRS. I understand what you are trying to do, but I disagree on the principle. If it's logically an interrupt, it should be described as an interrupt. If ACPI lacks the ability to describe things in that fashion, it's yet another reason that we shouldn't be pretending that DT and ACPI are the same thing. Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 26, 2015 at 02:50:01PM +0000, Mark Rutland wrote: > On Mon, Jan 26, 2015 at 02:47:29PM +0000, Mika Westerberg wrote: > > On Mon, Jan 26, 2015 at 02:37:24PM +0000, Mark Rutland wrote: > > > On Mon, Jan 26, 2015 at 02:29:33PM +0000, Mika Westerberg wrote: > > > > The HID over I2C specification allows to have the interrupt for a HID > > > > device to be GPIO instead of directly connected to the IO-APIC. > > > > > > > > Add support for this so that when the driver does not find proper interrupt > > > > number from the I2C client structure we check if the device has property > > > > named "gpios". This is then assumed to be the GPIO that serves as an > > > > interrupt for the device. > > > > > > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > > > --- > > > > .../devicetree/bindings/hid/hid-over-i2c.txt | 5 +- > > > > drivers/hid/i2c-hid/i2c-hid.c | 70 ++++++++++++++++------ > > > > 2 files changed, 56 insertions(+), 19 deletions(-) > > > > > > > > diff --git a/Documentation/devicetree/bindings/hid/hid-over-i2c.txt b/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > > > > index 488edcb264c4..8f4a99dad3b9 100644 > > > > --- a/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > > > > +++ b/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > > > > @@ -15,7 +15,10 @@ Required properties: > > > > - reg: i2c slave address > > > > - hid-descr-addr: HID descriptor address > > > > - interrupt-parent: the phandle for the interrupt controller > > > > -- interrupts: interrupt line > > > > +- interrupts: interrupt line if the device uses IO-APIC interrupts > > > > + > > > > +Optional properties: > > > > +- gpios: GPIO used as an interrupt if the device uses GPIO interrupts > > > > > > Elsewhere we've said that for a GPIO acting as an interrupt line, GPIO > > > controller should be marked as an interrupt-controller, and the GPIO > > > described as an interrupt line. That also gets you the appropriate > > > configuration for the GPIO as an interrupt. > > > > > > Does this GPIO serve any other purpose than an ersatz interrupt line? > > > > It is just an interrupt. > > > > > If not, it should probably be described as an interrupt. From the PoV of > > > this device, it's just an interrupt controller hooked up to the > > > interrupt pin. > > > > What I'm trying to do is to get a GPIO that is described in ACPI (as > > GpioInt() in _CRS) to be supported in this driver using gpiolib like > > this: > > > > desc = gpiod_get(&client->dev, NULL); > > > > This calls to find "gpios" property which ends up finding the GpioInt() > > in _CRS. > > I understand what you are trying to do, but I disagree on the principle. > If it's logically an interrupt, it should be described as an interrupt. It is a GPIO line that is used as interrupt. It is not IO-APIC interrupt or anything like that. It will be handled through a GPIO driver. > If ACPI lacks the ability to describe things in that fashion, it's yet > another reason that we shouldn't be pretending that DT and ACPI are the > same thing. I'm not saying they are the same thing (they're not). I'm trying to get a GPIO from ACPI translated to an interrupt so that the driver can use it. Preferably so that the DT description does not prevent people from using the same on non-ACPI platforms. ACPI can desribe Interrupt(), GpioInt() and GpioIo() just fine. It is the Microsoft HID over I2C specification that says it should be GpioInt() even though we have seen Interrupt() used there as well. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 26, 2015 at 03:16:37PM +0000, Mika Westerberg wrote: > On Mon, Jan 26, 2015 at 02:50:01PM +0000, Mark Rutland wrote: > > On Mon, Jan 26, 2015 at 02:47:29PM +0000, Mika Westerberg wrote: > > > On Mon, Jan 26, 2015 at 02:37:24PM +0000, Mark Rutland wrote: > > > > On Mon, Jan 26, 2015 at 02:29:33PM +0000, Mika Westerberg wrote: > > > > > The HID over I2C specification allows to have the interrupt for a HID > > > > > device to be GPIO instead of directly connected to the IO-APIC. > > > > > > > > > > Add support for this so that when the driver does not find proper interrupt > > > > > number from the I2C client structure we check if the device has property > > > > > named "gpios". This is then assumed to be the GPIO that serves as an > > > > > interrupt for the device. > > > > > > > > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > > > > --- > > > > > .../devicetree/bindings/hid/hid-over-i2c.txt | 5 +- > > > > > drivers/hid/i2c-hid/i2c-hid.c | 70 ++++++++++++++++------ > > > > > 2 files changed, 56 insertions(+), 19 deletions(-) > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/hid/hid-over-i2c.txt b/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > > > > > index 488edcb264c4..8f4a99dad3b9 100644 > > > > > --- a/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > > > > > +++ b/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > > > > > @@ -15,7 +15,10 @@ Required properties: > > > > > - reg: i2c slave address > > > > > - hid-descr-addr: HID descriptor address > > > > > - interrupt-parent: the phandle for the interrupt controller > > > > > -- interrupts: interrupt line > > > > > +- interrupts: interrupt line if the device uses IO-APIC interrupts > > > > > + > > > > > +Optional properties: > > > > > +- gpios: GPIO used as an interrupt if the device uses GPIO interrupts > > > > > > > > Elsewhere we've said that for a GPIO acting as an interrupt line, GPIO > > > > controller should be marked as an interrupt-controller, and the GPIO > > > > described as an interrupt line. That also gets you the appropriate > > > > configuration for the GPIO as an interrupt. > > > > > > > > Does this GPIO serve any other purpose than an ersatz interrupt line? > > > > > > It is just an interrupt. > > > > > > > If not, it should probably be described as an interrupt. From the PoV of > > > > this device, it's just an interrupt controller hooked up to the > > > > interrupt pin. > > > > > > What I'm trying to do is to get a GPIO that is described in ACPI (as > > > GpioInt() in _CRS) to be supported in this driver using gpiolib like > > > this: > > > > > > desc = gpiod_get(&client->dev, NULL); > > > > > > This calls to find "gpios" property which ends up finding the GpioInt() > > > in _CRS. > > > > I understand what you are trying to do, but I disagree on the principle. > > If it's logically an interrupt, it should be described as an interrupt. > > It is a GPIO line that is used as interrupt. It is not IO-APIC interrupt > or anything like that. It will be handled through a GPIO driver. I understand that the interrupt line is wired up to a GPIO controller, where the GPIO controller is able to raise an interrupt as required. However, from the PoV of the I2C device, this doesn't matter. From it's PoV it raises an interrupt, and that's all. It has no idea what happens to be wired up to its IRQ pin, and nor should it. > > If ACPI lacks the ability to describe things in that fashion, it's yet > > another reason that we shouldn't be pretending that DT and ACPI are the > > same thing. > > I'm not saying they are the same thing (they're not). I'm trying to get > a GPIO from ACPI translated to an interrupt so that the driver can use > it. Preferably so that the DT description does not prevent people from > using the same on non-ACPI platforms. If you're following the Microsoft HID over I2C ACPI spec, why is this DT binding document relevant. Assuming you're following the spec, you won't be using _DSD with "hid-over-i2c". If you're not following the spec then you aren't following the spec, so the spec is irrelevant. > ACPI can desribe Interrupt(), GpioInt() and GpioIo() just fine. It is > the Microsoft HID over I2C specification that says it should be > GpioInt() even though we have seen Interrupt() used there as well. Ok, so if the HID over I2C spec says that for ACPI, do that for ACPI. I don't follow why the DT binding should do this. Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 26, 2015 at 04:01:20PM +0000, Mark Rutland wrote: > On Mon, Jan 26, 2015 at 03:16:37PM +0000, Mika Westerberg wrote: > > On Mon, Jan 26, 2015 at 02:50:01PM +0000, Mark Rutland wrote: > > > On Mon, Jan 26, 2015 at 02:47:29PM +0000, Mika Westerberg wrote: > > > > On Mon, Jan 26, 2015 at 02:37:24PM +0000, Mark Rutland wrote: > > > > > On Mon, Jan 26, 2015 at 02:29:33PM +0000, Mika Westerberg wrote: > > > > > > The HID over I2C specification allows to have the interrupt for a HID > > > > > > device to be GPIO instead of directly connected to the IO-APIC. > > > > > > > > > > > > Add support for this so that when the driver does not find proper interrupt > > > > > > number from the I2C client structure we check if the device has property > > > > > > named "gpios". This is then assumed to be the GPIO that serves as an > > > > > > interrupt for the device. > > > > > > > > > > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > > > > > --- > > > > > > .../devicetree/bindings/hid/hid-over-i2c.txt | 5 +- > > > > > > drivers/hid/i2c-hid/i2c-hid.c | 70 ++++++++++++++++------ > > > > > > 2 files changed, 56 insertions(+), 19 deletions(-) > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/hid/hid-over-i2c.txt b/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > > > > > > index 488edcb264c4..8f4a99dad3b9 100644 > > > > > > --- a/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > > > > > > +++ b/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > > > > > > @@ -15,7 +15,10 @@ Required properties: > > > > > > - reg: i2c slave address > > > > > > - hid-descr-addr: HID descriptor address > > > > > > - interrupt-parent: the phandle for the interrupt controller > > > > > > -- interrupts: interrupt line > > > > > > +- interrupts: interrupt line if the device uses IO-APIC interrupts > > > > > > + > > > > > > +Optional properties: > > > > > > +- gpios: GPIO used as an interrupt if the device uses GPIO interrupts > > > > > > > > > > Elsewhere we've said that for a GPIO acting as an interrupt line, GPIO > > > > > controller should be marked as an interrupt-controller, and the GPIO > > > > > described as an interrupt line. That also gets you the appropriate > > > > > configuration for the GPIO as an interrupt. > > > > > > > > > > Does this GPIO serve any other purpose than an ersatz interrupt line? > > > > > > > > It is just an interrupt. > > > > > > > > > If not, it should probably be described as an interrupt. From the PoV of > > > > > this device, it's just an interrupt controller hooked up to the > > > > > interrupt pin. > > > > > > > > What I'm trying to do is to get a GPIO that is described in ACPI (as > > > > GpioInt() in _CRS) to be supported in this driver using gpiolib like > > > > this: > > > > > > > > desc = gpiod_get(&client->dev, NULL); > > > > > > > > This calls to find "gpios" property which ends up finding the GpioInt() > > > > in _CRS. > > > > > > I understand what you are trying to do, but I disagree on the principle. > > > If it's logically an interrupt, it should be described as an interrupt. > > > > It is a GPIO line that is used as interrupt. It is not IO-APIC interrupt > > or anything like that. It will be handled through a GPIO driver. > > I understand that the interrupt line is wired up to a GPIO controller, > where the GPIO controller is able to raise an interrupt as required. > > However, from the PoV of the I2C device, this doesn't matter. From it's > PoV it raises an interrupt, and that's all. It has no idea what happens > to be wired up to its IRQ pin, and nor should it. > > > > If ACPI lacks the ability to describe things in that fashion, it's yet > > > another reason that we shouldn't be pretending that DT and ACPI are the > > > same thing. > > > > I'm not saying they are the same thing (they're not). I'm trying to get > > a GPIO from ACPI translated to an interrupt so that the driver can use > > it. Preferably so that the DT description does not prevent people from > > using the same on non-ACPI platforms. > > If you're following the Microsoft HID over I2C ACPI spec, why is this DT > binding document relevant. Assuming you're following the spec, you won't > be using _DSD with "hid-over-i2c". If you're not following the spec then > you aren't following the spec, so the spec is irrelevant. > > > ACPI can desribe Interrupt(), GpioInt() and GpioIo() just fine. It is > > the Microsoft HID over I2C specification that says it should be > > GpioInt() even though we have seen Interrupt() used there as well. > > Ok, so if the HID over I2C spec says that for ACPI, do that for ACPI. > > I don't follow why the DT binding should do this. I added it there because I thought it's the right thing to do. After all it is assumed that there exists property "gpios" now for both ACPI and DT if I pass NULL in gpiod_get(). Obviously I was wrong. I'll remove the binding documentation from the next revision and call it "ACPI only" if it makes this work for you. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 26, 2015 at 04:13:56PM +0000, Mika Westerberg wrote: > On Mon, Jan 26, 2015 at 04:01:20PM +0000, Mark Rutland wrote: > > On Mon, Jan 26, 2015 at 03:16:37PM +0000, Mika Westerberg wrote: > > > On Mon, Jan 26, 2015 at 02:50:01PM +0000, Mark Rutland wrote: > > > > On Mon, Jan 26, 2015 at 02:47:29PM +0000, Mika Westerberg wrote: > > > > > On Mon, Jan 26, 2015 at 02:37:24PM +0000, Mark Rutland wrote: > > > > > > On Mon, Jan 26, 2015 at 02:29:33PM +0000, Mika Westerberg wrote: > > > > > > > The HID over I2C specification allows to have the interrupt for a HID > > > > > > > device to be GPIO instead of directly connected to the IO-APIC. > > > > > > > > > > > > > > Add support for this so that when the driver does not find proper interrupt > > > > > > > number from the I2C client structure we check if the device has property > > > > > > > named "gpios". This is then assumed to be the GPIO that serves as an > > > > > > > interrupt for the device. > > > > > > > > > > > > > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > > > > > > --- > > > > > > > .../devicetree/bindings/hid/hid-over-i2c.txt | 5 +- > > > > > > > drivers/hid/i2c-hid/i2c-hid.c | 70 ++++++++++++++++------ > > > > > > > 2 files changed, 56 insertions(+), 19 deletions(-) > > > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/hid/hid-over-i2c.txt b/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > > > > > > > index 488edcb264c4..8f4a99dad3b9 100644 > > > > > > > --- a/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > > > > > > > +++ b/Documentation/devicetree/bindings/hid/hid-over-i2c.txt > > > > > > > @@ -15,7 +15,10 @@ Required properties: > > > > > > > - reg: i2c slave address > > > > > > > - hid-descr-addr: HID descriptor address > > > > > > > - interrupt-parent: the phandle for the interrupt controller > > > > > > > -- interrupts: interrupt line > > > > > > > +- interrupts: interrupt line if the device uses IO-APIC interrupts > > > > > > > + > > > > > > > +Optional properties: > > > > > > > +- gpios: GPIO used as an interrupt if the device uses GPIO interrupts > > > > > > > > > > > > Elsewhere we've said that for a GPIO acting as an interrupt line, GPIO > > > > > > controller should be marked as an interrupt-controller, and the GPIO > > > > > > described as an interrupt line. That also gets you the appropriate > > > > > > configuration for the GPIO as an interrupt. > > > > > > > > > > > > Does this GPIO serve any other purpose than an ersatz interrupt line? > > > > > > > > > > It is just an interrupt. > > > > > > > > > > > If not, it should probably be described as an interrupt. From the PoV of > > > > > > this device, it's just an interrupt controller hooked up to the > > > > > > interrupt pin. > > > > > > > > > > What I'm trying to do is to get a GPIO that is described in ACPI (as > > > > > GpioInt() in _CRS) to be supported in this driver using gpiolib like > > > > > this: > > > > > > > > > > desc = gpiod_get(&client->dev, NULL); > > > > > > > > > > This calls to find "gpios" property which ends up finding the GpioInt() > > > > > in _CRS. > > > > > > > > I understand what you are trying to do, but I disagree on the principle. > > > > If it's logically an interrupt, it should be described as an interrupt. > > > > > > It is a GPIO line that is used as interrupt. It is not IO-APIC interrupt > > > or anything like that. It will be handled through a GPIO driver. > > > > I understand that the interrupt line is wired up to a GPIO controller, > > where the GPIO controller is able to raise an interrupt as required. > > > > However, from the PoV of the I2C device, this doesn't matter. From it's > > PoV it raises an interrupt, and that's all. It has no idea what happens > > to be wired up to its IRQ pin, and nor should it. > > > > > > If ACPI lacks the ability to describe things in that fashion, it's yet > > > > another reason that we shouldn't be pretending that DT and ACPI are the > > > > same thing. > > > > > > I'm not saying they are the same thing (they're not). I'm trying to get > > > a GPIO from ACPI translated to an interrupt so that the driver can use > > > it. Preferably so that the DT description does not prevent people from > > > using the same on non-ACPI platforms. > > > > If you're following the Microsoft HID over I2C ACPI spec, why is this DT > > binding document relevant. Assuming you're following the spec, you won't > > be using _DSD with "hid-over-i2c". If you're not following the spec then > > you aren't following the spec, so the spec is irrelevant. > > > > > ACPI can desribe Interrupt(), GpioInt() and GpioIo() just fine. It is > > > the Microsoft HID over I2C specification that says it should be > > > GpioInt() even though we have seen Interrupt() used there as well. > > > > Ok, so if the HID over I2C spec says that for ACPI, do that for ACPI. > > > > I don't follow why the DT binding should do this. > > I added it there because I thought it's the right thing to do. After all > it is assumed that there exists property "gpios" now for both ACPI and > DT if I pass NULL in gpiod_get(). That's not great. Now we have ACPI and DT concerns leaking into each other when they shouldn't necessarily. > Obviously I was wrong. Well, not quite. We shouldn't accept non-documented properties in DT, and documenting the property is certainly appreciated. The issue here seems to be an inconsistency between how we handle GPIO-backed interrupts described in DT vs ACPI. As far as I can tell from the specs I've looked at, GpioInt describes an interrupt that is backed by a GPIO. That sounds similar to what we would do in DT in that we are describing the resource logically as an interrupt. What I don't follow is why GpioInt seems to be translated as a GPIO rather than as an interrupt which happens to be backed by a GPIO. Were it not for that, the DT and ACPI cases would align better. > I'll remove the binding documentation from the next revision and call it > "ACPI only" if it makes this work for you. I would prefer not if that still leaves the property possible in the DT case. We should figure out how we cater for these differences more generally; I'm sure more are going to appear. Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 26, 2015 at 04:39:30PM +0000, Mark Rutland wrote: > What I don't follow is why GpioInt seems to be translated as a GPIO > rather than as an interrupt which happens to be backed by a GPIO. Were > it not for that, the DT and ACPI cases would align better. Because it *is* a GPIO. In my experience we have had two kinds of interrupts that the devices are connected to: pins connected directly to the interrupt controller (IO-APIC or whatever), and pins connected to the GPIO controller. This is the later. Now, the device in question has following resources: Name (_CRS, ResourceTemplate () { I2cSerialBus (0x004C, ControllerInitiated, 0x00061A80, AddressingMode7Bit, "\\_SB.PCI0.I2C6", 0x00, ResourceConsumer,,) GpioInt (Level, ActiveLow, Shared, PullDefault, 0x0000, "\\_SB.GPO0", 0x00, ResourceConsumer,,) { 0x004C } }) The GpioInt() above refers to GPIO controller ("\_SB.GPO0") and its pin number 0x4c. Normally I would do this in the driver regardless of where it is described (DT, ACPI, whatnot): desc = gpiod_get(dev, NULL); gpiod_direction_input(desc); irq = gpiod_to_irq(desc); Then the "irq" can be used for request_irq() and friends. Note how both DT and ACPI cases align just fine. If the above is not the right way to use GPIOs as interrupt, can you please tell me how it is done then? BTW, passing NULL to gpiod_get() implies property named "gpios" in DT (which is why I added it to the documentation). -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 27, 2015 at 10:16:10AM +0000, Mika Westerberg wrote: > On Mon, Jan 26, 2015 at 04:39:30PM +0000, Mark Rutland wrote: > > What I don't follow is why GpioInt seems to be translated as a GPIO > > rather than as an interrupt which happens to be backed by a GPIO. Were > > it not for that, the DT and ACPI cases would align better. > > Because it *is* a GPIO. I don't disagree on this point. However, this is irrelevant from the PoV of the device in question. It doesn't care what its interrupt line is wired up to so long as it behaves like an interrupt. Hopefully the example below will clarify what I'm getting at here. > In my experience we have had two kinds of interrupts that the devices > are connected to: pins connected directly to the interrupt controller > (IO-APIC or whatever), and pins connected to the GPIO controller. This > is the later. > > Now, the device in question has following resources: > > Name (_CRS, ResourceTemplate () { > I2cSerialBus (0x004C, ControllerInitiated, 0x00061A80, > AddressingMode7Bit, "\\_SB.PCI0.I2C6", > 0x00, ResourceConsumer,,) > GpioInt (Level, ActiveLow, Shared, PullDefault, 0x0000, > "\\_SB.GPO0", 0x00, ResourceConsumer,,) > { > 0x004C > } > }) > > The GpioInt() above refers to GPIO controller ("\_SB.GPO0") and its pin > number 0x4c. > > Normally I would do this in the driver regardless of where it is > described (DT, ACPI, whatnot): > > desc = gpiod_get(dev, NULL); > gpiod_direction_input(desc); > irq = gpiod_to_irq(desc); > > Then the "irq" can be used for request_irq() and friends. > > Note how both DT and ACPI cases align just fine. > > If the above is not the right way to use GPIOs as interrupt, can you > please tell me how it is done then? So lets say we have a device which generates an interrupt: device@f00 { compatible = "some-interrupting-device"; reg = <0xf00 0x100>; interrupts = < ... >; }; It's intended that this is connected to an interrupt controller: ic: interrupt-controller@b00 { compatible = "some-interrupt-controller"; reg = <0xb00 0x100>; #interrupt-cells = <1>; }; device@f00 { compatible = "some-interrupting-device"; reg = <0xf00 0x100>; interrupt-parent = <&ic>; interrupts = <0x3>; }; But in some cases, this gets connected to a GPIO controller. In these cases, the device is still logically generating an interrupt, and the fact that the endpoint is an interrupt controller is irrelevant from the PoV of the device. So we acknowledge that the GPIO controller is also capable of acting as an interrupt controller, and mark it as such: gc: gpio-controller@000 { compatible = "some-gpio-controller"; reg = <0x000 0x100>; #gpio-cells = <1>; #interrupt-cells = <1>; }; device@f00 { compatible = "some-interrupting-device"; reg = <0xf00 0x100>; interrupt-parent = <&gc>; interrupts = <0x1>; }; Thus the device binding only describes the logical interrupt, and the driver only needs to handle interrupts. In cases where the binding/driver actually care about the GPIO being a GPIO (e.g. for card detect in an MMC controller), describing the GPIO as a GPIO makes sense, and we can try gpio_to_irq as an optimisation over polling the state of the GPIO. > BTW, passing NULL to gpiod_get() implies property named "gpios" in DT > (which is why I added it to the documentation). Sure. My concern is that we should not need to deal with GPIOs in this case were the GPIO is only there to function as an interrupt. Given that GpioInt seems to describe an interrupt which happens to be backed by a GPIO, I don't understand what it is necessary to translate this as a GPIO rather than an interrupt. If it were going to be used as a GPIO, then it would be a GpioIO object, no? Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 27, 2015 at 10:39:25AM +0000, Mark Rutland wrote: > > If the above is not the right way to use GPIOs as interrupt, can you > > please tell me how it is done then? > > > So lets say we have a device which generates an interrupt: > > device@f00 { > compatible = "some-interrupting-device"; > reg = <0xf00 0x100>; > interrupts = < ... >; > }; > > It's intended that this is connected to an interrupt controller: > > ic: interrupt-controller@b00 { > compatible = "some-interrupt-controller"; > reg = <0xb00 0x100>; > #interrupt-cells = <1>; > }; > > device@f00 { > compatible = "some-interrupting-device"; > reg = <0xf00 0x100>; > interrupt-parent = <&ic>; > interrupts = <0x3>; > }; > > But in some cases, this gets connected to a GPIO controller. In these > cases, the device is still logically generating an interrupt, and the > fact that the endpoint is an interrupt controller is irrelevant from the > PoV of the device. So we acknowledge that the GPIO controller is also > capable of acting as an interrupt controller, and mark it as such: > > gc: gpio-controller@000 { > compatible = "some-gpio-controller"; > reg = <0x000 0x100>; > #gpio-cells = <1>; > #interrupt-cells = <1>; > }; > > device@f00 { > compatible = "some-interrupting-device"; > reg = <0xf00 0x100>; > interrupt-parent = <&gc>; > interrupts = <0x1>; > }; > > Thus the device binding only describes the logical interrupt, and the > driver only needs to handle interrupts. OK. > In cases where the binding/driver actually care about the GPIO being a > GPIO (e.g. for card detect in an MMC controller), describing the GPIO as > a GPIO makes sense, and we can try gpio_to_irq as an optimisation over > polling the state of the GPIO. Well, I've seen touch panels where you actually need to switch the GPIO to be output and do some magic before you can use the same GPIO as an interrupt. > > BTW, passing NULL to gpiod_get() implies property named "gpios" in DT > > (which is why I added it to the documentation). > > Sure. My concern is that we should not need to deal with GPIOs in this > case were the GPIO is only there to function as an interrupt. > > Given that GpioInt seems to describe an interrupt which happens to be > backed by a GPIO, I don't understand what it is necessary to translate > this as a GPIO rather than an interrupt. If it were going to be used as > a GPIO, then it would be a GpioIO object, no? OK, so where do you propose we handle the translation if not in the driver? Also keep in mind that some of the devices may have multiple GpioInt()s. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 27, 2015 at 10:59:31AM +0000, Mika Westerberg wrote: > On Tue, Jan 27, 2015 at 10:39:25AM +0000, Mark Rutland wrote: > > > If the above is not the right way to use GPIOs as interrupt, can you > > > please tell me how it is done then? > > > > > > So lets say we have a device which generates an interrupt: > > > > device@f00 { > > compatible = "some-interrupting-device"; > > reg = <0xf00 0x100>; > > interrupts = < ... >; > > }; > > > > It's intended that this is connected to an interrupt controller: > > > > ic: interrupt-controller@b00 { > > compatible = "some-interrupt-controller"; > > reg = <0xb00 0x100>; > > #interrupt-cells = <1>; > > }; > > > > device@f00 { > > compatible = "some-interrupting-device"; > > reg = <0xf00 0x100>; > > interrupt-parent = <&ic>; > > interrupts = <0x3>; > > }; > > > > But in some cases, this gets connected to a GPIO controller. In these > > cases, the device is still logically generating an interrupt, and the > > fact that the endpoint is an interrupt controller is irrelevant from the > > PoV of the device. So we acknowledge that the GPIO controller is also > > capable of acting as an interrupt controller, and mark it as such: > > > > gc: gpio-controller@000 { > > compatible = "some-gpio-controller"; > > reg = <0x000 0x100>; > > #gpio-cells = <1>; > > #interrupt-cells = <1>; > > }; > > > > device@f00 { > > compatible = "some-interrupting-device"; > > reg = <0xf00 0x100>; > > interrupt-parent = <&gc>; > > interrupts = <0x1>; > > }; > > > > Thus the device binding only describes the logical interrupt, and the > > driver only needs to handle interrupts. > > OK. > > > In cases where the binding/driver actually care about the GPIO being a > > GPIO (e.g. for card detect in an MMC controller), describing the GPIO as > > a GPIO makes sense, and we can try gpio_to_irq as an optimisation over > > polling the state of the GPIO. > > Well, I've seen touch panels where you actually need to switch the GPIO > to be output and do some magic before you can use the same GPIO as an > interrupt. Ok. That's a nasty case, but surely in that case the relevant GPIO shoiuld be a GpioIO object for output? > > > BTW, passing NULL to gpiod_get() implies property named "gpios" in DT > > > (which is why I added it to the documentation). > > > > Sure. My concern is that we should not need to deal with GPIOs in this > > case were the GPIO is only there to function as an interrupt. > > > > Given that GpioInt seems to describe an interrupt which happens to be > > backed by a GPIO, I don't understand what it is necessary to translate > > this as a GPIO rather than an interrupt. If it were going to be used as > > a GPIO, then it would be a GpioIO object, no? > > OK, so where do you propose we handle the translation if not in the > driver? Also keep in mind that some of the devices may have multiple > GpioInt()s. To me it seems that GpioInt objects should be translated to interrupts by some core code. How are interrupts described and handed in ACPI? Are they resource along the lines of GpioInts, or are they a completely separate class of device property? Thanks, Mark -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 27, 2015 at 11:14:58AM +0000, Mark Rutland wrote: > On Tue, Jan 27, 2015 at 10:59:31AM +0000, Mika Westerberg wrote: > > On Tue, Jan 27, 2015 at 10:39:25AM +0000, Mark Rutland wrote: > > > > If the above is not the right way to use GPIOs as interrupt, can you > > > > please tell me how it is done then? > > > > > > > > > So lets say we have a device which generates an interrupt: > > > > > > device@f00 { > > > compatible = "some-interrupting-device"; > > > reg = <0xf00 0x100>; > > > interrupts = < ... >; > > > }; > > > > > > It's intended that this is connected to an interrupt controller: > > > > > > ic: interrupt-controller@b00 { > > > compatible = "some-interrupt-controller"; > > > reg = <0xb00 0x100>; > > > #interrupt-cells = <1>; > > > }; > > > > > > device@f00 { > > > compatible = "some-interrupting-device"; > > > reg = <0xf00 0x100>; > > > interrupt-parent = <&ic>; > > > interrupts = <0x3>; > > > }; > > > > > > But in some cases, this gets connected to a GPIO controller. In these > > > cases, the device is still logically generating an interrupt, and the > > > fact that the endpoint is an interrupt controller is irrelevant from the > > > PoV of the device. So we acknowledge that the GPIO controller is also > > > capable of acting as an interrupt controller, and mark it as such: > > > > > > gc: gpio-controller@000 { > > > compatible = "some-gpio-controller"; > > > reg = <0x000 0x100>; > > > #gpio-cells = <1>; > > > #interrupt-cells = <1>; > > > }; > > > > > > device@f00 { > > > compatible = "some-interrupting-device"; > > > reg = <0xf00 0x100>; > > > interrupt-parent = <&gc>; > > > interrupts = <0x1>; > > > }; > > > > > > Thus the device binding only describes the logical interrupt, and the > > > driver only needs to handle interrupts. > > > > OK. > > > > > In cases where the binding/driver actually care about the GPIO being a > > > GPIO (e.g. for card detect in an MMC controller), describing the GPIO as > > > a GPIO makes sense, and we can try gpio_to_irq as an optimisation over > > > polling the state of the GPIO. > > > > Well, I've seen touch panels where you actually need to switch the GPIO > > to be output and do some magic before you can use the same GPIO as an > > interrupt. > > Ok. That's a nasty case, but surely in that case the relevant GPIO > shoiuld be a GpioIO object for output? I can't remember the details anymore, possibly it was GpioIo(). Nothing prevents you from using GpioIo() as an interrupt. > > > > > BTW, passing NULL to gpiod_get() implies property named "gpios" in DT > > > > (which is why I added it to the documentation). > > > > > > Sure. My concern is that we should not need to deal with GPIOs in this > > > case were the GPIO is only there to function as an interrupt. > > > > > > Given that GpioInt seems to describe an interrupt which happens to be > > > backed by a GPIO, I don't understand what it is necessary to translate > > > this as a GPIO rather than an interrupt. If it were going to be used as > > > a GPIO, then it would be a GpioIO object, no? > > > > OK, so where do you propose we handle the translation if not in the > > driver? Also keep in mind that some of the devices may have multiple > > GpioInt()s. > > To me it seems that GpioInt objects should be translated to interrupts > by some core code. How are interrupts described and handed in ACPI? Are > they resource along the lines of GpioInts, or are they a completely > separate class of device property? They are similar resources in _CRS, like GpioIo/GpioInt etc. Below is from another touch panel: Name (_CRS, ResourceTemplate () { I2cSerialBus (0x004C, ControllerInitiated, 0x00061A80, AddressingMode7Bit, "\\_SB.PCI0.I2C1", 0x00, ResourceConsumer,,) Interrupt (ResourceConsumer, Level, ActiveLow, Exclusive, ,, ) { 0x00000022, } }) If we see one of the above we automatically add it to client->irq in case of I2C device. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 27, 2015 at 11:30:41AM +0000, Mika Westerberg wrote: > On Tue, Jan 27, 2015 at 11:14:58AM +0000, Mark Rutland wrote: > > On Tue, Jan 27, 2015 at 10:59:31AM +0000, Mika Westerberg wrote: > > > On Tue, Jan 27, 2015 at 10:39:25AM +0000, Mark Rutland wrote: > > > > > If the above is not the right way to use GPIOs as interrupt, can you > > > > > please tell me how it is done then? > > > > > > > > > > > > So lets say we have a device which generates an interrupt: > > > > > > > > device@f00 { > > > > compatible = "some-interrupting-device"; > > > > reg = <0xf00 0x100>; > > > > interrupts = < ... >; > > > > }; > > > > > > > > It's intended that this is connected to an interrupt controller: > > > > > > > > ic: interrupt-controller@b00 { > > > > compatible = "some-interrupt-controller"; > > > > reg = <0xb00 0x100>; > > > > #interrupt-cells = <1>; > > > > }; > > > > > > > > device@f00 { > > > > compatible = "some-interrupting-device"; > > > > reg = <0xf00 0x100>; > > > > interrupt-parent = <&ic>; > > > > interrupts = <0x3>; > > > > }; > > > > > > > > But in some cases, this gets connected to a GPIO controller. In these > > > > cases, the device is still logically generating an interrupt, and the > > > > fact that the endpoint is an interrupt controller is irrelevant from the > > > > PoV of the device. So we acknowledge that the GPIO controller is also > > > > capable of acting as an interrupt controller, and mark it as such: > > > > > > > > gc: gpio-controller@000 { > > > > compatible = "some-gpio-controller"; > > > > reg = <0x000 0x100>; > > > > #gpio-cells = <1>; > > > > #interrupt-cells = <1>; > > > > }; > > > > > > > > device@f00 { > > > > compatible = "some-interrupting-device"; > > > > reg = <0xf00 0x100>; > > > > interrupt-parent = <&gc>; > > > > interrupts = <0x1>; > > > > }; > > > > > > > > Thus the device binding only describes the logical interrupt, and the > > > > driver only needs to handle interrupts. > > > > > > OK. > > > > > > > In cases where the binding/driver actually care about the GPIO being a > > > > GPIO (e.g. for card detect in an MMC controller), describing the GPIO as > > > > a GPIO makes sense, and we can try gpio_to_irq as an optimisation over > > > > polling the state of the GPIO. > > > > > > Well, I've seen touch panels where you actually need to switch the GPIO > > > to be output and do some magic before you can use the same GPIO as an > > > interrupt. > > > > Ok. That's a nasty case, but surely in that case the relevant GPIO > > shoiuld be a GpioIO object for output? > > I can't remember the details anymore, possibly it was GpioIo(). > > Nothing prevents you from using GpioIo() as an interrupt. Certainly. As I mention above, in the case of something like a card detect pin, it makes sense to be able to acquire an interrupt for the GPIO. > > > > > BTW, passing NULL to gpiod_get() implies property named "gpios" in DT > > > > > (which is why I added it to the documentation). > > > > > > > > Sure. My concern is that we should not need to deal with GPIOs in this > > > > case were the GPIO is only there to function as an interrupt. > > > > > > > > Given that GpioInt seems to describe an interrupt which happens to be > > > > backed by a GPIO, I don't understand what it is necessary to translate > > > > this as a GPIO rather than an interrupt. If it were going to be used as > > > > a GPIO, then it would be a GpioIO object, no? > > > > > > OK, so where do you propose we handle the translation if not in the > > > driver? Also keep in mind that some of the devices may have multiple > > > GpioInt()s. > > > > To me it seems that GpioInt objects should be translated to interrupts > > by some core code. How are interrupts described and handed in ACPI? Are > > they resource along the lines of GpioInts, or are they a completely > > separate class of device property? > > They are similar resources in _CRS, like GpioIo/GpioInt etc. Below is > from another touch panel: > > Name (_CRS, ResourceTemplate () { > I2cSerialBus (0x004C, ControllerInitiated, 0x00061A80, > AddressingMode7Bit, "\\_SB.PCI0.I2C1", 0x00, ResourceConsumer,,) > Interrupt (ResourceConsumer, Level, ActiveLow, Exclusive, ,, ) > { > 0x00000022, > } > }) > > If we see one of the above we automatically add it to client->irq in > case of I2C device. Ok, that allays my fear w.r.t. ordering of the resources. As I see it, the fact that we convert GpioInt entries to GPIOs rather than irqs when parsing _CRS is the issue here, and to me it makes no sense that we do so. Were we to treat them as interrupts, the binding is fine as-is, and we'd do the same thing in DT and ACPI. The reason GpioInt is separate from GpioIo is that a GpioInt _is_ an interrupt (which happens to be backed by a GPIO), and is not something that necessarily makes sense as a GPIO. So why do we currently ignore the GpioInt/GpioIo distinction and treat GpioInts as GPIOs rather than interrupts? Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 27, 2015 at 02:33:34PM +0000, Mark Rutland wrote: > Ok, that allays my fear w.r.t. ordering of the resources. > > As I see it, the fact that we convert GpioInt entries to GPIOs rather > than irqs when parsing _CRS is the issue here, and to me it makes no > sense that we do so. Were we to treat them as interrupts, the binding is > fine as-is, and we'd do the same thing in DT and ACPI. > > The reason GpioInt is separate from GpioIo is that a GpioInt _is_ an > interrupt (which happens to be backed by a GPIO), and is not something > that necessarily makes sense as a GPIO. I would rather say that GpioInt *is* a GPIO. That can then used as an interrupt but it should not prevent you from using it as GPIO instead. For example if you just want to poll that something is 0 or 1. That should be possible as well and nothing say that you cannot do that for GpioInt(). > So why do we currently ignore the GpioInt/GpioIo distinction and treat > GpioInts as GPIOs rather than interrupts? See above. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 27, 2015 at 02:41:27PM +0000, Mika Westerberg wrote: > On Tue, Jan 27, 2015 at 02:33:34PM +0000, Mark Rutland wrote: > > Ok, that allays my fear w.r.t. ordering of the resources. > > > > As I see it, the fact that we convert GpioInt entries to GPIOs rather > > than irqs when parsing _CRS is the issue here, and to me it makes no > > sense that we do so. Were we to treat them as interrupts, the binding is > > fine as-is, and we'd do the same thing in DT and ACPI. > > > > The reason GpioInt is separate from GpioIo is that a GpioInt _is_ an > > interrupt (which happens to be backed by a GPIO), and is not something > > that necessarily makes sense as a GPIO. > > I would rather say that GpioInt *is* a GPIO. That can then used as an > interrupt but it should not prevent you from using it as GPIO instead. > For example if you just want to poll that something is 0 or 1. That > should be possible as well and nothing say that you cannot do that for > GpioInt(). From my POV a GpioInt is logically an interrupt, or it would be a GpioIo. That doesn't necessarily mean it's invalid to try to query its state as a GPIO, but I do not think that it makes sense to handle it by default as a GPIO given that it was handed to us as a GpioInt so that it can be used as an interrupt. If it's just the case that ACPI and DT differ w.r.t. how this case (an interrupt line wired to a GPIO) is described, that in itself is fine; different standards have different models. However, I do not think we must change the DT binding and violate established DT practice simply becuase on the ACPI side things are different (nor would it make sense to do things the other way around). If the two have different rules, then we should handle those rules separately rather than trying to force the two together when they clearly don't fit. In the driver that can easily be achieved with separate probe paths. W.R.T. the DT binding, I do not think it makes sense to pretend this is also an ACPI binding. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 27, 2015 at 03:06:06PM +0000, Mark Rutland wrote: > On Tue, Jan 27, 2015 at 02:41:27PM +0000, Mika Westerberg wrote: > > On Tue, Jan 27, 2015 at 02:33:34PM +0000, Mark Rutland wrote: > > > Ok, that allays my fear w.r.t. ordering of the resources. > > > > > > As I see it, the fact that we convert GpioInt entries to GPIOs rather > > > than irqs when parsing _CRS is the issue here, and to me it makes no > > > sense that we do so. Were we to treat them as interrupts, the binding is > > > fine as-is, and we'd do the same thing in DT and ACPI. > > > > > > The reason GpioInt is separate from GpioIo is that a GpioInt _is_ an > > > interrupt (which happens to be backed by a GPIO), and is not something > > > that necessarily makes sense as a GPIO. > > > > I would rather say that GpioInt *is* a GPIO. That can then used as an > > interrupt but it should not prevent you from using it as GPIO instead. > > For example if you just want to poll that something is 0 or 1. That > > should be possible as well and nothing say that you cannot do that for > > GpioInt(). > > >From my POV a GpioInt is logically an interrupt, or it would be a > GpioIo. That doesn't necessarily mean it's invalid to try to query its > state as a GPIO, but I do not think that it makes sense to handle it by > default as a GPIO given that it was handed to us as a GpioInt so that it > can be used as an interrupt. > > If it's just the case that ACPI and DT differ w.r.t. how this case (an > interrupt line wired to a GPIO) is described, that in itself is fine; > different standards have different models. > > However, I do not think we must change the DT binding and violate > established DT practice simply becuase on the ACPI side things are > different (nor would it make sense to do things the other way around). > If the two have different rules, then we should handle those rules > separately rather than trying to force the two together when they > clearly don't fit. I agree. > In the driver that can easily be achieved with separate probe paths. Yes, that's what this patch is doing. It has different paths for interrupt and GPIO cases. If we find that there is an interrupt number already, then we use that directly. If not we try to look for a GPIO which we could use as an interrupt. The reason why this is not done only in the ACPI probe path is that there is nothing ACPI specific in gpiod_get() and friends. > W.R.T. the DT binding, I do not think it makes sense to pretend this > is also an ACPI binding. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 27, 2015 at 03:21:27PM +0000, Mika Westerberg wrote: > On Tue, Jan 27, 2015 at 03:06:06PM +0000, Mark Rutland wrote: > > On Tue, Jan 27, 2015 at 02:41:27PM +0000, Mika Westerberg wrote: > > > On Tue, Jan 27, 2015 at 02:33:34PM +0000, Mark Rutland wrote: > > > > Ok, that allays my fear w.r.t. ordering of the resources. > > > > > > > > As I see it, the fact that we convert GpioInt entries to GPIOs rather > > > > than irqs when parsing _CRS is the issue here, and to me it makes no > > > > sense that we do so. Were we to treat them as interrupts, the binding is > > > > fine as-is, and we'd do the same thing in DT and ACPI. > > > > > > > > The reason GpioInt is separate from GpioIo is that a GpioInt _is_ an > > > > interrupt (which happens to be backed by a GPIO), and is not something > > > > that necessarily makes sense as a GPIO. > > > > > > I would rather say that GpioInt *is* a GPIO. That can then used as an > > > interrupt but it should not prevent you from using it as GPIO instead. > > > For example if you just want to poll that something is 0 or 1. That > > > should be possible as well and nothing say that you cannot do that for > > > GpioInt(). > > > > >From my POV a GpioInt is logically an interrupt, or it would be a > > GpioIo. That doesn't necessarily mean it's invalid to try to query its > > state as a GPIO, but I do not think that it makes sense to handle it by > > default as a GPIO given that it was handed to us as a GpioInt so that it > > can be used as an interrupt. > > > > If it's just the case that ACPI and DT differ w.r.t. how this case (an > > interrupt line wired to a GPIO) is described, that in itself is fine; > > different standards have different models. > > > > However, I do not think we must change the DT binding and violate > > established DT practice simply becuase on the ACPI side things are > > different (nor would it make sense to do things the other way around). > > If the two have different rules, then we should handle those rules > > separately rather than trying to force the two together when they > > clearly don't fit. > > I agree. > > > In the driver that can easily be achieved with separate probe paths. > > Yes, that's what this patch is doing. It has different paths for > interrupt and GPIO cases. If we find that there is an interrupt number > already, then we use that directly. > > If not we try to look for a GPIO which we could use as an interrupt. My comment about separate probe paths was w.r.t. ACPI and DT. In DT, acquiring a GPIO here shouldn't be necessary (and ideally shouldn't be allowed) because the GPIO should be described as an interrupt (we need this to have the correct flags). > The reason why this is not done only in the ACPI probe path is that > there is nothing ACPI specific in gpiod_get() and friends. While the high-level APIs are not specific to ACPI or DT, the behaviour we want for DT is subtly different top the behaviour you want for ACPI. In DT we don't necessarily have the relevant flags (e.g. edge vs level, active high vs active low) unless the GPIO is described as an interrupt, so permitting it to be described as a GPIO permits situations which we cannot handle correctly. In the DT case therefore we must not acquire the interrupt GPIO as a GPIO; it must be described as an interrupt. So if we need to acquire a GPIO in the ACPI case, this should be limited to the ACPI case. Looking further at ACPI, the flags issue also seems to be a problem there. If the interrupt GPIO were described as a GpioIo, then you could acquire it as a GPIO with gpiod_get(), but wouldn't have the EdgeLevel and ActiveLevel properties, and therefore cannot necessarily correctly convert the GPIO to an irq, no? Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jan 27, 2015 at 03:57:47PM +0000, Mark Rutland wrote: > On Tue, Jan 27, 2015 at 03:21:27PM +0000, Mika Westerberg wrote: > > On Tue, Jan 27, 2015 at 03:06:06PM +0000, Mark Rutland wrote: > > > On Tue, Jan 27, 2015 at 02:41:27PM +0000, Mika Westerberg wrote: > > > > On Tue, Jan 27, 2015 at 02:33:34PM +0000, Mark Rutland wrote: > > > > > Ok, that allays my fear w.r.t. ordering of the resources. > > > > > > > > > > As I see it, the fact that we convert GpioInt entries to GPIOs rather > > > > > than irqs when parsing _CRS is the issue here, and to me it makes no > > > > > sense that we do so. Were we to treat them as interrupts, the binding is > > > > > fine as-is, and we'd do the same thing in DT and ACPI. > > > > > > > > > > The reason GpioInt is separate from GpioIo is that a GpioInt _is_ an > > > > > interrupt (which happens to be backed by a GPIO), and is not something > > > > > that necessarily makes sense as a GPIO. > > > > > > > > I would rather say that GpioInt *is* a GPIO. That can then used as an > > > > interrupt but it should not prevent you from using it as GPIO instead. > > > > For example if you just want to poll that something is 0 or 1. That > > > > should be possible as well and nothing say that you cannot do that for > > > > GpioInt(). > > > > > > >From my POV a GpioInt is logically an interrupt, or it would be a > > > GpioIo. That doesn't necessarily mean it's invalid to try to query its > > > state as a GPIO, but I do not think that it makes sense to handle it by > > > default as a GPIO given that it was handed to us as a GpioInt so that it > > > can be used as an interrupt. > > > > > > If it's just the case that ACPI and DT differ w.r.t. how this case (an > > > interrupt line wired to a GPIO) is described, that in itself is fine; > > > different standards have different models. > > > > > > However, I do not think we must change the DT binding and violate > > > established DT practice simply becuase on the ACPI side things are > > > different (nor would it make sense to do things the other way around). > > > If the two have different rules, then we should handle those rules > > > separately rather than trying to force the two together when they > > > clearly don't fit. > > > > I agree. > > > > > In the driver that can easily be achieved with separate probe paths. > > > > Yes, that's what this patch is doing. It has different paths for > > interrupt and GPIO cases. If we find that there is an interrupt number > > already, then we use that directly. > > > > If not we try to look for a GPIO which we could use as an interrupt. > > My comment about separate probe paths was w.r.t. ACPI and DT. In DT, > acquiring a GPIO here shouldn't be necessary (and ideally shouldn't be > allowed) because the GPIO should be described as an interrupt (we need > this to have the correct flags). > > > The reason why this is not done only in the ACPI probe path is that > > there is nothing ACPI specific in gpiod_get() and friends. > > While the high-level APIs are not specific to ACPI or DT, the behaviour > we want for DT is subtly different top the behaviour you want for ACPI. > > In DT we don't necessarily have the relevant flags (e.g. edge vs level, > active high vs active low) unless the GPIO is described as an interrupt, > so permitting it to be described as a GPIO permits situations which we > cannot handle correctly. In the DT case therefore we must not acquire > the interrupt GPIO as a GPIO; it must be described as an interrupt. > > So if we need to acquire a GPIO in the ACPI case, this should be limited > to the ACPI case. How about cases where platform data (no ACPI or DT) is used? You can also provide lookup table to the GPIOs from platform data. > Looking further at ACPI, the flags issue also seems to be a problem > there. If the interrupt GPIO were described as a GpioIo, then you could > acquire it as a GPIO with gpiod_get(), but wouldn't have the EdgeLevel > and ActiveLevel properties, and therefore cannot necessarily correctly > convert the GPIO to an irq, no? That's right but typically what I've seen the drivers configure the GPIO as they think is necessary. Anyway, I think I'll give up now and leave the i2c-hid.c driver to support only interrupts for now. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/hid/hid-over-i2c.txt b/Documentation/devicetree/bindings/hid/hid-over-i2c.txt index 488edcb264c4..8f4a99dad3b9 100644 --- a/Documentation/devicetree/bindings/hid/hid-over-i2c.txt +++ b/Documentation/devicetree/bindings/hid/hid-over-i2c.txt @@ -15,7 +15,10 @@ Required properties: - reg: i2c slave address - hid-descr-addr: HID descriptor address - interrupt-parent: the phandle for the interrupt controller -- interrupts: interrupt line +- interrupts: interrupt line if the device uses IO-APIC interrupts + +Optional properties: +- gpios: GPIO used as an interrupt if the device uses GPIO interrupts Example: diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c index 8f1dfc5c5d9c..d3c49ff4888e 100644 --- a/drivers/hid/i2c-hid/i2c-hid.c +++ b/drivers/hid/i2c-hid/i2c-hid.c @@ -37,6 +37,7 @@ #include <linux/mutex.h> #include <linux/acpi.h> #include <linux/of.h> +#include <linux/gpio/consumer.h> #include <linux/i2c/i2c-hid.h> @@ -144,6 +145,8 @@ struct i2c_hid { unsigned long flags; /* device flags */ wait_queue_head_t wait; /* For waiting the interrupt */ + struct gpio_desc *desc; + int irq; struct i2c_hid_platform_data pdata; }; @@ -782,16 +785,16 @@ static int i2c_hid_init_irq(struct i2c_client *client) struct i2c_hid *ihid = i2c_get_clientdata(client); int ret; - dev_dbg(&client->dev, "Requesting IRQ: %d\n", client->irq); + dev_dbg(&client->dev, "Requesting IRQ: %d\n", ihid->irq); - ret = request_threaded_irq(client->irq, NULL, i2c_hid_irq, + ret = request_threaded_irq(ihid->irq, NULL, i2c_hid_irq, IRQF_TRIGGER_LOW | IRQF_ONESHOT, client->name, ihid); if (ret < 0) { dev_warn(&client->dev, "Could not register for %s interrupt, irq = %d," " ret = %d\n", - client->name, client->irq, ret); + client->name, ihid->irq, ret); return ret; } @@ -838,6 +841,14 @@ static int i2c_hid_fetch_hid_descriptor(struct i2c_hid *ihid) } #ifdef CONFIG_ACPI + +/* Default GPIO mapping */ +static const struct acpi_gpio_params i2c_hid_irq_gpio = { 0, 0, true }; +static const struct acpi_gpio_mapping i2c_hid_acpi_gpios[] = { + { "gpios", &i2c_hid_irq_gpio, 1 }, + { }, +}; + static int i2c_hid_acpi_pdata(struct i2c_client *client, struct i2c_hid_platform_data *pdata) { @@ -863,7 +874,7 @@ static int i2c_hid_acpi_pdata(struct i2c_client *client, pdata->hid_descriptor_address = obj->integer.value; ACPI_FREE(obj); - return 0; + return acpi_dev_add_driver_gpios(adev, i2c_hid_acpi_gpios); } static const struct acpi_device_id i2c_hid_acpi_match[] = { @@ -927,12 +938,6 @@ static int i2c_hid_probe(struct i2c_client *client, dbg_hid("HID probe called for i2c 0x%02x\n", client->addr); - if (!client->irq) { - dev_err(&client->dev, - "HID over i2c has not been provided an Int IRQ\n"); - return -EINVAL; - } - ihid = kzalloc(sizeof(struct i2c_hid), GFP_KERNEL); if (!ihid) return -ENOMEM; @@ -952,6 +957,25 @@ static int i2c_hid_probe(struct i2c_client *client, ihid->pdata = *platform_data; } + if (client->irq > 0) { + ihid->irq = client->irq; + } else { + ihid->desc = gpiod_get(&client->dev, NULL); + if (IS_ERR(ihid->desc)) { + dev_err(&client->dev, "Failed to get GPIO interrupt\n"); + return PTR_ERR(ihid->desc); + } + + gpiod_direction_input(ihid->desc); + + ihid->irq = gpiod_to_irq(ihid->desc); + if (ihid->irq < 0) { + gpiod_put(ihid->desc); + dev_err(&client->dev, "Failed to convert GPIO to IRQ\n"); + return ihid->irq; + } + } + i2c_set_clientdata(client, ihid); ihid->client = client; @@ -1014,13 +1038,16 @@ err_mem_free: hid_destroy_device(hid); err_irq: - free_irq(client->irq, ihid); + free_irq(ihid->irq, ihid); err_pm: pm_runtime_put_noidle(&client->dev); pm_runtime_disable(&client->dev); err: + if (ihid->desc) + gpiod_put(ihid->desc); + i2c_hid_free_buffers(ihid); kfree(ihid); return ret; @@ -1039,13 +1066,18 @@ static int i2c_hid_remove(struct i2c_client *client) hid = ihid->hid; hid_destroy_device(hid); - free_irq(client->irq, ihid); + free_irq(ihid->irq, ihid); if (ihid->bufsize) i2c_hid_free_buffers(ihid); + if (ihid->desc) + gpiod_put(ihid->desc); + kfree(ihid); + acpi_dev_remove_driver_gpios(ACPI_COMPANION(&client->dev)); + return 0; } @@ -1057,9 +1089,9 @@ static int i2c_hid_suspend(struct device *dev) struct hid_device *hid = ihid->hid; int ret = 0; - disable_irq(client->irq); + disable_irq(ihid->irq); if (device_may_wakeup(&client->dev)) - enable_irq_wake(client->irq); + enable_irq_wake(ihid->irq); if (hid->driver && hid->driver->suspend) ret = hid->driver->suspend(hid, PMSG_SUSPEND); @@ -1077,13 +1109,13 @@ static int i2c_hid_resume(struct device *dev) struct i2c_hid *ihid = i2c_get_clientdata(client); struct hid_device *hid = ihid->hid; - enable_irq(client->irq); + enable_irq(ihid->irq); ret = i2c_hid_hwreset(client); if (ret) return ret; if (device_may_wakeup(&client->dev)) - disable_irq_wake(client->irq); + disable_irq_wake(ihid->irq); if (hid->driver && hid->driver->reset_resume) { ret = hid->driver->reset_resume(hid); @@ -1098,17 +1130,19 @@ static int i2c_hid_resume(struct device *dev) static int i2c_hid_runtime_suspend(struct device *dev) { struct i2c_client *client = to_i2c_client(dev); + struct i2c_hid *ihid = i2c_get_clientdata(client); i2c_hid_set_power(client, I2C_HID_PWR_SLEEP); - disable_irq(client->irq); + disable_irq(ihid->irq); return 0; } static int i2c_hid_runtime_resume(struct device *dev) { struct i2c_client *client = to_i2c_client(dev); + struct i2c_hid *ihid = i2c_get_clientdata(client); - enable_irq(client->irq); + enable_irq(ihid->irq); i2c_hid_set_power(client, I2C_HID_PWR_ON); return 0; }
The HID over I2C specification allows to have the interrupt for a HID device to be GPIO instead of directly connected to the IO-APIC. Add support for this so that when the driver does not find proper interrupt number from the I2C client structure we check if the device has property named "gpios". This is then assumed to be the GPIO that serves as an interrupt for the device. Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- .../devicetree/bindings/hid/hid-over-i2c.txt | 5 +- drivers/hid/i2c-hid/i2c-hid.c | 70 ++++++++++++++++------ 2 files changed, 56 insertions(+), 19 deletions(-)