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.c...@samsung.com>
>>> [ckeepax: For the Arizona changes]
>>> Acked-by: Charles Keepax <ckee...@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.

cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to