Re: [PATCH V3 2/2] serial: exar: Add RS-485 support for Sealevel XR17V35X based cards

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

 



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).

> 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).

> 
> 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.

> 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.

> +		/* 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





[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