Hi, 2010/12/21 Ming Lei <tom.leiming@xxxxxxxxx>: > Hi, > > 2010/12/21 Felipe Balbi <balbi@xxxxxx>: >> Hi, >> >> On Tue, Dec 21, 2010 at 07:27:10PM +0800, Ming Lei wrote: >>> >>> OK, I will do a performance comparison between inline and function >>> pointers. >> >> The comparisson should be between your version and struct musb_io_ops >> version. > > OK, I will do it. 1, test with musb cleanup v1 patches against 2.6.37-rc6-next-20101217 1), test case(running on host machine and musb working at gadget zero mode): $sudo ./tu -D /proc/bus/usb/BUS/DEV -c 2048 -t 5 -s 32768 -g 8 2), result: - cpu utilization: 22%~23%(observed by top) - performance: 17MB/sec 3), code size [tom@xxxxxxxxxxxxxx]$ size ../nfs/t/lib/modules/2.6.37-rc6-next-20101217+/kernel/drivers/usb/musb/*.ko text data bss dec hex filename 6782 488 56 7326 1c9e drivers/usb/musb/am35x.ko 63799 948 28 64775 fd07 drivers/usb/musb/musb_hdrc.ko 3744 328 0 4072 fe8 kernel/drivers/usb/musb/musbhsdma.ko 6708 488 56 7252 1c54 drivers/usb/musb/omap2430.ko 11031 416 72 11519 2cff drivers/usb/musb/tusb6010.ko 6476 328 20 6824 1aa8 drivers/usb/musb/tusb6010_omap.ko 2, test with attachment patch against musb cleanup v1 patches plus 2.6.37-rc6-next-20101217 1), test case(running on host machine and musb working at gadget zero mode): $sudo ./tu -D /proc/bus/usb/BUS/DEV -c 2048 -t 5 -s 32768 -g 8 2), result: - cpu utilization: 23%~24%(observed by top) - performance: 17MB/sec 3), code size [tom@xxxxxxxxxxxxxx]$ size ../nfs/t/lib/modules/2.6.37-rc6-next-20101217+/kernel/drivers/usb/musb/*.ko text data bss dec hex filename 7234 488 56 7778 1e62 drivers/usb/musb/am35x.ko 67329 948 44 68321 10ae1 drivers/usb/musb/musb_hdrc.ko 4296 328 0 4624 1210 drivers/usb/musb/musbhsdma.ko 7324 488 56 7868 1ebc drivers/usb/musb/omap2430.ko 12783 416 72 13271 33d7 drivers/usb/musb/tusb6010.ko 7236 328 20 7584 1da0 drivers/usb/musb/tusb6010_omap.ko 3, conclusion - almost same performance wrt. usbtest #5 - a little higher cpu utilization if functions pointers to musb_read{w,l} and musb_write{w,l} are taken, but the difference is not very obvious - there is obvious difference in code size of modules, functions pointers introduce larger code size both for caller and for callee Seems we should avoid to use function pointer to musb_read{w,l} and musb_write{w,l} as far as possible from above. Any suggestions or objections? BTW: - Attachment patch is only for test purpose; - DMA mode 0 is used in musb gadget OUT dir, which may consume much more cpu than mode 1, so usbtest #5 is taken to evaluate the effect of function pointers to musb_read{w,l} and musb_write{w,l}. thanks, -- Lei Ming
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index e65d2a3..c260887 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -129,6 +129,14 @@ EXPORT_SYMBOL(musb_readb); void (*musb_writeb)(void __iomem *addr, unsigned offset, u8 data); EXPORT_SYMBOL(musb_writeb); +u16 (*musb_readw)(const void __iomem *addr, unsigned offset); +u32 (*musb_readl)(const void __iomem *addr, unsigned offset); +void (*musb_writew)(void __iomem *addr, unsigned offset, u16 data); +void (*musb_writel)(void __iomem *addr, unsigned offset, u32 data); +EXPORT_SYMBOL(musb_readw); +EXPORT_SYMBOL(musb_writew); +EXPORT_SYMBOL(musb_readl); +EXPORT_SYMBOL(musb_writel); /*-------------------------------------------------------------------------*/ static inline struct musb *dev_to_musb(struct device *dev) @@ -2032,6 +2040,10 @@ bad_config: musb_readb = __musb_readb; musb_writeb = __musb_writeb; } + musb_readw = __musb_readw; + musb_writew = __musb_writew; + musb_readl = __musb_readl; + musb_writel = __musb_writel; dev_info(dev, "dma type: %s\n", get_dma_name(musb)); diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h index 78725df..9e8c540 100644 --- a/drivers/usb/musb/musb_core.h +++ b/drivers/usb/musb/musb_core.h @@ -64,6 +64,10 @@ struct musb_ep; extern u8 (*musb_readb)(const void __iomem *addr, unsigned offset); extern void (*musb_writeb)(void __iomem *addr, unsigned offset, u8 data); +extern u16 (*musb_readw)(const void __iomem *addr, unsigned offset); +extern u32 (*musb_readl)(const void __iomem *addr, unsigned offset); +extern void (*musb_writew)(void __iomem *addr, unsigned offset, u16 data); +extern void (*musb_writel)(void __iomem *addr, unsigned offset, u32 data); #include "musb_debug.h" #include "musb_dma.h" diff --git a/drivers/usb/musb/musb_io.h b/drivers/usb/musb/musb_io.h index dced1c6..f68d52f 100644 --- a/drivers/usb/musb/musb_io.h +++ b/drivers/usb/musb/musb_io.h @@ -60,31 +60,31 @@ static inline void writesb(const void __iomem *addr, const void *buf, int len) /* NOTE: these offsets are all in bytes */ -static inline u16 musb_readw(const void __iomem *addr, unsigned offset) +static inline u16 __musb_readw(const void __iomem *addr, unsigned offset) { return __raw_readw(addr + offset); } -static inline u32 musb_readl(const void __iomem *addr, unsigned offset) +static inline u32 __musb_readl(const void __iomem *addr, unsigned offset) { return __raw_readl(addr + offset); } -static inline void musb_writew(void __iomem *addr, unsigned offset, u16 data) +static inline void __musb_writew(void __iomem *addr, unsigned offset, u16 data) { __raw_writew(data, addr + offset); } -static inline void musb_writel(void __iomem *addr, unsigned offset, u32 data) +static inline void __musb_writel(void __iomem *addr, unsigned offset, u32 data) { __raw_writel(data, addr + offset); } #else -static inline u16 musb_readw(const void __iomem *addr, unsigned offset) +static inline u16 __musb_readw(const void __iomem *addr, unsigned offset) { return bfin_read16(addr + offset); } -static inline u32 musb_readl(const void __iomem *addr, unsigned offset) +static inline u32 __musb_readl(const void __iomem *addr, unsigned offset) { return (u32) (bfin_read16(addr + offset)); } -static inline void musb_writew(void __iomem *addr, unsigned offset, u16 data) +static inline void __musb_writew(void __iomem *addr, unsigned offset, u16 data) { bfin_write16(addr + offset, data); } -static inline void musb_writel(void __iomem *addr, unsigned offset, u32 data) +static inline void __musb_writel(void __iomem *addr, unsigned offset, u32 data) { bfin_write16(addr + offset, (u16) data); } #endif /* CONFIG_BLACKFIN */