[PATCH 1/7] media: cec: no need to check return value of debugfs_create functions

[PATCH 1/7] media: cec: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

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: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: linux-media@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/media/cec/core/cec-core.c | 27 ++++++++-------------------
 include/media/cec.h               |  2 --
 2 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/drivers/media/cec/core/cec-core.c b/drivers/media/cec/core/cec-core.c
index c599cd94dd62..562792f545ac 100644
--- a/drivers/media/cec/core/cec-core.c
+++ b/drivers/media/cec/core/cec-core.c
@@ -359,27 +359,16 @@ int cec_register_adapter(struct cec_adapter *adap,
 	if (!top_cec_dir)
 		return 0;
 
-	adap->cec_dir = debugfs_create_dir(dev_name(&adap->devnode.dev), top_cec_dir);
-	if (IS_ERR_OR_NULL(adap->cec_dir)) {
-		pr_warn("cec-%s: Failed to create debugfs dir\n", adap->name);
-		return 0;
-	}
-	adap->status_file = debugfs_create_devm_seqfile(&adap->devnode.dev,
-		"status", adap->cec_dir, cec_adap_status);
-	if (IS_ERR_OR_NULL(adap->status_file)) {
-		pr_warn("cec-%s: Failed to create status file\n", adap->name);
-		debugfs_remove_recursive(adap->cec_dir);
-		adap->cec_dir = NULL;
-		return 0;
-	}
+	adap->cec_dir = debugfs_create_dir(dev_name(&adap->devnode.dev),
+					   top_cec_dir);
+
+	debugfs_create_devm_seqfile(&adap->devnode.dev, "status", adap->cec_dir,
+				    cec_adap_status);
+
 	if (!adap->ops->error_inj_show || !adap->ops->error_inj_parse_line)
 		return 0;
-	adap->error_inj_file = debugfs_create_file("error-inj", 0644,
-						   adap->cec_dir, adap,
-						   &cec_error_inj_fops);
-	if (IS_ERR_OR_NULL(adap->error_inj_file))
-		pr_warn("cec-%s: Failed to create error-inj file\n",
-			adap->name);
+	debugfs_create_file("error-inj", 0644, adap->cec_dir, adap,
+			    &cec_error_inj_fops);
 #endif
 	return 0;
 }
diff --git a/include/media/cec.h b/include/media/cec.h
index c48b5f2e4b50..cd35ae6b7560 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -248,8 +248,6 @@ struct cec_adapter {
 #endif
 
 	struct dentry *cec_dir;
-	struct dentry *status_file;
-	struct dentry *error_inj_file;
 
 	u32 sequence;
 
-- 
2.28.0

[PATCH 2/7] media: coda: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

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: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: linux-media@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/media/platform/coda/coda-common.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
index 3ab3d976d8ca..9020be29d8f2 100644
--- a/drivers/media/platform/coda/coda-common.c
+++ b/drivers/media/platform/coda/coda-common.c
@@ -1937,9 +1937,6 @@ int coda_alloc_aux_buf(struct coda_dev *dev, struct coda_aux_buf *buf,
 		buf->blob.size = size;
 		buf->dentry = debugfs_create_blob(name, 0644, parent,
 						  &buf->blob);
-		if (!buf->dentry)
-			dev_warn(dev->dev,
-				 "failed to create debugfs entry %s\n", name);
 	}
 
 	return 0;
@@ -3211,8 +3208,6 @@ static int coda_probe(struct platform_device *pdev)
 	ida_init(&dev->ida);
 
 	dev->debugfs_root = debugfs_create_dir("coda", NULL);
-	if (!dev->debugfs_root)
-		dev_warn(&pdev->dev, "failed to create debugfs root\n");
 
 	/* allocate auxiliary per-device buffers for the BIT processor */
 	if (dev->devtype->product == CODA_DX6) {
-- 
2.28.0

Re: [PATCH 2/7] media: coda: no need to check return value of debugfs_create functions

[Philipp Zabel]

On Tue, 2020-08-18 at 15:36 +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.
>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: linux-media@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Thank you,

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

> ---
>  drivers/media/platform/coda/coda-common.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/media/platform/coda/coda-common.c b/drivers/media/platform/coda/coda-common.c
> index 3ab3d976d8ca..9020be29d8f2 100644
> --- a/drivers/media/platform/coda/coda-common.c
> +++ b/drivers/media/platform/coda/coda-common.c
> @@ -1937,9 +1937,6 @@ int coda_alloc_aux_buf(struct coda_dev *dev, struct coda_aux_buf *buf,
>  		buf->blob.size = size;
>  		buf->dentry = debugfs_create_blob(name, 0644, parent,
>  						  &buf->blob);
> -		if (!buf->dentry)
> -			dev_warn(dev->dev,
> -				 "failed to create debugfs entry %s\n", name);
>  	}
>  
>  	return 0;
> @@ -3211,8 +3208,6 @@ static int coda_probe(struct platform_device *pdev)
>  	ida_init(&dev->ida);
>  
>  	dev->debugfs_root = debugfs_create_dir("coda", NULL);
> -	if (!dev->debugfs_root)
> -		dev_warn(&pdev->dev, "failed to create debugfs root\n");
>  
>  	/* allocate auxiliary per-device buffers for the BIT processor */
>  	if (dev->devtype->product == CODA_DX6) {

[PATCH 3/7] media: exynos4-is: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

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: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Sylwester Nawrocki <s.nawrocki@samsung.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Kukjin Kim <kgene@kernel.org>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: linux-media@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-samsung-soc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/media/platform/exynos4-is/fimc-is.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-is.c b/drivers/media/platform/exynos4-is/fimc-is.c
index a474014f0a0f..019bb47df915 100644
--- a/drivers/media/platform/exynos4-is/fimc-is.c
+++ b/drivers/media/platform/exynos4-is/fimc-is.c
@@ -756,18 +756,12 @@ static void fimc_is_debugfs_remove(struct fimc_is *is)
 	is->debugfs_entry = NULL;
 }
 
-static int fimc_is_debugfs_create(struct fimc_is *is)
+static void fimc_is_debugfs_create(struct fimc_is *is)
 {
-	struct dentry *dentry;
-
 	is->debugfs_entry = debugfs_create_dir("fimc_is", NULL);
 
-	dentry = debugfs_create_file("fw_log", S_IRUGO, is->debugfs_entry,
-				     is, &fimc_is_fops);
-	if (!dentry)
-		fimc_is_debugfs_remove(is);
-
-	return is->debugfs_entry == NULL ? -EIO : 0;
+	debugfs_create_file("fw_log", S_IRUGO, is->debugfs_entry, is,
+			    &fimc_is_fops);
 }
 
 static int fimc_is_runtime_resume(struct device *dev);
@@ -853,9 +847,7 @@ static int fimc_is_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto err_pm;
 
-	ret = fimc_is_debugfs_create(is);
-	if (ret < 0)
-		goto err_sd;
+	fimc_is_debugfs_create(is);
 
 	ret = fimc_is_request_firmware(is, FIMC_IS_FW_FILENAME);
 	if (ret < 0)
@@ -868,7 +860,6 @@ static int fimc_is_probe(struct platform_device *pdev)
 
 err_dfs:
 	fimc_is_debugfs_remove(is);
-err_sd:
 	fimc_is_unregister_subdevs(is);
 err_pm:
 	pm_runtime_put_noidle(dev);
-- 
2.28.0

[PATCH 4/7] media: mtk-vpu: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

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: Minghsiu Tsai <minghsiu.tsai@mediatek.com>
Cc: Houlong Wei <houlong.wei@mediatek.com>
Cc: Andrew-CT Chen <andrew-ct.chen@mediatek.com>
Cc: Tiffany Lin <tiffany.lin@mediatek.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Matthias Brugger <matthias.bgg@gmail.com>
Cc: linux-media@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-mediatek@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/media/platform/mtk-vpu/mtk_vpu.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/media/platform/mtk-vpu/mtk_vpu.c b/drivers/media/platform/mtk-vpu/mtk_vpu.c
index d30c08983f56..36cb9b6131f7 100644
--- a/drivers/media/platform/mtk-vpu/mtk_vpu.c
+++ b/drivers/media/platform/mtk-vpu/mtk_vpu.c
@@ -849,10 +849,6 @@ static int mtk_vpu_probe(struct platform_device *pdev)
 #ifdef CONFIG_DEBUG_FS
 	vpu_debugfs = debugfs_create_file("mtk_vpu", S_IRUGO, NULL, (void *)dev,
 					  &vpu_debug_fops);
-	if (!vpu_debugfs) {
-		ret = -ENOMEM;
-		goto cleanup_ipi;
-	}
 #endif
 
 	/* Set PTCM to 96K and DTCM to 32K */
@@ -910,7 +906,6 @@ static int mtk_vpu_probe(struct platform_device *pdev)
 	of_reserved_mem_device_release(dev);
 #ifdef CONFIG_DEBUG_FS
 	debugfs_remove(vpu_debugfs);
-cleanup_ipi:
 #endif
 	memset(vpu->ipi_desc, 0, sizeof(struct vpu_ipi_desc) * IPI_MAX);
 vpu_mutex_destroy:
-- 
2.28.0

[PATCH 5/7] media: sti: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

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: Fabien Dessenne <fabien.dessenne@st.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Jean-Christophe Trotin <jean-christophe.trotin@st.com>
Cc: linux-media@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 .../media/platform/sti/bdisp/bdisp-debug.c    | 29 ++++---------------
 drivers/media/platform/sti/bdisp/bdisp-v4l2.c |  7 +----
 drivers/media/platform/sti/bdisp/bdisp.h      |  2 +-
 drivers/media/platform/sti/hva/hva-debugfs.c  | 22 +++-----------
 4 files changed, 12 insertions(+), 48 deletions(-)

diff --git a/drivers/media/platform/sti/bdisp/bdisp-debug.c b/drivers/media/platform/sti/bdisp/bdisp-debug.c
index 77ca7517fa3e..2b270093009c 100644
--- a/drivers/media/platform/sti/bdisp/bdisp-debug.c
+++ b/drivers/media/platform/sti/bdisp/bdisp-debug.c
@@ -637,35 +637,18 @@ DEFINE_SHOW_ATTRIBUTE(last_nodes_raw);
 DEFINE_SHOW_ATTRIBUTE(last_request);
 DEFINE_SHOW_ATTRIBUTE(perf);
 
-int bdisp_debugfs_create(struct bdisp_dev *bdisp)
+void bdisp_debugfs_create(struct bdisp_dev *bdisp)
 {
 	char dirname[16];
 
 	snprintf(dirname, sizeof(dirname), "%s%d", BDISP_NAME, bdisp->id);
 	bdisp->dbg.debugfs_entry = debugfs_create_dir(dirname, NULL);
-	if (!bdisp->dbg.debugfs_entry)
-		goto err;
 
-	if (!bdisp_dbg_create_entry(regs))
-		goto err;
-
-	if (!bdisp_dbg_create_entry(last_nodes))
-		goto err;
-
-	if (!bdisp_dbg_create_entry(last_nodes_raw))
-		goto err;
-
-	if (!bdisp_dbg_create_entry(last_request))
-		goto err;
-
-	if (!bdisp_dbg_create_entry(perf))
-		goto err;
-
-	return 0;
-
-err:
-	bdisp_debugfs_remove(bdisp);
-	return -ENOMEM;
+	bdisp_dbg_create_entry(regs);
+	bdisp_dbg_create_entry(last_nodes);
+	bdisp_dbg_create_entry(last_nodes_raw);
+	bdisp_dbg_create_entry(last_request);
+	bdisp_dbg_create_entry(perf);
 }
 
 void bdisp_debugfs_remove(struct bdisp_dev *bdisp)
diff --git a/drivers/media/platform/sti/bdisp/bdisp-v4l2.c b/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
index af2d5eb782ce..7d50d6cd073d 100644
--- a/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
+++ b/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
@@ -1360,11 +1360,7 @@ static int bdisp_probe(struct platform_device *pdev)
 	}
 
 	/* Debug */
-	ret = bdisp_debugfs_create(bdisp);
-	if (ret) {
-		dev_err(dev, "failed to create debugfs\n");
-		goto err_v4l2;
-	}
+	bdisp_debugfs_create(bdisp);
 
 	/* Power management */
 	pm_runtime_enable(dev);
@@ -1401,7 +1397,6 @@ static int bdisp_probe(struct platform_device *pdev)
 	pm_runtime_put(dev);
 err_dbg:
 	bdisp_debugfs_remove(bdisp);
-err_v4l2:
 	v4l2_device_unregister(&bdisp->v4l2_dev);
 err_clk:
 	if (!IS_ERR(bdisp->clock))
diff --git a/drivers/media/platform/sti/bdisp/bdisp.h b/drivers/media/platform/sti/bdisp/bdisp.h
index e309cde379ca..3fb009d24791 100644
--- a/drivers/media/platform/sti/bdisp/bdisp.h
+++ b/drivers/media/platform/sti/bdisp/bdisp.h
@@ -209,6 +209,6 @@ int bdisp_hw_get_and_clear_irq(struct bdisp_dev *bdisp);
 int bdisp_hw_update(struct bdisp_ctx *ctx);
 
 void bdisp_debugfs_remove(struct bdisp_dev *bdisp);
-int bdisp_debugfs_create(struct bdisp_dev *bdisp);
+void bdisp_debugfs_create(struct bdisp_dev *bdisp);
 void bdisp_dbg_perf_begin(struct bdisp_dev *bdisp);
 void bdisp_dbg_perf_end(struct bdisp_dev *bdisp);
diff --git a/drivers/media/platform/sti/hva/hva-debugfs.c b/drivers/media/platform/sti/hva/hva-debugfs.c
index 7d12a5b5d914..a86a07b6fbc7 100644
--- a/drivers/media/platform/sti/hva/hva-debugfs.c
+++ b/drivers/media/platform/sti/hva/hva-debugfs.c
@@ -337,25 +337,11 @@ DEFINE_SHOW_ATTRIBUTE(regs);
 void hva_debugfs_create(struct hva_dev *hva)
 {
 	hva->dbg.debugfs_entry = debugfs_create_dir(HVA_NAME, NULL);
-	if (!hva->dbg.debugfs_entry)
-		goto err;
 
-	if (!hva_dbg_create_entry(device))
-		goto err;
-
-	if (!hva_dbg_create_entry(encoders))
-		goto err;
-
-	if (!hva_dbg_create_entry(last))
-		goto err;
-
-	if (!hva_dbg_create_entry(regs))
-		goto err;
-
-	return;
-
-err:
-	hva_debugfs_remove(hva);
+	hva_dbg_create_entry(device);
+	hva_dbg_create_entry(encoders);
+	hva_dbg_create_entry(last);
+	hva_dbg_create_entry(regs);
 }
 
 void hva_debugfs_remove(struct hva_dev *hva)
-- 
2.28.0

[PATCH 6/7] media: radio: si476x: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

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: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Alexios Zavras <alexios.zavras@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Cc: linux-media@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/media/radio/radio-si476x.c | 66 ++++++------------------------
 1 file changed, 13 insertions(+), 53 deletions(-)

diff --git a/drivers/media/radio/radio-si476x.c b/drivers/media/radio/radio-si476x.c
index b203296de977..485b2d2d977d 100644
--- a/drivers/media/radio/radio-si476x.c
+++ b/drivers/media/radio/radio-si476x.c
@@ -1344,60 +1344,24 @@ static const struct file_operations radio_rsq_primary_fops = {
 };
 
 
-static int si476x_radio_init_debugfs(struct si476x_radio *radio)
+static void si476x_radio_init_debugfs(struct si476x_radio *radio)
 {
-	struct dentry	*dentry;
-	int		ret;
+	radio->debugfs = debugfs_create_dir(dev_name(radio->v4l2dev.dev), NULL);
 
-	dentry = debugfs_create_dir(dev_name(radio->v4l2dev.dev), NULL);
-	if (IS_ERR(dentry)) {
-		ret = PTR_ERR(dentry);
-		goto exit;
-	}
-	radio->debugfs = dentry;
-
-	dentry = debugfs_create_file("acf", S_IRUGO,
-				     radio->debugfs, radio, &radio_acf_fops);
-	if (IS_ERR(dentry)) {
-		ret = PTR_ERR(dentry);
-		goto cleanup;
-	}
+	debugfs_create_file("acf", S_IRUGO, radio->debugfs, radio,
+			    &radio_acf_fops);
 
-	dentry = debugfs_create_file("rds_blckcnt", S_IRUGO,
-				     radio->debugfs, radio,
-				     &radio_rds_blckcnt_fops);
-	if (IS_ERR(dentry)) {
-		ret = PTR_ERR(dentry);
-		goto cleanup;
-	}
+	debugfs_create_file("rds_blckcnt", S_IRUGO, radio->debugfs, radio,
+			    &radio_rds_blckcnt_fops);
 
-	dentry = debugfs_create_file("agc", S_IRUGO,
-				     radio->debugfs, radio, &radio_agc_fops);
-	if (IS_ERR(dentry)) {
-		ret = PTR_ERR(dentry);
-		goto cleanup;
-	}
+	debugfs_create_file("agc", S_IRUGO, radio->debugfs, radio,
+			    &radio_agc_fops);
 
-	dentry = debugfs_create_file("rsq", S_IRUGO,
-				     radio->debugfs, radio, &radio_rsq_fops);
-	if (IS_ERR(dentry)) {
-		ret = PTR_ERR(dentry);
-		goto cleanup;
-	}
+	debugfs_create_file("rsq", S_IRUGO, radio->debugfs, radio,
+			    &radio_rsq_fops);
 
-	dentry = debugfs_create_file("rsq_primary", S_IRUGO,
-				     radio->debugfs, radio,
-				     &radio_rsq_primary_fops);
-	if (IS_ERR(dentry)) {
-		ret = PTR_ERR(dentry);
-		goto cleanup;
-	}
-
-	return 0;
-cleanup:
-	debugfs_remove_recursive(radio->debugfs);
-exit:
-	return ret;
+	debugfs_create_file("rsq_primary", S_IRUGO, radio->debugfs, radio,
+			    &radio_rsq_primary_fops);
 }
 
 
@@ -1534,11 +1498,7 @@ static int si476x_radio_probe(struct platform_device *pdev)
 		goto exit;
 	}
 
-	rval = si476x_radio_init_debugfs(radio);
-	if (rval < 0) {
-		dev_err(&pdev->dev, "Could not create debugfs interface\n");
-		goto exit;
-	}
+	si476x_radio_init_debugfs(radio);
 
 	return 0;
 exit:
-- 
2.28.0

[PATCH 7/7] media: usb: uvc: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

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: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: linux-media@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/media/usb/uvc/uvc_debugfs.c | 20 ++++----------------
 1 file changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_debugfs.c b/drivers/media/usb/uvc/uvc_debugfs.c
index 2b8af4b54117..1a1258d4ffca 100644
--- a/drivers/media/usb/uvc/uvc_debugfs.c
+++ b/drivers/media/usb/uvc/uvc_debugfs.c
@@ -73,7 +73,6 @@ static struct dentry *uvc_debugfs_root_dir;
 void uvc_debugfs_init_stream(struct uvc_streaming *stream)
 {
 	struct usb_device *udev = stream->dev->udev;
-	struct dentry *dent;
 	char dir_name[33];
 
 	if (uvc_debugfs_root_dir == NULL)
@@ -82,22 +81,11 @@ void uvc_debugfs_init_stream(struct uvc_streaming *stream)
 	snprintf(dir_name, sizeof(dir_name), "%u-%u-%u", udev->bus->busnum,
 		 udev->devnum, stream->intfnum);
 
-	dent = debugfs_create_dir(dir_name, uvc_debugfs_root_dir);
-	if (IS_ERR_OR_NULL(dent)) {
-		uvc_printk(KERN_INFO, "Unable to create debugfs %s "
-			   "directory.\n", dir_name);
-		return;
-	}
-
-	stream->debugfs_dir = dent;
+	stream->debugfs_dir = debugfs_create_dir(dir_name,
+						 uvc_debugfs_root_dir);
 
-	dent = debugfs_create_file("stats", 0444, stream->debugfs_dir,
-				   stream, &uvc_debugfs_stats_fops);
-	if (IS_ERR_OR_NULL(dent)) {
-		uvc_printk(KERN_INFO, "Unable to create debugfs stats file.\n");
-		uvc_debugfs_cleanup_stream(stream);
-		return;
-	}
+	debugfs_create_file("stats", 0444, stream->debugfs_dir, stream,
+			    &uvc_debugfs_stats_fops);
 }
 
 void uvc_debugfs_cleanup_stream(struct uvc_streaming *stream)
-- 
2.28.0

Re: [PATCH 7/7] media: usb: uvc: no need to check return value of debugfs_create functions

[Laurent Pinchart]

Hi Greg,

Thank you for the patch.

On Tue, Aug 18, 2020 at 03:36:08PM +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.

Is there no value in warning the user that something went wrong ? Silent
failures are harder to debug.

> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: linux-media@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/media/usb/uvc/uvc_debugfs.c | 20 ++++----------------
>  1 file changed, 4 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_debugfs.c b/drivers/media/usb/uvc/uvc_debugfs.c
> index 2b8af4b54117..1a1258d4ffca 100644
> --- a/drivers/media/usb/uvc/uvc_debugfs.c
> +++ b/drivers/media/usb/uvc/uvc_debugfs.c
> @@ -73,7 +73,6 @@ static struct dentry *uvc_debugfs_root_dir;
>  void uvc_debugfs_init_stream(struct uvc_streaming *stream)
>  {
>  	struct usb_device *udev = stream->dev->udev;
> -	struct dentry *dent;
>  	char dir_name[33];
>  
>  	if (uvc_debugfs_root_dir == NULL)
> @@ -82,22 +81,11 @@ void uvc_debugfs_init_stream(struct uvc_streaming *stream)
>  	snprintf(dir_name, sizeof(dir_name), "%u-%u-%u", udev->bus->busnum,
>  		 udev->devnum, stream->intfnum);
>  
> -	dent = debugfs_create_dir(dir_name, uvc_debugfs_root_dir);
> -	if (IS_ERR_OR_NULL(dent)) {
> -		uvc_printk(KERN_INFO, "Unable to create debugfs %s "
> -			   "directory.\n", dir_name);
> -		return;
> -	}
> -
> -	stream->debugfs_dir = dent;
> +	stream->debugfs_dir = debugfs_create_dir(dir_name,
> +						 uvc_debugfs_root_dir);
>  
> -	dent = debugfs_create_file("stats", 0444, stream->debugfs_dir,
> -				   stream, &uvc_debugfs_stats_fops);
> -	if (IS_ERR_OR_NULL(dent)) {
> -		uvc_printk(KERN_INFO, "Unable to create debugfs stats file.\n");
> -		uvc_debugfs_cleanup_stream(stream);
> -		return;
> -	}
> +	debugfs_create_file("stats", 0444, stream->debugfs_dir, stream,
> +			    &uvc_debugfs_stats_fops);
>  }
>  
>  void uvc_debugfs_cleanup_stream(struct uvc_streaming *stream)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 7/7] media: usb: uvc: no need to check return value of debugfs_create functions

[Laurent Pinchart]

Hi Greg,

On Wed, Aug 19, 2020 at 02:47:19AM +0300, Laurent Pinchart wrote:
> Hi Greg,
> 
> Thank you for the patch.
> 
> On Tue, Aug 18, 2020 at 03:36:08PM +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.
> 
> Is there no value in warning the user that something went wrong ? Silent
> failures are harder to debug.

Could yous share your opinion about this ?

> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> > Cc: linux-media@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >  drivers/media/usb/uvc/uvc_debugfs.c | 20 ++++----------------
> >  1 file changed, 4 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/media/usb/uvc/uvc_debugfs.c b/drivers/media/usb/uvc/uvc_debugfs.c
> > index 2b8af4b54117..1a1258d4ffca 100644
> > --- a/drivers/media/usb/uvc/uvc_debugfs.c
> > +++ b/drivers/media/usb/uvc/uvc_debugfs.c
> > @@ -73,7 +73,6 @@ static struct dentry *uvc_debugfs_root_dir;
> >  void uvc_debugfs_init_stream(struct uvc_streaming *stream)
> >  {
> >  	struct usb_device *udev = stream->dev->udev;
> > -	struct dentry *dent;
> >  	char dir_name[33];
> >  
> >  	if (uvc_debugfs_root_dir == NULL)
> > @@ -82,22 +81,11 @@ void uvc_debugfs_init_stream(struct uvc_streaming *stream)
> >  	snprintf(dir_name, sizeof(dir_name), "%u-%u-%u", udev->bus->busnum,
> >  		 udev->devnum, stream->intfnum);
> >  
> > -	dent = debugfs_create_dir(dir_name, uvc_debugfs_root_dir);
> > -	if (IS_ERR_OR_NULL(dent)) {
> > -		uvc_printk(KERN_INFO, "Unable to create debugfs %s "
> > -			   "directory.\n", dir_name);
> > -		return;
> > -	}
> > -
> > -	stream->debugfs_dir = dent;
> > +	stream->debugfs_dir = debugfs_create_dir(dir_name,
> > +						 uvc_debugfs_root_dir);
> >  
> > -	dent = debugfs_create_file("stats", 0444, stream->debugfs_dir,
> > -				   stream, &uvc_debugfs_stats_fops);
> > -	if (IS_ERR_OR_NULL(dent)) {
> > -		uvc_printk(KERN_INFO, "Unable to create debugfs stats file.\n");
> > -		uvc_debugfs_cleanup_stream(stream);
> > -		return;
> > -	}
> > +	debugfs_create_file("stats", 0444, stream->debugfs_dir, stream,
> > +			    &uvc_debugfs_stats_fops);
> >  }
> >  
> >  void uvc_debugfs_cleanup_stream(struct uvc_streaming *stream)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 7/7] media: usb: uvc: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

On Thu, Sep 17, 2020 at 03:25:50PM +0300, Laurent Pinchart wrote:
> Hi Greg,
> 
> On Wed, Aug 19, 2020 at 02:47:19AM +0300, Laurent Pinchart wrote:
> > Hi Greg,
> > 
> > Thank you for the patch.
> > 
> > On Tue, Aug 18, 2020 at 03:36:08PM +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.
> > 
> > Is there no value in warning the user that something went wrong ? Silent
> > failures are harder to debug.
> 
> Could yous share your opinion about this ?

For debugfs, this isn't an issue, what can a user do with something like
"debugfs isn't working?  What does that mean???"

And if we _really_ want warnings like this, it should go into the
debugfs core, not require this to be done for every debugfs user, right?

debugfs is just there for kernel developers to help debug things, it's
not a dependancy on any userspace functionality, so if it works or not
should not be an issue for any user.

Unless that user is a kernel developer of course :)

thanks,

greg k-h

Re: [PATCH 7/7] media: usb: uvc: no need to check return value of debugfs_create functions

[Laurent Pinchart]

Hi Greg,

On Thu, Sep 17, 2020 at 02:34:26PM +0200, Greg Kroah-Hartman wrote:
> On Thu, Sep 17, 2020 at 03:25:50PM +0300, Laurent Pinchart wrote:
> > On Wed, Aug 19, 2020 at 02:47:19AM +0300, Laurent Pinchart wrote:
> > > On Tue, Aug 18, 2020 at 03:36:08PM +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.
> > > 
> > > Is there no value in warning the user that something went wrong ? Silent
> > > failures are harder to debug.
> > 
> > Could yous share your opinion about this ?
> 
> For debugfs, this isn't an issue, what can a user do with something like
> "debugfs isn't working?  What does that mean???"
> 
> And if we _really_ want warnings like this, it should go into the
> debugfs core, not require this to be done for every debugfs user, right?
> 
> debugfs is just there for kernel developers to help debug things, it's
> not a dependancy on any userspace functionality, so if it works or not
> should not be an issue for any user.
> 
> Unless that user is a kernel developer of course :)

Exactly my point :-)

I'm fine moving the error message to the debugfs core itself instead of
duplicating it in drivers. Maybe it's already there though, I haven't
checked. Not printing any message isn't a great idea in my opinion, it
makes debugging more difficult. I can't count the number of times where
I've had to add printk's and recompile the kernel to debug issues that
really should have generated at least a dev_dbg().

-- 
Regards,

Laurent Pinchart

Re: [PATCH 7/7] media: usb: uvc: no need to check return value of debugfs_create functions

[Greg Kroah-Hartman]

On Thu, Sep 17, 2020 at 03:37:57PM +0300, Laurent Pinchart wrote:
> Hi Greg,
> 
> On Thu, Sep 17, 2020 at 02:34:26PM +0200, Greg Kroah-Hartman wrote:
> > On Thu, Sep 17, 2020 at 03:25:50PM +0300, Laurent Pinchart wrote:
> > > On Wed, Aug 19, 2020 at 02:47:19AM +0300, Laurent Pinchart wrote:
> > > > On Tue, Aug 18, 2020 at 03:36:08PM +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.
> > > > 
> > > > Is there no value in warning the user that something went wrong ? Silent
> > > > failures are harder to debug.
> > > 
> > > Could yous share your opinion about this ?
> > 
> > For debugfs, this isn't an issue, what can a user do with something like
> > "debugfs isn't working?  What does that mean???"
> > 
> > And if we _really_ want warnings like this, it should go into the
> > debugfs core, not require this to be done for every debugfs user, right?
> > 
> > debugfs is just there for kernel developers to help debug things, it's
> > not a dependancy on any userspace functionality, so if it works or not
> > should not be an issue for any user.
> > 
> > Unless that user is a kernel developer of course :)
> 
> Exactly my point :-)
> 
> I'm fine moving the error message to the debugfs core itself instead of
> duplicating it in drivers. Maybe it's already there though, I haven't
> checked. Not printing any message isn't a great idea in my opinion, it
> makes debugging more difficult. I can't count the number of times where
> I've had to add printk's and recompile the kernel to debug issues that
> really should have generated at least a dev_dbg().

There are a lot of error messages that debugfs will print out if it can
not create a file:

inode.c 329 pr_err("Unable to pin filesystem for file '%s'\n", name);
inode.c 348 pr_err("Directory '%s' with parent '%s' already present!\n",
inode.c 351 pr_err("File '%s' in directory '%s' already present!\n",
inode.c 402 pr_err("out of free dentries, can not create file '%s'\n",
inode.c 563 pr_err("out of free dentries, can not create directory '%s'\n",
inode.c 610 pr_err("out of free dentries, can not create automount '%s'\n",
inode.c 668 pr_err("out of free dentries, can not create symlink '%s'\n",

So I think you are safe here.  If we are missing any, I'll gladly add
them.

Also given that you've never noticed this being a real error, means it's
probably not an issue :)

thanks,

greg k-h