I've a Dell Precision 670 here (four-year-old EM64T Xeon with ata_piix)
which doesn't like your commit 871af1210f13966ab911ed2166e4ab2ce775b99d
libata: Add 32bit PIO support. Full dmesg (and .config) attached, but
here's an extract showing the start of the error messages on ata2:
ata1: PATA max UDMA/100 cmd 0x1f0 ctl 0x3f6 bmdma 0xffa0 irq 14
ata2: PATA max UDMA/100 cmd 0x170 ctl 0x376 bmdma 0xffa8 irq 15
ata1: port disabled. ignoring.
ata2.00: ATAPI: GCR-8483B, 1.07, max UDMA/33
ata2.00: configured for UDMA/33
scsi 1:0:0:0: CD-ROM HL-DT-ST CD-ROM GCR-8483B 1.07 PQ: 0 ANSI: 5
ata2.00: qc timeout (cmd 0xa0)
ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
ata2.00: cmd a0/00:00:00:00:00/00:00:00:00:00/a0 tag 0
cdb 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
res 51/20:03:00:00:00/00:00:00:00:00/a0 Emask 0x5 (timeout)
ata2.00: status: { DRDY ERR }
ata2: soft resetting link
ata2.00: configured for UDMA/33
ata2: EH complete
linux-next behaved the same, so I played around, and the patch below seems
to fix it fine (though I've not yet tried it on other, working machines);
but I can well imagine you may prefer to do it differently. Or was I just
wrong to move that box's CD handling from ide to libata a few months ago?
[PATCH] libata sff: 32bit PIO use 16bit on slop
871af1210f13966ab911ed2166e4ab2ce775b99d libata: Add 32bit PIO support
causes errors on a four-year-old ata_piix Dell Precision 670. Using
16bit PIO instead of 32bit PIO on the odd 1, 2 or 3 chars fixes that.
Signed-off-by: Hugh Dickins <[email protected]>
---
drivers/ata/libata-sff.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
--- 2.6.29-rc2/drivers/ata/libata-sff.c 2009-01-17 10:40:12.000000000 +0000
+++ linux/drivers/ata/libata-sff.c 2009-01-21 12:32:01.000000000 +0000
@@ -774,17 +774,21 @@ unsigned int ata_sff_data_xfer32(struct
iowrite32_rep(data_addr, buf, words);
if (unlikely(slop)) {
- __le32 pad;
+ unsigned char slop_buf[4];
+ unsigned char *trailing_buf = buf + buflen - slop;
+
+ words = (slop + 1) >> 1;
if (rw == READ) {
- pad = cpu_to_le32(ioread32(ap->ioaddr.data_addr));
- memcpy(buf + buflen - slop, &pad, slop);
+ ioread16_rep(data_addr, slop_buf, words);
+ memcpy(trailing_buf, slop_buf, slop);
} else {
- memcpy(&pad, buf + buflen - slop, slop);
- iowrite32(le32_to_cpu(pad), ap->ioaddr.data_addr);
+ memcpy(slop_buf, trailing_buf, slop);
+ slop_buf[slop] = 0;
+ iowrite16_rep(data_addr, slop_buf, words);
}
- words++;
}
- return words << 2;
+
+ return buflen + (slop & 1);
}
EXPORT_SYMBOL_GPL(ata_sff_data_xfer32);
Hugh Dickins writes:
> I've a Dell Precision 670 here (four-year-old EM64T Xeon with ata_piix)
> which doesn't like your commit 871af1210f13966ab911ed2166e4ab2ce775b99d
> libata: Add 32bit PIO support. Full dmesg (and .config) attached, but
> here's an extract showing the start of the error messages on ata2:
>
> ata1: PATA max UDMA/100 cmd 0x1f0 ctl 0x3f6 bmdma 0xffa0 irq 14
> ata2: PATA max UDMA/100 cmd 0x170 ctl 0x376 bmdma 0xffa8 irq 15
> ata1: port disabled. ignoring.
> ata2.00: ATAPI: GCR-8483B, 1.07, max UDMA/33
> ata2.00: configured for UDMA/33
> scsi 1:0:0:0: CD-ROM HL-DT-ST CD-ROM GCR-8483B 1.07 PQ: 0 ANSI: 5
> ata2.00: qc timeout (cmd 0xa0)
> ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 frozen
> ata2.00: cmd a0/00:00:00:00:00/00:00:00:00:00/a0 tag 0
> cdb 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> res 51/20:03:00:00:00/00:00:00:00:00/a0 Emask 0x5 (timeout)
> ata2.00: status: { DRDY ERR }
> ata2: soft resetting link
> ata2.00: configured for UDMA/33
> ata2: EH complete
>
> linux-next behaved the same, so I played around, and the patch below seems
> to fix it fine (though I've not yet tried it on other, working machines);
> but I can well imagine you may prefer to do it differently. Or was I just
> wrong to move that box's CD handling from ide to libata a few months ago?
>
> [PATCH] libata sff: 32bit PIO use 16bit on slop
>
> 871af1210f13966ab911ed2166e4ab2ce775b99d libata: Add 32bit PIO support
> causes errors on a four-year-old ata_piix Dell Precision 670. Using
> 16bit PIO instead of 32bit PIO on the odd 1, 2 or 3 chars fixes that.
>
> Signed-off-by: Hugh Dickins <[email protected]>
> ---
>
> drivers/ata/libata-sff.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> --- 2.6.29-rc2/drivers/ata/libata-sff.c 2009-01-17 10:40:12.000000000 +0000
> +++ linux/drivers/ata/libata-sff.c 2009-01-21 12:32:01.000000000 +0000
> @@ -774,17 +774,21 @@ unsigned int ata_sff_data_xfer32(struct
> iowrite32_rep(data_addr, buf, words);
>
> if (unlikely(slop)) {
> - __le32 pad;
> + unsigned char slop_buf[4];
> + unsigned char *trailing_buf = buf + buflen - slop;
> +
> + words = (slop + 1) >> 1;
> if (rw == READ) {
> - pad = cpu_to_le32(ioread32(ap->ioaddr.data_addr));
> - memcpy(buf + buflen - slop, &pad, slop);
> + ioread16_rep(data_addr, slop_buf, words);
> + memcpy(trailing_buf, slop_buf, slop);
> } else {
> - memcpy(&pad, buf + buflen - slop, slop);
> - iowrite32(le32_to_cpu(pad), ap->ioaddr.data_addr);
> + memcpy(slop_buf, trailing_buf, slop);
> + slop_buf[slop] = 0;
> + iowrite16_rep(data_addr, slop_buf, words);
> }
> - words++;
> }
> - return words << 2;
> +
> + return buflen + (slop & 1);
> }
> EXPORT_SYMBOL_GPL(ata_sff_data_xfer32);
Thanks, this patch also fixes the 2.6.29-rc bmdma32-related ata_piix regression
I reported, where having a specific CD drive connected causes a complete system
hang when init starts udev <http://marc.info/?l=linux-ide&m=123244042628229&w=2>.
Tested-by: Mikael Pettersson <[email protected]>
On Wed, 21 Jan 2009 21:11:05 +0100
Mikael Pettersson <[email protected]> wrote:
> Hugh Dickins writes:
> > I've a Dell Precision 670 here (four-year-old EM64T Xeon with ata_piix)
> > which doesn't like your commit 871af1210f13966ab911ed2166e4ab2ce775b99d
> > libata: Add 32bit PIO support. Full dmesg (and .config) attached, but
> > here's an extract showing the start of the error messages on ata2:
Cool - so we need two different 32bit PIO methods - at least according to
the docs for the AMD we should use entirely 32bit I/O there. Fun fun
Alan
Hello.
Alan Cox wrote:
>> Hugh Dickins writes:
>> > I've a Dell Precision 670 here (four-year-old EM64T Xeon with ata_piix)
>> > which doesn't like your commit 871af1210f13966ab911ed2166e4ab2ce775b99d
>> > libata: Add 32bit PIO support. Full dmesg (and .config) attached, but
>> > here's an extract showing the start of the error messages on ata2:
>>
>
> Cool - so we need two different 32bit PIO methods - at least according to
> the docs for the AMD we should use entirely 32bit I/O there. Fun fun
>
Could you refer me to the exact AMD doc that requires that?
> Alan
MBR, Sergei
On Thu, 22 Jan 2009 01:51:17 +0300
Sergei Shtylyov <[email protected]> wrote:
> Hello.
>
> Alan Cox wrote:
>
> >> Hugh Dickins writes:
> >> > I've a Dell Precision 670 here (four-year-old EM64T Xeon with ata_piix)
> >> > which doesn't like your commit 871af1210f13966ab911ed2166e4ab2ce775b99d
> >> > libata: Add 32bit PIO support. Full dmesg (and .config) attached, but
> >> > here's an extract showing the start of the error messages on ata2:
> >>
> >
> > Cool - so we need two different 32bit PIO methods - at least according to
> > the docs for the AMD we should use entirely 32bit I/O there. Fun fun
> >
>
> Could you refer me to the exact AMD doc that requires that?
AMD762 page 82, under DevB:1x40 bit 14 and bit 12
"Note: only 32-bit writes to the data port are allowed when this bit is
set."
Hello.
Alan Cox wrote:
>>>> > I've a Dell Precision 670 here (four-year-old EM64T Xeon with ata_piix)
>>>> > which doesn't like your commit 871af1210f13966ab911ed2166e4ab2ce775b99d
>>>> > libata: Add 32bit PIO support. Full dmesg (and .config) attached, but
>>>> > here's an extract showing the start of the error messages on ata2:
>>>>
>>>>
>>> Cool - so we need two different 32bit PIO methods - at least according to
>>> the docs for the AMD we should use entirely 32bit I/O there. Fun fun
>>>
>>>
>> Could you refer me to the exact AMD doc that requires that?
>>
>
> AMD762 page 82, under DevB:1x40 bit 14 and bit 12
>
What exactly AMD-762 document has this? I'm not seeing it in either
stasheet ot software/BIOS design guide. Besides, AMD-762 is a north
bridge and doesn't include IDE.
> "Note: only 32-bit writes to the data port are allowed when this bit is
> set."
>
Now tell me who forces you to set that bit (I assume it's the write
buffer enable) for the ATAPI devices?
WBR, Sergei
Hello, I wrote:
>>>>> > I've a Dell Precision 670 here (four-year-old EM64T Xeon with
>>>>> ata_piix)
>>>>> > which doesn't like your commit
>>>>> 871af1210f13966ab911ed2166e4ab2ce775b99d
>>>>> > libata: Add 32bit PIO support. Full dmesg (and .config)
>>>>> attached, but
>>>>> > here's an extract showing the start of the error messages on ata2:
>>>>>
>>>> Cool - so we need two different 32bit PIO methods - at least
>>>> according to
>>>> the docs for the AMD we should use entirely 32bit I/O there. Fun fun
>>>>
>>> Could you refer me to the exact AMD doc that requires that?
>>>
>>
>> AMD762 page 82, under DevB:1x40 bit 14 and bit 12
>>
>
> What exactly AMD-762 document has this? I'm not seeing it in either
> stasheet ot software/BIOS design guide. Besides, AMD-762 is a north
> bridge and doesn't include IDE.
Finally found it in the AMD-768 datasheet.
>> "Note: only 32-bit writes to the data port are allowed when this bit is
>> set."
>>
>
> Now tell me who forces you to set that bit (I assume it's the write
> buffer enable) for the ATAPI devices?
Yes, it's the write buffer enable (about which I have written already).
\
MBR, Sergei
Hello, I wrote:
>>>>>> > I've a Dell Precision 670 here (four-year-old EM64T Xeon with
>>>>>> ata_piix)
>>>>>> > which doesn't like your commit
>>>>>> 871af1210f13966ab911ed2166e4ab2ce775b99d
>>>>>> > libata: Add 32bit PIO support. Full dmesg (and .config)
>>>>>> attached, but
>>>>>> > here's an extract showing the start of the error messages on
>>>>>> ata2:
>>>>>>
>>>>> Cool - so we need two different 32bit PIO methods - at least
>>>>> according to
>>>>> the docs for the AMD we should use entirely 32bit I/O there. Fun fun
>>>>>
>>>> Could you refer me to the exact AMD doc that requires that?
>>>>
>>>
>>> AMD762 page 82, under DevB:1x40 bit 14 and bit 12
>>>
>>
>> What exactly AMD-762 document has this? I'm not seeing it in either
>> stasheet ot software/BIOS design guide. Besides, AMD-762 is a north
>> bridge and doesn't include IDE.
>
> Finally found it in the AMD-768 datasheet.
>
>>> "Note: only 32-bit writes to the data port are allowed when this bit is
>>> set."
>>>
>>
>> Now tell me who forces you to set that bit (I assume it's the write
>> buffer enable) for the ATAPI devices?
>
> Yes, it's the write buffer enable (about which I have written already).
IMHO, this is the same situation as with the prefetch -- you want to
avoid additional data read (write in this case) cycles to with an ATAPI
device, so you have to turn it off.
MBR, Sergei
> >> "Note: only 32-bit writes to the data port are allowed when this bit is
> >> set."
> >>
> >
> > Now tell me who forces you to set that bit (I assume it's the write
> > buffer enable) for the ATAPI devices?
>
> Yes, it's the write buffer enable (about which I have written already).
Yeah that seems the best way to go - not I note that the old drivers/ide
driver programs it right for 16bit I/O or doing mixed 16/32 ..
Patch added to my tree which together with Hugh's change should do the
trick. The others I've checked seem to have no such rule except in VLB
space. Never tried ATAPI in anger on a VLB box but it's easy enough to
address and I'll do a third patch for those.
Alan
> [PATCH] libata sff: 32bit PIO use 16bit on slop
>
> 871af1210f13966ab911ed2166e4ab2ce775b99d libata: Add 32bit PIO support
> causes errors on a four-year-old ata_piix Dell Precision 670. Using
> 16bit PIO instead of 32bit PIO on the odd 1, 2 or 3 chars fixes that.
>
> Signed-off-by: Hugh Dickins <[email protected]>
For the 3 bytes of slop it should use a single iowrite32 but otherwise
that seems ok. We do need to handle the FIFO setup on the AMD differently
if we do this - something like this:
pata_amd: Program FIFO
From: Alan Cox <[email protected]>
With 32bit PIO we can use the posted write buffers, but only for 32bit I/O
cycles. This means we must disable the FIFO for ATAPI where a final 16bit
cycle may occur.
Rework the FIFO logic so that we disable the FIFO then selectively re-enable
it when we set the timings on AMD devices. Also fix a case where we scribbled
on PCI config 0x41 of Nvidia chips when we shouldn't.
Signed-off-by: Alan Cox <[email protected]>
---
drivers/ata/pata_amd.c | 78 +++++++++++++++++++++++++++++++++++++-----------
1 files changed, 60 insertions(+), 18 deletions(-)
diff --git a/drivers/ata/pata_amd.c b/drivers/ata/pata_amd.c
index 63719ab..c380a4e 100644
--- a/drivers/ata/pata_amd.c
+++ b/drivers/ata/pata_amd.c
@@ -24,7 +24,7 @@
#include <linux/libata.h>
#define DRV_NAME "pata_amd"
-#define DRV_VERSION "0.3.11"
+#define DRV_VERSION "0.4.1"
/**
* timing_setup - shared timing computation and load
@@ -145,6 +145,13 @@ static int amd_pre_reset(struct ata_link *link, unsigned long deadline)
return ata_sff_prereset(link, deadline);
}
+/**
+ * amd_cable_detect - report cable type
+ * @ap: port
+ *
+ * AMD controller/BIOS setups record the cable type in word 0x42
+ */
+
static int amd_cable_detect(struct ata_port *ap)
{
static const u32 bitmask[2] = {0x03, 0x0C};
@@ -158,6 +165,40 @@ static int amd_cable_detect(struct ata_port *ap)
}
/**
+ * amd_fifo_setup - set the PIO FIFO for ATA/ATAPI
+ * @ap: ATA interface
+ * @adev: ATA device
+ *
+ * Set the PCI fifo for this device according to the devices present
+ * on the bus at this point in time. We need to turn the post write buffer
+ * off for ATAPI devices as we may need to issue a word sized write to the
+ * device as the final I/O
+ */
+
+static void amd_fifo_setup(struct ata_port *ap)
+{
+ struct ata_device *adev;
+ struct pci_dev *pdev = to_pci_dev(ap->host->dev);
+ static const u8 fifobit[2] = { 0xC0, 0x30};
+ u8 fifo = fifobit[ap->port_no];
+ u8 r;
+
+
+ ata_for_each_dev(adev, &ap->link, ENABLED) {
+ if (adev->class == ATA_DEV_ATAPI)
+ fifo = 0;
+ }
+ if (pdev->device == PCI_DEVICE_ID_AMD_VIPER_7411) /* FIFO is broken */
+ fifo = 0;
+
+ /* On the later chips the read prefetch bits become no-op bits */
+ pci_read_config_byte(pdev, 0x41, &r);
+ r &= ~fifobit[ap->port_no];
+ r |= fifo;
+ pci_write_config_byte(pdev, 0x41, r);
+}
+
+/**
* amd33_set_piomode - set initial PIO mode data
* @ap: ATA interface
* @adev: ATA device
@@ -167,21 +208,25 @@ static int amd_cable_detect(struct ata_port *ap)
static void amd33_set_piomode(struct ata_port *ap, struct ata_device *adev)
{
+ amd_fifo_setup(ap);
timing_setup(ap, adev, 0x40, adev->pio_mode, 1);
}
static void amd66_set_piomode(struct ata_port *ap, struct ata_device *adev)
{
+ amd_fifo_setup(ap);
timing_setup(ap, adev, 0x40, adev->pio_mode, 2);
}
static void amd100_set_piomode(struct ata_port *ap, struct ata_device *adev)
{
+ amd_fifo_setup(ap);
timing_setup(ap, adev, 0x40, adev->pio_mode, 3);
}
static void amd133_set_piomode(struct ata_port *ap, struct ata_device *adev)
{
+ amd_fifo_setup(ap);
timing_setup(ap, adev, 0x40, adev->pio_mode, 4);
}
@@ -397,6 +442,16 @@ static struct ata_port_operations nv133_port_ops = {
.set_dmamode = nv133_set_dmamode,
};
+static void amd_clear_fifo(struct pci_dev *pdev)
+{
+ u8 fifo;
+ /* Disable the FIFO, the FIFO logic will re-enable it as
+ appropriate */
+ pci_read_config_byte(pdev, 0x41, &fifo);
+ fifo &= 0x0F;
+ pci_write_config_byte(pdev, 0x41, fifo);
+}
+
static int amd_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
{
static const struct ata_port_info info[10] = {
@@ -503,14 +558,8 @@ static int amd_init_one(struct pci_dev *pdev, const struct pci_device_id *id)
if (type < 3)
ata_pci_bmdma_clear_simplex(pdev);
-
- /* Check for AMD7411 */
- if (type == 3)
- /* FIFO is broken */
- pci_write_config_byte(pdev, 0x41, fifo & 0x0F);
- else
- pci_write_config_byte(pdev, 0x41, fifo | 0xF0);
-
+ if (pdev->vendor == PCI_VENDOR_ID_AMD)
+ amd_clear_fifo(pdev);
/* Cable detection on Nvidia chips doesn't work too well,
* cache BIOS programmed UDMA mode.
*/
@@ -534,20 +583,13 @@ static int amd_reinit_one(struct pci_dev *pdev)
rc = ata_pci_device_do_resume(pdev);
if (rc)
return rc;
-
+
if (pdev->vendor == PCI_VENDOR_ID_AMD) {
- u8 fifo;
- pci_read_config_byte(pdev, 0x41, &fifo);
- if (pdev->device == PCI_DEVICE_ID_AMD_VIPER_7411)
- /* FIFO is broken */
- pci_write_config_byte(pdev, 0x41, fifo & 0x0F);
- else
- pci_write_config_byte(pdev, 0x41, fifo | 0xF0);
+ amd_clear_fifo(pdev);
if (pdev->device == PCI_DEVICE_ID_AMD_VIPER_7409 ||
pdev->device == PCI_DEVICE_ID_AMD_COBRA_7401)
ata_pci_bmdma_clear_simplex(pdev);
}
-
ata_host_resume(host);
return 0;
}
And while we are at it fix the 32bit PIO handling for VLB for this case
too
pata_legacy: For VLB 32bit PIO don't try tricks with slop
From: Alan Cox <[email protected]>
These devices are generally used with ATA anyway and it seems that some
ATAPI will need us to issue the right number of words. Therefore as we
can't switch mid burst on VLB devices we should only use 32bit I/O for
suitable block sizes.
---
drivers/ata/pata_legacy.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/ata/pata_legacy.c b/drivers/ata/pata_legacy.c
index 6c1d778..e3bc1b4 100644
--- a/drivers/ata/pata_legacy.c
+++ b/drivers/ata/pata_legacy.c
@@ -283,9 +283,10 @@ static void pdc20230_set_piomode(struct ata_port *ap, struct ata_device *adev)
static unsigned int pdc_data_xfer_vlb(struct ata_device *dev,
unsigned char *buf, unsigned int buflen, int rw)
{
- if (ata_id_has_dword_io(dev->id)) {
+ int slop = buflen & 3;
+ /* 32bit I/O capable *and* we need to write a whole number of dwords */
+ if (ata_id_has_dword_io(dev->id) && (slop == 0 || slop == 3)) {
struct ata_port *ap = dev->link->ap;
- int slop = buflen & 3;
unsigned long flags;
local_irq_save(flags);
@@ -735,7 +736,7 @@ static unsigned int vlb32_data_xfer(struct ata_device *adev, unsigned char *buf,
struct ata_port *ap = adev->link->ap;
int slop = buflen & 3;
- if (ata_id_has_dword_io(adev->id)) {
+ if (ata_id_has_dword_io(adev->id) && (slop == 0 || slop == 3)) {
if (rw == WRITE)
iowrite32_rep(ap->ioaddr.data_addr, buf, buflen >> 2);
else
Hello.
Alan Cox wrote:
> pata_amd: Program FIFO
>
> From: Alan Cox <[email protected]>
>
> With 32bit PIO we can use the posted write buffers, but only for 32bit I/O
> cycles. This means we must disable the FIFO for ATAPI where a final 16bit
> cycle may occur.
>
> Rework the FIFO logic so that we disable the FIFO then selectively re-enable
> it when we set the timings on AMD devices. Also fix a case where we scribbled
> on PCI config 0x41 of Nvidia chips when we shouldn't.
>
> Signed-off-by: Alan Cox <[email protected]>
>
[...]
> diff --git a/drivers/ata/pata_amd.c b/drivers/ata/pata_amd.c
> index 63719ab..c380a4e 100644
> --- a/drivers/ata/pata_amd.c
> +++ b/drivers/ata/pata_amd.c
>
[...]
> @@ -158,6 +165,40 @@ static int amd_cable_detect(struct ata_port *ap)
> }
>
> /**
> + * amd_fifo_setup - set the PIO FIFO for ATA/ATAPI
> + * @ap: ATA interface
> + * @adev: ATA device
> + *
> + * Set the PCI fifo for this device according to the devices present
> + * on the bus at this point in time. We need to turn the post write buffer
> + * off for ATAPI devices as we may need to issue a word sized write to the
> + * device as the final I/O
> + */
> +
> +static void amd_fifo_setup(struct ata_port *ap)
> +{
> + struct ata_device *adev;
> + struct pci_dev *pdev = to_pci_dev(ap->host->dev);
> + static const u8 fifobit[2] = { 0xC0, 0x30};
> + u8 fifo = fifobit[ap->port_no];
> + u8 r;
> +
> +
> + ata_for_each_dev(adev, &ap->link, ENABLED) {
> + if (adev->class == ATA_DEV_ATAPI)
> + fifo = 0;
> + }
>
Er, couldn't we do that dynamically, based on which device is
executing the command now?
> + if (pdev->device == PCI_DEVICE_ID_AMD_VIPER_7411) /* FIFO is broken */
> + fifo = 0;
> +
> + /* On the later chips the read prefetch bits become no-op bits */
> + pci_read_config_byte(pdev, 0x41, &r);
> + r &= ~fifobit[ap->port_no];
>
Why not:
r &= ~fifo;
> + r |= fifo;
> + pci_write_config_byte(pdev, 0x41, r);
> +}
MBR, Sergei
O> > + ata_for_each_dev(adev, &ap->link, ENABLED) {
> > + if (adev->class == ATA_DEV_ATAPI)
> > + fifo = 0;
> > + }
> >
>
> Er, couldn't we do that dynamically, based on which device is
> executing the command now?
Possibly but PCI command cycles are expensive so you'd want to cache the
state and stuff. Better to get it correct firstly.
> > + if (pdev->device == PCI_DEVICE_ID_AMD_VIPER_7411) /* FIFO is broken */
> > + fifo = 0;
> > +
> > + /* On the later chips the read prefetch bits become no-op bits */
> > + pci_read_config_byte(pdev, 0x41, &r);
> > + r &= ~fifobit[ap->port_no];
> >
>
> Why not:
>
> r &= ~fifo;
Because then it wouldn't clear the bits if they were set already and we
wanted them off!
Hello.
Alan Cox wrote:
>>> + if (pdev->device == PCI_DEVICE_ID_AMD_VIPER_7411) /* FIFO is broken */
>>> + fifo = 0;
>>> +
>>> + /* On the later chips the read prefetch bits become no-op bits */
>>> + pci_read_config_byte(pdev, 0x41, &r);
>>> + r &= ~fifobit[ap->port_no];
>>>
>>>
>> Why not:
>>
>> r &= ~fifo;
>>
>
> Because then it wouldn't clear the bits if they were set already and we
> wanted them off!
>
Ah, missed the modification of 'fifo'... :-<
MBR, Sergei