David Miller wrote:
> From: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
> Date: Wed, 12 Oct 2011 16:52:10 +0200
>
> > David Miller wrote:
> >
> >> From: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
> >> Date: Tue, 11 Oct 2011 19:37:32 +0200
> >>
> >> > While at it remove redundant pci_get_slot() call as cy82c693_init_one()
> >> > already takes care of keeping the reference on the second port's PCI
> >> > device.
> >>
> >> Please do not submit unrelated changes with a bug fix, and as IDE is
> >> in long-term maintainence I would not accept a risky refinement like
> >> this anyways.
> >
> > Removed code is just bogus, we cannot fail in ->set_pio_mode method.
> >
> > Please take a look at the code:
> >
> > - /* select primary or secondary channel */
> > - if (hwif->index > 0) { /* drive is on the secondary channel */
> > - dev = pci_get_slot(dev->bus, dev->devfn+1);
> > - if (!dev) {
> > - printk(KERN_ERR "%s: tune_drive: "
> > - "Cannot find secondary interface!\n",
> > - drive->name);
> > - return;
> > - }
> > - }
> >
> > Please apply the patch,
>
> Sorry, I will not do that, please respin the patch with the unrelated
> pieces removed. I don't care how obvious it is to you.
>
> The IDE layer is not a place for refinements or simplifications any
> longer, I'm sorry if that isn't clear to you.
Calling pci_get_slot() inside ->set_pio_mode is a bug since this method
cannot fail but if you want to leave it as it is here is an updated patch:
From: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
Subject: [PATCH v2] cy82c693: fix PCI device selection
Wrong PCI device may be selected by cy82c693_set_pio_mode() if modular
IDE host drivers are used and there are additional IDE PCI devices
installed in the system. Fix it.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
---
drivers/ide/cy82c693.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
Index: b/drivers/ide/cy82c693.c
===================================================================
--- a/drivers/ide/cy82c693.c
+++ b/drivers/ide/cy82c693.c
@@ -1,7 +1,7 @@
/*
* Copyright (C) 1998-2000 Andreas S. Krebs (akrebs@xxxxxxxxxxxxx), Maintainer
* Copyright (C) 1998-2002 Andre Hedrick <andre@xxxxxxxxxxxxx>, Integrator
- * Copyright (C) 2007-2010 Bartlomiej Zolnierkiewicz
+ * Copyright (C) 2007-2011 Bartlomiej Zolnierkiewicz
*
* CYPRESS CY82C693 chipset IDE controller
*
@@ -90,7 +90,7 @@ static void cy82c693_set_pio_mode(ide_hw
u8 time_16, time_8;
/* select primary or secondary channel */
- if (hwif->index > 0) { /* drive is on the secondary channel */
+ if (drive->dn > 1) { /* drive is on the secondary channel */
dev = pci_get_slot(dev->bus, dev->devfn+1);
if (!dev) {
printk(KERN_ERR "%s: tune_drive: "
@@ -141,7 +141,7 @@ static void cy82c693_set_pio_mode(ide_hw
pci_write_config_byte(dev, CY82_IDE_SLAVE_IOW, time_16);
pci_write_config_byte(dev, CY82_IDE_SLAVE_8BIT, time_8);
}
- if (hwif->index > 0)
+ if (drive->dn > 1)
pci_dev_put(dev);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
