> -----Original Message----- > From: Leo Li [mailto:pku.leo@xxxxxxxxx] > Sent: Thursday, February 16, 2012 3:46 PM > To: Mehresh Ramneek-B31383 > Cc: Chen Peter-B29397; linux-usb@xxxxxxxxxxxxxxx; Li Yang-R58472 > Subject: Re: [PATCH][v4]fsl/usb:Add controller version based ULPI and > UTMI phy support > > 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 Hi Leo, thanks for your input. Similar code was in arch/powerpc/.../fsl_soc.c file for 85xx platforms. Later the code was moved to the driver. I reckon the reason was - this code is configuring usb controller (for a specific phy). Since the code is configuring the controller, the code was moved to the controller driver. Regards, Ramneek ��.n��������+%������w��{.n�����{���)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥