* [PATCH 2/4] libata: separate out ata_dev_configure()
2006-03-01 7:09 [PATCHSET] libata: reorganize ata_dev_identify(), take 3 Tejun Heo
@ 2006-03-01 7:09 ` Tejun Heo
2006-03-01 7:09 ` [PATCH 1/4] libata: convert dev->id to pointer Tejun Heo
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Tejun Heo @ 2006-03-01 7:09 UTC (permalink / raw)
To: jgarzik, linux-ide; +Cc: Tejun Heo
Separate out ata_dev_configure() from ata_dev_identify() such that
ata_dev_configure() only configures @dev according to passed in @id.
The function now does not disable device on failure, it just returns
appropirate error code.
As this change leaves ata_dev_identify() with only reading ID, calling
configure and disabling devices according to the results, this patch
also kills ata_dev_identify() and inlines the logic into
ata_bus_probe().
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/scsi/libata-core.c | 78 ++++++++++++++++++++++----------------------
1 files changed, 39 insertions(+), 39 deletions(-)
9e8f99c18ddb9615403cdeca438a2d3c26b012fb
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index a630cda..dcc6b06 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1050,45 +1050,31 @@ static int ata_dev_read_id(struct ata_po
}
/**
- * ata_dev_identify - obtain IDENTIFY x DEVICE page
- * @ap: port on which device we wish to probe resides
- * @device: device bus address, starting at zero
+ * ata_dev_configure - Configure the specified ATA/ATAPI device
+ * @ap: Port on which target device resides
+ * @dev: Target device to configure
*
- * Following bus reset, we issue the IDENTIFY [PACKET] DEVICE
- * command, and read back the 512-byte device information page.
- * The device information page is fed to us via the standard
- * PIO-IN protocol, but we hand-code it here. (TODO: investigate
- * using standard PIO-IN paths)
- *
- * After reading the device information page, we use several
- * bits of information from it to initialize data structures
- * that will be used during the lifetime of the ata_device.
- * Other data from the info page is used to disqualify certain
- * older ATA devices we do not wish to support.
+ * Configure @dev according to @dev->id. Generic and low-level
+ * driver specific fixups are also applied.
*
* LOCKING:
- * Inherited from caller. Some functions called by this function
- * obtain the host_set lock.
+ * Kernel thread context (may sleep)
+ *
+ * RETURNS:
+ * 0 on success, -errno otherwise
*/
-
-static void ata_dev_identify(struct ata_port *ap, unsigned int device)
+static int ata_dev_configure(struct ata_port *ap, struct ata_device *dev)
{
- struct ata_device *dev = &ap->device[device];
unsigned long xfer_modes;
int i, rc;
if (!ata_dev_present(dev)) {
DPRINTK("ENTER/EXIT (host %u, dev %u) -- nodev\n",
- ap->id, device);
- return;
+ ap->id, dev->devno);
+ return 0;
}
- DPRINTK("ENTER, host %u, dev %u\n", ap->id, device);
-
- WARN_ON(dev->id != NULL);
- rc = ata_dev_read_id(ap, dev, &dev->class, 1, &dev->id);
- if (rc)
- goto err_out;
+ DPRINTK("ENTER, host %u, dev %u\n", ap->id, dev->devno);
/*
* common ATA, ATAPI feature tests
@@ -1097,6 +1083,7 @@ static void ata_dev_identify(struct ata_
/* we require DMA support (bits 8 of word 49) */
if (!ata_id_has_dma(dev->id)) {
printk(KERN_DEBUG "ata%u: no dma\n", ap->id);
+ rc = -EINVAL;
goto err_out_nosup;
}
@@ -1121,12 +1108,12 @@ static void ata_dev_identify(struct ata_
/* print device info to dmesg */
printk(KERN_INFO "ata%u: dev %u ATA-%d, max %s, %Lu sectors:%s\n",
- ap->id, device,
+ ap->id, dev->devno,
ata_id_major_version(dev->id),
ata_mode_string(xfer_modes),
(unsigned long long)dev->n_sectors,
dev->flags & ATA_DFLAG_LBA48 ? " LBA48" : " LBA");
- } else {
+ } else {
/* CHS */
/* Default translation */
@@ -1143,7 +1130,7 @@ static void ata_dev_identify(struct ata_
/* print device info to dmesg */
printk(KERN_INFO "ata%u: dev %u ATA-%d, max %s, %Lu sectors: CHS %d/%d/%d\n",
- ap->id, device,
+ ap->id, dev->devno,
ata_id_major_version(dev->id),
ata_mode_string(xfer_modes),
(unsigned long long)dev->n_sectors,
@@ -1159,13 +1146,14 @@ static void ata_dev_identify(struct ata_
rc = atapi_cdb_len(dev->id);
if ((rc < 12) || (rc > ATAPI_CDB_LEN)) {
printk(KERN_WARNING "ata%u: unsupported CDB len\n", ap->id);
+ rc = -EINVAL;
goto err_out_nosup;
}
dev->cdb_len = (unsigned int) rc;
/* print device info to dmesg */
printk(KERN_INFO "ata%u: dev %u ATAPI, max %s\n",
- ap->id, device,
+ ap->id, dev->devno,
ata_mode_string(xfer_modes));
}
@@ -1176,14 +1164,13 @@ static void ata_dev_identify(struct ata_
ap->device[i].cdb_len);
DPRINTK("EXIT, drv_stat = 0x%x\n", ata_chk_status(ap));
- return;
+ return 0;
err_out_nosup:
printk(KERN_WARNING "ata%u: dev %u not supported, ignoring\n",
- ap->id, device);
-err_out:
- dev->class++; /* converts ATA_DEV_xxx into ATA_DEV_xxx_UNSUP */
+ ap->id, dev->devno);
DPRINTK("EXIT, err\n");
+ return rc;
}
@@ -1259,11 +1246,24 @@ static int ata_bus_probe(struct ata_port
goto err_out;
for (i = 0; i < ATA_MAX_DEVICES; i++) {
- ata_dev_identify(ap, i);
- if (ata_dev_present(&ap->device[i])) {
- found = 1;
- ata_dev_config(ap,i);
+ struct ata_device *dev = &ap->device[i];
+
+ if (!ata_dev_present(dev))
+ continue;
+
+ WARN_ON(dev->id != NULL);
+ if (ata_dev_read_id(ap, dev, &dev->class, 1, &dev->id)) {
+ dev->class = ATA_DEV_NONE;
+ continue;
}
+
+ if (ata_dev_configure(ap, dev)) {
+ dev->class++; /* disable device */
+ continue;
+ }
+
+ ata_dev_config(ap, i);
+ found = 1;
}
if ((!found) || (ap->flags & ATA_FLAG_PORT_DISABLED))
--
1.2.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 1/4] libata: convert dev->id to pointer
2006-03-01 7:09 [PATCHSET] libata: reorganize ata_dev_identify(), take 3 Tejun Heo
2006-03-01 7:09 ` [PATCH 2/4] libata: separate out ata_dev_configure() Tejun Heo
@ 2006-03-01 7:09 ` Tejun Heo
2006-03-03 22:31 ` Jeff Garzik
2006-03-01 7:09 ` [PATCH 3/4] libata: fold ata_dev_config() into ata_dev_configure() Tejun Heo
2006-03-01 7:09 ` [PATCH 4/4] libata: reorganize ata_bus_probe() Tejun Heo
3 siblings, 1 reply; 6+ messages in thread
From: Tejun Heo @ 2006-03-01 7:09 UTC (permalink / raw)
To: jgarzik, linux-ide; +Cc: Tejun Heo
Convert dev->id from array to pointer. This is to accomodate
revalidation. During revalidation, both old and new IDENTIFY pages
should be accessible and single ->id array doesn't cut it.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/scsi/libata-core.c | 20 +++++++++++++++++---
include/linux/libata.h | 2 +-
2 files changed, 18 insertions(+), 4 deletions(-)
6ae458d0e6b0f04d5092e79a8e45a4c187037d27
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 436520e..a630cda 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -907,7 +907,7 @@ unsigned int ata_pio_need_iordy(const st
* @dev: target device
* @p_class: pointer to class of the target device (may be changed)
* @post_reset: is this read ID post-reset?
- * @id: buffer to fill IDENTIFY page into
+ * @p_id: read IDENTIFY page (newly allocated)
*
* Read ID data from the specified device. ATA_CMD_ID_ATA is
* performed on ATA devices and ATA_CMD_ID_ATAPI on ATAPI
@@ -922,12 +922,13 @@ unsigned int ata_pio_need_iordy(const st
* 0 on success, -errno otherwise.
*/
static int ata_dev_read_id(struct ata_port *ap, struct ata_device *dev,
- unsigned int *p_class, int post_reset, u16 *id)
+ unsigned int *p_class, int post_reset, u16 **p_id)
{
unsigned int class = *p_class;
unsigned int using_edd;
struct ata_taskfile tf;
unsigned int err_mask = 0;
+ u16 *id;
const char *reason;
int rc;
@@ -941,6 +942,13 @@ static int ata_dev_read_id(struct ata_po
ata_dev_select(ap, dev->devno, 1, 1); /* select device 0/1 */
+ id = kmalloc(sizeof(id[0]) * ATA_ID_WORDS, GFP_KERNEL);
+ if (id == NULL) {
+ rc = -ENOMEM;
+ reason = "out of memory";
+ goto err_out;
+ }
+
retry:
ata_tf_init(ap, &tf, dev->devno);
@@ -1031,11 +1039,13 @@ static int ata_dev_read_id(struct ata_po
}
*p_class = class;
+ *p_id = id;
return 0;
err_out:
printk(KERN_WARNING "ata%u: dev %u failed to IDENTIFY (%s)\n",
ap->id, dev->devno, reason);
+ kfree(id);
return rc;
}
@@ -1075,7 +1085,8 @@ static void ata_dev_identify(struct ata_
DPRINTK("ENTER, host %u, dev %u\n", ap->id, device);
- rc = ata_dev_read_id(ap, dev, &dev->class, 1, dev->id);
+ WARN_ON(dev->id != NULL);
+ rc = ata_dev_read_id(ap, dev, &dev->class, 1, &dev->id);
if (rc)
goto err_out;
@@ -4728,11 +4739,14 @@ void ata_host_set_remove(struct ata_host
int ata_scsi_release(struct Scsi_Host *host)
{
struct ata_port *ap = (struct ata_port *) &host->hostdata[0];
+ int i;
DPRINTK("ENTER\n");
ap->ops->port_disable(ap);
ata_host_remove(ap, 0);
+ for (i = 0; i < ATA_MAX_DEVICES; i++)
+ kfree(ap->device[i].id);
DPRINTK("EXIT\n");
return 1;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 66b6847..68666b9 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -339,7 +339,7 @@ struct ata_device {
unsigned long flags; /* ATA_DFLAG_xxx */
unsigned int class; /* ATA_DEV_xxx */
unsigned int devno; /* 0 or 1 */
- u16 id[ATA_ID_WORDS]; /* IDENTIFY xxx DEVICE data */
+ u16 *id; /* IDENTIFY xxx DEVICE data */
u8 pio_mode;
u8 dma_mode;
u8 xfer_mode;
--
1.2.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/4] libata: fold ata_dev_config() into ata_dev_configure()
2006-03-01 7:09 [PATCHSET] libata: reorganize ata_dev_identify(), take 3 Tejun Heo
2006-03-01 7:09 ` [PATCH 2/4] libata: separate out ata_dev_configure() Tejun Heo
2006-03-01 7:09 ` [PATCH 1/4] libata: convert dev->id to pointer Tejun Heo
@ 2006-03-01 7:09 ` Tejun Heo
2006-03-01 7:09 ` [PATCH 4/4] libata: reorganize ata_bus_probe() Tejun Heo
3 siblings, 0 replies; 6+ messages in thread
From: Tejun Heo @ 2006-03-01 7:09 UTC (permalink / raw)
To: jgarzik, linux-ide; +Cc: Tejun Heo
ata_dev_config() needs to be done everytime a device is configured.
Fold it into ata_dev_configure().
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/scsi/libata-core.c | 48 ++++++++++++++++----------------------------
include/linux/libata.h | 1 -
2 files changed, 17 insertions(+), 32 deletions(-)
8686b79624ba861834b6648a043c1ca95ea806dd
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index dcc6b06..f9ab563 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1049,6 +1049,12 @@ static int ata_dev_read_id(struct ata_po
return rc;
}
+static inline u8 ata_dev_knobble(const struct ata_port *ap,
+ struct ata_device *dev)
+{
+ return ((ap->cbl == ATA_CBL_SATA) && (!ata_id_is_sata(dev->id)));
+}
+
/**
* ata_dev_configure - Configure the specified ATA/ATAPI device
* @ap: Port on which target device resides
@@ -1163,6 +1169,17 @@ static int ata_dev_configure(struct ata_
ap->host->max_cmd_len,
ap->device[i].cdb_len);
+ /* limit bridge transfers to udma5, 200 sectors */
+ if (ata_dev_knobble(ap, dev)) {
+ printk(KERN_INFO "ata%u(%u): applying bridge limits\n",
+ ap->id, dev->devno);
+ ap->udma_mask &= ATA_UDMA5;
+ dev->max_sectors = ATA_MAX_SECTORS;
+ }
+
+ if (ap->ops->dev_config)
+ ap->ops->dev_config(ap, dev);
+
DPRINTK("EXIT, drv_stat = 0x%x\n", ata_chk_status(ap));
return 0;
@@ -1173,35 +1190,6 @@ err_out_nosup:
return rc;
}
-
-static inline u8 ata_dev_knobble(const struct ata_port *ap,
- struct ata_device *dev)
-{
- return ((ap->cbl == ATA_CBL_SATA) && (!ata_id_is_sata(dev->id)));
-}
-
-/**
- * ata_dev_config - Run device specific handlers & check for SATA->PATA bridges
- * @ap: Bus
- * @i: Device
- *
- * LOCKING:
- */
-
-void ata_dev_config(struct ata_port *ap, unsigned int i)
-{
- /* limit bridge transfers to udma5, 200 sectors */
- if (ata_dev_knobble(ap, &ap->device[i])) {
- printk(KERN_INFO "ata%u(%u): applying bridge limits\n",
- ap->id, i);
- ap->udma_mask &= ATA_UDMA5;
- ap->device[i].max_sectors = ATA_MAX_SECTORS;
- }
-
- if (ap->ops->dev_config)
- ap->ops->dev_config(ap, &ap->device[i]);
-}
-
/**
* ata_bus_probe - Reset and probe ATA bus
* @ap: Bus to probe
@@ -1262,7 +1250,6 @@ static int ata_bus_probe(struct ata_port
continue;
}
- ata_dev_config(ap, i);
found = 1;
}
@@ -4963,7 +4950,6 @@ EXPORT_SYMBOL_GPL(ata_host_intr);
EXPORT_SYMBOL_GPL(ata_dev_classify);
EXPORT_SYMBOL_GPL(ata_id_string);
EXPORT_SYMBOL_GPL(ata_id_c_string);
-EXPORT_SYMBOL_GPL(ata_dev_config);
EXPORT_SYMBOL_GPL(ata_scsi_simulate);
EXPORT_SYMBOL_GPL(ata_eh_qc_complete);
EXPORT_SYMBOL_GPL(ata_eh_qc_retry);
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 68666b9..1aab218 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -542,7 +542,6 @@ extern void ata_id_string(const u16 *id,
unsigned int ofs, unsigned int len);
extern void ata_id_c_string(const u16 *id, unsigned char *s,
unsigned int ofs, unsigned int len);
-extern void ata_dev_config(struct ata_port *ap, unsigned int i);
extern void ata_bmdma_setup (struct ata_queued_cmd *qc);
extern void ata_bmdma_start (struct ata_queued_cmd *qc);
extern void ata_bmdma_stop(struct ata_queued_cmd *qc);
--
1.2.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/4] libata: reorganize ata_bus_probe()
2006-03-01 7:09 [PATCHSET] libata: reorganize ata_dev_identify(), take 3 Tejun Heo
` (2 preceding siblings ...)
2006-03-01 7:09 ` [PATCH 3/4] libata: fold ata_dev_config() into ata_dev_configure() Tejun Heo
@ 2006-03-01 7:09 ` Tejun Heo
3 siblings, 0 replies; 6+ messages in thread
From: Tejun Heo @ 2006-03-01 7:09 UTC (permalink / raw)
To: jgarzik, linux-ide; +Cc: Tejun Heo
Now that reset and configure are converted such that they don't modify
or disable libata core data structures, reorganize ata_bus_probe() to
reflect this change.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/scsi/libata-core.c | 46 ++++++++++++++++++++++++--------------------
1 files changed, 25 insertions(+), 21 deletions(-)
fd8657c1907afa3fc13b55ef4c274580d413527a
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index f9ab563..63ba1aa 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1207,35 +1207,40 @@ err_out_nosup:
static int ata_bus_probe(struct ata_port *ap)
{
- unsigned int i, found = 0;
+ unsigned int classes[ATA_MAX_DEVICES];
+ unsigned int i, rc, found = 0;
- if (ap->ops->probe_reset) {
- unsigned int classes[ATA_MAX_DEVICES];
- int rc;
-
- ata_port_probe(ap);
+ ata_port_probe(ap);
+ /* reset */
+ if (ap->ops->probe_reset) {
rc = ap->ops->probe_reset(ap, classes);
- if (rc == 0) {
- for (i = 0; i < ATA_MAX_DEVICES; i++) {
- if (classes[i] == ATA_DEV_UNKNOWN)
- classes[i] = ATA_DEV_NONE;
- ap->device[i].class = classes[i];
- }
- } else {
- printk(KERN_ERR "ata%u: probe reset failed, "
- "disabling port\n", ap->id);
- ata_port_disable(ap);
+ if (rc) {
+ printk("ata%u: reset failed (errno=%d)\n", ap->id, rc);
+ return rc;
}
- } else
+
+ for (i = 0; i < ATA_MAX_DEVICES; i++)
+ if (classes[i] == ATA_DEV_UNKNOWN)
+ classes[i] = ATA_DEV_NONE;
+ } else {
ap->ops->phy_reset(ap);
- if (ap->flags & ATA_FLAG_PORT_DISABLED)
- goto err_out;
+ for (i = 0; i < ATA_MAX_DEVICES; i++) {
+ if (!(ap->flags & ATA_FLAG_PORT_DISABLED))
+ classes[i] = ap->device[i].class;
+ else
+ ap->device[i].class = ATA_DEV_UNKNOWN;
+ }
+ ata_port_probe(ap);
+ }
+ /* read IDENTIFY page and configure devices */
for (i = 0; i < ATA_MAX_DEVICES; i++) {
struct ata_device *dev = &ap->device[i];
+ dev->class = classes[i];
+
if (!ata_dev_present(dev))
continue;
@@ -1253,7 +1258,7 @@ static int ata_bus_probe(struct ata_port
found = 1;
}
- if ((!found) || (ap->flags & ATA_FLAG_PORT_DISABLED))
+ if (!found)
goto err_out_disable;
ata_set_mode(ap);
@@ -1264,7 +1269,6 @@ static int ata_bus_probe(struct ata_port
err_out_disable:
ap->ops->port_disable(ap);
-err_out:
return -1;
}
--
1.2.1
^ permalink raw reply related [flat|nested] 6+ messages in thread