* [PATCHSET] libata: improve transfer mode handling
@ 2006-03-05 19:31 Tejun Heo
2006-03-05 19:31 ` [PATCH 2/6] libata: add xfer_mask handling functions 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 ` 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
` (4 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 ` [PATCH 2/6] libata: add xfer_mask handling functions 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 ` Tejun Heo
2006-03-12 0:03 ` Jeff Garzik
2006-03-05 19:31 ` [PATCH 4/6] libata: use xfer_mask helpers in ata_dev_set_mode() Tejun Heo
` (2 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 2/6] libata: add xfer_mask handling functions Tejun Heo
@ 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
` (3 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
` (3 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
2006-03-05 19:31 ` [PATCH 5/6] libata: reimplement ata_set_mode() using xfer_mask helpers 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
` (2 preceding siblings ...)
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-05 19:31 ` [PATCH 6/6] libata: kill unused xfer_mode functions Tejun Heo
2006-03-05 19:31 ` [PATCH 5/6] libata: reimplement ata_set_mode() using xfer_mask helpers 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
` (4 preceding siblings ...)
2006-03-05 19:31 ` [PATCH 6/6] libata: kill unused xfer_mode functions Tejun Heo
@ 2006-03-05 19:31 ` 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 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 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 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
2006-03-05 19:31 ` [PATCH 5/6] libata: reimplement ata_set_mode() using xfer_mask helpers Tejun Heo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.