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.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,PDS_BAD_THREAD_QP_64, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 2A5F5C2B9F4 for ; Tue, 22 Jun 2021 08:01:58 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 04E976128A for ; Tue, 22 Jun 2021 08:01:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230087AbhFVIEM (ORCPT ); Tue, 22 Jun 2021 04:04:12 -0400 Received: from mailout4.samsung.com ([203.254.224.34]:11230 "EHLO mailout4.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229677AbhFVIEJ (ORCPT ); Tue, 22 Jun 2021 04:04:09 -0400 Received: from epcas5p3.samsung.com (unknown [182.195.41.41]) by mailout4.samsung.com (KnoxPortal) with ESMTP id 20210622075241epoutp04d192f2dd8efb0f56961bb96319c332b7~K2DqAAmrc1791617916epoutp04p for ; Tue, 22 Jun 2021 07:52:41 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout4.samsung.com 20210622075241epoutp04d192f2dd8efb0f56961bb96319c332b7~K2DqAAmrc1791617916epoutp04p DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1624348361; bh=b2T6MY1kZ9SohFnqiNwSCGWC99b+qKgTnX9g0ikzxy4=; h=From:To:Cc:In-Reply-To:Subject:Date:References:From; b=SaagilmJU04NfgT1p9x30kLtXqAbRHn/vrcoutfK+Y+s8srA/7yFkj6lN8wkKWy2p 6XM5omcWH8cn2kgEkUudWgfMl3WCJRqeLZ9S8Ll0/pcL5v5NOr0uUXvIbnCbmu1mqC wTjpk7DxjlHcJiXpnmrvmWnB4SYr3cquwsg6roek= Received: from epsmges5p1new.samsung.com (unknown [182.195.42.73]) by epcas5p3.samsung.com (KnoxPortal) with ESMTP id 20210622075240epcas5p3df72ffba15fb235adbb38455a18cf33b~K2DpMu8Tr2865028650epcas5p3g; Tue, 22 Jun 2021 07:52:40 +0000 (GMT) Received: from epcas5p3.samsung.com ( [182.195.41.41]) by epsmges5p1new.samsung.com (Symantec Messaging Gateway) with SMTP id 04.F6.09476.8C691D06; Tue, 22 Jun 2021 16:52:40 +0900 (KST) Received: from epsmtrp2.samsung.com (unknown [182.195.40.14]) by epcas5p1.samsung.com (KnoxPortal) with ESMTPA id 20210622075225epcas5p13270a3a17544fa110af21aaff82ceb84~K2Dbc4heu2192421924epcas5p11; Tue, 22 Jun 2021 07:52:25 +0000 (GMT) Received: from epsmgms1p1new.samsung.com (unknown [182.195.42.41]) by epsmtrp2.samsung.com (KnoxPortal) with ESMTP id 20210622075225epsmtrp2e0157be39c97fe9c09a7116858b0408f~K2DbcFLR52613926139epsmtrp2a; Tue, 22 Jun 2021 07:52:25 +0000 (GMT) X-AuditID: b6c32a49-6a1ff70000002504-6f-60d196c8341a Received: from epsmtip1.samsung.com ( [182.195.34.30]) by epsmgms1p1new.samsung.com (Symantec Messaging Gateway) with SMTP id D4.DB.08394.9B691D06; Tue, 22 Jun 2021 16:52:25 +0900 (KST) Received: from mshams01 (unknown [107.122.12.94]) by epsmtip1.samsung.com (KnoxPortal) with ESMTPA id 20210622075224epsmtip123273eb378191cff03467278d9b125f7~K2DZ49-Lm1537015370epsmtip1o; Tue, 22 Jun 2021 07:52:23 +0000 (GMT) From: "M Tamseel Shams" To: "'Krzysztof Kozlowski'" , , , Cc: , , , , , In-Reply-To: <4b2576c1-c986-a4d8-d6cf-661ca056ecee@canonical.com> Subject: RE: [PATCH] serial: samsung: use dma_ops of DMA if attached Date: Tue, 22 Jun 2021 13:22:22 +0530 Message-ID: <007901d7673b$8a758d10$9f60a730$@samsung.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQGDfL5lNG5rzFQPe+FwbDp9GpLyVQGDsg9dAbmelg0Cc6DxagIMN/A2q4mTjPA= Content-Language: en-us X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrFKsWRmVeSWpSXmKPExsWy7bCmpu6JaRcTDHo3S1kceH+QxeLBvG1s Fs2L17NZTNnwgcmi//FrZouNb38wWWx6fI3V4vKuOWwWM87vY7I4s7iX3YHLY1ZDL5vHplWd bB77565h99i8pN6jb8sqRo/1W66yeHzeJBfAHsVlk5Kak1mWWqRvl8CVcfTORdaCDXYVcx6c Y2pg3GXUxcjJISFgIrFqw2SWLkYuDiGB3YwSzXcOsUM4nxglGvadYoNwvjFKfLrwlQWmZd/F d4wQib2MEo86nkI5zxglfj5ZyQhSxSagKzHpYBszSEJEoI1RYnpDBxOIwyxwhlHi8eLJTCBV nAKOEo++dgDN5eAQFnCRuDXDGyTMIqAqseznNLB1vAKWEqf/vGSGsAUlTs58AhZnFtCWWLbw NTPESQoSP58uYwWxRQT8JH7/vc8OUSMu8fLoEbCHJAQOcEjsmHiFFaLBRaJhdQsbhC0s8er4 FnYIW0ri87u9UPF8ifnzVkEtqJBYeeENlG0vceDKHLCbmQU0Jdbv0ocIy0pMPbWOCWIvn0Tv 7ydMEHFeiR3zYGxFif+7+6FWiUu8WzGFdQKj0iwkr81C8tosJC/MQti2gJFlFaNkakFxbnpq sWmBYV5quV5xYm5xaV66XnJ+7iZGcPLS8tzBePfBB71DjEwcjIcYJTiYlUR4X2RfTBDiTUms rEotyo8vKs1JLT7EKM3BoiTOu5T9UIKQQHpiSWp2ampBahFMlomDU6qBybgwVF+s6/Fr73s6 Mc8nd9rb+ihvva+jUNxRvTIuP5Fjms2iWQsdv+QlFZ1J3C0ecWud28cu4anfXlV5/prVNP+u yE6Xc3LKJiu6tVSfPkk2/bT5xq70HT8vpPrfkZp7Y+eN5leKjvWG2oIzeQ+r1HHv3yUoyCf1 iG+3QQ/j5YAPHE/lqprnbArnL1683OSh5aejbtX1TL8kJZ7vuiTBtbFHaXXdpOkLVtt9XMhw keHC6tWv52axhK1bPE3i0D1lrdOZc7teTWvK6Mt+IhXfKRnE0bbb4Y7FhMSnVlbr38b6Vmy+ wOi16N3T20yr5N5s/3g8z0O+5vnKRcIfeZLV9/3TnO/D4X3vY9tZIe4WDiWW4oxEQy3mouJE AKIOTsTNAwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprLIsWRmVeSWpSXmKPExsWy7bCSnO7OaRcTDO5MVrU48P4gi8WDedvY LJoXr2ezmLLhA5NF/+PXzBYb3/5gstj0+BqrxeVdc9gsZpzfx2RxZnEvuwOXx6yGXjaPTas6 2Tz2z13D7rF5Sb1H35ZVjB7rt1xl8fi8SS6APYrLJiU1J7MstUjfLoEr4+6NCUwFV2wrrqy9 zdLAeMmwi5GTQ0LARGLfxXeMXYxcHEICuxklepddZ4VIiEtM+7WfEcIWllj57zk7RNETRom7 x3ezgyTYBHQlJh1sYwZJiAh0MUqsvXoCbBSzwAVGie6jN6BaFjFJ3F24jRmkhVPAUeLR1w6W LkYODmEBF4lbM7xBwiwCqhLLfk5jAbF5BSwlTv95yQxhC0qcnPkELM4soC3R+7CVEcZetvA1 M8R5ChI/ny4DO1tEwE/i99/77BA14hIvjx5hn8AoPAvJqFlIRs1CMmoWkpYFjCyrGCVTC4pz 03OLDQsM81LL9YoTc4tL89L1kvNzNzGCY1BLcwfj9lUf9A4xMnEwHmKU4GBWEuF9kX0xQYg3 JbGyKrUoP76oNCe1+BCjNAeLkjjvha6T8UIC6YklqdmpqQWpRTBZJg5OqQYmP403+9Mq6zvl 1UrWCTNJ3ljplP5h741bsTqRXfKuZ9w72xS3LHB6mOR8+ugJMbbnx723sUys4z5art4karfO R7332KqmBYIvWn9tmcx+ZPvF2acfJbc9ddj83X3Bp4aasDt2/3k/35juwLg6yaSNz2VD8eeJ oe8V/Yo7f11z7Vd8+vpc6Z57ObEsm/tu/32wRn3ZrK3r188Je1C22O3inlbBNW1MnK8nafqV NW7d2FwcZKfy/bXEDK+OE1e2iQvG6PE3rzw64aDUJrsQMe6ZTjOerTx05+7rNZabXlc+2mSd JPjw1hqnW5HTdXenJ+TPDLPf+cHF4fi7TaaRrhO3Sbe/nyNaKLX3rFeSsNPpTCWW4oxEQy3m ouJEACewtQUwAwAA X-CMS-MailID: 20210622075225epcas5p13270a3a17544fa110af21aaff82ceb84 X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-Sendblock-Type: REQ_APPROVE CMS-TYPE: 105P X-CMS-RootMailID: 20210621044517epcas5p187affa518a18a3d019deb0c189cd8396 References: <20210621044916.41564-1-m.shams@samsung.com> <8935a448-04b7-91ce-203a-9f0d7e377052@canonical.com> <004f01d766a0$567b9860$0372c920$@samsung.com> <4b2576c1-c986-a4d8-d6cf-661ca056ecee@canonical.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > > Hi, > > > >> > >> 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 =7C 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 > >>> =40=40 -284,8 +284,13 =40=40 static void s3c24xx_serial_stop_tx(struc= t > >>> uart_port > >> *port) > >>> struct s3c24xx_uart_dma *dma =3D ourport->dma; > >>> struct circ_buf *xmit =3D &port->state->xmit; > >>> struct dma_tx_state state; > >>> + struct device *dma_map_ops_dev =3D ourport->port.dev; > >>> int count; > >>> > >>> + /* Pick dma_ops of DMA device if DMA device is attached */ > >> > >> You mention here and further comments =22dma_ops=22. 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? > >> > > Yes, 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. >=20 > Thanks, this should be in commit msg. >=20 Sure, will add this in commit msg. > > > >> 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 =3D 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 DMA is not attached, this part of code related to dma_engine or DMA > > API do not get called. There will not be any DMA operations at all. >=20 > Now I get it. The =22When=22 in your description followed by multiple com= ments =22if > DMA device is attached=22 confused me that you expect to use UART device = for > DMA operations if DMA is not attached... >=20 I will change the comments, to avoid this confusion. > > > >>> if (=21ourport->tx_enabled) > >>> return; > >>> > >>> =40=40 -298,7 +303,7 =40=40 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 =3D dma->tx_bytes_requested - state.residue; =40=40 -324,15 > >> +329,19 > >>> =40=40 static void s3c24xx_serial_tx_dma_complete(void *args) > >>> struct circ_buf *xmit =3D &port->state->xmit; > >>> struct s3c24xx_uart_dma *dma =3D ourport->dma; > >>> struct dma_tx_state state; > >>> + struct device *dma_map_ops_dev =3D 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 =3D dma->tx_chan->device->dev; >=20 > Example is this one - you use here =22if=22 suggesting there is =22else= =22. So what is the > else condition? There is none... >=20 > >>> > >>> dmaengine_tx_status(dma->tx_chan, dma->tx_cookie, &state); > >>> count =3D 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); =40=40 -408,7 +417,11 =40=40= static > >>> int s3c24xx_serial_start_tx_dma(struct s3c24xx_uart_port *ourport, > >>> struct uart_port *port =3D &ourport->port; > >>> struct circ_buf *xmit =3D &port->state->xmit; > >>> struct s3c24xx_uart_dma *dma =3D ourport->dma; > >>> + struct device *dma_map_ops_dev =3D ourport->port.dev; > >>> > >>> + /* Pick dma_ops of DMA device if DMA device is attached */ > >>> + if (dma && dma->tx_chan) > >>> + dma_map_ops_dev =3D dma->tx_chan->device->dev; > >>> > >>> if (ourport->tx_mode =21=3D S3C24XX_TX_DMA) > >>> enable_tx_dma(ourport); > >>> =40=40 -416,7 +429,7 =40=40 static int s3c24xx_serial_start_tx_dma(st= ruct > >> s3c24xx_uart_port *ourport, > >>> dma->tx_size =3D count & =7E(dma_get_cache_alignment() - 1); > >>> dma->tx_transfer_addr =3D 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 =3D dmaengine_prep_slave_single(dma->tx_chan, > >>> =40=40 -483,12 +496,17 =40=40 static void s3c24xx_uart_copy_rx_to_tty= (struct > >> s3c24xx_uart_port *ourport, > >>> struct tty_port *tty, int count) > >>> =7B > >>> struct s3c24xx_uart_dma *dma =3D ourport->dma; > >>> + struct device *dma_map_ops_dev =3D 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 =3D dma->rx_chan->device->dev; > >>> + > >>> if (=21count) > >>> 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 +=3D count; > >>> =40=40 -600,8 +618,13 =40=40 static void s3c24xx_serial_rx_dma_comple= te(void > >>> *args) static void s3c64xx_start_rx_dma(struct s3c24xx_uart_port > >>> *ourport) =7B > >>> struct s3c24xx_uart_dma *dma =3D ourport->dma; > >>> + struct device *dma_map_ops_dev =3D 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 =3D 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 =3D dmaengine_prep_slave_single(dma->rx_chan, > >>> =40=40 -983,6 +1006,7 =40=40 static int s3c24xx_serial_request_dma(st= ruct > >>> 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 choo= se as > base? > >> > >> Using my email address not from get_maintainers.pl also suggests that > >> you don't use anything recent as a base. > >> > > I used =22master=22 branch of main linux-next tree as the base. > > I will rebase on =22tty-next=22 branch of TTY tree and post again. > > > > Thanks & Regards, > > Tamseel Shams > > >=20 >=20 > Best regards, > Krzysztof Thanks & Regards, Tamseel Shams 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=-14.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 D0593C2B9F4 for ; Tue, 22 Jun 2021 07:54:56 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 962BA60234 for ; Tue, 22 Jun 2021 07:54:56 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 962BA60234 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=samsung.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org 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:References:MIME-Version:Message-ID:Date :Subject:In-Reply-To:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=l182UJkzMhnKkbBcuG2BN2VljuSfb1iFGJSgKf/9YZc=; b=G+3e7+cbBIxuD3 M+Oue4bEZLsXhAUcz4hGXf9JcfZw458ixokF3+fqAPd30Mmk/kWikxo3c4E5pjFmxXKGO/xcPKVPY C+B8DfRWo20czCYFYPYsJLfv3F9yTNaFd4OasRKzil5TI7qxwxo0fr6ogR6IVcnjzRBTcDNMvhW+z XMGUhGkmLvXWfplv2l8c+ss6udE2UAwWUxhwO6yG2Xwx4xtjTPE9SnPgH0du/TwGuMMxlq3dgOIiO sCP6Gi3WY1ycK0zmeQrC7j6GZWqLgYWDaImE0m5FzdRc1VTTnPC1nbckWJCIO2PKCqdTcdEUwn7pW E19s9Nh+P2g/2DDZ6Byg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lvbDb-0067Fk-BO; Tue, 22 Jun 2021 07:53:11 +0000 Received: from desiato.infradead.org ([2001:8b0:10b:1:d65d:64ff:fe57:4e05]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1lvbDY-0067FO-KA for linux-arm-kernel@bombadil.infradead.org; Tue, 22 Jun 2021 07:53:08 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=References:Content-Type: Content-Transfer-Encoding:MIME-Version:Message-ID:Date:Subject:In-Reply-To:Cc :To:From:Sender:Reply-To:Content-ID:Content-Description; bh=b2T6MY1kZ9SohFnqiNwSCGWC99b+qKgTnX9g0ikzxy4=; b=QwP/khp7VWozhfWbEfcEQpcWy/ C5slC07pGcas3AImKMWTC8mrTqT6SXUBt5T25R9eg+wbqcsw01dsB4a4gegxLF13iUQeULtCWyMa7 9pw/Ww9yX00NJ4BHIUikbjVBXu4pZ4gROnInHprcxjreFVWXCI9h/9t4HM/YU6Y9xmSbhXuVFnGDr ksgpirNUQ7Bjkn6GLSMjPMjichBAztQUsPWpGriyNY8OobN3aqRh150L4psSXAlI7UrVGIEF0lRrD fNQbSb6+rX9KPykpKshNvluEKckYG/er3Xv9iSAzfJMvSbPuz60mXmdmF0i+9mePDjg/10gYfGl2Z ne3LQBdg==; Received: from mailout1.samsung.com ([203.254.224.24]) by desiato.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1lvbDL-00AWnG-0T for linux-arm-kernel@lists.infradead.org; Tue, 22 Jun 2021 07:53:06 +0000 Received: from epcas5p3.samsung.com (unknown [182.195.41.41]) by mailout1.samsung.com (KnoxPortal) with ESMTP id 20210622075241epoutp01dea99ae58b29d4eafdef348b7db4e018~K2Dp19knc0554905549epoutp01h for ; Tue, 22 Jun 2021 07:52:41 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout1.samsung.com 20210622075241epoutp01dea99ae58b29d4eafdef348b7db4e018~K2Dp19knc0554905549epoutp01h DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1624348361; bh=b2T6MY1kZ9SohFnqiNwSCGWC99b+qKgTnX9g0ikzxy4=; h=From:To:Cc:In-Reply-To:Subject:Date:References:From; b=SaagilmJU04NfgT1p9x30kLtXqAbRHn/vrcoutfK+Y+s8srA/7yFkj6lN8wkKWy2p 6XM5omcWH8cn2kgEkUudWgfMl3WCJRqeLZ9S8Ll0/pcL5v5NOr0uUXvIbnCbmu1mqC wTjpk7DxjlHcJiXpnmrvmWnB4SYr3cquwsg6roek= Received: from epsmges5p1new.samsung.com (unknown [182.195.42.73]) by epcas5p3.samsung.com (KnoxPortal) with ESMTP id 20210622075240epcas5p3df72ffba15fb235adbb38455a18cf33b~K2DpMu8Tr2865028650epcas5p3g; Tue, 22 Jun 2021 07:52:40 +0000 (GMT) Received: from epcas5p3.samsung.com ( [182.195.41.41]) by epsmges5p1new.samsung.com (Symantec Messaging Gateway) with SMTP id 04.F6.09476.8C691D06; Tue, 22 Jun 2021 16:52:40 +0900 (KST) Received: from epsmtrp2.samsung.com (unknown [182.195.40.14]) by epcas5p1.samsung.com (KnoxPortal) with ESMTPA id 20210622075225epcas5p13270a3a17544fa110af21aaff82ceb84~K2Dbc4heu2192421924epcas5p11; Tue, 22 Jun 2021 07:52:25 +0000 (GMT) Received: from epsmgms1p1new.samsung.com (unknown [182.195.42.41]) by epsmtrp2.samsung.com (KnoxPortal) with ESMTP id 20210622075225epsmtrp2e0157be39c97fe9c09a7116858b0408f~K2DbcFLR52613926139epsmtrp2a; Tue, 22 Jun 2021 07:52:25 +0000 (GMT) X-AuditID: b6c32a49-6a1ff70000002504-6f-60d196c8341a Received: from epsmtip1.samsung.com ( [182.195.34.30]) by epsmgms1p1new.samsung.com (Symantec Messaging Gateway) with SMTP id D4.DB.08394.9B691D06; Tue, 22 Jun 2021 16:52:25 +0900 (KST) Received: from mshams01 (unknown [107.122.12.94]) by epsmtip1.samsung.com (KnoxPortal) with ESMTPA id 20210622075224epsmtip123273eb378191cff03467278d9b125f7~K2DZ49-Lm1537015370epsmtip1o; Tue, 22 Jun 2021 07:52:23 +0000 (GMT) From: "M Tamseel Shams" To: "'Krzysztof Kozlowski'" , , , Cc: , , , , , In-Reply-To: <4b2576c1-c986-a4d8-d6cf-661ca056ecee@canonical.com> Subject: RE: [PATCH] serial: samsung: use dma_ops of DMA if attached Date: Tue, 22 Jun 2021 13:22:22 +0530 Message-ID: <007901d7673b$8a758d10$9f60a730$@samsung.com> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQGDfL5lNG5rzFQPe+FwbDp9GpLyVQGDsg9dAbmelg0Cc6DxagIMN/A2q4mTjPA= Content-Language: en-us X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFtrFKsWRmVeSWpSXmKPExsWy7bCmpu6JaRcTDHo3S1kceH+QxeLBvG1s Fs2L17NZTNnwgcmi//FrZouNb38wWWx6fI3V4vKuOWwWM87vY7I4s7iX3YHLY1ZDL5vHplWd bB77565h99i8pN6jb8sqRo/1W66yeHzeJBfAHsVlk5Kak1mWWqRvl8CVcfTORdaCDXYVcx6c Y2pg3GXUxcjJISFgIrFqw2SWLkYuDiGB3YwSzXcOsUM4nxglGvadYoNwvjFKfLrwlQWmZd/F d4wQib2MEo86nkI5zxglfj5ZyQhSxSagKzHpYBszSEJEoI1RYnpDBxOIwyxwhlHi8eLJTCBV nAKOEo++dgDN5eAQFnCRuDXDGyTMIqAqseznNLB1vAKWEqf/vGSGsAUlTs58AhZnFtCWWLbw NTPESQoSP58uYwWxRQT8JH7/vc8OUSMu8fLoEbCHJAQOcEjsmHiFFaLBRaJhdQsbhC0s8er4 FnYIW0ri87u9UPF8ifnzVkEtqJBYeeENlG0vceDKHLCbmQU0Jdbv0ocIy0pMPbWOCWIvn0Tv 7ydMEHFeiR3zYGxFif+7+6FWiUu8WzGFdQKj0iwkr81C8tosJC/MQti2gJFlFaNkakFxbnpq sWmBYV5quV5xYm5xaV66XnJ+7iZGcPLS8tzBePfBB71DjEwcjIcYJTiYlUR4X2RfTBDiTUms rEotyo8vKs1JLT7EKM3BoiTOu5T9UIKQQHpiSWp2ampBahFMlomDU6qBybgwVF+s6/Fr73s6 Mc8nd9rb+ihvva+jUNxRvTIuP5Fjms2iWQsdv+QlFZ1J3C0ecWud28cu4anfXlV5/prVNP+u yE6Xc3LKJiu6tVSfPkk2/bT5xq70HT8vpPrfkZp7Y+eN5leKjvWG2oIzeQ+r1HHv3yUoyCf1 iG+3QQ/j5YAPHE/lqprnbArnL1683OSh5aejbtX1TL8kJZ7vuiTBtbFHaXXdpOkLVtt9XMhw keHC6tWv52axhK1bPE3i0D1lrdOZc7teTWvK6Mt+IhXfKRnE0bbb4Y7FhMSnVlbr38b6Vmy+ wOi16N3T20yr5N5s/3g8z0O+5vnKRcIfeZLV9/3TnO/D4X3vY9tZIe4WDiWW4oxEQy3mouJE AKIOTsTNAwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprLIsWRmVeSWpSXmKPExsWy7bCSnO7OaRcTDO5MVrU48P4gi8WDedvY LJoXr2ezmLLhA5NF/+PXzBYb3/5gstj0+BqrxeVdc9gsZpzfx2RxZnEvuwOXx6yGXjaPTas6 2Tz2z13D7rF5Sb1H35ZVjB7rt1xl8fi8SS6APYrLJiU1J7MstUjfLoEr4+6NCUwFV2wrrqy9 zdLAeMmwi5GTQ0LARGLfxXeMXYxcHEICuxklepddZ4VIiEtM+7WfEcIWllj57zk7RNETRom7 x3ezgyTYBHQlJh1sYwZJiAh0MUqsvXoCbBSzwAVGie6jN6BaFjFJ3F24jRmkhVPAUeLR1w6W LkYODmEBF4lbM7xBwiwCqhLLfk5jAbF5BSwlTv95yQxhC0qcnPkELM4soC3R+7CVEcZetvA1 M8R5ChI/ny4DO1tEwE/i99/77BA14hIvjx5hn8AoPAvJqFlIRs1CMmoWkpYFjCyrGCVTC4pz 03OLDQsM81LL9YoTc4tL89L1kvNzNzGCY1BLcwfj9lUf9A4xMnEwHmKU4GBWEuF9kX0xQYg3 JbGyKrUoP76oNCe1+BCjNAeLkjjvha6T8UIC6YklqdmpqQWpRTBZJg5OqQYmP403+9Mq6zvl 1UrWCTNJ3ljplP5h741bsTqRXfKuZ9w72xS3LHB6mOR8+ugJMbbnx723sUys4z5art4karfO R7332KqmBYIvWn9tmcx+ZPvF2acfJbc9ddj83X3Bp4aasDt2/3k/35juwLg6yaSNz2VD8eeJ oe8V/Yo7f11z7Vd8+vpc6Z57ObEsm/tu/32wRn3ZrK3r188Je1C22O3inlbBNW1MnK8nafqV NW7d2FwcZKfy/bXEDK+OE1e2iQvG6PE3rzw64aDUJrsQMe6ZTjOerTx05+7rNZabXlc+2mSd JPjw1hqnW5HTdXenJ+TPDLPf+cHF4fi7TaaRrhO3Sbe/nyNaKLX3rFeSsNPpTCWW4oxEQy3m ouJEACewtQUwAwAA X-CMS-MailID: 20210622075225epcas5p13270a3a17544fa110af21aaff82ceb84 X-Msg-Generator: CA X-Sendblock-Type: REQ_APPROVE CMS-TYPE: 105P X-CMS-RootMailID: 20210621044517epcas5p187affa518a18a3d019deb0c189cd8396 References: <20210621044916.41564-1-m.shams@samsung.com> <8935a448-04b7-91ce-203a-9f0d7e377052@canonical.com> <004f01d766a0$567b9860$0372c920$@samsung.com> <4b2576c1-c986-a4d8-d6cf-661ca056ecee@canonical.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210622_085303_595542_4C78B0A5 X-CRM114-Status: GOOD ( 44.80 ) 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="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org > > Hi, > > > >> > >> 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? > >> > > Yes, 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. > > Thanks, this should be in commit msg. > Sure, will add this in commit msg. > > > >> 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 DMA is not attached, this part of code related to dma_engine or DMA > > API do not get called. There will not be any DMA operations at all. > > Now I get it. The "When" in your description followed by multiple comments "if > DMA device is attached" confused me that you expect to use UART device for > DMA operations if DMA is not attached... > I will change the comments, to avoid this confusion. > > > >>> 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; > > Example is this one - you use here "if" suggesting there is "else". So what is the > else condition? There is none... > > >>> > >>> 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. > >> > > I used "master" branch of main linux-next tree as the base. > > I will rebase on "tty-next" branch of TTY tree and post again. > > > > Thanks & Regards, > > Tamseel Shams > > > > > Best regards, > Krzysztof Thanks & Regards, Tamseel Shams _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel