All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 1/4] staging: unisys: remove unneeded comparison
@ 2015-03-24 12:17 Sudip Mukherjee
  2015-03-24 12:17 ` [PATCH v4 2/4] staging: unisys: use local dev_t instead of global Sudip Mukherjee
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Sudip Mukherjee @ 2015-03-24 12:17 UTC (permalink / raw)
  To: Benjamin Romer, David Kershner, Greg Kroah-Hartman
  Cc: sparmaintainer, devel, linux-kernel, Dan Carpenter, Sudip Mukherjee

visorchipset_file_cleanup() is called from visorchipset_exit() which
is the module_exit function and this function is executing means
module_init succeeded, so registered will always be true at this time.
and majordev has also been initialized in the init function. hence
these comparisons will always be true.

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

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

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

diff --git a/drivers/staging/unisys/visorchipset/file.c b/drivers/staging/unisys/visorchipset/file.c
index 9ca7f1e..cbed1ba2 100644
--- a/drivers/staging/unisys/visorchipset/file.c
+++ b/drivers/staging/unisys/visorchipset/file.c
@@ -81,13 +81,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;
-	}
+	unregister_chrdev_region(majordev, 1);
+	majordev = MKDEV(0, 0);
+	registered = FALSE;
 }
 
 static int
-- 
1.8.1.2


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

* [PATCH v4 2/4] staging: unisys: use local dev_t instead of global
  2015-03-24 12:17 [PATCH v4 1/4] staging: unisys: remove unneeded comparison Sudip Mukherjee
@ 2015-03-24 12:17 ` Sudip Mukherjee
  2015-03-24 12:17 ` [PATCH v4 3/4] staging: unisys: use local variable " Sudip Mukherjee
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Sudip Mukherjee @ 2015-03-24 12:17 UTC (permalink / raw)
  To: Benjamin Romer, David Kershner, Greg Kroah-Hartman
  Cc: sparmaintainer, devel, linux-kernel, Dan Carpenter, Sudip Mukherjee

the dev_t is stored in visorchipset_platform_device.dev.devt, so we
can pass that value as an argument to visorchipset_file_cleanup()
instead of using the global variable in file.c

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

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

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

diff --git a/drivers/staging/unisys/visorchipset/file.c b/drivers/staging/unisys/visorchipset/file.c
index cbed1ba2..d62908d 100644
--- a/drivers/staging/unisys/visorchipset/file.c
+++ b/drivers/staging/unisys/visorchipset/file.c
@@ -76,13 +76,12 @@ visorchipset_file_init(dev_t major_dev, struct visorchannel **controlvm_channel)
 }
 
 void
-visorchipset_file_cleanup(void)
+visorchipset_file_cleanup(dev_t majordev)
 {
 	if (file_cdev.ops != NULL)
 		cdev_del(&file_cdev);
 	file_cdev.ops = NULL;
 	unregister_chrdev_region(majordev, 1);
-	majordev = MKDEV(0, 0);
 	registered = FALSE;
 }
 
diff --git a/drivers/staging/unisys/visorchipset/file.h b/drivers/staging/unisys/visorchipset/file.h
index dc7a195..b32a472 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 majordev);
 
 #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


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

* [PATCH v4 3/4] staging: unisys: use local variable instead of global
  2015-03-24 12:17 [PATCH v4 1/4] staging: unisys: remove unneeded comparison Sudip Mukherjee
  2015-03-24 12:17 ` [PATCH v4 2/4] staging: unisys: use local dev_t instead of global Sudip Mukherjee
@ 2015-03-24 12:17 ` Sudip Mukherjee
  2015-03-24 12:58   ` Dan Carpenter
  2015-03-24 12:17 ` [PATCH v4 4/4] staging: unisys: remove unused variable Sudip Mukherjee
  2015-03-24 12:30 ` [PATCH v4 1/4] staging: unisys: remove unneeded comparison Dan Carpenter
  3 siblings, 1 reply; 7+ messages in thread
From: Sudip Mukherjee @ 2015-03-24 12:17 UTC (permalink / raw)
  To: Benjamin Romer, David Kershner, Greg Kroah-Hartman
  Cc: sparmaintainer, devel, linux-kernel, Dan Carpenter, Sudip Mukherjee

dev_t was passed as an argument to visorchipset_file_init() which was
storing that value in a global variable so that it can be used while
cleanup. now we are no longer using the global variable in the cleanup
so we donot need to keep the global variable anymore.

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

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

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

diff --git a/drivers/staging/unisys/visorchipset/file.c b/drivers/staging/unisys/visorchipset/file.c
index d62908d..a52939a4 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 BOOL registered = FALSE;
 
 static int visorchipset_open(struct inode *inode, struct file *file);
@@ -55,21 +54,20 @@ 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(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;
 		registered = TRUE;
 	} 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;
 		registered = TRUE;
 	}
-	rc = cdev_add(&file_cdev, MKDEV(MAJOR(majordev), 0), 1);
+	rc = cdev_add(&file_cdev, major_dev, 1);
 	if (rc  < 0)
 		return -1;
 	return 0;
-- 
1.8.1.2


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

* [PATCH v4 4/4] staging: unisys: remove unused variable
  2015-03-24 12:17 [PATCH v4 1/4] staging: unisys: remove unneeded comparison Sudip Mukherjee
  2015-03-24 12:17 ` [PATCH v4 2/4] staging: unisys: use local dev_t instead of global Sudip Mukherjee
  2015-03-24 12:17 ` [PATCH v4 3/4] staging: unisys: use local variable " Sudip Mukherjee
@ 2015-03-24 12:17 ` Sudip Mukherjee
  2015-03-24 12:30 ` [PATCH v4 1/4] staging: unisys: remove unneeded comparison Dan Carpenter
  3 siblings, 0 replies; 7+ messages in thread
From: Sudip Mukherjee @ 2015-03-24 12:17 UTC (permalink / raw)
  To: Benjamin Romer, David Kershner, Greg Kroah-Hartman
  Cc: sparmaintainer, devel, linux-kernel, Dan Carpenter, Sudip Mukherjee

after the first patch of this series the registered variable was
no longer required.

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

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

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

diff --git a/drivers/staging/unisys/visorchipset/file.c b/drivers/staging/unisys/visorchipset/file.c
index a52939a4..24cac93 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 BOOL registered = FALSE;
 
 static int visorchipset_open(struct inode *inode, struct file *file);
 static int visorchipset_release(struct inode *inode, struct file *file);
@@ -60,12 +59,10 @@ visorchipset_file_init(dev_t major_dev, struct visorchannel **controlvm_channel)
 		/* dynamic major device number registration required */
 		if (alloc_chrdev_region(&major_dev, 0, 1, MYDRVNAME) < 0)
 			return -1;
-		registered = TRUE;
 	} else {
 		/* static major device number registration required */
 		if (register_chrdev_region(major_dev, 1, MYDRVNAME) < 0)
 			return -1;
-		registered = TRUE;
 	}
 	rc = cdev_add(&file_cdev, major_dev, 1);
 	if (rc  < 0)
@@ -80,7 +77,6 @@ visorchipset_file_cleanup(dev_t majordev)
 		cdev_del(&file_cdev);
 	file_cdev.ops = NULL;
 	unregister_chrdev_region(majordev, 1);
-	registered = FALSE;
 }
 
 static int
-- 
1.8.1.2


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

* Re: [PATCH v4 1/4] staging: unisys: remove unneeded comparison
  2015-03-24 12:17 [PATCH v4 1/4] staging: unisys: remove unneeded comparison Sudip Mukherjee
                   ` (2 preceding siblings ...)
  2015-03-24 12:17 ` [PATCH v4 4/4] staging: unisys: remove unused variable Sudip Mukherjee
@ 2015-03-24 12:30 ` Dan Carpenter
  2015-03-24 12:36   ` Sudip Mukherjee
  3 siblings, 1 reply; 7+ messages in thread
From: Dan Carpenter @ 2015-03-24 12:30 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Benjamin Romer, David Kershner, Greg Kroah-Hartman, devel,
	sparmaintainer, linux-kernel

On Tue, Mar 24, 2015 at 05:47:34PM +0530, Sudip Mukherjee wrote:
> visorchipset_file_cleanup() is called from visorchipset_exit() which
> is the module_exit function and this function is executing means
> module_init succeeded, so registered will always be true at this time.
> and majordev has also been initialized in the init function. hence
> these comparisons will always be true.
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>

This is a partial patch.  The "one thing per patch" means don't do half
a thing per patch.

We now set "registered" but there are no users.

regards,
dan carpenter


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

* Re: [PATCH v4 1/4] staging: unisys: remove unneeded comparison
  2015-03-24 12:30 ` [PATCH v4 1/4] staging: unisys: remove unneeded comparison Dan Carpenter
@ 2015-03-24 12:36   ` Sudip Mukherjee
  0 siblings, 0 replies; 7+ messages in thread
From: Sudip Mukherjee @ 2015-03-24 12:36 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Benjamin Romer, David Kershner, Greg Kroah-Hartman, devel,
	sparmaintainer, linux-kernel

On Tue, Mar 24, 2015 at 03:30:21PM +0300, Dan Carpenter wrote:
> On Tue, Mar 24, 2015 at 05:47:34PM +0530, Sudip Mukherjee wrote:
> > visorchipset_file_cleanup() is called from visorchipset_exit() which
> > is the module_exit function and this function is executing means
> > module_init succeeded, so registered will always be true at this time.
> > and majordev has also been initialized in the init function. hence
> > these comparisons will always be true.
> > 
> > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> 
> This is a partial patch.  The "one thing per patch" means don't do half
> a thing per patch.
> 
> We now set "registered" but there are no users.
it is being removed separately in the 4/4 patch.
do you want that to be combined here in this patch?
but then won't that become 2 different changes in a single patch?
my message says we are removing that as the comparison is always
true.

regards
sudip
> 
> regards,
> dan carpenter
> 

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

* Re: [PATCH v4 3/4] staging: unisys: use local variable instead of global
  2015-03-24 12:17 ` [PATCH v4 3/4] staging: unisys: use local variable " Sudip Mukherjee
@ 2015-03-24 12:58   ` Dan Carpenter
  0 siblings, 0 replies; 7+ messages in thread
From: Dan Carpenter @ 2015-03-24 12:58 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Benjamin Romer, David Kershner, Greg Kroah-Hartman, devel,
	sparmaintainer, linux-kernel

I don't like how these are broken up at all.  It's random grab bag of
renaming and deleting.

Do it like this, perhaps:

[patch 1] delete registered

Leave the if (MAJOR(majordev) >= 0) for now, because it just
confuses issues to mix it in.  Delete all references to registered.

[patch 1] use local variable in visorchipset_file_init()
Don't delete anything.

[patch 2] pass major_dev to visorchipset_file_cleanup()

This deletes the "if (MAJOR(majordev) >= 0)" condition because it's
obvious that can't be true.  This deletes all references to the
"majordev" global because that isn't needed any more.  The whole reason
it was needed originally was because of visorchipset_file_cleanup() so
it's wrong to do the deleting in this patch which is about
visorchipset_file_init().

regards,
dan carpenter


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

end of thread, other threads:[~2015-03-24 12:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-24 12:17 [PATCH v4 1/4] staging: unisys: remove unneeded comparison Sudip Mukherjee
2015-03-24 12:17 ` [PATCH v4 2/4] staging: unisys: use local dev_t instead of global Sudip Mukherjee
2015-03-24 12:17 ` [PATCH v4 3/4] staging: unisys: use local variable " Sudip Mukherjee
2015-03-24 12:58   ` Dan Carpenter
2015-03-24 12:17 ` [PATCH v4 4/4] staging: unisys: remove unused variable Sudip Mukherjee
2015-03-24 12:30 ` [PATCH v4 1/4] staging: unisys: remove unneeded comparison Dan Carpenter
2015-03-24 12:36   ` Sudip Mukherjee

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.