libata: update timing and fix pata_amd transfer mode selection

PATCH In review Tejun Heo 3 participants 38 messages Nov 6, 2007 – Nov 27, 2007 libata

lore

Share & tools

lore

Tejun Heo

author

2007-11-06 05:39 UTC #0 lore raw

PATCH 1/1 libata: update timing and fix pata_amd transfer mode selection

Hello,

This patchset cleans up and improves PATA timing related code and fix pata_amd transfer mode selection on top of the improvements. This patchset contains the following tweleve patches.

0001-ata_generic-unindent-loop-in-generic_set_mode.patch 0002-libata-export-xfermode-PATA-timing-related-functi.patch 0003-libata-clean-up-xfermode-PATA-timing-related-stuf.patch 0004-libata-kill-ata_id_to_dma_mode.patch 0005-libata-xfer_mask-is-unsigned-int-not-unsigned-long.patch 0006-libata-separate-out-ata_acpi_gtm_xfermask-from-pa.patch 0007-libata-fix-ata_acpi_gtm_xfermask.patch 0008-libata-implement-ata_timing_cycle2mode-and-use-it.patch 0009-libata-implement-ata_acpi_init_gtm.patch 0010-libata-reimplement-ata_acpi_cbl_80wire-using-ata_.patch 0011-libata-add-ATA_CBL_PATA_IGN.patch 0012-pata_amd-update-mode-selection-for-NV-PATAs.patch

0001-0005 are clean ups of timing handling code. 0006-0008 unifies timing handling code in libata-acpi.c and pata_acpi.c with the core timing code.

0009 implements initial GTM caching. I thought about moving this to LLDs but it's too much hassle. GTM is host wide and doing it from ->error_handler would require cross-port synchronization. Left alternative is adding a separate hook. Simply doing it during ACPI attach is simpler.

0010 reimplements ata_acpi_cbl_80wire() using ata_acpi_gtm_xfermask(). This new function takes @gtm argument. Both users (pata_via and pata_amd) are converted to pass initial GTM.

0011 implements ATA_CBL_PATA_IGN which tells libata to ignore cable type and supporess all related handling.

0012 updates mode selection for NV PATA controllers. We just can't tell what cable is attached on the controller either from host or drive side. So, for those controllers, pata_amd just sets cable type to ATA_CBL_PATA_IGN and use ->mode_filter to limit transfer mode according to BIOS/ACPI boot configuration. If that fails (not likely), pata_amd simply sets the highest allowed speed and let EH figure out the mess. With recent updates, EH should be able to figure it out pretty soon.

This patchset is against

#upstream-fixes (6bbfd53d47abd1fb20d7c93a9b19a75970b66f49)
+ update-speeddown patchset [1]

Thanks.

--
tejun

[1] http://thread.gmane.org/gmane.linux.ide/24497

Tejun Heo

author

2007-11-06 05:39 UTC #1 lore raw

PATCH 1/12 ata_generic: unindent loop in generic_set_mode()

Unindent loop body in generic_set_mode(). This is to ease future change.

Signed-off-by Tejun Heo <[email protected]>

---
 drivers/ata/ata_generic.c |   31 ++++++++++++++++---------------
 1 files changed, 16 insertions(+), 15 deletions(-)

drivers/ata/ata_generic.c +16 ~~-15~~

index 9032998..fb5434c 100644
@@ -54,21 +54,22 @@ static int generic_set_mode(struct ata_link *link, struct ata_device **unused)
 		dma_enabled = ioread8(ap->ioaddr.bmdma_addr + ATA_DMA_STATUS);
 
 	ata_link_for_each_dev(dev, link) {
-		if (ata_dev_enabled(dev)) {
-			/* We don't really care */
-			dev->pio_mode = XFER_PIO_0;
-			dev->dma_mode = XFER_MW_DMA_0;
-			/* We do need the right mode information for DMA or PIO
-			   and this comes from the current configuration flags */
-			if (dma_enabled & (1 << (5 + dev->devno))) {
-				ata_id_to_dma_mode(dev, XFER_MW_DMA_0);
-				dev->flags &= ~ATA_DFLAG_PIO;
-			} else {
-				ata_dev_printk(dev, KERN_INFO, "configured for PIO\n");
-				dev->xfer_mode = XFER_PIO_0;
-				dev->xfer_shift = ATA_SHIFT_PIO;
-				dev->flags |= ATA_DFLAG_PIO;
-			}
+		if (!ata_dev_enabled(dev))
+			continue;
+
+		/* We don't really care */
+		dev->pio_mode = XFER_PIO_0;
+		dev->dma_mode = XFER_MW_DMA_0;
+		/* We do need the right mode information for DMA or PIO
+		   and this comes from the current configuration flags */
+		if (dma_enabled & (1 << (5 + dev->devno))) {
+			ata_id_to_dma_mode(dev, XFER_MW_DMA_0);
+			dev->flags &= ~ATA_DFLAG_PIO;
+		} else {
+			ata_dev_printk(dev, KERN_INFO, "configured for PIO\n");
+			dev->xfer_mode = XFER_PIO_0;
+			dev->xfer_shift = ATA_SHIFT_PIO;
+			dev->flags |= ATA_DFLAG_PIO;
 		}
 	}
 	return 0;

Signature · 1 line

1.5.2.4

Tejun Heo

author

2007-11-06 05:39 UTC #2 lore raw

PATCH 2/12 libata: export xfermode / PATA timing related functions

In reply to Tejun Heo

Export the following xfermode related functions.

* ata_pack_xfermask()
* ata_unpack_xfermask()
* ata_xfer_mask2mode()
* ata_xfer_mode2mask()
* ata_xfer_mode2shift()
* ata_mode_string()
* ata_id_xfermask()
* ata_timing_find_mode()

These functions will be used later by LLD updates. While at it, change unsigned short @speed to u8 @xfer_mode in ata_timing_find_mode() for consistency.

Signed-off-by Tejun Heo <[email protected]>

---
 drivers/ata/libata-core.c |   33 +++++++++++++++++++--------------
 include/linux/libata.h    |   10 ++++++++++
 2 files changed, 29 insertions(+), 14 deletions(-)

drivers/ata/libata-core.c +19 ~~-14~~

index bd77927..d4506f8 100644
@@ -441,9 +441,8 @@ int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev,
  *	RETURNS:
  *	Packed xfer_mask.
  */
-static unsigned int ata_pack_xfermask(unsigned int pio_mask,
-				      unsigned int mwdma_mask,
-				      unsigned int udma_mask)
+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) |

@@ -460,10 +459,8 @@ static unsigned int ata_pack_xfermask(unsigned int pio_mask,
  *	Unpack @xfer_mask into @pio_mask, @mwdma_mask and @udma_mask.
  *	Any NULL distination masks will be ignored.
  */
-static void ata_unpack_xfermask(unsigned int xfer_mask,
-				unsigned int *pio_mask,
-				unsigned int *mwdma_mask,
-				unsigned int *udma_mask)
+void ata_unpack_xfermask(unsigned int xfer_mask, unsigned int *pio_mask,
+			 unsigned int *mwdma_mask, unsigned int *udma_mask)
 {
 	if (pio_mask)
 		*pio_mask = (xfer_mask & ATA_MASK_PIO) >> ATA_SHIFT_PIO;

@@ -496,7 +493,7 @@ static const struct ata_xfer_ent {
  *	RETURNS:
  *	Matching XFER_* value, 0 if no match found.
  */
-static u8 ata_xfer_mask2mode(unsigned int xfer_mask)
+u8 ata_xfer_mask2mode(unsigned int xfer_mask)
 {
 	int highbit = fls(xfer_mask) - 1;
 	const struct ata_xfer_ent *ent;

@@ -519,7 +516,7 @@ static u8 ata_xfer_mask2mode(unsigned int xfer_mask)
  *	RETURNS:
  *	Matching xfer_mask, 0 if no match found.
  */
-static unsigned int ata_xfer_mode2mask(u8 xfer_mode)
+unsigned int ata_xfer_mode2mask(u8 xfer_mode)
 {
 	const struct ata_xfer_ent *ent;

@@ -541,7 +538,7 @@ static unsigned int ata_xfer_mode2mask(u8 xfer_mode)
  *	RETURNS:
  *	Matching xfer_shift, -1 if no match found.
  */
-static int ata_xfer_mode2shift(unsigned int xfer_mode)
+int ata_xfer_mode2shift(unsigned int xfer_mode)
 {
 	const struct ata_xfer_ent *ent;

@@ -565,7 +562,7 @@ static int ata_xfer_mode2shift(unsigned int xfer_mode)
  *	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)
+const char *ata_mode_string(unsigned int xfer_mask)
 {
 	static const char * const xfer_mode_str[] = {
 		"PIO0",

@@ -1454,7 +1451,7 @@ static inline void ata_dump_id(const u16 *id)
  *	RETURNS:
  *	Computed xfermask
  */
-static unsigned int ata_id_xfermask(const u16 *id)
+unsigned int ata_id_xfermask(const u16 *id)
 {
 	unsigned int pio_mask, mwdma_mask, udma_mask;

@@ -2908,11 +2905,11 @@ void ata_timing_merge(const struct ata_timing *a, const struct ata_timing *b,
 	if (what & ATA_TIMING_UDMA   ) m->udma    = max(a->udma,    b->udma);
 }
 
-static const struct ata_timing *ata_timing_find_mode(unsigned short speed)
+const struct ata_timing *ata_timing_find_mode(u8 xfer_mode)
 {
 	const struct ata_timing *t;
 
-	for (t = ata_timing; t->mode != speed; t++)
+	for (t = ata_timing; t->mode != xfer_mode; t++)
 		if (t->mode == 0xFF)
 			return NULL;
 	return t;

@@ -7578,6 +7575,13 @@ EXPORT_SYMBOL_GPL(ata_std_dev_select);
 EXPORT_SYMBOL_GPL(sata_print_link_status);
 EXPORT_SYMBOL_GPL(ata_tf_to_fis);
 EXPORT_SYMBOL_GPL(ata_tf_from_fis);
+EXPORT_SYMBOL_GPL(ata_pack_xfermask);
+EXPORT_SYMBOL_GPL(ata_unpack_xfermask);
+EXPORT_SYMBOL_GPL(ata_xfer_mask2mode);
+EXPORT_SYMBOL_GPL(ata_xfer_mode2mask);
+EXPORT_SYMBOL_GPL(ata_xfer_mode2shift);
+EXPORT_SYMBOL_GPL(ata_mode_string);
+EXPORT_SYMBOL_GPL(ata_id_xfermask);
 EXPORT_SYMBOL_GPL(ata_check_status);
 EXPORT_SYMBOL_GPL(ata_altstatus);
 EXPORT_SYMBOL_GPL(ata_exec_command);

@@ -7645,6 +7649,7 @@ EXPORT_SYMBOL_GPL(ata_id_to_dma_mode);
 EXPORT_SYMBOL_GPL(ata_scsi_simulate);
 
 EXPORT_SYMBOL_GPL(ata_pio_need_iordy);
+EXPORT_SYMBOL_GPL(ata_timing_find_mode);
 EXPORT_SYMBOL_GPL(ata_timing_compute);
 EXPORT_SYMBOL_GPL(ata_timing_merge);

include/linux/libata.h +10 -0

index 3609c07..2198a61 100644
@@ -849,6 +849,15 @@ extern void ata_tf_read(struct ata_port *ap, struct ata_taskfile *tf);
 extern void ata_tf_to_fis(const struct ata_taskfile *tf,
 			  u8 pmp, int is_cmd, u8 *fis);
 extern void ata_tf_from_fis(const u8 *fis, struct ata_taskfile *tf);
+extern unsigned int ata_pack_xfermask(unsigned int pio_mask,
+			unsigned int mwdma_mask, unsigned int udma_mask);
+extern void ata_unpack_xfermask(unsigned int xfer_mask, unsigned int *pio_mask,
+			unsigned int *mwdma_mask, unsigned int *udma_mask);
+extern u8 ata_xfer_mask2mode(unsigned int xfer_mask);
+extern unsigned int ata_xfer_mode2mask(u8 xfer_mode);
+extern int ata_xfer_mode2shift(unsigned int xfer_mode);
+extern const char *ata_mode_string(unsigned int xfer_mask);
+extern unsigned int ata_id_xfermask(const u16 *id);
 extern void ata_noop_dev_select(struct ata_port *ap, unsigned int device);
 extern void ata_std_dev_select(struct ata_port *ap, unsigned int device);
 extern u8 ata_check_status(struct ata_port *ap);

@@ -918,6 +927,7 @@ extern int ata_cable_unknown(struct ata_port *ap);
  */
 
 extern unsigned int ata_pio_need_iordy(const struct ata_device *);
+extern const struct ata_timing *ata_timing_find_mode(u8 xfer_mode);
 extern int ata_timing_compute(struct ata_device *, unsigned short,
 			      struct ata_timing *, int, int);
 extern void ata_timing_merge(const struct ata_timing *,

Signature · 1 line

1.5.2.4

Tejun Heo

author

2007-11-06 05:39 UTC #3 lore raw

PATCH 3/12 libata: clean up xfermode / PATA timing related stuff

In reply to Tejun Heo

* s/ATA_BITS_(PIO|MWDMA|UDMA)/ATA_NR_\1_MODES/g

* Consistently use 0xff to indicate invalid transfer mode (0x00 is valid for PIO_SLOW).

* Make ata_xfer_mode2mask() return proper mode mask instead of just the highest bit.

* Sort ata_timing table in increasing xfermode order and update ata_timing_find_mode() accordingly.

This patch doesn't introduce any behavior change.

Signed-off-by Tejun Heo <[email protected]>

---
 drivers/ata/libata-core.c |   82 ++++++++++++++++++++++-----------------------
 include/linux/libata.h    |   21 ++++++-----
 2 files changed, 52 insertions(+), 51 deletions(-)

drivers/ata/libata-core.c +40 ~~-42~~

index d4506f8..dc7e2f1 100644
@@ -474,9 +474,9 @@ static const struct ata_xfer_ent {
 	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 },
+	{ ATA_SHIFT_PIO, ATA_NR_PIO_MODES, XFER_PIO_0 },
+	{ ATA_SHIFT_MWDMA, ATA_NR_MWDMA_MODES, XFER_MW_DMA_0 },
+	{ ATA_SHIFT_UDMA, ATA_NR_UDMA_MODES, XFER_UDMA_0 },
 	{ -1, },
 };

@@ -491,7 +491,7 @@ static const struct ata_xfer_ent {
  *	None.
  *
  *	RETURNS:
- *	Matching XFER_* value, 0 if no match found.
+ *	Matching XFER_* value, 0xff if no match found.
  */
 u8 ata_xfer_mask2mode(unsigned int xfer_mask)
 {

@@ -501,7 +501,7 @@ u8 ata_xfer_mask2mode(unsigned int xfer_mask)
 	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;
+	return 0xff;
 }
 
 /**

@@ -522,7 +522,8 @@ unsigned int ata_xfer_mode2mask(u8 xfer_mode)
 
 	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 ((2 << (ent->shift + xfer_mode - ent->base)) - 1)
+				& ~((1 << ent->shift) - 1);
 	return 0;
 }

@@ -1300,7 +1301,7 @@ void ata_id_to_dma_mode(struct ata_device *dev, u8 unknown)
 	/* Select the mode in use */
 	mode = ata_xfer_mask2mode(mask);
 
-	if (mode != 0) {
+	if (mode != 0xff) {
 		ata_dev_printk(dev, KERN_INFO, "configured for %s\n",
 		       ata_mode_string(mask));
 	} else {

@@ -2841,38 +2842,33 @@ int sata_set_spd(struct ata_link *link)
  */
 
 static const struct ata_timing ata_timing[] = {
+/*	{ XFER_PIO_SLOW, 120, 290, 240, 960, 290, 240, 960,   0 }, */
+	{ XFER_PIO_0,     70, 290, 240, 600, 165, 150, 600,   0 },
+	{ XFER_PIO_1,     50, 290,  93, 383, 125, 100, 383,   0 },
+	{ XFER_PIO_2,     30, 290,  40, 330, 100,  90, 240,   0 },
+	{ XFER_PIO_3,     30,  80,  70, 180,  80,  70, 180,   0 },
+	{ XFER_PIO_4,     25,  70,  25, 120,  70,  25, 120,   0 },
+	{ XFER_PIO_5,     15,  65,  25, 100,  65,  25, 100,   0 },
+	{ XFER_PIO_6,     10,  55,  20,  80,  55,  20,  80,   0 },
 
-	{ XFER_UDMA_6,     0,   0,   0,   0,   0,   0,   0,  15 },
-	{ XFER_UDMA_5,     0,   0,   0,   0,   0,   0,   0,  20 },
-	{ XFER_UDMA_4,     0,   0,   0,   0,   0,   0,   0,  30 },
-	{ XFER_UDMA_3,     0,   0,   0,   0,   0,   0,   0,  45 },
+	{ XFER_SW_DMA_0, 120,   0,   0,   0, 480, 480, 960,   0 },
+	{ XFER_SW_DMA_1,  90,   0,   0,   0, 240, 240, 480,   0 },
+	{ XFER_SW_DMA_2,  60,   0,   0,   0, 120, 120, 240,   0 },
 
-	{ XFER_MW_DMA_4,  25,   0,   0,   0,  55,  20,  80,   0 },
+	{ XFER_MW_DMA_0,  60,   0,   0,   0, 215, 215, 480,   0 },
+	{ XFER_MW_DMA_1,  45,   0,   0,   0,  80,  50, 150,   0 },
+	{ XFER_MW_DMA_2,  25,   0,   0,   0,  70,  25, 120,   0 },
 	{ XFER_MW_DMA_3,  25,   0,   0,   0,  65,  25, 100,   0 },
-	{ XFER_UDMA_2,     0,   0,   0,   0,   0,   0,   0,  60 },
-	{ XFER_UDMA_1,     0,   0,   0,   0,   0,   0,   0,  80 },
-	{ XFER_UDMA_0,     0,   0,   0,   0,   0,   0,   0, 120 },
+	{ XFER_MW_DMA_4,  25,   0,   0,   0,  55,  20,  80,   0 },
 
 /*	{ XFER_UDMA_SLOW,  0,   0,   0,   0,   0,   0,   0, 150 }, */
-
-	{ XFER_MW_DMA_2,  25,   0,   0,   0,  70,  25, 120,   0 },
-	{ XFER_MW_DMA_1,  45,   0,   0,   0,  80,  50, 150,   0 },
-	{ XFER_MW_DMA_0,  60,   0,   0,   0, 215, 215, 480,   0 },
-
-	{ XFER_SW_DMA_2,  60,   0,   0,   0, 120, 120, 240,   0 },
-	{ XFER_SW_DMA_1,  90,   0,   0,   0, 240, 240, 480,   0 },
-	{ XFER_SW_DMA_0, 120,   0,   0,   0, 480, 480, 960,   0 },
-
-	{ XFER_PIO_6,     10,  55,  20,  80,  55,  20,  80,   0 },
-	{ XFER_PIO_5,     15,  65,  25, 100,  65,  25, 100,   0 },
-	{ XFER_PIO_4,     25,  70,  25, 120,  70,  25, 120,   0 },
-	{ XFER_PIO_3,     30,  80,  70, 180,  80,  70, 180,   0 },
-
-	{ XFER_PIO_2,     30, 290,  40, 330, 100,  90, 240,   0 },
-	{ XFER_PIO_1,     50, 290,  93, 383, 125, 100, 383,   0 },
-	{ XFER_PIO_0,     70, 290, 240, 600, 165, 150, 600,   0 },
-
-/*	{ XFER_PIO_SLOW, 120, 290, 240, 960, 290, 240, 960,   0 }, */
+	{ XFER_UDMA_0,     0,   0,   0,   0,   0,   0,   0, 120 },
+	{ XFER_UDMA_1,     0,   0,   0,   0,   0,   0,   0,  80 },
+	{ XFER_UDMA_2,     0,   0,   0,   0,   0,   0,   0,  60 },
+	{ XFER_UDMA_3,     0,   0,   0,   0,   0,   0,   0,  45 },
+	{ XFER_UDMA_4,     0,   0,   0,   0,   0,   0,   0,  30 },
+	{ XFER_UDMA_5,     0,   0,   0,   0,   0,   0,   0,  20 },
+	{ XFER_UDMA_6,     0,   0,   0,   0,   0,   0,   0,  15 },
 
 	{ 0xFF }
 };

@@ -2907,12 +2903,14 @@ void ata_timing_merge(const struct ata_timing *a, const struct ata_timing *b,
 
 const struct ata_timing *ata_timing_find_mode(u8 xfer_mode)
 {
-	const struct ata_timing *t;
+	const struct ata_timing *t = ata_timing;
+
+	while (xfer_mode > t->mode)
+		t++;
 
-	for (t = ata_timing; t->mode != xfer_mode; t++)
-		if (t->mode == 0xFF)
-			return NULL;
-	return t;
+	if (xfer_mode == t->mode)
+		return t;
+	return NULL;
 }
 
 int ata_timing_compute(struct ata_device *adev, unsigned short speed,

@@ -3175,7 +3173,7 @@ int ata_do_set_mode(struct ata_link *link, struct ata_device **r_failed_dev)
 		dev->dma_mode = ata_xfer_mask2mode(dma_mask);
 
 		found = 1;
-		if (dev->dma_mode)
+		if (dev->dma_mode != 0xff)
 			used_dma = 1;
 	}
 	if (!found)

@@ -3186,7 +3184,7 @@ int ata_do_set_mode(struct ata_link *link, struct ata_device **r_failed_dev)
 		if (!ata_dev_enabled(dev))
 			continue;
 
-		if (!dev->pio_mode) {
+		if (dev->pio_mode == 0xff) {
 			ata_dev_printk(dev, KERN_WARNING, "no PIO support\n");
 			rc = -EINVAL;
 			goto out;

@@ -3200,7 +3198,7 @@ int ata_do_set_mode(struct ata_link *link, struct ata_device **r_failed_dev)
 
 	/* step 3: set host DMA timings */
 	ata_link_for_each_dev(dev, link) {
-		if (!ata_dev_enabled(dev) || !dev->dma_mode)
+		if (!ata_dev_enabled(dev) || dev->dma_mode == 0xff)
 			continue;
 
 		dev->xfer_mode = dev->dma_mode;

include/linux/libata.h +12 -9

index 2198a61..4deb8a7 100644
@@ -267,17 +267,20 @@ enum {
 	/* encoding various smaller bitmaps into a single
 	 * unsigned int bitmap
 	 */
-	ATA_BITS_PIO		= 7,
-	ATA_BITS_MWDMA		= 5,
-	ATA_BITS_UDMA		= 8,
+	ATA_NR_PIO_MODES	= 7,
+	ATA_NR_MWDMA_MODES	= 5,
+	ATA_NR_UDMA_MODES	= 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,
+	ATA_SHIFT_MWDMA		= ATA_SHIFT_PIO + ATA_NR_PIO_MODES,
+	ATA_SHIFT_UDMA		= ATA_SHIFT_MWDMA + ATA_NR_MWDMA_MODES,
+
+	ATA_MASK_PIO		= ((1 << ATA_NR_PIO_MODES) - 1)
+					<< ATA_SHIFT_PIO,
+	ATA_MASK_MWDMA		= ((1 << ATA_NR_MWDMA_MODES) - 1)
+					<< ATA_SHIFT_MWDMA,
+	ATA_MASK_UDMA		= ((1 << ATA_NR_UDMA_MODES) - 1)
+					<< ATA_SHIFT_UDMA,
 
 	/* size of buffer to pad xfers ending on unaligned boundaries */
 	ATA_DMA_PAD_SZ		= 4,

Signature · 1 line

1.5.2.4

Tejun Heo

author

2007-11-06 05:39 UTC #4 lore raw

PATCH 4/12 libata: kill ata_id_to_dma_mode()

In reply to Tejun Heo

ata_id_to_dma_mode() isn't quite generic. The function is basically privately implemented ata_id_xfermask() combined with hardcoded mode printing and configuration which are specific to ata_generic.

Kill the function and open code it in generic_set_mode() using generic xfermode handling functions.

Signed-off-by Tejun Heo <[email protected]>

---
 drivers/ata/ata_generic.c |   17 ++++++++++++++++-
 drivers/ata/libata-core.c |   43 -------------------------------------------
 include/linux/libata.h    |    1 -
 3 files changed, 16 insertions(+), 45 deletions(-)

drivers/ata/ata_generic.c +16 -1

index fb5434c..d11a4f1 100644
@@ -63,7 +63,22 @@ static int generic_set_mode(struct ata_link *link, struct ata_device **unused)
 		/* We do need the right mode information for DMA or PIO
 		   and this comes from the current configuration flags */
 		if (dma_enabled & (1 << (5 + dev->devno))) {
-			ata_id_to_dma_mode(dev, XFER_MW_DMA_0);
+			unsigned int xfer_mask = ata_id_xfermask(dev->id);
+			const char *name;
+
+			if (xfer_mask & (ATA_MASK_MWDMA | ATA_MASK_UDMA))
+				name = ata_mode_string(xfer_mask);
+			else {
+				/* SWDMA perhaps? */
+				name = "DMA";
+				xfer_mask |= ata_xfer_mode2mask(XFER_MW_DMA_0);
+			}
+
+			ata_dev_printk(dev, KERN_INFO, "configured for %s\n",
+				       name);
+
+			dev->xfer_mode = ata_xfer_mask2mode(xfer_mask);
+			dev->xfer_shift = ata_xfer_mode2shift(dev->xfer_mode);
 			dev->flags &= ~ATA_DFLAG_PIO;
 		} else {
 			ata_dev_printk(dev, KERN_INFO, "configured for PIO\n");

drivers/ata/libata-core.c +0 ~~-43~~

55 lines not on this page · raw message

include/linux/libata.h +0 -1

index 4deb8a7..f84106a 100644
@@ -888,7 +888,6 @@ extern void ata_id_string(const u16 *id, unsigned char *s,
 			  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_id_to_dma_mode(struct ata_device *dev, u8 unknown);
 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);

Signature · 1 line

1.5.2.4

Tejun Heo

author

2007-11-06 05:39 UTC #8 lore raw

PATCH 5/12 libata: xfer_mask is unsigned int not unsigned long

In reply to Tejun Heo

xfer_mask is unsigned int not unsigned long. Change ->mode_filter to take and return unsigned int.

While at it, rename @adev of ata_pci_default_filter() to @dev for consistency.

Signed-off-by Tejun Heo <[email protected]>

---
 drivers/ata/libata-sff.c       |    5 +++--
 drivers/ata/pata_acpi.c        |    2 +-
 drivers/ata/pata_ali.c         |    2 +-
 drivers/ata/pata_hpt366.c      |    2 +-
 drivers/ata/pata_hpt37x.c      |    4 ++--
 drivers/ata/pata_pdc2027x.c    |    4 ++--
 drivers/ata/pata_scc.c         |    2 +-
 drivers/ata/pata_serverworks.c |    4 ++--
 include/linux/libata.h         |    5 +++--
 9 files changed, 16 insertions(+), 14 deletions(-)

drivers/ata/libata-sff.c +3 -2

index 48acc09..2f2ca3d 100644
@@ -877,12 +877,13 @@ int ata_pci_clear_simplex(struct pci_dev *pdev)
 	return 0;
 }
 
-unsigned long ata_pci_default_filter(struct ata_device *adev, unsigned long xfer_mask)
+unsigned int ata_pci_default_filter(struct ata_device *dev,
+				    unsigned int xfer_mask)
 {
 	/* Filter out DMA modes if the device has been configured by
 	   the BIOS as PIO only */
 
-	if (adev->link->ap->ioaddr.bmdma_addr == NULL)
+	if (dev->link->ap->ioaddr.bmdma_addr == NULL)
 		xfer_mask &= ~(ATA_MASK_MWDMA | ATA_MASK_UDMA);
 	return xfer_mask;
 }

drivers/ata/pata_acpi.c +1 -1

index e4542ab..2323646 100644
@@ -164,7 +164,7 @@ static unsigned long pacpi_discover_modes(struct ata_port *ap, struct ata_device
  *	this case the list of discovered valid modes obtained by ACPI probing
  */
 
-static unsigned long pacpi_mode_filter(struct ata_device *adev, unsigned long mask)
+static unsigned int pacpi_mode_filter(struct ata_device *adev, unsigned int mask)
 {
 	struct pata_acpi *acpi = adev->link->ap->private_data;
 	return ata_pci_default_filter(adev, mask & acpi->mask[adev->devno]);

drivers/ata/pata_ali.c +1 -1

index 364534e..aae8a44 100644
@@ -105,7 +105,7 @@ static int ali_c2_cable_detect(struct ata_port *ap)
  *	fix that later on. Also ensure we do not do UDMA on WDC drives
  */
 
-static unsigned long ali_20_filter(struct ata_device *adev, unsigned long mask)
+static unsigned int ali_20_filter(struct ata_device *adev, unsigned int mask)
 {
 	char model_num[ATA_ID_PROD_LEN + 1];
 	/* No DMA on anything but a disk for now */

drivers/ata/pata_hpt366.c +1 -1

index 0713872..1f277d5 100644
@@ -174,7 +174,7 @@ static int hpt_dma_blacklisted(const struct ata_device *dev, char *modestr, cons
  *	Block UDMA on devices that cause trouble with this controller.
  */
 
-static unsigned long hpt366_filter(struct ata_device *adev, unsigned long mask)
+static unsigned int hpt366_filter(struct ata_device *adev, unsigned int mask)
 {
 	if (adev->class == ATA_DEV_ATA) {
 		if (hpt_dma_blacklisted(adev, "UDMA",  bad_ata33))

drivers/ata/pata_hpt37x.c +2 -2

index 3816b86..37f327e 100644
@@ -275,7 +275,7 @@ static const char *bad_ata100_5[] = {
  *	Block UDMA on devices that cause trouble with this controller.
  */
 
-static unsigned long hpt370_filter(struct ata_device *adev, unsigned long mask)
+static unsigned int hpt370_filter(struct ata_device *adev, unsigned int mask)
 {
 	if (adev->class == ATA_DEV_ATA) {
 		if (hpt_dma_blacklisted(adev, "UDMA", bad_ata33))

@@ -293,7 +293,7 @@ static unsigned long hpt370_filter(struct ata_device *adev, unsigned long mask)
  *	Block UDMA on devices that cause trouble with this controller.
  */
 
-static unsigned long hpt370a_filter(struct ata_device *adev, unsigned long mask)
+static unsigned int hpt370a_filter(struct ata_device *adev, unsigned int mask)
 {
 	if (adev->class == ATA_DEV_ATA) {
 		if (hpt_dma_blacklisted(adev, "UDMA100", bad_ata100_5))

drivers/ata/pata_pdc2027x.c +2 -2

index 2622577..dd20239 100644
@@ -67,7 +67,7 @@ static void pdc2027x_error_handler(struct ata_port *ap);
 static void pdc2027x_set_piomode(struct ata_port *ap, struct ata_device *adev);
 static void pdc2027x_set_dmamode(struct ata_port *ap, struct ata_device *adev);
 static int pdc2027x_check_atapi_dma(struct ata_queued_cmd *qc);
-static unsigned long pdc2027x_mode_filter(struct ata_device *adev, unsigned long mask);
+static unsigned int pdc2027x_mode_filter(struct ata_device *adev, unsigned int mask);
 static int pdc2027x_cable_detect(struct ata_port *ap);
 static int pdc2027x_set_mode(struct ata_link *link, struct ata_device **r_failed);

@@ -336,7 +336,7 @@ static void pdc2027x_error_handler(struct ata_port *ap)
  *	Block UDMA on devices that cause trouble with this controller.
  */
 
-static unsigned long pdc2027x_mode_filter(struct ata_device *adev, unsigned long mask)
+static unsigned int pdc2027x_mode_filter(struct ata_device *adev, unsigned int mask)
 {
 	unsigned char model_num[ATA_ID_PROD_LEN + 1];
 	struct ata_device *pair = ata_dev_pair(adev);

drivers/ata/pata_scc.c +1 -1

index ea2ef9f..0edcdf7 100644
@@ -258,7 +258,7 @@ static void scc_set_dmamode (struct ata_port *ap, struct ata_device *adev)
 		 JCTSStbl[offset][idx] << 16 | JCENVTtbl[offset][idx]);
 }
 
-unsigned long scc_mode_filter(struct ata_device *adev, unsigned long mask)
+unsigned int scc_mode_filter(struct ata_device *adev, unsigned int mask)
 {
 	/* errata A308 workaround: limit ATAPI UDMA mode to UDMA4 */
 	if (adev->class == ATA_DEV_ATAPI &&

drivers/ata/pata_serverworks.c +2 -2

index 8bed888..708c789 100644
@@ -195,7 +195,7 @@ static u8 serverworks_is_csb(struct pci_dev *pdev)
  *	bug we hit.
  */
 
-static unsigned long serverworks_osb4_filter(struct ata_device *adev, unsigned long mask)
+static unsigned int serverworks_osb4_filter(struct ata_device *adev, unsigned int mask)
 {
 	if (adev->class == ATA_DEV_ATA)
 		mask &= ~ATA_MASK_UDMA;

@@ -211,7 +211,7 @@ static unsigned long serverworks_osb4_filter(struct ata_device *adev, unsigned l
  *	Check the blacklist and disable UDMA5 if matched
  */
 
-static unsigned long serverworks_csb_filter(struct ata_device *adev, unsigned long mask)
+static unsigned int serverworks_csb_filter(struct ata_device *adev, unsigned int mask)
 {
 	const char *p;
 	char model_num[ATA_ID_PROD_LEN + 1];

include/linux/libata.h +3 -2

index f84106a..80c711a 100644
@@ -668,7 +668,7 @@ struct ata_port_operations {
 
 	void (*set_piomode) (struct ata_port *, struct ata_device *);
 	void (*set_dmamode) (struct ata_port *, struct ata_device *);
-	unsigned long (*mode_filter) (struct ata_device *, unsigned long);
+	unsigned int (*mode_filter) (struct ata_device *, unsigned int);
 
 	void (*tf_load) (struct ata_port *ap, const struct ata_taskfile *tf);
 	void (*tf_read) (struct ata_port *ap, struct ata_taskfile *tf);

@@ -989,7 +989,8 @@ extern int ata_pci_prepare_sff_host(struct pci_dev *pdev,
 				    const struct ata_port_info * const * ppi,
 				    struct ata_host **r_host);
 extern int pci_test_config_bits(struct pci_dev *pdev, const struct pci_bits *bits);
-extern unsigned long ata_pci_default_filter(struct ata_device *, unsigned long);
+extern unsigned int ata_pci_default_filter(struct ata_device *dev,
+					   unsigned int xfer_mask);
 #endif /* CONFIG_PCI */
 
 /*

Signature · 1 line

1.5.2.4

Tejun Heo

author

2007-11-06 05:39 UTC #16 lore raw

PATCH 6/12 libata: separate out ata_acpi_gtm_xfermask() from pacpi_discover_modes()

In reply to Tejun Heo

Finding out matching transfer mode from ACPI GTM values is useful for other purposes too. Separate out the function and timing tables from pata_acpi::pacpi_discover_modes().

Other than checking shared-configuration bit after doing ata_acpi_gtm() in pacpi_discover_modes() which should be safe, this patch doesn't introduce any behavior change.

Signed-off-by Tejun Heo <[email protected]>

Cc Alan Cox <[email protected]>

---
 drivers/ata/libata-acpi.c |   78 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/ata/pata_acpi.c   |   66 ++++++-------------------------------
 include/linux/libata.h    |   25 ++++++++++++++
 3 files changed, 114 insertions(+), 55 deletions(-)

drivers/ata/libata-acpi.c +78 -0

index 08a52dd..5ffa542 100644
@@ -424,6 +424,84 @@ int ata_acpi_cbl_80wire(struct ata_port *ap)
 
 EXPORT_SYMBOL_GPL(ata_acpi_cbl_80wire);
 
+/* Welcome to ACPI, bring a bucket */
+const unsigned int ata_acpi_pio_cycle[7] = {
+	600, 383, 240, 180, 120, 100, 80
+};
+EXPORT_SYMBOL_GPL(ata_acpi_pio_cycle);
+
+const unsigned int ata_acpi_mwdma_cycle[5] = {
+	480, 150, 120, 100, 80
+};
+EXPORT_SYMBOL_GPL(ata_acpi_mwdma_cycle);
+
+const unsigned int ata_acpi_udma_cycle[7] = {
+	120, 80, 60, 45, 30, 20, 15
+};
+EXPORT_SYMBOL_GPL(ata_acpi_udma_cycle);
+
+/**
+ * ata_acpi_gtm_xfermode - determine xfermode from GTM parameter
+ * @dev: target device
+ * @gtm: GTM parameter to use
+ *
+ * Determine xfermask for @dev from @gtm.
+ *
+ * LOCKING:
+ * None.
+ *
+ * RETURNS:
+ * Determined xfermask.
+ */
+unsigned int ata_acpi_gtm_xfermask(struct ata_device *dev,
+				   const struct ata_acpi_gtm *gtm)
+{
+	int unit, i;
+	u32 t;
+	unsigned long mask = (0x7f << ATA_SHIFT_UDMA) | (0x7 << ATA_SHIFT_MWDMA) | (0x1F << ATA_SHIFT_PIO);
+
+	/* we always use the 0 slot for crap hardware */
+	unit = dev->devno;
+	if (!(gtm->flags & 0x10))
+		unit = 0;
+
+	/* start by scanning for PIO modes */
+	for (i = 0; i < 7; i++) {
+		t = gtm->drive[unit].pio;
+		if (t <= ata_acpi_pio_cycle[i]) {
+			mask |= (2 << (ATA_SHIFT_PIO + i)) - 1;
+			break;
+		}
+	}
+
+	/* See if we have MWDMA or UDMA data. We don't bother with
+	 * MWDMA if UDMA is available as this means the BIOS set UDMA
+	 * and our error changedown if it works is UDMA to PIO anyway.
+	 */
+	if (gtm->flags & (1 << (2 * unit))) {
+		/* MWDMA */
+		for (i = 0; i < 5; i++) {
+			t = gtm->drive[unit].dma;
+			if (t <= ata_acpi_mwdma_cycle[i]) {
+				mask |= (2 << (ATA_SHIFT_MWDMA + i)) - 1;
+				break;
+			}
+		}
+	} else {
+		/* UDMA */
+		for (i = 0; i < 7; i++) {
+			t = gtm->drive[unit].dma;
+			if (t <= ata_acpi_udma_cycle[i]) {
+				mask |= (2 << (ATA_SHIFT_UDMA + i)) - 1;
+				break;
+			}
+		}
+	}
+
+	return mask;
+}
+EXPORT_SYMBOL_GPL(ata_acpi_gtm_xfermask);
+
 /**
  * taskfile_load_raw - send taskfile registers to host controller
  * @dev: target ATA device

drivers/ata/pata_acpi.c +11 ~~-55~~

101 lines not on this page · raw message

include/linux/libata.h +25 -0

index 80c711a..e626137 100644
@@ -955,11 +955,36 @@ enum {
 
 /* libata-acpi.c */
 #ifdef CONFIG_ATA_ACPI
+extern const unsigned int ata_acpi_pio_cycle[7];
+extern const unsigned int ata_acpi_mwdma_cycle[5];
+extern const unsigned int ata_acpi_udma_cycle[7];
+
 extern int ata_acpi_cbl_80wire(struct ata_port *ap);
 int ata_acpi_stm(const struct ata_port *ap, struct ata_acpi_gtm *stm);
 int ata_acpi_gtm(const struct ata_port *ap, struct ata_acpi_gtm *stm);
+unsigned int ata_acpi_gtm_xfermask(struct ata_device *dev,
+				   const struct ata_acpi_gtm *gtm);
+
 #else
 static inline int ata_acpi_cbl_80wire(struct ata_port *ap) { return 0; }
+
+static inline int ata_acpi_stm(const struct ata_port *ap,
+			       struct ata_acpi_gtm *stm)
+{
+	return -ENOSYS;
+}
+
+static inline int ata_acpi_gtm(const struct ata_port *ap,
+			       struct ata_acpi_gtm *stm)
+{
+	return -ENOSYS;
+}
+
+static inline unsigned int ata_acpi_gtm_xfermask(struct ata_device *dev,
+					const struct ata_acpi_gtm *gtm)
+{
+	return 0;
+}
 #endif
 
 #ifdef CONFIG_PCI

Signature · 1 line

1.5.2.4

Tejun Heo

author

2007-11-06 05:39 UTC #20 lore raw

PATCH 7/12 libata: fix ata_acpi_gtm_xfermask()

In reply to Tejun Heo

ata_acpi_gtm_xfermask() as separated out from pacpi_discover_modes() has various bugs. Fix them.

* The wrong comparison operator is used when finding for matching cycle resulting totally bogus result.

* With the comparion operator fixed, boundary condtion handling is clumsy.

* Setting of any DMA mask bit set all bits in PIO mask.

* MWDMA and UDMA blocks are swapped.

Signed-off-by Tejun Heo <[email protected]>

Cc Alan Cox <[email protected]>

---
 drivers/ata/libata-acpi.c |   42 +++++++++++++++++++++---------------------
 1 files changed, 21 insertions(+), 21 deletions(-)

drivers/ata/libata-acpi.c +21 ~~-21~~

index 5ffa542..06d9961 100644
@@ -456,49 +456,49 @@ EXPORT_SYMBOL_GPL(ata_acpi_udma_cycle);
 unsigned int ata_acpi_gtm_xfermask(struct ata_device *dev,
 				   const struct ata_acpi_gtm *gtm)
 {
+	unsigned int pio_mask = 0, mwdma_mask = 0, udma_mask = 0;
 	int unit, i;
 	u32 t;
-	unsigned long mask = (0x7f << ATA_SHIFT_UDMA) | (0x7 << ATA_SHIFT_MWDMA) | (0x1F << ATA_SHIFT_PIO);
 
 	/* we always use the 0 slot for crap hardware */
 	unit = dev->devno;
 	if (!(gtm->flags & 0x10))
 		unit = 0;
 
+	/* Values larger than the longest cycle results in 0 mask
+	 * while values equal to smaller than the shortest cycle
+	 * results in mask which includes all supported modes.
+	 * Disabled transfer method has the value of 0xffffffff which
+	 * will always result in 0 mask.
+	 */
+
 	/* start by scanning for PIO modes */
-	for (i = 0; i < 7; i++) {
-		t = gtm->drive[unit].pio;
-		if (t <= ata_acpi_pio_cycle[i]) {
-			mask |= (2 << (ATA_SHIFT_PIO + i)) - 1;
+	t = gtm->drive[unit].pio;
+	for (i = 0; i < ARRAY_SIZE(ata_acpi_pio_cycle); i++)
+		if (t > ata_acpi_pio_cycle[i])
 			break;
-		}
-	}
+	pio_mask = (1 << i) - 1;
 
 	/* See if we have MWDMA or UDMA data. We don't bother with
 	 * MWDMA if UDMA is available as this means the BIOS set UDMA
 	 * and our error changedown if it works is UDMA to PIO anyway.
 	 */
-	if (gtm->flags & (1 << (2 * unit))) {
+	t = gtm->drive[unit].dma;
+	if (!(gtm->flags & (1 << (2 * unit)))) {
 		/* MWDMA */
-		for (i = 0; i < 5; i++) {
-			t = gtm->drive[unit].dma;
-			if (t <= ata_acpi_mwdma_cycle[i]) {
-				mask |= (2 << (ATA_SHIFT_MWDMA + i)) - 1;
+		for (i = 0; i < ARRAY_SIZE(ata_acpi_mwdma_cycle); i++)
+			if (t > ata_acpi_mwdma_cycle[i])
 				break;
-			}
-		}
+		mwdma_mask = (1 << i) - 1;
 	} else {
 		/* UDMA */
-		for (i = 0; i < 7; i++) {
-			t = gtm->drive[unit].dma;
-			if (t <= ata_acpi_udma_cycle[i]) {
-				mask |= (2 << (ATA_SHIFT_UDMA + i)) - 1;
+		for (i = 0; i < ARRAY_SIZE(ata_acpi_udma_cycle); i++)
+			if (t > ata_acpi_udma_cycle[i])
 				break;
-			}
-		}
+		udma_mask = (1 << i) - 1;
 	}
 
-	return mask;
+	return ata_pack_xfermask(pio_mask, mwdma_mask, udma_mask);
 }
 EXPORT_SYMBOL_GPL(ata_acpi_gtm_xfermask);

Signature · 1 line

1.5.2.4

Tejun Heo

author

2007-11-06 05:39 UTC #24 lore raw

PATCH 8/12 libata: implement ata_timing_cycle2mode() and use it in libata-acpi and pata_acpi

In reply to Tejun Heo

libata-acpi is using separate timing tables for transfer modes although libata-core has the complete ata_timing table. Implement ata_timing_cycle2mode() to look for matching mode given transfer type and cycle duration and use it in libata-acpi and pata_acpi to replace private timing tables.

Signed-off-by Tejun Heo <[email protected]>

Cc Alan Cox <[email protected]>

---
 drivers/ata/libata-acpi.c |   62 +++++++++++----------------------------------
 drivers/ata/libata-core.c |   52 +++++++++++++++++++++++++++++++++++++
 drivers/ata/pata_acpi.c   |   13 +++++----
 include/linux/libata.h    |    5 +---
 4 files changed, 75 insertions(+), 57 deletions(-)

drivers/ata/libata-acpi.c +15 ~~-47~~

index 06d9961..7d982c2 100644
@@ -424,22 +424,6 @@ int ata_acpi_cbl_80wire(struct ata_port *ap)
 
 EXPORT_SYMBOL_GPL(ata_acpi_cbl_80wire);
 
-/* Welcome to ACPI, bring a bucket */
-const unsigned int ata_acpi_pio_cycle[7] = {
-	600, 383, 240, 180, 120, 100, 80
-};
-EXPORT_SYMBOL_GPL(ata_acpi_pio_cycle);
-
-const unsigned int ata_acpi_mwdma_cycle[5] = {
-	480, 150, 120, 100, 80
-};
-EXPORT_SYMBOL_GPL(ata_acpi_mwdma_cycle);
-
-const unsigned int ata_acpi_udma_cycle[7] = {
-	120, 80, 60, 45, 30, 20, 15
-};
-EXPORT_SYMBOL_GPL(ata_acpi_udma_cycle);
-
 /**
  * ata_acpi_gtm_xfermode - determine xfermode from GTM parameter
  * @dev: target device

@@ -456,49 +440,33 @@ EXPORT_SYMBOL_GPL(ata_acpi_udma_cycle);
 unsigned int ata_acpi_gtm_xfermask(struct ata_device *dev,
 				   const struct ata_acpi_gtm *gtm)
 {
-	unsigned int pio_mask = 0, mwdma_mask = 0, udma_mask = 0;
-	int unit, i;
-	u32 t;
+	unsigned int xfer_mask = 0;
+	unsigned int type;
+	int unit;
+	u8 mode;
 
 	/* we always use the 0 slot for crap hardware */
 	unit = dev->devno;
 	if (!(gtm->flags & 0x10))
 		unit = 0;
 
-	/* Values larger than the longest cycle results in 0 mask
-	 * while values equal to smaller than the shortest cycle
-	 * results in mask which includes all supported modes.
-	 * Disabled transfer method has the value of 0xffffffff which
-	 * will always result in 0 mask.
-	 */
-
-	/* start by scanning for PIO modes */
-	t = gtm->drive[unit].pio;
-	for (i = 0; i < ARRAY_SIZE(ata_acpi_pio_cycle); i++)
-		if (t > ata_acpi_pio_cycle[i])
-			break;
-	pio_mask = (1 << i) - 1;
+	/* PIO */
+	mode = ata_timing_cycle2mode(ATA_SHIFT_PIO, gtm->drive[unit].pio);
+	xfer_mask |= ata_xfer_mode2mask(mode);
 
 	/* See if we have MWDMA or UDMA data. We don't bother with
 	 * MWDMA if UDMA is available as this means the BIOS set UDMA
 	 * and our error changedown if it works is UDMA to PIO anyway.
 	 */
-	t = gtm->drive[unit].dma;
-	if (!(gtm->flags & (1 << (2 * unit)))) {
-		/* MWDMA */
-		for (i = 0; i < ARRAY_SIZE(ata_acpi_mwdma_cycle); i++)
-			if (t > ata_acpi_mwdma_cycle[i])
-				break;
-		mwdma_mask = (1 << i) - 1;
-	} else {
-		/* UDMA */
-		for (i = 0; i < ARRAY_SIZE(ata_acpi_udma_cycle); i++)
-			if (t > ata_acpi_udma_cycle[i])
-				break;
-		udma_mask = (1 << i) - 1;
-	}
+	if (!(gtm->flags & (1 << (2 * unit))))
+		type = ATA_SHIFT_MWDMA;
+	else
+		type = ATA_SHIFT_UDMA;
+
+	mode = ata_timing_cycle2mode(type, gtm->drive[unit].dma);
+	xfer_mask |= ata_xfer_mode2mask(mode);
 
-	return ata_pack_xfermask(pio_mask, mwdma_mask, udma_mask);
+	return xfer_mask;
 }
 EXPORT_SYMBOL_GPL(ata_acpi_gtm_xfermask);

drivers/ata/libata-core.c +52 -0

64 lines not on this page · raw message

drivers/ata/pata_acpi.c +7 -6

index fc6e3a5..8059259 100644
@@ -133,13 +133,14 @@ static void pacpi_set_piomode(struct ata_port *ap, struct ata_device *adev)
 {
 	int unit = adev->devno;
 	struct pata_acpi *acpi = ap->private_data;
+	const struct ata_timing *t;
 
 	if (!(acpi->gtm.flags & 0x10))
 		unit = 0;
 
 	/* Now stuff the nS values into the structure */
-	acpi->gtm.drive[unit].pio =
-		ata_acpi_pio_cycle[adev->pio_mode - XFER_PIO_0];
+	t = ata_timing_find_mode(adev->pio_mode);
+	acpi->gtm.drive[unit].pio = t->cycle;
 	ata_acpi_stm(ap, &acpi->gtm);
 	/* See what mode we actually got */
 	ata_acpi_gtm(ap, &acpi->gtm);

@@ -155,18 +156,18 @@ static void pacpi_set_dmamode(struct ata_port *ap, struct ata_device *adev)
 {
 	int unit = adev->devno;
 	struct pata_acpi *acpi = ap->private_data;
+	const struct ata_timing *t;
 
 	if (!(acpi->gtm.flags & 0x10))
 		unit = 0;
 
 	/* Now stuff the nS values into the structure */
+	t = ata_timing_find_mode(adev->dma_mode);
 	if (adev->dma_mode >= XFER_UDMA_0) {
-		acpi->gtm.drive[unit].dma =
-			ata_acpi_udma_cycle[adev->dma_mode - XFER_UDMA_0];
+		acpi->gtm.drive[unit].dma = t->udma;
 		acpi->gtm.flags |= (1 << (2 * unit));
 	} else {
-		acpi->gtm.drive[unit].dma =
-			ata_acpi_mwdma_cycle[adev->dma_mode - XFER_MW_DMA_0];
+		acpi->gtm.drive[unit].dma = t->cycle;
 		acpi->gtm.flags &= ~(1 << (2 * unit));
 	}
 	ata_acpi_stm(ap, &acpi->gtm);

include/linux/libata.h +1 -4

index e626137..3b9cebe 100644
@@ -935,6 +935,7 @@ extern int ata_timing_compute(struct ata_device *, unsigned short,
 extern void ata_timing_merge(const struct ata_timing *,
 			     const struct ata_timing *, struct ata_timing *,
 			     unsigned int);
+extern u8 ata_timing_cycle2mode(unsigned int xfer_shift, int cycle);
 
 enum {
 	ATA_TIMING_SETUP	= (1 << 0),

@@ -955,10 +956,6 @@ enum {
 
 /* libata-acpi.c */
 #ifdef CONFIG_ATA_ACPI
-extern const unsigned int ata_acpi_pio_cycle[7];
-extern const unsigned int ata_acpi_mwdma_cycle[5];
-extern const unsigned int ata_acpi_udma_cycle[7];
-
 extern int ata_acpi_cbl_80wire(struct ata_port *ap);
 int ata_acpi_stm(const struct ata_port *ap, struct ata_acpi_gtm *stm);
 int ata_acpi_gtm(const struct ata_port *ap, struct ata_acpi_gtm *stm);

Signature · 1 line

1.5.2.4

Tejun Heo

author

2007-11-06 05:39 UTC #28 lore raw

PATCH 9/12 libata: implement ata_acpi_init_gtm()

In reply to Tejun Heo

Add dev->__acpi_init_gtm and store initial GTM values on host initialization. If the field is valid, ata_acpi_init_gtm() returns pointer to the saved GTM structure; otherwise, NULL. This is to remember BIOS/firmware programmed initial timing for later use before reset and mode configuration modify it. The accessor is there to make building w/o ACPI easy dev->__acpi_init doesn't exist if ACPI is not enabled.

Signed-off-by Tejun Heo <[email protected]>

---
 drivers/ata/libata-acpi.c |    3 +++
 include/linux/libata.h    |   14 +++++++++++++-
 2 files changed, 16 insertions(+), 1 deletions(-)

drivers/ata/libata-acpi.c +3 -0

index 7d982c2..31365b3 100644
@@ -94,6 +94,9 @@ static void ata_acpi_associate_ide_port(struct ata_port *ap)
 
 		dev->acpi_handle = acpi_get_child(ap->acpi_handle, i);
 	}
+
+	if (ata_acpi_gtm(ap, &ap->__acpi_init_gtm) == 0)
+		ap->pflags |= ATA_PFLAG_INIT_GTM_VALID;
 }
 
 static void ata_acpi_handle_hotplug(struct ata_port *ap, struct kobject *kobj,

include/linux/libata.h +13 -1

index 3b9cebe..bad280f 100644
@@ -213,6 +213,7 @@ enum {
 	ATA_PFLAG_SUSPENDED	= (1 << 17), /* port is suspended (power) */
 	ATA_PFLAG_PM_PENDING	= (1 << 18), /* PM operation pending */
 	ATA_PFLAG_GTM_VALID	= (1 << 19), /* acpi_gtm data valid */
+	ATA_PFLAG_INIT_GTM_VALID = (1 << 20), /* initial gtm data valid */
 
 	/* struct ata_queued_cmd flags */
 	ATA_QCFLAG_ACTIVE	= (1 << 0), /* cmd not yet ack'd to scsi lyer */

@@ -659,6 +660,7 @@ struct ata_port {
 #ifdef CONFIG_ATA_ACPI
 	acpi_handle		acpi_handle;
 	struct ata_acpi_gtm	acpi_gtm;
+	struct ata_acpi_gtm	__acpi_init_gtm; /* use ata_acpi_init_gtm() */
 #endif
 	u8			sector_buf[ATA_SECT_SIZE]; /* owned by EH */
 };

@@ -956,13 +958,23 @@ enum {
 
 /* libata-acpi.c */
 #ifdef CONFIG_ATA_ACPI
+static inline const struct ata_acpi_gtm *ata_acpi_init_gtm(struct ata_port *ap)
+{
+	if (ap->pflags & ATA_PFLAG_INIT_GTM_VALID)
+		return &ap->__acpi_init_gtm;
+	return NULL;
+}
 extern int ata_acpi_cbl_80wire(struct ata_port *ap);
 int ata_acpi_stm(const struct ata_port *ap, struct ata_acpi_gtm *stm);
 int ata_acpi_gtm(const struct ata_port *ap, struct ata_acpi_gtm *stm);
 unsigned int ata_acpi_gtm_xfermask(struct ata_device *dev,
 				   const struct ata_acpi_gtm *gtm);
-
 #else
+static inline const struct ata_acpi_gtm *ata_acpi_init_gtm(struct ata_port *ap)
+{
+	return NULL;
+}
+
 static inline int ata_acpi_cbl_80wire(struct ata_port *ap) { return 0; }
 
 static inline int ata_acpi_stm(const struct ata_port *ap,

Signature · 1 line

1.5.2.4

Tejun Heo

author

2007-11-06 05:39 UTC #29 lore raw

PATCH 10/12 libata: reimplement ata_acpi_cbl_80wire() using ata_acpi_gtm_xfermask()

In reply to Tejun Heo

Reimplement ata_acpi_cbl_80wire() using ata_acpi_gtm_xfermask() and while at it relocate the function below ata_acpi_gtm_xfermask().

New ata_acpi_cbl_80wire() implementation takes @gtm, in both pata_via and pata_amd, use the initial GTM value. Both are trying to peek initial BIOS configuration, so using initial caching value makes sense. This fixes ACPI part of cable detection in pata_amd which previously always returned 0 because configuring PIO0 during reset clears DMA configuration.

Signed-off-by Tejun Heo <[email protected]>

Cc Alan Cox <[email protected]>

---
 drivers/ata/libata-acpi.c |   66 +++++++++++++++++++--------------------------
 drivers/ata/pata_amd.c    |    3 +-
 drivers/ata/pata_via.c    |    3 +-
 include/linux/libata.h    |   10 +++++--
 4 files changed, 39 insertions(+), 43 deletions(-)

drivers/ata/libata-acpi.c +28 ~~-38~~

index 31365b3..281dd2d 100644
@@ -390,44 +390,6 @@ static int ata_dev_get_GTF(struct ata_device *dev, struct ata_acpi_gtf **gtf,
 }
 
 /**
- * ata_acpi_cbl_80wire		-	Check for 80 wire cable
- * @ap: Port to check
- *
- * Return 1 if the ACPI mode data for this port indicates the BIOS selected
- * an 80wire mode.
- */
-
-int ata_acpi_cbl_80wire(struct ata_port *ap)
-{
-	struct ata_acpi_gtm gtm;
-	int valid = 0;
-
-	/* No _GTM data, no information */
-	if (ata_acpi_gtm(ap, &gtm) < 0)
-		return 0;
-
-	/* Split timing, DMA enabled */
-	if ((gtm.flags & 0x11) == 0x11 && gtm.drive[0].dma < 55)
-		valid |= 1;
-	if ((gtm.flags & 0x14) == 0x14 && gtm.drive[1].dma < 55)
-		valid |= 2;
-	/* Shared timing, DMA enabled */
-	if ((gtm.flags & 0x11) == 0x01 && gtm.drive[0].dma < 55)
-		valid |= 1;
-	if ((gtm.flags & 0x14) == 0x04 && gtm.drive[0].dma < 55)
-		valid |= 2;
-
-	/* Drive check */
-	if ((valid & 1) && ata_dev_enabled(&ap->link.device[0]))
-		return 1;
-	if ((valid & 2) && ata_dev_enabled(&ap->link.device[1]))
-		return 1;
-	return 0;
-}
-
-EXPORT_SYMBOL_GPL(ata_acpi_cbl_80wire);
-
-/**
  * ata_acpi_gtm_xfermode - determine xfermode from GTM parameter
  * @dev: target device
  * @gtm: GTM parameter to use

@@ -474,6 +436,34 @@ unsigned int ata_acpi_gtm_xfermask(struct ata_device *dev,
 EXPORT_SYMBOL_GPL(ata_acpi_gtm_xfermask);
 
 /**
+ * ata_acpi_cbl_80wire		-	Check for 80 wire cable
+ * @ap: Port to check
+ * @gtm: GTM data to use
+ *
+ * Return 1 if the @gtm indicates the BIOS selected an 80wire mode.
+ */
+int ata_acpi_cbl_80wire(struct ata_port *ap, const struct ata_acpi_gtm *gtm)
+{
+	struct ata_device *dev;
+
+	ata_link_for_each_dev(dev, &ap->link) {
+		unsigned int xfer_mask, udma_mask;
+
+		if (!ata_dev_enabled(dev))
+			continue;
+
+		xfer_mask = ata_acpi_gtm_xfermask(dev, gtm);
+		ata_unpack_xfermask(xfer_mask, NULL, NULL, &udma_mask);
+
+		if (udma_mask & ~ATA_UDMA_MASK_40C)
+			return 1;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(ata_acpi_cbl_80wire);
+
+/**
  * taskfile_load_raw - send taskfile registers to host controller
  * @dev: target ATA device
  * @gtf: raw ATA taskfile register set (0x1f1 - 0x1f7)

drivers/ata/pata_amd.c +2 -1

index c5779ad..f65fa6c 100644
@@ -271,7 +271,8 @@ static int nv_cable_detect(struct ata_port *ap)
  	if ((udma & 0xC4) == 0xC4 || (udma & 0xC400) == 0xC400)
 		cbl = ATA_CBL_PATA80;
 	/* And a triple check across suspend/resume with ACPI around */
-	if (ata_acpi_cbl_80wire(ap))
+	if (ata_acpi_init_gtm(ap) &&
+	    ata_acpi_cbl_80wire(ap, ata_acpi_init_gtm(ap)))
 		cbl = ATA_CBL_PATA80;
 	return cbl;
 }

drivers/ata/pata_via.c +2 -1

index a4175fb..3f28ed4 100644
@@ -185,7 +185,8 @@ static int via_cable_detect(struct ata_port *ap) {
 	if (ata66 & (0x10100000 >> (16 * ap->port_no)))
 		return ATA_CBL_PATA80;
 	/* Check with ACPI so we can spot BIOS reported SATA bridges */
-	if (ata_acpi_cbl_80wire(ap))
+	if (ata_acpi_init_gtm(ap) &&
+	    ata_acpi_cbl_80wire(ap, ata_acpi_init_gtm(ap)))
 		return ATA_CBL_PATA80;
 	return ATA_CBL_PATA40;
 }

include/linux/libata.h +7 -3

index bad280f..5ad2186 100644
@@ -964,19 +964,17 @@ static inline const struct ata_acpi_gtm *ata_acpi_init_gtm(struct ata_port *ap)
 		return &ap->__acpi_init_gtm;
 	return NULL;
 }
-extern int ata_acpi_cbl_80wire(struct ata_port *ap);
 int ata_acpi_stm(const struct ata_port *ap, struct ata_acpi_gtm *stm);
 int ata_acpi_gtm(const struct ata_port *ap, struct ata_acpi_gtm *stm);
 unsigned int ata_acpi_gtm_xfermask(struct ata_device *dev,
 				   const struct ata_acpi_gtm *gtm);
+int ata_acpi_cbl_80wire(struct ata_port *ap, const struct ata_acpi_gtm *gtm);
 #else
 static inline const struct ata_acpi_gtm *ata_acpi_init_gtm(struct ata_port *ap)
 {
 	return NULL;
 }
 
-static inline int ata_acpi_cbl_80wire(struct ata_port *ap) { return 0; }
-
 static inline int ata_acpi_stm(const struct ata_port *ap,
 			       struct ata_acpi_gtm *stm)
 {

@@ -994,6 +992,12 @@ static inline unsigned int ata_acpi_gtm_xfermask(struct ata_device *dev,
 {
 	return 0;
 }
+
+static inline int ata_acpi_cbl_80wire(struct ata_port *ap,
+				      const struct ata_acpi_gtm *gtm)
+{
+	return 0;
+}
 #endif
 
 #ifdef CONFIG_PCI

Signature · 1 line

1.5.2.4

Tejun Heo

author

2007-11-06 05:39 UTC #30 lore raw

PATCH 11/12 libata: add ATA_CBL_PATA_IGN

In reply to Tejun Heo

ATA_CBL_PATA_UNK indicates that the cable type can't be determined from the host side and might be either 80c or 40c. libata applies drive or other generic limit in this case. However, there are controllers where both host and drive side detections are misimplemented and the driver has to rely solely on private method - peeking BIOS or ACPI configuration or using some other private mechanism.

This patch adds ATA_CBL_PATA_IGN which tells libata to ignore the cable type completely and just let the LLD determine the transfer mode via host transfer mode masks and ->mode_filter().

Signed-off-by Tejun Heo <[email protected]>

Cc Alan Cox <[email protected]>

---
 drivers/ata/libata-core.c |   13 +++++++++++++
 include/linux/ata.h       |    7 ++++---
 include/linux/libata.h    |    1 +
 3 files changed, 18 insertions(+), 3 deletions(-)

drivers/ata/libata-core.c +13 -0

index 4372099..f75a26c 100644
@@ -2331,6 +2331,18 @@ int ata_cable_unknown(struct ata_port *ap)
 }
 
 /**
+ *	ata_cable_ignore	-	return ignored PATA cable.
+ *	@ap: port
+ *
+ *	Helper method for drivers which don't use cable type to limit
+ *	transfer mode.
+ */
+int ata_cable_ignore(struct ata_port *ap)
+{
+	return ATA_CBL_PATA_IGN;
+}
+
+/**
  *	ata_cable_sata	-	return SATA cable type
  *	@ap: port
  *

@@ -7707,4 +7719,5 @@ EXPORT_SYMBOL_GPL(ata_dev_try_classify);
 EXPORT_SYMBOL_GPL(ata_cable_40wire);
 EXPORT_SYMBOL_GPL(ata_cable_80wire);
 EXPORT_SYMBOL_GPL(ata_cable_unknown);
+EXPORT_SYMBOL_GPL(ata_cable_ignore);
 EXPORT_SYMBOL_GPL(ata_cable_sata);

include/linux/ata.h +4 -3

index e1323e2..d942dba 100644
@@ -271,9 +271,10 @@ enum {
 	ATA_CBL_NONE		= 0,
 	ATA_CBL_PATA40		= 1,
 	ATA_CBL_PATA80		= 2,
-	ATA_CBL_PATA40_SHORT	= 3,		/* 40 wire cable to high UDMA spec */
-	ATA_CBL_PATA_UNK	= 4,
-	ATA_CBL_SATA		= 5,
+	ATA_CBL_PATA40_SHORT	= 3,	/* 40 wire cable to high UDMA spec */
+	ATA_CBL_PATA_UNK	= 4,	/* don't know, maybe 80c? */
+	ATA_CBL_PATA_IGN	= 5,	/* don't know, ignore cable handling */
+	ATA_CBL_SATA		= 6,
 
 	/* SATA Status and Control Registers */
 	SCR_STATUS		= 0,

include/linux/libata.h +1 -0

index 5ad2186..5145a5c 100644
@@ -924,6 +924,7 @@ extern u8 ata_irq_on(struct ata_port *ap);
 extern int ata_cable_40wire(struct ata_port *ap);
 extern int ata_cable_80wire(struct ata_port *ap);
 extern int ata_cable_sata(struct ata_port *ap);
+extern int ata_cable_ignore(struct ata_port *ap);
 extern int ata_cable_unknown(struct ata_port *ap);
 
 /*

Signature · 1 line

1.5.2.4

Tejun Heo

author

2007-11-06 05:39 UTC #34 lore raw

PATCH 12/12 pata_amd: update mode selection for NV PATAs

In reply to Tejun Heo

Cable detection on NV PATA hosts isn't implemented and the CBLID- cable isn't wired according to the sepc either, so both host-side and generic drive-side cable detections are broken. Till now, nv_cable_detect() relied on peeking BIOS and ACPI configurations to upgrade to 80C but this often results in misdetection of 40C cable as 80C. Also, the original implementation was broken in that by the time BIOS configuration is read it has already been cleared by programming PIO0 during reset.

This patch reimplements NV mode selection such that...

* BIOS configuration value is stored during driver attach and restored on detach.

* Cable type is fixed to ATA_CBL_PATA_IGN and mode selection is soley done by nv_mode_filter() which peeks both BIOS and ACPI configurations and filter accordingly.

Signed-off-by Tejun Heo <[email protected]>

Cc Alan Cox <[email protected]>

---
 drivers/ata/pata_amd.c |  129 ++++++++++++++++++++++++++++++++++-------------
 1 files changed, 93 insertions(+), 36 deletions(-)

drivers/ata/pata_amd.c +93 ~~-36~~

index f65fa6c..711e8bb 100644
@@ -219,6 +219,62 @@ static void amd133_set_dmamode(struct ata_port *ap, struct ata_device *adev)
 	timing_setup(ap, adev, 0x40, adev->dma_mode, 4);
 }
 
+/* Both host-side and drive-side detection results are worthless on NV
+ * PATAs.  Ignore them and just follow what BIOS configured.  Both the
+ * current configuration in PCI config reg and ACPI GTM result are
+ * cached during driver attach and are consulted to select transfer
+ * mode.
+ */
+static unsigned int nv_mode_filter(struct ata_device *dev,
+				   unsigned int xfer_mask)
+{
+	static const unsigned int udma_mask_map[] =
+		{ ATA_UDMA2, ATA_UDMA1, ATA_UDMA0, 0,
+		  ATA_UDMA3, ATA_UDMA4, ATA_UDMA5, ATA_UDMA6 };
+	struct ata_port *ap = dev->link->ap;
+	char acpi_str[32] = "";
+	u32 saved_udma, udma;
+	const struct ata_acpi_gtm *gtm;
+	unsigned int bios_limit = 0, acpi_limit = 0, limit;
+
+	/* find out what BIOS configured */
+	udma = saved_udma = (unsigned long)ap->host->private_data;
+
+	if (ap->port_no == 0)
+		udma >>= 16;
+	if (dev->devno == 0)
+		udma >>= 8;
+
+	if ((udma & 0xc0) == 0xc0)
+		bios_limit = ata_pack_xfermask(0, 0, udma_mask_map[udma & 0x7]);
+
+	/* consult ACPI GTM too */
+	gtm = ata_acpi_init_gtm(ap);
+	if (gtm) {
+		acpi_limit = ata_acpi_gtm_xfermask(dev, gtm);
+
+		snprintf(acpi_str, sizeof(acpi_str), " (%u:%u:0x%x)",
+			 gtm->drive[0].dma, gtm->drive[1].dma, gtm->flags);
+	}
+
+	/* be optimistic, EH can take care of things if something goes wrong */
+	limit = bios_limit | acpi_limit;
+
+	/* If PIO or DMA isn't configured at all, don't limit.  Let EH
+	 * handle it.
+	 */
+	if (!(limit & ATA_MASK_PIO))
+		limit |= ATA_MASK_PIO;
+	if (!(limit & (ATA_MASK_MWDMA | ATA_MASK_UDMA)))
+		limit |= ATA_MASK_MWDMA | ATA_MASK_UDMA;
+
+	ata_port_printk(ap, KERN_DEBUG, "nv_mode_filter: 0x%x&0x%x->0x%x, "
+			"BIOS=0x%x (0x%x) ACPI=0x%x%s\n",
+			xfer_mask, limit, xfer_mask & limit, bios_limit,
+			saved_udma, acpi_limit, acpi_str);
+
+	return xfer_mask & limit;
+}
 
 /**
  *	nv_probe_init	-	cable detection

@@ -251,32 +307,6 @@ static void nv_error_handler(struct ata_port *ap)
 			       ata_std_postreset);
 }
 
-static int nv_cable_detect(struct ata_port *ap)
-{
-	static const u8 bitmask[2] = {0x03, 0x0C};
-	struct pci_dev *pdev = to_pci_dev(ap->host->dev);
-	u8 ata66;
-	u16 udma;
-	int cbl;
-
-	pci_read_config_byte(pdev, 0x52, &ata66);
-	if (ata66 & bitmask[ap->port_no])
-		cbl = ATA_CBL_PATA80;
-	else
-		cbl = ATA_CBL_PATA40;
-
- 	/* We now have to double check because the Nvidia boxes BIOS
- 	   doesn't always set the cable bits but does set mode bits */
- 	pci_read_config_word(pdev, 0x62 - 2 * ap->port_no, &udma);
- 	if ((udma & 0xC4) == 0xC4 || (udma & 0xC400) == 0xC400)
-		cbl = ATA_CBL_PATA80;
-	/* And a triple check across suspend/resume with ACPI around */
-	if (ata_acpi_init_gtm(ap) &&
-	    ata_acpi_cbl_80wire(ap, ata_acpi_init_gtm(ap)))
-		cbl = ATA_CBL_PATA80;
-	return cbl;
-}
-
 /**
  *	nv100_set_piomode	-	set initial PIO mode data
  *	@ap: ATA interface

@@ -314,6 +344,14 @@ static void nv133_set_dmamode(struct ata_port *ap, struct ata_device *adev)
 	timing_setup(ap, adev, 0x50, adev->dma_mode, 4);
 }
 
+static void nv_host_stop(struct ata_host *host)
+{
+	u32 udma = (unsigned long)host->private_data;
+
+	/* restore PCI config register 0x60 */
+	pci_write_config_dword(to_pci_dev(host->dev), 0x60, udma);
+}
+
 static struct scsi_host_template amd_sht = {
 	.module			= THIS_MODULE,
 	.name			= DRV_NAME,

@@ -478,7 +516,8 @@ static struct ata_port_operations nv100_port_ops = {
 	.thaw		= ata_bmdma_thaw,
 	.error_handler	= nv_error_handler,
 	.post_internal_cmd = ata_bmdma_post_internal_cmd,
-	.cable_detect	= nv_cable_detect,
+	.cable_detect	= ata_cable_ignore,
+	.mode_filter	= nv_mode_filter,
 
 	.bmdma_setup 	= ata_bmdma_setup,
 	.bmdma_start 	= ata_bmdma_start,

@@ -495,6 +534,7 @@ static struct ata_port_operations nv100_port_ops = {
 	.irq_on		= ata_irq_on,
 
 	.port_start	= ata_sff_port_start,
+	.host_stop	= nv_host_stop,
 };
 
 static struct ata_port_operations nv133_port_ops = {

@@ -511,7 +551,8 @@ static struct ata_port_operations nv133_port_ops = {
 	.thaw		= ata_bmdma_thaw,
 	.error_handler	= nv_error_handler,
 	.post_internal_cmd = ata_bmdma_post_internal_cmd,
-	.cable_detect	= nv_cable_detect,
+	.cable_detect	= ata_cable_ignore,
+	.mode_filter	= nv_mode_filter,
 
 	.bmdma_setup 	= ata_bmdma_setup,
 	.bmdma_start 	= ata_bmdma_start,

@@ -528,6 +569,7 @@ static struct ata_port_operations nv133_port_ops = {
 	.irq_on		= ata_irq_on,
 
 	.port_start	= ata_sff_port_start,
+	.host_stop	= nv_host_stop,
 };
 
 static int amd_init_one(struct pci_dev *pdev, const struct pci_device_id *id)

@@ -614,7 +656,8 @@ static int amd_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 			.port_ops = &amd100_port_ops
 		}
 	};
-	const struct ata_port_info *ppi[] = { NULL, NULL };
+	struct ata_port_info pi;
+	const struct ata_port_info *ppi[] = { &pi, NULL };
 	static int printed_version;
 	int type = id->driver_data;
 	u8 fifo;

@@ -628,6 +671,19 @@ static int amd_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (type == 1 && pdev->revision > 0x7)
 		type = 2;
 
+	/* Serenade ? */
+	if (type == 5 && pdev->subsystem_vendor == PCI_VENDOR_ID_AMD &&
+			 pdev->subsystem_device == PCI_DEVICE_ID_AMD_SERENADE)
+		type = 6;	/* UDMA 100 only */
+
+	/*
+	 * Okay, type is determined now.  Apply type-specific workarounds.
+	 */
+	pi = info[type];
+
+	if (type < 3)
+		ata_pci_clear_simplex(pdev);
+
 	/* Check for AMD7411 */
 	if (type == 3)
 		/* FIFO is broken */

@@ -635,16 +691,17 @@ static int amd_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
 	else
 		pci_write_config_byte(pdev, 0x41, fifo | 0xF0);
 
-	/* Serenade ? */
-	if (type == 5 && pdev->subsystem_vendor == PCI_VENDOR_ID_AMD &&
-			 pdev->subsystem_device == PCI_DEVICE_ID_AMD_SERENADE)
-		type = 6;	/* UDMA 100 only */
+	/* Cable detection on Nvidia chips doesn't work too well,
+	 * cache BIOS programmed UDMA mode.
+	 */
+	if (type == 7 || type == 8) {
+		u32 udma;
 
-	if (type < 3)
-		ata_pci_clear_simplex(pdev);
+		pci_read_config_dword(pdev, 0x60, &udma);
+		pi.private_data = (void *)(unsigned long)udma;
+	}
 
 	/* And fire it up */
-	ppi[0] = &info[type];
 	return ata_pci_init_one(pdev, ppi);
 }

Signature · 1 line

1.5.2.4

Alan Cox

2007-11-06 10:49 UTC #5 lore raw

Re: [PATCH 04/12] libata: kill ata_id_to_dma_mode()

In reply to Tejun Heo

Quoted 3 lines · On Tue, 6 Nov 2007 14:39:02 +0900 Tejun Heo <[email protected]> wrote:

ata_id_to_dma_mode() isn't quite generic. The function is basically privately implemented ata_id_xfermask() combined with hardcoded mode printing and configuration which are specific to ata_generic.

I anticpiated other users. This seems a backward move - we end up exporting lots of low level detail into the drivers which should be none of their business.

Alan Cox

2007-11-06 10:51 UTC #9 lore raw

Re: [PATCH 05/12] libata: xfer_mask is unsigned int not unsigned long

In reply to Tejun Heo

Quoted 7 lines · On Tue, 6 Nov 2007 14:39:03 +0900 Tejun Heo <[email protected]> wrote:

xfer_mask is unsigned int not unsigned long. Change ->mode_filter to take and return unsigned int.
While at it, rename @adev of ata_pci_default_filter() to @dev for consistency.
Signed-off-by Tejun Heo <[email protected]>

The filter type was purposefully unsigned long to allow for expansion (eg for SWDMA) without breaking drivers. No problem with it changing but I'd say "unsigned int" was the worst possible choice - its now shorter (no room for expansion) and size dependant on arch. u32 would be shorter and consistent across all platforms, ulong would have more room for expansion.

Alan

Alan Cox

2007-11-06 10:53 UTC #17 lore raw

Re: [PATCH 06/12] libata: separate out ata_acpi_gtm_xfermask() from pacpi_discover_modes()

In reply to Tejun Heo

Quoted 5 lines

drivers/ata/libata-acpi.c +5 -0

+/* Welcome to ACPI, bring a bucket */
+const unsigned int ata_acpi_pio_cycle[7] = {
+	600, 383, 240, 180, 120, 100, 80
+};
+EXPORT_SYMBOL_GPL(ata_acpi_pio_cycle);

drivers/ata/libata-acpi.c int ata_acpi_cbl_80wire(struct ata_port *ap) · line 441

Quoted 10 lines

drivers/ata/libata-acpi.c +10 -0

+
+const unsigned int ata_acpi_mwdma_cycle[5] = {
+	480, 150, 120, 100, 80
+};
+EXPORT_SYMBOL_GPL(ata_acpi_mwdma_cycle);
+
+const unsigned int ata_acpi_udma_cycle[7] = {
+	120, 80, 60, 45, 30, 20, 15
+};
+EXPORT_SYMBOL_GPL(ata_acpi_udma_cycle);

Do we really need to keep exporting all these things. So far this patch set has exported a set of very specific ACPI arrays and a load of internal functions. That to me says the splitting up is wrong.

One option would be to make those tables private and simply make the pata_acpi driver use the ata_timing functions

Alan

Alan Cox

2007-11-06 10:54 UTC #21 lore raw

Re: [PATCH 07/12] libata: fix ata_acpi_gtm_xfermask()

In reply to Tejun Heo

Quoted 9 lines · On Tue, 6 Nov 2007 14:39:05 +0900 Tejun Heo <[email protected]> wrote:

ata_acpi_gtm_xfermask() as separated out from pacpi_discover_modes() has various bugs. Fix them.
* The wrong comparison operator is used when finding for matching cycle resulting totally bogus result.
* With the comparion operator fixed, boundary condtion handling is clumsy.

Quoted 7 lines

* Setting of any DMA mask bit set all bits in PIO mask.
* MWDMA and UDMA blocks are swapped.
Signed-off-by Tejun Heo <[email protected]>
Cc Alan Cox <[email protected]>
---

Looks good to me

Alan Cox

Acked-by on v1 12/12

2007-11-06 10:58 UTC #35 lore raw

Re: [PATCH 12/12] pata_amd: update mode selection for NV PATAs

In reply to Tejun Heo

Looks good to me

Acked-by Alan Cox <[email protected]>

Alan Cox

2007-11-06 10:58 UTC #25 lore raw

Re: [PATCH 08/12] libata: implement ata_timing_cycle2mode() and use it in libata-acpi and pata_acpi

In reply to Tejun Heo

Quoted 5 lines · On Tue, 6 Nov 2007 14:39:06 +0900 Tejun Heo <[email protected]> wrote:

libata-acpi is using separate timing tables for transfer modes although libata-core has the complete ata_timing table. Implement ata_timing_cycle2mode() to look for matching mode given transfer type and cycle duration and use it in libata-acpi and pata_acpi to replace private timing tables.

Ok that makes sense having now seen the second chunk of patches. Just one problem:

drivers/ata/libata-acpi.c EXPORT_SYMBOL_GPL(ata_acpi_udma_cycle); · line 455

Quoted 3 lines

drivers/ata/libata-acpi.c +3 -0

+	/* PIO */
+	mode = ata_timing_cycle2mode(ATA_SHIFT_PIO, gtm->drive[unit].pio);
+	xfer_mask |= ata_xfer_mode2mask(mode);

No check v 0xFF

Quoted 8 lines

+ * ata_timing_cycle2mode - find xfer mode for the specified cycle duration
+ * @xfer_shift: ATA_SHIFT_* value for transfer type to examine.
+ * @cycle: cycle duration in ns
+ *
+ * Return matching xfer mode for @cycle. The returned mode is of
+ * the transfer type specified by @xfer_shift. If @cycle is too
+ * slow for @xfer_shift, 0xff is returned. If @cycle is faster
+ * than the fastest known mode, the fasted m

Can return 0xFF

Tejun Heo

author

2007-11-06 10:59 UTC #10 lore raw

Re: [PATCH 05/12] libata: xfer_mask is unsigned int not unsigned long

In reply to Alan Cox

Quoted 9 lines · Alan Cox

Quoted 6 lines · On Tue, 6 Nov 2007 14:39:03 +0900 Tejun Heo <[email protected]> wrote:
xfer_mask is unsigned int not unsigned long. Change ->mode_filter to take and return unsigned int.
While at it, rename @adev of ata_pci_default_filter() to @dev for consistency.

Quoted 7 lines

Quoted 1 line
Signed-off-by Tejun Heo <[email protected]>
The filter type was purposefully unsigned long to allow for expansion (eg for SWDMA) without breaking drivers. No problem with it changing but I'd say "unsigned int" was the worst possible choice - its now shorter (no room for expansion) and size dependant on arch. u32 would be shorter and consistent across all platforms, ulong would have more room for expansion.

I agree it should be one of u* types and am happy to convert. This patch is primarily for consistency as in libata-core, xfer_mask is unsigned int. My first proposal was u32 too but Jeff vetoed it. IIRC, Jeff has affection for machine-independent integral types. :-)

Anyways, I think ulong is worse than uint because ulong differs in size between 32 and 64 archs. What's the good of extra 32bits in flags space if it's not there on 32bit archs? Also, we currently only use 20bits of xfermask, 12 extra bits seem enough for the time, especially as everything is moving over to SATA where xfermode doesn't really matter, no?

Jeff, are you okay with u32 or 64?

Signature · 1 line

tejun

Alan Cox

Acked-by on v1 11/12

2007-11-06 10:59 UTC #31 lore raw

Re: [PATCH 11/12] libata: add ATA_CBL_PATA_IGN

In reply to Tejun Heo

Quoted 6 lines

This patch adds ATA_CBL_PATA_IGN which tells libata to ignore the cable type completely and just let the LLD determine the transfer mode via host transfer mode masks and ->mode_filter().
Signed-off-by Tejun Heo <[email protected]>
Cc Alan Cox <[email protected]>

Acked-by Alan Cox <[email protected]>

Tejun Heo

author

2007-11-06 11:01 UTC #18 lore raw

Re: [PATCH 06/12] libata: separate out ata_acpi_gtm_xfermask() from pacpi_discover_modes()

In reply to Alan Cox

Quoted 20 lines · Alan Cox

Quoted 5 lines
drivers/ata/libata-acpi.c +5 -0
+/* Welcome to ACPI, bring a bucket */
+const unsigned int ata_acpi_pio_cycle[7] = {
+	600, 383, 240, 180, 120, 100, 80
+};
+EXPORT_SYMBOL_GPL(ata_acpi_pio_cycle);
drivers/ata/libata-acpi.c int ata_acpi_cbl_80wire(struct ata_port *ap) · line 441
Quoted 10 lines
drivers/ata/libata-acpi.c +10 -0
+
+const unsigned int ata_acpi_mwdma_cycle[5] = {
+	480, 150, 120, 100, 80
+};
+EXPORT_SYMBOL_GPL(ata_acpi_mwdma_cycle);
+
+const unsigned int ata_acpi_udma_cycle[7] = {
+	120, 80, 60, 45, 30, 20, 15
+};
+EXPORT_SYMBOL_GPL(ata_acpi_udma_cycle);
Do we really need to keep exporting all these things. So far this patch set has exported a set of very specific ACPI arrays and a load of internal functions. That to me says the splitting up is wrong.

Quoted 2 lines

One option would be to make those tables private and simply make the pata_acpi driver use the ata_timing functions

This is mid-step of merging ACPI timing handling into the standard ata_timing mechanism. These will go away in later patch.

Signature · 1 line

tejun

Tejun Heo

author

2007-11-06 11:02 UTC #32 lore raw

Re: [PATCH 11/12] libata: add ATA_CBL_PATA_IGN

In reply to Alan Cox

Quoted 9 lines · Alan Cox

Quoted 6 lines
This patch adds ATA_CBL_PATA_IGN which tells libata to ignore the cable type completely and just let the LLD determine the transfer mode via host transfer mode masks and ->mode_filter().
Signed-off-by Tejun Heo <[email protected]>
Cc Alan Cox <[email protected]>
Acked-by Alan Cox <[email protected]>

I think this can be applied to pata_acpi too as we don't have any idea about actual cable there.

Thanks.

Signature · 1 line

tejun

Tejun Heo

author

2007-11-06 11:09 UTC #26 lore raw

Re: [PATCH 08/12] libata: implement ata_timing_cycle2mode() and use it in libata-acpi and pata_acpi

In reply to Alan Cox

Quoted 15 lines · Alan Cox

drivers/ata/libata-acpi.c EXPORT_SYMBOL_GPL(ata_acpi_udma_cycle); · line 455
Quoted 3 lines
drivers/ata/libata-acpi.c +3 -0
+	/* PIO */
+	mode = ata_timing_cycle2mode(ATA_SHIFT_PIO, gtm->drive[unit].pio);
+	xfer_mask |= ata_xfer_mode2mask(mode);
No check v 0xFF
Quoted 8 lines
+ * ata_timing_cycle2mode - find xfer mode for the specified cycle duration
+ * @xfer_shift: ATA_SHIFT_* value for transfer type to examine.
+ * @cycle: cycle duration in ns
+ *
+ * Return matching xfer mode for @cycle. The returned mode is of
+ * the transfer type specified by @xfer_shift. If @cycle is too
+ * slow for @xfer_shift, 0xff is returned. If @cycle is faster
+ * than the fastest known mode, the fasted m

Quoted 1 line

Can return 0xFF

The check is implicit in ata_xfer_mode2mask(), 0xff translates to xfermask 0, vice-versa. 0xff is internally a valid value indicating no mode.

Thanks.

Signature · 1 line

tejun

Tejun Heo

author

2007-11-06 11:21 UTC #6 lore raw

Re: [PATCH 04/12] libata: kill ata_id_to_dma_mode()

In reply to Alan Cox

Quoted 10 lines · Alan Cox

Quoted 3 lines · On Tue, 6 Nov 2007 14:39:02 +0900 Tejun Heo <[email protected]> wrote:
ata_id_to_dma_mode() isn't quite generic. The function is basically privately implemented ata_id_xfermask() combined with hardcoded mode printing and configuration which are specific to ata_generic.
I anticpiated other users. This seems a backward move - we end up exporting lots of low level detail into the drivers which should be none of their business.

Those functions need to be exported anyway for pata_amd and I think it's natural to export them as xfer mode and mask handling is pretty fundamental and having those helpers around is handy when implementing LLD-specific mode selection.

ata_id_to_dma_mode() just seemed a bit too specific to me. Sans ata_id_xfermask() part, printing configured mode and using "DMA" when there's no applicable DMA mode don't seem too useful as generic xfer mode/mask helper. I agree that the latter part of the function can be factored tho (setting xfer_mode, shift and clearing PIO flag according to determined xfer_mask). But since ata_generic() is currently the only user, factoring that part out seems an overkill.

Well, I guess this is a matter of taste after all. How about letting Jeff determine it? I'm okay as long as generic ata_id_xfermask() is used there.

Thanks.

Signature · 1 line

tejun

Alan Cox

2007-11-06 11:24 UTC #33 lore raw

Re: [PATCH 11/12] libata: add ATA_CBL_PATA_IGN

In reply to Tejun Heo

Quoted 5 lines · On Tue, 06 Nov 2007 20:02:28 +0900 Tejun Heo <[email protected]> wrote:

Quoted 4 lines · Alan Cox
Quoted 4 lines
This patch adds ATA_CBL_PATA_IGN which tells libata to ignore the cable type completely and just let the LLD determine the transfer mode via host transfer mode masks and ->mode_filter().

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
Signed-off-by Tejun Heo <[email protected]>
Cc Alan Cox <[email protected]>
Acked-by Alan Cox <[email protected]>
I think this can be applied to pata_acpi too as we don't have any idea about actual cable there.

Agreed entirely - and its beginning to look like pata_via is also a candidate as some boards have real issues.

Jeff Garzik

2007-11-06 17:53 UTC #11 lore raw

Re: [PATCH 05/12] libata: xfer_mask is unsigned int not unsigned long

In reply to Tejun Heo

Quoted 22 lines · Tejun Heo

Quoted 9 lines · Alan Cox
Quoted 6 lines · On Tue, 6 Nov 2007 14:39:03 +0900 Tejun Heo <[email protected]> wrote:
xfer_mask is unsigned int not unsigned long. Change ->mode_filter to take and return unsigned int.
While at it, rename @adev of ata_pci_default_filter() to @dev for consistency.
Quoted 6 lines
Quoted 1 line
Signed-off-by Tejun Heo <[email protected]>
The filter type was purposefully unsigned long to allow for expansion (eg for SWDMA) without breaking drivers. No problem with it changing but I'd say "unsigned int" was the worst possible choice - its now shorter (no room for expansion) and size dependant on arch. u32 would be shorter and consistent across all platforms, ulong would have more room for expansion.
I agree it should be one of u* types and am happy to convert. This patch is primarily for consistency as in libata-core, xfer_mask is unsigned int. My first proposal was u32 too but Jeff vetoed it. IIRC, Jeff has affection for machine-independent integral types. :-)

Quoted 7 lines

Anyways, I think ulong is worse than uint because ulong differs in size between 32 and 64 archs. What's the good of extra 32bits in flags space if it's not there on 32bit archs? Also, we currently only use 20bits of xfermask, 12 extra bits seem enough for the time, especially as everything is moving over to SATA where xfermode doesn't really matter, no?
Jeff, are you okay with u32 or 64?

unsigned long is IMO the best choice for bitmaps.

* it is a machine int on all(?) architectures

* it is 32-bit on 32-bit architectures

* it is consistent with test_bit(), set_bit() and lib/bitmap.c interfaces

* conversion to test_bit() and lib/bitmap.c interfaces as a future step is trivial

* your structs inflate on 64-bit due to pointers anyway, so I see little real consequence to using the lower 32 bits of an unsigned long as a portable meme.

Jeff

Tejun Heo

author

2007-11-07 01:12 UTC #12 lore raw

Re: [PATCH 05/12] libata: xfer_mask is unsigned int not unsigned long

Quoted 5 lines · Jeff Garzik

Quoted 1 line
Jeff, are you okay with u32 or 64?
unsigned long is IMO the best choice for bitmaps.
* it is a machine int on all(?) architectures

I don't really see much point in this. What's the advantage of a machine int for xfer mask?

Quoted 1 line

* it is 32-bit on 32-bit architectures

The problem I see for using native integral types for flags or mask is that it's not fixed in size, so you can only use the smallest of all the supported architectures. We know for all archs we support, both unsigned int and unsigned long are at least 32bits long but to me making the assumption about expected number of bits using u32 or u64 is much more important than other considerations.

Quoted 4 lines

* it is consistent with test_bit(), set_bit() and lib/bitmap.c interfaces
* conversion to test_bit() and lib/bitmap.c interfaces as a future step is trivial

I don't think it's likely that we'll need those heavy machineries for xfermask and the correct approach is to convert when the need actually arises.

Quoted 3 lines

* your structs inflate on 64-bit due to pointers anyway, so I see little real consequence to using the lower 32 bits of an unsigned long as a portable meme.

I'm not trying to optimize performance or size. I'm trying to make programming assumptions clear. I think it's silly to optimize anything for xfermask. It just has to work and be clear to people working with it.

An optimal but overkill solution would be using fast_u32 or fast_u64 types such that the compiler can choose as necessary but as we don't have that yet && xfermask handling is a very very cold path, I think we should be aiming for clarity.

Thanks.

Signature · 1 line

tejun

Tejun Heo

author

2007-11-23 01:08 UTC #36 lore raw

In reply to Tejun Heo

Quoted 47 lines · Tejun Heo

Hello,
This patchset cleans up and improves PATA timing related code and fix pata_amd transfer mode selection on top of the improvements. This patchset contains the following tweleve patches.
0001-ata_generic-unindent-loop-in-generic_set_mode.patch 0002-libata-export-xfermode-PATA-timing-related-functi.patch 0003-libata-clean-up-xfermode-PATA-timing-related-stuf.patch 0004-libata-kill-ata_id_to_dma_mode.patch 0005-libata-xfer_mask-is-unsigned-int-not-unsigned-long.patch 0006-libata-separate-out-ata_acpi_gtm_xfermask-from-pa.patch 0007-libata-fix-ata_acpi_gtm_xfermask.patch 0008-libata-implement-ata_timing_cycle2mode-and-use-it.patch 0009-libata-implement-ata_acpi_init_gtm.patch 0010-libata-reimplement-ata_acpi_cbl_80wire-using-ata_.patch 0011-libata-add-ATA_CBL_PATA_IGN.patch 0012-pata_amd-update-mode-selection-for-NV-PATAs.patch
0001-0005 are clean ups of timing handling code. 0006-0008 unifies timing handling code in libata-acpi.c and pata_acpi.c with the core timing code.
0009 implements initial GTM caching. I thought about moving this to LLDs but it's too much hassle. GTM is host wide and doing it from ->error_handler would require cross-port synchronization. Left alternative is adding a separate hook. Simply doing it during ACPI attach is simpler.
0010 reimplements ata_acpi_cbl_80wire() using ata_acpi_gtm_xfermask(). This new function takes @gtm argument. Both users (pata_via and pata_amd) are converted to pass initial GTM.
0011 implements ATA_CBL_PATA_IGN which tells libata to ignore cable type and supporess all related handling.
0012 updates mode selection for NV PATA controllers. We just can't tell what cable is attached on the controller either from host or drive side. So, for those controllers, pata_amd just sets cable type to ATA_CBL_PATA_IGN and use ->mode_filter to limit transfer mode according to BIOS/ACPI boot configuration. If that fails (not likely), pata_amd simply sets the highest allowed speed and let EH figure out the mess. With recent updates, EH should be able to figure it out pretty soon.
This patchset is against

Quoted 7 lines

   #upstream-fixes (6bbfd53d47abd1fb20d7c93a9b19a75970b66f49)
+ update-speeddown patchset [1]
Thanks.
--
tejun

Ping.

Signature · 1 line

tejun

Jeff Garzik

2007-11-24 01:13 UTC #13 lore raw

Re: [PATCH 05/12] libata: xfer_mask is unsigned int not unsigned long

In reply to Tejun Heo

Quoted 7 lines · Tejun Heo

xfer_mask is unsigned int not unsigned long. Change ->mode_filter to take and return unsigned int.
While at it, rename @adev of ata_pci_default_filter() to @dev for consistency.
Signed-off-by Tejun Heo <[email protected]>

Quoted 11 lines

---
 drivers/ata/libata-sff.c       |    5 +++--
 drivers/ata/pata_acpi.c        |    2 +-
 drivers/ata/pata_ali.c         |    2 +-
 drivers/ata/pata_hpt366.c      |    2 +-
 drivers/ata/pata_hpt37x.c      |    4 ++--
 drivers/ata/pata_pdc2027x.c    |    4 ++--
 drivers/ata/pata_scc.c         |    2 +-
 drivers/ata/pata_serverworks.c |    4 ++--
 include/linux/libata.h         |    5 +++--
 9 files changed, 16 insertions(+), 14 deletions(-)

as I noted I permit unsigned long, which is a naturally aligned machine int on our platforms.

Consistency patches in the other direction (moving ATA_MASK_* to a separate enum and marking with UL suffix) are welcome.

Jeff Garzik

2007-11-24 01:13 UTC #14 lore raw

Re: [PATCH 05/12] libata: xfer_mask is unsigned int not unsigned long

Quoted 20 lines · Jeff Garzik

Quoted 7 lines · Tejun Heo
xfer_mask is unsigned int not unsigned long. Change ->mode_filter to take and return unsigned int.
While at it, rename @adev of ata_pci_default_filter() to @dev for consistency.
Signed-off-by Tejun Heo <[email protected]>
Quoted 11 lines
---
 drivers/ata/libata-sff.c       |    5 +++--
 drivers/ata/pata_acpi.c        |    2 +-
 drivers/ata/pata_ali.c         |    2 +-
 drivers/ata/pata_hpt366.c      |    2 +-
 drivers/ata/pata_hpt37x.c      |    4 ++--
 drivers/ata/pata_pdc2027x.c    |    4 ++--
 drivers/ata/pata_scc.c         |    2 +-
 drivers/ata/pata_serverworks.c |    4 ++--
 include/linux/libata.h         |    5 +++--
 9 files changed, 16 insertions(+), 14 deletions(-)

Quoted 1 line

as I noted I permit unsigned long, which is a naturally aligned machine int on our platforms.

er, s/permit/prefer/

Jeff Garzik

2007-11-24 01:13 UTC #7 lore raw

Re: [PATCH 04/12] libata: kill ata_id_to_dma_mode()

In reply to Tejun Heo

Quoted 8 lines · Tejun Heo

ata_id_to_dma_mode() isn't quite generic. The function is basically privately implemented ata_id_xfermask() combined with hardcoded mode printing and configuration which are specific to ata_generic.
Kill the function and open code it in generic_set_mode() using generic xfermode handling functions.
Signed-off-by Tejun Heo <[email protected]>

ACK patches 1-4

Jeff Garzik

2007-11-24 01:14 UTC #19 lore raw

Re: [PATCH 06/12] libata: separate out ata_acpi_gtm_xfermask() from pacpi_discover_modes()

In reply to Tejun Heo

Quoted 29 lines · Tejun Heo

Finding out matching transfer mode from ACPI GTM values is useful for other purposes too. Separate out the function and timing tables from pata_acpi::pacpi_discover_modes().
Other than checking shared-configuration bit after doing ata_acpi_gtm() in pacpi_discover_modes() which should be safe, this patch doesn't introduce any behavior change.
Signed-off-by Tejun Heo <[email protected]>
Cc Alan Cox <[email protected]>
---
 drivers/ata/libata-acpi.c |   78 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/ata/pata_acpi.c   |   66 ++++++-------------------------------
 include/linux/libata.h    |   25 ++++++++++++++
 3 files changed, 114 insertions(+), 55 deletions(-)
drivers/ata/libata-acpi.c +5 -0
index 08a52dd..5ffa542 100644
@@ -424,6 +424,84 @@ int ata_acpi_cbl_80wire(struct ata_port *ap)
 
 EXPORT_SYMBOL_GPL(ata_acpi_cbl_80wire);
 
+/* Welcome to ACPI, bring a bucket */
+const unsigned int ata_acpi_pio_cycle[7] = {
+	600, 383, 240, 180, 120, 100, 80
+};
+EXPORT_SYMBOL_GPL(ata_acpi_pio_cycle);

drivers/ata/libata-acpi.c int ata_acpi_cbl_80wire(struct ata_port *ap) · line 441

Quoted 10 lines

drivers/ata/libata-acpi.c +10 -0

+
+const unsigned int ata_acpi_mwdma_cycle[5] = {
+	480, 150, 120, 100, 80
+};
+EXPORT_SYMBOL_GPL(ata_acpi_mwdma_cycle);
+
+const unsigned int ata_acpi_udma_cycle[7] = {
+	120, 80, 60, 45, 30, 20, 15
+};
+EXPORT_SYMBOL_GPL(ata_acpi_udma_cycle);

these exports seem a bit ugly

If pressed I will apply, but...

Jeff Garzik

2007-11-24 01:16 UTC #22 lore raw

Re: [PATCH 07/12] libata: fix ata_acpi_gtm_xfermask()

In reply to Tejun Heo

Quoted 6 lines · Tejun Heo

ata_acpi_gtm_xfermask() as separated out from pacpi_discover_modes() has various bugs. Fix them.
* The wrong comparison operator is used when finding for matching cycle resulting totally bogus result.

Quoted 6 lines

* With the comparion operator fixed, boundary condtion handling is clumsy.
* Setting of any DMA mask bit set all bits in PIO mask.
* MWDMA and UDMA blocks are swapped.

shouldn't this be combined with patch #6?

Jeff Garzik

2007-11-24 01:17 UTC #27 lore raw

Re: [PATCH 08/12] libata: implement ata_timing_cycle2mode() and use it in libata-acpi and pata_acpi

In reply to Tejun Heo

Quoted 28 lines · Tejun Heo

libata-acpi is using separate timing tables for transfer modes although libata-core has the complete ata_timing table. Implement ata_timing_cycle2mode() to look for matching mode given transfer type and cycle duration and use it in libata-acpi and pata_acpi to replace private timing tables.
Signed-off-by Tejun Heo <[email protected]>
Cc Alan Cox <[email protected]>
---
 drivers/ata/libata-acpi.c |   62 +++++++++++----------------------------------
 drivers/ata/libata-core.c |   52 +++++++++++++++++++++++++++++++++++++
 drivers/ata/pata_acpi.c   |   13 +++++----
 include/linux/libata.h    |    5 +---
 4 files changed, 75 insertions(+), 57 deletions(-)
drivers/ata/libata-acpi.c +0 -5
index 06d9961..7d982c2 100644
@@ -424,22 +424,6 @@ int ata_acpi_cbl_80wire(struct ata_port *ap)
 
 EXPORT_SYMBOL_GPL(ata_acpi_cbl_80wire);
 
-/* Welcome to ACPI, bring a bucket */
-const unsigned int ata_acpi_pio_cycle[7] = {
-	600, 383, 240, 180, 120, 100, 80
-};
-EXPORT_SYMBOL_GPL(ata_acpi_pio_cycle);

drivers/ata/libata-acpi.c int ata_acpi_cbl_80wire(struct ata_port *ap) · line 441

Quoted 10 lines

drivers/ata/libata-acpi.c +0 ~~-10~~

-
-const unsigned int ata_acpi_mwdma_cycle[5] = {
-	480, 150, 120, 100, 80
-};
-EXPORT_SYMBOL_GPL(ata_acpi_mwdma_cycle);
-
-const unsigned int ata_acpi_udma_cycle[7] = {
-	120, 80, 60, 45, 30, 20, 15
-};
-EXPORT_SYMBOL_GPL(ata_acpi_udma_cycle);

ah... they go away. nice.

Jeff Garzik

2007-11-24 01:18 UTC #37 lore raw

In reply to Tejun Heo

Quoted 36 lines · Tejun Heo

Hello,
This patchset cleans up and improves PATA timing related code and fix pata_amd transfer mode selection on top of the improvements. This patchset contains the following tweleve patches.
0001-ata_generic-unindent-loop-in-generic_set_mode.patch 0002-libata-export-xfermode-PATA-timing-related-functi.patch 0003-libata-clean-up-xfermode-PATA-timing-related-stuf.patch 0004-libata-kill-ata_id_to_dma_mode.patch 0005-libata-xfer_mask-is-unsigned-int-not-unsigned-long.patch 0006-libata-separate-out-ata_acpi_gtm_xfermask-from-pa.patch 0007-libata-fix-ata_acpi_gtm_xfermask.patch 0008-libata-implement-ata_timing_cycle2mode-and-use-it.patch 0009-libata-implement-ata_acpi_init_gtm.patch 0010-libata-reimplement-ata_acpi_cbl_80wire-using-ata_.patch 0011-libata-add-ATA_CBL_PATA_IGN.patch 0012-pata_amd-update-mode-selection-for-NV-PATAs.patch
0001-0005 are clean ups of timing handling code. 0006-0008 unifies timing handling code in libata-acpi.c and pata_acpi.c with the core timing code.
0009 implements initial GTM caching. I thought about moving this to LLDs but it's too much hassle. GTM is host wide and doing it from ->error_handler would require cross-port synchronization. Left alternative is adding a separate hook. Simply doing it during ACPI attach is simpler.
0010 reimplements ata_acpi_cbl_80wire() using ata_acpi_gtm_xfermask(). This new function takes @gtm argument. Both users (pata_via and pata_amd) are converted to pass initial GTM.
0011 implements ATA_CBL_PATA_IGN which tells libata to ignore cable type and supporess all related handling.

Quoted 8 lines

0012 updates mode selection for NV PATA controllers. We just can't tell what cable is attached on the controller either from host or drive side. So, for those controllers, pata_amd just sets cable type to ATA_CBL_PATA_IGN and use ->mode_filter to limit transfer mode according to BIOS/ACPI boot configuration. If that fails (not likely), pata_amd simply sets the highest allowed speed and let EH figure out the mess. With recent updates, EH should be able to figure it out pretty soon.

overall this seems pretty sane... but I lean towards applying it in 2.6.25 rather than 2.6.24, since we are fairly deep into 2.6.24-rc at this point, with little time to test these and the speed down changes to ensure no last-minute breakage.

Tejun Heo

author

2007-11-27 08:40 UTC #23 lore raw

Re: [PATCH 07/12] libata: fix ata_acpi_gtm_xfermask()

In reply to Jeff Garzik

Quoted 7 lines · Jeff Garzik

Quoted 6 lines · Tejun Heo
ata_acpi_gtm_xfermask() as separated out from pacpi_discover_modes() has various bugs. Fix them.
* The wrong comparison operator is used when finding for matching cycle resulting totally bogus result.

Quoted 8 lines

Quoted 6 lines
* With the comparion operator fixed, boundary condtion handling is clumsy.
* Setting of any DMA mask bit set all bits in PIO mask.
* MWDMA and UDMA blocks are swapped.
shouldn't this be combined with patch #6?

I did it that way at first but that made the patch difficult to review because the content changes (bug fixes) are hidden by moving whole function from pata_acpi to libata-acpi, so I separated out the fix part into this patch.

Thanks.

Signature · 1 line

tejun

Tejun Heo

author

2007-11-27 08:42 UTC #15 lore raw

Re: [PATCH 05/12] libata: xfer_mask is unsigned int not unsigned long

In reply to Jeff Garzik

Quoted 21 lines · Jeff Garzik

Quoted 20 lines · Jeff Garzik
Quoted 18 lines · Tejun Heo
xfer_mask is unsigned int not unsigned long. Change ->mode_filter to take and return unsigned int.
While at it, rename @adev of ata_pci_default_filter() to @dev for consistency.
Signed-off-by Tejun Heo <[email protected]>
---
 drivers/ata/libata-sff.c       |    5 +++--
 drivers/ata/pata_acpi.c        |    2 +-
 drivers/ata/pata_ali.c         |    2 +-
 drivers/ata/pata_hpt366.c      |    2 +-
 drivers/ata/pata_hpt37x.c      |    4 ++--
 drivers/ata/pata_pdc2027x.c    |    4 ++--
 drivers/ata/pata_scc.c         |    2 +-
 drivers/ata/pata_serverworks.c |    4 ++--
 include/linux/libata.h         |    5 +++--
 9 files changed, 16 insertions(+), 14 deletions(-)

Quoted 4 lines

Quoted 2 lines
as I noted I permit unsigned long, which is a naturally aligned machine int on our platforms.
er, s/permit/prefer/

Alright. I don't agree but don't have much problem either. Will update them the other way.

Signature · 1 line

tejun