http://bugzilla.kernel.org/show_bug.cgi?id=13399 --- Comment #21 from Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx> 2009-06-13 16:54:36 --- On Saturday 13 June 2009 18:29:07 bugzilla-daemon@xxxxxxxxxxxxxxxxxxx wrote: > http://bugzilla.kernel.org/show_bug.cgi?id=13399 > > > > > > --- Comment #20 from Borislav Petkov <bbpetkov@xxxxxxxx> 2009-06-13 16:29:05 --- > Hi Bart, > > thanks for analyzing this. > > I'm staring at the ATA_DRQ == 0 part in cdrom_newpc_intr: > > } else if (!blk_pc_request(rq)) { > ide_cd_request_sense_fixup(drive, cmd); > /* complain if we still have data left to transfer */ > uptodate = cmd->nleft ? 0 : 1; > if (uptodate == 0) > rq->cmd_flags |= REQ_FAILED; > } > goto out_end; > } > > so, in our case ide_cd_error_cmd() kills the rq prematurely and that's > why ide_complete_rq() oopses later. And this is caused by uptodate == Nope, it is block layer that kills it prematurely. There are two issues here: * OOPS (*the*regression* which should be taken care of first) (cause: unexpected interaction between ide/block) * handling of non-fully completed requests with "good" status (cause: stupid hardware) > 0. Now, here's how the ATA spec (d1532v1r4b-ATA-ATAPI-7) describes the > semantics of clearing of the DRQ bit by the drive: > > " > 5.14.5.5 DRQ (Data request) > > ... > > The DRQ bit shall be cleared to zero by the device: > 1) when the last word of the data transfer occurs; > 2) when the last word of the command packet transfer occurs for a > PACKET command. > " > > now there's a subtlety here wrt to what am I to do as an IRQ handler > when my drive clears the DRQ bit: do I _drain_ the last bytes remaining > (in our case 2) or do I fail the rq straightaway. I'm pretty sure > cmd->nleft is 2 in our case so I think that it might be only right to > drain the device first, i.e. do > > uptodate = (cmd->nleft - thislen) ? 0 : 1; > > and then later ide_pio_bytes() before completing the rq properly. Hmm? We should drain only if the device really wants to transfer more data... cmd->nleft is our concept, thislen reflects the hardware state. > And yes, this is against spec since the following sentence states that > the data can be drained only "... via DMA mode if DMARQ and DMACK- are > asserted and BSY is set to one." but we should give it a try... > > Ideas? Opinions? Yes. Please stop reading this stupid ATAPI spec, it is near to worthless when it comes to the real hardware implementations/bugs.. -- Configure bugmail: http://bugzilla.kernel.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching the assignee of the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
