Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1450551 > unrolled thread
| Started by | Chanwoo Choi <cw00.choi@samsung.com> |
|---|---|
| First post | 2016-07-26 14:20 +0200 |
| Last post | 2016-07-29 09:10 +0200 |
| Articles | 11 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 2/6] extcon: Add the support for extcon property according to extcon type Chanwoo Choi <cw00.choi@samsung.com> - 2016-07-26 14:20 +0200
Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type Guenter Roeck <groeck@google.com> - 2016-07-27 00:10 +0200
Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type Chris Zhong <zyw@rock-chips.com> - 2016-07-27 03:20 +0200
Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type Guenter Roeck <groeck@google.com> - 2016-07-27 03:50 +0200
Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type Chris Zhong <zyw@rock-chips.com> - 2016-07-27 04:20 +0200
Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type Chanwoo Choi <cw00.choi@samsung.com> - 2016-07-27 05:50 +0200
Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type Chanwoo Choi <cw00.choi@samsung.com> - 2016-07-27 06:00 +0200
Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type Chris Zhong <zyw@rock-chips.com> - 2016-07-27 06:40 +0200
Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type Guenter Roeck <groeck@google.com> - 2016-07-27 06:00 +0200
Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type Guenter Roeck <groeck@google.com> - 2016-07-27 19:30 +0200
Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type Chanwoo Choi <cw00.choi@samsung.com> - 2016-07-29 09:10 +0200
| From | Chanwoo Choi <cw00.choi@samsung.com> |
|---|---|
| Date | 2016-07-26 14:20 +0200 |
| Subject | [PATCH 2/6] extcon: Add the support for extcon property according to extcon type |
| Message-ID | <rZ9v3-1PL-3@gated-at.bofh.it> |
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>
---
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 = &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)
+
+/* 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)
+
+/* 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)
+
+/* 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)
+
+/*
+ * 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
[toc] | [next] | [standalone]
| From | Guenter Roeck <groeck@google.com> |
|---|---|
| Date | 2016-07-27 00:10 +0200 |
| Subject | Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type |
| Message-ID | <rZiI1-7He-1@gated-at.bofh.it> |
| In reply to | #1450551 |
On Tue, Jul 26, 2016 at 5:09 AM, 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>
> ---
> 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 = &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).
> +
> +/* 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.
> +/* 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) */
> +};
> +
> 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
>
[toc] | [prev] | [next] | [standalone]
| From | Chris Zhong <zyw@rock-chips.com> |
|---|---|
| Date | 2016-07-27 03:20 +0200 |
| Subject | Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type |
| Message-ID | <rZlFU-16J-9@gated-at.bofh.it> |
| In reply to | #1450942 |
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
[toc] | [prev] | [next] | [standalone]
| From | Guenter Roeck <groeck@google.com> |
|---|---|
| Date | 2016-07-27 03:50 +0200 |
| Subject | Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type |
| Message-ID | <rZm8V-1gn-1@gated-at.bofh.it> |
| In reply to | #1450983 |
Hi Chris, On Tue, Jul 26, 2016 at 6:15 PM, Chris Zhong <zyw@rock-chips.com> 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
[toc] | [prev] | [next] | [standalone]
| From | Chris Zhong <zyw@rock-chips.com> |
|---|---|
| Date | 2016-07-27 04:20 +0200 |
| Subject | Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type |
| Message-ID | <rZmBX-1Ls-7@gated-at.bofh.it> |
| In reply to | #1450997 |
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 <zyw@rock-chips.com> 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 > > >
[toc] | [prev] | [next] | [standalone]
| From | Chanwoo Choi <cw00.choi@samsung.com> |
|---|---|
| Date | 2016-07-27 05:50 +0200 |
| Subject | Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type |
| Message-ID | <rZo14-2xs-9@gated-at.bofh.it> |
| In reply to | #1451004 |
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 <zyw@rock-chips.com> 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
[toc] | [prev] | [next] | [standalone]
| From | Chanwoo Choi <cw00.choi@samsung.com> |
|---|---|
| Date | 2016-07-27 06:00 +0200 |
| Subject | Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type |
| Message-ID | <rZoaJ-2AM-3@gated-at.bofh.it> |
| In reply to | #1451025 |
On 2016년 07월 27일 12:51, Guenter Roeck wrote: > On Tue, Jul 26, 2016 at 8:42 PM, Chanwoo Choi <cw00.choi@samsung.com> 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 <zyw@rock-chips.com> 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
[toc] | [prev] | [next] | [standalone]
| From | Chris Zhong <zyw@rock-chips.com> |
|---|---|
| Date | 2016-07-27 06:40 +0200 |
| Subject | Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type |
| Message-ID | <rZoNr-38e-9@gated-at.bofh.it> |
| In reply to | #1451028 |
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 <cw00.choi@samsung.com> 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 <zyw@rock-chips.com> 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 <zyw@rock-chips.com> 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 > > >
[toc] | [prev] | [next] | [standalone]
| From | Guenter Roeck <groeck@google.com> |
|---|---|
| Date | 2016-07-27 06:00 +0200 |
| Subject | Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type |
| Message-ID | <rZoaJ-2AM-5@gated-at.bofh.it> |
| In reply to | #1451025 |
On Tue, Jul 26, 2016 at 8:42 PM, Chanwoo Choi <cw00.choi@samsung.com> 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 <zyw@rock-chips.com> 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
[toc] | [prev] | [next] | [standalone]
| From | Guenter Roeck <groeck@google.com> |
|---|---|
| Date | 2016-07-27 19:30 +0200 |
| Subject | Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type |
| Message-ID | <rZAOC-2kF-25@gated-at.bofh.it> |
| In reply to | #1450551 |
On Tue, Jul 26, 2016 at 5:09 AM, 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>
> ---
> 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 = &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);
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 EXTCON_TYPE_JACK:
memset(cable->jack_propval, sizeof(cable->jack_propval), 0);
break;
case EXTCON_TYPE_DISP:
memset(cable->disp_propval, sizeof(cable->disp_propval), 0);
break;
}
> +}
> +
> 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;
> +
Wonder if this should be inside the lock. Otherwise the cable state
might change after the check, but before the lock is acquired.
> + 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)
> +
> +/* 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)
> +
> +/* 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)
> +
> +/* 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)
> +
> +/*
> + * 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
>
[toc] | [prev] | [next] | [standalone]
| From | Chanwoo Choi <cw00.choi@samsung.com> |
|---|---|
| Date | 2016-07-29 09:10 +0200 |
| Subject | Re: [PATCH 2/6] extcon: Add the support for extcon property according to extcon type |
| Message-ID | <s0a5H-1oZ-7@gated-at.bofh.it> |
| In reply to | #1451429 |
Hi Guenter,
On 2016년 07월 28일 02:24, Guenter Roeck wrote:
> On Tue, Jul 26, 2016 at 5:09 AM, 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>
>> ---
>> 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 = &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);
>
> 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 EXTCON_TYPE_JACK:
> memset(cable->jack_propval, sizeof(cable->jack_propval), 0);
> break;
> case EXTCON_TYPE_DISP:
> memset(cable->disp_propval, sizeof(cable->disp_propval), 0);
> break;
> }
As you comment, I'll modify it as following:
But the each id is able to have the one more extcon type. So, I use the
'if' instead of 'switch'.
if (EXTCON_TYPE_USB & type)
memset(cable->usb_propval, sizeof(cable->usb_propval), 0);
if (EXTCON_TYPE_CHG & type)
memset(cable->chg_propval, sizeof(cable->chg_propval), 0);
if (EXTCON_TYPE_JACK & type)
memset(cable->jack_propval, sizeof(cable->jack_propval), 0);
if (EXTCON_TYPE_DISP & type)
memset(cable->disp_propval, sizeof(cable->disp_propval), 0);
>
>> +}
>> +
>> 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;
>> +
>
> Wonder if this should be inside the lock. Otherwise the cable state
> might change after the check, but before the lock is acquired.
>
>> + cable = &edev->cables[index];
>> + spin_lock_irqsave(&edev->lock, flags);
You're right. I'll fix it as following.
spin_lock_irqsave(&edev->lock, flags);
if (!is_extcon_attached(edev, id, index)) {
spin_unlock_irqrestore(&edev->lock, flags);
return 0;
}
cable = &edev->cables[index];
>> +
>> + /* 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);
[snip]
Regards,
Chanwoo Choi
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web