On Mon, Aug 01, 2016 at 03:43:32PM +0200, Hans Verkuil wrote: > > > On 07/15/2016 06:31 PM, Dmitry Torokhov wrote: > > Hi Hans, > > > > On Fri, Jul 15, 2016 at 01:27:21PM +0200, Hans Verkuil wrote: > >> For the upcoming 4.8 kernel I made a driver for the Pulse-Eight USB CEC adapter. > >> This is a usb device that shows up as a ttyACM0 device. It requires that you run > >> inputattach in order to communicate with it via serio. > >> > >> This all works well, but it would be nice to have a udev rule to automatically > >> start inputattach. That too works OK, but the problem comes when the USB device > >> is unplugged: the tty hangup is never handled by the serio framework so the > >> inputattach utility never exits and you have to kill it manually. > >> > >> By adding this hangup callback the inputattach utility now exists as soon as I > >> unplug the USB device. > >> > >> Is this the correct approach? > >> > >> BTW, the new driver is found here: > >> > >> https://git.linuxtv.org/media_tree.git/tree/drivers/staging/media/pulse8-cec > >> > >> Regards, > >> > >> Hans > >> > >> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx> > >> > >> --- > >> diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c > >> index 9c927d3..a615846 100644 > >> --- a/drivers/input/serio/serport.c > >> +++ b/drivers/input/serio/serport.c > >> @@ -248,6 +248,14 @@ static long serport_ldisc_compat_ioctl(struct tty_struct *tty, > >> } > >> #endif > >> > >> +static int serport_ldisc_hangup(struct tty_struct * tty) > >> +{ > >> + struct serport *serport = (struct serport *) tty->disc_data; > >> + > >> + serport_serio_close(serport->serio); > > > > I see what you mean, but this is not quite correct. I think we should > > make serport_serio_close() only reset the SERPORT_ACTIVE flag and have > > serport_ldisc_hangup() actually do: > > > > spin_lock_irqsave(&serport->lock, flags); > > set_bit(SERPORT_DEAD, &serport->flags); > > spin_unlock_irqrestore(&serport->lock, flags); > > > > wake_up_interruptible(&serport->wait); > > I'm preparing a v2 of this patch, but I wonder if in this hangup code > I also need to clear the SERPORT_ACTIVE flag. Or is it guaranteed that > close() always precedes hangup()? In which case close() always clears that > flag. No, we could get hangup and that would wake up the reader which will cause de-registration of serio port. As part of that process serport_serio_close() will be called (if serio port has been opened by someone). You should not need to clear "active" flag in hangup code. Thanks. -- Dmitry -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
