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]

 



>> >> 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.

It's ok to add it in the USB driver domain.  But we should not add more architecture/platform specific code in the core file as we all agreed in previous discussion.  How about add a new file like fsl_mxc_udc.c did?

- Leo
��.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