From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9D867C433F5 for ; Fri, 19 Nov 2021 08:09:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7E3AB61A81 for ; Fri, 19 Nov 2021 08:09:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233159AbhKSIMV (ORCPT ); Fri, 19 Nov 2021 03:12:21 -0500 Received: from goliath.siemens.de ([192.35.17.28]:33059 "EHLO goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229998AbhKSIMU (ORCPT ); Fri, 19 Nov 2021 03:12:20 -0500 X-Greylist: delayed 530 seconds by postgrey-1.27 at vger.kernel.org; Fri, 19 Nov 2021 03:12:20 EST Received: from mail2.sbs.de (mail2.sbs.de [192.129.41.66]) by goliath.siemens.de (8.15.2/8.15.2) with ESMTPS id 1AJ80JUq007181 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 19 Nov 2021 09:00:19 +0100 Received: from [167.87.1.34] ([167.87.1.34]) by mail2.sbs.de (8.15.2/8.15.2) with ESMTP id 1AJ80IiF025082; Fri, 19 Nov 2021 09:00:18 +0100 Subject: Re: [PATCH] Revert "serial: 8250: Don't touch RTS modem control while in rs485 mode" To: Su Bao Cheng , Lukas Wunner , Su Bao Cheng , gregkh@linuxfoundation.org Cc: linux-serial@vger.kernel.org, chao.zeng@siemens.com References: <20211027111644.1996921-1-baocheng.su@siemens.com> <20211027113938.GA9373@wunner.de> From: Jan Kiszka Message-ID: <61ace2ea-e0b3-599a-3098-8e8a2a4772fa@siemens.com> Date: Fri, 19 Nov 2021 09:00:14 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-serial@vger.kernel.org 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