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 X-Spam-Level: X-Spam-Status: No, score=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 24C70C4743C for ; Wed, 23 Jun 2021 06:10:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 03648613B3 for ; Wed, 23 Jun 2021 06:10:29 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229794AbhFWGMp (ORCPT ); Wed, 23 Jun 2021 02:12:45 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:48280 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229660AbhFWGMo (ORCPT ); Wed, 23 Jun 2021 02:12:44 -0400 Received: from mail-wr1-f71.google.com ([209.85.221.71]) by youngberry.canonical.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.93) (envelope-from ) id 1lvw5i-0003eB-Eo for linux-samsung-soc@vger.kernel.org; Wed, 23 Jun 2021 06:10:26 +0000 Received: by mail-wr1-f71.google.com with SMTP id f9-20020a5d64c90000b029011a3c2a0337so674733wri.0 for ; Tue, 22 Jun 2021 23:10:26 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=9t0SpY9WzRKzn18XeKzMvMayH4y5biXjE4wkqK4Ctvo=; b=bBKxYHDaahNY2nrudUnKrwelhWMgDvo+Qeym5xCnmO2DwGLKevYEYcuhih3otp2CAD 9nUyHr1ml5WkH0C8trPF+uNpX2ripARkLR2OD3gk8Am/Nl4P6WdyFaJg9ruOe53/HmB6 TjUDUYUJSjWqBZfIbQbLgdBsElKYG6/cx7K7bX5exduj31JMkH4OWy6ciozbGFFwSo+N vjaiPl63OzzaK+FEReqk0H71AcRG3WIzONZqNCNpljG3IS1QV68wdzqGn+8kodfX6G0F wyXzhj2Fg4Es6hO8yxcGUlsaw7jZb0/ZSg4HQ1x4j4VJ0MzsucKven3t1e3Ws1OM0pj6 UeAQ== X-Gm-Message-State: AOAM531zzpRJoSV/C/zOP4KkQH8Jpjx11pjGH/rMTSwqUk4m/ls1FGY4 Gl/zILX5ZRQUN1BbSm+1QHREf7nKIT/+KSUTVjSWXvy7cClsMj6YQUp9/Lq9NEcR/Y2TW5oyBjP 7pkvIkioR2QZe2YIl17vbcWvqiWCs5SqNn0UkRQk+KMhX3xFp X-Received: by 2002:a5d:4a01:: with SMTP id m1mr9772134wrq.51.1624428625725; Tue, 22 Jun 2021 23:10:25 -0700 (PDT) X-Google-Smtp-Source: ABdhPJws/Nlnt0zVsVciqND3NNjQEyw/qM1mD4FpFKusmptdlY8X28sBkZRDCbntz4v+/+ly1rbg5w== X-Received: by 2002:a5d:4a01:: with SMTP id m1mr9772121wrq.51.1624428625587; Tue, 22 Jun 2021 23:10:25 -0700 (PDT) Received: from [192.168.1.115] (xdsl-188-155-177-222.adslplus.ch. [188.155.177.222]) by smtp.gmail.com with ESMTPSA id n4sm1672835wrw.21.2021.06.22.23.10.24 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 22 Jun 2021 23:10:25 -0700 (PDT) Subject: Re: [PATCH v3] serial: samsung: use dma_ops of DMA if attached To: Tamseel Shams , gregkh@linuxfoundation.org, jirislaby@kernel.org Cc: linux-arm-kernel@lists.infradead.org, linux-samsung-soc@vger.kernel.org, linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org, alim.akhtar@samsung.com, ajaykumar.rs@samsung.com References: <20210622091710.44032-1-m.shams@samsung.com> From: Krzysztof Kozlowski Message-ID: Date: Wed, 23 Jun 2021 08:10:24 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <20210622091710.44032-1-m.shams@samsung.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-samsung-soc@vger.kernel.org On 22/06/2021 11:17, Tamseel Shams wrote: > When DMA is used for TX and RX by serial driver, it should > pass the DMA device pointer to DMA API instead of UART device > pointer. DMA device should be used for DMA API because only > the DMA device is aware of how the device connects to the memory. > There might be an extra level of address translation due to a > SMMU attached to the DMA device. When serial device pointer device > is used for DMA API, the DMA API will have no clue of the SMMU > attached to the DMA device. > > This patch is necessary to fix the SMMU page faults > which is observed when a DMA(with SMMU enabled) is attached > to UART for transfer. > > Signed-off-by: Tamseel Shams > Signed-off-by: Ajay Kumar > --- > Changes since v1: > 1. Rebased the patch on "tty-next" branch of TTY driver tree > > Changes since v2: > 1. Updated the commit message. > 2. Changed the comment description > > drivers/tty/serial/samsung_tty.c | 62 +++++++++++++++++++++++++------- > 1 file changed, 50 insertions(+), 12 deletions(-) > > diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c > index 9fbc61151c2e..7dd648cb7237 100644 > --- a/drivers/tty/serial/samsung_tty.c > +++ b/drivers/tty/serial/samsung_tty.c > @@ -284,8 +284,13 @@ static void s3c24xx_serial_stop_tx(struct uart_port *port) > struct s3c24xx_uart_dma *dma = ourport->dma; > struct circ_buf *xmit = &port->state->xmit; > struct dma_tx_state state; > + struct device *dma_map_ops_dev = ourport->port.dev; > int count; > > + /* Null pointer check for "dma" and "dma->tx_chan" */ The comment is not useful. > + if (dma && dma->tx_chan) > + dma_map_ops_dev = dma->tx_chan->device->dev; > + > if (!ourport->tx_enabled) > return; > > @@ -305,7 +310,7 @@ static void s3c24xx_serial_stop_tx(struct uart_port *port) > dmaengine_pause(dma->tx_chan); > dmaengine_tx_status(dma->tx_chan, dma->tx_cookie, &state); > dmaengine_terminate_all(dma->tx_chan); > - dma_sync_single_for_cpu(ourport->port.dev, > + dma_sync_single_for_cpu(dma_map_ops_dev, > dma->tx_transfer_addr, dma->tx_size, DMA_TO_DEVICE); > async_tx_ack(dma->tx_desc); > count = dma->tx_bytes_requested - state.residue; > @@ -331,14 +336,19 @@ static void s3c24xx_serial_tx_dma_complete(void *args) > struct circ_buf *xmit = &port->state->xmit; > struct s3c24xx_uart_dma *dma = ourport->dma; > struct dma_tx_state state; > + struct device *dma_map_ops_dev = ourport->port.dev; > unsigned long flags; > int count; > > + /* Null pointer check for "dma" and "dma->tx_chan" */ > + if (dma && dma->tx_chan) What is the "else" here? How is it even possible? Why having this "if" at all? > + dma_map_ops_dev = dma->tx_chan->device->dev; > + > dmaengine_tx_status(dma->tx_chan, dma->tx_cookie, &state); > count = dma->tx_bytes_requested - state.residue; > async_tx_ack(dma->tx_desc); > > - dma_sync_single_for_cpu(ourport->port.dev, dma->tx_transfer_addr, > + dma_sync_single_for_cpu(dma_map_ops_dev, dma->tx_transfer_addr, > dma->tx_size, DMA_TO_DEVICE); > > spin_lock_irqsave(&port->lock, flags); > @@ -436,6 +446,11 @@ static int s3c24xx_serial_start_tx_dma(struct s3c24xx_uart_port *ourport, > struct uart_port *port = &ourport->port; > struct circ_buf *xmit = &port->state->xmit; > struct s3c24xx_uart_dma *dma = ourport->dma; > + struct device *dma_map_ops_dev = ourport->port.dev; > + > + /* Null pointer check for "dma" and "dma->tx_chan" */ > + if (dma && dma->tx_chan) > + dma_map_ops_dev = dma->tx_chan->device->dev; > > if (ourport->tx_mode != S3C24XX_TX_DMA) > enable_tx_dma(ourport); > @@ -443,7 +458,7 @@ static int s3c24xx_serial_start_tx_dma(struct s3c24xx_uart_port *ourport, > dma->tx_size = count & ~(dma_get_cache_alignment() - 1); > dma->tx_transfer_addr = dma->tx_addr + xmit->tail; > > - dma_sync_single_for_device(ourport->port.dev, dma->tx_transfer_addr, > + dma_sync_single_for_device(dma_map_ops_dev, dma->tx_transfer_addr, > dma->tx_size, DMA_TO_DEVICE); > > dma->tx_desc = dmaengine_prep_slave_single(dma->tx_chan, > @@ -510,12 +525,17 @@ static void s3c24xx_uart_copy_rx_to_tty(struct s3c24xx_uart_port *ourport, > struct tty_port *tty, int count) > { > struct s3c24xx_uart_dma *dma = ourport->dma; > + struct device *dma_map_ops_dev = ourport->port.dev; > int copied; > > + /* Null pointer check for "dma" and "dma->rx_chan" */ > + if (dma && dma->rx_chan) > + dma_map_ops_dev = dma->rx_chan->device->dev; > + > if (!count) > return; > > - dma_sync_single_for_cpu(ourport->port.dev, dma->rx_addr, > + dma_sync_single_for_cpu(dma_map_ops_dev, dma->rx_addr, > dma->rx_size, DMA_FROM_DEVICE); > > ourport->port.icount.rx += count; > @@ -635,8 +655,13 @@ static void s3c24xx_serial_rx_dma_complete(void *args) > static void s3c64xx_start_rx_dma(struct s3c24xx_uart_port *ourport) > { > struct s3c24xx_uart_dma *dma = ourport->dma; > + struct device *dma_map_ops_dev = ourport->port.dev; > + > + /* Null pointer check for "dma" and "dma->rx_chan" */ > + if (dma && dma->rx_chan) > + dma_map_ops_dev = dma->rx_chan->device->dev; > > - dma_sync_single_for_device(ourport->port.dev, dma->rx_addr, > + dma_sync_single_for_device(dma_map_ops_dev, dma->rx_addr, > dma->rx_size, DMA_FROM_DEVICE); > > dma->rx_desc = dmaengine_prep_slave_single(dma->rx_chan, > @@ -1045,6 +1070,7 @@ static int s3c24xx_serial_request_dma(struct s3c24xx_uart_port *p) > struct s3c24xx_uart_dma *dma = p->dma; > struct dma_slave_caps dma_caps; > const char *reason = NULL; > + struct device *dma_map_ops_dev = p->port.dev; > int ret; > > /* Default slave configuration parameters */ > @@ -1102,18 +1128,25 @@ static int s3c24xx_serial_request_dma(struct s3c24xx_uart_port *p) > goto err_release_tx; > } > > - dma->rx_addr = dma_map_single(p->port.dev, dma->rx_buf, > + /* Null pointer check for "dma" and "dma->rx_chan" */ > + if (dma && dma->rx_chan) > + dma_map_ops_dev = dma->rx_chan->device->dev; > + > + dma->rx_addr = dma_map_single(dma_map_ops_dev, dma->rx_buf, > dma->rx_size, DMA_FROM_DEVICE); > - if (dma_mapping_error(p->port.dev, dma->rx_addr)) { > + if (dma_mapping_error(dma_map_ops_dev, dma->rx_addr)) { > reason = "DMA mapping error for RX buffer"; > ret = -EIO; > goto err_free_rx; > } > > + /* Null pointer check for "dma" and "dma->tx_chan" */ > + if (dma && dma->tx_chan) > + dma_map_ops_dev = dma->tx_chan->device->dev; > /* TX buffer */ > - dma->tx_addr = dma_map_single(p->port.dev, p->port.state->xmit.buf, > + dma->tx_addr = dma_map_single(dma_map_ops_dev, p->port.state->xmit.buf, > UART_XMIT_SIZE, DMA_TO_DEVICE); > - if (dma_mapping_error(p->port.dev, dma->tx_addr)) { > + if (dma_mapping_error(dma_map_ops_dev, dma->tx_addr)) { > reason = "DMA mapping error for TX buffer"; > ret = -EIO; > goto err_unmap_rx; > @@ -1122,7 +1155,9 @@ static int s3c24xx_serial_request_dma(struct s3c24xx_uart_port *p) > return 0; > > err_unmap_rx: > - dma_unmap_single(p->port.dev, dma->rx_addr, dma->rx_size, > + if (dma->rx_chan) > + dma_map_ops_dev = dma->rx_chan->device->dev; > + dma_unmap_single(dma_map_ops_dev, dma->rx_addr, dma->rx_size, > DMA_FROM_DEVICE); > err_free_rx: > kfree(dma->rx_buf); > @@ -1139,10 +1174,12 @@ static int s3c24xx_serial_request_dma(struct s3c24xx_uart_port *p) > static void s3c24xx_serial_release_dma(struct s3c24xx_uart_port *p) > { > struct s3c24xx_uart_dma *dma = p->dma; > + struct device *dma_map_ops_dev = p->port.dev; > > if (dma->rx_chan) { > + dma_map_ops_dev = dma->rx_chan->device->dev; > dmaengine_terminate_all(dma->rx_chan); > - dma_unmap_single(p->port.dev, dma->rx_addr, > + dma_unmap_single(dma_map_ops_dev, dma->rx_addr, > dma->rx_size, DMA_FROM_DEVICE); > kfree(dma->rx_buf); > dma_release_channel(dma->rx_chan); > @@ -1150,8 +1187,9 @@ static void s3c24xx_serial_release_dma(struct s3c24xx_uart_port *p) > } > > if (dma->tx_chan) { > + dma_map_ops_dev = dma->tx_chan->device->dev; > dmaengine_terminate_all(dma->tx_chan); > - dma_unmap_single(p->port.dev, dma->tx_addr, > + dma_unmap_single(dma_map_ops_dev, dma->tx_addr, > UART_XMIT_SIZE, DMA_TO_DEVICE); > dma_release_channel(dma->tx_chan); > dma->tx_chan = NULL; > Best regards, Krzysztof