dmaengine Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 1/6] dma: amba-pl08x: no need to cast away call to debugfs_create_file()
@ 2019-06-12 12:25 Greg Kroah-Hartman
  2019-06-12 12:25 ` [PATCH 2/6] dma: bcm-sba-raid: no need to check return value of debugfs_create functions Greg Kroah-Hartman
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-12 12:25 UTC (permalink / raw)
  To: dan.j.williams, vkoul; +Cc: Greg Kroah-Hartman, dmaengine, linux-kernel

No need to check the return value of debugfs_create_file(), so no need
to provide a fake "cast away" of the return value either.

Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: dmaengine@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/dma/amba-pl08x.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c
index 464725dcad00..9adc7a2fa3d3 100644
--- a/drivers/dma/amba-pl08x.c
+++ b/drivers/dma/amba-pl08x.c
@@ -2508,9 +2508,8 @@ DEFINE_SHOW_ATTRIBUTE(pl08x_debugfs);
 static void init_pl08x_debugfs(struct pl08x_driver_data *pl08x)
 {
 	/* Expose a simple debugfs interface to view all clocks */
-	(void) debugfs_create_file(dev_name(&pl08x->adev->dev),
-			S_IFREG | S_IRUGO, NULL, pl08x,
-			&pl08x_debugfs_fops);
+	debugfs_create_file(dev_name(&pl08x->adev->dev), S_IFREG | S_IRUGO,
+			    NULL, pl08x, &pl08x_debugfs_fops);
 }
 
 #else
-- 
2.22.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 2/6] dma: bcm-sba-raid: no need to check return value of debugfs_create functions
  2019-06-12 12:25 [PATCH 1/6] dma: amba-pl08x: no need to cast away call to debugfs_create_file() Greg Kroah-Hartman
@ 2019-06-12 12:25 ` Greg Kroah-Hartman
  2019-06-12 12:25 ` [PATCH 3/6] dma: coh901318: no need to cast away call to debugfs_create_file() Greg Kroah-Hartman
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-12 12:25 UTC (permalink / raw)
  To: dan.j.williams, vkoul; +Cc: Greg Kroah-Hartman, dmaengine, linux-kernel

When calling debugfs functions, there is no need to ever check the
return value.  The function can work or not, but the code logic should
never do something different based on this.

Also, because there is no need to save the file dentry, remove the
variable that was saving it as it was never even being used once set.

Cc: Vinod Koul <vkoul@kernel.org>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: dmaengine@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/dma/bcm-sba-raid.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/dma/bcm-sba-raid.c b/drivers/dma/bcm-sba-raid.c
index fa81d0177765..275e90fa829d 100644
--- a/drivers/dma/bcm-sba-raid.c
+++ b/drivers/dma/bcm-sba-raid.c
@@ -164,7 +164,6 @@ struct sba_device {
 	struct list_head reqs_free_list;
 	/* DebugFS directory entries */
 	struct dentry *root;
-	struct dentry *stats;
 };
 
 /* ====== Command helper routines ===== */
@@ -1716,17 +1715,11 @@ static int sba_probe(struct platform_device *pdev)
 
 	/* Create debugfs root entry */
 	sba->root = debugfs_create_dir(dev_name(sba->dev), NULL);
-	if (IS_ERR_OR_NULL(sba->root)) {
-		dev_err(sba->dev, "failed to create debugfs root entry\n");
-		sba->root = NULL;
-		goto skip_debugfs;
-	}
 
 	/* Create debugfs stats entry */
-	sba->stats = debugfs_create_devm_seqfile(sba->dev, "stats", sba->root,
-						 sba_debugfs_stats_show);
-	if (IS_ERR_OR_NULL(sba->stats))
-		dev_err(sba->dev, "failed to create debugfs stats file\n");
+	debugfs_create_devm_seqfile(sba->dev, "stats", sba->root,
+				    sba_debugfs_stats_show);
+
 skip_debugfs:
 
 	/* Register DMA device with Linux async framework */
-- 
2.22.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 3/6] dma: coh901318: no need to cast away call to debugfs_create_file()
  2019-06-12 12:25 [PATCH 1/6] dma: amba-pl08x: no need to cast away call to debugfs_create_file() Greg Kroah-Hartman
  2019-06-12 12:25 ` [PATCH 2/6] dma: bcm-sba-raid: no need to check return value of debugfs_create functions Greg Kroah-Hartman
@ 2019-06-12 12:25 ` Greg Kroah-Hartman
  2019-06-12 12:42   ` Linus Walleij
  2019-06-12 12:25 ` [PATCH 4/6] dma: pxa_dma: no need to check return value of debugfs_create functions Greg Kroah-Hartman
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-12 12:25 UTC (permalink / raw)
  To: dan.j.williams, vkoul
  Cc: Greg Kroah-Hartman, Linus Walleij, linux-arm-kernel, dmaengine,
	linux-kernel

No need to check the return value of debugfs_create_file(), so no need
to provide a fake "cast away" of the return value either.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: dmaengine@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/dma/coh901318.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/coh901318.c b/drivers/dma/coh901318.c
index 547786ac342b..e51d836afcc7 100644
--- a/drivers/dma/coh901318.c
+++ b/drivers/dma/coh901318.c
@@ -1378,10 +1378,8 @@ static int __init init_coh901318_debugfs(void)
 
 	dma_dentry = debugfs_create_dir("dma", NULL);
 
-	(void) debugfs_create_file("status",
-				   S_IFREG | S_IRUGO,
-				   dma_dentry, NULL,
-				   &coh901318_debugfs_status_operations);
+	debugfs_create_file("status", S_IFREG | S_IRUGO, dma_dentry, NULL,
+			    &coh901318_debugfs_status_operations);
 	return 0;
 }
 
-- 
2.22.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 4/6] dma: pxa_dma: no need to check return value of debugfs_create functions
  2019-06-12 12:25 [PATCH 1/6] dma: amba-pl08x: no need to cast away call to debugfs_create_file() Greg Kroah-Hartman
  2019-06-12 12:25 ` [PATCH 2/6] dma: bcm-sba-raid: no need to check return value of debugfs_create functions Greg Kroah-Hartman
  2019-06-12 12:25 ` [PATCH 3/6] dma: coh901318: no need to cast away call to debugfs_create_file() Greg Kroah-Hartman
@ 2019-06-12 12:25 ` Greg Kroah-Hartman
  2019-06-12 12:25 ` [PATCH 5/6] dma: mic_x100_dma: " Greg Kroah-Hartman
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-12 12:25 UTC (permalink / raw)
  To: dan.j.williams, vkoul
  Cc: Greg Kroah-Hartman, Daniel Mack, Haojian Zhuang, Robert Jarzmik,
	linux-arm-kernel, dmaengine, linux-kernel

When calling debugfs functions, there is no need to ever check the
return value.  The function can work or not, but the code logic should
never do something different based on this.

Also, because there is no need to save the file dentry, remove the
variable that was saving it as it was never even being used once set.

Cc: Daniel Mack <daniel@zonque.org>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>
Cc: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: dmaengine@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/dma/pxa_dma.c | 56 +++++++++----------------------------------
 1 file changed, 11 insertions(+), 45 deletions(-)

diff --git a/drivers/dma/pxa_dma.c b/drivers/dma/pxa_dma.c
index b429642f3e7a..0f698f49ee26 100644
--- a/drivers/dma/pxa_dma.c
+++ b/drivers/dma/pxa_dma.c
@@ -132,7 +132,6 @@ struct pxad_device {
 	spinlock_t			phy_lock;	/* Phy association */
 #ifdef CONFIG_DEBUG_FS
 	struct dentry			*dbgfs_root;
-	struct dentry			*dbgfs_state;
 	struct dentry			**dbgfs_chan;
 #endif
 };
@@ -326,31 +325,18 @@ static struct dentry *pxad_dbg_alloc_chan(struct pxad_device *pdev,
 					     int ch, struct dentry *chandir)
 {
 	char chan_name[11];
-	struct dentry *chan, *chan_state = NULL, *chan_descr = NULL;
-	struct dentry *chan_reqs = NULL;
+	struct dentry *chan;
 	void *dt;
 
 	scnprintf(chan_name, sizeof(chan_name), "%d", ch);
 	chan = debugfs_create_dir(chan_name, chandir);
 	dt = (void *)&pdev->phys[ch];
 
-	if (chan)
-		chan_state = debugfs_create_file("state", 0400, chan, dt,
-						 &chan_state_fops);
-	if (chan_state)
-		chan_descr = debugfs_create_file("descriptors", 0400, chan, dt,
-						 &descriptors_fops);
-	if (chan_descr)
-		chan_reqs = debugfs_create_file("requesters", 0400, chan, dt,
-						&requester_chan_fops);
-	if (!chan_reqs)
-		goto err_state;
+	debugfs_create_file("state", 0400, chan, dt, &chan_state_fops);
+	debugfs_create_file("descriptors", 0400, chan, dt, &descriptors_fops);
+	debugfs_create_file("requesters", 0400, chan, dt, &requester_chan_fops);
 
 	return chan;
-
-err_state:
-	debugfs_remove_recursive(chan);
-	return NULL;
 }
 
 static void pxad_init_debugfs(struct pxad_device *pdev)
@@ -358,40 +344,20 @@ static void pxad_init_debugfs(struct pxad_device *pdev)
 	int i;
 	struct dentry *chandir;
 
-	pdev->dbgfs_root = debugfs_create_dir(dev_name(pdev->slave.dev), NULL);
-	if (IS_ERR(pdev->dbgfs_root) || !pdev->dbgfs_root)
-		goto err_root;
-
-	pdev->dbgfs_state = debugfs_create_file("state", 0400, pdev->dbgfs_root,
-						pdev, &state_fops);
-	if (!pdev->dbgfs_state)
-		goto err_state;
-
 	pdev->dbgfs_chan =
-		kmalloc_array(pdev->nr_chans, sizeof(*pdev->dbgfs_state),
+		kmalloc_array(pdev->nr_chans, sizeof(struct dentry *),
 			      GFP_KERNEL);
 	if (!pdev->dbgfs_chan)
-		goto err_alloc;
+		return;
+
+	pdev->dbgfs_root = debugfs_create_dir(dev_name(pdev->slave.dev), NULL);
+
+	debugfs_create_file("state", 0400, pdev->dbgfs_root, pdev, &state_fops);
 
 	chandir = debugfs_create_dir("channels", pdev->dbgfs_root);
-	if (!chandir)
-		goto err_chandir;
 
-	for (i = 0; i < pdev->nr_chans; i++) {
+	for (i = 0; i < pdev->nr_chans; i++)
 		pdev->dbgfs_chan[i] = pxad_dbg_alloc_chan(pdev, i, chandir);
-		if (!pdev->dbgfs_chan[i])
-			goto err_chans;
-	}
-
-	return;
-err_chans:
-err_chandir:
-	kfree(pdev->dbgfs_chan);
-err_alloc:
-err_state:
-	debugfs_remove_recursive(pdev->dbgfs_root);
-err_root:
-	pr_err("pxad: debugfs is not available\n");
 }
 
 static void pxad_cleanup_debugfs(struct pxad_device *pdev)
-- 
2.22.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 5/6] dma: mic_x100_dma: no need to check return value of debugfs_create functions
  2019-06-12 12:25 [PATCH 1/6] dma: amba-pl08x: no need to cast away call to debugfs_create_file() Greg Kroah-Hartman
                   ` (2 preceding siblings ...)
  2019-06-12 12:25 ` [PATCH 4/6] dma: pxa_dma: no need to check return value of debugfs_create functions Greg Kroah-Hartman
@ 2019-06-12 12:25 ` " Greg Kroah-Hartman
  2019-06-12 15:44   ` Sudeep Dutt
  2019-06-12 12:25 ` [PATCH 6/6] dma: qcom: hidma: " Greg Kroah-Hartman
  2019-06-14  5:46 ` [PATCH 1/6] dma: amba-pl08x: no need to cast away call to debugfs_create_file() Vinod Koul
  5 siblings, 1 reply; 15+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-12 12:25 UTC (permalink / raw)
  To: dan.j.williams, vkoul
  Cc: Greg Kroah-Hartman, Sudeep Dutt, Ashutosh Dixit, dmaengine, linux-kernel

When calling debugfs functions, there is no need to ever check the
return value.  The function can work or not, but the code logic should
never do something different based on this.

Cc: Sudeep Dutt <sudeep.dutt@intel.com>
Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: dmaengine@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/dma/mic_x100_dma.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/mic_x100_dma.c b/drivers/dma/mic_x100_dma.c
index 6a91e28d537d..584e09661507 100644
--- a/drivers/dma/mic_x100_dma.c
+++ b/drivers/dma/mic_x100_dma.c
@@ -728,10 +728,8 @@ static int mic_dma_driver_probe(struct mbus_device *mbdev)
 	if (mic_dma_dbg) {
 		mic_dma_dev->dbg_dir = debugfs_create_dir(dev_name(&mbdev->dev),
 							  mic_dma_dbg);
-		if (mic_dma_dev->dbg_dir)
-			debugfs_create_file("mic_dma_reg", 0444,
-					    mic_dma_dev->dbg_dir, mic_dma_dev,
-					    &mic_dma_reg_fops);
+		debugfs_create_file("mic_dma_reg", 0444, mic_dma_dev->dbg_dir,
+				    mic_dma_dev, &mic_dma_reg_fops);
 	}
 	return 0;
 }
-- 
2.22.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 6/6] dma: qcom: hidma: no need to check return value of debugfs_create functions
  2019-06-12 12:25 [PATCH 1/6] dma: amba-pl08x: no need to cast away call to debugfs_create_file() Greg Kroah-Hartman
                   ` (3 preceding siblings ...)
  2019-06-12 12:25 ` [PATCH 5/6] dma: mic_x100_dma: " Greg Kroah-Hartman
@ 2019-06-12 12:25 ` " Greg Kroah-Hartman
  2019-06-12 15:24   ` Sinan Kaya
  2019-06-14  5:46 ` [PATCH 1/6] dma: amba-pl08x: no need to cast away call to debugfs_create_file() Vinod Koul
  5 siblings, 1 reply; 15+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-12 12:25 UTC (permalink / raw)
  To: dan.j.williams, vkoul
  Cc: Greg Kroah-Hartman, Sinan Kaya, Andy Gross, David Brown,
	linux-arm-kernel, linux-arm-msm, dmaengine, linux-kernel

When calling debugfs functions, there is no need to ever check the
return value.  The function can work or not, but the code logic should
never do something different based on this.

Also, because there is no need to save the file dentry, remove the
variables that were saving them as they were never even being used once
set.

Cc: Sinan Kaya <okaya@kernel.org>
Cc: Andy Gross <agross@kernel.org>
Cc: David Brown <david.brown@linaro.org>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-arm-msm@vger.kernel.org
Cc: dmaengine@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/dma/qcom/hidma.h     |  5 +----
 drivers/dma/qcom/hidma_dbg.c | 37 +++++++-----------------------------
 2 files changed, 8 insertions(+), 34 deletions(-)

diff --git a/drivers/dma/qcom/hidma.h b/drivers/dma/qcom/hidma.h
index 5f9966e82c0b..36357d02333a 100644
--- a/drivers/dma/qcom/hidma.h
+++ b/drivers/dma/qcom/hidma.h
@@ -101,8 +101,6 @@ struct hidma_chan {
 	 * It is used by the DMA complete notification to
 	 * locate the descriptor that initiated the transfer.
 	 */
-	struct dentry			*debugfs;
-	struct dentry			*stats;
 	struct hidma_dev		*dmadev;
 	struct hidma_desc		*running;
 
@@ -134,7 +132,6 @@ struct hidma_dev {
 	struct dma_device		ddev;
 
 	struct dentry			*debugfs;
-	struct dentry			*stats;
 
 	/* sysfs entry for the channel id */
 	struct device_attribute		*chid_attrs;
@@ -166,6 +163,6 @@ irqreturn_t hidma_ll_inthandler(int irq, void *arg);
 irqreturn_t hidma_ll_inthandler_msi(int irq, void *arg, int cause);
 void hidma_cleanup_pending_tre(struct hidma_lldev *llhndl, u8 err_info,
 				u8 err_code);
-int hidma_debug_init(struct hidma_dev *dmadev);
+void hidma_debug_init(struct hidma_dev *dmadev);
 void hidma_debug_uninit(struct hidma_dev *dmadev);
 #endif
diff --git a/drivers/dma/qcom/hidma_dbg.c b/drivers/dma/qcom/hidma_dbg.c
index 9523faf7acdc..994f448b64d8 100644
--- a/drivers/dma/qcom/hidma_dbg.c
+++ b/drivers/dma/qcom/hidma_dbg.c
@@ -146,17 +146,13 @@ void hidma_debug_uninit(struct hidma_dev *dmadev)
 	debugfs_remove_recursive(dmadev->debugfs);
 }
 
-int hidma_debug_init(struct hidma_dev *dmadev)
+void hidma_debug_init(struct hidma_dev *dmadev)
 {
-	int rc = 0;
 	int chidx = 0;
 	struct list_head *position = NULL;
+	struct dentry *dir;
 
 	dmadev->debugfs = debugfs_create_dir(dev_name(dmadev->ddev.dev), NULL);
-	if (!dmadev->debugfs) {
-		rc = -ENODEV;
-		return rc;
-	}
 
 	/* walk through the virtual channel list */
 	list_for_each(position, &dmadev->ddev.channels) {
@@ -165,32 +161,13 @@ int hidma_debug_init(struct hidma_dev *dmadev)
 		chan = list_entry(position, struct hidma_chan,
 				  chan.device_node);
 		sprintf(chan->dbg_name, "chan%d", chidx);
-		chan->debugfs = debugfs_create_dir(chan->dbg_name,
+		dir = debugfs_create_dir(chan->dbg_name,
 						   dmadev->debugfs);
-		if (!chan->debugfs) {
-			rc = -ENOMEM;
-			goto cleanup;
-		}
-		chan->stats = debugfs_create_file("stats", S_IRUGO,
-						  chan->debugfs, chan,
-						  &hidma_chan_fops);
-		if (!chan->stats) {
-			rc = -ENOMEM;
-			goto cleanup;
-		}
+		debugfs_create_file("stats", S_IRUGO, dir, chan,
+				    &hidma_chan_fops);
 		chidx++;
 	}
 
-	dmadev->stats = debugfs_create_file("stats", S_IRUGO,
-					    dmadev->debugfs, dmadev,
-					    &hidma_dma_fops);
-	if (!dmadev->stats) {
-		rc = -ENOMEM;
-		goto cleanup;
-	}
-
-	return 0;
-cleanup:
-	hidma_debug_uninit(dmadev);
-	return rc;
+	debugfs_create_file("stats", S_IRUGO, dmadev->debugfs, dmadev,
+			    &hidma_dma_fops);
 }
-- 
2.22.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/6] dma: coh901318: no need to cast away call to debugfs_create_file()
  2019-06-12 12:25 ` [PATCH 3/6] dma: coh901318: no need to cast away call to debugfs_create_file() Greg Kroah-Hartman
@ 2019-06-12 12:42   ` Linus Walleij
  0 siblings, 0 replies; 15+ messages in thread
From: Linus Walleij @ 2019-06-12 12:42 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Dan Williams, Vinod Koul, Linux ARM, dmaengine, linux-kernel

On Wed, Jun 12, 2019 at 2:26 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:

> No need to check the return value of debugfs_create_file(), so no need
> to provide a fake "cast away" of the return value either.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: dmaengine@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 6/6] dma: qcom: hidma: no need to check return value of debugfs_create functions
  2019-06-12 12:25 ` [PATCH 6/6] dma: qcom: hidma: " Greg Kroah-Hartman
@ 2019-06-12 15:24   ` Sinan Kaya
  2019-06-12 15:39     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 15+ messages in thread
From: Sinan Kaya @ 2019-06-12 15:24 UTC (permalink / raw)
  To: Greg Kroah-Hartman, dan.j.williams, vkoul
  Cc: Andy Gross, David Brown, linux-arm-kernel, linux-arm-msm,
	dmaengine, linux-kernel

On 6/12/2019 8:25 AM, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value.  The function can work or not, but the code logic should
> never do something different based on this.
> 
> Also, because there is no need to save the file dentry, remove the
> variables that were saving them as they were never even being used once
> set.
> 
> Cc: Sinan Kaya <okaya@kernel.org>
> Cc: Andy Gross <agross@kernel.org>
> Cc: David Brown <david.brown@linaro.org>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-arm-msm@vger.kernel.org
> Cc: dmaengine@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Interesting. Wouldn't debugfs_create_file() blow up if dir is NULL
for some reason?


+		debugfs_create_file("stats", S_IRUGO, dir, chan,
+				    &hidma_chan_fops);

Note that code ignores the return value of hidma_debug_init();
It was just trying to do clean up on debugfs failure by calling

	debugfs_remove_recursive(dmadev->debugfs);

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 6/6] dma: qcom: hidma: no need to check return value of debugfs_create functions
  2019-06-12 15:24   ` Sinan Kaya
@ 2019-06-12 15:39     ` Greg Kroah-Hartman
  2019-06-12 16:17       ` Sinan Kaya
  0 siblings, 1 reply; 15+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-12 15:39 UTC (permalink / raw)
  To: Sinan Kaya
  Cc: dan.j.williams, vkoul, Andy Gross, David Brown, linux-arm-kernel,
	linux-arm-msm, dmaengine, linux-kernel

On Wed, Jun 12, 2019 at 11:24:51AM -0400, Sinan Kaya wrote:
> On 6/12/2019 8:25 AM, Greg Kroah-Hartman wrote:
> > When calling debugfs functions, there is no need to ever check the
> > return value.  The function can work or not, but the code logic should
> > never do something different based on this.
> > 
> > Also, because there is no need to save the file dentry, remove the
> > variables that were saving them as they were never even being used once
> > set.
> > 
> > Cc: Sinan Kaya <okaya@kernel.org>
> > Cc: Andy Gross <agross@kernel.org>
> > Cc: David Brown <david.brown@linaro.org>
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Cc: Vinod Koul <vkoul@kernel.org>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: linux-arm-msm@vger.kernel.org
> > Cc: dmaengine@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> Interesting. Wouldn't debugfs_create_file() blow up if dir is NULL
> for some reason?

It will create a file in the root of debugfs.  But how will that happen?
debugfs_create_dir() can not return NULL.

> +		debugfs_create_file("stats", S_IRUGO, dir, chan,
> +				    &hidma_chan_fops);
> 
> Note that code ignores the return value of hidma_debug_init();
> It was just trying to do clean up on debugfs failure by calling
> 
> 	debugfs_remove_recursive(dmadev->debugfs);

Is that a problem?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 5/6] dma: mic_x100_dma: no need to check return value of debugfs_create functions
  2019-06-12 12:25 ` [PATCH 5/6] dma: mic_x100_dma: " Greg Kroah-Hartman
@ 2019-06-12 15:44   ` Sudeep Dutt
  0 siblings, 0 replies; 15+ messages in thread
From: Sudeep Dutt @ 2019-06-12 15:44 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Sudeep Dutt, dan.j.williams, vkoul, Ashutosh Dixit, dmaengine,
	linux-kernel

On Wed, 2019-06-12 at 14:25 +0200, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value.  The function can work or not, but the code logic should
> never do something different based on this.
> 

Thanks Greg.

Reviewed-by: Sudeep Dutt <sudeep.dutt@intel.com>

> Cc: Sudeep Dutt <sudeep.dutt@intel.com>
> Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: dmaengine@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/dma/mic_x100_dma.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/dma/mic_x100_dma.c b/drivers/dma/mic_x100_dma.c
> index 6a91e28d537d..584e09661507 100644
> --- a/drivers/dma/mic_x100_dma.c
> +++ b/drivers/dma/mic_x100_dma.c
> @@ -728,10 +728,8 @@ static int mic_dma_driver_probe(struct mbus_device *mbdev)
>  	if (mic_dma_dbg) {
>  		mic_dma_dev->dbg_dir = debugfs_create_dir(dev_name(&mbdev->dev),
>  							  mic_dma_dbg);
> -		if (mic_dma_dev->dbg_dir)
> -			debugfs_create_file("mic_dma_reg", 0444,
> -					    mic_dma_dev->dbg_dir, mic_dma_dev,
> -					    &mic_dma_reg_fops);
> +		debugfs_create_file("mic_dma_reg", 0444, mic_dma_dev->dbg_dir,
> +				    mic_dma_dev, &mic_dma_reg_fops);
>  	}
>  	return 0;
>  }



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 6/6] dma: qcom: hidma: no need to check return value of debugfs_create functions
  2019-06-12 15:39     ` Greg Kroah-Hartman
@ 2019-06-12 16:17       ` Sinan Kaya
  2019-06-12 16:47         ` Greg Kroah-Hartman
  0 siblings, 1 reply; 15+ messages in thread
From: Sinan Kaya @ 2019-06-12 16:17 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: dan.j.williams, vkoul, Andy Gross, David Brown, linux-arm-kernel,
	linux-arm-msm, dmaengine, linux-kernel

On 6/12/2019 11:39 AM, Greg Kroah-Hartman wrote:
>> Interesting. Wouldn't debugfs_create_file() blow up if dir is NULL
>> for some reason?
> It will create a file in the root of debugfs.  But how will that happen?
> debugfs_create_dir() can not return NULL.

I see.

> 
>> +		debugfs_create_file("stats", S_IRUGO, dir, chan,
>> +				    &hidma_chan_fops);
>>
>> Note that code ignores the return value of hidma_debug_init();
>> It was just trying to do clean up on debugfs failure by calling
>>
>> 	debugfs_remove_recursive(dmadev->debugfs);
> Is that a problem?

I just wanted to double check. You probably want to remove the return
value on debugfs_create_file() to prevent others from doing the same
thing.

Acked-by: Sinan Kaya <okaya@kernel.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 6/6] dma: qcom: hidma: no need to check return value of debugfs_create functions
  2019-06-12 16:17       ` Sinan Kaya
@ 2019-06-12 16:47         ` Greg Kroah-Hartman
  0 siblings, 0 replies; 15+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-12 16:47 UTC (permalink / raw)
  To: Sinan Kaya
  Cc: dan.j.williams, vkoul, Andy Gross, David Brown, linux-arm-kernel,
	linux-arm-msm, dmaengine, linux-kernel

On Wed, Jun 12, 2019 at 12:17:31PM -0400, Sinan Kaya wrote:
> On 6/12/2019 11:39 AM, Greg Kroah-Hartman wrote:
> >> Interesting. Wouldn't debugfs_create_file() blow up if dir is NULL
> >> for some reason?
> > It will create a file in the root of debugfs.  But how will that happen?
> > debugfs_create_dir() can not return NULL.
> 
> I see.
> 
> > 
> >> +		debugfs_create_file("stats", S_IRUGO, dir, chan,
> >> +				    &hidma_chan_fops);
> >>
> >> Note that code ignores the return value of hidma_debug_init();
> >> It was just trying to do clean up on debugfs failure by calling
> >>
> >> 	debugfs_remove_recursive(dmadev->debugfs);
> > Is that a problem?
> 
> I just wanted to double check. You probably want to remove the return
> value on debugfs_create_file() to prevent others from doing the same
> thing.

That is my long-term goal, I have a ways to go still :)

> Acked-by: Sinan Kaya <okaya@kernel.org>

Great, thanks for the review.

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/6] dma: amba-pl08x: no need to cast away call to debugfs_create_file()
  2019-06-12 12:25 [PATCH 1/6] dma: amba-pl08x: no need to cast away call to debugfs_create_file() Greg Kroah-Hartman
                   ` (4 preceding siblings ...)
  2019-06-12 12:25 ` [PATCH 6/6] dma: qcom: hidma: " Greg Kroah-Hartman
@ 2019-06-14  5:46 ` Vinod Koul
  2019-06-14  5:55   ` Greg Kroah-Hartman
  2019-06-18 16:00   ` Greg Kroah-Hartman
  5 siblings, 2 replies; 15+ messages in thread
From: Vinod Koul @ 2019-06-14  5:46 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: dan.j.williams, dmaengine, linux-kernel

On 12-06-19, 14:25, Greg Kroah-Hartman wrote:
> No need to check the return value of debugfs_create_file(), so no need
> to provide a fake "cast away" of the return value either.

Applied all after fixing the subsystem tag (dmaengine), thanks

-- 
~Vinod

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/6] dma: amba-pl08x: no need to cast away call to debugfs_create_file()
  2019-06-14  5:46 ` [PATCH 1/6] dma: amba-pl08x: no need to cast away call to debugfs_create_file() Vinod Koul
@ 2019-06-14  5:55   ` Greg Kroah-Hartman
  2019-06-18 16:00   ` Greg Kroah-Hartman
  1 sibling, 0 replies; 15+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-14  5:55 UTC (permalink / raw)
  To: Vinod Koul; +Cc: dan.j.williams, dmaengine, linux-kernel

On Fri, Jun 14, 2019 at 11:16:13AM +0530, Vinod Koul wrote:
> On 12-06-19, 14:25, Greg Kroah-Hartman wrote:
> > No need to check the return value of debugfs_create_file(), so no need
> > to provide a fake "cast away" of the return value either.
> 
> Applied all after fixing the subsystem tag (dmaengine), thanks

Sorry about that, and thanks!

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/6] dma: amba-pl08x: no need to cast away call to debugfs_create_file()
  2019-06-14  5:46 ` [PATCH 1/6] dma: amba-pl08x: no need to cast away call to debugfs_create_file() Vinod Koul
  2019-06-14  5:55   ` Greg Kroah-Hartman
@ 2019-06-18 16:00   ` Greg Kroah-Hartman
  1 sibling, 0 replies; 15+ messages in thread
From: Greg Kroah-Hartman @ 2019-06-18 16:00 UTC (permalink / raw)
  To: Vinod Koul; +Cc: dan.j.williams, dmaengine, linux-kernel

On Fri, Jun 14, 2019 at 11:16:13AM +0530, Vinod Koul wrote:
> On 12-06-19, 14:25, Greg Kroah-Hartman wrote:
> > No need to check the return value of debugfs_create_file(), so no need
> > to provide a fake "cast away" of the return value either.
> 
> Applied all after fixing the subsystem tag (dmaengine), thanks

Oops, messed that up, sorry.  Thanks for applying them!

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, back to index

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-12 12:25 [PATCH 1/6] dma: amba-pl08x: no need to cast away call to debugfs_create_file() Greg Kroah-Hartman
2019-06-12 12:25 ` [PATCH 2/6] dma: bcm-sba-raid: no need to check return value of debugfs_create functions Greg Kroah-Hartman
2019-06-12 12:25 ` [PATCH 3/6] dma: coh901318: no need to cast away call to debugfs_create_file() Greg Kroah-Hartman
2019-06-12 12:42   ` Linus Walleij
2019-06-12 12:25 ` [PATCH 4/6] dma: pxa_dma: no need to check return value of debugfs_create functions Greg Kroah-Hartman
2019-06-12 12:25 ` [PATCH 5/6] dma: mic_x100_dma: " Greg Kroah-Hartman
2019-06-12 15:44   ` Sudeep Dutt
2019-06-12 12:25 ` [PATCH 6/6] dma: qcom: hidma: " Greg Kroah-Hartman
2019-06-12 15:24   ` Sinan Kaya
2019-06-12 15:39     ` Greg Kroah-Hartman
2019-06-12 16:17       ` Sinan Kaya
2019-06-12 16:47         ` Greg Kroah-Hartman
2019-06-14  5:46 ` [PATCH 1/6] dma: amba-pl08x: no need to cast away call to debugfs_create_file() Vinod Koul
2019-06-14  5:55   ` Greg Kroah-Hartman
2019-06-18 16:00   ` Greg Kroah-Hartman

dmaengine Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dmaengine/0 dmaengine/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dmaengine dmaengine/ https://lore.kernel.org/dmaengine \
		dmaengine@vger.kernel.org dmaengine@archiver.kernel.org
	public-inbox-index dmaengine


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.dmaengine


AGPL code for this site: git clone https://public-inbox.org/ public-inbox