From mboxrd@z Thu Jan 1 00:00:00 1970 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752008AbeAQBEt (ORCPT + 1 other); Tue, 16 Jan 2018 20:04:49 -0500 Received: from mailout3.samsung.com ([203.254.224.33]:21281 "EHLO mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751710AbeAQBEg (ORCPT ); Tue, 16 Jan 2018 20:04:36 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 mailout3.samsung.com 20180117010434epoutp0303aef87fb24ab487294dcab8135f0944~Kc15898wH0422404224epoutp03k X-AuditID: b6c32a47-5ebff70000001126-61-5a5ea121aaff MIME-version: 1.0 Content-transfer-encoding: 8BIT Content-type: text/plain; charset="utf-8" Message-id: <5A5EA121.1070902@samsung.com> Date: Wed, 17 Jan 2018 10:04:33 +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 , Chen-Yu Tsai Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] extcon: axp288: Only reschedule charger-detection at boot when a SDP is detected In-reply-to: X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrOKsWRmVeSWpSXmKPExsWy7bCmma7iwrgog7VTzCzeHJ/OZHF51xw2 i9uNK9gsfh46z+TA4rHh0WpWj/f7rrJ59G1ZxejxeZNcAEtUqk1GamJKapFCal5yfkpmXrqt kndwvHO8qZmBoa6hpYW5kkJeYm6qrZKLT4CuW2YO0EolhbLEnFKgUEBicbGSvp1NUX5pSapC Rn5xia1StKGhkZ6hgbmekZGRnolxrJWRKVBJQmrG7s0dLAXvNSqubjjJ1sA4WaGLkZNDQsBE 4tXrDpYuRi4OIYEdjBJLexrYIJzvjBI3tjUywlQt27+PESKxm1Hi9aeJ7CAJXgFBiR+T7wG1 c3AwC8hLHLmUDRJmFtCUePFlEtTUe4wSKxeuYoGo15I49PoQG4jNIqAq0TL/G9gcNqD4/hc3 wOL8AooSV388BlssKhAhsROqRkSgQKLxxzY2iAUKEr/ubWIFsYUFsiVefd3ABGJzCthJvP6w lB3i6A1sEruv+EHYLhIzT/9ng7CFJV4d3wJVIy3xbNVGsMckBNoZJdr3zmOGcKYwSpy7fo8J ospY4tnCLiaIzXwSHYf/soN8LCHAK9HRJgRR4iEx+f8+qHJHia7fJ8AeBoYps0TnV58JjHKz kMJrFiK8ZiGF1wJG5lWMYqkFxbnpqcVGBcZ6xYm5xaV56XrJ+bmbGMGJTMt9B+O2cz6HGAU4 GJV4eAO+x0YJsSaWFVfmHmKU4GBWEuFtDI6JEuJNSaysSi3Kjy8qzUktPsRoCgzuicxSosn5 wCSbVxJvaGJpYGJmZmRuZgFMYOK8rQEuUUIC6YklqdmpqQWpRTB9TBycUg2MW+P3ir5NVlh1 LESpNiGH69Ca8gPzGc8f/WR2ktf6ibiPzY9l8SnPZ4l7NB5Yv1/u4amN6f2dDGbc7kr/XsVc nX+/7eGc/+0vXL6+rvK4UvVh68e+xgX1EpVme+9Vt6eHvfDUOeUkWyA+dz3n5adXnj3RnPzs YMsH04seL6U2SO1aezjqU2HHEiWW4oxEQy3mouJEAJzzMdV6AwAA X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrGLMWRmVeSWpSXmKPExsVy+t9jAV3FhXFRBh8naFi8OT6dyeLyrjls FrcbV7BZ/Dx0nsmBxWPDo9WsHu/3XWXz6NuyitHj8ya5AJYoLpuU1JzMstQifbsErozdmztY Ct5rVFzdcJKtgXGyQhcjJ4eEgInEsv37GEFsIYGdjBJHWv1BbF4BQYkfk++xdDFycDALyEsc uZQNYapLTJmS28XIBVT9gFFi9ZS3bBDlWhKHXh8Cs1kEVCVa5n9jB7HZgOL7X9wAi/MLKEpc /fGYEWSOqECERPeJShBTRKBAou9bJUgFs4CCxK97m1hBbGGBbIn2hQ9ZIFbtYJa4/W0dE0iC U8BO4vWHpewTGAVmITl0FsKhsxAOXcDIvIpRMrWgODc9t9iowCgvtVyvODG3uDQvXS85P3cT IzB0tx3W6t/B+HhJ/CFGAQ5GJR7eFT9jo4RYE8uKK3MPMUpwMCuJ8DYGx0QJ8aYkVlalFuXH F5XmpBYfYpTmYFES5+XPPxYpJJCeWJKanZpakFoEk2Xi4JRqYNy3MiFg+hurxfefVM699qTt 1bUm2Zin4Tw7FjgXVNXNDb36i/v42+dl7CWWe04wztUv5J9/u0Ix7PTEJbL/v5VN4rp4d1nz yolMHd2HG/aFywrpv1zRlBl20uXM5olzvZeu+uqjeLFt7+sfpvu1dkx2WxnQIOOwtOPoefNF B/y+uywom51/3XyDEktxRqKhFnNRcSIAfCK/OVkCAAA= X-CMS-MailID: 20180117010433epcas2p2051dcfe2f5371166611abd3da7ba2ab0 X-Msg-Generator: CA CMS-TYPE: 102P DLP-Filter: Pass X-CFilter-Loop: Reflected X-CMS-RootMailID: 20180114151027epcas3p4d76fcdd44ec7d8adeaa187abd5e5a825 X-RootMTR: 20180114151027epcas3p4d76fcdd44ec7d8adeaa187abd5e5a825 References: <20180114151021.11432-1-hdegoede@redhat.com> <20180114151021.11432-2-hdegoede@redhat.com> <5A5C3A85.9080109@samsung.com> <996966c8-6a31-eb48-579e-f8ef119a15d5@redhat.com> <5A5C6F7C.4090200@samsung.com> <4176b3af-01f8-9a5e-f6c8-4c446468918f@redhat.com> <5A5D3CA5.7040801@samsung.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: On 2018년 01월 16일 18:33, Hans de Goede wrote: > Hi, > > On 16-01-18 00:43, Chanwoo Choi wrote: >> On 2018년 01월 15일 20:32, Hans de Goede wrote: >>> HI, >>> >>> On 15-01-18 10:08, Chanwoo Choi wrote: >>>> On 2018년 01월 15일 17:36, Hans de Goede wrote: >>>>> Hi, >>>>> >>>>> On 15-01-18 06:22, Chanwoo Choi wrote: >>>>>> On 2018년 01월 15일 00:10, Hans de Goede wrote: >>>>>>> The only misdetection which can happen at boot due to data-lines mux issues >>>>>>> is detecting a non SDP as SDP, so we only need to retry if we detect a SDP >>>>>>> on our first detection. >>>>>>> >>>>>>> Note Vbus misdetection is not a problem, as soon as the drivers controlling >>>>>>> the Vbus path set it correctly we will get an interrupt which reschedules >>>>>>> the charger-detection. >>>>>>> >>>>>>> Also update the comment about the re-detection to reflect this. >>>>>>> >>>>>>> Signed-off-by: Hans de Goede >>>>>>> --- >>>>>>> drivers/extcon/extcon-axp288.c | 14 +++++++------- >>>>>>> 1 file changed, 7 insertions(+), 7 deletions(-) >>>>>>> >>>>>>> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c >>>>>>> index 63b99d5becd7..17e6808af0d1 100644 >>>>>>> --- a/drivers/extcon/extcon-axp288.c >>>>>>> +++ b/drivers/extcon/extcon-axp288.c >>>>>>> @@ -161,16 +161,16 @@ 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. >>>>>>> + * not set these things up correctly resulting in a wrong result for the >>>>>>> + * initial charger type detection and we end up 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 our first detect detects an SDP charger-type, we try again 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)); >>>>>>> + if (info->previous_cable == EXTCON_CHG_USB_SDP) >>>>>>> + queue_delayed_work(system_wq, &info->det_work, >>>>>>> + msecs_to_jiffies(2000)); >>>>>>> info->first_detect_done = true; >>>>>>> } >>>>>>> >>>>>> >>>>>> I understand why you add the second delayed_work because of dependency >>>>>> of other consumer driver. But, this patch is not proper method. It looks >>>>>> like the workaround. >>>>>> >>>>>> We need to consider the fundamental solution such as using OF graph >>>>>> or sending the pending notification when consumer driver is probed. >>>>> >>>>> I agree that having some sort of proper probe ordering here would be >>>>> better. But on these ACPI systems that is going to be quite tricky todo, >>>>> since we've no control over the firmware there. >>>>> >>>>> Note that you've already merged the workaround, this patch merely changes >>>>> the workaround to avoid it in cases where it is not necessary, so I would >>>>> really like to see this get merged. >>>> >>>> I merged your patch because I knew this issue related to dependency. >>>> >>>> But, I don't want to merge this patch until developing the fundamental >>>> method. All extcon provider driver have the same issue. I'll try to >>>> resolve this issue thro extcon framework. >>> >>> I really don't see how holding this very simple patch hostage is going to >>> help (or deter) finding a better solution for this. >>> >>> In the mean time my original patch seems to cause mis-detection of CDP ports >>> as SDP ports which this fixes. >> >> I disagree this patch for only one specific connector. Instead of adding second >> delayed_work for detection, you better to extend the time from 2 sec to 4 sec >> on first time detection. > > You are misreading the patch, it is changing the code to only do the first > detection, unless the result of the first detection is SDP. So I'm not adding > a second retry I'm not retrying at all unless really necessary. Because I don't prefer to add the second detection for only specific connector as I commented, I suggested the extending the delay time of first detection. You mentioned on following comment. If extcon provider driver detects the kind of connector after the enough delay time, you don't need to add the second detection. You can detect the correct type of connector with only first detection. >>>>>>> + * If our first detect detects an SDP charger-type, we try again after >>>>>>> + * 2 seconds to give the other drivers time to load and do their thing. -- Best Regards, Chanwoo Choi Samsung Electronics