Re: pata_sch: fix PSTS register reading.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello.

Denis Turischev wrote:

pata_sch: fix PSTS register reading.

According to Poulsbo SCH datasheet (p. 353): "Reading reserved bits returns
an indeterminate, inconsistent value".
Bit 7 of PSTS register read as 1 when there is slave disk drive attached,
despite being described as reserved with zero value. This causes wrong feature
decoding in ata_sff_tf_read.

  ata_sff_tf_read() doesn't decode anything.

Fix it by implementing pata_sch specific sff_tf_read with appropriate bit
masks on PSTS register.

  Your patch doesn't make any sense to me -- sorry...

Signed-off-by: Denis Turischev <denis@xxxxxxxxxxxxxx>
CC: stable@xxxxxxxxxx

--- linux-2.6.33-rc4.orig/drivers/ata/pata_sch.c 2010-01-13 07:15:00.000000000 +0200 +++ linux-2.6.33-rc4/drivers/ata/pata_sch.c 2010-01-20 11:19:16.000000000 +0200
@@ -49,6 +49,11 @@
    USD    = (1 << 31),    /* Use Synchronous DMA */
};

+/* see SCH datasheet page 353 */
+enum {
+    PSTSM    = 0x7f,        /* PSTS Bit Mask */
+};
+
static int sch_init_one(struct pci_dev *pdev,
             const struct pci_device_id *ent);
static void sch_set_piomode(struct ata_port *ap, struct ata_device *adev);
@@ -75,11 +80,47 @@
    ATA_BMDMA_SHT(DRV_NAME),
};

+/**
+ *    sch_tf_read - input device's ATA taskfile shadow registers
+ *    @ap: Port from which input is read
+ *    @tf: ATA taskfile register set for storing input
+ *
+ *    Note: Original code is ata_sff_tf_read().
+ */
+
+static void sch_tf_read(struct ata_port *ap, struct ata_taskfile *tf)
+{
+    struct ata_ioports *ioaddr = &ap->ioaddr;
+
+    tf->command = ata_sff_check_status(ap);
+    tf->feature = ioread8(ioaddr->error_addr) && PSTSM;

  1) you want a bitwise AND here (&);
2) what you're doing is wrong, you can't mask the features register like that -- are you sure that *this* is the PSTS register?


+    tf->nsect = ioread8(ioaddr->nsect_addr);
+    tf->lbal = ioread8(ioaddr->lbal_addr);
+    tf->lbam = ioread8(ioaddr->lbam_addr);
+    tf->lbah = ioread8(ioaddr->lbah_addr);
+    tf->device = ioread8(ioaddr->device_addr);
+
+    if (tf->flags & ATA_TFLAG_LBA48) {
+        if (likely(ioaddr->ctl_addr)) {

Useless check I think -- ioaddr->ctl_addr should always be non-zero with pata_sch.

+            iowrite8(tf->ctl | ATA_HOB, ioaddr->ctl_addr);
+            tf->hob_feature = ioread8(ioaddr->error_addr) && PSTSM;

  Same comments here.

+            tf->hob_nsect = ioread8(ioaddr->nsect_addr);
+            tf->hob_lbal = ioread8(ioaddr->lbal_addr);
+            tf->hob_lbam = ioread8(ioaddr->lbam_addr);
+            tf->hob_lbah = ioread8(ioaddr->lbah_addr);
+            iowrite8(tf->ctl, ioaddr->ctl_addr);
+            ap->last_ctl = tf->ctl;
+        } else
+            WARN_ON_ONCE(1);
+    }
+}
+
static struct ata_port_operations sch_pata_ops = {
    .inherits        = &ata_bmdma_port_ops,
    .cable_detect        = ata_cable_unknown,
    .set_piomode        = sch_set_piomode,
    .set_dmamode        = sch_set_dmamode,
+    .sff_tf_read        = sch_tf_read,
};

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux