| Message ID | 1303159802.7167.30.camel@mulgrave.site |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
> ata_port_desc(host->ports[i], "irq %d", pdev->irq); > + } > } else if (legacy_mode) { > if (!ata_port_is_dummy(host->ports[0])) { > rc = devm_request_irq(dev, ATA_PRIMARY_IRQ(pdev), This bit looks fine - in fact it's a nice clean up anyway > + } > + if (!(reg & ENPORT_SECONDARY)) { > + dev_printk(KERN_NOTICE, &pdev->dev, "Secondary port is disabled\n"); > + ppi[1] = &ata_dummy_port_info; > + } And you just broke split bridge setups. Also need to check if the bits are valid for this chipset in native mode officialy - Sergei probably knows the answer to that. We can detect the Mobility electronics split bridges at least (and I suspect they are the only 'common' CMD64x hot plug device indeed possibly the only one) because the parent bridge of the CMD64x will have a PCI vendor id of 0x14f2 and a device id 0x0001, 0x0002, or 0x0120. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2011-04-18 at 22:20 +0100, Alan Cox wrote: > We can detect the Mobility electronics split bridges at least (and I > suspect they are the only 'common' CMD64x hot plug device indeed possibly > the only one) because the parent bridge of the CMD64x will have a PCI > vendor id of 0x14f2 and a device id 0x0001, 0x0002, or 0x0120. Trying to wire in a bridge blacklist looks a bit dicey ... unless you're sure this is (and will always be) the only bridge? How should this wiring be done? most of the bridge checks just see if the bridge is in the system rather than walking up the device tree; is that OK? If not, what about just a legacy check on X86, so hedge with if (legacy || !CONFIG_X86) (or even just dump the legacy check, since you think everything is fine on x86 today?) James -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> If not, what about just a legacy check on X86, so hedge with > > if (legacy || !CONFIG_X86) > > (or even just dump the legacy check, since you think everything is fine > on x86 today?) Well in theory you can plug a cardbus one into a non x86 assuming the firmware doesn't explode in protest anyway ! I don't think the bridge walk is particularly tricky - it will always be the direct parent bridge So it's a matter of struct pci_dev *bridge = dev->bus->self; if (bridge && bridge->vendor == 0x14f2) Alan -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2011-04-19 at 15:36 +0100, Alan Cox wrote: > > If not, what about just a legacy check on X86, so hedge with > > > > if (legacy || !CONFIG_X86) > > > > (or even just dump the legacy check, since you think everything is fine > > on x86 today?) > > Well in theory you can plug a cardbus one into a non x86 assuming the > firmware doesn't explode in protest anyway ! > > I don't think the bridge walk is particularly tricky - it will always be > the direct parent bridge OK, that's what I was looking for. Most of the other bridge checks just see if the bridge is present in the system. > So it's a matter of > > struct pci_dev *bridge = dev->bus->self; > if (bridge && bridge->vendor == 0x14f2) Which vendor is 0x14f2? It probably should have a PCI_VENDOR_ID_... define. James -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > struct pci_dev *bridge = dev->bus->self; > > if (bridge && bridge->vendor == 0x14f2) > > Which vendor is 0x14f2? It probably should have a PCI_VENDOR_ID_... > define. Jeff whines about those, Its Mobility Electronics. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 19-04-2011 0:50, James Bottomley wrote: >> You shouldn't need to touch a single line of the core libata code, >> although it might be the best way of doing it. > So how about this, using the dummy port info mechanism. I get a > spurious ata2: DUMMY message, but I suppose libata people are used to > that. I still have to fix libata to prevent spurious irq information, > but that's cosmetic. > James > --- > diff --git a/drivers/ata/pata_cmd64x.c b/drivers/ata/pata_cmd64x.c > index 905ff76..10dabe9 100644 > --- a/drivers/ata/pata_cmd64x.c > +++ b/drivers/ata/pata_cmd64x.c > @@ -354,6 +361,19 @@ static int cmd64x_init_one(struct pci_dev *pdev, const struct pci_device_id *id) > mrdmode |= 0x02; /* Memory read line enable */ > pci_write_config_byte(pdev, MRDMODE, mrdmode); > > + /* check for enabled ports */ > + pci_read_config_byte(pdev, ENPORT,®); > + /* the cm643 primary port is always enabled */ > + if (id->driver_data != 0&& !(reg& ENPORT_PRIMARY)) { > + dev_printk(KERN_ERR,&pdev->dev, "Primary port is disabled; detaching\n"); This is no longer true. > + ppi[0] =&ata_dummy_port_info; > + > + } WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 19-04-2011 1:20, Alan Cox wrote: >> + } >> + if (!(reg& ENPORT_SECONDARY)) { >> + dev_printk(KERN_NOTICE,&pdev->dev, "Secondary port is disabled\n"); >> + ppi[1] =&ata_dummy_port_info; >> + } > And you just broke split bridge setups. Also need to check if the bits > are valid for this chipset in native mode officialy - Sergei probably > knows the answer to that. The PCI-649 spec. doesn't say anything about legacy/native mode WRT the channel enable bits. > We can detect the Mobility electronics split bridges at least (and I > suspect they are the only 'common' CMD64x hot plug device indeed possibly > the only one) because the parent bridge of the CMD64x will have a PCI > vendor id of 0x14f2 and a device id 0x0001, 0x0002, or 0x0120. What's the issue with these brodges anyway? Why the enable bits are not valid for them? WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > We can detect the Mobility electronics split bridges at least (and I > > suspect they are the only 'common' CMD64x hot plug device indeed possibly > > the only one) because the parent bridge of the CMD64x will have a PCI > > vendor id of 0x14f2 and a device id 0x0001, 0x0002, or 0x0120. > > What's the issue with these brodges anyway? Why the enable bits are not > valid for them? Good question. I'd assumed because they are hotplugged and so no firmware gets to set the bits properly ? -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 20-04-2011 1:19, Alan Cox wrote: >>> We can detect the Mobility electronics split bridges at least (and I >>> suspect they are the only 'common' CMD64x hot plug device indeed possibly >>> the only one) because the parent bridge of the CMD64x will have a PCI >>> vendor id of 0x14f2 and a device id 0x0001, 0x0002, or 0x0120. >> What's the issue with these brodges anyway? Why the enable bits are not >> valid for them? > Good question. I'd assumed because they are hotplugged and so no firmware > gets to set the bits properly ? The bits are strapped off the pins JP3/JP4. WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 20 Apr 2011 01:22:54 +0400 Sergei Shtylyov <sshtylyov@mvista.com> wrote: > On 20-04-2011 1:19, Alan Cox wrote: > > >>> We can detect the Mobility electronics split bridges at least (and I > >>> suspect they are the only 'common' CMD64x hot plug device indeed possibly > >>> the only one) because the parent bridge of the CMD64x will have a PCI > >>> vendor id of 0x14f2 and a device id 0x0001, 0x0002, or 0x0120. > > >> What's the issue with these brodges anyway? Why the enable bits are not > >> valid for them? > > > Good question. I'd assumed because they are hotplugged and so no firmware > > gets to set the bits properly ? > > The bits are strapped off the pins JP3/JP4. Beats me then. Whatever - its easy enough to work around and avoid exploding parisc and sparc so it definitely wants sorting -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c index f8380ce..b1b926c 100644 --- a/drivers/ata/libata-sff.c +++ b/drivers/ata/libata-sff.c @@ -2447,13 +2447,18 @@ int ata_pci_sff_activate_host(struct ata_host *host, return -ENOMEM; if (!legacy_mode && pdev->irq) { + int i; + rc = devm_request_irq(dev, pdev->irq, irq_handler, IRQF_SHARED, drv_name, host); if (rc) goto out; - ata_port_desc(host->ports[0], "irq %d", pdev->irq); - ata_port_desc(host->ports[1], "irq %d", pdev->irq); + for (i = 0; i < 2; i++) { + if (ata_port_is_dummy(host->ports[i])) + continue; + ata_port_desc(host->ports[i], "irq %d", pdev->irq); + } } else if (legacy_mode) { if (!ata_port_is_dummy(host->ports[0])) { rc = devm_request_irq(dev, ATA_PRIMARY_IRQ(pdev), diff --git a/drivers/ata/pata_cmd64x.c b/drivers/ata/pata_cmd64x.c index 905ff76..10dabe9 100644 --- a/drivers/ata/pata_cmd64x.c +++ b/drivers/ata/pata_cmd64x.c @@ -41,6 +41,9 @@ enum { CFR = 0x50, CFR_INTR_CH0 = 0x04, + ENPORT = 0x51, + ENPORT_PRIMARY = 0x04, + ENPORT_SECONDARY = 0x08, CMDTIM = 0x52, ARTTIM0 = 0x53, DRWTIM0 = 0x54, @@ -328,8 +331,12 @@ static int cmd64x_init_one(struct pci_dev *pdev, const struct pci_device_id *id) .port_ops = &cmd648_port_ops } }; - const struct ata_port_info *ppi[] = { &cmd_info[id->driver_data], NULL }; - u8 mrdmode; + const struct ata_port_info *ppi[] = { + &cmd_info[id->driver_data], + &cmd_info[id->driver_data], + NULL + }; + u8 mrdmode, reg; int rc; rc = pcim_enable_device(pdev); @@ -354,6 +361,19 @@ static int cmd64x_init_one(struct pci_dev *pdev, const struct pci_device_id *id) mrdmode |= 0x02; /* Memory read line enable */ pci_write_config_byte(pdev, MRDMODE, mrdmode); + /* check for enabled ports */ + pci_read_config_byte(pdev, ENPORT, ®); + /* the cm643 primary port is always enabled */ + if (id->driver_data != 0 && !(reg & ENPORT_PRIMARY)) { + dev_printk(KERN_ERR, &pdev->dev, "Primary port is disabled; detaching\n"); + ppi[0] = &ata_dummy_port_info; + + } + if (!(reg & ENPORT_SECONDARY)) { + dev_printk(KERN_NOTICE, &pdev->dev, "Secondary port is disabled\n"); + ppi[1] = &ata_dummy_port_info; + } + /* Force PIO 0 here.. */ /* PPC specific fixup copied from old driver */