* [PATCH v3] extcon: Modify the id and name of external connector
@ 2015-10-07 0:48 Chanwoo Choi
2015-10-07 7:33 ` Roger Quadros
0 siblings, 1 reply; 10+ messages in thread
From: Chanwoo Choi @ 2015-10-07 0:48 UTC (permalink / raw)
To: linux-kernel
Cc: k.kozlowski, ckeepax, gregkh, rogerq, ramakrishna.pallala,
patches, cw00.choi, myungjoo.ham
This patch modifies the id and name of external connector with the
additional prefix to clarify both attribute and meaning of external
connector as following:
- EXTCON_CHG_* mean the charger connector.
- EXTCON_JACK_* mean the jack connector.
- EXTCON_DISP_* mean the display port connector.
Following table show the new name of external connector with old name:
--------------------------------------------------
Old extcon name | New extcon name |
--------------------------------------------------
EXTCON_TA | EXTCON_CHG_USB_DCP |
EXTCON_CHARGE_DOWNSTREAM| EXTCON_CHG_USB_CDP |
EXTCON_FAST_CHARGER | EXTCON_CHG_USB_DCP_FAST|
EXTCON_SLOW_CHARGER | EXTCON_CHG_USB_DCP_SLOW|
--------------------------------------------------
EXTCON_MICROPHONE | EXTCON_JACK_MICROPHONE |
EXTCON_HEADPHONE | EXTCON_JACK_HEADPHONE |
EXTCON_LINE_IN | EXTCON_JACK_LINE_IN |
EXTCON_LINE_OUT | EXTCON_JACK_LINE_OUT |
EXTCON_VIDEO_IN | EXTCON_JACK_VIDEO_IN |
EXTCON_VIDEO_OUT | EXTCON_JACK_VIDEO_OUT |
EXTCON_SPDIF_IN | EXTCON_JACK_SPDIF_IN |
EXTCON_SPDIF_OUT | EXTCON_JACK_SPDIF_OUT |
--------------------------------------------------
EXTCON_HMDI | EXTCON_DISP_HDMI |
EXTCON_MHL | EXTCON_DISP_MHL |
EXTCON_DVI | EXTCON_DISP_DVI |
EXTCON_VGA | EXTCON_DISP_VGA |
--------------------------------------------------
And, when altering the name of USB charger connector, EXTCON refers to the
"Battery Charging v1.2 Spec and Adopters Agreement"[1] to use the standard
name of USB charging port as following. Following name of USB charging port
are already used in power_supply subsystem. We chan check it on patch[2].
- EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
- EXTCON_CHG_USB_CDP /* Charging Downstream Port */
- EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */
[1] www.usb.org/developers/docs/devclass_docs/BCv1.2_070312.zip
[2] commit 85efc8a18ced ("power_supply: Add types for USB chargers")
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
[ckeepax: For the Arizona changes]
Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
Changes from v2:
(https://lkml.org/lkml/2015/10/6/239)
- Remove the EXTCON_CHG_USB type to remove the possible confusion according to
Roger's comment and drop patch2 about EXTCON_CHG_USB.
- Fix the warning issue provided by scripts/checkpatch.pl
Changes from v1:
(https://lkml.org/lkml/2015/10/3/304)
- Add acked tag by Charles Keepax for arizona changes
- Modify the name of USB charger connector as following:
: EXTCON_CHG_USB_FAST -> EXTCON_CHG_USB_DCP_FAST
: EXTCON_CHG_USB_SLOW -> EXTCON_CHG_USB_DCP_SLOW
- Add the missing EXTCON_CHG_USB_ACA charger connector
- Add one more patch to support the EXTCON_CHG_USB when SDP port is connected
or not
drivers/extcon/extcon-arizona.c | 18 ++++++------
drivers/extcon/extcon-axp288.c | 12 ++++----
drivers/extcon/extcon-max14577.c | 17 +++++------
drivers/extcon/extcon-max77693.c | 32 +++++++++++----------
drivers/extcon/extcon-max77843.c | 27 ++++++++++--------
drivers/extcon/extcon-max8997.c | 21 +++++++-------
drivers/extcon/extcon-rt8973a.c | 4 +--
drivers/extcon/extcon-sm5502.c | 4 +--
drivers/extcon/extcon.c | 60 ++++++++++++++++++++-------------------
include/linux/extcon.h | 61 +++++++++++++++++++++++-----------------
10 files changed, 137 insertions(+), 119 deletions(-)
diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index a1ab0a56b798..e4890dd4fefd 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -137,9 +137,9 @@ static const int arizona_micd_levels[] = {
static const unsigned int arizona_cable[] = {
EXTCON_MECHANICAL,
- EXTCON_MICROPHONE,
- EXTCON_HEADPHONE,
- EXTCON_LINE_OUT,
+ EXTCON_JACK_MICROPHONE,
+ EXTCON_JACK_HEADPHONE,
+ EXTCON_JACK_LINE_OUT,
EXTCON_NONE,
};
@@ -600,7 +600,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
struct arizona_extcon_info *info = data;
struct arizona *arizona = info->arizona;
int id_gpio = arizona->pdata.hpdet_id_gpio;
- unsigned int report = EXTCON_HEADPHONE;
+ unsigned int report = EXTCON_JACK_HEADPHONE;
int ret, reading;
bool mic = false;
@@ -645,9 +645,9 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
/* Report high impedence cables as line outputs */
if (reading >= 5000)
- report = EXTCON_LINE_OUT;
+ report = EXTCON_JACK_LINE_OUT;
else
- report = EXTCON_HEADPHONE;
+ report = EXTCON_JACK_HEADPHONE;
ret = extcon_set_cable_state_(info->edev, report, true);
if (ret != 0)
@@ -732,7 +732,7 @@ err:
ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
/* Just report headphone */
- ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true);
+ ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true);
if (ret != 0)
dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
@@ -789,7 +789,7 @@ err:
ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
/* Just report headphone */
- ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true);
+ ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true);
if (ret != 0)
dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
@@ -915,7 +915,7 @@ static void arizona_micd_detect(struct work_struct *work)
arizona_identify_headphone(info);
ret = extcon_set_cable_state_(info->edev,
- EXTCON_MICROPHONE, true);
+ EXTCON_JACK_MICROPHONE, true);
if (ret != 0)
dev_err(arizona->dev, "Headset report failed: %d\n",
ret);
diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
index 9668d6a94e38..14e748051288 100644
--- a/drivers/extcon/extcon-axp288.c
+++ b/drivers/extcon/extcon-axp288.c
@@ -102,9 +102,9 @@ enum axp288_extcon_irq {
};
static const unsigned int axp288_extcon_cables[] = {
- EXTCON_SLOW_CHARGER,
- EXTCON_CHARGE_DOWNSTREAM,
- EXTCON_FAST_CHARGER,
+ EXTCON_CHG_USB_DCP_SLOW,
+ EXTCON_CHG_USB_CDP,
+ EXTCON_CHG_USB_DCP_FAST,
EXTCON_NONE,
};
@@ -192,18 +192,18 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
dev_dbg(info->dev, "sdp cable is connecetd\n");
notify_otg = true;
notify_charger = true;
- cable = EXTCON_SLOW_CHARGER;
+ cable = EXTCON_CHG_USB_DCP_SLOW;
break;
case DET_STAT_CDP:
dev_dbg(info->dev, "cdp cable is connecetd\n");
notify_otg = true;
notify_charger = true;
- cable = EXTCON_CHARGE_DOWNSTREAM;
+ cable = EXTCON_CHG_USB_CDP;
break;
case DET_STAT_DCP:
dev_dbg(info->dev, "dcp cable is connecetd\n");
notify_charger = true;
- cable = EXTCON_FAST_CHARGER;
+ cable = EXTCON_CHG_USB_DCP_FAST;
break;
default:
dev_warn(info->dev,
diff --git a/drivers/extcon/extcon-max14577.c b/drivers/extcon/extcon-max14577.c
index df0659d98e5a..15c2dcc73fc6 100644
--- a/drivers/extcon/extcon-max14577.c
+++ b/drivers/extcon/extcon-max14577.c
@@ -150,10 +150,10 @@ enum max14577_muic_acc_type {
static const unsigned int max14577_extcon_cable[] = {
EXTCON_USB,
- EXTCON_TA,
- EXTCON_FAST_CHARGER,
- EXTCON_SLOW_CHARGER,
- EXTCON_CHARGE_DOWNSTREAM,
+ EXTCON_CHG_USB_DCP,
+ EXTCON_CHG_USB_DCP_FAST,
+ EXTCON_CHG_USB_DCP_SLOW,
+ EXTCON_CHG_USB_CDP,
EXTCON_JIG,
EXTCON_NONE,
};
@@ -456,18 +456,19 @@ static int max14577_muic_chg_handler(struct max14577_muic_info *info)
extcon_set_cable_state_(info->edev, EXTCON_USB, attached);
break;
case MAX14577_CHARGER_TYPE_DEDICATED_CHG:
- extcon_set_cable_state_(info->edev, EXTCON_TA, attached);
+ extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP,
+ attached);
break;
case MAX14577_CHARGER_TYPE_DOWNSTREAM_PORT:
- extcon_set_cable_state_(info->edev, EXTCON_CHARGE_DOWNSTREAM,
+ extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_CDP,
attached);
break;
case MAX14577_CHARGER_TYPE_SPECIAL_500MA:
- extcon_set_cable_state_(info->edev, EXTCON_SLOW_CHARGER,
+ extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_SLOW,
attached);
break;
case MAX14577_CHARGER_TYPE_SPECIAL_1A:
- extcon_set_cable_state_(info->edev, EXTCON_FAST_CHARGER,
+ extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_FAST,
attached);
break;
case MAX14577_CHARGER_TYPE_NONE:
diff --git a/drivers/extcon/extcon-max77693.c b/drivers/extcon/extcon-max77693.c
index 35b9e118b2fb..794199920070 100644
--- a/drivers/extcon/extcon-max77693.c
+++ b/drivers/extcon/extcon-max77693.c
@@ -204,11 +204,11 @@ enum max77693_muic_acc_type {
static const unsigned int max77693_extcon_cable[] = {
EXTCON_USB,
EXTCON_USB_HOST,
- EXTCON_TA,
- EXTCON_FAST_CHARGER,
- EXTCON_SLOW_CHARGER,
- EXTCON_CHARGE_DOWNSTREAM,
- EXTCON_MHL,
+ EXTCON_CHG_USB_DCP,
+ EXTCON_CHG_USB_DCP_FAST,
+ EXTCON_CHG_USB_DCP_SLOW,
+ EXTCON_CHG_USB_CDP,
+ EXTCON_DISP_MHL,
EXTCON_JIG,
EXTCON_DOCK,
EXTCON_NONE,
@@ -505,7 +505,7 @@ static int max77693_muic_dock_handler(struct max77693_muic_info *info,
return ret;
extcon_set_cable_state_(info->edev, EXTCON_DOCK, attached);
- extcon_set_cable_state_(info->edev, EXTCON_MHL, attached);
+ extcon_set_cable_state_(info->edev, EXTCON_DISP_MHL, attached);
goto out;
case MAX77693_MUIC_ADC_AUDIO_MODE_REMOTE: /* Dock-Desk */
dock_id = EXTCON_DOCK;
@@ -605,7 +605,7 @@ static int max77693_muic_adc_ground_handler(struct max77693_muic_info *info)
case MAX77693_MUIC_GND_MHL:
case MAX77693_MUIC_GND_MHL_VB:
/* MHL or MHL with USB/TA cable */
- extcon_set_cable_state_(info->edev, EXTCON_MHL, attached);
+ extcon_set_cable_state_(info->edev, EXTCON_DISP_MHL, attached);
break;
default:
dev_err(info->dev, "failed to detect %s cable of gnd type\n",
@@ -801,10 +801,11 @@ static int max77693_muic_chg_handler(struct max77693_muic_info *info)
* - Support charging through micro-usb port without
* data connection
*/
- extcon_set_cable_state_(info->edev, EXTCON_TA, attached);
+ extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP,
+ attached);
if (!cable_attached)
- extcon_set_cable_state_(info->edev, EXTCON_MHL,
- cable_attached);
+ extcon_set_cable_state_(info->edev,
+ EXTCON_DISP_MHL, cable_attached);
break;
}
@@ -862,7 +863,7 @@ static int max77693_muic_chg_handler(struct max77693_muic_info *info)
extcon_set_cable_state_(info->edev, EXTCON_DOCK,
attached);
- extcon_set_cable_state_(info->edev, EXTCON_MHL,
+ extcon_set_cable_state_(info->edev, EXTCON_DISP_MHL,
attached);
break;
}
@@ -901,20 +902,21 @@ static int max77693_muic_chg_handler(struct max77693_muic_info *info)
break;
case MAX77693_CHARGER_TYPE_DEDICATED_CHG:
/* Only TA cable */
- extcon_set_cable_state_(info->edev, EXTCON_TA, attached);
+ extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP,
+ attached);
break;
}
break;
case MAX77693_CHARGER_TYPE_DOWNSTREAM_PORT:
- extcon_set_cable_state_(info->edev, EXTCON_CHARGE_DOWNSTREAM,
+ extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_CDP,
attached);
break;
case MAX77693_CHARGER_TYPE_APPLE_500MA:
- extcon_set_cable_state_(info->edev, EXTCON_SLOW_CHARGER,
+ extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_SLOW,
attached);
break;
case MAX77693_CHARGER_TYPE_APPLE_1A_2A:
- extcon_set_cable_state_(info->edev, EXTCON_FAST_CHARGER,
+ extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_FAST,
attached);
break;
case MAX77693_CHARGER_TYPE_DEAD_BATTERY:
diff --git a/drivers/extcon/extcon-max77843.c b/drivers/extcon/extcon-max77843.c
index fdd928542c19..97aebe5d67a6 100644
--- a/drivers/extcon/extcon-max77843.c
+++ b/drivers/extcon/extcon-max77843.c
@@ -122,11 +122,11 @@ enum max77843_muic_charger_type {
static const unsigned int max77843_extcon_cable[] = {
EXTCON_USB,
EXTCON_USB_HOST,
- EXTCON_TA,
- EXTCON_CHARGE_DOWNSTREAM,
- EXTCON_FAST_CHARGER,
- EXTCON_SLOW_CHARGER,
- EXTCON_MHL,
+ EXTCON_CHG_USB_DCP,
+ EXTCON_CHG_USB_CDP,
+ EXTCON_CHG_USB_DCP_FAST,
+ EXTCON_CHG_USB_DCP_SLOW,
+ EXTCON_DISP_MHL,
EXTCON_JIG,
EXTCON_NONE,
};
@@ -355,7 +355,7 @@ static int max77843_muic_adc_gnd_handler(struct max77843_muic_info *info)
if (ret < 0)
return ret;
- extcon_set_cable_state_(info->edev, EXTCON_MHL, attached);
+ extcon_set_cable_state_(info->edev, EXTCON_DISP_MHL, attached);
break;
default:
dev_err(info->dev, "failed to detect %s accessory(gnd:0x%x)\n",
@@ -494,7 +494,7 @@ static int max77843_muic_chg_handler(struct max77843_muic_info *info)
if (ret < 0)
return ret;
- extcon_set_cable_state_(info->edev, EXTCON_CHARGE_DOWNSTREAM,
+ extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_CDP,
attached);
break;
case MAX77843_MUIC_CHG_DEDICATED:
@@ -504,7 +504,8 @@ static int max77843_muic_chg_handler(struct max77843_muic_info *info)
if (ret < 0)
return ret;
- extcon_set_cable_state_(info->edev, EXTCON_TA, attached);
+ extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP,
+ attached);
break;
case MAX77843_MUIC_CHG_SPECIAL_500MA:
ret = max77843_muic_set_path(info,
@@ -513,7 +514,7 @@ static int max77843_muic_chg_handler(struct max77843_muic_info *info)
if (ret < 0)
return ret;
- extcon_set_cable_state_(info->edev, EXTCON_SLOW_CHARGER,
+ extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_SLOW,
attached);
break;
case MAX77843_MUIC_CHG_SPECIAL_1A:
@@ -523,7 +524,7 @@ static int max77843_muic_chg_handler(struct max77843_muic_info *info)
if (ret < 0)
return ret;
- extcon_set_cable_state_(info->edev, EXTCON_FAST_CHARGER,
+ extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_FAST,
attached);
break;
case MAX77843_MUIC_CHG_GND:
@@ -532,9 +533,11 @@ static int max77843_muic_chg_handler(struct max77843_muic_info *info)
/* Charger cable on MHL accessory is attach or detach */
if (gnd_type == MAX77843_MUIC_GND_MHL_VB)
- extcon_set_cable_state_(info->edev, EXTCON_TA, true);
+ extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP,
+ true);
else if (gnd_type == MAX77843_MUIC_GND_MHL)
- extcon_set_cable_state_(info->edev, EXTCON_TA, false);
+ extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP,
+ false);
break;
case MAX77843_MUIC_CHG_NONE:
break;
diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
index 7b1ef200b121..8dbec5febdb6 100644
--- a/drivers/extcon/extcon-max8997.c
+++ b/drivers/extcon/extcon-max8997.c
@@ -148,11 +148,11 @@ struct max8997_muic_info {
static const unsigned int max8997_extcon_cable[] = {
EXTCON_USB,
EXTCON_USB_HOST,
- EXTCON_TA,
- EXTCON_FAST_CHARGER,
- EXTCON_SLOW_CHARGER,
- EXTCON_CHARGE_DOWNSTREAM,
- EXTCON_MHL,
+ EXTCON_CHG_USB_DCP,
+ EXTCON_CHG_USB_DCP_FAST,
+ EXTCON_CHG_USB_DCP_SLOW,
+ EXTCON_CHG_USB_CDP,
+ EXTCON_DISP_MHL,
EXTCON_DOCK,
EXTCON_JIG,
EXTCON_NONE,
@@ -403,7 +403,7 @@ static int max8997_muic_adc_handler(struct max8997_muic_info *info)
return ret;
break;
case MAX8997_MUIC_ADC_MHL:
- extcon_set_cable_state_(info->edev, EXTCON_MHL, attached);
+ extcon_set_cable_state_(info->edev, EXTCON_DISP_MHL, attached);
break;
case MAX8997_MUIC_ADC_FACTORY_MODE_USB_OFF:
case MAX8997_MUIC_ADC_FACTORY_MODE_USB_ON:
@@ -486,18 +486,19 @@ static int max8997_muic_chg_handler(struct max8997_muic_info *info)
}
break;
case MAX8997_CHARGER_TYPE_DOWNSTREAM_PORT:
- extcon_set_cable_state_(info->edev, EXTCON_CHARGE_DOWNSTREAM,
+ extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_CDP,
attached);
break;
case MAX8997_CHARGER_TYPE_DEDICATED_CHG:
- extcon_set_cable_state_(info->edev, EXTCON_TA, attached);
+ extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP,
+ attached);
break;
case MAX8997_CHARGER_TYPE_500MA:
- extcon_set_cable_state_(info->edev, EXTCON_SLOW_CHARGER,
+ extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_SLOW,
attached);
break;
case MAX8997_CHARGER_TYPE_1A:
- extcon_set_cable_state_(info->edev, EXTCON_FAST_CHARGER,
+ extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_FAST,
attached);
break;
default:
diff --git a/drivers/extcon/extcon-rt8973a.c b/drivers/extcon/extcon-rt8973a.c
index 1bc3737ea01c..36bf1d63791c 100644
--- a/drivers/extcon/extcon-rt8973a.c
+++ b/drivers/extcon/extcon-rt8973a.c
@@ -93,7 +93,7 @@ static struct reg_data rt8973a_reg_data[] = {
static const unsigned int rt8973a_extcon_cable[] = {
EXTCON_USB,
EXTCON_USB_HOST,
- EXTCON_TA,
+ EXTCON_CHG_USB_DCP,
EXTCON_JIG,
EXTCON_NONE,
};
@@ -333,7 +333,7 @@ static int rt8973a_muic_cable_handler(struct rt8973a_muic_info *info,
con_sw = DM_DP_SWITCH_USB;
break;
case RT8973A_MUIC_ADC_TA:
- id = EXTCON_TA;
+ id = EXTCON_CHG_USB_DCP;
con_sw = DM_DP_SWITCH_OPEN;
break;
case RT8973A_MUIC_ADC_FACTORY_MODE_BOOT_OFF_USB:
diff --git a/drivers/extcon/extcon-sm5502.c b/drivers/extcon/extcon-sm5502.c
index 2945091bfd0e..7aac3cc7efd7 100644
--- a/drivers/extcon/extcon-sm5502.c
+++ b/drivers/extcon/extcon-sm5502.c
@@ -95,7 +95,7 @@ static struct reg_data sm5502_reg_data[] = {
static const unsigned int sm5502_extcon_cable[] = {
EXTCON_USB,
EXTCON_USB_HOST,
- EXTCON_TA,
+ EXTCON_CHG_USB_DCP,
EXTCON_NONE,
};
@@ -389,7 +389,7 @@ static int sm5502_muic_cable_handler(struct sm5502_muic_info *info,
vbus_sw = VBUSIN_SWITCH_VBUSOUT_WITH_USB;
break;
case SM5502_MUIC_ADC_OPEN_TA:
- id = EXTCON_TA;
+ id = EXTCON_CHG_USB_DCP;
con_sw = DM_DP_SWITCH_OPEN;
vbus_sw = VBUSIN_SWITCH_VBUSOUT;
break;
diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 8dd0af1d50bc..3caa993259fb 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -39,37 +39,39 @@
#define CABLE_NAME_MAX 30
static const char *extcon_name[] = {
- [EXTCON_NONE] = "NONE",
+ [EXTCON_NONE] = "EXTCON_NONE",
/* USB external connector */
- [EXTCON_USB] = "USB",
- [EXTCON_USB_HOST] = "USB-HOST",
-
- /* Charger external connector */
- [EXTCON_TA] = "TA",
- [EXTCON_FAST_CHARGER] = "FAST-CHARGER",
- [EXTCON_SLOW_CHARGER] = "SLOW-CHARGER",
- [EXTCON_CHARGE_DOWNSTREAM] = "CHARGE-DOWNSTREAM",
-
- /* Audio/Video external connector */
- [EXTCON_LINE_IN] = "LINE-IN",
- [EXTCON_LINE_OUT] = "LINE-OUT",
- [EXTCON_MICROPHONE] = "MICROPHONE",
- [EXTCON_HEADPHONE] = "HEADPHONE",
-
- [EXTCON_HDMI] = "HDMI",
- [EXTCON_MHL] = "MHL",
- [EXTCON_DVI] = "DVI",
- [EXTCON_VGA] = "VGA",
- [EXTCON_SPDIF_IN] = "SPDIF-IN",
- [EXTCON_SPDIF_OUT] = "SPDIF-OUT",
- [EXTCON_VIDEO_IN] = "VIDEO-IN",
- [EXTCON_VIDEO_OUT] = "VIDEO-OUT",
-
- /* Etc external connector */
- [EXTCON_DOCK] = "DOCK",
- [EXTCON_JIG] = "JIG",
- [EXTCON_MECHANICAL] = "MECHANICAL",
+ [EXTCON_USB] = "EXTCON_USB",
+ [EXTCON_USB_HOST] = "EXTCON_USB_HOST",
+
+ /* Charging external connector */
+ [EXTCON_CHG_USB_DCP] = "EXTCON_CHG_USB_DCP",
+ [EXTCON_CHG_USB_CDP] = "EXTCON_CHG_USB_CDP",
+ [EXTCON_CHG_USB_ACA] = "EXTCON_CHG_USB_ACA",
+ [EXTCON_CHG_USB_DCP_FAST] = "EXTCON_CHG_USB_DCP_FAST",
+ [EXTCON_CHG_USB_DCP_SLOW] = "EXTCON_CHG_USB_DCP_SLOW",
+
+ /* Jack external connector */
+ [EXTCON_JACK_MICROPHONE] = "EXTCON_JACK_MICROPHONE",
+ [EXTCON_JACK_HEADPHONE] = "EXTCON_JACK_HEADPHONE",
+ [EXTCON_JACK_LINE_IN] = "EXTCON_JACK_LINE_IN",
+ [EXTCON_JACK_LINE_OUT] = "EXTCON_JACK_LINE_OUT",
+ [EXTCON_JACK_VIDEO_IN] = "EXTCON_JACK_VIDEO_IN",
+ [EXTCON_JACK_VIDEO_OUT] = "EXTCON_JACK_VIDEO_OUT",
+ [EXTCON_JACK_SPDIF_IN] = "EXTCON_JACK_SPDIF_IN",
+ [EXTCON_JACK_SPDIF_OUT] = "EXTCON_JACK_SPDIF_OUT",
+
+ /* Display external connector */
+ [EXTCON_DISP_HDMI] = "EXTCON_DISP_HDMI",
+ [EXTCON_DISP_MHL] = "EXTCON_DISP_MHL",
+ [EXTCON_DISP_DVI] = "EXTCON_DISP_DVI",
+ [EXTCON_DISP_VGA] = "EXTCON_DISP_VGA",
+
+ /* Miscellaneous external connector */
+ [EXTCON_DOCK] = "EXTCON_DOCK",
+ [EXTCON_JIG] = "EXTCON_JIG",
+ [EXTCON_MECHANICAL] = "EXTCON_MECHANICAL",
NULL,
};
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index c0f8c4fc5d45..2625a89c2809 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -31,32 +31,41 @@
/*
* Define the unique id of supported external connectors
*/
-#define EXTCON_NONE 0
-
-#define EXTCON_USB 1 /* USB connector */
-#define EXTCON_USB_HOST 2
-
-#define EXTCON_TA 3 /* Charger connector */
-#define EXTCON_FAST_CHARGER 4
-#define EXTCON_SLOW_CHARGER 5
-#define EXTCON_CHARGE_DOWNSTREAM 6
-
-#define EXTCON_LINE_IN 7 /* Audio/Video connector */
-#define EXTCON_LINE_OUT 8
-#define EXTCON_MICROPHONE 9
-#define EXTCON_HEADPHONE 10
-#define EXTCON_HDMI 11
-#define EXTCON_MHL 12
-#define EXTCON_DVI 13
-#define EXTCON_VGA 14
-#define EXTCON_SPDIF_IN 15
-#define EXTCON_SPDIF_OUT 16
-#define EXTCON_VIDEO_IN 17
-#define EXTCON_VIDEO_OUT 18
-
-#define EXTCON_DOCK 19 /* Misc connector */
-#define EXTCON_JIG 20
-#define EXTCON_MECHANICAL 21
+#define EXTCON_NONE 0
+
+/* USB external connector */
+#define EXTCON_USB 1 /* Standard Downstream Port */
+#define EXTCON_USB_HOST 2
+
+/* Charging external connector */
+#define EXTCON_CHG_USB_DCP 5 /* Dedicated Charging Port */
+#define EXTCON_CHG_USB_CDP 6 /* Charging Downstream Port */
+#define EXTCON_CHG_USB_ACA 7 /* Accessory Charger Adapter */
+#define EXTCON_CHG_USB_DCP_FAST 8
+#define EXTCON_CHG_USB_DCP_SLOW 9
+
+/* Jack external connector */
+#define EXTCON_JACK_MICROPHONE 20
+#define EXTCON_JACK_HEADPHONE 21
+#define EXTCON_JACK_LINE_IN 22
+#define EXTCON_JACK_LINE_OUT 23
+#define EXTCON_JACK_VIDEO_IN 24
+#define EXTCON_JACK_VIDEO_OUT 25
+#define EXTCON_JACK_SPDIF_IN 26 /* Sony Philips Digital InterFace */
+#define EXTCON_JACK_SPDIF_OUT 27
+
+/* Display external connector */
+#define EXTCON_DISP_HDMI 40 /* High-Definition Multimedia Interface */
+#define EXTCON_DISP_MHL 41 /* Mobile High-Definition Link */
+#define EXTCON_DISP_DVI 42 /* Digital Visual Interface */
+#define EXTCON_DISP_VGA 43 /* Video Graphics Array */
+
+/* Miscellaneous external connector */
+#define EXTCON_DOCK 60
+#define EXTCON_JIG 61
+#define EXTCON_MECHANICAL 62
+
+#define EXTCON_NUM 63
struct extcon_cable;
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3] extcon: Modify the id and name of external connector
2015-10-07 0:48 [PATCH v3] extcon: Modify the id and name of external connector Chanwoo Choi
@ 2015-10-07 7:33 ` Roger Quadros
2015-10-07 7:42 ` Chanwoo Choi
0 siblings, 1 reply; 10+ messages in thread
From: Roger Quadros @ 2015-10-07 7:33 UTC (permalink / raw)
To: Chanwoo Choi, linux-kernel
Cc: k.kozlowski, ckeepax, gregkh, ramakrishna.pallala, patches,
myungjoo.ham
On 07/10/15 03:48, Chanwoo Choi wrote:
> This patch modifies the id and name of external connector with the
> additional prefix to clarify both attribute and meaning of external
> connector as following:
> - EXTCON_CHG_* mean the charger connector.
> - EXTCON_JACK_* mean the jack connector.
> - EXTCON_DISP_* mean the display port connector.
>
> Following table show the new name of external connector with old name:
> --------------------------------------------------
> Old extcon name | New extcon name |
> --------------------------------------------------
> EXTCON_TA | EXTCON_CHG_USB_DCP |
> EXTCON_CHARGE_DOWNSTREAM| EXTCON_CHG_USB_CDP |
> EXTCON_FAST_CHARGER | EXTCON_CHG_USB_DCP_FAST|
> EXTCON_SLOW_CHARGER | EXTCON_CHG_USB_DCP_SLOW|
I still have some concerns regarding the name DCP_FAST and DCP_SLOW.
If I see the charger IC drivers that use FAST & SLOW, they either call it
APPLE_500MA/1A or SPECIAL_500MA/1A.
They are probably not compliant with USB DCP specification and hence
the special name.
So, should we call them EXTCON_CHG_USB_SPECIAL_FAST/SLOW instead?
> --------------------------------------------------
> EXTCON_MICROPHONE | EXTCON_JACK_MICROPHONE |
> EXTCON_HEADPHONE | EXTCON_JACK_HEADPHONE |
> EXTCON_LINE_IN | EXTCON_JACK_LINE_IN |
> EXTCON_LINE_OUT | EXTCON_JACK_LINE_OUT |
> EXTCON_VIDEO_IN | EXTCON_JACK_VIDEO_IN |
> EXTCON_VIDEO_OUT | EXTCON_JACK_VIDEO_OUT |
> EXTCON_SPDIF_IN | EXTCON_JACK_SPDIF_IN |
> EXTCON_SPDIF_OUT | EXTCON_JACK_SPDIF_OUT |
> --------------------------------------------------
> EXTCON_HMDI | EXTCON_DISP_HDMI |
> EXTCON_MHL | EXTCON_DISP_MHL |
> EXTCON_DVI | EXTCON_DISP_DVI |
> EXTCON_VGA | EXTCON_DISP_VGA |
> --------------------------------------------------
>
> And, when altering the name of USB charger connector, EXTCON refers to the
> "Battery Charging v1.2 Spec and Adopters Agreement"[1] to use the standard
> name of USB charging port as following. Following name of USB charging port
> are already used in power_supply subsystem. We chan check it on patch[2].
> - EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
> - EXTCON_CHG_USB_CDP /* Charging Downstream Port */
> - EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */
>
> [1] www.usb.org/developers/docs/devclass_docs/BCv1.2_070312.zip
> [2] commit 85efc8a18ced ("power_supply: Add types for USB chargers")
>
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> [ckeepax: For the Arizona changes]
> Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
> Changes from v2:
> (https://lkml.org/lkml/2015/10/6/239)
> - Remove the EXTCON_CHG_USB type to remove the possible confusion according to
> Roger's comment and drop patch2 about EXTCON_CHG_USB.
> - Fix the warning issue provided by scripts/checkpatch.pl
>
> Changes from v1:
> (https://lkml.org/lkml/2015/10/3/304)
> - Add acked tag by Charles Keepax for arizona changes
> - Modify the name of USB charger connector as following:
> : EXTCON_CHG_USB_FAST -> EXTCON_CHG_USB_DCP_FAST
> : EXTCON_CHG_USB_SLOW -> EXTCON_CHG_USB_DCP_SLOW
> - Add the missing EXTCON_CHG_USB_ACA charger connector
> - Add one more patch to support the EXTCON_CHG_USB when SDP port is connected
> or not
>
> drivers/extcon/extcon-arizona.c | 18 ++++++------
> drivers/extcon/extcon-axp288.c | 12 ++++----
> drivers/extcon/extcon-max14577.c | 17 +++++------
> drivers/extcon/extcon-max77693.c | 32 +++++++++++----------
> drivers/extcon/extcon-max77843.c | 27 ++++++++++--------
> drivers/extcon/extcon-max8997.c | 21 +++++++-------
> drivers/extcon/extcon-rt8973a.c | 4 +--
> drivers/extcon/extcon-sm5502.c | 4 +--
> drivers/extcon/extcon.c | 60 ++++++++++++++++++++-------------------
> include/linux/extcon.h | 61 +++++++++++++++++++++++-----------------
> 10 files changed, 137 insertions(+), 119 deletions(-)
>
> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
> index a1ab0a56b798..e4890dd4fefd 100644
> --- a/drivers/extcon/extcon-arizona.c
> +++ b/drivers/extcon/extcon-arizona.c
> @@ -137,9 +137,9 @@ static const int arizona_micd_levels[] = {
>
> static const unsigned int arizona_cable[] = {
> EXTCON_MECHANICAL,
> - EXTCON_MICROPHONE,
> - EXTCON_HEADPHONE,
> - EXTCON_LINE_OUT,
> + EXTCON_JACK_MICROPHONE,
> + EXTCON_JACK_HEADPHONE,
> + EXTCON_JACK_LINE_OUT,
> EXTCON_NONE,
> };
>
> @@ -600,7 +600,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
> struct arizona_extcon_info *info = data;
> struct arizona *arizona = info->arizona;
> int id_gpio = arizona->pdata.hpdet_id_gpio;
> - unsigned int report = EXTCON_HEADPHONE;
> + unsigned int report = EXTCON_JACK_HEADPHONE;
> int ret, reading;
> bool mic = false;
>
> @@ -645,9 +645,9 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>
> /* Report high impedence cables as line outputs */
> if (reading >= 5000)
> - report = EXTCON_LINE_OUT;
> + report = EXTCON_JACK_LINE_OUT;
> else
> - report = EXTCON_HEADPHONE;
> + report = EXTCON_JACK_HEADPHONE;
>
> ret = extcon_set_cable_state_(info->edev, report, true);
> if (ret != 0)
> @@ -732,7 +732,7 @@ err:
> ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
>
> /* Just report headphone */
> - ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true);
> + ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true);
> if (ret != 0)
> dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
>
> @@ -789,7 +789,7 @@ err:
> ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
>
> /* Just report headphone */
> - ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true);
> + ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true);
> if (ret != 0)
> dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
>
> @@ -915,7 +915,7 @@ static void arizona_micd_detect(struct work_struct *work)
> arizona_identify_headphone(info);
>
> ret = extcon_set_cable_state_(info->edev,
> - EXTCON_MICROPHONE, true);
> + EXTCON_JACK_MICROPHONE, true);
> if (ret != 0)
> dev_err(arizona->dev, "Headset report failed: %d\n",
> ret);
> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
> index 9668d6a94e38..14e748051288 100644
> --- a/drivers/extcon/extcon-axp288.c
> +++ b/drivers/extcon/extcon-axp288.c
> @@ -102,9 +102,9 @@ enum axp288_extcon_irq {
> };
>
> static const unsigned int axp288_extcon_cables[] = {
> - EXTCON_SLOW_CHARGER,
> - EXTCON_CHARGE_DOWNSTREAM,
> - EXTCON_FAST_CHARGER,
> + EXTCON_CHG_USB_DCP_SLOW,
> + EXTCON_CHG_USB_CDP,
> + EXTCON_CHG_USB_DCP_FAST,
> EXTCON_NONE,
> };
>
> @@ -192,18 +192,18 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
> dev_dbg(info->dev, "sdp cable is connecetd\n");
as per comment this seems to be SDP.
> notify_otg = true;
> notify_charger = true;
> - cable = EXTCON_SLOW_CHARGER;
> + cable = EXTCON_CHG_USB_DCP_SLOW;
So shouldn't this be cable = EXTCON_USB?
> break;
> case DET_STAT_CDP:
> dev_dbg(info->dev, "cdp cable is connecetd\n");
> notify_otg = true;
> notify_charger = true;
> - cable = EXTCON_CHARGE_DOWNSTREAM;
> + cable = EXTCON_CHG_USB_CDP;
> break;
> case DET_STAT_DCP:
> dev_dbg(info->dev, "dcp cable is connecetd\n");
This looks like standard DCP.
> notify_charger = true;
> - cable = EXTCON_FAST_CHARGER;
> + cable = EXTCON_CHG_USB_DCP_FAST;
Shouldn't this be cable = EXTCON_CHG_USB_DCP?
> break;
> default:
> dev_warn(info->dev,
> diff --git a/drivers/extcon/extcon-max14577.c b/drivers/extcon/extcon-max14577.c
> index df0659d98e5a..15c2dcc73fc6 100644
> --- a/drivers/extcon/extcon-max14577.c
> +++ b/drivers/extcon/extcon-max14577.c
> @@ -150,10 +150,10 @@ enum max14577_muic_acc_type {
>
> static const unsigned int max14577_extcon_cable[] = {
> EXTCON_USB,
> - EXTCON_TA,
> - EXTCON_FAST_CHARGER,
> - EXTCON_SLOW_CHARGER,
> - EXTCON_CHARGE_DOWNSTREAM,
> + EXTCON_CHG_USB_DCP,
> + EXTCON_CHG_USB_DCP_FAST,
> + EXTCON_CHG_USB_DCP_SLOW,
> + EXTCON_CHG_USB_CDP,
> EXTCON_JIG,
> EXTCON_NONE,
> };
> @@ -456,18 +456,19 @@ static int max14577_muic_chg_handler(struct max14577_muic_info *info)
> extcon_set_cable_state_(info->edev, EXTCON_USB, attached);
> break;
> case MAX14577_CHARGER_TYPE_DEDICATED_CHG:
> - extcon_set_cable_state_(info->edev, EXTCON_TA, attached);
> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP,
> + attached);
> break;
> case MAX14577_CHARGER_TYPE_DOWNSTREAM_PORT:
> - extcon_set_cable_state_(info->edev, EXTCON_CHARGE_DOWNSTREAM,
> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_CDP,
> attached);
> break;
> case MAX14577_CHARGER_TYPE_SPECIAL_500MA:
> - extcon_set_cable_state_(info->edev, EXTCON_SLOW_CHARGER,
> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_SLOW,
> attached);
> break;
> case MAX14577_CHARGER_TYPE_SPECIAL_1A:
> - extcon_set_cable_state_(info->edev, EXTCON_FAST_CHARGER,
> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_FAST,
> attached);
> break;
> case MAX14577_CHARGER_TYPE_NONE:
> diff --git a/drivers/extcon/extcon-max77693.c b/drivers/extcon/extcon-max77693.c
> index 35b9e118b2fb..794199920070 100644
> --- a/drivers/extcon/extcon-max77693.c
> +++ b/drivers/extcon/extcon-max77693.c
> @@ -204,11 +204,11 @@ enum max77693_muic_acc_type {
> static const unsigned int max77693_extcon_cable[] = {
> EXTCON_USB,
> EXTCON_USB_HOST,
> - EXTCON_TA,
> - EXTCON_FAST_CHARGER,
> - EXTCON_SLOW_CHARGER,
> - EXTCON_CHARGE_DOWNSTREAM,
> - EXTCON_MHL,
> + EXTCON_CHG_USB_DCP,
> + EXTCON_CHG_USB_DCP_FAST,
> + EXTCON_CHG_USB_DCP_SLOW,
> + EXTCON_CHG_USB_CDP,
> + EXTCON_DISP_MHL,
> EXTCON_JIG,
> EXTCON_DOCK,
> EXTCON_NONE,
> @@ -505,7 +505,7 @@ static int max77693_muic_dock_handler(struct max77693_muic_info *info,
> return ret;
>
> extcon_set_cable_state_(info->edev, EXTCON_DOCK, attached);
> - extcon_set_cable_state_(info->edev, EXTCON_MHL, attached);
> + extcon_set_cable_state_(info->edev, EXTCON_DISP_MHL, attached);
> goto out;
> case MAX77693_MUIC_ADC_AUDIO_MODE_REMOTE: /* Dock-Desk */
> dock_id = EXTCON_DOCK;
> @@ -605,7 +605,7 @@ static int max77693_muic_adc_ground_handler(struct max77693_muic_info *info)
> case MAX77693_MUIC_GND_MHL:
> case MAX77693_MUIC_GND_MHL_VB:
> /* MHL or MHL with USB/TA cable */
> - extcon_set_cable_state_(info->edev, EXTCON_MHL, attached);
> + extcon_set_cable_state_(info->edev, EXTCON_DISP_MHL, attached);
> break;
> default:
> dev_err(info->dev, "failed to detect %s cable of gnd type\n",
> @@ -801,10 +801,11 @@ static int max77693_muic_chg_handler(struct max77693_muic_info *info)
> * - Support charging through micro-usb port without
> * data connection
> */
> - extcon_set_cable_state_(info->edev, EXTCON_TA, attached);
> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP,
> + attached);
> if (!cable_attached)
> - extcon_set_cable_state_(info->edev, EXTCON_MHL,
> - cable_attached);
> + extcon_set_cable_state_(info->edev,
> + EXTCON_DISP_MHL, cable_attached);
> break;
> }
>
> @@ -862,7 +863,7 @@ static int max77693_muic_chg_handler(struct max77693_muic_info *info)
>
> extcon_set_cable_state_(info->edev, EXTCON_DOCK,
> attached);
> - extcon_set_cable_state_(info->edev, EXTCON_MHL,
> + extcon_set_cable_state_(info->edev, EXTCON_DISP_MHL,
> attached);
> break;
> }
> @@ -901,20 +902,21 @@ static int max77693_muic_chg_handler(struct max77693_muic_info *info)
> break;
> case MAX77693_CHARGER_TYPE_DEDICATED_CHG:
> /* Only TA cable */
> - extcon_set_cable_state_(info->edev, EXTCON_TA, attached);
> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP,
> + attached);
> break;
> }
> break;
> case MAX77693_CHARGER_TYPE_DOWNSTREAM_PORT:
> - extcon_set_cable_state_(info->edev, EXTCON_CHARGE_DOWNSTREAM,
> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_CDP,
> attached);
> break;
> case MAX77693_CHARGER_TYPE_APPLE_500MA:
> - extcon_set_cable_state_(info->edev, EXTCON_SLOW_CHARGER,
> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_SLOW,
> attached);
Here it says APPLE_500MA. Looks like it is not same as USB DCP.
So probably USB_DCP_SLOW is a wrong name for it.
So EXTCON_CHG_USB_SPECIAL_SLOW?
> break;
> case MAX77693_CHARGER_TYPE_APPLE_1A_2A:
> - extcon_set_cable_state_(info->edev, EXTCON_FAST_CHARGER,
> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_FAST,
> attached);
EXTCON_CHG_USB_SPECIAL_FAST?
> break;
> case MAX77693_CHARGER_TYPE_DEAD_BATTERY:
> diff --git a/drivers/extcon/extcon-max77843.c b/drivers/extcon/extcon-max77843.c
> index fdd928542c19..97aebe5d67a6 100644
> --- a/drivers/extcon/extcon-max77843.c
> +++ b/drivers/extcon/extcon-max77843.c
> @@ -122,11 +122,11 @@ enum max77843_muic_charger_type {
> static const unsigned int max77843_extcon_cable[] = {
> EXTCON_USB,
> EXTCON_USB_HOST,
> - EXTCON_TA,
> - EXTCON_CHARGE_DOWNSTREAM,
> - EXTCON_FAST_CHARGER,
> - EXTCON_SLOW_CHARGER,
> - EXTCON_MHL,
> + EXTCON_CHG_USB_DCP,
> + EXTCON_CHG_USB_CDP,
> + EXTCON_CHG_USB_DCP_FAST,
> + EXTCON_CHG_USB_DCP_SLOW,
> + EXTCON_DISP_MHL,
> EXTCON_JIG,
> EXTCON_NONE,
> };
> @@ -355,7 +355,7 @@ static int max77843_muic_adc_gnd_handler(struct max77843_muic_info *info)
> if (ret < 0)
> return ret;
>
> - extcon_set_cable_state_(info->edev, EXTCON_MHL, attached);
> + extcon_set_cable_state_(info->edev, EXTCON_DISP_MHL, attached);
> break;
> default:
> dev_err(info->dev, "failed to detect %s accessory(gnd:0x%x)\n",
> @@ -494,7 +494,7 @@ static int max77843_muic_chg_handler(struct max77843_muic_info *info)
> if (ret < 0)
> return ret;
>
> - extcon_set_cable_state_(info->edev, EXTCON_CHARGE_DOWNSTREAM,
> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_CDP,
> attached);
> break;
> case MAX77843_MUIC_CHG_DEDICATED:
> @@ -504,7 +504,8 @@ static int max77843_muic_chg_handler(struct max77843_muic_info *info)
> if (ret < 0)
> return ret;
>
> - extcon_set_cable_state_(info->edev, EXTCON_TA, attached);
> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP,
> + attached);
> break;
> case MAX77843_MUIC_CHG_SPECIAL_500MA:
> ret = max77843_muic_set_path(info,
> @@ -513,7 +514,7 @@ static int max77843_muic_chg_handler(struct max77843_muic_info *info)
> if (ret < 0)
> return ret;
>
> - extcon_set_cable_state_(info->edev, EXTCON_SLOW_CHARGER,
> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_SLOW,
> attached);
Here it is SPECIAL_500MA. Again this might not be compliant to USB DCP.
EXTCON_CHG_USB_SPECIAL_SLOW?
> break;
> case MAX77843_MUIC_CHG_SPECIAL_1A:
> @@ -523,7 +524,7 @@ static int max77843_muic_chg_handler(struct max77843_muic_info *info)
> if (ret < 0)
> return ret;
>
> - extcon_set_cable_state_(info->edev, EXTCON_FAST_CHARGER,
> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_FAST,
> attached);
EXTCON_CHG_USB_SPECIAL_FAST?
> break;
> case MAX77843_MUIC_CHG_GND:
> @@ -532,9 +533,11 @@ static int max77843_muic_chg_handler(struct max77843_muic_info *info)
>
> /* Charger cable on MHL accessory is attach or detach */
> if (gnd_type == MAX77843_MUIC_GND_MHL_VB)
> - extcon_set_cable_state_(info->edev, EXTCON_TA, true);
> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP,
> + true);
> else if (gnd_type == MAX77843_MUIC_GND_MHL)
> - extcon_set_cable_state_(info->edev, EXTCON_TA, false);
> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP,
> + false);
> break;
> case MAX77843_MUIC_CHG_NONE:
> break;
> diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
> index 7b1ef200b121..8dbec5febdb6 100644
> --- a/drivers/extcon/extcon-max8997.c
> +++ b/drivers/extcon/extcon-max8997.c
> @@ -148,11 +148,11 @@ struct max8997_muic_info {
> static const unsigned int max8997_extcon_cable[] = {
> EXTCON_USB,
> EXTCON_USB_HOST,
> - EXTCON_TA,
> - EXTCON_FAST_CHARGER,
> - EXTCON_SLOW_CHARGER,
> - EXTCON_CHARGE_DOWNSTREAM,
> - EXTCON_MHL,
> + EXTCON_CHG_USB_DCP,
> + EXTCON_CHG_USB_DCP_FAST,
> + EXTCON_CHG_USB_DCP_SLOW,
> + EXTCON_CHG_USB_CDP,
> + EXTCON_DISP_MHL,
> EXTCON_DOCK,
> EXTCON_JIG,
> EXTCON_NONE,
> @@ -403,7 +403,7 @@ static int max8997_muic_adc_handler(struct max8997_muic_info *info)
> return ret;
> break;
> case MAX8997_MUIC_ADC_MHL:
> - extcon_set_cable_state_(info->edev, EXTCON_MHL, attached);
> + extcon_set_cable_state_(info->edev, EXTCON_DISP_MHL, attached);
> break;
> case MAX8997_MUIC_ADC_FACTORY_MODE_USB_OFF:
> case MAX8997_MUIC_ADC_FACTORY_MODE_USB_ON:
> @@ -486,18 +486,19 @@ static int max8997_muic_chg_handler(struct max8997_muic_info *info)
> }
> break;
> case MAX8997_CHARGER_TYPE_DOWNSTREAM_PORT:
> - extcon_set_cable_state_(info->edev, EXTCON_CHARGE_DOWNSTREAM,
> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_CDP,
> attached);
> break;
> case MAX8997_CHARGER_TYPE_DEDICATED_CHG:
> - extcon_set_cable_state_(info->edev, EXTCON_TA, attached);
> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP,
> + attached);
> break;
> case MAX8997_CHARGER_TYPE_500MA:
> - extcon_set_cable_state_(info->edev, EXTCON_SLOW_CHARGER,
> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_SLOW,
> attached);
EXTCON_CHG_USB_SPECIAL_SLOW?
> break;
> case MAX8997_CHARGER_TYPE_1A:
> - extcon_set_cable_state_(info->edev, EXTCON_FAST_CHARGER,
> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_FAST,
> attached);
EXTCON_CHG_USB_SPECIAL_FAST?
> break;
> default:
> diff --git a/drivers/extcon/extcon-rt8973a.c b/drivers/extcon/extcon-rt8973a.c
> index 1bc3737ea01c..36bf1d63791c 100644
> --- a/drivers/extcon/extcon-rt8973a.c
> +++ b/drivers/extcon/extcon-rt8973a.c
> @@ -93,7 +93,7 @@ static struct reg_data rt8973a_reg_data[] = {
> static const unsigned int rt8973a_extcon_cable[] = {
> EXTCON_USB,
> EXTCON_USB_HOST,
> - EXTCON_TA,
> + EXTCON_CHG_USB_DCP,
> EXTCON_JIG,
> EXTCON_NONE,
> };
> @@ -333,7 +333,7 @@ static int rt8973a_muic_cable_handler(struct rt8973a_muic_info *info,
> con_sw = DM_DP_SWITCH_USB;
> break;
> case RT8973A_MUIC_ADC_TA:
> - id = EXTCON_TA;
> + id = EXTCON_CHG_USB_DCP;
> con_sw = DM_DP_SWITCH_OPEN;
> break;
> case RT8973A_MUIC_ADC_FACTORY_MODE_BOOT_OFF_USB:
> diff --git a/drivers/extcon/extcon-sm5502.c b/drivers/extcon/extcon-sm5502.c
> index 2945091bfd0e..7aac3cc7efd7 100644
> --- a/drivers/extcon/extcon-sm5502.c
> +++ b/drivers/extcon/extcon-sm5502.c
> @@ -95,7 +95,7 @@ static struct reg_data sm5502_reg_data[] = {
> static const unsigned int sm5502_extcon_cable[] = {
> EXTCON_USB,
> EXTCON_USB_HOST,
> - EXTCON_TA,
> + EXTCON_CHG_USB_DCP,
> EXTCON_NONE,
> };
>
> @@ -389,7 +389,7 @@ static int sm5502_muic_cable_handler(struct sm5502_muic_info *info,
> vbus_sw = VBUSIN_SWITCH_VBUSOUT_WITH_USB;
> break;
> case SM5502_MUIC_ADC_OPEN_TA:
> - id = EXTCON_TA;
> + id = EXTCON_CHG_USB_DCP;
> con_sw = DM_DP_SWITCH_OPEN;
> vbus_sw = VBUSIN_SWITCH_VBUSOUT;
> break;
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index 8dd0af1d50bc..3caa993259fb 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -39,37 +39,39 @@
> #define CABLE_NAME_MAX 30
>
> static const char *extcon_name[] = {
> - [EXTCON_NONE] = "NONE",
> + [EXTCON_NONE] = "EXTCON_NONE",
>
> /* USB external connector */
> - [EXTCON_USB] = "USB",
> - [EXTCON_USB_HOST] = "USB-HOST",
> -
> - /* Charger external connector */
> - [EXTCON_TA] = "TA",
> - [EXTCON_FAST_CHARGER] = "FAST-CHARGER",
> - [EXTCON_SLOW_CHARGER] = "SLOW-CHARGER",
> - [EXTCON_CHARGE_DOWNSTREAM] = "CHARGE-DOWNSTREAM",
> -
> - /* Audio/Video external connector */
> - [EXTCON_LINE_IN] = "LINE-IN",
> - [EXTCON_LINE_OUT] = "LINE-OUT",
> - [EXTCON_MICROPHONE] = "MICROPHONE",
> - [EXTCON_HEADPHONE] = "HEADPHONE",
> -
> - [EXTCON_HDMI] = "HDMI",
> - [EXTCON_MHL] = "MHL",
> - [EXTCON_DVI] = "DVI",
> - [EXTCON_VGA] = "VGA",
> - [EXTCON_SPDIF_IN] = "SPDIF-IN",
> - [EXTCON_SPDIF_OUT] = "SPDIF-OUT",
> - [EXTCON_VIDEO_IN] = "VIDEO-IN",
> - [EXTCON_VIDEO_OUT] = "VIDEO-OUT",
> -
> - /* Etc external connector */
> - [EXTCON_DOCK] = "DOCK",
> - [EXTCON_JIG] = "JIG",
> - [EXTCON_MECHANICAL] = "MECHANICAL",
> + [EXTCON_USB] = "EXTCON_USB",
> + [EXTCON_USB_HOST] = "EXTCON_USB_HOST",
> +
> + /* Charging external connector */
> + [EXTCON_CHG_USB_DCP] = "EXTCON_CHG_USB_DCP",
> + [EXTCON_CHG_USB_CDP] = "EXTCON_CHG_USB_CDP",
> + [EXTCON_CHG_USB_ACA] = "EXTCON_CHG_USB_ACA",
> + [EXTCON_CHG_USB_DCP_FAST] = "EXTCON_CHG_USB_DCP_FAST",
> + [EXTCON_CHG_USB_DCP_SLOW] = "EXTCON_CHG_USB_DCP_SLOW",
> +
> + /* Jack external connector */
> + [EXTCON_JACK_MICROPHONE] = "EXTCON_JACK_MICROPHONE",
> + [EXTCON_JACK_HEADPHONE] = "EXTCON_JACK_HEADPHONE",
> + [EXTCON_JACK_LINE_IN] = "EXTCON_JACK_LINE_IN",
> + [EXTCON_JACK_LINE_OUT] = "EXTCON_JACK_LINE_OUT",
> + [EXTCON_JACK_VIDEO_IN] = "EXTCON_JACK_VIDEO_IN",
> + [EXTCON_JACK_VIDEO_OUT] = "EXTCON_JACK_VIDEO_OUT",
> + [EXTCON_JACK_SPDIF_IN] = "EXTCON_JACK_SPDIF_IN",
> + [EXTCON_JACK_SPDIF_OUT] = "EXTCON_JACK_SPDIF_OUT",
> +
> + /* Display external connector */
> + [EXTCON_DISP_HDMI] = "EXTCON_DISP_HDMI",
> + [EXTCON_DISP_MHL] = "EXTCON_DISP_MHL",
> + [EXTCON_DISP_DVI] = "EXTCON_DISP_DVI",
> + [EXTCON_DISP_VGA] = "EXTCON_DISP_VGA",
> +
> + /* Miscellaneous external connector */
> + [EXTCON_DOCK] = "EXTCON_DOCK",
> + [EXTCON_JIG] = "EXTCON_JIG",
> + [EXTCON_MECHANICAL] = "EXTCON_MECHANICAL",
>
> NULL,
> };
> diff --git a/include/linux/extcon.h b/include/linux/extcon.h
> index c0f8c4fc5d45..2625a89c2809 100644
> --- a/include/linux/extcon.h
> +++ b/include/linux/extcon.h
> @@ -31,32 +31,41 @@
> /*
> * Define the unique id of supported external connectors
> */
> -#define EXTCON_NONE 0
> -
> -#define EXTCON_USB 1 /* USB connector */
> -#define EXTCON_USB_HOST 2
> -
> -#define EXTCON_TA 3 /* Charger connector */
> -#define EXTCON_FAST_CHARGER 4
> -#define EXTCON_SLOW_CHARGER 5
> -#define EXTCON_CHARGE_DOWNSTREAM 6
> -
> -#define EXTCON_LINE_IN 7 /* Audio/Video connector */
> -#define EXTCON_LINE_OUT 8
> -#define EXTCON_MICROPHONE 9
> -#define EXTCON_HEADPHONE 10
> -#define EXTCON_HDMI 11
> -#define EXTCON_MHL 12
> -#define EXTCON_DVI 13
> -#define EXTCON_VGA 14
> -#define EXTCON_SPDIF_IN 15
> -#define EXTCON_SPDIF_OUT 16
> -#define EXTCON_VIDEO_IN 17
> -#define EXTCON_VIDEO_OUT 18
> -
> -#define EXTCON_DOCK 19 /* Misc connector */
> -#define EXTCON_JIG 20
> -#define EXTCON_MECHANICAL 21
> +#define EXTCON_NONE 0
> +
> +/* USB external connector */
> +#define EXTCON_USB 1 /* Standard Downstream Port */
> +#define EXTCON_USB_HOST 2
> +
> +/* Charging external connector */
> +#define EXTCON_CHG_USB_DCP 5 /* Dedicated Charging Port */
> +#define EXTCON_CHG_USB_CDP 6 /* Charging Downstream Port */
> +#define EXTCON_CHG_USB_ACA 7 /* Accessory Charger Adapter */
> +#define EXTCON_CHG_USB_DCP_FAST 8
> +#define EXTCON_CHG_USB_DCP_SLOW 9
> +
> +/* Jack external connector */
> +#define EXTCON_JACK_MICROPHONE 20
> +#define EXTCON_JACK_HEADPHONE 21
> +#define EXTCON_JACK_LINE_IN 22
> +#define EXTCON_JACK_LINE_OUT 23
> +#define EXTCON_JACK_VIDEO_IN 24
> +#define EXTCON_JACK_VIDEO_OUT 25
> +#define EXTCON_JACK_SPDIF_IN 26 /* Sony Philips Digital InterFace */
> +#define EXTCON_JACK_SPDIF_OUT 27
> +
> +/* Display external connector */
> +#define EXTCON_DISP_HDMI 40 /* High-Definition Multimedia Interface */
> +#define EXTCON_DISP_MHL 41 /* Mobile High-Definition Link */
> +#define EXTCON_DISP_DVI 42 /* Digital Visual Interface */
> +#define EXTCON_DISP_VGA 43 /* Video Graphics Array */
> +
> +/* Miscellaneous external connector */
> +#define EXTCON_DOCK 60
> +#define EXTCON_JIG 61
> +#define EXTCON_MECHANICAL 62
> +
> +#define EXTCON_NUM 63
>
> struct extcon_cable;
>
>
cheers,
-roger
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v3] extcon: Modify the id and name of external connector
2015-10-07 7:33 ` Roger Quadros
@ 2015-10-07 7:42 ` Chanwoo Choi
2015-10-07 7:50 ` Roger Quadros
0 siblings, 1 reply; 10+ messages in thread
From: Chanwoo Choi @ 2015-10-07 7:42 UTC (permalink / raw)
To: Roger Quadros, linux-kernel
Cc: k.kozlowski, ckeepax, gregkh, ramakrishna.pallala, patches,
myungjoo.ham
On 2015년 10월 07일 16:33, Roger Quadros wrote:
> On 07/10/15 03:48, Chanwoo Choi wrote:
>> This patch modifies the id and name of external connector with the
>> additional prefix to clarify both attribute and meaning of external
>> connector as following:
>> - EXTCON_CHG_* mean the charger connector.
>> - EXTCON_JACK_* mean the jack connector.
>> - EXTCON_DISP_* mean the display port connector.
>>
>> Following table show the new name of external connector with old name:
>> --------------------------------------------------
>> Old extcon name | New extcon name |
>> --------------------------------------------------
>> EXTCON_TA | EXTCON_CHG_USB_DCP |
>> EXTCON_CHARGE_DOWNSTREAM| EXTCON_CHG_USB_CDP |
>> EXTCON_FAST_CHARGER | EXTCON_CHG_USB_DCP_FAST|
>> EXTCON_SLOW_CHARGER | EXTCON_CHG_USB_DCP_SLOW|
>
> I still have some concerns regarding the name DCP_FAST and DCP_SLOW.
> If I see the charger IC drivers that use FAST & SLOW, they either call it
> APPLE_500MA/1A or SPECIAL_500MA/1A.
Right.
>
> They are probably not compliant with USB DCP specification and hence
> the special name.
>
> So, should we call them EXTCON_CHG_USB_SPECIAL_FAST/SLOW instead?
I don't prefer to use "SPECIAL" adjective for connector name.
I prefer following name at the first patch.
- EXTCON_CHG_USB_FAST
- EXTCON_CHG_USB_SLOW
>
>> --------------------------------------------------
>> EXTCON_MICROPHONE | EXTCON_JACK_MICROPHONE |
>> EXTCON_HEADPHONE | EXTCON_JACK_HEADPHONE |
>> EXTCON_LINE_IN | EXTCON_JACK_LINE_IN |
>> EXTCON_LINE_OUT | EXTCON_JACK_LINE_OUT |
>> EXTCON_VIDEO_IN | EXTCON_JACK_VIDEO_IN |
>> EXTCON_VIDEO_OUT | EXTCON_JACK_VIDEO_OUT |
>> EXTCON_SPDIF_IN | EXTCON_JACK_SPDIF_IN |
>> EXTCON_SPDIF_OUT | EXTCON_JACK_SPDIF_OUT |
>> --------------------------------------------------
>> EXTCON_HMDI | EXTCON_DISP_HDMI |
>> EXTCON_MHL | EXTCON_DISP_MHL |
>> EXTCON_DVI | EXTCON_DISP_DVI |
>> EXTCON_VGA | EXTCON_DISP_VGA |
>> --------------------------------------------------
>>
>> And, when altering the name of USB charger connector, EXTCON refers to the
>> "Battery Charging v1.2 Spec and Adopters Agreement"[1] to use the standard
>> name of USB charging port as following. Following name of USB charging port
>> are already used in power_supply subsystem. We chan check it on patch[2].
>> - EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
>> - EXTCON_CHG_USB_CDP /* Charging Downstream Port */
>> - EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */
>>
>> [1] www.usb.org/developers/docs/devclass_docs/BCv1.2_070312.zip
>> [2] commit 85efc8a18ced ("power_supply: Add types for USB chargers")
>>
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> [ckeepax: For the Arizona changes]
>> Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
>> ---
>> Changes from v2:
>> (https://lkml.org/lkml/2015/10/6/239)
>> - Remove the EXTCON_CHG_USB type to remove the possible confusion according to
>> Roger's comment and drop patch2 about EXTCON_CHG_USB.
>> - Fix the warning issue provided by scripts/checkpatch.pl
>>
>> Changes from v1:
>> (https://lkml.org/lkml/2015/10/3/304)
>> - Add acked tag by Charles Keepax for arizona changes
>> - Modify the name of USB charger connector as following:
>> : EXTCON_CHG_USB_FAST -> EXTCON_CHG_USB_DCP_FAST
>> : EXTCON_CHG_USB_SLOW -> EXTCON_CHG_USB_DCP_SLOW
>> - Add the missing EXTCON_CHG_USB_ACA charger connector
>> - Add one more patch to support the EXTCON_CHG_USB when SDP port is connected
>> or not
>>
>> drivers/extcon/extcon-arizona.c | 18 ++++++------
>> drivers/extcon/extcon-axp288.c | 12 ++++----
>> drivers/extcon/extcon-max14577.c | 17 +++++------
>> drivers/extcon/extcon-max77693.c | 32 +++++++++++----------
>> drivers/extcon/extcon-max77843.c | 27 ++++++++++--------
>> drivers/extcon/extcon-max8997.c | 21 +++++++-------
>> drivers/extcon/extcon-rt8973a.c | 4 +--
>> drivers/extcon/extcon-sm5502.c | 4 +--
>> drivers/extcon/extcon.c | 60 ++++++++++++++++++++-------------------
>> include/linux/extcon.h | 61 +++++++++++++++++++++++-----------------
>> 10 files changed, 137 insertions(+), 119 deletions(-)
>>
>> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
>> index a1ab0a56b798..e4890dd4fefd 100644
>> --- a/drivers/extcon/extcon-arizona.c
>> +++ b/drivers/extcon/extcon-arizona.c
>> @@ -137,9 +137,9 @@ static const int arizona_micd_levels[] = {
>>
>> static const unsigned int arizona_cable[] = {
>> EXTCON_MECHANICAL,
>> - EXTCON_MICROPHONE,
>> - EXTCON_HEADPHONE,
>> - EXTCON_LINE_OUT,
>> + EXTCON_JACK_MICROPHONE,
>> + EXTCON_JACK_HEADPHONE,
>> + EXTCON_JACK_LINE_OUT,
>> EXTCON_NONE,
>> };
>>
>> @@ -600,7 +600,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>> struct arizona_extcon_info *info = data;
>> struct arizona *arizona = info->arizona;
>> int id_gpio = arizona->pdata.hpdet_id_gpio;
>> - unsigned int report = EXTCON_HEADPHONE;
>> + unsigned int report = EXTCON_JACK_HEADPHONE;
>> int ret, reading;
>> bool mic = false;
>>
>> @@ -645,9 +645,9 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>>
>> /* Report high impedence cables as line outputs */
>> if (reading >= 5000)
>> - report = EXTCON_LINE_OUT;
>> + report = EXTCON_JACK_LINE_OUT;
>> else
>> - report = EXTCON_HEADPHONE;
>> + report = EXTCON_JACK_HEADPHONE;
>>
>> ret = extcon_set_cable_state_(info->edev, report, true);
>> if (ret != 0)
>> @@ -732,7 +732,7 @@ err:
>> ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
>>
>> /* Just report headphone */
>> - ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true);
>> + ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true);
>> if (ret != 0)
>> dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
>>
>> @@ -789,7 +789,7 @@ err:
>> ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
>>
>> /* Just report headphone */
>> - ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true);
>> + ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true);
>> if (ret != 0)
>> dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
>>
>> @@ -915,7 +915,7 @@ static void arizona_micd_detect(struct work_struct *work)
>> arizona_identify_headphone(info);
>>
>> ret = extcon_set_cable_state_(info->edev,
>> - EXTCON_MICROPHONE, true);
>> + EXTCON_JACK_MICROPHONE, true);
>> if (ret != 0)
>> dev_err(arizona->dev, "Headset report failed: %d\n",
>> ret);
>> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
>> index 9668d6a94e38..14e748051288 100644
>> --- a/drivers/extcon/extcon-axp288.c
>> +++ b/drivers/extcon/extcon-axp288.c
>> @@ -102,9 +102,9 @@ enum axp288_extcon_irq {
>> };
>>
>> static const unsigned int axp288_extcon_cables[] = {
>> - EXTCON_SLOW_CHARGER,
>> - EXTCON_CHARGE_DOWNSTREAM,
>> - EXTCON_FAST_CHARGER,
>> + EXTCON_CHG_USB_DCP_SLOW,
>> + EXTCON_CHG_USB_CDP,
>> + EXTCON_CHG_USB_DCP_FAST,
>> EXTCON_NONE,
>> };
>>
>> @@ -192,18 +192,18 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
>> dev_dbg(info->dev, "sdp cable is connecetd\n");
>
> as per comment this seems to be SDP.
This patch modifies the only connector name.
If some comment should be fixed, we need to implement on separate patch.
>
>> notify_otg = true;
>> notify_charger = true;
>> - cable = EXTCON_SLOW_CHARGER;
>> + cable = EXTCON_CHG_USB_DCP_SLOW;
>
> So shouldn't this be cable = EXTCON_USB?
No, EXTCON_CHG_USB_*_SLOW is right.
>
>> break;
>> case DET_STAT_CDP:
>> dev_dbg(info->dev, "cdp cable is connecetd\n");
>> notify_otg = true;
>> notify_charger = true;
>> - cable = EXTCON_CHARGE_DOWNSTREAM;
>> + cable = EXTCON_CHG_USB_CDP;
>> break;
>> case DET_STAT_DCP:
>> dev_dbg(info->dev, "dcp cable is connecetd\n");
>
> This looks like standard DCP.
ditto. Need the separate patch to fix it.
>
>> notify_charger = true;
>> - cable = EXTCON_FAST_CHARGER;
>> + cable = EXTCON_CHG_USB_DCP_FAST;
>
> Shouldn't this be cable = EXTCON_CHG_USB_DCP?
No, EXTCON_CHG_USB_*_FAST is right.
>
>> break;
>> default:
>> dev_warn(info->dev,
>> diff --git a/drivers/extcon/extcon-max14577.c b/drivers/extcon/extcon-max14577.c
>> index df0659d98e5a..15c2dcc73fc6 100644
>> --- a/drivers/extcon/extcon-max14577.c
>> +++ b/drivers/extcon/extcon-max14577.c
>> @@ -150,10 +150,10 @@ enum max14577_muic_acc_type {
>>
>> static const unsigned int max14577_extcon_cable[] = {
>> EXTCON_USB,
>> - EXTCON_TA,
>> - EXTCON_FAST_CHARGER,
>> - EXTCON_SLOW_CHARGER,
>> - EXTCON_CHARGE_DOWNSTREAM,
>> + EXTCON_CHG_USB_DCP,
>> + EXTCON_CHG_USB_DCP_FAST,
>> + EXTCON_CHG_USB_DCP_SLOW,
>> + EXTCON_CHG_USB_CDP,
>> EXTCON_JIG,
>> EXTCON_NONE,
>> };
>> @@ -456,18 +456,19 @@ static int max14577_muic_chg_handler(struct max14577_muic_info *info)
>> extcon_set_cable_state_(info->edev, EXTCON_USB, attached);
>> break;
>> case MAX14577_CHARGER_TYPE_DEDICATED_CHG:
>> - extcon_set_cable_state_(info->edev, EXTCON_TA, attached);
>> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP,
>> + attached);
>> break;
>> case MAX14577_CHARGER_TYPE_DOWNSTREAM_PORT:
>> - extcon_set_cable_state_(info->edev, EXTCON_CHARGE_DOWNSTREAM,
>> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_CDP,
>> attached);
>> break;
>> case MAX14577_CHARGER_TYPE_SPECIAL_500MA:
>> - extcon_set_cable_state_(info->edev, EXTCON_SLOW_CHARGER,
>> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_SLOW,
>> attached);
>> break;
>> case MAX14577_CHARGER_TYPE_SPECIAL_1A:
>> - extcon_set_cable_state_(info->edev, EXTCON_FAST_CHARGER,
>> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_FAST,
>> attached);
>> break;
>> case MAX14577_CHARGER_TYPE_NONE:
>> diff --git a/drivers/extcon/extcon-max77693.c b/drivers/extcon/extcon-max77693.c
>> index 35b9e118b2fb..794199920070 100644
>> --- a/drivers/extcon/extcon-max77693.c
>> +++ b/drivers/extcon/extcon-max77693.c
>> @@ -204,11 +204,11 @@ enum max77693_muic_acc_type {
>> static const unsigned int max77693_extcon_cable[] = {
>> EXTCON_USB,
>> EXTCON_USB_HOST,
>> - EXTCON_TA,
>> - EXTCON_FAST_CHARGER,
>> - EXTCON_SLOW_CHARGER,
>> - EXTCON_CHARGE_DOWNSTREAM,
>> - EXTCON_MHL,
>> + EXTCON_CHG_USB_DCP,
>> + EXTCON_CHG_USB_DCP_FAST,
>> + EXTCON_CHG_USB_DCP_SLOW,
>> + EXTCON_CHG_USB_CDP,
>> + EXTCON_DISP_MHL,
>> EXTCON_JIG,
>> EXTCON_DOCK,
>> EXTCON_NONE,
>> @@ -505,7 +505,7 @@ static int max77693_muic_dock_handler(struct max77693_muic_info *info,
>> return ret;
>>
>> extcon_set_cable_state_(info->edev, EXTCON_DOCK, attached);
>> - extcon_set_cable_state_(info->edev, EXTCON_MHL, attached);
>> + extcon_set_cable_state_(info->edev, EXTCON_DISP_MHL, attached);
>> goto out;
>> case MAX77693_MUIC_ADC_AUDIO_MODE_REMOTE: /* Dock-Desk */
>> dock_id = EXTCON_DOCK;
>> @@ -605,7 +605,7 @@ static int max77693_muic_adc_ground_handler(struct max77693_muic_info *info)
>> case MAX77693_MUIC_GND_MHL:
>> case MAX77693_MUIC_GND_MHL_VB:
>> /* MHL or MHL with USB/TA cable */
>> - extcon_set_cable_state_(info->edev, EXTCON_MHL, attached);
>> + extcon_set_cable_state_(info->edev, EXTCON_DISP_MHL, attached);
>> break;
>> default:
>> dev_err(info->dev, "failed to detect %s cable of gnd type\n",
>> @@ -801,10 +801,11 @@ static int max77693_muic_chg_handler(struct max77693_muic_info *info)
>> * - Support charging through micro-usb port without
>> * data connection
>> */
>> - extcon_set_cable_state_(info->edev, EXTCON_TA, attached);
>> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP,
>> + attached);
>> if (!cable_attached)
>> - extcon_set_cable_state_(info->edev, EXTCON_MHL,
>> - cable_attached);
>> + extcon_set_cable_state_(info->edev,
>> + EXTCON_DISP_MHL, cable_attached);
>> break;
>> }
>>
>> @@ -862,7 +863,7 @@ static int max77693_muic_chg_handler(struct max77693_muic_info *info)
>>
>> extcon_set_cable_state_(info->edev, EXTCON_DOCK,
>> attached);
>> - extcon_set_cable_state_(info->edev, EXTCON_MHL,
>> + extcon_set_cable_state_(info->edev, EXTCON_DISP_MHL,
>> attached);
>> break;
>> }
>> @@ -901,20 +902,21 @@ static int max77693_muic_chg_handler(struct max77693_muic_info *info)
>> break;
>> case MAX77693_CHARGER_TYPE_DEDICATED_CHG:
>> /* Only TA cable */
>> - extcon_set_cable_state_(info->edev, EXTCON_TA, attached);
>> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP,
>> + attached);
>> break;
>> }
>> break;
>> case MAX77693_CHARGER_TYPE_DOWNSTREAM_PORT:
>> - extcon_set_cable_state_(info->edev, EXTCON_CHARGE_DOWNSTREAM,
>> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_CDP,
>> attached);
>> break;
>> case MAX77693_CHARGER_TYPE_APPLE_500MA:
>> - extcon_set_cable_state_(info->edev, EXTCON_SLOW_CHARGER,
>> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_SLOW,
>> attached);
>
> Here it says APPLE_500MA. Looks like it is not same as USB DCP.
> So probably USB_DCP_SLOW is a wrong name for it.
>
> So EXTCON_CHG_USB_SPECIAL_SLOW?
If proper name is fixed, I'll modify the name on all extcon drivers.
>
>> break;
>> case MAX77693_CHARGER_TYPE_APPLE_1A_2A:
>> - extcon_set_cable_state_(info->edev, EXTCON_FAST_CHARGER,
>> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_FAST,
>> attached);
>
> EXTCON_CHG_USB_SPECIAL_FAST?
>
>> break;
>> case MAX77693_CHARGER_TYPE_DEAD_BATTERY:
>> diff --git a/drivers/extcon/extcon-max77843.c b/drivers/extcon/extcon-max77843.c
>> index fdd928542c19..97aebe5d67a6 100644
>> --- a/drivers/extcon/extcon-max77843.c
>> +++ b/drivers/extcon/extcon-max77843.c
>> @@ -122,11 +122,11 @@ enum max77843_muic_charger_type {
>> static const unsigned int max77843_extcon_cable[] = {
>> EXTCON_USB,
>> EXTCON_USB_HOST,
>> - EXTCON_TA,
>> - EXTCON_CHARGE_DOWNSTREAM,
>> - EXTCON_FAST_CHARGER,
>> - EXTCON_SLOW_CHARGER,
>> - EXTCON_MHL,
>> + EXTCON_CHG_USB_DCP,
>> + EXTCON_CHG_USB_CDP,
>> + EXTCON_CHG_USB_DCP_FAST,
>> + EXTCON_CHG_USB_DCP_SLOW,
>> + EXTCON_DISP_MHL,
>> EXTCON_JIG,
>> EXTCON_NONE,
>> };
>> @@ -355,7 +355,7 @@ static int max77843_muic_adc_gnd_handler(struct max77843_muic_info *info)
>> if (ret < 0)
>> return ret;
>>
>> - extcon_set_cable_state_(info->edev, EXTCON_MHL, attached);
>> + extcon_set_cable_state_(info->edev, EXTCON_DISP_MHL, attached);
>> break;
>> default:
>> dev_err(info->dev, "failed to detect %s accessory(gnd:0x%x)\n",
>> @@ -494,7 +494,7 @@ static int max77843_muic_chg_handler(struct max77843_muic_info *info)
>> if (ret < 0)
>> return ret;
>>
>> - extcon_set_cable_state_(info->edev, EXTCON_CHARGE_DOWNSTREAM,
>> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_CDP,
>> attached);
>> break;
>> case MAX77843_MUIC_CHG_DEDICATED:
>> @@ -504,7 +504,8 @@ static int max77843_muic_chg_handler(struct max77843_muic_info *info)
>> if (ret < 0)
>> return ret;
>>
>> - extcon_set_cable_state_(info->edev, EXTCON_TA, attached);
>> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP,
>> + attached);
>> break;
>> case MAX77843_MUIC_CHG_SPECIAL_500MA:
>> ret = max77843_muic_set_path(info,
>> @@ -513,7 +514,7 @@ static int max77843_muic_chg_handler(struct max77843_muic_info *info)
>> if (ret < 0)
>> return ret;
>>
>> - extcon_set_cable_state_(info->edev, EXTCON_SLOW_CHARGER,
>> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_SLOW,
>> attached);
>
> Here it is SPECIAL_500MA. Again this might not be compliant to USB DCP.
> EXTCON_CHG_USB_SPECIAL_SLOW?
>
>> break;
>> case MAX77843_MUIC_CHG_SPECIAL_1A:
>> @@ -523,7 +524,7 @@ static int max77843_muic_chg_handler(struct max77843_muic_info *info)
>> if (ret < 0)
>> return ret;
>>
>> - extcon_set_cable_state_(info->edev, EXTCON_FAST_CHARGER,
>> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_FAST,
>> attached);
>
> EXTCON_CHG_USB_SPECIAL_FAST?
>
>> break;
>> case MAX77843_MUIC_CHG_GND:
>> @@ -532,9 +533,11 @@ static int max77843_muic_chg_handler(struct max77843_muic_info *info)
>>
>> /* Charger cable on MHL accessory is attach or detach */
>> if (gnd_type == MAX77843_MUIC_GND_MHL_VB)
>> - extcon_set_cable_state_(info->edev, EXTCON_TA, true);
>> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP,
>> + true);
>> else if (gnd_type == MAX77843_MUIC_GND_MHL)
>> - extcon_set_cable_state_(info->edev, EXTCON_TA, false);
>> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP,
>> + false);
>> break;
>> case MAX77843_MUIC_CHG_NONE:
>> break;
>> diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
>> index 7b1ef200b121..8dbec5febdb6 100644
>> --- a/drivers/extcon/extcon-max8997.c
>> +++ b/drivers/extcon/extcon-max8997.c
>> @@ -148,11 +148,11 @@ struct max8997_muic_info {
>> static const unsigned int max8997_extcon_cable[] = {
>> EXTCON_USB,
>> EXTCON_USB_HOST,
>> - EXTCON_TA,
>> - EXTCON_FAST_CHARGER,
>> - EXTCON_SLOW_CHARGER,
>> - EXTCON_CHARGE_DOWNSTREAM,
>> - EXTCON_MHL,
>> + EXTCON_CHG_USB_DCP,
>> + EXTCON_CHG_USB_DCP_FAST,
>> + EXTCON_CHG_USB_DCP_SLOW,
>> + EXTCON_CHG_USB_CDP,
>> + EXTCON_DISP_MHL,
>> EXTCON_DOCK,
>> EXTCON_JIG,
>> EXTCON_NONE,
>> @@ -403,7 +403,7 @@ static int max8997_muic_adc_handler(struct max8997_muic_info *info)
>> return ret;
>> break;
>> case MAX8997_MUIC_ADC_MHL:
>> - extcon_set_cable_state_(info->edev, EXTCON_MHL, attached);
>> + extcon_set_cable_state_(info->edev, EXTCON_DISP_MHL, attached);
>> break;
>> case MAX8997_MUIC_ADC_FACTORY_MODE_USB_OFF:
>> case MAX8997_MUIC_ADC_FACTORY_MODE_USB_ON:
>> @@ -486,18 +486,19 @@ static int max8997_muic_chg_handler(struct max8997_muic_info *info)
>> }
>> break;
>> case MAX8997_CHARGER_TYPE_DOWNSTREAM_PORT:
>> - extcon_set_cable_state_(info->edev, EXTCON_CHARGE_DOWNSTREAM,
>> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_CDP,
>> attached);
>> break;
>> case MAX8997_CHARGER_TYPE_DEDICATED_CHG:
>> - extcon_set_cable_state_(info->edev, EXTCON_TA, attached);
>> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP,
>> + attached);
>> break;
>> case MAX8997_CHARGER_TYPE_500MA:
>> - extcon_set_cable_state_(info->edev, EXTCON_SLOW_CHARGER,
>> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_SLOW,
>> attached);
>
> EXTCON_CHG_USB_SPECIAL_SLOW?
>
>> break;
>> case MAX8997_CHARGER_TYPE_1A:
>> - extcon_set_cable_state_(info->edev, EXTCON_FAST_CHARGER,
>> + extcon_set_cable_state_(info->edev, EXTCON_CHG_USB_DCP_FAST,
>> attached);
>
> EXTCON_CHG_USB_SPECIAL_FAST?
>
>> break;
>> default:
>> diff --git a/drivers/extcon/extcon-rt8973a.c b/drivers/extcon/extcon-rt8973a.c
>> index 1bc3737ea01c..36bf1d63791c 100644
>> --- a/drivers/extcon/extcon-rt8973a.c
>> +++ b/drivers/extcon/extcon-rt8973a.c
>> @@ -93,7 +93,7 @@ static struct reg_data rt8973a_reg_data[] = {
>> static const unsigned int rt8973a_extcon_cable[] = {
>> EXTCON_USB,
>> EXTCON_USB_HOST,
>> - EXTCON_TA,
>> + EXTCON_CHG_USB_DCP,
>> EXTCON_JIG,
>> EXTCON_NONE,
>> };
>> @@ -333,7 +333,7 @@ static int rt8973a_muic_cable_handler(struct rt8973a_muic_info *info,
>> con_sw = DM_DP_SWITCH_USB;
>> break;
>> case RT8973A_MUIC_ADC_TA:
>> - id = EXTCON_TA;
>> + id = EXTCON_CHG_USB_DCP;
>> con_sw = DM_DP_SWITCH_OPEN;
>> break;
>> case RT8973A_MUIC_ADC_FACTORY_MODE_BOOT_OFF_USB:
>> diff --git a/drivers/extcon/extcon-sm5502.c b/drivers/extcon/extcon-sm5502.c
>> index 2945091bfd0e..7aac3cc7efd7 100644
>> --- a/drivers/extcon/extcon-sm5502.c
>> +++ b/drivers/extcon/extcon-sm5502.c
>> @@ -95,7 +95,7 @@ static struct reg_data sm5502_reg_data[] = {
>> static const unsigned int sm5502_extcon_cable[] = {
>> EXTCON_USB,
>> EXTCON_USB_HOST,
>> - EXTCON_TA,
>> + EXTCON_CHG_USB_DCP,
>> EXTCON_NONE,
>> };
>>
>> @@ -389,7 +389,7 @@ static int sm5502_muic_cable_handler(struct sm5502_muic_info *info,
>> vbus_sw = VBUSIN_SWITCH_VBUSOUT_WITH_USB;
>> break;
>> case SM5502_MUIC_ADC_OPEN_TA:
>> - id = EXTCON_TA;
>> + id = EXTCON_CHG_USB_DCP;
>> con_sw = DM_DP_SWITCH_OPEN;
>> vbus_sw = VBUSIN_SWITCH_VBUSOUT;
>> break;
>> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
>> index 8dd0af1d50bc..3caa993259fb 100644
>> --- a/drivers/extcon/extcon.c
>> +++ b/drivers/extcon/extcon.c
>> @@ -39,37 +39,39 @@
>> #define CABLE_NAME_MAX 30
>>
>> static const char *extcon_name[] = {
>> - [EXTCON_NONE] = "NONE",
>> + [EXTCON_NONE] = "EXTCON_NONE",
>>
>> /* USB external connector */
>> - [EXTCON_USB] = "USB",
>> - [EXTCON_USB_HOST] = "USB-HOST",
>> -
>> - /* Charger external connector */
>> - [EXTCON_TA] = "TA",
>> - [EXTCON_FAST_CHARGER] = "FAST-CHARGER",
>> - [EXTCON_SLOW_CHARGER] = "SLOW-CHARGER",
>> - [EXTCON_CHARGE_DOWNSTREAM] = "CHARGE-DOWNSTREAM",
>> -
>> - /* Audio/Video external connector */
>> - [EXTCON_LINE_IN] = "LINE-IN",
>> - [EXTCON_LINE_OUT] = "LINE-OUT",
>> - [EXTCON_MICROPHONE] = "MICROPHONE",
>> - [EXTCON_HEADPHONE] = "HEADPHONE",
>> -
>> - [EXTCON_HDMI] = "HDMI",
>> - [EXTCON_MHL] = "MHL",
>> - [EXTCON_DVI] = "DVI",
>> - [EXTCON_VGA] = "VGA",
>> - [EXTCON_SPDIF_IN] = "SPDIF-IN",
>> - [EXTCON_SPDIF_OUT] = "SPDIF-OUT",
>> - [EXTCON_VIDEO_IN] = "VIDEO-IN",
>> - [EXTCON_VIDEO_OUT] = "VIDEO-OUT",
>> -
>> - /* Etc external connector */
>> - [EXTCON_DOCK] = "DOCK",
>> - [EXTCON_JIG] = "JIG",
>> - [EXTCON_MECHANICAL] = "MECHANICAL",
>> + [EXTCON_USB] = "EXTCON_USB",
>> + [EXTCON_USB_HOST] = "EXTCON_USB_HOST",
>> +
>> + /* Charging external connector */
>> + [EXTCON_CHG_USB_DCP] = "EXTCON_CHG_USB_DCP",
>> + [EXTCON_CHG_USB_CDP] = "EXTCON_CHG_USB_CDP",
>> + [EXTCON_CHG_USB_ACA] = "EXTCON_CHG_USB_ACA",
>> + [EXTCON_CHG_USB_DCP_FAST] = "EXTCON_CHG_USB_DCP_FAST",
>> + [EXTCON_CHG_USB_DCP_SLOW] = "EXTCON_CHG_USB_DCP_SLOW",
>> +
>> + /* Jack external connector */
>> + [EXTCON_JACK_MICROPHONE] = "EXTCON_JACK_MICROPHONE",
>> + [EXTCON_JACK_HEADPHONE] = "EXTCON_JACK_HEADPHONE",
>> + [EXTCON_JACK_LINE_IN] = "EXTCON_JACK_LINE_IN",
>> + [EXTCON_JACK_LINE_OUT] = "EXTCON_JACK_LINE_OUT",
>> + [EXTCON_JACK_VIDEO_IN] = "EXTCON_JACK_VIDEO_IN",
>> + [EXTCON_JACK_VIDEO_OUT] = "EXTCON_JACK_VIDEO_OUT",
>> + [EXTCON_JACK_SPDIF_IN] = "EXTCON_JACK_SPDIF_IN",
>> + [EXTCON_JACK_SPDIF_OUT] = "EXTCON_JACK_SPDIF_OUT",
>> +
>> + /* Display external connector */
>> + [EXTCON_DISP_HDMI] = "EXTCON_DISP_HDMI",
>> + [EXTCON_DISP_MHL] = "EXTCON_DISP_MHL",
>> + [EXTCON_DISP_DVI] = "EXTCON_DISP_DVI",
>> + [EXTCON_DISP_VGA] = "EXTCON_DISP_VGA",
>> +
>> + /* Miscellaneous external connector */
>> + [EXTCON_DOCK] = "EXTCON_DOCK",
>> + [EXTCON_JIG] = "EXTCON_JIG",
>> + [EXTCON_MECHANICAL] = "EXTCON_MECHANICAL",
>>
>> NULL,
>> };
>> diff --git a/include/linux/extcon.h b/include/linux/extcon.h
>> index c0f8c4fc5d45..2625a89c2809 100644
>> --- a/include/linux/extcon.h
>> +++ b/include/linux/extcon.h
>> @@ -31,32 +31,41 @@
>> /*
>> * Define the unique id of supported external connectors
>> */
>> -#define EXTCON_NONE 0
>> -
>> -#define EXTCON_USB 1 /* USB connector */
>> -#define EXTCON_USB_HOST 2
>> -
>> -#define EXTCON_TA 3 /* Charger connector */
>> -#define EXTCON_FAST_CHARGER 4
>> -#define EXTCON_SLOW_CHARGER 5
>> -#define EXTCON_CHARGE_DOWNSTREAM 6
>> -
>> -#define EXTCON_LINE_IN 7 /* Audio/Video connector */
>> -#define EXTCON_LINE_OUT 8
>> -#define EXTCON_MICROPHONE 9
>> -#define EXTCON_HEADPHONE 10
>> -#define EXTCON_HDMI 11
>> -#define EXTCON_MHL 12
>> -#define EXTCON_DVI 13
>> -#define EXTCON_VGA 14
>> -#define EXTCON_SPDIF_IN 15
>> -#define EXTCON_SPDIF_OUT 16
>> -#define EXTCON_VIDEO_IN 17
>> -#define EXTCON_VIDEO_OUT 18
>> -
>> -#define EXTCON_DOCK 19 /* Misc connector */
>> -#define EXTCON_JIG 20
>> -#define EXTCON_MECHANICAL 21
>> +#define EXTCON_NONE 0
>> +
>> +/* USB external connector */
>> +#define EXTCON_USB 1 /* Standard Downstream Port */
>> +#define EXTCON_USB_HOST 2
>> +
>> +/* Charging external connector */
>> +#define EXTCON_CHG_USB_DCP 5 /* Dedicated Charging Port */
>> +#define EXTCON_CHG_USB_CDP 6 /* Charging Downstream Port */
>> +#define EXTCON_CHG_USB_ACA 7 /* Accessory Charger Adapter */
>> +#define EXTCON_CHG_USB_DCP_FAST 8
>> +#define EXTCON_CHG_USB_DCP_SLOW 9
>> +
>> +/* Jack external connector */
>> +#define EXTCON_JACK_MICROPHONE 20
>> +#define EXTCON_JACK_HEADPHONE 21
>> +#define EXTCON_JACK_LINE_IN 22
>> +#define EXTCON_JACK_LINE_OUT 23
>> +#define EXTCON_JACK_VIDEO_IN 24
>> +#define EXTCON_JACK_VIDEO_OUT 25
>> +#define EXTCON_JACK_SPDIF_IN 26 /* Sony Philips Digital InterFace */
>> +#define EXTCON_JACK_SPDIF_OUT 27
>> +
>> +/* Display external connector */
>> +#define EXTCON_DISP_HDMI 40 /* High-Definition Multimedia Interface */
>> +#define EXTCON_DISP_MHL 41 /* Mobile High-Definition Link */
>> +#define EXTCON_DISP_DVI 42 /* Digital Visual Interface */
>> +#define EXTCON_DISP_VGA 43 /* Video Graphics Array */
>> +
>> +/* Miscellaneous external connector */
>> +#define EXTCON_DOCK 60
>> +#define EXTCON_JIG 61
>> +#define EXTCON_MECHANICAL 62
>> +
>> +#define EXTCON_NUM 63
>>
>> struct extcon_cable;
>>
Thanks,
Chanwoo Choi
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v3] extcon: Modify the id and name of external connector
2015-10-07 7:42 ` Chanwoo Choi
@ 2015-10-07 7:50 ` Roger Quadros
2015-10-07 7:59 ` Chanwoo Choi
0 siblings, 1 reply; 10+ messages in thread
From: Roger Quadros @ 2015-10-07 7:50 UTC (permalink / raw)
To: Chanwoo Choi, linux-kernel
Cc: k.kozlowski, ckeepax, gregkh, ramakrishna.pallala, patches,
myungjoo.ham
On 07/10/15 10:42, Chanwoo Choi wrote:
> On 2015년 10월 07일 16:33, Roger Quadros wrote:
>> On 07/10/15 03:48, Chanwoo Choi wrote:
>>> This patch modifies the id and name of external connector with the
>>> additional prefix to clarify both attribute and meaning of external
>>> connector as following:
>>> - EXTCON_CHG_* mean the charger connector.
>>> - EXTCON_JACK_* mean the jack connector.
>>> - EXTCON_DISP_* mean the display port connector.
>>>
>>> Following table show the new name of external connector with old name:
>>> --------------------------------------------------
>>> Old extcon name | New extcon name |
>>> --------------------------------------------------
>>> EXTCON_TA | EXTCON_CHG_USB_DCP |
>>> EXTCON_CHARGE_DOWNSTREAM| EXTCON_CHG_USB_CDP |
>>> EXTCON_FAST_CHARGER | EXTCON_CHG_USB_DCP_FAST|
>>> EXTCON_SLOW_CHARGER | EXTCON_CHG_USB_DCP_SLOW|
>>
>> I still have some concerns regarding the name DCP_FAST and DCP_SLOW.
>> If I see the charger IC drivers that use FAST & SLOW, they either call it
>> APPLE_500MA/1A or SPECIAL_500MA/1A.
>
> Right.
>
>>
>> They are probably not compliant with USB DCP specification and hence
>> the special name.
>>
>> So, should we call them EXTCON_CHG_USB_SPECIAL_FAST/SLOW instead?
>
> I don't prefer to use "SPECIAL" adjective for connector name.
> I prefer following name at the first patch.
> - EXTCON_CHG_USB_FAST
> - EXTCON_CHG_USB_SLOW
OK, that is fine with me.
>
>>
>>> --------------------------------------------------
>>> EXTCON_MICROPHONE | EXTCON_JACK_MICROPHONE |
>>> EXTCON_HEADPHONE | EXTCON_JACK_HEADPHONE |
>>> EXTCON_LINE_IN | EXTCON_JACK_LINE_IN |
>>> EXTCON_LINE_OUT | EXTCON_JACK_LINE_OUT |
>>> EXTCON_VIDEO_IN | EXTCON_JACK_VIDEO_IN |
>>> EXTCON_VIDEO_OUT | EXTCON_JACK_VIDEO_OUT |
>>> EXTCON_SPDIF_IN | EXTCON_JACK_SPDIF_IN |
>>> EXTCON_SPDIF_OUT | EXTCON_JACK_SPDIF_OUT |
>>> --------------------------------------------------
>>> EXTCON_HMDI | EXTCON_DISP_HDMI |
>>> EXTCON_MHL | EXTCON_DISP_MHL |
>>> EXTCON_DVI | EXTCON_DISP_DVI |
>>> EXTCON_VGA | EXTCON_DISP_VGA |
>>> --------------------------------------------------
>>>
>>> And, when altering the name of USB charger connector, EXTCON refers to the
>>> "Battery Charging v1.2 Spec and Adopters Agreement"[1] to use the standard
>>> name of USB charging port as following. Following name of USB charging port
>>> are already used in power_supply subsystem. We chan check it on patch[2].
>>> - EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
>>> - EXTCON_CHG_USB_CDP /* Charging Downstream Port */
>>> - EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */
>>>
>>> [1] www.usb.org/developers/docs/devclass_docs/BCv1.2_070312.zip
>>> [2] commit 85efc8a18ced ("power_supply: Add types for USB chargers")
>>>
>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>>> [ckeepax: For the Arizona changes]
>>> Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
>>> ---
>>> Changes from v2:
>>> (https://lkml.org/lkml/2015/10/6/239)
>>> - Remove the EXTCON_CHG_USB type to remove the possible confusion according to
>>> Roger's comment and drop patch2 about EXTCON_CHG_USB.
>>> - Fix the warning issue provided by scripts/checkpatch.pl
>>>
>>> Changes from v1:
>>> (https://lkml.org/lkml/2015/10/3/304)
>>> - Add acked tag by Charles Keepax for arizona changes
>>> - Modify the name of USB charger connector as following:
>>> : EXTCON_CHG_USB_FAST -> EXTCON_CHG_USB_DCP_FAST
>>> : EXTCON_CHG_USB_SLOW -> EXTCON_CHG_USB_DCP_SLOW
>>> - Add the missing EXTCON_CHG_USB_ACA charger connector
>>> - Add one more patch to support the EXTCON_CHG_USB when SDP port is connected
>>> or not
>>>
>>> drivers/extcon/extcon-arizona.c | 18 ++++++------
>>> drivers/extcon/extcon-axp288.c | 12 ++++----
>>> drivers/extcon/extcon-max14577.c | 17 +++++------
>>> drivers/extcon/extcon-max77693.c | 32 +++++++++++----------
>>> drivers/extcon/extcon-max77843.c | 27 ++++++++++--------
>>> drivers/extcon/extcon-max8997.c | 21 +++++++-------
>>> drivers/extcon/extcon-rt8973a.c | 4 +--
>>> drivers/extcon/extcon-sm5502.c | 4 +--
>>> drivers/extcon/extcon.c | 60 ++++++++++++++++++++-------------------
>>> include/linux/extcon.h | 61 +++++++++++++++++++++++-----------------
>>> 10 files changed, 137 insertions(+), 119 deletions(-)
>>>
>>> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
>>> index a1ab0a56b798..e4890dd4fefd 100644
>>> --- a/drivers/extcon/extcon-arizona.c
>>> +++ b/drivers/extcon/extcon-arizona.c
>>> @@ -137,9 +137,9 @@ static const int arizona_micd_levels[] = {
>>>
>>> static const unsigned int arizona_cable[] = {
>>> EXTCON_MECHANICAL,
>>> - EXTCON_MICROPHONE,
>>> - EXTCON_HEADPHONE,
>>> - EXTCON_LINE_OUT,
>>> + EXTCON_JACK_MICROPHONE,
>>> + EXTCON_JACK_HEADPHONE,
>>> + EXTCON_JACK_LINE_OUT,
>>> EXTCON_NONE,
>>> };
>>>
>>> @@ -600,7 +600,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>>> struct arizona_extcon_info *info = data;
>>> struct arizona *arizona = info->arizona;
>>> int id_gpio = arizona->pdata.hpdet_id_gpio;
>>> - unsigned int report = EXTCON_HEADPHONE;
>>> + unsigned int report = EXTCON_JACK_HEADPHONE;
>>> int ret, reading;
>>> bool mic = false;
>>>
>>> @@ -645,9 +645,9 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>>>
>>> /* Report high impedence cables as line outputs */
>>> if (reading >= 5000)
>>> - report = EXTCON_LINE_OUT;
>>> + report = EXTCON_JACK_LINE_OUT;
>>> else
>>> - report = EXTCON_HEADPHONE;
>>> + report = EXTCON_JACK_HEADPHONE;
>>>
>>> ret = extcon_set_cable_state_(info->edev, report, true);
>>> if (ret != 0)
>>> @@ -732,7 +732,7 @@ err:
>>> ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
>>>
>>> /* Just report headphone */
>>> - ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true);
>>> + ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true);
>>> if (ret != 0)
>>> dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
>>>
>>> @@ -789,7 +789,7 @@ err:
>>> ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
>>>
>>> /* Just report headphone */
>>> - ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true);
>>> + ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true);
>>> if (ret != 0)
>>> dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
>>>
>>> @@ -915,7 +915,7 @@ static void arizona_micd_detect(struct work_struct *work)
>>> arizona_identify_headphone(info);
>>>
>>> ret = extcon_set_cable_state_(info->edev,
>>> - EXTCON_MICROPHONE, true);
>>> + EXTCON_JACK_MICROPHONE, true);
>>> if (ret != 0)
>>> dev_err(arizona->dev, "Headset report failed: %d\n",
>>> ret);
>>> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
>>> index 9668d6a94e38..14e748051288 100644
>>> --- a/drivers/extcon/extcon-axp288.c
>>> +++ b/drivers/extcon/extcon-axp288.c
>>> @@ -102,9 +102,9 @@ enum axp288_extcon_irq {
>>> };
>>>
>>> static const unsigned int axp288_extcon_cables[] = {
>>> - EXTCON_SLOW_CHARGER,
>>> - EXTCON_CHARGE_DOWNSTREAM,
>>> - EXTCON_FAST_CHARGER,
>>> + EXTCON_CHG_USB_DCP_SLOW,
>>> + EXTCON_CHG_USB_CDP,
>>> + EXTCON_CHG_USB_DCP_FAST,
>>> EXTCON_NONE,
>>> };
>>>
>>> @@ -192,18 +192,18 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
>>> dev_dbg(info->dev, "sdp cable is connecetd\n");
>>
>> as per comment this seems to be SDP.
>
> This patch modifies the only connector name.
> If some comment should be fixed, we need to implement on separate patch.
Agreed.
>
>>
>>> notify_otg = true;
>>> notify_charger = true;
>>> - cable = EXTCON_SLOW_CHARGER;
>>> + cable = EXTCON_CHG_USB_DCP_SLOW;
>>
>> So shouldn't this be cable = EXTCON_USB?
>
> No, EXTCON_CHG_USB_*_SLOW is right.
>
>>
>>> break;
>>> case DET_STAT_CDP:
>>> dev_dbg(info->dev, "cdp cable is connecetd\n");
>>> notify_otg = true;
>>> notify_charger = true;
>>> - cable = EXTCON_CHARGE_DOWNSTREAM;
>>> + cable = EXTCON_CHG_USB_CDP;
>>> break;
>>> case DET_STAT_DCP:
>>> dev_dbg(info->dev, "dcp cable is connecetd\n");
>>
>> This looks like standard DCP.
>
> ditto. Need the separate patch to fix it.
Agreed.
>
>>
>>> notify_charger = true;
>>> - cable = EXTCON_FAST_CHARGER;
>>> + cable = EXTCON_CHG_USB_DCP_FAST;
>>
>> Shouldn't this be cable = EXTCON_CHG_USB_DCP?
>
> No, EXTCON_CHG_USB_*_FAST is right.
Why? The case is DET_STAT_DCP.
>
>>
>>> break;
>>> default:
>>> dev_warn(info->dev,
<snip>
cheers,
-roger
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v3] extcon: Modify the id and name of external connector
2015-10-07 7:50 ` Roger Quadros
@ 2015-10-07 7:59 ` Chanwoo Choi
2015-10-07 8:09 ` Chanwoo Choi
0 siblings, 1 reply; 10+ messages in thread
From: Chanwoo Choi @ 2015-10-07 7:59 UTC (permalink / raw)
To: Roger Quadros, linux-kernel
Cc: k.kozlowski, ckeepax, gregkh, ramakrishna.pallala, patches,
myungjoo.ham
On 2015년 10월 07일 16:50, Roger Quadros wrote:
> On 07/10/15 10:42, Chanwoo Choi wrote:
>> On 2015년 10월 07일 16:33, Roger Quadros wrote:
>>> On 07/10/15 03:48, Chanwoo Choi wrote:
>>>> This patch modifies the id and name of external connector with the
>>>> additional prefix to clarify both attribute and meaning of external
>>>> connector as following:
>>>> - EXTCON_CHG_* mean the charger connector.
>>>> - EXTCON_JACK_* mean the jack connector.
>>>> - EXTCON_DISP_* mean the display port connector.
>>>>
>>>> Following table show the new name of external connector with old name:
>>>> --------------------------------------------------
>>>> Old extcon name | New extcon name |
>>>> --------------------------------------------------
>>>> EXTCON_TA | EXTCON_CHG_USB_DCP |
>>>> EXTCON_CHARGE_DOWNSTREAM| EXTCON_CHG_USB_CDP |
>>>> EXTCON_FAST_CHARGER | EXTCON_CHG_USB_DCP_FAST|
>>>> EXTCON_SLOW_CHARGER | EXTCON_CHG_USB_DCP_SLOW|
>>>
>>> I still have some concerns regarding the name DCP_FAST and DCP_SLOW.
>>> If I see the charger IC drivers that use FAST & SLOW, they either call it
>>> APPLE_500MA/1A or SPECIAL_500MA/1A.
>>
>> Right.
>>
>>>
>>> They are probably not compliant with USB DCP specification and hence
>>> the special name.
>>>
>>> So, should we call them EXTCON_CHG_USB_SPECIAL_FAST/SLOW instead?
>>
>> I don't prefer to use "SPECIAL" adjective for connector name.
>> I prefer following name at the first patch.
>> - EXTCON_CHG_USB_FAST
>> - EXTCON_CHG_USB_SLOW
>
> OK, that is fine with me.
>>
>>>
>>>> --------------------------------------------------
>>>> EXTCON_MICROPHONE | EXTCON_JACK_MICROPHONE |
>>>> EXTCON_HEADPHONE | EXTCON_JACK_HEADPHONE |
>>>> EXTCON_LINE_IN | EXTCON_JACK_LINE_IN |
>>>> EXTCON_LINE_OUT | EXTCON_JACK_LINE_OUT |
>>>> EXTCON_VIDEO_IN | EXTCON_JACK_VIDEO_IN |
>>>> EXTCON_VIDEO_OUT | EXTCON_JACK_VIDEO_OUT |
>>>> EXTCON_SPDIF_IN | EXTCON_JACK_SPDIF_IN |
>>>> EXTCON_SPDIF_OUT | EXTCON_JACK_SPDIF_OUT |
>>>> --------------------------------------------------
>>>> EXTCON_HMDI | EXTCON_DISP_HDMI |
>>>> EXTCON_MHL | EXTCON_DISP_MHL |
>>>> EXTCON_DVI | EXTCON_DISP_DVI |
>>>> EXTCON_VGA | EXTCON_DISP_VGA |
>>>> --------------------------------------------------
>>>>
>>>> And, when altering the name of USB charger connector, EXTCON refers to the
>>>> "Battery Charging v1.2 Spec and Adopters Agreement"[1] to use the standard
>>>> name of USB charging port as following. Following name of USB charging port
>>>> are already used in power_supply subsystem. We chan check it on patch[2].
>>>> - EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
>>>> - EXTCON_CHG_USB_CDP /* Charging Downstream Port */
>>>> - EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */
>>>>
>>>> [1] www.usb.org/developers/docs/devclass_docs/BCv1.2_070312.zip
>>>> [2] commit 85efc8a18ced ("power_supply: Add types for USB chargers")
>>>>
>>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>> [ckeepax: For the Arizona changes]
>>>> Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
>>>> ---
>>>> Changes from v2:
>>>> (https://lkml.org/lkml/2015/10/6/239)
>>>> - Remove the EXTCON_CHG_USB type to remove the possible confusion according to
>>>> Roger's comment and drop patch2 about EXTCON_CHG_USB.
>>>> - Fix the warning issue provided by scripts/checkpatch.pl
>>>>
>>>> Changes from v1:
>>>> (https://lkml.org/lkml/2015/10/3/304)
>>>> - Add acked tag by Charles Keepax for arizona changes
>>>> - Modify the name of USB charger connector as following:
>>>> : EXTCON_CHG_USB_FAST -> EXTCON_CHG_USB_DCP_FAST
>>>> : EXTCON_CHG_USB_SLOW -> EXTCON_CHG_USB_DCP_SLOW
>>>> - Add the missing EXTCON_CHG_USB_ACA charger connector
>>>> - Add one more patch to support the EXTCON_CHG_USB when SDP port is connected
>>>> or not
>>>>
>>>> drivers/extcon/extcon-arizona.c | 18 ++++++------
>>>> drivers/extcon/extcon-axp288.c | 12 ++++----
>>>> drivers/extcon/extcon-max14577.c | 17 +++++------
>>>> drivers/extcon/extcon-max77693.c | 32 +++++++++++----------
>>>> drivers/extcon/extcon-max77843.c | 27 ++++++++++--------
>>>> drivers/extcon/extcon-max8997.c | 21 +++++++-------
>>>> drivers/extcon/extcon-rt8973a.c | 4 +--
>>>> drivers/extcon/extcon-sm5502.c | 4 +--
>>>> drivers/extcon/extcon.c | 60 ++++++++++++++++++++-------------------
>>>> include/linux/extcon.h | 61 +++++++++++++++++++++++-----------------
>>>> 10 files changed, 137 insertions(+), 119 deletions(-)
>>>>
>>>> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
>>>> index a1ab0a56b798..e4890dd4fefd 100644
>>>> --- a/drivers/extcon/extcon-arizona.c
>>>> +++ b/drivers/extcon/extcon-arizona.c
>>>> @@ -137,9 +137,9 @@ static const int arizona_micd_levels[] = {
>>>>
>>>> static const unsigned int arizona_cable[] = {
>>>> EXTCON_MECHANICAL,
>>>> - EXTCON_MICROPHONE,
>>>> - EXTCON_HEADPHONE,
>>>> - EXTCON_LINE_OUT,
>>>> + EXTCON_JACK_MICROPHONE,
>>>> + EXTCON_JACK_HEADPHONE,
>>>> + EXTCON_JACK_LINE_OUT,
>>>> EXTCON_NONE,
>>>> };
>>>>
>>>> @@ -600,7 +600,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>>>> struct arizona_extcon_info *info = data;
>>>> struct arizona *arizona = info->arizona;
>>>> int id_gpio = arizona->pdata.hpdet_id_gpio;
>>>> - unsigned int report = EXTCON_HEADPHONE;
>>>> + unsigned int report = EXTCON_JACK_HEADPHONE;
>>>> int ret, reading;
>>>> bool mic = false;
>>>>
>>>> @@ -645,9 +645,9 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>>>>
>>>> /* Report high impedence cables as line outputs */
>>>> if (reading >= 5000)
>>>> - report = EXTCON_LINE_OUT;
>>>> + report = EXTCON_JACK_LINE_OUT;
>>>> else
>>>> - report = EXTCON_HEADPHONE;
>>>> + report = EXTCON_JACK_HEADPHONE;
>>>>
>>>> ret = extcon_set_cable_state_(info->edev, report, true);
>>>> if (ret != 0)
>>>> @@ -732,7 +732,7 @@ err:
>>>> ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
>>>>
>>>> /* Just report headphone */
>>>> - ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true);
>>>> + ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true);
>>>> if (ret != 0)
>>>> dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
>>>>
>>>> @@ -789,7 +789,7 @@ err:
>>>> ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
>>>>
>>>> /* Just report headphone */
>>>> - ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true);
>>>> + ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true);
>>>> if (ret != 0)
>>>> dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
>>>>
>>>> @@ -915,7 +915,7 @@ static void arizona_micd_detect(struct work_struct *work)
>>>> arizona_identify_headphone(info);
>>>>
>>>> ret = extcon_set_cable_state_(info->edev,
>>>> - EXTCON_MICROPHONE, true);
>>>> + EXTCON_JACK_MICROPHONE, true);
>>>> if (ret != 0)
>>>> dev_err(arizona->dev, "Headset report failed: %d\n",
>>>> ret);
>>>> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
>>>> index 9668d6a94e38..14e748051288 100644
>>>> --- a/drivers/extcon/extcon-axp288.c
>>>> +++ b/drivers/extcon/extcon-axp288.c
>>>> @@ -102,9 +102,9 @@ enum axp288_extcon_irq {
>>>> };
>>>>
>>>> static const unsigned int axp288_extcon_cables[] = {
>>>> - EXTCON_SLOW_CHARGER,
>>>> - EXTCON_CHARGE_DOWNSTREAM,
>>>> - EXTCON_FAST_CHARGER,
>>>> + EXTCON_CHG_USB_DCP_SLOW,
>>>> + EXTCON_CHG_USB_CDP,
>>>> + EXTCON_CHG_USB_DCP_FAST,
>>>> EXTCON_NONE,
>>>> };
>>>>
>>>> @@ -192,18 +192,18 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
>>>> dev_dbg(info->dev, "sdp cable is connecetd\n");
>>>
>>> as per comment this seems to be SDP.
>>
>> This patch modifies the only connector name.
>> If some comment should be fixed, we need to implement on separate patch.
>
> Agreed.
>>
>>>
>>>> notify_otg = true;
>>>> notify_charger = true;
>>>> - cable = EXTCON_SLOW_CHARGER;
>>>> + cable = EXTCON_CHG_USB_DCP_SLOW;
>>>
>>> So shouldn't this be cable = EXTCON_USB?
>>
>> No, EXTCON_CHG_USB_*_SLOW is right.
>>
>>>
>>>> break;
>>>> case DET_STAT_CDP:
>>>> dev_dbg(info->dev, "cdp cable is connecetd\n");
>>>> notify_otg = true;
>>>> notify_charger = true;
>>>> - cable = EXTCON_CHARGE_DOWNSTREAM;
>>>> + cable = EXTCON_CHG_USB_CDP;
>>>> break;
>>>> case DET_STAT_DCP:
>>>> dev_dbg(info->dev, "dcp cable is connecetd\n");
>>>
>>> This looks like standard DCP.
>>
>> ditto. Need the separate patch to fix it.
>
> Agreed.
>
>>
>>>
>>>> notify_charger = true;
>>>> - cable = EXTCON_FAST_CHARGER;
>>>> + cable = EXTCON_CHG_USB_DCP_FAST;
>>>
>>> Shouldn't this be cable = EXTCON_CHG_USB_DCP?
>>
>> No, EXTCON_CHG_USB_*_FAST is right.
>
> Why? The case is DET_STAT_DCP.
You're right. I'll fix it as following:
case DET_STAT_SDP -> EXTCON_USB
case DET_STAT_CDP -> EXTCON_CHG_USB_CDP
case DET_STAT_DCP -> EXTCON_CHG_USB_DCP
Thanks,
Chanwoo Choi
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v3] extcon: Modify the id and name of external connector
2015-10-07 7:59 ` Chanwoo Choi
@ 2015-10-07 8:09 ` Chanwoo Choi
2015-10-07 8:23 ` Roger Quadros
0 siblings, 1 reply; 10+ messages in thread
From: Chanwoo Choi @ 2015-10-07 8:09 UTC (permalink / raw)
To: Roger Quadros, linux-kernel
Cc: k.kozlowski, ckeepax, gregkh, ramakrishna.pallala, patches,
myungjoo.ham
Roger,
On 2015년 10월 07일 16:59, Chanwoo Choi wrote:
> On 2015년 10월 07일 16:50, Roger Quadros wrote:
>> On 07/10/15 10:42, Chanwoo Choi wrote:
>>> On 2015년 10월 07일 16:33, Roger Quadros wrote:
>>>> On 07/10/15 03:48, Chanwoo Choi wrote:
>>>>> This patch modifies the id and name of external connector with the
>>>>> additional prefix to clarify both attribute and meaning of external
>>>>> connector as following:
>>>>> - EXTCON_CHG_* mean the charger connector.
>>>>> - EXTCON_JACK_* mean the jack connector.
>>>>> - EXTCON_DISP_* mean the display port connector.
>>>>>
>>>>> Following table show the new name of external connector with old name:
>>>>> --------------------------------------------------
>>>>> Old extcon name | New extcon name |
>>>>> --------------------------------------------------
>>>>> EXTCON_TA | EXTCON_CHG_USB_DCP |
>>>>> EXTCON_CHARGE_DOWNSTREAM| EXTCON_CHG_USB_CDP |
>>>>> EXTCON_FAST_CHARGER | EXTCON_CHG_USB_DCP_FAST|
>>>>> EXTCON_SLOW_CHARGER | EXTCON_CHG_USB_DCP_SLOW|
>>>>
>>>> I still have some concerns regarding the name DCP_FAST and DCP_SLOW.
>>>> If I see the charger IC drivers that use FAST & SLOW, they either call it
>>>> APPLE_500MA/1A or SPECIAL_500MA/1A.
>>>
>>> Right.
>>>
>>>>
>>>> They are probably not compliant with USB DCP specification and hence
>>>> the special name.
>>>>
>>>> So, should we call them EXTCON_CHG_USB_SPECIAL_FAST/SLOW instead?
>>>
>>> I don't prefer to use "SPECIAL" adjective for connector name.
>>> I prefer following name at the first patch.
>>> - EXTCON_CHG_USB_FAST
>>> - EXTCON_CHG_USB_SLOW
>>
>> OK, that is fine with me.
>>>
>>>>
>>>>> --------------------------------------------------
>>>>> EXTCON_MICROPHONE | EXTCON_JACK_MICROPHONE |
>>>>> EXTCON_HEADPHONE | EXTCON_JACK_HEADPHONE |
>>>>> EXTCON_LINE_IN | EXTCON_JACK_LINE_IN |
>>>>> EXTCON_LINE_OUT | EXTCON_JACK_LINE_OUT |
>>>>> EXTCON_VIDEO_IN | EXTCON_JACK_VIDEO_IN |
>>>>> EXTCON_VIDEO_OUT | EXTCON_JACK_VIDEO_OUT |
>>>>> EXTCON_SPDIF_IN | EXTCON_JACK_SPDIF_IN |
>>>>> EXTCON_SPDIF_OUT | EXTCON_JACK_SPDIF_OUT |
>>>>> --------------------------------------------------
>>>>> EXTCON_HMDI | EXTCON_DISP_HDMI |
>>>>> EXTCON_MHL | EXTCON_DISP_MHL |
>>>>> EXTCON_DVI | EXTCON_DISP_DVI |
>>>>> EXTCON_VGA | EXTCON_DISP_VGA |
>>>>> --------------------------------------------------
>>>>>
>>>>> And, when altering the name of USB charger connector, EXTCON refers to the
>>>>> "Battery Charging v1.2 Spec and Adopters Agreement"[1] to use the standard
>>>>> name of USB charging port as following. Following name of USB charging port
>>>>> are already used in power_supply subsystem. We chan check it on patch[2].
>>>>> - EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
>>>>> - EXTCON_CHG_USB_CDP /* Charging Downstream Port */
>>>>> - EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */
>>>>>
>>>>> [1] www.usb.org/developers/docs/devclass_docs/BCv1.2_070312.zip
>>>>> [2] commit 85efc8a18ced ("power_supply: Add types for USB chargers")
>>>>>
>>>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>> [ckeepax: For the Arizona changes]
>>>>> Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
>>>>> ---
>>>>> Changes from v2:
>>>>> (https://lkml.org/lkml/2015/10/6/239)
>>>>> - Remove the EXTCON_CHG_USB type to remove the possible confusion according to
>>>>> Roger's comment and drop patch2 about EXTCON_CHG_USB.
>>>>> - Fix the warning issue provided by scripts/checkpatch.pl
>>>>>
>>>>> Changes from v1:
>>>>> (https://lkml.org/lkml/2015/10/3/304)
>>>>> - Add acked tag by Charles Keepax for arizona changes
>>>>> - Modify the name of USB charger connector as following:
>>>>> : EXTCON_CHG_USB_FAST -> EXTCON_CHG_USB_DCP_FAST
>>>>> : EXTCON_CHG_USB_SLOW -> EXTCON_CHG_USB_DCP_SLOW
>>>>> - Add the missing EXTCON_CHG_USB_ACA charger connector
>>>>> - Add one more patch to support the EXTCON_CHG_USB when SDP port is connected
>>>>> or not
>>>>>
>>>>> drivers/extcon/extcon-arizona.c | 18 ++++++------
>>>>> drivers/extcon/extcon-axp288.c | 12 ++++----
>>>>> drivers/extcon/extcon-max14577.c | 17 +++++------
>>>>> drivers/extcon/extcon-max77693.c | 32 +++++++++++----------
>>>>> drivers/extcon/extcon-max77843.c | 27 ++++++++++--------
>>>>> drivers/extcon/extcon-max8997.c | 21 +++++++-------
>>>>> drivers/extcon/extcon-rt8973a.c | 4 +--
>>>>> drivers/extcon/extcon-sm5502.c | 4 +--
>>>>> drivers/extcon/extcon.c | 60 ++++++++++++++++++++-------------------
>>>>> include/linux/extcon.h | 61 +++++++++++++++++++++++-----------------
>>>>> 10 files changed, 137 insertions(+), 119 deletions(-)
>>>>>
>>>>> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
>>>>> index a1ab0a56b798..e4890dd4fefd 100644
>>>>> --- a/drivers/extcon/extcon-arizona.c
>>>>> +++ b/drivers/extcon/extcon-arizona.c
>>>>> @@ -137,9 +137,9 @@ static const int arizona_micd_levels[] = {
>>>>>
>>>>> static const unsigned int arizona_cable[] = {
>>>>> EXTCON_MECHANICAL,
>>>>> - EXTCON_MICROPHONE,
>>>>> - EXTCON_HEADPHONE,
>>>>> - EXTCON_LINE_OUT,
>>>>> + EXTCON_JACK_MICROPHONE,
>>>>> + EXTCON_JACK_HEADPHONE,
>>>>> + EXTCON_JACK_LINE_OUT,
>>>>> EXTCON_NONE,
>>>>> };
>>>>>
>>>>> @@ -600,7 +600,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>>>>> struct arizona_extcon_info *info = data;
>>>>> struct arizona *arizona = info->arizona;
>>>>> int id_gpio = arizona->pdata.hpdet_id_gpio;
>>>>> - unsigned int report = EXTCON_HEADPHONE;
>>>>> + unsigned int report = EXTCON_JACK_HEADPHONE;
>>>>> int ret, reading;
>>>>> bool mic = false;
>>>>>
>>>>> @@ -645,9 +645,9 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>>>>>
>>>>> /* Report high impedence cables as line outputs */
>>>>> if (reading >= 5000)
>>>>> - report = EXTCON_LINE_OUT;
>>>>> + report = EXTCON_JACK_LINE_OUT;
>>>>> else
>>>>> - report = EXTCON_HEADPHONE;
>>>>> + report = EXTCON_JACK_HEADPHONE;
>>>>>
>>>>> ret = extcon_set_cable_state_(info->edev, report, true);
>>>>> if (ret != 0)
>>>>> @@ -732,7 +732,7 @@ err:
>>>>> ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
>>>>>
>>>>> /* Just report headphone */
>>>>> - ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true);
>>>>> + ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true);
>>>>> if (ret != 0)
>>>>> dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
>>>>>
>>>>> @@ -789,7 +789,7 @@ err:
>>>>> ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
>>>>>
>>>>> /* Just report headphone */
>>>>> - ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true);
>>>>> + ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true);
>>>>> if (ret != 0)
>>>>> dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
>>>>>
>>>>> @@ -915,7 +915,7 @@ static void arizona_micd_detect(struct work_struct *work)
>>>>> arizona_identify_headphone(info);
>>>>>
>>>>> ret = extcon_set_cable_state_(info->edev,
>>>>> - EXTCON_MICROPHONE, true);
>>>>> + EXTCON_JACK_MICROPHONE, true);
>>>>> if (ret != 0)
>>>>> dev_err(arizona->dev, "Headset report failed: %d\n",
>>>>> ret);
>>>>> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
>>>>> index 9668d6a94e38..14e748051288 100644
>>>>> --- a/drivers/extcon/extcon-axp288.c
>>>>> +++ b/drivers/extcon/extcon-axp288.c
>>>>> @@ -102,9 +102,9 @@ enum axp288_extcon_irq {
>>>>> };
>>>>>
>>>>> static const unsigned int axp288_extcon_cables[] = {
>>>>> - EXTCON_SLOW_CHARGER,
>>>>> - EXTCON_CHARGE_DOWNSTREAM,
>>>>> - EXTCON_FAST_CHARGER,
>>>>> + EXTCON_CHG_USB_DCP_SLOW,
>>>>> + EXTCON_CHG_USB_CDP,
>>>>> + EXTCON_CHG_USB_DCP_FAST,
>>>>> EXTCON_NONE,
>>>>> };
>>>>>
>>>>> @@ -192,18 +192,18 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
>>>>> dev_dbg(info->dev, "sdp cable is connecetd\n");
>>>>
>>>> as per comment this seems to be SDP.
>>>
>>> This patch modifies the only connector name.
>>> If some comment should be fixed, we need to implement on separate patch.
>>
>> Agreed.
>>>
>>>>
>>>>> notify_otg = true;
>>>>> notify_charger = true;
>>>>> - cable = EXTCON_SLOW_CHARGER;
>>>>> + cable = EXTCON_CHG_USB_DCP_SLOW;
>>>>
>>>> So shouldn't this be cable = EXTCON_USB?
>>>
>>> No, EXTCON_CHG_USB_*_SLOW is right.
>>>
>>>>
>>>>> break;
>>>>> case DET_STAT_CDP:
>>>>> dev_dbg(info->dev, "cdp cable is connecetd\n");
>>>>> notify_otg = true;
>>>>> notify_charger = true;
>>>>> - cable = EXTCON_CHARGE_DOWNSTREAM;
>>>>> + cable = EXTCON_CHG_USB_CDP;
>>>>> break;
>>>>> case DET_STAT_DCP:
>>>>> dev_dbg(info->dev, "dcp cable is connecetd\n");
>>>>
>>>> This looks like standard DCP.
>>>
>>> ditto. Need the separate patch to fix it.
>>
>> Agreed.
>>
>>>
>>>>
>>>>> notify_charger = true;
>>>>> - cable = EXTCON_FAST_CHARGER;
>>>>> + cable = EXTCON_CHG_USB_DCP_FAST;
>>>>
>>>> Shouldn't this be cable = EXTCON_CHG_USB_DCP?
>>>
>>> No, EXTCON_CHG_USB_*_FAST is right.
>>
>> Why? The case is DET_STAT_DCP.
>
> You're right. I'll fix it as following:
>
> case DET_STAT_SDP -> EXTCON_USB
> case DET_STAT_CDP -> EXTCON_CHG_USB_CDP
> case DET_STAT_DCP -> EXTCON_CHG_USB_DCP
I think that we need to consider about SDP and CDP port again
because SDP/CDP support the both data transmission and charging at the same time.
Case 1. if SDP is attached
Following connector type mean the each role of SDP.
- EXTCON_USB for data transmission
- EXTCON_CHG_USB_SDP for charging
Case 2. if CDP is attached
ditto.
- EXTCON_USB for data transmission
- EXTCON_CHG_USB_CDP for charging
Thanks,
Chanwoo Choi
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v3] extcon: Modify the id and name of external connector
2015-10-07 8:09 ` Chanwoo Choi
@ 2015-10-07 8:23 ` Roger Quadros
2015-10-07 8:30 ` Chanwoo Choi
0 siblings, 1 reply; 10+ messages in thread
From: Roger Quadros @ 2015-10-07 8:23 UTC (permalink / raw)
To: Chanwoo Choi, linux-kernel
Cc: k.kozlowski, ckeepax, gregkh, ramakrishna.pallala, patches,
myungjoo.ham
Chanwoo,
On 07/10/15 11:09, Chanwoo Choi wrote:
> Roger,
>
> On 2015년 10월 07일 16:59, Chanwoo Choi wrote:
>> On 2015년 10월 07일 16:50, Roger Quadros wrote:
>>> On 07/10/15 10:42, Chanwoo Choi wrote:
>>>> On 2015년 10월 07일 16:33, Roger Quadros wrote:
>>>>> On 07/10/15 03:48, Chanwoo Choi wrote:
>>>>>> This patch modifies the id and name of external connector with the
>>>>>> additional prefix to clarify both attribute and meaning of external
>>>>>> connector as following:
>>>>>> - EXTCON_CHG_* mean the charger connector.
>>>>>> - EXTCON_JACK_* mean the jack connector.
>>>>>> - EXTCON_DISP_* mean the display port connector.
>>>>>>
>>>>>> Following table show the new name of external connector with old name:
>>>>>> --------------------------------------------------
>>>>>> Old extcon name | New extcon name |
>>>>>> --------------------------------------------------
>>>>>> EXTCON_TA | EXTCON_CHG_USB_DCP |
>>>>>> EXTCON_CHARGE_DOWNSTREAM| EXTCON_CHG_USB_CDP |
>>>>>> EXTCON_FAST_CHARGER | EXTCON_CHG_USB_DCP_FAST|
>>>>>> EXTCON_SLOW_CHARGER | EXTCON_CHG_USB_DCP_SLOW|
>>>>>
>>>>> I still have some concerns regarding the name DCP_FAST and DCP_SLOW.
>>>>> If I see the charger IC drivers that use FAST & SLOW, they either call it
>>>>> APPLE_500MA/1A or SPECIAL_500MA/1A.
>>>>
>>>> Right.
>>>>
>>>>>
>>>>> They are probably not compliant with USB DCP specification and hence
>>>>> the special name.
>>>>>
>>>>> So, should we call them EXTCON_CHG_USB_SPECIAL_FAST/SLOW instead?
>>>>
>>>> I don't prefer to use "SPECIAL" adjective for connector name.
>>>> I prefer following name at the first patch.
>>>> - EXTCON_CHG_USB_FAST
>>>> - EXTCON_CHG_USB_SLOW
>>>
>>> OK, that is fine with me.
>>>>
>>>>>
>>>>>> --------------------------------------------------
>>>>>> EXTCON_MICROPHONE | EXTCON_JACK_MICROPHONE |
>>>>>> EXTCON_HEADPHONE | EXTCON_JACK_HEADPHONE |
>>>>>> EXTCON_LINE_IN | EXTCON_JACK_LINE_IN |
>>>>>> EXTCON_LINE_OUT | EXTCON_JACK_LINE_OUT |
>>>>>> EXTCON_VIDEO_IN | EXTCON_JACK_VIDEO_IN |
>>>>>> EXTCON_VIDEO_OUT | EXTCON_JACK_VIDEO_OUT |
>>>>>> EXTCON_SPDIF_IN | EXTCON_JACK_SPDIF_IN |
>>>>>> EXTCON_SPDIF_OUT | EXTCON_JACK_SPDIF_OUT |
>>>>>> --------------------------------------------------
>>>>>> EXTCON_HMDI | EXTCON_DISP_HDMI |
>>>>>> EXTCON_MHL | EXTCON_DISP_MHL |
>>>>>> EXTCON_DVI | EXTCON_DISP_DVI |
>>>>>> EXTCON_VGA | EXTCON_DISP_VGA |
>>>>>> --------------------------------------------------
>>>>>>
>>>>>> And, when altering the name of USB charger connector, EXTCON refers to the
>>>>>> "Battery Charging v1.2 Spec and Adopters Agreement"[1] to use the standard
>>>>>> name of USB charging port as following. Following name of USB charging port
>>>>>> are already used in power_supply subsystem. We chan check it on patch[2].
>>>>>> - EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
>>>>>> - EXTCON_CHG_USB_CDP /* Charging Downstream Port */
>>>>>> - EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */
>>>>>>
>>>>>> [1] www.usb.org/developers/docs/devclass_docs/BCv1.2_070312.zip
>>>>>> [2] commit 85efc8a18ced ("power_supply: Add types for USB chargers")
>>>>>>
>>>>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>>> [ckeepax: For the Arizona changes]
>>>>>> Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
>>>>>> ---
>>>>>> Changes from v2:
>>>>>> (https://lkml.org/lkml/2015/10/6/239)
>>>>>> - Remove the EXTCON_CHG_USB type to remove the possible confusion according to
>>>>>> Roger's comment and drop patch2 about EXTCON_CHG_USB.
>>>>>> - Fix the warning issue provided by scripts/checkpatch.pl
>>>>>>
>>>>>> Changes from v1:
>>>>>> (https://lkml.org/lkml/2015/10/3/304)
>>>>>> - Add acked tag by Charles Keepax for arizona changes
>>>>>> - Modify the name of USB charger connector as following:
>>>>>> : EXTCON_CHG_USB_FAST -> EXTCON_CHG_USB_DCP_FAST
>>>>>> : EXTCON_CHG_USB_SLOW -> EXTCON_CHG_USB_DCP_SLOW
>>>>>> - Add the missing EXTCON_CHG_USB_ACA charger connector
>>>>>> - Add one more patch to support the EXTCON_CHG_USB when SDP port is connected
>>>>>> or not
>>>>>>
>>>>>> drivers/extcon/extcon-arizona.c | 18 ++++++------
>>>>>> drivers/extcon/extcon-axp288.c | 12 ++++----
>>>>>> drivers/extcon/extcon-max14577.c | 17 +++++------
>>>>>> drivers/extcon/extcon-max77693.c | 32 +++++++++++----------
>>>>>> drivers/extcon/extcon-max77843.c | 27 ++++++++++--------
>>>>>> drivers/extcon/extcon-max8997.c | 21 +++++++-------
>>>>>> drivers/extcon/extcon-rt8973a.c | 4 +--
>>>>>> drivers/extcon/extcon-sm5502.c | 4 +--
>>>>>> drivers/extcon/extcon.c | 60 ++++++++++++++++++++-------------------
>>>>>> include/linux/extcon.h | 61 +++++++++++++++++++++++-----------------
>>>>>> 10 files changed, 137 insertions(+), 119 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
>>>>>> index a1ab0a56b798..e4890dd4fefd 100644
>>>>>> --- a/drivers/extcon/extcon-arizona.c
>>>>>> +++ b/drivers/extcon/extcon-arizona.c
>>>>>> @@ -137,9 +137,9 @@ static const int arizona_micd_levels[] = {
>>>>>>
>>>>>> static const unsigned int arizona_cable[] = {
>>>>>> EXTCON_MECHANICAL,
>>>>>> - EXTCON_MICROPHONE,
>>>>>> - EXTCON_HEADPHONE,
>>>>>> - EXTCON_LINE_OUT,
>>>>>> + EXTCON_JACK_MICROPHONE,
>>>>>> + EXTCON_JACK_HEADPHONE,
>>>>>> + EXTCON_JACK_LINE_OUT,
>>>>>> EXTCON_NONE,
>>>>>> };
>>>>>>
>>>>>> @@ -600,7 +600,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>>>>>> struct arizona_extcon_info *info = data;
>>>>>> struct arizona *arizona = info->arizona;
>>>>>> int id_gpio = arizona->pdata.hpdet_id_gpio;
>>>>>> - unsigned int report = EXTCON_HEADPHONE;
>>>>>> + unsigned int report = EXTCON_JACK_HEADPHONE;
>>>>>> int ret, reading;
>>>>>> bool mic = false;
>>>>>>
>>>>>> @@ -645,9 +645,9 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>>>>>>
>>>>>> /* Report high impedence cables as line outputs */
>>>>>> if (reading >= 5000)
>>>>>> - report = EXTCON_LINE_OUT;
>>>>>> + report = EXTCON_JACK_LINE_OUT;
>>>>>> else
>>>>>> - report = EXTCON_HEADPHONE;
>>>>>> + report = EXTCON_JACK_HEADPHONE;
>>>>>>
>>>>>> ret = extcon_set_cable_state_(info->edev, report, true);
>>>>>> if (ret != 0)
>>>>>> @@ -732,7 +732,7 @@ err:
>>>>>> ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
>>>>>>
>>>>>> /* Just report headphone */
>>>>>> - ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true);
>>>>>> + ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true);
>>>>>> if (ret != 0)
>>>>>> dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
>>>>>>
>>>>>> @@ -789,7 +789,7 @@ err:
>>>>>> ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
>>>>>>
>>>>>> /* Just report headphone */
>>>>>> - ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true);
>>>>>> + ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true);
>>>>>> if (ret != 0)
>>>>>> dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
>>>>>>
>>>>>> @@ -915,7 +915,7 @@ static void arizona_micd_detect(struct work_struct *work)
>>>>>> arizona_identify_headphone(info);
>>>>>>
>>>>>> ret = extcon_set_cable_state_(info->edev,
>>>>>> - EXTCON_MICROPHONE, true);
>>>>>> + EXTCON_JACK_MICROPHONE, true);
>>>>>> if (ret != 0)
>>>>>> dev_err(arizona->dev, "Headset report failed: %d\n",
>>>>>> ret);
>>>>>> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
>>>>>> index 9668d6a94e38..14e748051288 100644
>>>>>> --- a/drivers/extcon/extcon-axp288.c
>>>>>> +++ b/drivers/extcon/extcon-axp288.c
>>>>>> @@ -102,9 +102,9 @@ enum axp288_extcon_irq {
>>>>>> };
>>>>>>
>>>>>> static const unsigned int axp288_extcon_cables[] = {
>>>>>> - EXTCON_SLOW_CHARGER,
>>>>>> - EXTCON_CHARGE_DOWNSTREAM,
>>>>>> - EXTCON_FAST_CHARGER,
>>>>>> + EXTCON_CHG_USB_DCP_SLOW,
>>>>>> + EXTCON_CHG_USB_CDP,
>>>>>> + EXTCON_CHG_USB_DCP_FAST,
>>>>>> EXTCON_NONE,
>>>>>> };
>>>>>>
>>>>>> @@ -192,18 +192,18 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
>>>>>> dev_dbg(info->dev, "sdp cable is connecetd\n");
>>>>>
>>>>> as per comment this seems to be SDP.
>>>>
>>>> This patch modifies the only connector name.
>>>> If some comment should be fixed, we need to implement on separate patch.
>>>
>>> Agreed.
>>>>
>>>>>
>>>>>> notify_otg = true;
>>>>>> notify_charger = true;
>>>>>> - cable = EXTCON_SLOW_CHARGER;
>>>>>> + cable = EXTCON_CHG_USB_DCP_SLOW;
>>>>>
>>>>> So shouldn't this be cable = EXTCON_USB?
>>>>
>>>> No, EXTCON_CHG_USB_*_SLOW is right.
>>>>
>>>>>
>>>>>> break;
>>>>>> case DET_STAT_CDP:
>>>>>> dev_dbg(info->dev, "cdp cable is connecetd\n");
>>>>>> notify_otg = true;
>>>>>> notify_charger = true;
>>>>>> - cable = EXTCON_CHARGE_DOWNSTREAM;
>>>>>> + cable = EXTCON_CHG_USB_CDP;
>>>>>> break;
>>>>>> case DET_STAT_DCP:
>>>>>> dev_dbg(info->dev, "dcp cable is connecetd\n");
>>>>>
>>>>> This looks like standard DCP.
>>>>
>>>> ditto. Need the separate patch to fix it.
>>>
>>> Agreed.
>>>
>>>>
>>>>>
>>>>>> notify_charger = true;
>>>>>> - cable = EXTCON_FAST_CHARGER;
>>>>>> + cable = EXTCON_CHG_USB_DCP_FAST;
>>>>>
>>>>> Shouldn't this be cable = EXTCON_CHG_USB_DCP?
>>>>
>>>> No, EXTCON_CHG_USB_*_FAST is right.
>>>
>>> Why? The case is DET_STAT_DCP.
>>
>> You're right. I'll fix it as following:
>>
>> case DET_STAT_SDP -> EXTCON_USB
>> case DET_STAT_CDP -> EXTCON_CHG_USB_CDP
>> case DET_STAT_DCP -> EXTCON_CHG_USB_DCP
>
> I think that we need to consider about SDP and CDP port again
> because SDP/CDP support the both data transmission and charging at the same time.
>
> Case 1. if SDP is attached
> Following connector type mean the each role of SDP.
> - EXTCON_USB for data transmission
> - EXTCON_CHG_USB_SDP for charging
>
> Case 2. if CDP is attached
> ditto.
> - EXTCON_USB for data transmission
> - EXTCON_CHG_USB_CDP for charging
>
The only issue is that charging and data transmission happen
simultaneously. Can the framework allow both states to be
active simultaneously?
cheers,
-roger
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v3] extcon: Modify the id and name of external connector
2015-10-07 8:23 ` Roger Quadros
@ 2015-10-07 8:30 ` Chanwoo Choi
2015-10-07 8:52 ` Roger Quadros
0 siblings, 1 reply; 10+ messages in thread
From: Chanwoo Choi @ 2015-10-07 8:30 UTC (permalink / raw)
To: Roger Quadros, linux-kernel
Cc: k.kozlowski, ckeepax, gregkh, ramakrishna.pallala, patches,
myungjoo.ham
Roger,
On 2015년 10월 07일 17:23, Roger Quadros wrote:
> Chanwoo,
>
> On 07/10/15 11:09, Chanwoo Choi wrote:
>> Roger,
>>
>> On 2015년 10월 07일 16:59, Chanwoo Choi wrote:
>>> On 2015년 10월 07일 16:50, Roger Quadros wrote:
>>>> On 07/10/15 10:42, Chanwoo Choi wrote:
>>>>> On 2015년 10월 07일 16:33, Roger Quadros wrote:
>>>>>> On 07/10/15 03:48, Chanwoo Choi wrote:
>>>>>>> This patch modifies the id and name of external connector with the
>>>>>>> additional prefix to clarify both attribute and meaning of external
>>>>>>> connector as following:
>>>>>>> - EXTCON_CHG_* mean the charger connector.
>>>>>>> - EXTCON_JACK_* mean the jack connector.
>>>>>>> - EXTCON_DISP_* mean the display port connector.
>>>>>>>
>>>>>>> Following table show the new name of external connector with old name:
>>>>>>> --------------------------------------------------
>>>>>>> Old extcon name | New extcon name |
>>>>>>> --------------------------------------------------
>>>>>>> EXTCON_TA | EXTCON_CHG_USB_DCP |
>>>>>>> EXTCON_CHARGE_DOWNSTREAM| EXTCON_CHG_USB_CDP |
>>>>>>> EXTCON_FAST_CHARGER | EXTCON_CHG_USB_DCP_FAST|
>>>>>>> EXTCON_SLOW_CHARGER | EXTCON_CHG_USB_DCP_SLOW|
>>>>>>
>>>>>> I still have some concerns regarding the name DCP_FAST and DCP_SLOW.
>>>>>> If I see the charger IC drivers that use FAST & SLOW, they either call it
>>>>>> APPLE_500MA/1A or SPECIAL_500MA/1A.
>>>>>
>>>>> Right.
>>>>>
>>>>>>
>>>>>> They are probably not compliant with USB DCP specification and hence
>>>>>> the special name.
>>>>>>
>>>>>> So, should we call them EXTCON_CHG_USB_SPECIAL_FAST/SLOW instead?
>>>>>
>>>>> I don't prefer to use "SPECIAL" adjective for connector name.
>>>>> I prefer following name at the first patch.
>>>>> - EXTCON_CHG_USB_FAST
>>>>> - EXTCON_CHG_USB_SLOW
>>>>
>>>> OK, that is fine with me.
>>>>>
>>>>>>
>>>>>>> --------------------------------------------------
>>>>>>> EXTCON_MICROPHONE | EXTCON_JACK_MICROPHONE |
>>>>>>> EXTCON_HEADPHONE | EXTCON_JACK_HEADPHONE |
>>>>>>> EXTCON_LINE_IN | EXTCON_JACK_LINE_IN |
>>>>>>> EXTCON_LINE_OUT | EXTCON_JACK_LINE_OUT |
>>>>>>> EXTCON_VIDEO_IN | EXTCON_JACK_VIDEO_IN |
>>>>>>> EXTCON_VIDEO_OUT | EXTCON_JACK_VIDEO_OUT |
>>>>>>> EXTCON_SPDIF_IN | EXTCON_JACK_SPDIF_IN |
>>>>>>> EXTCON_SPDIF_OUT | EXTCON_JACK_SPDIF_OUT |
>>>>>>> --------------------------------------------------
>>>>>>> EXTCON_HMDI | EXTCON_DISP_HDMI |
>>>>>>> EXTCON_MHL | EXTCON_DISP_MHL |
>>>>>>> EXTCON_DVI | EXTCON_DISP_DVI |
>>>>>>> EXTCON_VGA | EXTCON_DISP_VGA |
>>>>>>> --------------------------------------------------
>>>>>>>
>>>>>>> And, when altering the name of USB charger connector, EXTCON refers to the
>>>>>>> "Battery Charging v1.2 Spec and Adopters Agreement"[1] to use the standard
>>>>>>> name of USB charging port as following. Following name of USB charging port
>>>>>>> are already used in power_supply subsystem. We chan check it on patch[2].
>>>>>>> - EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
>>>>>>> - EXTCON_CHG_USB_CDP /* Charging Downstream Port */
>>>>>>> - EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */
>>>>>>>
>>>>>>> [1] www.usb.org/developers/docs/devclass_docs/BCv1.2_070312.zip
>>>>>>> [2] commit 85efc8a18ced ("power_supply: Add types for USB chargers")
>>>>>>>
>>>>>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>>>> [ckeepax: For the Arizona changes]
>>>>>>> Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
>>>>>>> ---
>>>>>>> Changes from v2:
>>>>>>> (https://lkml.org/lkml/2015/10/6/239)
>>>>>>> - Remove the EXTCON_CHG_USB type to remove the possible confusion according to
>>>>>>> Roger's comment and drop patch2 about EXTCON_CHG_USB.
>>>>>>> - Fix the warning issue provided by scripts/checkpatch.pl
>>>>>>>
>>>>>>> Changes from v1:
>>>>>>> (https://lkml.org/lkml/2015/10/3/304)
>>>>>>> - Add acked tag by Charles Keepax for arizona changes
>>>>>>> - Modify the name of USB charger connector as following:
>>>>>>> : EXTCON_CHG_USB_FAST -> EXTCON_CHG_USB_DCP_FAST
>>>>>>> : EXTCON_CHG_USB_SLOW -> EXTCON_CHG_USB_DCP_SLOW
>>>>>>> - Add the missing EXTCON_CHG_USB_ACA charger connector
>>>>>>> - Add one more patch to support the EXTCON_CHG_USB when SDP port is connected
>>>>>>> or not
>>>>>>>
>>>>>>> drivers/extcon/extcon-arizona.c | 18 ++++++------
>>>>>>> drivers/extcon/extcon-axp288.c | 12 ++++----
>>>>>>> drivers/extcon/extcon-max14577.c | 17 +++++------
>>>>>>> drivers/extcon/extcon-max77693.c | 32 +++++++++++----------
>>>>>>> drivers/extcon/extcon-max77843.c | 27 ++++++++++--------
>>>>>>> drivers/extcon/extcon-max8997.c | 21 +++++++-------
>>>>>>> drivers/extcon/extcon-rt8973a.c | 4 +--
>>>>>>> drivers/extcon/extcon-sm5502.c | 4 +--
>>>>>>> drivers/extcon/extcon.c | 60 ++++++++++++++++++++-------------------
>>>>>>> include/linux/extcon.h | 61 +++++++++++++++++++++++-----------------
>>>>>>> 10 files changed, 137 insertions(+), 119 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
>>>>>>> index a1ab0a56b798..e4890dd4fefd 100644
>>>>>>> --- a/drivers/extcon/extcon-arizona.c
>>>>>>> +++ b/drivers/extcon/extcon-arizona.c
>>>>>>> @@ -137,9 +137,9 @@ static const int arizona_micd_levels[] = {
>>>>>>>
>>>>>>> static const unsigned int arizona_cable[] = {
>>>>>>> EXTCON_MECHANICAL,
>>>>>>> - EXTCON_MICROPHONE,
>>>>>>> - EXTCON_HEADPHONE,
>>>>>>> - EXTCON_LINE_OUT,
>>>>>>> + EXTCON_JACK_MICROPHONE,
>>>>>>> + EXTCON_JACK_HEADPHONE,
>>>>>>> + EXTCON_JACK_LINE_OUT,
>>>>>>> EXTCON_NONE,
>>>>>>> };
>>>>>>>
>>>>>>> @@ -600,7 +600,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>>>>>>> struct arizona_extcon_info *info = data;
>>>>>>> struct arizona *arizona = info->arizona;
>>>>>>> int id_gpio = arizona->pdata.hpdet_id_gpio;
>>>>>>> - unsigned int report = EXTCON_HEADPHONE;
>>>>>>> + unsigned int report = EXTCON_JACK_HEADPHONE;
>>>>>>> int ret, reading;
>>>>>>> bool mic = false;
>>>>>>>
>>>>>>> @@ -645,9 +645,9 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>>>>>>>
>>>>>>> /* Report high impedence cables as line outputs */
>>>>>>> if (reading >= 5000)
>>>>>>> - report = EXTCON_LINE_OUT;
>>>>>>> + report = EXTCON_JACK_LINE_OUT;
>>>>>>> else
>>>>>>> - report = EXTCON_HEADPHONE;
>>>>>>> + report = EXTCON_JACK_HEADPHONE;
>>>>>>>
>>>>>>> ret = extcon_set_cable_state_(info->edev, report, true);
>>>>>>> if (ret != 0)
>>>>>>> @@ -732,7 +732,7 @@ err:
>>>>>>> ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
>>>>>>>
>>>>>>> /* Just report headphone */
>>>>>>> - ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true);
>>>>>>> + ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true);
>>>>>>> if (ret != 0)
>>>>>>> dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
>>>>>>>
>>>>>>> @@ -789,7 +789,7 @@ err:
>>>>>>> ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
>>>>>>>
>>>>>>> /* Just report headphone */
>>>>>>> - ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true);
>>>>>>> + ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true);
>>>>>>> if (ret != 0)
>>>>>>> dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
>>>>>>>
>>>>>>> @@ -915,7 +915,7 @@ static void arizona_micd_detect(struct work_struct *work)
>>>>>>> arizona_identify_headphone(info);
>>>>>>>
>>>>>>> ret = extcon_set_cable_state_(info->edev,
>>>>>>> - EXTCON_MICROPHONE, true);
>>>>>>> + EXTCON_JACK_MICROPHONE, true);
>>>>>>> if (ret != 0)
>>>>>>> dev_err(arizona->dev, "Headset report failed: %d\n",
>>>>>>> ret);
>>>>>>> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
>>>>>>> index 9668d6a94e38..14e748051288 100644
>>>>>>> --- a/drivers/extcon/extcon-axp288.c
>>>>>>> +++ b/drivers/extcon/extcon-axp288.c
>>>>>>> @@ -102,9 +102,9 @@ enum axp288_extcon_irq {
>>>>>>> };
>>>>>>>
>>>>>>> static const unsigned int axp288_extcon_cables[] = {
>>>>>>> - EXTCON_SLOW_CHARGER,
>>>>>>> - EXTCON_CHARGE_DOWNSTREAM,
>>>>>>> - EXTCON_FAST_CHARGER,
>>>>>>> + EXTCON_CHG_USB_DCP_SLOW,
>>>>>>> + EXTCON_CHG_USB_CDP,
>>>>>>> + EXTCON_CHG_USB_DCP_FAST,
>>>>>>> EXTCON_NONE,
>>>>>>> };
>>>>>>>
>>>>>>> @@ -192,18 +192,18 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
>>>>>>> dev_dbg(info->dev, "sdp cable is connecetd\n");
>>>>>>
>>>>>> as per comment this seems to be SDP.
>>>>>
>>>>> This patch modifies the only connector name.
>>>>> If some comment should be fixed, we need to implement on separate patch.
>>>>
>>>> Agreed.
>>>>>
>>>>>>
>>>>>>> notify_otg = true;
>>>>>>> notify_charger = true;
>>>>>>> - cable = EXTCON_SLOW_CHARGER;
>>>>>>> + cable = EXTCON_CHG_USB_DCP_SLOW;
>>>>>>
>>>>>> So shouldn't this be cable = EXTCON_USB?
>>>>>
>>>>> No, EXTCON_CHG_USB_*_SLOW is right.
>>>>>
>>>>>>
>>>>>>> break;
>>>>>>> case DET_STAT_CDP:
>>>>>>> dev_dbg(info->dev, "cdp cable is connecetd\n");
>>>>>>> notify_otg = true;
>>>>>>> notify_charger = true;
>>>>>>> - cable = EXTCON_CHARGE_DOWNSTREAM;
>>>>>>> + cable = EXTCON_CHG_USB_CDP;
>>>>>>> break;
>>>>>>> case DET_STAT_DCP:
>>>>>>> dev_dbg(info->dev, "dcp cable is connecetd\n");
>>>>>>
>>>>>> This looks like standard DCP.
>>>>>
>>>>> ditto. Need the separate patch to fix it.
>>>>
>>>> Agreed.
>>>>
>>>>>
>>>>>>
>>>>>>> notify_charger = true;
>>>>>>> - cable = EXTCON_FAST_CHARGER;
>>>>>>> + cable = EXTCON_CHG_USB_DCP_FAST;
>>>>>>
>>>>>> Shouldn't this be cable = EXTCON_CHG_USB_DCP?
>>>>>
>>>>> No, EXTCON_CHG_USB_*_FAST is right.
>>>>
>>>> Why? The case is DET_STAT_DCP.
>>>
>>> You're right. I'll fix it as following:
>>>
>>> case DET_STAT_SDP -> EXTCON_USB
>>> case DET_STAT_CDP -> EXTCON_CHG_USB_CDP
>>> case DET_STAT_DCP -> EXTCON_CHG_USB_DCP
>>
>> I think that we need to consider about SDP and CDP port again
>> because SDP/CDP support the both data transmission and charging at the same time.
>>
>> Case 1. if SDP is attached
>> Following connector type mean the each role of SDP.
>> - EXTCON_USB for data transmission
>> - EXTCON_CHG_USB_SDP for charging
>>
>> Case 2. if CDP is attached
>> ditto.
>> - EXTCON_USB for data transmission
>> - EXTCON_CHG_USB_CDP for charging
>>
>
> The only issue is that charging and data transmission happen
> simultaneously. Can the framework allow both states to be
> active simultaneously?
Yes. Usually, EXTCON_USB is used for USB and PHY framework
and EXTCON_CHG_USB_SDP/EXTCON_CHG_USB_CDP will be used for power-supply framework.
For example,
In case of samsung mobile and wearable product,
when SDP is attached to device, the device can transfer the data
and charge the battery through same USB connector at the same time.
Thanks,
Chanwoo Choi
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v3] extcon: Modify the id and name of external connector
2015-10-07 8:30 ` Chanwoo Choi
@ 2015-10-07 8:52 ` Roger Quadros
2015-10-07 10:13 ` Chanwoo Choi
0 siblings, 1 reply; 10+ messages in thread
From: Roger Quadros @ 2015-10-07 8:52 UTC (permalink / raw)
To: Chanwoo Choi, linux-kernel
Cc: k.kozlowski, ckeepax, gregkh, ramakrishna.pallala, patches,
myungjoo.ham
On 07/10/15 11:30, Chanwoo Choi wrote:
> Roger,
>
> On 2015년 10월 07일 17:23, Roger Quadros wrote:
>> Chanwoo,
>>
>> On 07/10/15 11:09, Chanwoo Choi wrote:
>>> Roger,
>>>
>>> On 2015년 10월 07일 16:59, Chanwoo Choi wrote:
>>>> On 2015년 10월 07일 16:50, Roger Quadros wrote:
>>>>> On 07/10/15 10:42, Chanwoo Choi wrote:
>>>>>> On 2015년 10월 07일 16:33, Roger Quadros wrote:
>>>>>>> On 07/10/15 03:48, Chanwoo Choi wrote:
>>>>>>>> This patch modifies the id and name of external connector with the
>>>>>>>> additional prefix to clarify both attribute and meaning of external
>>>>>>>> connector as following:
>>>>>>>> - EXTCON_CHG_* mean the charger connector.
>>>>>>>> - EXTCON_JACK_* mean the jack connector.
>>>>>>>> - EXTCON_DISP_* mean the display port connector.
>>>>>>>>
>>>>>>>> Following table show the new name of external connector with old name:
>>>>>>>> --------------------------------------------------
>>>>>>>> Old extcon name | New extcon name |
>>>>>>>> --------------------------------------------------
>>>>>>>> EXTCON_TA | EXTCON_CHG_USB_DCP |
>>>>>>>> EXTCON_CHARGE_DOWNSTREAM| EXTCON_CHG_USB_CDP |
>>>>>>>> EXTCON_FAST_CHARGER | EXTCON_CHG_USB_DCP_FAST|
>>>>>>>> EXTCON_SLOW_CHARGER | EXTCON_CHG_USB_DCP_SLOW|
>>>>>>>
>>>>>>> I still have some concerns regarding the name DCP_FAST and DCP_SLOW.
>>>>>>> If I see the charger IC drivers that use FAST & SLOW, they either call it
>>>>>>> APPLE_500MA/1A or SPECIAL_500MA/1A.
>>>>>>
>>>>>> Right.
>>>>>>
>>>>>>>
>>>>>>> They are probably not compliant with USB DCP specification and hence
>>>>>>> the special name.
>>>>>>>
>>>>>>> So, should we call them EXTCON_CHG_USB_SPECIAL_FAST/SLOW instead?
>>>>>>
>>>>>> I don't prefer to use "SPECIAL" adjective for connector name.
>>>>>> I prefer following name at the first patch.
>>>>>> - EXTCON_CHG_USB_FAST
>>>>>> - EXTCON_CHG_USB_SLOW
>>>>>
>>>>> OK, that is fine with me.
>>>>>>
>>>>>>>
>>>>>>>> --------------------------------------------------
>>>>>>>> EXTCON_MICROPHONE | EXTCON_JACK_MICROPHONE |
>>>>>>>> EXTCON_HEADPHONE | EXTCON_JACK_HEADPHONE |
>>>>>>>> EXTCON_LINE_IN | EXTCON_JACK_LINE_IN |
>>>>>>>> EXTCON_LINE_OUT | EXTCON_JACK_LINE_OUT |
>>>>>>>> EXTCON_VIDEO_IN | EXTCON_JACK_VIDEO_IN |
>>>>>>>> EXTCON_VIDEO_OUT | EXTCON_JACK_VIDEO_OUT |
>>>>>>>> EXTCON_SPDIF_IN | EXTCON_JACK_SPDIF_IN |
>>>>>>>> EXTCON_SPDIF_OUT | EXTCON_JACK_SPDIF_OUT |
>>>>>>>> --------------------------------------------------
>>>>>>>> EXTCON_HMDI | EXTCON_DISP_HDMI |
>>>>>>>> EXTCON_MHL | EXTCON_DISP_MHL |
>>>>>>>> EXTCON_DVI | EXTCON_DISP_DVI |
>>>>>>>> EXTCON_VGA | EXTCON_DISP_VGA |
>>>>>>>> --------------------------------------------------
>>>>>>>>
>>>>>>>> And, when altering the name of USB charger connector, EXTCON refers to the
>>>>>>>> "Battery Charging v1.2 Spec and Adopters Agreement"[1] to use the standard
>>>>>>>> name of USB charging port as following. Following name of USB charging port
>>>>>>>> are already used in power_supply subsystem. We chan check it on patch[2].
>>>>>>>> - EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
>>>>>>>> - EXTCON_CHG_USB_CDP /* Charging Downstream Port */
>>>>>>>> - EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */
>>>>>>>>
>>>>>>>> [1] www.usb.org/developers/docs/devclass_docs/BCv1.2_070312.zip
>>>>>>>> [2] commit 85efc8a18ced ("power_supply: Add types for USB chargers")
>>>>>>>>
>>>>>>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>>>>> [ckeepax: For the Arizona changes]
>>>>>>>> Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
>>>>>>>> ---
>>>>>>>> Changes from v2:
>>>>>>>> (https://lkml.org/lkml/2015/10/6/239)
>>>>>>>> - Remove the EXTCON_CHG_USB type to remove the possible confusion according to
>>>>>>>> Roger's comment and drop patch2 about EXTCON_CHG_USB.
>>>>>>>> - Fix the warning issue provided by scripts/checkpatch.pl
>>>>>>>>
>>>>>>>> Changes from v1:
>>>>>>>> (https://lkml.org/lkml/2015/10/3/304)
>>>>>>>> - Add acked tag by Charles Keepax for arizona changes
>>>>>>>> - Modify the name of USB charger connector as following:
>>>>>>>> : EXTCON_CHG_USB_FAST -> EXTCON_CHG_USB_DCP_FAST
>>>>>>>> : EXTCON_CHG_USB_SLOW -> EXTCON_CHG_USB_DCP_SLOW
>>>>>>>> - Add the missing EXTCON_CHG_USB_ACA charger connector
>>>>>>>> - Add one more patch to support the EXTCON_CHG_USB when SDP port is connected
>>>>>>>> or not
>>>>>>>>
>>>>>>>> drivers/extcon/extcon-arizona.c | 18 ++++++------
>>>>>>>> drivers/extcon/extcon-axp288.c | 12 ++++----
>>>>>>>> drivers/extcon/extcon-max14577.c | 17 +++++------
>>>>>>>> drivers/extcon/extcon-max77693.c | 32 +++++++++++----------
>>>>>>>> drivers/extcon/extcon-max77843.c | 27 ++++++++++--------
>>>>>>>> drivers/extcon/extcon-max8997.c | 21 +++++++-------
>>>>>>>> drivers/extcon/extcon-rt8973a.c | 4 +--
>>>>>>>> drivers/extcon/extcon-sm5502.c | 4 +--
>>>>>>>> drivers/extcon/extcon.c | 60 ++++++++++++++++++++-------------------
>>>>>>>> include/linux/extcon.h | 61 +++++++++++++++++++++++-----------------
>>>>>>>> 10 files changed, 137 insertions(+), 119 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
>>>>>>>> index a1ab0a56b798..e4890dd4fefd 100644
>>>>>>>> --- a/drivers/extcon/extcon-arizona.c
>>>>>>>> +++ b/drivers/extcon/extcon-arizona.c
>>>>>>>> @@ -137,9 +137,9 @@ static const int arizona_micd_levels[] = {
>>>>>>>>
>>>>>>>> static const unsigned int arizona_cable[] = {
>>>>>>>> EXTCON_MECHANICAL,
>>>>>>>> - EXTCON_MICROPHONE,
>>>>>>>> - EXTCON_HEADPHONE,
>>>>>>>> - EXTCON_LINE_OUT,
>>>>>>>> + EXTCON_JACK_MICROPHONE,
>>>>>>>> + EXTCON_JACK_HEADPHONE,
>>>>>>>> + EXTCON_JACK_LINE_OUT,
>>>>>>>> EXTCON_NONE,
>>>>>>>> };
>>>>>>>>
>>>>>>>> @@ -600,7 +600,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>>>>>>>> struct arizona_extcon_info *info = data;
>>>>>>>> struct arizona *arizona = info->arizona;
>>>>>>>> int id_gpio = arizona->pdata.hpdet_id_gpio;
>>>>>>>> - unsigned int report = EXTCON_HEADPHONE;
>>>>>>>> + unsigned int report = EXTCON_JACK_HEADPHONE;
>>>>>>>> int ret, reading;
>>>>>>>> bool mic = false;
>>>>>>>>
>>>>>>>> @@ -645,9 +645,9 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>>>>>>>>
>>>>>>>> /* Report high impedence cables as line outputs */
>>>>>>>> if (reading >= 5000)
>>>>>>>> - report = EXTCON_LINE_OUT;
>>>>>>>> + report = EXTCON_JACK_LINE_OUT;
>>>>>>>> else
>>>>>>>> - report = EXTCON_HEADPHONE;
>>>>>>>> + report = EXTCON_JACK_HEADPHONE;
>>>>>>>>
>>>>>>>> ret = extcon_set_cable_state_(info->edev, report, true);
>>>>>>>> if (ret != 0)
>>>>>>>> @@ -732,7 +732,7 @@ err:
>>>>>>>> ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
>>>>>>>>
>>>>>>>> /* Just report headphone */
>>>>>>>> - ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true);
>>>>>>>> + ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true);
>>>>>>>> if (ret != 0)
>>>>>>>> dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
>>>>>>>>
>>>>>>>> @@ -789,7 +789,7 @@ err:
>>>>>>>> ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
>>>>>>>>
>>>>>>>> /* Just report headphone */
>>>>>>>> - ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true);
>>>>>>>> + ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true);
>>>>>>>> if (ret != 0)
>>>>>>>> dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
>>>>>>>>
>>>>>>>> @@ -915,7 +915,7 @@ static void arizona_micd_detect(struct work_struct *work)
>>>>>>>> arizona_identify_headphone(info);
>>>>>>>>
>>>>>>>> ret = extcon_set_cable_state_(info->edev,
>>>>>>>> - EXTCON_MICROPHONE, true);
>>>>>>>> + EXTCON_JACK_MICROPHONE, true);
>>>>>>>> if (ret != 0)
>>>>>>>> dev_err(arizona->dev, "Headset report failed: %d\n",
>>>>>>>> ret);
>>>>>>>> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
>>>>>>>> index 9668d6a94e38..14e748051288 100644
>>>>>>>> --- a/drivers/extcon/extcon-axp288.c
>>>>>>>> +++ b/drivers/extcon/extcon-axp288.c
>>>>>>>> @@ -102,9 +102,9 @@ enum axp288_extcon_irq {
>>>>>>>> };
>>>>>>>>
>>>>>>>> static const unsigned int axp288_extcon_cables[] = {
>>>>>>>> - EXTCON_SLOW_CHARGER,
>>>>>>>> - EXTCON_CHARGE_DOWNSTREAM,
>>>>>>>> - EXTCON_FAST_CHARGER,
>>>>>>>> + EXTCON_CHG_USB_DCP_SLOW,
>>>>>>>> + EXTCON_CHG_USB_CDP,
>>>>>>>> + EXTCON_CHG_USB_DCP_FAST,
>>>>>>>> EXTCON_NONE,
>>>>>>>> };
>>>>>>>>
>>>>>>>> @@ -192,18 +192,18 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
>>>>>>>> dev_dbg(info->dev, "sdp cable is connecetd\n");
>>>>>>>
>>>>>>> as per comment this seems to be SDP.
>>>>>>
>>>>>> This patch modifies the only connector name.
>>>>>> If some comment should be fixed, we need to implement on separate patch.
>>>>>
>>>>> Agreed.
>>>>>>
>>>>>>>
>>>>>>>> notify_otg = true;
>>>>>>>> notify_charger = true;
>>>>>>>> - cable = EXTCON_SLOW_CHARGER;
>>>>>>>> + cable = EXTCON_CHG_USB_DCP_SLOW;
>>>>>>>
>>>>>>> So shouldn't this be cable = EXTCON_USB?
>>>>>>
>>>>>> No, EXTCON_CHG_USB_*_SLOW is right.
>>>>>>
>>>>>>>
>>>>>>>> break;
>>>>>>>> case DET_STAT_CDP:
>>>>>>>> dev_dbg(info->dev, "cdp cable is connecetd\n");
>>>>>>>> notify_otg = true;
>>>>>>>> notify_charger = true;
>>>>>>>> - cable = EXTCON_CHARGE_DOWNSTREAM;
>>>>>>>> + cable = EXTCON_CHG_USB_CDP;
>>>>>>>> break;
>>>>>>>> case DET_STAT_DCP:
>>>>>>>> dev_dbg(info->dev, "dcp cable is connecetd\n");
>>>>>>>
>>>>>>> This looks like standard DCP.
>>>>>>
>>>>>> ditto. Need the separate patch to fix it.
>>>>>
>>>>> Agreed.
>>>>>
>>>>>>
>>>>>>>
>>>>>>>> notify_charger = true;
>>>>>>>> - cable = EXTCON_FAST_CHARGER;
>>>>>>>> + cable = EXTCON_CHG_USB_DCP_FAST;
>>>>>>>
>>>>>>> Shouldn't this be cable = EXTCON_CHG_USB_DCP?
>>>>>>
>>>>>> No, EXTCON_CHG_USB_*_FAST is right.
>>>>>
>>>>> Why? The case is DET_STAT_DCP.
>>>>
>>>> You're right. I'll fix it as following:
>>>>
>>>> case DET_STAT_SDP -> EXTCON_USB
>>>> case DET_STAT_CDP -> EXTCON_CHG_USB_CDP
>>>> case DET_STAT_DCP -> EXTCON_CHG_USB_DCP
>>>
>>> I think that we need to consider about SDP and CDP port again
>>> because SDP/CDP support the both data transmission and charging at the same time.
>>>
>>> Case 1. if SDP is attached
>>> Following connector type mean the each role of SDP.
>>> - EXTCON_USB for data transmission
>>> - EXTCON_CHG_USB_SDP for charging
>>>
>>> Case 2. if CDP is attached
>>> ditto.
>>> - EXTCON_USB for data transmission
>>> - EXTCON_CHG_USB_CDP for charging
>>>
>>
>> The only issue is that charging and data transmission happen
>> simultaneously. Can the framework allow both states to be
>> active simultaneously?
>
> Yes. Usually, EXTCON_USB is used for USB and PHY framework
> and EXTCON_CHG_USB_SDP/EXTCON_CHG_USB_CDP will be used for power-supply framework.
So USB data detection and Charger detection are separate extcon devices right?
OK in that case having EXTCON_USB as well as EXTCON_CHG_USB_SDP and CDP
make sense.
cheers,
-roger
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v3] extcon: Modify the id and name of external connector
2015-10-07 8:52 ` Roger Quadros
@ 2015-10-07 10:13 ` Chanwoo Choi
0 siblings, 0 replies; 10+ messages in thread
From: Chanwoo Choi @ 2015-10-07 10:13 UTC (permalink / raw)
To: Roger Quadros, linux-kernel
Cc: k.kozlowski, ckeepax, gregkh, ramakrishna.pallala, patches,
myungjoo.ham
Roger,
On 2015년 10월 07일 17:52, Roger Quadros wrote:
> On 07/10/15 11:30, Chanwoo Choi wrote:
>> Roger,
>>
>> On 2015년 10월 07일 17:23, Roger Quadros wrote:
>>> Chanwoo,
>>>
>>> On 07/10/15 11:09, Chanwoo Choi wrote:
>>>> Roger,
>>>>
>>>> On 2015년 10월 07일 16:59, Chanwoo Choi wrote:
>>>>> On 2015년 10월 07일 16:50, Roger Quadros wrote:
>>>>>> On 07/10/15 10:42, Chanwoo Choi wrote:
>>>>>>> On 2015년 10월 07일 16:33, Roger Quadros wrote:
>>>>>>>> On 07/10/15 03:48, Chanwoo Choi wrote:
>>>>>>>>> This patch modifies the id and name of external connector with the
>>>>>>>>> additional prefix to clarify both attribute and meaning of external
>>>>>>>>> connector as following:
>>>>>>>>> - EXTCON_CHG_* mean the charger connector.
>>>>>>>>> - EXTCON_JACK_* mean the jack connector.
>>>>>>>>> - EXTCON_DISP_* mean the display port connector.
>>>>>>>>>
>>>>>>>>> Following table show the new name of external connector with old name:
>>>>>>>>> --------------------------------------------------
>>>>>>>>> Old extcon name | New extcon name |
>>>>>>>>> --------------------------------------------------
>>>>>>>>> EXTCON_TA | EXTCON_CHG_USB_DCP |
>>>>>>>>> EXTCON_CHARGE_DOWNSTREAM| EXTCON_CHG_USB_CDP |
>>>>>>>>> EXTCON_FAST_CHARGER | EXTCON_CHG_USB_DCP_FAST|
>>>>>>>>> EXTCON_SLOW_CHARGER | EXTCON_CHG_USB_DCP_SLOW|
>>>>>>>>
>>>>>>>> I still have some concerns regarding the name DCP_FAST and DCP_SLOW.
>>>>>>>> If I see the charger IC drivers that use FAST & SLOW, they either call it
>>>>>>>> APPLE_500MA/1A or SPECIAL_500MA/1A.
>>>>>>>
>>>>>>> Right.
>>>>>>>
>>>>>>>>
>>>>>>>> They are probably not compliant with USB DCP specification and hence
>>>>>>>> the special name.
>>>>>>>>
>>>>>>>> So, should we call them EXTCON_CHG_USB_SPECIAL_FAST/SLOW instead?
>>>>>>>
>>>>>>> I don't prefer to use "SPECIAL" adjective for connector name.
>>>>>>> I prefer following name at the first patch.
>>>>>>> - EXTCON_CHG_USB_FAST
>>>>>>> - EXTCON_CHG_USB_SLOW
>>>>>>
>>>>>> OK, that is fine with me.
>>>>>>>
>>>>>>>>
>>>>>>>>> --------------------------------------------------
>>>>>>>>> EXTCON_MICROPHONE | EXTCON_JACK_MICROPHONE |
>>>>>>>>> EXTCON_HEADPHONE | EXTCON_JACK_HEADPHONE |
>>>>>>>>> EXTCON_LINE_IN | EXTCON_JACK_LINE_IN |
>>>>>>>>> EXTCON_LINE_OUT | EXTCON_JACK_LINE_OUT |
>>>>>>>>> EXTCON_VIDEO_IN | EXTCON_JACK_VIDEO_IN |
>>>>>>>>> EXTCON_VIDEO_OUT | EXTCON_JACK_VIDEO_OUT |
>>>>>>>>> EXTCON_SPDIF_IN | EXTCON_JACK_SPDIF_IN |
>>>>>>>>> EXTCON_SPDIF_OUT | EXTCON_JACK_SPDIF_OUT |
>>>>>>>>> --------------------------------------------------
>>>>>>>>> EXTCON_HMDI | EXTCON_DISP_HDMI |
>>>>>>>>> EXTCON_MHL | EXTCON_DISP_MHL |
>>>>>>>>> EXTCON_DVI | EXTCON_DISP_DVI |
>>>>>>>>> EXTCON_VGA | EXTCON_DISP_VGA |
>>>>>>>>> --------------------------------------------------
>>>>>>>>>
>>>>>>>>> And, when altering the name of USB charger connector, EXTCON refers to the
>>>>>>>>> "Battery Charging v1.2 Spec and Adopters Agreement"[1] to use the standard
>>>>>>>>> name of USB charging port as following. Following name of USB charging port
>>>>>>>>> are already used in power_supply subsystem. We chan check it on patch[2].
>>>>>>>>> - EXTCON_CHG_USB_DCP /* Dedicated Charging Port */
>>>>>>>>> - EXTCON_CHG_USB_CDP /* Charging Downstream Port */
>>>>>>>>> - EXTCON_CHG_USB_ACA /* Accessory Charger Adapter */
>>>>>>>>>
>>>>>>>>> [1] www.usb.org/developers/docs/devclass_docs/BCv1.2_070312.zip
>>>>>>>>> [2] commit 85efc8a18ced ("power_supply: Add types for USB chargers")
>>>>>>>>>
>>>>>>>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>>>>>> [ckeepax: For the Arizona changes]
>>>>>>>>> Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
>>>>>>>>> ---
>>>>>>>>> Changes from v2:
>>>>>>>>> (https://lkml.org/lkml/2015/10/6/239)
>>>>>>>>> - Remove the EXTCON_CHG_USB type to remove the possible confusion according to
>>>>>>>>> Roger's comment and drop patch2 about EXTCON_CHG_USB.
>>>>>>>>> - Fix the warning issue provided by scripts/checkpatch.pl
>>>>>>>>>
>>>>>>>>> Changes from v1:
>>>>>>>>> (https://lkml.org/lkml/2015/10/3/304)
>>>>>>>>> - Add acked tag by Charles Keepax for arizona changes
>>>>>>>>> - Modify the name of USB charger connector as following:
>>>>>>>>> : EXTCON_CHG_USB_FAST -> EXTCON_CHG_USB_DCP_FAST
>>>>>>>>> : EXTCON_CHG_USB_SLOW -> EXTCON_CHG_USB_DCP_SLOW
>>>>>>>>> - Add the missing EXTCON_CHG_USB_ACA charger connector
>>>>>>>>> - Add one more patch to support the EXTCON_CHG_USB when SDP port is connected
>>>>>>>>> or not
>>>>>>>>>
>>>>>>>>> drivers/extcon/extcon-arizona.c | 18 ++++++------
>>>>>>>>> drivers/extcon/extcon-axp288.c | 12 ++++----
>>>>>>>>> drivers/extcon/extcon-max14577.c | 17 +++++------
>>>>>>>>> drivers/extcon/extcon-max77693.c | 32 +++++++++++----------
>>>>>>>>> drivers/extcon/extcon-max77843.c | 27 ++++++++++--------
>>>>>>>>> drivers/extcon/extcon-max8997.c | 21 +++++++-------
>>>>>>>>> drivers/extcon/extcon-rt8973a.c | 4 +--
>>>>>>>>> drivers/extcon/extcon-sm5502.c | 4 +--
>>>>>>>>> drivers/extcon/extcon.c | 60 ++++++++++++++++++++-------------------
>>>>>>>>> include/linux/extcon.h | 61 +++++++++++++++++++++++-----------------
>>>>>>>>> 10 files changed, 137 insertions(+), 119 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
>>>>>>>>> index a1ab0a56b798..e4890dd4fefd 100644
>>>>>>>>> --- a/drivers/extcon/extcon-arizona.c
>>>>>>>>> +++ b/drivers/extcon/extcon-arizona.c
>>>>>>>>> @@ -137,9 +137,9 @@ static const int arizona_micd_levels[] = {
>>>>>>>>>
>>>>>>>>> static const unsigned int arizona_cable[] = {
>>>>>>>>> EXTCON_MECHANICAL,
>>>>>>>>> - EXTCON_MICROPHONE,
>>>>>>>>> - EXTCON_HEADPHONE,
>>>>>>>>> - EXTCON_LINE_OUT,
>>>>>>>>> + EXTCON_JACK_MICROPHONE,
>>>>>>>>> + EXTCON_JACK_HEADPHONE,
>>>>>>>>> + EXTCON_JACK_LINE_OUT,
>>>>>>>>> EXTCON_NONE,
>>>>>>>>> };
>>>>>>>>>
>>>>>>>>> @@ -600,7 +600,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>>>>>>>>> struct arizona_extcon_info *info = data;
>>>>>>>>> struct arizona *arizona = info->arizona;
>>>>>>>>> int id_gpio = arizona->pdata.hpdet_id_gpio;
>>>>>>>>> - unsigned int report = EXTCON_HEADPHONE;
>>>>>>>>> + unsigned int report = EXTCON_JACK_HEADPHONE;
>>>>>>>>> int ret, reading;
>>>>>>>>> bool mic = false;
>>>>>>>>>
>>>>>>>>> @@ -645,9 +645,9 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>>>>>>>>>
>>>>>>>>> /* Report high impedence cables as line outputs */
>>>>>>>>> if (reading >= 5000)
>>>>>>>>> - report = EXTCON_LINE_OUT;
>>>>>>>>> + report = EXTCON_JACK_LINE_OUT;
>>>>>>>>> else
>>>>>>>>> - report = EXTCON_HEADPHONE;
>>>>>>>>> + report = EXTCON_JACK_HEADPHONE;
>>>>>>>>>
>>>>>>>>> ret = extcon_set_cable_state_(info->edev, report, true);
>>>>>>>>> if (ret != 0)
>>>>>>>>> @@ -732,7 +732,7 @@ err:
>>>>>>>>> ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
>>>>>>>>>
>>>>>>>>> /* Just report headphone */
>>>>>>>>> - ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true);
>>>>>>>>> + ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true);
>>>>>>>>> if (ret != 0)
>>>>>>>>> dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
>>>>>>>>>
>>>>>>>>> @@ -789,7 +789,7 @@ err:
>>>>>>>>> ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
>>>>>>>>>
>>>>>>>>> /* Just report headphone */
>>>>>>>>> - ret = extcon_set_cable_state_(info->edev, EXTCON_HEADPHONE, true);
>>>>>>>>> + ret = extcon_set_cable_state_(info->edev, EXTCON_JACK_HEADPHONE, true);
>>>>>>>>> if (ret != 0)
>>>>>>>>> dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
>>>>>>>>>
>>>>>>>>> @@ -915,7 +915,7 @@ static void arizona_micd_detect(struct work_struct *work)
>>>>>>>>> arizona_identify_headphone(info);
>>>>>>>>>
>>>>>>>>> ret = extcon_set_cable_state_(info->edev,
>>>>>>>>> - EXTCON_MICROPHONE, true);
>>>>>>>>> + EXTCON_JACK_MICROPHONE, true);
>>>>>>>>> if (ret != 0)
>>>>>>>>> dev_err(arizona->dev, "Headset report failed: %d\n",
>>>>>>>>> ret);
>>>>>>>>> diff --git a/drivers/extcon/extcon-axp288.c b/drivers/extcon/extcon-axp288.c
>>>>>>>>> index 9668d6a94e38..14e748051288 100644
>>>>>>>>> --- a/drivers/extcon/extcon-axp288.c
>>>>>>>>> +++ b/drivers/extcon/extcon-axp288.c
>>>>>>>>> @@ -102,9 +102,9 @@ enum axp288_extcon_irq {
>>>>>>>>> };
>>>>>>>>>
>>>>>>>>> static const unsigned int axp288_extcon_cables[] = {
>>>>>>>>> - EXTCON_SLOW_CHARGER,
>>>>>>>>> - EXTCON_CHARGE_DOWNSTREAM,
>>>>>>>>> - EXTCON_FAST_CHARGER,
>>>>>>>>> + EXTCON_CHG_USB_DCP_SLOW,
>>>>>>>>> + EXTCON_CHG_USB_CDP,
>>>>>>>>> + EXTCON_CHG_USB_DCP_FAST,
>>>>>>>>> EXTCON_NONE,
>>>>>>>>> };
>>>>>>>>>
>>>>>>>>> @@ -192,18 +192,18 @@ static int axp288_handle_chrg_det_event(struct axp288_extcon_info *info)
>>>>>>>>> dev_dbg(info->dev, "sdp cable is connecetd\n");
>>>>>>>>
>>>>>>>> as per comment this seems to be SDP.
>>>>>>>
>>>>>>> This patch modifies the only connector name.
>>>>>>> If some comment should be fixed, we need to implement on separate patch.
>>>>>>
>>>>>> Agreed.
>>>>>>>
>>>>>>>>
>>>>>>>>> notify_otg = true;
>>>>>>>>> notify_charger = true;
>>>>>>>>> - cable = EXTCON_SLOW_CHARGER;
>>>>>>>>> + cable = EXTCON_CHG_USB_DCP_SLOW;
>>>>>>>>
>>>>>>>> So shouldn't this be cable = EXTCON_USB?
>>>>>>>
>>>>>>> No, EXTCON_CHG_USB_*_SLOW is right.
>>>>>>>
>>>>>>>>
>>>>>>>>> break;
>>>>>>>>> case DET_STAT_CDP:
>>>>>>>>> dev_dbg(info->dev, "cdp cable is connecetd\n");
>>>>>>>>> notify_otg = true;
>>>>>>>>> notify_charger = true;
>>>>>>>>> - cable = EXTCON_CHARGE_DOWNSTREAM;
>>>>>>>>> + cable = EXTCON_CHG_USB_CDP;
>>>>>>>>> break;
>>>>>>>>> case DET_STAT_DCP:
>>>>>>>>> dev_dbg(info->dev, "dcp cable is connecetd\n");
>>>>>>>>
>>>>>>>> This looks like standard DCP.
>>>>>>>
>>>>>>> ditto. Need the separate patch to fix it.
>>>>>>
>>>>>> Agreed.
>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>> notify_charger = true;
>>>>>>>>> - cable = EXTCON_FAST_CHARGER;
>>>>>>>>> + cable = EXTCON_CHG_USB_DCP_FAST;
>>>>>>>>
>>>>>>>> Shouldn't this be cable = EXTCON_CHG_USB_DCP?
>>>>>>>
>>>>>>> No, EXTCON_CHG_USB_*_FAST is right.
>>>>>>
>>>>>> Why? The case is DET_STAT_DCP.
>>>>>
>>>>> You're right. I'll fix it as following:
>>>>>
>>>>> case DET_STAT_SDP -> EXTCON_USB
>>>>> case DET_STAT_CDP -> EXTCON_CHG_USB_CDP
>>>>> case DET_STAT_DCP -> EXTCON_CHG_USB_DCP
>>>>
>>>> I think that we need to consider about SDP and CDP port again
>>>> because SDP/CDP support the both data transmission and charging at the same time.
>>>>
>>>> Case 1. if SDP is attached
>>>> Following connector type mean the each role of SDP.
>>>> - EXTCON_USB for data transmission
>>>> - EXTCON_CHG_USB_SDP for charging
>>>>
>>>> Case 2. if CDP is attached
>>>> ditto.
>>>> - EXTCON_USB for data transmission
>>>> - EXTCON_CHG_USB_CDP for charging
>>>>
>>>
>>> The only issue is that charging and data transmission happen
>>> simultaneously. Can the framework allow both states to be
>>> active simultaneously?
>>
>> Yes. Usually, EXTCON_USB is used for USB and PHY framework
>> and EXTCON_CHG_USB_SDP/EXTCON_CHG_USB_CDP will be used for power-supply framework.
>
> So USB data detection and Charger detection are separate extcon devices right?
Yes. When SDP is attached or detached, extcon device will update the state of both EXTCON_USB and EXTCON_CHG_USB_SDP.
And when CDP is attached or detached, extcon device will update the state of both EXTCON_USB and EXTCON_CHG_USB_CDP.
>
> OK in that case having EXTCON_USB as well as EXTCON_CHG_USB_SDP and CDP
> make sense.
OK. Thanks for your review and discussion.
Thanks,
Chanwoo Choi
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-10-07 10:13 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-07 0:48 [PATCH v3] extcon: Modify the id and name of external connector Chanwoo Choi
2015-10-07 7:33 ` Roger Quadros
2015-10-07 7:42 ` Chanwoo Choi
2015-10-07 7:50 ` Roger Quadros
2015-10-07 7:59 ` Chanwoo Choi
2015-10-07 8:09 ` Chanwoo Choi
2015-10-07 8:23 ` Roger Quadros
2015-10-07 8:30 ` Chanwoo Choi
2015-10-07 8:52 ` Roger Quadros
2015-10-07 10:13 ` Chanwoo Choi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).