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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7378FCCA473 for ; Mon, 27 Jun 2022 08:14:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233398AbiF0IOp (ORCPT ); Mon, 27 Jun 2022 04:14:45 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46688 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232354AbiF0IOn (ORCPT ); Mon, 27 Jun 2022 04:14:43 -0400 Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 366BB6265; Mon, 27 Jun 2022 01:14:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1656317681; x=1687853681; h=date:from:to:cc:subject:in-reply-to:message-id: references:mime-version; bh=qY1j80KT3SlS5urOwaGuqpU9KZ42BFvHtfekqn6Y8R4=; b=BbtMdfkvvcj9+uE425RBTGcYnO2MTIbz4bPKmvYkgs4hmDuyx5K+fV8K Q+d6yjpeG3xwcZrhIbl3zHSO8OpSZZHpGOcKmtPSWO6w6SOCo57Tn98I8 hyqWUdNvXe+QbMo24oqcbXro6Isl1sr7/QBcaWUFcGNebAFwrfh8FVwwm p2qZT9JDnQpkodv9xSXitVKRiFjvtj4Bd54uj2XHRIVR29kFGUwyd7woK YpXsRWazp97jv523cpr+PqPEohxOGMNK64aoEWuySS58BtMlNDGGcBSvG zb5G5i1/oZlNZgXwdpSPj5x3x9VfCW3m9t5BucSpsdziDHb3iqjH7S0fl A==; X-IronPort-AV: E=McAfee;i="6400,9594,10390"; a="367709141" X-IronPort-AV: E=Sophos;i="5.92,225,1650956400"; d="scan'208";a="367709141" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Jun 2022 01:14:41 -0700 X-IronPort-AV: E=Sophos;i="5.92,225,1650956400"; d="scan'208";a="646328063" Received: from gretavix-mobl3.amr.corp.intel.com ([10.249.43.78]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Jun 2022 01:14:37 -0700 Date: Mon, 27 Jun 2022 11:14:34 +0300 (EEST) From: =?ISO-8859-15?Q?Ilpo_J=E4rvinen?= To: Lino Sanfilippo cc: Greg Kroah-Hartman , Jiri Slaby , robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, Andy Shevchenko , vz@mleia.com, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-serial , LKML , lukas@wunner.de, p.rosenberger@kunbus.com, Lino Sanfilippo Subject: Re: [PATCH 7/8] serial: ar933x: Remove redundant assignment in rs485_config In-Reply-To: Message-ID: <033c8d2-3f2e-afe6-2e98-14a61c872b4b@linux.intel.com> References: <20220622154659.8710-1-LinoSanfilippo@gmx.de> <20220622154659.8710-8-LinoSanfilippo@gmx.de> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="8323329-658645688-1656317683=:1622" Precedence: bulk List-ID: X-Mailing-List: linux-serial@vger.kernel.org This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --8323329-658645688-1656317683=:1622 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT On Sun, 26 Jun 2022, Lino Sanfilippo wrote: > On 25.06.22 at 12:14, Ilpo Järvinen wrote: > > On Wed, 22 Jun 2022, Lino Sanfilippo wrote: > > > >> From: Lino Sanfilippo > >> > >> In uart_set_rs485_config() the serial core already assigns the passed > >> serial_rs485 struct to the uart port. > >> > >> So remove the assignment in the drivers rs485_config() function to avoid > >> redundancy. > >> > >> Signed-off-by: Lino Sanfilippo > >> --- > >> drivers/tty/serial/ar933x_uart.c | 1 - > >> 1 file changed, 1 deletion(-) > >> > >> diff --git a/drivers/tty/serial/ar933x_uart.c b/drivers/tty/serial/ar933x_uart.c > >> index ab2c5b2a1ce8..857e010d01dc 100644 > >> --- a/drivers/tty/serial/ar933x_uart.c > >> +++ b/drivers/tty/serial/ar933x_uart.c > >> @@ -591,7 +591,6 @@ static int ar933x_config_rs485(struct uart_port *port, > >> dev_err(port->dev, "RS485 needs rts-gpio\n"); > >> return 1; > >> } > >> - port->rs485 = *rs485conf; > >> return 0; > >> } > > > > Hmm, I realize that for some reason I missed cleaning up this particular > > driver after introducing the serial_rs485 sanitization. It shouldn't need > > that preceeding if block either because ar933x_no_rs485 gets applied if > > there's no rts_gpiod so the core clears SER_RS485_ENABLED. > > I think we still need that "if" in case that RS485 was not enabled at driver > startup (no rs485-enabled-at-boot-time) and no RTS GPIO was defined but then > RS485 is enabled via TIOCSRS485. > > Maybe in ar933x_uart_probe() > > if ((port->rs485.flags & SER_RS485_ENABLED) && > !up->rts_gpiod) { > dev_err(&pdev->dev, "lacking rts-gpio, disabling RS485\n"); > port->rs485.flags &= ~SER_RS485_ENABLED; > port->rs485_supported = &ar933x_no_rs485; > } > > should rather be I think it would be better (and what I should have done while moving the check there in the first place but I missed it). In addition, however, it would be useful to not print unnecessarily: > if (!up->rts_gpiod) { if (port->rs485.flags & SER_RS485_ENABLED) { > dev_err(&pdev->dev, "lacking rts-gpio, disabling RS485\n"); > port->rs485.flags &= ~SER_RS485_ENABLED; } > port->rs485_supported = &ar933x_no_rs485; > } -- i. --8323329-658645688-1656317683=:1622-- 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 242ADC43334 for ; Mon, 27 Jun 2022 08:16:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Type:MIME-Version: References:Message-ID:In-Reply-To:Subject:cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Di8RzHaVQYPQH7shRtI39z4tqihKTybiKSn+z4wriJU=; b=qwfetUMEFeT22MHHsZy7EeGL8R G1VA7coLeBHaF7VHBdLN/TU6bbhTXgKuGz9EKPLlk+vtsWXLfPhyHtEAgVpKuBboO9EjCgGN6GW7y P8Piefd6bEeWseclPksy+qmodUlFLyodDTUdWbHmp+ldpcbxrBtbwGDlqBwe5t7fsUFV5tdAJLXJ/ +31mue5XWWYszRM9yUXZGRtRNnhDpSEvWMFrjuG9SQgzyuEkDHKvB7wEDki9ZBdQ9g6fIS5uUgToJ 5B7aFL2EaetuYzJjOKHroNn0GdmEaT9tk/YnX7Kpx9hSmL7H2S6FPKMeFNxo0XgWPG4Jpki13cjDD lNrQTfGw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1o5jtt-00GjPP-Fu; Mon, 27 Jun 2022 08:15:17 +0000 Received: from mga18.intel.com ([134.134.136.126]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1o5jtK-00Gj2N-Tq for linux-arm-kernel@lists.infradead.org; Mon, 27 Jun 2022 08:14:44 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1656317682; x=1687853682; h=date:from:to:cc:subject:in-reply-to:message-id: references:mime-version; bh=qY1j80KT3SlS5urOwaGuqpU9KZ42BFvHtfekqn6Y8R4=; b=gy2ikPxpeHi3BTLuAvBC/xnb+cWsiA1KPMkm86ye8GN/JBlrXP2XMiwD QIG/YfniHeGNv6ZDJw/QZ/Okr8UIHdxWEr6eiriHPRzCvPYe47s39i/Ay FfK6ndmkyTCDxh9GpIKqI+NG+L222pi0eedY5AuADFJ4cXqYj3Z0E/v9a VmvzssN+Su5nDhBTR+3wPs/BmyqRL/CG5OHoXgpBHQVTpYrdn8UiywhxX 9IauZdCDXlV6nJmvclFdU3EgtPYIIVJ7iO090/dwu7arVCtyw+MkuxMYj IpKV4+sZ+vSkTrggMMseqnCZiOoOmixGxmDs2UD7Gt/keXqPiy2XMUAKv Q==; X-IronPort-AV: E=McAfee;i="6400,9594,10390"; a="264437037" X-IronPort-AV: E=Sophos;i="5.92,225,1650956400"; d="scan'208";a="264437037" Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga106.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Jun 2022 01:14:41 -0700 X-IronPort-AV: E=Sophos;i="5.92,225,1650956400"; d="scan'208";a="646328063" Received: from gretavix-mobl3.amr.corp.intel.com ([10.249.43.78]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 27 Jun 2022 01:14:37 -0700 Date: Mon, 27 Jun 2022 11:14:34 +0300 (EEST) From: =?ISO-8859-15?Q?Ilpo_J=E4rvinen?= To: Lino Sanfilippo cc: Greg Kroah-Hartman , Jiri Slaby , robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, Andy Shevchenko , vz@mleia.com, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, linux-serial , LKML , lukas@wunner.de, p.rosenberger@kunbus.com, Lino Sanfilippo Subject: Re: [PATCH 7/8] serial: ar933x: Remove redundant assignment in rs485_config In-Reply-To: Message-ID: <033c8d2-3f2e-afe6-2e98-14a61c872b4b@linux.intel.com> References: <20220622154659.8710-1-LinoSanfilippo@gmx.de> <20220622154659.8710-8-LinoSanfilippo@gmx.de> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="8323329-658645688-1656317683=:1622" X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220627_011443_066963_D48E4EC5 X-CRM114-Status: GOOD ( 27.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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. --8323329-658645688-1656317683=:1622 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT On Sun, 26 Jun 2022, Lino Sanfilippo wrote: > On 25.06.22 at 12:14, Ilpo Järvinen wrote: > > On Wed, 22 Jun 2022, Lino Sanfilippo wrote: > > > >> From: Lino Sanfilippo > >> > >> In uart_set_rs485_config() the serial core already assigns the passed > >> serial_rs485 struct to the uart port. > >> > >> So remove the assignment in the drivers rs485_config() function to avoid > >> redundancy. > >> > >> Signed-off-by: Lino Sanfilippo > >> --- > >> drivers/tty/serial/ar933x_uart.c | 1 - > >> 1 file changed, 1 deletion(-) > >> > >> diff --git a/drivers/tty/serial/ar933x_uart.c b/drivers/tty/serial/ar933x_uart.c > >> index ab2c5b2a1ce8..857e010d01dc 100644 > >> --- a/drivers/tty/serial/ar933x_uart.c > >> +++ b/drivers/tty/serial/ar933x_uart.c > >> @@ -591,7 +591,6 @@ static int ar933x_config_rs485(struct uart_port *port, > >> dev_err(port->dev, "RS485 needs rts-gpio\n"); > >> return 1; > >> } > >> - port->rs485 = *rs485conf; > >> return 0; > >> } > > > > Hmm, I realize that for some reason I missed cleaning up this particular > > driver after introducing the serial_rs485 sanitization. It shouldn't need > > that preceeding if block either because ar933x_no_rs485 gets applied if > > there's no rts_gpiod so the core clears SER_RS485_ENABLED. > > I think we still need that "if" in case that RS485 was not enabled at driver > startup (no rs485-enabled-at-boot-time) and no RTS GPIO was defined but then > RS485 is enabled via TIOCSRS485. > > Maybe in ar933x_uart_probe() > > if ((port->rs485.flags & SER_RS485_ENABLED) && > !up->rts_gpiod) { > dev_err(&pdev->dev, "lacking rts-gpio, disabling RS485\n"); > port->rs485.flags &= ~SER_RS485_ENABLED; > port->rs485_supported = &ar933x_no_rs485; > } > > should rather be I think it would be better (and what I should have done while moving the check there in the first place but I missed it). In addition, however, it would be useful to not print unnecessarily: > if (!up->rts_gpiod) { if (port->rs485.flags & SER_RS485_ENABLED) { > dev_err(&pdev->dev, "lacking rts-gpio, disabling RS485\n"); > port->rs485.flags &= ~SER_RS485_ENABLED; } > port->rs485_supported = &ar933x_no_rs485; > } -- i. --8323329-658645688-1656317683=:1622 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --8323329-658645688-1656317683=:1622--