[PATCH 1/4] staging: dgnc: remove unneeded else

[PATCH 1/4] staging: dgnc: remove unneeded else

[Sudip Mukherjee]

If pci_enable_device() fails then we can return directly.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
---

I saw there is some pending dgnc patch in outreachy, so this series
might not apply. This series is based on staging-testing.
I will resend v2 if it fails.

 drivers/staging/dgnc/dgnc_driver.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_driver.c b/drivers/staging/dgnc/dgnc_driver.c
index fc6d298..c32f208 100644
--- a/drivers/staging/dgnc/dgnc_driver.c
+++ b/drivers/staging/dgnc/dgnc_driver.c
@@ -283,13 +283,13 @@ static int dgnc_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	/* wake up and enable device */
 	rc = pci_enable_device(pdev);
 
-	if (rc < 0) {
-		rc = -EIO;
-	} else {
-		rc = dgnc_found_board(pdev, ent->driver_data);
-		if (rc == 0)
-			dgnc_NumBoards++;
-	}
+	if (rc)
+		return -EIO;
+
+	rc = dgnc_found_board(pdev, ent->driver_data);
+	if (rc == 0)
+		dgnc_NumBoards++;
+
 	return rc;
 }
 
-- 
1.9.1

[PATCH 2/4] staging: dgnc: remove pci_unregister_driver

[Sudip Mukherjee]

If pci_register_driver() fails then dgnc_NumBoards can never be more
than zero. dgnc_NumBoards is incremented only at the end of a successful
probe. And moreover if the pci driver has failed to register then we
never need to unregister it. Lets just print the warning, perform the
cleanup and exit with the error code.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
---
 drivers/staging/dgnc/dgnc_driver.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_driver.c b/drivers/staging/dgnc/dgnc_driver.c
index c32f208..1d1d5c8 100644
--- a/drivers/staging/dgnc/dgnc_driver.c
+++ b/drivers/staging/dgnc/dgnc_driver.c
@@ -181,23 +181,14 @@ static int __init dgnc_init_module(void)
 	 * Find and configure all the cards
 	 */
 	rc = pci_register_driver(&dgnc_driver);
-
-	/*
-	 * If something went wrong in the scan, bail out of driver.
-	 */
-	if (rc < 0) {
-		/* Only unregister if it was actually registered. */
-		if (dgnc_NumBoards)
-			pci_unregister_driver(&dgnc_driver);
-		else
-			pr_warn("WARNING: dgnc driver load failed.  No Digi Neo or Classic boards found.\n");
-
+	if (rc) {
+		pr_warn("WARNING: dgnc driver load failed.  No Digi Neo or Classic boards found.\n");
 		dgnc_cleanup_module();
-	} else {
-		dgnc_create_driver_sysfiles(&dgnc_driver);
+		return rc;
 	}
+	dgnc_create_driver_sysfiles(&dgnc_driver);
 
-	return rc;
+	return 0;
 }
 
 module_init(dgnc_init_module);
-- 
1.9.1

[PATCH 3/4] staging: dgnc: unregister pci driver

[Sudip Mukherjee]

We may choose to load the module without the hardware present. That will
register the pci driver but since probe will not succeed so
dgnc_NumBoards will be 0. Now if we unload the module then the pci
driver stays registered as dgnc_NumBoards is 0. And if we try to load
the module again it fails with the error:
"Driver 'dgnc' is already registered."

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
---
 drivers/staging/dgnc/dgnc_driver.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_driver.c b/drivers/staging/dgnc/dgnc_driver.c
index 1d1d5c8..22a92d1 100644
--- a/drivers/staging/dgnc/dgnc_driver.c
+++ b/drivers/staging/dgnc/dgnc_driver.c
@@ -156,8 +156,7 @@ static void dgnc_cleanup_module(void)
 
 	dgnc_tty_post_uninit();
 
-	if (dgnc_NumBoards)
-		pci_unregister_driver(&dgnc_driver);
+	pci_unregister_driver(&dgnc_driver);
 }
 
 /*
-- 
1.9.1

[PATCH 4/4] staging: dgnc: cleanup properly

[Sudip Mukherjee]

dgnc_cleanup_module() was called when the module unloaded to do a total
cleanup and it was also called if pci_register_driver() fails. But
dgnc_cleanup_module() will try dgnc_remove_driver_sysfiles() but the
sysfiles will be created only if pci_register_driver() succeeds.
So if pci_register_driver() fails and we try dgnc_cleanup_module() then we
were getting:

[  942.001479] BUG: unable to handle kernel NULL pointer dereference at 00000018
[  942.001482] IP: [<c122c7a8>] sysfs_remove_file_ns+0x8/0x20

with part of the call trace as:

[  942.001544] Call Trace:
[  942.001555]  [<c149acc6>] driver_remove_file+0x16/0x20
[  942.001571]  [<f864a708>] dgnc_remove_driver_sysfiles+0x18/0x40 [dgnc]
[  942.001575]  [<f8643ac7>] dgnc_cleanup_module+0x47/0x260 [dgnc]
[  942.001577]  [<f86cb000>] ? 0xf86cb000
[  942.001580]  [<f86cb1e6>] dgnc_init_module+0x1e6/0x1000 [dgnc]

Lets have a separate cleanup function which will execute
dgnc_remove_driver_sysfiles() depending on the argument passed to it.

Reported-by: Navy Cheng <navych@126.com>
Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
---

Hi Greg,
If you remember this problem was reported by Navy Cheng on the
kernelnewbies list but his report did not contain the call trace. He
just reported that his system hangs after reloading dgnc module.

 drivers/staging/dgnc/dgnc_driver.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/dgnc/dgnc_driver.c b/drivers/staging/dgnc/dgnc_driver.c
index 22a92d1..4eb410e 100644
--- a/drivers/staging/dgnc/dgnc_driver.c
+++ b/drivers/staging/dgnc/dgnc_driver.c
@@ -125,12 +125,7 @@ static struct pci_driver dgnc_driver = {
  *
  ************************************************************************/
 
-/*
- * dgnc_cleanup_module()
- *
- * Module unload.  This is where it all ends.
- */
-static void dgnc_cleanup_module(void)
+static void cleanup(bool sysfiles)
 {
 	int i;
 	unsigned long flags;
@@ -142,7 +137,8 @@ static void dgnc_cleanup_module(void)
 	/* Turn off poller right away. */
 	del_timer_sync(&dgnc_poll_timer);
 
-	dgnc_remove_driver_sysfiles(&dgnc_driver);
+	if (sysfiles)
+		dgnc_remove_driver_sysfiles(&dgnc_driver);
 
 	device_destroy(dgnc_class, MKDEV(dgnc_Major, 0));
 	class_destroy(dgnc_class);
@@ -155,7 +151,16 @@ static void dgnc_cleanup_module(void)
 	}
 
 	dgnc_tty_post_uninit();
+}
 
+/*
+ * dgnc_cleanup_module()
+ *
+ * Module unload.  This is where it all ends.
+ */
+static void dgnc_cleanup_module(void)
+{
+	cleanup(true);
 	pci_unregister_driver(&dgnc_driver);
 }
 
@@ -182,7 +187,7 @@ static int __init dgnc_init_module(void)
 	rc = pci_register_driver(&dgnc_driver);
 	if (rc) {
 		pr_warn("WARNING: dgnc driver load failed.  No Digi Neo or Classic boards found.\n");
-		dgnc_cleanup_module();
+		cleanup(false);
 		return rc;
 	}
 	dgnc_create_driver_sysfiles(&dgnc_driver);
-- 
1.9.1