Hi Felipe, On 29 June 2016 at 16:20, Felipe Balbi <balbi@xxxxxxxxxx> wrote: > > Hi, > > Baolin Wang <baolin.wang@xxxxxxxxxx> writes: >>> Baolin Wang <baolin.wang@xxxxxxxxxx> writes: >>>> For supporting the usb charger, it adds the usb_charger_init() and >>>> usb_charger_exit() functions for usb charger initialization and exit. >>>> >>>> It will report to the usb charger when the gadget state is changed, >>>> then the usb charger can do the power things. >>>> >>>> Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxx> >>>> Reviewed-by: Li Jun <jun.li@xxxxxxx> >>>> Tested-by: Li Jun <jun.li@xxxxxxx> >>> >>> Before anything, I must say that I really liked this patch. It's >>> minimaly invasive to udc core and does all the necessary changes. If it >>> wasn't for the extra charger class, this would've been perfect. >>> >>> Can't you just tie a charger to a UDC and avoid the charger class >>> completely? >>> >>>> static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA) >>>> { >>>> + if (gadget->charger) >>> >>> I guess you could do this check inside >>> usb_gadget_set_cur_limit_by_type() itself. >> >> We will access the 'gadget->charger->type' member when issuing >> usb_gadget_set_cur_limit_by_type(), so I think I should leave the >> check here in next new version. > > Here's what I mean: > > int usb_charger_set_cur_limit(struct usb_gadget *gadget, unsigned int mA) > { > struct usb_charger *charger; > enum usb_charger_type type; > > if (!gadget->charger) > return 0; > > charger = gadget->charger; > type = charger->type; > > return __usb_charger_set_cur_limit(charger, type, mA); > } But that means we need to export both 'usb_charger_set_cur_limit()' function and '__usb_charger_set_cur_limit()' function in charger.c file. Cause some user may want to set the current limitation by one charger type parameter (may be not from charger->type), like by issuing '__usb_charger_set_cur_limit(charger, SDP_TYPE, mA)'. How do you think about this situation? Thanks. > > static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget, unsigned mA) > { > usb_charger_set_cur_limit(gadget, mA); > > if (!gadget->ops->vbus_draw) > return -EOPNOTSUPP; > return gadget->ops->vbus_draw(gadget, mA); > } > > -- > balbi -- Baolin.wang Best Regards -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html