On Thursday 23 February 2012 09:25:25 Puneet Saxena wrote:
> --- a/common/usb_storage.c
> +++ b/common/usb_storage.c
> 
> -static unsigned char usb_stor_buf[512];
> -static ccb usb_ccb;
> +#ifdef ARCH_DMA_MINALIGN
> +     static ccb usb_ccb __attribute__((aligned(ARCH_DMA_MINALIGN)));
> +#else
> +     static ccb usb_ccb;
> +#endif

don't use ifdef's.  you may assume that ARCH_DMA_MINALIGN is always defined.  
after all, the ALLOC_CACHE_ALIGN_BUFFER() helper does.

> int usb_stor_get_info(struct usb_device *dev, struct us_data *ss,
>                               block_dev_desc_t *dev_desc)
>  {
>       unsigned char perq, modi;
> -     unsigned long cap[2];
> +     ALLOC_CACHE_ALIGN_BUFFER(unsigned long, cap, 2);
> +     ALLOC_CACHE_ALIGN_BUFFER(unsigned char, usb_stor_buf, 36);
>       unsigned long *capacity, *blksz;
>       ccb *pccb = &usb_ccb;
> 
> +     /* GJ */
> +     memset(usb_stor_buf, 0, sizeof(usb_stor_buf));

you shrunk the buffer to 36 bytes, so that's good.  but the memset() should be 
dropped.  see what i posted to Marek when he tried moving this buffer locally 
if you want background.

> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
>
>  static void flush_invalidate(u32 addr, int size, int flush)
>  {
> +     /*
> +      * Size is the bytes actually moved during transaction,
> +      * which may not equal to the cache line. This results
> +      * stop address passed for invalidating cache may not be aligned.
> +      * Therfore making size as nultiple of cache line size.
> +      */
> +     if (size & (ARCH_DMA_MINALIGN - 1))
> +                     size = ((size / ARCH_DMA_MINALIGN) + 1)
> +                             * ARCH_DMA_MINALIGN;

i don't think this logic should exist at all.  the arch is responsible for 
flushing enough of its cache to cover the requested size.

> --- a/include/scsi.h
> +++ b/include/scsi.h
> 
>  typedef struct SCSI_cmd_block{
>       unsigned char           cmd[16];                        /* command */
> -     unsigned char           sense_buf[64];          /* for request sense */
> +     /* for request sense */
> +#ifdef ARCH_DMA_MINALIGN
> +     unsigned char           sense_buf[64]
> +             __attribute__((aligned(ARCH_DMA_MINALIGN)));
> +#else
> +     unsigned char           sense_buf[64];
> +#endif
>
> --- a/include/usb.h
> +++ b/include/usb.h
> struct usb_device {
>       int epmaxpacketout[16];         /* OUTput endpoint specific maximums */
> 
>       int configno;                   /* selected config number */
> -     struct usb_device_descriptor descriptor; /* Device Descriptor */
> +      /* Device Descriptor */
> +#ifdef ARCH_DMA_MINALIGN
> +     struct usb_device_descriptor descriptor
> +             __attribute__((aligned(ARCH_DMA_MINALIGN)));
> +#else
> +     struct usb_device_descriptor descriptor;
> +#endif

i don't think either of these headers should be changed.  find & fix the code 
that is causing a problem.

wrt the other random ALLOC_CACHE_ALIGN_BUFFER() changes, they look OK to me.
-mike

Attachment: signature.asc
Description: This is a digitally signed message part.

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to