Re: [PATCH] lp: implement proper detach function for parport_driver lp

2013-06-20 Thread Hannes Weisbach

Am 19.06.2013 um 20:32 schrieb Greg Kroah-Hartman:
> 
>> They are doing what I am doing: translating whatever the
>> user does on a /dev/parportN-node and sending device-specific commands
>> over USB.  When they do parport_announce_port(), lp.c should also be
>> initialized and they should have the same problem.
> 
> Why hasn't anyone reported that problem then?  Surely someone must use
> these, they've been in the kernel tree for over a decade now.
I don't know.  Maybe nobody uses them (anymore/ever)?  Nobody tried to
rmmod the kernel module or compiled the driver into the kernel?

The solution to the problem (if someone encountered it) was probably
policy-based: just don't unload the module.  The comment in lp.c is
clear: /* Write this some day.  */ ;)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] lp: implement proper detach function for parport_driver lp

2013-06-20 Thread Hannes Weisbach

Am 19.06.2013 um 20:32 schrieb Greg Kroah-Hartman:
 
 They are doing what I am doing: translating whatever the
 user does on a /dev/parportN-node and sending device-specific commands
 over USB.  When they do parport_announce_port(), lp.c should also be
 initialized and they should have the same problem.
 
 Why hasn't anyone reported that problem then?  Surely someone must use
 these, they've been in the kernel tree for over a decade now.
I don't know.  Maybe nobody uses them (anymore/ever)?  Nobody tried to
rmmod the kernel module or compiled the driver into the kernel?

The solution to the problem (if someone encountered it) was probably
policy-based: just don't unload the module.  The comment in lp.c is
clear: /* Write this some day.  */ ;)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] lp: implement proper detach function for parport_driver lp

2013-06-19 Thread Greg Kroah-Hartman
On Wed, Jun 19, 2013 at 07:04:51PM +0200, Hannes Weisbach wrote:
> >> Signed-off-by: Hannes Weisbach 
> >> ---
> >> Granted, for normal parport drivers this is usually not an issue,
> >> because the device does not go away. However, I am currently writing a
> >> Linux device driver for a USB to parallel port converter [0] and
> >> therefore need proper detaching. Additionally, the wrong ref count
> >> keeps me from simply rmmod my driver and insmod a new version while
> >> developing and testing.
> > 
> > Really?  We already have a usb to parallel port driver in the kernel
> > tree that seems to work just fine.
> 
> >  It's been there since the 2.3 kernel
> > days, so either it has the same problem, or your driver is doing
> > something odd.
> 
> Do you mean the USB Printer class driver for pseudo parallel port
> adapters?  They don't use the char/lp.c printer driver.  (Or I didn't
> see it).  I'm writing a driver for USB2LPT [0], which gives you a real
> /dev/parportN-device in user space, with which you can do all the
> bit-twiddling like with a real parallel port.
> 
> Or do you mean USS720 (misc/uss720.c) and MOS7715 (serial/mos7720.c)
> drivers.

Yes, I mean these.

> They are doing what I am doing: translating whatever the
> user does on a /dev/parportN-node and sending device-specific commands
> over USB.  When they do parport_announce_port(), lp.c should also be
> initialized and they should have the same problem.

Why hasn't anyone reported that problem then?  Surely someone must use
these, they've been in the kernel tree for over a decade now.

> On second thought, my patch might not be optimal.  lp.c stores
> instance-structs in an array of size 8.  So after 8 re-plugs, lp.c
> will not instantiate any more printer devices.  I think I better go
> all the way and replace that array with a list, to have a proper
> solution.

Proper solutions are always good :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] lp: implement proper detach function for parport_driver lp

2013-06-19 Thread Hannes Weisbach
>> Signed-off-by: Hannes Weisbach 
>> ---
>> Granted, for normal parport drivers this is usually not an issue,
>> because the device does not go away. However, I am currently writing a
>> Linux device driver for a USB to parallel port converter [0] and
>> therefore need proper detaching. Additionally, the wrong ref count
>> keeps me from simply rmmod my driver and insmod a new version while
>> developing and testing.
> 
> Really?  We already have a usb to parallel port driver in the kernel
> tree that seems to work just fine.

>  It's been there since the 2.3 kernel
> days, so either it has the same problem, or your driver is doing
> something odd.

Do you mean the USB Printer class driver for pseudo parallel port
adapters?  They don't use the char/lp.c printer driver.  (Or I didn't
see it).  I'm writing a driver for USB2LPT [0], which gives you a real
/dev/parportN-device in user space, with which you can do all the
bit-twiddling like with a real parallel port.

Or do you mean USS720 (misc/uss720.c) and MOS7715 (serial/mos7720.c)
drivers.  They are doing what I am doing: translating whatever the
user does on a /dev/parportN-node and sending device-specific commands
over USB.  When they do parport_announce_port(), lp.c should also be
initialized and they should have the same problem.

On second thought, my patch might not be optimal.  lp.c stores
instance-structs in an array of size 8.  So after 8 re-plugs, lp.c
will not instantiate any more printer devices.  I think I better go
all the way and replace that array with a list, to have a proper
solution.

[0] 
http://www-user.tu-chemnitz.de/~heha/bastelecke/Rund%20um%20den%20PC/USB2LPT/index.html.en--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] lp: implement proper detach function for parport_driver lp

2013-06-19 Thread Hannes Weisbach
 Signed-off-by: Hannes Weisbach hannes_weisb...@gmx.net
 ---
 Granted, for normal parport drivers this is usually not an issue,
 because the device does not go away. However, I am currently writing a
 Linux device driver for a USB to parallel port converter [0] and
 therefore need proper detaching. Additionally, the wrong ref count
 keeps me from simply rmmod my driver and insmod a new version while
 developing and testing.
 
 Really?  We already have a usb to parallel port driver in the kernel
 tree that seems to work just fine.

  It's been there since the 2.3 kernel
 days, so either it has the same problem, or your driver is doing
 something odd.

Do you mean the USB Printer class driver for pseudo parallel port
adapters?  They don't use the char/lp.c printer driver.  (Or I didn't
see it).  I'm writing a driver for USB2LPT [0], which gives you a real
/dev/parportN-device in user space, with which you can do all the
bit-twiddling like with a real parallel port.

Or do you mean USS720 (misc/uss720.c) and MOS7715 (serial/mos7720.c)
drivers.  They are doing what I am doing: translating whatever the
user does on a /dev/parportN-node and sending device-specific commands
over USB.  When they do parport_announce_port(), lp.c should also be
initialized and they should have the same problem.

On second thought, my patch might not be optimal.  lp.c stores
instance-structs in an array of size 8.  So after 8 re-plugs, lp.c
will not instantiate any more printer devices.  I think I better go
all the way and replace that array with a list, to have a proper
solution.

[0] 
http://www-user.tu-chemnitz.de/~heha/bastelecke/Rund%20um%20den%20PC/USB2LPT/index.html.en--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] lp: implement proper detach function for parport_driver lp

2013-06-19 Thread Greg Kroah-Hartman
On Wed, Jun 19, 2013 at 07:04:51PM +0200, Hannes Weisbach wrote:
  Signed-off-by: Hannes Weisbach hannes_weisb...@gmx.net
  ---
  Granted, for normal parport drivers this is usually not an issue,
  because the device does not go away. However, I am currently writing a
  Linux device driver for a USB to parallel port converter [0] and
  therefore need proper detaching. Additionally, the wrong ref count
  keeps me from simply rmmod my driver and insmod a new version while
  developing and testing.
  
  Really?  We already have a usb to parallel port driver in the kernel
  tree that seems to work just fine.
 
   It's been there since the 2.3 kernel
  days, so either it has the same problem, or your driver is doing
  something odd.
 
 Do you mean the USB Printer class driver for pseudo parallel port
 adapters?  They don't use the char/lp.c printer driver.  (Or I didn't
 see it).  I'm writing a driver for USB2LPT [0], which gives you a real
 /dev/parportN-device in user space, with which you can do all the
 bit-twiddling like with a real parallel port.
 
 Or do you mean USS720 (misc/uss720.c) and MOS7715 (serial/mos7720.c)
 drivers.

Yes, I mean these.

 They are doing what I am doing: translating whatever the
 user does on a /dev/parportN-node and sending device-specific commands
 over USB.  When they do parport_announce_port(), lp.c should also be
 initialized and they should have the same problem.

Why hasn't anyone reported that problem then?  Surely someone must use
these, they've been in the kernel tree for over a decade now.

 On second thought, my patch might not be optimal.  lp.c stores
 instance-structs in an array of size 8.  So after 8 re-plugs, lp.c
 will not instantiate any more printer devices.  I think I better go
 all the way and replace that array with a list, to have a proper
 solution.

Proper solutions are always good :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] lp: implement proper detach function for parport_driver lp

2013-06-03 Thread Greg Kroah-Hartman
On Sat, Jun 01, 2013 at 10:02:21PM +0200, Hannes Weisbach wrote:
> From: Hannes Weisbach 
> 
> The lp pardevice driver does not have a proper detach function. Consequently, 
> parport_unregister_device() is not called when the underlying parport driver 
> calls parport_remove_port(). As a result, the ref count of the parport 
> driver's module does not go to zero and the driver module cannot be unloaded.
> The attached patch unregisters all lp pardevices which are on the 
> to-be-detached parport.

Please wrap your changelog comments at 72 columns or so.

> Signed-off-by: Hannes Weisbach 
> ---
> Granted, for normal parport drivers this is usually not an issue,
> because the device does not go away. However, I am currently writing a
> Linux device driver for a USB to parallel port converter [0] and
> therefore need proper detaching. Additionally, the wrong ref count
> keeps me from simply rmmod my driver and insmod a new version while
> developing and testing.

Really?  We already have a usb to parallel port driver in the kernel
tree that seems to work just fine.  It's been there since the 2.3 kernel
days, so either it has the same problem, or your driver is doing
something odd.

Can you test the in-kernel driver and let me know if it has the same
problem?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] lp: implement proper detach function for parport_driver lp

2013-06-03 Thread Greg Kroah-Hartman
On Sat, Jun 01, 2013 at 10:02:21PM +0200, Hannes Weisbach wrote:
 From: Hannes Weisbach hannes_weisb...@gmx.net
 
 The lp pardevice driver does not have a proper detach function. Consequently, 
 parport_unregister_device() is not called when the underlying parport driver 
 calls parport_remove_port(). As a result, the ref count of the parport 
 driver's module does not go to zero and the driver module cannot be unloaded.
 The attached patch unregisters all lp pardevices which are on the 
 to-be-detached parport.

Please wrap your changelog comments at 72 columns or so.

 Signed-off-by: Hannes Weisbach hannes_weisb...@gmx.net
 ---
 Granted, for normal parport drivers this is usually not an issue,
 because the device does not go away. However, I am currently writing a
 Linux device driver for a USB to parallel port converter [0] and
 therefore need proper detaching. Additionally, the wrong ref count
 keeps me from simply rmmod my driver and insmod a new version while
 developing and testing.

Really?  We already have a usb to parallel port driver in the kernel
tree that seems to work just fine.  It's been there since the 2.3 kernel
days, so either it has the same problem, or your driver is doing
something odd.

Can you test the in-kernel driver and let me know if it has the same
problem?

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] lp: implement proper detach function for parport_driver lp

2013-06-01 Thread Hannes Weisbach
From: Hannes Weisbach 

The lp pardevice driver does not have a proper detach function. Consequently, 
parport_unregister_device() is not called when the underlying parport driver 
calls parport_remove_port(). As a result, the ref count of the parport driver's 
module does not go to zero and the driver module cannot be unloaded.
The attached patch unregisters all lp pardevices which are on the 
to-be-detached parport.

Signed-off-by: Hannes Weisbach 
---
Granted, for normal parport drivers this is usually not an issue, because the 
device does not go away. However, I am currently writing a Linux device driver 
for a USB to parallel port converter [0] and therefore need proper detaching. 
Additionally, the wrong ref count keeps me from simply rmmod my driver and 
insmod a new version while developing and testing.

 drivers/char/lp.c |   12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/char/lp.c b/drivers/char/lp.c
index 0913d79..57e6941 100644
--- a/drivers/char/lp.c
+++ b/drivers/char/lp.c
@@ -930,7 +930,17 @@ static void lp_attach (struct parport *port)
 
 static void lp_detach (struct parport *port)
 {
-   /* Write this some day. */
+   int offset;
+
+   for (offset = 0; offset < LP_NO; offset++) {
+   if (lp_table[offset].dev == NULL)
+   continue;
+   if (lp_table[offset].dev->port == port) {
+   device_destroy(lp_class, MKDEV(LP_MAJOR, offset));
+   parport_unregister_device(lp_table[offset].dev);
+   }
+   }
+
 #ifdef CONFIG_LP_CONSOLE
if (console_registered == port) {
unregister_console();

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] lp: implement proper detach function for parport_driver lp

2013-06-01 Thread Hannes Weisbach
From: Hannes Weisbach hannes_weisb...@gmx.net

The lp pardevice driver does not have a proper detach function. Consequently, 
parport_unregister_device() is not called when the underlying parport driver 
calls parport_remove_port(). As a result, the ref count of the parport driver's 
module does not go to zero and the driver module cannot be unloaded.
The attached patch unregisters all lp pardevices which are on the 
to-be-detached parport.

Signed-off-by: Hannes Weisbach hannes_weisb...@gmx.net
---
Granted, for normal parport drivers this is usually not an issue, because the 
device does not go away. However, I am currently writing a Linux device driver 
for a USB to parallel port converter [0] and therefore need proper detaching. 
Additionally, the wrong ref count keeps me from simply rmmod my driver and 
insmod a new version while developing and testing.

 drivers/char/lp.c |   12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/char/lp.c b/drivers/char/lp.c
index 0913d79..57e6941 100644
--- a/drivers/char/lp.c
+++ b/drivers/char/lp.c
@@ -930,7 +930,17 @@ static void lp_attach (struct parport *port)
 
 static void lp_detach (struct parport *port)
 {
-   /* Write this some day. */
+   int offset;
+
+   for (offset = 0; offset  LP_NO; offset++) {
+   if (lp_table[offset].dev == NULL)
+   continue;
+   if (lp_table[offset].dev-port == port) {
+   device_destroy(lp_class, MKDEV(LP_MAJOR, offset));
+   parport_unregister_device(lp_table[offset].dev);
+   }
+   }
+
 #ifdef CONFIG_LP_CONSOLE
if (console_registered == port) {
unregister_console(lpcons);

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/