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

[Marek Vasut <marex@denx.de>]

On Wednesday, November 05, 2014 at 07:00:32 AM, Peng Fan wrote:
> ? 11/5/2014 1:33 AM, 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.
> > [...]
> 
> Passing struct usb_ehci * to board code seems exports ehci register
> definition to board layer.

Yeah.

> How about just use
> "int board_usb_phy_mode(int index)" without using 'val' or 'struct
> usb_ehci *ehci'.

The board part might need to read the EHCI registers though. How would the
board part be able to do it if you didn't pass the *ehci in ?