[PATCH 5/13] ide: add IDE_HFLAG_ERROR_STOPS_FIFO host flag

[PATCH 5/13] ide: add IDE_HFLAG_ERROR_STOPS_FIFO host flag

[Bartlomiej Zolnierkiewicz]

Add IDE_HFLAG_ERROR_STOPS_FIFO host flag and use it instead
of hwif->err_stops_fifo.  As a side-effect this change fixes
hwif->err_stops_fifo not being restored by ide_hwif_restore().

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-io.c           |    3 ++-
 drivers/ide/pci/pdc202xx_new.c |    3 +--
 drivers/ide/pci/pdc202xx_old.c |    8 ++++----
 include/linux/ide.h            |    3 ++-
 4 files changed, 9 insertions(+), 8 deletions(-)

Index: b/drivers/ide/ide-io.c
===================================================================
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -519,7 +519,8 @@ static ide_startstop_t ide_ata_error(ide
 		}
 	}
 
-	if ((stat & DRQ_STAT) && rq_data_dir(rq) == READ && hwif->err_stops_fifo == 0)
+	if ((stat & DRQ_STAT) && rq_data_dir(rq) == READ &&
+	    (hwif->host_flags & IDE_HFLAG_ERROR_STOPS_FIFO) == 0)
 		try_to_flush_leftover_data(drive);
 
 	if (rq->errors >= ERROR_MAX || blk_noretry_request(rq)) {
Index: b/drivers/ide/pci/pdc202xx_new.c
===================================================================
--- a/drivers/ide/pci/pdc202xx_new.c
+++ b/drivers/ide/pci/pdc202xx_new.c
@@ -471,8 +471,6 @@ static void __devinit init_hwif_pdc202ne
 	hwif->quirkproc = &pdcnew_quirkproc;
 	hwif->resetproc = &pdcnew_reset;
 
-	hwif->err_stops_fifo = 1;
-
 	hwif->drives[0].autotune = hwif->drives[1].autotune = 1;
 
 	if (hwif->dma_base == 0)
@@ -510,6 +508,7 @@ static struct pci_dev * __devinit pdc202
 		.init_chipset	= init_chipset_pdcnew, \
 		.init_hwif	= init_hwif_pdc202new, \
 		.host_flags	= IDE_HFLAG_POST_SET_MODE | \
+				  IDE_HFLAG_ERROR_STOPS_FIFO | \
 				  IDE_HFLAG_OFF_BOARD, \
 		.pio_mask	= ATA_PIO4, \
 		.mwdma_mask	= ATA_MWDMA2, \
Index: b/drivers/ide/pci/pdc202xx_old.c
===================================================================
--- a/drivers/ide/pci/pdc202xx_old.c
+++ b/drivers/ide/pci/pdc202xx_old.c
@@ -317,8 +317,6 @@ static void __devinit init_hwif_pdc202xx
 	if (hwif->pci_dev->device != PCI_DEVICE_ID_PROMISE_20246)
 		hwif->resetproc = &pdc202xx_reset;
 
-	hwif->err_stops_fifo = 1;
-
 	hwif->drives[0].autotune = hwif->drives[1].autotune = 1;
 
 	if (hwif->dma_base == 0)
@@ -393,7 +391,8 @@ static void __devinit pdc202ata4_fixup_i
 		.init_hwif	= init_hwif_pdc202xx, \
 		.init_dma	= init_dma_pdc202xx, \
 		.extra		= 48, \
-		.host_flags	= IDE_HFLAG_OFF_BOARD, \
+		.host_flags	= IDE_HFLAG_ERROR_STOPS_FIFO | \
+				  IDE_HFLAG_OFF_BOARD, \
 		.pio_mask	= ATA_PIO4, \
 		.mwdma_mask	= ATA_MWDMA2, \
 		.udma_mask	= udma, \
@@ -406,7 +405,8 @@ static ide_pci_device_t pdc202xx_chipset
 		.init_hwif	= init_hwif_pdc202xx,
 		.init_dma	= init_dma_pdc202xx,
 		.extra		= 16,
-		.host_flags	= IDE_HFLAG_OFF_BOARD,
+		.host_flags	= IDE_HFLAG_ERROR_STOPS_FIFO |
+				  IDE_HFLAG_OFF_BOARD,
 		.pio_mask	= ATA_PIO4,
 		.mwdma_mask	= ATA_MWDMA2,
 		.udma_mask	= ATA_UDMA2,
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -799,7 +799,6 @@ typedef struct hwif_s {
 	unsigned	auto_poll  : 1; /* supports nop auto-poll */
 	unsigned	sg_mapped  : 1;	/* sg_table and sg_nents are ready */
 	unsigned	no_io_32bit : 1; /* 1 = can not do 32-bit IO ops */
-	unsigned	err_stops_fifo : 1; /* 1=data FIFO is cleared by an error */
 	unsigned	mmio       : 1; /* host uses MMIO */
 
 	struct device	gendev;
@@ -1261,6 +1260,8 @@ enum {
 	IDE_HFLAG_NO_LBA48		= (1 << 17),
 	/* no LBA48 DMA */
 	IDE_HFLAG_NO_LBA48_DMA		= (1 << 18),
+	/* data FIFO is cleared by an error */
+	IDE_HFLAG_ERROR_STOPS_FIFO	= (1 << 19),
 };
 
 #ifdef CONFIG_BLK_DEV_OFFBOARD

Re: [PATCH 5/13] ide: add IDE_HFLAG_ERROR_STOPS_FIFO host flag

[Jeff Garzik]

On Wed, Sep 05, 2007 at 11:52:57PM +0200, Bartlomiej Zolnierkiewicz wrote:
> 
> Add IDE_HFLAG_ERROR_STOPS_FIFO host flag and use it instead
> of hwif->err_stops_fifo.  As a side-effect this change fixes
> hwif->err_stops_fifo not being restored by ide_hwif_restore().
> 
> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> ---
>  drivers/ide/ide-io.c           |    3 ++-
>  drivers/ide/pci/pdc202xx_new.c |    3 +--
>  drivers/ide/pci/pdc202xx_old.c |    8 ++++----
>  include/linux/ide.h            |    3 ++-
>  4 files changed, 9 insertions(+), 8 deletions(-)

Hum, I wonder if libata needs something like this.

	Jeff

Re: [PATCH 5/13] ide: add IDE_HFLAG_ERROR_STOPS_FIFO host flag

[Jeff Garzik]

Alan Cox wrote:
> On Thu, 6 Sep 2007 04:13:56 -0400
> Jeff Garzik <jeff@garzik.org> wrote:
> 
>> On Wed, Sep 05, 2007 at 11:52:57PM +0200, Bartlomiej Zolnierkiewicz wrote:
>>> Add IDE_HFLAG_ERROR_STOPS_FIFO host flag and use it instead
>>> of hwif->err_stops_fifo.  As a side-effect this change fixes
>>> hwif->err_stops_fifo not being restored by ide_hwif_restore().
>>>
>>> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
>>> ---
>>>  drivers/ide/ide-io.c           |    3 ++-
>>>  drivers/ide/pci/pdc202xx_new.c |    3 +--
>>>  drivers/ide/pci/pdc202xx_old.c |    8 ++++----
>>>  include/linux/ide.h            |    3 ++-
>>>  4 files changed, 9 insertions(+), 8 deletions(-)
>> Hum, I wonder if libata needs something like this.
> 
> You'd have to add the drain data on error hack first, right now we will
> reset in that case (which for one or two devices has another problem in
> that it won't clear a stuck FIFO)

Nod.  There was nothing ever wrong with Mark's drain patch, I just never 
saw a solid justification for it outside of root 
accidentally/intentionally misprogramming things -- a situation whereall 
sorts of lockups and hangs and explosions can occur, even with the 
fifo-drain change.  We could fill the kernel with code protecting root 
from himself.

But I was thinking for ATAPI it would be useful to report how much data 
was drained from the FIFO, for improved diagnostics.  And if there are 
other conditions that imply draining would be useful, by all means lets 
merge it.

	Jeff

Re: [PATCH 5/13] ide: add IDE_HFLAG_ERROR_STOPS_FIFO host flag

[Alan Cox]

On Thu, 6 Sep 2007 04:13:56 -0400
Jeff Garzik <jeff@garzik.org> wrote:

> On Wed, Sep 05, 2007 at 11:52:57PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > 
> > Add IDE_HFLAG_ERROR_STOPS_FIFO host flag and use it instead
> > of hwif->err_stops_fifo.  As a side-effect this change fixes
> > hwif->err_stops_fifo not being restored by ide_hwif_restore().
> > 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > ---
> >  drivers/ide/ide-io.c           |    3 ++-
> >  drivers/ide/pci/pdc202xx_new.c |    3 +--
> >  drivers/ide/pci/pdc202xx_old.c |    8 ++++----
> >  include/linux/ide.h            |    3 ++-
> >  4 files changed, 9 insertions(+), 8 deletions(-)
> 
> Hum, I wonder if libata needs something like this.

You'd have to add the drain data on error hack first, right now we will
reset in that case (which for one or two devices has another problem in
that it won't clear a stuck FIFO)

Alan

Re: [PATCH 5/13] ide: add IDE_HFLAG_ERROR_STOPS_FIFO host flag

[Alan Cox]

> > You'd have to add the drain data on error hack first, right now we will
> > reset in that case (which for one or two devices has another problem in
> > that it won't clear a stuck FIFO)
> 
> Nod.  There was nothing ever wrong with Mark's drain patch, I just never 
> saw a solid justification for it outside of root 

You don't need to be root, any user can do this with the right commands
via SG_IO - and it happens on errors occassionally for real.

Re: [PATCH 5/13] ide: add IDE_HFLAG_ERROR_STOPS_FIFO host flag

[Jeff Garzik]

Alan Cox wrote:
>>> You'd have to add the drain data on error hack first, right now we will
>>> reset in that case (which for one or two devices has another problem in
>>> that it won't clear a stuck FIFO)
>> Nod.  There was nothing ever wrong with Mark's drain patch, I just never 
>> saw a solid justification for it outside of root 
> 
> You don't need to be root, any user can do this with the right commands
> via SG_IO - and it happens on errors occassionally for real.

Which commands?

	Jeff

Re: [PATCH 5/13] ide: add IDE_HFLAG_ERROR_STOPS_FIFO host flag

[Alan Cox]

On Sat, 08 Sep 2007 12:42:52 -0400
Jeff Garzik <jeff@garzik.org> wrote:

> Alan Cox wrote:
> >>> You'd have to add the drain data on error hack first, right now we will
> >>> reset in that case (which for one or two devices has another problem in
> >>> that it won't clear a stuck FIFO)
> >> Nod.  There was nothing ever wrong with Mark's drain patch, I just never 
> >> saw a solid justification for it outside of root 
> > 
> > You don't need to be root, any user can do this with the right commands
> > via SG_IO - and it happens on errors occassionally for real.
> 
> Which commands?

Should work for anything where you send data to the drive and give the
wrong values to the SG_IO for the I/O size.

Alan