* [PATCH] Fix simplex adapters with libata
@ 2007-03-08 9:12 Petr Vandrovec
2007-03-08 13:02 ` Alan Cox
2007-03-09 12:39 ` Jeff Garzik
0 siblings, 2 replies; 6+ messages in thread
From: Petr Vandrovec @ 2007-03-08 9:12 UTC (permalink / raw)
To: jgarzik; +Cc: linux-ide
Hello Jeff,
[please cc me, I'm not subscribed to linux-ide]
Recently I got my hands on nVidia's MCP61 PM-AM board, and
it contains IDE chip configured by BIOS with only primary
channel enabled. This confuses code which probes for
device DMA capabilities - it gets 0x60 (happy duplex
device) from primary channel BMDMA, but 0xFF (nobody here)
from secondary channel BMDMA. Due to this code then believes
that chip is simplex. I do not address this problem in
my patch, as I'm not sure how to handle this. Probably
ata_pci_init_one should have bitmap of enabled/possible
interfaces instead of their count, but it looks like
quite intrusive change, and maybe we do not care - for device
with only one channel simplex and regular DMA engines are
same.
But making device simplex pointed out that support for
DMA on simplex devices is currently broken - ata_dev_xfermask
tests whether device is simplex and if it is whether DMA
engine was assigned to this port. If not then it strips
out DMA bits from device. Problem is that code which assigns
DMA engine to port in ata_set_mode first detect device
mode and assigns DMA engine to channel only if some DMA
capable device was found.
And as xfermask stripped out DMA bits, host->simplex_claimed
is always NULL with current implementation.
By allowing DMA either if simplex_claimed is NULL or if it
points to current port DMA can be finally used - it gets
assigned to first port which contains any DMA capable
device.
Patch is for current Linus git tree (post 2.6.21-rc3).
Thanks,
Petr Vandrovec
Before:
pata_amd 0000:00:06.0: version 0.2.8
PCI: Setting latency timer of device 0000:00:06.0 to 64
ata5: PATA max UDMA/133 cmd 0x000101f0 ctl 0x000103f6 bmdma 0x0001f000 irq 14
ata6: PATA max UDMA/133 cmd 0x00010170 ctl 0x00010376 bmdma 0x0001f008 irq 15
scsi4 : pata_amd
ata5.00: ATAPI, max UDMA/66
ata5.00: simplex DMA is claimed by other device, disabling DMA
ata5.00: configured for PIO4
scsi5 : pata_amd
ata6: port disabled. ignoring.
ata6: reset failed, giving up
scsi 4:0:0:0: CD-ROM ATAPI DVD W DH16W1P LG12 PQ: 0 ANSI: 5
After:
pata_amd 0000:00:06.0: version 0.2.8
PCI: Setting latency timer of device 0000:00:06.0 to 64
ata5: PATA max UDMA/133 cmd 0x000101f0 ctl 0x000103f6 bmdma 0x0001f000 irq 14
ata6: PATA max UDMA/133 cmd 0x00010170 ctl 0x00010376 bmdma 0x0001f008 irq 15
scsi4 : pata_amd
ata5.00: ATAPI, max UDMA/66
ata5.00: configured for UDMA/33
scsi5 : pata_amd
ata6: port disabled. ignoring.
ata6: reset failed, giving up
scsi 4:0:0:0: CD-ROM ATAPI DVD W DH16W1P LG12 PQ: 0 ANSI: 5
Signed-off-by: Petr Vandrovec <petr@vandrovec.name>
diff -uprdN linux/drivers/ata/libata-core.c linux/drivers/ata/libata-core.c
--- linux/drivers/ata/libata-core.c 2007-03-07 22:13:24.000000000 -0800
+++ linux/drivers/ata/libata-core.c 2007-03-08 00:15:37.000000000 -0800
@@ -3455,7 +3455,8 @@ static void ata_dev_xfermask(struct ata_
"device is on DMA blacklist, disabling DMA\n");
}
- if ((host->flags & ATA_HOST_SIMPLEX) && host->simplex_claimed != ap) {
+ if ((host->flags & ATA_HOST_SIMPLEX) &&
+ host->simplex_claimed && host->simplex_claimed != ap) {
xfer_mask &= ~(ATA_MASK_MWDMA | ATA_MASK_UDMA);
ata_dev_printk(dev, KERN_WARNING, "simplex DMA is claimed by "
"other device, disabling DMA\n");
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Fix simplex adapters with libata
2007-03-08 9:12 [PATCH] Fix simplex adapters with libata Petr Vandrovec
@ 2007-03-08 13:02 ` Alan Cox
2007-03-08 14:49 ` Mark Lord
2007-03-09 12:39 ` Jeff Garzik
1 sibling, 1 reply; 6+ messages in thread
From: Alan Cox @ 2007-03-08 13:02 UTC (permalink / raw)
To: Petr Vandrovec; +Cc: jgarzik, linux-ide
> And as xfermask stripped out DMA bits, host->simplex_claimed
> is always NULL with current implementation.
>
> By allowing DMA either if simplex_claimed is NULL or if it
> points to current port DMA can be finally used - it gets
> assigned to first port which contains any DMA capable
> device.
Doh, sorry I should have caught that when testing.
> Signed-off-by: Petr Vandrovec <petr@vandrovec.name>
Acked-by: Alan Cox <alan@redhat.com>
>
> diff -uprdN linux/drivers/ata/libata-core.c linux/drivers/ata/libata-core.c
> --- linux/drivers/ata/libata-core.c 2007-03-07 22:13:24.000000000 -0800
> +++ linux/drivers/ata/libata-core.c 2007-03-08 00:15:37.000000000 -0800
> @@ -3455,7 +3455,8 @@ static void ata_dev_xfermask(struct ata_
> "device is on DMA blacklist, disabling DMA\n");
> }
>
> - if ((host->flags & ATA_HOST_SIMPLEX) && host->simplex_claimed != ap) {
> + if ((host->flags & ATA_HOST_SIMPLEX) &&
> + host->simplex_claimed && host->simplex_claimed != ap) {
> xfer_mask &= ~(ATA_MASK_MWDMA | ATA_MASK_UDMA);
> ata_dev_printk(dev, KERN_WARNING, "simplex DMA is claimed by "
> "other device, disabling DMA\n");
> -
> 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
--
--
Sick of rip off UK rail fares ? Learn how to get far cheaper fares
http://zeniv.linux.org.uk/~alan/GTR/
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Fix simplex adapters with libata
2007-03-08 13:02 ` Alan Cox
@ 2007-03-08 14:49 ` Mark Lord
2007-03-08 15:05 ` Mark Lord
2007-03-09 1:47 ` vandrove
0 siblings, 2 replies; 6+ messages in thread
From: Mark Lord @ 2007-03-08 14:49 UTC (permalink / raw)
To: Alan Cox; +Cc: Petr Vandrovec, jgarzik, linux-ide
Alan Cox wrote:
>
>> diff -uprdN linux/drivers/ata/libata-core.c linux/drivers/ata/libata-core.c
>> --- linux/drivers/ata/libata-core.c 2007-03-07 22:13:24.000000000 -0800
>> +++ linux/drivers/ata/libata-core.c 2007-03-08 00:15:37.000000000 -0800
>> @@ -3455,7 +3455,8 @@ static void ata_dev_xfermask(struct ata_
>> "device is on DMA blacklist, disabling DMA\n");
>> }
>>
>> - if ((host->flags & ATA_HOST_SIMPLEX) && host->simplex_claimed != ap) {
>> + if ((host->flags & ATA_HOST_SIMPLEX) &&
>> + host->simplex_claimed && host->simplex_claimed != ap) {
A different version of this fix just went upstream for 2.6.21 via Jeff.
Which of the two is correct?
I believe the other one looks like this:
>> - if ((host->flags & ATA_HOST_SIMPLEX) && host->simplex_claimed != ap) {
>> - if ((host->flags & ATA_HOST_SIMPLEX) && host->simplex_claimed == ap) {
Cheers
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Fix simplex adapters with libata
2007-03-08 14:49 ` Mark Lord
@ 2007-03-08 15:05 ` Mark Lord
2007-03-09 1:47 ` vandrove
1 sibling, 0 replies; 6+ messages in thread
From: Mark Lord @ 2007-03-08 15:05 UTC (permalink / raw)
To: Alan Cox; +Cc: Petr Vandrovec, jgarzik, linux-ide
Mark Lord wrote:
>..
> A different version of this fix just went upstream for 2.6.21 via Jeff.
> Which of the two is correct?
>
> I believe the other one looks like this:
>
>>> - if ((host->flags & ATA_HOST_SIMPLEX) && host->simplex_claimed != ap) {
>>> - if ((host->flags & ATA_HOST_SIMPLEX) && host->simplex_claimed == ap) {
Mmm.. or was the upstream patch the other way 'round ? (lost the posting here)
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Fix simplex adapters with libata
2007-03-08 14:49 ` Mark Lord
2007-03-08 15:05 ` Mark Lord
@ 2007-03-09 1:47 ` vandrove
1 sibling, 0 replies; 6+ messages in thread
From: vandrove @ 2007-03-09 1:47 UTC (permalink / raw)
To: Mark Lord; +Cc: Alan Cox, Petr Vandrovec, jgarzik, linux-ide
Quoting Mark Lord <liml@rtr.ca>:
> Alan Cox wrote:
> >
> >> diff -uprdN linux/drivers/ata/libata-core.c
> linux/drivers/ata/libata-core.c
> >> --- linux/drivers/ata/libata-core.c 2007-03-07 22:13:24.000000000 -0800
> >> +++ linux/drivers/ata/libata-core.c 2007-03-08 00:15:37.000000000 -0800
> >> @@ -3455,7 +3455,8 @@ static void ata_dev_xfermask(struct ata_
> >> "device is on DMA blacklist, disabling DMA\n");
> >> }
> >>
> >> - if ((host->flags & ATA_HOST_SIMPLEX) && host->simplex_claimed != ap) {
> >> + if ((host->flags & ATA_HOST_SIMPLEX) &&
> >> + host->simplex_claimed && host->simplex_claimed != ap) {
>
> A different version of this fix just went upstream for 2.6.21 via Jeff.
> Which of the two is correct?
>
> I believe the other one looks like this:
>
> >> - if ((host->flags & ATA_HOST_SIMPLEX) && host->simplex_claimed != ap) {
> >> + if ((host->flags & ATA_HOST_SIMPLEX) && host->simplex_claimed == ap) {
This one does not look correct to me. It will not disable DMA for anything, as
simplex_claimed will be NULL on first pass, and either NULL or first port on
second pass, so it will never match currently tested port, and so it will leave
DMA enabled for both channels, which is deadly for real simplex controllers.
Petr
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Fix simplex adapters with libata
2007-03-08 9:12 [PATCH] Fix simplex adapters with libata Petr Vandrovec
2007-03-08 13:02 ` Alan Cox
@ 2007-03-09 12:39 ` Jeff Garzik
1 sibling, 0 replies; 6+ messages in thread
From: Jeff Garzik @ 2007-03-09 12:39 UTC (permalink / raw)
To: Petr Vandrovec; +Cc: linux-ide
Petr Vandrovec wrote:
> Hello Jeff,
>
> [please cc me, I'm not subscribed to linux-ide]
>
> Recently I got my hands on nVidia's MCP61 PM-AM board, and
> it contains IDE chip configured by BIOS with only primary
> channel enabled. This confuses code which probes for
> device DMA capabilities - it gets 0x60 (happy duplex
> device) from primary channel BMDMA, but 0xFF (nobody here)
> from secondary channel BMDMA. Due to this code then believes
> that chip is simplex. I do not address this problem in
> my patch, as I'm not sure how to handle this. Probably
> ata_pci_init_one should have bitmap of enabled/possible
> interfaces instead of their count, but it looks like
> quite intrusive change, and maybe we do not care - for device
> with only one channel simplex and regular DMA engines are
> same.
>
> But making device simplex pointed out that support for
> DMA on simplex devices is currently broken - ata_dev_xfermask
> tests whether device is simplex and if it is whether DMA
> engine was assigned to this port. If not then it strips
> out DMA bits from device. Problem is that code which assigns
> DMA engine to port in ata_set_mode first detect device
> mode and assigns DMA engine to channel only if some DMA
> capable device was found.
>
> And as xfermask stripped out DMA bits, host->simplex_claimed
> is always NULL with current implementation.
>
> By allowing DMA either if simplex_claimed is NULL or if it
> points to current port DMA can be finally used - it gets
> assigned to first port which contains any DMA capable
> device.
>
> Patch is for current Linus git tree (post 2.6.21-rc3).
> Thanks,
> Petr Vandrovec
>
> Before:
> pata_amd 0000:00:06.0: version 0.2.8
> PCI: Setting latency timer of device 0000:00:06.0 to 64
> ata5: PATA max UDMA/133 cmd 0x000101f0 ctl 0x000103f6 bmdma 0x0001f000 irq 14
> ata6: PATA max UDMA/133 cmd 0x00010170 ctl 0x00010376 bmdma 0x0001f008 irq 15
> scsi4 : pata_amd
> ata5.00: ATAPI, max UDMA/66
> ata5.00: simplex DMA is claimed by other device, disabling DMA
> ata5.00: configured for PIO4
> scsi5 : pata_amd
> ata6: port disabled. ignoring.
> ata6: reset failed, giving up
> scsi 4:0:0:0: CD-ROM ATAPI DVD W DH16W1P LG12 PQ: 0 ANSI: 5
>
> After:
> pata_amd 0000:00:06.0: version 0.2.8
> PCI: Setting latency timer of device 0000:00:06.0 to 64
> ata5: PATA max UDMA/133 cmd 0x000101f0 ctl 0x000103f6 bmdma 0x0001f000 irq 14
> ata6: PATA max UDMA/133 cmd 0x00010170 ctl 0x00010376 bmdma 0x0001f008 irq 15
> scsi4 : pata_amd
> ata5.00: ATAPI, max UDMA/66
> ata5.00: configured for UDMA/33
> scsi5 : pata_amd
> ata6: port disabled. ignoring.
> ata6: reset failed, giving up
> scsi 4:0:0:0: CD-ROM ATAPI DVD W DH16W1P LG12 PQ: 0 ANSI: 5
>
> Signed-off-by: Petr Vandrovec <petr@vandrovec.name>
applied to #upstream-fixes
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-03-09 12:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-03-08 9:12 [PATCH] Fix simplex adapters with libata Petr Vandrovec
2007-03-08 13:02 ` Alan Cox
2007-03-08 14:49 ` Mark Lord
2007-03-08 15:05 ` Mark Lord
2007-03-09 1:47 ` vandrove
2007-03-09 12:39 ` Jeff Garzik
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).