[PATCH] staging: panel: remove duplicate code

[PATCH] staging: panel: remove duplicate code

[Sudip Mukherjee]

both the misc_deregister(), parport_release() and
parport_unregister_device() is there in the module_exit function also.
detach is called from parport_unregister_driver() and by the time
detach executes misc_deregister(), parport_release() and
parport_unregister_device() has already executed marking
keypad_initialized and lcd.initialized  as false. so this part of the
code will never execute.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 drivers/staging/panel/panel.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/staging/panel/panel.c b/drivers/staging/panel/panel.c
index ea54fb4..1d8ed8b 100644
--- a/drivers/staging/panel/panel.c
+++ b/drivers/staging/panel/panel.c
@@ -2252,20 +2252,6 @@ static void panel_detach(struct parport *port)
 	}
 
 	unregister_reboot_notifier(&panel_notifier);
-
-	if (keypad.enabled && keypad_initialized) {
-		misc_deregister(&keypad_dev);
-		keypad_initialized = 0;
-	}
-
-	if (lcd.enabled && lcd.initialized) {
-		misc_deregister(&lcd_dev);
-		lcd.initialized = false;
-	}
-
-	parport_release(pprt);
-	parport_unregister_device(pprt);
-	pprt = NULL;
 }
 
 static struct parport_driver panel_driver = {
-- 
1.8.1.2

Re: [PATCH] staging: panel: remove duplicate code

[Dan Carpenter]

On Tue, Apr 07, 2015 at 01:55:01PM +0530, Sudip Mukherjee wrote:
> both the misc_deregister(), parport_release() and
> parport_unregister_device() is there in the module_exit function also.
> detach is called from parport_unregister_driver() and by the time
> detach executes misc_deregister(), parport_release() and
> parport_unregister_device() has already executed marking
> keypad_initialized and lcd.initialized  as false. so this part of the
> code will never execute.
> 
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>

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

A better subject might have been "remove dead code" but that was
explained pretty well in the patch desription.

I hadn't looked at this driver much before.  It sucks that
parport_driver ->attach() functions can't fail...  I think we don't
need the "keypad_initialized" and "lcd.initialized" variables because
"if (pprt)" is enough to tell us whether or not the attach function
succeeded.

TODO: Staging: panel: remove some redundent variables.

regards,
dan carpenter

Re: [PATCH] staging: panel: remove duplicate code

[Sudip Mukherjee]

On Tue, Apr 07, 2015 at 11:49:30AM +0300, Dan Carpenter wrote:
> On Tue, Apr 07, 2015 at 01:55:01PM +0530, Sudip Mukherjee wrote:
> 
> I hadn't looked at this driver much before.  It sucks that
> parport_driver ->attach() functions can't fail... 

then maybe, we can change the code of parport. currently attach and
parport_register_driver never fails. we can modify it so that if attach
fails then parport_register_driver will also fail. will not be that much
difficult as it has been used only in 13 places.
your views ?

and since we are discussing parallel ports, few days back i saw one
post in ubuntuforums that his scanner is not working because of
lack of ppscsi.I mailed Tim Waugh, but he is not interested to work
with ppscsi anymore. parallel port scanners are almost a thing of the past
now. do you think it is worth that i pick up the code and modify
it for our latest kernel and submit to Greg ?

regards
sudip

 
> regards,
> dan carpenter
> 

Re: [PATCH] staging: panel: remove duplicate code

[Greg Kroah-Hartman]

On Tue, Apr 07, 2015 at 02:55:02PM +0530, Sudip Mukherjee wrote:
> On Tue, Apr 07, 2015 at 11:49:30AM +0300, Dan Carpenter wrote:
> > On Tue, Apr 07, 2015 at 01:55:01PM +0530, Sudip Mukherjee wrote:
> > 
> > I hadn't looked at this driver much before.  It sucks that
> > parport_driver ->attach() functions can't fail... 
> 
> then maybe, we can change the code of parport. currently attach and
> parport_register_driver never fails. we can modify it so that if attach
> fails then parport_register_driver will also fail. will not be that much
> difficult as it has been used only in 13 places.
> your views ?
> 
> and since we are discussing parallel ports, few days back i saw one
> post in ubuntuforums that his scanner is not working because of
> lack of ppscsi.I mailed Tim Waugh, but he is not interested to work
> with ppscsi anymore. parallel port scanners are almost a thing of the past
> now. do you think it is worth that i pick up the code and modify
> it for our latest kernel and submit to Greg ?

If you have some parport hardware, and want to take it on, that would be
great.  The code needs a maintainer, and the apis are _really_ old and
messy, as you have found out.

I'll be glad to shephard parport patches to Linus, as I've been doing
that semi-regularly for a few years now, through my char-misc git tree.

thanks,

greg k-h

Re: [PATCH] staging: panel: remove duplicate code

[Dan Carpenter]

On Tue, Apr 07, 2015 at 02:55:02PM +0530, Sudip Mukherjee wrote:
> On Tue, Apr 07, 2015 at 11:49:30AM +0300, Dan Carpenter wrote:
> > On Tue, Apr 07, 2015 at 01:55:01PM +0530, Sudip Mukherjee wrote:
> > 
> > I hadn't looked at this driver much before.  It sucks that
> > parport_driver ->attach() functions can't fail... 
> 
> then maybe, we can change the code of parport. currently attach and
> parport_register_driver never fails. we can modify it so that if attach
> fails then parport_register_driver will also fail. will not be that much
> difficult as it has been used only in 13 places.
> your views ?

If you write the patch then I will review it.  :)

> 
> and since we are discussing parallel ports, few days back i saw one
> post in ubuntuforums that his scanner is not working because of
> lack of ppscsi.I mailed Tim Waugh, but he is not interested to work
> with ppscsi anymore. parallel port scanners are almost a thing of the past
> now. do you think it is worth that i pick up the code and modify
> it for our latest kernel and submit to Greg ?

If you want to do the work, I won't say no! :P

regards,
dan carpenter

Re: [PATCH] staging: panel: remove duplicate code

[Sudip Mukherjee]

On Tue, Apr 07, 2015 at 11:44:29AM +0200, Greg Kroah-Hartman wrote:
> On Tue, Apr 07, 2015 at 02:55:02PM +0530, Sudip Mukherjee wrote:
> > On Tue, Apr 07, 2015 at 11:49:30AM +0300, Dan Carpenter wrote:
> > > On Tue, Apr 07, 2015 at 01:55:01PM +0530, Sudip Mukherjee wrote:
> > > 
> > > I hadn't looked at this driver much before.  It sucks that
> > > parport_driver ->attach() functions can't fail... 
> > 
> > then maybe, we can change the code of parport. currently attach and
> > parport_register_driver never fails. we can modify it so that if attach
> > fails then parport_register_driver will also fail. will not be that much
> > difficult as it has been used only in 13 places.
> > your views ?
> > 
> > and since we are discussing parallel ports, few days back i saw one
> > post in ubuntuforums that his scanner is not working because of
> > lack of ppscsi.I mailed Tim Waugh, but he is not interested to work
> > with ppscsi anymore. parallel port scanners are almost a thing of the past
> > now. do you think it is worth that i pick up the code and modify
> > it for our latest kernel and submit to Greg ?
> 
> If you have some parport hardware, and want to take it on, that would be
> great.  The code needs a maintainer, and the apis are _really_ old and
> messy, as you have found out.
what kind of parport hardware will you suggest?

regards
sudip

> 
> I'll be glad to shephard parport patches to Linus, as I've been doing
> that semi-regularly for a few years now, through my char-misc git tree.
> 
> thanks,
> 
> greg k-h

Re: [PATCH] staging: panel: remove duplicate code

[Greg Kroah-Hartman]

On Tue, Apr 07, 2015 at 03:26:58PM +0530, Sudip Mukherjee wrote:
> On Tue, Apr 07, 2015 at 11:44:29AM +0200, Greg Kroah-Hartman wrote:
> > On Tue, Apr 07, 2015 at 02:55:02PM +0530, Sudip Mukherjee wrote:
> > > On Tue, Apr 07, 2015 at 11:49:30AM +0300, Dan Carpenter wrote:
> > > > On Tue, Apr 07, 2015 at 01:55:01PM +0530, Sudip Mukherjee wrote:
> > > > 
> > > > I hadn't looked at this driver much before.  It sucks that
> > > > parport_driver ->attach() functions can't fail... 
> > > 
> > > then maybe, we can change the code of parport. currently attach and
> > > parport_register_driver never fails. we can modify it so that if attach
> > > fails then parport_register_driver will also fail. will not be that much
> > > difficult as it has been used only in 13 places.
> > > your views ?
> > > 
> > > and since we are discussing parallel ports, few days back i saw one
> > > post in ubuntuforums that his scanner is not working because of
> > > lack of ppscsi.I mailed Tim Waugh, but he is not interested to work
> > > with ppscsi anymore. parallel port scanners are almost a thing of the past
> > > now. do you think it is worth that i pick up the code and modify
> > > it for our latest kernel and submit to Greg ?
> > 
> > If you have some parport hardware, and want to take it on, that would be
> > great.  The code needs a maintainer, and the apis are _really_ old and
> > messy, as you have found out.
> what kind of parport hardware will you suggest?

No idea, go look at the drivers and code to figure that out.

Re: [PATCH] staging: panel: remove duplicate code

[Sudip Mukherjee]

On Tue, Apr 07, 2015 at 12:45:23PM +0300, Dan Carpenter wrote:
> On Tue, Apr 07, 2015 at 02:55:02PM +0530, Sudip Mukherjee wrote:
> 
> If you write the patch then I will review it.  :)

sure, I am on it.

> 
> > lack of ppscsi.I mailed Tim Waugh, but he is not interested to work
> > with ppscsi anymore. parallel port scanners are almost a thing of the past
> > now. do you think it is worth that i pick up the code and modify
> > it for our latest kernel and submit to Greg ?
> 
> If you want to do the work, I won't say no! :P
and, can you please suggest what kind of hardware should i try to
find to work with ppscsi. i saw parallel port scanners or zip drives
will be using that module. any other hardware apart from these?


regards
sudip

> 
> regards,
> dan carpenter
> 

Re: [PATCH] staging: panel: remove duplicate code

[Willy Tarreau]

On Tue, Apr 07, 2015 at 12:12:06PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Apr 07, 2015 at 03:26:58PM +0530, Sudip Mukherjee wrote:
> > On Tue, Apr 07, 2015 at 11:44:29AM +0200, Greg Kroah-Hartman wrote:
> > > On Tue, Apr 07, 2015 at 02:55:02PM +0530, Sudip Mukherjee wrote:
> > > > On Tue, Apr 07, 2015 at 11:49:30AM +0300, Dan Carpenter wrote:
> > > > > On Tue, Apr 07, 2015 at 01:55:01PM +0530, Sudip Mukherjee wrote:
> > > > > 
> > > > > I hadn't looked at this driver much before.  It sucks that
> > > > > parport_driver ->attach() functions can't fail... 
> > > > 
> > > > then maybe, we can change the code of parport. currently attach and
> > > > parport_register_driver never fails. we can modify it so that if attach
> > > > fails then parport_register_driver will also fail. will not be that much
> > > > difficult as it has been used only in 13 places.
> > > > your views ?
> > > > 
> > > > and since we are discussing parallel ports, few days back i saw one
> > > > post in ubuntuforums that his scanner is not working because of
> > > > lack of ppscsi.I mailed Tim Waugh, but he is not interested to work
> > > > with ppscsi anymore. parallel port scanners are almost a thing of the past
> > > > now. do you think it is worth that i pick up the code and modify
> > > > it for our latest kernel and submit to Greg ?
> > > 
> > > If you have some parport hardware, and want to take it on, that would be
> > > great.  The code needs a maintainer, and the apis are _really_ old and
> > > messy, as you have found out.
> > what kind of parport hardware will you suggest?
> 
> No idea, go look at the drivers and code to figure that out.

Not-so-old laptops used to have parport connectors for a while, and can be
found at a very cheap price or even for free by now with dead batteries.
I've been using mine as a JTAG connector exclusively until I replaced this
laptop.

A number of old mini-itx boards used to have a parallell port and were easy
to hack on (eg: those with a jack power connector, onboard video and a PXE
capable NIC to boot the freshly built kernel over the network). They're less
easy to find though.

If you want to run on your PC, you'll have to order a PCIe parallel port NIC
I guess.

Cheers,
Willy

Re: [PATCH] staging: panel: remove duplicate code

[Sudip Mukherjee]

On Tue, Apr 07, 2015 at 04:14:59PM +0200, Willy Tarreau wrote:
> > > > > now. do you think it is worth that i pick up the code and modify
> > > > > it for our latest kernel and submit to Greg ?
> > > > 
> > > > If you have some parport hardware, and want to take it on, that would be
> > > > great.  The code needs a maintainer, and the apis are _really_ old and
> > > > messy, as you have found out.
> > > what kind of parport hardware will you suggest?
> > 
> > No idea, go look at the drivers and code to figure that out.
> 
> Not-so-old laptops used to have parport connectors for a while, and can be
> found at a very cheap price or even for free by now with dead batteries.
free laptop sounds good.. :)
where can i get some?

> 
> A number of old mini-itx boards used to have a parallell port and were easy
> to hack on (eg: those with a jack power connector, onboard video and a PXE
> capable NIC to boot the freshly built kernel over the network). They're less
> easy to find though.
> 
> If you want to run on your PC, you'll have to order a PCIe parallel port NIC
> I guess.
i already have pc with parallel port, and on that i check your panel
codes before sending. but here we were discussing about the required
hardware to test ppscsi module.

regards
sudip
> 
> Cheers,
> Willy
> 

Re: [PATCH] staging: panel: remove duplicate code

[Willy Tarreau]

On Tue, Apr 07, 2015 at 07:51:17PM +0530, Sudip Mukherjee wrote:
> On Tue, Apr 07, 2015 at 04:14:59PM +0200, Willy Tarreau wrote:
> > Not-so-old laptops used to have parport connectors for a while, and can be
> > found at a very cheap price or even for free by now with dead batteries.
> free laptop sounds good.. :)
> where can i get some?

Friends replacing theirs in general, or your employer's dead stock :-)

> > If you want to run on your PC, you'll have to order a PCIe parallel port NIC

s/NIC/card above

> > I guess.
> i already have pc with parallel port, and on that i check your panel
> codes before sending. but here we were discussing about the required
> hardware to test ppscsi module.

Ah sorry for being out of topic then, I missed this point, I thought
it was for PP in general.

Regards,
Willy

Re: [PATCH] staging: panel: remove duplicate code

[Sudip Mukherjee]

On Tue, Apr 07, 2015 at 12:12:06PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Apr 07, 2015 at 03:26:58PM +0530, Sudip Mukherjee wrote:
> > On Tue, Apr 07, 2015 at 11:44:29AM +0200, Greg Kroah-Hartman wrote:
> > > On Tue, Apr 07, 2015 at 02:55:02PM +0530, Sudip Mukherjee wrote:
> > > > On Tue, Apr 07, 2015 at 11:49:30AM +0300, Dan Carpenter wrote:
> > > > > On Tue, Apr 07, 2015 at 01:55:01PM +0530, Sudip Mukherjee wrote:
> > > > > 
> > > > > I hadn't looked at this driver much before.  It sucks that
> > > > > parport_driver ->attach() functions can't fail... 
> > > > 
> > > > then maybe, we can change the code of parport. currently attach and
> > > > parport_register_driver never fails. we can modify it so that if attach
> > > > fails then parport_register_driver will also fail. will not be that much
> > > > difficult as it has been used only in 13 places.
> > > > your views ?
> > > > 
> > > > and since we are discussing parallel ports, few days back i saw one
> > > > post in ubuntuforums that his scanner is not working because of
> > > > lack of ppscsi.I mailed Tim Waugh, but he is not interested to work
> > > > with ppscsi anymore. parallel port scanners are almost a thing of the past
> > > > now. do you think it is worth that i pick up the code and modify
> > > > it for our latest kernel and submit to Greg ?
> > > 
> > > If you have some parport hardware, and want to take it on, that would be
> > > great.  The code needs a maintainer, and the apis are _really_ old and
> > > messy, as you have found out.
> > what kind of parport hardware will you suggest?
> 
> No idea, go look at the drivers and code to figure that out.
i guess i asked the question in a wrong way.
i already have pc with parport. what i wanted to ask was do i
need any other hardware (like parallel port scanner) to start with
ppscsi?

regards
sudip