[libata] ata_piix: Consolidate PCS register writing
authorJeff Garzik <jeff@garzik.org>
Tue, 11 Jul 2006 15:48:50 +0000 (11:48 -0400)
committerJeff Garzik <jeff@garzik.org>
Sun, 16 Jul 2006 15:48:28 +0000 (11:48 -0400)
Prior to this patch, the driver would do this for each port:
read 8-bit PCS
write 8-bit PCS
read 8-bit PCS
write 8-bit PCS

In the field, flaky behavior has been observed related to this register.
In particular, these overzealous register writes can cause misdetection
problems.

Update to do the following once (not once per port) at boot:
read 16-bit PCS
if needs changing,
write 16-bit PCS

And thereafter, we only perform a 'read 16-bit PCS' per port.

This should eliminate all PCS writes in many cases, and be more friendly
in the cases where we do need to enable ports.

Signed-off-by: Jeff Garzik <jeff@garzik.org>
drivers/scsi/ata_piix.c

index bf41dd3a35d2243a7a27d46fcfd918a0f5fece7b..18b3542236d2d3a078d62f8cac51878fdbb34855 100644 (file)
@@ -141,6 +141,7 @@ enum {
 
 struct piix_map_db {
        const u32 mask;
+       const u32 port_enable;
        const int map[][4];
 };
 
@@ -294,6 +295,7 @@ static const struct ata_port_operations piix_sata_ops = {
 
 static const struct piix_map_db ich5_map_db = {
        .mask = 0x7,
+       .port_enable = 0x3,
        .map = {
                /* PM   PS   SM   SS       MAP  */
                {  P0,  NA,  P1,  NA }, /* 000b */
@@ -309,6 +311,7 @@ static const struct piix_map_db ich5_map_db = {
 
 static const struct piix_map_db ich6_map_db = {
        .mask = 0x3,
+       .port_enable = 0xf,
        .map = {
                /* PM   PS   SM   SS       MAP */
                {  P0,  P2,  P1,  P3 }, /* 00b */
@@ -320,6 +323,7 @@ static const struct piix_map_db ich6_map_db = {
 
 static const struct piix_map_db ich6m_map_db = {
        .mask = 0x3,
+       .port_enable = 0x5,
        .map = {
                /* PM   PS   SM   SS       MAP */
                {  P0,  P2,  RV,  RV }, /* 00b */
@@ -519,44 +523,25 @@ static int piix_sata_prereset(struct ata_port *ap)
        struct piix_host_priv *hpriv = ap->host_set->private_data;
        const unsigned int *map = hpriv->map;
        int base = 2 * ap->hard_port_no;
-       unsigned int present_mask = 0;
+       unsigned int present = 0;
        int port, i;
-       u8 pcs;
+       u16 pcs;
 
-       pci_read_config_byte(pdev, ICH5_PCS, &pcs);
+       pci_read_config_word(pdev, ICH5_PCS, &pcs);
        DPRINTK("ata%u: ENTER, pcs=0x%x base=%d\n", ap->id, pcs, base);
 
-       /* enable all ports on this ap and wait for them to settle */
-       for (i = 0; i < 2; i++) {
-               port = map[base + i];
-               if (port >= 0)
-                       pcs |= 1 << port;
-       }
-
-       pci_write_config_byte(pdev, ICH5_PCS, pcs);
-       msleep(100);
-
-       /* let's see which devices are present */
-       pci_read_config_byte(pdev, ICH5_PCS, &pcs);
-
        for (i = 0; i < 2; i++) {
                port = map[base + i];
                if (port < 0)
                        continue;
                if (ap->flags & PIIX_FLAG_IGNORE_PCS || pcs & 1 << (4 + port))
-                       present_mask |= 1 << i;
-               else
-                       pcs &= ~(1 << port);
+                       present = 1;
        }
 
-       /* disable offline ports on non-AHCI controllers */
-       if (!(ap->flags & PIIX_FLAG_AHCI))
-               pci_write_config_byte(pdev, ICH5_PCS, pcs);
-
        DPRINTK("ata%u: LEAVE, pcs=0x%x present_mask=0x%x\n",
                ap->id, pcs, present_mask);
 
-       if (!present_mask) {
+       if (!present) {
                ata_port_printk(ap, KERN_INFO, "SATA port has no device.\n");
                ap->eh_context.i.action &= ~ATA_EH_RESET_MASK;
                return 0;
@@ -770,6 +755,22 @@ static int __devinit piix_check_450nx_errata(struct pci_dev *ata_dev)
        return no_piix_dma;
 }
 
+static void __devinit piix_init_pcs(struct pci_dev *pdev,
+                                   const struct piix_map_db *map_db)
+{
+       u16 pcs, new_pcs;
+
+       pci_read_config_word(pdev, ICH5_PCS, &pcs);
+
+       new_pcs = pcs | map_db->port_enable;
+
+       if (new_pcs != pcs) {
+               DPRINTK("updating PCS from 0x%x to 0x%x\n", pcs, new_pcs);
+               pci_write_config_word(pdev, ICH5_PCS, new_pcs);
+               msleep(150);
+       }
+}
+
 static void __devinit piix_init_sata_map(struct pci_dev *pdev,
                                         struct ata_port_info *pinfo,
                                         const struct piix_map_db *map_db)
@@ -871,9 +872,11 @@ static int piix_init_one (struct pci_dev *pdev, const struct pci_device_id *ent)
        }
 
        /* Initialize SATA map */
-       if (host_flags & ATA_FLAG_SATA)
+       if (host_flags & ATA_FLAG_SATA) {
                piix_init_sata_map(pdev, port_info,
                                   piix_map_db_table[ent->driver_data]);
+               piix_init_pcs(pdev, piix_map_db_table[ent->driver_data]);
+       }
 
        /* On ICH5, some BIOSen disable the interrupt using the
         * PCI_COMMAND_INTX_DISABLE bit added in PCI 2.3.