Skip to contentSkip to conversation

IR driver for USB-UIRT device

PATCH Superseded Sean Young 2 participants 6 messages May 5, 2021 – May 6, 2021

Subsystems (4)
Media (V4L/DVB) USB USB serial RC-core / lirc framework

v2 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Sean Young

author maintainer
#0 lore raw

v2 cover IR driver for USB-UIRT device

This is a new rc-core driver for the USB-UIRT which you can see here
http://www.usbuirt.com/

This device is supported in lirc, via the usb serial kernel driver. This driver is both for rc-core, which means it can use kernel/BPF decoding ec. Also this implement is superior because it can:
- support learning mode
- setting transmit carrier
- larger transmits using streaming tx command

Changes since v1:
- Review comments from Oliver Neukum
- Simplified wideband read function

Sean Young (3):
USB: serial: move ftdi_sio.h into include directories
media: rc: new driver for USB-UIRT device
USB: serial: blacklist USB-UIRT when driver is selected

 drivers/media/rc/Kconfig                      |  11 +
 drivers/media/rc/Makefile                     |   1 +
 drivers/media/rc/uirt.c                       | 727 ++++++++++++++++++
 drivers/usb/serial/ftdi_sio.c                 |  10 +-
 .../serial => include/linux/usb}/ftdi_sio.h   |   0
 5 files changed, 748 insertions(+), 1 deletion(-)
 create mode 100644 drivers/media/rc/uirt.c
 rename {drivers/usb/serial => include/linux/usb}/ftdi_sio.h (100%)
Signature · 1 line
2.31.1
author maintainer
#1 lore raw

PATCH v2 1/3 USB: serial: move ftdi_sio.h into include directories

A following commit adds a new IR driver uirt which needs the ftdi defines; the hardware uses an fdti usb serial port.

Signed-off-by Sean Young <[email protected]>
---
 drivers/usb/serial/ftdi_sio.c                        | 2 +-
 {drivers/usb/serial => include/linux/usb}/ftdi_sio.h | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename {drivers/usb/serial => include/linux/usb}/ftdi_sio.h (100%)
drivers/usb/serial/ftdi_sio.c +1 -1
index c867592477c9..542073d2f0dd 100644
@@ -41,7 +41,7 @@
 #include <linux/serial.h>
 #include <linux/gpio/driver.h>
 #include <linux/usb/serial.h>
-#include "ftdi_sio.h"
+#include <linux/usb/ftdi_sio.h>
 #include "ftdi_sio_ids.h"
 
 #define DRIVER_AUTHOR "Greg Kroah-Hartman <[email protected]>, Bill Ryder <[email protected]>, Kuba Ober <[email protected]>, Andreas Mohr, Johan Hovold <[email protected]>"
include/linux/usb/ftdi_sio.h renamed from drivers/usb/serial/ftdi_sio.h +0 -0
similarity index 100%
rename from drivers/usb/serial/ftdi_sio.h
rename to include/linux/usb/ftdi_sio.h
Signature · 1 line
2.31.1
author maintainer
#2 lore raw

PATCH v2 2/3 media: rc: new driver for USB-UIRT device

In reply to Sean Young

See http://www.usbuirt.com/

Signed-off-by Sean Young <[email protected]>
---
 drivers/media/rc/Kconfig  |  11 +
 drivers/media/rc/Makefile |   1 +
 drivers/media/rc/uirt.c   | 727 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 739 insertions(+)
 create mode 100644 drivers/media/rc/uirt.c
drivers/media/rc/Kconfig +11 -0
index f016b35c2b17..dff85a5bfd1e 100644
@@ -532,6 +532,17 @@ config IR_TOY
 	   To compile this driver as a module, choose M here: the module will be
 	   called ir_toy.
 
+config IR_UIRT
+	tristate "USB-UIRT"
+	depends on RC_CORE
+	depends on USB_ARCH_HAS_HCD
+	help
+	   Say Y here if you want to use the USB-UIRT. See
+	   http://www.usbuirt.com/
+
+	   To compile this driver as a module, choose M here: the module will be
+	   called uirt.
+
 endif #RC_DEVICES
 
 endif #RC_CORE
drivers/media/rc/Makefile +1 -0
index 4927f585ebc2..17037915e742 100644
@@ -50,3 +50,4 @@ obj-$(CONFIG_IR_MTK) += mtk-cir.o
 obj-$(CONFIG_IR_TANGO) += tango-ir.o
 obj-$(CONFIG_RC_XBOX_DVD) += xbox_remote.o
 obj-$(CONFIG_IR_TOY) += ir_toy.o
+obj-$(CONFIG_IR_UIRT) += uirt.o
drivers/media/rc/uirt.c new file +727 -0

727 lines not on this page · raw message

Signature · 1 line
2.31.1
#3 lore raw

Re: [PATCH v2 2/3] media: rc: new driver for USB-UIRT device

Am Mittwoch, den 05.05.2021, 22:57 +0100 schrieb Sean Young:

Quoted 4 lines
drivers/media/rc/uirt.c new file +4 -0
+static void uirt_disconnect(struct usb_interface *intf)
+{
+       struct uirt *ir = usb_get_intfdata(intf);
+

drivers/media/rc/uirt.c line 708

Quoted 10 lines
drivers/media/rc/uirt.c new file +10 -0
+       rc_unregister_device(ir->rc);
+       usb_set_intfdata(intf, NULL);
+       usb_kill_urb(ir->urb_out);
+       usb_free_urb(ir->urb_out);
+       usb_kill_urb(ir->urb_in);
+       usb_free_urb(ir->urb_in);
+       kfree(ir->in);
+       kfree(ir->out);
+       kfree(ir);
+}

Hi,

almost. Going through this again, it looks like you have a race condition here.

CPU A CPU B

usb_kill_urb(ir->urb_out);
usb_free_urb(ir->urb_out);

uirt_in_callback() uirt_response(struct uirt *uirt, u32 len) err = usb_submit_urb(uirt->urb_out, GFP_ATOMIC);

BANG, you are using freeed memory

Regards
Oliver

author maintainer
#4 lore raw

Re: [PATCH v2 2/3] media: rc: new driver for USB-UIRT device

Hi Oliver,

Quoted 23 lines · Oliver Neukum

Am Mittwoch, den 05.05.2021, 22:57 +0100 schrieb Sean Young:

Quoted 4 lines
drivers/media/rc/uirt.c new file +4 -0
+static void uirt_disconnect(struct usb_interface *intf)
+{
+       struct uirt *ir = usb_get_intfdata(intf);
+

drivers/media/rc/uirt.c line 708

Quoted 10 lines
drivers/media/rc/uirt.c new file +10 -0
+       rc_unregister_device(ir->rc);
+       usb_set_intfdata(intf, NULL);
+       usb_kill_urb(ir->urb_out);
+       usb_free_urb(ir->urb_out);
+       usb_kill_urb(ir->urb_in);
+       usb_free_urb(ir->urb_in);
+       kfree(ir->in);
+       kfree(ir->out);
+       kfree(ir);
+}

Hi,

almost. Going through this again, it looks like you have a race condition here.

CPU A CPU B

Quoted 8 lines

usb_kill_urb(ir->urb_out);
usb_free_urb(ir->urb_out);

uirt_in_callback() uirt_response(struct uirt *uirt, u32 len) err = usb_submit_urb(uirt->urb_out, GFP_ATOMIC);

BANG, you are using freeed memory

Nice catch, I had missed this. I'll send out a v3 with this fixed.

This needs fixing, for sure. Having said that, once we are in the disconnect handler aren't all urbs/endpoints disabled in usb_disable_interface()? Does this mean the usb_kill_urb() are superfluous?

Thanks,

Sean

author maintainer
#5 lore raw

PATCH v2 3/3 USB: serial: blacklist USB-UIRT when driver is selected

In reply to Sean Young

The USB-UIRT device has its own driver, so blacklist the fdti driver from using it if the driver has been enabled.

Signed-off-by Sean Young <[email protected]>
---
 drivers/usb/serial/ftdi_sio.c | 8 ++++++++
 1 file changed, 8 insertions(+)
drivers/usb/serial/ftdi_sio.c +8 -0
index 542073d2f0dd..2320bda57796 100644
@@ -95,7 +95,9 @@ static int   ftdi_jtag_probe(struct usb_serial *serial);
 static int   ftdi_NDI_device_setup(struct usb_serial *serial);
 static int   ftdi_stmclite_probe(struct usb_serial *serial);
 static int   ftdi_8u2232c_probe(struct usb_serial *serial);
+#if !IS_ENABLED(CONFIG_IR_UIRT)
 static void  ftdi_USB_UIRT_setup(struct ftdi_private *priv);
+#endif
 static void  ftdi_HE_TIRA1_setup(struct ftdi_private *priv);
 
 static const struct ftdi_sio_quirk ftdi_jtag_quirk = {

@@ -106,9 +108,11 @@ static const struct ftdi_sio_quirk ftdi_NDI_device_quirk = {
 	.probe	= ftdi_NDI_device_setup,
 };
 
+#if !IS_ENABLED(CONFIG_IR_UIRT)
 static const struct ftdi_sio_quirk ftdi_USB_UIRT_quirk = {
 	.port_probe = ftdi_USB_UIRT_setup,
 };
+#endif
 
 static const struct ftdi_sio_quirk ftdi_HE_TIRA1_quirk = {
 	.port_probe = ftdi_HE_TIRA1_setup,

@@ -568,8 +572,10 @@ static const struct usb_device_id id_table_combined[] = {
 	{ USB_DEVICE(OCT_VID, OCT_DK201_PID) },
 	{ USB_DEVICE(FTDI_VID, FTDI_HE_TIRA1_PID),
 		.driver_info = (kernel_ulong_t)&ftdi_HE_TIRA1_quirk },
+#if !IS_ENABLED(CONFIG_IR_UIRT)
 	{ USB_DEVICE(FTDI_VID, FTDI_USB_UIRT_PID),
 		.driver_info = (kernel_ulong_t)&ftdi_USB_UIRT_quirk },
+#endif
 	{ USB_DEVICE(FTDI_VID, PROTEGO_SPECIAL_1) },
 	{ USB_DEVICE(FTDI_VID, PROTEGO_R2X0) },
 	{ USB_DEVICE(FTDI_VID, PROTEGO_SPECIAL_3) },

@@ -2292,6 +2298,7 @@ static int ftdi_sio_port_probe(struct usb_serial_port *port)
 	return 0;
 }
 
+#if !IS_ENABLED(CONFIG_IR_UIRT)
 /* Setup for the USB-UIRT device, which requires hardwired
  * baudrate (38400 gets mapped to 312500) */
 /* Called from usbserial:serial_probe */

@@ -2301,6 +2308,7 @@ static void ftdi_USB_UIRT_setup(struct ftdi_private *priv)
 	priv->custom_divisor = 77;
 	priv->force_baud = 38400;
 }
+#endif
 
 /* Setup for the HE-TIRA1 device, which requires hardwired
  * baudrate (38400 gets mapped to 100000) and RTS-CTS enabled.  */
Signature · 1 line
2.31.1