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

[Chanwoo Choi]

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.

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?

> +			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