Skip to contentSkip to conversation

ide: fix ->quirk_list checking in ide_do_request()

PATCH In review Bartlomiej Zolnierkiewicz 2 participants 13 messages Nov 3, 2008 – Nov 9, 2008 libata

lore
Share & tools
lore
#0 lore raw

PATCH 1/6 ide: fix ->quirk_list checking in ide_do_request()

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] ide: fix ->quirk_list checking in ide_do_request()

Fix nIEN quirk check to also omit quirky devices using pdc202xx_{new,old} host drivers for which ->quirk_list == 2.

---
against pata-2.6 tree

 drivers/ide/ide-io.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/ide/ide-io.c +1 -1
Index: b/drivers/ide/ide-io.c
===================================================================
@@ -1028,7 +1028,7 @@ static void ide_do_request (ide_hwgroup_
 			 * set nIEN for previous hwif, drives in the
 			 * quirk_list may not like intr setups/cleanups
 			 */
-			if (drive->quirk_list != 1)
+			if (drive->quirk_list == 0)
 				hwif->tp_ops->set_irq(hwif, 0);
 		}
 		hwgroup->hwif = hwif;
#1 lore raw

PATCH 2/6 ide: always set nIEN on idle devices

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] ide: always set nIEN on idle devices

* Set nIEN for previous port/device in ide_do_request() also if port uses a non-shared IRQ.

* Remove no longer needed ide_hwif_t.sharing_irq.

---
 drivers/ide/ide-io.c    |    2 +-
 drivers/ide/ide-probe.c |    4 +---
 include/linux/ide.h     |    1 -
 3 files changed, 2 insertions(+), 5 deletions(-)
drivers/ide/ide-io.c +1 -1
Index: b/drivers/ide/ide-io.c
===================================================================
@@ -1009,7 +1009,7 @@ void do_ide_request(struct request_queue
 again:
 		hwif = drive->hwif;
 
-		if (hwgroup->hwif->sharing_irq && hwif != hwgroup->hwif) {
+		if (hwif != hwgroup->hwif) {
 			/*
 			 * set nIEN for previous hwif, drives in the
 			 * quirk_list may not like intr setups/cleanups
drivers/ide/ide-probe.c +1 -3
Index: b/drivers/ide/ide-probe.c
===================================================================
@@ -1059,7 +1059,6 @@ static int init_irq (ide_hwif_t *hwif)
 
 		if (h && h->hwgroup) {  /* scan only initialized ports */
 			if (hwif->irq == h->irq) {
-				hwif->sharing_irq = h->sharing_irq = 1;
 				if (hwif->chipset != ide_pci ||
 				    h->chipset != ide_pci) {
 					save_match(hwif, h, &match);
@@ -1152,8 +1151,7 @@ static int init_irq (ide_hwif_t *hwif)
 		io_ports->data_addr, hwif->irq);
 #endif /* __mc68000__ */
 	if (match)
-		printk(KERN_CONT " (%sed with %s)",
-			hwif->sharing_irq ? "shar" : "serializ", match->name);
+		printk(KERN_CONT " (serialized with %s)", match->name);
 	printk(KERN_CONT "\n");
 
 	mutex_unlock(&ide_cfg_mtx);
include/linux/ide.h +0 -1
Index: b/include/linux/ide.h
===================================================================
@@ -835,7 +835,6 @@ typedef struct hwif_s {
 
 	unsigned	present    : 1;	/* this interface exists */
 	unsigned	serialized : 1;	/* serialized all channel operation */
-	unsigned	sharing_irq: 1;	/* 1 = sharing irq with another hwif */
 	unsigned	sg_mapped  : 1;	/* sg_table and sg_nents are ready */
 
 	struct device		gendev;
#2 lore raw

PATCH 3/6 rz1000: apply chipset quirks early

In reply to Bartlomiej Zolnierkiewicz

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] rz1000: apply chipset quirks early

* Use pci_name(dev) instead of hwif->name in init_hwif_rz1000().

* init_hwif_rz1000() -> rz1000_init_chipset(). Update rz1000_init_one() to use rz1000_init_chipset() and add now required rz1000_remove().

* Remove superfluous ide_rz1000 chipset type.

---
 drivers/ide/ide-proc.c |    1 -
 drivers/ide/rz1000.c   |   36 +++++++++++++++++++++++++-----------
 include/linux/ide.h    |    3 +--
 3 files changed, 26 insertions(+), 14 deletions(-)
drivers/ide/ide-proc.c +0 -1
Index: b/drivers/ide/ide-proc.c
===================================================================
@@ -46,7 +46,6 @@ static int proc_ide_read_imodel
 	case ide_qd65xx:	name = "qd65xx";	break;
 	case ide_umc8672:	name = "umc8672";	break;
 	case ide_ht6560b:	name = "ht6560b";	break;
-	case ide_rz1000:	name = "rz1000";	break;
 	case ide_trm290:	name = "trm290";	break;
 	case ide_cy82c693:	name = "cy82c693";	break;
 	case ide_4drives:	name = "4drives";	break;
drivers/ide/rz1000.c +25 -11
Index: b/drivers/ide/rz1000.c
===================================================================
@@ -22,34 +22,48 @@
 
 #define DRV_NAME "rz1000"
 
-static void __devinit init_hwif_rz1000 (ide_hwif_t *hwif)
+static unsigned int __devinit rz1000_init_chipset(struct pci_dev *dev)
 {
-	struct pci_dev *dev = to_pci_dev(hwif->dev);
 	u16 reg;
 
 	if (!pci_read_config_word (dev, 0x40, &reg) &&
 	    !pci_write_config_word(dev, 0x40, reg & 0xdfff)) {
 		printk(KERN_INFO "%s: disabled chipset read-ahead "
-			"(buggy RZ1000/RZ1001)\n", hwif->name);
+			"(buggy RZ1000/RZ1001)\n", pci_name(dev));
+		return 0;
 	} else {
-		if (hwif->mate)
-			hwif->mate->serialized = hwif->serialized = 1;
-		hwif->host_flags |= IDE_HFLAG_NO_UNMASK_IRQS;
 		printk(KERN_INFO "%s: serialized, disabled unmasking "
-			"(buggy RZ1000/RZ1001)\n", hwif->name);
+			"(buggy RZ1000/RZ1001)\n", pci_name(dev));
+		return 1;
 	}
 }
 
 static const struct ide_port_info rz1000_chipset __devinitdata = {
 	.name		= DRV_NAME,
-	.init_hwif	= init_hwif_rz1000,
-	.chipset	= ide_rz1000,
 	.host_flags	= IDE_HFLAG_NO_DMA,
 };
 
 static int __devinit rz1000_init_one(struct pci_dev *dev, const struct pci_device_id *id)
 {
-	return ide_pci_init_one(dev, &rz1000_chipset, NULL);
+	struct ide_port_info d = rz1000_chipset;
+	int rc;
+
+	rc = pci_enable_device(dev);
+	if (rc)
+		return rc;
+
+	if (rz1000_init_chipset(dev)) {
+		d.host_flags |= IDE_HFLAG_SERIALIZE;
+		d.host_flags |= IDE_HFLAG_NO_UNMASK_IRQS;
+	}
+
+	return ide_pci_init_one(dev, &d, NULL);
+}
+
+static void rz1000_remove(struct pci_dev *dev)
+{
+	ide_pci_remove(dev);
+	pci_disable_device(dev);
 }
 
 static const struct pci_device_id rz1000_pci_tbl[] = {
@@ -63,7 +77,7 @@ static struct pci_driver rz1000_pci_driv
 	.name		= "RZ1000_IDE",
 	.id_table	= rz1000_pci_tbl,
 	.probe		= rz1000_init_one,
-	.remove		= ide_pci_remove,
+	.remove		= rz1000_remove,
 };
 
 static int __init rz1000_ide_init(void)
include/linux/ide.h +1 -2
Index: b/include/linux/ide.h
===================================================================
@@ -163,8 +163,7 @@ typedef int (ide_ack_intr_t)(struct hwif
 enum {		ide_unknown,	ide_generic,	ide_pci,
 		ide_cmd640,	ide_dtc2278,	ide_ali14xx,
 		ide_qd65xx,	ide_umc8672,	ide_ht6560b,
-		ide_rz1000,	ide_trm290,
-		ide_cy82c693,	ide_4drives,
+		ide_trm290,	ide_cy82c693,	ide_4drives,
 		ide_pmac,	ide_acorn,
 		ide_au1xxx,	ide_palm3710
 };
#3 lore raw

Re: [PATCH 3/6] rz1000: apply chipset quirks early

Hello.

Quoted 17 lines · Bartlomiej Zolnierkiewicz

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] rz1000: apply chipset quirks early

* Use pci_name(dev) instead of hwif->name in init_hwif_rz1000().

* init_hwif_rz1000() -> rz1000_init_chipset(). Update rz1000_init_one() to use rz1000_init_chipset() and add now required rz1000_remove().

* Remove superfluous ide_rz1000 chipset type.

Index: b/drivers/ide/rz1000.c
===================================================================

drivers/ide/rz1000.c line 25

Quoted 5 lines
drivers/ide/rz1000.c +1 -1
@@ -22,34 +22,48 @@
 
 #define DRV_NAME "rz1000"
 
-static void __devinit init_hwif_rz1000 (ide_hwif_t *hwif)
+static unsigned int __devinit rz1000_init_chipset(struct pci_dev *dev)

Hm, this is not a real init_chipset() method and it does just one thing --
disables readahead, so rz1000_disable_readahead() or something of that sort seems like a better name...

MBR, Sergei

#4 lore raw

Re: [PATCH 3/6] rz1000: apply chipset quirks early

Quoted 6 lines · Hello, I wrote:
Quoted 6 lines

Index: b/drivers/ide/rz1000.c
===================================================================

drivers/ide/rz1000.c +0 -0
@@ -22,34 +22,48 @@
 
Quoted 7 lines

drivers/ide/rz1000.c line 25

Quoted 4 lines
drivers/ide/rz1000.c +1 -1
 #define DRV_NAME "rz1000"
 
-static void __devinit init_hwif_rz1000 (ide_hwif_t *hwif)
+static unsigned int __devinit rz1000_init_chipset(struct pci_dev *dev)

Hm, this is not a real init_chipset() method and it does just one
thing -- disables readahead, so rz1000_disable_readahead() or something of that sort seems like a better name...

Hm, 'unsigned' result type wasn't terribly helpful here as well. :-)

MBR, Sergei

#5 lore raw

Re: [PATCH 3/6] rz1000: apply chipset quirks early

Quoted 10 lines · Sergei Shtylyov
Quoted 8 lines · Hello, I wrote:
Quoted 8 lines

Index: b/drivers/ide/rz1000.c
===================================================================

drivers/ide/rz1000.c +0 -0
@@ -22,34 +22,48 @@
 
 #define DRV_NAME "rz1000"
 
Quoted 8 lines
Quoted 6 lines

drivers/ide/rz1000.c line 25

Quoted 2 lines
drivers/ide/rz1000.c +1 -1
-static void __devinit init_hwif_rz1000 (ide_hwif_t *hwif)
+static unsigned int __devinit rz1000_init_chipset(struct pci_dev *dev)

Hm, this is not a real init_chipset() method and it does just one
thing -- disables readahead, so rz1000_disable_readahead() or something of that sort seems like a better name...

Hm, 'unsigned' result type wasn't terribly helpful here as well. :-)

Agreed, v1->v2 interdiff:

...
v2:
* unsigned int rz1000_init_chipset() -> int rz1000_disable_readahead() per Sergei's suggestion.

Cc: Sergei Shtylyov <[email protected]>
...

drivers/ide/rz1000.c +2 -2
diff -u b/drivers/ide/rz1000.c b/drivers/ide/rz1000.c
@@ -22,7 +22,7 @@
 
 #define DRV_NAME "rz1000"
 
-static unsigned int __devinit rz1000_init_chipset(struct pci_dev *dev)
+static int __devinit rz1000_disable_readahead(struct pci_dev *dev)
 {
 	u16 reg;
 
@@ -52,7 +52,7 @@
 	if (rc)
 		return rc;
 
-	if (rz1000_init_chipset(dev)) {
+	if (rz1000_disable_readahead(dev)) {
 		d.host_flags |= IDE_HFLAG_SERIALIZE;
 		d.host_flags |= IDE_HFLAG_NO_UNMASK_IRQS;
 	}
#6 lore raw

PATCH 4/6 trm290: add IDE_HFLAG_TRM290 host flag

In reply to Bartlomiej Zolnierkiewicz

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] trm290: add IDE_HFLAG_TRM290 host flag

* Add IDE_HFLAG_TRM290 host flag and use it in ide_build_dmatable().

* Remove no longer needed ide_trm290 chipset type.

---
 drivers/ide/ide-dma-sff.c |    2 +-
 drivers/ide/ide-proc.c    |    1 -
 drivers/ide/trm290.c      |    4 ++--
 include/linux/ide.h       |    4 +++-
 4 files changed, 6 insertions(+), 5 deletions(-)
drivers/ide/ide-dma-sff.c +1 -1
Index: b/drivers/ide/ide-dma-sff.c
===================================================================
@@ -98,10 +98,10 @@ int ide_build_dmatable(ide_drive_t *driv
 {
 	ide_hwif_t *hwif = drive->hwif;
 	__le32 *table = (__le32 *)hwif->dmatable_cpu;
-	unsigned int is_trm290	= (hwif->chipset == ide_trm290) ? 1 : 0;
 	unsigned int count = 0;
 	int i;
 	struct scatterlist *sg;
+	u8 is_trm290 = !!(hwif->host_flags & IDE_HFLAG_TRM290);
 
 	hwif->sg_nents = ide_build_sglist(drive, rq);
 	if (hwif->sg_nents == 0)
drivers/ide/ide-proc.c +0 -1
Index: b/drivers/ide/ide-proc.c
===================================================================
@@ -46,7 +46,6 @@ static int proc_ide_read_imodel
 	case ide_qd65xx:	name = "qd65xx";	break;
 	case ide_umc8672:	name = "umc8672";	break;
 	case ide_ht6560b:	name = "ht6560b";	break;
-	case ide_trm290:	name = "trm290";	break;
 	case ide_cy82c693:	name = "cy82c693";	break;
 	case ide_4drives:	name = "4drives";	break;
 	case ide_pmac:		name = "mac-io";	break;
drivers/ide/trm290.c +2 -2
Index: b/drivers/ide/trm290.c
===================================================================
@@ -328,10 +328,10 @@ static struct ide_dma_ops trm290_dma_ops
 static const struct ide_port_info trm290_chipset __devinitdata = {
 	.name		= DRV_NAME,
 	.init_hwif	= init_hwif_trm290,
-	.chipset	= ide_trm290,
 	.port_ops	= &trm290_port_ops,
 	.dma_ops	= &trm290_dma_ops,
-	.host_flags	= IDE_HFLAG_NO_ATAPI_DMA |
+	.host_flags	= IDE_HFLAG_TRM290 |
+			  IDE_HFLAG_NO_ATAPI_DMA |
 #if 0 /* play it safe for now */
 			  IDE_HFLAG_TRUST_BIOS_FOR_DMA |
 #endif
include/linux/ide.h +3 -1
Index: b/include/linux/ide.h
===================================================================
@@ -163,7 +163,7 @@ typedef int (ide_ack_intr_t)(struct hwif
 enum {		ide_unknown,	ide_generic,	ide_pci,
 		ide_cmd640,	ide_dtc2278,	ide_ali14xx,
 		ide_qd65xx,	ide_umc8672,	ide_ht6560b,
-		ide_trm290,	ide_cy82c693,	ide_4drives,
+		ide_cy82c693,	ide_4drives,
 		ide_pmac,	ide_acorn,
 		ide_au1xxx,	ide_palm3710
 };
@@ -1364,6 +1364,8 @@ enum {
 	IDE_HFLAG_IO_32BIT		= (1 << 24),
 	/* unmask IRQs */
 	IDE_HFLAG_UNMASK_IRQS		= (1 << 25),
+	/* host is TRM290 */
+	IDE_HFLAG_TRM290		= (1 << 26),
 	/* serialize ports if DMA is possible (for sl82c105) */
 	IDE_HFLAG_SERIALIZE_DMA		= (1 << 27),
 	/* force host out of "simplex" mode */
Acked-by on v1 4/6
#7 lore raw

Re: [PATCH 4/6] trm290: add IDE_HFLAG_TRM290 host flag

Quoted 4 lines · Bartlomiej Zolnierkiewicz

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] trm290: add IDE_HFLAG_TRM290 host flag

* Add IDE_HFLAG_TRM290 host flag and use it in ide_build_dmatable().

Actually, I've considered removing TRM290 hacks from that function and
re-implemeting the whole thing in the driver but that wouldn't sever this driver's ties with CONFIG_BLK_DEV_IDEDMA_SFF since the latter gets selected by CONFIG_BLK_DEV_IDEDMA_PCI...

Quoted 3 lines

* Remove no longer needed ide_trm290 chipset type.

drivers/ide/ide-dma-sff.c int ide_build_dmatable(ide_drive_t *driv · line 104

Quoted 13 lines

Index: b/drivers/ide/ide-dma-sff.c
===================================================================

drivers/ide/ide-dma-sff.c +1 -1
@@ -98,10 +98,10 @@ int ide_build_dmatable(ide_drive_t *driv
 {
 	ide_hwif_t *hwif = drive->hwif;
 	__le32 *table = (__le32 *)hwif->dmatable_cpu;
-	unsigned int is_trm290	= (hwif->chipset == ide_trm290) ? 1 : 0;
 	unsigned int count = 0;
 	int i;
 	struct scatterlist *sg;
+	u8 is_trm290 = !!(hwif->host_flags & IDE_HFLAG_TRM290);

I'd keep all the initilaized variables together...

MBR, Sergei

#8 lore raw

PATCH 5/6 cy82c693: remove superfluous ide_cy82c693 chipset type

In reply to Bartlomiej Zolnierkiewicz

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] cy82c693: remove superfluous ide_cy82c693 chipset type

Since CY82C693 doesn't require serialization we may as well use the default ide_pci chipset type.

---
 drivers/ide/cy82c693.c |    1 -
 drivers/ide/ide-proc.c |    1 -
 include/linux/ide.h    |    3 +--
 3 files changed, 1 insertion(+), 4 deletions(-)
drivers/ide/cy82c693.c +0 -1
Index: b/drivers/ide/cy82c693.c
===================================================================
@@ -292,7 +292,6 @@ static const struct ide_port_info cy82c6
 	.name		= DRV_NAME,
 	.init_iops	= init_iops_cy82c693,
 	.port_ops	= &cy82c693_port_ops,
-	.chipset	= ide_cy82c693,
 	.host_flags	= IDE_HFLAG_SINGLE,
 	.pio_mask	= ATA_PIO4,
 	.swdma_mask	= ATA_SWDMA2,
drivers/ide/ide-proc.c +0 -1
Index: b/drivers/ide/ide-proc.c
===================================================================
@@ -46,7 +46,6 @@ static int proc_ide_read_imodel
 	case ide_qd65xx:	name = "qd65xx";	break;
 	case ide_umc8672:	name = "umc8672";	break;
 	case ide_ht6560b:	name = "ht6560b";	break;
-	case ide_cy82c693:	name = "cy82c693";	break;
 	case ide_4drives:	name = "4drives";	break;
 	case ide_pmac:		name = "mac-io";	break;
 	case ide_au1xxx:	name = "au1xxx";	break;
include/linux/ide.h +1 -2
Index: b/include/linux/ide.h
===================================================================
@@ -163,8 +163,7 @@ typedef int (ide_ack_intr_t)(struct hwif
 enum {		ide_unknown,	ide_generic,	ide_pci,
 		ide_cmd640,	ide_dtc2278,	ide_ali14xx,
 		ide_qd65xx,	ide_umc8672,	ide_ht6560b,
-		ide_cy82c693,	ide_4drives,
-		ide_pmac,	ide_acorn,
+		ide_4drives,	ide_pmac,	ide_acorn,
 		ide_au1xxx,	ide_palm3710
 };
 
#9 lore raw

PATCH 6/6 ide: rework handling of serialized ports

In reply to Bartlomiej Zolnierkiewicz

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] ide: rework handling of serialized ports

* hpt366: set IDE_HFLAG_SERIALIZE in hwif->host_flags if needed in init_hwif_hpt366(). Remove HPT_SERIALIZE_IO while at it.

* Set IDE_HFLAG_SERIALIZE in hwif->host_flags if needed in ide_init_port().

* Convert init_irq() to use IDE_HFLAG_SERIALIZE together with hwif->host to find out ports which need to be serialized.

* Remove no longer needed save_match() and ide_hwif_t.serialized.

This patch should fix the incorrect grouping of port(s) from host(s) that need serialization with port(s) that happen to use the same IRQ(s) but are from the host(s) that don't need it.

---
 drivers/ide/hpt366.c    |    8 -------
 drivers/ide/ide-probe.c |   50 ++++--------------------------------------------
 include/linux/ide.h     |    1 
 3 files changed, 6 insertions(+), 53 deletions(-)
drivers/ide/hpt366.c +1 -7
Index: b/drivers/ide/hpt366.c
===================================================================
@@ -135,7 +135,6 @@
 /* various tuning parameters */
 #define HPT_RESET_STATE_ENGINE
 #undef	HPT_DELAY_INTERRUPT
-#define HPT_SERIALIZE_IO	0
 
 static const char *quirk_drives[] = {
 	"QUANTUM FIREBALLlct08 08",
@@ -1288,7 +1287,6 @@ static u8 hpt3xx_cable_detect(ide_hwif_t
 static void __devinit init_hwif_hpt366(ide_hwif_t *hwif)
 {
 	struct hpt_info *info	= hpt3xx_get_info(hwif->dev);
-	int serialize		= HPT_SERIALIZE_IO;
 	u8  chip_type		= info->chip_type;
 
 	/* Cache the channel's MISC. control registers' offset */
@@ -1305,13 +1303,9 @@ static void __devinit init_hwif_hpt366(i
 		 * Clock is shared between the channels,
 		 * so we'll have to serialize them... :-(
 		 */
-		serialize = 1;
+		hwif->host_flags |= IDE_HFLAG_SERIALIZE;
 		hwif->rw_disk = &hpt3xxn_rw_disk;
 	}
-
-	/* Serialize access to this device if needed */
-	if (serialize && hwif->mate)
-		hwif->serialized = hwif->mate->serialized = 1;
 }
 
 static int __devinit init_dma_hpt366(ide_hwif_t *hwif,
drivers/ide/ide-probe.c +5 -45

72 lines not on this page · raw message

include/linux/ide.h +0 -1
Index: b/include/linux/ide.h
===================================================================
@@ -832,7 +832,6 @@ typedef struct hwif_s {
 	unsigned	extra_ports;	/* number of extra dma ports */
 
 	unsigned	present    : 1;	/* this interface exists */
-	unsigned	serialized : 1;	/* serialized all channel operation */
 	unsigned	sg_mapped  : 1;	/* sg_table and sg_nents are ready */
 
 	struct device		gendev;
Acked-by on v1 6/6
#10 lore raw

Re: [PATCH 6/6] ide: rework handling of serialized ports

Quoted 12 lines · Bartlomiej Zolnierkiewicz

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] ide: rework handling of serialized ports

* hpt366: set IDE_HFLAG_SERIALIZE in hwif->host_flags if needed in init_hwif_hpt366(). Remove HPT_SERIALIZE_IO while at it.

* Set IDE_HFLAG_SERIALIZE in hwif->host_flags if needed in ide_init_port().

* Convert init_irq() to use IDE_HFLAG_SERIALIZE together with hwif->host to find out ports which need to be serialized.

Quoted 8 lines

* Remove no longer needed save_match() and ide_hwif_t.serialized.

This patch should fix the incorrect grouping of port(s) from host(s) that need serialization with port(s) that happen to use the same IRQ(s) but are from the host(s) that don't need it.

Quoted 5 lines

static int __devinit init_dma_hpt366(ide_hwif_t *hwif,
Index: b/drivers/ide/ide-probe.c
===================================================================

drivers/ide/ide-probe.c +0 -0

[...]

drivers/ide/ide-probe.c static int init_irq (ide_hwif_t *hwif) · line 1035

Quoted 3 lines
drivers/ide/ide-probe.c +3 -0
+			if (hwif->host_flags & IDE_HFLAG_SERIALIZE) {
+				if (hwif->host == h->host)
+					match = h;

Can be collapsed into a single *if*, and braces not needed either way.

drivers/ide/ide-probe.c static void ide_init_port(ide_hwif_t *hw · line 1402

Quoted 10 lines
drivers/ide/ide-probe.c +2 -4
@@ -1436,10 +1398,8 @@ static void ide_init_port(ide_hwif_t *hw
 	}
 
 	if ((d->host_flags & IDE_HFLAG_SERIALIZE) ||
-	    ((d->host_flags & IDE_HFLAG_SERIALIZE_DMA) && hwif->dma_base)) {
-		if (hwif->mate)
-			hwif->mate->serialized = hwif->serialized = 1;
-	}
+	    ((d->host_flags & IDE_HFLAG_SERIALIZE_DMA) && hwif->dma_base))
+		hwif->host_flags |= IDE_HFLAG_SERIALIZE;

I don't see what use is forcing this flag at all here -- it doesn't seem
to be checked only in init_irq() now, so this *if* could just be moved there...

MBR, Sergei

#11 lore raw

Re: [PATCH 6/6] ide: rework handling of serialized ports

Quoted 10 lines · Hello, I wrote:
Quoted 4 lines

Index: b/drivers/ide/ide-probe.c
===================================================================

drivers/ide/ide-probe.c +0 -0

[...]

Quoted 3 lines
drivers/ide/ide-probe.c +0 -0
@@ -1436,10 +1398,8 @@ static void ide_init_port(ide_hwif_t *hw
     }
 
Quoted 10 lines

drivers/ide/ide-probe.c static void ide_init_port(ide_hwif_t *hw · line 1402

Quoted 7 lines
drivers/ide/ide-probe.c +2 -4
     if ((d->host_flags & IDE_HFLAG_SERIALIZE) ||
-        ((d->host_flags & IDE_HFLAG_SERIALIZE_DMA) && hwif->dma_base)) {
-        if (hwif->mate)
-            hwif->mate->serialized = hwif->serialized = 1;
-    }
+        ((d->host_flags & IDE_HFLAG_SERIALIZE_DMA) && hwif->dma_base))
+        hwif->host_flags |= IDE_HFLAG_SERIALIZE;

I don't see what use is forcing this flag at all here -- it doesn't
seem to be checked only in init_irq() now, so this *if* could just be moved there...

Oops, rephrasing hurts sometimes: I meant "it seems", of course. :-)

MBR, Sergei

#12 lore raw

Re: [PATCH 6/6] ide: rework handling of serialized ports

Quoted 24 lines · Sergei Shtylyov
Quoted 10 lines · Hello, I wrote:
Quoted 4 lines

Index: b/drivers/ide/ide-probe.c
===================================================================

drivers/ide/ide-probe.c +0 -0

[...]

Quoted 3 lines
drivers/ide/ide-probe.c +0 -0
@@ -1436,10 +1398,8 @@ static void ide_init_port(ide_hwif_t *hw
     }
 
Quoted 11 lines

drivers/ide/ide-probe.c static void ide_init_port(ide_hwif_t *hw · line 1402

Quoted 7 lines
drivers/ide/ide-probe.c +2 -4
     if ((d->host_flags & IDE_HFLAG_SERIALIZE) ||
-        ((d->host_flags & IDE_HFLAG_SERIALIZE_DMA) && hwif->dma_base)) {
-        if (hwif->mate)
-            hwif->mate->serialized = hwif->serialized = 1;
-    }
+        ((d->host_flags & IDE_HFLAG_SERIALIZE_DMA) && hwif->dma_base))
+        hwif->host_flags |= IDE_HFLAG_SERIALIZE;

I don't see what use is forcing this flag at all here -- it doesn't
seem to be checked only in init_irq() now, so this *if* could just be moved there...

Quoted 1 line

Oops, rephrasing hurts sometimes: I meant "it seems", of course. :-)

I prefer to leave it in ide_init_port() because the future ide_hwgroup_t removal depends on this flag in deciding whether serialization is needed.

Actually it is better to force the flag on host instead of port so:

...
v2:
* Set host's ->host_flags field instead of port's copy.
...

drivers/ide/hpt366.c +1 -1
diff -u b/drivers/ide/hpt366.c b/drivers/ide/hpt366.c
@@ -1303,7 +1303,7 @@
 		 * Clock is shared between the channels,
 		 * so we'll have to serialize them... :-(
 		 */
-		hwif->host_flags |= IDE_HFLAG_SERIALIZE;
+		hwif->host->host_flags |= IDE_HFLAG_SERIALIZE;
 		hwif->rw_disk = &hpt3xxn_rw_disk;
 	}
 }
drivers/ide/ide-probe.c +2 -2
diff -u b/drivers/ide/ide-probe.c b/drivers/ide/ide-probe.c
@@ -1030,7 +1030,7 @@
 		ide_hwif_t *h = ide_ports[index];
 
 		if (h && h->hwgroup) {  /* scan only initialized ports */
-			if (hwif->host_flags & IDE_HFLAG_SERIALIZE) {
+			if (hwif->host->host_flags & IDE_HFLAG_SERIALIZE) {
 				if (hwif->host == h->host)
 					match = h;
 			}
@@ -1399,7 +1399,7 @@
 
 	if ((d->host_flags & IDE_HFLAG_SERIALIZE) ||
 	    ((d->host_flags & IDE_HFLAG_SERIALIZE_DMA) && hwif->dma_base))
-		hwif->host_flags |= IDE_HFLAG_SERIALIZE;
+		hwif->host->host_flags |= IDE_HFLAG_SERIALIZE;
 
 	if (d->host_flags & IDE_HFLAG_RQSIZE_256)
 		hwif->rqsize = 256;