Re: [PATCH] IDE: don't offer IDE_GENERIC on ia64

[Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>]

Hi,

On 8/15/05, Bjorn Helgaas <bjorn.helgaas@hp.com> wrote:
> On Friday 12 August 2005 2:40 am, Alan Cox wrote:
> > Assuming all IA-64 boxes are PCI or better then you actually want to
> > edit include/asm-ia64/ide.h and edit ide_default_io_base where someone
> > years ago cut and pasted x86-32 values so that case 2-5 are removed.
> > Then you will just probe the compatibility mode PCI addresses for system
> > IDE channels.
> 
> Thanks for the pointer.  There shouldn't be anything arch-
> specific required for ia64, so I think we can get rid of
> just about everything in asm-ia64/ide.h, since everything
> we care about will be discovered by PCI IDE.

Agreed but I have few comments:
* is this change OK w.r.t. IA64_HP_SIM?
* removing IDE_ARCH_OBSOLETE_INIT define has some implications,
  * non-functional ide-cs driver (but there is no PCMCIA on IA64?)
  * ordering change for ide-pnp interfaces in case of no IDE devices
    on default IDE PCI ports, (but there aren't any ide-pnp devices on IA64?)
  * non-functional HDIO_REGISTER_HWIF ioctl (ain't really working either)
  are these implication fine with IA64?

> There's no ia64 reason to limit MAX_HWIFS, so I used
> CONFIG_IDE_MAX_HWIFS, resulting in a little more Kconfig
> ugliness.  Maybe that should move into linux/ide.h, so
> arches only need to define MAX_HWIFS if they have a need
> for it?

This changes default default MAX_HWIFS value from
10 (6 in case of !CONFIG_PCI) to 4 which is not desirable.

IMO the best thing for now is to leave MAX_HWIFS alone.

> I noticed that on a box with no devices on ide1, we probed
> ide1 twice -- once via ide_setup_pci_device() and again via
> ide_generic_init().  This isn't fatal, since the generic probe
> uses the I/O ports setup by PCI IDE, but it's unnecessary
> and a bit ugly.
> 
> I stuck in a "hwif->noprobe = 1" at the point where probe_hwif()
> decides the interface isn't present, which prevents the second
> probe by ide_generic_init().

Please separate this change out - it is a nice fix but it affects other
archs/drivers and somebody needs to audit all IDE host drivers that
they set hwif->noprobe = 0 correctly before probing in case of re-using
hwif that was already owned/probed by other driver.

> Other comments or advice?

Please make IDE_GENERIC depend on !IA_64.

Thanks!
Bartlomiej

> Index: work-vga/drivers/ide/Kconfig
> ===================================================================
> --- work-vga.orig/drivers/ide/Kconfig   2005-08-11 15:39:27.000000000 -0600
> +++ work-vga/drivers/ide/Kconfig        2005-08-15 12:08:05.000000000 -0600
> @@ -52,9 +52,9 @@
> 
>  if IDE
> 
> -config IDE_MAX_HWIFS
> +config IDE_MAX_HWIFS
>         int "Max IDE interfaces"
> -       depends on ALPHA || SUPERH
> +       depends on ALPHA || SUPERH || IA64
>         default 4
>         help
>           This is the maximum number of IDE hardware interfaces that will
> Index: work-vga/include/asm-ia64/ide.h
> ===================================================================
> --- work-vga.orig/include/asm-ia64/ide.h        2005-08-03 16:48:31.000000000 -0600
> +++ work-vga/include/asm-ia64/ide.h     2005-08-15 14:29:44.000000000 -0600
> @@ -14,58 +14,12 @@
>  #ifdef __KERNEL__
> 
>  #include <linux/config.h>
> -
> -#include <linux/irq.h>
> +#include <asm-generic/ide_iops.h>
> 
>  #ifndef MAX_HWIFS
> -# ifdef CONFIG_PCI
> -#define MAX_HWIFS      10
> -# else
> -#define MAX_HWIFS      6
> -# endif
> -#endif
> -
> -#define IDE_ARCH_OBSOLETE_DEFAULTS
> -
> -static inline int ide_default_irq(unsigned long base)
> -{
> -       switch (base) {
> -             case 0x1f0: return isa_irq_to_vector(14);
> -             case 0x170: return isa_irq_to_vector(15);
> -             case 0x1e8: return isa_irq_to_vector(11);
> -             case 0x168: return isa_irq_to_vector(10);
> -             case 0x1e0: return isa_irq_to_vector(8);
> -             case 0x160: return isa_irq_to_vector(12);
> -             default:
> -               return 0;
> -       }
> -}
> -
> -static inline unsigned long ide_default_io_base(int index)
> -{
> -       switch (index) {
> -             case 0: return 0x1f0;
> -             case 1: return 0x170;
> -             case 2: return 0x1e8;
> -             case 3: return 0x168;
> -             case 4: return 0x1e0;
> -             case 5: return 0x160;
> -             default:
> -               return 0;
> -       }
> -}
> -
> -#define IDE_ARCH_OBSOLETE_INIT
> -#define ide_default_io_ctl(base)       ((base) + 0x206) /* obsolete */
> -
> -#ifdef CONFIG_PCI
> -#define ide_init_default_irq(base)     (0)
> -#else
> -#define ide_init_default_irq(base)     ide_default_irq(base)
> +#define MAX_HWIFS      CONFIG_IDE_MAX_HWIFS
>  #endif
> 
> -#include <asm-generic/ide_iops.h>
> -
>  #endif /* __KERNEL__ */
> 
>  #endif /* __ASM_IA64_IDE_H */
> Index: work-vga/include/linux/ide.h
> ===================================================================
> --- work-vga.orig/include/linux/ide.h   2005-08-03 16:48:32.000000000 -0600
> +++ work-vga/include/linux/ide.h        2005-08-15 12:43:38.000000000 -0600
> @@ -266,7 +266,7 @@
> 
>  #include <asm/ide.h>
> 
> -/* needed on alpha, x86/x86_64, ia64, mips, ppc32 and sh */
> +/* needed on alpha, x86/x86_64, mips, ppc32 and sh */
>  #ifndef IDE_ARCH_OBSOLETE_DEFAULTS
>  # define ide_default_io_base(index)    (0)
>  # define ide_default_irq(base)         (0)
> Index: work-vga/drivers/ide/ide-probe.c
> ===================================================================
> --- work-vga.orig/drivers/ide/ide-probe.c       2005-08-09 15:09:59.000000000 -0600
> +++ work-vga/drivers/ide/ide-probe.c    2005-08-15 14:30:33.000000000 -0600
> @@ -852,6 +852,7 @@
> 
>         if (!hwif->present) {
>                 ide_hwif_release_regions(hwif);
> +               hwif->noprobe = 1;
>                 return;
>         }