[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: People with ATAPI problems: possible fix



Hi,

I used to have the problem described by Branden Robinson in a previous
mail on DebianPPC: a Samsung CDRW/DVD that would not eject discs nor
play, nor anything.
I patched a 2.4.24-ben1 with your code and now the CD features are
working fine. I didn't test burning CDs or playing DVDs yet.

Cheers,
Marco

Le dim 15/02/2004 à 03:48, Benjamin Herrenschmidt a écrit :
> Hi !
> 
> Recently, there have been lost of reports of ATAPI issues, typically
> with CD/DVD burners, where DMA would be disabled for no obvious reasons,
> with the driver spitting a message about a timeout waiting for dbdma
> command stop.
> 
> The problem is that our driver doesn't deal with buffer underruns due
> to the drive not sending as many data as requested.
> 
> This patch is an attempt at fixing this. Please let me know if it
> helps.
> 
> Ben.
> 
> ===== drivers/ide/ppc/pmac.c 1.50 vs edited =====
> --- 1.50/drivers/ide/ppc/pmac.c	Sat Feb 14 19:29:16 2004
> +++ edited/drivers/ide/ppc/pmac.c	Sun Feb 15 13:47:12 2004
> @@ -55,7 +55,7 @@
>  
>  #define IDE_PMAC_DEBUG
>  
> -#define DMA_WAIT_TIMEOUT	100
> +#define DMA_WAIT_TIMEOUT	50
>  
>  typedef struct pmac_ide_hwif {
>  	unsigned long			regbase;
> @@ -2032,8 +2032,11 @@
>  	dstat = readl(&dma->status);
>  	writel(((RUN|WAKE|DEAD) << 16), &dma->control);
>  	pmac_ide_destroy_dmatable(drive);
> -	/* verify good dma status */
> -	return (dstat & (RUN|DEAD|ACTIVE)) != RUN;
> +	/* verify good dma status. we don't check for ACTIVE beeing 0. We should...
> +	 * in theory, but with ATAPI decices doing buffer underruns, that would
> +	 * cause us to disable DMA, which isn't what we want
> +	 */
> +	return (dstat & (RUN|DEAD)) != RUN;
>  }
>  
>  /*
> @@ -2047,7 +2050,7 @@
>  {
>  	pmac_ide_hwif_t* pmif = (pmac_ide_hwif_t *)HWIF(drive)->hwif_data;
>  	volatile struct dbdma_regs *dma;
> -	unsigned long status;
> +	unsigned long status, timeout;
>  
>  	if (pmif == NULL)
>  		return 0;
> @@ -2063,17 +2066,8 @@
>  	 * - The dbdma fifo hasn't yet finished flushing to
>  	 * to system memory when the disk interrupt occurs.
>  	 * 
> -	 * The trick here is to increment drive->waiting_for_dma,
> -	 * and return as if no interrupt occurred. If the counter
> -	 * reach a certain timeout value, we then return 1. If
> -	 * we really got the interrupt, it will happen right away
> -	 * again.
> -	 * Apple's solution here may be more elegant. They issue
> -	 * a DMA channel interrupt (a separate irq line) via a DBDMA
> -	 * NOP command just before the STOP, and wait for both the
> -	 * disk and DBDMA interrupts to have completed.
>  	 */
> - 
> +
>  	/* If ACTIVE is cleared, the STOP command have passed and
>  	 * transfer is complete.
>  	 */
> @@ -2085,15 +2079,26 @@
>  			called while not waiting\n", HWIF(drive)->index);
>  
>  	/* If dbdma didn't execute the STOP command yet, the
> -	 * active bit is still set */
> -	drive->waiting_for_dma++;
> -	if (drive->waiting_for_dma >= DMA_WAIT_TIMEOUT) {
> -		printk(KERN_WARNING "ide%d, timeout waiting \
> -			for dbdma command stop\n", HWIF(drive)->index);
> -		return 1;
> -	}
> -	udelay(5);
> -	return 0;
> +	 * active bit is still set. We consider that we aren't
> +	 * sharing interrupts (which is hopefully the case with
> +	 * those controllers) and so we just try to flush the
> +	 * channel for pending data in the fifo
> +	 */
> +	udelay(1);
> +	writel((FLUSH << 16) | FLUSH, &dma->control);
> +	timeout = 0;
> +	for (;;) {
> +		udelay(1);
> +		status = readl(&dma->status);
> +		if ((status & FLUSH) == 0)
> +			break;
> +		if (++timeout > 100) {
> +			printk(KERN_WARNING "ide%d, ide_dma_test_irq \
> +			timeout flushing channel\n", HWIF(drive)->index);
> +			break;
> +		}
> +	}	
> +	return 1;
>  }
>  
>  static int __pmac
> 
> 

Attachment: signature.asc
Description: Ceci est une partie de message =?ISO-8859-1?Q?num=E9riquement?= =?ISO-8859-1?Q?_sign=E9e=2E?=


Reply to: