2007-02-20 17:09:40

by Alan

[permalink] [raw]
Subject: [PATCH] ACPI driver support for pata

- Add a driver for motherboard ACPI method devices
- Link it after normal drivers so ACPI is not preferred
- Hook the AMD driver to prefer ACPI for the Nvidia devices if ACPI is
active
- While we are at it fix up the simplex clear the AMD driver.

Depends upon the set_mode -> do_set_mode wrapper patch

Signed-off-by: Alan Cox <[email protected]>

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.20-mm2/drivers/ata/Kconfig linux-2.6.20-mm2/drivers/ata/Kconfig
--- linux.vanilla-2.6.20-mm2/drivers/ata/Kconfig 2007-02-20 13:37:58.000000000 +0000
+++ linux-2.6.20-mm2/drivers/ata/Kconfig 2007-02-20 13:43:29.000000000 +0000
@@ -174,6 +174,15 @@
You can disable this at kernel boot time by using the
option libata.noacpi=1

+config PATA_ACPI
+ tristate "ACPI firmware driver for PATA"
+ depends on ACPI && PCI && SATA_ACPI
+ help
+ This option enables an ACPI method driver which drives
+ motherboard PATA controller interfaces through the ACPI
+ firmware in the BIOS. This driver can sometimes handle
+ otherwise unsupported hardware.
+
config PATA_ALI
tristate "ALi PATA support (Experimental)"
depends on PCI && EXPERIMENTAL
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.20-mm2/drivers/ata/libata-acpi.c linux-2.6.20-mm2/drivers/ata/libata-acpi.c
--- linux.vanilla-2.6.20-mm2/drivers/ata/libata-acpi.c 2007-02-20 13:37:58.000000000 +0000
+++ linux-2.6.20-mm2/drivers/ata/libata-acpi.c 2007-02-20 13:43:40.000000000 +0000
@@ -15,7 +15,7 @@
#include <linux/libata.h>
#include <linux/pci.h>
#include "libata.h"
-
+#include "libata-acpi.h"
#include <acpi/acpi_bus.h>
#include <acpi/acnames.h>
#include <acpi/acnamesp.h>
@@ -696,3 +696,88 @@
}


+int ata_acpi_gtm(const struct ata_port *ap, void *handle, struct acpi_gtm *gtm)
+{
+ acpi_status status;
+ struct acpi_buffer output;
+
+ output.length = ACPI_ALLOCATE_BUFFER;
+ output.pointer = NULL;
+
+ status = acpi_evaluate_object(handle, "_GTM", NULL, &output);
+
+ if (ACPI_FAILURE(status)) {
+ ata_port_printk(ap, KERN_ERR, "ACPI get timing mode failed.\n");
+ return -EINVAL;
+ }
+ if (output.length < sizeof(struct acpi_gtm) || output.pointer == NULL) {
+ ata_port_printk(ap, KERN_ERR, "ACPI get timing provided invalid data.\n");
+ return -EINVAL;
+ }
+ memcpy(gtm, output.pointer, sizeof(struct acpi_gtm));
+ kfree(output.pointer);
+ return 0;
+}
+
+int ata_acpi_stm(const struct ata_port *ap, void *handle, struct acpi_gtm *stm)
+{
+ acpi_status status;
+ struct acpi_object_list input;
+ union acpi_object in_params[3];
+
+ in_params[0].type = ACPI_TYPE_BUFFER;
+ in_params[0].buffer.length = sizeof(struct acpi_gtm);
+ in_params[0].buffer.pointer = (u8 *)stm;
+ /* Buffers for id may need byteswapping ? */
+ in_params[1].type = ACPI_TYPE_BUFFER;
+ in_params[1].buffer.length = 512;
+ in_params[1].buffer.pointer = (u8 *)ap->device[0].id;
+ in_params[2].type = ACPI_TYPE_BUFFER;
+ in_params[2].buffer.length = 512;
+ in_params[2].buffer.pointer = (u8 *)ap->device[1].id;
+
+ input.count = 3;
+ input.pointer = in_params;
+
+ status = acpi_evaluate_object(handle, "_STM", &input, NULL);
+
+ if (ACPI_FAILURE(status)) {
+ ata_port_printk(ap, KERN_ERR, "ACPI set timing mode failed.\n");
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static void *ata_pata_find_handle(struct device *dev, int port)
+{
+ acpi_handle handle;
+ acpi_integer devfn;
+
+ if (noacpi)
+ return NULL;
+ if (pata_get_dev_handle(dev, &handle, &devfn) < 0)
+ return NULL;
+ return acpi_get_child(handle, port);
+}
+
+void *ata_pata_acpi_handle(const struct ata_port *ap)
+{
+ return ata_pata_find_handle(ap->host->dev, ap->port_no);
+}
+
+int ata_pata_acpi_present(struct pci_dev *pdev)
+{
+ int present = 0;
+
+ if(ata_pata_find_handle(&pdev->dev, 0))
+ present |= 1;
+ if(ata_pata_find_handle(&pdev->dev, 1))
+ present |= 2;
+ return present;
+}
+
+
+EXPORT_SYMBOL_GPL(ata_acpi_gtm);
+EXPORT_SYMBOL_GPL(ata_acpi_stm);
+EXPORT_SYMBOL_GPL(ata_pata_acpi_handle);
+EXPORT_SYMBOL_GPL(ata_pata_acpi_present);
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.20-mm2/drivers/ata/libata-acpi.h linux-2.6.20-mm2/drivers/ata/libata-acpi.h
--- linux.vanilla-2.6.20-mm2/drivers/ata/libata-acpi.h 1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.20-mm2/drivers/ata/libata-acpi.h 2007-02-20 13:43:49.000000000 +0000
@@ -0,0 +1,47 @@
+/*
+ * libata-acpi.h - helper library for ATA ACPI drivers
+ *
+ * Copyright 2007 Red Hat, Inc. All rights reserved.
+ * Copyright 2007 Alan Cox
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; see the file COPYING. If not, write to
+ * the Free Software Foundation, 675 Mass Ave, Cambridge, MA 02139, USA.
+ *
+ *
+ * libata documentation is available via 'make {ps|pdf}docs',
+ * as Documentation/DocBook/libata.*
+ *
+ */
+
+#ifndef __LIBATA_ACPI_H__
+#define __LIBATA_ACPI_H__
+
+struct acpi_drive
+{
+ u32 pio;
+ u32 dma;
+};
+
+struct acpi_gtm {
+ struct acpi_drive drive[2];
+ u32 flags;
+};
+
+extern int ata_acpi_gtm(const struct ata_port *p, void *handle, struct acpi_gtm *gtm);
+extern int ata_acpi_stm(const struct ata_port *ap, void *handle, struct acpi_gtm *stm);
+extern void *ata_pata_acpi_handle(const struct ata_port *ap);
+extern int ata_pata_acpi_present(struct pci_dev *pdev);
+
+#endif
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.20-mm2/drivers/ata/Makefile linux-2.6.20-mm2/drivers/ata/Makefile
--- linux.vanilla-2.6.20-mm2/drivers/ata/Makefile 2007-02-20 13:37:58.000000000 +0000
+++ linux-2.6.20-mm2/drivers/ata/Makefile 2007-02-20 13:52:14.000000000 +0000
@@ -60,6 +60,8 @@
obj-$(CONFIG_PATA_TRIFLEX) += pata_triflex.o
obj-$(CONFIG_PATA_IXP4XX_CF) += pata_ixp4xx_cf.o
obj-$(CONFIG_PATA_PLATFORM) += pata_platform.o
+# Should be last but two libata driver
+obj-$(CONFIG_PATA_ACPI) += pata_acpi.o
# Should be last but one libata driver
obj-$(CONFIG_ATA_GENERIC) += ata_generic.o
# Should be last libata driver
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.20-mm2/drivers/ata/pata_acpi.c linux-2.6.20-mm2/drivers/ata/pata_acpi.c
--- linux.vanilla-2.6.20-mm2/drivers/ata/pata_acpi.c 1970-01-01 01:00:00.000000000 +0100
+++ linux-2.6.20-mm2/drivers/ata/pata_acpi.c 2007-02-20 15:02:51.000000000 +0000
@@ -0,0 +1,392 @@
+/*
+ * ACPI PATA driver
+ *
+ * (c) 2007 Red Hat <[email protected]>
+ *
+ * TODO - restore modes after mode_filter chews them up
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/init.h>
+#include <linux/blkdev.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <scsi/scsi_host.h>
+#include <acpi/acpi_bus.h>
+#include <acpi/acnames.h>
+#include <acpi/acnamesp.h>
+#include <acpi/acparser.h>
+#include <acpi/acexcep.h>
+#include <acpi/acmacros.h>
+#include <acpi/actypes.h>
+
+#include <linux/libata.h>
+#include <linux/ata.h>
+
+#include "libata-acpi.h"
+
+#define DRV_NAME "pata_acpi"
+#define DRV_VERSION "0.0.3"
+
+struct pata_acpi {
+ void *handle;
+ struct acpi_gtm gtm;
+ void *last;
+};
+
+/**
+ * pacpi_pre_reset - check for 40/80 pin
+ * @ap: Port
+ *
+ * Perform the PATA port setup we need.
+ */
+
+static int pacpi_pre_reset(struct ata_port *ap)
+{
+ struct pata_acpi *acpi = ap->private_data;
+ int i;
+
+ if (acpi->handle == NULL || ata_acpi_gtm(ap, acpi->handle, &acpi->gtm) < 0)
+ return -ENODEV;
+ ap->cbl = ATA_CBL_PATA40;
+
+ /* If the firmware proposed mode is UDMA3 or better then we know
+ it is 80wire, or 40wire short */
+ for (i = 0; i < 2; i++) {
+ if (acpi->gtm.flags & (1 << 2 * i)) /* UDMA in use ? */
+ if (acpi->gtm.drive[i].dma < 55) /* 60nS is UDMA2 */
+ ap->cbl = ATA_CBL_PATA80;
+ }
+ return ata_std_prereset(ap);
+}
+
+/**
+ * pacpi_error_handler - Setup and error handler
+ * @ap: Port to handle
+ *
+ * LOCKING:
+ * None (inherited from caller).
+ */
+
+static void pacpi_error_handler(struct ata_port *ap)
+{
+ return ata_bmdma_drive_eh(ap, pacpi_pre_reset, ata_std_softreset,
+ NULL, ata_std_postreset);
+}
+
+/* Welcome to ACPI, bring a bucket */
+static const unsigned int pio_cycle[7] = {
+ 600, 383, 240, 180, 120, 100, 80
+};
+static const unsigned int mwdma_cycle[5] = {
+ 480, 150, 120, 100, 80
+};
+static const unsigned int udma_cycle[7] = {
+ 120, 80, 60, 45, 30, 20, 15
+};
+
+/**
+ * pacpi_mode_filter - filter non ACPI modes
+ * @adev: ATA device
+ * @mask: proposed modes
+ *
+ * Try the modes available and see which ones the ACPI method will
+ * set up sensibly. From this we get a mask of ACPI modes we can use
+ */
+
+static unsigned long pacpi_mode_filter(const struct ata_port *ap, struct ata_device *adev, unsigned long mask)
+{
+ int unit = adev->devno;
+ struct pata_acpi *acpi = ap->private_data;
+ int i;
+ u32 t;
+
+ struct acpi_gtm probe;
+
+ probe = acpi->gtm;
+
+ /* We always use the 0 slot for crap hardware */
+ if (!(probe.flags & 0x10))
+ unit = 0;
+
+
+ /* In order to generate a valid mode mask as we need cycle through
+ trying each proposed speed in turn */
+
+ /* Start by scanning for PIO modes */
+ for (i = 0; i < 7; i++) {
+ probe.drive[unit].pio = pio_cycle[i];
+ ata_acpi_stm(ap, acpi->handle, &probe);
+ ata_acpi_gtm(ap, acpi->handle, &probe);
+ t = probe.drive[unit].pio;
+ if (t == 0xFFFFFFFF || (i && t >= pio_cycle[i-1]))
+ mask &= ~(1 << (i + ATA_SHIFT_PIO));
+ }
+
+ /* Select MWDMA */
+ probe.flags &= ~(1 << (2 * unit));
+
+ /* Scan for MWDMA modes */
+ for (i = 0; i < 5; i++) {
+ u32 t;
+ probe.drive[unit].dma = mwdma_cycle[i];
+ ata_acpi_stm(ap, acpi->handle, &probe);
+ ata_acpi_gtm(ap, acpi->handle, &probe);
+
+ t = probe.drive[unit].dma;
+
+ if (t == 0xFFFFFFFF || (i && t >= mwdma_cycle[i-1]))
+ mask &= ~ (1 << (i + ATA_SHIFT_MWDMA));
+ }
+
+ /* Select UDMA */
+ probe.flags |= (1 << (2 * unit));
+
+ /* Scan for UDMA modes */
+ for (i = 0; i < 7; i++) {
+ u32 t;
+ probe.drive[unit].dma = udma_cycle[i];
+ ata_acpi_stm(ap, acpi->handle, &probe);
+ ata_acpi_gtm(ap, acpi->handle, &probe);
+
+ t = probe.drive[unit].dma;
+
+ if (t == 0xFFFFFFFF || (i && t >= udma_cycle[i-1]))
+ mask &= ~ (1 << (i + ATA_SHIFT_UDMA));
+ }
+
+ /* Restore the programmed timings */
+ ata_acpi_stm(ap, acpi->handle, &acpi->gtm);
+ /* And finally we can hand back the list of speeds that actually are
+ supported by the BIOS */
+ return ata_pci_default_filter(ap, adev, mask);
+}
+
+/**
+ * pacpi_set_piomode - set initial PIO mode data
+ * @ap: ATA interface
+ * @adev: ATA device
+ */
+
+static void pacpi_set_piomode(struct ata_port *ap, struct ata_device *adev)
+{
+ int unit = adev->devno;
+ struct pata_acpi *acpi = ap->private_data;
+
+ if(!(acpi->gtm.flags & 0x10))
+ unit = 0;
+
+ /* Now stuff the nS values into the structure */
+ acpi->gtm.drive[unit].pio = pio_cycle[adev->pio_mode - XFER_PIO_0];
+ ata_acpi_stm(ap, acpi->handle, &acpi->gtm);
+ /* See what mode we actually got */
+ ata_acpi_gtm(ap, acpi->handle, &acpi->gtm);
+}
+
+/**
+ * pacpi_set_dmamode - set initial DMA mode data
+ * @ap: ATA interface
+ * @adev: ATA device
+ */
+
+static void pacpi_set_dmamode(struct ata_port *ap, struct ata_device *adev)
+{
+ int unit = adev->devno;
+ struct pata_acpi *acpi = ap->private_data;
+
+ if(!(acpi->gtm.flags & 0x10))
+ unit = 0;
+
+ /* Now stuff the nS values into the structure */
+ if (adev->dma_mode >= XFER_UDMA_0) {
+ acpi->gtm.drive[unit].dma = udma_cycle[adev->dma_mode - XFER_UDMA_0];
+ acpi->gtm.flags |= (1 << (2 * unit));
+ } else {
+ acpi->gtm.drive[unit].dma = mwdma_cycle[adev->dma_mode - XFER_MW_DMA_0];
+ acpi->gtm.flags &= ~(1 << (2 * unit));
+ }
+ ata_acpi_stm(ap, acpi->handle, &acpi->gtm);
+ /* See what mode we actually got */
+ ata_acpi_gtm(ap, acpi->handle, &acpi->gtm);
+}
+
+/**
+ * pacpi_qc_issue_prot - command issue
+ * @qc: command pending
+ *
+ * Called when the libata layer is about to issue a command. We wrap
+ * this interface so that we can load the correct ATA timings if
+ * neccessary.
+ */
+
+static unsigned int pacpi_qc_issue_prot(struct ata_queued_cmd *qc)
+{
+ struct ata_port *ap = qc->ap;
+ struct ata_device *adev = qc->dev;
+ struct pata_acpi *acpi = ap->private_data;
+
+ if (acpi->gtm.flags & 0x10)
+ return ata_qc_issue_prot(qc);
+
+ if (adev != acpi->last) {
+ pacpi_set_piomode(ap, adev);
+ if (adev->dma_mode)
+ pacpi_set_dmamode(ap, adev);
+ acpi->last = adev;
+ }
+ return ata_qc_issue_prot(qc);
+}
+
+/**
+ * pacpi_port_start - port setup
+ * @ap: ATA port being set up
+ *
+ * Use the port_start hook to maintain private control structures
+ */
+
+static int pacpi_port_start(struct ata_port *ap)
+{
+ struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+ struct pata_acpi *acpi;
+
+ int ret;
+
+ acpi = ap->private_data = devm_kzalloc(&pdev->dev, sizeof(struct pata_acpi), GFP_KERNEL);
+ if (ap->private_data == NULL)
+ return -ENOMEM;
+ acpi->handle = ata_pata_acpi_handle(ap);
+
+ ret = ata_port_start(ap);
+ if (ret < 0)
+ return ret;
+
+ return ret;
+}
+
+static struct scsi_host_template pacpi_sht = {
+ .module = THIS_MODULE,
+ .name = DRV_NAME,
+ .ioctl = ata_scsi_ioctl,
+ .queuecommand = ata_scsi_queuecmd,
+ .can_queue = ATA_DEF_QUEUE,
+ .this_id = ATA_SHT_THIS_ID,
+ .sg_tablesize = LIBATA_MAX_PRD,
+ .cmd_per_lun = ATA_SHT_CMD_PER_LUN,
+ .emulated = ATA_SHT_EMULATED,
+ .use_clustering = ATA_SHT_USE_CLUSTERING,
+ .proc_name = DRV_NAME,
+ .dma_boundary = ATA_DMA_BOUNDARY,
+ .slave_configure = ata_scsi_slave_config,
+ .slave_destroy = ata_scsi_slave_destroy,
+ /* Use standard CHS mapping rules */
+ .bios_param = ata_std_bios_param,
+ .resume = ata_scsi_device_resume,
+ .suspend = ata_scsi_device_suspend,
+};
+
+static const struct ata_port_operations pacpi_ops = {
+ .port_disable = ata_port_disable,
+ .set_piomode = pacpi_set_piomode,
+ .set_dmamode = pacpi_set_dmamode,
+ .mode_filter = pacpi_mode_filter,
+
+ /* Task file is PCI ATA format, use helpers */
+ .tf_load = ata_tf_load,
+ .tf_read = ata_tf_read,
+ .check_status = ata_check_status,
+ .exec_command = ata_exec_command,
+ .dev_select = ata_std_dev_select,
+
+ .freeze = ata_bmdma_freeze,
+ .thaw = ata_bmdma_thaw,
+ .error_handler = pacpi_error_handler,
+ .post_internal_cmd = ata_bmdma_post_internal_cmd,
+
+ /* BMDMA handling is PCI ATA format, use helpers */
+ .bmdma_setup = ata_bmdma_setup,
+ .bmdma_start = ata_bmdma_start,
+ .bmdma_stop = ata_bmdma_stop,
+ .bmdma_status = ata_bmdma_status,
+ .qc_prep = ata_qc_prep,
+ .qc_issue = pacpi_qc_issue_prot,
+ .data_xfer = ata_data_xfer,
+
+ /* Timeout handling */
+ .irq_handler = ata_interrupt,
+ .irq_clear = ata_bmdma_irq_clear,
+ .irq_on = ata_irq_on,
+ .irq_ack = ata_irq_ack,
+
+ /* Generic PATA PCI ATA helpers */
+ .port_start = pacpi_port_start,
+};
+
+
+/**
+ * pacpi_init_one - Register ACPI ATA PCI device with kernel services
+ * @pdev: PCI device to register
+ * @ent: Entry in pacpi_pci_tbl matching with @pdev
+ *
+ * Called from kernel PCI layer.
+ *
+ * LOCKING:
+ * Inherited from PCI layer (may sleep).
+ *
+ * RETURNS:
+ * Zero on success, or -ERRNO value.
+ */
+
+static int pacpi_init_one (struct pci_dev *pdev, const struct pci_device_id *id)
+{
+ static struct ata_port_info info = {
+ .sht = &pacpi_sht,
+ .flags = ATA_FLAG_SLAVE_POSS | ATA_FLAG_SRST,
+
+ .pio_mask = 0x1f,
+ .mwdma_mask = 0x07,
+ .udma_mask = 0x3f,
+
+ .port_ops = &pacpi_ops,
+ };
+ struct ata_port_info *port_info[1] = { &info };
+
+ if (!ata_pata_acpi_present(pdev))
+ return -ENODEV;
+ return ata_pci_init_one(pdev, port_info, 1);
+}
+
+static const struct pci_device_id pacpi_pci_tbl[] = {
+ { PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID, PCI_CLASS_STORAGE_IDE << 8, 0xFFFFFF00UL, 1},
+ { } /* terminate list */
+};
+
+static struct pci_driver pacpi_pci_driver = {
+ .name = DRV_NAME,
+ .id_table = pacpi_pci_tbl,
+ .probe = pacpi_init_one,
+ .remove = ata_pci_remove_one,
+ .suspend = ata_pci_device_suspend,
+ .resume = ata_pci_device_resume,
+};
+
+static int __init pacpi_init(void)
+{
+ return pci_register_driver(&pacpi_pci_driver);
+}
+
+static void __exit pacpi_exit(void)
+{
+ pci_unregister_driver(&pacpi_pci_driver);
+}
+
+module_init(pacpi_init);
+module_exit(pacpi_exit);
+
+MODULE_AUTHOR("Alan Cox");
+MODULE_DESCRIPTION("SCSI low-level driver for ATA in ACPI mode");
+MODULE_LICENSE("GPL");
+MODULE_DEVICE_TABLE(pci, pacpi_pci_tbl);
+MODULE_VERSION(DRV_VERSION);
+
diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.20-mm2/drivers/ata/pata_amd.c linux-2.6.20-mm2/drivers/ata/pata_amd.c
--- linux.vanilla-2.6.20-mm2/drivers/ata/pata_amd.c 2007-02-20 13:37:58.000000000 +0000
+++ linux-2.6.20-mm2/drivers/ata/pata_amd.c 2007-02-20 14:24:49.000000000 +0000
@@ -25,7 +25,7 @@
#include <linux/libata.h>

#define DRV_NAME "pata_amd"
-#define DRV_VERSION "0.2.7"
+#define DRV_VERSION "0.2.9"

/**
* timing_setup - shared timing computation and load
@@ -642,6 +642,11 @@
if (type == 1 && rev > 0x7)
type = 2;

+#if defined(CONFIG_ATA_ACPI)
+ /* Prefer the ACPI driver for Nvidia hardware */
+ if (pdev->vendor == PCI_VENDOR_ID_NVIDIA && ata_pata_acpi_present(pdev))
+ return -ENODEV;
+#endif
/* Check for AMD7411 */
if (type == 3)
/* FIFO is broken */
@@ -654,7 +659,7 @@
pdev->subsystem_device == PCI_DEVICE_ID_AMD_SERENADE)
type = 6; /* UDMA 100 only */

- if (type < 3)
+ if (type < 7)
ata_pci_clear_simplex(pdev);

/* And fire it up */
@@ -673,9 +678,7 @@
pci_write_config_byte(pdev, 0x41, fifo & 0x0F);
else
pci_write_config_byte(pdev, 0x41, fifo | 0xF0);
- if (pdev->device == PCI_DEVICE_ID_AMD_VIPER_7409 ||
- pdev->device == PCI_DEVICE_ID_AMD_COBRA_7401)
- ata_pci_clear_simplex(pdev);
+ ata_pci_clear_simplex(pdev);
}
return ata_pci_device_resume(pdev);
}


2007-02-22 05:47:21

by Robert Hancock

[permalink] [raw]
Subject: Re: [PATCH] ACPI driver support for pata

Alan wrote:
> - Add a driver for motherboard ACPI method devices
> - Link it after normal drivers so ACPI is not preferred
> - Hook the AMD driver to prefer ACPI for the Nvidia devices if ACPI is
> active
> - While we are at it fix up the simplex clear the AMD driver.
>
> Depends upon the set_mode -> do_set_mode wrapper patch
>
> Signed-off-by: Alan Cox <[email protected]>

I tried out 2.6.21-rc1 with the pata_acpi patch. First off, when you
enable pata_acpi, it appears that the Fedora mkinitrd stuff decides that
that should be loaded as the first driver. On boot it promptly attempts
to attach to all of the ATA controllers, including the nForce SATA
controllers, which somehow it fails to actually drive causing a failure
to mount the root filesystem. I got around that by blacklisting
pata_acpi in /etc/modprobe.conf before installing the kernel and
un-blacklisting it afterwards, so it wouldn't make the initrd try and
load it, but there must be a better solution.

It does seem to drive the PATA controllers, but the cable detection
doesn't seem to be working:

scsi5 : pata_acpi
ata5.00: ATAPI, max UDMA/66
ata5.00: limited to UDMA/33 due to 40-wire cable
ata5.00: configured for UDMA/33

I can assure you that is an 80-wire cable on that device :-) I suppose
next step is adding debug to see where it is having issues?

--
Robert Hancock Saskatoon, SK, Canada
To email, remove "nospam" from [email protected]
Home Page: http://www.roberthancock.com/

2007-02-22 06:59:03

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] ACPI driver support for pata

Robert Hancock wrote:
> Alan wrote:
>> - Add a driver for motherboard ACPI method devices
>> - Link it after normal drivers so ACPI is not preferred
>> - Hook the AMD driver to prefer ACPI for the Nvidia devices if ACPI is
>> active
>> - While we are at it fix up the simplex clear the AMD driver.
>>
>> Depends upon the set_mode -> do_set_mode wrapper patch
>>
>> Signed-off-by: Alan Cox <[email protected]>
>
> I tried out 2.6.21-rc1 with the pata_acpi patch. First off, when you
> enable pata_acpi, it appears that the Fedora mkinitrd stuff decides that
> that should be loaded as the first driver. On boot it promptly attempts
> to attach to all of the ATA controllers, including the nForce SATA
> controllers, which somehow it fails to actually drive causing a failure
> to mount the root filesystem. I got around that by blacklisting
> pata_acpi in /etc/modprobe.conf before installing the kernel and
> un-blacklisting it afterwards, so it wouldn't make the initrd try and
> load it, but there must be a better solution.
>
> It does seem to drive the PATA controllers, but the cable detection
> doesn't seem to be working:
>
> scsi5 : pata_acpi
> ata5.00: ATAPI, max UDMA/66
> ata5.00: limited to UDMA/33 due to 40-wire cable
> ata5.00: configured for UDMA/33

Honestly I don't think pata_acpi is the best way to go.

Unless we know /nothing/ about the controller (not true, in sata_nv
case), I think it would be better to initiate ACPI actions from specific
drivers, rather than forcing the user to switch from pata_amd to pata_acpi.

Jeff



2007-02-22 13:29:19

by Alan

[permalink] [raw]
Subject: Re: [PATCH] ACPI driver support for pata

> > It does seem to drive the PATA controllers, but the cable detection
> > doesn't seem to be working:

The cable detection is broken in the base code not in the ACPI driver.
It's totally hosed for most chipsets in PIIX.

> > scsi5 : pata_acpi
> > ata5.00: ATAPI, max UDMA/66
> > ata5.00: limited to UDMA/33 due to 40-wire cable
> > ata5.00: configured for UDMA/33
>
> Honestly I don't think pata_acpi is the best way to go.

ACPI is the only way to do cable handling on the Nvidia PATA chipset. The
problem you see here is core libata breakage from some recent change,
it's broken most chipsets.

Alan

2007-02-22 17:11:41

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] ACPI driver support for pata

Alan wrote:
> ACPI is the only way to do cable handling on the Nvidia PATA chipset. The

You failed to quote the salient part of the message. Disliking a
separate pata_acpi driver in no way invalidates your statement (quoted
above).

Jeff


2007-02-22 17:26:56

by Alan

[permalink] [raw]
Subject: Re: [PATCH] ACPI driver support for pata

On Thu, 22 Feb 2007 12:11:32 -0500
Jeff Garzik <[email protected]> wrote:

> Alan wrote:
> > ACPI is the only way to do cable handling on the Nvidia PATA chipset. The
>
> You failed to quote the salient part of the message. Disliking a
> separate pata_acpi driver in no way invalidates your statement (quoted
> above).

If you drive a device by the ACPI interface you don't get to fiddle with
it directly or you end up in a murky world of undefined and ungood
behaviour. Testing and vendor information both say pata_acpi is the right
way to drive Nvidia PATA ports.

Cable problem appears to be that
- Tejun fixed the drive side detect logic to be stricter and correct
- Large numbers of controllers with host side detect don't do drive side
detect

Changing the code to clip rates only for 40wire or unknown cables
improves the behaviour.

In the ACPI case I found another corner case with one BIOS which chooses
to boot in PIO mode and which therefore fooled the cable detect trick
used by pata_acpi. It now does further cable checking in ->mode_filter
which combined with the patch in -mm to do the cable logic last means it
works for that case too.

Alan

2007-02-23 00:16:13

by Robert Hancock

[permalink] [raw]
Subject: Re: [PATCH] ACPI driver support for pata

Alan wrote:
> On Thu, 22 Feb 2007 12:11:32 -0500
> Jeff Garzik <[email protected]> wrote:
>
>> Alan wrote:
>>> ACPI is the only way to do cable handling on the Nvidia PATA chipset. The
>> You failed to quote the salient part of the message. Disliking a
>> separate pata_acpi driver in no way invalidates your statement (quoted
>> above).
>
> If you drive a device by the ACPI interface you don't get to fiddle with
> it directly or you end up in a murky world of undefined and ungood
> behaviour. Testing and vendor information both say pata_acpi is the right
> way to drive Nvidia PATA ports.

Couldn't be do this generically inside libata core somehow, i.e. try to
use ACPI to set the proper mode and fall back to the driver-specific
mode setting code if that didn't work? I think if we could do that it
would solve a number of problems (i.e. we could prevent it from doing
this on SATA controllers which appear to be IDE based on the PCI ID,
like the NVIDIA SATA controllers, since the _GTM and _STM methods seem
to have undefined behavior on SATA). This would also eliminate the need
for mkinitrd, people, etc. to know that they're supposed to be loading
this other pata_acpi driver instead, since any PATA driver could take
advantage of this feature.

--
Robert Hancock Saskatoon, SK, Canada
To email, remove "nospam" from [email protected]
Home Page: http://www.roberthancock.com/

2007-02-23 09:38:08

by Allen Martin

[permalink] [raw]
Subject: RE: [PATCH] ACPI driver support for pata

> Couldn't be do this generically inside libata core somehow,
> i.e. try to
> use ACPI to set the proper mode and fall back to the driver-specific
> mode setting code if that didn't work? I think if we could do that it
> would solve a number of problems (i.e. we could prevent it from doing
> this on SATA controllers which appear to be IDE based on the PCI ID,
> like the NVIDIA SATA controllers, since the _GTM and _STM
> methods seem
> to have undefined behavior on SATA).

_GTM and _STM don't have undefined behavior on SATA. They are there for
compatability with the MS Windows ATA driver. Any SATA device that
reports PCI class code 0101 should implement them if that controller is
supposed to work on Windows. They are NOPs for SATA controllers
generally.

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information. Any unauthorized review, use, disclosure or distribution
is prohibited. If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

2007-02-23 11:46:51

by Alan

[permalink] [raw]
Subject: Re: [PATCH] ACPI driver support for pata

> Couldn't be do this generically inside libata core somehow, i.e. try to
> use ACPI to set the proper mode and fall back to the driver-specific
> mode setting code if that didn't work? I think if we could do that it

We want to use the native hebaviour first

> would solve a number of problems (i.e. we could prevent it from doing
> this on SATA controllers which appear to be IDE based on the PCI ID,
> like the NVIDIA SATA controllers, since the _GTM and _STM methods seem
> to have undefined behavior on SATA). This would also eliminate the need

We check this already. A PATA interface without GTM/STM and friends will
not be picked up by pata_acpi but passed on to ata_generic.

> for mkinitrd, people, etc. to know that they're supposed to be loading
> this other pata_acpi driver instead, since any PATA driver could take
> advantage of this feature.

The link order is set up so that we try things in a very specific
deliberate order

- Hardware specific driver (unless it deliberately punts to ACPI)
- ACPI driver using _GTM/_STM etc
- Generic PCI driver ("stay in the mode the BIOS set and pray")
- ISA register compatibility mode (PIO)

This is essentially the strategy used by the old IDE layer since way back
when (minus ACPI support) and has served us pretty well.

Alan

2007-02-23 14:38:12

by Robert Hancock

[permalink] [raw]
Subject: Re: [PATCH] ACPI driver support for pata

Alan wrote:
> The link order is set up so that we try things in a very specific
> deliberate order
>
> - Hardware specific driver (unless it deliberately punts to ACPI)
> - ACPI driver using _GTM/_STM etc
> - Generic PCI driver ("stay in the mode the BIOS set and pray")
> - ISA register compatibility mode (PIO)
>
> This is essentially the strategy used by the old IDE layer since way back
> when (minus ACPI support) and has served us pretty well.

Unfortunately, link order appears to have no effect when the drivers in
question are built modular, as appears to be the typical case, unlike
old IDE where they were generally built in because of all the other
busted behavior if you built the drivers as modules. This means that
userspace tools like mkinitrd, udev, etc. will need to be taught to try
to load drivers in this order, and not to try to use pata_acpi on an
Nvidia SATA controller, for example, as the Fedora mkinitrd seems to do.
This is more complicated than just that if you have multiple
controllers, you have to be even more careful with the load ordering to
ensure this ordering applies system-wide, since the pata_acpi or generic
drivers will attach to anything, while trying to load a driver for the
first controller, you may end up with the second controller getting
those drivers attached to them when they had a controller-specific
driver available that should have been used instead.

Plus there's controller hotplug - if multiple loaded drivers are listed
as being able to drive a piece of hardware, does anything define which
driver will get first dibs on attaching to the device when it's
hotplugged? I don't know how this ordering will be enforced in that case.

--
Robert Hancock Saskatoon, SK, Canada
To email, remove "nospam" from [email protected]
Home Page: http://www.roberthancock.com/


2007-02-23 14:49:35

by Alan

[permalink] [raw]
Subject: Re: [PATCH] ACPI driver support for pata

> Unfortunately, link order appears to have no effect when the drivers in
> question are built modular, as appears to be the typical case, unlike
> old IDE where they were generally built in because of all the other
> busted behavior if you built the drivers as modules. This means that
> userspace tools like mkinitrd, udev, etc. will need to be taught to try
> to load drivers in this order, and not to try to use pata_acpi on an

They already have to do this because of the sata_nv module for one
example where ordering matters. So it's already the case, and it has to
be dealt with run time because the resolution is dynamic and depends upon
user configurable boot settings.

> Plus there's controller hotplug - if multiple loaded drivers are listed
> as being able to drive a piece of hardware, does anything define which
> driver will get first dibs on attaching to the device when it's
> hotplugged? I don't know how this ordering will be enforced in that case.

That feature is lacking in the PCI layer at the moment which is
problematic. When the PCI layer develops this feature I'll make use of
it. In the mean time send patches to Greg KH for that if you want to hack
on it.

Alan