Re: [PATCH] Clearing FIFOs in RS485 emulation mode causes subsequent transmits to break

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

 



(Third time's the charm... Apologies for flooding)

On Sun, Dec 11, 2016 at 9:52 PM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Sun, Dec 11, 2016 at 08:57:35PM +1100, Daniel Jędrychowski wrote:
> > When in RS485 emulation mode, __do_stop_tx_rs485() calls serial8250_clear_fifos
> > ().
> > This not only clears the FIFOs, but also sets all bits in their control
> > register (UART_FCR) to 0.
> >
> > One of the effects of this is the disabling of the FIFOs, which turns them into
> > single-byte holding registers.
> > The rest of the driver doesn't know this, which results in the lions share of
> > characters passed into a write call to be dropped.
> >
> > (I can supply logic analyzer screenshots if necessary)
> >
> > This fix replaces the serial8250_clear_fifos() call to
> > serial8250_clear_and_reinit_fifos() - this prevents the "dropped characters"
> > issue from manifesting again while retaining the requirement of clearing the RX
> > FIFO after transmission if the SER_RS485_RX_DURING_TX flag is disabled.
>
> Odd, why has no one else seen this before?  Any idea how it could have
> ever worked properly in the past?  Has anything changed in this area
> recently?
>

I can only speak for myself - I encountered this while getting the
Beaglebone's serial ports to use the emulated 485 mode for work.

Given the BB's popularity, I'm surprised no one encountered this bug
as well. I *did* do a lot of digging to find a fix before I spent a
week poking around in the kernel source, but came up empty. Let's hope
people on the list can shed some light on this when I resubmit this.

> > (P.S. This is the first time submitting a patch for me. Apologies if I missed
> > something in the formatting or if the description is unclear)
>
> No need for this "P.S." in the changelog, you can put that below the ---
> line if you want to make comments that do not show up in the commit.
>

Noted, thanks.

> >
> > Signed-off-by: Daniel Jedrychowski <avistel@xxxxxxxxx>
> > ---
> >  drivers/tty/serial/8250/8250_port.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/
> > 8250_port.c
> > index 1731b98..080d5a5 100644
> > --- a/drivers/tty/serial/8250/8250_port.c
> > +++ b/drivers/tty/serial/8250/8250_port.c
> > @@ -1411,7 +1411,7 @@ static void __do_stop_tx_rs485(struct uart_8250_port *p)
> >       * Enable previously disabled RX interrupts.
> >       */
> >      if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX)) {
> > -        serial8250_clear_fifos(p);
> > +        serial8250_clear_and_reinit_fifos(p);
> >
> >          p->ier |= UART_IER_RLSI | UART_IER_RDI;
> >          serial_port_out(&p->port, UART_IER, p->ier);
>
> Hm, something went wrong with your patch and the tabs got turned into
> spaces so it can't be applied :(
>
> Can you try not using the web interface for gmail?  It's known to be
> broken for patches (see Documentation/email_clients.txt for details.)
>
> thanks,
>
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[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