[PATCH libata-dev#upstream 1/2] libata: export several functions

[PATCH libata-dev#upstream 1/2] libata: export several functions

[Tejun Heo]

Export ata_pack_xfermask(), ata_unpack_xfermask() and
ata_mode_string().

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
 drivers/ata/libata-core.c |   16 ++++++++--------
 include/linux/libata.h    |    5 +++++
 2 files changed, 13 insertions(+), 8 deletions(-)

Index: work/drivers/ata/libata-core.c
===================================================================
--- work.orig/drivers/ata/libata-core.c
+++ work/drivers/ata/libata-core.c
@@ -424,9 +424,8 @@ int ata_build_rw_tf(struct ata_taskfile 
  *	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) |
@@ -443,10 +442,8 @@ static unsigned int ata_pack_xfermask(un
  *	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;
@@ -548,7 +545,7 @@ static int ata_xfer_mode2shift(unsigned 
  *	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",
@@ -6323,6 +6320,9 @@ EXPORT_SYMBOL_GPL(ata_noop_dev_select);
 EXPORT_SYMBOL_GPL(ata_std_dev_select);
 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_mode_string);
 EXPORT_SYMBOL_GPL(ata_check_status);
 EXPORT_SYMBOL_GPL(ata_altstatus);
 EXPORT_SYMBOL_GPL(ata_exec_command);
Index: work/include/linux/libata.h
===================================================================
--- work.orig/include/linux/libata.h
+++ work/include/linux/libata.h
@@ -761,6 +761,11 @@ extern void ata_tf_load(struct ata_port 
 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 *fis, u8 pmp);
 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 const char *ata_mode_string(unsigned int xfer_mask);
 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);

[PATCH libata-dev#upstream 2/2] pata_amd: don't configure udma mode faster than BIOS did

[Tejun Heo]

On many nvidia boards, BIOSen often set CABLE bit in EIDE Controller
Configuration Register even when 40c cable is attached but configures
transfer mode correctly (<= udma33).  As pata_amd depends on the CABLE
bit to determine the cable type and thus the highest allowed udma
mode, this often results in incorrectly configured device resulting in
CRC errors and DMA disabling.

This patch implements mode_filter for pata_amd which makes it avoid
configuring udma mode higher than BIOS did.

Signed-off-by: Tejun Heo <htejun@gmail.com>
---
Equivalent patch posted for drivers/ide/amd74xx.c too.

 drivers/ata/pata_amd.c |   80 +++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 71 insertions(+), 9 deletions(-)

Index: work/drivers/ata/pata_amd.c
===================================================================
--- work.orig/drivers/ata/pata_amd.c
+++ work/drivers/ata/pata_amd.c
@@ -27,6 +27,44 @@
 #define DRV_NAME "pata_amd"
 #define DRV_VERSION "0.2.7"
 
+static unsigned long amd_nv_mode_filter(const struct ata_port *ap,
+					struct ata_device *adev,
+					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 };
+	u32 udma = (unsigned long)ap->host->private_data;
+	unsigned int pio_mask, mwdma_mask, udma_mask, limit;
+
+	/* Cable detection is horrible on these controllers.  Don't
+	 * configure UDMA mode faster than BIOS did.
+	 */
+	if (ap->port_no == 0)
+		udma >>= 16;
+	if (adev->devno == 0)
+		udma >>= 8;
+
+	/* if BIOS didn't configure UDMA, don't filter */
+	if ((udma & 0xc0) != 0xc0)
+		return xfer_mask;
+
+	/* determine limit, don't go below UDMA2 */
+	limit = udma_mask_map[udma & 0x07] | ATA_UDMA2;
+
+	/* limit xfer_mask */
+	ata_unpack_xfermask(xfer_mask, &pio_mask, &mwdma_mask, &udma_mask);
+	if (!(udma_mask & ~limit))
+		return xfer_mask;
+
+	udma_mask &= limit;
+	xfer_mask = ata_pack_xfermask(pio_mask, mwdma_mask, udma_mask);
+	ata_dev_printk(adev, KERN_INFO,
+		       "speed limited to %s to honor BIOS limit\n",
+		       ata_mode_string(xfer_mask));
+	return xfer_mask;
+}
+
 /**
  *	timing_setup		-	shared timing computation and load
  *	@ap: ATA port being set up
@@ -374,7 +412,7 @@ static struct ata_port_operations amd66_
 	.port_disable	= ata_port_disable,
 	.set_piomode	= amd66_set_piomode,
 	.set_dmamode	= amd66_set_dmamode,
-	.mode_filter	= ata_pci_default_filter,
+	.mode_filter	= amd_nv_mode_filter,
 	.tf_load	= ata_tf_load,
 	.tf_read	= ata_tf_read,
 	.check_status 	= ata_check_status,
@@ -406,7 +444,7 @@ static struct ata_port_operations amd100
 	.port_disable	= ata_port_disable,
 	.set_piomode	= amd100_set_piomode,
 	.set_dmamode	= amd100_set_dmamode,
-	.mode_filter	= ata_pci_default_filter,
+	.mode_filter	= amd_nv_mode_filter,
 	.tf_load	= ata_tf_load,
 	.tf_read	= ata_tf_read,
 	.check_status 	= ata_check_status,
@@ -438,7 +476,7 @@ static struct ata_port_operations amd133
 	.port_disable	= ata_port_disable,
 	.set_piomode	= amd133_set_piomode,
 	.set_dmamode	= amd133_set_dmamode,
-	.mode_filter	= ata_pci_default_filter,
+	.mode_filter	= amd_nv_mode_filter,
 	.tf_load	= ata_tf_load,
 	.tf_read	= ata_tf_read,
 	.check_status 	= ata_check_status,
@@ -470,7 +508,7 @@ static struct ata_port_operations nv100_
 	.port_disable	= ata_port_disable,
 	.set_piomode	= nv100_set_piomode,
 	.set_dmamode	= nv100_set_dmamode,
-	.mode_filter	= ata_pci_default_filter,
+	.mode_filter	= amd_nv_mode_filter,
 	.tf_load	= ata_tf_load,
 	.tf_read	= ata_tf_read,
 	.check_status 	= ata_check_status,
@@ -502,7 +540,7 @@ static struct ata_port_operations nv133_
 	.port_disable	= ata_port_disable,
 	.set_piomode	= nv133_set_piomode,
 	.set_dmamode	= nv133_set_dmamode,
-	.mode_filter	= ata_pci_default_filter,
+	.mode_filter	= amd_nv_mode_filter,
 	.tf_load	= ata_tf_load,
 	.tf_read	= ata_tf_read,
 	.check_status 	= ata_check_status,
@@ -614,9 +652,10 @@ static int amd_init_one(struct pci_dev *
 			.port_ops = &amd100_port_ops
 		}
 	};
-	static struct ata_port_info *port_info[2];
 	static int printed_version;
+	struct ata_port_info pinfo, *port_info[2];
 	int type = id->driver_data;
+	u32 udma = 0;
 	u8 rev;
 	u8 fifo;
 
@@ -645,12 +684,35 @@ static int amd_init_one(struct pci_dev *
 	if (type < 3)
 		ata_pci_clear_simplex(pdev);
 
-	/* And fire it up */
+	/* Cache BIOS configured mode in host private_data, will be
+	 * used to limit UDMA transfer mode.
+	 */
+	if (pdev->vendor == PCI_VENDOR_ID_AMD)
+		pci_read_config_dword(pdev, 0x50, &udma);
+	else
+		pci_read_config_dword(pdev, 0x60, &udma);
 
-	port_info[0] = port_info[1] = &info[type];
+	/* And fire it up */
+	pinfo = info[type];
+	pinfo.private_data = (void *)(unsigned long)udma;
+	port_info[0] = port_info[1] = &pinfo;
 	return ata_pci_init_one(pdev, port_info, 2);
 }
 
+static void amd_remove_one(struct pci_dev *pdev)
+{
+	struct ata_host *host = dev_get_drvdata(&pdev->dev);
+	u32 udma = (unsigned long)host->private_data;
+
+	/* restore BIOS configured mode */
+	if (pdev->vendor == PCI_VENDOR_ID_AMD)
+		pci_write_config_dword(pdev, 0x50, udma);
+	else
+		pci_write_config_dword(pdev, 0x60, udma);
+
+	return ata_pci_remove_one(pdev);
+}
+
 static int amd_reinit_one(struct pci_dev *pdev)
 {
 	if (pdev->vendor == PCI_VENDOR_ID_AMD) {
@@ -695,7 +757,7 @@ static struct pci_driver amd_pci_driver 
 	.name 		= DRV_NAME,
 	.id_table	= amd,
 	.probe 		= amd_init_one,
-	.remove		= ata_pci_remove_one,
+	.remove		= amd_remove_one,
 	.suspend	= ata_pci_device_suspend,
 	.resume		= amd_reinit_one,
 };

Re: [PATCH libata-dev#upstream 2/2] pata_amd: don't configure udma mode faster than BIOS did

[Alan]

> This patch implements mode_filter for pata_amd which makes it avoid
> configuring udma mode higher than BIOS did.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>

NAK, wrong for the same reason as it is wrong on the IDE one, and even
more so as we will support hotplug and the like in time too.

Re: [PATCH libata-dev#upstream 2/2] pata_amd: don't configure udma mode faster than BIOS did

[Tejun Heo]

Alan wrote:
>> This patch implements mode_filter for pata_amd which makes it avoid
>> configuring udma mode higher than BIOS did.
>>
>> Signed-off-by: Tejun Heo <htejun@gmail.com>
> 
> NAK, wrong for the same reason as it is wrong on the IDE one, and even
> more so as we will support hotplug and the like in time too.

Yeap, it's wrong for hotplug but...

1. I don't think PATA hotplug is gonna be popular and even if we get
autodetection wrong (slower than optimal, which is way better than
faster than possible) on hotplug, we can allow the user to tune it
afterwards.

2. We don't have any way to detect cable type reliably on that
controller, so getting cable type correctly automatically on hotplug is
impossible anyway.

Thanks.

-- 
tejun

Re: [PATCH libata-dev#upstream 2/2] pata_amd: don't configure udma mode faster than BIOS did

[Alan]

> 2. We don't have any way to detect cable type reliably on that
> controller, so getting cable type correctly automatically on hotplug is
> impossible anyway.

Ask the drive. For the cases where controller logic is unreliable return
CBL_UNK and let the drive deal with it. If the drive is wrong AND the
controller is wrong then the error handling logic will resolve it.

Alan