RE: [PATCH][v4]fsl/usb:Add controller version based ULPI and UTMI phy support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----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�����٥



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux