[U-Boot] [PATCH 3/4][v2]include:configs:ls1021atwr: Enable USB IP support

[U-Boot] [PATCH 3/4][v2]include:configs:ls1021atwr: Enable USB IP support

[Ramneek Mehresh]

Enable USB IP support for both EHCI and XHCI for
ls1021atwr platform

Signed-off-by: Ramneek Mehresh <ramneek.mehresh@freescale.com>
---
 include/configs/ls1021atwr.h | 36 ++++++++++++++++++++++++++++++++++++
 include/linux/usb/xhci-fsl.h |  5 +++++
 2 files changed, 41 insertions(+)

diff --git a/include/configs/ls1021atwr.h b/include/configs/ls1021atwr.h
index a13876b..f208638 100644
--- a/include/configs/ls1021atwr.h
+++ b/include/configs/ls1021atwr.h
@@ -28,6 +28,42 @@
 #define CONFIG_SYS_INIT_RAM_SIZE	OCRAM_SIZE
 
 /*
+ * USB
+ */
+
+/* EHCI Support - disbaled by default as
+ * there is no signal coming out of soc on
+ * this board for this controller. However,
+ * the silicon still has this controller,
+ * and anyone can use this controller by
+ * taking signals out on their board.
+ */
+/*#define CONFIG_HAS_FSL_DR_USB*/
+
+#ifdef CONFIG_HAS_FSL_DR_USB
+#define CONFIG_USB_EHCI
+#define CONFIG_USB_EHCI_FSL
+#define CONFIG_EHCI_HCD_INIT_AFTER_RESET
+#endif
+
+/* XHCI Support - enabled by default */
+#define CONFIG_HAS_FSL_XHCI_USB
+
+#ifdef CONFIG_HAS_FSL_XHCI_USB
+#define CONFIG_USB_XHCI_FSL
+#define CONFIG_USB_XHCI_DWC3
+#define CONFIG_USB_XHCI
+#define CONFIG_USB_MAX_CONTROLLER_COUNT        1
+#define CONFIG_SYS_USB_XHCI_MAX_ROOT_PORTS     2
+#endif
+
+#if defined(CONFIG_HAS_FSL_DR_USB) || defined(CONFIG_HAS_FSL_XHCI_USB)
+#define CONFIG_CMD_USB
+#define CONFIG_USB_STORAGE
+#define CONFIG_CMD_EXT2
+#endif
+
+/*
  * Generic Timer Definitions
  */
 #define GENERIC_TIMER_CLK		12500000
diff --git a/include/linux/usb/xhci-fsl.h b/include/linux/usb/xhci-fsl.h
index 8eaab2c..329abf7 100644
--- a/include/linux/usb/xhci-fsl.h
+++ b/include/linux/usb/xhci-fsl.h
@@ -46,6 +46,11 @@
 #define USBOTGSS_IRQ_SET_1_OEVT_EN		BIT(16)
 #define USBOTGSS_IRQ_SET_1_DMADISABLECLR_EN	BIT(17)
 
+#ifdef CONFIG_LS102XA
+#define CONFIG_SYS_FSL_XHCI_USB1_ADDR CONFIG_SYS_LS102XA_XHCI_USB1_ADDR
+#define CONFIG_SYS_FSL_XHCI_USB2_ADDR 0
+#endif
+
 struct fsl_xhci {
 	struct xhci_hccr *hcd;
 	struct dwc3 *dwc3_reg;
-- 
1.8.3.1

[U-Boot] [PATCH 3/4][v2]include:configs:ls1021atwr: Enable USB IP support

[Marek Vasut]

On Thursday, April 23, 2015 at 07:03:14 PM, Ramneek Mehresh wrote:
> Enable USB IP support for both EHCI and XHCI for
> ls1021atwr platform
> 
> Signed-off-by: Ramneek Mehresh <ramneek.mehresh@freescale.com>
> ---
>  include/configs/ls1021atwr.h | 36 ++++++++++++++++++++++++++++++++++++
>  include/linux/usb/xhci-fsl.h |  5 +++++
>  2 files changed, 41 insertions(+)
> 
> diff --git a/include/configs/ls1021atwr.h b/include/configs/ls1021atwr.h
> index a13876b..f208638 100644
> --- a/include/configs/ls1021atwr.h
> +++ b/include/configs/ls1021atwr.h
> @@ -28,6 +28,42 @@
>  #define CONFIG_SYS_INIT_RAM_SIZE	OCRAM_SIZE
> 
>  /*
> + * USB
> + */
> +
> +/* EHCI Support - disbaled by default as
> + * there is no signal coming out of soc on
> + * this board for this controller. However,
> + * the silicon still has this controller,
> + * and anyone can use this controller by
> + * taking signals out on their board.
> + */

Multiline comment again ;-)

> +/*#define CONFIG_HAS_FSL_DR_USB*/
> +
> +#ifdef CONFIG_HAS_FSL_DR_USB
> +#define CONFIG_USB_EHCI
> +#define CONFIG_USB_EHCI_FSL
> +#define CONFIG_EHCI_HCD_INIT_AFTER_RESET
> +#endif
> +
> +/* XHCI Support - enabled by default */
> +#define CONFIG_HAS_FSL_XHCI_USB
> +
> +#ifdef CONFIG_HAS_FSL_XHCI_USB
> +#define CONFIG_USB_XHCI_FSL
> +#define CONFIG_USB_XHCI_DWC3
> +#define CONFIG_USB_XHCI
> +#define CONFIG_USB_MAX_CONTROLLER_COUNT        1
> +#define CONFIG_SYS_USB_XHCI_MAX_ROOT_PORTS     2
> +#endif
> +
> +#if defined(CONFIG_HAS_FSL_DR_USB) || defined(CONFIG_HAS_FSL_XHCI_USB)
> +#define CONFIG_CMD_USB
> +#define CONFIG_USB_STORAGE
> +#define CONFIG_CMD_EXT2
> +#endif
> +
> +/*
>   * Generic Timer Definitions
>   */
>  #define GENERIC_TIMER_CLK		12500000
> diff --git a/include/linux/usb/xhci-fsl.h b/include/linux/usb/xhci-fsl.h
> index 8eaab2c..329abf7 100644
> --- a/include/linux/usb/xhci-fsl.h
> +++ b/include/linux/usb/xhci-fsl.h
> @@ -46,6 +46,11 @@
>  #define USBOTGSS_IRQ_SET_1_OEVT_EN		BIT(16)
>  #define USBOTGSS_IRQ_SET_1_DMADISABLECLR_EN	BIT(17)
> 
> +#ifdef CONFIG_LS102XA
> +#define CONFIG_SYS_FSL_XHCI_USB1_ADDR CONFIG_SYS_LS102XA_XHCI_USB1_ADDR
> +#define CONFIG_SYS_FSL_XHCI_USB2_ADDR 0

Do you need to define this _bogus_ address at all? If so, then the driver
which depends on this is broken. Why can't you just define a list of addresses
instead ? You would be able to use ARRAY_SIZE() in the driver to determine
how many controllers there are then. This is how it would look like:

#define CONFIG_FOO_BAR_ADDRS { USB1_ADDR, USB2_ADDR, ..., USBn_ADDR }

In the driver, there'd be:

<type> addrs[] = CONFIG_FOO_BAR_ADDRS;

> +#endif
> +
>  struct fsl_xhci {
>  	struct xhci_hccr *hcd;
>  	struct dwc3 *dwc3_reg;

[U-Boot] [PATCH 3/4][v2]include:configs:ls1021atwr: Enable USB IP support

[Ramneek Mehresh]

> -----Original Message-----
> From: Marek Vasut [mailto:marex at denx.de]
> Sent: Friday, April 24, 2015 9:00 AM
> To: Mehresh Ramneek-B31383
> Cc: u-boot at lists.denx.de
> Subject: Re: [PATCH 3/4][v2]include:configs:ls1021atwr: Enable USB IP
> support
> 
> On Thursday, April 23, 2015 at 07:03:14 PM, Ramneek Mehresh wrote:
> > Enable USB IP support for both EHCI and XHCI for ls1021atwr platform
> >
> > Signed-off-by: Ramneek Mehresh <ramneek.mehresh@freescale.com>
> > ---
> >  include/configs/ls1021atwr.h | 36
> > ++++++++++++++++++++++++++++++++++++
> >  include/linux/usb/xhci-fsl.h |  5 +++++
> >  2 files changed, 41 insertions(+)
> >
> > diff --git a/include/configs/ls1021atwr.h
> > b/include/configs/ls1021atwr.h index a13876b..f208638 100644
> > --- a/include/configs/ls1021atwr.h
> > +++ b/include/configs/ls1021atwr.h
> > @@ -28,6 +28,42 @@
> >  #define CONFIG_SYS_INIT_RAM_SIZE	OCRAM_SIZE
> >
> >  /*
> > + * USB
> > + */
> > +
> > +/* EHCI Support - disbaled by default as
> > + * there is no signal coming out of soc on
> > + * this board for this controller. However,
> > + * the silicon still has this controller,
> > + * and anyone can use this controller by
> > + * taking signals out on their board.
> > + */
> 
> Multiline comment again ;-)
> 
> > +/*#define CONFIG_HAS_FSL_DR_USB*/
> > +
> > +#ifdef CONFIG_HAS_FSL_DR_USB
> > +#define CONFIG_USB_EHCI
> > +#define CONFIG_USB_EHCI_FSL
> > +#define CONFIG_EHCI_HCD_INIT_AFTER_RESET #endif
> > +
> > +/* XHCI Support - enabled by default */ #define
> > +CONFIG_HAS_FSL_XHCI_USB
> > +
> > +#ifdef CONFIG_HAS_FSL_XHCI_USB
> > +#define CONFIG_USB_XHCI_FSL
> > +#define CONFIG_USB_XHCI_DWC3
> > +#define CONFIG_USB_XHCI
> > +#define CONFIG_USB_MAX_CONTROLLER_COUNT        1
> > +#define CONFIG_SYS_USB_XHCI_MAX_ROOT_PORTS     2
> > +#endif
> > +
> > +#if defined(CONFIG_HAS_FSL_DR_USB) ||
> > +defined(CONFIG_HAS_FSL_XHCI_USB) #define CONFIG_CMD_USB
> #define
> > +CONFIG_USB_STORAGE #define CONFIG_CMD_EXT2 #endif
> > +
> > +/*
> >   * Generic Timer Definitions
> >   */
> >  #define GENERIC_TIMER_CLK		12500000
> > diff --git a/include/linux/usb/xhci-fsl.h
> > b/include/linux/usb/xhci-fsl.h index 8eaab2c..329abf7 100644
> > --- a/include/linux/usb/xhci-fsl.h
> > +++ b/include/linux/usb/xhci-fsl.h
> > @@ -46,6 +46,11 @@
> >  #define USBOTGSS_IRQ_SET_1_OEVT_EN		BIT(16)
> >  #define USBOTGSS_IRQ_SET_1_DMADISABLECLR_EN	BIT(17)
> >
> > +#ifdef CONFIG_LS102XA
> > +#define CONFIG_SYS_FSL_XHCI_USB1_ADDR
> > +CONFIG_SYS_LS102XA_XHCI_USB1_ADDR #define
> > +CONFIG_SYS_FSL_XHCI_USB2_ADDR 0
> 
> Do you need to define this _bogus_ address at all? If so, then the driver
> which depends on this is broken. Why can't you just define a list of addresses
> instead ? You would be able to use ARRAY_SIZE() in the driver to determine
> how many controllers there are then. This is how it would look like:
> 
> #define CONFIG_FOO_BAR_ADDRS { USB1_ADDR, USB2_ADDR, ...,
> USBn_ADDR }
> 
> In the driver, there'd be:
> 
> <type> addrs[] = CONFIG_FOO_BAR_ADDRS;
> 
I agree to use an array for defining list of controller addresses. However, the no.
of controller(s) to be initialized on a particular platform is determined by CONFIG_USB_MAX_CONTROLLER_COUNT
used in usb_init() function. This macro is defined in each platform file, and defines the index argument passed on to
xhci_hcd_init(). There may be some platform on which we can have more than one controller in soc, but only
one is used (exposed via external connector). Hence,  CONFIG_USB_MAX_CONTROLLER_COUNT is defined by
platform header file.
Hence, I can assign controller address on the basis of address:
struct fsl_xhci *ctx = &fsl_xhci;
ctx->hcd = addrs[index];

> > +#endif
> > +
> >  struct fsl_xhci {
> >  	struct xhci_hccr *hcd;
> >  	struct dwc3 *dwc3_reg;

[U-Boot] [PATCH 3/4][v2]include:configs:ls1021atwr: Enable USB IP support

[Marek Vasut]

On Monday, April 27, 2015 at 07:44:15 AM, Ramneek Mehresh wrote:
[...]
> > > diff --git a/include/linux/usb/xhci-fsl.h
> > > b/include/linux/usb/xhci-fsl.h index 8eaab2c..329abf7 100644
> > > --- a/include/linux/usb/xhci-fsl.h
> > > +++ b/include/linux/usb/xhci-fsl.h
> > > @@ -46,6 +46,11 @@
> > > 
> > >  #define USBOTGSS_IRQ_SET_1_OEVT_EN		BIT(16)
> > >  #define USBOTGSS_IRQ_SET_1_DMADISABLECLR_EN	BIT(17)
> > > 
> > > +#ifdef CONFIG_LS102XA
> > > +#define CONFIG_SYS_FSL_XHCI_USB1_ADDR
> > > +CONFIG_SYS_LS102XA_XHCI_USB1_ADDR #define
> > > +CONFIG_SYS_FSL_XHCI_USB2_ADDR 0
> > 
> > Do you need to define this _bogus_ address at all? If so, then the driver
> > which depends on this is broken. Why can't you just define a list of
> > addresses instead ? You would be able to use ARRAY_SIZE() in the driver
> > to determine how many controllers there are then. This is how it would
> > look like:
> > 
> > #define CONFIG_FOO_BAR_ADDRS { USB1_ADDR, USB2_ADDR, ...,
> > USBn_ADDR }
> > 
> > In the driver, there'd be:
> > 
> > <type> addrs[] = CONFIG_FOO_BAR_ADDRS;
> 
> I agree to use an array for defining list of controller addresses. However,
> the no. of controller(s) to be initialized on a particular platform is
> determined by CONFIG_USB_MAX_CONTROLLER_COUNT used in usb_init() function.
> This macro is defined in each platform file, and defines the index
> argument passed on to xhci_hcd_init(). There may be some platform on which
> we can have more than one controller in soc, but only one is used (exposed
> via external connector). Hence,  CONFIG_USB_MAX_CONTROLLER_COUNT is
> defined by platform header file.

Ah right, sorry. Keep the CONFIG_USB_MAX_CONTROLLER_COUNT macro then.

> Hence, I can assign controller address on the basis of address:
> struct fsl_xhci *ctx = &fsl_xhci;
> ctx->hcd = addrs[index];

Yeah

Best regards,
Marek Vasut