On Wed, Oct 03, 2007 at 04:41:37PM +0100, Adrian McMenamin wrote:
> On Wed, October 3, 2007 7:18 am, Paul Mundt wrote:
> > On Tue, Oct 02, 2007 at 10:09:27PM +0100, Adrian McMenamin wrote:
> >> Fix SH DMAC code to correctly handle PVR2 cascade DMA.
> >>
> >> This updates http://lkml.org/lkml/2007/10/2/276
> >>
> >> (I decided it was better to have the true size of the transfer put in
> >> via the API and refactor this here. And calc_xmit_shift(chan) should
> >> return 5 but only returns 3 so I've not used it here)
> >>
> > It would be helpful to know why calc_xmit_shift() is broken here rather
> > than just coding around it, as this will have implications for the other
> > DMA channels on SH7091/SH7750.
>
>
> >From include/asm-sh/cpu-sh4/dma.h
>
> 53 /*
> 54 * The DMA count is defined as the number of bytes to transfer.
> 55 */
> 56 static unsigned int ts_shift[] __maybe_unused = {
> 57 [XMIT_SZ_64BIT] = 3,
> 58 [XMIT_SZ_8BIT] = 0,
> 59 [XMIT_SZ_16BIT] = 1,
> 60 [XMIT_SZ_32BIT] = 2,
> 61 [XMIT_SZ_256BIT] = 5,
> 62 };
> 63 #endif
>
> ie ts_shift returns the number of bytes per transfer, but is then used as
> a bit shift:
>
Er, no it doesn't, try again. ts_shift returns the transfer size _shift_.
The comment refers to the DMA count, which is a different matter. Your
32-byte transfer where you want the >> 5 shift == ts_shift[XMIT_SZ_256BIT],
and there's nothing wrong with that. So the issue becomes why you don't
get a ts_shift[] offset of XMIT_SZ_256BIT for that channel, and that's
the bug that has to be fixed.
> 45 /*
> 46 * We determine the correct shift size based off of the CHCR transmit size
> 47 * for the given channel. Since we know that it will take:
> 48 *
> 49 * info->count >> ts_shift[transmit_size]
> 50 *
> 51 * iterations to complete the transfer.
> 52 */
> 53 static inline unsigned int calc_xmit_shift(struct dma_channel *chan)
> 54 {
> 55 u32 chcr = ctrl_inl(CHCR[chan->chan]);
> 56
> 57 return ts_shift[(chcr & CHCR_TS_MASK)>>CHCR_TS_SHIFT];
> 58 }
>
So for PVR2 cascade, what does the CHCR value work out to? Both
CHCR_TS_MASK and CHCR_TS_SHIFT haven't changed for SH7750, so this
suggests that either the CHCR value is just wrong or we've had a
long-standing bug with the CHCR.TS mask.
Either way, we are not going to hardcode a ts_shift value when the CHCR
has all of the information encoded in it!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
