* [PATCH 0/2] Add support for USB accessories in the max77843 extcon driver
[not found] <CGME20170329115509epcas5p1414c3cf20f3c8dd08e55c572540bd034@epcas5p1.samsung.com>
@ 2017-03-29 11:54 ` Andi Shyti
[not found] ` <CGME20170329115509epcas5p14fddb50b71177529674457026a743c61@epcas5p1.samsung.com>
[not found] ` <CGME20170329115509epcas5p16e5dd395695d94a294a91da6f113e5fe@epcas5p1.samsung.com>
0 siblings, 2 replies; 5+ messages in thread
From: Andi Shyti @ 2017-03-29 11:54 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Jaewon Kim
Cc: Seung-Woo Kim, Krzysztof Kozlowski, linux-kernel, Andi Shyti, Andi Shyti
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
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 | 50 ++++++++++++++++------------------------
1 file changed, 20 insertions(+), 30 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] extcon: max77843: improve the code and minimize duplicated lines
[not found] ` <CGME20170329115509epcas5p14fddb50b71177529674457026a743c61@epcas5p1.samsung.com>
@ 2017-03-29 11:54 ` Andi Shyti
0 siblings, 0 replies; 5+ messages in thread
From: Andi Shyti @ 2017-03-29 11:54 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Jaewon Kim
Cc: Seung-Woo Kim, Krzysztof Kozlowski, linux-kernel, Andi Shyti, Andi Shyti
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] 5+ messages in thread
* [PATCH 2/2] extcon: max77843: support USB accessories as external USB hosts
[not found] ` <CGME20170329115509epcas5p16e5dd395695d94a294a91da6f113e5fe@epcas5p1.samsung.com>
@ 2017-03-29 11:54 ` Andi Shyti
2017-03-29 14:27 ` Krzysztof Kozlowski
0 siblings, 1 reply; 5+ messages in thread
From: Andi Shyti @ 2017-03-29 11:54 UTC (permalink / raw)
To: MyungJoo Ham, Chanwoo Choi, Jaewon Kim
Cc: Seung-Woo Kim, Krzysztof Kozlowski, linux-kernel, Andi Shyti, Andi Shyti
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 | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/drivers/extcon/extcon-max77843.c b/drivers/extcon/extcon-max77843.c
index fcdabc4b4025..00795fe9ab60 100644
--- a/drivers/extcon/extcon-max77843.c
+++ b/drivers/extcon/extcon-max77843.c
@@ -271,6 +271,13 @@ static int max77843_muic_get_cable_type(struct max77843_muic_info *info,
} else {
*attached = true;
switch (adc) {
+ 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 +410,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 +439,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] 5+ messages in thread
* Re: [PATCH 2/2] extcon: max77843: support USB accessories as external USB hosts
2017-03-29 11:54 ` [PATCH 2/2] extcon: max77843: support USB accessories as external USB hosts Andi Shyti
@ 2017-03-29 14:27 ` Krzysztof Kozlowski
2017-03-29 14:50 ` Andi Shyti
0 siblings, 1 reply; 5+ messages in thread
From: Krzysztof Kozlowski @ 2017-03-29 14:27 UTC (permalink / raw)
To: Andi Shyti
Cc: MyungJoo Ham, Chanwoo Choi, Jaewon Kim, Seung-Woo Kim,
linux-kernel, Andi Shyti
On Wed, Mar 29, 2017 at 2:54 PM, Andi Shyti <andi.shyti@samsung.com> wrote:
> 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 | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/extcon/extcon-max77843.c b/drivers/extcon/extcon-max77843.c
> index fcdabc4b4025..00795fe9ab60 100644
> --- a/drivers/extcon/extcon-max77843.c
> +++ b/drivers/extcon/extcon-max77843.c
> @@ -271,6 +271,13 @@ static int max77843_muic_get_cable_type(struct max77843_muic_info *info,
> } else {
> *attached = true;
> switch (adc) {
> + 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:
In commit msg you mentioned it but in code not. It looks weird to
handle a vague "reserved" type in a specific way. How about adding
some meaningful names instead of "reserved"? Or at least documenting,
that they match Gear VR. Really, imagine later one would be looking at
the code and trying to figure out why "reserved" means something
Best regards,
Krzysztof
> + 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 +410,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 +439,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 [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] extcon: max77843: support USB accessories as external USB hosts
2017-03-29 14:27 ` Krzysztof Kozlowski
@ 2017-03-29 14:50 ` Andi Shyti
0 siblings, 0 replies; 5+ messages in thread
From: Andi Shyti @ 2017-03-29 14:50 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andi Shyti, MyungJoo Ham, Chanwoo Choi, Jaewon Kim,
Seung-Woo Kim, linux-kernel, Andi Shyti
Hi Krzysztof,
> > + 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:
>
> In commit msg you mentioned it but in code not. It looks weird to
> handle a vague "reserved" type in a specific way. How about adding
> some meaningful names instead of "reserved"? Or at least documenting,
> that they match Gear VR. Really, imagine later one would be looking at
> the code and trying to figure out why "reserved" means something
I know it's misleading, but that's the name from the datasheet,
that's why I tried to be more specific in the commit log.
In any case RESERVED_ACC stands for "reserved for external
accessory".
The Gear VR corresponds to MAX77843_MUIC_ADC_RESERVED_ACC_4, but
I don't think it's the only one, that's why it wouldn't be right
to call it e.g. MAX77843_MUIC_ADC_GEAR_VR.
I can of course add a comment on top of the above lines to make
it clearer.
Thanks,
Andi
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-03-29 14:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20170329115509epcas5p1414c3cf20f3c8dd08e55c572540bd034@epcas5p1.samsung.com>
2017-03-29 11:54 ` [PATCH 0/2] Add support for USB accessories in the max77843 extcon driver Andi Shyti
[not found] ` <CGME20170329115509epcas5p14fddb50b71177529674457026a743c61@epcas5p1.samsung.com>
2017-03-29 11:54 ` [PATCH 1/2] extcon: max77843: improve the code and minimize duplicated lines Andi Shyti
[not found] ` <CGME20170329115509epcas5p16e5dd395695d94a294a91da6f113e5fe@epcas5p1.samsung.com>
2017-03-29 11:54 ` [PATCH 2/2] extcon: max77843: support USB accessories as external USB hosts Andi Shyti
2017-03-29 14:27 ` Krzysztof Kozlowski
2017-03-29 14:50 ` Andi Shyti
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.