Messages in this thread | | | Subject | Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type | From | Chris Zhong <> | Date | Wed, 27 Jul 2016 09:15:25 +0800 |
| |
Hi Chanwoo
On 07/27/2016 08:31 AM, Chanwoo Choi wrote: > Hi Guenter, > > 2016년 7월 27일 수요일, Guenter Roeck<groeck@google.com > <mailto:groeck@google.com>>님이 작성한 메시지: > > On Tue, Jul 26, 2016 at 5:09 AM, Chanwoo Choi > <cw00.choi@samsung.com <javascript:;>> 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 <javascript:;>> > > --- > > 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 = &edev->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 << index) : 0; > > return extcon_update_state(edev, 1 << index, state); > > } > > EXPORT_SYMBOL_GPL(extcon_set_cable_state_); > > > > /** > > + * extcon_get_property() - Get the property value of a specific > cable. > > + * @edev: the extcon device that has the cable. > > + * @id: the unique id of each external > connector > > + * in extcon enumeration. > > + * @prop: the property id among enum extcon_property. > > + * @prop_val: the pointer which store the value of > property. > > + * > > + * When getting the property value of external connector, the > external connector > > + * should be attached. If detached state, function just return > 0 without > > + * property value. Also, the each property should be included > in the list of > > + * supported properties according to the type of external > connectors. > > + * > > + * Returns 0 if success or error number if fail > > + */ > > +int extcon_get_property(struct extcon_dev *edev, unsigned int id, > > + unsigned int prop, > > + union extcon_property_value > *prop_val) > > +{ > > + struct extcon_cable *cable; > > + unsigned long flags; > > + int index, ret = 0; > > + > > + *prop_val = (union extcon_property_value)(0); > > + > > + if (!edev) > > + return -EINVAL; > > + > > + /* Check whether the property 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; > > + > > + /* > > + * Check whether the external connector is attached. > > + * If external connector is detached, the user can not > > + * get the property value. > > + */ > > + if (!is_extcon_attached(edev, id, index)) > > + return 0; > > + > > + cable = &edev->cables[index]; > > + spin_lock_irqsave(&edev->lock, flags); > > + > > + /* Get the property value according to extcon type */ > > + switch (prop) { > > + case EXTCON_PROP_USB_MIN ... EXTCON_PROP_USB_MAX: > > + *prop_val = cable->usb_propval[prop - > EXTCON_PROP_USB_MIN]; > > + break; > > + case EXTCON_PROP_CHG_MIN ... EXTCON_PROP_CHG_MAX: > > + *prop_val = cable->chg_propval[prop - > EXTCON_PROP_CHG_MIN]; > > + break; > > + case EXTCON_PROP_JACK_MIN ... EXTCON_PROP_JACK_MAX: > > + *prop_val = cable->jack_propval[prop - > EXTCON_PROP_JACK_MIN]; > > + break; > > + case EXTCON_PROP_DISP_MIN ... EXTCON_PROP_DISP_MAX: > > + *prop_val = cable->disp_propval[prop - > EXTCON_PROP_DISP_MIN]; > > + break; > > + default: > > + ret = -EINVAL; > > + break; > > + } > > + > > + spin_unlock_irqrestore(&edev->lock, flags); > > + > > + return ret; > > +} > > +EXPORT_SYMBOL_GPL(extcon_get_property); > > + > > +/** > > + * extcon_set_property() - Set the property value of a specific > cable. > > + * @edev: the extcon device that has the cable. > > + * @id: the unique id of each external > connector > > + * in extcon enumeration. > > + * @prop: the property id among enum extcon_property. > > + * @prop_val: the pointer including the new value of > property. > > + * > > + * The each property should be included in the list of > supported properties > > + * according to the type of external connectors. > > + * > > + * Returns 0 if success or error number if fail > > + */ > > +int extcon_set_property(struct extcon_dev *edev, unsigned int id, > > + unsigned int prop, > > + union extcon_property_value > prop_val) > > +{ > > + struct extcon_cable *cable; > > + unsigned long flags; > > + int index, ret = 0; > > + > > + if (!edev) > > + return -EINVAL; > > + > > + /* Check whether the property 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; > > + > > + cable = &edev->cables[index]; > > + spin_lock_irqsave(&edev->lock, flags); > > + > > + /* Set the property value according to extcon type */ > > + switch (prop) { > > + case EXTCON_PROP_USB_MIN ... EXTCON_PROP_USB_MAX: > > + cable->usb_propval[prop - EXTCON_PROP_USB_MIN] = > prop_val; > > + break; > > + case EXTCON_PROP_CHG_MIN ... EXTCON_PROP_CHG_MAX: > > + cable->chg_propval[prop - EXTCON_PROP_CHG_MIN] = > prop_val; > > + break; > > + case EXTCON_PROP_JACK_MIN ... EXTCON_PROP_JACK_MAX: > > + cable->jack_propval[prop - EXTCON_PROP_JACK_MIN] > = prop_val; > > + break; > > + case EXTCON_PROP_DISP_MIN ... EXTCON_PROP_DISP_MAX: > > + cable->disp_propval[prop - EXTCON_PROP_DISP_MIN] > = prop_val; > > + break; > > + default: > > + ret = -EINVAL; > > + break; > > + } > > + > > + spin_unlock_irqrestore(&edev->lock, flags); > > + > > + return ret; > > +} > > +EXPORT_SYMBOL_GPL(extcon_set_property); > > + > > +/** > > * extcon_get_extcon_dev() - Get the extcon device instance > from the name > > * @extcon_name: The extcon name provided with > extcon_dev_register() > > */ > > diff --git a/include/linux/extcon.h b/include/linux/extcon.h > > index 46d802892c82..296d1452dcb4 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) > > EXTCON_PROP_USB_MAX - EXTCON_PROP_USB_MIN + 1 > > Otherwise the array won't have enough entries, and writing the last > property will end up overwriting usb_bits (because all other arrays > have a size of 0) > > > You're right. I'll fix it. > > > > + > > +/* 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) > > EXTCON_PROP_CHG_MAX - EXTCON_PROP_CHG_MIN + 1 > > Otherwise the array won't have any entries. > > > ok. > > > > +/* 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 > > > ok. > > > > + > > +/* 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.
> > > + > > +/* > > + * 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) */ > > +}; > > + > > struct extcon_cable; > > > > /** > > @@ -167,6 +229,17 @@ extern int extcon_set_cable_state_(struct > extcon_dev *edev, unsigned int id, > > bool cable_state); > > > > /* > > + * get/set_property access the property value of each external > connector. > > + * They are used to access the property of each cable based on > the property id. > > + */ > > +extern int extcon_get_property(struct extcon_dev *edev, > unsigned int id, > > + unsigned int prop, > > + union extcon_property_value > *prop_val); > > +extern int extcon_set_property(struct extcon_dev *edev, > unsigned int id, > > + unsigned int prop, > > + union extcon_property_value > prop_val); > > + > > +/* > > * Following APIs are to monitor every action of a notifier. > > * Registrar gets notified for every external port of a > connection device. > > * Probably this could be used to debug an action of notifier; > however, > > @@ -239,6 +312,19 @@ static inline int > extcon_set_cable_state_(struct extcon_dev *edev, > > return 0; > > } > > > > +static inline int extcon_get_property(struct extcon_dev *edev, > unsigned int id, > > + unsigned int prop, > > + union > extcon_property_value *prop_val) > > +{ > > + return 0; > > +} > > +static inline int extcon_set_property(struct extcon_dev *edev, > unsigned int id, > > + unsigned int prop, > > + union > extcon_property_value prop_val) > > +{ > > + return 0; > > +} > > + > > static inline struct extcon_dev *extcon_get_extcon_dev(const > char *extcon_name) > > { > > return NULL; > > -- > > 1.9.1 > > > > > Regards, > Chanwoo Choi
| |