All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Add support for USB accessories in the max77843 extcon
@ 2017-04-02  5:35 Andi Shyti
  2017-04-02  5:35 ` [PATCH v2 1/2] extcon: max77843: improve the code and minimize duplicated lines Andi Shyti
  2017-04-02  5:35 ` [PATCH v2 2/2] extcon: max77843: support USB accessories as external USB hosts Andi Shyti
  0 siblings, 2 replies; 7+ messages in thread
From: Andi Shyti @ 2017-04-02  5:35 UTC (permalink / raw)
  To: MyungJoo Ham, Chanwoo Choi, Jaewon Kim
  Cc: Seung-Woo Kim, Krzysztof Kozlowski, linux-kernel, Andi Shyti, Andi Shyti

From: Andi Shyti <andi@smida.it>

Hi,

this patchset add handling of external USB accessories that are
connected to the extcon max77843 device.

The first patch is preparatory, that mainly aims to improve the
readability of the second patch. It makes some code refactoring
reducing the duplicated code, the result is exactly the same.

It has been tested on TM2 with mainline kernel 4.11-rc4.

Andi

Changelog v1 -> v2
------------------
Added a comment to explain better the meaning of the
MAX77843_MUIC_ADC_RESERVED_ACC_* devices as recommended by
Krzysztof.

Andi Shyti (2):
  extcon: max77843: improve the code and minimize duplicated lines
  extcon: max77843: support USB accessories as external USB hosts

 drivers/extcon/extcon-max77843.c | 57 +++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 30 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 1/2] extcon: max77843: improve the code and minimize duplicated lines
  2017-04-02  5:35 [PATCH v2 0/2] Add support for USB accessories in the max77843 extcon Andi Shyti
@ 2017-04-02  5:35 ` Andi Shyti
  2017-04-03  7:53   ` Chanwoo Choi
  2017-04-02  5:35 ` [PATCH v2 2/2] extcon: max77843: support USB accessories as external USB hosts Andi Shyti
  1 sibling, 1 reply; 7+ messages in thread
From: Andi Shyti @ 2017-04-02  5:35 UTC (permalink / raw)
  To: MyungJoo Ham, Chanwoo Choi, Jaewon Kim
  Cc: Seung-Woo Kim, Krzysztof Kozlowski, linux-kernel, Andi Shyti, Andi Shyti

From: Andi Shyti <andi.shyti@samsung.com>

Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/extcon/extcon-max77843.c | 33 ++++++++-------------------------
 1 file changed, 8 insertions(+), 25 deletions(-)

diff --git a/drivers/extcon/extcon-max77843.c b/drivers/extcon/extcon-max77843.c
index 6e722d552cf1..fcdabc4b4025 100644
--- a/drivers/extcon/extcon-max77843.c
+++ b/drivers/extcon/extcon-max77843.c
@@ -264,37 +264,20 @@ static int max77843_muic_get_cable_type(struct max77843_muic_info *info,
 		chg_type = info->status[MAX77843_MUIC_STATUS2] &
 				MAX77843_MUIC_STATUS2_CHGTYP_MASK;
 
-		/* Check GROUND accessory with charger cable */
-		if (adc == MAX77843_MUIC_ADC_GROUND) {
-			if (chg_type == MAX77843_MUIC_CHG_NONE) {
-				/*
-				 * The following state when charger cable is
-				 * disconnected but the GROUND accessory still
-				 * connected.
-				 */
-				*attached = false;
-				cable_type = info->prev_chg_type;
-				info->prev_chg_type = MAX77843_MUIC_CHG_NONE;
-			} else {
-
-				/*
-				 * The following state when charger cable is
-				 * connected on the GROUND accessory.
-				 */
-				*attached = true;
-				cable_type = MAX77843_MUIC_CHG_GND;
-				info->prev_chg_type = MAX77843_MUIC_CHG_GND;
-			}
-			break;
-		}
-
 		if (chg_type == MAX77843_MUIC_CHG_NONE) {
 			*attached = false;
 			cable_type = info->prev_chg_type;
 			info->prev_chg_type = MAX77843_MUIC_CHG_NONE;
 		} else {
 			*attached = true;
-			cable_type = info->prev_chg_type = chg_type;
+			switch (adc) {
+			case MAX77843_MUIC_ADC_GROUND:
+				info->prev_chg_type = MAX77843_MUIC_CHG_GND;
+				break;
+			default:
+				info->prev_chg_type = chg_type;
+			}
+			cable_type = info->prev_chg_type;
 		}
 		break;
 	case MAX77843_CABLE_GROUP_ADC_GND:
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/2] extcon: max77843: support USB accessories as external USB hosts
  2017-04-02  5:35 [PATCH v2 0/2] Add support for USB accessories in the max77843 extcon Andi Shyti
  2017-04-02  5:35 ` [PATCH v2 1/2] extcon: max77843: improve the code and minimize duplicated lines Andi Shyti
@ 2017-04-02  5:35 ` Andi Shyti
  2017-04-03  8:27   ` Chanwoo Choi
  1 sibling, 1 reply; 7+ messages in thread
From: Andi Shyti @ 2017-04-02  5:35 UTC (permalink / raw)
  To: MyungJoo Ham, Chanwoo Choi, Jaewon Kim
  Cc: Seung-Woo Kim, Krzysztof Kozlowski, linux-kernel, Andi Shyti, Andi Shyti

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
+			 * 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:
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/2] extcon: max77843: improve the code and minimize duplicated lines
  2017-04-02  5:35 ` [PATCH v2 1/2] extcon: max77843: improve the code and minimize duplicated lines Andi Shyti
@ 2017-04-03  7:53   ` Chanwoo Choi
  0 siblings, 0 replies; 7+ messages in thread
From: Chanwoo Choi @ 2017-04-03  7:53 UTC (permalink / raw)
  To: Andi Shyti, MyungJoo Ham, Jaewon Kim
  Cc: Seung-Woo Kim, Krzysztof Kozlowski, linux-kernel, Andi Shyti, Andi Shyti

Hi,

Actually, I checked this patch on following four case.
It is same operation between before and after.
-------------------------------------------------------------
       | adc                       | chg_type               |
-------------------------------------------------------------
case 1 | MAX77843_MUIC_ADC_GROUND  | MAX77843_MUIC_CHG_NONE |
case 2 | MAX77843_MUIC_ADC_GROUND  |                        |
case 3 |                           | MAX77843_MUIC_CHG_NONE |
case 4 |                           |                        |
-------------------------------------------------------------


But, I don't want to apply this patch.

By applying this patch, I think there is no huge benefits
and certainly I prefer to remain the all descriptions for specific use-case.
(- This patch deletes the 13 lines and add new 8 lines except for the comment.)

On 2017년 04월 02일 14:35, Andi Shyti wrote:
> From: Andi Shyti <andi.shyti@samsung.com>
> 
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> ---
>  drivers/extcon/extcon-max77843.c | 33 ++++++++-------------------------
>  1 file changed, 8 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-max77843.c b/drivers/extcon/extcon-max77843.c
> index 6e722d552cf1..fcdabc4b4025 100644
> --- a/drivers/extcon/extcon-max77843.c
> +++ b/drivers/extcon/extcon-max77843.c
> @@ -264,37 +264,20 @@ static int max77843_muic_get_cable_type(struct max77843_muic_info *info,
>  		chg_type = info->status[MAX77843_MUIC_STATUS2] &
>  				MAX77843_MUIC_STATUS2_CHGTYP_MASK;
>  
> -		/* Check GROUND accessory with charger cable */
> -		if (adc == MAX77843_MUIC_ADC_GROUND) {
> -			if (chg_type == MAX77843_MUIC_CHG_NONE) {
> -				/*
> -				 * The following state when charger cable is
> -				 * disconnected but the GROUND accessory still
> -				 * connected.
> -				 */
> -				*attached = false;
> -				cable_type = info->prev_chg_type;
> -				info->prev_chg_type = MAX77843_MUIC_CHG_NONE;
> -			} else {
> -
> -				/*
> -				 * The following state when charger cable is
> -				 * connected on the GROUND accessory.
> -				 */
> -				*attached = true;
> -				cable_type = MAX77843_MUIC_CHG_GND;
> -				info->prev_chg_type = MAX77843_MUIC_CHG_GND;
> -			}
> -			break;
> -		}
> -
>  		if (chg_type == MAX77843_MUIC_CHG_NONE) {
>  			*attached = false;
>  			cable_type = info->prev_chg_type;
>  			info->prev_chg_type = MAX77843_MUIC_CHG_NONE;
>  		} else {
>  			*attached = true;
> -			cable_type = info->prev_chg_type = chg_type;
> +			switch (adc) {
> +			case MAX77843_MUIC_ADC_GROUND:
> +				info->prev_chg_type = MAX77843_MUIC_CHG_GND;
> +				break;
> +			default:
> +				info->prev_chg_type = chg_type;
> +			}
> +			cable_type = info->prev_chg_type;
>  		}
>  		break;
>  	case MAX77843_CABLE_GROUP_ADC_GND:
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] extcon: max77843: support USB accessories as external USB hosts
  2017-04-02  5:35 ` [PATCH v2 2/2] extcon: max77843: support USB accessories as external USB hosts Andi Shyti
@ 2017-04-03  8:27   ` Chanwoo Choi
  2017-04-03  9:41     ` Andi Shyti
  0 siblings, 1 reply; 7+ messages in thread
From: Chanwoo Choi @ 2017-04-03  8:27 UTC (permalink / raw)
  To: Andi Shyti, MyungJoo Ham, Jaewon Kim
  Cc: Seung-Woo Kim, Krzysztof Kozlowski, linux-kernel, Andi Shyti, Andi Shyti

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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] extcon: max77843: support USB accessories as external USB hosts
  2017-04-03  8:27   ` Chanwoo Choi
@ 2017-04-03  9:41     ` Andi Shyti
  2017-04-03 10:08       ` Chanwoo Choi
  0 siblings, 1 reply; 7+ messages in thread
From: Andi Shyti @ 2017-04-03  9:41 UTC (permalink / raw)
  To: Chanwoo Choi
  Cc: MyungJoo Ham, Jaewon Kim, Seung-Woo Kim, Krzysztof Kozlowski,
	linux-kernel, Andi Shyti, Andi Shyti

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 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 2/2] extcon: max77843: support USB accessories as external USB hosts
  2017-04-03  9:41     ` Andi Shyti
@ 2017-04-03 10:08       ` Chanwoo Choi
  0 siblings, 0 replies; 7+ messages in thread
From: Chanwoo Choi @ 2017-04-03 10:08 UTC (permalink / raw)
  To: Andi Shyti
  Cc: MyungJoo Ham, Jaewon Kim, Seung-Woo Kim, Krzysztof Kozlowski,
	linux-kernel, Andi Shyti

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-04-03 10:09 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-02  5:35 [PATCH v2 0/2] Add support for USB accessories in the max77843 extcon Andi Shyti
2017-04-02  5:35 ` [PATCH v2 1/2] extcon: max77843: improve the code and minimize duplicated lines Andi Shyti
2017-04-03  7:53   ` Chanwoo Choi
2017-04-02  5:35 ` [PATCH v2 2/2] extcon: max77843: support USB accessories as external USB hosts Andi Shyti
2017-04-03  8:27   ` Chanwoo Choi
2017-04-03  9:41     ` Andi Shyti
2017-04-03 10:08       ` Chanwoo Choi

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.