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

[Chris Zhong <zyw-TNX95d0MmH7DzftRWevZcw@public.gmane.org>]

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@rock-chips.com>
>>
>> Signed-off-by: Chris Zhong <zyw@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
>
>
>



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip