* [PATCH 0/2] Tiny modification of i2c-hid
@ 2016-10-13 9:30 Benjamin Tissoires
2016-10-13 9:30 ` [PATCH 1/2] Revert "HID: i2c-hid: Add support for ACPI GPIO interrupts" Benjamin Tissoires
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Benjamin Tissoires @ 2016-10-13 9:30 UTC (permalink / raw)
To: Jiri Kosina, Mika Westerberg; +Cc: David Arcari, linux-input, linux-kernel
Hi Jiri,
David and I are facing an issue in RHEL with the HP Zbook 15 Studio mWS.
This laptops uses the pinctrl-sunrisepoint controller for the GPIOs and
it failed on RHEL. We found out what the issue was, but in the meantime
realized that part of the code we have in i2c-hid is not required anymore.
The actual issue is fixed here: https://lkml.org/lkml/2016/10/12/493
but it would be more convenient (for us) and cleaner (fo everybody) to just
remove the extra boiler-plate in i2c-hid and let i2c-core handling the
attributions of the IRQ.
Cheers,
Benjamin
David Arcari (2):
Revert "HID: i2c-hid: Add support for ACPI GPIO interrupts"
HID: i2c-hid: exit if the IRQ is not valid
drivers/hid/i2c-hid/i2c-hid.c | 78 ++++++++++++++-----------------------------
1 file changed, 25 insertions(+), 53 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/2] Revert "HID: i2c-hid: Add support for ACPI GPIO interrupts" 2016-10-13 9:30 [PATCH 0/2] Tiny modification of i2c-hid Benjamin Tissoires @ 2016-10-13 9:30 ` Benjamin Tissoires 2016-10-13 10:24 ` Mika Westerberg 2016-10-13 9:30 ` [PATCH 2/2] HID: i2c-hid: exit if the IRQ is not valid Benjamin Tissoires ` (2 subsequent siblings) 3 siblings, 1 reply; 8+ messages in thread From: Benjamin Tissoires @ 2016-10-13 9:30 UTC (permalink / raw) To: Jiri Kosina, Mika Westerberg; +Cc: David Arcari, linux-input, linux-kernel From: David Arcari <darcari@redhat.com> This reverts commit a485923efbb8 ("HID: i2c-hid: Add support for ACPI GPIO interrupts") and commit a7d2bf25a483 ("HID: i2c-hid: Do not fail probing if gpiolib is not enabled") at the same time. Since commit c884fbd45214 ("gpio / ACPI: Add support for retrieving GpioInt resources from a device") i2c_core already set the IRQ by looking into the ACPI tree and retrieving the gpioInt. So we just have some boiler-plate here that is not needed anymore. The only downside effect here is that now we are not exiting early enough if the irq is set to -EPROBE_DEFER or any other error, but this is going to be fixed in the following patch. Signed-off-by: David Arcari <darcari@redhat.com> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- drivers/hid/i2c-hid/i2c-hid.c | 71 +++++++++++-------------------------------- 1 file changed, 18 insertions(+), 53 deletions(-) diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c index b3ec4f2..4cd606c 100644 --- a/drivers/hid/i2c-hid/i2c-hid.c +++ b/drivers/hid/i2c-hid/i2c-hid.c @@ -37,7 +37,6 @@ #include <linux/mutex.h> #include <linux/acpi.h> #include <linux/of.h> -#include <linux/gpio/consumer.h> #include <linux/i2c/i2c-hid.h> @@ -145,8 +144,6 @@ 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; @@ -808,16 +805,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", ihid->irq); + dev_dbg(&client->dev, "Requesting IRQ: %d\n", client->irq); - ret = request_threaded_irq(ihid->irq, NULL, i2c_hid_irq, + ret = request_threaded_irq(client->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, ihid->irq, ret); + client->name, client->irq, ret); return ret; } @@ -864,14 +861,6 @@ 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) { @@ -882,7 +871,6 @@ static int i2c_hid_acpi_pdata(struct i2c_client *client, union acpi_object *obj; struct acpi_device *adev; acpi_handle handle; - int ret; handle = ACPI_HANDLE(&client->dev); if (!handle || acpi_bus_get_device(handle, &adev)) @@ -898,9 +886,7 @@ static int i2c_hid_acpi_pdata(struct i2c_client *client, pdata->hid_descriptor_address = obj->integer.value; ACPI_FREE(obj); - /* GPIOs are optional */ - ret = acpi_dev_add_driver_gpios(adev, i2c_hid_acpi_gpios); - return ret < 0 && ret != -ENXIO ? ret : 0; + return 0; } static const struct acpi_device_id i2c_hid_acpi_match[] = { @@ -964,6 +950,12 @@ 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; @@ -983,23 +975,6 @@ static int i2c_hid_probe(struct i2c_client *client, ihid->pdata = *platform_data; } - if (client->irq > 0) { - ihid->irq = client->irq; - } else if (ACPI_COMPANION(&client->dev)) { - ihid->desc = gpiod_get(&client->dev, NULL, GPIOD_IN); - if (IS_ERR(ihid->desc)) { - dev_err(&client->dev, "Failed to get GPIO interrupt\n"); - return PTR_ERR(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; @@ -1064,16 +1039,13 @@ err_mem_free: hid_destroy_device(hid); err_irq: - free_irq(ihid->irq, ihid); + free_irq(client->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; @@ -1092,18 +1064,13 @@ static int i2c_hid_remove(struct i2c_client *client) hid = ihid->hid; hid_destroy_device(hid); - free_irq(ihid->irq, ihid); + free_irq(client->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; } @@ -1142,11 +1109,11 @@ static int i2c_hid_suspend(struct device *dev) /* Save some power */ i2c_hid_set_power(client, I2C_HID_PWR_SLEEP); - disable_irq(ihid->irq); + disable_irq(client->irq); } if (device_may_wakeup(&client->dev)) { - wake_status = enable_irq_wake(ihid->irq); + wake_status = enable_irq_wake(client->irq); if (!wake_status) ihid->irq_wake_enabled = true; else @@ -1166,7 +1133,7 @@ static int i2c_hid_resume(struct device *dev) int wake_status; if (device_may_wakeup(&client->dev) && ihid->irq_wake_enabled) { - wake_status = disable_irq_wake(ihid->irq); + wake_status = disable_irq_wake(client->irq); if (!wake_status) ihid->irq_wake_enabled = false; else @@ -1179,7 +1146,7 @@ static int i2c_hid_resume(struct device *dev) pm_runtime_set_active(dev); pm_runtime_enable(dev); - enable_irq(ihid->irq); + enable_irq(client->irq); ret = i2c_hid_hwreset(client); if (ret) return ret; @@ -1197,19 +1164,17 @@ 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(ihid->irq); + disable_irq(client->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(ihid->irq); + enable_irq(client->irq); i2c_hid_set_power(client, I2C_HID_PWR_ON); return 0; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] Revert "HID: i2c-hid: Add support for ACPI GPIO interrupts" 2016-10-13 9:30 ` [PATCH 1/2] Revert "HID: i2c-hid: Add support for ACPI GPIO interrupts" Benjamin Tissoires @ 2016-10-13 10:24 ` Mika Westerberg 0 siblings, 0 replies; 8+ messages in thread From: Mika Westerberg @ 2016-10-13 10:24 UTC (permalink / raw) To: Benjamin Tissoires; +Cc: Jiri Kosina, David Arcari, linux-input, linux-kernel On Thu, Oct 13, 2016 at 11:30:44AM +0200, Benjamin Tissoires wrote: > From: David Arcari <darcari@redhat.com> > > This reverts commit a485923efbb8 ("HID: i2c-hid: Add support for ACPI > GPIO interrupts") and commit a7d2bf25a483 ("HID: i2c-hid: Do not fail > probing if gpiolib is not enabled") at the same time. > > Since commit c884fbd45214 ("gpio / ACPI: Add support for retrieving > GpioInt resources from a device") i2c_core already set the IRQ by > looking into the ACPI tree and retrieving the gpioInt. So we just > have some boiler-plate here that is not needed anymore. > > The only downside effect here is that now we are not exiting early > enough if the irq is set to -EPROBE_DEFER or any other error, but > this is going to be fixed in the following patch. > > Signed-off-by: David Arcari <darcari@redhat.com> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> I went through my collection of ACPI dumps from different machines and I did not find anything using plain GpioIo() resource. So I think this should be safe thing to do. Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] HID: i2c-hid: exit if the IRQ is not valid 2016-10-13 9:30 [PATCH 0/2] Tiny modification of i2c-hid Benjamin Tissoires 2016-10-13 9:30 ` [PATCH 1/2] Revert "HID: i2c-hid: Add support for ACPI GPIO interrupts" Benjamin Tissoires @ 2016-10-13 9:30 ` Benjamin Tissoires 2016-10-13 10:25 ` Mika Westerberg 2016-10-14 13:50 ` [PATCH 0/2] Tiny modification of i2c-hid Jiri Kosina 2016-10-14 14:02 ` Jiri Kosina 3 siblings, 1 reply; 8+ messages in thread From: Benjamin Tissoires @ 2016-10-13 9:30 UTC (permalink / raw) To: Jiri Kosina, Mika Westerberg; +Cc: David Arcari, linux-input, linux-kernel From: David Arcari <darcari@redhat.com> When i2c-core doesn't find the IRQ associated to the GPIO because the gpiochip is not available, it assigns -EPROBE_DEFER to the irq. We need to bail out there and on any other error in an IRQ. Signed-off-by: David Arcari <darcari@redhat.com> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- drivers/hid/i2c-hid/i2c-hid.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c index 4cd606c..fe6b4e0 100644 --- a/drivers/hid/i2c-hid/i2c-hid.c +++ b/drivers/hid/i2c-hid/i2c-hid.c @@ -956,6 +956,13 @@ static int i2c_hid_probe(struct i2c_client *client, return -EINVAL; } + if (client->irq < 0) { + if (client->irq != -EPROBE_DEFER) + dev_err(&client->dev, + "HID over i2c doesn't have a valid IRQ\n"); + return client->irq; + } + ihid = kzalloc(sizeof(struct i2c_hid), GFP_KERNEL); if (!ihid) return -ENOMEM; -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] HID: i2c-hid: exit if the IRQ is not valid 2016-10-13 9:30 ` [PATCH 2/2] HID: i2c-hid: exit if the IRQ is not valid Benjamin Tissoires @ 2016-10-13 10:25 ` Mika Westerberg 0 siblings, 0 replies; 8+ messages in thread From: Mika Westerberg @ 2016-10-13 10:25 UTC (permalink / raw) To: Benjamin Tissoires; +Cc: Jiri Kosina, David Arcari, linux-input, linux-kernel On Thu, Oct 13, 2016 at 11:30:45AM +0200, Benjamin Tissoires wrote: > From: David Arcari <darcari@redhat.com> > > When i2c-core doesn't find the IRQ associated to the GPIO because > the gpiochip is not available, it assigns -EPROBE_DEFER to the irq. > We need to bail out there and on any other error in an IRQ. > > Signed-off-by: David Arcari <darcari@redhat.com> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] Tiny modification of i2c-hid 2016-10-13 9:30 [PATCH 0/2] Tiny modification of i2c-hid Benjamin Tissoires 2016-10-13 9:30 ` [PATCH 1/2] Revert "HID: i2c-hid: Add support for ACPI GPIO interrupts" Benjamin Tissoires 2016-10-13 9:30 ` [PATCH 2/2] HID: i2c-hid: exit if the IRQ is not valid Benjamin Tissoires @ 2016-10-14 13:50 ` Jiri Kosina 2016-10-14 13:55 ` Benjamin Tissoires 2016-10-14 14:02 ` Jiri Kosina 3 siblings, 1 reply; 8+ messages in thread From: Jiri Kosina @ 2016-10-14 13:50 UTC (permalink / raw) To: Benjamin Tissoires Cc: Mika Westerberg, David Arcari, linux-input, linux-kernel On Thu, 13 Oct 2016, Benjamin Tissoires wrote: > Hi Jiri, > > David and I are facing an issue in RHEL with the HP Zbook 15 Studio mWS. > This laptops uses the pinctrl-sunrisepoint controller for the GPIOs and > it failed on RHEL. We found out what the issue was, but in the meantime > realized that part of the code we have in i2c-hid is not required anymore. > > The actual issue is fixed here: https://lkml.org/lkml/2016/10/12/493 > but it would be more convenient (for us) and cleaner (fo everybody) to just > remove the extra boiler-plate in i2c-hid and let i2c-core handling the > attributions of the IRQ. I'd like things like this to go in only during mergw window. Is there any principal reason why this should go in still for 4.9? Thanks, -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] Tiny modification of i2c-hid 2016-10-14 13:50 ` [PATCH 0/2] Tiny modification of i2c-hid Jiri Kosina @ 2016-10-14 13:55 ` Benjamin Tissoires 0 siblings, 0 replies; 8+ messages in thread From: Benjamin Tissoires @ 2016-10-14 13:55 UTC (permalink / raw) To: Jiri Kosina; +Cc: Mika Westerberg, David Arcari, linux-input, linux-kernel On Oct 14 2016 or thereabouts, Jiri Kosina wrote: > On Thu, 13 Oct 2016, Benjamin Tissoires wrote: > > > Hi Jiri, > > > > David and I are facing an issue in RHEL with the HP Zbook 15 Studio mWS. > > This laptops uses the pinctrl-sunrisepoint controller for the GPIOs and > > it failed on RHEL. We found out what the issue was, but in the meantime > > realized that part of the code we have in i2c-hid is not required anymore. > > > > The actual issue is fixed here: https://lkml.org/lkml/2016/10/12/493 > > but it would be more convenient (for us) and cleaner (fo everybody) to just > > remove the extra boiler-plate in i2c-hid and let i2c-core handling the > > attributions of the IRQ. > > I'd like things like this to go in only during mergw window. Is there any > principal reason why this should go in still for 4.9? There is no particular rush from our side. As long as you take it in your tree and we know it will be schedule for 4.10, that should be OK. So do as you think is the best :) Cheers, Benjamin ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] Tiny modification of i2c-hid 2016-10-13 9:30 [PATCH 0/2] Tiny modification of i2c-hid Benjamin Tissoires ` (2 preceding siblings ...) 2016-10-14 13:50 ` [PATCH 0/2] Tiny modification of i2c-hid Jiri Kosina @ 2016-10-14 14:02 ` Jiri Kosina 3 siblings, 0 replies; 8+ messages in thread From: Jiri Kosina @ 2016-10-14 14:02 UTC (permalink / raw) To: Benjamin Tissoires Cc: Mika Westerberg, David Arcari, linux-input, linux-kernel On Thu, 13 Oct 2016, Benjamin Tissoires wrote: > Hi Jiri, > > David and I are facing an issue in RHEL with the HP Zbook 15 Studio mWS. > This laptops uses the pinctrl-sunrisepoint controller for the GPIOs and > it failed on RHEL. We found out what the issue was, but in the meantime > realized that part of the code we have in i2c-hid is not required anymore. > > The actual issue is fixed here: https://lkml.org/lkml/2016/10/12/493 > but it would be more convenient (for us) and cleaner (fo everybody) to just > remove the extra boiler-plate in i2c-hid and let i2c-core handling the > attributions of the IRQ. Both applied to hid.git#for-4.10/i2c-hid. -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-10-14 14:02 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-13 9:30 [PATCH 0/2] Tiny modification of i2c-hid Benjamin Tissoires 2016-10-13 9:30 ` [PATCH 1/2] Revert "HID: i2c-hid: Add support for ACPI GPIO interrupts" Benjamin Tissoires 2016-10-13 10:24 ` Mika Westerberg 2016-10-13 9:30 ` [PATCH 2/2] HID: i2c-hid: exit if the IRQ is not valid Benjamin Tissoires 2016-10-13 10:25 ` Mika Westerberg 2016-10-14 13:50 ` [PATCH 0/2] Tiny modification of i2c-hid Jiri Kosina 2016-10-14 13:55 ` Benjamin Tissoires 2016-10-14 14:02 ` Jiri Kosina
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.