Re: [PATCH v2 2/2] extcon: max77843: support USB accessories as external USB hosts

[Chanwoo Choi]

Hi,

On 2017ë 04ì 02ì 14:35, Andi Shyti wrote:
> From: Andi Shyti <andi.shyti@xxxxxxxxxxx>
> 
> 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@xxxxxxxxxxx>
> ---
>  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