[U-Boot] [PATCH V5 02/18] usb: ehci-mx6: add support for host mode otg port

[Marek Vasut]

Dear Troy Kisky,

> On 9/27/2013 10:50 AM, Marek Vasut wrote:
> > Dear Troy Kisky,
> > 
> >> Previously, only host1 was supported using an index of 0.
> >> Now, otg has index 0, host1 is 1, host2 is 2, host3 is 3.
> >> Since OTG requires usbmode to be set after reset, I added
> >> CONFIG_EHCI_HCD_INIT_AFTER_RESET to nitrogen6x.h and
> >> mx6qsabreauto.h.
> >> 
> >> I also added a weak function board_ehci_power to handle
> >> turning power on/off for otg.
> >> 
> >> Signed-off-by: Troy Kisky <troy.kisky at boundarydevices.com>
> >> 
> >> ---
> >> V4: new patch, replaces "usb: gadget: mv_udc: fix hardware udc address
> >> for i.MX6" and has the bonus of giving OTG host mode support
> >> 
> >> V5: use CONFIG_EHCI_HCD_INIT_AFTER_RESET instead of a weak function.
> >> Return error if otg_id is high.
> >> ---
> >> 
> >>   drivers/usb/host/ehci-mx6.c     | 123
> >> 
> >> +++++++++++++++++++++++++++------------- include/configs/mx6qsabreauto.h
> >> |
> >> 
> >>    3 +-
> >>   
> >>   include/configs/nitrogen6x.h    |   3 +-
> >>   3 files changed, 87 insertions(+), 42 deletions(-)
> >> 
> >> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c
> >> index eb24af5..ff4424d 100644
> >> --- a/drivers/usb/host/ehci-mx6.c
> >> +++ b/drivers/usb/host/ehci-mx6.c
> >> @@ -35,6 +35,7 @@
> >> 
> >>   #define USBPHY_CTRL_CLKGATE			0x40000000
> >>   #define USBPHY_CTRL_ENUTMILEVEL3		0x00008000
> >>   #define USBPHY_CTRL_ENUTMILEVEL2		0x00004000
> >> 
> >> +#define USBPHY_CTRL_OTD_ID_BIT			27
> > 
> > Why is this anarchism used here? Why is it not defined as the rest of the
> > values? Btw. its OTG_ID_VALUE , not OTD.
> 
> Good catch on the typo!
> 
> > [...]
> > 
> >> @@ -123,31 +156,36 @@ static int usbh1_phy_enable(void)
> >> 
> >>   	/* Power up the PHY */
> >>   	__raw_writel(0, phy_reg + USBPHY_PWD);
> >>   	/* enable FS/LS device */
> >> 
> >> -	val = __raw_readl(phy_reg + USBPHY_CTRL);
> >> +	val = __raw_readl(phy_ctrl);
> >> 
> >>   	val |= (USBPHY_CTRL_ENUTMILEVEL2 | USBPHY_CTRL_ENUTMILEVEL3);
> >> 
> >> -	__raw_writel(val, phy_reg + USBPHY_CTRL);
> >> +	__raw_writel(val, phy_ctrl);
> >> 
> >> -	return 0;
> >> +	return (val >> USBPHY_CTRL_OTD_ID_BIT) & 1;
> > 
> > See above, you can use return (val & USBPHY_CTRL_OTG_ID_VALUE); here
> > instead.
> 
> Yes, I can do that, and also change my documentation along with it.
> 
> >>   }
> >> 
> >> -static void usbh1_oc_config(void)
> >> +struct usbnc_regs {
> >> +	u32	ctrl[4];	/* otg/host1-3 */
> > 
> > This won't scale. Align this with the # of controllers you claim you have
> > here, why dont you?
> 
> You've lost me here. Can you post the struct you'd like to see?

Something like u32 ctrl[CONFIG_HOW_MANY_PORTS_YOU_HAVE] or so would suffice. I 
think you have such a MAX_PORTS #define already.

> 
> > [...]
> > 
> >> diff --git a/include/configs/mx6qsabreauto.h
> >> b/include/configs/mx6qsabreauto.h index 5530fc6..9e48a49 100644
> >> --- a/include/configs/mx6qsabreauto.h
> >> +++ b/include/configs/mx6qsabreauto.h
> >> @@ -23,7 +23,8 @@
> >> 
> >>   #define CONFIG_USB_STORAGE
> >>   #define CONFIG_USB_HOST_ETHER
> >>   #define CONFIG_USB_ETHER_ASIX
> >> 
> >> -#define CONFIG_MXC_USB_PORT	1
> >> +#define CONFIG_USB_MAX_CONTROLLER_COUNT 2
> >> +#define CONFIG_EHCI_HCD_INIT_AFTER_RESET	/* For OTG port */
> >> 
> >>   #define CONFIG_MXC_USB_PORTSC	(PORT_PTS_UTMI | PORT_PTS_PTW)
> >>   #define CONFIG_MXC_USB_FLAGS	0
> >> 
> >> diff --git a/include/configs/nitrogen6x.h b/include/configs/nitrogen6x.h
> >> index 3454b86..2e5ae13 100644
> >> --- a/include/configs/nitrogen6x.h
> >> +++ b/include/configs/nitrogen6x.h
> >> @@ -119,7 +119,8 @@
> >> 
> >>   #define CONFIG_USB_HOST_ETHER
> >>   #define CONFIG_USB_ETHER_ASIX
> >>   #define CONFIG_USB_ETHER_SMSC95XX
> >> 
> >> -#define CONFIG_MXC_USB_PORT	1
> >> +#define CONFIG_USB_MAX_CONTROLLER_COUNT 2
> >> +#define CONFIG_EHCI_HCD_INIT_AFTER_RESET	/* For OTG port */
> >> 
> >>   #define CONFIG_MXC_USB_PORTSC	(PORT_PTS_UTMI | PORT_PTS_PTW)
> >>   #define CONFIG_MXC_USB_FLAGS	0
> > 
> > Does this patch do two things here ? I think it does, if I'm right, split
> > this board adjustment away please.
> 
> If I split this, USB will stop working between the 2 patches. The OTG
> port will be initialized
> instead of the main USB port, and it won't work because it requires
> CONFIG_EHCI_HCD_INIT_AFTER_RESET.

OK, keep it.

Best regards,
Marek Vasut