Bug#622997: libata-sff/pata_cmd64x problem with hardwired configurations
Hi,
Sergei Shtylyov wrote:
> Hello.
>
> James Bottomley wrote:
>
> >>> I can get all of this working by fixing up all the hard coded knowledge
> >>> in libata-sff only to use a single port.
> >>> However, I can't fix the libata-sff driver until I know how to tell
> >>> there's only one port wired. Does anyone with cmd649 knowledge have any
> >>> idea how I might tell this?
>
> >> The secondary port is enabled in the PCI config. space: register 0x51 bit
> >> 3 controls this. Unfortunately, pata_cmd64x driver still doesn't check the
> >> channel enable bits; the cmd64x driver does though, so it might be worth trying...
>
> > So this is the enablebits code in driver/ide
>
> We still need to ensure that these are indeed the enable bits that are at
> play here...
>
> > that's missing from any of
> > the libata stuff?
>
> Not from any. It's handled in some drivers, like pata_hpt37x, for example --
> in their prereset() methods.
>
> > Should this be generic in libata-sff? ...
>
> There's even libata core function, pci_test_config_bits() that facilitates
> this check.
>
> > I mean even on an x86 where arbitrary memory can be poked without consequence,
> > trying to activate a disabled port will still produce lots of noise.
>
> The story with pata_cmd64x is somewhat convoluted: originally (when the
> libata driver was created) in the IDE driver there were only enable bits for the
> secondary channel of PCI0646 (and even those wrong); I fixed those and added the
> enable bits for other chips back in 2007 but it never got propagated to
> pata_cmd64x. Well, I have some plans for changes to this driver but not much
> time to actually do them currently...
From atang tree, orginally posted in 2009:
From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [PATCH v2] pata_cmd64x: add enablebits checking
Fixes IDE -> libata regression.
IDE's cmd64x host driver has been supporting enablebits checking
since the initial driver's merge.
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
drivers/ata/pata_cmd64x.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
Index: b/drivers/ata/pata_cmd64x.c
===================================================================
--- a/drivers/ata/pata_cmd64x.c
+++ b/drivers/ata/pata_cmd64x.c
@@ -79,6 +79,40 @@ static int cmd648_cable_detect(struct at
}
/**
+ * cmd64x_prereset - perform reset handling
+ * @link: ATA link
+ * @deadline: deadline jiffies for the operation
+ *
+ * Reset sequence checking enable bits to see which ports are
+ * active.
+ */
+
+static int cmd64x_prereset(struct ata_link *link, unsigned long deadline)
+{
+ static const struct pci_bits cmd64x_enable_bits[] = {
+ { 0x51, 1, 0x04, 0x04 },
+ { 0x51, 1, 0x08, 0x08 }
+ };
+
+ struct ata_port *ap = link->ap;
+ struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+
+ /*
+ * The original PCI0643 and PCI0646 didn't have the primary
+ * channel enable bit, it appeared starting with PCI0646U
+ * (i.e. revision ID 3).
+ */
+ if (ap->port_no == 0 && (pdev->device == PCI_DEVICE_ID_CMD_643 ||
+ (pdev->device == PCI_DEVICE_ID_CMD_646 && pdev->revision < 3)))
+ goto out;
+
+ if (!pci_test_config_bits(pdev, &cmd64x_enable_bits[ap->port_no]))
+ return -ENOENT;
+out:
+ return ata_sff_prereset(link, deadline);
+}
+
+/**
* cmd64x_set_timing - set PIO and MWDMA timing
* @ap: ATA interface
* @adev: ATA device
@@ -266,6 +300,7 @@ static const struct ata_port_operations
.inherits = &ata_bmdma_port_ops,
.set_piomode = cmd64x_set_piomode,
.set_dmamode = cmd64x_set_dmamode,
+ .prereset = cmd64x_prereset,
};
static struct ata_port_operations cmd64x_port_ops = {
Reply to: