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

Message ID cover.1624006513.git.sean@mess.org (mailing list archive)
Headers
Series IR driver for USB-UIRT device |

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