From mboxrd@z Thu Jan 1 00:00:00 1970 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753402AbeABAyl (ORCPT + 1 other); Mon, 1 Jan 2018 19:54:41 -0500 Received: from mailout2.samsung.com ([203.254.224.25]:45786 "EHLO mailout2.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751726AbeABAyk (ORCPT ); Mon, 1 Jan 2018 19:54:40 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.samsung.com 20180102005438epoutp02c5838e14e257bd82497ae6d6748bc4b1~F2B8r1-K-2289422894epoutp02N X-AuditID: b6c32a47-5ebff70000001126-56-5a4ad84d94f9 MIME-version: 1.0 Content-transfer-encoding: 8BIT Content-type: text/plain; charset="UTF-8" Message-id: <5A4AD84D.600@samsung.com> Date: Tue, 02 Jan 2018 09:54:37 +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: linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/4] extcon: axp288: Redo charger type dection a couple of seconds after probe() In-reply-to: <20171222123616.9562-3-hdegoede@redhat.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrHKsWRmVeSWpSXmKPExsWy7bCmma7vDa8og/cTtCzeHJ/OZHF51xw2 i9uNK9gcmD3e77vK5tG3ZRWjx+dNcgHMUak2GamJKalFCql5yfkpmXnptkrewfHO8aZmBoa6 hpYW5koKeYm5qbZKLj4Bum6ZOUCLlBTKEnNKgUIBicXFSvp2NkX5pSWpChn5xSW2StGGhkZ6 hgbmekZGRnomxrFWRqZAJQmpGZ/PPmIqeK9SsfjJctYGxgeyXYwcHBICJhJnJyV2MXJxCAns YJRYuWoqM4TznVFi4uT/jF2MnGBFJ/v2MkIkdjNKfG2fyg6S4BUQlPgx+R4LyCRmAXmJI5ey QcLMApoSW3evZ4eov8cocfPVYah6NYlp59qZQGwWAVWJ28uus4HYbAJaEvtf3ACz+QUUJa7+ eAy2WFQgQmLn/G9gvSICQRI3vq9ghVigIPHr3iYwW1ggTeLInCssIDangIXE5Ys3wQ6VEFjA JnGl7RYzxAcuEie2n2GFsIUlXh3fwg5hS0s8W7URqqGdUaJ97zxmCGcKo8S56/eYIKqMJZ4t 7GKCWM0n0XH4Lzsk8HglOtqEIEo8JD5vXAoNLkeJeZ8es0K8v5NRou3/XMYJjHKzkEJsFiLE ZiGF2AJG5lWMYqkFxbnpqcVGBcZ6xYm5xaV56XrJ+bmbGMFpS8t9B+O2cz6HGAU4GJV4eDfE e0UJsSaWFVfmHmKU4GBWEuGtVfSMEuJNSaysSi3Kjy8qzUktPsRoCgzwicxSosn5wJSaVxJv aGJpYGJmZmRuZgFMYeK8rQEuUUIC6YklqdmpqQWpRTB9TBycUg2M9oVX791is9mx+cQbt4vR uYoty3KfX5XgMZ558LbGRbdER+P25anfHk5Our3W8qHwMq+j0xLym2dM53rD+S+i1WXPuzbv aSLP2RyXCT+8kJd2+dE62YKIix5/HgZ/2lLAJ8B55eXzu3f/fq5bfWHn92mSNq88bB5ahJ98 N4/7YtkSS5VmyZ2uh5VYijMSDbWYi4oTAbVFIB5xAwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrALMWRmVeSWpSXmKPExsVy+t9jAV3fG15RBku2i1u8OT6dyeLyrjls FrcbV7A5MHu833eVzaNvyypGj8+b5AKYo7hsUlJzMstSi/TtErgyPp99xFTwXqVi8ZPlrA2M D2S7GDk5JARMJE727WXsYuTiEBLYySixZ+8lVpAEr4CgxI/J91i6GDk4mAXkJY5cygYJMwuo S0yat4gZov4Bo8SVAxOYIOrVJKadawezWQRUJW4vu84GYrMJaEnsf3EDzOYXUJS4+uMxI8hM UYEIie4TlSBhEYEAiZ+n+tkh5itI/Lq3CewEYYE0iQ1TGhBuW7V5A9h8TgELicsXbzJOYBSY heTUWQinzkJy6gJG5lWMkqkFxbnpucVGBUZ5qeV6xYm5xaV56XrJ+bmbGIHhuu2wVv8OxsdL 4g8xCnAwKvHwboj3ihJiTSwrrsw9xCjBwawkwlur6BklxJuSWFmVWpQfX1Sak1p8iFGag0VJ nJc//1ikkEB6YklqdmpqQWoRTJaJg1OqgdEorDjry5+DH6V/Xc2Qf70k9+clt2vCNReEw6WN g/Le24m2MZxdUjYvWDriJfPmj/VrzFLklOJK0q23bvEXmCSZ8Ynhpu7Vt7NqOEzuz449/Xcz J6fpkuObL026O+PPoZgLB7cvimlSz7qTej8vfnPL9ECvtU86dmhHLLKKka4Xf/KsnmnHaiMl luKMREMt5qLiRADcfa5gUwIAAA== X-CMS-MailID: 20180102005437epcas2p4210b86159b4d6ce4ff199df8da6d5075 X-Msg-Generator: CA CMS-TYPE: 102P DLP-Filter: Pass X-CFilter-Loop: Reflected X-CMS-RootMailID: 20171222123640epcas3p190e0800ab69794c55de84381f8a543fb X-RootMTR: 20171222123640epcas3p190e0800ab69794c55de84381f8a543fb References: <20171222123616.9562-1-hdegoede@redhat.com> <20171222123616.9562-3-hdegoede@redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: Hi Hans, s/dection/detection on patch title. On 2017년 12월 22일 21:36, Hans de Goede wrote: > The axp288 extcon code depends on other drivers to do things like mux the > data lines, enable/disable vbus based on the id-pin, etc. > > Sometimes the BIOS has not set these things up correctly resulting in the > initial charger cable type detection giving a wrong result and we end up > not charging or charging at only 0.5A. > > This commit starts a second charger-detection cycle a couple of seconds > after the first one finishes, giving the other drivers time to load and > do their thing. > > Signed-off-by: Hans de Goede > --- > drivers/extcon/extcon-axp288.c | 32 ++++++++++++++++++++++++++++++-- > 1 file changed, 30 insertions(+), 2 deletions(-) > > diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c > index 386afb7d1160..cc7c35c7ff02 100644 > --- a/drivers/extcon/extcon-axp288.c > +++ b/drivers/extcon/extcon-axp288.c > @@ -1,6 +1,7 @@ > /* > * extcon-axp288.c - X-Power AXP288 PMIC extcon cable detection driver > * > + * Copyright (C) 2016-2017 Hans de Goede > * Copyright (C) 2015 Intel Corporation > * Author: Ramakrishna Pallala > * > @@ -97,9 +98,11 @@ struct axp288_extcon_info { > struct device *dev; > struct regmap *regmap; > struct regmap_irq_chip_data *regmap_irqc; > + struct delayed_work det_work; > int irq[EXTCON_IRQ_END]; > struct extcon_dev *edev; > unsigned int previous_cable; > + bool first_detect_done; The first_detect_done is used only one time in the axp288_handle_chrg_det_event(). The other function don't use it. So, you better to define and use 'static bool first_detect_done' in the axp288_handle_chrg_det_event() instead of defining the 'first_detect_done' in the struct axp288_extcon_info. > }; > > /* Power up/down reason string array */ > @@ -137,6 +140,25 @@ static void axp288_extcon_log_rsi(struct axp288_extcon_info *info) > regmap_write(info->regmap, AXP288_PS_BOOT_REASON_REG, clear_mask); > } > > +static void axp288_chrg_detect_complete(struct axp288_extcon_info *info) > +{ > + /* > + * We depend on other drivers to do things like mux the data lines, > + * enable/disable vbus based on the id-pin, etc. Sometimes the BIOS has > + * not set these things up correctly resulting in the initial charger > + * cable type detection giving a wrong result and we end up not charging > + * or charging at only 0.5A. > + * > + * So we schedule a second cable type detection after 2 seconds to > + * give the other drivers time to load and do their thing. > + */ > + if (!info->first_detect_done) { > + queue_delayed_work(system_wq, &info->det_work, > + msecs_to_jiffies(2000)); > + info->first_detect_done = true; > + } > +} The axp288_chrg_detect_complete() is only used in the axp288_handle_chrg_det_event() and axp288_chrg_detect_complete() is not a complicate. I think that you don't need to make the separate function. I'd like you to add the > + > static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info) > { > int ret, stat, cfg, pwr_stat; > @@ -201,6 +223,8 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info) > info->previous_cable = cable; > } > > + axp288_chrg_detect_complete(info); As I commented, you better to add the code directly instead of separate function. > + > return 0; > > dev_det_ret: > @@ -222,8 +246,11 @@ static irqreturn_t axp288_extcon_isr(int irq, void *data) > return IRQ_HANDLED; > } > > -static void axp288_extcon_enable(struct axp288_extcon_info *info) > +static void axp288_extcon_det_work(struct work_struct *work) > { > + struct axp288_extcon_info *info = > + container_of(work, struct axp288_extcon_info, det_work.work); > + > regmap_update_bits(info->regmap, AXP288_BC_GLOBAL_REG, > BC_GLOBAL_RUN, 0); > /* Enable the charger detection logic */ > @@ -245,6 +272,7 @@ static int axp288_extcon_probe(struct platform_device *pdev) > info->regmap = axp20x->regmap; > info->regmap_irqc = axp20x->regmap_irqc; > info->previous_cable = EXTCON_NONE; > + INIT_DELAYED_WORK(&info->det_work, axp288_extcon_det_work); > > platform_set_drvdata(pdev, info); > > @@ -287,7 +315,7 @@ static int axp288_extcon_probe(struct platform_device *pdev) > } > > /* Start charger cable type detection */ > - axp288_extcon_enable(info); > + queue_delayed_work(system_wq, &info->det_work, 0); > > return 0; > } > -- Best Regards, Chanwoo Choi Samsung Electronics