Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932238AbcDFLbO (ORCPT ); Wed, 6 Apr 2016 07:31:14 -0400 Received: from mail-yw0-f175.google.com ([209.85.161.175]:33945 "EHLO mail-yw0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753071AbcDFLbM (ORCPT ); Wed, 6 Apr 2016 07:31:12 -0400 MIME-Version: 1.0 In-Reply-To: References: <6c594cc66fd06b575b04cc8bb0fe0374d0501d4d.1459494744.git.baolin.wang@linaro.org> Date: Wed, 6 Apr 2016 19:31:11 +0800 Message-ID: Subject: Re: [PATCH v9 1/4] gadget: Introduce the usb charger framework From: Baolin Wang To: Jun Li Cc: "balbi@kernel.org" , "gregkh@linuxfoundation.org" , "sre@kernel.org" , "dbaryshkov@gmail.com" , "dwmw2@infradead.org" , "peter.chen@freescale.com" , "stern@rowland.harvard.edu" , "r.baldyga@samsung.com" , "yoshihiro.shimoda.uh@renesas.com" , "lee.jones@linaro.org" , "broonie@kernel.org" , "ckeepax@opensource.wolfsonmicro.com" , "patches@opensource.wolfsonmicro.com" , "linux-pm@vger.kernel.org" , "linux-usb@vger.kernel.org" , "device-mainlining@lists.linuxfoundation.org" , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2825 Lines: 74 On 6 April 2016 at 16:26, Jun Li 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. > > Ideal is with your framework, diff users only need implement > uchger->charger_detect(). :) > -- Baolin.wang Best Regards