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=unavailable 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 6F6CFC48BE5 for ; Mon, 21 Jun 2021 09:12:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 486ED61059 for ; Mon, 21 Jun 2021 09:12:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230326AbhFUJOt (ORCPT ); Mon, 21 Jun 2021 05:14:49 -0400 Received: from youngberry.canonical.com ([91.189.89.112]:40480 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230175AbhFUJOr (ORCPT ); Mon, 21 Jun 2021 05:14:47 -0400 Received: from mail-ed1-f71.google.com ([209.85.208.71]) by youngberry.canonical.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.93) (envelope-from ) id 1lvFyq-000857-36 for linux-kernel@vger.kernel.org; Mon, 21 Jun 2021 09:12:32 +0000 Received: by mail-ed1-f71.google.com with SMTP id f12-20020a056402150cb029038fdcfb6ea2so5602759edw.14 for ; Mon, 21 Jun 2021 02:12:32 -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=OtPiQ6OO2TAsBjszsku45Z1ropnsh33Pp8/MGScgIYo=; b=M+gpl9CZQTikc+nvHXgsiMO/LVArv04ajvk/i5xI+J/A4W4U4hZgz4PXuZ+p1aYwau smj1OaEwQbwLvToRSoyQNn69EsUoi2cuX8132sg5UqkD+O9VOEVjQI9yNRIh6FGBUY5b FTGqM1dUT4w9wPnHrPxrN9r5slH2c9ya++3vLnbi1sNQoixaMcNpglei7gsZX8PkQhNy 09TZW3oqu0ysPqE3A9RDP3Kw+dI9cCr6/FDKo+rvPwU4YDMd0kAbJnR6W3+TUEde+mGr TwNosSJNgYxslyt3VeFklcG+UvbHghg5ZAN9XCx4byD2X/NJOoODKTqocOmvs6Gl5o2k IHPw== X-Gm-Message-State: AOAM533xBbc6fHeNm6z6TefTNeFd+zFmLtYk61kx5qbOIGkRr4QesL0p rjspBUrZSJ51qU8ZfFZy9waS3TOp1dvF9tobCYhZf9MlycGGx9p7vgqSgLtbACSIXt/Z7u21phU 1zi+PQBm8IUahb/Wv+LNTuugiV5QI3v+F085uqOJXrQ== X-Received: by 2002:aa7:d918:: with SMTP id a24mr20823931edr.235.1624266751824; Mon, 21 Jun 2021 02:12:31 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwx35TuP3/cSWQbzdJJMY8jXV8k6q7uF2M0jTZBEo1Y1rV/AqfoC9y+u6ebe35GLSRbSnmTRg== X-Received: by 2002:aa7:d918:: with SMTP id a24mr20823912edr.235.1624266751658; Mon, 21 Jun 2021 02:12:31 -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 v26sm4608693ejk.70.2021.06.21.02.12.30 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 21 Jun 2021 02:12:31 -0700 (PDT) Subject: Re: [PATCH] serial: samsung: use dma_ops of DMA if attached To: Tamseel Shams , kgene@kernel.org, gregkh@linuxfoundation.org, jslaby@suse.com 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: <20210621044916.41564-1-m.shams@samsung.com> From: Krzysztof Kozlowski Message-ID: <8935a448-04b7-91ce-203a-9f0d7e377052@canonical.com> Date: Mon, 21 Jun 2021 11:12:30 +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: <20210621044916.41564-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-kernel@vger.kernel.org Hi, Thanks for the patch. On 21/06/2021 06:49, 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. Hmmm, but why DMA device pointer should be used? > > 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 > --- > drivers/tty/serial/samsung_tty.c | 60 +++++++++++++++++++++++++------- > 1 file changed, 48 insertions(+), 12 deletions(-) > > diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c > index b923683e6a25..5bdc7dd2a5e2 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; > > + /* Pick dma_ops of DMA device if DMA device is attached */ You mention here and further comments "dma_ops". I don't see you changing the DMA ops, but the device. It's quite confusing. I think you meant a DMA device shall be passed to DMA API? Second question: you write that DMA devices should be used if DMA is attached and in the code you follow such pattern a lot: > + if (dma && dma->tx_chan) > + dma_map_ops_dev = dma->tx_chan->device->dev; > + Are you trying to say that if DMA is not attached, UART device should be used? If DMA is not attached, how are the DMA operations used then? > if (!ourport->tx_enabled) > return; > > @@ -298,7 +303,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; > @@ -324,15 +329,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; > > + /* Pick dma_ops of DMA device if DMA device is attached */ > + if (dma && dma->tx_chan) > + 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); > @@ -408,7 +417,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; > > + /* Pick dma_ops of DMA device if DMA device is attached */ > + 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); > @@ -416,7 +429,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, > @@ -483,12 +496,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; > > + /* Pick dma_ops of DMA device if DMA device is attached */ > + 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; > @@ -600,8 +618,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; > > - dma_sync_single_for_device(ourport->port.dev, dma->rx_addr, > + /* Pick dma_ops of DMA device if DMA device is attached */ > + if (dma && dma->rx_chan) > + dma_map_ops_dev = dma->rx_chan->device->dev; > + > + 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, > @@ -983,6 +1006,7 @@ static int s3c24xx_serial_request_dma(struct s3c24xx_uart_port *p) Offset of hunks looks here significantly different than mainline. The patch should be based and tested mainline tree. Which one did you choose as base? Using my email address not from get_maintainers.pl also suggests that you don't use anything recent as a base. Best regards, Krzysztof