On Thu, Feb 16, 2012 at 4:53 PM, Mehresh Ramneek-B31383 <B31383@xxxxxxxxxxxxx> wrote: > > >> -----Original Message----- >> From: Chen Peter-B29397 >> Sent: Thursday, February 16, 2012 2:16 PM >> To: Mehresh Ramneek-B31383 >> Cc: linux-usb@xxxxxxxxxxxxxxx; Li Yang-R58472 >> Subject: RE: [PATCH][v4]fsl/usb:Add controller version based ULPI and >> UTMI phy support >> >> >> >> >> > > > >> > > > Top commit >> > > > commit 5407a3c3d942e75d4d123d213fd692bce5acc961 >> > > > Author: Felipe Balbi <balbi@xxxxxx> >> > > > Date: Wed Feb 15 09:34:26 2012 +0200 >> > > > >> > > > Hi Peter, could you please help me test this patch in IMX platform >> > > It causes compile error at i.mx platform due to struct >> > usb_sys_interface >> > > *usb_sys_regs is only defined at PowerPC platform. >> > > Below is the error message: >> > > >> > > /home/b29397/work/code/git/linus/linux- >> > > 2.6/drivers/usb/gadget/fsl_udc_core.c: In function >> > 'dr_controller_setup': >> > > /home/b29397/work/code/git/linus/linux- >> > > 2.6/drivers/usb/gadget/fsl_udc_core.c:269: error: 'ctrl' undeclared >> > > (first use in this function) >> > > /home/b29397/work/code/git/linus/linux- >> > > 2.6/drivers/usb/gadget/fsl_udc_core.c:269: error: (Each undeclared >> > > identifier is reported only once >> > > /home/b29397/work/code/git/linus/linux- >> > > 2.6/drivers/usb/gadget/fsl_udc_core.c:269: error: for each function >> > > it appears in.) >> > > /home/b29397/work/code/git/linus/linux- >> > > 2.6/drivers/usb/gadget/fsl_udc_core.c:269: error: 'usb_sys_regs' >> > > undeclared (first use in this function) >> > > make[4]: *** [drivers/usb/gadget/fsl_udc_core.o] Error 1 >> > > make[3]: *** [drivers/usb/gadget] Error 2 >> > > >> > These will come since there are IMX SOC based macros which are >> > excluding ctrl and usb_sys_regs for IMX platforms with CONFIG_ARCH_MXC >> > macro >> > >> > The only way to resolve this is that USB controller version of IMX can >> > be added in some IMX configuration/platform file, and CONFIG_ARCH_MXC >> > is removed from USB gadget driver file...we should refrain from >> > including SOC based macros in IP driver files >> > >> > Can you or someone from IMX team support this...I'm not too familiar >> > with IMX platform >> > >> >> The current problem is usb_sys_regs has not defined at i.mx platform, not >> related your USB controller version code. >> Temp way: >> Use pdata->have_sysif_regs to judge where the usb_sys_regs can be >> accessed, and keep as much as access usb_sys_regs code to a function to >> keep code clean. >> Long way: >> Does usb_sys_regs are related to PHY? Move all PHY related operation out >> of controller file, as PHY is platform related. In fact, i.mx has similar >> PHY registers, currently, we put it at arch code, but it needs to move to >> PHY driver in future. >> >> >> BR, >> Peter >> > First of all I do not understand why CONFIG_ARCH_MXC macro is being used inside FSL USB > Gadget driver file ? > > There is an active effort going on in community at various levels to remove SOC based MACROS from IP driver > files.....Why are we not talking about someone cleaning IMX macro from this driver file ? > > Instead of using CONFIG_ARCH_MXC macro, usb_get_ver_info() can be used to shield this code for IMX > based platforms...as discussed previously, this function returns 0 for IMX platforms... Arguments from both of you makes sense. The platform #ifdef's are introduced when merging the MPC/ARM/Coldfire drivers as a temporary workaround. Ideally we need to remove both the architecture MACROs and the system interface register from the common part of the driver. Put all the architecture specific code in files like fsl_mxc_udc.c or somewhere in arch/powerpc/. Ramneek, Actually, the issue you wanted to address with this patch is very similar to the situation we had on the MPC83xx platforms. We put the code under arch/powerpc/platforms/83xx/usb.c. You can take that as a reference also. - Leo -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html