Bug 14310
Summary: | [bisected] 2.6.31 regression sis5513 PIO Mode 0 hang | ||
---|---|---|---|
Product: | IO/Storage | Reporter: | David Fries (David) |
Component: | IDE | Assignee: | io_ide (io_ide) |
Status: | CLOSED CODE_FIX | ||
Severity: | blocking | CC: | alan, bzolnier, jbohac, rjw |
Priority: | P1 | ||
Hardware: | All | ||
OS: | Linux | ||
Kernel Version: | 2.6.31 | Subsystem: | |
Regression: | Yes | Bisected commit-id: | |
Bug Depends on: | |||
Bug Blocks: | 13615 | ||
Attachments: | debug patch for sis5513 issue |
Description
David Fries
2009-10-03 02:52:50 UTC
First-Bad-Commit : 6029336426a2b43e4bc6f4a84be8789a047d139e Notify-Also : linux-ide@vger.kernel.org The bad commit exposed a bug which was fixed by e13ee546bb06453939014c7b854e77fb643fd6f1 sis5513: fix PIO setup for ATAPI devices I can still reproduce this on 2.6.31.5, where the e13ee546bb06453939014c7b854e77fb643fd6f1 fix is included. Reverting 6029336426a2b43e4bc6f4a84be8789a047d139e helps. Without reverting 6029336426a2b43e4bc6f4a84be8789a047d139e the system hangs on "ide-cd driver 5.00", but sometimes I also get {DriveReady SeekComplete Error} errors on hda before the hang -- never seen them without 6029336426a2b43e4bc6f4a84be8789a047d139e. A few interesting lines of output before the hang: (hand-copied, no serial console here) hda: ST38421A <strange_nonASCII_triangle_character_here> , ATA DISK drive hdc: LTN5269, ATAPI CD/DVD-ROM drive hdc: tPIO > 2, assuming tPIO=2 hdc: tPIO > 2, assuming tPIO=2 .. .. <hda seek errors> .. possibly failed opcode: 0xc6 hda: lost interrupt hda: <hda seek error> possibly failed opcode: 0x20 unknown patrition table ide-cd driver 5.00 <hang> I don't have regular access to this machine, but if help is needed, I can arrange access to the machine. Can we re-open this bug? Created attachment 23513 [details]
debug patch for sis5513 issue
Jiri, please try the patch from http://bugzilla.kernel.org/attachment.cgi?id=23513 Thanks. (In reply to comment #5) > Jiri, please try the patch from > > http://bugzilla.kernel.org/attachment.cgi?id=23513 Great, this patch, applied on 2.6.31.5, fixes the problem for me. Out of curiosity, I also tried to revert the original fix (e13ee546bb06453939014c7b854e77fb643fd6f1) and, with the debug patch still applied, it still worked. So maybe 6029336426a2b43e4bc6f4a84be8789a047d139e causes two independent problems in sis5513? One fixed with e13ee546bb06453939014c7b854e77fb643fd6f1 and one fixed with this debug patch? It would be great if this patch could be added to the stable tree. Thanks! On Tuesday 27 October 2009 09:35:27 bugzilla-daemon@bugzilla.kernel.org wrote: > http://bugzilla.kernel.org/show_bug.cgi?id=14310 > > > > > > --- Comment #6 from Jiri Bohac <jbohac@jikos.cz> 2009-10-27 08:35:25 --- > (In reply to comment #5) > > Jiri, please try the patch from > > > > http://bugzilla.kernel.org/attachment.cgi?id=23513 > > Great, this patch, applied on 2.6.31.5, fixes the problem for me. > > Out of curiosity, I also tried to revert the original fix > (e13ee546bb06453939014c7b854e77fb643fd6f1) and, with the debug patch still > applied, it still worked. Thanks for checking this. > So maybe 6029336426a2b43e4bc6f4a84be8789a047d139e causes two independent > problems in sis5513? One fixed with e13ee546bb06453939014c7b854e77fb643fd6f1 > and one fixed with this debug patch? Yes, your findings are correct. > It would be great if this patch could be added to the stable tree. Thanks! The problem is that the change in the debug patch affects all host drivers and at this moment is not safe enough neither for -stable nor for -rc. Somebody needs to go and review all host drivers and their usage of ->media field for tuning logic present in ->set_pio_mode implementations first. Before this happens we probably should revert commit 6029336 but I'll leave this up to the maintainer (I seriously doubt he will ever go and review any IDE drivers or make any IDE fixes so please just ping him about the revert). Thanks, From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com> Date: Thu, 5 Nov 2009 16:54:27 +0100 > The problem is that the change in the debug patch affects all host drivers > and at this moment is not safe enough neither for -stable nor for -rc. > > Somebody needs to go and review all host drivers and their usage of ->media > field for tuning logic present in ->set_pio_mode implementations first. The drivers which do this are: drivers/ide/alim15x3.c drivers/ide/it8172.c drivers/ide/it8213.c drivers/ide/pdc202xx_old.c drivers/ide/piix.c drivers/ide/qd65xx.c drivers/ide/sis5513.c drivers/ide/slc90e66.c While the majority of them seem to use it to decide whether to enable prefetching or not, alim15x3.c uses the ->media value to decide whether to turn on the ATA or the ATAPI FIFO. I don't think it's safe to call ->set_pio_mode() until we actually know the media type. I suppose we could guard the prefetch/FIFO changes in these drivers with a test like: if (!(hwif->port_flags & IDE_PFLAG_PROBING)) or something like that. But in this stage of the game, it's just too risky and reverting the early PIO0 commit is the best way to go. I'll do that now. You turn the FIFO off if you are probing. Always You cannot know the media type until you probe, and you cannot properly probe until you've set the interface to a sane known state. The media type should be set to indicate no media at that point. Agreed on reverting it - it generally only affects non PC platforms anyway - the PC ones tend to come up with the controller either in PIO 0 or configured to match the drive by the BIOS - both of which are generally acceptable. |