Re: [PATCH] serdev: BREAK/FRAME/PARITY/OVERRUN notification prototype V2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Dec 30, 2021 at 01:30:18PM +0100, Greg Kroah-Hartman wrote:
> On Sun, Dec 12, 2021 at 10:21:28PM +0900, Magnus Damm wrote:
> > From: Magnus Damm <damm+renesas@xxxxxxxxxxxxx>
> > 
> > Allow serdev device drivers get notified by hardware errors such as BREAK,
> > FRAME, PARITY and OVERRUN.
> > 
> > With this patch, in the event of an error detected in the UART device driver
> > the serdev_device_driver will get the newly introduced ->error() callback
> > invoked if serdev_device_set_error_mask() has previously been used to enable
> > the type of error. The errors are taken straight from the TTY layer and fed
> > into the serdev_device_driver after filtering out only enabled errors.
> > 
> > Without this patch the hardware errors never reach the serdev_device_driver.
> > 
> > Signed-off-by: Magnus Damm <damm+renesas@xxxxxxxxxxxxx>
> > ---
> > 
> >  Applies to linux-5.16-rc4
> > 
> >  Change since V1:
> >  - Use __set_bit() instead of set_bit() in ttyport_receive_buf()
> >  - Switch to assign_bit() in ttyport_set_error_mask()
> > 
> >  Thanks to Geert for feedback!
> > 
> >  The following prototype patch is using serdev error notifications:
> >  [PATCH] r8a77995 Draak SCIF0 LED and KEY Serdev prototype V2
> 
> Looks good, now applied to my tty tree.

I really don't think this is ready to be merged. There's been no
discussion about the design of this interface and importantly there are
no users (there was an RFC floating around but that one too has issues).

Some of the problems with this patch include:

 - performance penalty for all serdev drivers due to unconditional per
   character processing
 - flagged characters are still being forwarded to the consumer (e.g.
   NUL chars inserted on breaks)
 - it only works with some broken serial drivers which do not honour
   TTY_DRIVER_REAL_RAW
 - interface basically limited to the hacky led/input driver mentioned
   above since it does not match flags with characters

I suggest reverting for now.

Johan



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux