MIME-version: 1.0
Content-transfer-encoding: 8BIT
Content-type: text/plain; charset="UTF-8"
Message-id: <58E2078A.9070103@samsung.com>
Date: Mon, 03 Apr 2017 17:27:54 +0900
From: Chanwoo Choi <cw00.choi@samsung.com>
Organization: Samsung Electronics
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101
        Thunderbird/31.6.0
To: Andi Shyti <andi.shyti@gmail.com>,
        MyungJoo Ham <myungjoo.ham@samsung.com>,
        Jaewon Kim <jaewon02.kim@samsung.com>
Cc: Seung-Woo Kim <sw0312.kim@samsung.com>,
        Krzysztof Kozlowski <krzk@kernel.org>, linux-kernel@vger.kernel.org,
        Andi Shyti <andi.shyti@samsung.com>, Andi Shyti <andi@etezian.org>
Subject: Re: [PATCH v2 2/2] extcon: max77843: support USB accessories as
 external USB hosts
In-reply-to: <20170402053542.29570-3-andi@etezian.org>
CMS-TYPE: 105P
DLP-Filter: Pass
References: <20170402053542.29570-1-andi@etezian.org>
        <CGME20170402053626epcas4p28872c873de3f935bc174ff74d0069e76@epcas4p2.samsung.com>
        <20170402053542.29570-3-andi@etezian.org>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 5720
Lines: 141

Hi,

On 2017년 04월 02일 14:35, Andi Shyti wrote:
> From: Andi Shyti <andi.shyti@samsung.com>
> 
> The ADC state defines the resistance that a USB device has in
> order to distinguish between devices.
> 
> The external accessories (like the Gear VR) are defined as:
> 
> MAX77843_MUIC_ADC_RESERVED_ACC_1
> MAX77843_MUIC_ADC_RESERVED_ACC_2
> MAX77843_MUIC_ADC_RESERVED_ACC_3
> MAX77843_MUIC_ADC_RESERVED_ACC_4
> MAX77843_MUIC_ADC_RESERVED_ACC_5
> 
> and they should be set as such in the extcon driver.
> 
> Do not ignore interrupts generated by external accessories by
> handling cables of the type from any of the above.
> 
> Do not handle any charging related action. This can be
> misleading as also external devices might have some power and in
> such cases the MUIC might consider them as power providers. In
> this case check the value of the adc and if it corresponds to
> one or the MAX77843_MUIC_ADC_RESERVED_ACC_*, return a no-action
> value (MAX77843_MUIC_CHG_NONE).
> 
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
>  drivers/extcon/extcon-max77843.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-max77843.c b/drivers/extcon/extcon-max77843.c
> index fcdabc4b4025..154d65afd45a 100644
> --- a/drivers/extcon/extcon-max77843.c
> +++ b/drivers/extcon/extcon-max77843.c
> @@ -271,6 +271,20 @@ static int max77843_muic_get_cable_type(struct max77843_muic_info *info,
>  		} else {
>  			*attached = true;
>  			switch (adc) {
> +			/*
> +			 * the MAX77843_MUIC_ADC_RESERVED_ACC_* type are
> +			 * external eccessories and as such they don't

s/eccessories/accessories

> +			 * need any charging action. In this case just
> +			 * return MAX77843_MUIC_CHG_NONE so that charging type
> +			 * interrupts are ignored.
> +			 */
> +			case MAX77843_MUIC_ADC_RESERVED_ACC_1:
> +			case MAX77843_MUIC_ADC_RESERVED_ACC_2:
> +			case MAX77843_MUIC_ADC_RESERVED_ACC_3:
> +			case MAX77843_MUIC_ADC_RESERVED_ACC_4:
> +			case MAX77843_MUIC_ADC_RESERVED_ACC_5:
> +				info->prev_chg_type = MAX77843_MUIC_CHG_NONE;
> +				break;
>  			case MAX77843_MUIC_ADC_GROUND:
>  				info->prev_chg_type = MAX77843_MUIC_CHG_GND;
>  				break;
> @@ -403,6 +417,11 @@ static int max77843_muic_adc_handler(struct max77843_muic_info *info)
>  
>  	switch (cable_type) {
>  	case MAX77843_MUIC_ADC_GROUND:
> +	case MAX77843_MUIC_ADC_RESERVED_ACC_1:
> +	case MAX77843_MUIC_ADC_RESERVED_ACC_2:
> +	case MAX77843_MUIC_ADC_RESERVED_ACC_3:
> +	case MAX77843_MUIC_ADC_RESERVED_ACC_4:
> +	case MAX77843_MUIC_ADC_RESERVED_ACC_5:
>  		ret = max77843_muic_adc_gnd_handler(info);
>  		if (ret < 0)
>  			return ret;
> @@ -427,11 +446,6 @@ static int max77843_muic_adc_handler(struct max77843_muic_info *info)
>  	case MAX77843_MUIC_ADC_REMOTE_S10_BUTTON:
>  	case MAX77843_MUIC_ADC_REMOTE_S11_BUTTON:
>  	case MAX77843_MUIC_ADC_REMOTE_S12_BUTTON:
> -	case MAX77843_MUIC_ADC_RESERVED_ACC_1:
> -	case MAX77843_MUIC_ADC_RESERVED_ACC_2:
> -	case MAX77843_MUIC_ADC_RESERVED_ACC_3:
> -	case MAX77843_MUIC_ADC_RESERVED_ACC_4:
> -	case MAX77843_MUIC_ADC_RESERVED_ACC_5:
>  	case MAX77843_MUIC_ADC_AUDIO_DEVICE_TYPE2:
>  	case MAX77843_MUIC_ADC_PHONE_POWERED_DEV:
>  	case MAX77843_MUIC_ADC_TTY_CONVERTER:
> 

The extcon-max77843.c driver was implemented with real use-case
by testing the various kind of external connectors.

I want to apply the part for real use-case of this patch.
As I knew, you tested this patch with only MAX77843_MUIC_ADC_RESERVED_ACC_4.
So, I prefer to apply following patch only for MAX77843_MUIC_ADC_RESERVED_ACC_4.

--
 drivers/extcon/extcon-max77843.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/extcon/extcon-max77843.c b/drivers/extcon/extcon-max77843.c
index fcdabc4b4025..45367d74bc01 100644
--- a/drivers/extcon/extcon-max77843.c
+++ b/drivers/extcon/extcon-max77843.c
@@ -271,6 +271,16 @@ static int max77843_muic_get_cable_type(struct max77843_muic_info *info,
                } else {
                        *attached = true;
                        switch (adc) {
+                       case MAX77843_MUIC_ADC_RESERVED_ACC_4:
+                               /*
+                                * The MAX77843_MUIC_ADC_RESERVED_ACC_* type are
+                                * external accessories and as such they don't
+                                * need any charging action. In this case just
+                                * return MAX77843_MUIC_CHG_NONE so that
+                                * charging type interrupts are ignored.
+                                */
+                               info->prev_chg_type = MAX77843_MUIC_CHG_NONE;
+                               break;
                        case MAX77843_MUIC_ADC_GROUND:
                                info->prev_chg_type = MAX77843_MUIC_CHG_GND;
                                break;
@@ -403,6 +413,7 @@ static int max77843_muic_adc_handler(struct max77843_muic_info *info)

        switch (cable_type) {
        case MAX77843_MUIC_ADC_GROUND:
+       case MAX77843_MUIC_ADC_RESERVED_ACC_4:
                ret = max77843_muic_adc_gnd_handler(info);
                if (ret < 0)
                        return ret;
@@ -430,7 +441,6 @@ static int max77843_muic_adc_handler(struct max77843_muic_info *info)
        case MAX77843_MUIC_ADC_RESERVED_ACC_1:
        case MAX77843_MUIC_ADC_RESERVED_ACC_2:
        case MAX77843_MUIC_ADC_RESERVED_ACC_3:
-       case MAX77843_MUIC_ADC_RESERVED_ACC_4:
        case MAX77843_MUIC_ADC_RESERVED_ACC_5:
        case MAX77843_MUIC_ADC_AUDIO_DEVICE_TYPE2:
        case MAX77843_MUIC_ADC_PHONE_POWERED_DEV:


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics