On Thu, 31 Aug 2023, Andy Shevchenko wrote: > On Thu, Aug 31, 2023 at 03:48:08PM -0400, Matthew Howell wrote: > > From: Matthew Howell <matthew.howell@xxxxxxxxxxxx > > > > Sealevel XR17V35X based cards utilize DTR to control RS-485 Enable, but > > the current implementation in 8250_exar uses RTS for the auto-RS485-Enable > > mode of the XR17V35X UARTs. This patch implements sealevel_rs485_config to > > Please, read Submitting Patches documentation, in particular find there > the paragraph that matches to "This patch". With that, amend commit message > accordingly. > > We refer to the functions as func() (note the parentheses). I did read it but it was not clear that the parentheses are part of the 'rules'. > > configure the XR17V35X for DTR control of RS485 Enable and assigns it to > > Your lines have trailing whitespaces, please get rid of them. > > > Sealevel cards in pci_sealevel_setup. > > > Fixed defines and various format issues from previous submissions. > > What does this mean? If it's a changelog, use the correct place for it > (see below). Sorry, I did not realize. I found that section of the document unclear on first pass. > > > > Link: > > https://lore.kernel.org/linux-serial/b2a721-227-14ef-75eb-36244ba2942@xxxxxxxxxxxx > > Tags must occupy a single line: a single line per each tag. > > > > > Tag block must have no blank lines. > > Most of these is described in the above mentioned documentation. Sorry, I had missed that tags are exempt from the normal character per line limit. > > Signed-off-by: Matthew Howell <matthew.howell@xxxxxxxxxxxx> > > --- > > Here you add comments and/or changelog. > > > diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c > > index 3886f78ecbbf..6b28f5a3df17 100644 > > ... > > > +#define UART_EXAR_DLD 0x02 /* Divisor Fractional */ > > +#define UART_EXAR_DLD_485_POLARITY 0x80 /* RS-485 Enable Signal Polarity */ > > Mixed TABs and spaces in a wrong order, usually we use only TABs in such cases. > > ... > > > +static int sealevel_rs485_config(struct uart_port *port, struct ktermios *termios, > > + struct serial_rs485 *rs485) > > +{ > > + u8 __iomem *p = port->membase; > > + u8 old_lcr; > > + > > + generic_rs485_config(port, termios, rs485); > > > + if (rs485->flags & SER_RS485_ENABLED) { > > Seems you haven't seen / ignored my comments. Please, read my previous reply. You said !!() is redundant and I have removed !!(). Previous feedback also suggested that is_rs485 is not needed, but I had reverted both changes as I initially thought it was the cause of a breakage. However, further testing found the breakage was unrelated to this patch series. Therefore, I attempted to address both suggestions by removing is_rs485 and !!() in this submission. I did not ignore your comments and I do not appreciate these insenuations. I have made changes based on every one of your comments in the previous submission, I just did not always address the comment in exactly you suggested. Please, clarify how this fails to address your comments and I will be happy to correct it in the next submission. > > + /* Set EFR[4]=1 to enable enhanced feature registers */ > > + writeb(readb(p + UART_XR_EFR) | UART_EFR_ECB, p + UART_XR_EFR); > > + > > + /* Set MCR to use DTR as Auto-RS485 Enable signal */ > > + writeb(UART_MCR_OUT1, p + UART_MCR); > > + > > + /* Store original LCR and set LCR[7]=1 to enable access to DLD register */ > > + old_lcr = readb(p + UART_LCR); > > + writeb(old_lcr | UART_LCR_DLAB, p + UART_LCR); > > + > > + /* Set DLD[7]=1 for inverted RS485 Enable logic */ > > + writeb(readb(p + UART_EXAR_DLD) | UART_EXAR_DLD_485_POLARITY, p + UART_EXAR_DLD); > > + > > + writeb(old_lcr, p + UART_LCR); > > + } > > + > > + return 0; > > + } > > -- > With Best Regards, > Andy Shevchenko > > >
