Here is a patch to parport_pc.c that adds udev support. Without it sysfs
does not have enough information to give to udev so the lp and parport nodes
can be created. The only problem I have is the kernel_oops when I do the
following: insmod parport, insmod parport_pc, rmmod parport_pc, rmmod
parport, insmod parport, insmod parport_pc, rmmod parport_pc, kernel oops.
>From the part of the traceback I can read the problem is in parport_pc_exit.
Any help in tracking down the problem would be appreciated. If this is
acceptable how can I get this added to the mainline? This is my first patch
so please be gentle.
Thanks,
Jason Dravet
Signed-off-by: Jason Dravet <[email protected]>
--- /usr/src/linux-2.6.14/drivers/parport/parport_pc.c.orig 2005-12-30
13:52:48.000000000 -0600
+++ /usr/src/linux-2.6.14/drivers/parport/parport_pc.c 2005-12-30
19:56:20.000000000 -0600
@@ -55,6 +55,7 @@
#include <linux/pci.h>
#include <linux/pnp.h>
#include <linux/sysctl.h>
+#include <linux/sysfs.h>
#include <asm/io.h>
#include <asm/dma.h>
@@ -99,6 +100,8 @@ static struct superio_struct { /* For Su
int dma;
} superios[NR_SUPERIOS] __devinitdata = { {0,},};
+static struct class *parallel_class;
+
static int user_specified;
#if defined(CONFIG_PARPORT_PC_SUPERIO) || \
(defined(CONFIG_PARPORT_1284) && defined(CONFIG_PARPORT_PC_FIFO))
@@ -2232,6 +2235,25 @@ struct parport *parport_pc_probe_port (u
is mandatory (see above) */
p->dma = PARPORT_DMA_NONE;
+ parallel_class = class_create(THIS_MODULE, "lp");
+ if (p->base == 888) /* 888 is dec for 378h */
+ {
+ class_device_create(parallel_class, NULL, MKDEV(6, 0), NULL, "lp0");
+ class_device_create(parallel_class, NULL, MKDEV(99, 0), NULL,
"parport0");
+ }
+
+ if (p->base == 632) /* 632 is dec for 278h */
+ {
+ class_device_create(parallel_class, NULL, MKDEV(6, 1), NULL, "lp1");
+ class_device_create(parallel_class, NULL, MKDEV(99, 1), NULL,
"parport1");
+ }
+
+ if (p->base == 956) /* 956 is dec for 3BCh */
+ {
+ class_device_create(parallel_class, NULL, MKDEV(6, 2), NULL, "lp2");
+ class_device_create(parallel_class, NULL, MKDEV(99, 2), NULL,
"parport2");
+ }
+
#ifdef CONFIG_PARPORT_PC_FIFO
if (parport_ECP_supported(p) &&
p->dma != PARPORT_DMA_NOFIFO &&
@@ -3406,6 +3428,13 @@ static void __exit parport_pc_exit(void)
if (pnp_registered_parport)
pnp_unregister_driver (&parport_pc_pnp_driver);
+ class_device_destroy(parallel_class, MKDEV(99, 2));
+ class_device_destroy(parallel_class, MKDEV(6, 2));
+ class_device_destroy(parallel_class, MKDEV(99, 1));
+ class_device_destroy(parallel_class, MKDEV(6, 1));
+ class_device_destroy(parallel_class, MKDEV(99, 0));
+ class_device_destroy(parallel_class, MKDEV(6, 0));
+
spin_lock(&ports_lock);
while (!list_empty(&ports_list)) {
struct parport_pc_private *priv;
> + parallel_class = class_create(THIS_MODULE, "lp");
> + if (p->base == 888) /* 888 is dec for 378h */
I would prefer to actually see == 0x378 in the code, because the
hexademical number is what you see everywhere else, such as the BIOS POST
and /proc/ioports. This also applies to 0x278 and 0x3BC below.
> + {
> + class_device_create(parallel_class, NULL, MKDEV(6, 0), NULL,
> "lp0");
> + class_device_create(parallel_class, NULL, MKDEV(99, 0), NULL,
> "parport0");
> + }
Background info before: Because I burnt my on-board LPT port (applying too
much volts or milliamps), I bought a dual-slot PCI add-in card. This card
provides "parport1" and "parport2" at ports at 0xC800 and 0xC00
(/proc/ioports).
There are a number of problems in your code:
1- testing just for 0x378/0x278/0x3BC is not enough
2- parport0 could be 0xC800 (address may vary) if you do not
have any onboard LPT ports.
2=> that is why I think you should not reserver "lp0"/"parport0"
for 0x378.
All this applies to the other chunks too of course...:
> + if (p->base == 632) /* 632 is dec for 278h */
> + {
> + class_device_create(parallel_class, NULL, MKDEV(6, 1), NULL,
> "lp1");
> + class_device_create(parallel_class, NULL, MKDEV(99, 1), NULL,
> "parport1");
> + }
> +
> + if (p->base == 956) /* 956 is dec for 3BCh */
> + {
> + class_device_create(parallel_class, NULL, MKDEV(6, 2), NULL,
> "lp2");
> + class_device_create(parallel_class, NULL, MKDEV(99, 2), NULL,
> "parport2");
> + }
> +
> + class_device_destroy(parallel_class, MKDEV(99, 2));
> + class_device_destroy(parallel_class, MKDEV(6, 2));
> + class_device_destroy(parallel_class, MKDEV(99, 1));
> + class_device_destroy(parallel_class, MKDEV(6, 1));
> + class_device_destroy(parallel_class, MKDEV(99, 0));
> + class_device_destroy(parallel_class, MKDEV(6, 0));
> +
Jan Engelhardt
--
| Alphagate Systems, http://alphagate.hopto.org/
| jengelh's site, http://jengelh.hopto.org/
Jason Dravet napsal(a):
> Here is a patch to parport_pc.c that adds udev support. Without it
> sysfs does not have enough information to give to udev so the lp and
> parport nodes can be created. The only problem I have is the
> kernel_oops when I do the following: insmod parport, insmod parport_pc,
> rmmod parport_pc, rmmod parport, insmod parport, insmod parport_pc,
> rmmod parport_pc, kernel oops.
[snip]
> + if (p->base == 888) /* 888 is dec for 378h */
> + {
> + class_device_create(parallel_class, NULL, MKDEV(6, 0), NULL,
> "lp0");
> + class_device_create(parallel_class, NULL, MKDEV(99, 0), NULL,
> "parport0");
> + }
use please
if () {
}
instead of
if ()
{
}
like the surrounding code
thanks,
--
Jiri Slaby http://www.fi.muni.cz/~xslaby
\_.-^-._ [email protected] _.-^-._/
B67499670407CE62ACC8 22A032CC55C339D47A7E
Thank you for the reply. Comments inline.
>I would prefer to actually see == 0x378 in the code, because the
>hexademical number is what you see everywhere else, such as the BIOS POST
>and /proc/ioports. This also applies to 0x278 and 0x3BC below.
>
This is what I wanted but I could not figure out how do it. If you tell me
how I will be happy to change it. I tried if (p->base == 0x378) but then
class_device_create does not get executed.
> > + {
> > + class_device_create(parallel_class, NULL, MKDEV(6, 0), NULL,
> > "lp0");
> > + class_device_create(parallel_class, NULL, MKDEV(99, 0), NULL,
> > "parport0");
> > + }
>
>Background info before: Because I burnt my on-board LPT port (applying too
>much volts or milliamps), I bought a dual-slot PCI add-in card. This card
>provides "parport1" and "parport2" at ports at 0xC800 and 0xC00
>(/proc/ioports).
>
The last experience I have with off board cards was about 5 years ago. The
choices for the two parallel ports were 378, 278, or 3BC. I was not aware
that you had flexibility now.
>
>There are a number of problems in your code:
>
>1- testing just for 0x378/0x278/0x3BC is not enough
>
>2- parport0 could be 0xC800 (address may vary) if you do not
> have any onboard LPT ports.
> 2=> that is why I think you should not reserver "lp0"/"parport0"
> for 0x378.
As I said above I was not aware todays off board parallel ports had more
choices. I will see what I can do to fix this. Do you have any
suggestions?
Thanks,
Jason
>From: Jiri Slaby <[email protected]>
>To: Jason Dravet <[email protected]>
>CC: [email protected], [email protected]
>Subject: Re: RFC: add udev support to parport_pc
>Date: Sat, 31 Dec 2005 18:59:15 +0100
>
>Jason Dravet napsal(a):
> > Here is a patch to parport_pc.c that adds udev support. Without it
> > sysfs does not have enough information to give to udev so the lp and
> > parport nodes can be created. The only problem I have is the
> > kernel_oops when I do the following: insmod parport, insmod parport_pc,
> > rmmod parport_pc, rmmod parport, insmod parport, insmod parport_pc,
> > rmmod parport_pc, kernel oops.
>[snip]
> > + if (p->base == 888) /* 888 is dec for 378h */
> > + {
> > + class_device_create(parallel_class, NULL, MKDEV(6, 0), NULL,
> > "lp0");
> > + class_device_create(parallel_class, NULL, MKDEV(99, 0), NULL,
> > "parport0");
> > + }
>use please
>if () {
>}
>instead of
>if ()
>{
>}
>like the surrounding code
>
>thanks,
>--
>Jiri Slaby http://www.fi.muni.cz/~xslaby
Will do. Thanks,
Jason
Here is version 2 of the patch. It should address the issues pointed out so
far.
Thanks,
Jason
--- /usr/src/linux-2.6.14/drivers/parport/parport_pc.c.orig 2005-12-30
13:52:48.000000000 -0600
+++ /usr/src/linux-2.6.14/drivers/parport/parport_pc.c 2005-12-31
13:17:42.000000000 -0600
@@ -14,6 +14,7 @@
* More PCI support now conditional on CONFIG_PCI, 03/2001, Paul G.
* Various hacks, Fred Barnes, 04/2001
* Updated probing logic - Adam Belay <[email protected]>
+ * Added sysfs and udev - Jason Dravet <[email protected]>
*/
/* This driver should work with any hardware that is broadly compatible
@@ -55,6 +56,7 @@
#include <linux/pci.h>
#include <linux/pnp.h>
#include <linux/sysctl.h>
+#include <linux/sysfs.h>
#include <asm/io.h>
#include <asm/dma.h>
@@ -99,6 +101,9 @@ static struct superio_struct { /* For Su
int dma;
} superios[NR_SUPERIOS] __devinitdata = { {0,},};
+static struct class *parallel_class;
+int countports = 0;
+
static int user_specified;
#if defined(CONFIG_PARPORT_PC_SUPERIO) || \
(defined(CONFIG_PARPORT_1284) && defined(CONFIG_PARPORT_PC_FIFO))
@@ -2232,6 +2237,11 @@ struct parport *parport_pc_probe_port (u
is mandatory (see above) */
p->dma = PARPORT_DMA_NONE;
+ parallel_class = class_create(THIS_MODULE, "lp");
+ class_device_create(parallel_class, NULL, MKDEV(6, countports), NULL,
"lp%i", countports);
+ class_device_create(parallel_class, NULL, MKDEV(99, countports), NULL,
"parport%i", countports);
+ countports++;
+
#ifdef CONFIG_PARPORT_PC_FIFO
if (parport_ECP_supported(p) &&
p->dma != PARPORT_DMA_NOFIFO &&
@@ -3406,6 +3416,13 @@ static void __exit parport_pc_exit(void)
if (pnp_registered_parport)
pnp_unregister_driver (&parport_pc_pnp_driver);
+ printk (KERN_WARNING "countports = %i \n", countports);
+ for (countports--; countports >=0; countports--) {
+ printk (KERN_WARNING "countports loop = %i \n", countports);
+ class_device_destroy(parallel_class, MKDEV(99, countports));
+ class_device_destroy(parallel_class, MKDEV(6, countports));
+ }
+
spin_lock(&ports_lock);
while (!list_empty(&ports_list)) {
struct parport_pc_private *priv;
>> I would prefer to actually see == 0x378 in the code, because the
>> hexademical number is what you see everywhere else, such as the BIOS POST
>> and /proc/ioports. This also applies to 0x278 and 0x3BC below.
>>
> This is what I wanted but I could not figure out how do it. If you tell me how
> I will be happy to change it. I tried if (p->base == 0x378) but then
> class_device_create does not get executed.
>
Sounds very strange to me that by changing a base-10 integer into base-16
does not execute the branch anymore...
>> Background info before: Because I burnt my on-board LPT port (applying too
>> much volts or milliamps), I bought a dual-slot PCI add-in card. This card
>> provides "parport1" and "parport2" at ports at 0xC800 and 0xC00
>> (/proc/ioports).
>>
> The last experience I have with off board cards was about 5 years ago. The
> choices for the two parallel ports were 378, 278, or 3BC. I was not aware that
> you had flexibility now.
>
Well, I can tell you even more: I also have an ISA LPT expansion card in an
ol 386 that gets assigned 278 and 3BC respectively.
> As I said above I was not aware todays off board parallel ports had more
> choices. I will see what I can do to fix this. Do you have any suggestions?
I have not yet checked the BIOS POST message of the i686 where
the PCI LPT card is in... maybe it's just Linux that remaps
it to high ports. Gotta test it in Windows. BBL.
Anyway, as for the patch I think you should walk down the list of
already-collected ioports (in userspace that would equal to `grep parport
/proc/ioports`) and assign each of them consecutive device names.
Jan Engelhardt
--