Re: [PATCH 1/2] ide: Add tx4939ide driver

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

 



On Fri, 12 Sep 2008 19:34:06 +0400, Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx> wrote:
> >>   You shouldn't fake the BMDMA interrupt. If it's not there, it's not 
> >>there. Or does this actually happen?
> 
> > IIRC, Yes.
> 
>     Hum, let me think... worth printing a message if this happens.

It might be.  I'll try it.

> >>>+		/*
> >>>+		 * If only one of XFERINT and HOST was asserted, mask
> >>>+		 * this interrupt and wait for an another one.  Note
> 
> >>   This comment somewhat contradicts the code which returns 1 if only 
> >>HOST interupt is asserted if ERR is set.
> 
>     Which is not its business to test. I think you should remove that above 
> check -- if there's INTRQ asserted, then it's asserted. I wonder if BMIDE 
> interrupt bit gets set in that case (suspecting it's not)...

Well, let me explain a bit.  The datasheed say I should wait _both_
XFERINT and HOST interrupt.  So, if only one of them was asserted, I
mask it and wait another one.  But on the error case, only HOST was
asserted and XFERINT was never asserted.  Then I could not exit from
"waiting another one" state, until timeout.

> >>>+			pr_debug("%s: weired interrupt status. "
> >>>  
> 
> >>   Weird.
> 
> > Sure.  But it can happen IIRC...
> 
>     I meant the typo. :-)

Oh, thanks!

> >>>+	__ide_flush_dcache_range((unsigned long)addr, size);
> 
> >>   Why is this needed BTW?
> 
> > Do you mean __ide_flush_dcache_range?  This is needed to avoid cache
> > inconsistency on PIO drive.  PIO transfer only writes to cache but
> > upper layers expects the data is in main memory.
> 
>     Hum, then I wonder why it's MIPS specific...

SPARC also have it.  And there were some discussions for ARM IIRC.

> >>>+	.read_sff_dma_status	= tx4939ide_read_sff_dma_status,
> 
> >>   Hum, it should be re-defined in both LE and BE mode (but actually not 
> >>called anyway).
> 
> > What do you mean?  Please elaborate?
> 
>     I mean that in LE mode you're using ide_read_sff_dma_status() but not 
> setting hwif->dma_base, so it won't work. But since it shouldn't be called in 
> this driver's case, this doesn't hurt.

Yes, I see, thanks.

---
Atsushi Nemoto


[Index of Archives]     [Linux MIPS Home]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Linux]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux