Re: [PATCH 2/2] ide-generic: handle probing of legacy io-ports

From: Borislav Petkov
Date: Sun Aug 03 2008 - 10:55:19 EST


On Sun, Aug 03, 2008 at 04:45:39PM +0200, Borislav Petkov wrote:
> On Sun, Aug 03, 2008 at 04:11:10PM +0200, Bartlomiej Zolnierkiewicz wrote:
> >
> > On Sunday 03 August 2008, Borislav Petkov wrote:
> > > From: Borislav Petkov <petkovbb@xxxxxxxxx>
> > > Date: Sun, 3 Aug 2008 09:28:53 +0200
> > > Subject: [PATCH 2/2] ide-generic: handle probing of legacy io-ports
> > >
> > > Avoid probing the io-ports in case an IDE PCI controller is present and it uses
> > > the legacy iobases. If we still want to enforce the probing, we do
> > >
> > > ide_generic.probe_mask=0x3f
> > >
> > > on the kernel command line.
> > >
> > > Signed-off-by: Borislav Petkov <petkovbb@xxxxxxxxx>
> >
> > Thanks for reworking the patch, looks much better now.
> >
> > There are still some issues to address though.
> >
> > > ---
> > > drivers/ide/ide-generic.c | 23 ++++++++++++++++++-----
> > > 1 files changed, 18 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/ide/ide-generic.c b/drivers/ide/ide-generic.c
> > > index 8fe8b5b..7d79616 100644
> > > --- a/drivers/ide/ide-generic.c
> > > +++ b/drivers/ide/ide-generic.c
> > > @@ -27,7 +27,7 @@
> > >
> > > #define DRV_NAME "ide_generic"
> > >
> > > -static int probe_mask = 0x03;
> > > +static int probe_mask = 0x00;
> >
> > No need to initialize it now.
>
> right, static.
>
> > > module_param(probe_mask, int, 0);
> > > MODULE_PARM_DESC(probe_mask, "probe mask for legacy ISA IDE ports");
> > >
> > > @@ -105,18 +105,31 @@ static int __init ide_generic_init(void)
> > > hw_regs_t hw[MAX_HWIFS], *hws[MAX_HWIFS];
> > > struct ide_host *host;
> > > unsigned long io_addr;
> > > - int i, rc;
> > > + int i, rc, dummy, primary = 0, secondary = 0;
> > >
> > > #ifdef CONFIG_MIPS
> > > if (!ide_probe_legacy())
> > > return -ENODEV;
> > > #endif
> > > - printk(KERN_INFO DRV_NAME ": please use \"probe_mask=0x3f\" module "
> > > - "parameter for probing all legacy ISA IDE ports\n");
> > > + ata_legacy_check_iobases(&primary, &secondary, &dummy);
> > > +
> > > + if (primary) {
> >
> > Shouldn't this also check for secondary?
>
> you don't have to since primary is set to one in all cases.

crap, forget what i said here ^ :(.

--
Regards/Gruss,
Boris.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/