mbox series

[v4,0/4] DeviceTree Support for USB-HID Devices and CP2112

Message ID 20230206135016.6737-1-kaehndan@gmail.com
Headers show
Series DeviceTree Support for USB-HID Devices and CP2112 | expand

Message

Daniel Kaehn Feb. 6, 2023, 1:50 p.m. UTC
This patchset allows USB-HID devices to have DeviceTree bindings through sharing
the USB of_node with the HID driver, and adds such a binding and driver
implementation for the CP2112 USB to SMBus Bridge (which necessitated the
USB-HID change). This change allows a CP2112 permanently attached in hardware to
be described in DT and interoperate with other drivers, and exposed the threaded
interrupt bug fixed in patch 0003.

Changes in v4:
 - Moved silabs,cp2112.yaml to /Documentation/devicetree/bindings/i2c

Changes in v3:
 - Additional fixups to silabs,cp2112.yaml to address comments

Changes in v2:
 - Added more detail to silabs,cp2112.yaml dt-binding
 - Moved silabs,cp2112.yaml to /Documentation/devicetree/bindings/input
 - Added support for setting smbus clock-frequency from DT in hid-cp2112.c
 - Added freeing of of_nodes on error paths of _probe in hid-cp2112.c

Danny Kaehn (4):
  dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge
  HID: usbhid: Share USB device devicetree node with child HID device
  HID: cp2112: Fix driver not registering GPIO IRQ chip as threaded
  HID: cp2112: Devicetree Support

 .../bindings/i2c/silabs,cp2112.yaml           | 112 ++++++++++++++++++
 drivers/hid/hid-cp2112.c                      |  23 +++-
 drivers/hid/usbhid/hid-core.c                 |   2 +
 3 files changed, 135 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml

Comments

Dmitry Torokhov Feb. 6, 2023, 11:15 p.m. UTC | #1
On Mon, Feb 06, 2023 at 07:50:15AM -0600, Danny Kaehn wrote:
> The CP2112 generates interrupts from a polling routine on a thread,
> and can only support threaded interrupts. This patch configures the
> gpiochip irq chip with this flag, disallowing consumers to request
> a hard IRQ from this driver, which resulted in a segfault previously.

This looks like a bugfix not dependent on anything else in the series
and can be applied separately...

> 
> Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
> ---
>  drivers/hid/hid-cp2112.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
> index 1e16b0fa310d..27cadadda7c9 100644
> --- a/drivers/hid/hid-cp2112.c
> +++ b/drivers/hid/hid-cp2112.c
> @@ -1354,6 +1354,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	girq->parents = NULL;
>  	girq->default_type = IRQ_TYPE_NONE;
>  	girq->handler = handle_simple_irq;
> +	girq->threaded = true;
>  
>  	ret = gpiochip_add_data(&dev->gc, dev);
>  	if (ret < 0) {
> -- 
> 2.25.1
>
Daniel Kaehn Feb. 7, 2023, 12:34 p.m. UTC | #2
On Mon, Feb 6, 2023 at 5:15 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> On Mon, Feb 06, 2023 at 07:50:15AM -0600, Danny Kaehn wrote:
> > The CP2112 generates interrupts from a polling routine on a thread,
> > and can only support threaded interrupts. This patch configures the
> > gpiochip irq chip with this flag, disallowing consumers to request
> > a hard IRQ from this driver, which resulted in a segfault previously.
>
> This looks like a bugfix not dependent on anything else in the series
> and can be applied separately...

This is correct (though usage of this patchset to instantiate drivers
which request interrupts will most of the time be broken without this
patch). Does this mean I should submit this patch independently from
the rest of the series? Or should I just include a message to the
maintainer describing what you said (that this can be applied
separately)?

Thanks,

Danny Kaehn

>
> >
> > Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
> > ---
> >  drivers/hid/hid-cp2112.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
> > index 1e16b0fa310d..27cadadda7c9 100644
> > --- a/drivers/hid/hid-cp2112.c
> > +++ b/drivers/hid/hid-cp2112.c
> > @@ -1354,6 +1354,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
> >       girq->parents = NULL;
> >       girq->default_type = IRQ_TYPE_NONE;
> >       girq->handler = handle_simple_irq;
> > +     girq->threaded = true;
> >
> >       ret = gpiochip_add_data(&dev->gc, dev);
> >       if (ret < 0) {
> > --
> > 2.25.1
> >
>
> --
> Dmitry