mbox series

[v5,0/2] IR driver for USB-UIRT device

Message ID cover.1624006513.git.sean@mess.org
Headers show
Series IR driver for USB-UIRT device | expand

Message

Sean Young June 18, 2021, 10:18 a.m. UTC
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
 - Much better latency since it is a kernel driver

Changes since v4:
 - Fixed clang warning/uninitialized variable usage
 - Some cosmetic changes

Changes since v3:
 - Review comments from Johan Hovold
 - Do not move the ftdi_sio.h file an copy FTDI_* definitions instead

Changes since v2:
 - Fixed race condition is disconnect
 - Removed superfluous kmalloc in short tx

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

Sean Young (2):
  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       | 744 ++++++++++++++++++++++++++++++++++
 drivers/usb/serial/ftdi_sio.c |   6 +-
 4 files changed, 760 insertions(+), 2 deletions(-)
 create mode 100644 drivers/media/rc/uirt.c

Comments

Johan Hovold July 2, 2021, 10:01 a.m. UTC | #1
On Fri, Jun 18, 2021 at 11:18:45AM +0100, Sean Young wrote:
> 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

>  - Much better latency since it is a kernel driver


A ball-park number to back this claim up would be good here. Your driver
sets the ftdi latency timer to 50 ms which adds quite a bit of latency
for short packets (e.g. a single key press?) to begin with.

> Changes since v4:

>  - Fixed clang warning/uninitialized variable usage

>  - Some cosmetic changes

> 

> Changes since v3:

>  - Review comments from Johan Hovold


In the future, please include some details on what changed also when
addressing review feedback.

>  - Do not move the ftdi_sio.h file an copy FTDI_* definitions instead

> 

> Changes since v2:

>  - Fixed race condition is disconnect

>  - Removed superfluous kmalloc in short tx

> 

> Changes since v1:

>  - Review comments from Oliver Neukum

>  - Simplified wideband read function

> 

> Sean Young (2):

>   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       | 744 ++++++++++++++++++++++++++++++++++

>  drivers/usb/serial/ftdi_sio.c |   6 +-

>  4 files changed, 760 insertions(+), 2 deletions(-)

>  create mode 100644 drivers/media/rc/uirt.c


Johan
Sean Young July 2, 2021, 12:59 p.m. UTC | #2
On Fri, Jul 02, 2021 at 12:01:59PM +0200, Johan Hovold wrote:
> On Fri, Jun 18, 2021 at 11:18:45AM +0100, Sean Young wrote:

> > 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

> >  - Much better latency since it is a kernel driver

> 

> A ball-park number to back this claim up would be good here. Your driver

> sets the ftdi latency timer to 50 ms which adds quite a bit of latency

> for short packets (e.g. a single key press?) to begin with.


Yes, I'll do some measurements. This is based on observations with other
in kernel drivers.

> > Changes since v4:

> >  - Fixed clang warning/uninitialized variable usage

> >  - Some cosmetic changes

> > 

> > Changes since v3:

> >  - Review comments from Johan Hovold

> 

> In the future, please include some details on what changed also when

> addressing review feedback.


Right, good point. Will do.

> >  - Do not move the ftdi_sio.h file an copy FTDI_* definitions instead

> > 

> > Changes since v2:

> >  - Fixed race condition is disconnect

> >  - Removed superfluous kmalloc in short tx

> > 

> > Changes since v1:

> >  - Review comments from Oliver Neukum

> >  - Simplified wideband read function

> > 

> > Sean Young (2):

> >   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       | 744 ++++++++++++++++++++++++++++++++++

> >  drivers/usb/serial/ftdi_sio.c |   6 +-

> >  4 files changed, 760 insertions(+), 2 deletions(-)

> >  create mode 100644 drivers/media/rc/uirt.c

> 

> Johan


Thanks
Sean