From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sergei Shtylyov Subject: Re: [PATCH] pata_hpt37x: Fix outstanding bug reports on the HPT374 and 37x cable detect Date: Tue, 06 Nov 2007 17:45:02 +0300 Message-ID: <47307DEE.3050700@ru.mvista.com> References: <20071105225338.20ef476f@the-village.bc.nu> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Return-path: Received: from homer.mvista.com ([63.81.120.155]:31960 "EHLO imap.sh.mvista.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1752416AbXKFOou (ORCPT ); Tue, 6 Nov 2007 09:44:50 -0500 In-Reply-To: <20071105225338.20ef476f@the-village.bc.nu> Sender: linux-ide-owner@vger.kernel.org List-Id: linux-ide@vger.kernel.org To: Alan Cox Cc: akpm@osdl.org, jeff@garzik.org, linux-ide@vger.kernel.org Alan Cox wrote: > - Read frequency correctly > - Correct cable detect handling I'm still not sure it's correct... > - Fix wrong filter test > Signed-off-by: Alan Cox > diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.24-rc1/drivers/ata/pata_hpt37x.c linux-2.6.24-rc1/drivers/ata/pata_hpt37x.c > --- linux.vanilla-2.6.24-rc1/drivers/ata/pata_hpt37x.c 2007-11-01 11:41:54.000000000 +0000 > +++ linux-2.6.24-rc1/drivers/ata/pata_hpt37x.c 2007-11-05 22:21:31.000000000 +0000 > @@ -295,7 +295,7 @@ > > static unsigned long hpt370a_filter(struct ata_device *adev, unsigned long mask) > { > - if (adev->class != ATA_DEV_ATA) { > + if (adev->class == ATA_DEV_ATA) { > if (hpt_dma_blacklisted(adev, "UDMA100", bad_ata100_5)) > mask &= ~ (0x1F << ATA_SHIFT_UDMA); > } > @@ -359,28 +359,25 @@ > { 0x50, 1, 0x04, 0x04 }, > { 0x54, 1, 0x04, 0x04 } > }; > - u16 mcr3, mcr6; > + u16 mcr3; > u8 ata66; > struct ata_port *ap = link->ap; > struct pci_dev *pdev = to_pci_dev(ap->host->dev); > + unsigned int mcrbase = 0x50 + 4 * ap->port_no; > > if (!pci_test_config_bits(pdev, &hpt37x_enable_bits[ap->port_no])) > return -ENOENT; > > /* Do the extra channel work */ > - pci_read_config_word(pdev, 0x52, &mcr3); > - pci_read_config_word(pdev, 0x56, &mcr6); > + pci_read_config_word(pdev, mcrbase + 2, &mcr3); > /* Set bit 15 of 0x52 to enable TCBLID as input > - Set bit 15 of 0x56 to enable FCBLID as input > */ > - pci_write_config_word(pdev, 0x52, mcr3 | 0x8000); > - pci_write_config_word(pdev, 0x56, mcr6 | 0x8000); > + pci_write_config_word(pdev, mcrbase + 2, mcr3 | 0x8000); > pci_read_config_byte(pdev, 0x5A, &ata66); > /* Reset TCBLID/FCBLID to output */ > pci_write_config_word(pdev, 0x52, mcr3); > - pci_write_config_word(pdev, 0x56, mcr6); > > - if (ata66 & (1 << ap->port_no)) > + if (ata66 & (2 >> ap->port_no)) But has the same issue with hpt37x_pre_reset() been corrected? I don't see this in the current driver. > ap->cbl = ATA_CBL_PATA40; > else > ap->cbl = ATA_CBL_PATA80; > @@ -844,6 +841,25 @@ > /* Never went stable */ > return 0; > } > + > +static u32 hpt374_read_freq(struct pci_dev *pdev) > +{ > + u32 freq; > + unsigned long io_base = pci_resource_start(pdev, 4); No new line after declaration block... > + if (PCI_FUNC(pdev->devfn) & 1) { > + struct pci_dev *pdev_0 = pci_get_slot(pdev->bus, pdev->devfn - 1); > + /* Someone hot plugged the controller on us ? */ > + if (pdev_0 == NULL) > + return 0; > + io_base = pci_resource_start(pdev_0, 4); > + freq = inl(io_base + 0x90); > + pci_dev_put(pdev_0); > + } > + else > + freq = inl(io_base + 0x90); > + return freq; > +} > + > /** > * hpt37x_init_one - Initialise an HPT37X/302 > * @dev: PCI device > @@ -902,7 +918,7 @@ > .flags = ATA_FLAG_SLAVE_POSS, > .pio_mask = 0x1f, > .mwdma_mask = 0x07, > - .udma_mask = 0x0f, > + .udma_mask = ATA_UDMA5, > .port_ops = &hpt370_port_ops > }; > /* HPT370A - UDMA100 */ > @@ -911,7 +927,7 @@ > .flags = ATA_FLAG_SLAVE_POSS, > .pio_mask = 0x1f, > .mwdma_mask = 0x07, > - .udma_mask = 0x0f, > + .udma_mask = ATA_UDMA5, > .port_ops = &hpt370a_port_ops > }; > /* HPT371, 372 and friends - UDMA133 */ > @@ -1047,9 +1063,16 @@ > outb(0x0e, iobase + 0x9c); > > /* Some devices do not let this value be accessed via PCI space > - according to the old driver */ > + according to the old driver. In addition we must use the value > + from FN 0 on the HPT374 */ > + > + if (chip_table == &hpt374) { > + freq = hpt374_read_freq(dev); > + if (freq == 0) > + return -ENODEV; Not sure whether this check is necessary... > + } else > + freq = inl(iobase + 0x90); > > - freq = inl(iobase + 0x90); > if ((freq >> 12) != 0xABCDE) { > int i; > u8 sr; MBR, Sergei