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
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
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
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
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