Commit 54c38444fad6a99b4b19512f8f0055d69115e69e

DISCUSSION Tejun Heo 2 participants 3 messages Sep 16, 2009 – Sep 17, 2009 libata

Share & tools

Tejun Heo

author

2009-09-16 11:56 UTC #0 lore raw

Jeff,

Commit 54c38444fad6a99b4b19512f8f0055d69115e69e triggers WARN_ON_ONCE(ap->pflags & ATA_PFLAG_FROZEN); in ata_qc_complete().

Quoted 1 line

From the commit history, it seems the change was to add DMA engine

disable to sil_freeze(), right? I can't see why changing the orders would matter there. ata_port_abort() just changes the internal state of libata and doesn't affect low level driver or the hardware. Whether doing it before or after __ata_port_freeze() shouldn't make any different to sata_sil's freeze callback. Am I missing something?

Thanks.

Signature · 1 line

tejun

Jeff Garzik

2009-09-17 07:11 UTC #1 lore raw

Quoted 8 lines · Tejun Heo

Jeff,
Commit 54c38444fad6a99b4b19512f8f0055d69115e69e triggers WARN_ON_ONCE(ap->pflags& ATA_PFLAG_FROZEN); in ata_qc_complete().
Quoted 1 line
From the commit history, it seems the change was to add DMA engine
disable to sil_freeze(), right? I can't see why changing the orders would matter there. ata_port_abort() just changes the internal state of libata and doesn't affect low level driver or the hardware.

ata_port_abort does more than change internal state: it calls ata_qc_complete, which could trigger a taskfile register read.

All SFF BMDMA controllers /must/ disable the DMA engine, before touching the taskfile device register block.

Jeff

Tejun Heo

author

2009-09-17 07:30 UTC #2 lore raw

Quoted 10 lines · Jeff Garzik

Quoted 8 lines · Tejun Heo
Jeff,
Commit 54c38444fad6a99b4b19512f8f0055d69115e69e triggers WARN_ON_ONCE(ap->pflags& ATA_PFLAG_FROZEN); in ata_qc_complete().
Quoted 1 line
From the commit history, it seems the change was to add DMA engine
disable to sil_freeze(), right? I can't see why changing the orders would matter there. ata_port_abort() just changes the internal state of libata and doesn't affect low level driver or the hardware.

Quoted 5 lines

ata_port_abort does more than change internal state: it calls ata_qc_complete, which could trigger a taskfile register read.
All SFF BMDMA controllers /must/ disable the DMA engine, before touching the taskfile device register block.

Ah.. right, the result tf filling. Well, then, I guess we'll have to move the WARN_ON_ONCE(). I'll post a patch soon.

Thanks.

Signature · 1 line

tejun