* [PATCH] pata_hpt37x: Fix outstanding bug reports on the HPT374 and 37x cable detect
@ 2007-11-05 22:53 Alan Cox
2007-11-06 2:42 ` Andrew Morton
2007-11-06 14:45 ` Sergei Shtylyov
0 siblings, 2 replies; 5+ messages in thread
From: Alan Cox @ 2007-11-05 22:53 UTC (permalink / raw)
To: akpm, jeff, linux-ide
- Read frequency correctly
- Correct cable detect handling
- Fix wrong filter test
Signed-off-by: Alan Cox <alan@redhat.com>
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))
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);
+ 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;
+ } else
+ freq = inl(iobase + 0x90);
- freq = inl(iobase + 0x90);
if ((freq >> 12) != 0xABCDE) {
int i;
u8 sr;
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] pata_hpt37x: Fix outstanding bug reports on the HPT374 and 37x cable detect
2007-11-05 22:53 [PATCH] pata_hpt37x: Fix outstanding bug reports on the HPT374 and 37x cable detect Alan Cox
@ 2007-11-06 2:42 ` Andrew Morton
2007-11-06 10:40 ` Alan Cox
2007-11-06 14:45 ` Sergei Shtylyov
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2007-11-06 2:42 UTC (permalink / raw)
To: Alan Cox; +Cc: jeff, linux-ide
On Mon, 5 Nov 2007 22:53:38 +0000 Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> +static u32 hpt374_read_freq(struct pci_dev *pdev)
> +{
> + u32 freq;
> + unsigned long io_base = pci_resource_start(pdev, 4);
> + 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;
> +}
hm, pci_resource_start() returns a resource_size_t and I guess this (and a
heck of a lot of other) code is bust on (whatever machine we added that for).
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] pata_hpt37x: Fix outstanding bug reports on the HPT374 and 37x cable detect
2007-11-05 22:53 [PATCH] pata_hpt37x: Fix outstanding bug reports on the HPT374 and 37x cable detect Alan Cox
2007-11-06 2:42 ` Andrew Morton
@ 2007-11-06 14:45 ` Sergei Shtylyov
2007-11-06 14:59 ` Alan Cox
1 sibling, 1 reply; 5+ messages in thread
From: Sergei Shtylyov @ 2007-11-06 14:45 UTC (permalink / raw)
To: Alan Cox; +Cc: akpm, jeff, linux-ide
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 <alan@redhat.com>
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] pata_hpt37x: Fix outstanding bug reports on the HPT374 and 37x cable detect
2007-11-06 14:45 ` Sergei Shtylyov
@ 2007-11-06 14:59 ` Alan Cox
0 siblings, 0 replies; 5+ messages in thread
From: Alan Cox @ 2007-11-06 14:59 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: akpm, jeff, linux-ide
> > + if (chip_table == &hpt374) {
> > + freq = hpt374_read_freq(dev);
> > + if (freq == 0)
> > + return -ENODEV;
>
> Not sure whether this check is necessary...
If the device is hot unplugged we can get a zero return error case. It
seems cleaner to catch the error case rather than attempt to prove it
does no harm later.
pre_reset one fixed in my tree and will check and push.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2007-11-06 14:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-11-05 22:53 [PATCH] pata_hpt37x: Fix outstanding bug reports on the HPT374 and 37x cable detect Alan Cox
2007-11-06 2:42 ` Andrew Morton
2007-11-06 10:40 ` Alan Cox
2007-11-06 14:45 ` Sergei Shtylyov
2007-11-06 14:59 ` Alan Cox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).