Message ID | 20230914121300.845493-1-w.egorov@phytec.de |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:172:b0:3f2:4152:657d with SMTP id h50csp306294vqi; Thu, 14 Sep 2023 05:20:38 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHtd3Bk2GMAM09MUOd3gzLXPMP81CSQn0s7iY6AfSjhtdYwApk4ChK1RTiFSEJ0SK6HXPSr X-Received: by 2002:a17:902:7488:b0:1c3:ebfc:5198 with SMTP id h8-20020a170902748800b001c3ebfc5198mr3772586pll.11.1694694038604; Thu, 14 Sep 2023 05:20:38 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1694694038; cv=none; d=google.com; s=arc-20160816; b=MGPf3s9XUixbtcAxpqxQdWwgR442HO912QIIl3KA0y/hYunpOKY/e3+8ljem9OEZc4 tRGW0gFwucblpF0Gc+gGZdTdv7Dz3SGcZff0rt+mE9x1nk2BKcpsMn/g67xBvOiLWo55 BhUq5lm59rbiTXfTc5TIywWWcDgIqQ04W2sk5B7ti4fCZGot3R6+F5AzOSP9hfwmtQMn DwbVUAk1GCz8fy+RnqUrvWlLogHZIeTrmpvHYhUkg4N/079kRaLVF72NRW/eMDXUoebh kw0zTICB3gDf7yl1BOD2SVgPH/wIpqKUAW4rLfPaQx2a5CIt4TVmFTOIZ6pkrSen5qaV FHtw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=ETD3xM2ZBKZbiQCDs/sZ2/dtvMuFjr8psnKM9WSeTU8=; fh=DKYvAVOTEgdZS/5sIi2EaSdEVZk9fXp2PqUR8vv8wS0=; b=k9iQjO2/Fv4/MNjm3K4N+jH5M+yExBVtNfXV/kY28hLryFBEEAWOBNeeBMLhUcxJ7B APf5euqF0nuwhT4ULG+9bUmBm/v5MfNKRJiJPh/HOImVK8n+BYTyIH143EQyHNZlp5cL hArZQdzzRtYTom45C/bx9+zcH+apOpjJILt5QklPEWjjUdXogQJHMylBjrle/o9U91kI 2kHSUrmzAJFM2/nsHcm9G61OZ+ThmsmAxPjjrMO9X5mDafvrHlMHle4ULFNDU6K3N4IV o+O+F2m9MINeecbb8at2GgIToUAaHosTb1kTdMS1el06pGK+trJ6Te+7PCS1y65NrEZv nYnw== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@phytec.de header.s=a4 header.b=bbmq2LS4; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from snail.vger.email (snail.vger.email. [23.128.96.37]) by mx.google.com with ESMTPS id x6-20020a170902a38600b001c370dc4445si1615952pla.210.2023.09.14.05.20.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Sep 2023 05:20:38 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37; Authentication-Results: mx.google.com; dkim=fail header.i=@phytec.de header.s=a4 header.b=bbmq2LS4; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id 64847819DFE0; Thu, 14 Sep 2023 05:13:21 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234522AbjINMNR (ORCPT <rfc822;chrisfriedt@gmail.com> + 35 others); Thu, 14 Sep 2023 08:13:17 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38012 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S238254AbjINMNQ (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 14 Sep 2023 08:13:16 -0400 Received: from mickerik.phytec.de (mickerik.phytec.de [91.26.50.163]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DCFC71FC0 for <linux-kernel@vger.kernel.org>; Thu, 14 Sep 2023 05:13:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; d=phytec.de; s=a4; c=relaxed/simple; q=dns/txt; i=@phytec.de; t=1694693589; x=1697285589; h=From:Sender:Reply-To:Subject:Date:Message-ID:To:CC:MIME-Version:Content-Type: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=s3tNlvvUmtI8ptMGyvvsPrGgLl/WPnVCwK8f76krFqY=; b=bbmq2LS4zDUab/IAhQWcG6misYXXjdTYHQ9KbY866Kral3cZFU00btdPcNXuHRdY Bre+ga0ZSbi+P3675V8EpulNsnMk9GfbUTs0P4IWV7l5rw4Ubr/1qvK/w9J5RkVb 0sd1gmdQ9u5cpk2O8YgVXQDnpLS/eTmhSwjsKy4sB9Q=; X-AuditID: ac14000a-6e25770000001e37-a5-6502f8d47b91 Received: from berlix.phytec.de (Unknown_Domain [172.25.0.12]) (using TLS with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mickerik.phytec.de (PHYTEC Mail Gateway) with SMTP id 8D.75.07735.4D8F2056; Thu, 14 Sep 2023 14:13:09 +0200 (CEST) Received: from augenblix2.phytec.de (172.25.0.11) by Berlix.phytec.de (172.25.0.12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.18; Thu, 14 Sep 2023 14:13:08 +0200 From: Wadim Egorov <w.egorov@phytec.de> To: <jic23@kernel.org>, <lars@metafoo.de>, <robh@kernel.org>, <heiko@sntech.de>, <mugunthanvnm@ti.com>, <peter.ujfalusi@ti.com> CC: <linux-iio@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <upstream@lists.phytec.de>, <nm@ti.com> Subject: [PATCH] iio: adc: ti_am335x_adc: Make DMAs optional Date: Thu, 14 Sep 2023 14:13:00 +0200 Message-ID: <20230914121300.845493-1-w.egorov@phytec.de> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Originating-IP: [172.25.0.11] X-ClientProxiedBy: Berlix.phytec.de (172.25.0.12) To Berlix.phytec.de (172.25.0.12) X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrDLMWRmVeSWpSXmKPExsWyRpKBR/fqD6ZUgz49i/+PXrNaPGhaxWSx ZPJ8Vot5R96xWFzeNYfN4uqDcos3P84yWcz6+IPV4v+eHewW3e/UHbg8Nq3qZPPo725h9Vjy 5hCrx/Zr85g9jt/YzuTxeZNcAFsUl01Kak5mWWqRvl0CV8aTLYcYC05wVVydHdfAeJKji5GT Q0LAROJfzxPWLkYuDiGBJUwSredPsEE4Txgldl2fxwRSxSagLnFnwzewKhGBRkaJC5umM4Ik mAWyJI6tPgBmCwvYSEzctwPMZhFQlfjUd44VxOYVsJBYceI+C8Q6eYmZl76zQ8QFJU7OfMIC MUdeonnrbGYIW0Li4IsXYLYQUPzFpeVwvdPOvWaGsEMltn7ZzjSBUWAWklGzkIyahWTUAkbm VYxCuZnJ2alFmdl6BRmVJanJeimpmxhBMSDCwLWDsW+OxyFGJg7GQ4wSHMxKIrxstkypQrwp iZVVqUX58UWlOanFhxilOViUxHnv9zAlCgmkJ5akZqemFqQWwWSZODilGhinbmT0TdrOUBZz cb3kzuos/iUm9gvnTDpR0RW0uGwO3xlvhYSpYSc0XoUd4mt+/23xcyXZc7dc8gS/bXGevmlG 8968nYEPVveZJWXk2oa5aGy6EW0isnQ2p7yL8JWFzU8zvbN7aubGKqT43azguXzu9hq/fYvO pV5dUDdb21FdaufWlEadDRJKLMUZiYZazEXFiQAEjkT+bwIAAA== Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Thu, 14 Sep 2023 05:13:21 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1777015496544989852 X-GMAIL-MSGID: 1777015496544989852 |
Series |
iio: adc: ti_am335x_adc: Make DMAs optional
|
|
Commit Message
Wadim Egorov
Sept. 14, 2023, 12:13 p.m. UTC
DMAs are optional. Even if the DMA request is unsuccessfully,
the ADC can still work properly.
Make tiadc_request_dma() not fail if we do not provide dmas &
dma-names properties.
This actually fixes the wrong error handling of the tiadc_request_dma()
result where the probing only failed if -EPROPE_DEFER was returned.
Fixes: f438b9da75eb ("drivers: iio: ti_am335x_adc: add dma support")
Signed-off-by: Wadim Egorov <w.egorov@phytec.de>
---
drivers/iio/adc/ti_am335x_adc.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
Comments
On Thu, 14 Sep 2023 14:13:00 +0200 Wadim Egorov <w.egorov@phytec.de> wrote: > DMAs are optional. Even if the DMA request is unsuccessfully, > the ADC can still work properly. > Make tiadc_request_dma() not fail if we do not provide dmas & > dma-names properties. > > This actually fixes the wrong error handling of the tiadc_request_dma() > result where the probing only failed if -EPROPE_DEFER was returned. > > Fixes: f438b9da75eb ("drivers: iio: ti_am335x_adc: add dma support") > No line break here. Fixes tag is part of the main tag block. > Signed-off-by: Wadim Egorov <w.egorov@phytec.de> > --- > drivers/iio/adc/ti_am335x_adc.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c > index 8db7a01cb5fb..e14aa9254ab1 100644 > --- a/drivers/iio/adc/ti_am335x_adc.c > +++ b/drivers/iio/adc/ti_am335x_adc.c > @@ -543,8 +543,11 @@ static int tiadc_request_dma(struct platform_device *pdev, > if (IS_ERR(dma->chan)) { > int ret = PTR_ERR(dma->chan); > > + if (ret != -ENODEV) > + return dev_err_probe(&pdev->dev, ret, > + "RX DMA channel request failed\n"); > dma->chan = NULL; > - return ret; > + return 0; > } > > /* RX buffer */ > @@ -670,7 +673,7 @@ static int tiadc_probe(struct platform_device *pdev) > platform_set_drvdata(pdev, indio_dev); > > err = tiadc_request_dma(pdev, adc_dev); > - if (err && err == -EPROBE_DEFER) > + if (err) So this looks like a more subtle change than you are describing. In the original code, we backed off only if the return was a PROBE_DEFER, otherwise we carried on. Your change seems to make that happen for any non -ENODEV error, including PROBE_DEFER. That's fine, but it's not what the description implies. Whilst tiadc_request_dma will fail today if the dmas etc is not provided, that seems like correct behavior to me. A function requesting dma fails if it isn't available. The handling of whether to carry on the job for the caller. So I think it should just be if (err && err != -EINVAL) goto err_dma; and no change in tiadc_request_dma() However, the case you describe should have worked find with existing code as it wasn't -EPROBE_DEFER, so I don't understand why you were looking at this code block in the first place? Jonathan > goto err_dma; > > return 0;
On 14/09/23 17:43, Wadim Egorov wrote: > DMAs are optional. Even if the DMA request is unsuccessfully, > the ADC can still work properly. > Make tiadc_request_dma() not fail if we do not provide dmas & > dma-names properties. > > This actually fixes the wrong error handling of the tiadc_request_dma() > result where the probing only failed if -EPROPE_DEFER was returned. > > Fixes: f438b9da75eb ("drivers: iio: ti_am335x_adc: add dma support") > > Signed-off-by: Wadim Egorov <w.egorov@phytec.de> > --- > drivers/iio/adc/ti_am335x_adc.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c > index 8db7a01cb5fb..e14aa9254ab1 100644 > --- a/drivers/iio/adc/ti_am335x_adc.c > +++ b/drivers/iio/adc/ti_am335x_adc.c > @@ -543,8 +543,11 @@ static int tiadc_request_dma(struct platform_device *pdev, > if (IS_ERR(dma->chan)) { > int ret = PTR_ERR(dma->chan); > > + if (ret != -ENODEV) Hi Wadim, Thanks for adding this fix but now there is no deferred probe mechanism available here for the driver. We need to include that as well. Regards ~B-Kapoor > + return dev_err_probe(&pdev->dev, ret, > + "RX DMA channel request failed\n"); > dma->chan = NULL; > - return ret; > + return 0; > } > > /* RX buffer */ > @@ -670,7 +673,7 @@ static int tiadc_probe(struct platform_device *pdev) > platform_set_drvdata(pdev, indio_dev); > > err = tiadc_request_dma(pdev, adc_dev); > - if (err && err == -EPROBE_DEFER) > + if (err) > goto err_dma; > > return 0; > -- > 2.25.1 > > >
Hi Jonathan, Am 17.09.23 um 12:45 schrieb Jonathan Cameron: > On Thu, 14 Sep 2023 14:13:00 +0200 > Wadim Egorov <w.egorov@phytec.de> wrote: > >> DMAs are optional. Even if the DMA request is unsuccessfully, >> the ADC can still work properly. >> Make tiadc_request_dma() not fail if we do not provide dmas & >> dma-names properties. >> >> This actually fixes the wrong error handling of the tiadc_request_dma() >> result where the probing only failed if -EPROPE_DEFER was returned. >> >> Fixes: f438b9da75eb ("drivers: iio: ti_am335x_adc: add dma support") >> > No line break here. Fixes tag is part of the main tag block. >> Signed-off-by: Wadim Egorov <w.egorov@phytec.de> > >> --- >> drivers/iio/adc/ti_am335x_adc.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c >> index 8db7a01cb5fb..e14aa9254ab1 100644 >> --- a/drivers/iio/adc/ti_am335x_adc.c >> +++ b/drivers/iio/adc/ti_am335x_adc.c >> @@ -543,8 +543,11 @@ static int tiadc_request_dma(struct platform_device *pdev, >> if (IS_ERR(dma->chan)) { >> int ret = PTR_ERR(dma->chan); >> >> + if (ret != -ENODEV) >> + return dev_err_probe(&pdev->dev, ret, >> + "RX DMA channel request failed\n"); >> dma->chan = NULL; >> - return ret; >> + return 0; >> } >> >> /* RX buffer */ >> @@ -670,7 +673,7 @@ static int tiadc_probe(struct platform_device *pdev) >> platform_set_drvdata(pdev, indio_dev); >> >> err = tiadc_request_dma(pdev, adc_dev); >> - if (err && err == -EPROBE_DEFER) >> + if (err) > So this looks like a more subtle change than you are describing. > In the original code, we backed off only if the return was a PROBE_DEFER, otherwise > we carried on. > > Your change seems to make that happen for any non -ENODEV error, including PROBE_DEFER. > That's fine, but it's not what the description implies. > > Whilst tiadc_request_dma will fail today if the dmas etc is not provided, that seems > like correct behavior to me. A function requesting dma fails if it isn't available. > The handling of whether to carry on the job for the caller. That makes sense, yes. But stm32-adc is doing the same in its dma request function. So I assumed we can do it like that. > > So I think it should just be > if (err && err != -EINVAL) > goto err_dma; We will end up failing if no dmas are configured because the request returns -ENODEV. So I think it needs to be a check for non -ENODEV. > > and no change in tiadc_request_dma() > > However, the case you describe should have worked find with existing code > as it wasn't -EPROBE_DEFER, so I don't understand why you were looking at this > code block in the first place? Providing wrong dmas in the device tree should've made the driver fail to probe. Regards, Wadim > > Jonathan > > >> goto err_dma; >> >> return 0;
On Tue, 19 Sep 2023 10:21:28 +0000 Wadim Egorov <W.Egorov@phytec.de> wrote: > Hi Jonathan, > > Am 17.09.23 um 12:45 schrieb Jonathan Cameron: > > On Thu, 14 Sep 2023 14:13:00 +0200 > > Wadim Egorov <w.egorov@phytec.de> wrote: > > > >> DMAs are optional. Even if the DMA request is unsuccessfully, > >> the ADC can still work properly. > >> Make tiadc_request_dma() not fail if we do not provide dmas & > >> dma-names properties. > >> > >> This actually fixes the wrong error handling of the tiadc_request_dma() > >> result where the probing only failed if -EPROPE_DEFER was returned. > >> > >> Fixes: f438b9da75eb ("drivers: iio: ti_am335x_adc: add dma support") > >> > > No line break here. Fixes tag is part of the main tag block. > >> Signed-off-by: Wadim Egorov <w.egorov@phytec.de> > > > >> --- > >> drivers/iio/adc/ti_am335x_adc.c | 7 +++++-- > >> 1 file changed, 5 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c > >> index 8db7a01cb5fb..e14aa9254ab1 100644 > >> --- a/drivers/iio/adc/ti_am335x_adc.c > >> +++ b/drivers/iio/adc/ti_am335x_adc.c > >> @@ -543,8 +543,11 @@ static int tiadc_request_dma(struct platform_device *pdev, > >> if (IS_ERR(dma->chan)) { > >> int ret = PTR_ERR(dma->chan); > >> > >> + if (ret != -ENODEV) > >> + return dev_err_probe(&pdev->dev, ret, > >> + "RX DMA channel request failed\n"); > >> dma->chan = NULL; > >> - return ret; > >> + return 0; > >> } > >> > >> /* RX buffer */ > >> @@ -670,7 +673,7 @@ static int tiadc_probe(struct platform_device *pdev) > >> platform_set_drvdata(pdev, indio_dev); > >> > >> err = tiadc_request_dma(pdev, adc_dev); > >> - if (err && err == -EPROBE_DEFER) > >> + if (err) > > So this looks like a more subtle change than you are describing. > > In the original code, we backed off only if the return was a PROBE_DEFER, otherwise > > we carried on. > > > > Your change seems to make that happen for any non -ENODEV error, including PROBE_DEFER. > > That's fine, but it's not what the description implies. > > > > Whilst tiadc_request_dma will fail today if the dmas etc is not provided, that seems > > like correct behavior to me. A function requesting dma fails if it isn't available. > > The handling of whether to carry on the job for the caller. > > That makes sense, yes. But stm32-adc is doing the same in its dma > request function. > So I assumed we can do it like that. > > > > > So I think it should just be > > if (err && err != -EINVAL) > > goto err_dma; > > We will end up failing if no dmas are configured because the request > returns -ENODEV. > So I think it needs to be a check for non -ENODEV. That makes sense. I wonder if a long time back that returned -EINVAL, hence the wrong value here. If you can do a bit of checking in the git history that would be good as it will change how far we backport this. > > > > > and no change in tiadc_request_dma() > > > > However, the case you describe should have worked find with existing code > > as it wasn't -EPROBE_DEFER, so I don't understand why you were looking at this > > code block in the first place? > > Providing wrong dmas in the device tree should've made the driver fail > to probe. Agreed, Thanks, Jonathan > > Regards, > Wadim > > > > > Jonathan > > > > > >> goto err_dma; > >> > >> return 0; > >
diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c index 8db7a01cb5fb..e14aa9254ab1 100644 --- a/drivers/iio/adc/ti_am335x_adc.c +++ b/drivers/iio/adc/ti_am335x_adc.c @@ -543,8 +543,11 @@ static int tiadc_request_dma(struct platform_device *pdev, if (IS_ERR(dma->chan)) { int ret = PTR_ERR(dma->chan); + if (ret != -ENODEV) + return dev_err_probe(&pdev->dev, ret, + "RX DMA channel request failed\n"); dma->chan = NULL; - return ret; + return 0; } /* RX buffer */ @@ -670,7 +673,7 @@ static int tiadc_probe(struct platform_device *pdev) platform_set_drvdata(pdev, indio_dev); err = tiadc_request_dma(pdev, adc_dev); - if (err && err == -EPROBE_DEFER) + if (err) goto err_dma; return 0;