On 12.11.21 07:14, Su Bao Cheng wrote: > On 2021/10/27 下午7:39, Lukas Wunner wrote: >> On Wed, Oct 27, 2021 at 07:16:44PM +0800, Su Bao Cheng wrote: >>> This reverts commit f45709df7731ad36306a28a3e1af7309d55c35f5. >>> >>> The `serial8250_do_set_mctrl` not only used by userspace ioctl but >>> also the kernel itself. >>> >>> During tty_open, the uart_port_startup sets the MCR to 0, and then use >>> set_mctrl to restore the MCR, so at this time, the MCR read does not >>> reflect the desired value. >> >> I don't quite follow. Where is uart_port_startup() setting the MCR to 0? >> Are you referring to the call to uart_port_dtr_rts()? That function should >> set RTS correctly according to RS485 state, so I don't see where any >> breakage may occur. >> >> What is the user-visible issue that you seek to fix with the revert? >> > > Sorry for the late response, the company exchange server does not work > for me at this moment, so I have to use the private email instead. > > The issue is observed on omap8250 hardware (CPU: AM6548). the use case > is RS485 half-duplex (2 wire mode), in this mode the RTS pin is used to > control the direction and is software controller via the MCR[1] > register. The problem is that the RS485 transmitting is OK, but the > receiving is not working. Similar issue also exists for the RS422, i.e. > the 4-wire full-duplex mode of RS485, but this time the TX does not > work, RX is fine. > > The MCR is set to 0 at this line within uart_port_startup(): > retval = uport->ops->startup(uport); > > On omap8250, the startup() points to omap_8250_startup(), within it: > up->mcr = 0; > > For software controlled RTS pin of RS485 half-duplex, when not in the > transmitting, the MCR[1] should be constant to indicate the current > direction is receiving. This is set in serial8250_em485_stop_tx(). > > So after this point of setting the MCR to 0, this up->mcr register > mirror does not reflect the actual desired value anymore. Further > checking against it leads to false result. > > Another possible fix could be, instead of setting the mcr to 0 blindly, > one could check if the current operating mode is RS485 half-duplex, and > if so, mask the MCR[1], so that this bit is not changed. Because the > MCR[1] will be changed to the correct value before TX in > serial8250_em485_start_tx(), this change would not impact the transmitting. > >From this description, it seems like we have a rather fundamental regression here. Was RS485-half-duplex / RS422 tested after this change, Lukas? A revert is just a workaround, I would say. But unless we have a quick idea for the proper fix, it may be the option for stable at least. Jan -- Siemens AG, T RDA IOT Corporate Competence Center Embedded Linux
