Messages in this thread | | | Date | Thu, 15 Oct 2015 17:39:59 +0900 | From | Chanwoo Choi <> | Subject | Re: [PATCH v4] extcon: Modify the id and name of external connector |
| |
Roger,
On 2015년 10월 15일 16:16, Roger Quadros wrote: > Chanwoo, > > On 15/10/15 05:22, Chanwoo Choi wrote: >> Hi Roger, >> >> On 2015년 10월 14일 16:13, Roger Quadros wrote: >>> Chanwoo, >>> >>> On 08/10/15 12:24, 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_FAST | >>>> EXTCON_SLOW_CHARGER | 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_SDP /* Standard Downstream Port */ >>>> - 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 v3: >>>> (https://lkml.org/lkml/2015/10/6/984) >>>> - Modify the name of fast/slow charger connector as following: >>>> : EXTCON_CHG_USB_DCP_FAST -> EXTCON_CHG_USB_FAST >>>> : EXTCON_CHG_USB_DCP_SLOW -> EXTCON_CHG_USB_SLOW >>>> - Add EXTCON_CHG_USB_SDP to mean the charging connector of SDP (Standard >>>> Downstream Port) >>>> >>>> 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 | 61 ++++++++++++++++++++------------------- >>>> include/linux/extcon.h | 62 +++++++++++++++++++++++----------------- >>>> 10 files changed, 139 insertions(+), 119 deletions(-) >>>> > > <snip> > >>>> 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..f345d492d4a1 100644 >>>> --- a/drivers/extcon/extcon.c >>>> +++ b/drivers/extcon/extcon.c >>>> @@ -39,37 +39,40 @@ >>>> #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", >>> >>> Should the name string be "USB-PERIPHERAL"? >> >> I think 'PERIPHERAL' is not necessary. The extcon name is >> used for only end user using the platform developer. >> 'PERIPHERAL' might cause the confusion to end user. >> > OK. >>> >>>> + [EXTCON_USB_HOST] = "EXTCON_USB_HOST", >>> >>> Why prefix EXTCON and change hyphen to underscore? >>> Wasn't the original version i.e. "USB-HOST" better? >> >> Agreee. >> >>> >>> Why the change in the name strings? Who is the end user of the name string? >>> If the end use is just for information to a human user then the human readable >>> format makes more sense. i.e. "MHL" or "MICROPHONE" makes more sense than >>> "EXTCON_DISP_MHL" or "EXTCON_JACK_MICROPHONE" >> >> Your comment make sense. I'll not modify the name of external connector. >> I'll use the existing name. >> > > OK. Sorry for not pointing this out earlier.
No problem.
Thanks for your review.
Regards, Chanwoo Choi
| |