Skip to contentSkip to conversation

libata: improve timing code and fix pata_amd transfer mode selection, take #2

PATCH New Tejun Heo 3 participants 20 messages Nov 27, 2007 – Dec 2, 2007 libata

lore
Share & tools
lore

Tejun Heo

author
#0 lore raw

PATCH 1/1 libata: improve timing code and fix pata_amd transfer mode selection, take #2

Hello,

This is the second take of improve-timing-code-and-fix-pata_amd patchset.

Changes from the last take [L] are...

* 0005-libata-xfer_mask-is-unsigned-int-not-unsigned-long.patch is reversed such that xfer_mask is consistently unsigned long.

This patchset is against

#upstream (51a7ee37eaa85b8c35fe6090618e34ed4ce1d316)
+ improve-speed-down patchset, take #3 [1]

This patchset is also available as the following git tree.

master.kernel.org:/pub/scm/linux/kernel/git/tj/libata-dev.git improve-timing-code-and-fix-pata_amd

Thanks.

--
tejun

[L] http://thread.gmane.org/gmane.linux.ide/24524
[1] http://thread.gmane.org/gmane.linux.ide/25406

author
#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
#2 lore raw

Re: [PATCH 01/12] ata_generic: unindent loop in generic_set_mode()

Quoted 7 lines · Tejun Heo

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(-)

applied 1-12 to #upstream, with comments

author
#3 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 fe453c7..6077dc1 100644
@@ -453,9 +453,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) |

@@ -472,10 +471,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;

@@ -508,7 +505,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;

@@ -531,7 +528,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;
 

@@ -553,7 +550,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;
 

@@ -577,7 +574,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",

@@ -1466,7 +1463,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;
 

@@ -2853,11 +2850,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;

@@ -7557,6 +7554,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);

@@ -7622,6 +7626,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 a1b47d8..12d62df 100644
@@ -848,6 +848,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);

@@ -917,6 +926,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
author
#4 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 6077dc1..4f06ba7 100644
@@ -486,9 +486,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, },
 };
 

@@ -503,7 +503,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)
 {

@@ -513,7 +513,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;
 }
 
 /**

@@ -534,7 +534,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;
 }
 

@@ -1312,7 +1313,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 {

@@ -2786,38 +2787,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 }
 };

@@ -2852,12 +2848,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,

@@ -3120,7 +3118,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)

@@ -3131,7 +3129,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;

@@ -3145,7 +3143,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 12d62df..2a8c925 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
author
#5 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 2a8c925..006183e 100644
@@ -887,7 +887,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
#6 lore raw

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

Quoted 6 lines · On Tue, 27 Nov 2007 19:43:41 +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.

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

That one I still think is a bad idea. This sort of internal knowledge belongs in library routines. It is a pretty generic function for the case where the device is using existing modes and wants to display them.

author
#7 lore raw

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

Quoted 7 lines · Alan Cox
Quoted 4 lines · On Tue, 27 Nov 2007 19:43:41 +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.

Quoted 6 lines
Quoted 2 lines

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

That one I still think is a bad idea. This sort of internal knowledge belongs in library routines. It is a pretty generic function for the case where the device is using existing modes and wants to display them.

The thing that bothers me is that the code assumes SWDMA and reports "DMA" and sets xfer_mask for MWDMA0. This is specific to ata_generic and with that part removed, what can be factored out is...

	ata_dev_printk(dev, KERN_INFO, "configured for %s\n",
		       ata_mode_string(xfer_mask));
	dev->xfer_mode = ata_xfer_mask2mode(xfer_mask);
	dev->xfer_shift = ata_xfer_mode2shift(dev->xfer_mode);

Unfortunately, ata_generic won't be able to use such generic helper because it uses unspecified DMA and PIO modes which generic helper wouldn't support.

Unless we are gonna have other drivers which would blindly trust device setting regarding PIO/DMA modes, I don't think ata_id_to_dma_mode() would be useful, and if we are going to have more such drivers, we at least need to rename ata_id_to_dma_mode() to note that the function blindly trusts device reported configuration.

Thanks.

Signature · 1 line
tejun
#8 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]>

[responding to Alan's objection, and subsequent thread]

Yeah, it's a matter of taste. I applied, figuring "in for a dime in for a dozen" :)

I'm not thrilled with all these fine-grained exports of timing tables and mode mask functions.

I applied the patches, but I would really like to hide more of these details away from LLD [if possible, the obvious disclaimed...]

Jeff

author
#9 lore raw

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

In reply to Tejun Heo

Jeff says xfer_mask is unsigned long not unsigned int. Convert all xfermask fields and handling functions to deal with unsigned longs.

Signed-off-by Tejun Heo <[email protected]>
---
 drivers/ata/libata-core.c |   29 ++++++++++++++-------------
 include/linux/libata.h    |   46 ++++++++++++++++++++++++--------------------
 2 files changed, 40 insertions(+), 35 deletions(-)
drivers/ata/libata-core.c +15 -14
index 88ed491..53caad7 100644
@@ -453,8 +453,9 @@ int ata_build_rw_tf(struct ata_taskfile *tf, struct ata_device *dev,
  *	RETURNS:
  *	Packed xfer_mask.
  */
-unsigned int ata_pack_xfermask(unsigned int pio_mask,
-			       unsigned int mwdma_mask, unsigned int udma_mask)
+unsigned long ata_pack_xfermask(unsigned long pio_mask,
+				unsigned long mwdma_mask,
+				unsigned long udma_mask)
 {
 	return ((pio_mask << ATA_SHIFT_PIO) & ATA_MASK_PIO) |
 		((mwdma_mask << ATA_SHIFT_MWDMA) & ATA_MASK_MWDMA) |

@@ -471,8 +472,8 @@ 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.
  */
-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 long xfer_mask, unsigned long *pio_mask,
+			 unsigned long *mwdma_mask, unsigned long *udma_mask)
 {
 	if (pio_mask)
 		*pio_mask = (xfer_mask & ATA_MASK_PIO) >> ATA_SHIFT_PIO;

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

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

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

@@ -575,7 +576,7 @@ 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>".
  */
-const char *ata_mode_string(unsigned int xfer_mask)
+const char *ata_mode_string(unsigned long xfer_mask)
 {
 	static const char * const xfer_mode_str[] = {
 		"PIO0",

@@ -1422,9 +1423,9 @@ static inline void ata_dump_id(const u16 *id)
  *	RETURNS:
  *	Computed xfermask
  */
-unsigned int ata_id_xfermask(const u16 *id)
+unsigned long ata_id_xfermask(const u16 *id)
 {
-	unsigned int pio_mask, mwdma_mask, udma_mask;
+	unsigned long pio_mask, mwdma_mask, udma_mask;
 
 	/* Usual case. Word 53 indicates word 64 is valid */
 	if (id[ATA_ID_FIELD_VALID] & (1 << 1)) {

@@ -2048,7 +2049,7 @@ int ata_dev_configure(struct ata_device *dev)
 	struct ata_eh_context *ehc = &dev->link->eh_context;
 	int print_info = ehc->i.flags & ATA_EHI_PRINTINFO;
 	const u16 *id = dev->id;
-	unsigned int xfer_mask;
+	unsigned long xfer_mask;
 	char revbuf[7];		/* XYZ-99\0 */
 	char fwrevbuf[ATA_ID_FW_REV_LEN+1];
 	char modelbuf[ATA_ID_PROD_LEN+1];

@@ -2905,8 +2906,8 @@ int ata_timing_compute(struct ata_device *adev, unsigned short speed,
 int ata_down_xfermask_limit(struct ata_device *dev, unsigned int sel)
 {
 	char buf[32];
-	unsigned int orig_mask, xfer_mask;
-	unsigned int pio_mask, mwdma_mask, udma_mask;
+	unsigned long orig_mask, xfer_mask;
+	unsigned long pio_mask, mwdma_mask, udma_mask;
 	int quiet, highbit;
 
 	quiet = !!(sel & ATA_DNXFER_QUIET);

@@ -3050,7 +3051,7 @@ int ata_do_set_mode(struct ata_link *link, struct ata_device **r_failed_dev)
 
 	/* step 1: calculate xfer_mask */
 	ata_link_for_each_dev(dev, link) {
-		unsigned int pio_mask, dma_mask;
+		unsigned long pio_mask, dma_mask;
 		unsigned int mode_mask;
 
 		if (!ata_dev_enabled(dev))
include/linux/libata.h +25 -21

82 lines not on this page · raw message

Signature · 1 line
1.5.2.4
author
#10 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 785390c..d24a011 100644
@@ -421,6 +421,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 long 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 f01722e..b631eef 100644
@@ -957,11 +957,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 long 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
author
#11 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 d24a011..6dc785d 100644
@@ -453,49 +453,49 @@ EXPORT_SYMBOL_GPL(ata_acpi_udma_cycle);
 unsigned long ata_acpi_gtm_xfermask(struct ata_device *dev,
 				    const struct ata_acpi_gtm *gtm)
 {
+	unsigned long 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
author
#12 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 6dc785d..ba6b61c 100644
@@ -421,22 +421,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

@@ -453,49 +437,33 @@ EXPORT_SYMBOL_GPL(ata_acpi_udma_cycle);
 unsigned long ata_acpi_gtm_xfermask(struct ata_device *dev,
 				    const struct ata_acpi_gtm *gtm)
 {
-	unsigned long pio_mask = 0, mwdma_mask = 0, udma_mask = 0;
-	int unit, i;
-	u32 t;
+	unsigned long 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 a4737a3..244098a 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 b631eef..a7b999a 100644
@@ -937,6 +937,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),

@@ -957,10 +958,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
author
#13 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 ba6b61c..357f9a2 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 a7b999a..f056b2c 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 */

@@ -662,6 +663,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 */
 };

@@ -958,13 +960,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 long 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
author
#14 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 357f9a2..5ef2841 100644
@@ -387,44 +387,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

@@ -471,6 +433,34 @@ unsigned long 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 long 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 f056b2c..7375341 100644
@@ -966,19 +966,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 long 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)
 {

@@ -996,6 +994,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
author
#15 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 9f57b8b..e778dee 100644
@@ -2352,6 +2352,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
  *

@@ -7684,4 +7696,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 e35c740..3992557 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 7375341..62d09c8 100644
@@ -926,6 +926,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
author
#16 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..92d8d46 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 long nv_mode_filter(struct ata_device *dev,
+				    unsigned long 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 long 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%lx&0x%lx->0x%lx, "
+			"BIOS=0x%lx (0x%x) ACPI=0x%lx%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
Acked-by on v1 12/12
#17 lore raw

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

Quoted 14 lines · On Tue, 27 Nov 2007 19:43:49 +0900 Tejun Heo <[email protected]> wrote:

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.

Quoted 6 lines

* 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]>
Acked-by Alan Cox <[email protected]>
#18 lore raw

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

In reply to Tejun Heo
Quoted 18 lines · 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.

Quoted 5 lines
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(-)

Yeah, I think this is a better way to do things long term. Thanks...

#19 lore raw

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

Quoted 7 lines
Quoted 5 lines
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(-)

Yeah, I think this is a better way to do things long term. Thanks...

Ditto - and I believe we should make the same change to VIA. A subset of VIA boards are showing the same troubles.