[patch against 2.6.1-rc1-mm1] replace check_region with request_region in isp16.c

[patch against 2.6.1-rc1-mm1] replace check_region with request_region in isp16.c

[Jesper Juhl]

Hi,

When building linux 2.6.1-rc1-mm1 with "allmodconfig" I saw this warning :

drivers/cdrom/isp16.c
drivers/cdrom/isp16.c: In function `isp16_init':
drivers/cdrom/isp16.c:124: warning: `check_region' is deprecated (declared at include/linux/ioport.h:119)

So I desided to read up up check_region. The material I found indicates to
me (if I interpreted it correctly) that in previous kernels you had to
use check_region/request_region pairs to ensure that the region you wanted
to access was available before requesting it. With newer kernels there is
a chance that the driver could get interrupted in-between the calls to
check_region and request_region, so it was desided to get rid of
check_region and instead have request_region return an error if the region
requested is unavailable.
>From what I've read, it seems that getting rid of check_region should be
as simple as just replacing it with request_region and check the return
value, so that's what the patch below does.

One thing that surprised me was that the isp16 driver does not seem to
ever call request_region, it only ever calls check_region which confuses
me a bit - wouldn't it need to (also with older kernels) always call
request_region ?

In any case, I don't have the hardware to test this driver, so I can't
verify if it still works after patching it, all I can say is that it
compiles without warnings or errors with the patch.

I would appreciate it is someone could take a quick look at the patch and
verify that it does the correct thing.
Patch is against 2.6.1-rc1-mm1


--- linux-2.6.1-rc1-mm1-orig/drivers/cdrom/isp16.c      2003-12-31 05:48:26.000000000 +0100
+++ linux-2.6.1-rc1-mm1/drivers/cdrom/isp16.c   2004-01-03 17:55:14.000000000 +0100
@@ -121,7 +121,7 @@ int __init isp16_init(void)
                return (0);
        }

-       if (check_region(ISP16_IO_BASE, ISP16_IO_SIZE)) {
+       if (!request_region(ISP16_IO_BASE, ISP16_IO_SIZE, "isp16 cdrom")) {
                printk("ISP16: i/o ports already in use.\n");
                return (-EIO);
        }



Kind regards,

Jesper Juhl

Re: [patch against 2.6.1-rc1-mm1] replace check_region with request_region in isp16.c

[Muli Ben-Yehuda]

[-- Attachment #1: Type: text/plain, Size: 751 bytes --]

On Sat, Jan 03, 2004 at 06:06:20PM +0100, Jesper Juhl wrote:

> One thing that surprised me was that the isp16 driver does not seem to
> ever call request_region, it only ever calls check_region which confuses
> me a bit - wouldn't it need to (also with older kernels) always call
> request_region ?

I think so. It looks broken as it was, playing with the IO region
without requesting it first. Is anyone actually using this driver? 

> I would appreciate it is someone could take a quick look at the patch and
> verify that it does the correct thing.

Looks correct to me. 

Chers, 
Muli 
-- 
Muli Ben-Yehuda
http://www.mulix.org | http://mulix.livejournal.com/

"the nucleus of linux oscillates my world" - gccbot@#offtopic


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [patch against 2.6.1-rc1-mm1] replace check_region with request_region in isp16.c

[Petri Koistinen]

Hi!

On Sat, 3 Jan 2004, Jesper Juhl wrote:

> drivers/cdrom/isp16.c
> drivers/cdrom/isp16.c: In function `isp16_init':
> drivers/cdrom/isp16.c:124: warning: `check_region' is deprecated (declared at include/linux/ioport.h:119)

> I would appreciate it is someone could take a quick look at the patch

There was recent discussion about this, see:

http://tinyurl.com/38hum and http://tinyurl.com/2wexw

Petri

Re: [patch against 2.6.1-rc1-mm1] replace check_region with request_region in isp16.c

[Jesper Juhl]

On Sat, 3 Jan 2004, Petri Koistinen wrote:

> Hi!
>
> On Sat, 3 Jan 2004, Jesper Juhl wrote:
>
> > drivers/cdrom/isp16.c
> > drivers/cdrom/isp16.c: In function `isp16_init':
> > drivers/cdrom/isp16.c:124: warning: `check_region' is deprecated (declared at include/linux/ioport.h:119)
>
> > I would appreciate it is someone could take a quick look at the patch
>
> There was recent discussion about this, see:
>
> http://tinyurl.com/38hum and http://tinyurl.com/2wexw
>
Thank you for pointing that out. Seems like I'm duplicating effort here.

/Jesper

Re: [patch against 2.6.1-rc1-mm1] replace check_region with reque st_region in isp16.c

[Jesper Juhl]

On Sat, 3 Jan 2004, Muli Ben-Yehuda wrote:

> On Sat, Jan 03, 2004 at 06:06:20PM +0100, Jesper Juhl wrote:
>
> > One thing that surprised me was that the isp16 driver does not seem to
> > ever call request_region, it only ever calls check_region which
> confuses
> > me a bit - wouldn't it need to (also with older kernels) always call
> > request_region ?
>
> I think so. It looks broken as it was, playing with the IO region
> without requesting it first. Is anyone actually using this driver?
>
> > I would appreciate it is someone could take a quick look at the patch
> and
> > verify that it does the correct thing.
>
> Looks correct to me.
>
Thank you for looking at it, but I don't think I'll be taking it any
further since (as Petri Koistinen pointed out) Omkhar Arasaratnam seems to
already have a patch for this.

Guess I should have searched the LKML archives more thoroughly before
digging into this.. Oh well, I'll just go look at some other bits..


/Jesper Juhl