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 DC3D2C4167D for ; Sat, 4 Nov 2023 15:47:01 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229916AbjKDPrB (ORCPT ); Sat, 4 Nov 2023 11:47:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38698 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229453AbjKDPq7 (ORCPT ); Sat, 4 Nov 2023 11:46:59 -0400 Received: from connect.vanmierlo.com (fieber.vanmierlo.com [84.243.197.177]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8F644DB; Sat, 4 Nov 2023 08:46:53 -0700 (PDT) X-Footer: dmFubWllcmxvLmNvbQ== Received: from roundcube.vanmierlo.com ([192.168.37.37]) (authenticated user m.brock@vanmierlo.com) by connect.vanmierlo.com (Kerio Connect 10.0.2 patch 1) with ESMTPA; Sat, 4 Nov 2023 16:46:45 +0100 MIME-Version: 1.0 Date: Sat, 04 Nov 2023 16:46:45 +0100 From: m.brock@vanmierlo.com To: Manikanta Guntupalli Cc: git@amd.com, michal.simek@amd.com, gregkh@linuxfoundation.org, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, linux-serial@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, jirislaby@kernel.org, linux-arm-kernel@lists.infradead.org, radhey.shyam.pandey@amd.com, srinivas.goud@amd.com, shubhrajyoti.datta@amd.com, manion05gk@gmail.com Subject: Re: [PATCH V3 2/2] tty: serial: uartps: Add rs485 support to uartps driver In-Reply-To: <20231024144847.2316941-3-manikanta.guntupalli@amd.com> References: <20231024144847.2316941-1-manikanta.guntupalli@amd.com> <20231024144847.2316941-3-manikanta.guntupalli@amd.com> Message-ID: <22a8b01cf7a1df0dffd78eb80bfab819@vanmierlo.com> X-Sender: m.brock@vanmierlo.com Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Manikanta Guntupalli wrote on 2023-10-24 16:48: > @@ -193,6 +196,7 @@ MODULE_PARM_DESC(rx_timeout, "Rx timeout, 1-255"); > * @clk_rate_change_nb: Notifier block for clock changes > * @quirks: Flags for RXBS support. > * @cts_override: Modem control state override > + * @gpiod: Pointer to the gpio descriptor Change gpiod to gpiod_rts maybe? Later someone might want to use a gpio for cts/dtr/dsr/dcd/ri as well. > */ > struct cdns_uart { > struct uart_port *port; > @@ -203,10 +207,19 @@ struct cdns_uart { > struct notifier_block clk_rate_change_nb; > u32 quirks; > bool cts_override; > + struct gpio_desc *gpiod; > }; > struct cdns_platform_data { > u32 quirks; > }; > + > +struct serial_rs485 cdns_rs485_supported = { > + .flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | > + SER_RS485_RTS_AFTER_SEND, > + .delay_rts_before_send = 1, > + .delay_rts_after_send = 1, > +}; > + > #define to_cdns_uart(_nb) container_of(_nb, struct cdns_uart, \ > clk_rate_change_nb) > > @@ -305,6 +318,79 @@ static void cdns_uart_handle_rx(void *dev_id, > unsigned int isrstatus) > tty_flip_buffer_push(&port->state->port); > } > > +/** > + * cdns_rs485_config_gpio_rts_high - Configure GPIO/RTS to high > + * @cdns_uart: Handle to the cdns_uart > + */ > +static void cdns_rs485_config_gpio_rts_high(struct cdns_uart > *cdns_uart) > +{ > + u32 val; > + > + if (cdns_uart->gpiod) { > + gpiod_set_value(cdns_uart->gpiod, 1); > + } else { > + val = readl(cdns_uart->port->membase + CDNS_UART_MODEMCR); > + val &= ~CDNS_UART_MODEMCR_RTS; > + writel(val, cdns_uart->port->membase + CDNS_UART_MODEMCR); > + } > +} > + > +/** > + * cdns_rs485_config_gpio_rts_low - Configure GPIO/RTS to low > + * @cdns_uart: Handle to the cdns_uart > + */ > +static void cdns_rs485_config_gpio_rts_low(struct cdns_uart > *cdns_uart) > +{ > + u32 val; > + > + if (cdns_uart->gpiod) { > + gpiod_set_value(cdns_uart->gpiod, 0); > + } else { > + val = readl(cdns_uart->port->membase + CDNS_UART_MODEMCR); > + val |= CDNS_UART_MODEMCR_RTS; > + writel(val, cdns_uart->port->membase + CDNS_UART_MODEMCR); > + } > +} > + > +/** > + * cdns_rs485_tx_setup - Tx setup specific to rs485 > + * @cdns_uart: Handle to the cdns_uart > + */ > +static void cdns_rs485_tx_setup(struct cdns_uart *cdns_uart) > +{ > + if (cdns_uart->port->rs485.flags & SER_RS485_RTS_ON_SEND) > + cdns_rs485_config_gpio_rts_high(cdns_uart); > + else > + cdns_rs485_config_gpio_rts_low(cdns_uart); > +} > + > +/** > + * cdns_rs485_rx_setup - Rx setup specific to rs485 > + * @cdns_uart: Handle to the cdns_uart > + */ > +static void cdns_rs485_rx_setup(struct cdns_uart *cdns_uart) > +{ > + if (cdns_uart->port->rs485.flags & SER_RS485_RTS_AFTER_SEND) > + cdns_rs485_config_gpio_rts_high(cdns_uart); > + else > + cdns_rs485_config_gpio_rts_low(cdns_uart); > +} Why not simply create: void cdns_rs485_driver_enable(struct cdns_uart *cdns_uart, bool enable) And let it handle the rs485.flags itself? > + > +/** > + * cdns_uart_tx_empty - Check whether TX is empty > + * @port: Handle to the uart port structure > + * > + * Return: TIOCSER_TEMT on success, 0 otherwise > + */ > +static unsigned int cdns_uart_tx_empty(struct uart_port *port) > +{ > + unsigned int status; > + > + status = readl(port->membase + CDNS_UART_SR) & > + (CDNS_UART_SR_TXEMPTY | CDNS_UART_SR_TACTIVE); > + return (status == CDNS_UART_SR_TXEMPTY) ? TIOCSER_TEMT : 0; > +} > + > /** > * cdns_uart_handle_tx - Handle the bytes to be Txed. > * @dev_id: Id of the UART port > @@ -571,6 +657,8 @@ static int cdns_uart_clk_notifier_cb(struct > notifier_block *nb, > static void cdns_uart_start_tx(struct uart_port *port) > { > unsigned int status; > + unsigned long time_out; > + struct cdns_uart *cdns_uart = port->private_data; > > if (uart_tx_stopped(port)) > return; > @@ -589,8 +677,31 @@ static void cdns_uart_start_tx(struct uart_port > *port) > > writel(CDNS_UART_IXR_TXEMPTY, port->membase + CDNS_UART_ISR); > + if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED) { > + cdns_rs485_tx_setup(cdns_uart); > + if (cdns_uart->port->rs485.delay_rts_before_send) > + mdelay(cdns_uart->port->rs485.delay_rts_before_send); Would it not be better to start a timer here with a callback that enables the TXEMPTY interrupt? That would automatically call cdns_uart_handle_tx(). > + } > + > cdns_uart_handle_tx(port); > > + if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED) { > + time_out = jiffies + usecs_to_jiffies(TX_TIMEOUT); > + /* Wait for tx completion */ > + while ((cdns_uart_tx_empty(cdns_uart->port) != TIOCSER_TEMT) && > + time_before(jiffies, time_out)) > + cpu_relax(); > + > + if (cdns_uart->port->rs485.delay_rts_after_send) > + mdelay(cdns_uart->port->rs485.delay_rts_after_send); > + > + /* > + * Default Rx should be setup, because RX signaling path > + * need to enable to receive data. > + */ > + cdns_rs485_rx_setup(cdns_uart); > + } I think this should be done from the TXEMPTY interrupt. And again schedule a timer to drop the DE line. You really can do this without using mdelay(). > + > /* Enable the TX Empty interrupt */ > writel(CDNS_UART_IXR_TXEMPTY, port->membase + CDNS_UART_IER); > } > @@ -602,6 +713,14 @@ static void cdns_uart_start_tx(struct uart_port > *port) > static void cdns_uart_stop_tx(struct uart_port *port) > { > unsigned int regval; > + struct cdns_uart *cdns_uart = port->private_data; > + > + if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED) { > + if (cdns_uart->port->rs485.delay_rts_after_send) > + mdelay(cdns_uart->port->rs485.delay_rts_after_send); > + > + cdns_rs485_rx_setup(cdns_uart); > + } Again, start a timer and wait for completion? > regval = readl(port->membase + CDNS_UART_CR); > regval |= CDNS_UART_CR_TX_DIS; > @@ -626,21 +745,6 @@ static void cdns_uart_stop_rx(struct uart_port > *port) > writel(regval, port->membase + CDNS_UART_CR); > } > > -/** > - * cdns_uart_tx_empty - Check whether TX is empty > - * @port: Handle to the uart port structure > - * > - * Return: TIOCSER_TEMT on success, 0 otherwise > - */ > -static unsigned int cdns_uart_tx_empty(struct uart_port *port) > -{ > - unsigned int status; > - > - status = readl(port->membase + CDNS_UART_SR) & > - (CDNS_UART_SR_TXEMPTY | CDNS_UART_SR_TACTIVE); > - return (status == CDNS_UART_SR_TXEMPTY) ? TIOCSER_TEMT : 0; > -} > - > /** > * cdns_uart_break_ctl - Based on the input ctl we have to start or > stop > * transmitting char breaks > @@ -829,6 +933,9 @@ static int cdns_uart_startup(struct uart_port > *port) > (CDNS_UART_CR_TXRST | CDNS_UART_CR_RXRST)) > cpu_relax(); > > + if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED) > + cdns_rs485_rx_setup(cdns_uart); > + > /* > * Clear the RX disable bit and then set the RX enable bit to enable > * the receiver. > @@ -1455,6 +1562,23 @@ MODULE_DEVICE_TABLE(of, cdns_uart_of_match); > /* Temporary variable for storing number of instances */ > static int instances; > > +/** > + * cdns_rs485_config - Called when an application calls TIOCSRS485 > ioctl. > + * @port: Pointer to the uart_port structure > + * @termios: Pointer to the ktermios structure > + * @rs485: Pointer to the serial_rs485 structure > + * > + * Return: 0 > + */ > +static int cdns_rs485_config(struct uart_port *port, struct ktermios > *termios, > + struct serial_rs485 *rs485) > +{ > + if (rs485->flags & SER_RS485_ENABLED) > + dev_dbg(port->dev, "Setting UART to RS485\n"); > + Shouldn't you force automatic RTS control off here? And also call cdns_rs485_rx_setup() > + return 0; > +} > + > /** > * cdns_uart_probe - Platform driver probe > * @pdev: Pointer to the platform device structure > @@ -1463,6 +1587,7 @@ static int instances; > */ > static int cdns_uart_probe(struct platform_device *pdev) > { > + u32 val; > int rc, id, irq; > struct uart_port *port; > struct resource *res; > @@ -1597,9 +1722,23 @@ static int cdns_uart_probe(struct > platform_device *pdev) > port->private_data = cdns_uart_data; > port->read_status_mask = CDNS_UART_IXR_TXEMPTY | CDNS_UART_IXR_RXTRIG > | > CDNS_UART_IXR_OVERRUN | CDNS_UART_IXR_TOUT; > + port->rs485_config = cdns_rs485_config; > + port->rs485_supported = cdns_rs485_supported; > cdns_uart_data->port = port; > platform_set_drvdata(pdev, port); > > + rc = uart_get_rs485_mode(port); > + if (rc) > + goto err_out_clk_notifier; > + > + cdns_uart_data->gpiod = devm_gpiod_get_optional(&pdev->dev, "rts", > + GPIOD_OUT_LOW); > + if (IS_ERR(cdns_uart_data->gpiod)) { > + rc = PTR_ERR(cdns_uart_data->gpiod); > + dev_err(port->dev, "xuartps: devm_gpiod_get_optional failed\n"); > + goto err_out_clk_notifier; > + } > + > pm_runtime_use_autosuspend(&pdev->dev); > pm_runtime_set_autosuspend_delay(&pdev->dev, > UART_AUTOSUSPEND_TIMEOUT); > pm_runtime_set_active(&pdev->dev); > @@ -1638,6 +1777,16 @@ static int cdns_uart_probe(struct > platform_device *pdev) > cdns_uart_data->cts_override = > of_property_read_bool(pdev->dev.of_node, > "cts-override"); > > + if (cdns_uart_data->port->rs485.flags & SER_RS485_ENABLED) { > + if (!cdns_uart_data->gpiod) { > + val = readl(cdns_uart_data->port->membase > + + CDNS_UART_MODEMCR); > + val |= CDNS_UART_MODEMCR_RTS; > + writel(val, cdns_uart_data->port->membase > + + CDNS_UART_MODEMCR); > + } > + } Simply call cdns_rs485_rx_setup() ? > + > instances++; > > return 0; > @@ -1646,6 +1795,7 @@ static int cdns_uart_probe(struct platform_device > *pdev) > pm_runtime_disable(&pdev->dev); > pm_runtime_set_suspended(&pdev->dev); > pm_runtime_dont_use_autosuspend(&pdev->dev); > +err_out_clk_notifier: > #ifdef CONFIG_COMMON_CLK > clk_notifier_unregister(cdns_uart_data->uartclk, > &cdns_uart_data->clk_rate_change_nb); Please also modify cdns_uart_[s|g]et_mctrl() so they support rts-gpios. Maarten 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 74594C4332F for ; Sat, 4 Nov 2023 15:47:49 +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-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Message-ID:References:In-Reply-To:Subject:Cc:To:From :Date:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=noP9zyJRmzi0pORpK1GtjtTQf9p4U2QujRMVk4gkNgY=; b=rrzEGNMasVC5ws5dzJ8Um+Eg7w 8oNffxPUdyb2wWwrmmzgMOA0KAuDKDArnkNCuAIJV4V1YWM1wFBqxtom0uwXpByUkQckgaPSUNjN7 FJTt+dZkgEtcDkHnbLhqEnwx37294XR7/5EQ+i/FyCh4Alr1is6RSz60YJF0mJw0m6+UeLux1o1o3 vFQqam0/QgMEx5Qyh2Q6J0iQ+DjV88OD9v3U6+Twpq+RU+NcTPi9pqxrGvaetIQKmLMG2XykItaso njrI9JWDPaz4TGVmrmipFJv+7c2UxhVDBWsBZCruG2PYaz8dUeV5Uq7wkBTEx63wF5PauAkFzIoew DUWfp69g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qzIrj-00DYdo-1O; Sat, 04 Nov 2023 15:47:15 +0000 Received: from fieber.vanmierlo.com ([84.243.197.177] helo=connect.vanmierlo.com) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qzIrb-00DYbW-1X for linux-arm-kernel@lists.infradead.org; Sat, 04 Nov 2023 15:47:12 +0000 X-Footer: dmFubWllcmxvLmNvbQ== Received: from roundcube.vanmierlo.com ([192.168.37.37]) (authenticated user m.brock@vanmierlo.com) by connect.vanmierlo.com (Kerio Connect 10.0.2 patch 1) with ESMTPA; Sat, 4 Nov 2023 16:46:45 +0100 MIME-Version: 1.0 Date: Sat, 04 Nov 2023 16:46:45 +0100 From: m.brock@vanmierlo.com To: Manikanta Guntupalli Cc: git@amd.com, michal.simek@amd.com, gregkh@linuxfoundation.org, robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org, linux-serial@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, jirislaby@kernel.org, linux-arm-kernel@lists.infradead.org, radhey.shyam.pandey@amd.com, srinivas.goud@amd.com, shubhrajyoti.datta@amd.com, manion05gk@gmail.com Subject: Re: [PATCH V3 2/2] tty: serial: uartps: Add rs485 support to uartps driver In-Reply-To: <20231024144847.2316941-3-manikanta.guntupalli@amd.com> References: <20231024144847.2316941-1-manikanta.guntupalli@amd.com> <20231024144847.2316941-3-manikanta.guntupalli@amd.com> Message-ID: <22a8b01cf7a1df0dffd78eb80bfab819@vanmierlo.com> X-Sender: m.brock@vanmierlo.com X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20231104_084707_807878_967293D0 X-CRM114-Status: GOOD ( 29.51 ) 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Manikanta Guntupalli wrote on 2023-10-24 16:48: > @@ -193,6 +196,7 @@ MODULE_PARM_DESC(rx_timeout, "Rx timeout, 1-255"); > * @clk_rate_change_nb: Notifier block for clock changes > * @quirks: Flags for RXBS support. > * @cts_override: Modem control state override > + * @gpiod: Pointer to the gpio descriptor Change gpiod to gpiod_rts maybe? Later someone might want to use a gpio for cts/dtr/dsr/dcd/ri as well. > */ > struct cdns_uart { > struct uart_port *port; > @@ -203,10 +207,19 @@ struct cdns_uart { > struct notifier_block clk_rate_change_nb; > u32 quirks; > bool cts_override; > + struct gpio_desc *gpiod; > }; > struct cdns_platform_data { > u32 quirks; > }; > + > +struct serial_rs485 cdns_rs485_supported = { > + .flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND | > + SER_RS485_RTS_AFTER_SEND, > + .delay_rts_before_send = 1, > + .delay_rts_after_send = 1, > +}; > + > #define to_cdns_uart(_nb) container_of(_nb, struct cdns_uart, \ > clk_rate_change_nb) > > @@ -305,6 +318,79 @@ static void cdns_uart_handle_rx(void *dev_id, > unsigned int isrstatus) > tty_flip_buffer_push(&port->state->port); > } > > +/** > + * cdns_rs485_config_gpio_rts_high - Configure GPIO/RTS to high > + * @cdns_uart: Handle to the cdns_uart > + */ > +static void cdns_rs485_config_gpio_rts_high(struct cdns_uart > *cdns_uart) > +{ > + u32 val; > + > + if (cdns_uart->gpiod) { > + gpiod_set_value(cdns_uart->gpiod, 1); > + } else { > + val = readl(cdns_uart->port->membase + CDNS_UART_MODEMCR); > + val &= ~CDNS_UART_MODEMCR_RTS; > + writel(val, cdns_uart->port->membase + CDNS_UART_MODEMCR); > + } > +} > + > +/** > + * cdns_rs485_config_gpio_rts_low - Configure GPIO/RTS to low > + * @cdns_uart: Handle to the cdns_uart > + */ > +static void cdns_rs485_config_gpio_rts_low(struct cdns_uart > *cdns_uart) > +{ > + u32 val; > + > + if (cdns_uart->gpiod) { > + gpiod_set_value(cdns_uart->gpiod, 0); > + } else { > + val = readl(cdns_uart->port->membase + CDNS_UART_MODEMCR); > + val |= CDNS_UART_MODEMCR_RTS; > + writel(val, cdns_uart->port->membase + CDNS_UART_MODEMCR); > + } > +} > + > +/** > + * cdns_rs485_tx_setup - Tx setup specific to rs485 > + * @cdns_uart: Handle to the cdns_uart > + */ > +static void cdns_rs485_tx_setup(struct cdns_uart *cdns_uart) > +{ > + if (cdns_uart->port->rs485.flags & SER_RS485_RTS_ON_SEND) > + cdns_rs485_config_gpio_rts_high(cdns_uart); > + else > + cdns_rs485_config_gpio_rts_low(cdns_uart); > +} > + > +/** > + * cdns_rs485_rx_setup - Rx setup specific to rs485 > + * @cdns_uart: Handle to the cdns_uart > + */ > +static void cdns_rs485_rx_setup(struct cdns_uart *cdns_uart) > +{ > + if (cdns_uart->port->rs485.flags & SER_RS485_RTS_AFTER_SEND) > + cdns_rs485_config_gpio_rts_high(cdns_uart); > + else > + cdns_rs485_config_gpio_rts_low(cdns_uart); > +} Why not simply create: void cdns_rs485_driver_enable(struct cdns_uart *cdns_uart, bool enable) And let it handle the rs485.flags itself? > + > +/** > + * cdns_uart_tx_empty - Check whether TX is empty > + * @port: Handle to the uart port structure > + * > + * Return: TIOCSER_TEMT on success, 0 otherwise > + */ > +static unsigned int cdns_uart_tx_empty(struct uart_port *port) > +{ > + unsigned int status; > + > + status = readl(port->membase + CDNS_UART_SR) & > + (CDNS_UART_SR_TXEMPTY | CDNS_UART_SR_TACTIVE); > + return (status == CDNS_UART_SR_TXEMPTY) ? TIOCSER_TEMT : 0; > +} > + > /** > * cdns_uart_handle_tx - Handle the bytes to be Txed. > * @dev_id: Id of the UART port > @@ -571,6 +657,8 @@ static int cdns_uart_clk_notifier_cb(struct > notifier_block *nb, > static void cdns_uart_start_tx(struct uart_port *port) > { > unsigned int status; > + unsigned long time_out; > + struct cdns_uart *cdns_uart = port->private_data; > > if (uart_tx_stopped(port)) > return; > @@ -589,8 +677,31 @@ static void cdns_uart_start_tx(struct uart_port > *port) > > writel(CDNS_UART_IXR_TXEMPTY, port->membase + CDNS_UART_ISR); > + if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED) { > + cdns_rs485_tx_setup(cdns_uart); > + if (cdns_uart->port->rs485.delay_rts_before_send) > + mdelay(cdns_uart->port->rs485.delay_rts_before_send); Would it not be better to start a timer here with a callback that enables the TXEMPTY interrupt? That would automatically call cdns_uart_handle_tx(). > + } > + > cdns_uart_handle_tx(port); > > + if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED) { > + time_out = jiffies + usecs_to_jiffies(TX_TIMEOUT); > + /* Wait for tx completion */ > + while ((cdns_uart_tx_empty(cdns_uart->port) != TIOCSER_TEMT) && > + time_before(jiffies, time_out)) > + cpu_relax(); > + > + if (cdns_uart->port->rs485.delay_rts_after_send) > + mdelay(cdns_uart->port->rs485.delay_rts_after_send); > + > + /* > + * Default Rx should be setup, because RX signaling path > + * need to enable to receive data. > + */ > + cdns_rs485_rx_setup(cdns_uart); > + } I think this should be done from the TXEMPTY interrupt. And again schedule a timer to drop the DE line. You really can do this without using mdelay(). > + > /* Enable the TX Empty interrupt */ > writel(CDNS_UART_IXR_TXEMPTY, port->membase + CDNS_UART_IER); > } > @@ -602,6 +713,14 @@ static void cdns_uart_start_tx(struct uart_port > *port) > static void cdns_uart_stop_tx(struct uart_port *port) > { > unsigned int regval; > + struct cdns_uart *cdns_uart = port->private_data; > + > + if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED) { > + if (cdns_uart->port->rs485.delay_rts_after_send) > + mdelay(cdns_uart->port->rs485.delay_rts_after_send); > + > + cdns_rs485_rx_setup(cdns_uart); > + } Again, start a timer and wait for completion? > regval = readl(port->membase + CDNS_UART_CR); > regval |= CDNS_UART_CR_TX_DIS; > @@ -626,21 +745,6 @@ static void cdns_uart_stop_rx(struct uart_port > *port) > writel(regval, port->membase + CDNS_UART_CR); > } > > -/** > - * cdns_uart_tx_empty - Check whether TX is empty > - * @port: Handle to the uart port structure > - * > - * Return: TIOCSER_TEMT on success, 0 otherwise > - */ > -static unsigned int cdns_uart_tx_empty(struct uart_port *port) > -{ > - unsigned int status; > - > - status = readl(port->membase + CDNS_UART_SR) & > - (CDNS_UART_SR_TXEMPTY | CDNS_UART_SR_TACTIVE); > - return (status == CDNS_UART_SR_TXEMPTY) ? TIOCSER_TEMT : 0; > -} > - > /** > * cdns_uart_break_ctl - Based on the input ctl we have to start or > stop > * transmitting char breaks > @@ -829,6 +933,9 @@ static int cdns_uart_startup(struct uart_port > *port) > (CDNS_UART_CR_TXRST | CDNS_UART_CR_RXRST)) > cpu_relax(); > > + if (cdns_uart->port->rs485.flags & SER_RS485_ENABLED) > + cdns_rs485_rx_setup(cdns_uart); > + > /* > * Clear the RX disable bit and then set the RX enable bit to enable > * the receiver. > @@ -1455,6 +1562,23 @@ MODULE_DEVICE_TABLE(of, cdns_uart_of_match); > /* Temporary variable for storing number of instances */ > static int instances; > > +/** > + * cdns_rs485_config - Called when an application calls TIOCSRS485 > ioctl. > + * @port: Pointer to the uart_port structure > + * @termios: Pointer to the ktermios structure > + * @rs485: Pointer to the serial_rs485 structure > + * > + * Return: 0 > + */ > +static int cdns_rs485_config(struct uart_port *port, struct ktermios > *termios, > + struct serial_rs485 *rs485) > +{ > + if (rs485->flags & SER_RS485_ENABLED) > + dev_dbg(port->dev, "Setting UART to RS485\n"); > + Shouldn't you force automatic RTS control off here? And also call cdns_rs485_rx_setup() > + return 0; > +} > + > /** > * cdns_uart_probe - Platform driver probe > * @pdev: Pointer to the platform device structure > @@ -1463,6 +1587,7 @@ static int instances; > */ > static int cdns_uart_probe(struct platform_device *pdev) > { > + u32 val; > int rc, id, irq; > struct uart_port *port; > struct resource *res; > @@ -1597,9 +1722,23 @@ static int cdns_uart_probe(struct > platform_device *pdev) > port->private_data = cdns_uart_data; > port->read_status_mask = CDNS_UART_IXR_TXEMPTY | CDNS_UART_IXR_RXTRIG > | > CDNS_UART_IXR_OVERRUN | CDNS_UART_IXR_TOUT; > + port->rs485_config = cdns_rs485_config; > + port->rs485_supported = cdns_rs485_supported; > cdns_uart_data->port = port; > platform_set_drvdata(pdev, port); > > + rc = uart_get_rs485_mode(port); > + if (rc) > + goto err_out_clk_notifier; > + > + cdns_uart_data->gpiod = devm_gpiod_get_optional(&pdev->dev, "rts", > + GPIOD_OUT_LOW); > + if (IS_ERR(cdns_uart_data->gpiod)) { > + rc = PTR_ERR(cdns_uart_data->gpiod); > + dev_err(port->dev, "xuartps: devm_gpiod_get_optional failed\n"); > + goto err_out_clk_notifier; > + } > + > pm_runtime_use_autosuspend(&pdev->dev); > pm_runtime_set_autosuspend_delay(&pdev->dev, > UART_AUTOSUSPEND_TIMEOUT); > pm_runtime_set_active(&pdev->dev); > @@ -1638,6 +1777,16 @@ static int cdns_uart_probe(struct > platform_device *pdev) > cdns_uart_data->cts_override = > of_property_read_bool(pdev->dev.of_node, > "cts-override"); > > + if (cdns_uart_data->port->rs485.flags & SER_RS485_ENABLED) { > + if (!cdns_uart_data->gpiod) { > + val = readl(cdns_uart_data->port->membase > + + CDNS_UART_MODEMCR); > + val |= CDNS_UART_MODEMCR_RTS; > + writel(val, cdns_uart_data->port->membase > + + CDNS_UART_MODEMCR); > + } > + } Simply call cdns_rs485_rx_setup() ? > + > instances++; > > return 0; > @@ -1646,6 +1795,7 @@ static int cdns_uart_probe(struct platform_device > *pdev) > pm_runtime_disable(&pdev->dev); > pm_runtime_set_suspended(&pdev->dev); > pm_runtime_dont_use_autosuspend(&pdev->dev); > +err_out_clk_notifier: > #ifdef CONFIG_COMMON_CLK > clk_notifier_unregister(cdns_uart_data->uartclk, > &cdns_uart_data->clk_rate_change_nb); Please also modify cdns_uart_[s|g]et_mctrl() so they support rts-gpios. Maarten _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel