Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-29 Thread Chanwoo Choi 
Hi Guenter,

On 2016년 07월 28일 02:24, Guenter Roeck wrote:
> On Tue, Jul 26, 2016 at 5:09 AM, Chanwoo Choi  wrote:
>> This patch support the extcon property for the external connector
>> because each external connector might have the property according to
>> the H/W design and the specific characteristics.
>>
>> - EXTCON_PROP_USB_[property name]
>> - EXTCON_PROP_CHG_[property name]
>> - EXTCON_PROP_JACK_[property name]
>> - EXTCON_PROP_DISP_[property name]
>>
>> Add the new extcon APIs to get/set the property value as following:
>> - int extcon_get_property(struct extcon_dev *edev, unsigned int id,
>> unsigned int prop,
>> union extcon_property_value *prop_val)
>> - int extcon_set_property(struct extcon_dev *edev, unsigned int id,
>> unsigned int prop,
>> union extcon_property_value prop_val)
>>
>> Signed-off-by: Chanwoo Choi 
>> ---
>>  drivers/extcon/extcon.c | 206 
>> +++-
>>  include/linux/extcon.h  |  86 
>>  2 files changed, 291 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
>> index b1e6ee6194bc..2317aaea063f 100644
>> --- a/drivers/extcon/extcon.c
>> +++ b/drivers/extcon/extcon.c
>> @@ -196,6 +196,11 @@ struct extcon_cable {
>> struct device_attribute attr_state;
>>
>> struct attribute *attrs[3]; /* to be fed to attr_g.attrs */
>> +
>> +   union extcon_property_value usb_propval[EXTCON_PROP_USB_CNT];
>> +   union extcon_property_value chg_propval[EXTCON_PROP_CHG_CNT];
>> +   union extcon_property_value jack_propval[EXTCON_PROP_JACK_CNT];
>> +   union extcon_property_value disp_propval[EXTCON_PROP_DISP_CNT];
>>  };
>>
>>  static struct class *extcon_class;
>> @@ -248,6 +253,28 @@ static int find_cable_index_by_id(struct extcon_dev 
>> *edev, const unsigned int id
>> return -EINVAL;
>>  }
>>
>> +static int get_extcon_type(unsigned int prop)
>> +{
>> +   switch (prop) {
>> +   case EXTCON_PROP_USB_MIN ... EXTCON_PROP_USB_MAX:
>> +   return EXTCON_TYPE_USB;
>> +   case EXTCON_PROP_CHG_MIN ... EXTCON_PROP_CHG_MAX:
>> +   return EXTCON_TYPE_CHG;
>> +   case EXTCON_PROP_JACK_MIN ... EXTCON_PROP_JACK_MAX:
>> +   return EXTCON_TYPE_JACK;
>> +   case EXTCON_PROP_DISP_MIN ... EXTCON_PROP_DISP_MAX:
>> +   return EXTCON_TYPE_DISP;
>> +   default:
>> +   return -EINVAL;
>> +   }
>> +}
>> +
>> +static bool is_extcon_attached(struct extcon_dev *edev, unsigned int id,
> 
> 'id' isn't used.

I'll remove it on parameter list.

> 
>> +   unsigned int index)
>> +{
>> +   return ((!!(edev->state & (1 << index))) == 1) ? true : false;
> 
> Minor comment: This is identical to
> 
>  return !!(edev->state & (1 << index));
> or, with bitops
>  return !!(edev->state & BIT(index));

I'll use the bitops as you comment.

return !!(edev->state & BIT(index));

> 
>> +}
>> +
>>  static bool is_extcon_changed(u32 prev, u32 new, int idx, bool *attached)
>>  {
>> if (((prev >> idx) & 0x1) != ((new >> idx) & 0x1)) {
>> @@ -258,6 +285,41 @@ static bool is_extcon_changed(u32 prev, u32 new, int 
>> idx, bool *attached)
>> return false;
>>  }
>>
>> +static bool is_extcon_property_supported(unsigned int id, unsigned int prop)
>> +{
>> +   unsigned int type;
>> +
>> +   /* Check whether the property is supported or not. */
>> +   type = get_extcon_type(prop);
>> +   if (type < 0)
>> +   return false;
>> +
>> +   /* Check whether a specific extcon id supports the property or not. 
>> */
>> +   if (extcon_info[id].type | type)
>> +   return true;
>> +
>> +   return false;
> 
> simpler:
> return !!(extcon_info[id].type & type);


OK. I'll use it as you comment.

> 
> Strictly speaking, the !! isn't even necessary in those mask
> operations since C auto-converts to bool, though people sometimes get
> confused by that.

Thanks for your explanation. For readability, I remain the '!!' operation.

> 
>> +}
>> +
>> +#define INIT_PROPERTY(name, name_lower, type)  \
>> +   if (EXTCON_TYPE_##name || type) {   \
>> +   for (i = 0; i < EXTCON_PROP_##name##_CNT; i++)  \
>> +   cable->name_lower##_propval[i] = val;   \
>> +   }   \
>> +
>> +static void init_property(struct extcon_dev *edev, unsigned int id, int 
>> index)
>> +{
>> +   unsigned int type = extcon_info[id].type;
>> +   struct extcon_cable *cable = >cables[index];
>> +   union extcon_property_value val = (union extcon_property_value)(0);
>> +   int i;
>> +
>> +

Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-29 Thread Chanwoo Choi 
Hi Guenter,

On 2016년 07월 28일 02:24, Guenter Roeck wrote:
> On Tue, Jul 26, 2016 at 5:09 AM, Chanwoo Choi  wrote:
>> This patch support the extcon property for the external connector
>> because each external connector might have the property according to
>> the H/W design and the specific characteristics.
>>
>> - EXTCON_PROP_USB_[property name]
>> - EXTCON_PROP_CHG_[property name]
>> - EXTCON_PROP_JACK_[property name]
>> - EXTCON_PROP_DISP_[property name]
>>
>> Add the new extcon APIs to get/set the property value as following:
>> - int extcon_get_property(struct extcon_dev *edev, unsigned int id,
>> unsigned int prop,
>> union extcon_property_value *prop_val)
>> - int extcon_set_property(struct extcon_dev *edev, unsigned int id,
>> unsigned int prop,
>> union extcon_property_value prop_val)
>>
>> Signed-off-by: Chanwoo Choi 
>> ---
>>  drivers/extcon/extcon.c | 206 
>> +++-
>>  include/linux/extcon.h  |  86 
>>  2 files changed, 291 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
>> index b1e6ee6194bc..2317aaea063f 100644
>> --- a/drivers/extcon/extcon.c
>> +++ b/drivers/extcon/extcon.c
>> @@ -196,6 +196,11 @@ struct extcon_cable {
>> struct device_attribute attr_state;
>>
>> struct attribute *attrs[3]; /* to be fed to attr_g.attrs */
>> +
>> +   union extcon_property_value usb_propval[EXTCON_PROP_USB_CNT];
>> +   union extcon_property_value chg_propval[EXTCON_PROP_CHG_CNT];
>> +   union extcon_property_value jack_propval[EXTCON_PROP_JACK_CNT];
>> +   union extcon_property_value disp_propval[EXTCON_PROP_DISP_CNT];
>>  };
>>
>>  static struct class *extcon_class;
>> @@ -248,6 +253,28 @@ static int find_cable_index_by_id(struct extcon_dev 
>> *edev, const unsigned int id
>> return -EINVAL;
>>  }
>>
>> +static int get_extcon_type(unsigned int prop)
>> +{
>> +   switch (prop) {
>> +   case EXTCON_PROP_USB_MIN ... EXTCON_PROP_USB_MAX:
>> +   return EXTCON_TYPE_USB;
>> +   case EXTCON_PROP_CHG_MIN ... EXTCON_PROP_CHG_MAX:
>> +   return EXTCON_TYPE_CHG;
>> +   case EXTCON_PROP_JACK_MIN ... EXTCON_PROP_JACK_MAX:
>> +   return EXTCON_TYPE_JACK;
>> +   case EXTCON_PROP_DISP_MIN ... EXTCON_PROP_DISP_MAX:
>> +   return EXTCON_TYPE_DISP;
>> +   default:
>> +   return -EINVAL;
>> +   }
>> +}
>> +
>> +static bool is_extcon_attached(struct extcon_dev *edev, unsigned int id,
> 
> 'id' isn't used.

I'll remove it on parameter list.

> 
>> +   unsigned int index)
>> +{
>> +   return ((!!(edev->state & (1 << index))) == 1) ? true : false;
> 
> Minor comment: This is identical to
> 
>  return !!(edev->state & (1 << index));
> or, with bitops
>  return !!(edev->state & BIT(index));

I'll use the bitops as you comment.

return !!(edev->state & BIT(index));

> 
>> +}
>> +
>>  static bool is_extcon_changed(u32 prev, u32 new, int idx, bool *attached)
>>  {
>> if (((prev >> idx) & 0x1) != ((new >> idx) & 0x1)) {
>> @@ -258,6 +285,41 @@ static bool is_extcon_changed(u32 prev, u32 new, int 
>> idx, bool *attached)
>> return false;
>>  }
>>
>> +static bool is_extcon_property_supported(unsigned int id, unsigned int prop)
>> +{
>> +   unsigned int type;
>> +
>> +   /* Check whether the property is supported or not. */
>> +   type = get_extcon_type(prop);
>> +   if (type < 0)
>> +   return false;
>> +
>> +   /* Check whether a specific extcon id supports the property or not. 
>> */
>> +   if (extcon_info[id].type | type)
>> +   return true;
>> +
>> +   return false;
> 
> simpler:
> return !!(extcon_info[id].type & type);


OK. I'll use it as you comment.

> 
> Strictly speaking, the !! isn't even necessary in those mask
> operations since C auto-converts to bool, though people sometimes get
> confused by that.

Thanks for your explanation. For readability, I remain the '!!' operation.

> 
>> +}
>> +
>> +#define INIT_PROPERTY(name, name_lower, type)  \
>> +   if (EXTCON_TYPE_##name || type) {   \
>> +   for (i = 0; i < EXTCON_PROP_##name##_CNT; i++)  \
>> +   cable->name_lower##_propval[i] = val;   \
>> +   }   \
>> +
>> +static void init_property(struct extcon_dev *edev, unsigned int id, int 
>> index)
>> +{
>> +   unsigned int type = extcon_info[id].type;
>> +   struct extcon_cable *cable = >cables[index];
>> +   union extcon_property_value val = (union extcon_property_value)(0);
>> +   int i;
>> +
>> +   INIT_PROPERTY(USB, usb, type);
>> +

Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-27 Thread Guenter Roeck 
On Tue, Jul 26, 2016 at 5:09 AM, Chanwoo Choi  wrote:
> This patch support the extcon property for the external connector
> because each external connector might have the property according to
> the H/W design and the specific characteristics.
>
> - EXTCON_PROP_USB_[property name]
> - EXTCON_PROP_CHG_[property name]
> - EXTCON_PROP_JACK_[property name]
> - EXTCON_PROP_DISP_[property name]
>
> Add the new extcon APIs to get/set the property value as following:
> - int extcon_get_property(struct extcon_dev *edev, unsigned int id,
> unsigned int prop,
> union extcon_property_value *prop_val)
> - int extcon_set_property(struct extcon_dev *edev, unsigned int id,
> unsigned int prop,
> union extcon_property_value prop_val)
>
> Signed-off-by: Chanwoo Choi 
> ---
>  drivers/extcon/extcon.c | 206 
> +++-
>  include/linux/extcon.h  |  86 
>  2 files changed, 291 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index b1e6ee6194bc..2317aaea063f 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -196,6 +196,11 @@ struct extcon_cable {
> struct device_attribute attr_state;
>
> struct attribute *attrs[3]; /* to be fed to attr_g.attrs */
> +
> +   union extcon_property_value usb_propval[EXTCON_PROP_USB_CNT];
> +   union extcon_property_value chg_propval[EXTCON_PROP_CHG_CNT];
> +   union extcon_property_value jack_propval[EXTCON_PROP_JACK_CNT];
> +   union extcon_property_value disp_propval[EXTCON_PROP_DISP_CNT];
>  };
>
>  static struct class *extcon_class;
> @@ -248,6 +253,28 @@ static int find_cable_index_by_id(struct extcon_dev 
> *edev, const unsigned int id
> return -EINVAL;
>  }
>
> +static int get_extcon_type(unsigned int prop)
> +{
> +   switch (prop) {
> +   case EXTCON_PROP_USB_MIN ... EXTCON_PROP_USB_MAX:
> +   return EXTCON_TYPE_USB;
> +   case EXTCON_PROP_CHG_MIN ... EXTCON_PROP_CHG_MAX:
> +   return EXTCON_TYPE_CHG;
> +   case EXTCON_PROP_JACK_MIN ... EXTCON_PROP_JACK_MAX:
> +   return EXTCON_TYPE_JACK;
> +   case EXTCON_PROP_DISP_MIN ... EXTCON_PROP_DISP_MAX:
> +   return EXTCON_TYPE_DISP;
> +   default:
> +   return -EINVAL;
> +   }
> +}
> +
> +static bool is_extcon_attached(struct extcon_dev *edev, unsigned int id,

'id' isn't used.

> +   unsigned int index)
> +{
> +   return ((!!(edev->state & (1 << index))) == 1) ? true : false;

Minor comment: This is identical to

 return !!(edev->state & (1 << index));
or, with bitops
 return !!(edev->state & BIT(index));

> +}
> +
>  static bool is_extcon_changed(u32 prev, u32 new, int idx, bool *attached)
>  {
> if (((prev >> idx) & 0x1) != ((new >> idx) & 0x1)) {
> @@ -258,6 +285,41 @@ static bool is_extcon_changed(u32 prev, u32 new, int 
> idx, bool *attached)
> return false;
>  }
>
> +static bool is_extcon_property_supported(unsigned int id, unsigned int prop)
> +{
> +   unsigned int type;
> +
> +   /* Check whether the property is supported or not. */
> +   type = get_extcon_type(prop);
> +   if (type < 0)
> +   return false;
> +
> +   /* Check whether a specific extcon id supports the property or not. */
> +   if (extcon_info[id].type | type)
> +   return true;
> +
> +   return false;

simpler:
return !!(extcon_info[id].type & type);

Strictly speaking, the !! isn't even necessary in those mask
operations since C auto-converts to bool, though people sometimes get
confused by that.

> +}
> +
> +#define INIT_PROPERTY(name, name_lower, type)  \
> +   if (EXTCON_TYPE_##name || type) {   \
> +   for (i = 0; i < EXTCON_PROP_##name##_CNT; i++)  \
> +   cable->name_lower##_propval[i] = val;   \
> +   }   \
> +
> +static void init_property(struct extcon_dev *edev, unsigned int id, int 
> index)
> +{
> +   unsigned int type = extcon_info[id].type;
> +   struct extcon_cable *cable = >cables[index];
> +   union extcon_property_value val = (union extcon_property_value)(0);
> +   int i;
> +
> +   INIT_PROPERTY(USB, usb, type);
> +   INIT_PROPERTY(CHG, chg, type);
> +   INIT_PROPERTY(JACK, jack, type);
> +   INIT_PROPERTY(DISP, disp, type);

Just wondering if this would be a bit cleaner/simpler.

switch(type) {
case EXTCON_TYPE_USB:
memset(cable->usb_propval, sizeof(cable->usb_propval), 0);
break;
case EXTCON_TYPE_CHG:
memset(cable->chg_propval, sizeof(cable->chg_propval),

Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-27 Thread Guenter Roeck 
On Tue, Jul 26, 2016 at 5:09 AM, Chanwoo Choi  wrote:
> This patch support the extcon property for the external connector
> because each external connector might have the property according to
> the H/W design and the specific characteristics.
>
> - EXTCON_PROP_USB_[property name]
> - EXTCON_PROP_CHG_[property name]
> - EXTCON_PROP_JACK_[property name]
> - EXTCON_PROP_DISP_[property name]
>
> Add the new extcon APIs to get/set the property value as following:
> - int extcon_get_property(struct extcon_dev *edev, unsigned int id,
> unsigned int prop,
> union extcon_property_value *prop_val)
> - int extcon_set_property(struct extcon_dev *edev, unsigned int id,
> unsigned int prop,
> union extcon_property_value prop_val)
>
> Signed-off-by: Chanwoo Choi 
> ---
>  drivers/extcon/extcon.c | 206 
> +++-
>  include/linux/extcon.h  |  86 
>  2 files changed, 291 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index b1e6ee6194bc..2317aaea063f 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -196,6 +196,11 @@ struct extcon_cable {
> struct device_attribute attr_state;
>
> struct attribute *attrs[3]; /* to be fed to attr_g.attrs */
> +
> +   union extcon_property_value usb_propval[EXTCON_PROP_USB_CNT];
> +   union extcon_property_value chg_propval[EXTCON_PROP_CHG_CNT];
> +   union extcon_property_value jack_propval[EXTCON_PROP_JACK_CNT];
> +   union extcon_property_value disp_propval[EXTCON_PROP_DISP_CNT];
>  };
>
>  static struct class *extcon_class;
> @@ -248,6 +253,28 @@ static int find_cable_index_by_id(struct extcon_dev 
> *edev, const unsigned int id
> return -EINVAL;
>  }
>
> +static int get_extcon_type(unsigned int prop)
> +{
> +   switch (prop) {
> +   case EXTCON_PROP_USB_MIN ... EXTCON_PROP_USB_MAX:
> +   return EXTCON_TYPE_USB;
> +   case EXTCON_PROP_CHG_MIN ... EXTCON_PROP_CHG_MAX:
> +   return EXTCON_TYPE_CHG;
> +   case EXTCON_PROP_JACK_MIN ... EXTCON_PROP_JACK_MAX:
> +   return EXTCON_TYPE_JACK;
> +   case EXTCON_PROP_DISP_MIN ... EXTCON_PROP_DISP_MAX:
> +   return EXTCON_TYPE_DISP;
> +   default:
> +   return -EINVAL;
> +   }
> +}
> +
> +static bool is_extcon_attached(struct extcon_dev *edev, unsigned int id,

'id' isn't used.

> +   unsigned int index)
> +{
> +   return ((!!(edev->state & (1 << index))) == 1) ? true : false;

Minor comment: This is identical to

 return !!(edev->state & (1 << index));
or, with bitops
 return !!(edev->state & BIT(index));

> +}
> +
>  static bool is_extcon_changed(u32 prev, u32 new, int idx, bool *attached)
>  {
> if (((prev >> idx) & 0x1) != ((new >> idx) & 0x1)) {
> @@ -258,6 +285,41 @@ static bool is_extcon_changed(u32 prev, u32 new, int 
> idx, bool *attached)
> return false;
>  }
>
> +static bool is_extcon_property_supported(unsigned int id, unsigned int prop)
> +{
> +   unsigned int type;
> +
> +   /* Check whether the property is supported or not. */
> +   type = get_extcon_type(prop);
> +   if (type < 0)
> +   return false;
> +
> +   /* Check whether a specific extcon id supports the property or not. */
> +   if (extcon_info[id].type | type)
> +   return true;
> +
> +   return false;

simpler:
return !!(extcon_info[id].type & type);

Strictly speaking, the !! isn't even necessary in those mask
operations since C auto-converts to bool, though people sometimes get
confused by that.

> +}
> +
> +#define INIT_PROPERTY(name, name_lower, type)  \
> +   if (EXTCON_TYPE_##name || type) {   \
> +   for (i = 0; i < EXTCON_PROP_##name##_CNT; i++)  \
> +   cable->name_lower##_propval[i] = val;   \
> +   }   \
> +
> +static void init_property(struct extcon_dev *edev, unsigned int id, int 
> index)
> +{
> +   unsigned int type = extcon_info[id].type;
> +   struct extcon_cable *cable = >cables[index];
> +   union extcon_property_value val = (union extcon_property_value)(0);
> +   int i;
> +
> +   INIT_PROPERTY(USB, usb, type);
> +   INIT_PROPERTY(CHG, chg, type);
> +   INIT_PROPERTY(JACK, jack, type);
> +   INIT_PROPERTY(DISP, disp, type);

Just wondering if this would be a bit cleaner/simpler.

switch(type) {
case EXTCON_TYPE_USB:
memset(cable->usb_propval, sizeof(cable->usb_propval), 0);
break;
case EXTCON_TYPE_CHG:
memset(cable->chg_propval, sizeof(cable->chg_propval), 0);
break;
case

Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-26 Thread Chris Zhong 

Hi Chanwoo

On 07/27/2016 11:57 AM, Chanwoo Choi wrote:

On 2016년 07월 27일 12:51, Guenter Roeck wrote:

On Tue, Jul 26, 2016 at 8:42 PM, Chanwoo Choi  wrote:

Hi Chris,

On 2016년 07월 27일 11:09, Chris Zhong wrote:

Hi Guernter

On 07/27/2016 09:44 AM, Guenter Roeck wrote:

Hi Chris,

On Tue, Jul 26, 2016 at 6:15 PM, Chris Zhong  wrote:

[ ... ]


  > +
  > +/* Properties of EXTCON_TYPE_DISP. */
  > +#define EXTCON_PROP_DISP_MIN   150
  > +#define EXTCON_PROP_DISP_MAX   150
  > +#define EXTCON_PROP_DISP_CNT   (EXTCON_PROP_DISP_MAX -
  EXTCON_PROP_DISP_MIN)

   + 1


ok.

Tested with these "+1", it works for my DP patch.


You should be able to use
https://chromium-review.googlesource.com/#/c/363623/1 as baseline (if
you didn't do that already).

Thanks,
Guenter

Thanks Guenter, and I saw this bug has fixed in extcon-test branch.



Do you test it with extcon_set_property_capability()?
And if you test this patch-es, could you send the tested-by tag for these 
patches?

Yes, the new API need this extcon_set_property_capability to be called 
before setting property.
On this basis, My DP patches works well with a little bit modification. 
Then, I will post the V7 version soon, base on this patches and 
Guenter's FIXUP patch.
Please feel free to add my Tested-by: Chris Zhong  
tag in the next version.




For my part I did. Above link is public, so you should be able to see
the complete patch set which uses the new API from
drivers/extcon/extcon-cros_ec.c.

I'll re-test tomorrow with the updated patches from your test branch.






OK. Thanks.

Regards,
Chanwoo Choi

Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-26 Thread Chris Zhong 

Hi Chanwoo

On 07/27/2016 11:57 AM, Chanwoo Choi wrote:

On 2016년 07월 27일 12:51, Guenter Roeck wrote:

On Tue, Jul 26, 2016 at 8:42 PM, Chanwoo Choi  wrote:

Hi Chris,

On 2016년 07월 27일 11:09, Chris Zhong wrote:

Hi Guernter

On 07/27/2016 09:44 AM, Guenter Roeck wrote:

Hi Chris,

On Tue, Jul 26, 2016 at 6:15 PM, Chris Zhong  wrote:

[ ... ]


  > +
  > +/* Properties of EXTCON_TYPE_DISP. */
  > +#define EXTCON_PROP_DISP_MIN   150
  > +#define EXTCON_PROP_DISP_MAX   150
  > +#define EXTCON_PROP_DISP_CNT   (EXTCON_PROP_DISP_MAX -
  EXTCON_PROP_DISP_MIN)

   + 1


ok.

Tested with these "+1", it works for my DP patch.


You should be able to use
https://chromium-review.googlesource.com/#/c/363623/1 as baseline (if
you didn't do that already).

Thanks,
Guenter

Thanks Guenter, and I saw this bug has fixed in extcon-test branch.



Do you test it with extcon_set_property_capability()?
And if you test this patch-es, could you send the tested-by tag for these 
patches?

Yes, the new API need this extcon_set_property_capability to be called 
before setting property.
On this basis, My DP patches works well with a little bit modification. 
Then, I will post the V7 version soon, base on this patches and 
Guenter's FIXUP patch.
Please feel free to add my Tested-by: Chris Zhong  
tag in the next version.




For my part I did. Above link is public, so you should be able to see
the complete patch set which uses the new API from
drivers/extcon/extcon-cros_ec.c.

I'll re-test tomorrow with the updated patches from your test branch.






OK. Thanks.

Regards,
Chanwoo Choi

Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-26 Thread Chanwoo Choi 
On 2016년 07월 27일 12:51, Guenter Roeck wrote:
> On Tue, Jul 26, 2016 at 8:42 PM, Chanwoo Choi  wrote:
>> Hi Chris,
>>
>> On 2016년 07월 27일 11:09, Chris Zhong wrote:
>>> Hi Guernter
>>>
>>> On 07/27/2016 09:44 AM, Guenter Roeck wrote:
 Hi Chris,

 On Tue, Jul 26, 2016 at 6:15 PM, Chris Zhong  wrote:

 [ ... ]

>>  > +
>>  > +/* Properties of EXTCON_TYPE_DISP. */
>>  > +#define EXTCON_PROP_DISP_MIN   150
>>  > +#define EXTCON_PROP_DISP_MAX   150
>>  > +#define EXTCON_PROP_DISP_CNT   (EXTCON_PROP_DISP_MAX -
>>  EXTCON_PROP_DISP_MIN)
>>
>>   + 1
>>
>>
>> ok.
>
> Tested with these "+1", it works for my DP patch.
>
 You should be able to use
 https://chromium-review.googlesource.com/#/c/363623/1 as baseline (if
 you didn't do that already).

 Thanks,
 Guenter
>>>
>>> Thanks Guenter, and I saw this bug has fixed in extcon-test branch.
>>>
>>>
>>
>> Do you test it with extcon_set_property_capability()?
>> And if you test this patch-es, could you send the tested-by tag for these 
>> patches?
>>
> 
> For my part I did. Above link is public, so you should be able to see
> the complete patch set which uses the new API from
> drivers/extcon/extcon-cros_ec.c.
> 
> I'll re-test tomorrow with the updated patches from your test branch.

OK. Thanks. 

Regards,
Chanwoo Choi

Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-26 Thread Chanwoo Choi 
On 2016년 07월 27일 12:51, Guenter Roeck wrote:
> On Tue, Jul 26, 2016 at 8:42 PM, Chanwoo Choi  wrote:
>> Hi Chris,
>>
>> On 2016년 07월 27일 11:09, Chris Zhong wrote:
>>> Hi Guernter
>>>
>>> On 07/27/2016 09:44 AM, Guenter Roeck wrote:
 Hi Chris,

 On Tue, Jul 26, 2016 at 6:15 PM, Chris Zhong  wrote:

 [ ... ]

>>  > +
>>  > +/* Properties of EXTCON_TYPE_DISP. */
>>  > +#define EXTCON_PROP_DISP_MIN   150
>>  > +#define EXTCON_PROP_DISP_MAX   150
>>  > +#define EXTCON_PROP_DISP_CNT   (EXTCON_PROP_DISP_MAX -
>>  EXTCON_PROP_DISP_MIN)
>>
>>   + 1
>>
>>
>> ok.
>
> Tested with these "+1", it works for my DP patch.
>
 You should be able to use
 https://chromium-review.googlesource.com/#/c/363623/1 as baseline (if
 you didn't do that already).

 Thanks,
 Guenter
>>>
>>> Thanks Guenter, and I saw this bug has fixed in extcon-test branch.
>>>
>>>
>>
>> Do you test it with extcon_set_property_capability()?
>> And if you test this patch-es, could you send the tested-by tag for these 
>> patches?
>>
> 
> For my part I did. Above link is public, so you should be able to see
> the complete patch set which uses the new API from
> drivers/extcon/extcon-cros_ec.c.
> 
> I'll re-test tomorrow with the updated patches from your test branch.

OK. Thanks. 

Regards,
Chanwoo Choi

Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-26 Thread Guenter Roeck 
On Tue, Jul 26, 2016 at 8:42 PM, Chanwoo Choi  wrote:
> Hi Chris,
>
> On 2016년 07월 27일 11:09, Chris Zhong wrote:
>> Hi Guernter
>>
>> On 07/27/2016 09:44 AM, Guenter Roeck wrote:
>>> Hi Chris,
>>>
>>> On Tue, Jul 26, 2016 at 6:15 PM, Chris Zhong  wrote:
>>>
>>> [ ... ]
>>>
>  > +
>  > +/* Properties of EXTCON_TYPE_DISP. */
>  > +#define EXTCON_PROP_DISP_MIN   150
>  > +#define EXTCON_PROP_DISP_MAX   150
>  > +#define EXTCON_PROP_DISP_CNT   (EXTCON_PROP_DISP_MAX -
>  EXTCON_PROP_DISP_MIN)
>
>   + 1
>
>
> ok.

 Tested with these "+1", it works for my DP patch.

>>> You should be able to use
>>> https://chromium-review.googlesource.com/#/c/363623/1 as baseline (if
>>> you didn't do that already).
>>>
>>> Thanks,
>>> Guenter
>>
>> Thanks Guenter, and I saw this bug has fixed in extcon-test branch.
>>
>>
>
> Do you test it with extcon_set_property_capability()?
> And if you test this patch-es, could you send the tested-by tag for these 
> patches?
>

For my part I did. Above link is public, so you should be able to see
the complete patch set which uses the new API from
drivers/extcon/extcon-cros_ec.c.

I'll re-test tomorrow with the updated patches from your test branch.

> I'll send the next version after a few days.

Great.

Thanks,
Guenter

Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-26 Thread Guenter Roeck 
On Tue, Jul 26, 2016 at 8:42 PM, Chanwoo Choi  wrote:
> Hi Chris,
>
> On 2016년 07월 27일 11:09, Chris Zhong wrote:
>> Hi Guernter
>>
>> On 07/27/2016 09:44 AM, Guenter Roeck wrote:
>>> Hi Chris,
>>>
>>> On Tue, Jul 26, 2016 at 6:15 PM, Chris Zhong  wrote:
>>>
>>> [ ... ]
>>>
>  > +
>  > +/* Properties of EXTCON_TYPE_DISP. */
>  > +#define EXTCON_PROP_DISP_MIN   150
>  > +#define EXTCON_PROP_DISP_MAX   150
>  > +#define EXTCON_PROP_DISP_CNT   (EXTCON_PROP_DISP_MAX -
>  EXTCON_PROP_DISP_MIN)
>
>   + 1
>
>
> ok.

 Tested with these "+1", it works for my DP patch.

>>> You should be able to use
>>> https://chromium-review.googlesource.com/#/c/363623/1 as baseline (if
>>> you didn't do that already).
>>>
>>> Thanks,
>>> Guenter
>>
>> Thanks Guenter, and I saw this bug has fixed in extcon-test branch.
>>
>>
>
> Do you test it with extcon_set_property_capability()?
> And if you test this patch-es, could you send the tested-by tag for these 
> patches?
>

For my part I did. Above link is public, so you should be able to see
the complete patch set which uses the new API from
drivers/extcon/extcon-cros_ec.c.

I'll re-test tomorrow with the updated patches from your test branch.

> I'll send the next version after a few days.

Great.

Thanks,
Guenter

Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-26 Thread Chanwoo Choi 
Hi Chris,

On 2016년 07월 27일 11:09, Chris Zhong wrote:
> Hi Guernter
> 
> On 07/27/2016 09:44 AM, Guenter Roeck wrote:
>> Hi Chris,
>>
>> On Tue, Jul 26, 2016 at 6:15 PM, Chris Zhong  wrote:
>>
>> [ ... ]
>>
  > +
  > +/* Properties of EXTCON_TYPE_DISP. */
  > +#define EXTCON_PROP_DISP_MIN   150
  > +#define EXTCON_PROP_DISP_MAX   150
  > +#define EXTCON_PROP_DISP_CNT   (EXTCON_PROP_DISP_MAX -
  EXTCON_PROP_DISP_MIN)

   + 1


 ok.
>>>
>>> Tested with these "+1", it works for my DP patch.
>>>
>> You should be able to use
>> https://chromium-review.googlesource.com/#/c/363623/1 as baseline (if
>> you didn't do that already).
>>
>> Thanks,
>> Guenter
> 
> Thanks Guenter, and I saw this bug has fixed in extcon-test branch.
> 
> 

Do you test it with extcon_set_property_capability()?
And if you test this patch-es, could you send the tested-by tag for these 
patches?

I'll send the next version after a few days.

Regards,
Chanwoo Choi

Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-26 Thread Chanwoo Choi 
Hi Chris,

On 2016년 07월 27일 11:09, Chris Zhong wrote:
> Hi Guernter
> 
> On 07/27/2016 09:44 AM, Guenter Roeck wrote:
>> Hi Chris,
>>
>> On Tue, Jul 26, 2016 at 6:15 PM, Chris Zhong  wrote:
>>
>> [ ... ]
>>
  > +
  > +/* Properties of EXTCON_TYPE_DISP. */
  > +#define EXTCON_PROP_DISP_MIN   150
  > +#define EXTCON_PROP_DISP_MAX   150
  > +#define EXTCON_PROP_DISP_CNT   (EXTCON_PROP_DISP_MAX -
  EXTCON_PROP_DISP_MIN)

   + 1


 ok.
>>>
>>> Tested with these "+1", it works for my DP patch.
>>>
>> You should be able to use
>> https://chromium-review.googlesource.com/#/c/363623/1 as baseline (if
>> you didn't do that already).
>>
>> Thanks,
>> Guenter
> 
> Thanks Guenter, and I saw this bug has fixed in extcon-test branch.
> 
> 

Do you test it with extcon_set_property_capability()?
And if you test this patch-es, could you send the tested-by tag for these 
patches?

I'll send the next version after a few days.

Regards,
Chanwoo Choi

Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-26 Thread Chris Zhong 

Hi Guernter

On 07/27/2016 09:44 AM, Guenter Roeck wrote:

Hi Chris,

On Tue, Jul 26, 2016 at 6:15 PM, Chris Zhong  wrote:

[ ... ]


 > +
 > +/* Properties of EXTCON_TYPE_DISP. */
 > +#define EXTCON_PROP_DISP_MIN   150
 > +#define EXTCON_PROP_DISP_MAX   150
 > +#define EXTCON_PROP_DISP_CNT   (EXTCON_PROP_DISP_MAX -
 EXTCON_PROP_DISP_MIN)

  + 1


ok.


Tested with these "+1", it works for my DP patch.


You should be able to use
https://chromium-review.googlesource.com/#/c/363623/1 as baseline (if
you didn't do that already).

Thanks,
Guenter


Thanks Guenter, and I saw this bug has fixed in extcon-test branch.


Thanks
Chris Zhong

Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-26 Thread Chris Zhong 

Hi Guernter

On 07/27/2016 09:44 AM, Guenter Roeck wrote:

Hi Chris,

On Tue, Jul 26, 2016 at 6:15 PM, Chris Zhong  wrote:

[ ... ]


 > +
 > +/* Properties of EXTCON_TYPE_DISP. */
 > +#define EXTCON_PROP_DISP_MIN   150
 > +#define EXTCON_PROP_DISP_MAX   150
 > +#define EXTCON_PROP_DISP_CNT   (EXTCON_PROP_DISP_MAX -
 EXTCON_PROP_DISP_MIN)

  + 1


ok.


Tested with these "+1", it works for my DP patch.


You should be able to use
https://chromium-review.googlesource.com/#/c/363623/1 as baseline (if
you didn't do that already).

Thanks,
Guenter


Thanks Guenter, and I saw this bug has fixed in extcon-test branch.


Thanks
Chris Zhong

Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-26 Thread Guenter Roeck 
Hi Chris,

On Tue, Jul 26, 2016 at 6:15 PM, Chris Zhong  wrote:

[ ... ]

>>
>> > +
>> > +/* Properties of EXTCON_TYPE_DISP. */
>> > +#define EXTCON_PROP_DISP_MIN   150
>> > +#define EXTCON_PROP_DISP_MAX   150
>> > +#define EXTCON_PROP_DISP_CNT   (EXTCON_PROP_DISP_MAX -
>> EXTCON_PROP_DISP_MIN)
>>
>>  + 1
>>
>>
>> ok.
>
>
> Tested with these "+1", it works for my DP patch.
>

You should be able to use
https://chromium-review.googlesource.com/#/c/363623/1 as baseline (if
you didn't do that already).

Thanks,
Guenter

Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-26 Thread Guenter Roeck 
Hi Chris,

On Tue, Jul 26, 2016 at 6:15 PM, Chris Zhong  wrote:

[ ... ]

>>
>> > +
>> > +/* Properties of EXTCON_TYPE_DISP. */
>> > +#define EXTCON_PROP_DISP_MIN   150
>> > +#define EXTCON_PROP_DISP_MAX   150
>> > +#define EXTCON_PROP_DISP_CNT   (EXTCON_PROP_DISP_MAX -
>> EXTCON_PROP_DISP_MIN)
>>
>>  + 1
>>
>>
>> ok.
>
>
> Tested with these "+1", it works for my DP patch.
>

You should be able to use
https://chromium-review.googlesource.com/#/c/363623/1 as baseline (if
you didn't do that already).

Thanks,
Guenter

Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-26 Thread Chris Zhong 

Hi Chanwoo

On 07/27/2016 08:31 AM, Chanwoo Choi wrote:

Hi Guenter,

2016년 7월 27일 수요일, Guenter Roeck>님이 작성한 메시지:


On Tue, Jul 26, 2016 at 5:09 AM, Chanwoo Choi
> wrote:
> This patch support the extcon property for the external connector
> because each external connector might have the property according to
> the H/W design and the specific characteristics.
>
> - EXTCON_PROP_USB_[property name]
> - EXTCON_PROP_CHG_[property name]
> - EXTCON_PROP_JACK_[property name]
> - EXTCON_PROP_DISP_[property name]
>
> Add the new extcon APIs to get/set the property value as following:
> - int extcon_get_property(struct extcon_dev *edev, unsigned int id,
> unsigned int prop,
> union extcon_property_value *prop_val)
> - int extcon_set_property(struct extcon_dev *edev, unsigned int id,
> unsigned int prop,
> union extcon_property_value prop_val)
>
> Signed-off-by: Chanwoo Choi >
> ---
>  drivers/extcon/extcon.c | 206
+++-
>  include/linux/extcon.h  |  86 
>  2 files changed, 291 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index b1e6ee6194bc..2317aaea063f 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -196,6 +196,11 @@ struct extcon_cable {
> struct device_attribute attr_state;
>
> struct attribute *attrs[3]; /* to be fed to attr_g.attrs */
> +
> +   union extcon_property_value
usb_propval[EXTCON_PROP_USB_CNT];
> +   union extcon_property_value
chg_propval[EXTCON_PROP_CHG_CNT];
> +   union extcon_property_value
jack_propval[EXTCON_PROP_JACK_CNT];
> +   union extcon_property_value
disp_propval[EXTCON_PROP_DISP_CNT];
>  };
>
>  static struct class *extcon_class;
> @@ -248,6 +253,28 @@ static int find_cable_index_by_id(struct
extcon_dev *edev, const unsigned int id
> return -EINVAL;
>  }
>
> +static int get_extcon_type(unsigned int prop)
> +{
> +   switch (prop) {
> +   case EXTCON_PROP_USB_MIN ... EXTCON_PROP_USB_MAX:
> +   return EXTCON_TYPE_USB;
> +   case EXTCON_PROP_CHG_MIN ... EXTCON_PROP_CHG_MAX:
> +   return EXTCON_TYPE_CHG;
> +   case EXTCON_PROP_JACK_MIN ... EXTCON_PROP_JACK_MAX:
> +   return EXTCON_TYPE_JACK;
> +   case EXTCON_PROP_DISP_MIN ... EXTCON_PROP_DISP_MAX:
> +   return EXTCON_TYPE_DISP;
> +   default:
> +   return -EINVAL;
> +   }
> +}
> +
> +static bool is_extcon_attached(struct extcon_dev *edev,
unsigned int id,
> +   unsigned int index)
> +{
> +   return ((!!(edev->state & (1 << index))) == 1) ? true :
false;
> +}
> +
>  static bool is_extcon_changed(u32 prev, u32 new, int idx, bool
*attached)
>  {
> if (((prev >> idx) & 0x1) != ((new >> idx) & 0x1)) {
> @@ -258,6 +285,41 @@ static bool is_extcon_changed(u32 prev, u32
new, int idx, bool *attached)
> return false;
>  }
>
> +static bool is_extcon_property_supported(unsigned int id,
unsigned int prop)
> +{
> +   unsigned int type;
> +

int


ok.


> +   /* Check whether the property is supported or not. */
> +   type = get_extcon_type(prop);
> +   if (type < 0)

otherwise type is never < 0


you're right.


> +   return false;
> +
> +   /* Check whether a specific extcon id supports the
property or not. */
> +   if (extcon_info[id].type | type)

This is always true ?


It is my mistake. Use '&' instead of '|'.


> +   return true;
> +
> +   return false;
> +}
> +
> +#define INIT_PROPERTY(name, name_lower, type) \
> +   if (EXTCON_TYPE_##name || type) {  \

This is always true ?


It is my mistake. Use '&' instead of '||'.


> +   for (i = 0; i < EXTCON_PROP_##name##_CNT; i++) 
\
> +   cable->name_lower##_propval[i] = val;   
   \

> +   }  \
> +
> +static void init_property(struct extcon_dev *edev, unsigned int
id, int index)
> +{
> +   unsigned int type = extcon_info[id].type;
> +   struct extcon_cable *cable = >cables[index];
> +   union extcon_property_value val = (union
extcon_property_value)(0);
> +   int i;
> +
> +   INIT_PROPERTY(USB, usb, type);
> +

Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-26 Thread Chris Zhong 

Hi Chanwoo

On 07/27/2016 08:31 AM, Chanwoo Choi wrote:

Hi Guenter,

2016년 7월 27일 수요일, Guenter Roeck>님이 작성한 메시지:


On Tue, Jul 26, 2016 at 5:09 AM, Chanwoo Choi
> wrote:
> This patch support the extcon property for the external connector
> because each external connector might have the property according to
> the H/W design and the specific characteristics.
>
> - EXTCON_PROP_USB_[property name]
> - EXTCON_PROP_CHG_[property name]
> - EXTCON_PROP_JACK_[property name]
> - EXTCON_PROP_DISP_[property name]
>
> Add the new extcon APIs to get/set the property value as following:
> - int extcon_get_property(struct extcon_dev *edev, unsigned int id,
> unsigned int prop,
> union extcon_property_value *prop_val)
> - int extcon_set_property(struct extcon_dev *edev, unsigned int id,
> unsigned int prop,
> union extcon_property_value prop_val)
>
> Signed-off-by: Chanwoo Choi >
> ---
>  drivers/extcon/extcon.c | 206
+++-
>  include/linux/extcon.h  |  86 
>  2 files changed, 291 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index b1e6ee6194bc..2317aaea063f 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -196,6 +196,11 @@ struct extcon_cable {
> struct device_attribute attr_state;
>
> struct attribute *attrs[3]; /* to be fed to attr_g.attrs */
> +
> +   union extcon_property_value
usb_propval[EXTCON_PROP_USB_CNT];
> +   union extcon_property_value
chg_propval[EXTCON_PROP_CHG_CNT];
> +   union extcon_property_value
jack_propval[EXTCON_PROP_JACK_CNT];
> +   union extcon_property_value
disp_propval[EXTCON_PROP_DISP_CNT];
>  };
>
>  static struct class *extcon_class;
> @@ -248,6 +253,28 @@ static int find_cable_index_by_id(struct
extcon_dev *edev, const unsigned int id
> return -EINVAL;
>  }
>
> +static int get_extcon_type(unsigned int prop)
> +{
> +   switch (prop) {
> +   case EXTCON_PROP_USB_MIN ... EXTCON_PROP_USB_MAX:
> +   return EXTCON_TYPE_USB;
> +   case EXTCON_PROP_CHG_MIN ... EXTCON_PROP_CHG_MAX:
> +   return EXTCON_TYPE_CHG;
> +   case EXTCON_PROP_JACK_MIN ... EXTCON_PROP_JACK_MAX:
> +   return EXTCON_TYPE_JACK;
> +   case EXTCON_PROP_DISP_MIN ... EXTCON_PROP_DISP_MAX:
> +   return EXTCON_TYPE_DISP;
> +   default:
> +   return -EINVAL;
> +   }
> +}
> +
> +static bool is_extcon_attached(struct extcon_dev *edev,
unsigned int id,
> +   unsigned int index)
> +{
> +   return ((!!(edev->state & (1 << index))) == 1) ? true :
false;
> +}
> +
>  static bool is_extcon_changed(u32 prev, u32 new, int idx, bool
*attached)
>  {
> if (((prev >> idx) & 0x1) != ((new >> idx) & 0x1)) {
> @@ -258,6 +285,41 @@ static bool is_extcon_changed(u32 prev, u32
new, int idx, bool *attached)
> return false;
>  }
>
> +static bool is_extcon_property_supported(unsigned int id,
unsigned int prop)
> +{
> +   unsigned int type;
> +

int


ok.


> +   /* Check whether the property is supported or not. */
> +   type = get_extcon_type(prop);
> +   if (type < 0)

otherwise type is never < 0


you're right.


> +   return false;
> +
> +   /* Check whether a specific extcon id supports the
property or not. */
> +   if (extcon_info[id].type | type)

This is always true ?


It is my mistake. Use '&' instead of '|'.


> +   return true;
> +
> +   return false;
> +}
> +
> +#define INIT_PROPERTY(name, name_lower, type) \
> +   if (EXTCON_TYPE_##name || type) {  \

This is always true ?


It is my mistake. Use '&' instead of '||'.


> +   for (i = 0; i < EXTCON_PROP_##name##_CNT; i++) 
\
> +   cable->name_lower##_propval[i] = val;   
   \

> +   }  \
> +
> +static void init_property(struct extcon_dev *edev, unsigned int
id, int index)
> +{
> +   unsigned int type = extcon_info[id].type;
> +   struct extcon_cable *cable = >cables[index];
> +   union extcon_property_value val = (union
extcon_property_value)(0);
> +   int i;
> +
> +   INIT_PROPERTY(USB, usb, type);
> +   INIT_PROPERTY(CHG, chg, type);
> +   INIT_PROPERTY(JACK, jack, type);
> +

Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-26 Thread Guenter Roeck 
On Tue, Jul 26, 2016 at 5:09 AM, Chanwoo Choi  wrote:
> This patch support the extcon property for the external connector
> because each external connector might have the property according to
> the H/W design and the specific characteristics.
>
> - EXTCON_PROP_USB_[property name]
> - EXTCON_PROP_CHG_[property name]
> - EXTCON_PROP_JACK_[property name]
> - EXTCON_PROP_DISP_[property name]
>
> Add the new extcon APIs to get/set the property value as following:
> - int extcon_get_property(struct extcon_dev *edev, unsigned int id,
> unsigned int prop,
> union extcon_property_value *prop_val)
> - int extcon_set_property(struct extcon_dev *edev, unsigned int id,
> unsigned int prop,
> union extcon_property_value prop_val)
>
> Signed-off-by: Chanwoo Choi 
> ---
>  drivers/extcon/extcon.c | 206 
> +++-
>  include/linux/extcon.h  |  86 
>  2 files changed, 291 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index b1e6ee6194bc..2317aaea063f 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -196,6 +196,11 @@ struct extcon_cable {
> struct device_attribute attr_state;
>
> struct attribute *attrs[3]; /* to be fed to attr_g.attrs */
> +
> +   union extcon_property_value usb_propval[EXTCON_PROP_USB_CNT];
> +   union extcon_property_value chg_propval[EXTCON_PROP_CHG_CNT];
> +   union extcon_property_value jack_propval[EXTCON_PROP_JACK_CNT];
> +   union extcon_property_value disp_propval[EXTCON_PROP_DISP_CNT];
>  };
>
>  static struct class *extcon_class;
> @@ -248,6 +253,28 @@ static int find_cable_index_by_id(struct extcon_dev 
> *edev, const unsigned int id
> return -EINVAL;
>  }
>
> +static int get_extcon_type(unsigned int prop)
> +{
> +   switch (prop) {
> +   case EXTCON_PROP_USB_MIN ... EXTCON_PROP_USB_MAX:
> +   return EXTCON_TYPE_USB;
> +   case EXTCON_PROP_CHG_MIN ... EXTCON_PROP_CHG_MAX:
> +   return EXTCON_TYPE_CHG;
> +   case EXTCON_PROP_JACK_MIN ... EXTCON_PROP_JACK_MAX:
> +   return EXTCON_TYPE_JACK;
> +   case EXTCON_PROP_DISP_MIN ... EXTCON_PROP_DISP_MAX:
> +   return EXTCON_TYPE_DISP;
> +   default:
> +   return -EINVAL;
> +   }
> +}
> +
> +static bool is_extcon_attached(struct extcon_dev *edev, unsigned int id,
> +   unsigned int index)
> +{
> +   return ((!!(edev->state & (1 << index))) == 1) ? true : false;
> +}
> +
>  static bool is_extcon_changed(u32 prev, u32 new, int idx, bool *attached)
>  {
> if (((prev >> idx) & 0x1) != ((new >> idx) & 0x1)) {
> @@ -258,6 +285,41 @@ static bool is_extcon_changed(u32 prev, u32 new, int 
> idx, bool *attached)
> return false;
>  }
>
> +static bool is_extcon_property_supported(unsigned int id, unsigned int prop)
> +{
> +   unsigned int type;
> +

int

> +   /* Check whether the property is supported or not. */
> +   type = get_extcon_type(prop);
> +   if (type < 0)

otherwise type is never < 0

> +   return false;
> +
> +   /* Check whether a specific extcon id supports the property or not. */
> +   if (extcon_info[id].type | type)

This is always true ?

> +   return true;
> +
> +   return false;
> +}
> +
> +#define INIT_PROPERTY(name, name_lower, type)  \
> +   if (EXTCON_TYPE_##name || type) {   \

This is always true ?

> +   for (i = 0; i < EXTCON_PROP_##name##_CNT; i++)  \
> +   cable->name_lower##_propval[i] = val;   \
> +   }   \
> +
> +static void init_property(struct extcon_dev *edev, unsigned int id, int 
> index)
> +{
> +   unsigned int type = extcon_info[id].type;
> +   struct extcon_cable *cable = >cables[index];
> +   union extcon_property_value val = (union extcon_property_value)(0);
> +   int i;
> +
> +   INIT_PROPERTY(USB, usb, type);
> +   INIT_PROPERTY(CHG, chg, type);
> +   INIT_PROPERTY(JACK, jack, type);
> +   INIT_PROPERTY(DISP, disp, type);
> +}
> +
>  static ssize_t state_show(struct device *dev, struct device_attribute *attr,
>   char *buf)
>  {
> @@ -421,7 +483,7 @@ int extcon_get_cable_state_(struct extcon_dev *edev, 
> const unsigned int id)
> if (edev->max_supported && edev->max_supported <= index)
> return -EINVAL;
>
> -   return !!(edev->state & (1 << index));
> +   return (int)(is_extcon_attached(edev, id, index));
>  }
>  EXPORT_SYMBOL_GPL(extcon_get_cable_state_);
>
> @@ -449,12 +511,154 @@ int extcon_set_cable_state_(struct extcon_dev *edev, 
> unsigned int id,
> if

Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-26 Thread Guenter Roeck 
On Tue, Jul 26, 2016 at 5:09 AM, Chanwoo Choi  wrote:
> This patch support the extcon property for the external connector
> because each external connector might have the property according to
> the H/W design and the specific characteristics.
>
> - EXTCON_PROP_USB_[property name]
> - EXTCON_PROP_CHG_[property name]
> - EXTCON_PROP_JACK_[property name]
> - EXTCON_PROP_DISP_[property name]
>
> Add the new extcon APIs to get/set the property value as following:
> - int extcon_get_property(struct extcon_dev *edev, unsigned int id,
> unsigned int prop,
> union extcon_property_value *prop_val)
> - int extcon_set_property(struct extcon_dev *edev, unsigned int id,
> unsigned int prop,
> union extcon_property_value prop_val)
>
> Signed-off-by: Chanwoo Choi 
> ---
>  drivers/extcon/extcon.c | 206 
> +++-
>  include/linux/extcon.h  |  86 
>  2 files changed, 291 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index b1e6ee6194bc..2317aaea063f 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -196,6 +196,11 @@ struct extcon_cable {
> struct device_attribute attr_state;
>
> struct attribute *attrs[3]; /* to be fed to attr_g.attrs */
> +
> +   union extcon_property_value usb_propval[EXTCON_PROP_USB_CNT];
> +   union extcon_property_value chg_propval[EXTCON_PROP_CHG_CNT];
> +   union extcon_property_value jack_propval[EXTCON_PROP_JACK_CNT];
> +   union extcon_property_value disp_propval[EXTCON_PROP_DISP_CNT];
>  };
>
>  static struct class *extcon_class;
> @@ -248,6 +253,28 @@ static int find_cable_index_by_id(struct extcon_dev 
> *edev, const unsigned int id
> return -EINVAL;
>  }
>
> +static int get_extcon_type(unsigned int prop)
> +{
> +   switch (prop) {
> +   case EXTCON_PROP_USB_MIN ... EXTCON_PROP_USB_MAX:
> +   return EXTCON_TYPE_USB;
> +   case EXTCON_PROP_CHG_MIN ... EXTCON_PROP_CHG_MAX:
> +   return EXTCON_TYPE_CHG;
> +   case EXTCON_PROP_JACK_MIN ... EXTCON_PROP_JACK_MAX:
> +   return EXTCON_TYPE_JACK;
> +   case EXTCON_PROP_DISP_MIN ... EXTCON_PROP_DISP_MAX:
> +   return EXTCON_TYPE_DISP;
> +   default:
> +   return -EINVAL;
> +   }
> +}
> +
> +static bool is_extcon_attached(struct extcon_dev *edev, unsigned int id,
> +   unsigned int index)
> +{
> +   return ((!!(edev->state & (1 << index))) == 1) ? true : false;
> +}
> +
>  static bool is_extcon_changed(u32 prev, u32 new, int idx, bool *attached)
>  {
> if (((prev >> idx) & 0x1) != ((new >> idx) & 0x1)) {
> @@ -258,6 +285,41 @@ static bool is_extcon_changed(u32 prev, u32 new, int 
> idx, bool *attached)
> return false;
>  }
>
> +static bool is_extcon_property_supported(unsigned int id, unsigned int prop)
> +{
> +   unsigned int type;
> +

int

> +   /* Check whether the property is supported or not. */
> +   type = get_extcon_type(prop);
> +   if (type < 0)

otherwise type is never < 0

> +   return false;
> +
> +   /* Check whether a specific extcon id supports the property or not. */
> +   if (extcon_info[id].type | type)

This is always true ?

> +   return true;
> +
> +   return false;
> +}
> +
> +#define INIT_PROPERTY(name, name_lower, type)  \
> +   if (EXTCON_TYPE_##name || type) {   \

This is always true ?

> +   for (i = 0; i < EXTCON_PROP_##name##_CNT; i++)  \
> +   cable->name_lower##_propval[i] = val;   \
> +   }   \
> +
> +static void init_property(struct extcon_dev *edev, unsigned int id, int 
> index)
> +{
> +   unsigned int type = extcon_info[id].type;
> +   struct extcon_cable *cable = >cables[index];
> +   union extcon_property_value val = (union extcon_property_value)(0);
> +   int i;
> +
> +   INIT_PROPERTY(USB, usb, type);
> +   INIT_PROPERTY(CHG, chg, type);
> +   INIT_PROPERTY(JACK, jack, type);
> +   INIT_PROPERTY(DISP, disp, type);
> +}
> +
>  static ssize_t state_show(struct device *dev, struct device_attribute *attr,
>   char *buf)
>  {
> @@ -421,7 +483,7 @@ int extcon_get_cable_state_(struct extcon_dev *edev, 
> const unsigned int id)
> if (edev->max_supported && edev->max_supported <= index)
> return -EINVAL;
>
> -   return !!(edev->state & (1 << index));
> +   return (int)(is_extcon_attached(edev, id, index));
>  }
>  EXPORT_SYMBOL_GPL(extcon_get_cable_state_);
>
> @@ -449,12 +511,154 @@ int extcon_set_cable_state_(struct extcon_dev *edev, 
> unsigned int id,
> if (edev->max_supported && edev->max_supported <=

[PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-26 Thread Chanwoo Choi 
This patch support the extcon property for the external connector
because each external connector might have the property according to
the H/W design and the specific characteristics.

- EXTCON_PROP_USB_[property name]
- EXTCON_PROP_CHG_[property name]
- EXTCON_PROP_JACK_[property name]
- EXTCON_PROP_DISP_[property name]

Add the new extcon APIs to get/set the property value as following:
- int extcon_get_property(struct extcon_dev *edev, unsigned int id,
unsigned int prop,
union extcon_property_value *prop_val)
- int extcon_set_property(struct extcon_dev *edev, unsigned int id,
unsigned int prop,
union extcon_property_value prop_val)

Signed-off-by: Chanwoo Choi 
---
 drivers/extcon/extcon.c | 206 +++-
 include/linux/extcon.h  |  86 
 2 files changed, 291 insertions(+), 1 deletion(-)

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index b1e6ee6194bc..2317aaea063f 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -196,6 +196,11 @@ struct extcon_cable {
struct device_attribute attr_state;
 
struct attribute *attrs[3]; /* to be fed to attr_g.attrs */
+
+   union extcon_property_value usb_propval[EXTCON_PROP_USB_CNT];
+   union extcon_property_value chg_propval[EXTCON_PROP_CHG_CNT];
+   union extcon_property_value jack_propval[EXTCON_PROP_JACK_CNT];
+   union extcon_property_value disp_propval[EXTCON_PROP_DISP_CNT];
 };
 
 static struct class *extcon_class;
@@ -248,6 +253,28 @@ static int find_cable_index_by_id(struct extcon_dev *edev, 
const unsigned int id
return -EINVAL;
 }
 
+static int get_extcon_type(unsigned int prop)
+{
+   switch (prop) {
+   case EXTCON_PROP_USB_MIN ... EXTCON_PROP_USB_MAX:
+   return EXTCON_TYPE_USB;
+   case EXTCON_PROP_CHG_MIN ... EXTCON_PROP_CHG_MAX:
+   return EXTCON_TYPE_CHG;
+   case EXTCON_PROP_JACK_MIN ... EXTCON_PROP_JACK_MAX:
+   return EXTCON_TYPE_JACK;
+   case EXTCON_PROP_DISP_MIN ... EXTCON_PROP_DISP_MAX:
+   return EXTCON_TYPE_DISP;
+   default:
+   return -EINVAL;
+   }
+}
+
+static bool is_extcon_attached(struct extcon_dev *edev, unsigned int id,
+   unsigned int index)
+{
+   return ((!!(edev->state & (1 << index))) == 1) ? true : false;
+}
+
 static bool is_extcon_changed(u32 prev, u32 new, int idx, bool *attached)
 {
if (((prev >> idx) & 0x1) != ((new >> idx) & 0x1)) {
@@ -258,6 +285,41 @@ static bool is_extcon_changed(u32 prev, u32 new, int idx, 
bool *attached)
return false;
 }
 
+static bool is_extcon_property_supported(unsigned int id, unsigned int prop)
+{
+   unsigned int type;
+
+   /* Check whether the property is supported or not. */
+   type = get_extcon_type(prop);
+   if (type < 0)
+   return false;
+
+   /* Check whether a specific extcon id supports the property or not. */
+   if (extcon_info[id].type | type)
+   return true;
+
+   return false;
+}
+
+#define INIT_PROPERTY(name, name_lower, type)  \
+   if (EXTCON_TYPE_##name || type) {   \
+   for (i = 0; i < EXTCON_PROP_##name##_CNT; i++)  \
+   cable->name_lower##_propval[i] = val;   \
+   }   \
+
+static void init_property(struct extcon_dev *edev, unsigned int id, int index)
+{
+   unsigned int type = extcon_info[id].type;
+   struct extcon_cable *cable = >cables[index];
+   union extcon_property_value val = (union extcon_property_value)(0);
+   int i;
+
+   INIT_PROPERTY(USB, usb, type);
+   INIT_PROPERTY(CHG, chg, type);
+   INIT_PROPERTY(JACK, jack, type);
+   INIT_PROPERTY(DISP, disp, type);
+}
+
 static ssize_t state_show(struct device *dev, struct device_attribute *attr,
  char *buf)
 {
@@ -421,7 +483,7 @@ int extcon_get_cable_state_(struct extcon_dev *edev, const 
unsigned int id)
if (edev->max_supported && edev->max_supported <= index)
return -EINVAL;
 
-   return !!(edev->state & (1 << index));
+   return (int)(is_extcon_attached(edev, id, index));
 }
 EXPORT_SYMBOL_GPL(extcon_get_cable_state_);
 
@@ -449,12 +511,154 @@ int extcon_set_cable_state_(struct extcon_dev *edev, 
unsigned int id,
if (edev->max_supported && edev->max_supported <= index)
return -EINVAL;
 
+   /*
+* Initialize the value of extcon property before setting
+* the detached state for an external connector.
+*/
+   if (!cable_state)
+   init_property(edev, id, index);
+
+   /* Set the state for external connector as the detached state. */
state =

[PATCH 2/6] extcon: Add the support for extcon property according to extcon type

2016-07-26 Thread Chanwoo Choi 
This patch support the extcon property for the external connector
because each external connector might have the property according to
the H/W design and the specific characteristics.

- EXTCON_PROP_USB_[property name]
- EXTCON_PROP_CHG_[property name]
- EXTCON_PROP_JACK_[property name]
- EXTCON_PROP_DISP_[property name]

Add the new extcon APIs to get/set the property value as following:
- int extcon_get_property(struct extcon_dev *edev, unsigned int id,
unsigned int prop,
union extcon_property_value *prop_val)
- int extcon_set_property(struct extcon_dev *edev, unsigned int id,
unsigned int prop,
union extcon_property_value prop_val)

Signed-off-by: Chanwoo Choi 
---
 drivers/extcon/extcon.c | 206 +++-
 include/linux/extcon.h  |  86 
 2 files changed, 291 insertions(+), 1 deletion(-)

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index b1e6ee6194bc..2317aaea063f 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -196,6 +196,11 @@ struct extcon_cable {
struct device_attribute attr_state;
 
struct attribute *attrs[3]; /* to be fed to attr_g.attrs */
+
+   union extcon_property_value usb_propval[EXTCON_PROP_USB_CNT];
+   union extcon_property_value chg_propval[EXTCON_PROP_CHG_CNT];
+   union extcon_property_value jack_propval[EXTCON_PROP_JACK_CNT];
+   union extcon_property_value disp_propval[EXTCON_PROP_DISP_CNT];
 };
 
 static struct class *extcon_class;
@@ -248,6 +253,28 @@ static int find_cable_index_by_id(struct extcon_dev *edev, 
const unsigned int id
return -EINVAL;
 }
 
+static int get_extcon_type(unsigned int prop)
+{
+   switch (prop) {
+   case EXTCON_PROP_USB_MIN ... EXTCON_PROP_USB_MAX:
+   return EXTCON_TYPE_USB;
+   case EXTCON_PROP_CHG_MIN ... EXTCON_PROP_CHG_MAX:
+   return EXTCON_TYPE_CHG;
+   case EXTCON_PROP_JACK_MIN ... EXTCON_PROP_JACK_MAX:
+   return EXTCON_TYPE_JACK;
+   case EXTCON_PROP_DISP_MIN ... EXTCON_PROP_DISP_MAX:
+   return EXTCON_TYPE_DISP;
+   default:
+   return -EINVAL;
+   }
+}
+
+static bool is_extcon_attached(struct extcon_dev *edev, unsigned int id,
+   unsigned int index)
+{
+   return ((!!(edev->state & (1 << index))) == 1) ? true : false;
+}
+
 static bool is_extcon_changed(u32 prev, u32 new, int idx, bool *attached)
 {
if (((prev >> idx) & 0x1) != ((new >> idx) & 0x1)) {
@@ -258,6 +285,41 @@ static bool is_extcon_changed(u32 prev, u32 new, int idx, 
bool *attached)
return false;
 }
 
+static bool is_extcon_property_supported(unsigned int id, unsigned int prop)
+{
+   unsigned int type;
+
+   /* Check whether the property is supported or not. */
+   type = get_extcon_type(prop);
+   if (type < 0)
+   return false;
+
+   /* Check whether a specific extcon id supports the property or not. */
+   if (extcon_info[id].type | type)
+   return true;
+
+   return false;
+}
+
+#define INIT_PROPERTY(name, name_lower, type)  \
+   if (EXTCON_TYPE_##name || type) {   \
+   for (i = 0; i < EXTCON_PROP_##name##_CNT; i++)  \
+   cable->name_lower##_propval[i] = val;   \
+   }   \
+
+static void init_property(struct extcon_dev *edev, unsigned int id, int index)
+{
+   unsigned int type = extcon_info[id].type;
+   struct extcon_cable *cable = >cables[index];
+   union extcon_property_value val = (union extcon_property_value)(0);
+   int i;
+
+   INIT_PROPERTY(USB, usb, type);
+   INIT_PROPERTY(CHG, chg, type);
+   INIT_PROPERTY(JACK, jack, type);
+   INIT_PROPERTY(DISP, disp, type);
+}
+
 static ssize_t state_show(struct device *dev, struct device_attribute *attr,
  char *buf)
 {
@@ -421,7 +483,7 @@ int extcon_get_cable_state_(struct extcon_dev *edev, const 
unsigned int id)
if (edev->max_supported && edev->max_supported <= index)
return -EINVAL;
 
-   return !!(edev->state & (1 << index));
+   return (int)(is_extcon_attached(edev, id, index));
 }
 EXPORT_SYMBOL_GPL(extcon_get_cable_state_);
 
@@ -449,12 +511,154 @@ int extcon_set_cable_state_(struct extcon_dev *edev, 
unsigned int id,
if (edev->max_supported && edev->max_supported <= index)
return -EINVAL;
 
+   /*
+* Initialize the value of extcon property before setting
+* the detached state for an external connector.
+*/
+   if (!cable_state)
+   init_property(edev, id, index);
+
+   /* Set the state for external connector as the detached state. */
state = cable_state ? (1 <<