lkml.org 
[lkml]   [2018]   [Jan]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] extcon: axp288: Only reschedule charger-detection at boot when a SDP is detected
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 <hdegoede@redhat.com>
>>>>> ---
>>>>> 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

\
 
 \ /
  Last update: 2018-01-16 00:44    [W:0.062 / U:0.784 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site