2003-02-14 23:45:01

by Bob Miller

Subject: [PATCH 2.5.60 5/9] Update the Archimedes parallel port driver for new module API.

The patch below updates the Archimedes parallel port driver to use the new
module interfaces. This hasn't been test (sorry no hardware).


--
Bob Miller Email: [email protected]
Open Source Development Lab Phone: 503.626.2455 Ext. 17

diff -Nru a/drivers/parport/parport_arc.c b/drivers/parport/parport_arc.c
--- a/drivers/parport/parport_arc.c Fri Feb 14 09:50:44 2003
+++ b/drivers/parport/parport_arc.c Fri Feb 14 09:50:44 2003
@@ -65,18 +65,14 @@
return data_copy;
}

-static void arc_inc_use_count(void)
+static int arc_inc_use_count(void)
{
-#ifdef MODULE
- MOD_INC_USE_COUNT;
-#endif
+ return try_module_get(THIS_MODULE);
}

static void arc_dec_use_count(void)
{
-#ifdef MODULE
- MOD_DEC_USE_COUNT;
-#endif
+ module_put(THIS_MODULE);
}

static struct parport_operations parport_arc_ops =
@@ -123,18 +119,24 @@
{
/* Archimedes hardware provides only one port, at a fixed address */
struct parport *p;
+ struct resource res;
+ char *fake_name = "parport probe");

- if (check_region(PORT_BASE, 1))
+ res = request_region(PORT_BASE, 1, fake_name);
+ if (res == NULL)
return 0;

p = parport_register_port (PORT_BASE, IRQ_PRINTERACK,
PARPORT_DMA_NONE, &parport_arc_ops);

- if (!p)
+ if (!p) {
+ release_region(PORT_BASE, 1);
return 0;
+ }

p->modes = PARPORT_MODE_ARCSPP;
p->size = 1;
+ rename_region(res, p->name);

printk(KERN_INFO "%s: Archimedes on-board port, using irq %d\n",
p->irq);

2003-02-15 00:27:16

by Bob Miller

Subject: Re: [PATCH 2.5.60 5/9] Update the Archimedes parallel port driver for new module API.

On Sat, Feb 15, 2003 at 01:18:23AM +0100, Lars Magne Ingebrigtsen wrote:
> Bob Miller <[email protected]> writes:
>
> > --- a/drivers/parport/parport_arc.c Fri Feb 14 09:50:44 2003
>
> [...]
>
> > + char *fake_name = "parport probe");
>
> Surely that can't be correct? (The parenthesis at the end there, I
> mean...)
>
Sigh... Corrected diff below...

--
Bob Miller Email: [email protected]
Open Source Development Lab Phone: 503.626.2455 Ext. 17


diff -Nru a/drivers/parport/parport_arc.c b/drivers/parport/parport_arc.c
--- a/drivers/parport/parport_arc.c Fri Feb 14 09:50:44 2003
+++ b/drivers/parport/parport_arc.c Fri Feb 14 09:50:44 2003
@@ -65,18 +65,14 @@
return data_copy;
}

-static void arc_inc_use_count(void)
+static int arc_inc_use_count(void)
{
-#ifdef MODULE
- MOD_INC_USE_COUNT;
-#endif
+ return try_module_get(THIS_MODULE);
}

static void arc_dec_use_count(void)
{
-#ifdef MODULE
- MOD_DEC_USE_COUNT;
-#endif
+ module_put(THIS_MODULE);
}

static struct parport_operations parport_arc_ops =
@@ -123,18 +119,24 @@
{
/* Archimedes hardware provides only one port, at a fixed address */
struct parport *p;
+ struct resource res;
+ char *fake_name = "parport probe";

- if (check_region(PORT_BASE, 1))
+ res = request_region(PORT_BASE, 1, fake_name);
+ if (res == NULL)
return 0;

p = parport_register_port (PORT_BASE, IRQ_PRINTERACK,
PARPORT_DMA_NONE, &parport_arc_ops);

- if (!p)
+ if (!p) {
+ release_region(PORT_BASE, 1);
return 0;
+ }

p->modes = PARPORT_MODE_ARCSPP;
p->size = 1;
+ rename_region(res, p->name);

printk(KERN_INFO "%s: Archimedes on-board port, using irq %d\n",
p->irq);

2003-02-15 09:54:35

by Russell King

Subject: Re: [PATCH 2.5.60 5/9] Update the Archimedes parallel port driver for new module API.

On Fri, Feb 14, 2003 at 04:37:00PM -0800, Bob Miller wrote:
> -static void arc_inc_use_count(void)
> +static int arc_inc_use_count(void)
> {
> -#ifdef MODULE
> - MOD_INC_USE_COUNT;
> -#endif
> + return try_module_get(THIS_MODULE);
> }

Isn't one of the points of the module system that we don't try to run
code inside a module without the module being reference counted?

The normal way this is done is to add the module structure pointer into
a structure, and run try_module_get() from code external to the module
in question. The above method would seem to violate that.

Rusty - comments?

--
Russell King ([email protected]) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html

2003-02-17 02:53:07

by Rusty Russell

Subject: Re: [PATCH 2.5.60 5/9] Update the Archimedes parallel port driver for new module API.

In message <[email protected]> you write:
> On Fri, Feb 14, 2003 at 04:37:00PM -0800, Bob Miller wrote:
> > -static void arc_inc_use_count(void)
> > +static int arc_inc_use_count(void)
> > {
> > -#ifdef MODULE
> > - MOD_INC_USE_COUNT;
> > -#endif
> > + return try_module_get(THIS_MODULE);
> > }
>
> Isn't one of the points of the module system that we don't try to run
> code inside a module without the module being reference counted?

Exactly. You can do it if you *know* you already hold a reference
count, but as a general rule it's damn suspicious.

This looks wrong,
Rusty.
--
Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

2003-02-17 16:55:54

by Bob Miller

Subject: Re: [PATCH 2.5.60 5/9] Update the Archimedes parallel port driver for new module API.

On Mon, Feb 17, 2003 at 01:54:59PM +1100, Rusty Russell wrote:
> In message <[email protected]> you write:
> > On Fri, Feb 14, 2003 at 04:37:00PM -0800, Bob Miller wrote:
> > > -static void arc_inc_use_count(void)
> > > +static int arc_inc_use_count(void)
> > > {
> > > -#ifdef MODULE
> > > - MOD_INC_USE_COUNT;
> > > -#endif
> > > + return try_module_get(THIS_MODULE);
> > > }
> >
> > Isn't one of the points of the module system that we don't try to run
> > code inside a module without the module being reference counted?
>
> Exactly. You can do it if you *know* you already hold a reference
> count, but as a general rule it's damn suspicious.
>
> This looks wrong,
> Rusty.

Thanks for the feed back. I'll fix this and re-submit.

--
Bob Miller Email: [email protected]
Open Source Development Lab Phone: 503.626.2455 Ext. 17