Re: [PATCH] Add a global ide=off switch for drivers/ide

2007-10-25 Thread Andi Kleen
On Thursday 25 October 2007 23:07:23 Bartlomiej Zolnierkiewicz wrote:

 On Monday 15 October 2007, Andi Kleen wrote:
  
  Had a situation where drivers/ide was compiled in, but I wanted to turn 
  it off to let the drivers/ata drivers take over. I ended up using 
  ide*=noprobe,
  but that was somewhat clumpsy because I wasn't sure how many IDE interfaces
  the machine really had.
  
  Add a global ide=off switch to handle this situation better.
 
 Overall looks OK but I think we should limit it to IDE built-in case
 (when IDE is modular it is all up to the user-space anyway).

Disagree. It's useful for the modular case too e.g. if you
have the ide modules in your initrd and you want to not load
them for some reason (e.g. debugging) 

Besides adding so many ifdefs would be ugly in my opinion.
 
  The patch is a little bigger because I tried to cover all modules.
 
 A few still needs to be covered:
 - drivers/scsi/ide-scsi.c (other directory)
 - drivers/ide/legacy/ide_platform.c (new driver)
 - drivers/ide/legacy/ide-cs.c (late_initcall)
 - drivers/ide/pci/sgiioc4.c (ditto, not a SFF-PCI driver)

Thanks I'll fix those.

  I'm also not 100% sure ENODEV is the right error return for this 
  case, but I didn't come up with a better one.
 
 -EPERM?  IMO it would be more appropriate (and easy to distinguish
 from the real -ENODEV).

Ok.

 
  +int ide_off;
  +EXPORT_SYMBOL(ide_off);
  +
 
 _GPL?
 
 Please cover it with #ifdef/#endif CONFIG_BLK_DEV_IDE as it should be
 valid only when IDE is built-in.


 
 This way we don't pollute device/host drivers with CONFIG_BLK_DEV_IDE #ifdefs.

What CONFIG_BLK_DEV_IDE ifdefs? I added the check only to code that is already
conditional to this I believe and there were no additional ifdefs at all.

-Andi
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Add a global ide=off switch for drivers/ide

2007-10-15 Thread Andi Kleen

Had a situation where drivers/ide was compiled in, but I wanted to turn 
it off to let the drivers/ata drivers take over. I ended up using ide*=noprobe,
but that was somewhat clumpsy because I wasn't sure how many IDE interfaces
the machine really had.

Add a global ide=off switch to handle this situation better.

The patch is a little bigger because I tried to cover all modules.

I'm also not 100% sure ENODEV is the right error return for this 
case, but I didn't come up with a better one.

The ARM/MIPS part is uncompiled.

Signed-off-by: Andi Kleen [EMAIL PROTECTED]

Index: linux-2.6.23-rc8-misc/drivers/ide/ide.c
===
--- linux-2.6.23-rc8-misc.orig/drivers/ide/ide.c
+++ linux-2.6.23-rc8-misc/drivers/ide/ide.c
@@ -91,6 +91,9 @@ static const u8 ide_hwif_to_major[] = { 
 static int idebus_parameter;   /* holds the idebus= parameter */
 static int system_bus_speed;   /* holds what we think is VESA/PCI bus speed */
 
+int ide_off;
+EXPORT_SYMBOL(ide_off);
+
 DEFINE_MUTEX(ide_cfg_mtx);
  __cacheline_aligned_in_smp DEFINE_SPINLOCK(ide_lock);
 
@@ -1249,6 +1252,13 @@ static int __init ide_setup(char *s)
return 0;
 
printk(KERN_INFO ide_setup: %s, s);
+
+   if (!strcmp(s, ide=off)) {
+   printk( : IDE disabled\n);
+   ide_off = 1;
+   return 1;
+   }
+
init_ide_data ();
 
 #ifdef CONFIG_BLK_DEV_IDEDOUBLER
@@ -1717,6 +1727,9 @@ static int __init ide_init(void)
 {
int ret;
 
+   if (ide_off)
+   return -ENODEV;
+
printk(KERN_INFO Uniform Multi-Platform E-IDE driver  REVISION \n);
system_bus_speed = ide_system_bus_speed();
 
Index: linux-2.6.23-rc8-misc/drivers/ide/setup-pci.c
===
--- linux-2.6.23-rc8-misc.orig/drivers/ide/setup-pci.c
+++ linux-2.6.23-rc8-misc/drivers/ide/setup-pci.c
@@ -793,6 +793,8 @@ static LIST_HEAD(ide_pci_drivers);
 int __ide_pci_register_driver(struct pci_driver *driver, struct module *module,
  const char *mod_name)
 {
+   if (ide_off)
+   return -ENODEV;
if(!pre_init)
return __pci_register_driver(driver, module, mod_name);
driver-driver.owner = module;
Index: linux-2.6.23-rc8-misc/include/linux/ide.h
===
--- linux-2.6.23-rc8-misc.orig/include/linux/ide.h
+++ linux-2.6.23-rc8-misc/include/linux/ide.h
@@ -871,6 +871,8 @@ typedef struct ide_driver_s ide_driver_t
 
 extern struct mutex ide_setting_mtx;
 
+extern int ide_off;
+
 int set_io_32bit(ide_drive_t *, int);
 int set_pio_mode(ide_drive_t *, int);
 int set_using_dma(ide_drive_t *, int);
Index: linux-2.6.23-rc8-misc/drivers/ide/arm/bast-ide.c
===
--- linux-2.6.23-rc8-misc.orig/drivers/ide/arm/bast-ide.c
+++ linux-2.6.23-rc8-misc/drivers/ide/arm/bast-ide.c
@@ -52,6 +52,9 @@ bastide_register(unsigned int base, unsi
 
 static int __init bastide_init(void)
 {
+   if (ide_off)
+   return -ENODEV;
+
/* we can treat the VR1000 and the BAST the same */
 
if (!(machine_is_bast() || machine_is_vr1000()))
Index: linux-2.6.23-rc8-misc/drivers/ide/arm/icside.c
===
--- linux-2.6.23-rc8-misc.orig/drivers/ide/arm/icside.c
+++ linux-2.6.23-rc8-misc/drivers/ide/arm/icside.c
@@ -824,6 +824,8 @@ static struct ecard_driver icside_driver
 
 static int __init icside_init(void)
 {
+   if (ide_off)
+   return -ENODEV;
return ecard_register_driver(icside_driver);
 }
 
Index: linux-2.6.23-rc8-misc/drivers/ide/arm/rapide.c
===
--- linux-2.6.23-rc8-misc.orig/drivers/ide/arm/rapide.c
+++ linux-2.6.23-rc8-misc/drivers/ide/arm/rapide.c
@@ -113,6 +113,8 @@ static struct ecard_driver rapide_driver
 
 static int __init rapide_init(void)
 {
+   if (ide_off)
+   return -ENODEV;
return ecard_register_driver(rapide_driver);
 }
 
Index: linux-2.6.23-rc8-misc/drivers/ide/ide-cd.c
===
--- linux-2.6.23-rc8-misc.orig/drivers/ide/ide-cd.c
+++ linux-2.6.23-rc8-misc/drivers/ide/ide-cd.c
@@ -3568,6 +3568,8 @@ static void __exit ide_cdrom_exit(void)
 
 static int __init ide_cdrom_init(void)
 {
+   if (ide_off)
+   return -ENODEV;
return driver_register(ide_cdrom_driver.gen_driver);
 }
 
Index: linux-2.6.23-rc8-misc/drivers/ide/ide-disk.c
===
--- linux-2.6.23-rc8-misc.orig/drivers/ide/ide-disk.c
+++ linux-2.6.23-rc8-misc/drivers/ide/ide-disk.c
@@ -1329,6 +1329,8 @@ static void __exit idedisk_exit (void)
 
 static int __init idedisk_init(void)
 {
+   if (ide_off)
+   return