Hi Marek,

> Dear Lukasz Majewski,
> 
> > Hi Marek,
> > 
> > > Dear Lukasz Majewski,
> > > 
> > > > The s3c udc driver sends data in a max packet size. Therefore
> > > > the dcache invalidate range shall be equal to max packet, not
> > > > the entire DMA_BUFFER_SIZE.
> > > > 
> > > > Signed-off-by: Lukasz Majewski <l.majew...@samsung.com>
> > > > Cc: Marek Vasut <ma...@denx.de>
> > > > ---
> > > > 
> > > >  drivers/usb/gadget/s3c_udc_otg_xfer_dma.c |    2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/usb/gadget/s3c_udc_otg_xfer_dma.c
> > > > b/drivers/usb/gadget/s3c_udc_otg_xfer_dma.c index
> > > > d7af5e9..5e3ba76 100644 ---
> > > > a/drivers/usb/gadget/s3c_udc_otg_xfer_dma.c +++
> > > > b/drivers/usb/gadget/s3c_udc_otg_xfer_dma.c @@ -117,7 +117,7 @@
> > > > static int setdma_rx(struct s3c_ep *ep, struct s3c_request *req)
> > > > 
> > > >         invalidate_dcache_range((unsigned long)
> > > > 
> > > > ep->dev->dma_buf[ep_num], (unsigned long)
> > > > ep->dev->dma_buf[ep_num]
> > > > -                               + DMA_BUFFER_SIZE);
> > > > +                               + ep->ep.maxpacket);
> > > 
> > > Is this maxpacket _always_ multiple of cacheline big or will you
> > > need some ROUND_UP() call here ?
> > 
> > The maxpacket value is equal to 64 B for EP0 and 512 B for EP1 and
> > EP2 (wMaxPacketSize field of the descriptor).
> > 
> > Moreover this invalidation is done on already memaligned buffer
> > (which is 16 KiB). In other words, we are copying data to specially
> > prepared buffer (per UDC device EP, not usb_request).
> > 
> > In my opinion the maxpacket don't need to be rounded up.
> 
> So it can never happen that ep maxpacket is unaligned?

maxpacket can be defined as e.g. 16 Bytes (wMaxPacketSize). 

However the underlying buffer (on which we perform dcache invalidation)
is already cache line aware (defined with memalign).
Also we copy the usb request data there (yes I know that this is not
the best possible solution). Afterwards address of this buffer is a
starting point for DMA.
Since we are sending in the HW one packet at a time (with maxpacket
size), then it should be enough to invalidate only up to 512 B.
Previously the whole buffer (DMA_BUFFER_SIZE -> 16 KiB) was
invalidated each time.

Since we are invalidating cache content corresponding to buffer mapped
memory, we are safe when D-cache controller invalidates 32B
(CACHE_LINE_SIZE) instead of 16B.

> 
> > > Best regards,
> > > Marek Vasut
> > 
> > Since this is Samsung's UDC specific (not THOR download) - would it
> > be possible to take this patch from this patch series (of course if
> > my above rationale is acceptable for you)?
> 
> You mean for .10 ?

This patch hasn't introduced regressions for DFU/UMS. It can be applied
to .10 or to u-boot-usb/next.

> 
> Best regards,
> Marek Vasut



-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to