Re: [PATCH v5 0/2] IR driver for USB-UIRT device

From: Sean Young <sean@mess.org>
To: Johan Hovold <johan@kernel.org>
Cc: linux-media@vger.kernel.org, linux-usb@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jon Rhees <support@usbuirt.com>, Oliver Neukum <oneukum@suse.com>
Subject: Re: [PATCH v5 0/2] IR driver for USB-UIRT device
Date: Fri, 2 Jul 2021 13:59:57 +0100	[thread overview]
Message-ID: <20210702125957.GA29760@gofer.mess.org> (raw)
In-Reply-To: <YN7kF17pfhDr1ccy@hovoldconsulting.com>

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