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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5785AD61.3070307@rock-chips.com>
Date:	Wed, 13 Jul 2016 10:54:25 +0800
From:	Chris Zhong <zyw@...k-chips.com>
To:	Chanwoo Choi <cw00.choi@...sung.com>, dianders@...omium.org,
	tfiga@...omium.org, heiko@...ech.de, yzq@...k-chips.com,
	groeck@...omium.org, myungjoo.ham@...sung.com, wulf@...k-chips.com,
	marcheu@...omium.org
Cc:	linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [v5 PATCH 1/5] extcon: Add Type-C and DP support

Hi Chanwoo Choi

On 07/13/2016 10:05 AM, Chanwoo Choi wrote:
> 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@...k-chips.com>
>>>>
>>>> Signed-off-by: Chris Zhong <zyw@...k-chips.com>
>>>> ---
>>>>
>>>> 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

There are 4 modes for Type-C DP alt mode:
1) USB host only  :

extcon_set_cable_state(edev, EXTCON_USB_HOST, 1);
extcon_set_cable_state(edev, EXTCON_USB, 0);
extcon_set_cable_state(edev, EXTCON_DISP_DP, 0);

2) USB device only

extcon_set_cable_state(edev, EXTCON_USB_HOST, 0);
extcon_set_cable_state(edev, EXTCON_USB, 1);
extcon_set_cable_state(edev, EXTCON_DISP_DP, 0);

3) DP only

extcon_set_cable_state(edev, EXTCON_USB_HOST, 0);
extcon_set_cable_state(edev, EXTCON_USB, 0);
extcon_set_cable_state(edev, EXTCON_DISP_DP, 1);

4) USB + DP

extcon_set_cable_state(edev, EXTCON_USB_HOST, 1);
extcon_set_cable_state(edev, EXTCON_USB, 0);
extcon_set_cable_state(edev, EXTCON_DISP_DP, 1);


for 3rd mode: DP only, there is only EXTCON_DISP_DP is attached, the 
EXTCON_USB_HOST
and EXTCON_USB are detached, Can I set the property into these 2 
detached cable?
or just call extcon_set_cable_state(edev, EXTCON_DISP_DP, 
EXTCON_PROP_TYPEC_POLARITY, 0 or 1);

>
>
>
>