Hi Guenter, On 2016년 08월 02일 04:48, Guenter Roeck wrote: > On Sun, Jul 31, 2016 at 10:50 PM, Chanwoo Choi <cw00.c...@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.c...@samsung.com> >> Tested-by: Chris Zhong <z...@rock-chips.com> > > Reviewed-by: Guenter Roeck <gro...@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