MIME-version: 1.0
Content-type: text/plain; charset=UTF-8
Content-transfer-encoding: 8BIT
Message-id: <58E21F2F.4060001@samsung.com>
Date: Mon, 03 Apr 2017 19:08:47 +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@samsung.com>
Cc: MyungJoo Ham <myungjoo.ham@samsung.com>,
        Jaewon Kim <jaewon02.kim@samsung.com>,
        Seung-Woo Kim <sw0312.kim@samsung.com>,
        Krzysztof Kozlowski <krzk@kernel.org>, linux-kernel@vger.kernel.org,
        Andi Shyti <andi@etezian.org>
Subject: Re: [PATCH v2 2/2] extcon: max77843: support USB accessories as
 external USB hosts
In-reply-to: <20170403094135.26fz3ge67p265m37@gangnam.samsung>
CMS-TYPE: 101P
DLP-Filter: Pass
References: <20170402053542.29570-1-andi@etezian.org>
 <CGME20170402053626epcas4p28872c873de3f935bc174ff74d0069e76@epcas4p2.samsung.com>
 <20170402053542.29570-3-andi@etezian.org> <58E2078A.9070103@samsung.com>
 <20170403094135.26fz3ge67p265m37@gangnam.samsung>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4281
Lines: 104

On 2017년 04월 03일 18:41, Andi Shyti wrote:
> Hi Chanwoo,
> 
>>> @@ -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.
> 
> OK, I will do as you say, although I don't agree (as we
> discussed on review.tizen.org).

I'm not sure that other MAX77843_MUIC_ADC_RESERVED_ACC_1/2/3/5 are always
same with MAX77843_MUIC_ADC_RESERVED_ACC_4. Also, we don't have the 
any test case for MAX77843_MUIC_ADC_RESERVED_ACC_1/2/3/5.

> 
> I will drop patch 1 and change only 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:
> 
> This patch, of course, doesn't make sense because it's based on
> top of the previous that you asked me to drop. I will send the
> correct one.
> 
> Thanks,
> Andi
> 
> 
> .
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics