[U-Boot] [PATCH v3] colibri_vf: Add board_usb_phy_mode function

[Marek Vasut <marex@denx.de>]

On Friday, November 06, 2015 at 08:00:01 AM, maitysanchayan at gmail.com wrote:
> On 15-10-30 14:20:29, Marek Vasut wrote:
> > On Friday, October 30, 2015 at 01:26:59 PM, Sanchayan Maity wrote:
> > > Add board_usb_phy_mode function for detecting whether a port is
> > > being used as host or client using a GPIO. On Colibri Vybrid we
> > > provide the GPIO 102 for this very same purpose.
> > > 
> > > Signed-off-by: Sanchayan Maity <maitysanchayan@gmail.com>
> > > ---
> > > Changes since v1:
> > > 
> > > Move the GPIO request call to the board_init function as all
> > > further calls to board_usb_phy_mode will actually result in the
> > > gpio_request failing.
> > > 
> > > Changes since v2:
> > > 
> > > Instead of returning 0 from board_usb_phy_mode return it as
> > > USB_INIT_HOST.
> > > ---
> > > 
> > >  board/toradex/colibri_vf/colibri_vf.c | 23 ++++++++++++++++++++++-
> > >  1 file changed, 22 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/board/toradex/colibri_vf/colibri_vf.c
> > > b/board/toradex/colibri_vf/colibri_vf.c index a6d1c5b..9878671 100644
> > > --- a/board/toradex/colibri_vf/colibri_vf.c
> > > +++ b/board/toradex/colibri_vf/colibri_vf.c
> > > @@ -21,6 +21,7 @@
> > > 
> > >  #include <i2c.h>
> > >  #include <g_dnl.h>
> > >  #include <asm/gpio.h>
> > > 
> > > +#include <usb.h>
> > > 
> > >  DECLARE_GLOBAL_DATA_PTR;
> > > 
> > > @@ -34,6 +35,7 @@ DECLARE_GLOBAL_DATA_PTR;
> > > 
> > >  			PAD_CTL_DSE_50ohm | PAD_CTL_OBE_IBE_ENABLE)
> > >  
> > >  #define USB_PEN_GPIO           83
> > > 
> > > +#define USB_CDET_GPIO			102
> > > 
> > >  static struct ddrmc_cr_setting colibri_vf_cr_settings[] = {
> > >  
> > >  	/* levelling */
> > > 
> > > @@ -92,6 +94,7 @@ static struct ddrmc_cr_setting
> > > colibri_vf_cr_settings[] = {
> > > 
> > >  static const iomux_v3_cfg_t usb_pads[] = {
> > >  
> > >  	VF610_PAD_PTD4__GPIO_83,
> > > 
> > > +	VF610_PAD_PTC29__GPIO_102,
> > > 
> > >  };
> > >  
> > >  int dram_init(void)
> > > 
> > > @@ -280,7 +283,6 @@ static void setup_iomux_gpio(void)
> > > 
> > >  		VF610_PAD_PTB23__GPIO_93,
> > >  		VF610_PAD_PTB26__GPIO_96,
> > >  		VF610_PAD_PTB28__GPIO_98,
> > > 
> > > -		VF610_PAD_PTC29__GPIO_102,
> > > 
> > >  		VF610_PAD_PTC30__GPIO_103,
> > >  		VF610_PAD_PTA7__GPIO_134,
> > >  	
> > >  	};
> > > 
> > > @@ -509,6 +511,10 @@ int board_init(void)
> > > 
> > >  	setbits_le32(&scsc->sosc_ctr, SCSC_SOSC_CTR_SOSC_EN);
> > > 
> > > +#ifdef CONFIG_USB_EHCI_VF
> > > +	gpio_request(USB_CDET_GPIO, "usb-cdet-gpio");
> > > +#endif
> > > +
> > > 
> > >  	return 0;
> > >  
> > >  }
> > > 
> > > @@ -554,4 +560,19 @@ int board_ehci_hcd_init(int port)
> > > 
> > >  	}
> > >  	return 0;
> > >  
> > >  }
> > > 
> > > +
> > > +int board_usb_phy_mode(int port)
> > > +{
> > > +	switch (port) {
> > > +	case 0:
> > > +		return gpio_get_value(USB_CDET_GPIO);
> > 
> > So what would happen to this code in case we re-number the USB_INIT_HOST
> > and USB_INIT_DEVICE or in case the GPIO API starts returning something
> > else but 0 or 1 here ?
> 
> Sorry for the delay in reply. Had got tied up elsewhere.

Yeah, no problem, I can relate to that.

> Currently USB_INIT_HOST and USB_INIT_DEVICE are defined with an enum. If we
> were to switch this would not work as I intended. I guess this would be
> more appropriate then
> 
> if (gpio_get_value(USB_CDET_GPIO))
> 	return USB_INIT_DEVICE;
> else
> 	return USB_INIT_HOST;

Yeah.

> For the GPIO, isn't gpio_get_value always suppose to return the actual
> state of GPIO? Would we ever change the GPIO API to report otherwise?

It will report 0 for GPIO inactive and non-zero for GPIO active, that's
the behavior of it.

> Concerning the other thread from Stefan's query I will add the comments.

Thanks!