ide.c as a module

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

drivers/ide/ide.c 2004-03-11 20:31:51.000000000 +0100 renamed from drivers/ide/ide.c 2003-11-28 19:26:20.000000000 +0100 +0 -0

@@ -514,11 +514,7 @@
 
 void ide_probe_module (int revaldiate)

Quoted 6 lines

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

drivers/ide/ide.c 2004-03-11 20:31:51.000000000 +0100 line 520

Quoted 9 lines

drivers/ide/ide.c 2004-03-11 20:31:51.000000000 +0100 renamed from drivers/ide/ide.c 2003-11-28 19:26:20.000000000 +0100 +1 -5

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

(quoted diff) +4 -1

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

And get rid of ide_probe pointer.

Quoted 1 line · Daniel Tram Lux

Hi,

Hi,

Quoted 5 lines

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

Quoted 5 lines

drivers/ide/ide.c 2004-03-11 20:31:51.000000000 +0100 renamed from drivers/ide/ide.c 2003-11-28 19:26:20.000000000 +0100 +0 -0

@@ -514,11 +514,7 @@
 
 void ide_probe_module (int revaldiate)

drivers/ide/ide.c 2004-03-11 20:31:51.000000000 +0100 line 520

Quoted 10 lines

drivers/ide/ide.c 2004-03-11 20:31:51.000000000 +0100 renamed from drivers/ide/ide.c 2003-11-28 19:26:20.000000000 +0100 +1 -5

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

And get rid of ide_probe pointer.

--bart

Quoted 5 lines · Bartlomiej Zolnierkiewicz

Quoted 1 line · Daniel Tram Lux

Hi,

Hi,

Quoted 8 lines

Quoted 5 lines

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(); <--------
}

Quoted 3 lines

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?

Quoted 21 lines

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

Quoted 6 lines

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

drivers/ide/ide.c 2004-03-11 20:31:51.000000000 +0100 line 520

Quoted 9 lines

drivers/ide/ide.c 2004-03-11 20:31:51.000000000 +0100 renamed from drivers/ide/ide.c 2003-11-28 19:26:20.000000000 +0100 +1 -5

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

Review

Quoted 5 lines

(quoted diff) +4 -1

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

Quoted 3 lines

And get rid of ide_probe pointer.

--bart

Quoted 15 lines · Daniel Tram Lux

Quoted 5 lines · Bartlomiej Zolnierkiewicz

Quoted 1 line · Daniel Tram Lux

Hi,

Hi,

Quoted 8 lines

Quoted 5 lines

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().

Quoted 8 lines

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() :-).

Quoted 8 lines

Quoted 3 lines

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.

Quoted 21 lines

Quoted 21 lines

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

Quoted 6 lines

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

drivers/ide/ide.c 2004-03-11 20:31:51.000000000 +0100 line 520

Quoted 9 lines

drivers/ide/ide.c 2004-03-11 20:31:51.000000000 +0100 renamed from drivers/ide/ide.c 2003-11-28 19:26:20.000000000 +0100 +1 -5

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

Quoted 9 lines

Review

Quoted 5 lines

(quoted diff) +4 -1

-		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

Quoted 3 lines

Quoted 3 lines

And get rid of ide_probe pointer.

--bart

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

Quoted 3 lines · Daniel Tram Lux

Hi,

I tried with using only your suggested changes and removing the ide_probe

                            ^^^^

Your patch + changes or only changes?

Quoted 15 lines

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);
	}

Quoted 5 lines

#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

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:

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.

On Fri, 19 Dec 2003 17:26:46 +0100 Bartlomiej Zolnierkiewicz <[email protected]> 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?

On Fri, 19 Dec 2003 17:26:46 +0100 Bartlomiej Zolnierkiewicz <[email protected]> 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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/