[PATCH 1/3] add USB setup code for 8349emds PB

[PATCH 1/3] add USB setup code for 8349emds PB

[Li Yang]

Add board specific initialization code for USB
to work in both MPH and DR mode for MPC8349EMDS
PB board.

Signed-off-by: Li Yang <leoli@freescale.com>
---

 arch/powerpc/platforms/83xx/Kconfig       |    4 ++
 arch/powerpc/platforms/83xx/mpc834x_sys.c |   79 +++++++++++++++++++++++++++++
 arch/powerpc/platforms/83xx/mpc834x_sys.h |    2 +
 arch/powerpc/platforms/83xx/mpc83xx.h     |   18 +++++++
 4 files changed, 103 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/platforms/83xx/Kconfig
index edcd5b8..5371645 100644
--- a/arch/powerpc/platforms/83xx/Kconfig
+++ b/arch/powerpc/platforms/83xx/Kconfig
@@ -59,4 +59,8 @@ config PPC_MPC836x
 	select PPC_INDIRECT_PCI
 	default y if MPC8360E_PB
 
+config 834x_USB_SUPPORT
+	bool
+	default y if MPC834x_SYS && (USB || USB_GADGET)
+
 endmenu
diff --git a/arch/powerpc/platforms/83xx/mpc834x_sys.c b/arch/powerpc/platforms/83xx/mpc834x_sys.c
index 80b735a..aff7ae3 100644
--- a/arch/powerpc/platforms/83xx/mpc834x_sys.c
+++ b/arch/powerpc/platforms/83xx/mpc834x_sys.c
@@ -35,14 +35,88 @@
 #include <asm/prom.h>
 #include <asm/udbg.h>
 #include <sysdev/fsl_soc.h>
+#include <linux/fsl_devices.h>
 
 #include "mpc83xx.h"
+#include "mpc834x_sys.h"
 
 #ifndef CONFIG_PCI
 unsigned long isa_io_base = 0;
 unsigned long isa_mem_base = 0;
 #endif
 
+static volatile u8 *bcsr_regs = NULL;
+
+#ifdef CONFIG_834x_USB_SUPPORT
+/* Note: This is only for PB, not for PB+PIB
+ * On PB only port0 is connected using ULPI */
+static int mpc834x_usb_cfg(void)
+{
+	unsigned long sccr, sicrl;
+	void __iomem *immap;
+	struct device_node *np = NULL;
+	int port0_is_dr = 0;
+
+	if ((np = of_find_compatible_node(np, "usb", "fsl-usb2-dr")) != NULL)
+		port0_is_dr = 1;
+	if ((np = of_find_compatible_node(np, "usb", "fsl-usb2-mph")) != NULL){
+		if (port0_is_dr) {
+			printk(KERN_WARNING
+				"There is only one USB port on PB board! \n");
+			return -1;
+		} else if (!port0_is_dr)
+			/* No usb port enabled */
+			return -1;
+	}
+
+	immap = ioremap(get_immrbase(), 0x100000);
+	if (!immap)
+		return -1;
+
+	/* Configure clock */
+	sccr = in_be32(immap + MPC83XX_SCCR_OFFS);
+	if (port0_is_dr)
+		sccr |= MPC83XX_SCCR_USB_DRCM_11;  /* 1:3 */
+	else
+		sccr |= MPC83XX_SCCR_USB_MPHCM_11; /* 1:3 */
+	out_be32(immap + MPC83XX_SCCR_OFFS, sccr);
+
+	/* Configure Pin */
+	sicrl = in_be32(immap + MPC83XX_SICRL_OFFS);
+	/* set port0 only */
+	if (port0_is_dr)
+		sicrl |= MPC83XX_SICRL_USB0;
+	else
+		sicrl &= ~(MPC83XX_SICRL_USB0);
+	out_be32(immap + MPC83XX_SICRL_OFFS, sicrl);
+
+	iounmap(immap);
+
+	/* Map BCSR area */
+	np = of_find_node_by_name(NULL, "bcsr");
+	if (np != 0) {
+		struct resource res;
+
+		of_address_to_resource(np, 0, &res);
+		bcsr_regs = ioremap(res.start, res.end - res.start + 1);
+		of_node_put(np);
+	}
+	if (!bcsr_regs)
+		return -1;
+
+	/*
+	 * if SYS board is plug into PIB board,
+	 * force to use the PHY on SYS board
+	 */
+	if (!(bcsr_regs[5] & BCSR5_INT_USB))
+		bcsr_regs[5] |= BCSR5_INT_USB;
+	iounmap(bcsr_regs);
+	return 0;
+}
+
+#endif /* CONFIG_834x_USB_SUPPORT */
+
+
 /* ************************************************************************
  *
  * Setup the architecture
@@ -65,6 +139,7 @@ static void __init mpc834x_sys_setup_arch(void)
 			loops_per_jiffy = 50000000 / HZ;
 		of_node_put(np);
 	}
+
 #ifdef CONFIG_PCI
 	for (np = NULL; (np = of_find_node_by_type(np, "pci")) != NULL;)
 		add_bridge(np);
@@ -72,6 +147,10 @@ static void __init mpc834x_sys_setup_arch(void)
 	ppc_md.pci_exclude_device = mpc83xx_exclude_device;
 #endif
 
+#ifdef CONFIG_834x_USB_SUPPORT
+	mpc834x_usb_cfg();
+#endif
+
 #ifdef  CONFIG_ROOT_NFS
 	ROOT_DEV = Root_NFS;
 #else
diff --git a/arch/powerpc/platforms/83xx/mpc834x_sys.h b/arch/powerpc/platforms/83xx/mpc834x_sys.h
index 7d5bbef..52f14a3 100644
--- a/arch/powerpc/platforms/83xx/mpc834x_sys.h
+++ b/arch/powerpc/platforms/83xx/mpc834x_sys.h
@@ -20,4 +20,6 @@
 #define PIRQC	MPC83xx_IRQ_EXT6
 #define PIRQD	MPC83xx_IRQ_EXT7
 
+#define BCSR5_INT_USB		0x02
+
 #endif				/* __MACH_MPC83XX_SYS_H__ */
diff --git a/arch/powerpc/platforms/83xx/mpc83xx.h b/arch/powerpc/platforms/83xx/mpc83xx.h
index 01cae10..9cd03b5 100644
--- a/arch/powerpc/platforms/83xx/mpc83xx.h
+++ b/arch/powerpc/platforms/83xx/mpc83xx.h
@@ -4,6 +4,24 @@
 #include <linux/init.h>
 #include <linux/device.h>
 
+/* System Clock Control Register */
+#define MPC83XX_SCCR_OFFS          0xA08
+#define MPC83XX_SCCR_USB_MPHCM_11  0x00c00000
+#define MPC83XX_SCCR_USB_MPHCM_01  0x00400000
+#define MPC83XX_SCCR_USB_MPHCM_10  0x00800000
+#define MPC83XX_SCCR_USB_DRCM_11   0x00300000
+#define MPC83XX_SCCR_USB_DRCM_01   0x00100000
+#define MPC83XX_SCCR_USB_DRCM_10   0x00200000
+
+/* system i/o configuration register low */
+#define MPC83XX_SICRL_OFFS         0x114
+#define MPC83XX_SICRL_USB0         0x40000000
+#define MPC83XX_SICRL_USB1         0x20000000
+
+/* system i/o configuration register high */
+#define MPC83XX_SICRH_OFFS         0x118
+#define MPC83XX_SICRH_USB_UTMI     0x00020000
+
 /*
  * Declaration for the various functions exported by the
  * mpc83xx_* files. Mostly for use by mpc83xx_setup

Re: [PATCH 1/3] add USB setup code for 8349emds PB

[Arnd Bergmann]

On Tuesday 06 February 2007 10:05, Li Yang wrote:
>  
> +static volatile u8 *bcsr_regs = NULL;

A 'static volatile u8*' type sounds very wrong. You use the variable
only in one function, so it should be local to that. There is normally
no reason to ever mark a variable volatile in driver code, instead
you want it to be '__iomem'. It should probably also be 'void *'
instead of 'u8 *', so it doesn't get dereferenced by accident.
Your references should be changed to use in_8/out_8 or readb/writeb,
depending on whether it's on-chip or PCI space.

> +	if ((np = of_find_compatible_node(np, "usb", "fsl-usb2-dr")) != NULL)
> +		port0_is_dr = 1;
> +	if ((np = of_find_compatible_node(np, "usb", "fsl-usb2-mph")) != NULL){
> +		if (port0_is_dr) {
> +			printk(KERN_WARNING
> +				"There is only one USB port on PB board! \n");
> +			return -1;
> +		} else if (!port0_is_dr)
> +			/* No usb port enabled */
> +			return -1;
> +	}

I'm not sure if scanning through the device tree to find mmio addresses
is still considered valid style. I would instead prefer you to provide a
of_platform_driver that attaches to the respective 'compatible' property
and does the register access in its probe() function in order to not
conflict with other drivers.

	Arnd <><

Re: [PATCH 1/3] add USB setup code for 8349emds PB

[Kumar Gala]

On Feb 6, 2007, at 3:05 AM, Li Yang wrote:

> Add board specific initialization code for USB
> to work in both MPH and DR mode for MPC8349EMDS
> PB board.
>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
>
> arch/powerpc/platforms/83xx/Kconfig       |    4 ++
> arch/powerpc/platforms/83xx/mpc834x_sys.c |   79 +++++++++++++++++++ 
> ++++++++++
> arch/powerpc/platforms/83xx/mpc834x_sys.h |    2 +
> arch/powerpc/platforms/83xx/mpc83xx.h     |   18 +++++++
> 4 files changed, 103 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/ 
> platforms/83xx/Kconfig
> index edcd5b8..5371645 100644
> --- a/arch/powerpc/platforms/83xx/Kconfig
> +++ b/arch/powerpc/platforms/83xx/Kconfig
> @@ -59,4 +59,8 @@ config PPC_MPC836x
> 	select PPC_INDIRECT_PCI
> 	default y if MPC8360E_PB
> +config 834x_USB_SUPPORT
> +	bool
> +	default y if MPC834x_SYS && (USB || USB_GADGET)
> +

I'm still not convinced we need a new config option for this.

> endmenu
> diff --git a/arch/powerpc/platforms/83xx/mpc834x_sys.c b/arch/ 
> powerpc/platforms/83xx/mpc834x_sys.c
> index 80b735a..aff7ae3 100644
> --- a/arch/powerpc/platforms/83xx/mpc834x_sys.c
> +++ b/arch/powerpc/platforms/83xx/mpc834x_sys.c
> @@ -35,14 +35,88 @@
> #include <asm/prom.h>
> #include <asm/udbg.h>
> #include <sysdev/fsl_soc.h>
> +#include <linux/fsl_devices.h>
> #include "mpc83xx.h"
> +#include "mpc834x_sys.h"
> #ifndef CONFIG_PCI
> unsigned long isa_io_base = 0;
> unsigned long isa_mem_base = 0;
> #endif
> +static volatile u8 *bcsr_regs = NULL;
> +
> +#ifdef CONFIG_834x_USB_SUPPORT
> +/* Note: This is only for PB, not for PB+PIB
> + * On PB only port0 is connected using ULPI */

Can you tell if we are on PIB by what type of phy is used?

> +static int mpc834x_usb_cfg(void)
> +{
> +	unsigned long sccr, sicrl;
> +	void __iomem *immap;
> +	struct device_node *np = NULL;
> +	int port0_is_dr = 0;
> +
> +	if ((np = of_find_compatible_node(np, "usb", "fsl-usb2-dr")) !=  
> NULL)
> +		port0_is_dr = 1;
> +	if ((np = of_find_compatible_node(np, "usb", "fsl-usb2-mph")) !=  
> NULL){
> +		if (port0_is_dr) {
> +			printk(KERN_WARNING
> +				"There is only one USB port on PB board! \n");
> +			return -1;
> +		} else if (!port0_is_dr)
> +			/* No usb port enabled */
> +			return -1;
> +	}
> +
> +	immap = ioremap(get_immrbase(), 0x100000);

Do we really need to remap 1M, both registers live in one 4k page 0x1000

> +	if (!immap)
> +		return -1;
> +
> +	/* Configure clock */
> +	sccr = in_be32(immap + MPC83XX_SCCR_OFFS);
> +	if (port0_is_dr)
> +		sccr |= MPC83XX_SCCR_USB_DRCM_11;  /* 1:3 */
> +	else
> +		sccr |= MPC83XX_SCCR_USB_MPHCM_11; /* 1:3 */
> +	out_be32(immap + MPC83XX_SCCR_OFFS, sccr);
> +
> +	/* Configure Pin */
> +	sicrl = in_be32(immap + MPC83XX_SICRL_OFFS);
> +	/* set port0 only */
> +	if (port0_is_dr)
> +		sicrl |= MPC83XX_SICRL_USB0;
> +	else
> +		sicrl &= ~(MPC83XX_SICRL_USB0);
> +	out_be32(immap + MPC83XX_SICRL_OFFS, sicrl);
> +
> +	iounmap(immap);
> +
> +	/* Map BCSR area */
> +	np = of_find_node_by_name(NULL, "bcsr");
> +	if (np != 0) {
> +		struct resource res;
> +
> +		of_address_to_resource(np, 0, &res);
> +		bcsr_regs = ioremap(res.start, res.end - res.start + 1);
> +		of_node_put(np);
> +	}
> +	if (!bcsr_regs)
> +		return -1;
> +
> +	/*
> +	 * if SYS board is plug into PIB board,
> +	 * force to use the PHY on SYS board
> +	 */
> +	if (!(bcsr_regs[5] & BCSR5_INT_USB))
> +		bcsr_regs[5] |= BCSR5_INT_USB;
> +	iounmap(bcsr_regs);
> +	return 0;
> +}
> +
> +#endif /* CONFIG_834x_USB_SUPPORT */
> +
> +
> /*  
> ********************************************************************** 
> **
>  *
>  * Setup the architecture
> @@ -65,6 +139,7 @@ static void __init mpc834x_sys_setup_arch(void)
> 			loops_per_jiffy = 50000000 / HZ;
> 		of_node_put(np);
> 	}
> +
> #ifdef CONFIG_PCI
> 	for (np = NULL; (np = of_find_node_by_type(np, "pci")) != NULL;)
> 		add_bridge(np);
> @@ -72,6 +147,10 @@ static void __init mpc834x_sys_setup_arch(void)
> 	ppc_md.pci_exclude_device = mpc83xx_exclude_device;
> #endif
> +#ifdef CONFIG_834x_USB_SUPPORT
> +	mpc834x_usb_cfg();
> +#endif
> +
> #ifdef  CONFIG_ROOT_NFS
> 	ROOT_DEV = Root_NFS;
> #else
> diff --git a/arch/powerpc/platforms/83xx/mpc834x_sys.h b/arch/ 
> powerpc/platforms/83xx/mpc834x_sys.h
> index 7d5bbef..52f14a3 100644
> --- a/arch/powerpc/platforms/83xx/mpc834x_sys.h
> +++ b/arch/powerpc/platforms/83xx/mpc834x_sys.h
> @@ -20,4 +20,6 @@
> #define PIRQC	MPC83xx_IRQ_EXT6
> #define PIRQD	MPC83xx_IRQ_EXT7
> +#define BCSR5_INT_USB		0x02
> +
> #endif				/* __MACH_MPC83XX_SYS_H__ */
> diff --git a/arch/powerpc/platforms/83xx/mpc83xx.h b/arch/powerpc/ 
> platforms/83xx/mpc83xx.h
> index 01cae10..9cd03b5 100644
> --- a/arch/powerpc/platforms/83xx/mpc83xx.h
> +++ b/arch/powerpc/platforms/83xx/mpc83xx.h
> @@ -4,6 +4,24 @@
> #include <linux/init.h>
> #include <linux/device.h>
> +/* System Clock Control Register */
> +#define MPC83XX_SCCR_OFFS          0xA08
> +#define MPC83XX_SCCR_USB_MPHCM_11  0x00c00000
> +#define MPC83XX_SCCR_USB_MPHCM_01  0x00400000
> +#define MPC83XX_SCCR_USB_MPHCM_10  0x00800000
> +#define MPC83XX_SCCR_USB_DRCM_11   0x00300000
> +#define MPC83XX_SCCR_USB_DRCM_01   0x00100000
> +#define MPC83XX_SCCR_USB_DRCM_10   0x00200000
> +
> +/* system i/o configuration register low */
> +#define MPC83XX_SICRL_OFFS         0x114
> +#define MPC83XX_SICRL_USB0         0x40000000
> +#define MPC83XX_SICRL_USB1         0x20000000
> +
> +/* system i/o configuration register high */
> +#define MPC83XX_SICRH_OFFS         0x118
> +#define MPC83XX_SICRH_USB_UTMI     0x00020000
> +
> /*
>  * Declaration for the various functions exported by the
>  * mpc83xx_* files. Mostly for use by mpc83xx_setup

Re: [PATCH 1/3] add USB setup code for 8349emds PB

[Kumar Gala]

On Feb 6, 2007, at 8:37 AM, Kumar Gala wrote:

>
> On Feb 6, 2007, at 3:05 AM, Li Yang wrote:
>
>> Add board specific initialization code for USB
>> to work in both MPH and DR mode for MPC8349EMDS
>> PB board.
>>
>> Signed-off-by: Li Yang <leoli@freescale.com>
>> ---
>>
>> arch/powerpc/platforms/83xx/Kconfig       |    4 ++
>> arch/powerpc/platforms/83xx/mpc834x_sys.c |   79 +++++++++++++++++++
>> ++++++++++
>> arch/powerpc/platforms/83xx/mpc834x_sys.h |    2 +
>> arch/powerpc/platforms/83xx/mpc83xx.h     |   18 +++++++
>> 4 files changed, 103 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/
>> platforms/83xx/Kconfig
>> index edcd5b8..5371645 100644
>> --- a/arch/powerpc/platforms/83xx/Kconfig
>> +++ b/arch/powerpc/platforms/83xx/Kconfig
>> @@ -59,4 +59,8 @@ config PPC_MPC836x
>> 	select PPC_INDIRECT_PCI
>> 	default y if MPC8360E_PB
>> +config 834x_USB_SUPPORT
>> +	bool
>> +	default y if MPC834x_SYS && (USB || USB_GADGET)
>> +
>
> I'm still not convinced we need a new config option for this.

Ok, I see what this now accomplishes, but why don't we just build the  
support in always, lets just remove the config option altogether?

- k

RE: [PATCH 1/3] add USB setup code for 8349emds PB

[Li Yang-r58472]

> > +	if ((np =3D of_find_compatible_node(np, "usb", "fsl-usb2-dr")) =
!=3D
NULL)
> > +		port0_is_dr =3D 1;
> > +	if ((np =3D of_find_compatible_node(np, "usb", "fsl-usb2-mph")) =
!=3D
NULL){
> > +		if (port0_is_dr) {
> > +			printk(KERN_WARNING
> > +				"There is only one USB port on PB board!
\n");
> > +			return -1;
> > +		} else if (!port0_is_dr)
> > +			/* No usb port enabled */
> > +			return -1;
> > +	}
>=20
> I'm not sure if scanning through the device tree to find mmio
addresses
> is still considered valid style. I would instead prefer you to provide
a
> of_platform_driver that attaches to the respective 'compatible'
property
> and does the register access in its probe() function in order to not
> conflict with other drivers.

The driver will not use of_platform_driver for it to be used through
multiple architectures.

- Leo

RE: [PATCH 1/3] add USB setup code for 8349emds PB

[Li Yang-r58472]

> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Wednesday, February 07, 2007 12:31 AM
> To: Kumar Gala
> Cc: Li Yang-r58472; linuxppc-dev@ozlabs.org; Paul
> Subject: Re: [PATCH 1/3] add USB setup code for 8349emds PB
>=20
>=20
> On Feb 6, 2007, at 8:37 AM, Kumar Gala wrote:
>=20
> >
> > On Feb 6, 2007, at 3:05 AM, Li Yang wrote:
> >
> >> Add board specific initialization code for USB
> >> to work in both MPH and DR mode for MPC8349EMDS
> >> PB board.
> >>
> >> Signed-off-by: Li Yang <leoli@freescale.com>
> >> ---
> >>
> >> arch/powerpc/platforms/83xx/Kconfig       |    4 ++
> >> arch/powerpc/platforms/83xx/mpc834x_sys.c |   79
+++++++++++++++++++
> >> ++++++++++
> >> arch/powerpc/platforms/83xx/mpc834x_sys.h |    2 +
> >> arch/powerpc/platforms/83xx/mpc83xx.h     |   18 +++++++
> >> 4 files changed, 103 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/
> >> platforms/83xx/Kconfig
> >> index edcd5b8..5371645 100644
> >> --- a/arch/powerpc/platforms/83xx/Kconfig
> >> +++ b/arch/powerpc/platforms/83xx/Kconfig
> >> @@ -59,4 +59,8 @@ config PPC_MPC836x
> >> 	select PPC_INDIRECT_PCI
> >> 	default y if MPC8360E_PB
> >> +config 834x_USB_SUPPORT
> >> +	bool
> >> +	default y if MPC834x_SYS && (USB || USB_GADGET)
> >> +
> >
> > I'm still not convinced we need a new config option for this.
>=20
> Ok, I see what this now accomplishes, but why don't we just build the
> support in always, lets just remove the config option altogether?

To save some space and time for embedded product which doesn't really
need usb?

- Leo

Re: [PATCH 1/3] add USB setup code for 8349emds PB

[Kumar Gala]

On Feb 6, 2007, at 8:27 PM, Li Yang-r58472 wrote:

>> -----Original Message-----
>> From: Kumar Gala [mailto:galak@kernel.crashing.org]
>> Sent: Wednesday, February 07, 2007 12:31 AM
>> To: Kumar Gala
>> Cc: Li Yang-r58472; linuxppc-dev@ozlabs.org; Paul
>> Subject: Re: [PATCH 1/3] add USB setup code for 8349emds PB
>>
>>
>> On Feb 6, 2007, at 8:37 AM, Kumar Gala wrote:
>>
>>>
>>> On Feb 6, 2007, at 3:05 AM, Li Yang wrote:
>>>
>>>> Add board specific initialization code for USB
>>>> to work in both MPH and DR mode for MPC8349EMDS
>>>> PB board.
>>>>
>>>> Signed-off-by: Li Yang <leoli@freescale.com>
>>>> ---
>>>>
>>>> arch/powerpc/platforms/83xx/Kconfig       |    4 ++
>>>> arch/powerpc/platforms/83xx/mpc834x_sys.c |   79
> +++++++++++++++++++
>>>> ++++++++++
>>>> arch/powerpc/platforms/83xx/mpc834x_sys.h |    2 +
>>>> arch/powerpc/platforms/83xx/mpc83xx.h     |   18 +++++++
>>>> 4 files changed, 103 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/
>>>> platforms/83xx/Kconfig
>>>> index edcd5b8..5371645 100644
>>>> --- a/arch/powerpc/platforms/83xx/Kconfig
>>>> +++ b/arch/powerpc/platforms/83xx/Kconfig
>>>> @@ -59,4 +59,8 @@ config PPC_MPC836x
>>>> 	select PPC_INDIRECT_PCI
>>>> 	default y if MPC8360E_PB
>>>> +config 834x_USB_SUPPORT
>>>> +	bool
>>>> +	default y if MPC834x_SYS && (USB || USB_GADGET)
>>>> +
>>>
>>> I'm still not convinced we need a new config option for this.
>>
>> Ok, I see what this now accomplishes, but why don't we just build the
>> support in always, lets just remove the config option altogether?
>
> To save some space and time for embedded product which doesn't really
> need usb?

Its not worth the savings.  We are talking about a reference board  
here, if someone doesn't want this init code in their board port they  
are free to remove it.

- k

RE: [PATCH 1/3] add USB setup code for 8349emds PB

[Li Yang-r58472]

>=20
> > endmenu
> > diff --git a/arch/powerpc/platforms/83xx/mpc834x_sys.c b/arch/
> > powerpc/platforms/83xx/mpc834x_sys.c
> > index 80b735a..aff7ae3 100644
> > --- a/arch/powerpc/platforms/83xx/mpc834x_sys.c
> > +++ b/arch/powerpc/platforms/83xx/mpc834x_sys.c
> > @@ -35,14 +35,88 @@
> > #include <asm/prom.h>
> > #include <asm/udbg.h>
> > #include <sysdev/fsl_soc.h>
> > +#include <linux/fsl_devices.h>
> > #include "mpc83xx.h"
> > +#include "mpc834x_sys.h"
> > #ifndef CONFIG_PCI
> > unsigned long isa_io_base =3D 0;
> > unsigned long isa_mem_base =3D 0;
> > #endif
> > +static volatile u8 *bcsr_regs =3D NULL;
> > +
> > +#ifdef CONFIG_834x_USB_SUPPORT
> > +/* Note: This is only for PB, not for PB+PIB
> > + * On PB only port0 is connected using ULPI */
>=20
> Can you tell if we are on PIB by what type of phy is used?

No, PIB supports all PHYs including ULPI.  We only deal with PB only
case for now, as PIB is far more complex.

- Leo

Re: [PATCH 1/3] add USB setup code for 8349emds PB

[Arnd Bergmann]

On Wednesday 07 February 2007 03:22, Li Yang-r58472 wrote:
> 
> > I'm not sure if scanning through the device tree to find mmio
> > addresses is still considered valid style. I would instead prefer
> > you to provide a of_platform_driver that attaches to the
> > respective 'compatible' property and does the register access
> > in its probe() function in order to not conflict with other drivers.
> 
> The driver will not use of_platform_driver for it to be used through
> multiple architectures.

Huh? I don't understand. You currently use of_find_compatible_node(),
which is also platform-specific.

	Arnd <><

RE: [PATCH 1/3] add USB setup code for 8349emds PB

[Li Yang-r58472]

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Wednesday, February 07, 2007 4:47 PM
> To: Li Yang-r58472
> Cc: linuxppc-dev@ozlabs.org; Paul
> Subject: Re: [PATCH 1/3] add USB setup code for 8349emds PB
>=20
> On Wednesday 07 February 2007 03:22, Li Yang-r58472 wrote:
> >
> > > I'm not sure if scanning through the device tree to find mmio
> > > addresses is still considered valid style. I would instead prefer
> > > you to provide a of_platform_driver that attaches to the
> > > respective 'compatible' property and does the register access
> > > in its probe() function in order to not conflict with other
drivers.
> >
> > The driver will not use of_platform_driver for it to be used through
> > multiple architectures.
>=20
> Huh? I don't understand. You currently use of_find_compatible_node(),
> which is also platform-specific.

Do you mean to create an of_plarform_driver here just to do the platform
initialization?  The true USB driver can only takes platform_device as
it is used by multiple architectures.
btw: We don't access the mmio address of the device here.  We just need
to read some property from the device tree.

- Leo

Re: [PATCH 1/3] add USB setup code for 8349emds PB

[Arnd Bergmann]

On Wednesday 07 February 2007 10:16, Li Yang-r58472 wrote:
> Do you mean to create an of_plarform_driver here just to do the platform
> initialization? =A0The true USB driver can only takes platform_device as
> it is used by multiple architectures.
> btw: We don't access the mmio address of the device here. =A0We just need
> to read some property from the device tree.

The usb host controllers already come with an abstraction layer, e.g. there
is ehci-fsl.c, ehci-au1xxx.c and some more. All powerpc bases systems that
have the host controller on a local bus instead of PCI should use a common
driver for that, I believe one has already been posted some time ago,
but I'm not entirely sure about that.

As mentioned by Kumar Gala, we should eventually migrate over the users
of ehci-fsl to a new ehci-of, but getting rid of ehci-fsl means removing
all its users in arch/ppc first.

	Arnd <><

RE: [PATCH 1/3] add USB setup code for 8349emds PB

[Li Yang-r58472]

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Wednesday, February 07, 2007 6:02 PM
> To: linuxppc-dev@ozlabs.org
> Cc: Li Yang-r58472; Paul
> Subject: Re: [PATCH 1/3] add USB setup code for 8349emds PB
>=20
> On Wednesday 07 February 2007 10:16, Li Yang-r58472 wrote:
> > Do you mean to create an of_plarform_driver here just to do the =
platform
> > initialization? =A0The true USB driver can only takes =
platform_device as
> > it is used by multiple architectures.
> > btw: We don't access the mmio address of the device here. =A0We just =
need
> > to read some property from the device tree.
>=20
> The usb host controllers already come with an abstraction layer, e.g. =
there
> is ehci-fsl.c, ehci-au1xxx.c and some more. All powerpc bases systems =
that
> have the host controller on a local bus instead of PCI should use a =
common
> driver for that, I believe one has already been posted some time ago,
> but I'm not entirely sure about that.
>=20
> As mentioned by Kumar Gala, we should eventually migrate over the =
users
> of ehci-fsl to a new ehci-of, but getting rid of ehci-fsl means =
removing
> all its users in arch/ppc first.

How about a non-PCI ehci driver for all architectures (such as arm, =
m68k)?  Then, platform_bus is a more generic interface than of_platform.

- Leo

Re: [PATCH 1/3] add USB setup code for 8349emds PB

[Arnd Bergmann]

On Wednesday 07 February 2007 11:10, Li Yang-r58472 wrote:
> How about a non-PCI ehci driver for all architectures (such as arm, m68k)?
> Then, platform_bus is a more generic interface than of_platform. 

Yes, that's right. The main difference is that if you scan all of devices
at boot time, you already have a node for the USB controller and you
shouldn't need to create another node that does not reflect the actual
hierarchy in the linux device tree.

A platform_device is really meant to be 'something that can't be probed
but we know it is there', like the interrupt controller on a PC.
With the of device tree, we can probe anything, so ideally, we should
not need to create any platform devices at all.

	Arnd <><

Re: [PATCH 1/3] add USB setup code for 8349emds PB

[Kumar Gala]

On Feb 7, 2007, at 4:44 AM, Arnd Bergmann wrote:

> On Wednesday 07 February 2007 11:10, Li Yang-r58472 wrote:
>> How about a non-PCI ehci driver for all architectures (such as  
>> arm, m68k)?
>> Then, platform_bus is a more generic interface than of_platform.
>
> Yes, that's right. The main difference is that if you scan all of  
> devices
> at boot time, you already have a node for the USB controller and you
> shouldn't need to create another node that does not reflect the actual
> hierarchy in the linux device tree.
>
> A platform_device is really meant to be 'something that can't be  
> probed
> but we know it is there', like the interrupt controller on a PC.
> With the of device tree, we can probe anything, so ideally, we should
> not need to create any platform devices at all.

I think the concern is how to handle the case on something like ARM w/ 
o the device-tree.  We can support both mechanisms in the driver

However, the bigger issue is getting so we can remove arch/ppc  
consumers.

- k

Re: [PATCH 1/3] add USB setup code for 8349emds PB

[Kumar Gala]

On Feb 6, 2007, at 11:47 PM, Li Yang wrote:

> Add board specific initialization code for USB
> to work in both MPH and DR mode for MPC8349EMDS
> PB board.
>
> Signed-off-by: Li Yang <leoli@freescale.com> ---
> arch/powerpc/platforms/83xx/mpc834x_sys.c |   74 +++++++++++++++++++ 
> ++++++++++
> arch/powerpc/platforms/83xx/mpc834x_sys.h |    2 +
> arch/powerpc/platforms/83xx/mpc83xx.h     |   18 +++++++
> 3 files changed, 94 insertions(+), 0 deletions(-)

applied

- k

[PATCH 1/3] add USB setup code for 8349emds PB

[Li Yang]

Add board specific initialization code for USB
to work in both MPH and DR mode for MPC8349EMDS
PB board.

Signed-off-by: Li Yang <leoli@freescale.com> 
---
 arch/powerpc/platforms/83xx/mpc834x_sys.c |   74 +++++++++++++++++++++++++++++
 arch/powerpc/platforms/83xx/mpc834x_sys.h |    2 +
 arch/powerpc/platforms/83xx/mpc83xx.h     |   18 +++++++
 3 files changed, 94 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/mpc834x_sys.c b/arch/powerpc/platforms/83xx/mpc834x_sys.c
index 80b735a..3d20360 100644
--- a/arch/powerpc/platforms/83xx/mpc834x_sys.c
+++ b/arch/powerpc/platforms/83xx/mpc834x_sys.c
@@ -35,14 +35,85 @@
 #include <asm/prom.h>
 #include <asm/udbg.h>
 #include <sysdev/fsl_soc.h>
+#include <linux/fsl_devices.h>
 
 #include "mpc83xx.h"
+#include "mpc834x_sys.h"
 
 #ifndef CONFIG_PCI
 unsigned long isa_io_base = 0;
 unsigned long isa_mem_base = 0;
 #endif
 
+/* Note: This is only for PB, not for PB+PIB
+ * On PB only port0 is connected using ULPI */
+static int mpc834x_usb_cfg(void)
+{
+	unsigned long sccr, sicrl;
+	void __iomem *immap;
+	void __iomem *bcsr_regs = NULL;
+	u8 bcsr5;
+	struct device_node *np = NULL;
+	int port0_is_dr = 0;
+
+	if ((np = of_find_compatible_node(np, "usb", "fsl-usb2-dr")) != NULL)
+		port0_is_dr = 1;
+	if ((np = of_find_compatible_node(np, "usb", "fsl-usb2-mph")) != NULL){
+		if (port0_is_dr) {
+			printk(KERN_WARNING
+				"There is only one USB port on PB board! \n");
+			return -1;
+		} else if (!port0_is_dr)
+			/* No usb port enabled */
+			return -1;
+	}
+
+	immap = ioremap(get_immrbase(), 0x100000);
+	if (!immap)
+		return -1;
+
+	/* Configure clock */
+	sccr = in_be32(immap + MPC83XX_SCCR_OFFS);
+	if (port0_is_dr)
+		sccr |= MPC83XX_SCCR_USB_DRCM_11;  /* 1:3 */
+	else
+		sccr |= MPC83XX_SCCR_USB_MPHCM_11; /* 1:3 */
+	out_be32(immap + MPC83XX_SCCR_OFFS, sccr);
+
+	/* Configure Pin */
+	sicrl = in_be32(immap + MPC83XX_SICRL_OFFS);
+	/* set port0 only */
+	if (port0_is_dr)
+		sicrl |= MPC83XX_SICRL_USB0;
+	else
+		sicrl &= ~(MPC83XX_SICRL_USB0);
+	out_be32(immap + MPC83XX_SICRL_OFFS, sicrl);
+
+	iounmap(immap);
+
+	/* Map BCSR area */
+	np = of_find_node_by_name(NULL, "bcsr");
+	if (np != 0) {
+		struct resource res;
+
+		of_address_to_resource(np, 0, &res);
+		bcsr_regs = ioremap(res.start, res.end - res.start + 1);
+		of_node_put(np);
+	}
+	if (!bcsr_regs)
+		return -1;
+
+	/*
+	 * if SYS board is plug into PIB board,
+	 * force to use the PHY on SYS board
+	 */
+	bcsr5 = in_8(bcsr_regs + 5);
+	if (!(bcsr5 & BCSR5_INT_USB))
+		out_8(bcsr_regs + 5, (bcsr5 | BCSR5_INT_USB));
+	iounmap(bcsr_regs);
+	return 0;
+}
+
 /* ************************************************************************
  *
  * Setup the architecture
@@ -65,6 +136,7 @@ static void __init mpc834x_sys_setup_arch(void)
 			loops_per_jiffy = 50000000 / HZ;
 		of_node_put(np);
 	}
+
 #ifdef CONFIG_PCI
 	for (np = NULL; (np = of_find_node_by_type(np, "pci")) != NULL;)
 		add_bridge(np);
@@ -72,6 +144,8 @@ static void __init mpc834x_sys_setup_arch(void)
 	ppc_md.pci_exclude_device = mpc83xx_exclude_device;
 #endif
 
+	mpc834x_usb_cfg();
+
 #ifdef  CONFIG_ROOT_NFS
 	ROOT_DEV = Root_NFS;
 #else
diff --git a/arch/powerpc/platforms/83xx/mpc834x_sys.h b/arch/powerpc/platforms/83xx/mpc834x_sys.h
index 7d5bbef..52f14a3 100644
--- a/arch/powerpc/platforms/83xx/mpc834x_sys.h
+++ b/arch/powerpc/platforms/83xx/mpc834x_sys.h
@@ -20,4 +20,6 @@
 #define PIRQC	MPC83xx_IRQ_EXT6
 #define PIRQD	MPC83xx_IRQ_EXT7
 
+#define BCSR5_INT_USB		0x02
+
 #endif				/* __MACH_MPC83XX_SYS_H__ */
diff --git a/arch/powerpc/platforms/83xx/mpc83xx.h b/arch/powerpc/platforms/83xx/mpc83xx.h
index 01cae10..9cd03b5 100644
--- a/arch/powerpc/platforms/83xx/mpc83xx.h
+++ b/arch/powerpc/platforms/83xx/mpc83xx.h
@@ -4,6 +4,24 @@
 #include <linux/init.h>
 #include <linux/device.h>
 
+/* System Clock Control Register */
+#define MPC83XX_SCCR_OFFS          0xA08
+#define MPC83XX_SCCR_USB_MPHCM_11  0x00c00000
+#define MPC83XX_SCCR_USB_MPHCM_01  0x00400000
+#define MPC83XX_SCCR_USB_MPHCM_10  0x00800000
+#define MPC83XX_SCCR_USB_DRCM_11   0x00300000
+#define MPC83XX_SCCR_USB_DRCM_01   0x00100000
+#define MPC83XX_SCCR_USB_DRCM_10   0x00200000
+
+/* system i/o configuration register low */
+#define MPC83XX_SICRL_OFFS         0x114
+#define MPC83XX_SICRL_USB0         0x40000000
+#define MPC83XX_SICRL_USB1         0x20000000
+
+/* system i/o configuration register high */
+#define MPC83XX_SICRH_OFFS         0x118
+#define MPC83XX_SICRH_USB_UTMI     0x00020000
+
 /*
  * Declaration for the various functions exported by the
  * mpc83xx_* files. Mostly for use by mpc83xx_setup

RE: [PATCH 1/3] add USB setup code for 8349emds PB

[Li Yang-r58472]

> >> Huh, it would look like:
> >>
> >> #if defined(CONFIG_USB) || defined(CONFIG_USB_GADGET).
> >
> > Are you sure we don't need CONFIG_USB_MODULE and
> > CONFIG_USB_GADGET_MODULE any more?  At least in some early kernel
> > version, they are needed explicitly.
>=20
> I'm not seeing any references to either CONFIG option.

There is no direct reference to the CONFIG_*_MODULE.  But if you
configure a driver to be module, the CONFIG_*_MODULE is defined not
CONFIG_*.  Here we need the init code when USB host or gadget is
selected either as built-in or module.

- Leo

RE: [PATCH 1/3] add USB setup code for 8349emds PB

[Li Yang-r58472]

> >>>>> diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/
> >>>>> platforms/83xx/Kconfig
> >>>>> index edcd5b8..5371645 100644
> >>>>> --- a/arch/powerpc/platforms/83xx/Kconfig
> >>>>> +++ b/arch/powerpc/platforms/83xx/Kconfig
> >>>>> @@ -59,4 +59,8 @@ config PPC_MPC836x
> >>>>> 	select PPC_INDIRECT_PCI
> >>>>> 	default y if MPC8360E_PB
> >>>>> +config 834x_USB_SUPPORT
> >>>>> +	bool
> >>>>> +	default y if MPC834x_SYS && (USB || USB_GADGET)
> >>>>> +
> >>>>
> >>>> Do we really need to introduce a new config option?
> >>>
> >>> It will be neater to make use of the Kconfig feature than judging
> > all
> >>> the macros and module variations.  It will be a very long #if
> > defined
> >>> the other way.
> >>
> >> Huh, it would look like:
> >>
> >> #if defined(CONFIG_USB) || defined(CONFIG_USB_GADGET).
> >
> > Are you sure we don't need CONFIG_USB_MODULE and
> > CONFIG_USB_GADGET_MODULE any more?  At least in some early kernel
> > version, they are needed explicitly.
>=20
> I'm not seeing any references to either CONFIG option.
>=20
> Also, I'm not seeing any code to support OTG for ehci/83xx... (in the
> kernel tree).  Are there patches for this?

The complete OTG protocol support is not submitted yet, but the OTG mode
support for ehci-fsl is submitted to linux-usb.

- Leo

Re: [PATCH 1/3] add USB setup code for 8349emds PB

[Kumar Gala]

On Feb 5, 2007, at 9:46 PM, Li Yang-r58472 wrote:

>> -----Original Message-----
>> From: Kumar Gala [mailto:galak@kernel.crashing.org]
>> Sent: Tuesday, February 06, 2007 11:31 AM
>> To: Li Yang-r58472
>> Cc: Paul; linuxppc-dev@ozlabs.org
>> Subject: Re: [PATCH 1/3] add USB setup code for 8349emds PB
>>
>>
>> On Feb 5, 2007, at 9:15 PM, Li Yang-r58472 wrote:
>>
>>>> -----Original Message-----
>>>> From: Kumar Gala [mailto:galak@kernel.crashing.org]
>>>> Sent: Monday, February 05, 2007 11:04 PM
>>>> To: Li Yang-r58472
>>>> Cc: Paul; linuxppc-dev@ozlabs.org
>>>> Subject: Re: [PATCH 1/3] add USB setup code for 8349emds PB
>>>>
>>>>
>>>> On Feb 5, 2007, at 3:09 AM, Li Yang wrote:
>>>>
>>>> You have no description about what this patch does and why its
>>>> needed.
>>>
>>> Add cpu and board specific initialization code for USB to work in
> both
>>> MPH and DR mode for MPC8349EMDS PB board.
>>>>
>>>>> Signed-off-by: Li Yang <leoli@freescale.com>
>>>>> ---
>>>>> arch/powerpc/platforms/83xx/Kconfig       |    4 ++
>>>>> arch/powerpc/platforms/83xx/mpc834x_sys.c |   77
> +++++++++++++++++++
>>>>> ++++++++++
>>>>> arch/powerpc/platforms/83xx/mpc834x_sys.h |   23 +++++++++
>>>>> 3 files changed, 104 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/
>>>>> platforms/83xx/Kconfig
>>>>> index edcd5b8..5371645 100644
>>>>> --- a/arch/powerpc/platforms/83xx/Kconfig
>>>>> +++ b/arch/powerpc/platforms/83xx/Kconfig
>>>>> @@ -59,4 +59,8 @@ config PPC_MPC836x
>>>>> 	select PPC_INDIRECT_PCI
>>>>> 	default y if MPC8360E_PB
>>>>> +config 834x_USB_SUPPORT
>>>>> +	bool
>>>>> +	default y if MPC834x_SYS && (USB || USB_GADGET)
>>>>> +
>>>>
>>>> Do we really need to introduce a new config option?
>>>
>>> It will be neater to make use of the Kconfig feature than judging
> all
>>> the macros and module variations.  It will be a very long #if
> defined
>>> the other way.
>>
>> Huh, it would look like:
>>
>> #if defined(CONFIG_USB) || defined(CONFIG_USB_GADGET).
>
> Are you sure we don't need CONFIG_USB_MODULE and
> CONFIG_USB_GADGET_MODULE any more?  At least in some early kernel
> version, they are needed explicitly.

I'm not seeing any references to either CONFIG option.

Also, I'm not seeing any code to support OTG for ehci/83xx... (in the  
kernel tree).  Are there patches for this?

- k

RE: [PATCH 1/3] add USB setup code for 8349emds PB

[Li Yang-r58472]

> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Tuesday, February 06, 2007 11:31 AM
> To: Li Yang-r58472
> Cc: Paul; linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH 1/3] add USB setup code for 8349emds PB
>=20
>=20
> On Feb 5, 2007, at 9:15 PM, Li Yang-r58472 wrote:
>=20
> >> -----Original Message-----
> >> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> >> Sent: Monday, February 05, 2007 11:04 PM
> >> To: Li Yang-r58472
> >> Cc: Paul; linuxppc-dev@ozlabs.org
> >> Subject: Re: [PATCH 1/3] add USB setup code for 8349emds PB
> >>
> >>
> >> On Feb 5, 2007, at 3:09 AM, Li Yang wrote:
> >>
> >> You have no description about what this patch does and why its
> >> needed.
> >
> > Add cpu and board specific initialization code for USB to work in
both
> > MPH and DR mode for MPC8349EMDS PB board.
> >>
> >>> Signed-off-by: Li Yang <leoli@freescale.com>
> >>> ---
> >>> arch/powerpc/platforms/83xx/Kconfig       |    4 ++
> >>> arch/powerpc/platforms/83xx/mpc834x_sys.c |   77
+++++++++++++++++++
> >>> ++++++++++
> >>> arch/powerpc/platforms/83xx/mpc834x_sys.h |   23 +++++++++
> >>> 3 files changed, 104 insertions(+), 0 deletions(-)
> >>>
> >>> diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/
> >>> platforms/83xx/Kconfig
> >>> index edcd5b8..5371645 100644
> >>> --- a/arch/powerpc/platforms/83xx/Kconfig
> >>> +++ b/arch/powerpc/platforms/83xx/Kconfig
> >>> @@ -59,4 +59,8 @@ config PPC_MPC836x
> >>> 	select PPC_INDIRECT_PCI
> >>> 	default y if MPC8360E_PB
> >>> +config 834x_USB_SUPPORT
> >>> +	bool
> >>> +	default y if MPC834x_SYS && (USB || USB_GADGET)
> >>> +
> >>
> >> Do we really need to introduce a new config option?
> >
> > It will be neater to make use of the Kconfig feature than judging
all
> > the macros and module variations.  It will be a very long #if
defined
> > the other way.
>=20
> Huh, it would look like:
>=20
> #if defined(CONFIG_USB) || defined(CONFIG_USB_GADGET).

Are you sure we don't need CONFIG_USB_MODULE and
CONFIG_USB_GADGET_MODULE any more?  At least in some early kernel
version, they are needed explicitly.

- Leo

Re: [PATCH 1/3] add USB setup code for 8349emds PB

[Kumar Gala]

On Feb 5, 2007, at 9:15 PM, Li Yang-r58472 wrote:

>> -----Original Message-----
>> From: Kumar Gala [mailto:galak@kernel.crashing.org]
>> Sent: Monday, February 05, 2007 11:04 PM
>> To: Li Yang-r58472
>> Cc: Paul; linuxppc-dev@ozlabs.org
>> Subject: Re: [PATCH 1/3] add USB setup code for 8349emds PB
>>
>>
>> On Feb 5, 2007, at 3:09 AM, Li Yang wrote:
>>
>> You have no description about what this patch does and why its  
>> needed.
>
> Add cpu and board specific initialization code for USB to work in both
> MPH and DR mode for MPC8349EMDS PB board.
>>
>>> Signed-off-by: Li Yang <leoli@freescale.com>
>>> ---
>>> arch/powerpc/platforms/83xx/Kconfig       |    4 ++
>>> arch/powerpc/platforms/83xx/mpc834x_sys.c |   77 +++++++++++++++++++
>>> ++++++++++
>>> arch/powerpc/platforms/83xx/mpc834x_sys.h |   23 +++++++++
>>> 3 files changed, 104 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/
>>> platforms/83xx/Kconfig
>>> index edcd5b8..5371645 100644
>>> --- a/arch/powerpc/platforms/83xx/Kconfig
>>> +++ b/arch/powerpc/platforms/83xx/Kconfig
>>> @@ -59,4 +59,8 @@ config PPC_MPC836x
>>> 	select PPC_INDIRECT_PCI
>>> 	default y if MPC8360E_PB
>>> +config 834x_USB_SUPPORT
>>> +	bool
>>> +	default y if MPC834x_SYS && (USB || USB_GADGET)
>>> +
>>
>> Do we really need to introduce a new config option?
>
> It will be neater to make use of the Kconfig feature than judging all
> the macros and module variations.  It will be a very long #if defined
> the other way.

Huh, it would look like:

#if defined(CONFIG_USB) || defined(CONFIG_USB_GADGET).

Not very long.

>>> @@ -65,6 +120,7 @@ static void __init mpc834x_sys_setup_arch(void)
>>> 			loops_per_jiffy = 50000000 / HZ;
>>> 		of_node_put(np);
>>> 	}
>>> +
>>> #ifdef CONFIG_PCI
>>> 	for (np = NULL; (np = of_find_node_by_type(np, "pci")) != NULL;)
>>> 		add_bridge(np);
>>> @@ -72,6 +128,27 @@ static void __init mpc834x_sys_setup_arch(void)
>>> 	ppc_md.pci_exclude_device = mpc83xx_exclude_device;
>>> #endif
>>> +#ifdef CONFIG_834x_USB_SUPPORT
>>> +	mpc834x_usb_cfg();
>>> +
>>> +	/* Map BCSR area */
>>> +	np = of_find_node_by_name(NULL, "bcsr");
>>> +	if (np != 0) {
>>> +		struct resource res;
>>> +
>>> +		of_address_to_resource(np, 0, &res);
>>> +		bcsr_regs = ioremap(res.start, res.end - res.start + 1);
>>> +		of_node_put(np);
>>> +	}
>>> +
>>> +	/*
>>> +	 * if SYS board is plug into PIB board,
>>> +	 * force to use the PHY on SYS board
>>> +	 * */
>>> +	if ((bcsr_regs != NULL) && !(bcsr_regs[5] & BCSR5_INT_USB))
>>> +		bcsr_regs[5] |= BCSR5_INT_USB;
>>
>> why not fold all this into mpc834x_usb_cfg() since its all board
>> related config code?
>
> Well, not quite.  The code in mpc834x_usb_cfg() is 834x CPU specific
> initialization.  The bcsr is board specific.

If that's the case what are they doing in mpc834x_sys.c which is a  
board specific file?

Rather, I see the setting for SCCR/SICRL as board specific.

- k

RE: [PATCH 1/3] add USB setup code for 8349emds PB

[Li Yang-r58472]

> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Monday, February 05, 2007 11:04 PM
> To: Li Yang-r58472
> Cc: Paul; linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH 1/3] add USB setup code for 8349emds PB
>=20
>=20
> On Feb 5, 2007, at 3:09 AM, Li Yang wrote:
>=20
> You have no description about what this patch does and why its needed.

Add cpu and board specific initialization code for USB to work in both
MPH and DR mode for MPC8349EMDS PB board.
>=20
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > ---
> > arch/powerpc/platforms/83xx/Kconfig       |    4 ++
> > arch/powerpc/platforms/83xx/mpc834x_sys.c |   77 +++++++++++++++++++
> > ++++++++++
> > arch/powerpc/platforms/83xx/mpc834x_sys.h |   23 +++++++++
> > 3 files changed, 104 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/
> > platforms/83xx/Kconfig
> > index edcd5b8..5371645 100644
> > --- a/arch/powerpc/platforms/83xx/Kconfig
> > +++ b/arch/powerpc/platforms/83xx/Kconfig
> > @@ -59,4 +59,8 @@ config PPC_MPC836x
> > 	select PPC_INDIRECT_PCI
> > 	default y if MPC8360E_PB
> > +config 834x_USB_SUPPORT
> > +	bool
> > +	default y if MPC834x_SYS && (USB || USB_GADGET)
> > +
>=20
> Do we really need to introduce a new config option?

It will be neater to make use of the Kconfig feature than judging all
the macros and module variations.  It will be a very long #if defined
the other way.

> > @@ -65,6 +120,7 @@ static void __init mpc834x_sys_setup_arch(void)
> > 			loops_per_jiffy =3D 50000000 / HZ;
> > 		of_node_put(np);
> > 	}
> > +
> > #ifdef CONFIG_PCI
> > 	for (np =3D NULL; (np =3D of_find_node_by_type(np, "pci")) !=3D =
NULL;)
> > 		add_bridge(np);
> > @@ -72,6 +128,27 @@ static void __init mpc834x_sys_setup_arch(void)
> > 	ppc_md.pci_exclude_device =3D mpc83xx_exclude_device;
> > #endif
> > +#ifdef CONFIG_834x_USB_SUPPORT
> > +	mpc834x_usb_cfg();
> > +
> > +	/* Map BCSR area */
> > +	np =3D of_find_node_by_name(NULL, "bcsr");
> > +	if (np !=3D 0) {
> > +		struct resource res;
> > +
> > +		of_address_to_resource(np, 0, &res);
> > +		bcsr_regs =3D ioremap(res.start, res.end - res.start + 1);
> > +		of_node_put(np);
> > +	}
> > +
> > +	/*
> > +	 * if SYS board is plug into PIB board,
> > +	 * force to use the PHY on SYS board
> > +	 * */
> > +	if ((bcsr_regs !=3D NULL) && !(bcsr_regs[5] & BCSR5_INT_USB))
> > +		bcsr_regs[5] |=3D BCSR5_INT_USB;
>=20
> why not fold all this into mpc834x_usb_cfg() since its all board
> related config code?

Well, not quite.  The code in mpc834x_usb_cfg() is 834x CPU specific
initialization.  The bcsr is board specific.

- Leo

Re: [PATCH 1/3] add USB setup code for 8349emds PB

[Kumar Gala]

On Feb 5, 2007, at 3:09 AM, Li Yang wrote:

You have no description about what this patch does and why its needed.

> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
> arch/powerpc/platforms/83xx/Kconfig       |    4 ++
> arch/powerpc/platforms/83xx/mpc834x_sys.c |   77 +++++++++++++++++++ 
> ++++++++++
> arch/powerpc/platforms/83xx/mpc834x_sys.h |   23 +++++++++
> 3 files changed, 104 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/ 
> platforms/83xx/Kconfig
> index edcd5b8..5371645 100644
> --- a/arch/powerpc/platforms/83xx/Kconfig
> +++ b/arch/powerpc/platforms/83xx/Kconfig
> @@ -59,4 +59,8 @@ config PPC_MPC836x
> 	select PPC_INDIRECT_PCI
> 	default y if MPC8360E_PB
> +config 834x_USB_SUPPORT
> +	bool
> +	default y if MPC834x_SYS && (USB || USB_GADGET)
> +

Do we really need to introduce a new config option?

> endmenu
> diff --git a/arch/powerpc/platforms/83xx/mpc834x_sys.c b/arch/ 
> powerpc/platforms/83xx/mpc834x_sys.c
> index 80b735a..9b6abe9 100644
> --- a/arch/powerpc/platforms/83xx/mpc834x_sys.c
> +++ b/arch/powerpc/platforms/83xx/mpc834x_sys.c
> @@ -35,14 +35,69 @@
> #include <asm/prom.h>
> #include <asm/udbg.h>
> #include <sysdev/fsl_soc.h>
> +#include <linux/fsl_devices.h>
> #include "mpc83xx.h"
> +#include "mpc834x_sys.h"
> #ifndef CONFIG_PCI
> unsigned long isa_io_base = 0;
> unsigned long isa_mem_base = 0;
> #endif
> +static volatile u8 *bcsr_regs = NULL;
> +
> +#ifdef CONFIG_834x_USB_SUPPORT
> +/* Note: This is only for PB, not for PB+PIB
> + * On PB only port0 is connected using ULPI */
> +static int mpc834x_usb_cfg(void)
> +{
> +	unsigned long sccr, sicrl;
> +	volatile unsigned long *p;
> +	unsigned long __iomem *immap;
> +	struct device_node *np = NULL;
> +	int port0_is_dr = 0;
> +
> +	if ((np = of_find_compatible_node(np, "usb", "fsl-usb2-dr")) !=  
> NULL)
> +		port0_is_dr = 1;
> +	if ((np = of_find_compatible_node(np, "usb", "fsl-usb2-mph")) !=  
> NULL){
> +		if (port0_is_dr) {
> +			printk(KERN_WARNING
> +				"There is only one USB port on PB board! \n");
> +			return -1;
> +		} else if (!port0_is_dr)
> +			/* No usb port enabled */
> +			return -1;
> +	}
> +
> +	immap = ioremap(get_immrbase(), 0x100000);

How about checking if ioremap returned something valid.

> +
> +	/* Configure clock */
> +	p = (volatile unsigned long *)((u32)immap + MPC83XX_SCCR_OFFS);
> +	sccr = *p;

Don't do direct pointer access like this, use in_be32/out_be32

> +	if (port0_is_dr)
> +		sccr |= MPC83XX_SCCR_USB_DRCM_11;  /* 1:3 */
> +	else
> +		sccr |= MPC83XX_SCCR_USB_MPHCM_11; /* 1:3 */
> +	*p = sccr;
> +
> +	/* Configure Pin */
> +	p = (volatile unsigned long *)((u32)immap + MPC83XX_SICRL_OFFS);
> +	sicrl = *p;
> +	/* set port0 only */
> +	if (port0_is_dr)
> +		sicrl |= MPC83XX_SICRL_USB0;
> +	else
> +		sicrl &= ~(MPC83XX_SICRL_USB0);
> +	*p = sicrl;
> +
> +	iounmap(immap);
> +	return 0;
> +}
> +
> +#endif /* CONFIG_834x_USB_SUPPORT */
> +
> +
> /*  
> ********************************************************************** 
> **
>  *
>  * Setup the architecture
> @@ -65,6 +120,7 @@ static void __init mpc834x_sys_setup_arch(void)
> 			loops_per_jiffy = 50000000 / HZ;
> 		of_node_put(np);
> 	}
> +
> #ifdef CONFIG_PCI
> 	for (np = NULL; (np = of_find_node_by_type(np, "pci")) != NULL;)
> 		add_bridge(np);
> @@ -72,6 +128,27 @@ static void __init mpc834x_sys_setup_arch(void)
> 	ppc_md.pci_exclude_device = mpc83xx_exclude_device;
> #endif
> +#ifdef CONFIG_834x_USB_SUPPORT
> +	mpc834x_usb_cfg();
> +
> +	/* Map BCSR area */
> +	np = of_find_node_by_name(NULL, "bcsr");
> +	if (np != 0) {
> +		struct resource res;
> +
> +		of_address_to_resource(np, 0, &res);
> +		bcsr_regs = ioremap(res.start, res.end - res.start + 1);
> +		of_node_put(np);
> +	}
> +
> +	/*
> +	 * if SYS board is plug into PIB board,
> +	 * force to use the PHY on SYS board
> +	 * */
> +	if ((bcsr_regs != NULL) && !(bcsr_regs[5] & BCSR5_INT_USB))
> +		bcsr_regs[5] |= BCSR5_INT_USB;

why not fold all this into mpc834x_usb_cfg() since its all board  
related config code?

iounmap(bcsr) ?

> +#endif
> +
> #ifdef  CONFIG_ROOT_NFS
> 	ROOT_DEV = Root_NFS;
> #else
> diff --git a/arch/powerpc/platforms/83xx/mpc834x_sys.h b/arch/ 
> powerpc/platforms/83xx/mpc834x_sys.h
> index 7d5bbef..c2786fe 100644
> --- a/arch/powerpc/platforms/83xx/mpc834x_sys.h
> +++ b/arch/powerpc/platforms/83xx/mpc834x_sys.h
> @@ -20,4 +20,27 @@
> #define PIRQC	MPC83xx_IRQ_EXT6
> #define PIRQD	MPC83xx_IRQ_EXT7
> +#define BCSR_PHYS_ADDR		((uint)0xf8000000)
> +#define BCSR_SIZE		((uint)(32 * 1024))
> + +#define BCSR5_OFF		0x05
> +#define BCSR5_INT_USB		0x02
> +
> +#define MPC83XX_SCCR_OFFS          0xA08
> +#define MPC83XX_SCCR_USB_MPHCM_11  0x00c00000
> +#define MPC83XX_SCCR_USB_MPHCM_01  0x00400000
> +#define MPC83XX_SCCR_USB_MPHCM_10  0x00800000
> +#define MPC83XX_SCCR_USB_DRCM_11   0x00300000
> +#define MPC83XX_SCCR_USB_DRCM_01   0x00100000
> +#define MPC83XX_SCCR_USB_DRCM_10   0x00200000
> +
> +/* system i/o configuration register low */
> +#define MPC83XX_SICRL_OFFS         0x114
> +#define MPC83XX_SICRL_USB0         0x40000000
> +#define MPC83XX_SICRL_USB1         0x20000000
> +
> +/* system i/o configuration register high */
> +#define MPC83XX_SICRH_OFFS         0x118
> +#define MPC83XX_SICRH_USB_UTMI     0x00020000

You should stick these defines in mpc83xx.h so they are available to  
everyone.

- k

[PATCH 1/3] add USB setup code for 8349emds PB

[Li Yang]

Signed-off-by: Li Yang <leoli@freescale.com>
---
 arch/powerpc/platforms/83xx/Kconfig       |    4 ++
 arch/powerpc/platforms/83xx/mpc834x_sys.c |   77 +++++++++++++++++++++++++++++
 arch/powerpc/platforms/83xx/mpc834x_sys.h |   23 +++++++++
 3 files changed, 104 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/platforms/83xx/Kconfig
index edcd5b8..5371645 100644
--- a/arch/powerpc/platforms/83xx/Kconfig
+++ b/arch/powerpc/platforms/83xx/Kconfig
@@ -59,4 +59,8 @@ config PPC_MPC836x
 	select PPC_INDIRECT_PCI
 	default y if MPC8360E_PB
 
+config 834x_USB_SUPPORT
+	bool
+	default y if MPC834x_SYS && (USB || USB_GADGET)
+
 endmenu
diff --git a/arch/powerpc/platforms/83xx/mpc834x_sys.c b/arch/powerpc/platforms/83xx/mpc834x_sys.c
index 80b735a..9b6abe9 100644
--- a/arch/powerpc/platforms/83xx/mpc834x_sys.c
+++ b/arch/powerpc/platforms/83xx/mpc834x_sys.c
@@ -35,14 +35,69 @@
 #include <asm/prom.h>
 #include <asm/udbg.h>
 #include <sysdev/fsl_soc.h>
+#include <linux/fsl_devices.h>
 
 #include "mpc83xx.h"
+#include "mpc834x_sys.h"
 
 #ifndef CONFIG_PCI
 unsigned long isa_io_base = 0;
 unsigned long isa_mem_base = 0;
 #endif
 
+static volatile u8 *bcsr_regs = NULL;
+
+#ifdef CONFIG_834x_USB_SUPPORT
+/* Note: This is only for PB, not for PB+PIB
+ * On PB only port0 is connected using ULPI */
+static int mpc834x_usb_cfg(void)
+{
+	unsigned long sccr, sicrl;
+	volatile unsigned long *p;
+	unsigned long __iomem *immap;
+	struct device_node *np = NULL;
+	int port0_is_dr = 0;
+
+	if ((np = of_find_compatible_node(np, "usb", "fsl-usb2-dr")) != NULL)
+		port0_is_dr = 1;
+	if ((np = of_find_compatible_node(np, "usb", "fsl-usb2-mph")) != NULL){
+		if (port0_is_dr) {
+			printk(KERN_WARNING
+				"There is only one USB port on PB board! \n");
+			return -1;
+		} else if (!port0_is_dr)
+			/* No usb port enabled */
+			return -1;
+	}
+
+	immap = ioremap(get_immrbase(), 0x100000);
+
+	/* Configure clock */
+	p = (volatile unsigned long *)((u32)immap + MPC83XX_SCCR_OFFS);
+	sccr = *p;
+	if (port0_is_dr)
+		sccr |= MPC83XX_SCCR_USB_DRCM_11;  /* 1:3 */
+	else
+		sccr |= MPC83XX_SCCR_USB_MPHCM_11; /* 1:3 */
+	*p = sccr;
+
+	/* Configure Pin */
+	p = (volatile unsigned long *)((u32)immap + MPC83XX_SICRL_OFFS);
+	sicrl = *p;
+	/* set port0 only */
+	if (port0_is_dr)
+		sicrl |= MPC83XX_SICRL_USB0;
+	else
+		sicrl &= ~(MPC83XX_SICRL_USB0);
+	*p = sicrl;
+
+	iounmap(immap);
+	return 0;
+}
+
+#endif /* CONFIG_834x_USB_SUPPORT */
+
+
 /* ************************************************************************
  *
  * Setup the architecture
@@ -65,6 +120,7 @@ static void __init mpc834x_sys_setup_arch(void)
 			loops_per_jiffy = 50000000 / HZ;
 		of_node_put(np);
 	}
+
 #ifdef CONFIG_PCI
 	for (np = NULL; (np = of_find_node_by_type(np, "pci")) != NULL;)
 		add_bridge(np);
@@ -72,6 +128,27 @@ static void __init mpc834x_sys_setup_arch(void)
 	ppc_md.pci_exclude_device = mpc83xx_exclude_device;
 #endif
 
+#ifdef CONFIG_834x_USB_SUPPORT
+	mpc834x_usb_cfg();
+
+	/* Map BCSR area */
+	np = of_find_node_by_name(NULL, "bcsr");
+	if (np != 0) {
+		struct resource res;
+
+		of_address_to_resource(np, 0, &res);
+		bcsr_regs = ioremap(res.start, res.end - res.start + 1);
+		of_node_put(np);
+	}
+
+	/*
+	 * if SYS board is plug into PIB board,
+	 * force to use the PHY on SYS board
+	 * */
+	if ((bcsr_regs != NULL) && !(bcsr_regs[5] & BCSR5_INT_USB))
+		bcsr_regs[5] |= BCSR5_INT_USB;
+#endif
+
 #ifdef  CONFIG_ROOT_NFS
 	ROOT_DEV = Root_NFS;
 #else
diff --git a/arch/powerpc/platforms/83xx/mpc834x_sys.h b/arch/powerpc/platforms/83xx/mpc834x_sys.h
index 7d5bbef..c2786fe 100644
--- a/arch/powerpc/platforms/83xx/mpc834x_sys.h
+++ b/arch/powerpc/platforms/83xx/mpc834x_sys.h
@@ -20,4 +20,27 @@
 #define PIRQC	MPC83xx_IRQ_EXT6
 #define PIRQD	MPC83xx_IRQ_EXT7
 
+#define BCSR_PHYS_ADDR		((uint)0xf8000000)
+#define BCSR_SIZE		((uint)(32 * 1024))
+ 
+#define BCSR5_OFF		0x05
+#define BCSR5_INT_USB		0x02
+
+#define MPC83XX_SCCR_OFFS          0xA08
+#define MPC83XX_SCCR_USB_MPHCM_11  0x00c00000
+#define MPC83XX_SCCR_USB_MPHCM_01  0x00400000
+#define MPC83XX_SCCR_USB_MPHCM_10  0x00800000
+#define MPC83XX_SCCR_USB_DRCM_11   0x00300000
+#define MPC83XX_SCCR_USB_DRCM_01   0x00100000
+#define MPC83XX_SCCR_USB_DRCM_10   0x00200000
+
+/* system i/o configuration register low */
+#define MPC83XX_SICRL_OFFS         0x114
+#define MPC83XX_SICRL_USB0         0x40000000
+#define MPC83XX_SICRL_USB1         0x20000000
+
+/* system i/o configuration register high */
+#define MPC83XX_SICRH_OFFS         0x118
+#define MPC83XX_SICRH_USB_UTMI     0x00020000
+
 #endif				/* __MACH_MPC83XX_SYS_H__ */