Hi, 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. if we have that requirement, that's totally fine. Just rename __usb_charger_set_cur_limit() back to _usb_charger_set_cur_limit_by_type() and expose both. But set_cur_limit_by_type can assume its arguments are valid at all times. -- balbi
Attachment:
signature.asc
Description: PGP signature