Skip to contentSkip to conversation

ide: sanitize struct ide_port_ops documentation

PATCH In review Bartlomiej Zolnierkiewicz 3 participants 13 messages Jul 26, 2008 – Sep 21, 2008

Subsystems (2)
SCSI libata
lore
Share & tools
lore

Bartlomiej Zolnierkiewicz

author
#0 lore raw

PATCH 1/10 ide: sanitize struct ide_port_ops documentation

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>

---
some easy/trivial changes just to keep the ball rolling...

 include/linux/ide.h |   27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)
include/linux/ide.h +18 -9
Index: b/include/linux/ide.h
===================================================================
@@ -508,24 +508,33 @@ struct ide_tp_ops {
 
 extern const struct ide_tp_ops default_tp_ops;
 
+/**
+ * struct ide_port_ops - IDE port operations
+ *
+ * init_dev:		host specific initialization of a device
+ * set_pio_mode:	routine to program host for PIO mode
+ * set_dma_mode:	routine to program host for DMA mode
+ * selectproc:		tweaks hardware to select drive
+ * reset_poll:		chipset polling based on hba specifics
+ * pre_reset:		chipset specific changes to default for device-hba resets
+ * resetproc:		routine to reset controller after a disk reset
+ * maskproc:		special host masking for drive selection
+ * quirkproc:		check host's drive quirk list
+ *
+ * mdma_filter:		filter MDMA modes
+ * udma_filter:		filter UDMA modes
+ *
+ * cable_detect:	detect cable type
+ */
 struct ide_port_ops {
-	/* host specific initialization of a device */
 	void	(*init_dev)(ide_drive_t *);
-	/* routine to program host for PIO mode */
 	void	(*set_pio_mode)(ide_drive_t *, const u8);
-	/* routine to program host for DMA mode */
 	void	(*set_dma_mode)(ide_drive_t *, const u8);
-	/* tweaks hardware to select drive */
 	void	(*selectproc)(ide_drive_t *);
-	/* chipset polling based on hba specifics */
 	int	(*reset_poll)(ide_drive_t *);
-	/* chipset specific changes to default for device-hba resets */
 	void	(*pre_reset)(ide_drive_t *);
-	/* routine to reset controller after a disk reset */
 	void	(*resetproc)(ide_drive_t *);
-	/* special host masking for drive selection */
 	void	(*maskproc)(ide_drive_t *, int);
-	/* check host's drive quirk list */
 	void	(*quirkproc)(ide_drive_t *);
 
 	u8	(*mdma_filter)(ide_drive_t *);
author
#1 lore raw

PATCH 2/10 ide: fix EXABYTENEST handling in probe_for_drive()

Fix EXABYTENEST handling in probe_for_drive() (so drive->present is checked for "nested" device) and cleanup enable_nest().

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
---
 drivers/ide/ide-probe.c |   16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)
drivers/ide/ide-probe.c +7 -9
Index: b/drivers/ide/ide-probe.c
===================================================================
@@ -542,11 +542,6 @@ static void enable_nest (ide_drive_t *dr
 		printk(KERN_CONT "failed (status = 0x%02x)\n", stat);
 	else
 		printk(KERN_CONT "success\n");
-
-	/* if !(success||timed-out) */
-	if (do_probe(drive, ATA_CMD_ID_ATA) >= 2)
-		/* look for ATAPI device */
-		(void)do_probe(drive, ATA_CMD_ID_ATAPI);
 }
 
 /**

@@ -586,19 +581,22 @@ static inline u8 probe_for_drive (ide_dr
 	strcpy(m, "UNKNOWN");
 
 	/* skip probing? */
-	if (!drive->noprobe)
-	{
+	if (!drive->noprobe) {
+retry:
 		/* if !(success||timed-out) */
 		if (do_probe(drive, ATA_CMD_ID_ATA) >= 2)
 			/* look for ATAPI device */
 			(void)do_probe(drive, ATA_CMD_ID_ATAPI);
+
 		if (!drive->present)
 			/* drive not found */
 			return 0;
 
-		if (strstr(m, "E X A B Y T E N E S T"))
+		if (strstr(m, "E X A B Y T E N E S T")) {
 			enable_nest(drive);
-	
+			goto retry;
+		}
+
 		/* identification failed? */
 		if (!drive->id_read) {
 			if (drive->media == ide_disk) {
author
#2 lore raw

PATCH 3/10 ide: enhance ide_busy_sleep()

In reply to Bartlomiej Zolnierkiewicz

* Make ide_busy_sleep() take timeout value as a parameter and also allow use of AltStatus Register if requested with altstatus parameter. Update existing users accordingly.

* Convert ide_driveid_update() and actual_try_to_identify() to use ide_busy_sleep().

There should be no functional changes caused by this patch.

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
---
 drivers/ide/ide-iops.c  |   15 +++++----------
 drivers/ide/ide-probe.c |   30 ++++++++++++------------------
 include/linux/ide.h     |    2 ++
 3 files changed, 19 insertions(+), 28 deletions(-)
drivers/ide/ide-iops.c +5 -10
Index: b/drivers/ide/ide-iops.c
===================================================================
@@ -666,7 +666,7 @@ int ide_driveid_update(ide_drive_t *driv
 	ide_hwif_t *hwif = drive->hwif;
 	const struct ide_tp_ops *tp_ops = hwif->tp_ops;
 	u16 *id;
-	unsigned long timeout, flags;
+	unsigned long flags;
 	u8 stat;
 
 	/*

@@ -678,16 +678,11 @@ int ide_driveid_update(ide_drive_t *driv
 	tp_ops->set_irq(hwif, 0);
 	msleep(50);
 	tp_ops->exec_command(hwif, ATA_CMD_ID_ATA);
-	timeout = jiffies + WAIT_WORSTCASE;
-	do {
-		if (time_after(jiffies, timeout)) {
-			SELECT_MASK(drive, 0);
-			return 0;	/* drive timed-out */
-		}
 
-		msleep(50);	/* give drive a breather */
-		stat = tp_ops->read_altstatus(hwif);
-	} while (stat & ATA_BUSY);
+	if (ide_busy_sleep(hwif, WAIT_WORSTCASE, 1)) {
+		SELECT_MASK(drive, 0);
+		return 0;
+	}
 
 	msleep(50);	/* wait for IRQ and ATA_DRQ */
 	stat = tp_ops->read_status(hwif);
drivers/ide/ide-probe.c +12 -18
Index: b/drivers/ide/ide-probe.c
===================================================================
@@ -291,17 +291,9 @@ static int actual_try_to_identify (ide_d
 	tp_ops->exec_command(hwif, cmd);
 
 	timeout = ((cmd == ATA_CMD_ID_ATA) ? WAIT_WORSTCASE : WAIT_PIDENTIFY) / 2;
-	timeout += jiffies;
-	do {
-		if (time_after(jiffies, timeout)) {
-			/* drive timed-out */
-			return 1;
-		}
-		/* give drive a breather */
-		msleep(50);
-		s = use_altstatus ? tp_ops->read_altstatus(hwif)
-				  : tp_ops->read_status(hwif);
-	} while (s & ATA_BUSY);
+
+	if (ide_busy_sleep(hwif, timeout, use_altstatus))
+		return 1;
 
 	/* wait for IRQ and ATA_DRQ */
 	msleep(50);

@@ -383,19 +375,21 @@ static int try_to_identify (ide_drive_t 
 	return retval;
 }
 
-static int ide_busy_sleep(ide_hwif_t *hwif)
+int ide_busy_sleep(ide_hwif_t *hwif, unsigned long timeout, int altstatus)
 {
-	unsigned long timeout = jiffies + WAIT_WORSTCASE;
 	u8 stat;
 
+	timeout += jiffies;
+
 	do {
-		msleep(50);
-		stat = hwif->tp_ops->read_status(hwif);
+		msleep(50);	/* give drive a breather */
+		stat = altstatus ? hwif->tp_ops->read_altstatus(hwif)
+				 : hwif->tp_ops->read_status(hwif);
 		if ((stat & ATA_BUSY) == 0)
 			return 0;
 	} while (time_before(jiffies, timeout));
 
-	return 1;
+	return 1;	/* drive timed-out */
 }
 
 static u8 ide_read_device(ide_drive_t *drive)

@@ -489,7 +483,7 @@ static int do_probe (ide_drive_t *drive,
 			SELECT_DRIVE(drive);
 			msleep(50);
 			tp_ops->exec_command(hwif, ATA_CMD_DEV_RESET);
-			(void)ide_busy_sleep(hwif);
+			(void)ide_busy_sleep(hwif, WAIT_WORSTCASE, 0);
 			rc = try_to_identify(drive, cmd);
 		}
 

@@ -529,7 +523,7 @@ static void enable_nest (ide_drive_t *dr
 	msleep(50);
 	tp_ops->exec_command(hwif, ATA_EXABYTE_ENABLE_NEST);
 
-	if (ide_busy_sleep(hwif)) {
+	if (ide_busy_sleep(hwif, WAIT_WORSTCASE, 0)) {
 		printk(KERN_CONT "failed (timeout)\n");
 		return;
 	}
include/linux/ide.h +2 -0
Index: b/include/linux/ide.h
===================================================================
@@ -932,6 +932,8 @@ void ide_fix_driveid(u16 *);
 
 extern void ide_fixstring(u8 *, const int, const int);
 
+int ide_busy_sleep(ide_hwif_t *, unsigned long, int);
+
 int ide_wait_stat(ide_startstop_t *, ide_drive_t *, u8, u8, unsigned long);
 
 extern ide_startstop_t ide_do_reset (ide_drive_t *);
author
#3 lore raw

PATCH 4/10 ide: remove no longer needed BUG_ON()-s from init_irq()

In reply to Bartlomiej Zolnierkiewicz

init_irq() is now called only during initial host registration so these BUG_ON()-s are no loner needed (+ the last one was done too late anyway).

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
---
 drivers/ide/ide-probe.c |    5 -----
 1 file changed, 5 deletions(-)
drivers/ide/ide-probe.c +0 -5
Index: b/drivers/ide/ide-probe.c
===================================================================
@@ -1031,11 +1031,6 @@ static int init_irq (ide_hwif_t *hwif)
 	ide_hwgroup_t *hwgroup;
 	ide_hwif_t *match = NULL;
 
-
-	BUG_ON(in_interrupt());
-	BUG_ON(irqs_disabled());	
-	BUG_ON(hwif == NULL);
-
 	mutex_lock(&ide_cfg_mtx);
 	hwif->hwgroup = NULL;
 #if MAX_HWIFS > 1
author
#4 lore raw

PATCH 5/10 ide: remove IDE_CHIPSET_* macros

In reply to Bartlomiej Zolnierkiewicz

They just obfuscate the code.

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
---
 drivers/ide/ide-probe.c |    3 ++-
 include/linux/ide.h     |    4 ----
 2 files changed, 2 insertions(+), 5 deletions(-)
drivers/ide/ide-probe.c +2 -1
Index: b/drivers/ide/ide-probe.c
===================================================================
@@ -1105,7 +1105,8 @@ static int init_irq (ide_hwif_t *hwif)
 		sa = IRQF_SHARED;
 #endif /* __mc68000__ */
 
-		if (IDE_CHIPSET_IS_PCI(hwif->chipset))
+		if (hwif->chipset == ide_pci || hwif->chipset == ide_cmd646 ||
+		    hwif->chipset == ide_ali14xx)
 			sa = IRQF_SHARED;
 
 		if (io_ports->ctl_addr)
include/linux/ide.h +0 -4
Index: b/include/linux/ide.h
===================================================================
@@ -482,10 +482,6 @@ typedef struct ide_drive_s ide_drive_t;
 
 #define to_ide_device(dev)container_of(dev, ide_drive_t, gendev)
 
-#define IDE_CHIPSET_PCI_MASK	\
-    ((1<<ide_pci)|(1<<ide_cmd646)|(1<<ide_ali14xx))
-#define IDE_CHIPSET_IS_PCI(c)	((IDE_CHIPSET_PCI_MASK >> (c)) & 1)
-
 struct ide_task_s;
 struct ide_port_info;
author
#5 lore raw

PATCH 6/10 ide: remove unused _IDE_C and _IDE_DISK defines

In reply to Bartlomiej Zolnierkiewicz
Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
---
 drivers/ide/ide-disk.c |    3 ---
 drivers/ide/ide.c      |    2 --
 2 files changed, 5 deletions(-)
drivers/ide/ide-disk.c +0 -3
Index: b/drivers/ide/ide-disk.c
===================================================================
@@ -30,9 +30,6 @@
 #include <linux/delay.h>
 #include <linux/mutex.h>
 #include <linux/leds.h>
-
-#define _IDE_DISK
-
 #include <linux/ide.h>
 
 #include <asm/byteorder.h>
drivers/ide/ide.c +0 -2
Index: b/drivers/ide/ide.c
===================================================================
@@ -44,8 +44,6 @@
  *  inspiration from lots of linux users, esp.  [email protected]
  */
 
-#define _IDE_C			/* Tell ide.h it's really us */
-
 #include <linux/module.h>
 #include <linux/types.h>
 #include <linux/string.h>
author
#6 lore raw

PATCH 7/10 ide: remove needless drive->present checks from device drivers

In reply to Bartlomiej Zolnierkiewicz

Remove needless drive->present checks from ->probe methods (device model takes care of that).

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
---
 drivers/ide/ide-cd.c     |    4 ++--
 drivers/ide/ide-disk.c   |    3 +--
 drivers/ide/ide-floppy.c |    4 ++--
 drivers/ide/ide-tape.c   |    4 ++--
 drivers/scsi/ide-scsi.c  |    1 -
 5 files changed, 7 insertions(+), 9 deletions(-)
drivers/ide/ide-cd.c +2 -2
Index: b/drivers/ide/ide-cd.c
===================================================================
@@ -2105,10 +2105,10 @@ static int ide_cd_probe(ide_drive_t *dri
 
 	if (!strstr("ide-cdrom", drive->driver_req))
 		goto failed;
-	if (!drive->present)
-		goto failed;
+
 	if (drive->media != ide_cdrom && drive->media != ide_optical)
 		goto failed;
+
 	/* skip drives that we were told to ignore */
 	if (ignore != NULL) {
 		if (strstr(ignore, drive->name)) {
drivers/ide/ide-disk.c +1 -2
Index: b/drivers/ide/ide-disk.c
===================================================================
@@ -1143,8 +1143,7 @@ static int ide_disk_probe(ide_drive_t *d
 	/* strstr("foo", "") is non-NULL */
 	if (!strstr("ide-disk", drive->driver_req))
 		goto failed;
-	if (!drive->present)
-		goto failed;
+
 	if (drive->media != ide_disk)
 		goto failed;
drivers/ide/ide-floppy.c +2 -2
Index: b/drivers/ide/ide-floppy.c
===================================================================
@@ -1389,10 +1389,10 @@ static int ide_floppy_probe(ide_drive_t 
 
 	if (!strstr("ide-floppy", drive->driver_req))
 		goto failed;
-	if (!drive->present)
-		goto failed;
+
 	if (drive->media != ide_floppy)
 		goto failed;
+
 	if (!idefloppy_identify_device(drive, drive->id)) {
 		printk(KERN_ERR "ide-floppy: %s: not supported by this version"
 				" of ide-floppy\n", drive->name);
drivers/ide/ide-tape.c +2 -2
Index: b/drivers/ide/ide-tape.c
===================================================================
@@ -2645,10 +2645,10 @@ static int ide_tape_probe(ide_drive_t *d
 
 	if (!strstr("ide-tape", drive->driver_req))
 		goto failed;
-	if (!drive->present)
-		goto failed;
+
 	if (drive->media != ide_tape)
 		goto failed;
+
 	if (!idetape_identify_device(drive)) {
 		printk(KERN_ERR "ide-tape: %s: not supported by this version of"
 				" the driver\n", drive->name);
drivers/scsi/ide-scsi.c +0 -1
Index: b/drivers/scsi/ide-scsi.c
===================================================================
@@ -821,7 +821,6 @@ static int ide_scsi_probe(ide_drive_t *d
 		return -ENODEV;
 
 	if (!strstr("ide-scsi", drive->driver_req) ||
-	    !drive->present ||
 	    drive->media == ide_disk ||
 	    !(host = scsi_host_alloc(&idescsi_template,sizeof(idescsi_scsi_t))))
 		return -ENODEV;
author
#7 lore raw

PATCH 8/10 ide: check drive->present in ide_get_paired_drive()

In reply to Bartlomiej Zolnierkiewicz

* Change ide_get_paired_drive() to return NULL if peer device is not present and update all users accordingly.

While at it:

* ide_get_paired_drive() -> ide_get_pair_dev()

* Use ide_get_pair_dev() in cs5530.c, sc1200.c and via82cxxx.c.

There should be no functional changes caused by this patch.

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
---
 drivers/ide/arm/palm_bk3710.c |    4 ++--
 drivers/ide/pci/cs5530.c      |    4 ++--
 drivers/ide/pci/cs5535.c      |    4 ++--
 drivers/ide/pci/opti621.c     |    4 ++--
 drivers/ide/pci/sc1200.c      |    4 ++--
 drivers/ide/pci/siimage.c     |    4 ++--
 drivers/ide/pci/via82cxxx.c   |    4 ++--
 include/linux/ide.h           |    6 +++---
 8 files changed, 17 insertions(+), 17 deletions(-)
drivers/ide/arm/palm_bk3710.c +2 -2
Index: b/drivers/ide/arm/palm_bk3710.c
===================================================================
@@ -180,7 +180,7 @@ static void palm_bk3710_setpiomode(void 
 	val32 |= (t2i << (dev ? 8 : 0));
 	writel(val32, base + BK3710_DATRCVR);
 
-	if (mate && mate->present) {
+	if (mate) {
 		u8 mode2 = ide_get_best_pio_mode(mate, 255, 4);
 
 		if (mode2 < mode)

@@ -230,7 +230,7 @@ static void palm_bk3710_set_pio_mode(ide
 	 * Obtain the drive PIO data for tuning the Palm Chip registers
 	 */
 	cycle_time = ide_pio_cycle_time(drive, pio);
-	mate = ide_get_paired_drive(drive);
+	mate = ide_get_pair_dev(drive);
 	palm_bk3710_setpiomode(base, mate, is_slave, cycle_time, pio);
 }
drivers/ide/pci/cs5530.c +2 -2
Index: b/drivers/ide/pci/cs5530.c
===================================================================
@@ -81,11 +81,11 @@ static void cs5530_set_pio_mode(ide_driv
 static u8 cs5530_udma_filter(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif = drive->hwif;
-	ide_drive_t *mate = &hwif->drives[(drive->dn & 1) ^ 1];
+	ide_drive_t *mate = ide_get_pair_dev(drive);
 	u16 *mateid = mate->id;
 	u8 mask = hwif->ultra_mask;
 
-	if (mate->present == 0)
+	if (mate == NULL)
 		goto out;
 
 	if (ata_id_has_dma(mateid) && __ide_dma_bad_drive(mate) == 0) {
drivers/ide/pci/cs5535.c +2 -2
Index: b/drivers/ide/pci/cs5535.c
===================================================================
@@ -80,12 +80,12 @@ static void cs5535_set_speed(ide_drive_t
 
 	/* Set the PIO timings */
 	if (speed < XFER_SW_DMA_0) {
-		ide_drive_t *pair = ide_get_paired_drive(drive);
+		ide_drive_t *pair = ide_get_pair_dev(drive);
 		u8 cmd, pioa;
 
 		cmd = pioa = speed - XFER_PIO_0;
 
-		if (pair->present) {
+		if (pair) {
 			u8 piob = ide_get_best_pio_mode(pair, 255, 4);
 
 			if (piob < cmd)
drivers/ide/pci/opti621.c +2 -2
Index: b/drivers/ide/pci/opti621.c
===================================================================
@@ -137,7 +137,7 @@ static u8 read_reg(int reg)
 static void opti621_set_pio_mode(ide_drive_t *drive, const u8 pio)
 {
 	ide_hwif_t *hwif = drive->hwif;
-	ide_drive_t *pair = ide_get_paired_drive(drive);
+	ide_drive_t *pair = ide_get_pair_dev(drive);
 	unsigned long flags;
 	u8 tim, misc, addr_pio = pio, clk;
 

@@ -153,7 +153,7 @@ static void opti621_set_pio_mode(ide_dri
 
 	drive->drive_data = XFER_PIO_0 + pio;
 
-	if (pair->present) {
+	if (pair) {
 		if (pair->drive_data && pair->drive_data < drive->drive_data)
 			addr_pio = pair->drive_data - XFER_PIO_0;
 	}
drivers/ide/pci/sc1200.c +2 -2
Index: b/drivers/ide/pci/sc1200.c
===================================================================
@@ -104,11 +104,11 @@ static void sc1200_tunepio(ide_drive_t *
 static u8 sc1200_udma_filter(ide_drive_t *drive)
 {
 	ide_hwif_t *hwif = drive->hwif;
-	ide_drive_t *mate = &hwif->drives[(drive->dn & 1) ^ 1];
+	ide_drive_t *mate = ide_get_pair_dev(drive);
 	u16 *mateid = mate->id;
 	u8 mask = hwif->ultra_mask;
 
-	if (mate->present == 0)
+	if (mate == NULL)
 		goto out;
 
 	if (ata_id_has_dma(mateid) && __ide_dma_bad_drive(mate) == 0) {
drivers/ide/pci/siimage.c +2 -2
Index: b/drivers/ide/pci/siimage.c
===================================================================
@@ -245,7 +245,7 @@ static void sil_set_pio_mode(ide_drive_t
 
 	ide_hwif_t *hwif	= HWIF(drive);
 	struct pci_dev *dev	= to_pci_dev(hwif->dev);
-	ide_drive_t *pair	= ide_get_paired_drive(drive);
+	ide_drive_t *pair	= ide_get_pair_dev(drive);
 	u32 speedt		= 0;
 	u16 speedp		= 0;
 	unsigned long addr	= siimage_seldev(drive, 0x04);

@@ -259,7 +259,7 @@ static void sil_set_pio_mode(ide_drive_t
 	u8 unit			= drive->select.b.unit;
 
 	/* trim *taskfile* PIO to the slowest of the master/slave */
-	if (pair->present) {
+	if (pair) {
 		u8 pair_pio = ide_get_best_pio_mode(pair, 255, 4);
 
 		if (pair_pio < tf_pio)
drivers/ide/pci/via82cxxx.c +2 -2
Index: b/drivers/ide/pci/via82cxxx.c
===================================================================
@@ -154,7 +154,7 @@ static void via_set_speed(ide_hwif_t *hw
 static void via_set_drive(ide_drive_t *drive, const u8 speed)
 {
 	ide_hwif_t *hwif = drive->hwif;
-	ide_drive_t *peer = hwif->drives + (~drive->dn & 1);
+	ide_drive_t *peer = ide_get_pair_dev(drive);
 	struct pci_dev *dev = to_pci_dev(hwif->dev);
 	struct ide_host *host = pci_get_drvdata(dev);
 	struct via82cxxx_dev *vdev = host->host_priv;

@@ -173,7 +173,7 @@ static void via_set_drive(ide_drive_t *d
 
 	ide_timing_compute(drive, speed, &t, T, UT);
 
-	if (peer->present) {
+	if (peer) {
 		ide_timing_compute(peer, peer->current_speed, &p, T, UT);
 		ide_timing_merge(&p, &t, &t, IDE_TIMING_8BIT);
 	}
include/linux/ide.h +3 -3
Index: b/include/linux/ide.h
===================================================================
@@ -1460,10 +1460,10 @@ static inline int hwif_to_node(ide_hwif_
 	return hwif->dev ? pcibus_to_node(dev->bus) : -1;
 }
 
-static inline ide_drive_t *ide_get_paired_drive(ide_drive_t *drive)
+static inline ide_drive_t *ide_get_pair_dev(ide_drive_t *drive)
 {
-	ide_hwif_t *hwif	= HWIF(drive);
+	ide_drive_t *peer = &drive->hwif->drives[(drive->dn ^ 1) & 1];
 
-	return &hwif->drives[(drive->dn ^ 1) & 1];
+	return peer->present ? peer : NULL;
 }
 #endif /* _IDE_H */
Acked-by on v1 8/10
#8 lore raw

Re: [PATCH 08/10] ide: check drive->present in ide_get_paired_drive()

Hello.

Quoted 7 lines · Bartlomiej Zolnierkiewicz

* Change ide_get_paired_drive() to return NULL if peer device is not present and update all users accordingly.

While at it:

* ide_get_paired_drive() -> ide_get_pair_dev()

Hm, ide_get_mate_dev() appeals to me more.

Quoted 6 lines

* Use ide_get_pair_dev() in cs5530.c, sc1200.c and via82cxxx.c.

There should be no functional changes caused by this patch.

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
Acked-by Sergei Shtylyov <[email protected]>
Quoted 6 lines

Index: b/drivers/ide/pci/cs5530.c
===================================================================

drivers/ide/pci/cs5530.c +0 -0
@@ -81,11 +81,11 @@ static void cs5530_set_pio_mode(ide_driv
 static u8 cs5530_udma_filter(ide_drive_t *drive)

drivers/ide/pci/cs5530.c static void cs5530_set_pio_mode(ide_driv · line 89

Quoted 10 lines
drivers/ide/pci/cs5530.c +2 -2
 {
 	ide_hwif_t *hwif = drive->hwif;
-	ide_drive_t *mate = &hwif->drives[(drive->dn & 1) ^ 1];
+	ide_drive_t *mate = ide_get_pair_dev(drive);
 	u16 *mateid = mate->id;
 	u8 mask = hwif->ultra_mask;
 
-	if (mate->present == 0)
+	if (mate == NULL)

Hum... no == in some places and == isn't very consistent...

MBR, Sergei

author
#9 lore raw

PATCH 9/10 ide: use correct data phase for SMART READ DATA / LOG in ide_cmd_ioctl()

In reply to Bartlomiej Zolnierkiewicz
Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
---
 drivers/ide/ide-taskfile.c |   14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)
drivers/ide/ide-taskfile.c +12 -2
Index: b/drivers/ide/ide-taskfile.c
===================================================================
@@ -758,15 +758,25 @@ int ide_cmd_ioctl (ide_drive_t *drive, u
 		tf->lbam  = 0x4f;
 		tf->lbah  = 0xc2;
 		tfargs.tf_flags = IDE_TFLAG_OUT_TF | IDE_TFLAG_IN_NSECT;
+
+		/* SMART READ DATA / LOG */
+		if (tf->feature == 0xD0 || tf->feature == 0xD5)
+			tfargs.data_phase = TASKFILE_IN;
+		else
+			tfargs.data_phase = TASKFILE_NO_DATA;
 	} else {
 		tf->nsect = args[1];
 		tfargs.tf_flags = IDE_TFLAG_OUT_FEATURE |
 				  IDE_TFLAG_OUT_NSECT | IDE_TFLAG_IN_NSECT;
+
+		if (args[3])
+			tfargs.data_phase = TASKFILE_IN;
+		else
+			tfargs.data_phase = TASKFILE_NO_DATA;
 	}
 	tf->command = args[0];
-	tfargs.data_phase = args[3] ? TASKFILE_IN : TASKFILE_NO_DATA;
 
-	if (args[3]) {
+	if (tfargs.data_phase == TASKFILE_IN) {
 		tfargs.tf_flags |= IDE_TFLAG_IO_16BIT;
 		bufsize = SECTOR_WORDS * 4 * args[3];
 		buf = kzalloc(bufsize, GFP_KERNEL);
author
#10 lore raw

PATCH 10/10 ide: remove CONFIG_IDEDISK_MULTI_MODE

In reply to Bartlomiej Zolnierkiewicz

Use multi PIO by default when available and remove no longer needed CONFIG_IDEDISK_MULTI_MODE (it should be safe nowadays, despite to what help entry has been saying).

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
---
 drivers/ide/Kconfig     |   23 -----------------------
 drivers/ide/ide-probe.c |    6 ++----
 2 files changed, 2 insertions(+), 27 deletions(-)
drivers/ide/Kconfig +0 -23
Index: b/drivers/ide/Kconfig
===================================================================
@@ -141,29 +141,6 @@ config BLK_DEV_IDEDISK
 
 	  If unsure, say Y.
 
-config IDEDISK_MULTI_MODE
-	bool "Use multiple sector mode for Programmed Input/Output by default"
-	help
-	  This setting is irrelevant for most IDE disks, with direct memory
-	  access, to which multiple sector mode does not apply. Multiple sector
-	  mode is a feature of most modern IDE hard drives, permitting the
-	  transfer of multiple sectors per Programmed Input/Output interrupt,
-	  rather than the usual one sector per interrupt. When this feature is
-	  enabled, it can reduce operating system overhead for disk Programmed
-	  Input/Output. On some systems, it also can increase the data
-	  throughput of Programmed Input/Output. Some drives, however, seemed
-	  to run slower with multiple sector mode enabled. Some drives claimed
-	  to support multiple sector mode, but lost data at some settings.
-	  Under rare circumstances, such failures could result in massive
-	  filesystem corruption.
-
-	  If you get the following error, try to say Y here:
-
-	  hda: set_multmode: status=0x51 { DriveReady SeekComplete Error }
-	  hda: set_multmode: error=0x04 { DriveStatusError }
-
-	  If in doubt, say N.
-
 config BLK_DEV_IDECS
 	tristate "PCMCIA IDE support"
 	depends on PCMCIA
drivers/ide/ide-probe.c +2 -4
Index: b/drivers/ide/ide-probe.c
===================================================================
@@ -89,16 +89,14 @@ static void ide_disk_init_mult_count(ide
 	u8 max_multsect = id[ATA_ID_MAX_MULTSECT] & 0xff;
 
 	if (max_multsect) {
-#ifdef CONFIG_IDEDISK_MULTI_MODE
 		if ((max_multsect / 2) > 1)
 			id[ATA_ID_MULTSECT] = max_multsect | 0x100;
 		else
 			id[ATA_ID_MULTSECT] &= ~0x1ff;
 
 		drive->mult_req = id[ATA_ID_MULTSECT] & 0xff;
-#endif
-		if ((id[ATA_ID_MULTSECT] & 0x100) &&
-		    (id[ATA_ID_MULTSECT] & 0xff))
+
+		if (drive->mult_req)
 			drive->special.b.set_multmode = 1;
 	}
 }
#11 lore raw
In reply to Bartlomiej Zolnierkiewicz
Quoted 12 lines · On Sat, 26 Jul 2008 15:40:10 +0200 Bartlomiej Zolnierkiewicz wrote:
Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>

---
some easy/trivial changes just to keep the ball rolling...

 include/linux/ide.h |   27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

Index: b/include/linux/ide.h
===================================================================

include/linux/ide.h struct ide_tp_ops { · line 514

Quoted 7 lines
include/linux/ide.h +4 -0
@@ -508,24 +508,33 @@ struct ide_tp_ops {
 
 extern const struct ide_tp_ops default_tp_ops;
 
+/**
+ * struct ide_port_ops - IDE port operations
+ *
+ * init_dev:		host specific initialization of a device

Good idea, Bart, but please put an '@' in front of each field name, e.g.,

* @init_dev: host specific ...

See Documentation/kernel-doc-nano-HOWTO.txt for details (or ask).

Quoted 26 lines
include/linux/ide.h +14 -6
+ * set_pio_mode:	routine to program host for PIO mode
+ * set_dma_mode:	routine to program host for DMA mode
+ * selectproc:		tweaks hardware to select drive
+ * reset_poll:		chipset polling based on hba specifics
+ * pre_reset:		chipset specific changes to default for device-hba resets
+ * resetproc:		routine to reset controller after a disk reset
+ * maskproc:		special host masking for drive selection
+ * quirkproc:		check host's drive quirk list
+ *
+ * mdma_filter:		filter MDMA modes
+ * udma_filter:		filter UDMA modes
+ *
+ * cable_detect:	detect cable type
+ */
 struct ide_port_ops {
-	/* host specific initialization of a device */
 	void	(*init_dev)(ide_drive_t *);
-	/* routine to program host for PIO mode */
 	void	(*set_pio_mode)(ide_drive_t *, const u8);
-	/* routine to program host for DMA mode */
 	void	(*set_dma_mode)(ide_drive_t *, const u8);
-	/* tweaks hardware to select drive */
 	void	(*selectproc)(ide_drive_t *);
-	/* chipset polling based on hba specifics */
 	int	(*reset_poll)(ide_drive_t *);
-	/* chipset specific changes to default for device-hba resets */

include/linux/ide.h struct ide_tp_ops {

Quoted 10 lines
include/linux/ide.h +0 -4
 	void	(*pre_reset)(ide_drive_t *);
-	/* routine to reset controller after a disk reset */
 	void	(*resetproc)(ide_drive_t *);
-	/* special host masking for drive selection */
 	void	(*maskproc)(ide_drive_t *, int);
-	/* check host's drive quirk list */
 	void	(*quirkproc)(ide_drive_t *);
 
 	u8	(*mdma_filter)(ide_drive_t *);
--

---
~Randy
Linux Plumbers Conference, 17-19 September 2008, Portland, Oregon USA
http://linuxplumbersconf.org/

author
#12 lore raw
Quoted 17 lines · Randy Dunlap
Quoted 15 lines · On Sat, 26 Jul 2008 15:40:10 +0200 Bartlomiej Zolnierkiewicz wrote:
Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>

---
some easy/trivial changes just to keep the ball rolling...

 include/linux/ide.h |   27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

Index: b/include/linux/ide.h
===================================================================

include/linux/ide.h +0 -0
@@ -508,24 +508,33 @@ struct ide_tp_ops {
 
 extern const struct ide_tp_ops default_tp_ops;
Quoted 8 lines

include/linux/ide.h struct ide_tp_ops { · line 514

Quoted 4 lines
include/linux/ide.h +4 -0
+/**
+ * struct ide_port_ops - IDE port operations
+ *
+ * init_dev:		host specific initialization of a device

Good idea, Bart, but please put an '@' in front of each field name, e.g.,

* @init_dev: host specific ...

Doh, I knew I was forgetting something. Fixed.

Quoted 1 line

See Documentation/kernel-doc-nano-HOWTO.txt for details (or ask).

Thanks for the pointer.

From: Bartlomiej Zolnierkiewicz <[email protected]>
Subject: [PATCH] ide: sanitize struct ide_port_ops documentation (take 2)

v2:
Add missing '@'-s. (Noticed by Randy Dunlap)

Cc Randy Dunlap <[email protected]>
Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
---
 include/linux/ide.h |   27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)
include/linux/ide.h +18 -9
Index: b/include/linux/ide.h
===================================================================
@@ -508,24 +508,33 @@ struct ide_tp_ops {
 
 extern const struct ide_tp_ops default_tp_ops;
 
+/**
+ * struct ide_port_ops - IDE port operations
+ *
+ * @init_dev:		host specific initialization of a device
+ * @set_pio_mode:	routine to program host for PIO mode
+ * @set_dma_mode:	routine to program host for DMA mode
+ * @selectproc:		tweaks hardware to select drive
+ * @reset_poll:		chipset polling based on hba specifics
+ * @pre_reset:		chipset specific changes to default for device-hba resets
+ * @resetproc:		routine to reset controller after a disk reset
+ * @maskproc:		special host masking for drive selection
+ * @quirkproc:		check host's drive quirk list
+ *
+ * @mdma_filter:	filter MDMA modes
+ * @udma_filter:	filter UDMA modes
+ *
+ * @cable_detect:	detect cable type
+ */
 struct ide_port_ops {
-	/* host specific initialization of a device */
 	void	(*init_dev)(ide_drive_t *);
-	/* routine to program host for PIO mode */
 	void	(*set_pio_mode)(ide_drive_t *, const u8);
-	/* routine to program host for DMA mode */
 	void	(*set_dma_mode)(ide_drive_t *, const u8);
-	/* tweaks hardware to select drive */
 	void	(*selectproc)(ide_drive_t *);
-	/* chipset polling based on hba specifics */
 	int	(*reset_poll)(ide_drive_t *);
-	/* chipset specific changes to default for device-hba resets */
 	void	(*pre_reset)(ide_drive_t *);
-	/* routine to reset controller after a disk reset */
 	void	(*resetproc)(ide_drive_t *);
-	/* special host masking for drive selection */
 	void	(*maskproc)(ide_drive_t *, int);
-	/* check host's drive quirk list */
 	void	(*quirkproc)(ide_drive_t *);
 
 	u8	(*mdma_filter)(ide_drive_t *);