Re: [patch] ide.c as a module

[Daniel Tram Lux <daniel@starbattle.com>]

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