* Re: [PATCH] pci: Provide support for parsing PCI DT ranges property
From: Andrew Murray @ 2013-01-14 9:15 UTC (permalink / raw)
To: Thierry Reding
Cc: Michal Simek, linux-pci@vger.kernel.org, devicetree-discuss,
Liviu Dudau, rob.herring@calxeda.com, Rob Herring, linuxppc-dev
In-Reply-To: <20121220082500.GA32617@avionic-0098.adnet.avionic-design.de>
On Thu, Dec 20, 2012 at 08:25:00AM +0000, Thierry Reding wrote:
> On Wed, Dec 12, 2012 at 04:37:50PM +0000, Andrew Murray wrote:
> [...]
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> [...]
> > +=09start =3D of_get_property(node, "ranges", &rlen);
> > +=09if (start =3D=3D NULL)
> > +=09=09return NULL;
> > +
> > +=09end =3D start + rlen;
>=20
> I'm currently rewriting large parts of the Tegra PCIe controller driver
> and I'm trying to use this new API. This seems to work fine, except that
> I think this line needs to be:
>=20
> =09end =3D start + rlen / sizeof(__be32);
>=20
> Otherwise we'll try to process 4 times as many ranges as there are.
>=20
> Thierry
Good catch. Thanks for taking this on.
Andrew Murray
^ permalink raw reply
* Re: [PATCH] pci: Provide support for parsing PCI DT ranges property
From: Andrew Murray @ 2013-01-14 9:24 UTC (permalink / raw)
To: Grant Likely
Cc: Michal Simek, linux-pci@vger.kernel.org, devicetree-discuss,
Thierry Reding, Liviu Dudau, rob.herring@calxeda.com, Rob Herring,
linuxppc-dev
In-Reply-To: <CACxGe6uZATX2uBjuDUsJ489-jmvFbLm-NQ6Q3SGqRVky9XGJYg@mail.gmail.com>
On Sat, Dec 15, 2012 at 01:06:41AM +0000, Grant Likely wrote:
> On Wed, Dec 12, 2012 at 4:37 PM, Andrew Murray <Andrew.Murray@arm.com> wr=
ote:
> > DT bindings for PCI host bridges often use the ranges property to descr=
ibe
> > memory and IO ranges - this binding tends to be the same across archite=
ctures
> > yet several parsing implementations exist, e.g. arch/mips/pci/pci.c,
> > arch/powerpc/kernel/pci-common.c, arch/sparc/kernel/pci.c and
> > arch/microblaze/pci/pci-common.c (clone of PPC). Some of these duplicat=
e
> > functionality provided by drivers/of/address.c.
>=20
> Hi Andrew,
>=20
> Thanks for looking into this. This definitely needs to be done.
>=20
> However, I cannot merge this patch as-is because it actually makes
> things worse by adding yet another implementation of the parsing code.
> Plus it doesn't actually have any users. :-)
I understand. Though I see Thierry has included this in his patch set - I
guess that means there is potentially one user now :).
>=20
> Instead, move the existing code that you need out of
> arch/powerpc/kernel/pci-common.c into a shared place and add in the
> features you need. Bonus points if you fixup microblaze or others at
> the same time.
In most part the patch I submitted was the common code from powerpc but
without quirks and tie-ins to powerpc structures. I'd like to convert
powerpc to using my patch and others but won't get time to do this at
present :(
>=20
> g.
>=20
Andrew Murray
^ permalink raw reply
* [PATCH v3 0/3] Fix the Build error for fsl_mxc_udc.c
From: Peter Chen @ 2013-01-14 10:12 UTC (permalink / raw)
To: shawn.guo, balbi, kernel, gregkh, r58472
Cc: linux-usb, linuxppc-dev, linux-arm-kernel
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 | 23 +++++----
drivers/usb/gadget/fsl_udc_core.c | 52 +++++++++++++-------
drivers/usb/gadget/fsl_usb2_udc.h | 13 ++++--
include/linux/fsl_devices.h | 8 +++
11 files changed, 87 insertions(+), 55 deletions(-)
^ permalink raw reply
* [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
From: Peter Chen @ 2013-01-14 10:12 UTC (permalink / raw)
To: shawn.guo, balbi, kernel, gregkh, r58472
Cc: linux-usb, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1358158361-25550-1-git-send-email-peter.chen@freescale.com>
As mach/hardware.h is deleted, we need to use platform_device_id to
differentiate SoCs.
Besides 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 | 11 ++--
drivers/usb/gadget/fsl_udc_core.c | 52 +++++++++++++-------
drivers/usb/gadget/fsl_usb2_udc.h | 13 ++++--
include/linux/fsl_devices.h | 8 +++
6 files changed, 65 insertions(+), 35 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..6df45f7 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;
@@ -28,7 +26,7 @@ static struct clk *mxc_ipg_clk;
#define USBPHYCTRL_OTGBASE_OFFSET 0x608
#define USBPHYCTRL_EVDO (1 << 23)
-int fsl_udc_clk_init(struct platform_device *pdev)
+int fsl_udc_clk_init(enum fsl_udc_type devtype, struct platform_device *pdev)
{
struct fsl_usb2_platform_data *pdata;
unsigned long freq;
@@ -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 (!(devtype == IMX51_UDC)) {
freq = clk_get_rate(mxc_per_clk);
if (pdata->phy_mode != FSL_USB2_PHY_ULPI &&
(freq < 59999000 || freq > 60001000)) {
@@ -79,10 +77,11 @@ eclkrate:
return ret;
}
-void fsl_udc_clk_finalize(struct platform_device *pdev)
+void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
+ struct platform_device *pdev)
{
struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
- if (cpu_is_mx35()) {
+ if (devtype == IMX35_UDC) {
unsigned int v;
/* workaround ENGcm09152 for i.MX35 */
diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c
index c19f7f1..c32119b 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,17 +2439,13 @@ 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");
return -ENOMEM;
}
+ udc_controller->devtype = pdev->id_entry->driver_data;
pdata = pdev->dev.platform_data;
udc_controller->pdata = pdata;
spin_lock_init(&udc_controller->lock);
@@ -2505,7 +2502,7 @@ static int __init fsl_udc_probe(struct platform_device *pdev)
#endif
/* Initialize USB clocks */
- ret = fsl_udc_clk_init(pdev);
+ ret = fsl_udc_clk_init(udc_controller->devtype, pdev);
if (ret < 0)
goto err_iounmap_noclk;
@@ -2547,7 +2544,7 @@ static int __init fsl_udc_probe(struct platform_device *pdev)
dr_controller_setup(udc_controller);
}
- fsl_udc_clk_finalize(pdev);
+ fsl_udc_clk_finalize(udc_controller->devtype, pdev);
/* Setup gadget structure */
udc_controller->gadget.ops = &fsl_gadget_ops;
@@ -2756,22 +2753,41 @@ 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",
+ .driver_data = IMX25_UDC,
+ }, {
+ .name = "imx-udc-mx27",
+ .driver_data = IMX27_UDC,
+ }, {
+ .name = "imx-udc-mx31",
+ .driver_data = IMX31_UDC,
+ }, {
+ .name = "imx-udc-mx35",
+ .driver_data = IMX35_UDC,
+ }, {
+ .name = "imx-udc-mx51",
+ .driver_data = IMX51_UDC,
+ }
+};
+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,
},
};
diff --git a/drivers/usb/gadget/fsl_usb2_udc.h b/drivers/usb/gadget/fsl_usb2_udc.h
index f61a967..bc1f6d0 100644
--- a/drivers/usb/gadget/fsl_usb2_udc.h
+++ b/drivers/usb/gadget/fsl_usb2_udc.h
@@ -505,6 +505,8 @@ struct fsl_udc {
u32 ep0_dir; /* Endpoint zero direction: can be
USB_DIR_IN or USB_DIR_OUT */
u8 device_address; /* Device USB address */
+ /* devtype for kinds of SoC, only i.mx uses it now */
+ enum fsl_udc_type devtype;
};
/*-------------------------------------------------------------------------*/
@@ -591,15 +593,18 @@ 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_init(enum fsl_udc_type devtype, struct platform_device *pdev);
+void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
+ struct platform_device *pdev);
void fsl_udc_clk_release(void);
#else
-static inline int fsl_udc_clk_init(struct platform_device *pdev)
+static inline int fsl_udc_clk_init(enum fsl_udc_type devtype,
+ struct platform_device *pdev)
{
return 0;
}
-static inline void fsl_udc_clk_finalize(struct platform_device *pdev)
+static inline void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
+ struct platform_device *pdev)
{
}
static inline void fsl_udc_clk_release(void)
diff --git a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h
index a82296a..7cb3fe0 100644
--- a/include/linux/fsl_devices.h
+++ b/include/linux/fsl_devices.h
@@ -66,6 +66,14 @@ enum fsl_usb2_phy_modes {
FSL_USB2_PHY_SERIAL,
};
+enum fsl_udc_type {
+ IMX25_UDC,
+ IMX27_UDC,
+ IMX31_UDC,
+ IMX35_UDC,
+ IMX51_UDC,
+};
+
struct clk;
struct platform_device;
--
1.7.0.4
^ permalink raw reply related
* [PATCH v3 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
From: Peter Chen @ 2013-01-14 10:12 UTC (permalink / raw)
To: shawn.guo, balbi, kernel, gregkh, r58472
Cc: linux-usb, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1358158361-25550-1-git-send-email-peter.chen@freescale.com>
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 | 12 +++++++-----
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index 6df45f7..0e858e6 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(enum fsl_udc_type devtype, struct platform_device *pdev)
@@ -83,15 +84,16 @@ void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
if (devtype == IMX35_UDC) {
unsigned int v;
+ void __iomem *phy_regs = ioremap((unsigned long)pdata->regs +
+ MX35_USBPHYCTRL_OFFSET, 512);
/* workaround ENGcm09152 for i.MX35 */
if (pdata->workaround & FLS_USB2_WORKAROUND_ENGCM09152) {
- v = readl(MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
- USBPHYCTRL_OTGBASE_OFFSET));
+ 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);
}
/* ULPI transceivers don't need usbpll */
--
1.7.0.4
^ permalink raw reply related
* [PATCH v3 3/3] ARM: i.MX clock: Change the connection-id for fsl-usb2-udc
From: Peter Chen @ 2013-01-14 10:12 UTC (permalink / raw)
To: shawn.guo, balbi, kernel, gregkh, r58472
Cc: linux-usb, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1358158361-25550-1-git-send-email-peter.chen@freescale.com>
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
* Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
From: Felipe Balbi @ 2013-01-14 10:16 UTC (permalink / raw)
To: Peter Chen
Cc: r58472, gregkh, linux-usb, balbi, kernel, shawn.guo, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <1358158361-25550-2-git-send-email-peter.chen@freescale.com>
[-- Attachment #1: Type: text/plain, Size: 998 bytes --]
Hi,
On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
> @@ -2756,22 +2753,41 @@ 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",
> + .driver_data = IMX25_UDC,
> + }, {
> + .name = "imx-udc-mx27",
> + .driver_data = IMX27_UDC,
> + }, {
> + .name = "imx-udc-mx31",
> + .driver_data = IMX31_UDC,
> + }, {
> + .name = "imx-udc-mx35",
> + .driver_data = IMX35_UDC,
> + }, {
> + .name = "imx-udc-mx51",
> + .driver_data = IMX51_UDC,
> + }
> +};
I wonder if your driver-data is actually needed since you can use string
comparisson to achieve the exact same outcome.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v3 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
From: Felipe Balbi @ 2013-01-14 10:17 UTC (permalink / raw)
To: Peter Chen
Cc: r58472, gregkh, linux-usb, balbi, kernel, shawn.guo, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <1358158361-25550-3-git-send-email-peter.chen@freescale.com>
[-- Attachment #1: Type: text/plain, Size: 1427 bytes --]
On Mon, Jan 14, 2013 at 06:12:40PM +0800, Peter Chen wrote:
> 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 | 12 +++++++-----
> 1 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
> index 6df45f7..0e858e6 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(enum fsl_udc_type devtype, struct platform_device *pdev)
> @@ -83,15 +84,16 @@ void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
> struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
> if (devtype == IMX35_UDC) {
> unsigned int v;
> + void __iomem *phy_regs = ioremap((unsigned long)pdata->regs +
> + MX35_USBPHYCTRL_OFFSET, 512);
as I said before, this should be passed via struct resource, not pdata.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
From: Marc Kleine-Budde @ 2013-01-14 10:18 UTC (permalink / raw)
To: balbi
Cc: r58472, gregkh, linux-usb, Peter Chen, kernel, shawn.guo,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20130114101642.GB10874@arwen.pp.htv.fi>
[-- Attachment #1: Type: text/plain, Size: 1437 bytes --]
On 01/14/2013 11:16 AM, Felipe Balbi wrote:
> Hi,
>
> On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
>> @@ -2756,22 +2753,41 @@ 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",
>> + .driver_data = IMX25_UDC,
>> + }, {
>> + .name = "imx-udc-mx27",
>> + .driver_data = IMX27_UDC,
>> + }, {
>> + .name = "imx-udc-mx31",
>> + .driver_data = IMX31_UDC,
>> + }, {
>> + .name = "imx-udc-mx35",
>> + .driver_data = IMX35_UDC,
>> + }, {
>> + .name = "imx-udc-mx51",
>> + .driver_data = IMX51_UDC,
>> + }
>> +};
>
> I wonder if your driver-data is actually needed since you can use string
> comparisson to achieve the exact same outcome.
Why use a string compare, if the kernel infrastructure already does this
for you?
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
From: Felipe Balbi @ 2013-01-14 10:24 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: r58472, gregkh, linux-usb, balbi, Peter Chen, kernel, shawn.guo,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <50F3DB8D.6050104@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 1382 bytes --]
On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
> On 01/14/2013 11:16 AM, Felipe Balbi wrote:
> > Hi,
> >
> > On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
> >> @@ -2756,22 +2753,41 @@ 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",
> >> + .driver_data = IMX25_UDC,
> >> + }, {
> >> + .name = "imx-udc-mx27",
> >> + .driver_data = IMX27_UDC,
> >> + }, {
> >> + .name = "imx-udc-mx31",
> >> + .driver_data = IMX31_UDC,
> >> + }, {
> >> + .name = "imx-udc-mx35",
> >> + .driver_data = IMX35_UDC,
> >> + }, {
> >> + .name = "imx-udc-mx51",
> >> + .driver_data = IMX51_UDC,
> >> + }
> >> +};
> >
> > I wonder if your driver-data is actually needed since you can use string
> > comparisson to achieve the exact same outcome.
>
> Why use a string compare, if the kernel infrastructure already does this
> for you?
what do you mean ? What kernel infrastructure is doing waht for me ?
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
From: Marc Kleine-Budde @ 2013-01-14 10:34 UTC (permalink / raw)
To: balbi
Cc: r58472, gregkh, linux-usb, Peter Chen, kernel, shawn.guo,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20130114102444.GD10874@arwen.pp.htv.fi>
[-- Attachment #1: Type: text/plain, Size: 1951 bytes --]
On 01/14/2013 11:24 AM, Felipe Balbi wrote:
> On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
>> On 01/14/2013 11:16 AM, Felipe Balbi wrote:
>>> Hi,
>>>
>>> On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
>>>> @@ -2756,22 +2753,41 @@ 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",
>>>> + .driver_data = IMX25_UDC,
>>>> + }, {
>>>> + .name = "imx-udc-mx27",
>>>> + .driver_data = IMX27_UDC,
>>>> + }, {
>>>> + .name = "imx-udc-mx31",
>>>> + .driver_data = IMX31_UDC,
>>>> + }, {
>>>> + .name = "imx-udc-mx35",
>>>> + .driver_data = IMX35_UDC,
>>>> + }, {
>>>> + .name = "imx-udc-mx51",
>>>> + .driver_data = IMX51_UDC,
>>>> + }
>>>> +};
>>>
>>> I wonder if your driver-data is actually needed since you can use string
>>> comparisson to achieve the exact same outcome.
>>
>> Why use a string compare, if the kernel infrastructure already does this
>> for you?
>
> what do you mean ? What kernel infrastructure is doing waht for me ?
The kernel infrastructure is doing the string compare for you to match
the device against the driver (via platform_device_id->name). You get
the a pointer to the driver_data for free. So you don't need any string
compare in the driver later.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
From: Felipe Balbi @ 2013-01-14 10:39 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: r58472, gregkh, linux-usb, balbi, Peter Chen, kernel, shawn.guo,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <50F3DF1D.3040706@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 1945 bytes --]
On Mon, Jan 14, 2013 at 11:34:05AM +0100, Marc Kleine-Budde wrote:
> On 01/14/2013 11:24 AM, Felipe Balbi wrote:
> > On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
> >> On 01/14/2013 11:16 AM, Felipe Balbi wrote:
> >>> Hi,
> >>>
> >>> On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
> >>>> @@ -2756,22 +2753,41 @@ 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",
> >>>> + .driver_data = IMX25_UDC,
> >>>> + }, {
> >>>> + .name = "imx-udc-mx27",
> >>>> + .driver_data = IMX27_UDC,
> >>>> + }, {
> >>>> + .name = "imx-udc-mx31",
> >>>> + .driver_data = IMX31_UDC,
> >>>> + }, {
> >>>> + .name = "imx-udc-mx35",
> >>>> + .driver_data = IMX35_UDC,
> >>>> + }, {
> >>>> + .name = "imx-udc-mx51",
> >>>> + .driver_data = IMX51_UDC,
> >>>> + }
> >>>> +};
> >>>
> >>> I wonder if your driver-data is actually needed since you can use string
> >>> comparisson to achieve the exact same outcome.
> >>
> >> Why use a string compare, if the kernel infrastructure already does this
> >> for you?
> >
> > what do you mean ? What kernel infrastructure is doing waht for me ?
>
> The kernel infrastructure is doing the string compare for you to match
> the device against the driver (via platform_device_id->name). You get
> the a pointer to the driver_data for free. So you don't need any string
> compare in the driver later.
but current driver data is just duplicating name with an integer, it's
pretty useless driver data.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
From: Marc Kleine-Budde @ 2013-01-14 10:50 UTC (permalink / raw)
To: balbi
Cc: r58472, gregkh, linux-usb, Peter Chen, kernel, shawn.guo,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20130114103952.GF10874@arwen.pp.htv.fi>
[-- Attachment #1: Type: text/plain, Size: 2666 bytes --]
On 01/14/2013 11:39 AM, Felipe Balbi wrote:
> On Mon, Jan 14, 2013 at 11:34:05AM +0100, Marc Kleine-Budde wrote:
>> On 01/14/2013 11:24 AM, Felipe Balbi wrote:
>>> On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
>>>> On 01/14/2013 11:16 AM, Felipe Balbi wrote:
>>>>> Hi,
>>>>>
>>>>> On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
>>>>>> @@ -2756,22 +2753,41 @@ 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",
>>>>>> + .driver_data = IMX25_UDC,
>>>>>> + }, {
>>>>>> + .name = "imx-udc-mx27",
>>>>>> + .driver_data = IMX27_UDC,
>>>>>> + }, {
>>>>>> + .name = "imx-udc-mx31",
>>>>>> + .driver_data = IMX31_UDC,
>>>>>> + }, {
>>>>>> + .name = "imx-udc-mx35",
>>>>>> + .driver_data = IMX35_UDC,
>>>>>> + }, {
>>>>>> + .name = "imx-udc-mx51",
>>>>>> + .driver_data = IMX51_UDC,
>>>>>> + }
>>>>>> +};
>>>>>
>>>>> I wonder if your driver-data is actually needed since you can use string
>>>>> comparisson to achieve the exact same outcome.
>>>>
>>>> Why use a string compare, if the kernel infrastructure already does this
>>>> for you?
>>>
>>> what do you mean ? What kernel infrastructure is doing waht for me ?
>>
>> The kernel infrastructure is doing the string compare for you to match
>> the device against the driver (via platform_device_id->name). You get
>> the a pointer to the driver_data for free. So you don't need any string
>> compare in the driver later.
>
> but current driver data is just duplicating name with an integer, it's
> pretty useless driver data.
I don't think so - another argument:
Less code. As struct platform_device_id is a static array the space is
allocated anyway. So it doesn't make any difference if driver_data is
NULL or not. Later you just need to make an integer comparison instead
of a call to a strcmp(), if you have a switch/case and IMX*_UDC is an
enum, the compiler will warn you if you've missed an IMX variant.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
From: Felipe Balbi @ 2013-01-14 10:53 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: r58472, gregkh, linux-usb, balbi, Peter Chen, kernel, shawn.guo,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <50F3E301.7040509@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 2745 bytes --]
Hi,
On Mon, Jan 14, 2013 at 11:50:41AM +0100, Marc Kleine-Budde wrote:
> On 01/14/2013 11:39 AM, Felipe Balbi wrote:
> > On Mon, Jan 14, 2013 at 11:34:05AM +0100, Marc Kleine-Budde wrote:
> >> On 01/14/2013 11:24 AM, Felipe Balbi wrote:
> >>> On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
> >>>> On 01/14/2013 11:16 AM, Felipe Balbi wrote:
> >>>>> Hi,
> >>>>>
> >>>>> On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
> >>>>>> @@ -2756,22 +2753,41 @@ 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",
> >>>>>> + .driver_data = IMX25_UDC,
> >>>>>> + }, {
> >>>>>> + .name = "imx-udc-mx27",
> >>>>>> + .driver_data = IMX27_UDC,
> >>>>>> + }, {
> >>>>>> + .name = "imx-udc-mx31",
> >>>>>> + .driver_data = IMX31_UDC,
> >>>>>> + }, {
> >>>>>> + .name = "imx-udc-mx35",
> >>>>>> + .driver_data = IMX35_UDC,
> >>>>>> + }, {
> >>>>>> + .name = "imx-udc-mx51",
> >>>>>> + .driver_data = IMX51_UDC,
> >>>>>> + }
> >>>>>> +};
> >>>>>
> >>>>> I wonder if your driver-data is actually needed since you can use string
> >>>>> comparisson to achieve the exact same outcome.
> >>>>
> >>>> Why use a string compare, if the kernel infrastructure already does this
> >>>> for you?
> >>>
> >>> what do you mean ? What kernel infrastructure is doing waht for me ?
> >>
> >> The kernel infrastructure is doing the string compare for you to match
> >> the device against the driver (via platform_device_id->name). You get
> >> the a pointer to the driver_data for free. So you don't need any string
> >> compare in the driver later.
> >
> > but current driver data is just duplicating name with an integer, it's
> > pretty useless driver data.
>
> I don't think so - another argument:
> Less code. As struct platform_device_id is a static array the space is
> allocated anyway. So it doesn't make any difference if driver_data is
> NULL or not. Later you just need to make an integer comparison instead
> of a call to a strcmp(), if you have a switch/case and IMX*_UDC is an
> enum, the compiler will warn you if you've missed an IMX variant.
fair enough, but then don't create a different enum value for each imx
instance if they're mostly the same. Differentiate only what's actually
different.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
From: Marc Kleine-Budde @ 2013-01-14 11:03 UTC (permalink / raw)
To: balbi
Cc: r58472, gregkh, linux-usb, Peter Chen, kernel, shawn.guo,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20130114105357.GH10874@arwen.pp.htv.fi>
[-- Attachment #1: Type: text/plain, Size: 3428 bytes --]
On 01/14/2013 11:53 AM, Felipe Balbi wrote:
> Hi,
>
> On Mon, Jan 14, 2013 at 11:50:41AM +0100, Marc Kleine-Budde wrote:
>> On 01/14/2013 11:39 AM, Felipe Balbi wrote:
>>> On Mon, Jan 14, 2013 at 11:34:05AM +0100, Marc Kleine-Budde wrote:
>>>> On 01/14/2013 11:24 AM, Felipe Balbi wrote:
>>>>> On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
>>>>>> On 01/14/2013 11:16 AM, Felipe Balbi wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
>>>>>>>> @@ -2756,22 +2753,41 @@ 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",
>>>>>>>> + .driver_data = IMX25_UDC,
>>>>>>>> + }, {
>>>>>>>> + .name = "imx-udc-mx27",
>>>>>>>> + .driver_data = IMX27_UDC,
>>>>>>>> + }, {
>>>>>>>> + .name = "imx-udc-mx31",
>>>>>>>> + .driver_data = IMX31_UDC,
>>>>>>>> + }, {
>>>>>>>> + .name = "imx-udc-mx35",
>>>>>>>> + .driver_data = IMX35_UDC,
>>>>>>>> + }, {
>>>>>>>> + .name = "imx-udc-mx51",
>>>>>>>> + .driver_data = IMX51_UDC,
>>>>>>>> + }
>>>>>>>> +};
>>>>>>>
>>>>>>> I wonder if your driver-data is actually needed since you can use string
>>>>>>> comparisson to achieve the exact same outcome.
>>>>>>
>>>>>> Why use a string compare, if the kernel infrastructure already does this
>>>>>> for you?
>>>>>
>>>>> what do you mean ? What kernel infrastructure is doing waht for me ?
>>>>
>>>> The kernel infrastructure is doing the string compare for you to match
>>>> the device against the driver (via platform_device_id->name). You get
>>>> the a pointer to the driver_data for free. So you don't need any string
>>>> compare in the driver later.
>>>
>>> but current driver data is just duplicating name with an integer, it's
>>> pretty useless driver data.
>>
>> I don't think so - another argument:
>> Less code. As struct platform_device_id is a static array the space is
>> allocated anyway. So it doesn't make any difference if driver_data is
>> NULL or not. Later you just need to make an integer comparison instead
>> of a call to a strcmp(), if you have a switch/case and IMX*_UDC is an
>> enum, the compiler will warn you if you've missed an IMX variant.
>
> fair enough, but then don't create a different enum value for each imx
> instance if they're mostly the same. Differentiate only what's actually
> different.
Usually there isn't any Changelog between IP cores used in the different
fsl processors (at least available outside of fsl), that makes it quite
difficult to say if something found on one imx is really the same as on
the other one. And they (usually) don't provide any versioning
information in a register or the documentation.
just my 2¢
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
From: Felipe Balbi @ 2013-01-14 11:06 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: r58472, gregkh, linux-usb, balbi, Peter Chen, kernel, shawn.guo,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <50F3E5E8.2000905@pengutronix.de>
[-- Attachment #1: Type: text/plain, Size: 3564 bytes --]
On Mon, Jan 14, 2013 at 12:03:04PM +0100, Marc Kleine-Budde wrote:
> On 01/14/2013 11:53 AM, Felipe Balbi wrote:
> > Hi,
> >
> > On Mon, Jan 14, 2013 at 11:50:41AM +0100, Marc Kleine-Budde wrote:
> >> On 01/14/2013 11:39 AM, Felipe Balbi wrote:
> >>> On Mon, Jan 14, 2013 at 11:34:05AM +0100, Marc Kleine-Budde wrote:
> >>>> On 01/14/2013 11:24 AM, Felipe Balbi wrote:
> >>>>> On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wrote:
> >>>>>> On 01/14/2013 11:16 AM, Felipe Balbi wrote:
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>> On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
> >>>>>>>> @@ -2756,22 +2753,41 @@ 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",
> >>>>>>>> + .driver_data = IMX25_UDC,
> >>>>>>>> + }, {
> >>>>>>>> + .name = "imx-udc-mx27",
> >>>>>>>> + .driver_data = IMX27_UDC,
> >>>>>>>> + }, {
> >>>>>>>> + .name = "imx-udc-mx31",
> >>>>>>>> + .driver_data = IMX31_UDC,
> >>>>>>>> + }, {
> >>>>>>>> + .name = "imx-udc-mx35",
> >>>>>>>> + .driver_data = IMX35_UDC,
> >>>>>>>> + }, {
> >>>>>>>> + .name = "imx-udc-mx51",
> >>>>>>>> + .driver_data = IMX51_UDC,
> >>>>>>>> + }
> >>>>>>>> +};
> >>>>>>>
> >>>>>>> I wonder if your driver-data is actually needed since you can use string
> >>>>>>> comparisson to achieve the exact same outcome.
> >>>>>>
> >>>>>> Why use a string compare, if the kernel infrastructure already does this
> >>>>>> for you?
> >>>>>
> >>>>> what do you mean ? What kernel infrastructure is doing waht for me ?
> >>>>
> >>>> The kernel infrastructure is doing the string compare for you to match
> >>>> the device against the driver (via platform_device_id->name). You get
> >>>> the a pointer to the driver_data for free. So you don't need any string
> >>>> compare in the driver later.
> >>>
> >>> but current driver data is just duplicating name with an integer, it's
> >>> pretty useless driver data.
> >>
> >> I don't think so - another argument:
> >> Less code. As struct platform_device_id is a static array the space is
> >> allocated anyway. So it doesn't make any difference if driver_data is
> >> NULL or not. Later you just need to make an integer comparison instead
> >> of a call to a strcmp(), if you have a switch/case and IMX*_UDC is an
> >> enum, the compiler will warn you if you've missed an IMX variant.
> >
> > fair enough, but then don't create a different enum value for each imx
> > instance if they're mostly the same. Differentiate only what's actually
> > different.
>
> Usually there isn't any Changelog between IP cores used in the different
> fsl processors (at least available outside of fsl), that makes it quite
> difficult to say if something found on one imx is really the same as on
> the other one. And they (usually) don't provide any versioning
> information in a register or the documentation.
>
> just my 2¢
$SUBJECT is trying to differentiate a single feature (or maybe two) to
replace cpu_is_xxx(), then expose that on driver_data without creating
one enum value for each release from fsl.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [Solved] Interrupt handler not executed
From: R.Patil @ 2013-01-14 11:34 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <1357928171.5475.4@snotra>
Hello Scott,
Thanks for your immediate response. The problem has been resolved. Actually=
, I was wrong in pointing out the problem.=20
The problem was lying in interrupt handler. The first statement in interrup=
t handler was disable=5Firq() which was leading into a deadlock. I replaced=
'disable=5Firq' with 'disable=5Firq=5Fnosync()' and it worked. =20
Thanks and Regards,
Rahul Patil
-----Scott Wood <scottwood@freescale.com> wrote: -----
To: <R.Patil@mei-india.com>
From: Scott Wood <scottwood@freescale.com>
Date: 01/11/2013 11:46PM
Cc: <linuxppc-dev@lists.ozlabs.org>
Subject: Re: Interrupt handler not executed
On 01/11/2013 01:36:29 AM, R.Patil@mei-india.com wrote:
> Hello,
>=20
> We are working on board based on Freescale MPC8313ERDB. We have =20
> ported linux 3.0.46 kernel on it. In one of device driver written by =20
> us, we need to take some action upon asserting IRQ0 interrupt. For =20
> this we have written interrupt handler which takes care of this. We =20
> are able register interrupt handler successfully with the help of =20
> 'request=5Firq'. We confirmed this by checking respective entry in =20
> '/proc/interrupts'. We have also confirmed assertion of interrupt =20
> line (IRQ0) on oscilloscope. The problem is, interrupt handler does =20
> not execute upon asserting the interrupt line.
What IRQ number did you pass to request=5Firq()? request=5Firq() takes =20
virtual interrupt numbers, not anything out of the chip manual.
If that's not the issue, is the interrupt configured properly for level =20
and sense?
-Scott
Email Disclaimer:
---------------------------
This e-mail and any files transmitted with it are for the sole use of the i=
ntended recipient(s) and may contain confidential and privileged informati=
on. Computer viruses can be transmitted via email.The recipient should chec=
k this email and any attachments for the presence of viruses. The company =
accepts no liability for any damage caused by any virus transmitted by this=
email.
^ permalink raw reply
* [linuxppc-dev][PATCH] powerpc/fsl_pci: Store the pci controller device pointer in the pci controller structure.
From: Varun Sethi @ 2013-01-14 11:28 UTC (permalink / raw)
To: linuxppc-dev, galak, scottwood; +Cc: Varun Sethi
The pci controller structure has a provision to store the device strcuture
pointer of the corresponding platform device. Currently this information is
not stored during fsl pci controller initialization. This information is
required while dealing with iommu groups for pci devices connected to the fsl
pci controller. For the case where the pci devices can't be paritioned, they
would fall under the same device group as the pci controller.
This patch stores the platform device information in the pci controller
structure during initialization.
Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
---
arch/powerpc/sysdev/fsl_pci.c | 9 +++++++--
arch/powerpc/sysdev/fsl_pci.h | 2 +-
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c
index 92a5915..b393ae7 100644
--- a/arch/powerpc/sysdev/fsl_pci.c
+++ b/arch/powerpc/sysdev/fsl_pci.c
@@ -421,13 +421,16 @@ void fsl_pcibios_fixup_bus(struct pci_bus *bus)
}
}
-int __init fsl_add_bridge(struct device_node *dev, int is_primary)
+int __init fsl_add_bridge(struct platform_device *pdev, int is_primary)
{
int len;
struct pci_controller *hose;
struct resource rsrc;
const int *bus_range;
u8 hdr_type, progif;
+ struct device_node *dev;
+
+ dev = pdev->dev.of_node;
if (!of_device_is_available(dev)) {
pr_warning("%s: disabled\n", dev->full_name);
@@ -453,6 +456,8 @@ int __init fsl_add_bridge(struct device_node *dev, int is_primary)
if (!hose)
return -ENOMEM;
+ /* set platform device as the parent */
+ hose->parent = &pdev->dev;
hose->first_busno = bus_range ? bus_range[0] : 0x0;
hose->last_busno = bus_range ? bus_range[1] : 0xff;
@@ -880,7 +885,7 @@ static int fsl_pci_probe(struct platform_device *pdev)
#endif
node = pdev->dev.of_node;
- ret = fsl_add_bridge(node, fsl_pci_primary == node);
+ ret = fsl_add_bridge(pdev, fsl_pci_primary == node);
#ifdef CONFIG_SWIOTLB
if (ret == 0) {
diff --git a/arch/powerpc/sysdev/fsl_pci.h b/arch/powerpc/sysdev/fsl_pci.h
index d078537..c495c00 100644
--- a/arch/powerpc/sysdev/fsl_pci.h
+++ b/arch/powerpc/sysdev/fsl_pci.h
@@ -91,7 +91,7 @@ struct ccsr_pci {
__be32 pex_err_cap_r3; /* 0x.e34 - PCIE error capture register 0 */
};
-extern int fsl_add_bridge(struct device_node *dev, int is_primary);
+extern int fsl_add_bridge(struct platform_device *pdev, int is_primary);
extern void fsl_pcibios_fixup_bus(struct pci_bus *bus);
extern int mpc83xx_add_bridge(struct device_node *dev);
u64 fsl_pci_immrbar_base(struct pci_controller *hose);
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH v3 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
From: Peter Chen @ 2013-01-14 12:56 UTC (permalink / raw)
To: Felipe Balbi
Cc: r58472, gregkh, linux-usb, Marc Kleine-Budde, kernel, shawn.guo,
linuxppc-dev, linux-arm-kernel
In-Reply-To: <20130114110600.GI10874@arwen.pp.htv.fi>
On Mon, Jan 14, 2013 at 01:06:00PM +0200, Felipe Balbi wrote:
> On Mon, Jan 14, 2013 at 12:03:04PM +0100, Marc Kleine-Budde wrote:
> > On 01/14/2013 11:53 AM, Felipe Balbi wrote:
> > > Hi,
> > >=20
> > > On Mon, Jan 14, 2013 at 11:50:41AM +0100, Marc Kleine-Budde wrote:
> > >> On 01/14/2013 11:39 AM, Felipe Balbi wrote:
> > >>> On Mon, Jan 14, 2013 at 11:34:05AM +0100, Marc Kleine-Budde wrote=
:
> > >>>> On 01/14/2013 11:24 AM, Felipe Balbi wrote:
> > >>>>> On Mon, Jan 14, 2013 at 11:18:53AM +0100, Marc Kleine-Budde wro=
te:
> > >>>>>> On 01/14/2013 11:16 AM, Felipe Balbi wrote:
> > >>>>>>> Hi,
> > >>>>>>>
> > >>>>>>> On Mon, Jan 14, 2013 at 06:12:39PM +0800, Peter Chen wrote:
> > >>>>>>>> @@ -2756,22 +2753,41 @@ static int fsl_udc_otg_resume(struct=
device *dev)
> > >>>>>>>> =20
> > >>>>>>>> return fsl_udc_resume(NULL);
> > >>>>>>>> }
> > >>>>>>>> -
> > >>>>>>>> /*---------------------------------------------------------=
----------------
> > >>>>>>>> Register entry point for the peripheral controller driver
> > >>>>>>>> -----------------------------------------------------------=
---------------*/
> > >>>>>>>> -
> > >>>>>>>> +static const struct platform_device_id fsl_udc_devtype[] =3D=
{
> > >>>>>>>> + {
> > >>>>>>>> + .name =3D "imx-udc-mx25",
> > >>>>>>>> + .driver_data =3D IMX25_UDC,
> > >>>>>>>> + }, {
> > >>>>>>>> + .name =3D "imx-udc-mx27",
> > >>>>>>>> + .driver_data =3D IMX27_UDC,
> > >>>>>>>> + }, {
> > >>>>>>>> + .name =3D "imx-udc-mx31",
> > >>>>>>>> + .driver_data =3D IMX31_UDC,
> > >>>>>>>> + }, {
> > >>>>>>>> + .name =3D "imx-udc-mx35",
> > >>>>>>>> + .driver_data =3D IMX35_UDC,
> > >>>>>>>> + }, {
> > >>>>>>>> + .name =3D "imx-udc-mx51",
> > >>>>>>>> + .driver_data =3D IMX51_UDC,
> > >>>>>>>> + }
> > >>>>>>>> +};
> > >>>>>>>
> > >>>>>>> I wonder if your driver-data is actually needed since you can=
use string
> > >>>>>>> comparisson to achieve the exact same outcome.
> > >>>>>>
> > >>>>>> Why use a string compare, if the kernel infrastructure already=
does this
> > >>>>>> for you?
> > >>>>>
> > >>>>> what do you mean ? What kernel infrastructure is doing waht for=
me ?
> > >>>>
> > >>>> The kernel infrastructure is doing the string compare for you to=
match
> > >>>> the device against the driver (via platform_device_id->name). Yo=
u get
> > >>>> the a pointer to the driver_data for free. So you don't need any=
string
> > >>>> compare in the driver later.
> > >>>
> > >>> but current driver data is just duplicating name with an integer,=
it's
> > >>> pretty useless driver data.
> > >>
> > >> I don't think so - another argument:
> > >> Less code. As struct platform_device_id is a static array the spac=
e is
> > >> allocated anyway. So it doesn't make any difference if driver_data=
is
> > >> NULL or not. Later you just need to make an integer comparison ins=
tead
> > >> of a call to a strcmp(), if you have a switch/case and IMX*_UDC is=
an
> > >> enum, the compiler will warn you if you've missed an IMX variant.
> > >=20
> > > fair enough, but then don't create a different enum value for each =
imx
> > > instance if they're mostly the same. Differentiate only what's actu=
ally
> > > different.
> >=20
> > Usually there isn't any Changelog between IP cores used in the differ=
ent
> > fsl processors (at least available outside of fsl), that makes it qui=
te
> > difficult to say if something found on one imx is really the same as =
on
> > the other one. And they (usually) don't provide any versioning
> > information in a register or the documentation.
> >=20
> > just my 2=A2
>=20
> $SUBJECT is trying to differentiate a single feature (or maybe two) to
> replace cpu_is_xxx(), then expose that on driver_data without creating
> one enum value for each release from fsl.
Felipe, every one or two SoCs may have their special operations for
integrate PHY interface, clk operation, or workaround for IC limitation.
Maybe, it will add more future or SoCs (maybe not for this driver)
in the future, using enum is easier than string comparison for expanding =
something.
>=20
> --=20
> balbi
--=20
Best Regards,
Peter Chen
^ permalink raw reply
* Re: [PATCH v3 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
From: Peter Chen @ 2013-01-14 12:58 UTC (permalink / raw)
To: Felipe Balbi
Cc: r58472, gregkh, linux-usb, kernel, shawn.guo, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20130114101708.GC10874@arwen.pp.htv.fi>
On Mon, Jan 14, 2013 at 12:17:08PM +0200, Felipe Balbi wrote:
> On Mon, Jan 14, 2013 at 06:12:40PM +0800, Peter Chen wrote:
> > 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 | 12 +++++++-----
> > 1 files changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
> > index 6df45f7..0e858e6 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(enum fsl_udc_type devtype, struct platform_device *pdev)
> > @@ -83,15 +84,16 @@ void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
> > struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
> > if (devtype == IMX35_UDC) {
> > unsigned int v;
> > + void __iomem *phy_regs = ioremap((unsigned long)pdata->regs +
> > + MX35_USBPHYCTRL_OFFSET, 512);
>
> as I said before, this should be passed via struct resource, not pdata.
My careless, will change at next version
>
> --
> balbi
--
Best Regards,
Peter Chen
^ permalink raw reply
* Re: [PATCH v3 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
From: Russell King - ARM Linux @ 2013-01-14 13:10 UTC (permalink / raw)
To: Peter Chen
Cc: r58472, gregkh, linux-usb, balbi, kernel, shawn.guo, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <1358158361-25550-3-git-send-email-peter.chen@freescale.com>
On Mon, Jan 14, 2013 at 06:12:40PM +0800, Peter Chen wrote:
> @@ -83,15 +84,16 @@ void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
> struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
> if (devtype == IMX35_UDC) {
> unsigned int v;
> + void __iomem *phy_regs = ioremap((unsigned long)pdata->regs +
> + MX35_USBPHYCTRL_OFFSET, 512);
Consider that ioremap() can fail.
^ permalink raw reply
* Re: [PATCH v3 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
From: Peter Chen @ 2013-01-14 13:16 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: r58472, gregkh, linux-usb, balbi, kernel, shawn.guo, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20130114131056.GB23505@n2100.arm.linux.org.uk>
On Mon, Jan 14, 2013 at 01:10:56PM +0000, Russell King - ARM Linux wrote:
> On Mon, Jan 14, 2013 at 06:12:40PM +0800, Peter Chen wrote:
> > @@ -83,15 +84,16 @@ void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
> > struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
> > if (devtype == IMX35_UDC) {
> > unsigned int v;
> > + void __iomem *phy_regs = ioremap((unsigned long)pdata->regs +
> > + MX35_USBPHYCTRL_OFFSET, 512);
>
> Consider that ioremap() can fail.
>
Thanks, will check NULL pointer.
--
Best Regards,
Peter Chen
^ permalink raw reply
* [PATCH v4 0/3] Fix the Build error for fsl_mxc_udc.c
From: Peter Chen @ 2013-01-14 14:16 UTC (permalink / raw)
To: shawn.guo, balbi, kernel, gregkh, r58472
Cc: linux-usb, linuxppc-dev, linux-arm-kernel
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 | 36 ++++++++++----
drivers/usb/gadget/fsl_udc_core.c | 54 ++++++++++++++-------
drivers/usb/gadget/fsl_usb2_udc.h | 13 ++++--
include/linux/fsl_devices.h | 8 +++
11 files changed, 102 insertions(+), 55 deletions(-)
^ permalink raw reply
* [PATCH v4 1/3] usb: fsl-mxc-udc: replace cpu_is_xxx() with platform_device_id
From: Peter Chen @ 2013-01-14 14:16 UTC (permalink / raw)
To: shawn.guo, balbi, kernel, gregkh, r58472
Cc: linux-usb, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1358172969-14898-1-git-send-email-peter.chen@freescale.com>
As mach/hardware.h is deleted, we need to use platform_device_id to
differentiate SoCs.
Besides 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 | 11 ++--
drivers/usb/gadget/fsl_udc_core.c | 52 +++++++++++++-------
drivers/usb/gadget/fsl_usb2_udc.h | 13 ++++--
include/linux/fsl_devices.h | 8 +++
6 files changed, 65 insertions(+), 35 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..6df45f7 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;
@@ -28,7 +26,7 @@ static struct clk *mxc_ipg_clk;
#define USBPHYCTRL_OTGBASE_OFFSET 0x608
#define USBPHYCTRL_EVDO (1 << 23)
-int fsl_udc_clk_init(struct platform_device *pdev)
+int fsl_udc_clk_init(enum fsl_udc_type devtype, struct platform_device *pdev)
{
struct fsl_usb2_platform_data *pdata;
unsigned long freq;
@@ -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 (!(devtype == IMX51_UDC)) {
freq = clk_get_rate(mxc_per_clk);
if (pdata->phy_mode != FSL_USB2_PHY_ULPI &&
(freq < 59999000 || freq > 60001000)) {
@@ -79,10 +77,11 @@ eclkrate:
return ret;
}
-void fsl_udc_clk_finalize(struct platform_device *pdev)
+void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
+ struct platform_device *pdev)
{
struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
- if (cpu_is_mx35()) {
+ if (devtype == IMX35_UDC) {
unsigned int v;
/* workaround ENGcm09152 for i.MX35 */
diff --git a/drivers/usb/gadget/fsl_udc_core.c b/drivers/usb/gadget/fsl_udc_core.c
index c19f7f1..c32119b 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,17 +2439,13 @@ 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");
return -ENOMEM;
}
+ udc_controller->devtype = pdev->id_entry->driver_data;
pdata = pdev->dev.platform_data;
udc_controller->pdata = pdata;
spin_lock_init(&udc_controller->lock);
@@ -2505,7 +2502,7 @@ static int __init fsl_udc_probe(struct platform_device *pdev)
#endif
/* Initialize USB clocks */
- ret = fsl_udc_clk_init(pdev);
+ ret = fsl_udc_clk_init(udc_controller->devtype, pdev);
if (ret < 0)
goto err_iounmap_noclk;
@@ -2547,7 +2544,7 @@ static int __init fsl_udc_probe(struct platform_device *pdev)
dr_controller_setup(udc_controller);
}
- fsl_udc_clk_finalize(pdev);
+ fsl_udc_clk_finalize(udc_controller->devtype, pdev);
/* Setup gadget structure */
udc_controller->gadget.ops = &fsl_gadget_ops;
@@ -2756,22 +2753,41 @@ 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",
+ .driver_data = IMX25_UDC,
+ }, {
+ .name = "imx-udc-mx27",
+ .driver_data = IMX27_UDC,
+ }, {
+ .name = "imx-udc-mx31",
+ .driver_data = IMX31_UDC,
+ }, {
+ .name = "imx-udc-mx35",
+ .driver_data = IMX35_UDC,
+ }, {
+ .name = "imx-udc-mx51",
+ .driver_data = IMX51_UDC,
+ }
+};
+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,
},
};
diff --git a/drivers/usb/gadget/fsl_usb2_udc.h b/drivers/usb/gadget/fsl_usb2_udc.h
index f61a967..bc1f6d0 100644
--- a/drivers/usb/gadget/fsl_usb2_udc.h
+++ b/drivers/usb/gadget/fsl_usb2_udc.h
@@ -505,6 +505,8 @@ struct fsl_udc {
u32 ep0_dir; /* Endpoint zero direction: can be
USB_DIR_IN or USB_DIR_OUT */
u8 device_address; /* Device USB address */
+ /* devtype for kinds of SoC, only i.mx uses it now */
+ enum fsl_udc_type devtype;
};
/*-------------------------------------------------------------------------*/
@@ -591,15 +593,18 @@ 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_init(enum fsl_udc_type devtype, struct platform_device *pdev);
+void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
+ struct platform_device *pdev);
void fsl_udc_clk_release(void);
#else
-static inline int fsl_udc_clk_init(struct platform_device *pdev)
+static inline int fsl_udc_clk_init(enum fsl_udc_type devtype,
+ struct platform_device *pdev)
{
return 0;
}
-static inline void fsl_udc_clk_finalize(struct platform_device *pdev)
+static inline void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
+ struct platform_device *pdev)
{
}
static inline void fsl_udc_clk_release(void)
diff --git a/include/linux/fsl_devices.h b/include/linux/fsl_devices.h
index a82296a..7cb3fe0 100644
--- a/include/linux/fsl_devices.h
+++ b/include/linux/fsl_devices.h
@@ -66,6 +66,14 @@ enum fsl_usb2_phy_modes {
FSL_USB2_PHY_SERIAL,
};
+enum fsl_udc_type {
+ IMX25_UDC,
+ IMX27_UDC,
+ IMX31_UDC,
+ IMX35_UDC,
+ IMX51_UDC,
+};
+
struct clk;
struct platform_device;
--
1.7.0.4
^ permalink raw reply related
* [PATCH v4 2/3] usb: fsl_mxc_udc: replace MX35_IO_ADDRESS to ioremap
From: Peter Chen @ 2013-01-14 14:16 UTC (permalink / raw)
To: shawn.guo, balbi, kernel, gregkh, r58472
Cc: linux-usb, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1358172969-14898-1-git-send-email-peter.chen@freescale.com>
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 | 27 +++++++++++++++++++++------
drivers/usb/gadget/fsl_udc_core.c | 4 +++-
drivers/usb/gadget/fsl_usb2_udc.h | 4 ++--
3 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/gadget/fsl_mxc_udc.c b/drivers/usb/gadget/fsl_mxc_udc.c
index 6df45f7..e505d60 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(enum fsl_udc_type devtype, struct platform_device *pdev)
@@ -77,28 +78,42 @@ eclkrate:
return ret;
}
-void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
+int fsl_udc_clk_finalize(enum fsl_udc_type devtype,
struct platform_device *pdev)
{
struct fsl_usb2_platform_data *pdata = pdev->dev.platform_data;
+ int ret = 0;
+
if (devtype == IMX35_UDC) {
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;
+ }
/* workaround ENGcm09152 for i.MX35 */
if (pdata->workaround & FLS_USB2_WORKAROUND_ENGCM09152) {
- v = readl(MX35_IO_ADDRESS(MX35_USB_BASE_ADDR +
- USBPHYCTRL_OTGBASE_OFFSET));
+ 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 c32119b..4391d49 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -2544,7 +2544,9 @@ static int __init fsl_udc_probe(struct platform_device *pdev)
dr_controller_setup(udc_controller);
}
- fsl_udc_clk_finalize(udc_controller->devtype, pdev);
+ ret = fsl_udc_clk_finalize(udc_controller->devtype, 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 bc1f6d0..7ead5f7 100644
--- a/drivers/usb/gadget/fsl_usb2_udc.h
+++ b/drivers/usb/gadget/fsl_usb2_udc.h
@@ -594,7 +594,7 @@ 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(enum fsl_udc_type devtype, struct platform_device *pdev);
-void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
+int fsl_udc_clk_finalize(enum fsl_udc_type devtype,
struct platform_device *pdev);
void fsl_udc_clk_release(void);
#else
@@ -603,7 +603,7 @@ static inline int fsl_udc_clk_init(enum fsl_udc_type devtype,
{
return 0;
}
-static inline void fsl_udc_clk_finalize(enum fsl_udc_type devtype,
+static inline int fsl_udc_clk_finalize(enum fsl_udc_type devtype,
struct platform_device *pdev)
{
}
--
1.7.0.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox