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. > > 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 at rock-chips.com> >> >> Signed-off-by: Chris Zhong <zyw at rock-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. Thanks Chris > >> + return true; >> default: >> break; >> } >> @@ -547,6 +555,26 @@ int extcon_get_cable_property(struct extcon_dev *edev, unsigned int id, >> enum extcon_property prop, >> union extcon_property_value *val) >> { >> + struct extcon_cable *cable; >> + int index; >> + >> + if (!edev) >> + return -EINVAL; >> + >> + /* Check the property whether is supported or not */ >> + if (!is_extcon_property_supported(id, prop)) >> + return -EINVAL; >> + >> + /* Find the cable index of external connector by using id */ >> + index = find_cable_index_by_id(edev, id); >> + if (index < 0) >> + return index; >> + >> + /* Store the property value */ >> + cable = &edev->cables[index]; >> + >> + val->intval = cable->propval[prop].intval; >> + >> return 0; >> } > After I develop it about get_cable_property, I'll send RFC patchset. > >> >> diff --git a/include/linux/extcon.h b/include/linux/extcon.h >> index f6f0a8d..50ef87f 100644 >> --- a/include/linux/extcon.h >> +++ b/include/linux/extcon.h >> @@ -77,6 +77,7 @@ enum extcon_type { >> #define EXTCON_DISP_MHL 41 /* Mobile High-Definition Link */ >> #define EXTCON_DISP_DVI 42 /* Digital Visual Interface */ >> #define EXTCON_DISP_VGA 43 /* Video Graphics Array */ >> +#define EXTCON_DISP_DP 44 /* DisplayPort */ >> >> /* Miscellaneous external connector */ >> #define EXTCON_DOCK 60 >> @@ -108,9 +109,13 @@ enum extcon_property { >> * - EXTCON_PROP_USB_USB >> * @type: integer (int value) >> * @value: 0 (low) or 1 (high) >> + * - EXTCON_PROP_TYPEC_POLARITY, >> + * @type: integer (int value) >> + * @value: 0 (normal) or 1 (flip) >> */ >> EXTCON_PROP_USB_ID = 0, >> EXTCON_PROP_USB_VBUS, >> + EXTCON_PROP_TYPEC_POLARITY, >> >> /* Properties of EXTCON_TYPE_CHG. */ >> /* Properties of EXTCON_TYPE_JACK. */ >> @@ -225,6 +230,14 @@ extern int extcon_get_cable_state_(struct extcon_dev *edev, unsigned int id); >> extern int extcon_set_cable_state_(struct extcon_dev *edev, unsigned int id, >> bool cable_state); >> >> +extern int extcon_get_cable_property(struct extcon_dev *edev, unsigned int id, >> + enum extcon_property prop, >> + union extcon_property_value *val); >> + >> +extern int extcon_set_cable_property(struct extcon_dev *edev, unsigned int id, >> + enum extcon_property prop, >> + union extcon_property_value val); >> + > ditto. > > Thanks, > Chanwoo Choi > > >