[U-Boot] [PATCH 3/4] sunxi: common VBUS detection logic in usbc

[Paul Kocialkowski]

Le dimanche 15 mars 2015 à 19:19 +0100, Marek Vasut a écrit :
> On Sunday, March 15, 2015 at 06:27:46 PM, Paul Kocialkowski wrote:
> > VBUS detection could be needed not only by the musb code (to prevent host
> > mode), but also by e.g. gadget drivers to start only when a cable is
> > connected.
> > 
> > In addition, this allows more flexibility in vbus detection, as it could
> > easily be extended to other USBC indexes. Eventually, this would help
> > making musb support independent from a hardcoded USB controller index (0).
> > 
> > Signed-off-by: Paul Kocialkowski <contact at paulk.fr>
> 
> Hi!
> 
> [...]
> 
> > @@ -113,6 +120,17 @@ static int get_vbus_gpio(int index)
> >  	return -1;
> >  }
> > 
> > +static int get_vbus_detect_gpio(int index)
> > +{
> > +	if (use_axp_vbus_detect(index))
> > +		return -1;
> > +
> > +	switch (index) {
> > +	case 0: return sunxi_name_to_gpio(CONFIG_USB0_VBUS_DET);
> 
> What happens if the index != 0 here ? Default branch with some error
> report might be a good idea here.

Well, I'm going to have a very stupid answer here, that is: I copied
what was already being done nearby, namely in get_vbus_gpio for this
one. So I think that we should modify both functions accordingly.

I don't think it makes so much sense to print an error here because this
will never be called directly, but instead through
sunxi_usbc_request_resources which will make noise when it fails.
We could perhaps add some error printing in sunxi_usbc_request_resources
to make it clear what failed, but I'm not really for it.

> > +	}
> > +	return -1;
> > +}
> > +
> 
> [...]
> 
> > @@ -270,3 +297,34 @@ void sunxi_usbc_vbus_disable(int index)
> >  	if (sunxi_usbc->gpio_vbus != -1)
> >  		gpio_direction_output(sunxi_usbc->gpio_vbus, 0);
> >  }
> > +
> > +int sunxi_usbc_vbus_detect(int index)
> > +{
> > +	struct sunxi_usbc_hcd *sunxi_usbc = &sunxi_usbc_hcd[index];
> > +	int err;
> > +
> > +#ifdef AXP_VBUS_DETECT
> 
> Can't you just wrap this ifdef into use_axp_vbus_detect(), so the
> code isn't so riddled with the ifdeffery ?

I agree those ifdefs are not used consistently and should be moved to
one place. Your suggestion looks fine to me.

> > +	if (use_axp_vbus_detect(index)) {
> > +		err = axp_get_vbus();
> > +		if (err < 0)
> > +			return err;
> > +	} else {
> > +#endif
> > +		if (sunxi_usbc->gpio_vbus_det == -1) {
> > +			eprintf("Error: invalid vbus detection pin\n");
> > +			return -1;
> > +		}
> > +
> > +		err = gpio_direction_input(sunxi_usbc->gpio_vbus_det);
> > +		if (err)
> > +			return err;
> > +
> > +		err = gpio_get_value(sunxi_usbc->gpio_vbus_det);
> > +		if (err < 0)
> > +			return err;
> > +#ifdef AXP_VBUS_DETECT
> > +	}
> > +#endif
> > +
> > +	return err;
> 
> [...]

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150315/1d7fa5a1/attachment.sig>