On Tue, Jan 15, 2013 at 10:03:46PM +0800, Shawn Guo wrote:
> On Tue, Jan 15, 2013 at 10:29:33AM +0800, Peter Chen wrote:
> > As mach/hardware.h is deleted, we need to use platform_device_id to
> > differentiate SoCs. Besides, one cpu_is_mx35 is useless as it has
> > already used pdata to differentiate runtime
> >
> > Meanwhile we update the platform code accordingly.
> >
> > Signed-off-by: Peter Chen <peter.chen@xxxxxxxxxxxxx>
> > ---
> > arch/arm/mach-imx/devices/devices-common.h | 1 +
> > arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c | 15 ++++---
> > drivers/usb/gadget/fsl_mxc_udc.c | 24 +++++-------
> > drivers/usb/gadget/fsl_udc_core.c | 42 +++++++++++++--------
> > 4 files changed, 45 insertions(+), 37 deletions(-)
>
> Since we are splitting the original patch anyway, it's a bit strange
> to me that you are mixing arch/arm/mach-imx and drivers/usb/gadget
> in this patch. I'm fine with it, since I assume all the patches to
> go via USB tree anyway.
>
> >
> > diff --git a/arch/arm/mach-imx/devices/devices-common.h b/arch/arm/mach-imx/devices/devices-common.h
> > index 6277baf..9bd5777 100644
> > --- a/arch/arm/mach-imx/devices/devices-common.h
> > +++ b/arch/arm/mach-imx/devices/devices-common.h
> > @@ -63,6 +63,7 @@ struct platform_device *__init imx_add_flexcan(
> >
> > #include <linux/fsl_devices.h>
> > struct imx_fsl_usb2_udc_data {
> > + const char *devid;
> > resource_size_t iobase;
> > resource_size_t irq;
> > };
> > diff --git a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
> > index 37e4439..fb527c7 100644
> > --- a/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
> > +++ b/arch/arm/mach-imx/devices/platform-fsl-usb2-udc.c
> > @@ -11,35 +11,36 @@
> > #include "../hardware.h"
> > #include "devices-common.h"
> >
> > -#define imx_fsl_usb2_udc_data_entry_single(soc) \
> > +#define imx_fsl_usb2_udc_data_entry_single(soc, _devid) \
> > { \
> > + .devid = _devid, \
> > .iobase = soc ## _USB_OTG_BASE_ADDR, \
> > .irq = soc ## _INT_USB_OTG, \
> > }
> >
> > #ifdef CONFIG_SOC_IMX25
> > const struct imx_fsl_usb2_udc_data imx25_fsl_usb2_udc_data __initconst =
> > - imx_fsl_usb2_udc_data_entry_single(MX25);
> > + imx_fsl_usb2_udc_data_entry_single(MX25, "imx-udc-mx25");
> > #endif /* ifdef CONFIG_SOC_IMX25 */
> >
> > #ifdef CONFIG_SOC_IMX27
> > const struct imx_fsl_usb2_udc_data imx27_fsl_usb2_udc_data __initconst =
> > - imx_fsl_usb2_udc_data_entry_single(MX27);
> > + imx_fsl_usb2_udc_data_entry_single(MX27, "imx-udc-mx27");
> > #endif /* ifdef CONFIG_SOC_IMX27 */
> >
> > #ifdef CONFIG_SOC_IMX31
> > const struct imx_fsl_usb2_udc_data imx31_fsl_usb2_udc_data __initconst =
> > - imx_fsl_usb2_udc_data_entry_single(MX31);
> > + imx_fsl_usb2_udc_data_entry_single(MX31, "imx-udc-mx31");
> > #endif /* ifdef CONFIG_SOC_IMX31 */
> >
> > #ifdef CONFIG_SOC_IMX35
> > const struct imx_fsl_usb2_udc_data imx35_fsl_usb2_udc_data __initconst =
> > - imx_fsl_usb2_udc_data_entry_single(MX35);
> > + imx_fsl_usb2_udc_data_entry_single(MX35, "imx-udc-mx35");
> > #endif /* ifdef CONFIG_SOC_IMX35 */
> >
> > #ifdef CONFIG_SOC_IMX51
> > const struct imx_fsl_usb2_udc_data imx51_fsl_usb2_udc_data __initconst =
> > - imx_fsl_usb2_udc_data_entry_single(MX51);
> > + imx_fsl_usb2_udc_data_entry_single(MX51, "imx-udc-mx51");
> > #endif
> >
> > struct platform_device *__init imx_add_fsl_usb2_udc(
> > @@ -57,7 +58,7 @@ struct platform_device *__init imx_add_fsl_usb2_udc(
> > .flags = IORESOURCE_IRQ,
> > },
> > };
> > - return imx_add_platform_device_dmamask("fsl-usb2-udc", -1,
> > + return imx_add_platform_device_dmamask(data->devid, -1,
> > res, ARRAY_SIZE(res),
> > pdata, sizeof(*pdata), DMA_BIT_MASK(32));
> > }
>
> <snip>
>
> > +static const struct platform_device_id fsl_udc_devtype[] = {
> > + {
> > + .name = "imx-udc-mx25",
> > + }, {
> > + .name = "imx-udc-mx27",
> > + }, {
> > + .name = "imx-udc-mx31",
> > + }, {
> > + .name = "imx-udc-mx35",
> > + }, {
> > + .name = "imx-udc-mx51",
> > + }
> > +};
>
> From what I understand balbi's comment, he dislikes this full list of
> device id. Instead, he prefers to something like below.
>
> static const struct platform_device_id fsl_udc_devtype[] = {
> {
> .name = "imx-udc-mx27",
> }, {
> .name = "imx-udc-mx51",
> }
> };
>
> It basically tells that we are handling two type of devices here, one
> is imx-udc-mx27 type and the other is imx-udc-mx51 type, with mx25/31/35
> completely compatible with mx27 type. We choose mx27 instead of mx25
> to define the type because mx27 Si came out earlier than mx25. That
> said, we generally choose the earlies SoC name to define a particular
> version of IP block, since hardware version is mostly unavailable or
> unreliable.
>
> But that also means in platform code which create the platform_device,
> you will need to use name "imx-udc-mx27" for even mx25/31/35.
>
> imx_fsl_usb2_udc_data_entry_single(MX25, "imx-udc-mx27");
> imx_fsl_usb2_udc_data_entry_single(MX31, "imx-udc-mx27");
> imx_fsl_usb2_udc_data_entry_single(MX35, "imx-udc-mx27");
>
> Considering this is a piece of code we will not use for any new
> hardware, I'm fine with either way.
>
> So, balbi, it's all your call to accept the series as it is or ask for
> another iteration.
Thanks Shawn. Let's see Felipe's comment, nevertheless, I will send v6 patch
due to a compile error at mx25
>
> Shawn
--
Best Regards,
Peter Chen
--
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
