Hi > -----Original Message----- > From: Baolin Wang [mailto:baolin.wang@xxxxxxxxxx] > Sent: Wednesday, April 06, 2016 7:31 PM > To: Jun Li <jun.li@xxxxxxx> > Cc: balbi@xxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; sre@xxxxxxxxxx; > dbaryshkov@xxxxxxxxx; dwmw2@xxxxxxxxxxxxx; peter.chen@xxxxxxxxxxxxx; > stern@xxxxxxxxxxxxxxxxxxx; r.baldyga@xxxxxxxxxxx; > yoshihiro.shimoda.uh@xxxxxxxxxxx; lee.jones@xxxxxxxxxx; broonie@xxxxxxxxxx; > ckeepax@xxxxxxxxxxxxxxxxxxxxxxxxxxx; patches@xxxxxxxxxxxxxxxxxxxxxxxxxxx; > linux-pm@xxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; device- > mainlining@xxxxxxxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx > Subject: Re: [PATCH v9 1/4] gadget: Introduce the usb charger framework > > On 6 April 2016 at 16:26, Jun Li <jun.li@xxxxxxx> wrote: > > Hi > > > >> + */ > >> +static enum usb_charger_type > >> +usb_charger_get_type_by_others(struct usb_charger *uchger) { > >> + if (uchger->type != UNKNOWN_TYPE) > >> + return uchger->type; > >> + > >> + if (uchger->psy) { > >> + union power_supply_propval val; > >> + > >> + power_supply_get_property(uchger->psy, > >> + POWER_SUPPLY_PROP_CHARGE_TYPE, > >> + &val); > >> + switch (val.intval) { > >> + case POWER_SUPPLY_TYPE_USB: > >> + uchger->type = SDP_TYPE; > >> + break; > >> + case POWER_SUPPLY_TYPE_USB_DCP: > >> + uchger->type = DCP_TYPE; > >> + break; > >> + case POWER_SUPPLY_TYPE_USB_CDP: > >> + uchger->type = CDP_TYPE; > >> + break; > >> + case POWER_SUPPLY_TYPE_USB_ACA: > >> + uchger->type = ACA_TYPE; > >> + break; > >> + default: > >> + uchger->type = UNKNOWN_TYPE; > >> + break; > >> + } > >> + } else if (uchger->get_charger_type) { > >> + uchger->type = uchger->get_charger_type(uchger); > >> + } else { > >> + uchger->type = UNKNOWN_TYPE; > >> + } > >> + > >> + return uchger->type; > >> +} > >> + > > > > I think we may don't need this usb_charger_get_type_by_others(). > > "uchger->type" is set in one place is enough, that is: by > > uchger->charger_detect() in usb_charger_detect_type(), then > > usb_charger_get_type_by_others() is replaced by usb_charger_get_type(). > > > > uchger->charger_detect() can have diff implementations no matter > > what kind of mechanism is used, for your PMIC case, you can just > > directly get the type value by power_supply_get_property(); with that, > > we can have one central place to set uchger->type. > > After uchger->type is set, charger type detection is no need to be > > involved until charger type changes. > > > > Then next question is where is to call usb_charger_detect_type(), We > > need make sure it finished before usb gadget connect. > > Yeah, that's the point: where? It is hard for usb charger framework to > control, which will make it more complicated. The > 'usb_charger_detect_type()' is used for detecting the charger type > manually on user's platform, and user should call it at the right time to > avoid affecting gadget enumeration. Otherwise user can implement some > callbacks showed in 'usb_charger_get_type_by_others()' function to get > charger type. I think this is controllable and simple for the usb charger > framework. As my suggested, Can this detection be in usb_udc_vbus_handler(), and before usb_udc_connect_control()? We should be able to find some central place where it is between vbus detection and gadget connect. > > > > > Ideal is with your framework, diff users only need implement > > uchger->charger_detect(). :) > > > > -- > Baolin.wang > Best Regards ��.n��������+%������w��{.n�����{���)�� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
