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

[Marek Vasut]

Dear Lukasz Majewski,

> Hi Marek,
> 
> > 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 at samsung.com>
> > > > > > > Cc: Marek Vasut <marex at 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.
> 
> I've looked into the v7_dcache_inval_range() function at cache_v7.c
> and .... you are right here.
> Cache doesn't touch the last and fist line if they aren't properly
> aligned. Obviously I need ROUND(CACHE_LINE_SIZE) there.
> 
> Thanks for opening my eyes....
> 
> > 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
> 
> I will prepare v2 with rounding here.

Thanks ;-)