On Saturday, October 24, 2015 at 08:19:36 PM, maitysanchayan@gmail.com wrote:
On 15-10-24 19:50:13, Marek Vasut wrote:
On Saturday, October 24, 2015 at 06:23:44 PM, maitysanchayan@gmail.com
wrote:
On 15-10-24 18:16:20, Marek Vasut wrote:
On Saturday, October 24, 2015 at 06:08:57 PM, maitysanchayan@gmail.com
wrote:
On 15-10-24 18:08:53, Marek Vasut wrote:
On Saturday, October 24, 2015 at 05:23:05 PM, maitysanchayan@gmail.com
wrote:
> Hello, > > On 15-10-24 12:09:43, Fabio Estevam wrote: > > Hi Marek, > > > > On Fri, Oct 23, 2015 at 4:23 PM, Marek Vasut marex@denx.de
wrote:
> > >> Any inputs on the below? > > > > > > I don't have a Vybrid device, CCing Fabio. > > > > I don't have access to a Vybrid board either. > > > > Sanchayan, > > > > Does drivers/usb/host/ehci-mx6.c behave the same way? > > No. > > I included the particular piece of code below > > if (init == USB_INIT_DEVICE && index == 1) > > return -ENODEV; > > if (init == USB_INIT_HOST && index == 0) > > return -ENODEV; > > in the ehci-vf driver because our requirement was to have one > port as client and other as host. Since on USB start both > ports get configured as host as it iterates depending on USB > EHCI controller count, the above was meant to stop the port > required as client to be configured as host and vice versa > while using client functionality such as DFU. > > I made the mistake of not thinking that this is not a generic > use case, someone might want it the other way around or such. > > So coming to the main question, what would be the correct way > to fix this? I tested that even if the above four lines are > removed and USB start configures both ports as host, calling > dfu later will still result in correct functioning. So is this > ok and the four lines should be nuked or a more appropriate > way would be to add something like > board_ehci_hcd_init_with_type(int index, enum usb_init _type > init) which would be a weak function and have the board > specific code call this to do the above which is currently > done in ehci-vf. > > I wasn't sure about the right approach to take so I asked.
Brief glare over the driver tells me that those four lines are complete nonsense and should be removed.
Yes very much so. But is removing just ok or it would be better to actually restrict as per a board's requirement what gets configured as host and what gets configured as client by adding the weak function hook I was talking about?
The mx6 ones does board_usb_phy_mode() to determine this restriction. But (!) it'd be even better if this information was obtained from DT. It'd be nice if someone started working on converting i.MX to DT.
Yes, that's how iMX6 does it. However note that Vybrid USB is not a true OTG.
I quote the Vybrid TRM here,
"The USB is not a true OTG. It can be configured by software to function either as a peripheral or as host. The ID pin, which is unique for OTG operation, is not present in this implementation. There are no five pin interface. The user will get four pin host/device interface."
Can't the user use a GPIO instead of dedicated OTG switch pin ?
Yes. In fact in Linux we use the extcon framework along with the GPIO to have automatic switch over between client and host.
I will send a patch by Monday to remove the incorrect code. Will later send a patch to incoporate the correct fix. Since all boards might not have a GPIO to do this I had not considered this option but if this is acceptable it is fine by me.
The boards should be able to implement such a GPIO-based switching in the board_usb_phy_mode(), no ?
btw please send both patches at the same time, this "later" usually means "never" in my opinion.