diff mbox

[v6,1/6] extcon: Add Type-C and DP support

Message ID 1469106808-28297-2-git-send-email-zyw@rock-chips.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Zhong July 21, 2016, 1:13 p.m. UTC
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 v6:
- move the EXTCON_PROP_TYPEC_POLARITY to EXTCON_TYPE_USB in _supported
Series-changes: 5
- support get property

Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
Changes in v1: None

 drivers/extcon/extcon.c | 26 ++++++++++++++++++++++++++
 include/linux/extcon.h  | 13 +++++++++++++
 2 files changed, 39 insertions(+)

Comments

Chanwoo Choi July 22, 2016, 9:29 a.m. UTC | #1
Hi Chris,

I'm sorry for late reply. I finished the first draft to support the extcon property.
You can check the patches[1]. But, I need more time to test it. After tested it,
I'll send the patches.

[1] https://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/log/?h=extcon-test

Chanwoo Choi (4):
  extcon: Add the extcon_type to group each connector into five category
  extcon: Add the support for extcon property according to type of connector
  extcon: Rename the extcon_set/get_state() to maintain the function naming pattern
  extcon: Add the sync APIs to support the  notification for extcon property

Chris Zhong (1):
  extcon: Add EXTCON_DISP_DP and the property for USB Type-C

Regards,
Chanwoo Choi

On 2016년 07월 21일 22:13, 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 v6:
> - move the EXTCON_PROP_TYPEC_POLARITY to EXTCON_TYPE_USB in _supported
> Series-changes: 5
> - support get property
> 
> Changes in v5: None
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
> Changes in v1: None
> 
>  drivers/extcon/extcon.c | 26 ++++++++++++++++++++++++++
>  include/linux/extcon.h  | 13 +++++++++++++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index a1117db..f79b510 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 | EXTCON_TYPE_USB,
> +		.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;
> @@ -547,6 +553,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;
>  }
>  
> 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);
> +
>  /*
>   * Following APIs are to monitor every action of a notifier.
>   * Registrar gets notified for every external port of a connection device.
>
Chris Zhong July 22, 2016, 9:40 a.m. UTC | #2
Hi Chanwoo Choi

Thanks for your help, I am going to post V7 next week base on these 
latest extcon patch.

Regards
Chris Zhong

On 07/22/2016 05:29 PM, Chanwoo Choi wrote:
> Hi Chris,
>
> I'm sorry for late reply. I finished the first draft to support the extcon property.
> You can check the patches[1]. But, I need more time to test it. After tested it,
> I'll send the patches.
>
> [1] https://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/log/?h=extcon-test
>
> Chanwoo Choi (4):
>    extcon: Add the extcon_type to group each connector into five category
>    extcon: Add the support for extcon property according to type of connector
>    extcon: Rename the extcon_set/get_state() to maintain the function naming pattern
>    extcon: Add the sync APIs to support the  notification for extcon property
>
> Chris Zhong (1):
>    extcon: Add EXTCON_DISP_DP and the property for USB Type-C
>
> Regards,
> Chanwoo Choi
>
> On 2016년 07월 21일 22:13, 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 v6:
>> - move the EXTCON_PROP_TYPEC_POLARITY to EXTCON_TYPE_USB in _supported
>> Series-changes: 5
>> - support get property
>>
>> Changes in v5: None
>> Changes in v4: None
>> Changes in v3: None
>> Changes in v2: None
>> Changes in v1: None
>>
>>   drivers/extcon/extcon.c | 26 ++++++++++++++++++++++++++
>>   include/linux/extcon.h  | 13 +++++++++++++
>>   2 files changed, 39 insertions(+)
>>
>> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
>> index a1117db..f79b510 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 | EXTCON_TYPE_USB,
>> +		.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;
>> @@ -547,6 +553,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;
>>   }
>>   
>> 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);
>> +
>>   /*
>>    * Following APIs are to monitor every action of a notifier.
>>    * Registrar gets notified for every external port of a connection device.
>>
>
>
>
Guenter Roeck July 22, 2016, 5:23 p.m. UTC | #3
To create a baseline, I'll pick up the changes from the extcon
repository and make them available in chromeos-4.4.

Guenter

On Fri, Jul 22, 2016 at 2:40 AM, Chris Zhong <zyw@rock-chips.com> wrote:
> Hi Chanwoo Choi
>
> Thanks for your help, I am going to post V7 next week base on these latest
> extcon patch.
>
> Regards
> Chris Zhong
>
>
> On 07/22/2016 05:29 PM, Chanwoo Choi wrote:
>>
>> Hi Chris,
>>
>> I'm sorry for late reply. I finished the first draft to support the extcon
>> property.
>> You can check the patches[1]. But, I need more time to test it. After
>> tested it,
>> I'll send the patches.
>>
>> [1]
>> https://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/log/?h=extcon-test
>>
>> Chanwoo Choi (4):
>>    extcon: Add the extcon_type to group each connector into five category
>>    extcon: Add the support for extcon property according to type of
>> connector
>>    extcon: Rename the extcon_set/get_state() to maintain the function
>> naming pattern
>>    extcon: Add the sync APIs to support the  notification for extcon
>> property
>>
>> Chris Zhong (1):
>>    extcon: Add EXTCON_DISP_DP and the property for USB Type-C
>>
>> Regards,
>> Chanwoo Choi
>>
>> On 2016년 07월 21일 22:13, 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 v6:
>>> - move the EXTCON_PROP_TYPEC_POLARITY to EXTCON_TYPE_USB in _supported
>>> Series-changes: 5
>>> - support get property
>>>
>>> Changes in v5: None
>>> Changes in v4: None
>>> Changes in v3: None
>>> Changes in v2: None
>>> Changes in v1: None
>>>
>>>   drivers/extcon/extcon.c | 26 ++++++++++++++++++++++++++
>>>   include/linux/extcon.h  | 13 +++++++++++++
>>>   2 files changed, 39 insertions(+)
>>>
>>> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
>>> index a1117db..f79b510 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 | EXTCON_TYPE_USB,
>>> +               .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;
>>> @@ -547,6 +553,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;
>>>   }
>>>   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);
>>> +
>>>   /*
>>>    * Following APIs are to monitor every action of a notifier.
>>>    * Registrar gets notified for every external port of a connection
>>> device.
>>>
>>
>>
>>
>
>
Guenter Roeck July 22, 2016, 6:21 p.m. UTC | #4
Hi,

On Fri, Jul 22, 2016 at 2:29 AM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> Hi Chris,
>
> I'm sorry for late reply. I finished the first draft to support the extcon property.
> You can check the patches[1]. But, I need more time to test it. After tested it,
> I'll send the patches.
>
> [1] https://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/log/?h=extcon-test
>
> Chanwoo Choi (4):
>   extcon: Add the extcon_type to group each connector into five category
>   extcon: Add the support for extcon property according to type of connector
>   extcon: Rename the extcon_set/get_state() to maintain the function naming pattern
>   extcon: Add the sync APIs to support the  notification for extcon property
>
> Chris Zhong (1):
>   extcon: Add EXTCON_DISP_DP and the property for USB Type-C
>

Couple of comments:

- In extcon.c, line 560: 'state' is unused.

- extcon_set_property(edev, EXTCON_USB, EXTCON_USB_VBUS, 0);
  does not work. As written, it has to be something like:

  union extcon_property_value prop;
  ...
  prop.intval = vbus_state;
  extcon_set_property(edev, EXTCON_USB, EXTCON_USB_VBUS, prop);

- For USB, the state of EXTCON_USB, EXTCON_USB_HOST, and
EXTCON_DISP_DP tend to change at the same time, together with the
associated properties.
  It might be desirable to have an equivalent of extcon_sync_all(edev);

Thanks,
Guenter
Guenter Roeck July 22, 2016, 9:40 p.m. UTC | #5
On Fri, Jul 22, 2016 at 2:29 AM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
> Hi Chris,
>
> I'm sorry for late reply. I finished the first draft to support the extcon property.
> You can check the patches[1]. But, I need more time to test it. After tested it,
> I'll send the patches.
>
> [1] https://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/log/?h=extcon-test
>
> Chanwoo Choi (4):
>   extcon: Add the extcon_type to group each connector into five category
>   extcon: Add the support for extcon property according to type of connector
>   extcon: Rename the extcon_set/get_state() to maintain the function naming pattern
>   extcon: Add the sync APIs to support the  notification for extcon property
>

Some additional feedback:

- IS_PROPERTY(), is_extcon_property_supported():
   The logic should probably be
        if (EXTCON_TYPE_##name & type) {
   and
        if (!(extcon_info[id].type & type)) {

  (logical & instead of logical | )

- extcon_get_property():
  - ret is not initialized.
  - case EXTCON_PROP_DISP_MIN ... EXTCON_PROP_DISP_MAX:
    misses a break statement

Guenter

> Chris Zhong (1):
>   extcon: Add EXTCON_DISP_DP and the property for USB Type-C
>
> Regards,
> Chanwoo Choi
>
> On 2016년 07월 21일 22:13, 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 v6:
>> - move the EXTCON_PROP_TYPEC_POLARITY to EXTCON_TYPE_USB in _supported
>> Series-changes: 5
>> - support get property
>>
>> Changes in v5: None
>> Changes in v4: None
>> Changes in v3: None
>> Changes in v2: None
>> Changes in v1: None
>>
>>  drivers/extcon/extcon.c | 26 ++++++++++++++++++++++++++
>>  include/linux/extcon.h  | 13 +++++++++++++
>>  2 files changed, 39 insertions(+)
>>
>> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
>> index a1117db..f79b510 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 | EXTCON_TYPE_USB,
>> +             .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;
>> @@ -547,6 +553,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;
>>  }
>>
>> 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);
>> +
>>  /*
>>   * Following APIs are to monitor every action of a notifier.
>>   * Registrar gets notified for every external port of a connection device.
>>
>
Chanwoo Choi July 23, 2016, 2:19 a.m. UTC | #6
Hi,

2016-07-23 3:21 GMT+09:00 Guenter Roeck <groeck@google.com>:
> Hi,
>
> On Fri, Jul 22, 2016 at 2:29 AM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
>> Hi Chris,
>>
>> I'm sorry for late reply. I finished the first draft to support the extcon property.
>> You can check the patches[1]. But, I need more time to test it. After tested it,
>> I'll send the patches.
>>
>> [1] https://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/log/?h=extcon-test
>>
>> Chanwoo Choi (4):
>>   extcon: Add the extcon_type to group each connector into five category
>>   extcon: Add the support for extcon property according to type of connector
>>   extcon: Rename the extcon_set/get_state() to maintain the function naming pattern
>>   extcon: Add the sync APIs to support the  notification for extcon property
>>
>> Chris Zhong (1):
>>   extcon: Add EXTCON_DISP_DP and the property for USB Type-C
>>
>
> Couple of comments:
>
> - In extcon.c, line 560: 'state' is unused.
>
> - extcon_set_property(edev, EXTCON_USB, EXTCON_USB_VBUS, 0);
>   does not work. As written, it has to be something like:

you shoud use the 'union extcon_property_valu' as below.

>
>   union extcon_property_value prop;
>   ...
>   prop.intval = vbus_state;
>   extcon_set_property(edev, EXTCON_USB, EXTCON_USB_VBUS, prop);
>
> - For USB, the state of EXTCON_USB, EXTCON_USB_HOST, and
> EXTCON_DISP_DP tend to change at the same time, together with the
> associated properties.
>   It might be desirable to have an equivalent of extcon_sync_all(edev);

Each external connector has the separate notifier independently.
All of operation in extcon should be done by each external connector unit.

I think that extcon_sync_all() is not appropriate. If the some extcon device
support the many connectors, maybe extcon send the un-needed notifications
for all connectors.

Also, extcon_sync_all(edev) is same operation with follwoing three
function calls:
extcon_sync(edev, EXTCON_USB);
extcon_sync(edev, EXTCON_USB_HOST);
extcon_sync(edev, EXTCON_DISP_DP);

Also,
EXTCON_DISP_DP has the both EXTCON_TYPE_USB and EXTCON_TYPE_DISP.
So, the extcon client driver is able to set the vbus property with
EXTCON_DISP_DP as following:
- extcon_set_property(edev, EXTCON_DISP_DP, EXTCON_USB_VBUS, prop)

Thanks,
Chanwoo Choi
Chanwoo Choi July 23, 2016, 2:20 a.m. UTC | #7
Hi,

2016-07-23 6:40 GMT+09:00 Guenter Roeck <groeck@google.com>:
> On Fri, Jul 22, 2016 at 2:29 AM, Chanwoo Choi <cw00.choi@samsung.com> wrote:
>> Hi Chris,
>>
>> I'm sorry for late reply. I finished the first draft to support the extcon property.
>> You can check the patches[1]. But, I need more time to test it. After tested it,
>> I'll send the patches.
>>
>> [1] https://git.kernel.org/cgit/linux/kernel/git/chanwoo/extcon.git/log/?h=extcon-test
>>
>> Chanwoo Choi (4):
>>   extcon: Add the extcon_type to group each connector into five category
>>   extcon: Add the support for extcon property according to type of connector
>>   extcon: Rename the extcon_set/get_state() to maintain the function naming pattern
>>   extcon: Add the sync APIs to support the  notification for extcon property
>>
>
> Some additional feedback:
>
> - IS_PROPERTY(), is_extcon_property_supported():
>    The logic should probably be
>         if (EXTCON_TYPE_##name & type) {
>    and
>         if (!(extcon_info[id].type & type)) {
>
>   (logical & instead of logical | )
>
> - extcon_get_property():
>   - ret is not initialized.
>   - case EXTCON_PROP_DISP_MIN ... EXTCON_PROP_DISP_MAX:
>     misses a break statement

Thanks for report. I'll check it.

Thanks,
Chanwoo Choi

>
> Guenter
>
>> Chris Zhong (1):
>>   extcon: Add EXTCON_DISP_DP and the property for USB Type-C
>>
>> Regards,
>> Chanwoo Choi
>>
>> On 2016년 07월 21일 22:13, 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 v6:
>>> - move the EXTCON_PROP_TYPEC_POLARITY to EXTCON_TYPE_USB in _supported
>>> Series-changes: 5
>>> - support get property
>>>
>>> Changes in v5: None
>>> Changes in v4: None
>>> Changes in v3: None
>>> Changes in v2: None
>>> Changes in v1: None
>>>
>>>  drivers/extcon/extcon.c | 26 ++++++++++++++++++++++++++
>>>  include/linux/extcon.h  | 13 +++++++++++++
>>>  2 files changed, 39 insertions(+)
>>>
>>> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
>>> index a1117db..f79b510 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 | EXTCON_TYPE_USB,
>>> +             .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;
>>> @@ -547,6 +553,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;
>>>  }
>>>
>>> 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);
>>> +
>>>  /*
>>>   * Following APIs are to monitor every action of a notifier.
>>>   * Registrar gets notified for every external port of a connection device.
>>>
>>
diff mbox

Patch

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index a1117db..f79b510 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 | EXTCON_TYPE_USB,
+		.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;
@@ -547,6 +553,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;
 }
 
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);
+
 /*
  * Following APIs are to monitor every action of a notifier.
  * Registrar gets notified for every external port of a connection device.