On Wed, Jan 16, 2013 at 09:48:25AM +0800, Peter Chen wrote:
> 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.
right :-)
> Thanks Shawn. Let's see Felipe's comment, nevertheless, I will send v6 patch
> due to a compile error at mx25
Shawn is right.
--
balbi
Attachment:
signature.asc
Description: Digital signature
