lkml.org 
[lkml]   [2016]   [Aug]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 2/6] extcon: Add the support for extcon property according to extcon type
Hi Guenter,

On 2016년 08월 02일 04:48, Guenter Roeck wrote:
> On Sun, Jul 31, 2016 at 10:50 PM, Chanwoo Choi <cw00.choi@samsung.com> 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 <cw00.choi@samsung.com>
>> Tested-by: Chris Zhong <zyw@rock-chips.com>
>
> Reviewed-by: Guenter Roeck <groeck@chromium.org>
>
> [ couple of nitpicks below ]
>
>> ---
>> drivers/extcon/extcon.c | 201 +++++++++++++++++++++++++++++++++++++++++++++++-
>> include/linux/extcon.h | 86 +++++++++++++++++++++
>> 2 files changed, 286 insertions(+), 1 deletion(-)
>>

[snip]

>> static ssize_t state_show(struct device *dev, struct device_attribute *attr,
>> char *buf)
>> {
>> @@ -421,7 +475,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, index));
>
> Conversion from bool to int (0, 1) is automatic, so
>
> return is_extcon_attached(edev, index);
>
> would be identical.

OK. I'll fix it.

[snip]

>> diff --git a/include/linux/extcon.h b/include/linux/extcon.h
>> index 46d802892c82..5a97e52abefb 100644
>> --- a/include/linux/extcon.h
>> +++ b/include/linux/extcon.h
>> @@ -77,6 +77,68 @@
>>
>> #define EXTCON_NUM 63
>>
>> +/*
>> + * Define the property of supported external connectors.
>> + *
>> + * When adding the new extcon property, they *must* have
>> + * the type/value/default information. Also, you *have to*
>> + * modify the EXTCON_PROP_[type]_START/END definitions
>> + * which mean the range of the supported properties
>> + * for each extcon type.
>> + *
>> + * The naming style of property
>> + * : EXTCON_PROP_[type]_[property name]
>> + *
>> + * EXTCON_PROP_USB_[property name] : USB property
>> + * EXTCON_PROP_CHG_[property name] : Charger property
>> + * EXTCON_PROP_JACK_[property name] : Jack property
>> + * EXTCON_PROP_DISP_[property name] : Display property
>> + */
>> +
>> +/*
>> + * Properties of EXTCON_TYPE_USB.
>> + *
>> + * - EXTCON_PROP_USB_ID
>> + * @type: integer (intval)
>> + * @value: 0 (low) or 1 (high)
>> + * @default: 0 (low)
>> + * - EXTCON_PROP_USB_VBUS
>> + * @type: integer (intval)
>> + * @value: 0 (low) or 1 (high)
>> + * @default: 0 (low)
>> + */
>> +#define EXTCON_PROP_USB_ID 0
>> +#define EXTCON_PROP_USB_VBUS 1
>> +
>> +#define EXTCON_PROP_USB_MIN 0
>> +#define EXTCON_PROP_USB_MAX 1
>> +#define EXTCON_PROP_USB_CNT (EXTCON_PROP_USB_MAX - EXTCON_PROP_USB_MIN + 1)
>> +
>> +/* Properties of EXTCON_TYPE_CHG. */
>> +#define EXTCON_PROP_CHG_MIN 50
>> +#define EXTCON_PROP_CHG_MAX 50
>> +#define EXTCON_PROP_CHG_CNT (EXTCON_PROP_CHG_MAX - EXTCON_PROP_CHG_MIN + 1)
>> +
>> +/* Properties of EXTCON_TYPE_JACK. */
>> +#define EXTCON_PROP_JACK_MIN 100
>> +#define EXTCON_PROP_JACK_MAX 100
>> +#define EXTCON_PROP_JACK_CNT (EXTCON_PROP_JACK_MAX - EXTCON_PROP_JACK_MIN + 1)
>> +
>> +/* 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)
>> +
>> +/*
>> + * Define the type of property's value.
>> + *
>> + * Define the property's value as union type. Because each property
>> + * would need the different data type to store it.
>> + */
>> +union extcon_property_value {
>> + int intval; /* type : interger (intval) */
>
> integer

OK. I'll fix it.

Thanks for review.

Regards,
Chanwoo Choi


\
 
 \ /
  Last update: 2016-08-02 12:21    [W:0.236 / U:0.048 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site