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

[Peng Fan <B51431@freescale.com>]

? 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. How about just use
"int board_usb_phy_mode(int index)" without using 'val' or 'struct 
usb_ehci *ehci'.

> Best regards,
> Marek Vasut
>
Regards,
Peng.