[U-Boot] [PATCH v2 1/3] usb:ehci-mx6 add board_ehci_usb_mode function

[Marek Vasut]

On Tuesday, November 04, 2014 at 02:29:56 PM, Peng Fan wrote:
> Hi Marek,
> 
> 在 11/4/2014 7:01 PM, Marek Vasut 写道:
> > On Tuesday, November 04, 2014 at 11:50:29 AM, Peng Fan wrote:
> >> 在 11/4/2014 6:33 PM, Marek Vasut 写道:
> >>> On Tuesday, November 04, 2014 at 08:50:00 AM, Peng Fan wrote:
> >>>> Include a weak function board_ehci_usb_mode to gives board code
> >>>> a choice.
> >>> 
> >>> What choice?
> >>> 
> >>>> If the board want the otg port work in host mode but not
> >>>> device mode, this should be handled.
> >>> 
> >>> How?
> >>> 
> >>> Also, isn't usb_phy_enable() supposed to do exactly this kind of
> >>> selection between device and host mode ?
> >> 
> >> In mx6sxsabresd board, there are two usb port, one used for otg, the
> >> other used for host. However they are connected to SOC USB controller
> >> otg1 core and otg2 core respectively. Like following:
> >> 
> >> OTG1 CORE <----> board otg port
> >> OTG2 CORE <----> board host port
> >> 
> >> However the board do not have ID pin set for board host port. If just
> >> use usb_phy_enable, the board host port will not work, because
> >> "type = usb_phy_enable(index, ehci) ? USB_INIT_DEVICE : USB_INIT_HOST;"
> >> will always set type with USB_INIT_DEVICE.
> >> 
> >> Because i did not find way to handle this situation in
> >> board/freescale/mx6sxsabresd/mx6sxsabresd.c, add this function to let
> >> board level code handle handle 'type', if board level code want to set
> >> it's own 'type'.
> > 
> > This part in usb_phy_enable()
> > 
> > 163         return val & USBPHY_CTRL_OTG_ID;
> > 
> > should be replaced by some kind of a board-specific callback then, with
> > default implmentation being the above (reading the phy ctrl register).
> 
> How about using the following piece of code?
> in ehci-mx6.c
> 
> unsigned int __weak board_usb_phy_mode(int index, unsigned int val)
> {
> 	return val & USBPHY_CTRL_OTG_ID;
> }
> 
> replace "return val & USBPHY_CTRL_OTG_ID;" using "
> return board_usb_phy_mode(index, val);"
> 
> In board file,
> unsigned int board_usb_phy_mode(int index, unsigned int val)

Why not pass in full struct usb_ehci * instead ? Passing some ad-hoc $val into
the function doesn't seem like a scalable future-proof solution.
[...]
Best regards,
Marek Vasut