[U-Boot] Doubt in USB driver for Vybrid vf610

[Marek Vasut]

On Saturday, October 24, 2015 at 08:19:36 PM, maitysanchayan at gmail.com wrote:
> On 15-10-24 19:50:13, Marek Vasut wrote:
> > On Saturday, October 24, 2015 at 06:23:44 PM, maitysanchayan at gmail.com 
wrote:
> > > On 15-10-24 18:16:20, Marek Vasut wrote:
> > > > On Saturday, October 24, 2015 at 06:08:57 PM,
> > > > maitysanchayan at gmail.com
> > 
> > wrote:
> > > > > On 15-10-24 18:08:53, Marek Vasut wrote:
> > > > > > On Saturday, October 24, 2015 at 05:23:05 PM,
> > > > > > maitysanchayan at 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 at 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.