linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/3] Fix the Build error for fsl_mxc_udc.c
@ 2013-01-15  2:29 Peter Chen
  2013-01-15  2:29 ` [PATCH v5 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id Peter Chen
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Peter Chen @ 2013-01-15  2:29 UTC (permalink / raw)
  To: shawn.guo, balbi, kernel, gregkh, r58472
  Cc: linux-usb, linuxppc-dev, linux-arm-kernel

Changes for v5:
- Using strcmp to get specific SoC
- Delete one cpu_is_mx35() as it has already pdata runtime check

Changes for v4:
- Using pdev's struct resource to do ioremap
- Add ioremap return value check

Changes for v3:
- Split the one big patch into three patches

Changes for v2:
- Add const for fsl_udc_devtype
- Do ioremap for phy address at fsl-mxc-udc

Peter Chen (3):
  usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
  usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
  ARM: i.MX clock: Change the connection-id for fsl-usb2-udc

 arch/arm/mach-imx/clk-imx25.c                     |    6 +-
 arch/arm/mach-imx/clk-imx27.c                     |    6 +-
 arch/arm/mach-imx/clk-imx31.c                     |    6 +-
 arch/arm/mach-imx/clk-imx35.c                     |    6 +-
 arch/arm/mach-imx/clk-imx51-imx53.c               |    6 +-
 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                  |   40 ++++++++++++------
 drivers/usb/gadget/fsl_udc_core.c                 |   46 +++++++++++++--------
 drivers/usb/gadget/fsl_usb2_udc.h                 |    5 +-
 10 files changed, 82 insertions(+), 55 deletions(-)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v5 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
  2013-01-15  2:29 [PATCH v5 0/3] Fix the Build error for fsl_mxc_udc.c Peter Chen
@ 2013-01-15  2:29 ` Peter Chen
  2013-01-15 14:03   ` Shawn Guo
  2013-01-15  2:29 ` [PATCH v5 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap Peter Chen
  2013-01-15  2:29 ` [PATCH v5 3/3] ARM: i.MX clock: Change the connection-id for fsl-usb2-udc Peter Chen
  2 siblings, 1 reply; 9+ messages in thread
From: Peter Chen @ 2013-01-15  2:29 UTC (permalink / raw)
  To: shawn.guo, balbi, kernel, gregkh, r58472
  Cc: linux-usb, linuxppc-dev, linux-arm-kernel

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@freescale.com>
---
 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(-)

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));
 }
diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index 1b0f086..1176bd8 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -18,8 +18,6 @@
 #include <linux/platform_device.h>
 #include <linux/io.h>
 
-#include <mach/hardware.h>
-
 static struct clk *mxc_ahb_clk;
 static struct clk *mxc_per_clk;
 static struct clk *mxc_ipg_clk;
@@ -59,7 +57,7 @@ int fsl_udc_clk_init(struct platform_device *pdev)
 	clk_prepare_enable(mxc_per_clk);
 
 	/* make sure USB_CLK is running at 60 MHz +/- 1000 Hz */
-	if (!cpu_is_mx51()) {
+	if (strcmp(pdev->id_entry->name, "imx-udc-mx51")) {
 		freq = clk_get_rate(mxc_per_clk);
 		if (pdata->phy_mode != FSL_USB2_PHY_ULPI &&
 		    (freq < 59999000 || freq > 60001000)) {
@@ -82,17 +80,15 @@ eclkrate:
 void fsl_udc_clk_finalize(struct platform_device *pdev)
 {
 	struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
-	if (cpu_is_mx35()) {
-		unsigned int v;
-
-		/* workaround ENGcm09152 for i.MX35 */
-		if (pdata->workaround & FLS_USB2_WORKAROUND_ENGCM09152) {
-			v = readl(MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
-					USBPHYCTRL_OTGBASE_OFFSET));
-			writel(v | USBPHYCTRL_EVDO,
-				MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
-					USBPHYCTRL_OTGBASE_OFFSET));
-		}
+	unsigned int v;
+
+	/* workaround ENGcm09152 for i.MX35 */
+	if (pdata->workaround & FLS_USB2_WORKAROUND_ENGCM09152) {
+		v = readl(MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
+				USBPHYCTRL_OTGBASE_OFFSET));
+		writel(v | USBPHYCTRL_EVDO,
+			MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
+				USBPHYCTRL_OTGBASE_OFFSET));
 	}
 
 	/* ULPI transceivers don't need usbpll */
diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c
index c19f7f1..c971e84 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -41,6 +41,7 @@
 #include <linux/fsl_devices.h>
 #include <linux/dmapool.h>
 #include <linux/delay.h>
+#include <linux/of_device.h>
 
 #include <asm/byteorder.h>
 #include <asm/io.h>
@@ -2438,11 +2439,6 @@ static int __init fsl_udc_probe(struct platform_device *pdev)
 	unsigned int i;
 	u32 dccparams;
 
-	if (strcmp(pdev->name, driver_name)) {
-		VDBG("Wrong device");
-		return -ENODEV;
-	}
-
 	udc_controller = kzalloc(sizeof(struct fsl_udc), GFP_KERNEL);
 	if (udc_controller == NULL) {
 		ERR("malloc udc failed\n");
@@ -2756,22 +2752,36 @@ static int fsl_udc_otg_resume(struct device *dev)
 
 	return fsl_udc_resume(NULL);
 }
-
 /*-------------------------------------------------------------------------
 	Register entry point for the peripheral controller driver
 --------------------------------------------------------------------------*/
-
+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",
+	}
+};
+MODULE_DEVICE_TABLE(platform, fsl_udc_devtype);
 static struct platform_driver udc_driver = {
-	.remove  = __exit_p(fsl_udc_remove),
+	.remove		= __exit_p(fsl_udc_remove),
+	/* Just for FSL i.mx SoC currently */
+	.id_table	= fsl_udc_devtype,
 	/* these suspend and resume are not usb suspend and resume */
-	.suspend = fsl_udc_suspend,
-	.resume  = fsl_udc_resume,
-	.driver  = {
-		.name = (char *)driver_name,
-		.owner = THIS_MODULE,
-		/* udc suspend/resume called from OTG driver */
-		.suspend = fsl_udc_otg_suspend,
-		.resume  = fsl_udc_otg_resume,
+	.suspend	= fsl_udc_suspend,
+	.resume		= fsl_udc_resume,
+	.driver		= {
+			.name = (char *)driver_name,
+			.owner = THIS_MODULE,
+			/* udc suspend/resume called from OTG driver */
+			.suspend = fsl_udc_otg_suspend,
+			.resume  = fsl_udc_otg_resume,
 	},
 };
 
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v5 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
  2013-01-15  2:29 [PATCH v5 0/3] Fix the Build error for fsl_mxc_udc.c Peter Chen
  2013-01-15  2:29 ` [PATCH v5 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id Peter Chen
@ 2013-01-15  2:29 ` Peter Chen
  2013-01-15  2:29 ` [PATCH v5 3/3] ARM: i.MX clock: Change the connection-id for fsl-usb2-udc Peter Chen
  2 siblings, 0 replies; 9+ messages in thread
From: Peter Chen @ 2013-01-15  2:29 UTC (permalink / raw)
  To: shawn.guo, balbi, kernel, gregkh, r58472
  Cc: linux-usb, linuxppc-dev, linux-arm-kernel

As mach/hardware.h is deleted, we can't visit platform code at driver.
It has no phy driver to combine with this controller, so it has to use
ioremap to map phy address as a workaround.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 drivers/usb/gadget/fsl_mxc_udc.c  |   30 +++++++++++++++++++++++-------
 drivers/usb/gadget/fsl_udc_core.c |    4 +++-
 drivers/usb/gadget/fsl_usb2_udc.h |    5 +++--
 3 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index 1176bd8..bb65c46 100644
--- a/drivers/usb/gadget/fsl_mxc_udc.c
+++ b/drivers/usb/gadget/fsl_mxc_udc.c
@@ -23,7 +23,8 @@ static struct clk *mxc_per_clk;
 static struct clk *mxc_ipg_clk;
 
 /* workaround ENGcm09152 for i.MX35 */
-#define USBPHYCTRL_OTGBASE_OFFSET	0x608
+#define MX35_USBPHYCTRL_OFFSET		0x600
+#define USBPHYCTRL_OTGBASE_OFFSET	0x8
 #define USBPHYCTRL_EVDO			(1 << 23)
 
 int fsl_udc_clk_init(struct platform_device *pdev)
@@ -77,25 +78,40 @@ eclkrate:
 	return ret;
 }
 
-void fsl_udc_clk_finalize(struct platform_device *pdev)
+int fsl_udc_clk_finalize(struct platform_device *pdev)
 {
 	struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
-	unsigned int v;
+	int ret = 0;
 
 	/* workaround ENGcm09152 for i.MX35 */
 	if (pdata->workaround & FLS_USB2_WORKAROUND_ENGCM09152) {
-		v = readl(MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
-				USBPHYCTRL_OTGBASE_OFFSET));
+		unsigned int v;
+		struct resource *res = platform_get_resource
+			(pdev, IORESOURCE_MEM, 0);
+		void __iomem *phy_regs = ioremap(res->start +
+						MX35_USBPHYCTRL_OFFSET, 512);
+		if (!phy_regs) {
+			dev_err(&pdev->dev, "ioremap for phy address fails\n");
+			ret = -EINVAL;
+			goto ioremap_err;
+		}
+
+		v = readl(phy_regs + USBPHYCTRL_OTGBASE_OFFSET);
 		writel(v | USBPHYCTRL_EVDO,
-			MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
-				USBPHYCTRL_OTGBASE_OFFSET));
+			phy_regs + USBPHYCTRL_OTGBASE_OFFSET);
+
+		iounmap(phy_regs);
 	}
 
+
+ioremap_err:
 	/* ULPI transceivers don't need usbpll */
 	if (pdata->phy_mode == FSL_USB2_PHY_ULPI) {
 		clk_disable_unprepare(mxc_per_clk);
 		mxc_per_clk = NULL;
 	}
+
+	return ret;
 }
 
 void fsl_udc_clk_release(void)
diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c
index c971e84..347b1ed 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -2543,7 +2543,9 @@ static int __init fsl_udc_probe(struct platform_device *pdev)
 		dr_controller_setup(udc_controller);
 	}
 
-	fsl_udc_clk_finalize(pdev);
+	ret = fsl_udc_clk_finalize(pdev);
+	if (ret)
+		goto err_free_irq;
 
 	/* Setup gadget structure */
 	udc_controller->gadget.ops = &fsl_gadget_ops;
diff --git a/drivers/usb/gadget/fsl_usb2_udc.h b/drivers/usb/gadget/fsl_usb2_udc.h
index f61a967..c6703bb 100644
--- a/drivers/usb/gadget/fsl_usb2_udc.h
+++ b/drivers/usb/gadget/fsl_usb2_udc.h
@@ -592,15 +592,16 @@ static inline struct ep_queue_head *get_qh_by_ep(struct fsl_ep *ep)
 struct platform_device;
 #ifdef CONFIG_ARCH_MXC
 int fsl_udc_clk_init(struct platform_device *pdev);
-void fsl_udc_clk_finalize(struct platform_device *pdev);
+int fsl_udc_clk_finalize(struct platform_device *pdev);
 void fsl_udc_clk_release(void);
 #else
 static inline int fsl_udc_clk_init(struct platform_device *pdev)
 {
 	return 0;
 }
-static inline void fsl_udc_clk_finalize(struct platform_device *pdev)
+static inline int fsl_udc_clk_finalize(struct platform_device *pdev)
 {
+	return 0;
 }
 static inline void fsl_udc_clk_release(void)
 {
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v5 3/3] ARM: i.MX clock: Change the connection-id for fsl-usb2-udc
  2013-01-15  2:29 [PATCH v5 0/3] Fix the Build error for fsl_mxc_udc.c Peter Chen
  2013-01-15  2:29 ` [PATCH v5 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id Peter Chen
  2013-01-15  2:29 ` [PATCH v5 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap Peter Chen
@ 2013-01-15  2:29 ` Peter Chen
  2 siblings, 0 replies; 9+ messages in thread
From: Peter Chen @ 2013-01-15  2:29 UTC (permalink / raw)
  To: shawn.guo, balbi, kernel, gregkh, r58472
  Cc: linux-usb, linuxppc-dev, linux-arm-kernel

As we use platform_device_id for fsl-usb2-udc driver, it needs to
change clk connection-id, or the related devm_clk_get will be failed.

Signed-off-by: Peter Chen <peter.chen@freescale.com>
---
 arch/arm/mach-imx/clk-imx25.c       |    6 +++---
 arch/arm/mach-imx/clk-imx27.c       |    6 +++---
 arch/arm/mach-imx/clk-imx31.c       |    6 +++---
 arch/arm/mach-imx/clk-imx35.c       |    6 +++---
 arch/arm/mach-imx/clk-imx51-imx53.c |    6 +++---
 5 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm/mach-imx/clk-imx25.c b/arch/arm/mach-imx/clk-imx25.c
index b197aa7..67e353d 100644
--- a/arch/arm/mach-imx/clk-imx25.c
+++ b/arch/arm/mach-imx/clk-imx25.c
@@ -254,9 +254,9 @@ int __init mx25_clocks_init(void)
 	clk_register_clkdev(clk[ipg], "ipg", "mxc-ehci.2");
 	clk_register_clkdev(clk[usbotg_ahb], "ahb", "mxc-ehci.2");
 	clk_register_clkdev(clk[usb_div], "per", "mxc-ehci.2");
-	clk_register_clkdev(clk[ipg], "ipg", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usbotg_ahb], "ahb", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usb_div], "per", "fsl-usb2-udc");
+	clk_register_clkdev(clk[ipg], "ipg", "imx-udc-mx25");
+	clk_register_clkdev(clk[usbotg_ahb], "ahb", "imx-udc-mx25");
+	clk_register_clkdev(clk[usb_div], "per", "imx-udc-mx25");
 	clk_register_clkdev(clk[nfc_ipg_per], NULL, "imx25-nand.0");
 	/* i.mx25 has the i.mx35 type cspi */
 	clk_register_clkdev(clk[cspi1_ipg], NULL, "imx35-cspi.0");
diff --git a/arch/arm/mach-imx/clk-imx27.c b/arch/arm/mach-imx/clk-imx27.c
index 4c1d1e4..1ffe3b5 100644
--- a/arch/arm/mach-imx/clk-imx27.c
+++ b/arch/arm/mach-imx/clk-imx27.c
@@ -236,9 +236,9 @@ int __init mx27_clocks_init(unsigned long fref)
 	clk_register_clkdev(clk[lcdc_ahb_gate], "ahb", "imx21-fb.0");
 	clk_register_clkdev(clk[csi_ahb_gate], "ahb", "imx27-camera.0");
 	clk_register_clkdev(clk[per4_gate], "per", "imx27-camera.0");
-	clk_register_clkdev(clk[usb_div], "per", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usb_ipg_gate], "ipg", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usb_ahb_gate], "ahb", "fsl-usb2-udc");
+	clk_register_clkdev(clk[usb_div], "per", "imx-udc-mx27");
+	clk_register_clkdev(clk[usb_ipg_gate], "ipg", "imx-udc-mx27");
+	clk_register_clkdev(clk[usb_ahb_gate], "ahb", "imx-udc-mx27");
 	clk_register_clkdev(clk[usb_div], "per", "mxc-ehci.0");
 	clk_register_clkdev(clk[usb_ipg_gate], "ipg", "mxc-ehci.0");
 	clk_register_clkdev(clk[usb_ahb_gate], "ahb", "mxc-ehci.0");
diff --git a/arch/arm/mach-imx/clk-imx31.c b/arch/arm/mach-imx/clk-imx31.c
index 8be64e0..ef66eaf 100644
--- a/arch/arm/mach-imx/clk-imx31.c
+++ b/arch/arm/mach-imx/clk-imx31.c
@@ -139,9 +139,9 @@ int __init mx31_clocks_init(unsigned long fref)
 	clk_register_clkdev(clk[usb_div_post], "per", "mxc-ehci.2");
 	clk_register_clkdev(clk[usb_gate], "ahb", "mxc-ehci.2");
 	clk_register_clkdev(clk[ipg], "ipg", "mxc-ehci.2");
-	clk_register_clkdev(clk[usb_div_post], "per", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usb_gate], "ahb", "fsl-usb2-udc");
-	clk_register_clkdev(clk[ipg], "ipg", "fsl-usb2-udc");
+	clk_register_clkdev(clk[usb_div_post], "per", "imx-udc-mx31");
+	clk_register_clkdev(clk[usb_gate], "ahb", "imx-udc-mx31");
+	clk_register_clkdev(clk[ipg], "ipg", "imx-udc-mx31");
 	clk_register_clkdev(clk[csi_gate], NULL, "mx3-camera.0");
 	/* i.mx31 has the i.mx21 type uart */
 	clk_register_clkdev(clk[uart1_gate], "per", "imx21-uart.0");
diff --git a/arch/arm/mach-imx/clk-imx35.c b/arch/arm/mach-imx/clk-imx35.c
index 66f3d65..69fe9c8 100644
--- a/arch/arm/mach-imx/clk-imx35.c
+++ b/arch/arm/mach-imx/clk-imx35.c
@@ -251,9 +251,9 @@ int __init mx35_clocks_init()
 	clk_register_clkdev(clk[usb_div], "per", "mxc-ehci.2");
 	clk_register_clkdev(clk[ipg], "ipg", "mxc-ehci.2");
 	clk_register_clkdev(clk[usbotg_gate], "ahb", "mxc-ehci.2");
-	clk_register_clkdev(clk[usb_div], "per", "fsl-usb2-udc");
-	clk_register_clkdev(clk[ipg], "ipg", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usbotg_gate], "ahb", "fsl-usb2-udc");
+	clk_register_clkdev(clk[usb_div], "per", "imx-udc-mx35");
+	clk_register_clkdev(clk[ipg], "ipg", "imx-udc-mx35");
+	clk_register_clkdev(clk[usbotg_gate], "ahb", "imx-udc-mx35");
 	clk_register_clkdev(clk[wdog_gate], NULL, "imx2-wdt.0");
 	clk_register_clkdev(clk[nfc_div], NULL, "imx25-nand.0");
 	clk_register_clkdev(clk[csi_gate], NULL, "mx3-camera.0");
diff --git a/arch/arm/mach-imx/clk-imx51-imx53.c b/arch/arm/mach-imx/clk-imx51-imx53.c
index 579023f..fb7cb84 100644
--- a/arch/arm/mach-imx/clk-imx51-imx53.c
+++ b/arch/arm/mach-imx/clk-imx51-imx53.c
@@ -269,9 +269,9 @@ static void __init mx5_clocks_common_init(unsigned long rate_ckil,
 	clk_register_clkdev(clk[usboh3_per_gate], "per", "mxc-ehci.2");
 	clk_register_clkdev(clk[usboh3_gate], "ipg", "mxc-ehci.2");
 	clk_register_clkdev(clk[usboh3_gate], "ahb", "mxc-ehci.2");
-	clk_register_clkdev(clk[usboh3_per_gate], "per", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usboh3_gate], "ipg", "fsl-usb2-udc");
-	clk_register_clkdev(clk[usboh3_gate], "ahb", "fsl-usb2-udc");
+	clk_register_clkdev(clk[usboh3_per_gate], "per", "imx-udc-mx51");
+	clk_register_clkdev(clk[usboh3_gate], "ipg", "imx-udc-mx51");
+	clk_register_clkdev(clk[usboh3_gate], "ahb", "imx-udc-mx51");
 	clk_register_clkdev(clk[nfc_gate], NULL, "imx51-nand");
 	clk_register_clkdev(clk[ssi1_ipg_gate], NULL, "imx-ssi.0");
 	clk_register_clkdev(clk[ssi2_ipg_gate], NULL, "imx-ssi.1");
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v5 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
  2013-01-15  2:29 ` [PATCH v5 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id Peter Chen
@ 2013-01-15 14:03   ` Shawn Guo
  2013-01-16  1:48     ` Peter Chen
  0 siblings, 1 reply; 9+ messages in thread
From: Shawn Guo @ 2013-01-15 14:03 UTC (permalink / raw)
  To: Peter Chen
  Cc: r58472, gregkh, linux-usb, balbi, kernel, linuxppc-dev, linux-arm-kernel

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@freescale.com>
> ---
>  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.

Shawn

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
  2013-01-15 14:03   ` Shawn Guo
@ 2013-01-16  1:48     ` Peter Chen
  2013-01-17  9:14       ` Felipe Balbi
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Chen @ 2013-01-16  1:48 UTC (permalink / raw)
  To: Shawn Guo
  Cc: r58472, gregkh, linux-usb, balbi, kernel, linuxppc-dev, linux-arm-kernel

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@freescale.com>
> > ---
> >  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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
  2013-01-16  1:48     ` Peter Chen
@ 2013-01-17  9:14       ` Felipe Balbi
  2013-01-17  9:28         ` Peter Chen
  0 siblings, 1 reply; 9+ messages in thread
From: Felipe Balbi @ 2013-01-17  9:14 UTC (permalink / raw)
  To: Peter Chen
  Cc: r58472, gregkh, linux-usb, balbi, kernel, Shawn Guo,
	linuxppc-dev, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 6058 bytes --]

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@freescale.com>
> > > ---
> > >  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 #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
  2013-01-17  9:14       ` Felipe Balbi
@ 2013-01-17  9:28         ` Peter Chen
  2013-01-17  9:32           ` Felipe Balbi
  0 siblings, 1 reply; 9+ messages in thread
From: Peter Chen @ 2013-01-17  9:28 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: r58472, gregkh, linux-usb, kernel, Shawn Guo, linuxppc-dev,
	linux-arm-kernel

On Thu, Jan 17, 2013 at 11:14:22AM +0200, Felipe Balbi wrote:
> 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@freescale.com>
> > > > ---
> > > >  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.

In fact, this driver is just the temp solution, we will use chipidea
in future, so just take all i.mx usb as two kinds of ip are ok.

Do you want me to change like Shawn said, or current version is ok?

> 
> -- 
> balbi



-- 

Best Regards,
Peter Chen

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v5 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
  2013-01-17  9:28         ` Peter Chen
@ 2013-01-17  9:32           ` Felipe Balbi
  0 siblings, 0 replies; 9+ messages in thread
From: Felipe Balbi @ 2013-01-17  9:32 UTC (permalink / raw)
  To: Peter Chen
  Cc: r58472, gregkh, linux-usb, Felipe Balbi, kernel, Shawn Guo,
	linuxppc-dev, linux-arm-kernel

[-- Attachment #1: Type: text/plain, Size: 2001 bytes --]

Hi,

On Thu, Jan 17, 2013 at 05:28:30PM +0800, Peter Chen wrote:

<trimming>

> > > > 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.
> 
> In fact, this driver is just the temp solution, we will use chipidea
> in future, so just take all i.mx usb as two kinds of ip are ok.
> 
> Do you want me to change like Shawn said, or current version is ok?

yes, please resend with changes.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2013-01-17  9:32 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-15  2:29 [PATCH v5 0/3] Fix the Build error for fsl_mxc_udc.c Peter Chen
2013-01-15  2:29 ` [PATCH v5 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id Peter Chen
2013-01-15 14:03   ` Shawn Guo
2013-01-16  1:48     ` Peter Chen
2013-01-17  9:14       ` Felipe Balbi
2013-01-17  9:28         ` Peter Chen
2013-01-17  9:32           ` Felipe Balbi
2013-01-15  2:29 ` [PATCH v5 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap Peter Chen
2013-01-15  2:29 ` [PATCH v5 3/3] ARM: i.MX clock: Change the connection-id for fsl-usb2-udc Peter Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).