[U-Boot] [PATCH 01/10] usb:udc:s3c: Reduce dcache invalidate range for UDC receive buffer

[Marek Vasut <marex@denx.de>]

Dear Lukasz Majewski,

> 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.majewski@samsung.com>
> > > > > Cc: Marek Vasut <marex@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.

I get it, but if the size is lower than CACHE_LINE_SIZE, it will not get 
invalidated at all. Thus the roundup might be needed.
 
> > > > 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.

If it's not an explicit fix, this will go to -next

Best regards,