[PATCH v5 1/5] staging: unisys: remove redundant variable

[PATCH v5 1/5] staging: unisys: remove redundant variable

[Sudip Mukherjee]

remove the variable "registered", which was used in the cleanup() to
detect if the driver has successfully initialized. the cleanup()
is called from module_exit, so its obvious that the module has
successfully initialized. if the initialization had failed, then
we will never be in the cleanup().

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

v5: reordered the patch series
v4: messed up the subject in v3
v3: broke the previous patch in series

 drivers/staging/unisys/visorchipset/file.c | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/file.c b/drivers/staging/unisys/visorchipset/file.c
index 9ca7f1e..b426762 100644
--- a/drivers/staging/unisys/visorchipset/file.c
+++ b/drivers/staging/unisys/visorchipset/file.c
@@ -31,7 +31,6 @@
 static struct cdev file_cdev;
 static struct visorchannel **file_controlvm_channel;
 static dev_t majordev = -1; /**< indicates major num for device */
-static BOOL registered = FALSE;
 
 static int visorchipset_open(struct inode *inode, struct file *file);
 static int visorchipset_release(struct inode *inode, struct file *file);
@@ -62,12 +61,10 @@ visorchipset_file_init(dev_t major_dev, struct visorchannel **controlvm_channel)
 		/* dynamic major device number registration required */
 		if (alloc_chrdev_region(&majordev, 0, 1, MYDRVNAME) < 0)
 			return -1;
-		registered = TRUE;
 	} else {
 		/* static major device number registration required */
 		if (register_chrdev_region(majordev, 1, MYDRVNAME) < 0)
 			return -1;
-		registered = TRUE;
 	}
 	rc = cdev_add(&file_cdev, MKDEV(MAJOR(majordev), 0), 1);
 	if (rc  < 0)
@@ -81,12 +78,9 @@ visorchipset_file_cleanup(void)
 	if (file_cdev.ops != NULL)
 		cdev_del(&file_cdev);
 	file_cdev.ops = NULL;
-	if (registered) {
-		if (MAJOR(majordev) >= 0) {
-			unregister_chrdev_region(majordev, 1);
-			majordev = MKDEV(0, 0);
-		}
-		registered = FALSE;
+	if (MAJOR(majordev) >= 0) {
+		unregister_chrdev_region(majordev, 1);
+		majordev = MKDEV(0, 0);
 	}
 }
 
-- 
1.8.1.2

[PATCH v5 2/5] staging: unisys: use local variable

[Sudip Mukherjee]

we are getting dev_t as an argument in the function, so use the local
variable instead of the global variable "majordev".
this global variable will be removed in one of the next patch of the
series.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

v5: reordered the patch series
v4: messed up the subject in v3
v3: broke the previous patch in series

 drivers/staging/unisys/visorchipset/file.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/file.c b/drivers/staging/unisys/visorchipset/file.c
index b426762..6ebe7f7 100644
--- a/drivers/staging/unisys/visorchipset/file.c
+++ b/drivers/staging/unisys/visorchipset/file.c
@@ -57,16 +57,16 @@ visorchipset_file_init(dev_t major_dev, struct visorchannel **controlvm_channel)
 	majordev = major_dev;
 	cdev_init(&file_cdev, &visorchipset_fops);
 	file_cdev.owner = THIS_MODULE;
-	if (MAJOR(majordev) == 0) {
+	if (MAJOR(major_dev) == 0) {
 		/* dynamic major device number registration required */
-		if (alloc_chrdev_region(&majordev, 0, 1, MYDRVNAME) < 0)
+		if (alloc_chrdev_region(&major_dev, 0, 1, MYDRVNAME) < 0)
 			return -1;
 	} else {
 		/* static major device number registration required */
-		if (register_chrdev_region(majordev, 1, MYDRVNAME) < 0)
+		if (register_chrdev_region(major_dev, 1, MYDRVNAME) < 0)
 			return -1;
 	}
-	rc = cdev_add(&file_cdev, MKDEV(MAJOR(majordev), 0), 1);
+	rc = cdev_add(&file_cdev, MKDEV(MAJOR(major_dev), 0), 1);
 	if (rc  < 0)
 		return -1;
 	return 0;
-- 
1.8.1.2

[PATCH v5 3/5] staging: unisys: use local variable in cleanup

[Sudip Mukherjee]

the dev_t was being stored in visorchipset_platform_device.dev.devt
while initializing the module. so pass that value as an argument to
cleanup() so that it can use this local variable instead of the global
variable.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

v5: reordered the patch series
v4: messed up the subject in v3
v3: broke the previous patch in series

 drivers/staging/unisys/visorchipset/file.c              | 6 +++---
 drivers/staging/unisys/visorchipset/file.h              | 2 +-
 drivers/staging/unisys/visorchipset/visorchipset_main.c | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/file.c b/drivers/staging/unisys/visorchipset/file.c
index 6ebe7f7..257cfdd 100644
--- a/drivers/staging/unisys/visorchipset/file.c
+++ b/drivers/staging/unisys/visorchipset/file.c
@@ -73,13 +73,13 @@ visorchipset_file_init(dev_t major_dev, struct visorchannel **controlvm_channel)
 }
 
 void
-visorchipset_file_cleanup(void)
+visorchipset_file_cleanup(dev_t major_dev)
 {
 	if (file_cdev.ops != NULL)
 		cdev_del(&file_cdev);
 	file_cdev.ops = NULL;
-	if (MAJOR(majordev) >= 0) {
-		unregister_chrdev_region(majordev, 1);
+	if (MAJOR(major_dev) >= 0) {
+		unregister_chrdev_region(major_dev, 1);
 		majordev = MKDEV(0, 0);
 	}
 }
diff --git a/drivers/staging/unisys/visorchipset/file.h b/drivers/staging/unisys/visorchipset/file.h
index dc7a195..51f7699 100644
--- a/drivers/staging/unisys/visorchipset/file.h
+++ b/drivers/staging/unisys/visorchipset/file.h
@@ -22,6 +22,6 @@
 
 int visorchipset_file_init(dev_t majorDev,
 			   struct visorchannel **pControlVm_channel);
-void visorchipset_file_cleanup(void);
+void visorchipset_file_cleanup(dev_t major_dev);
 
 #endif
diff --git a/drivers/staging/unisys/visorchipset/visorchipset_main.c b/drivers/staging/unisys/visorchipset/visorchipset_main.c
index 9c8605d..f2663d2c 100644
--- a/drivers/staging/unisys/visorchipset/visorchipset_main.c
+++ b/drivers/staging/unisys/visorchipset/visorchipset_main.c
@@ -2278,7 +2278,7 @@ visorchipset_exit(void)
 
 	visorchannel_destroy(controlvm_channel);
 
-	visorchipset_file_cleanup();
+	visorchipset_file_cleanup(visorchipset_platform_device.dev.devt);
 	POSTCODE_LINUX_2(DRIVER_EXIT_PC, POSTCODE_SEVERITY_INFO);
 }
 
-- 
1.8.1.2

[PATCH v5 4/5] staging: unisys: remove global dev_t

[Sudip Mukherjee]

the global variable majordev is no longer required, as it is not being
used anywhere.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

v5: reordered the patch series
v4: messed up the subject in v3
v3: broke the previous patch in series

 drivers/staging/unisys/visorchipset/file.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/file.c b/drivers/staging/unisys/visorchipset/file.c
index 257cfdd..890869a 100644
--- a/drivers/staging/unisys/visorchipset/file.c
+++ b/drivers/staging/unisys/visorchipset/file.c
@@ -30,7 +30,6 @@
 
 static struct cdev file_cdev;
 static struct visorchannel **file_controlvm_channel;
-static dev_t majordev = -1; /**< indicates major num for device */
 
 static int visorchipset_open(struct inode *inode, struct file *file);
 static int visorchipset_release(struct inode *inode, struct file *file);
@@ -54,7 +53,6 @@ visorchipset_file_init(dev_t major_dev, struct visorchannel **controlvm_channel)
 	int rc = 0;
 
 	file_controlvm_channel = controlvm_channel;
-	majordev = major_dev;
 	cdev_init(&file_cdev, &visorchipset_fops);
 	file_cdev.owner = THIS_MODULE;
 	if (MAJOR(major_dev) == 0) {
@@ -80,7 +78,6 @@ visorchipset_file_cleanup(dev_t major_dev)
 	file_cdev.ops = NULL;
 	if (MAJOR(major_dev) >= 0) {
 		unregister_chrdev_region(major_dev, 1);
-		majordev = MKDEV(0, 0);
 	}
 }
 
-- 
1.8.1.2

[PATCH v5 5/5] staging: unisys: remove comparison

[Sudip Mukherjee]

the comparison is always true as the dev_t has been initialized in the
init function and we are sending that initialized dev_t to the
cleanup().

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

v5: reordered the patch series
v4: messed up the subject in v3
v3: broke the previous patch in series

 drivers/staging/unisys/visorchipset/file.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/unisys/visorchipset/file.c b/drivers/staging/unisys/visorchipset/file.c
index 890869a..39b19af 100644
--- a/drivers/staging/unisys/visorchipset/file.c
+++ b/drivers/staging/unisys/visorchipset/file.c
@@ -76,9 +76,7 @@ visorchipset_file_cleanup(dev_t major_dev)
 	if (file_cdev.ops != NULL)
 		cdev_del(&file_cdev);
 	file_cdev.ops = NULL;
-	if (MAJOR(major_dev) >= 0) {
-		unregister_chrdev_region(major_dev, 1);
-	}
+	unregister_chrdev_region(major_dev, 1);
 }
 
 static int
-- 
1.8.1.2

Re: [PATCH v5 1/5] staging: unisys: remove redundant variable

[Dan Carpenter]

Great.  :)

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>

regards,
dan carpenter

Re: [PATCH v5 1/5] staging: unisys: remove redundant variable

[Greg Kroah-Hartman]

On Tue, Mar 24, 2015 at 08:47:26PM +0530, Sudip Mukherjee wrote:
> remove the variable "registered", which was used in the cleanup() to
> detect if the driver has successfully initialized. the cleanup()
> is called from module_exit, so its obvious that the module has
> successfully initialized. if the initialization had failed, then
> we will never be in the cleanup().
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
> 
> v5: reordered the patch series
> v4: messed up the subject in v3
> v3: broke the previous patch in series

I need an ack from Benjamin and/or David before I can take these, as
they are the maintainers of the driver, and have the ability to test
these patches.

I'll just wait to apply them until that happens.

Ben/David?

thanks,

greg k-h

Re: [PATCH v5 1/5] staging: unisys: remove redundant variable

[Ben Romer]

On 03/26/2015 08:01 AM, Greg Kroah-Hartman wrote:
> I need an ack from Benjamin and/or David before I can take these, as
> they are the maintainers of the driver, and have the ability to test
> these patches.
>
> I'll just wait to apply them until that happens.
>
> Ben/David?

I'll test them today. :)

-- Ben

Re: [PATCH v5 1/5] staging: unisys: remove redundant variable

[Ben Romer]

On 03/26/2015 09:47 AM, Ben Romer wrote:
> On 03/26/2015 08:01 AM, Greg Kroah-Hartman wrote:
>> I need an ack from Benjamin and/or David before I can take these, as
>> they are the maintainers of the driver, and have the ability to test
>> these patches.
>>
>> I'll just wait to apply them until that happens.
>>
>> Ben/David?
>
> I'll test them today. :)
>
> -- Ben
>

I've tested them and everything appears to function correctly, so please 
accept this patch series.

Thanks!

-- Ben

Signed-off-by: Benjamin Romer <benjamin.romer@unisys.com>