Re: [PATCH] serial: amba-pl011: add RS485 support

[Lino Sanfilippo <LinoSanfilippo@gmx.de>]

Hi,

On 16.06.21 at 08:18, Jiri Slaby wrote:

>> +    if (port->rs485.delay_rts_before_send)
>> +        mdelay(port->rs485.delay_rts_before_send);
>
> This is up to 1 second delay with interrupts disabled. Definitely not nice. 8250 clamps this to 100 ms at least, why did you choose 1000 ms?
>

I got this value from rs485.yaml where the max number for both rts delay before and after send
is specified as 1000ms. I thought to keep it up to the user to choose a sane setting. But yes,
its a very long time with interrupts disabled. I will send an updated version of this patch that
limits both values to 100ms.


os(struct uart_port *port, struct ktermios *termios,
>>       unsigned int lcr_h, old_cr;
>>       unsigned long flags;
>>       unsigned int baud, quot, clkdiv;
>> +    unsigned int bits;
>>         if (uap->vendor->oversampling)
>>           clkdiv = 8;
>> @@ -1968,25 +2049,32 @@ pl011_set_termios(struct uart_port *port, struct ktermios *termios,
>>       switch (termios->c_cflag & CSIZE) {
>>       case CS5:
>>           lcr_h = UART01x_LCRH_WLEN_5;
>> +        bits = 7;
>>           break;
>>       case CS6:
>>           lcr_h = UART01x_LCRH_WLEN_6;
>> +        bits = 8;
>>           break;
>>       case CS7:
>>           lcr_h = UART01x_LCRH_WLEN_7;
>> +        bits = 9;
>>           break;
>>       default: // CS8
>>           lcr_h = UART01x_LCRH_WLEN_8;
>> +        bits = 10;
>>           break;
>>       }
>> -    if (termios->c_cflag & CSTOPB)
>> +    if (termios->c_cflag & CSTOPB) {
>>           lcr_h |= UART01x_LCRH_STP2;
>> +        bits++;
>> +    }
>>       if (termios->c_cflag & PARENB) {
>>           lcr_h |= UART01x_LCRH_PEN;
>>           if (!(termios->c_cflag & PARODD))
>>               lcr_h |= UART01x_LCRH_EPS;
>>           if (termios->c_cflag & CMSPAR)
>>               lcr_h |= UART011_LCRH_SPS;
>> +        bits++;
>>       }
>
> You can do simply:
>   bits = tty_get_frame_size(termios->c_cflag);
> now:
> https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/commit/?h=tty-testing&id=3ec2ff37230e1c961d4b0d0118dd23c46b5bcdbb

Ah thats a nice function, thanks for pointing at it. I will adjust the code accordingly.


>
> thanks,

Thanks for the review!

Regards,
Lino