From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751657AbcGNBDx (ORCPT ); Wed, 13 Jul 2016 21:03:53 -0400 Received: from regular1.263xmail.com ([211.150.99.130]:34114 "EHLO regular1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751530AbcGNBDr (ORCPT ); Wed, 13 Jul 2016 21:03:47 -0400 X-263anti-spam: KSV:0; X-MAIL-GRAY: 0 X-MAIL-DELIVERY: 1 X-KSVirus-check: 0 X-ABS-CHECKED: 4 X-ADDR-CHECKED: 0 X-RL-SENDER: zyw@rock-chips.com X-FST-TO: linux-kernel@vger.kernel.org X-SENDER-IP: 103.29.142.67 X-LOGIN-NAME: zyw@rock-chips.com X-UNIQUE-TAG: X-ATTACHMENT-NUM: 0 X-DNS-TYPE: 0 Subject: Re: [v5 PATCH 1/5] extcon: Add Type-C and DP support To: Chanwoo Choi , dianders@chromium.org, tfiga@chromium.org, heiko@sntech.de, yzq@rock-chips.com, groeck@chromium.org, myungjoo.ham@samsung.com, wulf@rock-chips.com, marcheu@chromium.org References: <1468336188-565-1-git-send-email-zyw@rock-chips.com> <1468336188-565-2-git-send-email-zyw@rock-chips.com> <57859532.1010002@samsung.com> <57859BDA.7080309@rock-chips.com> <5785A1F3.1020703@samsung.com> <5785AD61.3070307@rock-chips.com> <5786E1B6.2000805@samsung.com> Cc: linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org From: Chris Zhong Message-ID: <5786E4E7.30505@rock-chips.com> Date: Thu, 14 Jul 2016 09:03:35 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: <5786E1B6.2000805@samsung.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Chanwoo Choi On 07/14/2016 08:49 AM, Chanwoo Choi wrote: > Hi Chris, > > On 2016년 07월 13일 11:54, Chris Zhong wrote: >> 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 >>>>>> >>>>>> Signed-off-by: Chris Zhong >>>>>> --- >>>>>> >>>>>> 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); > I'm thinking to solve this issue. In result, we can add one more type to specific connector. > For EXTCON_DISP_DP, we can add the two type as following. And EXTCON_PROP_TYPEC_POLARITY property > should be added to EXTCON_TYPE_USB. > > + [EXTCON_DISP_DP] = { > + .type = EXTCON_TYPE_DISP | EXTCON_TYPE_USB, > + .id = EXTCON_DISP_DP, > + .name = "DP", > + }, > > So, as you mentioned, EXTCON_DISP_DP can set the EXTCON_PROP_TYPEC_POLARITY property as following: > - extcon_set_cable_state(edev, EXTCON_DISP_DP, EXTCON_PROP_TYPEC_POLARITY, 0 or 1); > > I'll again developing the extcon property. > > Thanks, > Chanwoo Choi > Nice idea, But I am thinking about is it compatible with real DisplayPort without Type-C. Maybe we should add a new cable: EXTCON_DISP_DP_ALT + [EXTCON_DISP_DP] = { + .type = EXTCON_TYPE_DISP, + .id = EXTCON_DISP_DP, + .name = "DP", + }, + [EXTCON_DISP_DP_ALT] = { + .type = EXTCON_TYPE_DISP | EXTCON_TYPE_USB, + .id = EXTCON_DISP_DP, + .name = "DP ALT", + }, > >