[PATCH] Add USB to MPC8349 PB platform support

[PATCH] Add USB to MPC8349 PB platform support

[Li Yang]

This adds USB platform support to MPC8349 PB.  It works with the fsl_usb2_udc driver.

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

 arch/powerpc/platforms/83xx/Kconfig       |   4  ++
 arch/powerpc/platforms/83xx/mpc834x_sys.c |   72 +++++++++++++++++++++++++++++
 arch/powerpc/platforms/83xx/mpc834x_sys.h |   24 ++++++++++
 3 files changed, 113 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/platforms/83xx/Kconfig
index 7675e67..8404cdf 100644
--- a/arch/powerpc/platforms/83xx/Kconfig
+++ b/arch/powerpc/platforms/83xx/Kconfig
@@ -24,4 +31,14 @@ config MPC834x
 	select PPC_INDIRECT_PCI
 	default y if MPC834x_SYS

+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 7e789d2..f10d4ae 100644
--- a/arch/powerpc/platforms/83xx/mpc834x_sys.c
+++ b/arch/powerpc/platforms/83xx/mpc834x_sys.c
@@ -36,6 +36,7 @@ #include <asm/irq.h>
 #include <asm/prom.h>
 #include <asm/udbg.h>
 #include <sysdev/fsl_soc.h>
+#include <linux/fsl_devices.h>
 
 #include "mpc83xx.h"
 
@@ -71,6 +72,72 @@ mpc83xx_map_irq(struct pci_dev *dev, uns
 }
 #endif				/* CONFIG_PCI */
 
+#ifdef CONFIG_834x_USB_SUPPORT
+void mpc834x_usb_board_cfg(void)
+{
+	unsigned char __iomem *bcsr;
+	volatile unsigned char *bcsr5_p;
+
+	/*
+	 * if SYS board is plug into PIB board,
+	 * force to use the PHY on SYS board
+	 * */
+	bcsr = ioremap(BCSR_PHYS_ADDR, BCSR_SIZE);
+	bcsr5_p = bcsr + BCSR5_OFF;
+	if ( (*bcsr5_p & BCSR5_INT_USB) == 0 )
+		*bcsr5_p = (*bcsr5_p | BCSR5_INT_USB);
+	iounmap(bcsr);
+}
+
+/* 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
@@ -102,6 +169,11 @@ #ifdef CONFIG_PCI
 	ppc_md.pci_exclude_device = mpc83xx_exclude_device;
 #endif
 
+#ifdef CONFIG_834x_USB_SUPPORT
+	mpc834x_usb_cfg();
+	mpc834x_usb_board_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 fedecb7..30e45e8 100644
--- a/arch/powerpc/platforms/83xx/mpc834x_sys.h
+++ b/arch/powerpc/platforms/83xx/mpc834x_sys.h
@@ -20,4 +20,28 @@ #define PIRQB	MPC83xx_IRQ_EXT5
 #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__ */

Re: [PATCH] Add USB to MPC8349 PB platform support

[Kumar Gala]

Nack, my expectation is this is all setup by the boot loader.

- k

On Jul 14, 2006, at 6:52 AM, Li Yang wrote:

> This adds USB platform support to MPC8349 PB.  It works with the  
> fsl_usb2_udc driver.
>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
>
> arch/powerpc/platforms/83xx/Kconfig       |   4  ++
> arch/powerpc/platforms/83xx/mpc834x_sys.c |   72 +++++++++++++++++++ 
> ++++++++++
> arch/powerpc/platforms/83xx/mpc834x_sys.h |   24 ++++++++++
> 3 files changed, 113 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/ 
> platforms/83xx/Kconfig
> index 7675e67..8404cdf 100644
> --- a/arch/powerpc/platforms/83xx/Kconfig
> +++ b/arch/powerpc/platforms/83xx/Kconfig
> @@ -24,4 +31,14 @@ config MPC834x
> 	select PPC_INDIRECT_PCI
> 	default y if MPC834x_SYS
>
> +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 7e789d2..f10d4ae 100644
> --- a/arch/powerpc/platforms/83xx/mpc834x_sys.c
> +++ b/arch/powerpc/platforms/83xx/mpc834x_sys.c
> @@ -36,6 +36,7 @@ #include <asm/irq.h>
> #include <asm/prom.h>
> #include <asm/udbg.h>
> #include <sysdev/fsl_soc.h>
> +#include <linux/fsl_devices.h>
> #include "mpc83xx.h"
> @@ -71,6 +72,72 @@ mpc83xx_map_irq(struct pci_dev *dev, uns
> }
> #endif				/* CONFIG_PCI */
> +#ifdef CONFIG_834x_USB_SUPPORT
> +void mpc834x_usb_board_cfg(void)
> +{
> +	unsigned char __iomem *bcsr;
> +	volatile unsigned char *bcsr5_p;
> +
> +	/*
> +	 * if SYS board is plug into PIB board,
> +	 * force to use the PHY on SYS board
> +	 * */
> +	bcsr = ioremap(BCSR_PHYS_ADDR, BCSR_SIZE);
> +	bcsr5_p = bcsr + BCSR5_OFF;
> +	if ( (*bcsr5_p & BCSR5_INT_USB) == 0 )
> +		*bcsr5_p = (*bcsr5_p | BCSR5_INT_USB);
> +	iounmap(bcsr);
> +}
> +
> +/* 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
> @@ -102,6 +169,11 @@ #ifdef CONFIG_PCI
> 	ppc_md.pci_exclude_device = mpc83xx_exclude_device;
> #endif
> +#ifdef CONFIG_834x_USB_SUPPORT
> +	mpc834x_usb_cfg();
> +	mpc834x_usb_board_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 fedecb7..30e45e8 100644
> --- a/arch/powerpc/platforms/83xx/mpc834x_sys.h
> +++ b/arch/powerpc/platforms/83xx/mpc834x_sys.h
> @@ -20,4 +20,28 @@ #define PIRQB	MPC83xx_IRQ_EXT5
> #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__ */
>
>
>

Re: [PATCH] Add USB to MPC8349 PB platform support

[Li Yang]

On 7/14/06, Kumar Gala <galak@kernel.crashing.org> wrote:
> Nack, my expectation is this is all setup by the boot loader.

That's a good wish. ;)  However, USB is not required by bootloader. So
it is not likely to be initialized there.  And if we put it in
bootloader, it will be hard to change the mode(MPH/DR), which requires
a re-burn of bootloader.  It's better that we make sure it's correctly
configured here.
>
> - k
>
> On Jul 14, 2006, at 6:52 AM, Li Yang wrote:
>
> > This adds USB platform support to MPC8349 PB.  It works with the
> > fsl_usb2_udc driver.
> >
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > ---

Re: [PATCH] Add USB to MPC8349 PB platform support

[Kumar Gala]

On Jul 14, 2006, at 11:21 AM, Li Yang wrote:

> On 7/14/06, Kumar Gala <galak@kernel.crashing.org> wrote:
>> Nack, my expectation is this is all setup by the boot loader.
>
> That's a good wish. ;)  However, USB is not required by bootloader. So
> it is not likely to be initialized there.  And if we put it in
> bootloader, it will be hard to change the mode(MPH/DR), which requires
> a re-burn of bootloader.  It's better that we make sure it's correctly
> configured here.

I disagree.  You are coming from this from a board that does  
everything under the sun.  I'd like to avoid having this type of  
initialization in the kernel.  There is a whole additional kitchen  
sink that could move into the kernel as well.

- kumar 
  

Re: [PATCH] Add USB to MPC8349 PB platform support

[Hollis Blanchard]

On Mon, 2006-07-17 at 14:16 -0500, Kumar Gala wrote:
> On Jul 14, 2006, at 11:21 AM, Li Yang wrote:
> 
> > On 7/14/06, Kumar Gala <galak@kernel.crashing.org> wrote:
> >> Nack, my expectation is this is all setup by the boot loader.
> >
> > That's a good wish. ;)  However, USB is not required by bootloader. So
> > it is not likely to be initialized there.  And if we put it in
> > bootloader, it will be hard to change the mode(MPH/DR), which requires
> > a re-burn of bootloader.  It's better that we make sure it's correctly
> > configured here.
> 
> I disagree.  You are coming from this from a board that does  
> everything under the sun.  I'd like to avoid having this type of  
> initialization in the kernel.  There is a whole additional kitchen  
> sink that could move into the kernel as well.

Seems to me that it's far better to have init code in the kernel than
firmware. For one example, look at the x86 video card init problem
PowerPC Linux has. It's also far easier to fix/deploy Linux code than
firmware code, as Li observed, and on top of that it's less work for
non-UBoot firmwares in the future.

-Hollis

Re: [linux-usb-devel] [PATCH] Add USB to MPC8349 PB platform support

[David Brownell]

On Monday 17 July 2006 1:08 pm, Hollis Blanchard wrote:
>
> Seems to me that it's far better to have init code in the kernel than
> firmware.

If there's a general Linux policy on the issue, I think that'd be it.

Plus, remember that boot firmware _can't_ always be changed/bugfixed;
sometimes it can, but not as a general rule.

- Dave


> For one example, look at the x86 video card init problem 
> PowerPC Linux has. It's also far easier to fix/deploy Linux code than
> firmware code, as Li observed, and on top of that it's less work for
> non-UBoot firmwares in the future.
> 
> -Hollis
> 

RE: [linux-usb-devel] [PATCH] Add USB to MPC8349 PB platform support

[Li Yang-r58472]

> -----Original Message-----
> From: David Brownell [mailto:david-b@pacbell.net]
> Sent: Tuesday, July 18, 2006 6:58 AM
> To: linux-usb-devel@lists.sourceforge.net
> Cc: Hollis Blanchard; Kumar Gala; linuxppc-dev@ozlabs.org; Li
Yang-r58472
> Subject: Re: [linux-usb-devel] [PATCH] Add USB to MPC8349 PB platform
support
>=20
> On Monday 17 July 2006 1:08 pm, Hollis Blanchard wrote:
> >
> > Seems to me that it's far better to have init code in the kernel
than
> > firmware.
>=20
> If there's a general Linux policy on the issue, I think that'd be it.

Do we have a general policy for this now?
>=20
> Plus, remember that boot firmware _can't_ always be changed/bugfixed;
> sometimes it can, but not as a general rule.
>=20
> - Dave
>=20
>=20
> > For one example, look at the x86 video card init problem
> > PowerPC Linux has. It's also far easier to fix/deploy Linux code
than
> > firmware code, as Li observed, and on top of that it's less work for
> > non-UBoot firmwares in the future.
> >
> > -Hollis
> >

Re: [PATCH] Add USB to MPC8349 PB platform support

[Timur Tabi]

Hollis Blanchard wrote:

> Seems to me that it's far better to have init code in the kernel than
> firmware. For one example, look at the x86 video card init problem
> PowerPC Linux has. It's also far easier to fix/deploy Linux code than
> firmware code, as Li observed, and on top of that it's less work for
> non-UBoot firmwares in the future.

I also agree 100%.  The boot loader should only be required to initialize whatever hardware it uses.  The kernel should be required to initialize whatever hardware it supports.  The kernel should not assume that USB or PCI or whatever has already been initialized by the boot loader.  This doesn't happen on x86, so we should it happen on PPC?

-- 
Timur Tabi
Linux Kernel Developer @ Freescale

Re: [PATCH] Add USB to MPC8349 PB platform support

[Kumar Gala]

On Jul 19, 2006, at 3:13 PM, Timur Tabi wrote:

> Hollis Blanchard wrote:
>
>> Seems to me that it's far better to have init code in the kernel than
>> firmware. For one example, look at the x86 video card init problem
>> PowerPC Linux has. It's also far easier to fix/deploy Linux code than
>> firmware code, as Li observed, and on top of that it's less work for
>> non-UBoot firmwares in the future.
>
> I also agree 100%.  The boot loader should only be required to  
> initialize whatever hardware it uses.  The kernel should be  
> required to initialize whatever hardware it supports.  The kernel  
> should not assume that USB or PCI or whatever has already been  
> initialized by the boot loader.  This doesn't happen on x86, so we  
> should it happen on PPC?

Let's not drag what x86 does as a good or bad example of anything :)

- k

Re: [PATCH] Add USB to MPC8349 PB platform support

[Dan Malek]

On Jul 17, 2006, at 3:16 PM, Kumar Gala wrote:

> I disagree.  You are coming from this from a board that does
> everything under the sun.  I'd like to avoid having this type of
> initialization in the kernel.  There is a whole additional kitchen
> sink that could move into the kernel as well.

Well, I'm going to have to disagree with your disagreement :-)
The kernel should not assume things are properly initialized
and rely on the boot rom to do such things.  I have several
reasons for this.  One is that we are always pressed to make
embedded systems boot more quickly, and taking time to
initialize things in the boot rom just makes that a totally
inflexible system design.  We don't need to initialize things
we don't use, or can postpone until later.  Two, it makes
us dependent upon a particular boot rom, or boot rom
behavior, that not all boards may choose to support.
Three, board designs may have external logic that requires
a certain start up sequence or control register access
that complicates the boot rom in it's ability to share
code or implementation.

There are more, but I think you see the trend.  In my
years of doing this kind of development, you can't
assume a boot rom is going to do much more than initialize
memory and load the kernel.  I prefer the flexibility
to be in the kernel, and not in the boot rom, because it
is so much easier to develop and control.

Thanks.

	-- Dan

Re: [PATCH] Add USB to MPC8349 PB platform support

[Kumar Gala]

On Jul 17, 2006, at 3:17 PM, Dan Malek wrote:

>
> On Jul 17, 2006, at 3:16 PM, Kumar Gala wrote:
>
>> I disagree.  You are coming from this from a board that does
>> everything under the sun.  I'd like to avoid having this type of
>> initialization in the kernel.  There is a whole additional kitchen
>> sink that could move into the kernel as well.
>
> Well, I'm going to have to disagree with your disagreement :-)
> The kernel should not assume things are properly initialized
> and rely on the boot rom to do such things.  I have several
> reasons for this.  One is that we are always pressed to make
> embedded systems boot more quickly, and taking time to
> initialize things in the boot rom just makes that a totally
> inflexible system design.  We don't need to initialize things
> we don't use, or can postpone until later.  Two, it makes
> us dependent upon a particular boot rom, or boot rom
> behavior, that not all boards may choose to support.
> Three, board designs may have external logic that requires
> a certain start up sequence or control register access
> that complicates the boot rom in it's ability to share
> code or implementation.

Well, I think there is a coupling that exists between whatever your  
boot rom is and the kernel.  If you are trying to optimize boot time  
I'd say one thing you would want is to avoid multiple writing the  
same configuration registers.

I dont have an issue if a fixed function board decides to do these  
things in their kernel init instead of their boot rom.  I however,  
don't want thousand and one config options to support all the various  
ways one can configure the Freescale board.

> There are more, but I think you see the trend.  In my
> years of doing this kind of development, you can't
> assume a boot rom is going to do much more than initialize
> memory and load the kernel.  I prefer the flexibility
> to be in the kernel, and not in the boot rom, because it
> is so much easier to develop and control.

- kumar

Re: [PATCH] Add USB to MPC8349 PB platform support

[Dan Malek]

On Jul 17, 2006, at 5:39 PM, Kumar Gala wrote:

> Well, I think there is a coupling that exists between whatever your  
> boot rom is and the kernel.

There shouldn't be, I've always said that, but Freescale
seems to want to insist on it :-)  The problem is people
creating this evaluation/demo boards seem to think
they are workstations, which they are not.  There is
such a minimal amount of information needed to
actually get a system from the boot rom to the kernel
that I don't understand why we are complicating this
process for embedded systems.

Here is my product development experience.  People
buy these evaluation boards to test a few of the features
they need for a product.  These boards try to be everything
to everyone, but in reality have never done what someone
has wanted very well.  Products developed using Linux
and many different boot roms are very focused on a particular
set of features.  Their requirements are nothing like that of
an evaluation board, and all of these cute workstation
features we are pushing into these evaluation board ports
do nothing to help get these products customized for market.

> If you are trying to optimize boot time I'd say one thing you would  
> want is to avoid multiple writing the same configuration registers.

That's what I said.  Just do it in Linux when the feature is
started, either as a built-in driver or loadable module.  No
need to do this in a boot rom if there isn't any need for it.

> I dont have an issue if a fixed function board decides to do these  
> things in their kernel init instead of their boot rom.  I however,  
> don't want thousand and one config options to support all the  
> various ways one can configure the Freescale board.

I don't understand how configuration options fit into this
discussion.  In this particular discussion, if you have selected
the USB option, then include the proper initialization code
in the kernel for the board.  No additional options needed.

Thanks.

	-- Dan

RE: [PATCH] Add USB to MPC8349 PB platform support

[Li Yang-r58472]

> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Tuesday, July 18, 2006 5:39 AM
> To: Dan Malek
> Cc: Li Yang-r58472; linuxppc-dev@ozlabs.org;
linux-usb-devel@lists.sourceforge.net
> Subject: Re: [PATCH] Add USB to MPC8349 PB platform support
>=20
>=20
> On Jul 17, 2006, at 3:17 PM, Dan Malek wrote:
>=20
> >
> > On Jul 17, 2006, at 3:16 PM, Kumar Gala wrote:
> >
> >> I disagree.  You are coming from this from a board that does
> >> everything under the sun.  I'd like to avoid having this type of
> >> initialization in the kernel.  There is a whole additional kitchen
> >> sink that could move into the kernel as well.
> >
> > Well, I'm going to have to disagree with your disagreement :-)
> > The kernel should not assume things are properly initialized
> > and rely on the boot rom to do such things.  I have several
> > reasons for this.  One is that we are always pressed to make
> > embedded systems boot more quickly, and taking time to
> > initialize things in the boot rom just makes that a totally
> > inflexible system design.  We don't need to initialize things
> > we don't use, or can postpone until later.  Two, it makes
> > us dependent upon a particular boot rom, or boot rom
> > behavior, that not all boards may choose to support.
> > Three, board designs may have external logic that requires
> > a certain start up sequence or control register access
> > that complicates the boot rom in it's ability to share
> > code or implementation.
>=20
> Well, I think there is a coupling that exists between whatever your
> boot rom is and the kernel.  If you are trying to optimize boot time
> I'd say one thing you would want is to avoid multiple writing the
> same configuration registers.
>=20
> I dont have an issue if a fixed function board decides to do these
> things in their kernel init instead of their boot rom.  I however,
> don't want thousand and one config options to support all the various
> ways one can configure the Freescale board.

We won't have the thousand and one config options, making use of the
device
tree.  So this is not a problem.
>=20
> > There are more, but I think you see the trend.  In my
> > years of doing this kind of development, you can't
> > assume a boot rom is going to do much more than initialize
> > memory and load the kernel.  I prefer the flexibility
> > to be in the kernel, and not in the boot rom, because it
> > is so much easier to develop and control.
>=20
> - kumar

Re: [PATCH] Add USB to MPC8349 PB platform support

[Kumar Gala]

[snip]

>> Well, I think there is a coupling that exists between whatever your
>> boot rom is and the kernel.  If you are trying to optimize boot time
>> I'd say one thing you would want is to avoid multiple writing the
>> same configuration registers.
>>
>> I dont have an issue if a fixed function board decides to do these
>> things in their kernel init instead of their boot rom.  I however,
>> don't want thousand and one config options to support all the various
>> ways one can configure the Freescale board.
>
> We won't have the thousand and one config options, making use of the
> device
> tree.  So this is not a problem.

I'm talking about opening the door to a ton of options, not that we  
have them now.  For example, your patch doesnt handle the USB PHYs if  
they are on the MDS instead of the SYS board.  It doesn't handle  
setting SCCR properly for different frequency choices.  I'm concerned  
about where to draw the line because of all the ways a user can  
configure the MDS board.

- kumar

Re: [PATCH] Add USB to MPC8349 PB platform support

[Dan Malek]

On Jul 18, 2006, at 9:52 AM, Kumar Gala wrote:

> .....  I'm concerned about where to draw the line because of all  
> the ways a user can configure the MDS board.

IMHO, you choose one configuration that works and
make that the board port.  If someone wants to change
it later, it would be a good exercise for them to learn
how to do such things  From years of working with these
things I'll suggest that it's not worthwhile to make this
so complicated that you think it's going to be everything
for anyone.  It's not the way people use these boards.

The best thing we should do is clearly document
one board configuration and have the Linux configuration
that matches.  It's horribly misleading to present a board
that is infinitely configurable and implying you can
find a Linux configuration to match.  They are guaranteed
to find something that doesn't work on their first
attempt, and then all of the "broken this" and "broken
that" finger pointing starts :-)

Thanks.

	-- Dan

Re: [PATCH] Add USB to MPC8349 PB platform support

[Wolfgang Denk]

In message <41AD230F-91E8-442A-B9F9-9892D14BEF12@kernel.crashing.org> you wrote:
> 
> I'm talking about opening the door to a ton of options, not that we  
> have them now.  For example, your patch doesnt handle the USB PHYs if  

If you really assume that all this has  to  be  handled  in  so  many
configuration  options  then it probably makes not much difference if
you do this in the kernel  or  in  any  boot  loader  -  you're  just
shifting  effort  and  responsibility  to somebody else. I agree with
Dan's argumentation.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Chapter 1 -- The story so  far:
In the beginning the Universe was created. This has  made  a  lot  of
people very angry and been widely regarded as a bad move.

RE: [PATCH] Add USB to MPC8349 PB platform support

[Li Yang-r58472]

> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Tuesday, July 18, 2006 9:53 PM
> To: Li Yang-r58472
> Cc: Dan Malek; linuxppc-dev@ozlabs.org;
linux-usb-devel@lists.sourceforge.net
> Subject: Re: [PATCH] Add USB to MPC8349 PB platform support
>=20
> [snip]
>=20
> >> Well, I think there is a coupling that exists between whatever your
> >> boot rom is and the kernel.  If you are trying to optimize boot
time
> >> I'd say one thing you would want is to avoid multiple writing the
> >> same configuration registers.
> >>
> >> I dont have an issue if a fixed function board decides to do these
> >> things in their kernel init instead of their boot rom.  I however,
> >> don't want thousand and one config options to support all the
various
> >> ways one can configure the Freescale board.
> >
> > We won't have the thousand and one config options, making use of the
> > device
> > tree.  So this is not a problem.
>=20
> I'm talking about opening the door to a ton of options, not that we
> have them now.  For example, your patch doesnt handle the USB PHYs if
> they are on the MDS instead of the SYS board.  It doesn't handle
> setting SCCR properly for different frequency choices.  I'm concerned
> about where to draw the line because of all the ways a user can
> configure the MDS board.

My understanding is that in embedded world only this kind of versatile
development boards needs to have Linux port in mainstream kernel.  A
fixed function customer board used in embedded product will never get
into kernel source.

The reasons for Freescale to provide this kind of boards are: first,
verify functions on chip; second, demonstrate chip functions to
customer; third, validate customer application on reference board;
forth, help customer to build their own product by cutting down the
board and software.

Doing that, we do need some configure options to demonstrate main
functions.  But there won't be too many options to confuse the user.
Only key working modes are needed.  In your example, I think making USB
PHYs configurable for MDS is reasonable, but for SCCR it isn't as it is
well commented in code and is not likely to be modified.  Keep in mind
that our only purpose is making customers easier and faster in
developing their own products.
>=20
> - kumar

Re: [PATCH] Add USB to MPC8349 PB platform support

[Kumar Gala]

On Jul 19, 2006, at 1:30 AM, Li Yang-r58472 wrote:

>> -----Original Message-----
>> From: Kumar Gala [mailto:galak@kernel.crashing.org]
>> Sent: Tuesday, July 18, 2006 9:53 PM
>> To: Li Yang-r58472
>> Cc: Dan Malek; linuxppc-dev@ozlabs.org;
> linux-usb-devel@lists.sourceforge.net
>> Subject: Re: [PATCH] Add USB to MPC8349 PB platform support
>>
>> [snip]
>>
>>>> Well, I think there is a coupling that exists between whatever your
>>>> boot rom is and the kernel.  If you are trying to optimize boot
> time
>>>> I'd say one thing you would want is to avoid multiple writing the
>>>> same configuration registers.
>>>>
>>>> I dont have an issue if a fixed function board decides to do these
>>>> things in their kernel init instead of their boot rom.  I however,
>>>> don't want thousand and one config options to support all the
> various
>>>> ways one can configure the Freescale board.
>>>
>>> We won't have the thousand and one config options, making use of the
>>> device
>>> tree.  So this is not a problem.
>>
>> I'm talking about opening the door to a ton of options, not that we
>> have them now.  For example, your patch doesnt handle the USB PHYs if
>> they are on the MDS instead of the SYS board.  It doesn't handle
>> setting SCCR properly for different frequency choices.  I'm concerned
>> about where to draw the line because of all the ways a user can
>> configure the MDS board.
>
> My understanding is that in embedded world only this kind of versatile
> development boards needs to have Linux port in mainstream kernel.  A
> fixed function customer board used in embedded product will never get
> into kernel source.

This is an incorrect assumption.  Its more often that people dont  
post their ports to the Linux kernel for acceptance.  We will accept  
any port that is willing to work with the community.   for example

> The reasons for Freescale to provide this kind of boards are: first,
> verify functions on chip; second, demonstrate chip functions to
> customer; third, validate customer application on reference board;
> forth, help customer to build their own product by cutting down the
> board and software.
>
> Doing that, we do need some configure options to demonstrate main
> functions.  But there won't be too many options to confuse the user.
> Only key working modes are needed.  In your example, I think making  
> USB
> PHYs configurable for MDS is reasonable, but for SCCR it isn't as  
> it is
> well commented in code and is not likely to be modified.  Keep in mind
> that our only purpose is making customers easier and faster in
> developing their own products.

That's fine, however Freescale should figure out what the "common"  
modes of usage are before hand and document them.  Then the community  
can have a sense of the scope of what is needed and how to try to  
best accomplish that, and if anything is missing.

I disagree about the SCCR, I think its very reasonable for a customer  
to set the frequencies of their MDS board to match what their  
production system is going to be, which means the SCCR would need to  
be modified to match.

At the end of the day we will get some version of your patch into the  
kernel.  I just wanted to get some discussion on how we handle the  
reference boards that are extremely configurable.

- kumar

Re: [PATCH] Add USB to MPC8349 PB platform support

[Dan Malek]

On Jul 19, 2006, at 9:14 AM, Kumar Gala wrote:

> This is an incorrect assumption.  Its more often that people dont  
> post their ports to the Linux kernel for acceptance.  We will  
> accept any port that is willing to work with the community.   for  
> example

I agree.  The customers of board ports I've done over the years
are always eager to get these into the public sources.  It just
seems we run out of time during the pressure of trying to get
the products done, and they just issue them on CDs or for
download afterward.

Thanks.

	-- Dan

Re: [PATCH] Add USB to MPC8349 PB platform support

[Li Yang]

On 7/19/06, Dan Malek <dan@embeddedalley.com> wrote:
>
> On Jul 19, 2006, at 9:14 AM, Kumar Gala wrote:
>
> > This is an incorrect assumption.  Its more often that people dont
> > post their ports to the Linux kernel for acceptance.  We will
> > accept any port that is willing to work with the community.   for
> > example
>
> I agree.  The customers of board ports I've done over the years
> are always eager to get these into the public sources.  It just
> seems we run out of time during the pressure of trying to get
> the products done, and they just issue them on CDs or for
> download afterward.

But why?  Most embedded products facing end-user wouldn't like users
to modify the system by themselves.  Sometimes they even put effort in
preventing user to do so.  If no one else is going to modify the code,
what is the value of putting them in public sources?  Just to show the
compliance with GPL?  The only kind of products I can think of, which
want the users to modify the code is reference boards, IMHO.  Please
correct me if I'm wrong.

Re: [linux-usb-devel] [PATCH] Add USB to MPC8349 PB platform support

[Pete Zaitcev]

On Thu, 20 Jul 2006 02:59:44 +0800, "Li Yang" <LeoLi@freescale.com> wrote:

> But why?  Most embedded products facing end-user wouldn't like users
> to modify the system by themselves.

Certainly vendors won't like to give consumers extra freedoms.
However, consumers like those freedoms, even if they would not
use them in any specific instance.

>  Sometimes they even put effort in preventing user to do so.

Jerks and criminals, that's what they are. Why would Linux developers
care about needs of these people?

Free software is about protecting the rights of users against
vendors who "wouldn't like" users doing what they want with the
product they bought.

> The only kind of products I can think of, which
> want the users to modify the code is reference boards, IMHO.

Ever heard of WRT54? But in fact, every product out there may provide
its owner an additional value when customized. I'd love to fix some
bugs in my microwave, for instance. The silly thing will not start
the timer if the door is open. It probably uses the same function
which prevents cooking from starting (it also has a hardware interlock
that cuts the magnetron, but this is different).

-- Pete

RE: [linux-usb-devel] [PATCH] Add USB to MPC8349 PB platform support

[Li Yang-r58472]

> -----Original Message-----
> From: Pete Zaitcev [mailto:zaitcev@redhat.com]
> Sent: Thursday, July 20, 2006 3:19 AM
> To: Li Yang-r58472
> Cc: dan@embeddedalley.com; linuxppc-dev@ozlabs.org;
galak@kernel.crashing.org;
> linux-usb-devel@lists.sourceforge.net; zaitcev@redhat.com
> Subject: Re: [linux-usb-devel] [PATCH] Add USB to MPC8349 PB platform
support
>=20
> On Thu, 20 Jul 2006 02:59:44 +0800, "Li Yang" <LeoLi@freescale.com>
wrote:
>=20
> > But why?  Most embedded products facing end-user wouldn't like users
> > to modify the system by themselves.
>=20
> Certainly vendors won't like to give consumers extra freedoms.
> However, consumers like those freedoms, even if they would not
> use them in any specific instance.
>=20
> >  Sometimes they even put effort in preventing user to do so.
>=20
> Jerks and criminals, that's what they are. Why would Linux developers
> care about needs of these people?
>=20
> Free software is about protecting the rights of users against
> vendors who "wouldn't like" users doing what they want with the
> product they bought.
>=20
> > The only kind of products I can think of, which
> > want the users to modify the code is reference boards, IMHO.
>=20
> Ever heard of WRT54? But in fact, every product out there may provide
> its owner an additional value when customized. I'd love to fix some
> bugs in my microwave, for instance. The silly thing will not start
> the timer if the door is open. It probably uses the same function
> which prevents cooking from starting (it also has a hardware interlock
> that cuts the magnetron, but this is different).

You are right.  We really need to stick up for the freedom of software
as Richard Stallman advocated.  :) However, I was talking about code
that goes into public kernel source tree.  It's ok to distribute the
code with CD or web link.  But if the code is not generic enough or
can't be reused, it won't qualify to be included in mainstream kernel
tree, I think.

-- Leo

Re: [linux-usb-devel] [PATCH] Add USB to MPC8349 PB platform support

[David Brownell]

On Wednesday 19 July 2006 11:59 am, Li Yang wrote:
> On 7/19/06, Dan Malek <dan@embeddedalley.com> wrote:
> >
> > On Jul 19, 2006, at 9:14 AM, Kumar Gala wrote:
> >
> > > This is an incorrect assumption.  Its more often that people dont
> > > post their ports to the Linux kernel for acceptance.  We will
> > > accept any port that is willing to work with the community.   for
> > > example
> >
> > I agree.  The customers of board ports I've done over the years
> > are always eager to get these into the public sources.  It just
> > seems we run out of time during the pressure of trying to get
> > the products done, and they just issue them on CDs or for
> > download afterward.
> 
> But why?

Because the notion of a "board port" of a driver is usually bogus;
as a rule, a driver doesn't need board-specific knowledge, any
board using the same chip could work the same.  (And probably should;
most of the time I've seen board-specfic knowledge in drivers, it's
been masking bugs.)

And once the driver for a chip is open sourced, it can be reused
on other boards, generalized, and bugfixed ... none of which is
going to happen if you don't push the driver upstream.  That means
the second or third product using that chip (or its cousins) could
become more robust, faster, easier to use, and otherwise "better".

No matter how good your developers are, there will be bugs in the
code they produce.  Pushing drivers upstream increases the pool
of developers who _could_ fix the bugs and otherwise improve the
code, given time and incentive.


> Most embedded products facing end-user wouldn't like users 
> to modify the system by themselves.  Sometimes they even put effort in
> preventing user to do so.  If no one else is going to modify the code,
> what is the value of putting them in public sources?  Just to show the
> compliance with GPL?  The only kind of products I can think of, which
> want the users to modify the code is reference boards, IMHO.  Please
> correct me if I'm wrong.

OK, you're wrong.  :)

Because the "user" in this case is more like "other developers" than
dumb "end users" ... though you need to keep in mind that the "Free" in
"Free Software" refers to people being able to change from "end users"
to "developers" in the inevitable cases where an original developer
doesn't have time for basics like bug fixing older software, or adding
essential capabilities that somehow got overlooked in the original
rush to market.

And yes, that is sometimes contrary to the goals of some folk who
build embedded products.  It's the usual tradeoff of whether the
product goal is to solve end user problems or make money for some
vendor.  Wearing my end user hat, the answer is clear.

- Dave

Re: [PATCH] Add USB to MPC8349 PB platform support

[Marc Leeman]

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

> I agree.  The customers of board ports I've done over the years
> are always eager to get these into the public sources.  It just
> seems we run out of time during the pressure of trying to get
> the products done, and they just issue them on CDs or for
> download afterward.

For our boards, I am somewhat more eager to get our port in U-Boot than
in the Linux kernel for a number of reasons.

First; it is always interesting to see how other ppl handle simular
problems during boot up.

Secondly; U-Boot is used to set configuration options at bootup, e.g.
for multi purpose pins. Having your changes (to registers), fixes or
silicon bugs might help ppl and avoid them to look for a silicon bug
workaround.

If I submit a port of our board to U-Boot, clearly comment the changes
(e.g. enable GPIO on pin X while setting registers); I hope that some
ppl can use it. Next to this, Wolfgang is doing a *very* good job of
keeping his tree clean and giving good pointers to problems and clean up
strategies that serve us all.

Having my changes for our boards that are very specific since they are
connected to FPGAs would IMHO serve little purpose in the Linux kernel:
there is very limited access to the hardware and it would only serve to
clutter the kernel with an exponentionally growing number of ports that
most ppl will never use. I think (even though using ARCH and
CROSS_COMPILE during compilation limits the audience); the target
audience between U-Boot and the Linux kernel is slightly different.

Though I do agree that there is a gap: it would be nice to have some
place to submit the kernel patches; possibly not included in the main
tree; just to see how other ppl are tackling the same problem and
avoiding re-doing the same work for the nth time (e.g. writing some
communciation protocol where there are a number of references of ppl
haveing done them, but no real code).

-- 
  greetz, marc
Aeryn died so you could live John. She would want you to keep fighting.
	D'Argo - Season of Death
scorpius.homelinux.org 2.6.17 #2 PREEMPT Thu Jun 22 07:18:33 CEST 2006 GNU/Linux

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

Re: [PATCH] Add USB to MPC8349 PB platform support

[Kumar Gala]

[snip]

> Having my changes for our boards that are very specific since they are
> connected to FPGAs would IMHO serve little purpose in the Linux  
> kernel:
> there is very limited access to the hardware and it would only  
> serve to
> clutter the kernel with an exponentionally growing number of ports  
> that
> most ppl will never use. I think (even though using ARCH and
> CROSS_COMPILE during compilation limits the audience); the target
> audience between U-Boot and the Linux kernel is slightly different.
>
> Though I do agree that there is a gap: it would be nice to have some
> place to submit the kernel patches; possibly not included in the main
> tree; just to see how other ppl are tackling the same problem and
> avoiding re-doing the same work for the nth time (e.g. writing some
> communciation protocol where there are a number of references of ppl
> haveing done them, but no real code).

This should be the kernel.  The general rule of thumb I've used is if  
its useful to more than one other person its worth trying to get into  
the kernel.  However, I can see if you are doing a one off kernel for  
your embedded product that getting your changes into the kernel  
wouldn't be worth while.  You have to have a desire to interact with  
the community if you want to get your code in.

Personally, I see it useful if for no other reason that someone will  
fixup my board port if/when they change something which will make my  
moving to a newer kernel release that much easier.

- k

Re: [PATCH] Add USB to MPC8349 PB platform support

[Marc Leeman]

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

> >Though I do agree that there is a gap: it would be nice to have some
> >place to submit the kernel patches;
> 
> This should be the kernel.  The general rule of thumb I've used is if  
> its useful to more than one other person its worth trying to get into  
> the kernel.  However, I can see if you are doing a one off kernel for 

That is about the same rule that I use; luckily (a credit to the Linux
kernel and other developers); I don't have to change much stuff like
that.
 
> your embedded product that getting your changes into the kernel  
> wouldn't be worth while.  You have to have a desire to interact with  
> the community if you want to get your code in.

There is a grey zone, but let's talk about a specific case:

In my current queue, I have to write a Host Port Interface (HPI) protocol
(serial) to a TI DSP. I would imagine that this is a useful
contribution. However, using HPI is almost by nature limited to specific
embedded designs (most of which differ slightly from one another).
Furthermore; I will need to use a number of GPIO pins on a 834x
processor. Using this needs to be backed by configuration settings in
U-Boot. If someone else makes a similar design; it would most likely be
with another processor family; and even then; they'll have other pins
connected/used.

Though the protocol would be a real nice addition; the physical
connection/configuration make including it in the main kernel tree
difficult.

> Personally, I see it useful if for no other reason that someone will  
> fixup my board port if/when they change something which will make my  
> moving to a newer kernel release that much easier.

Even though I would welcome this; our boards are included in larger
expensive systems that would just be shipped back in case of problems;
but we've never had functional (linux) problems (yet).

Come to think of it, I have a number of minor patches for 8349SYS based
configurations; where can I find the last devel code (next to 2.6.17)
for patch creation (and to whom to send them back)?

-- 
  greetz, marc
If you must address me, do so as Your Supreme Eminence. Which you
should be doing anyway.
	Rygel - Throne for a Loss
scorpius.homelinux.org 2.6.17 #2 PREEMPT Thu Jun 22 07:18:33 CEST 2006 GNU/Linux

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

Re: [PATCH] Add USB to MPC8349 PB platform support

[Kumar Gala]

On Jul 20, 2006, at 2:21 AM, Marc Leeman wrote:

>>> Though I do agree that there is a gap: it would be nice to have some
>>> place to submit the kernel patches;
>>
>> This should be the kernel.  The general rule of thumb I've used is if
>> its useful to more than one other person its worth trying to get into
>> the kernel.  However, I can see if you are doing a one off kernel for
>
> That is about the same rule that I use; luckily (a credit to the Linux
> kernel and other developers); I don't have to change much stuff like
> that.
>
>> your embedded product that getting your changes into the kernel
>> wouldn't be worth while.  You have to have a desire to interact with
>> the community if you want to get your code in.
>
> There is a grey zone, but let's talk about a specific case:
>
> In my current queue, I have to write a Host Port Interface (HPI)  
> protocol
> (serial) to a TI DSP. I would imagine that this is a useful
> contribution. However, using HPI is almost by nature limited to  
> specific
> embedded designs (most of which differ slightly from one another).
> Furthermore; I will need to use a number of GPIO pins on a 834x
> processor. Using this needs to be backed by configuration settings in
> U-Boot. If someone else makes a similar design; it would most  
> likely be
> with another processor family; and even then; they'll have other pins
> connected/used.
>
> Though the protocol would be a real nice addition; the physical
> connection/configuration make including it in the main kernel tree
> difficult.

Maybe, but your are just talking about abstracting what GPIO pins are  
used, which is a minor issue.

>> Personally, I see it useful if for no other reason that someone will
>> fixup my board port if/when they change something which will make my
>> moving to a newer kernel release that much easier.
>
> Even though I would welcome this; our boards are included in larger
> expensive systems that would just be shipped back in case of problems;
> but we've never had functional (linux) problems (yet).
>
> Come to think of it, I have a number of minor patches for 8349SYS  
> based
> configurations; where can I find the last devel code (next to 2.6.17)
> for patch creation (and to whom to send them back)?

Grab a git kernel from kernel.org/git, I'd say to use either linux/ 
kernel/git/torvalds/linux-2.6.git (Linus's tree) or linux/kernel/git/ 
paulus/powerpc.git (Paul's tree).

You'd send the patches to me as I'm the maintainer for 83xx stuff.

- kumar

Re: [PATCH] Add USB to MPC8349 PB platform support

[Kumar Gala]

On Jul 14, 2006, at 6:52 AM, Li Yang wrote:

> This adds USB platform support to MPC8349 PB.  It works with the  
> fsl_usb2_udc driver.
>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
>
> arch/powerpc/platforms/83xx/Kconfig       |   4  ++
> arch/powerpc/platforms/83xx/mpc834x_sys.c |   72 +++++++++++++++++++ 
> ++++++++++
> arch/powerpc/platforms/83xx/mpc834x_sys.h |   24 ++++++++++
> 3 files changed, 113 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/ 
> platforms/83xx/Kconfig
> index 7675e67..8404cdf 100644
> --- a/arch/powerpc/platforms/83xx/Kconfig
> +++ b/arch/powerpc/platforms/83xx/Kconfig
> @@ -24,4 +31,14 @@ config MPC834x
> 	select PPC_INDIRECT_PCI
> 	default y if MPC834x_SYS
>
> +config 834x_USB_SUPPORT
> +	bool
> +	default y if MPC834x_SYS && (USB || USB_GADGET)
> +	
> endmenu

I dont think we need this, we should be able to use the USB, and 834x  
CONFIG options in the code.

> diff --git a/arch/powerpc/platforms/83xx/mpc834x_sys.c b/arch/ 
> powerpc/platforms/83xx/mpc834x_sys.c
> index 7e789d2..f10d4ae 100644
> --- a/arch/powerpc/platforms/83xx/mpc834x_sys.c
> +++ b/arch/powerpc/platforms/83xx/mpc834x_sys.c
> @@ -36,6 +36,7 @@ #include <asm/irq.h>
> #include <asm/prom.h>
> #include <asm/udbg.h>
> #include <sysdev/fsl_soc.h>
> +#include <linux/fsl_devices.h>
> #include "mpc83xx.h"
> @@ -71,6 +72,72 @@ mpc83xx_map_irq(struct pci_dev *dev, uns
> }
> #endif				/* CONFIG_PCI */
> +#ifdef CONFIG_834x_USB_SUPPORT

Just make it dependent on CONFIG_USB_EHCI_HCD or something like that.

> +void mpc834x_usb_board_cfg(void)
> +{
> +	unsigned char __iomem *bcsr;
> +	volatile unsigned char *bcsr5_p;
> +
> +	/*
> +	 * if SYS board is plug into PIB board,
> +	 * force to use the PHY on SYS board
> +	 * */
> +	bcsr = ioremap(BCSR_PHYS_ADDR, BCSR_SIZE);
> +	bcsr5_p = bcsr + BCSR5_OFF;
> +	if ( (*bcsr5_p & BCSR5_INT_USB) == 0 )
> +		*bcsr5_p = (*bcsr5_p | BCSR5_INT_USB);
> +	iounmap(bcsr);
> +}
> +
> +/* 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;

This really needs to take into account the platform frequency.  I  
guess technically, at 1:3 we will always be below the max 133Mhz.

> +
> +	/* 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;
> +

Why dont we just do all this in fsl_usb_of_init

> +	iounmap(immap);
> +	return 0;
> +}
> +
> +#endif /* CONFIG_834x_USB_SUPPORT */
> +
> /*  
> ********************************************************************** 
> **
>  *
>  * Setup the architecture
> @@ -102,6 +169,11 @@ #ifdef CONFIG_PCI
> 	ppc_md.pci_exclude_device = mpc83xx_exclude_device;
> #endif
> +#ifdef CONFIG_834x_USB_SUPPORT
> +	mpc834x_usb_cfg();
> +	mpc834x_usb_board_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 fedecb7..30e45e8 100644
> --- a/arch/powerpc/platforms/83xx/mpc834x_sys.h
> +++ b/arch/powerpc/platforms/83xx/mpc834x_sys.h
> @@ -20,4 +20,28 @@ #define PIRQB	MPC83xx_IRQ_EXT5
> #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

Are these really generic to all MPC83XX?  We should move these to  
include something like asm-ppc/mpc83xx.h

> +
> +
> #endif				/* __MACH_MPC83XX_SYS_H__ */

- kumar

RE: [PATCH] Add USB to MPC8349 PB platform support

[Li Yang-r58472]

> -----Original Message-----
> From: linuxppc-dev-bounces+leoli=3Dfreescale.com@ozlabs.org
> [mailto:linuxppc-dev-bounces+leoli=3Dfreescale.com@ozlabs.org] On =
Behalf
Of Kumar
> Gala
> Sent: Thursday, July 20, 2006 4:09 AM
> To: Li Yang-r58472
> Cc: linuxppc-dev@ozlabs.org; linux-usb-devel@lists.sourceforge.net
> Subject: Re: [PATCH] Add USB to MPC8349 PB platform support
>=20
>=20
> On Jul 14, 2006, at 6:52 AM, Li Yang wrote:
>=20
> > This adds USB platform support to MPC8349 PB.  It works with the
> > fsl_usb2_udc driver.
> >
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > ---
> >
> > arch/powerpc/platforms/83xx/Kconfig       |   4  ++
> > arch/powerpc/platforms/83xx/mpc834x_sys.c |   72 +++++++++++++++++++
> > ++++++++++
> > arch/powerpc/platforms/83xx/mpc834x_sys.h |   24 ++++++++++
> > 3 files changed, 113 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/83xx/Kconfig b/arch/powerpc/
> > platforms/83xx/Kconfig
> > index 7675e67..8404cdf 100644
> > --- a/arch/powerpc/platforms/83xx/Kconfig
> > +++ b/arch/powerpc/platforms/83xx/Kconfig
> > @@ -24,4 +31,14 @@ config MPC834x
> > 	select PPC_INDIRECT_PCI
> > 	default y if MPC834x_SYS
> >
> > +config 834x_USB_SUPPORT
> > +	bool
> > +	default y if MPC834x_SYS && (USB || USB_GADGET)
> > +
> > endmenu
>=20
> I dont think we need this, we should be able to use the USB, and 834x
> CONFIG options in the code.
>=20
> > diff --git a/arch/powerpc/platforms/83xx/mpc834x_sys.c b/arch/
> > powerpc/platforms/83xx/mpc834x_sys.c
> > index 7e789d2..f10d4ae 100644
> > --- a/arch/powerpc/platforms/83xx/mpc834x_sys.c
> > +++ b/arch/powerpc/platforms/83xx/mpc834x_sys.c
> > @@ -36,6 +36,7 @@ #include <asm/irq.h>
> > #include <asm/prom.h>
> > #include <asm/udbg.h>
> > #include <sysdev/fsl_soc.h>
> > +#include <linux/fsl_devices.h>
> > #include "mpc83xx.h"
> > @@ -71,6 +72,72 @@ mpc83xx_map_irq(struct pci_dev *dev, uns
> > }
> > #endif				/* CONFIG_PCI */
> > +#ifdef CONFIG_834x_USB_SUPPORT
>=20
> Just make it dependent on CONFIG_USB_EHCI_HCD or something like that.

Will need to check EHCI_HCD/GADGET and their _MODULE variations though.
Not very neat I think.  Anyway, it's your call.
>=20
> > +void mpc834x_usb_board_cfg(void)
> > +{
> > +	unsigned char __iomem *bcsr;
> > +	volatile unsigned char *bcsr5_p;
> > +
> > +	/*
> > +	 * if SYS board is plug into PIB board,
> > +	 * force to use the PHY on SYS board
> > +	 * */
> > +	bcsr =3D ioremap(BCSR_PHYS_ADDR, BCSR_SIZE);
> > +	bcsr5_p =3D bcsr + BCSR5_OFF;
> > +	if ( (*bcsr5_p & BCSR5_INT_USB) =3D=3D 0 )
> > +		*bcsr5_p =3D (*bcsr5_p | BCSR5_INT_USB);
> > +	iounmap(bcsr);
> > +}
> > +
> > +/* 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 =3D NULL;
> > +	int port0_is_dr =3D 0;
> > +
> > +	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;
> > +	}
> > +
> > +	immap =3D ioremap(get_immrbase(), 0x100000);
> > +
> > +	/* Configure clock */
> > +	p =3D (volatile unsigned long *)((u32)immap + MPC83XX_SCCR_OFFS);
> > +	sccr =3D *p;
> > +	if (port0_is_dr)
> > +		sccr |=3D MPC83XX_SCCR_USB_DRCM_11;  /* 1:3 */
> > +	else
> > +		sccr |=3D MPC83XX_SCCR_USB_MPHCM_11; /* 1:3 */
> > +	*p =3D sccr;
>=20
> This really needs to take into account the platform frequency.  I
> guess technically, at 1:3 we will always be below the max 133Mhz.

Generally, I assume the user is using a core frequency around the
recommended frequency in spec.  In some special cases, they do need to
use a lower core frequency.  But, I think it's too special to be
addressed here.  Anyway it's easy to change for users.
>=20
> > +
> > +	/* Configure Pin */
> > +	p =3D (volatile unsigned long *)((u32)immap + MPC83XX_SICRL_OFFS);
> > +	sicrl =3D *p;
> > +	/* set port0 only */
> > +	if (port0_is_dr) +		sicrl |=3D MPC83XX_SICRL_USB0;
> > +	else +		sicrl &=3D ~(MPC83XX_SICRL_USB0);
> > +	*p =3D sicrl;
> > +
>=20
> Why dont we just do all this in fsl_usb_of_init

It's easier to do in fsl_usb_of_init.  But do you think it's ok to put
834x specific code there?
>=20
> > +	iounmap(immap);
> > +	return 0;
> > +}
> > +
> > +#endif /* CONFIG_834x_USB_SUPPORT */
> > +
> > /*
> >
**********************************************************************
> > **
> >  *
> >  * Setup the architecture
> > @@ -102,6 +169,11 @@ #ifdef CONFIG_PCI
> > 	ppc_md.pci_exclude_device =3D mpc83xx_exclude_device;
> > #endif
> > +#ifdef CONFIG_834x_USB_SUPPORT
> > +	mpc834x_usb_cfg();
> > +	mpc834x_usb_board_cfg();
> > +#endif
> > +
> > #ifdef  CONFIG_ROOT_NFS
> > 	ROOT_DEV =3D Root_NFS;
> > #else
> > diff --git a/arch/powerpc/platforms/83xx/mpc834x_sys.h b/arch/
> > powerpc/platforms/83xx/mpc834x_sys.h
> > index fedecb7..30e45e8 100644
> > --- a/arch/powerpc/platforms/83xx/mpc834x_sys.h
> > +++ b/arch/powerpc/platforms/83xx/mpc834x_sys.h
> > @@ -20,4 +20,28 @@ #define PIRQB	MPC83xx_IRQ_EXT5
> > #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
>=20
> Are these really generic to all MPC83XX?  We should move these to
> include something like asm-ppc/mpc83xx.h

No, probably need to change the naming to MPC834x_*.  The offsets are
generic, but the USB bits are not.  If there is an asm/mpc834x.h, it
will be better there.
>=20
> > +
> > +
> > #endif				/* __MACH_MPC83XX_SYS_H__ */
>=20
> - kumar
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev