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 <[email protected]>
---
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(-)
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
--
2.34.1
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 <[email protected]>
> ---
> 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,
--
Jiri Kosina
SUSE Labs
On Thu, Aug 25, 2022 at 11:26 AM Jiri Kosina <[email protected]> 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 <[email protected]>
> > ---
> > 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
Le 30/08/2022 à 13:56, Benjamin Tissoires a écrit :
> On Thu, Aug 25, 2022 at 11:26 AM Jiri Kosina <[email protected]> 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 <[email protected]>
>>> ---
>>> 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