Re: [linux-usb-devel] [PATCH] UDC support for MIPS/AU1200 andGeode/CS5536

[Thomas Dahlmann]

Oliver Neukum wrote:

> Am Dienstag, 10. Januar 2006 12:02 schrieb Thomas Dahlmann:
>  
>
> > Oliver Neukum wrote:
> >
> >    
> >
> > > Am Montag, 9. Januar 2006 19:03 schrieb Jordan Crouse:
> > >
> > >
> > >      
> > >
> > > > > From the "two-birds-one-stone" department, I am pleased to present USB UDC
> > > > support for both the MIPS Au1200 SoC and the Geode CS5535 south bridge.  
> > > > Also, coming soon (in the next few days), OTG, which has been removed from
> > > > the usb_host patch, and put into its own patch (as per David's comments).
> > > >
> > > > This patch is against current linux-mips git, but it should apply for Linus's
> > > > tree as well.
> > > >
> > > > Regards,
> > > > Jordan
> > > >
> > > >   
> > > >
> > > >        
> > > +        VDBG("udc_read_bytes(): %d bytes\n", bytes);
> > > +
> > > +        /* dwords first */
> > > +        for (i = 0; i < bytes / UDC_DWORD_BYTES; i++) {
> > > +               *((u32*) (buf + (i<<2))) = readl(dev->rxfifo); 
> > > +        }
> > >
> > > Is there any reason you don't increment by 4?
> > >
> > > 	Regards
> > > 		Oliver
> > >
> > >
> > >
> > >
> > >
> > >      
> > The loop is for reading dwords only, so "i < bytes / UDC_DWORD_BYTES" cuts
> > off remaining 1,2 or 3 bytes which are handled by the next loop.
> > But you are right, incrementing by 4 may look better,  as
> >
> >        for (i = 0; i < bytes - bytes % UDC_DWORD_BYTES; i+=4) {
> >               *((u32*) (buf + i)) = readl(dev->rxfifo); 
> >        }
> >    
>
> Not only will it look better, but it'll save you a shift operation.
> You might even compute start and finish values before the loop and
> save an addition in the body.
>
> 	Regards
> 		Oliver
>
>
>
>  
I have changed this for the next patch release. Thanks for the input !

Regards,
Thomas


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/