From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752304AbdDDGbz (ORCPT ); Tue, 4 Apr 2017 02:31:55 -0400 Received: from mailout1.samsung.com ([203.254.224.24]:25466 "EHLO epoutp01.samsung.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750961AbdDDGbx (ORCPT ); Tue, 4 Apr 2017 02:31:53 -0400 X-AuditID: b6c32a37-f79f46d000003503-b6-58e33dcc29f4 MIME-version: 1.0 Content-transfer-encoding: 8BIT Content-type: text/plain; charset="UTF-8" Message-id: <58E33DCC.6020905@samsung.com> Date: Tue, 04 Apr 2017 15:31:40 +0900 From: Chanwoo Choi Organization: Samsung Electronics User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 To: Hans de Goede , MyungJoo Ham Cc: Takashi Iwai , linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/3] extcon: intel-cht-wc: Disable external 5v boost converter on probe In-reply-to: <20170403112630.30625-2-hdegoede@redhat.com> X-Brightmail-Tracker: H4sIAAAAAAAAA01Sa0hTYRjuO2eXY7U8zUtvhnU80EVh0zMvm6GWJTUrwgpiXcxO7uAsd2Fn sxuR/smwm6Wg2CBLgvKP10QzUpYgYURZXrKa1brYWJZJJpHWtpPkv+d7vud5n+97eAlcPi2O IPJNNs5qYgtoyXxR64NoheJRqlsXN9gVpfH2VGKaZ3cdEs3L4lsSzedmL7ZepP16v1+ivdhS h7TNvSe1E03Ls0R7uRQDx+o5K8WZcs36fFNeKr11V87GnMSkOEbBJGvUNGVijVwqnbEtS7Ep v8CXSFOFbIHdR2WxPE/HpqVYzXYbRxnMvC2V3scwKiUTp1aqVCplQnz2WlWiT3KQM3wamsYt JdSx9tGvWBFqXFqKggggE6CqswwJOByeuOolpWg+ISfbEHRUTWLCoQSDmQ8ObNbx++m4WLhw IPj57WLALiMXw1S5S1SKCAInV0B33xE/jZPRcKejXiroXQg6b9aKBX0MXLjzFPfrReRKcF+K 99MSH905OiTx42AyCvqn3IHxYaQO2q9NSv04lNwJQz9viYX5adD3qzyAQ8hsqPg4EPAGkcng ae0S+XOBrJTC+9+lgSwgI6GpCxf+kgGvJzxiAYeAp6dFKuBlUDczjAneEgTNDte/QWcRjNwu /ueIh5E3Llx4xSIY+3FeLATI4OwZuQC1cK7dLqjTocNZLhJ66EDwYHhKUoao6jnVVf+vrnpO dTUIr0PhnIU35nE8Y4lX8qyRt5vylLlmYxMKrF6Mug01PN7mRCSB6IWymg3vdHIxW8gfNzoR EDgdKpuk3Dq5TM8eP8FZzTlWewHHO1Gir/nLeERYrtm3yCZbDpOQHJeQpPJtXTLD0Etk4fUD OjmZx9q4Ixxn4ayzPowIiihC6adGdxulD+/p/0zMJF1he2q+eEIcRXsiU9INm+0nR2XOrEOD 2ym1tNCeaVJID78NO9yf+TyqYkwBN9S7lr04vfrqgVVLbdNHXdOrKdJz21Cm6vaWfB+raVhz JXqdcaUVG6//1OcNb9yBp2yxvBpv7F2QqNMH1w7tj+273iue95wW8QaWicGtPPsXTDZuuJAD AAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrJIsWRmVeSWpSXmKPExsVy+t9jQd0zto8jDLbMF7F4c3w6k8XlXXPY LG43rmCzeLn5DZMDi8f7fVfZPPq2rGL02Hy62uPzJrkAlig3m4zUxJTUIoXUvOT8lMy8dFul 0BA3XQslhbzE3FRbpQhd35AgJYWyxJxSIM/IAA04OAe4Byvp2yW4ZTy/8Ze5oF2hYueL90wN jBsluxg5OSQETCT+XPzICmGLSVy4t56ti5GLQ0hgFqPEj8kzmEASvAKCQPY9li5GDg5mAXmJ I5eyQcLMAuoSk+YtYoaof8Ao8WnCOzaIei2J3q0XmUHqWQRUJR73G4OE2YDC+1/cACvhF1CU uPrjMSNIiahAhET3iUqQsIhAgMTPU/3sEOPtJC79mgx2mrBArMSUZ9egTtvNKNHXeo4RJMEp YCnxatsBlgmMgrOQXDoL4dJZSC5dwMi8ilEitSC5oDgpPdcwL7Vcrzgxt7g0L10vOT93EyM4 mp5J7WA8uMv9EKMAB6MSD+8Cp0cRQqyJZcWVuYcYJTiYlUR4vyk8jhDiTUmsrEotyo8vKs1J LT7EaAr06kRmKdHkfGCk55XEG5qYm5gbG1iYW1qaGCmJ8zbOfhYuJJCeWJKanZpakFoE08fE wSnVwGhxgv1CxOLabWVH/VUSsoTOSk656zrNZDMT47Y5lRLJ3cVfJBIaxXYvmrfOa5PA0l97 8terJDYW7VjQMfemcfekx9PcC7RnRh7WeTJvpRVDGueKVSfd9Kof6+9e2RN8ob46/qrsi85v a8R147fOnV/yIzb53Owf6kzc772a3cL7OeNfmWmXuiixFGckGmoxFxUnAgBm1z3qvAIAAA== X-MTR: 20000000000000000@CPGS X-CMS-MailID: 20170404063140epcas1p2f8e0c6a576294ec12dba513521f45392 X-Msg-Generator: CA X-Sender-IP: 203.254.230.26 X-Local-Sender: =?UTF-8?B?7LWc7LCs7JqwG1RpemVuIFBsYXRmb3JtIExhYihTL1fshLw=?= =?UTF-8?B?7YSwKRvsgrzshLHsoITsnpAbU2VuaW9yIEVuZ2luZWVy?= X-Global-Sender: =?UTF-8?B?Q2hhbndvbyBDaG9pG1RpemVuIFBsYXRmb3JtIExhYi4bU2Ft?= =?UTF-8?B?c3VuZyBFbGVjdHJvbmljcxtTZW5pb3IgRW5naW5lZXI=?= X-Sender-Code: =?UTF-8?B?QzEwG1NUQUYbQzEwVjgxMTE=?= CMS-TYPE: 101P DLP-Filter: Pass X-CFilter-Loop: Reflected X-CMS-RootMailID: 20170403112709epcas3p2c1736a0218aca67642ea2774e9e69f71 X-RootMTR: 20170403112709epcas3p2c1736a0218aca67642ea2774e9e69f71 References: <20170403112630.30625-1-hdegoede@redhat.com> <20170403112630.30625-2-hdegoede@redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 2017년 04월 03일 20:26, Hans de Goede wrote: > Disable the 5v boost converter on probe in case it was left on by > the BIOS, this fixes 2 problems: > > 1) This gets seen by the external battery charger as a valid Vbus > supply and it then tries to feed Vsys from this creating a > feedback loop which causes aprox. 300 mA extra battery drain > (and unless we drive the external-charger-disable pin high it > also tries to charge the battery causing even more feedback). > 2) This gets seen by the pwrsrc block as a SDP USB Vbus supply > > Since the external battery charger has its own 5v boost converter > which does not have these issues, we simply turn the separate > external 5v boost converter off and leave it off entirely. > > Signed-off-by: Hans de Goede > --- > drivers/extcon/extcon-intel-cht-wc.c | 38 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/drivers/extcon/extcon-intel-cht-wc.c b/drivers/extcon/extcon-intel-cht-wc.c > index f1c43af..8651f63 100644 > --- a/drivers/extcon/extcon-intel-cht-wc.c > +++ b/drivers/extcon/extcon-intel-cht-wc.c > @@ -64,6 +64,11 @@ > #define CHT_WC_PWRSRC_ID_GND BIT(3) > #define CHT_WC_PWRSRC_ID_FLOAT BIT(4) > > +#define CHT_WC_GPIO0_CTLO 0x6e2b > +#define CHT_WC_GPIO_CTLO_OUTPUT BIT(0) CHT_WC_GPIO0_CTLO or CHT_WC_GPIO_CTLO ? Because CHT_WC_GPIO_CTLO_OUTPUT defintion use just 'GPIO' instead of 'GPIO0'. I prefer to change the definition name as following with '_MASK' postfix. - CHT_WC_GPIO_CTLO_OUTPUT -> CHT_WC_GPIO_CTLO_OUTPUT_MASK > +#define CHT_WC_GPIO0_CTLO 0x6e2b > + > +#define CHT_WC_VBUS_ENABLE_GPIO 2 I think that 'CHT_WC_GPIO0_CTLO + CHT_WC_VBUS_ENABLE_GPIO' is a separate register address. Maybe you can define the separate register offset instead of the separate the CHT_WC_VBUS_ENABLE_GPIO offset. > + > enum cht_wc_usb_id { > USB_ID_OTG, > USB_ID_GND, > @@ -170,6 +175,24 @@ static void cht_wc_extcon_set_phymux(struct cht_wc_extcon_data *ext, u8 state) > dev_err(ext->dev, "Error writing phyctrl: %d\n", ret); > } > > +static void cht_wc_extcon_set_5v_boost(struct cht_wc_extcon_data *ext, > + bool enable) > +{ > + int ret, val; > + > + val = enable ? CHT_WC_GPIO_CTLO_OUTPUT : 0; > + > + /* > + * The 5V boost converter is enabled through a gpio on the PMIC, since > + * there currently is no gpio driver we access the gpio reg directly. > + */ > + ret = regmap_update_bits(ext->regmap, > + CHT_WC_GPIO0_CTLO + CHT_WC_VBUS_ENABLE_GPIO, > + CHT_WC_GPIO_CTLO_OUTPUT, val); > + if (ret) > + dev_err(ext->dev, "Error writing Vbus GPIO CTLO: %d\n", ret); > +} > + > /* Small helper to sync EXTCON_CHG_USB_SDP and EXTCON_USB state */ > static void cht_wc_extcon_set_state(struct cht_wc_extcon_data *ext, > unsigned int cable, bool state) > @@ -280,6 +303,21 @@ static int cht_wc_extcon_probe(struct platform_device *pdev) > if (IS_ERR(ext->edev)) > return PTR_ERR(ext->edev); > > + /* > + * When a host-cable is detected the BIOS enables an external 5v boost > + * converter to power connected devices there are 2 problems with this: > + * 1) This gets seen by the external battery charger as a valid Vbus > + * supply and it then tries to feed Vsys from this creating a > + * feedback loop which causes aprox. 300 mA extra battery drain > + * (and unless we drive the external-charger-disable pin high it > + * also tries to charge the battery causing even more feedback). > + * 2) This gets seen by the pwrsrc block as a SDP USB Vbus supply > + * Since the external battery charger has its own 5v boost converter > + * which does not have these issues, we simply turn the separate > + * external 5v boost converter off and leave it off entirely. > + */ > + cht_wc_extcon_set_5v_boost(ext, false); > + > /* Enable sw control */ > ret = cht_wc_extcon_sw_control(ext, true); > if (ret) > -- Best Regards, Chanwoo Choi Samsung Electronics