[PATCH] Add USB support to mpc8349-mitx board port

[PATCH] Add USB support to mpc8349-mitx board port

[Grant Likely]

From: Grant Likely <grant.likely@secretlab.ca>

This patch depends on Li Yang's patch: "83xx USB platform code rework"

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---

 arch/powerpc/platforms/83xx/mpc834x_itx.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/mpc834x_itx.c b/arch/powerpc/platforms/83xx/mpc834x_itx.c
index 40a0194..1ace7ac 100644
--- a/arch/powerpc/platforms/83xx/mpc834x_itx.c
+++ b/arch/powerpc/platforms/83xx/mpc834x_itx.c
@@ -63,6 +63,8 @@ static void __init mpc834x_itx_setup_arch(void)
 
 	ppc_md.pci_exclude_device = mpc83xx_exclude_device;
 #endif
+
+	mpc834x_usb_cfg();
 }
 
 static void __init mpc834x_itx_init_IRQ(void)

Re: [PATCH] Add USB support to mpc8349-mitx board port

[Arnd Bergmann]

On Saturday 07 July 2007, Grant Likely wrote:
> --- a/arch/powerpc/platforms/83xx/mpc834x_itx.c
> +++ b/arch/powerpc/platforms/83xx/mpc834x_itx.c
> @@ -63,6 +63,8 @@ static void __init mpc834x_itx_setup_arch(void)
> =A0
> =A0=A0=A0=A0=A0=A0=A0=A0ppc_md.pci_exclude_device =3D mpc83xx_exclude_dev=
ice;
> =A0#endif
> +
> +=A0=A0=A0=A0=A0=A0=A0mpc834x_usb_cfg();
> =A0}

Why is that necessary? Shouldn't there be an of_platform_driver that
simply does all the setup automatically if the device is present?

	Arnd <><

Re: [PATCH] Add USB support to mpc8349-mitx board port

[Grant Likely]

On 7/6/07, Arnd Bergmann <arnd@arndb.de> wrote:
> On Saturday 07 July 2007, Grant Likely wrote:
> > --- a/arch/powerpc/platforms/83xx/mpc834x_itx.c
> > +++ b/arch/powerpc/platforms/83xx/mpc834x_itx.c
> > @@ -63,6 +63,8 @@ static void __init mpc834x_itx_setup_arch(void)
> >
> > ppc_md.pci_exclude_device = mpc83xx_exclude_device;
> > #endif
> > +
> > +mpc834x_usb_cfg();
> > }
>
> Why is that necessary? Shouldn't there be an of_platform_driver that
> simply does all the setup automatically if the device is present?

This call is actually for SoC setup stuff.  There are actually 2 USB
engines on the SoC, and this call figures out what mode the pins are
supposed to be in.  I think this stuff firmly falls into the platform
setup arena since it is highly chip specific.

You bring up a good point though.  The current usb support code in
sysdev/fsl_soc.c uses arch_initcall(fsl_usb_of_init) to go searching
for USB interfaces in the device tree, which are EHCI (and sometimes
OTG) compatible.  fsl_usb_of_init calls
platform_device_register_simple() for each USB device found which is
kind of a round about way of doing things.  That should probably be
depreciated and an of_platform_device binding should be added to the
EHCI driver.

This same comment probably goes for the other arch_initcall functions
in fsl_soc.c which do exactly the same thing for other devices.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: [PATCH] Add USB support to mpc8349-mitx board port

[John Rigby]

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

On 7/6/07, Grant Likely <grant.likely@secretlab.ca> wrote:
>
> On 7/6/07, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Saturday 07 July 2007, Grant Likely wrote:
> > > --- a/arch/powerpc/platforms/83xx/mpc834x_itx.c
> > > +++ b/arch/powerpc/platforms/83xx/mpc834x_itx.c
> > > @@ -63,6 +63,8 @@ static void __init mpc834x_itx_setup_arch(void)
> > >
> > > ppc_md.pci_exclude_device = mpc83xx_exclude_device;
> > > #endif
> > > +
> > > +mpc834x_usb_cfg();
> > > }
> >
> > Why is that necessary? Shouldn't there be an of_platform_driver that
> > simply does all the setup automatically if the device is present?
>
> This call is actually for SoC setup stuff.  There are actually 2 USB
> engines on the SoC, and this call figures out what mode the pins are
> supposed to be in.  I think this stuff firmly falls into the platform
> setup arena since it is highly chip specific.
>
> You bring up a good point though.  The current usb support code in
> sysdev/fsl_soc.c uses arch_initcall(fsl_usb_of_init) to go searching
> for USB interfaces in the device tree, which are EHCI (and sometimes
> OTG) compatible.  fsl_usb_of_init calls
> platform_device_register_simple() for each USB device found which is
> kind of a round about way of doing things.  That should probably be
> depreciated and an of_platform_device binding should be added to the
> EHCI driver.
>
> This same comment probably goes for the other arch_initcall functions
> in fsl_soc.c which do exactly the same thing for other devices.


This depends,  some devices in fsl_soc.c may exist on non-powerpc SoCs
that do not have OF.  The USB core in 8349 for example also is in the
arm based mx27 and mx31.  These devices should remain platform devices
and the glue in fls_soc.c will continure to be needed.


Cheers,
> g.
>
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.
> grant.likely@secretlab.ca
> (403) 399-0195
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>

[-- Attachment #2: Type: text/html, Size: 2887 bytes --]

Re: [PATCH] Add USB support to mpc8349-mitx board port

[Grant Likely]

On 7/7/07, John Rigby <jcrigby@gmail.com> wrote:
> > This same comment probably goes for the other arch_initcall functions
> > in fsl_soc.c which do exactly the same thing for other devices.
>
> This depends,  some devices in fsl_soc.c may exist on non-powerpc SoCs
> that do not have OF.  The USB core in 8349 for example also is in the
> arm based mx27 and mx31.  These devices should remain platform devices
> and the glue in fls_soc.c will continure to be needed.

I disagree; the current method is "glue for the glue".  There is no
reason why the driver cannot have two bindings; one for
platform_device and one for of_platform_device.  It's about the same
amount of code, but uses less indirection for the device tree case.

Cheers,
g.


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: [PATCH] Add USB support to mpc8349-mitx board port

[John Rigby]

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

I see your point, I agree the two layers of glue is bad.  But this means
the double code is spread across many drivers instead of being in one
place (fsl_soc.c).

On 7/7/07, Grant Likely <grant.likely@secretlab.ca> wrote:
>
> On 7/7/07, John Rigby <jcrigby@gmail.com> wrote:
> > > This same comment probably goes for the other arch_initcall functions
> > > in fsl_soc.c which do exactly the same thing for other devices.
> >
> > This depends,  some devices in fsl_soc.c may exist on non-powerpc SoCs
> > that do not have OF.  The USB core in 8349 for example also is in the
> > arm based mx27 and mx31.  These devices should remain platform devices
> > and the glue in fls_soc.c will continure to be needed.
>
> I disagree; the current method is "glue for the glue".  There is no
> reason why the driver cannot have two bindings; one for
> platform_device and one for of_platform_device.  It's about the same
> amount of code, but uses less indirection for the device tree case.
>
> Cheers,
> g.
>
>
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.
> grant.likely@secretlab.ca
> (403) 399-0195
>

[-- Attachment #2: Type: text/html, Size: 1617 bytes --]

Re: [PATCH] Add USB support to mpc8349-mitx board port

[Grant Likely]

On 7/8/07, John Rigby <jcrigby@gmail.com> wrote:
> I see your point, I agree the two layers of glue is bad.  But this means
> the double code is spread across many drivers instead of being in one
> place (fsl_soc.c).

???  I don't understand what you mean.  Are you saying that it's
better to have all the OF bindings in one place (fsl_soc.c) instead of
with each individual driver?

Cheers,
g.


-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: [PATCH] Add USB support to mpc8349-mitx board port

[John Rigby]

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

I'm just saying that an argument can be made that having all the OF to
non-OF
glue in one place is better than having dual bindings in every driver.

If you count ugliness in terms of file count the fsl_soc.c solution
gets a score of 1 and the dual binding solution gets an score of N.
Where N is the number of drivers.

John

On 7/9/07, Grant Likely <grant.likely@secretlab.ca> wrote:
>
> On 7/8/07, John Rigby <jcrigby@gmail.com> wrote:
> > I see your point, I agree the two layers of glue is bad.  But this means
> > the double code is spread across many drivers instead of being in one
> > place (fsl_soc.c).
>
> ???  I don't understand what you mean.  Are you saying that it's
> better to have all the OF bindings in one place (fsl_soc.c) instead of
> with each individual driver?
>
> Cheers,
> g.
>
>
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.
> grant.likely@secretlab.ca
> (403) 399-0195
>

[-- Attachment #2: Type: text/html, Size: 1388 bytes --]