On Wed, Apr 06, 2016 at 04:58:03PM +0300, Felipe Balbi wrote:
>
> Hi,
>
> Jun Li <jun.li@xxxxxxx> 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
--
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
