Re: [v5 PATCH 1/5] extcon: Add Type-C and DP support
From: Chris Zhong
Date: Wed Jul 13 2016 - 21:04:04 EST
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 <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
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",
+ },