linux-kernel - Re: [PATCH 2/2] extcon: axp288: Only reschedule charger-detection at boot when a SDP is detected

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-id: <5A5C3A85.9080109@samsung.com>
Date:   Mon, 15 Jan 2018 14:22:13 +0900
From:   Chanwoo Choi <cw00.choi@...sung.com>
To:     Hans de Goede <hdegoede@...hat.com>,
        MyungJoo Ham <myungjoo.ham@...sung.com>,
        Chen-Yu Tsai <wens@...e.org>
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] extcon: axp288: Only reschedule charger-detection
 at boot when a SDP is detected

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@...hat.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 don't know what is best solution right now. I'll consider the appropriate
method for all extcon provider drivers.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics