Re: [PATCH] libata_acpi: A different strategy for using ACPI information

Re: [PATCH] libata_acpi: A different strategy for using ACPI information

[Robert Hancock]

Alan Cox wrote:
> Lots of BIOSen simply return the BIOS set modes via the ACPI methods and
> pass back the values you give it across suspend/resume. Thus instead of
> trying to do clever stuff with this data we instead use it as a way to
> take a sneak peak at cable type information when viable. This should help
> us catch more of the laptops that do weird stuff, the VIA SATA bridges
> and the totally horked Nvidia cable handling.
> 
> For now its only used by the VIA and AMD/NV driver until we get a better
> idea of whether this is a sensible idea or not.
> 
> Opinions ?
> 
> Signed-off-by: Alan Cox <alan@redhat.com>

Looks fairly reasonable to me. However, I suspect any use of _GTM is 
somewhat dangerous (at least after the resume) unless we use the _STM 
and _GTF methods in the proper sequence when resuming. (Is that in the 
-mm tree now?)

Keep in mind that in the pata_acpi case where we don't do anything to 
program the hardware directly, we can still use _STM to program a lower 
speed than the BIOS chose if we decide to do this. Windows does indeed 
do this (you can force PIO mode in the control panel, and it will also 
reduce UDMA speeds or drop to PIO if there are too many CRC errors or 
timeouts), so this should be safe. We just had better be sure that the 
speed we give it is valid, since there is no sane way for the function 
to indicate failure. (Thus the problem with the "cram in all possible 
values to see what it supports" strategy for determining mode limits..)

-- 
Robert Hancock      Saskatoon, SK, Canada
To email, remove "nospam" from hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/

Re: [PATCH] libata_acpi: A different strategy for using ACPI information

[Alan Cox]

> Looks fairly reasonable to me. However, I suspect any use of _GTM is 
> somewhat dangerous (at least after the resume) unless we use the _STM 
> and _GTF methods in the proper sequence when resuming. (Is that in the 
> -mm tree now?)

Yes - and we only use it in these drivers to check for cable evidence not
for anything more serious.

> speed we give it is valid, since there is no sane way for the function 
> to indicate failure. (Thus the problem with the "cram in all possible 
> values to see what it supports" strategy for determining mode limits..)

The spec I have says it'll hand back the mode it actually uses which is
effectively a solution for 'failure'

[PATCH] libata_acpi: A different strategy for using ACPI information

[Alan Cox]

Lots of BIOSen simply return the BIOS set modes via the ACPI methods and
pass back the values you give it across suspend/resume. Thus instead of
trying to do clever stuff with this data we instead use it as a way to
take a sneak peak at cable type information when viable. This should help
us catch more of the laptops that do weird stuff, the VIA SATA bridges
and the totally horked Nvidia cable handling.

For now its only used by the VIA and AMD/NV driver until we get a better
idea of whether this is a sensible idea or not.

Opinions ?

Signed-off-by: Alan Cox <alan@redhat.com>

diff -u --new-file --exclude-from /usr/src/exclude --recursive linux.vanilla-2.6.22-rc6-mm1/drivers/ata/libata-acpi.c linux-2.6.22-rc6-mm1/drivers/ata/libata-acpi.c
--- linux.vanilla-2.6.22-rc6-mm1/drivers/ata/libata-acpi.c	2007-07-02 20:50:11.000000000 +0100
+++ linux-2.6.22-rc6-mm1/drivers/ata/libata-acpi.c	2007-07-02 21:02:24.000000000 +0100
@@ -296,6 +296,44 @@
 }
 
 /**
+ * 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->device[0]))
+		return 1;
+	if ((valid & 2) && ata_dev_enabled(&ap->device[1]))
+		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)
diff -u --new-file --exclude-from /usr/src/exclude --recursive linux.vanilla-2.6.22-rc6-mm1/include/linux/libata.h linux-2.6.22-rc6-mm1/include/linux/libata.h
--- linux.vanilla-2.6.22-rc6-mm1/include/linux/libata.h	2007-07-02 20:50:14.000000000 +0100
+++ linux-2.6.22-rc6-mm1/include/linux/libata.h	2007-07-02 21:29:18.000000000 +0100
@@ -867,6 +872,13 @@
 				  ATA_TIMING_CYCLE | ATA_TIMING_UDMA,
 };
 
+/* libata-acpi.c */
+#ifdef CONFIG_ATA_ACPI
+extern int ata_acpi_cbl_80wire(struct ata_port *ap);
+#else
+static inline int ata_acpi_cbl_80wire(struct ata_port *ap) { return 0; }
+#endif
+
 
 #ifdef CONFIG_PCI
 struct pci_bits {
diff -u --new-file --exclude-from /usr/src/exclude --recursive linux.vanilla-2.6.22-rc6-mm1/drivers/ata/pata_amd.c linux-2.6.22-rc6-mm1/drivers/ata/pata_amd.c
--- linux.vanilla-2.6.22-rc6-mm1/drivers/ata/pata_amd.c	2007-07-02 20:50:11.000000000 +0100
+++ linux-2.6.22-rc6-mm1/drivers/ata/pata_amd.c	2007-07-02 21:03:00.000000000 +0100
@@ -268,6 +268,9 @@
  	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_cbl_80wire(ap))
+		cbl = ATA_CBL_PATA80;
 	return cbl;
 }
 
diff -u --new-file --exclude-from /usr/src/exclude --recursive linux.vanilla-2.6.22-rc6-mm1/drivers/ata/pata_via.c linux-2.6.22-rc6-mm1/drivers/ata/pata_via.c
--- linux.vanilla-2.6.22-rc6-mm1/drivers/ata/pata_via.c	2007-07-02 20:48:49.000000000 +0100
+++ linux-2.6.22-rc6-mm1/drivers/ata/pata_via.c	2007-07-02 21:04:59.000000000 +0100
@@ -180,6 +180,9 @@
 	   two drives */
 	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))
+		return ATA_CBL_PATA80;
 	return ATA_CBL_PATA40;
 }
 

Re: [PATCH] libata_acpi: A different strategy for using ACPI information

[Tejun Heo]

Alan Cox wrote:
> Lots of BIOSen simply return the BIOS set modes via the ACPI methods and
> pass back the values you give it across suspend/resume. Thus instead of
> trying to do clever stuff with this data we instead use it as a way to
> take a sneak peak at cable type information when viable. This should help
> us catch more of the laptops that do weird stuff, the VIA SATA bridges
> and the totally horked Nvidia cable handling.
> 
> For now its only used by the VIA and AMD/NV driver until we get a better
> idea of whether this is a sensible idea or not.
> 
> Opinions ?

If you don't do _STM first, it basically boils down to reading BIOS
setting during boot.  Over suspend/resume cycle, the values are
preserved by libata-acpi.  At this point, I think honoring the BIOS
setting is the right thing to do.  It just took us too long and I'm not
too sure about using ACPI for this.  It's generic so it's certainly
better in some aspect.  It's just a bit obfuscating, IMHO.

-- 
tejun