* [PATCHSET] libata: improve transfer mode handling
@ 2006-03-05 19:31 Tejun Heo
2006-03-05 19:31 ` [PATCH 1/6] libata: improve xfer mask constants and update ata_mode_string() Tejun Heo
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: Tejun Heo @ 2006-03-05 19:31 UTC (permalink / raw)
To: jgarzik, albertcc, alan, linux-ide, htejun
Hello, Jeff, Alan and Albert.
This is the second take of improve-transfer-mode-handling patchset.
The last take[1] made low level driver API uglier and Alan rejected
the API changes. Changes from the last take are...
* No low level driver API change. All changes are confined tolibata
core layer proper.
* Fewer/simpler xfer_mask helpers.
* Better splitted patches.
This patchset makes transfer mode determination done in
ata_dev_xfermask() before any of actual configuration happens. This
should help integrating Alan's changes easier.
This patchset is against the current upstream[2] + port_task
patchset[3]. However, all the patches except the last one applies
against upstream with offsets and the reject of the last patch is
trivial. I'll post two versions of the last patch - one against
upstream + port_task, the other against upstream.
Thanks.
--
tejun
[1] http://marc.theaimsgroup.com/?l=linux-ide&m=114009896413075&w=2
[2] fbfda6e71bbdd3b4d41a56c3f20f31762c455a5e
[3] http://marc.theaimsgroup.com/?l=linux-ide&m=114154013813744&w=2
(dang, gmane still down.)
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/6] libata: add xfer_mask handling functions
2006-03-05 19:31 [PATCHSET] libata: improve transfer mode handling Tejun Heo
2006-03-05 19:31 ` [PATCH 1/6] libata: improve xfer mask constants and update ata_mode_string() Tejun Heo
@ 2006-03-05 19:31 ` Tejun Heo
2006-03-07 5:51 ` Albert Lee
2006-03-12 0:01 ` Jeff Garzik
2006-03-05 19:31 ` [PATCH 3/6] libata: use ata_id_xfermask() in ata_dev_configure() Tejun Heo
` (3 subsequent siblings)
5 siblings, 2 replies; 13+ messages in thread
From: Tejun Heo @ 2006-03-05 19:31 UTC (permalink / raw)
To: jgarzik, albertcc, alan, linux-ide; +Cc: Tejun Heo
Add ata_pack_xfermask(), ata_xfer_mask2mode(), ata_xfer_mode2mask(),
ata_xfer_mode2shift() and ata_id_xfermask(). These functions will be
used by following patches to simplify xfer_mask handling.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/scsi/libata-core.c | 147 ++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 147 insertions(+), 0 deletions(-)
3247748741bb32ca18e3fa3c0dd14690dbd184a6
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 18418c8..9946618 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -231,6 +231,108 @@ int ata_rwcmd_protocol(struct ata_queued
return -1;
}
+/**
+ * ata_pack_xfermask - Pack pio, mwdma and udma masks into xfer_mask
+ * @pio_mask: pio_mask
+ * @mwdma_mask: mwdma_mask
+ * @udma_mask: udma_mask
+ *
+ * Pack @pio_mask, @mwdma_mask and @udma_mask into a single
+ * unsigned int xfer_mask.
+ *
+ * LOCKING:
+ * None.
+ *
+ * RETURNS:
+ * Packed xfer_mask.
+ */
+static unsigned int ata_pack_xfermask(unsigned int pio_mask,
+ unsigned int mwdma_mask,
+ unsigned int udma_mask)
+{
+ return ((pio_mask << ATA_SHIFT_PIO) & ATA_MASK_PIO) |
+ ((mwdma_mask << ATA_SHIFT_MWDMA) & ATA_MASK_MWDMA) |
+ ((udma_mask << ATA_SHIFT_UDMA) & ATA_MASK_UDMA);
+}
+
+static const struct ata_xfer_ent {
+ unsigned int shift, bits;
+ u8 base;
+} ata_xfer_tbl[] = {
+ { ATA_SHIFT_PIO, ATA_BITS_PIO, XFER_PIO_0 },
+ { ATA_SHIFT_MWDMA, ATA_BITS_MWDMA, XFER_MW_DMA_0 },
+ { ATA_SHIFT_UDMA, ATA_BITS_UDMA, XFER_UDMA_0 },
+ { -1, },
+};
+
+/**
+ * ata_xfer_mask2mode - Find matching XFER_* for the given xfer_mask
+ * @xfer_mask: xfer_mask of interest
+ *
+ * Return matching XFER_* value for @xfer_mask. Only the highest
+ * bit of @xfer_mask is considered.
+ *
+ * LOCKING:
+ * None.
+ *
+ * RETURNS:
+ * Matching XFER_* value, 0 if no match found.
+ */
+static u8 ata_xfer_mask2mode(unsigned int xfer_mask)
+{
+ int highbit = fls(xfer_mask) - 1;
+ const struct ata_xfer_ent *ent;
+
+ for (ent = ata_xfer_tbl; ent->shift >= 0; ent++)
+ if (highbit >= ent->shift && highbit < ent->shift + ent->bits)
+ return ent->base + highbit - ent->shift;
+ return 0;
+}
+
+/**
+ * ata_xfer_mode2mask - Find matching xfer_mask for XFER_*
+ * @xfer_mode: XFER_* of interest
+ *
+ * Return matching xfer_mask for @xfer_mode.
+ *
+ * LOCKING:
+ * None.
+ *
+ * RETURNS:
+ * Matching xfer_mask, 0 if no match found.
+ */
+static unsigned int ata_xfer_mode2mask(u8 xfer_mode)
+{
+ const struct ata_xfer_ent *ent;
+
+ for (ent = ata_xfer_tbl; ent->shift >= 0; ent++)
+ if (xfer_mode >= ent->base && xfer_mode < ent->base + ent->bits)
+ return 1 << (ent->shift + xfer_mode - ent->base);
+ return 0;
+}
+
+/**
+ * ata_xfer_mode2shift - Find matching xfer_shift for XFER_*
+ * @xfer_mode: XFER_* of interest
+ *
+ * Return matching xfer_shift for @xfer_mode.
+ *
+ * LOCKING:
+ * None.
+ *
+ * RETURNS:
+ * Matching xfer_shift, -1 if no match found.
+ */
+static int ata_xfer_mode2shift(unsigned int xfer_mode)
+{
+ const struct ata_xfer_ent *ent;
+
+ for (ent = ata_xfer_tbl; ent->shift >= 0; ent++)
+ if (xfer_mode >= ent->base && xfer_mode < ent->base + ent->bits)
+ return ent->shift;
+ return -1;
+}
+
static const char * const xfer_mode_str[] = {
"PIO0",
"PIO1",
@@ -682,6 +784,51 @@ static inline void ata_dump_id(const u16
id[93]);
}
+/**
+ * ata_id_xfermask - Compute xfermask from the given IDENTIFY data
+ * @id: IDENTIFY data to compute xfer mask from
+ *
+ * Compute the xfermask for this device. This is not as trivial
+ * as it seems if we must consider early devices correctly.
+ *
+ * FIXME: pre IDE drive timing (do we care ?).
+ *
+ * LOCKING:
+ * None.
+ *
+ * RETURNS:
+ * Computed xfermask
+ */
+static unsigned int ata_id_xfermask(const u16 *id)
+{
+ unsigned int pio_mask, mwdma_mask, udma_mask;
+
+ /* Usual case. Word 53 indicates word 64 is valid */
+ if (id[ATA_ID_FIELD_VALID] & (1 << 1)) {
+ pio_mask = id[ATA_ID_PIO_MODES] & 0x03;
+ pio_mask <<= 3;
+ pio_mask |= 0x7;
+ } else {
+ /* If word 64 isn't valid then Word 51 high byte holds
+ * the PIO timing number for the maximum. Turn it into
+ * a mask.
+ */
+ pio_mask = (2 << (id[ATA_ID_OLD_PIO_MODES] & 0xFF)) - 1 ;
+
+ /* But wait.. there's more. Design your standards by
+ * committee and you too can get a free iordy field to
+ * process. However its the speeds not the modes that
+ * are supported... Note drivers using the timing API
+ * will get this right anyway
+ */
+ }
+
+ mwdma_mask = id[ATA_ID_MWDMA_MODES] & 0x07;
+ udma_mask = id[ATA_ID_UDMA_MODES] & 0xff;
+
+ return ata_pack_xfermask(pio_mask, mwdma_mask, udma_mask);
+}
+
/*
* Compute the PIO modes available for this device. This is not as
* trivial as it seems if we must consider early devices correctly.
--
1.2.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 1/6] libata: improve xfer mask constants and update ata_mode_string()
2006-03-05 19:31 [PATCHSET] libata: improve transfer mode handling Tejun Heo
@ 2006-03-05 19:31 ` Tejun Heo
2006-03-12 0:03 ` Jeff Garzik
2006-03-05 19:31 ` [PATCH 2/6] libata: add xfer_mask handling functions Tejun Heo
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Tejun Heo @ 2006-03-05 19:31 UTC (permalink / raw)
To: jgarzik, albertcc, alan, linux-ide; +Cc: Tejun Heo
Add ATA_BITS_*, ATA_MASK_* macros and reorder xfer_mask fields such
that higher transfer mode is placed at higher order bit. As thie
reordering breaks ata_mode_string(), this patch also rewrites
ata_mode_string().
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/scsi/libata-core.c | 44 +++++++++++++++++---------------------------
include/linux/libata.h | 16 ++++++++++++----
2 files changed, 29 insertions(+), 31 deletions(-)
ae787be679d5411a1877134ba7d2cfbc7e1cdb69
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 6d8aa86..18418c8 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -232,6 +232,14 @@ int ata_rwcmd_protocol(struct ata_queued
}
static const char * const xfer_mode_str[] = {
+ "PIO0",
+ "PIO1",
+ "PIO2",
+ "PIO3",
+ "PIO4",
+ "MWDMA0",
+ "MWDMA1",
+ "MWDMA2",
"UDMA/16",
"UDMA/25",
"UDMA/33",
@@ -240,49 +248,31 @@ static const char * const xfer_mode_str[
"UDMA/100",
"UDMA/133",
"UDMA7",
- "MWDMA0",
- "MWDMA1",
- "MWDMA2",
- "PIO0",
- "PIO1",
- "PIO2",
- "PIO3",
- "PIO4",
};
/**
- * ata_udma_string - convert UDMA bit offset to string
- * @mask: mask of bits supported; only highest bit counts.
+ * ata_mode_string - convert xfer_mask to string
+ * @xfer_mask: mask of bits supported; only highest bit counts.
*
* Determine string which represents the highest speed
- * (highest bit in @udma_mask).
+ * (highest bit in @modemask).
*
* LOCKING:
* None.
*
* RETURNS:
* Constant C string representing highest speed listed in
- * @udma_mask, or the constant C string "<n/a>".
+ * @mode_mask, or the constant C string "<n/a>".
*/
-static const char *ata_mode_string(unsigned int mask)
+static const char *ata_mode_string(unsigned int xfer_mask)
{
- int i;
-
- for (i = 7; i >= 0; i--)
- if (mask & (1 << i))
- goto out;
- for (i = ATA_SHIFT_MWDMA + 2; i >= ATA_SHIFT_MWDMA; i--)
- if (mask & (1 << i))
- goto out;
- for (i = ATA_SHIFT_PIO + 4; i >= ATA_SHIFT_PIO; i--)
- if (mask & (1 << i))
- goto out;
+ int highbit;
+ highbit = fls(xfer_mask) - 1;
+ if (highbit >= 0 && highbit < ARRAY_SIZE(xfer_mode_str))
+ return xfer_mode_str[highbit];
return "<n/a>";
-
-out:
- return xfer_mode_str[i];
}
/**
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 1567492..239408e 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -188,11 +188,19 @@ enum {
PORT_DISABLED = 2,
/* encoding various smaller bitmaps into a single
- * unsigned long bitmap
+ * unsigned int bitmap
*/
- ATA_SHIFT_UDMA = 0,
- ATA_SHIFT_MWDMA = 8,
- ATA_SHIFT_PIO = 11,
+ ATA_BITS_PIO = 5,
+ ATA_BITS_MWDMA = 3,
+ ATA_BITS_UDMA = 8,
+
+ ATA_SHIFT_PIO = 0,
+ ATA_SHIFT_MWDMA = ATA_SHIFT_PIO + ATA_BITS_PIO,
+ ATA_SHIFT_UDMA = ATA_SHIFT_MWDMA + ATA_BITS_MWDMA,
+
+ ATA_MASK_PIO = ((1 << ATA_BITS_PIO) - 1) << ATA_SHIFT_PIO,
+ ATA_MASK_MWDMA = ((1 << ATA_BITS_MWDMA) - 1) << ATA_SHIFT_MWDMA,
+ ATA_MASK_UDMA = ((1 << ATA_BITS_UDMA) - 1) << ATA_SHIFT_UDMA,
/* size of buffer to pad xfers ending on unaligned boundaries */
ATA_DMA_PAD_SZ = 4,
--
1.2.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/6] libata: use ata_id_xfermask() in ata_dev_configure()
2006-03-05 19:31 [PATCHSET] libata: improve transfer mode handling Tejun Heo
2006-03-05 19:31 ` [PATCH 1/6] libata: improve xfer mask constants and update ata_mode_string() Tejun Heo
2006-03-05 19:31 ` [PATCH 2/6] libata: add xfer_mask handling functions Tejun Heo
@ 2006-03-05 19:31 ` Tejun Heo
2006-03-05 19:31 ` [PATCH 5/6] libata: reimplement ata_set_mode() using xfer_mask helpers Tejun Heo
` (2 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Tejun Heo @ 2006-03-05 19:31 UTC (permalink / raw)
To: jgarzik, albertcc, alan, linux-ide; +Cc: Tejun Heo
Replace quick & dirty max transfer mode determination in
ata_dev_configure() with ata_id_xfermask(). While at it, rename
xfer_modes variable to xfer_mask and make it unsigned int for
consistency.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/scsi/libata-core.c | 16 ++++++----------
1 files changed, 6 insertions(+), 10 deletions(-)
58efdc973c86dc4fc8c7e4b43874a462a777257c
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 9946618..84a8550 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1224,7 +1224,7 @@ static inline u8 ata_dev_knobble(const s
static int ata_dev_configure(struct ata_port *ap, struct ata_device *dev,
int print_info)
{
- unsigned long xfer_modes;
+ unsigned int xfer_mask;
int i, rc;
if (!ata_dev_present(dev)) {
@@ -1255,12 +1255,8 @@ static int ata_dev_configure(struct ata_
goto err_out_nosup;
}
- /* quick-n-dirty find max transfer mode; for printk only */
- xfer_modes = dev->id[ATA_ID_UDMA_MODES];
- if (!xfer_modes)
- xfer_modes = (dev->id[ATA_ID_MWDMA_MODES]) << ATA_SHIFT_MWDMA;
- if (!xfer_modes)
- xfer_modes = ata_pio_modes(dev);
+ /* find max transfer mode; for printk only */
+ xfer_mask = ata_id_xfermask(dev->id);
ata_dump_id(dev->id);
@@ -1284,7 +1280,7 @@ static int ata_dev_configure(struct ata_
"max %s, %Lu sectors: %s\n",
ap->id, dev->devno,
ata_id_major_version(dev->id),
- ata_mode_string(xfer_modes),
+ ata_mode_string(xfer_mask),
(unsigned long long)dev->n_sectors,
lba_desc);
} else {
@@ -1308,7 +1304,7 @@ static int ata_dev_configure(struct ata_
"max %s, %Lu sectors: CHS %u/%u/%u\n",
ap->id, dev->devno,
ata_id_major_version(dev->id),
- ata_mode_string(xfer_modes),
+ ata_mode_string(xfer_mask),
(unsigned long long)dev->n_sectors,
dev->cylinders, dev->heads, dev->sectors);
}
@@ -1329,7 +1325,7 @@ static int ata_dev_configure(struct ata_
/* print device info to dmesg */
if (print_info)
printk(KERN_INFO "ata%u: dev %u ATAPI, max %s\n",
- ap->id, dev->devno, ata_mode_string(xfer_modes));
+ ap->id, dev->devno, ata_mode_string(xfer_mask));
}
ap->host->max_cmd_len = 0;
--
1.2.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 6/6] libata: kill unused xfer_mode functions
2006-03-05 19:31 [PATCHSET] libata: improve transfer mode handling Tejun Heo
` (4 preceding siblings ...)
2006-03-05 19:31 ` [PATCH 4/6] libata: use xfer_mask helpers in ata_dev_set_mode() Tejun Heo
@ 2006-03-05 19:31 ` Tejun Heo
2006-03-05 19:35 ` Tejun Heo
5 siblings, 1 reply; 13+ messages in thread
From: Tejun Heo @ 2006-03-05 19:31 UTC (permalink / raw)
To: jgarzik, albertcc, alan, linux-ide; +Cc: Tejun Heo
Preceding xfer_mask changes make the following functions unused.
ata_pio_modes(), base_from_shift(), ata_pr_blacklisted(), fgb()
Kill them. Also, as xfer_mode_str[] is now only used by
ata_mode_string(), move it into the function.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
This patch is against upstream + port_task patchset.
drivers/scsi/libata-core.c | 108 +++++++-------------------------------------
1 files changed, 18 insertions(+), 90 deletions(-)
7d1819a2066d9286e0a89e11808f7b5be92409b5
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index d211f0b..5acb079 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -67,7 +67,6 @@ static void ata_set_mode(struct ata_port
static void ata_dev_set_xfermode(struct ata_port *ap, struct ata_device *dev);
static unsigned int ata_dev_xfermask(struct ata_port *ap,
struct ata_device *dev);
-static int fgb(u32 bitmap);
static unsigned int ata_unique_id = 1;
static struct workqueue_struct *ata_wq;
@@ -331,25 +330,6 @@ static int ata_xfer_mode2shift(unsigned
return -1;
}
-static const char * const xfer_mode_str[] = {
- "PIO0",
- "PIO1",
- "PIO2",
- "PIO3",
- "PIO4",
- "MWDMA0",
- "MWDMA1",
- "MWDMA2",
- "UDMA/16",
- "UDMA/25",
- "UDMA/33",
- "UDMA/44",
- "UDMA/66",
- "UDMA/100",
- "UDMA/133",
- "UDMA7",
-};
-
/**
* ata_mode_string - convert xfer_mask to string
* @xfer_mask: mask of bits supported; only highest bit counts.
@@ -364,9 +344,26 @@ static const char * const xfer_mode_str[
* Constant C string representing highest speed listed in
* @mode_mask, or the constant C string "<n/a>".
*/
-
static const char *ata_mode_string(unsigned int xfer_mask)
{
+ static const char * const xfer_mode_str[] = {
+ "PIO0",
+ "PIO1",
+ "PIO2",
+ "PIO3",
+ "PIO4",
+ "MWDMA0",
+ "MWDMA1",
+ "MWDMA2",
+ "UDMA/16",
+ "UDMA/25",
+ "UDMA/33",
+ "UDMA/44",
+ "UDMA/66",
+ "UDMA/100",
+ "UDMA/133",
+ "UDMA7",
+ };
int highbit;
highbit = fls(xfer_mask) - 1;
@@ -827,35 +824,6 @@ static unsigned int ata_id_xfermask(cons
return ata_pack_xfermask(pio_mask, mwdma_mask, udma_mask);
}
-/*
- * Compute the PIO modes available for this device. This is not as
- * trivial as it seems if we must consider early devices correctly.
- *
- * FIXME: pre IDE drive timing (do we care ?).
- */
-
-static unsigned int ata_pio_modes(const struct ata_device *adev)
-{
- u16 modes;
-
- /* Usual case. Word 53 indicates word 64 is valid */
- if (adev->id[ATA_ID_FIELD_VALID] & (1 << 1)) {
- modes = adev->id[ATA_ID_PIO_MODES] & 0x03;
- modes <<= 3;
- modes |= 0x7;
- return modes;
- }
-
- /* If word 64 isn't valid then Word 51 high byte holds the PIO timing
- number for the maximum. Turn it into a mask and return it */
- modes = (2 << ((adev->id[ATA_ID_OLD_PIO_MODES] >> 8) & 0xFF)) - 1 ;
- return modes;
- /* But wait.. there's more. Design your standards by committee and
- you too can get a free iordy field to process. However its the
- speeds not the modes that are supported... Note drivers using the
- timing API will get this right anyway */
-}
-
/**
* ata_port_queue_task - Queue port_task
* @ap: The ata_port to queue port_task for
@@ -1728,26 +1696,6 @@ int ata_timing_compute(struct ata_device
return 0;
}
-static const struct {
- unsigned int shift;
- u8 base;
-} xfer_mode_classes[] = {
- { ATA_SHIFT_UDMA, XFER_UDMA_0 },
- { ATA_SHIFT_MWDMA, XFER_MW_DMA_0 },
- { ATA_SHIFT_PIO, XFER_PIO_0 },
-};
-
-static u8 base_from_shift(unsigned int shift)
-{
- int i;
-
- for (i = 0; i < ARRAY_SIZE(xfer_mode_classes); i++)
- if (xfer_mode_classes[i].shift == shift)
- return xfer_mode_classes[i].base;
-
- return 0xff;
-}
-
static void ata_dev_set_mode(struct ata_port *ap, struct ata_device *dev)
{
if (!ata_dev_present(dev) || (ap->flags & ATA_FLAG_PORT_DISABLED))
@@ -2596,13 +2544,6 @@ int ata_dev_revalidate(struct ata_port *
return rc;
}
-static void ata_pr_blacklisted(const struct ata_port *ap,
- const struct ata_device *dev)
-{
- printk(KERN_WARNING "ata%u: dev %u is on DMA blacklist, disabling DMA\n",
- ap->id, dev->devno);
-}
-
static const char * const ata_dma_blacklist [] = {
"WDC AC11000H",
"WDC AC22100H",
@@ -2690,19 +2631,6 @@ static unsigned int ata_dev_xfermask(str
return xfer_mask;
}
-/* find greatest bit */
-static int fgb(u32 bitmap)
-{
- unsigned int i;
- int x = -1;
-
- for (i = 0; i < 32; i++)
- if (bitmap & (1 << i))
- x = i;
-
- return x;
-}
-
/**
* ata_dev_set_xfermode - Issue SET FEATURES - XFER MODE command
* @ap: Port associated with device @dev
--
1.2.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/6] libata: use xfer_mask helpers in ata_dev_set_mode()
2006-03-05 19:31 [PATCHSET] libata: improve transfer mode handling Tejun Heo
` (3 preceding siblings ...)
2006-03-05 19:31 ` [PATCH 5/6] libata: reimplement ata_set_mode() using xfer_mask helpers Tejun Heo
@ 2006-03-05 19:31 ` Tejun Heo
2006-03-05 19:31 ` [PATCH 6/6] libata: kill unused xfer_mode functions Tejun Heo
5 siblings, 0 replies; 13+ messages in thread
From: Tejun Heo @ 2006-03-05 19:31 UTC (permalink / raw)
To: jgarzik, albertcc, alan, linux-ide; +Cc: Tejun Heo
Rewrite hardcoded xfer_mode string determination in ata_dev_set_mode()
using xfer_mask helpers.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/scsi/libata-core.c | 15 ++++-----------
1 files changed, 4 insertions(+), 11 deletions(-)
6c1926e74a4b5266dbcbb4156dee075a05bcc36f
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 84a8550..8849512 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1752,9 +1752,6 @@ static u8 base_from_shift(unsigned int s
static void ata_dev_set_mode(struct ata_port *ap, struct ata_device *dev)
{
- int ofs, idx;
- u8 base;
-
if (!ata_dev_present(dev) || (ap->flags & ATA_FLAG_PORT_DISABLED))
return;
@@ -1763,22 +1760,18 @@ static void ata_dev_set_mode(struct ata_
ata_dev_set_xfermode(ap, dev);
- base = base_from_shift(dev->xfer_shift);
- ofs = dev->xfer_mode - base;
- idx = ofs + dev->xfer_shift;
- WARN_ON(idx >= ARRAY_SIZE(xfer_mode_str));
-
if (ata_dev_revalidate(ap, dev, 0)) {
printk(KERN_ERR "ata%u: failed to revalidate after set "
"xfermode, disabled\n", ap->id);
ata_port_disable(ap);
}
- DPRINTK("idx=%d xfer_shift=%u, xfer_mode=0x%x, base=0x%x, offset=%d\n",
- idx, dev->xfer_shift, (int)dev->xfer_mode, (int)base, ofs);
+ DPRINTK("xfer_shift=%u, xfer_mode=0x%x\n",
+ dev->xfer_shift, (int)dev->xfer_mode);
printk(KERN_INFO "ata%u: dev %u configured for %s\n",
- ap->id, dev->devno, xfer_mode_str[idx]);
+ ap->id, dev->devno,
+ ata_mode_string(ata_xfer_mode2mask(dev->xfer_mode)));
}
static int ata_host_set_pio(struct ata_port *ap)
--
1.2.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5/6] libata: reimplement ata_set_mode() using xfer_mask helpers
2006-03-05 19:31 [PATCHSET] libata: improve transfer mode handling Tejun Heo
` (2 preceding siblings ...)
2006-03-05 19:31 ` [PATCH 3/6] libata: use ata_id_xfermask() in ata_dev_configure() Tejun Heo
@ 2006-03-05 19:31 ` Tejun Heo
2006-03-05 19:31 ` [PATCH 4/6] libata: use xfer_mask helpers in ata_dev_set_mode() Tejun Heo
2006-03-05 19:31 ` [PATCH 6/6] libata: kill unused xfer_mode functions Tejun Heo
5 siblings, 0 replies; 13+ messages in thread
From: Tejun Heo @ 2006-03-05 19:31 UTC (permalink / raw)
To: jgarzik, albertcc, alan, linux-ide; +Cc: Tejun Heo
Use xfer_mask helpers to determine transfer mode. This rewrite also
makes transfer mode determination done before any actual
configuration. This patch doesn't result in any functional changes.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/scsi/libata-core.c | 233 +++++++++++++++-----------------------------
1 files changed, 79 insertions(+), 154 deletions(-)
d06b4f9f33fd7a5ffbb93837f5b4a69d4fa91dbc
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 8849512..d211f0b 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -65,11 +65,9 @@ static unsigned int ata_dev_init_params(
struct ata_device *dev);
static void ata_set_mode(struct ata_port *ap);
static void ata_dev_set_xfermode(struct ata_port *ap, struct ata_device *dev);
-static unsigned int ata_get_mode_mask(const struct ata_port *ap, int shift);
+static unsigned int ata_dev_xfermask(struct ata_port *ap,
+ struct ata_device *dev);
static int fgb(u32 bitmap);
-static int ata_choose_xfer_mode(const struct ata_port *ap,
- u8 *xfer_mode_out,
- unsigned int *xfer_shift_out);
static unsigned int ata_unique_id = 1;
static struct workqueue_struct *ata_wq;
@@ -1776,51 +1774,42 @@ static void ata_dev_set_mode(struct ata_
static int ata_host_set_pio(struct ata_port *ap)
{
- unsigned int mask;
- int x, i;
- u8 base, xfer_mode;
-
- mask = ata_get_mode_mask(ap, ATA_SHIFT_PIO);
- x = fgb(mask);
- if (x < 0) {
- printk(KERN_WARNING "ata%u: no PIO support\n", ap->id);
- return -1;
- }
-
- base = base_from_shift(ATA_SHIFT_PIO);
- xfer_mode = base + x;
-
- DPRINTK("base 0x%x xfer_mode 0x%x mask 0x%x x %d\n",
- (int)base, (int)xfer_mode, mask, x);
+ int i;
for (i = 0; i < ATA_MAX_DEVICES; i++) {
struct ata_device *dev = &ap->device[i];
- if (ata_dev_present(dev)) {
- dev->pio_mode = xfer_mode;
- dev->xfer_mode = xfer_mode;
- dev->xfer_shift = ATA_SHIFT_PIO;
- if (ap->ops->set_piomode)
- ap->ops->set_piomode(ap, dev);
+
+ if (!ata_dev_present(dev))
+ continue;
+
+ if (!dev->pio_mode) {
+ printk(KERN_WARNING "ata%u: no PIO support\n", ap->id);
+ return -1;
}
+
+ dev->xfer_mode = dev->pio_mode;
+ dev->xfer_shift = ATA_SHIFT_PIO;
+ if (ap->ops->set_piomode)
+ ap->ops->set_piomode(ap, dev);
}
return 0;
}
-static void ata_host_set_dma(struct ata_port *ap, u8 xfer_mode,
- unsigned int xfer_shift)
+static void ata_host_set_dma(struct ata_port *ap)
{
int i;
for (i = 0; i < ATA_MAX_DEVICES; i++) {
struct ata_device *dev = &ap->device[i];
- if (ata_dev_present(dev)) {
- dev->dma_mode = xfer_mode;
- dev->xfer_mode = xfer_mode;
- dev->xfer_shift = xfer_shift;
- if (ap->ops->set_dmamode)
- ap->ops->set_dmamode(ap, dev);
- }
+
+ if (!ata_dev_present(dev) || !dev->dma_mode)
+ continue;
+
+ dev->xfer_mode = dev->dma_mode;
+ dev->xfer_shift = ata_xfer_mode2shift(dev->dma_mode);
+ if (ap->ops->set_dmamode)
+ ap->ops->set_dmamode(ap, dev);
}
}
@@ -1835,28 +1824,34 @@ static void ata_host_set_dma(struct ata_
*/
static void ata_set_mode(struct ata_port *ap)
{
- unsigned int xfer_shift;
- u8 xfer_mode;
- int rc;
+ int i, rc;
- /* step 1: always set host PIO timings */
- rc = ata_host_set_pio(ap);
- if (rc)
- goto err_out;
+ /* step 1: calculate xfer_mask */
+ for (i = 0; i < ATA_MAX_DEVICES; i++) {
+ struct ata_device *dev = &ap->device[i];
+ unsigned int xfer_mask;
+
+ if (!ata_dev_present(dev))
+ continue;
- /* step 2: choose the best data xfer mode */
- xfer_mode = xfer_shift = 0;
- rc = ata_choose_xfer_mode(ap, &xfer_mode, &xfer_shift);
+ xfer_mask = ata_dev_xfermask(ap, dev);
+
+ dev->pio_mode = ata_xfer_mask2mode(xfer_mask & ATA_MASK_PIO);
+ dev->dma_mode = ata_xfer_mask2mode(xfer_mask & (ATA_MASK_MWDMA |
+ ATA_MASK_UDMA));
+ }
+
+ /* step 2: always set host PIO timings */
+ rc = ata_host_set_pio(ap);
if (rc)
goto err_out;
- /* step 3: if that xfer mode isn't PIO, set host DMA timings */
- if (xfer_shift != ATA_SHIFT_PIO)
- ata_host_set_dma(ap, xfer_mode, xfer_shift);
+ /* step 3: set host DMA timings */
+ ata_host_set_dma(ap);
/* step 4: update devices' xfer mode */
- ata_dev_set_mode(ap, &ap->device[0]);
- ata_dev_set_mode(ap, &ap->device[1]);
+ for (i = 0; i < ATA_MAX_DEVICES; i++)
+ ata_dev_set_mode(ap, &ap->device[i]);
if (ap->flags & ATA_FLAG_PORT_DISABLED)
return;
@@ -2654,77 +2649,45 @@ static int ata_dma_blacklisted(const str
return 0;
}
-static unsigned int ata_get_mode_mask(const struct ata_port *ap, int shift)
+/**
+ * ata_dev_xfermask - Compute supported xfermask of the given device
+ * @ap: Port on which the device to compute xfermask for resides
+ * @dev: Device to compute xfermask for
+ *
+ * Compute supported xfermask of @dev. This function is
+ * responsible for applying all known limits including host
+ * controller limits, device blacklist, etc...
+ *
+ * LOCKING:
+ * None.
+ *
+ * RETURNS:
+ * Computed xfermask.
+ */
+static unsigned int ata_dev_xfermask(struct ata_port *ap,
+ struct ata_device *dev)
{
- const struct ata_device *master, *slave;
- unsigned int mask;
+ unsigned long xfer_mask;
+ int i;
- master = &ap->device[0];
- slave = &ap->device[1];
+ xfer_mask = ata_pack_xfermask(ap->pio_mask, ap->mwdma_mask,
+ ap->udma_mask);
- WARN_ON(!ata_dev_present(master) && !ata_dev_present(slave));
-
- if (shift == ATA_SHIFT_UDMA) {
- mask = ap->udma_mask;
- if (ata_dev_present(master)) {
- mask &= (master->id[ATA_ID_UDMA_MODES] & 0xff);
- if (ata_dma_blacklisted(master)) {
- mask = 0;
- ata_pr_blacklisted(ap, master);
- }
- }
- if (ata_dev_present(slave)) {
- mask &= (slave->id[ATA_ID_UDMA_MODES] & 0xff);
- if (ata_dma_blacklisted(slave)) {
- mask = 0;
- ata_pr_blacklisted(ap, slave);
- }
- }
- }
- else if (shift == ATA_SHIFT_MWDMA) {
- mask = ap->mwdma_mask;
- if (ata_dev_present(master)) {
- mask &= (master->id[ATA_ID_MWDMA_MODES] & 0x07);
- if (ata_dma_blacklisted(master)) {
- mask = 0;
- ata_pr_blacklisted(ap, master);
- }
- }
- if (ata_dev_present(slave)) {
- mask &= (slave->id[ATA_ID_MWDMA_MODES] & 0x07);
- if (ata_dma_blacklisted(slave)) {
- mask = 0;
- ata_pr_blacklisted(ap, slave);
- }
- }
- }
- else if (shift == ATA_SHIFT_PIO) {
- mask = ap->pio_mask;
- if (ata_dev_present(master)) {
- /* spec doesn't return explicit support for
- * PIO0-2, so we fake it
- */
- u16 tmp_mode = master->id[ATA_ID_PIO_MODES] & 0x03;
- tmp_mode <<= 3;
- tmp_mode |= 0x7;
- mask &= tmp_mode;
- }
- if (ata_dev_present(slave)) {
- /* spec doesn't return explicit support for
- * PIO0-2, so we fake it
- */
- u16 tmp_mode = slave->id[ATA_ID_PIO_MODES] & 0x03;
- tmp_mode <<= 3;
- tmp_mode |= 0x7;
- mask &= tmp_mode;
- }
- }
- else {
- mask = 0xffffffff; /* shut up compiler warning */
- BUG();
+ /* use port-wide xfermask for now */
+ for (i = 0; i < ATA_MAX_DEVICES; i++) {
+ struct ata_device *d = &ap->device[i];
+ if (!ata_dev_present(d))
+ continue;
+ xfer_mask &= ata_id_xfermask(d->id);
+ if (ata_dma_blacklisted(d))
+ xfer_mask &= ~(ATA_MASK_MWDMA | ATA_MASK_UDMA);
}
- return mask;
+ if (ata_dma_blacklisted(dev))
+ printk(KERN_WARNING "ata%u: dev %u is on DMA blacklist, "
+ "disabling DMA\n", ap->id, dev->devno);
+
+ return xfer_mask;
}
/* find greatest bit */
@@ -2741,44 +2704,6 @@ static int fgb(u32 bitmap)
}
/**
- * ata_choose_xfer_mode - attempt to find best transfer mode
- * @ap: Port for which an xfer mode will be selected
- * @xfer_mode_out: (output) SET FEATURES - XFER MODE code
- * @xfer_shift_out: (output) bit shift that selects this mode
- *
- * Based on host and device capabilities, determine the
- * maximum transfer mode that is amenable to all.
- *
- * LOCKING:
- * PCI/etc. bus probe sem.
- *
- * RETURNS:
- * Zero on success, negative on error.
- */
-
-static int ata_choose_xfer_mode(const struct ata_port *ap,
- u8 *xfer_mode_out,
- unsigned int *xfer_shift_out)
-{
- unsigned int mask, shift;
- int x, i;
-
- for (i = 0; i < ARRAY_SIZE(xfer_mode_classes); i++) {
- shift = xfer_mode_classes[i].shift;
- mask = ata_get_mode_mask(ap, shift);
-
- x = fgb(mask);
- if (x >= 0) {
- *xfer_mode_out = xfer_mode_classes[i].base + x;
- *xfer_shift_out = shift;
- return 0;
- }
- }
-
- return -1;
-}
-
-/**
* ata_dev_set_xfermode - Issue SET FEATURES - XFER MODE command
* @ap: Port associated with device @dev
* @dev: Device to which command will be sent
--
1.2.4
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 6/6] libata: kill unused xfer_mode functions
2006-03-05 19:31 ` [PATCH 6/6] libata: kill unused xfer_mode functions Tejun Heo
@ 2006-03-05 19:35 ` Tejun Heo
0 siblings, 0 replies; 13+ messages in thread
From: Tejun Heo @ 2006-03-05 19:35 UTC (permalink / raw)
To: jgarzik, albertcc, alan, linux-ide
Preceding xfer_mask changes make the following functions unused.
ata_pio_modes(), base_from_shift(), ata_pr_blacklisted(), fgb()
Kill them. Also, as xfer_mode_str[] is now only used by
ata_mode_string(), move it into the function.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
This patch is against upstream.
drivers/scsi/libata-core.c | 108 +++++++-------------------------------------
1 files changed, 18 insertions(+), 90 deletions(-)
Index: work/drivers/scsi/libata-core.c
===================================================================
--- work.orig/drivers/scsi/libata-core.c 2006-03-06 04:14:11.000000000 +0900
+++ work/drivers/scsi/libata-core.c 2006-03-06 04:14:30.000000000 +0900
@@ -67,7 +67,6 @@ static void ata_set_mode(struct ata_port
static void ata_dev_set_xfermode(struct ata_port *ap, struct ata_device *dev);
static unsigned int ata_dev_xfermask(struct ata_port *ap,
struct ata_device *dev);
-static int fgb(u32 bitmap);
static unsigned int ata_unique_id = 1;
static struct workqueue_struct *ata_wq;
@@ -331,25 +330,6 @@ static int ata_xfer_mode2shift(unsigned
return -1;
}
-static const char * const xfer_mode_str[] = {
- "PIO0",
- "PIO1",
- "PIO2",
- "PIO3",
- "PIO4",
- "MWDMA0",
- "MWDMA1",
- "MWDMA2",
- "UDMA/16",
- "UDMA/25",
- "UDMA/33",
- "UDMA/44",
- "UDMA/66",
- "UDMA/100",
- "UDMA/133",
- "UDMA7",
-};
-
/**
* ata_mode_string - convert xfer_mask to string
* @xfer_mask: mask of bits supported; only highest bit counts.
@@ -364,9 +344,26 @@ static const char * const xfer_mode_str[
* Constant C string representing highest speed listed in
* @mode_mask, or the constant C string "<n/a>".
*/
-
static const char *ata_mode_string(unsigned int xfer_mask)
{
+ static const char * const xfer_mode_str[] = {
+ "PIO0",
+ "PIO1",
+ "PIO2",
+ "PIO3",
+ "PIO4",
+ "MWDMA0",
+ "MWDMA1",
+ "MWDMA2",
+ "UDMA/16",
+ "UDMA/25",
+ "UDMA/33",
+ "UDMA/44",
+ "UDMA/66",
+ "UDMA/100",
+ "UDMA/133",
+ "UDMA7",
+ };
int highbit;
highbit = fls(xfer_mask) - 1;
@@ -827,35 +824,6 @@ static unsigned int ata_id_xfermask(cons
return ata_pack_xfermask(pio_mask, mwdma_mask, udma_mask);
}
-/*
- * Compute the PIO modes available for this device. This is not as
- * trivial as it seems if we must consider early devices correctly.
- *
- * FIXME: pre IDE drive timing (do we care ?).
- */
-
-static unsigned int ata_pio_modes(const struct ata_device *adev)
-{
- u16 modes;
-
- /* Usual case. Word 53 indicates word 64 is valid */
- if (adev->id[ATA_ID_FIELD_VALID] & (1 << 1)) {
- modes = adev->id[ATA_ID_PIO_MODES] & 0x03;
- modes <<= 3;
- modes |= 0x7;
- return modes;
- }
-
- /* If word 64 isn't valid then Word 51 high byte holds the PIO timing
- number for the maximum. Turn it into a mask and return it */
- modes = (2 << ((adev->id[ATA_ID_OLD_PIO_MODES] >> 8) & 0xFF)) - 1 ;
- return modes;
- /* But wait.. there's more. Design your standards by committee and
- you too can get a free iordy field to process. However its the
- speeds not the modes that are supported... Note drivers using the
- timing API will get this right anyway */
-}
-
static inline void
ata_queue_packet_task(struct ata_port *ap)
{
@@ -1718,26 +1686,6 @@ int ata_timing_compute(struct ata_device
return 0;
}
-static const struct {
- unsigned int shift;
- u8 base;
-} xfer_mode_classes[] = {
- { ATA_SHIFT_UDMA, XFER_UDMA_0 },
- { ATA_SHIFT_MWDMA, XFER_MW_DMA_0 },
- { ATA_SHIFT_PIO, XFER_PIO_0 },
-};
-
-static u8 base_from_shift(unsigned int shift)
-{
- int i;
-
- for (i = 0; i < ARRAY_SIZE(xfer_mode_classes); i++)
- if (xfer_mode_classes[i].shift == shift)
- return xfer_mode_classes[i].base;
-
- return 0xff;
-}
-
static void ata_dev_set_mode(struct ata_port *ap, struct ata_device *dev)
{
if (!ata_dev_present(dev) || (ap->flags & ATA_FLAG_PORT_DISABLED))
@@ -2586,13 +2534,6 @@ int ata_dev_revalidate(struct ata_port *
return rc;
}
-static void ata_pr_blacklisted(const struct ata_port *ap,
- const struct ata_device *dev)
-{
- printk(KERN_WARNING "ata%u: dev %u is on DMA blacklist, disabling DMA\n",
- ap->id, dev->devno);
-}
-
static const char * const ata_dma_blacklist [] = {
"WDC AC11000H",
"WDC AC22100H",
@@ -2680,19 +2621,6 @@ static unsigned int ata_dev_xfermask(str
return xfer_mask;
}
-/* find greatest bit */
-static int fgb(u32 bitmap)
-{
- unsigned int i;
- int x = -1;
-
- for (i = 0; i < 32; i++)
- if (bitmap & (1 << i))
- x = i;
-
- return x;
-}
-
/**
* ata_dev_set_xfermode - Issue SET FEATURES - XFER MODE command
* @ap: Port associated with device @dev
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/6] libata: add xfer_mask handling functions
2006-03-05 19:31 ` [PATCH 2/6] libata: add xfer_mask handling functions Tejun Heo
@ 2006-03-07 5:51 ` Albert Lee
2006-03-07 6:47 ` Tejun Heo
2006-03-12 0:01 ` Jeff Garzik
1 sibling, 1 reply; 13+ messages in thread
From: Albert Lee @ 2006-03-07 5:51 UTC (permalink / raw)
To: Tejun Heo; +Cc: jgarzik, alan, linux-ide
Tejun Heo wrote:
> Add ata_pack_xfermask(), ata_xfer_mask2mode(), ata_xfer_mode2mask(),
> ata_xfer_mode2shift() and ata_id_xfermask(). These functions will be
> used by following patches to simplify xfer_mask handling.
>
> Signed-off-by: Tejun Heo <htejun@gmail.com>
>
> (snip)
> +/**
> + * ata_id_xfermask - Compute xfermask from the given IDENTIFY data
> + * @id: IDENTIFY data to compute xfer mask from
> + *
> + * Compute the xfermask for this device. This is not as trivial
> + * as it seems if we must consider early devices correctly.
> + *
> + * FIXME: pre IDE drive timing (do we care ?).
> + *
> + * LOCKING:
> + * None.
> + *
> + * RETURNS:
> + * Computed xfermask
> + */
> +static unsigned int ata_id_xfermask(const u16 *id)
> +{
> + unsigned int pio_mask, mwdma_mask, udma_mask;
> +
> + /* Usual case. Word 53 indicates word 64 is valid */
> + if (id[ATA_ID_FIELD_VALID] & (1 << 1)) {
> + pio_mask = id[ATA_ID_PIO_MODES] & 0x03;
> + pio_mask <<= 3;
> + pio_mask |= 0x7;
> + } else {
> + /* If word 64 isn't valid then Word 51 high byte holds
> + * the PIO timing number for the maximum. Turn it into
> + * a mask.
> + */
> + pio_mask = (2 << (id[ATA_ID_OLD_PIO_MODES] & 0xFF)) - 1 ;
> +
> + /* But wait.. there's more. Design your standards by
> + * committee and you too can get a free iordy field to
> + * process. However its the speeds not the modes that
> + * are supported... Note drivers using the timing API
> + * will get this right anyway
> + */
> + }
> +
> + mwdma_mask = id[ATA_ID_MWDMA_MODES] & 0x07;
> + udma_mask = id[ATA_ID_UDMA_MODES] & 0xff;
> +
> + return ata_pack_xfermask(pio_mask, mwdma_mask, udma_mask);
> +}
> +
> /*
We have ap->pio_mask, ap->mwdma_mask and ap->udma_mask.
Just thinking what if we have these masks in ata_device?
Maybe we can save some bitwise operations and make the code more intuitive to read?
Ex. ata_id_xfermask() can store the calculated masks to dev->pio_mask, dev->mwdma_mask, etc.
Ex. ata_mode_string() can take mode (XFER_UDMA_7..) as parameter and translate the given mode to string.
Ex. to print out the max mode support by the device,
1. ata_id_xfermask() calculates and saves the masks to dev->pio_mask, etc.
2. Another function, say, ata_dev_max_mode() takes dev as parameter,
packs the mode by ata_pack_xfermask() internally,
find the max mode supported by the drive,
then use ata_xfer_mask2mode() to return the mode.
3. ata_mode_string() translates the mode, say XFER_UDMA_7, to string literal.
Albert
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/6] libata: add xfer_mask handling functions
2006-03-07 5:51 ` Albert Lee
@ 2006-03-07 6:47 ` Tejun Heo
0 siblings, 0 replies; 13+ messages in thread
From: Tejun Heo @ 2006-03-07 6:47 UTC (permalink / raw)
To: albertl; +Cc: jgarzik, alan, linux-ide
Albert Lee wrote:
> Tejun Heo wrote:
>
>>Add ata_pack_xfermask(), ata_xfer_mask2mode(), ata_xfer_mode2mask(),
>>ata_xfer_mode2shift() and ata_id_xfermask(). These functions will be
>>used by following patches to simplify xfer_mask handling.
>>
>>Signed-off-by: Tejun Heo <htejun@gmail.com>
>>
>> (snip)
>>+/**
>>+ * ata_id_xfermask - Compute xfermask from the given IDENTIFY data
>>+ * @id: IDENTIFY data to compute xfer mask from
>>+ *
>>+ * Compute the xfermask for this device. This is not as trivial
>>+ * as it seems if we must consider early devices correctly.
>>+ *
>>+ * FIXME: pre IDE drive timing (do we care ?).
>>+ *
>>+ * LOCKING:
>>+ * None.
>>+ *
>>+ * RETURNS:
>>+ * Computed xfermask
>>+ */
>>+static unsigned int ata_id_xfermask(const u16 *id)
>>+{
>>+ unsigned int pio_mask, mwdma_mask, udma_mask;
>>+
>>+ /* Usual case. Word 53 indicates word 64 is valid */
>>+ if (id[ATA_ID_FIELD_VALID] & (1 << 1)) {
>>+ pio_mask = id[ATA_ID_PIO_MODES] & 0x03;
>>+ pio_mask <<= 3;
>>+ pio_mask |= 0x7;
>>+ } else {
>>+ /* If word 64 isn't valid then Word 51 high byte holds
>>+ * the PIO timing number for the maximum. Turn it into
>>+ * a mask.
>>+ */
>>+ pio_mask = (2 << (id[ATA_ID_OLD_PIO_MODES] & 0xFF)) - 1 ;
>>+
>>+ /* But wait.. there's more. Design your standards by
>>+ * committee and you too can get a free iordy field to
>>+ * process. However its the speeds not the modes that
>>+ * are supported... Note drivers using the timing API
>>+ * will get this right anyway
>>+ */
>>+ }
>>+
>>+ mwdma_mask = id[ATA_ID_MWDMA_MODES] & 0x07;
>>+ udma_mask = id[ATA_ID_UDMA_MODES] & 0xff;
>>+
>>+ return ata_pack_xfermask(pio_mask, mwdma_mask, udma_mask);
>>+}
>>+
>> /*
>
>
> We have ap->pio_mask, ap->mwdma_mask and ap->udma_mask.
> Just thinking what if we have these masks in ata_device?
We need those masks in both ap and dev. ap capability should be stored
in ap->*_mask and dev capabilities in dev->*_mask. e.g. speed limits due
to controller quirks should be done by limiting ap->*_mask in
->dev_config() while device speeding down due to transmission errors
should be done by limiting dev->*_mask.
> Maybe we can save some bitwise operations and make the code more intuitive to read?
>
> Ex. ata_id_xfermask() can store the calculated masks to dev->pio_mask, dev->mwdma_mask, etc.
This will be done when adding dev->*_mask.
> Ex. ata_mode_string() can take mode (XFER_UDMA_7..) as parameter and translate the given mode to string.
Hmmm... I don't know. My plan is to unify transfer mode handling to
unsigned int xfer_mask in libata core layer proper. It's easier to pass
around, mask, manipulate, etc... That's why I made ata_mode_string()
take xfer_mask instead of mode constants. But I'm okay either way.
Converting back and forth between xfermask and mode constant isn't
difficult.
> Ex. to print out the max mode support by the device,
> 1. ata_id_xfermask() calculates and saves the masks to dev->pio_mask, etc.
> 2. Another function, say, ata_dev_max_mode() takes dev as parameter,
> packs the mode by ata_pack_xfermask() internally,
> find the max mode supported by the drive,
> then use ata_xfer_mask2mode() to return the mode.
> 3. ata_mode_string() translates the mode, say XFER_UDMA_7, to string literal.
Hmm.. 1, 2 and 3 can just be done like the following.
printk("max speed=%s\n", ata_mode_string(ata_id_xfermask(id));
I think it's simpler this way. No?
--
tejun
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/6] libata: add xfer_mask handling functions
2006-03-05 19:31 ` [PATCH 2/6] libata: add xfer_mask handling functions Tejun Heo
2006-03-07 5:51 ` Albert Lee
@ 2006-03-12 0:01 ` Jeff Garzik
2006-03-12 3:34 ` [PATCH] libata: check Word 88 validity in ata_id_xfer_mask() Tejun Heo
1 sibling, 1 reply; 13+ messages in thread
From: Jeff Garzik @ 2006-03-12 0:01 UTC (permalink / raw)
To: Tejun Heo; +Cc: albertcc, alan, linux-ide
Tejun Heo wrote:
> +static unsigned int ata_id_xfermask(const u16 *id)
> +{
> + unsigned int pio_mask, mwdma_mask, udma_mask;
> +
> + /* Usual case. Word 53 indicates word 64 is valid */
> + if (id[ATA_ID_FIELD_VALID] & (1 << 1)) {
> + pio_mask = id[ATA_ID_PIO_MODES] & 0x03;
> + pio_mask <<= 3;
> + pio_mask |= 0x7;
> + } else {
> + /* If word 64 isn't valid then Word 51 high byte holds
> + * the PIO timing number for the maximum. Turn it into
> + * a mask.
> + */
> + pio_mask = (2 << (id[ATA_ID_OLD_PIO_MODES] & 0xFF)) - 1 ;
> +
> + /* But wait.. there's more. Design your standards by
> + * committee and you too can get a free iordy field to
> + * process. However its the speeds not the modes that
> + * are supported... Note drivers using the timing API
> + * will get this right anyway
> + */
> + }
> +
> + mwdma_mask = id[ATA_ID_MWDMA_MODES] & 0x07;
> + udma_mask = id[ATA_ID_UDMA_MODES] & 0xff;
Check Word 53 to see if Word 88 is valid.
Otherwise OK.
Jeff
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/6] libata: improve xfer mask constants and update ata_mode_string()
2006-03-05 19:31 ` [PATCH 1/6] libata: improve xfer mask constants and update ata_mode_string() Tejun Heo
@ 2006-03-12 0:03 ` Jeff Garzik
0 siblings, 0 replies; 13+ messages in thread
From: Jeff Garzik @ 2006-03-12 0:03 UTC (permalink / raw)
To: Tejun Heo; +Cc: albertcc, alan, linux-ide
Tejun Heo wrote:
> Add ATA_BITS_*, ATA_MASK_* macros and reorder xfer_mask fields such
> that higher transfer mode is placed at higher order bit. As thie
> reordering breaks ata_mode_string(), this patch also rewrites
> ata_mode_string().
>
> Signed-off-by: Tejun Heo <htejun@gmail.com>
applied 1-6, please send follow-up patch per comments on patch #2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] libata: check Word 88 validity in ata_id_xfer_mask()
2006-03-12 0:01 ` Jeff Garzik
@ 2006-03-12 3:34 ` Tejun Heo
0 siblings, 0 replies; 13+ messages in thread
From: Tejun Heo @ 2006-03-12 3:34 UTC (permalink / raw)
To: Jeff Garzik; +Cc: albertcc, alan, linux-ide
Check bit 2 of Word 53 for Word 88 validity before using Word 88 to
determine UDMA mask. Note that the original xfer mask implementation
using ata_get_mode_mask() didn't consider bit 2 of Word 53. This
patch introduces different (correct) behavior.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
libata-core.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 42d43b5..e2529e9 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -819,7 +819,10 @@ static unsigned int ata_id_xfermask(cons
}
mwdma_mask = id[ATA_ID_MWDMA_MODES] & 0x07;
- udma_mask = id[ATA_ID_UDMA_MODES] & 0xff;
+
+ udma_mask = 0;
+ if (id[ATA_ID_FIELD_VALID] & (1 << 2))
+ udma_mask = id[ATA_ID_UDMA_MODES] & 0xff;
return ata_pack_xfermask(pio_mask, mwdma_mask, udma_mask);
}
^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2006-03-12 3:34 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-03-05 19:31 [PATCHSET] libata: improve transfer mode handling Tejun Heo
2006-03-05 19:31 ` [PATCH 1/6] libata: improve xfer mask constants and update ata_mode_string() Tejun Heo
2006-03-12 0:03 ` Jeff Garzik
2006-03-05 19:31 ` [PATCH 2/6] libata: add xfer_mask handling functions Tejun Heo
2006-03-07 5:51 ` Albert Lee
2006-03-07 6:47 ` Tejun Heo
2006-03-12 0:01 ` Jeff Garzik
2006-03-12 3:34 ` [PATCH] libata: check Word 88 validity in ata_id_xfer_mask() Tejun Heo
2006-03-05 19:31 ` [PATCH 3/6] libata: use ata_id_xfermask() in ata_dev_configure() Tejun Heo
2006-03-05 19:31 ` [PATCH 5/6] libata: reimplement ata_set_mode() using xfer_mask helpers Tejun Heo
2006-03-05 19:31 ` [PATCH 4/6] libata: use xfer_mask helpers in ata_dev_set_mode() Tejun Heo
2006-03-05 19:31 ` [PATCH 6/6] libata: kill unused xfer_mode functions Tejun Heo
2006-03-05 19:35 ` Tejun Heo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).