| Message ID | 6ee2132c415db2fc90e7fa2106db427a914cc566.1655563907.git.christophe.jaillet@wanadoo.fr |
|---|---|
| State | New |
| Headers | show |
| Series | HID: cp2112: Remove some dead code | expand |
On Sat, 18 Jun 2022, Christophe JAILLET wrote: > Commit 13de9cca514e ("HID: cp2112: add IRQ chip handling") has introduced > cp2112_allocate_irq() that seems to be unused since 2016. > > Remove it, remove the associated resources and part of the remove() > function that frees the resources allocated in cp2112_allocate_irq(). > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > Compile tested only. > > Maybe the issue is completely elsewhere and the probe() should call > cp2112_allocate_irq() in some cases. Benjamin, could you please take a look? Apparently you were aware of the code being dead due to the __maybe_unused annotation ... :) What was the point? Thanks,
Le 30/08/2022 à 13:56, Benjamin Tissoires a écrit : > On Thu, Aug 25, 2022 at 11:26 AM Jiri Kosina <jikos@kernel.org> wrote: >> >> On Sat, 18 Jun 2022, Christophe JAILLET wrote: >> >>> Commit 13de9cca514e ("HID: cp2112: add IRQ chip handling") has introduced >>> cp2112_allocate_irq() that seems to be unused since 2016. >>> >>> Remove it, remove the associated resources and part of the remove() >>> function that frees the resources allocated in cp2112_allocate_irq(). >>> >>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> >>> --- >>> Compile tested only. >>> >>> Maybe the issue is completely elsewhere and the probe() should call >>> cp2112_allocate_irq() in some cases. >> >> Benjamin, could you please take a look? Apparently you were aware of the >> code being dead due to the __maybe_unused annotation ... :) What was the >> point? >> > > Looks like I kept that code around for the CI I am running on HID patches. > > IIRC, I left the code in the tree because it might have been useful to > others when they need to declare IRQs on the board. So yes, it is > entirely dead code upstream :/ > > I am applying the following 3 patches on the current master tree to be > able to declare hid-cp2112 as an i2c-hid transport: > https://gitlab.freedesktop.org/bentiss/gitlab-kernel-ci/-/tree/master/VM > > Those 3 patches can not be upstreamed because platform drivers are a > dead thing, but I have no other ideas on how I can declare an i2c-hid > device on top of hid-cp2112. Given that we don't have DT on x86_64 > vm, I can not rely on that to have my custom sensor (or maybe I can > but I am not aware of it). > > So unless anybody has a better idea, I won't fight against removing > that code, but it's more convenient for me to have it. > > Cheers, > Benjamin > This was just a clean-up patch. I personally don't really care if applied or not. So, from my POV if it helps, it can stay. On the other side, I guess that it could also easily become another patch in your serie. Do what is best. CJ
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c index 1e16b0fa310d..67a5ac6be922 100644 --- a/drivers/hid/hid-cp2112.c +++ b/drivers/hid/hid-cp2112.c @@ -167,7 +167,6 @@ struct cp2112_device { u8 *in_out_buffer; struct mutex lock; - struct gpio_desc *desc[8]; bool gpio_poll; struct delayed_work gpio_poll_worker; unsigned long irq_mask; @@ -1183,51 +1182,6 @@ static int cp2112_gpio_irq_type(struct irq_data *d, unsigned int type) return 0; } -static int __maybe_unused cp2112_allocate_irq(struct cp2112_device *dev, - int pin) -{ - int ret; - - if (dev->desc[pin]) - return -EINVAL; - - dev->desc[pin] = gpiochip_request_own_desc(&dev->gc, pin, - "HID/I2C:Event", - GPIO_ACTIVE_HIGH, - GPIOD_IN); - if (IS_ERR(dev->desc[pin])) { - dev_err(dev->gc.parent, "Failed to request GPIO\n"); - return PTR_ERR(dev->desc[pin]); - } - - ret = cp2112_gpio_direction_input(&dev->gc, pin); - if (ret < 0) { - dev_err(dev->gc.parent, "Failed to set GPIO to input dir\n"); - goto err_desc; - } - - ret = gpiochip_lock_as_irq(&dev->gc, pin); - if (ret) { - dev_err(dev->gc.parent, "Failed to lock GPIO as interrupt\n"); - goto err_desc; - } - - ret = gpiod_to_irq(dev->desc[pin]); - if (ret < 0) { - dev_err(dev->gc.parent, "Failed to translate GPIO to IRQ\n"); - goto err_lock; - } - - return ret; - -err_lock: - gpiochip_unlock_as_irq(&dev->gc, pin); -err_desc: - gpiochip_free_own_desc(dev->desc[pin]); - dev->desc[pin] = NULL; - return ret; -} - static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id) { struct cp2112_device *dev; @@ -1388,7 +1342,6 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id) static void cp2112_remove(struct hid_device *hdev) { struct cp2112_device *dev = hid_get_drvdata(hdev); - int i; sysfs_remove_group(&hdev->dev.kobj, &cp2112_attr_group); i2c_del_adapter(&dev->adap); @@ -1398,11 +1351,6 @@ static void cp2112_remove(struct hid_device *hdev) cancel_delayed_work_sync(&dev->gpio_poll_worker); } - for (i = 0; i < ARRAY_SIZE(dev->desc); i++) { - gpiochip_unlock_as_irq(&dev->gc, i); - gpiochip_free_own_desc(dev->desc[i]); - } - gpiochip_remove(&dev->gc); /* i2c_del_adapter has finished removing all i2c devices from our * adapter. Well behaved devices should no longer call our cp2112_xfer
Commit 13de9cca514e ("HID: cp2112: add IRQ chip handling") has introduced cp2112_allocate_irq() that seems to be unused since 2016. Remove it, remove the associated resources and part of the remove() function that frees the resources allocated in cp2112_allocate_irq(). Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- Compile tested only. Maybe the issue is completely elsewhere and the probe() should call cp2112_allocate_irq() in some cases. --- drivers/hid/hid-cp2112.c | 52 ---------------------------------------- 1 file changed, 52 deletions(-)