Re: [v5 PATCH 1/5] extcon: Add Type-C and DP support

[Chanwoo Choi]

Hi Chris,

On 2016ë 07ì 13ì 10:39, Chris Zhong wrote:
> Hi Chanwoo Choi
> 
> 
> On 07/13/2016 09:11 AM, Chanwoo Choi wrote:
>> Hi Chris,
>>
>> I'm now developing the extcon property on extcon-test branch.
>> But, it has not been completed.
>>
>> On next version, I'll remove the notification about extcon property
>> and only support the following two functions.
>> - extcon_set_cable_property()
>> - extcon_get_cable_property()
>>
>> Because the number of properties would be risen and the all properties
>> depend on the specific external connector(e.g., EXTCON_PROP_USB_VBUS
>> depend on the EXTCON_TYPE_USB type). When the specific external connector
>> is detached, extcon framework should make the property state as default state.
> 
> Yes, I think getting the notification from cable state is enough, actually I am using it like you said.

OK. 

> 
>>
>> It may send the too many notification for extcon property.
>> For example, Assume that EXTCON_TYPE_USB has the over 20 properties,
>> when EXTCON_USB or EXTCON_USB_HOST is detached, extcon should send
>> the notification for the over 20 properties and one more notificaiton
>> for state of external connector.
>>
>> So, I'll send the RFC patchset without the notification of proerty.
>>
>> Lastly,
>> I have a comment on below.
>>
>> Thanks,
>> Chanwoo Choi
>>
>> On 2016ë 07ì 13ì 00:09, Chris Zhong wrote:
>>> Add EXTCON_DISP_DP for the Display external connector. For Type-C
>>> connector the DisplayPort can work as an Alternate Mode(VESA DisplayPort
>>> Alt Mode on USB Type-C Standard). The Type-C support both normal and
>>> flipped orientation, so add a property to extcon.
>>>
>>> Signe-off-by: Chris Zhong <zyw@xxxxxxxxxxxxxx>
>>>
>>> Signed-off-by: Chris Zhong <zyw@xxxxxxxxxxxxxx>
>>> ---
>>>
>>> Changes in v5:
>>> - support get property
>>>
>>> Changes in v4: None
>>> Changes in v3: None
>>> Changes in v2: None
>>> Changes in v1: None
>>>
>>>   drivers/extcon/extcon.c | 28 ++++++++++++++++++++++++++++
>>>   include/linux/extcon.h  | 13 +++++++++++++
>>>   2 files changed, 41 insertions(+)
>>>
>>> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
>>> index a1117db..2591b28 100644
>>> --- a/drivers/extcon/extcon.c
>>> +++ b/drivers/extcon/extcon.c
>>> @@ -157,6 +157,11 @@ struct __extcon_info {
>>>           .id = EXTCON_DISP_VGA,
>>>           .name = "VGA",
>>>       },
>>> +    [EXTCON_DISP_DP] = {
>>> +        .type = EXTCON_TYPE_DISP,
>>> +        .id = EXTCON_DISP_DP,
>>> +        .name = "DP",
>>> +    },
>>>         /* Miscellaneous external connector */
>>>       [EXTCON_DOCK] = {
>>> @@ -270,6 +275,7 @@ static bool is_extcon_property_supported(unsigned int id,
>>>           switch (prop) {
>>>           case EXTCON_PROP_USB_ID:
>>>           case EXTCON_PROP_USB_VBUS:
>>> +        case EXTCON_PROP_TYPEC_POLARITY:
>>>               return true;
>>>           default:
>>>               break;
>>> @@ -286,6 +292,8 @@ static bool is_extcon_property_supported(unsigned int id,
>>>           }
>>>       case EXTCON_TYPE_DISP:
>>>           switch (prop) {
>>> +        case EXTCON_PROP_TYPEC_POLARITY:
>> Should EXTCON_PROP_TYPEC_POLARITY property add to both EXTCON_TYPE_USB and EXTCON_TYP_DISP?
>> EXTCON_PROP_TYPEC_POLARITY is the property of USB C-type?
> 
> it is for USB Type-C, But at Display Port alt mode, both EXTCON_USB and EXTCON_USB_HOST may be detached. Does it support set the property to a detached cable, if so, I think move this case to EXTCON_USB is fine.

One external connector can set the state of one more external connector
if the one connector support the various functions.
For example, EXTCON_USB and EXTCON_CHG_USB_SDP
The existing extcon driver[1](e.g., max14577/max77693 etc.) set the state of both EXTCON_USB and EXTCON_CHG_USB_SDP connector at the same time
when usb cable is attached. Because in this case, the usb connector uses as both power supply(EXTCON_CHG_USB_SDP) and data transfer(EXTCON_USB).
[1] https://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/commit/?h=extcon-next&id=8b45b6a0741678902810d7be95e635c210fbb198

So, DP Alt mode uses the USB Type-C. So, When USB C-type connector is attached for DP Alt mode,
Maybe, you can set the following two state of connector and one property:
- extcon_set_cable_state(edev, [EXTCON_USB or EXTCON_USB_HOST], 1);
- extcon_set_cable_state(edev, EXTCON_DISP_DP, 1);
- extcon_set_cable_state(edev, [EXTCON_USB or EXTCON_USB_HOST], EXTCON_PROP_TYPEC_POLARITY, 0 or 1);

Thanks,
Chanwoo Choi