Fix debugfs_create_file's error checking method for usb/gadget/

PATCH Superseded Zhaolei 3 participants 6 messages Oct 17, 2008 – Oct 20, 2008

Subsystems (2)

USB Arm Samsung Exynos

lore

Share & tools

lore

Zhaolei

author

2008-10-17 12:56 UTC #0 lore raw

PATCH 1/1 Fix debugfs_create_file's error checking method for usb/gadget/

Hi,

debugfs_create_file() returns NULL if an error occurs, returns -ENODEV when debugfs is not enabled in the kernel.

Signed-off-by Zhao Lei <[email protected]>

---
 drivers/usb/gadget/s3c2410_udc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/usb/gadget/s3c2410_udc.c +1 -1

index 29d13eb..4ba50ef 100644
@@ -1894,7 +1894,7 @@ static int s3c2410_udc_probe(struct platform_device *pdev)
 		udc->regs_info = debugfs_create_file("registers", S_IRUGO,
 				s3c2410_udc_debugfs_root,
 				udc, &s3c2410_udc_debugfs_fops);
-		if (IS_ERR(udc->regs_info)) {
+		if (IS_ERR(udc->regs_info) || !udc->regs_info) {
 			dev_warn(dev, "debugfs file creation failed %ld\n",
 				 PTR_ERR(udc->regs_info));
 			udc->regs_info = NULL;

Signature · 1 line

1.5.5.3

Alan Stern

2008-10-17 15:18 UTC #1 lore raw

Quoted 15 lines · Zhaolei

Hi,
debugfs_create_file() returns NULL if an error occurs, returns -ENODEV when debugfs is not enabled in the kernel.
Signed-off-by Zhao Lei <[email protected]>
---
 drivers/usb/gadget/s3c2410_udc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/usb/gadget/s3c2410_udc.c +0 -0
index 29d13eb..4ba50ef 100644

drivers/usb/gadget/s3c2410_udc.c static int s3c2410_udc_probe(struct platform_device *pdev) · line 1900

Quoted 8 lines

drivers/usb/gadget/s3c2410_udc.c +1 -1

@@ -1894,7 +1894,7 @@ static int s3c2410_udc_probe(struct platform_device *pdev)
 		udc->regs_info = debugfs_create_file("registers", S_IRUGO,
 				s3c2410_udc_debugfs_root,
 				udc, &s3c2410_udc_debugfs_fops);
-		if (IS_ERR(udc->regs_info)) {
+		if (IS_ERR(udc->regs_info) || !udc->regs_info) {
 			dev_warn(dev, "debugfs file creation failed %ld\n",
 				 PTR_ERR(udc->regs_info));
 			udc->regs_info = NULL;

In fact the original code and your patch are both wrong. The test should simply be:

		if (!udc->regs_info) {
			dev_warn(dev, "debugfs file creation failed\n");

(The line setting udc->regs_info to NULL can then be removed.)

The driver should be able to work even if debugfs isn't enabled in the kernel.

Alan Stern

From: "Alan Stern" <[email protected]>
To: "Zhaolei" <[email protected]>
Cc: <[email protected]>; <[email protected]>; <[email protected]>
Sent: Friday, October 17, 2008 11:18 PM
Subject: Re: [PATCH] Fix debugfs_create_file's error checking method for usb/gadget/

Quoted 29 lines · Zhaolei

Quoted 16 lines
Hi,
debugfs_create_file() returns NULL if an error occurs, returns -ENODEV when debugfs is not enabled in the kernel.
Signed-off-by Zhao Lei <[email protected]>
---
 drivers/usb/gadget/s3c2410_udc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/usb/gadget/s3c2410_udc.c +0 -0
index 29d13eb..4ba50ef 100644
drivers/usb/gadget/s3c2410_udc.c static int s3c2410_udc_probe(struct platform_device *pdev) · line 1900
Quoted 8 lines
drivers/usb/gadget/s3c2410_udc.c +1 -1
@@ -1894,7 +1894,7 @@ static int s3c2410_udc_probe(struct platform_device *pdev)
 udc->regs_info = debugfs_create_file("registers", S_IRUGO,
 s3c2410_udc_debugfs_root,
 udc, &s3c2410_udc_debugfs_fops);
- if (IS_ERR(udc->regs_info)) {
+ if (IS_ERR(udc->regs_info) || !udc->regs_info) {
 dev_warn(dev, "debugfs file creation failed %ld\n",
 PTR_ERR(udc->regs_info));
 udc->regs_info = NULL;
In fact the original code and your patch are both wrong. The test should simply be:

Quoted 3 lines

if (!udc->regs_info) {
dev_warn(dev, "debugfs file creation failed\n");

Hello, Alan Stern

If we only check if (!udc->regs_info), that is no problem except that we can't see dev_warn when debugfs isn't enabled in the kernel. It this warning message is not necessary, I agree with you.

Thanks!

Quoted 9 lines

(The line setting udc->regs_info to NULL can then be removed.)
The driver should be able to work even if debugfs isn't enabled in the kernel.
Alan Stern
ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éÝ¶¥Šwÿº{.nÇ+‰·¥Š{±þG«�éÿŠ{ayºÊ‡Ú™ë,j¢f£¢·hš�ï�êÿ‘êçz_è®(éšŽŠÝ¢j"�ú¶m§ÿÿ¾«þG«�éÿ¢¸?™¨èÚ&£ø§~�á¶iO•æ¬z·švØ^¶m§ÿÿÃÿ¶ìÿ¢¸?–I¥

Zhaolei

author

2008-10-20 01:21 UTC #3 lore raw

Quoted 29 lines · Zhaolei

Quoted 16 lines
Hi,
debugfs_create_file() returns NULL if an error occurs, returns -ENODEV when debugfs is not enabled in the kernel.
Signed-off-by Zhao Lei <[email protected]>
---
 drivers/usb/gadget/s3c2410_udc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/usb/gadget/s3c2410_udc.c +0 -0
index 29d13eb..4ba50ef 100644
drivers/usb/gadget/s3c2410_udc.c static int s3c2410_udc_probe(struct platform_device *pdev) · line 1900
Quoted 8 lines
drivers/usb/gadget/s3c2410_udc.c +1 -1
@@ -1894,7 +1894,7 @@ static int s3c2410_udc_probe(struct platform_device *pdev)
 udc->regs_info = debugfs_create_file("registers", S_IRUGO,
 s3c2410_udc_debugfs_root,
 udc, &s3c2410_udc_debugfs_fops);
- if (IS_ERR(udc->regs_info)) {
+ if (IS_ERR(udc->regs_info) || !udc->regs_info) {
 dev_warn(dev, "debugfs file creation failed %ld\n",
 PTR_ERR(udc->regs_info));
 udc->regs_info = NULL;
In fact the original code and your patch are both wrong. The test should simply be:

Quoted 3 lines

if (!udc->regs_info) {
dev_warn(dev, "debugfs file creation failed\n");

Hello, Alan Stern

If we only check if (!udc->regs_info), that is no problem except that we can't see dev_warn when debugfs isn't enabled in the kernel. It this warning message is not necessary, I agree with you. ->If this warning message is not necessary, I agree with you.

Thanks!

Quoted 9 lines

(The line setting udc->regs_info to NULL can then be removed.)
The driver should be able to work even if debugfs isn't enabled in the kernel.
Alan Stern
N²rys²¶v-ºn?S'z¶¡}²zjv¾'z?z¢s~?ÿz®¢T¢¢"jy.A¶ÿ¶®'ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éÝ¶¥Šwÿº{.nÇ+‰·¥Š{±þG«�éÿŠ{ayºÊ‡Ú™ë,j¢f£¢·hš�ï�êÿ‘êçz_è®(éšŽŠÝ¢j"�ú¶m§ÿÿ¾«þG«�éÿ¢¸?™¨èÚ&£ø§~�á¶iO•æ¬z·švØ^¶m§ÿÿÃÿ¶ìÿ¢¸?–I¥

Greg KH

2008-10-20 03:49 UTC #4 lore raw

In reply to Zhaolei

Quoted 36 lines · Zhaolei

From: "Alan Stern" <[email protected]>
To: "Zhaolei" <[email protected]>
Cc: <[email protected]>; <[email protected]>; <[email protected]>
Sent: Friday, October 17, 2008 11:18 PM
Subject: Re: [PATCH] Fix debugfs_create_file's error checking method for usb/gadget/
Quoted 29 lines · Zhaolei
Quoted 16 lines
Hi,
debugfs_create_file() returns NULL if an error occurs, returns -ENODEV when debugfs is not enabled in the kernel.
Signed-off-by Zhao Lei <[email protected]>
---
 drivers/usb/gadget/s3c2410_udc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/usb/gadget/s3c2410_udc.c +0 -0
index 29d13eb..4ba50ef 100644
drivers/usb/gadget/s3c2410_udc.c static int s3c2410_udc_probe(struct platform_device *pdev) · line 1900
Quoted 8 lines
drivers/usb/gadget/s3c2410_udc.c +1 -1
@@ -1894,7 +1894,7 @@ static int s3c2410_udc_probe(struct platform_device *pdev)
 udc->regs_info = debugfs_create_file("registers", S_IRUGO,
 s3c2410_udc_debugfs_root,
 udc, &s3c2410_udc_debugfs_fops);
- if (IS_ERR(udc->regs_info)) {
+ if (IS_ERR(udc->regs_info) || !udc->regs_info) {
 dev_warn(dev, "debugfs file creation failed %ld\n",
 PTR_ERR(udc->regs_info));
 udc->regs_info = NULL;
In fact the original code and your patch are both wrong. The test should simply be:

Quoted 8 lines

Quoted 3 lines
if (!udc->regs_info) {
dev_warn(dev, "debugfs file creation failed\n");
Hello, Alan Stern
If we only check if (!udc->regs_info), that is no problem except that we can't see dev_warn when debugfs isn't enabled in the kernel. It this warning message is not necessary, I agree with you.

Why would you want to see that message if debugfs wasn't selected? You shouldn't need to see it, that is why the debugfs code doesn't return NULL if it is compiled out. I did it that way to make all callers much simpler :)

thanks,

greg k-h

Zhaolei

author

2008-10-20 10:54 UTC #5 lore raw

PATCH v2 1/1 Fix debugfs_create_file's error checking method for usb/gadget/

debugfs_create_file() returns NULL if an error occurs, returns -ENODEV when debugfs is not enabled in the kernel.

Signed-off-by Zhao Lei <[email protected]>

---
 drivers/usb/gadget/s3c2410_udc.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

drivers/usb/gadget/s3c2410_udc.c +2 -5

index 48f51b1..00ba06b 100644
@@ -1894,11 +1894,8 @@ static int s3c2410_udc_probe(struct platform_device *pdev)
 		udc->regs_info = debugfs_create_file("registers", S_IRUGO,
 				s3c2410_udc_debugfs_root,
 				udc, &s3c2410_udc_debugfs_fops);
-		if (IS_ERR(udc->regs_info)) {
-			dev_warn(dev, "debugfs file creation failed %ld\n",
-				 PTR_ERR(udc->regs_info));
-			udc->regs_info = NULL;
-		}
+		if (!udc->regs_info)
+			dev_warn(dev, "debugfs file creation failed\n");
 	}
 
 	dev_dbg(dev, "probe ok\n");

Signature · 1 line

1.5.5.3