Patchwork [1/4] powerpc/fsl_soc.c: prepare for addition of mpc5121 USB code

login
register
mail settings
Submitter Anatolij Gustschin
Date April 27, 2010, 4:11 p.m.
Message ID <1272384698-4359-2-git-send-email-agust@denx.de>
Download mbox | patch
Permalink /patch/51091/
State Superseded
Delegated to: Grant Likely
Headers show

Comments

Anatolij Gustschin - April 27, 2010, 4:11 p.m.
Factor out common code for registering a FSL EHCI platform
device into new fsl_usb2_register_device() function. This
is done to avoid code duplication while adding code for
instantiating of MPC5121 dual role USB platform devices.
Then, the subsequent patch can use
for_each_compatible_node(np, NULL, "fsl,mpc5121-usb2-dr") {
        ...
        fsl_usb2_register_device();
}

Signed-off-by: Anatolij Gustschin <agust@denx.de>
Cc: Kumar Gala <galak@kernel.crashing.org>
Cc: Grant Likely <grant.likely@secretlab.ca>
---
 arch/powerpc/sysdev/fsl_soc.c |  231 +++++++++++++++++++---------------------
 1 files changed, 110 insertions(+), 121 deletions(-)
Grant Likely - April 27, 2010, 4:51 p.m.
On Tue, Apr 27, 2010 at 10:11 AM, Anatolij Gustschin <agust@denx.de> wrote:
> Factor out common code for registering a FSL EHCI platform
> device into new fsl_usb2_register_device() function. This
> is done to avoid code duplication while adding code for
> instantiating of MPC5121 dual role USB platform devices.
> Then, the subsequent patch can use
> for_each_compatible_node(np, NULL, "fsl,mpc5121-usb2-dr") {
>        ...
>        fsl_usb2_register_device();
> }
>
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> Cc: Kumar Gala <galak@kernel.crashing.org>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> ---
>  arch/powerpc/sysdev/fsl_soc.c |  231 +++++++++++++++++++---------------------

Hi Anatolij,

Thanks for this work.  However, I've got concerns.

Forgive me for ragging on code that you didn't write, but this
fsl_soc.c code for registering the USB device really doesn't belong
here anymore.  It should be part of the drivers/usb/host/ehci-fsl.c
and the driver should do of-style binding (Which should be a lot
easier if I manage to get the merge of platform bus and of_platform
bus into 2.6.35).

This patch series makes the fsl_soc.c code even more complicated, and
scatters what is essentially driver code over even more places in the
arch/powerpc tree.  I'm really not keen on it being merged in this
form.

g.
Anatolij Gustschin - May 6, 2010, 7:18 p.m.
Hi Grant,

On Tue, 27 Apr 2010 10:51:21 -0600
Grant Likely <grant.likely@secretlab.ca> wrote:

> On Tue, Apr 27, 2010 at 10:11 AM, Anatolij Gustschin <agust@denx.de> wrote:
> > Factor out common code for registering a FSL EHCI platform
> > device into new fsl_usb2_register_device() function. This
> > is done to avoid code duplication while adding code for
> > instantiating of MPC5121 dual role USB platform devices.
> > Then, the subsequent patch can use
> > for_each_compatible_node(np, NULL, "fsl,mpc5121-usb2-dr") {
> >        ...
> >        fsl_usb2_register_device();
> > }
> >
> > Signed-off-by: Anatolij Gustschin <agust@denx.de>
> > Cc: Kumar Gala <galak@kernel.crashing.org>
> > Cc: Grant Likely <grant.likely@secretlab.ca>
> > ---
> >  arch/powerpc/sysdev/fsl_soc.c |  231 +++++++++++++++++++---------------------
> 
> Hi Anatolij,
> 
> Thanks for this work.  However, I've got concerns.
> 
> Forgive me for ragging on code that you didn't write, but this
> fsl_soc.c code for registering the USB device really doesn't belong
> here anymore.  It should be part of the drivers/usb/host/ehci-fsl.c
> and the driver should do of-style binding (Which should be a lot
> easier if I manage to get the merge of platform bus and of_platform
> bus into 2.6.35).

Now I moved the USB devices registration code to
drivers/usb/host/ehci-fsl.c and added of-style binding there. It
works for one platform driver based on your test-devicetree branch.
It seems I can't bind more than one driver to the device described
in the tree. But I need to bind at least 2 drivers, ehci-hcd and
fsl-usb2-udc. For USB OTG support I need additional one to be bound
to the same USB dual role device (also tree different drivers
simultaneously).
I also can't register UDC device in the ehci-fsl.c since registering
of the UDC device should also be possible independent of the EHCI-HDC
driver (for USB device support we do not need host controller driver).
Is it possible to bind more than one driver to the same device
simultaneously? If so, how can I implement this?

> This patch series makes the fsl_soc.c code even more complicated, and
> scatters what is essentially driver code over even more places in the
> arch/powerpc tree.  I'm really not keen on it being merged in this
> form.

Now I see one reason why it has been originally implemented
this way.

Thanks,
Anatolij
Grant Likely - May 19, 2010, 8:47 p.m.
On Thu, May 6, 2010 at 1:18 PM, Anatolij Gustschin <agust@denx.de> wrote:
> Hi Grant,
>
> On Tue, 27 Apr 2010 10:51:21 -0600
> Grant Likely <grant.likely@secretlab.ca> wrote:
>
>> On Tue, Apr 27, 2010 at 10:11 AM, Anatolij Gustschin <agust@denx.de> wrote:
>> > Factor out common code for registering a FSL EHCI platform
>> > device into new fsl_usb2_register_device() function. This
>> > is done to avoid code duplication while adding code for
>> > instantiating of MPC5121 dual role USB platform devices.
>> > Then, the subsequent patch can use
>> > for_each_compatible_node(np, NULL, "fsl,mpc5121-usb2-dr") {
>> >        ...
>> >        fsl_usb2_register_device();
>> > }
>> >
>> > Signed-off-by: Anatolij Gustschin <agust@denx.de>
>> > Cc: Kumar Gala <galak@kernel.crashing.org>
>> > Cc: Grant Likely <grant.likely@secretlab.ca>
>> > ---
>> >  arch/powerpc/sysdev/fsl_soc.c |  231 +++++++++++++++++++---------------------
>>
>> Hi Anatolij,
>>
>> Thanks for this work.  However, I've got concerns.
>>
>> Forgive me for ragging on code that you didn't write, but this
>> fsl_soc.c code for registering the USB device really doesn't belong
>> here anymore.  It should be part of the drivers/usb/host/ehci-fsl.c
>> and the driver should do of-style binding (Which should be a lot
>> easier if I manage to get the merge of platform bus and of_platform
>> bus into 2.6.35).
>
> Now I moved the USB devices registration code to
> drivers/usb/host/ehci-fsl.c and added of-style binding there. It
> works for one platform driver based on your test-devicetree branch.
> It seems I can't bind more than one driver to the device described
> in the tree. But I need to bind at least 2 drivers, ehci-hcd and
> fsl-usb2-udc. For USB OTG support I need additional one to be bound
> to the same USB dual role device (also tree different drivers
> simultaneously).
> I also can't register UDC device in the ehci-fsl.c since registering
> of the UDC device should also be possible independent of the EHCI-HDC
> driver (for USB device support we do not need host controller driver).
> Is it possible to bind more than one driver to the same device
> simultaneously? If so, how can I implement this?

No, the linux driver model can bind exactly one driver to a struct
device.  However, that doesn't mean you can't have multiple struct
devices (in whatever form they come) to tell the Linux kernel about
all the details of a single hardware device.

>> This patch series makes the fsl_soc.c code even more complicated, and
>> scatters what is essentially driver code over even more places in the
>> arch/powerpc tree.  I'm really not keen on it being merged in this
>> form.
>
> Now I see one reason why it has been originally implemented
> this way.

Should be a solvable problem.  The fsl_soc.c stuff was originally
written simply because the infrastructure wasn't there for doing it
any other way.  We're long past that point now.  I don't have time to
dig into the details now (merge window and all), but ping me in a few
weeks and I'll take another look to see if I can help you.

g.
Anatolij Gustschin - June 11, 2010, 11:24 a.m.
On Wed, 19 May 2010 14:47:47 -0600
Grant Likely <grant.likely@secretlab.ca> wrote:

> On Thu, May 6, 2010 at 1:18 PM, Anatolij Gustschin <agust@denx.de> wrote:
> > Hi Grant,
> >
> > On Tue, 27 Apr 2010 10:51:21 -0600
> > Grant Likely <grant.likely@secretlab.ca> wrote:
> >
> >> On Tue, Apr 27, 2010 at 10:11 AM, Anatolij Gustschin <agust@denx.de> wrote:
> >> > Factor out common code for registering a FSL EHCI platform
> >> > device into new fsl_usb2_register_device() function. This
> >> > is done to avoid code duplication while adding code for
> >> > instantiating of MPC5121 dual role USB platform devices.
> >> > Then, the subsequent patch can use
> >> > for_each_compatible_node(np, NULL, "fsl,mpc5121-usb2-dr") {
> >> >        ...
> >> >        fsl_usb2_register_device();
> >> > }
> >> >
> >> > Signed-off-by: Anatolij Gustschin <agust@denx.de>
> >> > Cc: Kumar Gala <galak@kernel.crashing.org>
> >> > Cc: Grant Likely <grant.likely@secretlab.ca>
> >> > ---
> >> >  arch/powerpc/sysdev/fsl_soc.c |  231 +++++++++++++++++++---------------------
> >>
> >> Hi Anatolij,
> >>
> >> Thanks for this work.  However, I've got concerns.
> >>
> >> Forgive me for ragging on code that you didn't write, but this
> >> fsl_soc.c code for registering the USB device really doesn't belong
> >> here anymore.  It should be part of the drivers/usb/host/ehci-fsl.c
> >> and the driver should do of-style binding (Which should be a lot
> >> easier if I manage to get the merge of platform bus and of_platform
> >> bus into 2.6.35).
> >
> > Now I moved the USB devices registration code to
> > drivers/usb/host/ehci-fsl.c and added of-style binding there. It
> > works for one platform driver based on your test-devicetree branch.
> > It seems I can't bind more than one driver to the device described
> > in the tree. But I need to bind at least 2 drivers, ehci-hcd and
> > fsl-usb2-udc. For USB OTG support I need additional one to be bound
> > to the same USB dual role device (also tree different drivers
> > simultaneously).
> > I also can't register UDC device in the ehci-fsl.c since registering
> > of the UDC device should also be possible independent of the EHCI-HDC
> > driver (for USB device support we do not need host controller driver).
> > Is it possible to bind more than one driver to the same device
> > simultaneously? If so, how can I implement this?
> 
> No, the linux driver model can bind exactly one driver to a struct
> device.  However, that doesn't mean you can't have multiple struct
> devices (in whatever form they come) to tell the Linux kernel about
> all the details of a single hardware device.
> 
> >> This patch series makes the fsl_soc.c code even more complicated, and
> >> scatters what is essentially driver code over even more places in the
> >> arch/powerpc tree.  I'm really not keen on it being merged in this
> >> form.
> >
> > Now I see one reason why it has been originally implemented
> > this way.
> 
> Should be a solvable problem.  The fsl_soc.c stuff was originally
> written simply because the infrastructure wasn't there for doing it
> any other way.  We're long past that point now.  I don't have time to
> dig into the details now (merge window and all), but ping me in a few
> weeks and I'll take another look to see if I can help you.

Hi Grant,

Ping. Do you have any suggestions how to realize this using current
infrastructure? Thanks!

Anatolij
Grant Likely - June 11, 2010, 3:47 p.m.
On Fri, Jun 11, 2010 at 5:24 AM, Anatolij Gustschin <agust@denx.de> wrote:
> On Wed, 19 May 2010 14:47:47 -0600
> Grant Likely <grant.likely@secretlab.ca> wrote:
>
>> On Thu, May 6, 2010 at 1:18 PM, Anatolij Gustschin <agust@denx.de> wrote:
>> > Hi Grant,
>> >
>> > On Tue, 27 Apr 2010 10:51:21 -0600
>> > Grant Likely <grant.likely@secretlab.ca> wrote:
>> >
>> >> On Tue, Apr 27, 2010 at 10:11 AM, Anatolij Gustschin <agust@denx.de> wrote:
>> >> > Factor out common code for registering a FSL EHCI platform
>> >> > device into new fsl_usb2_register_device() function. This
>> >> > is done to avoid code duplication while adding code for
>> >> > instantiating of MPC5121 dual role USB platform devices.
>> >> > Then, the subsequent patch can use
>> >> > for_each_compatible_node(np, NULL, "fsl,mpc5121-usb2-dr") {
>> >> >        ...
>> >> >        fsl_usb2_register_device();
>> >> > }
>> >> >
>> >> > Signed-off-by: Anatolij Gustschin <agust@denx.de>
>> >> > Cc: Kumar Gala <galak@kernel.crashing.org>
>> >> > Cc: Grant Likely <grant.likely@secretlab.ca>
>> >> > ---
>> >> >  arch/powerpc/sysdev/fsl_soc.c |  231 +++++++++++++++++++---------------------
>> >>
>> >> Hi Anatolij,
>> >>
>> >> Thanks for this work.  However, I've got concerns.
>> >>
>> >> Forgive me for ragging on code that you didn't write, but this
>> >> fsl_soc.c code for registering the USB device really doesn't belong
>> >> here anymore.  It should be part of the drivers/usb/host/ehci-fsl.c
>> >> and the driver should do of-style binding (Which should be a lot
>> >> easier if I manage to get the merge of platform bus and of_platform
>> >> bus into 2.6.35).
>> >
>> > Now I moved the USB devices registration code to
>> > drivers/usb/host/ehci-fsl.c and added of-style binding there. It
>> > works for one platform driver based on your test-devicetree branch.
>> > It seems I can't bind more than one driver to the device described
>> > in the tree. But I need to bind at least 2 drivers, ehci-hcd and
>> > fsl-usb2-udc. For USB OTG support I need additional one to be bound
>> > to the same USB dual role device (also tree different drivers
>> > simultaneously).
>> > I also can't register UDC device in the ehci-fsl.c since registering
>> > of the UDC device should also be possible independent of the EHCI-HDC
>> > driver (for USB device support we do not need host controller driver).
>> > Is it possible to bind more than one driver to the same device
>> > simultaneously? If so, how can I implement this?
>>
>> No, the linux driver model can bind exactly one driver to a struct
>> device.  However, that doesn't mean you can't have multiple struct
>> devices (in whatever form they come) to tell the Linux kernel about
>> all the details of a single hardware device.
>>
>> >> This patch series makes the fsl_soc.c code even more complicated, and
>> >> scatters what is essentially driver code over even more places in the
>> >> arch/powerpc tree.  I'm really not keen on it being merged in this
>> >> form.
>> >
>> > Now I see one reason why it has been originally implemented
>> > this way.
>>
>> Should be a solvable problem.  The fsl_soc.c stuff was originally
>> written simply because the infrastructure wasn't there for doing it
>> any other way.  We're long past that point now.  I don't have time to
>> dig into the details now (merge window and all), but ping me in a few
>> weeks and I'll take another look to see if I can help you.
>
> Hi Grant,
>
> Ping. Do you have any suggestions how to realize this using current
> infrastructure? Thanks!

It sounds like the USB OTG controller is effectively a compound device
with one host controller and one device controller plus some logic to
switch between the two.  I'm not a USB expert, so correct me if I'm
wrong here.

You've got 2 choices for implementing this:
A) create a 'master' of_driver which registers 2 platform devices it
it's probe routine.
B) Rework the drivers so that both fsl-ehci and fsl-usb2-udc bits are
called directly (essentially one driver handles both modes)

Option A probably makes the most sense as it has the least amount of
impact on current code.  Essentially, you create an of_driver and put
into it's probe hook the code that is currently in fsl_soc.c.  Then it
will bind in the normal of_platform_bus_type manner and can register
the appropriate platform devices for each platform.

Some important points though; when you create the platform devices,
make sure you set the parent pointer correctly so the new devices are
registered as children of the of_device.

Second, you can eliminate the call to platform_device_add_data() by
setting the of_node pointer in the platform device (the of_node is now
part of struct device).  Make the driver look up its own data from the
device tree instead of passing it in an anonymous data structure.

On that note, the current code looks racy anyway because it registers
the device before attaching the platform_data.  It's probably okay
because of how early it is run and no drivers are registered yet, but
it is still wrong.  It should be: allocate; add data; then register.
But I digress.  Eliminating the platform data makes this problem go
away.

I hope this helps.

g.

Patch

diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index b91f7ac..976218c 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -209,6 +209,49 @@  static int __init of_add_fixed_phys(void)
 arch_initcall(of_add_fixed_phys);
 #endif /* CONFIG_FIXED_PHY */
 
+struct fsl_usb2_dev_data {
+	char *dr_mode;		/* controller mode */
+	char *drivers[3];	/* drivers to instantiate for this mode */
+	enum fsl_usb2_operating_modes op_mode;	/* operating mode */
+};
+
+struct fsl_usb2_dev_data dr_data[] __initdata = {
+	{
+		"host",
+		{ "fsl-ehci", NULL, NULL, },
+		FSL_USB2_DR_HOST,
+	},
+	{
+		"otg",
+		{ "fsl-ehci", "fsl-usb2-udc", "fsl-usb2-otg", },
+		FSL_USB2_DR_OTG,
+	},
+	{
+		"periferal",
+		{ "fsl-usb2-udc", NULL, NULL, },
+		FSL_USB2_DR_DEVICE,
+	},
+};
+
+struct fsl_usb2_dev_data mph_data[] __initdata = {
+	{ NULL,		{ "fsl-ehci",	NULL,		}, FSL_USB2_MPH_HOST, },
+};
+
+struct fsl_usb2_dev_data * __init get_dr_data(struct device_node *np)
+{
+	const unsigned char *prop;
+	int i;
+
+	prop = of_get_property(np, "dr_mode", NULL);
+	if (prop) {
+		for (i = 0; i < ARRAY_SIZE(dr_data); i++) {
+			if (!strcmp(prop, dr_data[i].dr_mode))
+				return &dr_data[i];
+		}
+	}
+	return &dr_data[0]; /* mode not specified, use host */
+}
+
 static enum fsl_usb2_phy_modes determine_usb_phy(const char *phy_type)
 {
 	if (!phy_type)
@@ -225,151 +268,97 @@  static enum fsl_usb2_phy_modes determine_usb_phy(const char *phy_type)
 	return FSL_USB2_PHY_NONE;
 }
 
-static int __init fsl_usb_of_init(void)
+int __init fsl_usb2_register_device(struct device_node *np,
+			struct fsl_usb2_dev_data *dev_data,
+			struct fsl_usb2_platform_data *pdata,
+			unsigned int idx)
 {
-	struct device_node *np;
-	unsigned int i = 0;
-	struct platform_device *usb_dev_mph = NULL, *usb_dev_dr_host = NULL,
-		*usb_dev_dr_client = NULL;
+	struct platform_device *usb_dev;
+	const unsigned char *prop;
+	struct resource r[2];
+	int registered = 0;
 	int ret;
+	int i;
+
+	if (!(dev_data && pdata))
+		return -EINVAL;
+
+	memset(&r, 0, sizeof(r));
+	ret = of_address_to_resource(np, 0, &r[0]);
+	if (ret)
+		return -ENODEV;
+
+	ret = of_irq_to_resource(np, 0, &r[1]);
+	if (ret == NO_IRQ)
+		return -ENODEV;
+
+	pdata->operating_mode = dev_data->op_mode;
+
+	prop = of_get_property(np, "phy_type", NULL);
+	pdata->phy_mode = determine_usb_phy(prop);
+	ret = 0;
+	for (i = 0; i < ARRAY_SIZE(dev_data->drivers); i++) {
+		if (!dev_data->drivers[i])
+			break;
+		usb_dev = platform_device_register_simple(
+					dev_data->drivers[i], idx, r, 2);
+		if (IS_ERR(usb_dev)) {
+			ret = PTR_ERR(usb_dev);
+			goto out;
+		}
 
-	for_each_compatible_node(np, NULL, "fsl-usb2-mph") {
-		struct resource r[2];
-		struct fsl_usb2_platform_data usb_data;
-		const unsigned char *prop = NULL;
-
-		memset(&r, 0, sizeof(r));
-		memset(&usb_data, 0, sizeof(usb_data));
-
-		ret = of_address_to_resource(np, 0, &r[0]);
+		usb_dev->dev.coherent_dma_mask = 0xffffffffUL;
+		usb_dev->dev.dma_mask = &usb_dev->dev.coherent_dma_mask;
+		ret = platform_device_add_data(usb_dev, pdata,
+				sizeof(struct fsl_usb2_platform_data));
 		if (ret)
-			goto err;
-
-		of_irq_to_resource(np, 0, &r[1]);
-
-		usb_dev_mph =
-		    platform_device_register_simple("fsl-ehci", i, r, 2);
-		if (IS_ERR(usb_dev_mph)) {
-			ret = PTR_ERR(usb_dev_mph);
-			goto err;
-		}
+			platform_device_unregister(usb_dev);
+		else
+			registered++;
+	}
+out:
+	if (!registered)
+		irq_dispose_mapping(r[1].end);
 
-		usb_dev_mph->dev.coherent_dma_mask = 0xffffffffUL;
-		usb_dev_mph->dev.dma_mask = &usb_dev_mph->dev.coherent_dma_mask;
+	return ret;
+}
 
-		usb_data.operating_mode = FSL_USB2_MPH_HOST;
+static int __init fsl_usb_of_init(void)
+{
+	struct device_node *np;
+	const unsigned char *prop;
+	struct fsl_usb2_platform_data pdata;
+	unsigned int idx = 0;
+	int ret;
 
+	for_each_compatible_node(np, NULL, "fsl-usb2-mph") {
+		memset(&pdata, 0, sizeof(pdata));
 		prop = of_get_property(np, "port0", NULL);
 		if (prop)
-			usb_data.port_enables |= FSL_USB2_PORT0_ENABLED;
+			pdata.port_enables |= FSL_USB2_PORT0_ENABLED;
 
 		prop = of_get_property(np, "port1", NULL);
 		if (prop)
-			usb_data.port_enables |= FSL_USB2_PORT1_ENABLED;
+			pdata.port_enables |= FSL_USB2_PORT1_ENABLED;
 
-		prop = of_get_property(np, "phy_type", NULL);
-		usb_data.phy_mode = determine_usb_phy(prop);
-
-		ret =
-		    platform_device_add_data(usb_dev_mph, &usb_data,
-					     sizeof(struct
-						    fsl_usb2_platform_data));
+		ret = fsl_usb2_register_device(np, mph_data, &pdata, idx++);
 		if (ret)
-			goto unreg_mph;
-		i++;
+			return ret;
 	}
 
 	for_each_compatible_node(np, NULL, "fsl-usb2-dr") {
-		struct resource r[2];
-		struct fsl_usb2_platform_data usb_data;
-		const unsigned char *prop = NULL;
-
 		if (!of_device_is_available(np))
 			continue;
 
-		memset(&r, 0, sizeof(r));
-		memset(&usb_data, 0, sizeof(usb_data));
-
-		ret = of_address_to_resource(np, 0, &r[0]);
+		memset(&pdata, 0, sizeof(pdata));
+		ret = fsl_usb2_register_device(np, get_dr_data(np),
+						&pdata, idx++);
 		if (ret)
-			goto unreg_mph;
-
-		of_irq_to_resource(np, 0, &r[1]);
-
-		prop = of_get_property(np, "dr_mode", NULL);
-
-		if (!prop || !strcmp(prop, "host")) {
-			usb_data.operating_mode = FSL_USB2_DR_HOST;
-			usb_dev_dr_host = platform_device_register_simple(
-					"fsl-ehci", i, r, 2);
-			if (IS_ERR(usb_dev_dr_host)) {
-				ret = PTR_ERR(usb_dev_dr_host);
-				goto err;
-			}
-		} else if (prop && !strcmp(prop, "peripheral")) {
-			usb_data.operating_mode = FSL_USB2_DR_DEVICE;
-			usb_dev_dr_client = platform_device_register_simple(
-					"fsl-usb2-udc", i, r, 2);
-			if (IS_ERR(usb_dev_dr_client)) {
-				ret = PTR_ERR(usb_dev_dr_client);
-				goto err;
-			}
-		} else if (prop && !strcmp(prop, "otg")) {
-			usb_data.operating_mode = FSL_USB2_DR_OTG;
-			usb_dev_dr_host = platform_device_register_simple(
-					"fsl-ehci", i, r, 2);
-			if (IS_ERR(usb_dev_dr_host)) {
-				ret = PTR_ERR(usb_dev_dr_host);
-				goto err;
-			}
-			usb_dev_dr_client = platform_device_register_simple(
-					"fsl-usb2-udc", i, r, 2);
-			if (IS_ERR(usb_dev_dr_client)) {
-				ret = PTR_ERR(usb_dev_dr_client);
-				goto err;
-			}
-		} else {
-			ret = -EINVAL;
-			goto err;
-		}
-
-		prop = of_get_property(np, "phy_type", NULL);
-		usb_data.phy_mode = determine_usb_phy(prop);
-
-		if (usb_dev_dr_host) {
-			usb_dev_dr_host->dev.coherent_dma_mask = 0xffffffffUL;
-			usb_dev_dr_host->dev.dma_mask = &usb_dev_dr_host->
-				dev.coherent_dma_mask;
-			if ((ret = platform_device_add_data(usb_dev_dr_host,
-						&usb_data, sizeof(struct
-						fsl_usb2_platform_data))))
-				goto unreg_dr;
-		}
-		if (usb_dev_dr_client) {
-			usb_dev_dr_client->dev.coherent_dma_mask = 0xffffffffUL;
-			usb_dev_dr_client->dev.dma_mask = &usb_dev_dr_client->
-				dev.coherent_dma_mask;
-			if ((ret = platform_device_add_data(usb_dev_dr_client,
-						&usb_data, sizeof(struct
-						fsl_usb2_platform_data))))
-				goto unreg_dr;
-		}
-		i++;
+			return ret;
 	}
-	return 0;
 
-unreg_dr:
-	if (usb_dev_dr_host)
-		platform_device_unregister(usb_dev_dr_host);
-	if (usb_dev_dr_client)
-		platform_device_unregister(usb_dev_dr_client);
-unreg_mph:
-	if (usb_dev_mph)
-		platform_device_unregister(usb_dev_mph);
-err:
-	return ret;
+	return 0;
 }
-
 arch_initcall(fsl_usb_of_init);
 
 #if defined(CONFIG_FSL_SOC_BOOKE) || defined(CONFIG_PPC_86xx)