[patch] ide.c as a module

[patch] ide.c as a module

[Daniel Tram Lux]

Hi,

I needed the ide-subsytem as a module on 2.4.23 and noticed (due to the missing modprobe on the embedded linux system)
that ide.c tries to load the module ide-probe-mod which is called ide-detect now.
The patch also get's rid of the need for ide-probe-mini alias ide-detect, but I don't know if that is desired? (it was in my case).

Regards

Daniel Lux

P.S.
please c/c me for comments. 

--- linux-2.4.23.org/drivers/ide/ide.c  2003-11-28 19:26:20.000000000 +0100
+++ linux-2.4.23/drivers/ide/ide.c      2004-03-11 20:31:51.000000000 +0100
@@ -514,11 +514,7 @@

 void ide_probe_module (int revaldiate)
 {
-       if (!ide_probe) {
-#if  defined(CONFIG_BLK_DEV_IDE_MODULE)
-               (void) request_module("ide-probe-mod");
-#endif
-       } else {
+       if (ide_probe) {
                (void) ide_probe->init();
        }
        revalidate_drives(revaldiate);
@@ -3018,13 +3014,13 @@
                banner_printed = 1;
        }

+       initializing = 1;
        init_ide_data();

 #ifndef CLASSIC_BUILTINS_METHOD
        ide_init_builtin_subdrivers();
 #endif /* CLASSIC_BUILTINS_METHOD */

-       initializing = 1;
        ide_init_builtin_drivers();
        initializing = 0;

@@ -3043,6 +3039,9 @@
 MODULE_PARM(options,"s");
 MODULE_LICENSE("GPL");

+extern int ideprobe_init_module();
+extern void ideprobe_cleanup_module (void);
+
 static void __init parse_options (char *line)
 {
        char *next = line;
@@ -3059,14 +3058,19 @@

 int init_module (void)
 {
+        int res;
+
        parse_options(options);
-       return ide_init();
+       res = ide_init();
+       ideprobe_init_module();
+       return(res);
 }

 void cleanup_module (void)
 {
        int index;
-
+
+       ideprobe_cleanup_module();
        unregister_reboot_notifier(&ide_notifier);
        for (index = 0; index < MAX_HWIFS; ++index) {
                ide_unregister(index);

Re: [patch] ide.c as a module

[Bartlomiej Zolnierkiewicz]

On Thursday 11 of December 2003 21:25, Daniel Tram Lux wrote:
> Hi,

Hi,

> I needed the ide-subsytem as a module on 2.4.23 and noticed (due to the
> missing modprobe on the embedded linux system) that ide.c tries to load the
> module ide-probe-mod which is called ide-detect now. The patch also get's
> rid of the need for ide-probe-mini alias ide-detect, but I don't know if
> that is desired? (it was in my case).

It is incorrect, it will make most of modules for PCI IDE chipsets fail
due to always calling ide_init() from ide.c:init_module().

You need to modprobe ide-detect if you are using generic IDE code
(no chipset specific driver - probably the case for your embedded system).

You are right that ide-probe-mini alias is not needed, ide-probe-mini.c should
be renamed to ide-detect.c (or ide-detect.o to ide-probe-mini.o).

> --- linux-2.4.23.org/drivers/ide/ide.c  2003-11-28 19:26:20.000000000 +0100
> +++ linux-2.4.23/drivers/ide/ide.c      2004-03-11 20:31:51.000000000 +0100
> @@ -514,11 +514,7 @@
>
>  void ide_probe_module (int revaldiate)
>  {
> -       if (!ide_probe) {
> -#if  defined(CONFIG_BLK_DEV_IDE_MODULE)
> -               (void) request_module("ide-probe-mod");
> -#endif
> -       } else {
> +       if (ide_probe) {
>                 (void) ide_probe->init();
>         }
>         revalidate_drives(revaldiate);

You should make this change in ide_register_hw() instead:

-		ide_probe_module();
+#ifdef MODULE
+		if (ideprobe_init_module() == -EBUSY)
+#endif
+			ideprobe_init();

And get rid of ide_probe pointer.

--bart

Re: [patch] ide.c as a module

[Bartlomiej Zolnierkiewicz]

On Thursday 11 of December 2003 22:25, Bartlomiej Zolnierkiewicz wrote:
> > --- linux-2.4.23.org/drivers/ide/ide.c  2003-11-28 19:26:20.000000000
> > +0100 +++ linux-2.4.23/drivers/ide/ide.c      2004-03-11
> > 20:31:51.000000000 +0100 @@ -514,11 +514,7 @@
> >
> >  void ide_probe_module (int revaldiate)
> >  {
> > -       if (!ide_probe) {
> > -#if  defined(CONFIG_BLK_DEV_IDE_MODULE)
> > -               (void) request_module("ide-probe-mod");
> > -#endif
> > -       } else {
> > +       if (ide_probe) {
> >                 (void) ide_probe->init();
> >         }
> >         revalidate_drives(revaldiate);
>
> You should make this change in ide_register_hw() instead:
>
> -		ide_probe_module();
> +#ifdef MODULE
> +		if (ideprobe_init_module() == -EBUSY)
> +#endif
> +			ideprobe_init();
>
> And get rid of ide_probe pointer.

and of course of ide_probe_module().

Re: [patch] ide.c as a module

[Daniel Tram Lux]

On Thu, Dec 11, 2003 at 10:25:14PM +0100, Bartlomiej Zolnierkiewicz wrote:
> 
> On Thursday 11 of December 2003 21:25, Daniel Tram Lux wrote:
> > Hi,
> 
> Hi,
> 
> > I needed the ide-subsytem as a module on 2.4.23 and noticed (due to the
> > missing modprobe on the embedded linux system) that ide.c tries to load the
> > module ide-probe-mod which is called ide-detect now. The patch also get's
> > rid of the need for ide-probe-mini alias ide-detect, but I don't know if
> > that is desired? (it was in my case).
> 
> It is incorrect, it will make most of modules for PCI IDE chipsets fail
> due to always calling ide_init() from ide.c:init_module().

ide_init is called from ide.c:init_module()
in the original version:

int init_module (void)
{
	parse_options(options);
	return ide_init(); <--------
}


> 
> You need to modprobe ide-detect if you are using generic IDE code
> (no chipset specific driver - probably the case for your embedded system).
> 

I know this, but ide-detect is basically an empty module, only calling ideprobe_init_module()
can't this be done right away from ide.c or are there any reasons to delay the call
until later at a user defined point of time?

> You are right that ide-probe-mini alias is not needed, ide-probe-mini.c should
> be renamed to ide-detect.c (or ide-detect.o to ide-probe-mini.o).
> 
> > --- linux-2.4.23.org/drivers/ide/ide.c  2003-11-28 19:26:20.000000000 +0100
> > +++ linux-2.4.23/drivers/ide/ide.c      2004-03-11 20:31:51.000000000 +0100
> > @@ -514,11 +514,7 @@
> >
> >  void ide_probe_module (int revaldiate)
> >  {
> > -       if (!ide_probe) {
> > -#if  defined(CONFIG_BLK_DEV_IDE_MODULE)
> > -               (void) request_module("ide-probe-mod");
> > -#endif
> > -       } else {
> > +       if (ide_probe) {
> >                 (void) ide_probe->init();
> >         }
> >         revalidate_drives(revaldiate);
> 
> You should make this change in ide_register_hw() instead:
> 
> -		ide_probe_module();
> +#ifdef MODULE
> +		if (ideprobe_init_module() == -EBUSY)
> +#endif
> +			ideprobe_init();

Your patch will (if MODULE is defined) call ideprobe_init() twice, 
once from ideprobe_init_module() and once from ideprobe_init()
should ideprobe_init really not only be called once and only once?


> 
> And get rid of ide_probe pointer.
> 
> --bart

Re: [patch] ide.c as a module

[Bartlomiej Zolnierkiewicz]

On Friday 12 of December 2003 10:20, Daniel Tram Lux wrote:
> On Thu, Dec 11, 2003 at 10:25:14PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > On Thursday 11 of December 2003 21:25, Daniel Tram Lux wrote:
> > > Hi,
> >
> > Hi,
> >
> > > I needed the ide-subsytem as a module on 2.4.23 and noticed (due to the
> > > missing modprobe on the embedded linux system) that ide.c tries to load
> > > the module ide-probe-mod which is called ide-detect now. The patch also
> > > get's rid of the need for ide-probe-mini alias ide-detect, but I don't
> > > know if that is desired? (it was in my case).
> >
> > It is incorrect, it will make most of modules for PCI IDE chipsets fail
> > due to always calling ide_init() from ide.c:init_module().
>
> ide_init is called from ide.c:init_module()
> in the original version:
>
> int init_module (void)
> {
> 	parse_options(options);
> 	return ide_init(); <--------
> }

I was thinking about ideprobe_init_module() not ide_init() :-).

> > You need to modprobe ide-detect if you are using generic IDE code
> > (no chipset specific driver - probably the case for your embedded
> > system).
>
> I know this, but ide-detect is basically an empty module, only calling
> ideprobe_init_module() can't this be done right away from ide.c or are
> there any reasons to delay the call until later at a user defined point of
> time?

YES.  If you have some PCI IDE, you must load PCI chipset module before
loading ide-detect.o otherwise (you load ide-detect.o before your PCI chipset
module) generic IDE driver will own your IDE ports and you cannot use your
specific PCI chipset driver.

> > You are right that ide-probe-mini alias is not needed, ide-probe-mini.c
> > should be renamed to ide-detect.c (or ide-detect.o to ide-probe-mini.o).
> >
> > > --- linux-2.4.23.org/drivers/ide/ide.c  2003-11-28 19:26:20.000000000
> > > +0100 +++ linux-2.4.23/drivers/ide/ide.c      2004-03-11
> > > 20:31:51.000000000 +0100 @@ -514,11 +514,7 @@
> > >
> > >  void ide_probe_module (int revaldiate)
> > >  {
> > > -       if (!ide_probe) {
> > > -#if  defined(CONFIG_BLK_DEV_IDE_MODULE)
> > > -               (void) request_module("ide-probe-mod");
> > > -#endif
> > > -       } else {
> > > +       if (ide_probe) {
> > >                 (void) ide_probe->init();
> > >         }
> > >         revalidate_drives(revaldiate);
> >
> > You should make this change in ide_register_hw() instead:
> >
> > -		ide_probe_module();
> > +#ifdef MODULE
> > +		if (ideprobe_init_module() == -EBUSY)
> > +#endif
> > +			ideprobe_init();
>
> Your patch will (if MODULE is defined) call ideprobe_init() twice,
> once from ideprobe_init_module() and once from ideprobe_init()
> should ideprobe_init really not only be called once and only once?

No, think about loading chipset modules.  Some (i.e. ide-cs.o) are
calling ide_register_hw() and expect that this function will probe drives
(without need to load ide-detect.o by user).  Change above preserves
current behavior (note that you may have more than one chipset module).

The same (probing for drives by chipset module) can be done for PCI
drivers but requires a some more work in case of 2.4.x kernels.

--bart

> > And get rid of ide_probe pointer.
> >
> > --bart

Re: [patch] ide.c as a module

[Daniel Tram Lux]

Hi,

I tried with using only your suggested changes and removing the ide_probe
ptr, but due to (in include/asm-i386/ide.h) where CONFIG_BLK_DEV_IDEPCI is 
indeed undefined:

static __inline__ void ide_init_default_hwifs(void)
{
#ifndef CONFIG_BLK_DEV_IDEPCI
	hw_regs_t hw;
	int index;

	for(index = 0; index < MAX_HWIFS; index++) {
		memset(&hw, 0, sizeof hw);
		ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
		hw.irq = ide_default_irq(ide_default_io_base(index));
		ide_register_hw(&hw, NULL);
	}
#endif /* CONFIG_BLK_DEV_IDEPCI */
}

I get the following probing messages (I enabled a debug message which is why there are so
many messages):

Using /lib/modules/2.4.23/kernel/drivers/ide/ide-core.o
Uniform Multi-Platform E-IDE driver Revision: 7.00beta4-2.4
ide: Assuming 33MHz system bus speed for PIO modes; override with idebus=xx
probing for hda: present=0, media=32, probetype=ATA
hda: SanDisk SDCFB-256, CFA DISK drive
probing for hdb: present=0, media=32, probetype=ATA
probing for hdb: present=0, media=32, probetype=ATAPI
probing for hdc: present=0, media=32, probetype=ATA
hdc: IC25N040ATMR04-0, ATA DISK drive
probing for hdd: present=0, media=32, probetype=ATA
probing for hdd: present=0, media=32, probetype=ATAPI
probing for hde: present=0, media=32, probetype=ATA
probing for hde: present=0, media=32, probetype=ATAPI
probing for hdf: present=0, media=32, probetype=ATA
probing for hdf: present=0, media=32, probetype=ATAPI
probing for hdg: present=0, media=32, probetype=ATA
probing for hdg: present=0, media=32, probetype=ATAPI
probing for hdh: present=0, media=32, probetype=ATA
probing for hdh: present=0, media=32, probetype=ATAPI
probing for hdi: present=0, media=32, probetype=ATA
probing for hdi: present=0, media=32, probetype=ATAPI
probing for hdj: present=0, media=32, probetype=ATA
probing for hdj: present=0, media=32, probetype=ATAPI
probing for hdk: present=0, media=32, probetype=ATA
probing for hdk: present=0, media=32, probetype=ATAPI
probing for hdl: present=0, media=32, probetype=ATA
probing for hdl: present=0, media=32, probetype=ATAPI
ide0 at 0x1f0-0x1f7,0x3f6 on irq 14
ide1 at 0x170-0x177,0x376 on irq 15
probing for hdc: present=0, media=32, probetype=ATA
hdc: IC25N040ATMR04-0, ATA DISK drive
probing for hdd: present=0, media=32, probetype=ATA
probing for hdd: present=0, media=32, probetype=ATAPI
probing for hde: present=0, media=32, probetype=ATA
probing for hde: present=0, media=32, probetype=ATAPI
probing for hdf: present=0, media=32, probetype=ATA
probing for hdf: present=0, media=32, probetype=ATAPI
probing for hdg: present=0, media=32, probetype=ATA
probing for hdg: present=0, media=32, probetype=ATAPI
probing for hdh: present=0, media=32, probetype=ATA
probing for hdh: present=0, media=32, probetype=ATAPI
probing for hdi: present=0, media=32, probetype=ATA
probing for hdi: present=0, media=32, probetype=ATAPI
probing for hdj: present=0, media=32, probetype=ATA
probing for hdj: present=0, media=32, probetype=ATAPI
probing for hdk: present=0, media=32, probetype=ATA
probing for hdk: present=0, media=32, probetype=ATAPI
probing for hdl: present=0, media=32, probetype=ATA
probing for hdl: present=0, media=32, probetype=ATAPI
ide1 at 0x170-0x177,0x376 on irq 15
probing for hde: present=0, media=32, probetype=ATA
probing for hde: present=0, media=32, probetype=ATAPI
probing for hdf: present=0, media=32, probetype=ATA
probing for hdf: present=0, media=32, probetype=ATAPI
probing for hdg: present=0, media=32, probetype=ATA
probing for hdg: present=0, media=32, probetype=ATAPI
probing for hdh: present=0, media=32, probetype=ATA
probing for hdh: present=0, media=32, probetype=ATAPI
probing for hdi: present=0, media=32, probetype=ATA
probing for hdi: present=0, media=32, probetype=ATAPI
probing for hdj: present=0, media=32, probetype=ATA
probing for hdj: present=0, media=32, probetype=ATAPI
probing for hdk: present=0, media=32, probetype=ATA
probing for hdk: present=0, media=32, probetype=ATAPI
probing for hdl: present=0, media=32, probetype=ATA
probing for hdl: present=0, media=32, probetype=ATAPI
probing for hde: present=0, media=32, probetype=ATA
probing for hde: present=0, media=32, probetype=ATAPI
probing for hdf: present=0, media=32, probetype=ATA
probing for hdf: present=0, media=32, probetype=ATAPI
probing for hdg: present=0, media=32, probetype=ATA
probing for hdg: present=0, media=32, probetype=ATAPI
probing for hdh: present=0, media=32, probetype=ATA
probing for hdh: present=0, media=32, probetype=ATAPI
probing for hdi: present=0, media=32, probetype=ATA
probing for hdi: present=0, media=32, probetype=ATAPI
probing for hdj: present=0, media=32, probetype=ATA
probing for hdj: present=0, media=32, probetype=ATAPI
probing for hdk: present=0, media=32, probetype=ATA
probing for hdk: present=0, media=32, probetype=ATAPI
probing for hdl: present=0, media=32, probetype=ATA
probing for hdl: present=0, media=32, probetype=ATAPI
probing for hde: present=0, media=32, probetype=ATA
probing for hde: present=0, media=32, probetype=ATAPI
probing for hdf: present=0, media=32, probetype=ATA
probing for hdf: present=0, media=32, probetype=ATAPI
probing for hdg: present=0, media=32, probetype=ATA
probing for hdg: present=0, media=32, probetype=ATAPI
probing for hdh: present=0, media=32, probetype=ATA
probing for hdh: present=0, media=32, probetype=ATAPI
probing for hdi: present=0, media=32, probetype=ATA
probing for hdi: present=0, media=32, probetype=ATAPI
probing for hdj: present=0, media=32, probetype=ATA
probing for hdj: present=0, media=32, probetype=ATAPI
probing for hdk: present=0, media=32, probetype=ATA
probing for hdk: present=0, media=32, probetype=ATAPI
probing for hdl: present=0, media=32, probetype=ATA
probing for hdl: present=0, media=32, probetype=ATAPI
probing for hde: present=0, media=32, probetype=ATA
probing for hde: present=0, media=32, probetype=ATAPI
probing for hdf: present=0, media=32, probetype=ATA
probing for hdf: present=0, media=32, probetype=ATAPI
probing for hdg: present=0, media=32, probetype=ATA
probing for hdg: present=0, media=32, probetype=ATAPI
probing for hdh: present=0, media=32, probetype=ATA
probing for hdh: present=0, media=32, probetype=ATAPI
probing for hdi: present=0, media=32, probetype=ATA
probing for hdi: present=0, media=32, probetype=ATAPI
probing for hdj: present=0, media=32, probetype=ATA
probing for hdj: present=0, media=32, probetype=ATAPI
probing for hdk: present=0, media=32, probetype=ATA
probing for hdk: present=0, media=32, probetype=ATAPI
probing for hdl: present=0, media=32, probetype=ATA
probing for hdl: present=0, media=32, probetype=ATAPI
 hda:end_request: I/O error, dev 03:00 (hda), sector 0
end_request: I/O error, dev 03:00 (hda), sector 2
end_request: I/O error, dev 03:00 (hda), sector 4
end_request: I/O error, dev 03:00 (hda), sector 6
end_request: I/O error, dev 03:00 (hda), sector 0
end_request: I/O error, dev 03:00 (hda), sector 2
end_request: I/O error, dev 03:00 (hda), sector 4
end_request: I/O error, dev 03:00 (hda), sector 6
 unable to read partition table
 hdc:end_request: I/O error, dev 16:00 (hdc), sector 0
end_request: I/O error, dev 16:00 (hdc), sector 2
end_request: I/O error, dev 16:00 (hdc), sector 4
end_request: I/O error, dev 16:00 (hdc), sector 6
end_request: I/O error, dev 16:00 (hdc), sector 0
end_request: I/O error, dev 16:00 (hdc), sector 2
end_request: I/O error, dev 16:00 (hdc), sector 4
end_request: I/O error, dev 16:00 (hdc), sector 6
 unable to read partition table

Any ideas how to avoid this?

Regards

Daniel Lux 

On Fri, Dec 12, 2003 at 02:30:36PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Friday 12 of December 2003 10:20, Daniel Tram Lux wrote:
> > On Thu, Dec 11, 2003 at 10:25:14PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > > On Thursday 11 of December 2003 21:25, Daniel Tram Lux wrote:
> > > > Hi,
> > >
> > > Hi,
> > >
> > > > I needed the ide-subsytem as a module on 2.4.23 and noticed (due to the
> > > > missing modprobe on the embedded linux system) that ide.c tries to load
> > > > the module ide-probe-mod which is called ide-detect now. The patch also
> > > > get's rid of the need for ide-probe-mini alias ide-detect, but I don't
> > > > know if that is desired? (it was in my case).
> > >
> > > It is incorrect, it will make most of modules for PCI IDE chipsets fail
> > > due to always calling ide_init() from ide.c:init_module().
> >
> > ide_init is called from ide.c:init_module()
> > in the original version:
> >
> > int init_module (void)
> > {
> > 	parse_options(options);
> > 	return ide_init(); <--------
> > }
> 
> I was thinking about ideprobe_init_module() not ide_init() :-).
> 
> > > You need to modprobe ide-detect if you are using generic IDE code
> > > (no chipset specific driver - probably the case for your embedded
> > > system).
> >
> > I know this, but ide-detect is basically an empty module, only calling
> > ideprobe_init_module() can't this be done right away from ide.c or are
> > there any reasons to delay the call until later at a user defined point of
> > time?
> 
> YES.  If you have some PCI IDE, you must load PCI chipset module before
> loading ide-detect.o otherwise (you load ide-detect.o before your PCI chipset
> module) generic IDE driver will own your IDE ports and you cannot use your
> specific PCI chipset driver.
> 
> > > You are right that ide-probe-mini alias is not needed, ide-probe-mini.c
> > > should be renamed to ide-detect.c (or ide-detect.o to ide-probe-mini.o).
> > >
> > > > --- linux-2.4.23.org/drivers/ide/ide.c  2003-11-28 19:26:20.000000000
> > > > +0100 +++ linux-2.4.23/drivers/ide/ide.c      2004-03-11
> > > > 20:31:51.000000000 +0100 @@ -514,11 +514,7 @@
> > > >
> > > >  void ide_probe_module (int revaldiate)
> > > >  {
> > > > -       if (!ide_probe) {
> > > > -#if  defined(CONFIG_BLK_DEV_IDE_MODULE)
> > > > -               (void) request_module("ide-probe-mod");
> > > > -#endif
> > > > -       } else {
> > > > +       if (ide_probe) {
> > > >                 (void) ide_probe->init();
> > > >         }
> > > >         revalidate_drives(revaldiate);
> > >
> > > You should make this change in ide_register_hw() instead:
> > >
> > > -		ide_probe_module();
> > > +#ifdef MODULE
> > > +		if (ideprobe_init_module() == -EBUSY)
> > > +#endif
> > > +			ideprobe_init();
> >
> > Your patch will (if MODULE is defined) call ideprobe_init() twice,
> > once from ideprobe_init_module() and once from ideprobe_init()
> > should ideprobe_init really not only be called once and only once?
> 
> No, think about loading chipset modules.  Some (i.e. ide-cs.o) are
> calling ide_register_hw() and expect that this function will probe drives
> (without need to load ide-detect.o by user).  Change above preserves
> current behavior (note that you may have more than one chipset module).
> 
> The same (probing for drives by chipset module) can be done for PCI
> drivers but requires a some more work in case of 2.4.x kernels.
> 
> --bart
> 
> > > And get rid of ide_probe pointer.
> > >
> > > --bart

Re: [patch] ide.c as a module

[Bartlomiej Zolnierkiewicz]

On Friday 12 of December 2003 15:42, Daniel Tram Lux wrote:
> Hi,
>
> I tried with using only your suggested changes and removing the ide_probe
                            ^^^^
Your patch + changes or only changes?

> ptr, but due to (in include/asm-i386/ide.h) where CONFIG_BLK_DEV_IDEPCI is
> indeed undefined:
>
> static __inline__ void ide_init_default_hwifs(void)
> {
> #ifndef CONFIG_BLK_DEV_IDEPCI
> 	hw_regs_t hw;
> 	int index;
>
> 	for(index = 0; index < MAX_HWIFS; index++) {
> 		memset(&hw, 0, sizeof hw);
> 		ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
> 		hw.irq = ide_default_irq(ide_default_io_base(index));
> 		ide_register_hw(&hw, NULL);
> 	}
> #endif /* CONFIG_BLK_DEV_IDEPCI */
> }
>
> I get the following probing messages (I enabled a debug message which is
> why there are so many messages):

"initializing = 1" must be moved from ide_init() to ide_init_data()
(just before ide_init_default_hwifs() call).

--bart

Re: [patch] ide.c as a module

[Daniel Tram Lux]

Hi Bartlomiej,

I applied your changes only, reverting all of mine...

moving the initializing=1 also solves the multiple init problem...thanks

here is how the patch looks like now:

--- ./linux-2.4.23.org/drivers/ide/ide.c        2003-11-28 19:26:20.000000000 +0100
+++ ./linux-2.4.23/drivers/ide/ide.c    2004-03-12 17:56:28.000000000 +0100
@@ -188,7 +188,6 @@
  */
 ide_module_t *ide_chipsets;
 ide_module_t *ide_modules;
-ide_module_t *ide_probe;
 
 /*
  * This is declared extern in ide.h, for access by other IDE modules:
@@ -307,6 +306,7 @@
                init_hwif_data(index);
 
        /* Add default hw interfaces */
+       initializing = 1;
        ide_init_default_hwifs();
 
        idebus_parameter = 0;
@@ -512,20 +512,6 @@
        }
 }
 
-void ide_probe_module (int revaldiate)
-{
-       if (!ide_probe) {
-#if  defined(CONFIG_BLK_DEV_IDE_MODULE)
-               (void) request_module("ide-probe-mod");
-#endif
-       } else {
-               (void) ide_probe->init();
-       }
-       revalidate_drives(revaldiate);
-}
-
-EXPORT_SYMBOL(ide_probe_module);
-
 void ide_driver_module (int revaldiate)
 {
        int index;
@@ -534,7 +520,8 @@
        for (index = 0; index < MAX_HWIFS; ++index)
                if (ide_hwifs[index].present)
                        goto search;
-       ide_probe_module(revaldiate);
+        ideprobe_init();
+        revalidate_drives(revaldiate);
 search:
        while (module) {
                (void) module->init();
@@ -1141,6 +1128,7 @@
 }
 
 EXPORT_SYMBOL(ide_setup_ports);
+extern int ideprobe_init_module (void);
 
 /*
  * Register an IDE interface, specifing exactly the registers etc
@@ -1181,7 +1169,10 @@
        hwif->chipset = hw->chipset;
 
        if (!initializing) {
-               ide_probe_module(1);
+#ifdef MODULE
+           if (ideprobe_init_module() == -EBUSY)
+#endif
+                ideprobe_init();
 #ifdef CONFIG_PROC_FS
                create_proc_ide_interfaces();
 #endif
@@ -2954,7 +2945,6 @@
  */
 devfs_handle_t ide_devfs_handle;
 
-EXPORT_SYMBOL(ide_probe);
 EXPORT_SYMBOL(ide_devfs_handle);
 
 static int ide_notify_reboot (struct notifier_block *this, unsigned long event, void *x)
@@ -3024,7 +3014,6 @@
        ide_init_builtin_subdrivers();
 #endif /* CLASSIC_BUILTINS_METHOD */
 
-       initializing = 1;
        ide_init_builtin_drivers();
        initializing = 0;



--- ./linux-2.4.23.org/drivers/ide/ide-probe.c  2003-11-28 19:26:20.000000000 +0100
+++ ./linux-2.4.23/drivers/ide/ide-probe.c      2004-03-12 17:56:37.000000000 +0100
@@ -1407,8 +1407,6 @@
                        probe_hwif_init(&ide_hwifs[index]);
 #endif /* HWIF_PROBE_CLASSIC_METHOD */
 
-       if (!ide_probe)
-               ide_probe = &ideprobe_module;
        MOD_DEC_USE_COUNT;
        return 0;
 }
@@ -1436,7 +1434,6 @@
 
 void ideprobe_cleanup_module (void)
 {
-       ide_probe = NULL;
        ide_xlate_1024_hook = 0;
 }
 EXPORT_SYMBOL(ideprobe_init_module);


Regards
   Daniel

On Fri, Dec 12, 2003 at 04:46:04PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Friday 12 of December 2003 15:42, Daniel Tram Lux wrote:
> > Hi,
> >
> > I tried with using only your suggested changes and removing the ide_probe
>                             ^^^^
> Your patch + changes or only changes?
> 
> > ptr, but due to (in include/asm-i386/ide.h) where CONFIG_BLK_DEV_IDEPCI is
> > indeed undefined:
> >
> > static __inline__ void ide_init_default_hwifs(void)
> > {
> > #ifndef CONFIG_BLK_DEV_IDEPCI
> > 	hw_regs_t hw;
> > 	int index;
> >
> > 	for(index = 0; index < MAX_HWIFS; index++) {
> > 		memset(&hw, 0, sizeof hw);
> > 		ide_init_hwif_ports(&hw, ide_default_io_base(index), 0, NULL);
> > 		hw.irq = ide_default_irq(ide_default_io_base(index));
> > 		ide_register_hw(&hw, NULL);
> > 	}
> > #endif /* CONFIG_BLK_DEV_IDEPCI */
> > }
> >
> > I get the following probing messages (I enabled a debug message which is
> > why there are so many messages):
> 
> "initializing = 1" must be moved from ide_init() to ide_init_data()
> (just before ide_init_default_hwifs() call).
> 
> --bart

Re: [patch] ide.c as a module

[Bartlomiej Zolnierkiewicz]

On Friday 12 of December 2003 18:17, Daniel Tram Lux wrote:
> Hi Bartlomiej,
>
> I applied your changes only, reverting all of mine...
>
> moving the initializing=1 also solves the multiple init problem...thanks
>
> here is how the patch looks like now:

Thanks.  I noticed that in 2.4 ide_probe_module() has revaldiate parameter
(I am currently fixing modules in 2.6 so I over-looked it before), so I need
to check if these changes are sufficient.  If so I will submit to Marcelo.

Regards,
--bart

Re: [patch] ide.c as a module

[Krzysztof Halasa]

BTW: modular IDE in 2.4.23 is still problematic - you can't unload the
chipset driver (piix.o or something like) which in turn references the
core IDE module.

Is anyone working on it? I could probably fix it but I don't want to
duplicate the efforts.
-- 
Krzysztof Halasa, B*FH

Re: [patch] ide.c as a module

[Bartlomiej Zolnierkiewicz]

On Thursday 18 of December 2003 01:29, Krzysztof Halasa wrote:
> BTW: modular IDE in 2.4.23 is still problematic - you can't unload the
> chipset driver (piix.o or something like) which in turn references the
> core IDE module.

It is probably too much work to fix it (properly) in 2.4.x and 2.6.x...

Please note that there is no refcounting in IDE drivers,
there is no host object type, also table of IDE ports (ide_hwifs[]) is static.

I hope 2.7 will obsolete drivers/ide...

--bart

Re: [patch] ide.c as a module

[Randy.Dunlap]

On Fri, 19 Dec 2003 17:26:46 +0100 Bartlomiej Zolnierkiewicz <B.Zolnierkiewicz@elka.pw.edu.pl> wrote:

| On Thursday 18 of December 2003 01:29, Krzysztof Halasa wrote:
| > BTW: modular IDE in 2.4.23 is still problematic - you can't unload the
| > chipset driver (piix.o or something like) which in turn references the
| > core IDE module.
| 
| It is probably too much work to fix it (properly) in 2.4.x and 2.6.x...
| 
| Please note that there is no refcounting in IDE drivers,
| there is no host object type, also table of IDE ports (ide_hwifs[]) is static.
| 
| I hope 2.7 will obsolete drivers/ide...

in favor of libata or what?

--
~Randy
MOTD:  Always include version info.

Re: [patch] ide.c as a module

[Bartlomiej Zolnierkiewicz]

On Friday 19 of December 2003 18:05, Randy.Dunlap wrote:
> On Fri, 19 Dec 2003 17:26:46 +0100 Bartlomiej Zolnierkiewicz <B.Zolnierkiewicz@elka.pw.edu.pl> wrote:
> | On Thursday 18 of December 2003 01:29, Krzysztof Halasa wrote:
> | > BTW: modular IDE in 2.4.23 is still problematic - you can't unload the
> | > chipset driver (piix.o or something like) which in turn references the
> | > core IDE module.
> |
> | It is probably too much work to fix it (properly) in 2.4.x and 2.6.x...
> |
> | Please note that there is no refcounting in IDE drivers,
> | there is no host object type, also table of IDE ports (ide_hwifs[]) is
> | static.
> |
> | I hope 2.7 will obsolete drivers/ide...
>
> in favor of libata or what?

yes, in favor of libata, but don't tell jeff ;-).

--bart

Re: [patch] ide.c as a module

[Andre Hedrick]

Bartlomiej,

There is a way, you just need to use a bigger hammer.
But if you are going to rootwad ./drivers/ide, I want the satisfaction of
MAD (mutual assured destruction!)

Cheers,

Andre Hedrick
LAD Storage Consulting Group

On Fri, 19 Dec 2003, Randy.Dunlap wrote:

> On Fri, 19 Dec 2003 17:26:46 +0100 Bartlomiej Zolnierkiewicz <B.Zolnierkiewicz@elka.pw.edu.pl> wrote:
> 
> | On Thursday 18 of December 2003 01:29, Krzysztof Halasa wrote:
> | > BTW: modular IDE in 2.4.23 is still problematic - you can't unload the
> | > chipset driver (piix.o or something like) which in turn references the
> | > core IDE module.
> | 
> | It is probably too much work to fix it (properly) in 2.4.x and 2.6.x...
> | 
> | Please note that there is no refcounting in IDE drivers,
> | there is no host object type, also table of IDE ports (ide_hwifs[]) is static.
> | 
> | I hope 2.7 will obsolete drivers/ide...
> 
> in favor of libata or what?
> 
> --
> ~Randy
> MOTD:  Always include version info.
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>