Re: msk(4) require to sync status buffer

[KIYOHARA Takashi <kiyohara%kk.iij4u.or.jp@localhost>]

Hi! tsutsui-san,


From: Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>
Date: Mon, 25 Feb 2008 02:16:24 +0900

> kiyohara%kk.iij4u.or.jp@localhost wrote:

> > > > +               /* Write back after clearing SK_Y2_STOPC_OWN of opcode 
> > > > */
> > > > +               MSK_CDSTSYNC(sc, sc->sk_status_idx,
> > > > +                   BUS_DMASYNC_PREREAD|BUS_DMASYNC_PREWRITE);
> > > > +
> > > 
> > > Why do you say "write back" here?
> > > 
> > > If there is no date transfered from memory to the device,
> > > no writeback is needed.
> > 
> > "write back" comment no necessary.
> 
> Then why do you think PREREAD/PREWRITE ops are requied here?

The reason is that the msk driver clears SK_Y2_STOPC_OWN, and the DMA
buffer is returned to LSI.  And, LSI writes status reading that
SK_Y2_STOPC_OWN is clear.


> > > > -               MSK_CDSTSYNC(sc, sc->sk_status_idx,
> > > > -                   BUS_DMASYNC_POSTREAD|BUS_DMASYNC_POSTWRITE);
> > > > +               MSK_CDSTSYNC(sc, sc->sk_status_idx, 
> > > > BUS_DMASYNC_POSTREAD);
> > > 
> > > This seems okay as the first one in this function.
> > 
> > This change no need.
> 
> As noted in the previous mail, if there is no WRITE op
> (DMA xfer from CPU to device), POSTWRITE is not needed here.
> (though POSTWRITE is no-op on most implemetation)

I think that it should do POSTREAD/POSTWRITE here because we are doing
PREREAD/PREWRITE beforehand.


> > > >                 cur_st = &sc->sk_status_ring[sc->sk_status_idx];
> > > >         }
> > > > +       /* Invalidate the status buffer that has already been cached */
> > > > +       MSK_CDSTSYNC(sc, sc->sk_status_idx, BUS_DMASYNC_PREREAD);
> > > 
> > > In your patch, only the last descriptor gets PREREAD op,
> > > i.e. the rest DMA descriptors are not flushed from cache.
> > > (then you had to add extra PREREAD op in the above?)
> > > You have to call PREREAD op after sk_status_ring is fetched
> > > in while loop.
> > 
> > Why?
> > I have MSK_CDSTSYNCed with the comment for "write back" on the previous
> > state.
> 
> Strictly speaking, PREREAD or PREWRITE don't imply cache ops.
> We should call bus_dmamap_sync(9) ops as defined in its API,
> i.e. PREREAD/PREWRITE should be called before DMA started
> and POSTREAD/POSTWRITE should be called after DMA xfer.

The DMA transfer of the status buffer that we had read at the end was not
completed.  And status is sure to be written to this buffer at next.
And, this buffer might be cached on the memory.  In a word, it is
necessary to invalidate. 

Thanks,
--
kiyohara