Thread (7 messages) 7 messages, 3 authors, 2011-10-13

Re: [PATCH] cy82c693: fix PCI device selection

From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Date: 2011-10-13 10:42:12
Also in: lkml

David Miller wrote:
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Date: Wed, 12 Oct 2011 16:52:10 +0200
quoted
David Miller wrote:
quoted
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Date: Tue, 11 Oct 2011 19:37:32 +0200
quoted
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@gmail.com>
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@gmail.com>
---
 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@altavista.net), Maintainer
  *  Copyright (C) 1998-2002 Andre Hedrick <andre@linux-ide.org>, 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);
 }
 
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help