Hi,
I found the RXW4C_IRA/TXW4C_IRA is only available with UART1(LDN0)
by following spec and it's reserved with UART2~4(LDN1~3):
http://html.alldatasheet.com/html-pdf/257955/FINTEK/F81216/5519/25/F81216.html
Also I had confirmed with Fintek H/W engineer, the RXW4C_IRA/TXW4C_IRA
will not functional when IRA_MODE disabled.
For this code with RS485, I prefer the code of Ricardo fixed. Due to
the H/W limit, It's only can be 2 situations, (1) RTS high when TX and
low when idle/read or (2) inverted. So SER_RS485_RTS_ON_SEND and
SER_RS485_RTS_AFTER_SEND must be not the same when RS485_URA enabled.
We should manually control RTS when we need RTS all high or low when
TX/RX.
I had some suggestion, should we return -EINVAL when
SER_RS485_RTS_ON_SEND or SER_RS485_RTS_AFTER_SEND enabled but
SER_RS485_ENABLED disabled?
Ricardo Ribalda Delgado 於 2017/11/2 下午 05:15 寫道:
Hi Lukas
After a closed look to the driver, I believe that it is not wrong, or
at least it is only badly documented/commented.
The user case is: the user wants delay_rts_before_send and/or
delay_rts_after_send but does not want automatic handling of RTS.
If that is the case, RTS_INVERT cannot be used, because it only works
when RS485_URA is enabled (hw limination), and therefore we need to
clear all the flags except SER_RS485_ENABLED, because if that flag is
0, delay_rts_* are ignored.
The datasheet in this case is not very clarifying, perhaps Peter Hong
has some internal information regarding the hardware here, and can
tell us if TXW4C_IRA/RXW4C_IRA does not work without RS485_URA.
It that is the case we should move the check at the beginning something like:
Replace:
if (rs485->flags & SER_RS485_ENABLED)
memset(rs485->padding, 0, sizeof(rs485->padding));
else
memset(rs485, 0, sizeof(*rs485));
with:
/* Check for rs485 enabled and invalid rts configuration*/
if (!(rs485->flags & SER_RS485_ENABLED) ||
(!(rs485->flags & SER_RS485_RTS_ON_SEND)) ==
!(rs485->flags & SER_RS485_RTS_AFTER_SEND)) {
memset(rs485, 0, sizeof(*rs485));
}
else {
memset(rs485->padding, 0, sizeof(rs485->padding));
config |= RS485_URA;
}
Thanks for your patch and sorry for the delay
PS: to make it official
Nacked-by: Ricardo Ribalda <ricardo.ribalda@xxxxxxxxx> (Until Peter
Hong has a response)
On Fri, Oct 27, 2017 at 10:14 AM, Lukas Wunner <lukas@xxxxxxxxx> wrote:
On Thu, Oct 26, 2017 at 10:26:27AM +0200, Ricardo Ribalda Delgado wrote:
On Tue, Oct 24, 2017 at 8:37 PM, Lukas Wunner <lukas@xxxxxxxxx> wrote:
This driver's ->rs485_config callback checks if SER_RS485_RTS_ON_SEND
and SER_RS485_RTS_AFTER_SEND have the same value. If they do, it means
the user has passed in nonsensical data with the TIOCSRS485 ioctl()
since RTS must have a different polarity when sending and when not
sending. In this case, rs485 mode is not enabled (the RS485_URA bit
is not set in the RS485 Enable Register) and this is supposed to be
signaled back to the user by clearing the SER_RS485_ENABLED bit in
struct serial_rs485 ... except a missing tilde character is preventing
that from happening.
Thanks for your patch. Hope you haven't lost much time debugging it.
I haven't, I don't even have the hardware, just stumbled across the bug
while reading the rs485 code of various drivers. I did have to chase
down the datasheet of this chip though to find out what the RS485_URA bit
exactly does. :-) There's an older version of the datasheet which doesn't
specify the RS485 bits yet.
Your patch looks good to me. I would only replace "nonsensical" with
"invalid" on the subject of the patch.
Happy to respin. Could you provide an Acked-by for the patch? Thanks!
Lukas
--
With Best Regards,
Peter Hong
--
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
