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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 64A48C433EF for ; Tue, 8 Feb 2022 10:05:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=FWJXvuz8XNZcWeIfGTxB/asT2Bbxt92CAxl902uCbE4=; b=KHeE6Hm4h7szg/ B24IOsI6A5Qdk9xdxYsfvIlM3yRtOKIOKNt+OSwt6AgY44IhtjY+apC50LNQukXNzbciHScm5hS/v lZWfLpg5/kF3DRfyQjjtNnbN0YEXE7n46iZzUF+YHQZnYQ1ztACICkk+LN+ld2QPYh0ujC33V1eZ/ 8VkK8nXJoVAZtbRXYkoabKPtSioTqdYRVPU3rmEb39YtrW7iZO9YXgY5pKwTOJVxb7X3GxjDNYd+1 A4wJbIyqu9K+klFd9ugJQnapCkzkw89YfHdAI53sMyZqZooUu0ITjQWFg//aaeIaZUwn7vYC9CRrL w0ZMo0Et6HHbrqlz0jMA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nHNLw-00DLCp-Bo; Tue, 08 Feb 2022 10:04:04 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nHNLs-00DLC1-TD for linux-arm-kernel@lists.infradead.org; Tue, 08 Feb 2022 10:04:02 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 1E30D61546; Tue, 8 Feb 2022 10:04:00 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id A0D1CC340ED; Tue, 8 Feb 2022 10:03:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1644314639; bh=9XRxX5n5xmPLXTFls/kdqprhfHPlsurq93wudUcHvmI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ydMfYgvttYlxl9uRj9Idy5Mfb8u3n3OXW3As+08WOsMeVgyEFsIELiokvKXAza88c q+ArZi2JeMzdkwiYIC0h7rfZzDkecdHEsCX5Opcb2I4tCAzLDMedbto4z8meqsvHsG itZ329OsvY3CyLkSCy7t5yq0VrQU3vmhnOF9VhfA= Date: Tue, 8 Feb 2022 11:03:56 +0100 From: Greg Kroah-Hartman To: Harald Seiler Cc: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= , Ahmad Fatoum , linux-serial@vger.kernel.org, Sascha Hauer , linux-kernel@vger.kernel.org, NXP Linux Team , Fabio Estevam , Shawn Guo , Jiri Slaby , Pengutronix Kernel Team , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH] tty: serial: imx: Add fast path when rs485 delays are 0 Message-ID: References: <20220119145204.238767-1-hws@denx.de> <20220119151145.zft47rzebnabiej2@pengutronix.de> <0df5d9ea2081f5d798f80297efb973f542dae183.camel@denx.de> <20220119162122.jmnz2hxid76p4hli@pengutronix.de> <5cab27cab5a39ef5e19992bc54e57c3f6106dafe.camel@denx.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <5cab27cab5a39ef5e19992bc54e57c3f6106dafe.camel@denx.de> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220208_020401_042884_B9A19FB5 X-CRM114-Status: GOOD ( 38.10 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Jan 19, 2022 at 05:59:46PM +0100, Harald Seiler wrote: > Hi, > = > On Wed, 2022-01-19 at 17:21 +0100, Uwe Kleine-K=F6nig wrote: > > On Wed, Jan 19, 2022 at 04:20:12PM +0100, Harald Seiler wrote: > > > Hi, > > > = > > > On Wed, 2022-01-19 at 16:11 +0100, Uwe Kleine-K=F6nig wrote: > > > > On Wed, Jan 19, 2022 at 03:52:03PM +0100, Harald Seiler wrote: > > > > > Right now, even when `delay_rts_before_send` and `delay_rts_after= _send` > > > > > are 0, the hrtimer is triggered (with timeout 0) which can introd= uce a > > > > > few 100us of additional overhead on slower i.MX platforms. > > > > > = > > > > > Implement a fast path when the delays are 0, where the RTS signal= is > > > > > toggled immediately instead of going through an hrtimer. This fa= st path > > > > > behaves identical to the code before delay support was implemente= d. > > > > > = > > > > > Signed-off-by: Harald Seiler > > > > > --- > > > > > drivers/tty/serial/imx.c | 18 ++++++++++++++---- > > > > > 1 file changed, 14 insertions(+), 4 deletions(-) > > > > > = > > > > > diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c > > > > > index df8a0c8b8b29..67bbbb69229d 100644 > > > > > --- a/drivers/tty/serial/imx.c > > > > > +++ b/drivers/tty/serial/imx.c > > > > > @@ -455,9 +455,14 @@ static void imx_uart_stop_tx(struct uart_por= t *port) > > > > > if (port->rs485.flags & SER_RS485_ENABLED) { > > > > > if (sport->tx_state =3D=3D SEND) { > > > > > sport->tx_state =3D WAIT_AFTER_SEND; > > > > > - start_hrtimer_ms(&sport->trigger_stop_tx, > > > > > + > > > > > + if (port->rs485.delay_rts_after_send > 0) { > > > > > + start_hrtimer_ms(&sport->trigger_stop_tx, > > > > > port->rs485.delay_rts_after_send); > > > > > - return; > > > > > + return; > > > > > + } > > > > > + > > > > > + /* continue without any delay */ > > > > = > > > > Is it right to keep the assignment sport->tx_state =3D WAIT_AFTER_S= END ? > > > = > > > I am keeping the assignment intentionally, to fall into the > > > if(state =3D=3D WAIT_AFTER_RTS) below (which then sets the state to O= FF). > > > I originally had the code structured like this: > > > = > > > if (port->rs485.delay_rts_after_send > 0) { > > > sport->tx_state =3D WAIT_AFTER_SEND; > > > start_hrtimer_ms(&sport->trigger_stop_tx, > > > port->rs485.delay_rts_after_send); > > > return; > > > } else { > > > /* continue without any delay */ > > > sport->tx_state =3D WAIT_AFTER_SEND; > > > } > > > = > > > This is functionally identical, but maybe a bit more explicit. > > > = > > > Not sure what is more clear to read? > > = > > I didn't oppose to the readability thing. With your patch you skip > > starting the stop_tx timer and that would usually care for calling > > imx_uart_stop_tx and setting sport->tx_state =3D OFF. This doesn't happ= en > > with your patch any more. > = > Not starting the timer is the entire point of the patch - instead, the > code which would run inside the timer callback now runs immediately. To > do this, I set the tx_state to WAIT_AFTER_SEND and _don't_ do the early > return which leads into the if(tx_state =3D=3D WAIT_AFTER_SEND) below. T= his > is the code-path which normally runs later in the hrtimer callback. > = > I suppose it would have been good to provide more context lines in the > patch... Here is the relevant bit (in the changed version now): > = > if (sport->tx_state =3D=3D SEND) { > sport->tx_state =3D WAIT_AFTER_SEND; > = > if (port->rs485.delay_rts_after_send > 0) { > start_hrtimer_ms(&sport->trigger_stop_tx, > port->rs485.delay_rts_after_send); > return; > } > = > /* continue without any delay */ > } > = > if (sport->tx_state =3D=3D WAIT_AFTER_RTS || > sport->tx_state =3D=3D WAIT_AFTER_SEND) { > /* ... actual rts toggling ... */ > = > sport->tx_state =3D OFF; > } > = Uwe, any thoughts about if this patch should be taken or not? thanks, greg k-h _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel