From mboxrd@z Thu Jan 1 00:00:00 1970 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750825AbeAOXni (ORCPT + 1 other); Mon, 15 Jan 2018 18:43:38 -0500 Received: from mailout1.samsung.com ([203.254.224.24]:16443 "EHLO mailout1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750740AbeAOXng (ORCPT ); Mon, 15 Jan 2018 18:43:36 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 mailout1.samsung.com 20180115234334epoutp01906bc0585a18c9bb59e4d548693b677f~KIF50pJ-u0573805738epoutp01w X-AuditID: b6c32a35-c51ff700000010dd-10-5a5d3ca62ed8 MIME-version: 1.0 Content-transfer-encoding: 8BIT Content-type: text/plain; charset="utf-8" Message-id: <5A5D3CA5.7040801@samsung.com> Date: Tue, 16 Jan 2018 08:43: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: <4176b3af-01f8-9a5e-f6c8-4c446468918f@redhat.com> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrGKsWRmVeSWpSXmKPExsWy7bCmru4ym9gogzdtnBZvjk9nsri8aw6b xe3GFWwWPw+dZ3Jg8djwaDWrx/t9V9k8+rasYvT4vEkugCUq1SYjNTEltUghNS85PyUzL91W yTs43jne1MzAUNfQ0sJcSSEvMTfVVsnFJ0DXLTMHaKWSQlliTilQKCCxuFhJ386mKL+0JFUh I7+4xFYp2tDQSM/QwFzPyMhIz8Q41srIFKgkITXj+sWigjUKFYs3vmJrYJwn2cXIySEhYCLx es1jxi5GLg4hgR2MEv0NS1ghnO+MEhsvXGSCqTo47yAbRGI3o8SpI1dYQBK8AoISPybfA7I5 OJgF5CWOXMoGCTMLaEq8+DKJBaL+HqPEp78bGSHqtSQ2tUHUswioSnTO8QIJswGF97+4wQZi 8wsoSlz98RisXFQgQmLn/G/sILaIQIFE449tbBDzFSR+3dvECmILC2RLvPq6AexOTgE7iQc/ ZoHtlRBYwSYx9e0HRogHXCRO/TjMAmELS7w6voUd5AYJAWmJS0dtIerbGSXa985jhnCmMEqc u34P6ntjiWcLu5ggNvNJvPvawwrRzCvR0SYEUeIhMfn/PqhyR4mu3yegnn/MJLH29ifGCYxy s5DCaxYivGYhhdcCRuZVjGKpBcW56anFhgWGesWJucWleel6yfm5mxjBaUzLdAfjlHM+hxgF OBiVeHgttsVECbEmlhVX5h5ilOBgVhLhbQwGCvGmJFZWpRblxxeV5qQWH2I0BQb3RGYp0eR8 YIrNK4k3NLE0MDEzAqYuS0NDJXHegACXKCGB9MSS1OzU1ILUIpg+Jg5OqQbGxNmaqu+qWqNS fjAlCJ5M0N7yrld35qNf28rnsBq+k7f007nir6e9qKp+yaWkmu3ZXsfKrhvbcTu8MjffoGC8 Iu/e7reMqtuW9rW4/8j2T+pb3ygut1HtcECKRe5aj5fnp5deMUn79f3hg2uKNq1sFWK75F7u TUzuUUrovvbNSDS84raBC5cSS3FGoqEWc1FxIgA1CxsUeQMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrCLMWRmVeSWpSXmKPExsVy+t9jAd2lNrFRBv2dehZvjk9nsri8aw6b xe3GFWwWPw+dZ3Jg8djwaDWrx/t9V9k8+rasYvT4vEkugCWKyyYlNSezLLVI3y6BK+P6xaKC NQoVize+YmtgnCfZxcjJISFgInFw3kE2EFtIYCejxK6dViA2r4CgxI/J91i6GDk4mAXkJY5c yoYw1SWmTMntYuQCqn4AVD1pERtEuZbEpjaIchYBVYnOOV4gYTag8P4XN8BK+AUUJa7+eMwI UiIqECHRfaISxBQRKJDo+1YJUsEsoCDx694mVhBbWCBbon3hQxaITY+ZJO6s2s8EkuAUsJN4 8GMWywRGgVlI7pyFcOcshDsXMDKvYpRMLSjOTc8tNiowzEst1ytOzC0uzUvXS87P3cQIDNtt h7X6djDeXxJ/iFGAg1GJh3fCjpgoIdbEsuLK3EOMEhzMSiK8jcFAId6UxMqq1KL8+KLSnNTi Q4zSHCxK4ry3845FCgmkJ5akZqemFqQWwWSZODilGhiV4pI4DlSdry6cxtCQkmN9PVn0yHWh C4YZ01jtdnxmWfw3bttSEa9j8hParq9JuXNkz5lnZe53LFPyNje3bpdctWXu1aPSMstmhL4U Zpi74uunBed7vy6dJ5Mdox3/N3K9boGN2qLGYxPqVt07pHW43/VR1ytTnr4PBrNCaoOKMljP 7uj0jj6hxFKckWioxVxUnAgADvBLUlcCAAA= X-CMS-MailID: 20180115234333epcas1p3ab6d4d145d6a19d09a12ead669df8bbe X-Msg-Generator: CA CMS-TYPE: 101P 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> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Return-Path: 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. -- Best Regards, Chanwoo Choi Samsung Electronics