On Wed, Apr 06, 2016 at 04:58:03PM +0300, Felipe Balbi wrote: > > Hi, > > Jun Li <jun...@nxp.com> writes: > >> >> >> > Since we already have get_charger_type callback at usb_charger > >> >> >> > structure, why we still need this API at usb_gadget_ops? > >> >> >> > >> >> >> In case some users want to get charger type at gadget level. > >> >> >> > >> >> > Why gadget needs to know charger type? I also don't catch the > >> >> > intent of > >> >> > >> >> because some gadgets need to call usb_gadget_vbus_draw(), although > >> >> for that they need power in mA rather. > >> > > >> > In below change of usb_gadget_vbus_draw(), already can get charger > >> > type via usb_charger_get_type(). > >> > > >> > static inline int usb_gadget_vbus_draw(struct usb_gadget *gadget, > >> > unsigned mA) { > >> > + enum usb_charger_type type; > >> > + > >> > + if (gadget->charger) { > >> > + type = usb_charger_get_type(gadget->charger); > >> > + usb_charger_set_cur_limit_by_type(gadget->charger, type, > >> mA); > >> > + } > >> > + > >> > if (!gadget->ops->vbus_draw) > >> > return -EOPNOTSUPP; > >> > return gadget->ops->vbus_draw(gadget, mA); > >> > > >> > Could you detail in what situation gadget->ops-> get_charger_type() is > >> used? > >> > >> isn't it right there in the code ? Isn't usb_gadget_vbus_draw() calling > >> it ? What did I miss here ? > > > > Well, that's true, so my real meaning is why gadget need get charger type > > via another new api gadget->ops->get_charger_type(). > > because of semantics. usb_gadget_vbus_draw() is *only* supposed to > connect a load across vbus and gnd so some battery can be charged. Also, > we need to abstract away this ->get_charger_type() operation because it > might be different for each UDC.
In this patch set, there are two ->get_charger_type in below two structures, as my understanding, get_charger_type at struct usb_charger can be implemented at UDC drivers; But I don't see necessary we need to implement get_charger_type for usb_gadget_ops at UDC drivers again. What do you think? struct usb_gadget_ops { .... struct usb_ep *(*match_ep)(struct usb_gadget *, + /* get the charger type */ + enum usb_charger_type (*get_charger_type)(struct usb_gadget *); }; struct usb_charger { ... + /* user can get charger type by implementing this callback */ + enum usb_charger_type (*get_charger_type)(struct usb_charger *); } > > $subject has a fragility, however: It's assuming that we should always > call ->get_charger_type() before ->vbus_draw(), but that's a good > default, I'd say. > -- Best Regards, Peter Chen