[PATCH 0/4] extcon: Block the usgae of extcon_set/get/update_state() except for extcon core

[PATCH 0/4] extcon: Block the usgae of extcon_set/get/update_state() except for extcon core

[Chanwoo Choi]

This patchset blocks the usgae of extcon_get/get_update_state()
except for extcon core. Instead, extcon framework provide the
other APIs with unique extcon id as following:
- extcon_set_cable_state_()
- extcon_get_cable_state_()

Chanwoo Choi (4):
  extcon: arizona: Remove the usage of extcon_update_state()
  extcon: adc-jack: Remove the usage of extcon_set_state()
  extcon: Remove the state_store() to prevent the wrong access
  extcon: Block the bit masking operation for cable state except for extcon core

 drivers/extcon/extcon-adc-jack.c       | 26 +++++++++++----------
 drivers/extcon/extcon-arizona.c        | 11 +++++----
 drivers/extcon/extcon.c                | 41 ++--------------------------------
 include/linux/extcon.h                 | 30 -------------------------
 include/linux/extcon/extcon-adc-jack.h |  4 ++--
 5 files changed, 25 insertions(+), 87 deletions(-)

-- 
1.9.1

[PATCH 1/4] extcon: arizona: Remove the usage of extcon_update_state()

[Chanwoo Choi]

This patch remvoes the usage of extcon_update_state() because
the extcon_update_state() use directly the bit masking calculation
to change the state of external connector without the unique id of
external connector. It makes the code diffcult to read it.
So, this patch uses the extcon_set_cable_state_() instead.

Cc: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Cc: patches@opensource.wolfsonmicro.com
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/extcon/extcon-arizona.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index 1d8e0a57bd51..a7161ecd0a1a 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -1149,10 +1149,13 @@ static irqreturn_t arizona_jackdet(int irq, void *data)
 					 info->micd_ranges[i].key, 0);
 		input_sync(info->input);
 
-		ret = extcon_update_state(info->edev, 0xffffffff, 0);
-		if (ret != 0)
-			dev_err(arizona->dev, "Removal report failed: %d\n",
-				ret);
+		for (i = 0; i < ARRAY_SIZE(arizona_cable) - 1; i++) {
+			ret = extcon_set_cable_state_(info->edev,
+					arizona_cable[i], false);
+			if (ret != 0)
+				dev_err(arizona->dev,
+					"Removal report failed: %d\n", ret);
+		}
 
 		regmap_update_bits(arizona->regmap,
 				   ARIZONA_JACK_DETECT_DEBOUNCE,
-- 
1.9.1

[PATCH 2/4] extcon: adc-jack: Remove the usage of extcon_set_state()

[Chanwoo Choi]

This patch removes the usage of extcon_set_state() because it uses the bit
masking to change the state of external connectors. The extcon framework
should handle the state by extcon_set/get_cable_state_() with extcon id.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/extcon/extcon-adc-jack.c       | 26 ++++++++++++++------------
 include/linux/extcon/extcon-adc-jack.h |  4 ++--
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c
index 48dec94b487b..e62e6cd7e00a 100644
--- a/drivers/extcon/extcon-adc-jack.c
+++ b/drivers/extcon/extcon-adc-jack.c
@@ -3,6 +3,9 @@
  *
  * Analog Jack extcon driver with ADC-based detection capability.
  *
+ * Copyright (C) 2016 Samsung Electronics
+ * Chanwoo Choi <cw00.choi@samsung.com>
+ *
  * Copyright (C) 2012 Samsung Electronics
  * MyungJoo Ham <myungjoo.ham@samsung.com>
  *
@@ -58,7 +61,7 @@ static void adc_jack_handler(struct work_struct *work)
 	struct adc_jack_data *data = container_of(to_delayed_work(work),
 			struct adc_jack_data,
 			handler);
-	u32 state = 0;
+	struct adc_jack_cond *def;
 	int ret, adc_val;
 	int i;
 
@@ -70,17 +73,18 @@ static void adc_jack_handler(struct work_struct *work)
 
 	/* Get state from adc value with adc_conditions */
 	for (i = 0; i < data->num_conditions; i++) {
-		struct adc_jack_cond *def = &data->adc_conditions[i];
-		if (!def->state)
-			break;
+		def = &data->adc_conditions[i];
 		if (def->min_adc <= adc_val && def->max_adc >= adc_val) {
-			state = def->state;
-			break;
+			extcon_set_cable_state_(data->edev, def->id, true);
+			return;
 		}
 	}
-	/* if no def has met, it means state = 0 (no cables attached) */
 
-	extcon_set_state(data->edev, state);
+	/* Set the detached state if adc value is not included in the range */
+	for (i = 0; i < data->num_conditions; i++) {
+		def = &data->adc_conditions[i];
+		extcon_set_cable_state_(data->edev, def->id, false);
+	}
 }
 
 static irqreturn_t adc_jack_irq_thread(int irq, void *_data)
@@ -114,16 +118,14 @@ static int adc_jack_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
-	if (!pdata->adc_conditions ||
-			!pdata->adc_conditions[0].state) {
+	if (!pdata->adc_conditions) {
 		dev_err(&pdev->dev, "error: adc_conditions not defined.\n");
 		return -EINVAL;
 	}
 	data->adc_conditions = pdata->adc_conditions;
 
 	/* Check the length of array and set num_conditions */
-	for (i = 0; data->adc_conditions[i].state; i++)
-		;
+	for (i = 0; data->adc_conditions[i].id != EXTCON_NONE; i++);
 	data->num_conditions = i;
 
 	data->chan = iio_channel_get(&pdev->dev, pdata->consumer_channel);
diff --git a/include/linux/extcon/extcon-adc-jack.h b/include/linux/extcon/extcon-adc-jack.h
index ac85f2061351..a0e03b13b449 100644
--- a/include/linux/extcon/extcon-adc-jack.h
+++ b/include/linux/extcon/extcon-adc-jack.h
@@ -20,8 +20,8 @@
 
 /**
  * struct adc_jack_cond - condition to use an extcon state
- * @state:		the corresponding extcon state (if 0, this struct
  *			denotes the last adc_jack_cond element among the array)
+ * @id:			the unique id of each external connector
  * @min_adc:		min adc value for this condition
  * @max_adc:		max adc value for this condition
  *
@@ -33,7 +33,7 @@
  * because when no adc_jack_cond is met, state = 0 is automatically chosen.
  */
 struct adc_jack_cond {
-	u32 state;	/* extcon state value. 0 if invalid */
+	unsigned int id;
 	u32 min_adc;
 	u32 max_adc;
 };
-- 
1.9.1

[PATCH 3/4] extcon: Remove the state_store() to prevent the wrong access

[Chanwoo Choi]

This patch removes the state_store() which change the state of external
connectors with bit masking on user-space. It is wrong access to modify
the change the state of external connectors.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/extcon/extcon.c | 21 +--------------------
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 319659c6af28..ad5e4546f82c 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -174,26 +174,7 @@ static ssize_t state_show(struct device *dev, struct device_attribute *attr,
 
 	return count;
 }
-
-static ssize_t state_store(struct device *dev, struct device_attribute *attr,
-			   const char *buf, size_t count)
-{
-	u32 state;
-	ssize_t ret = 0;
-	struct extcon_dev *edev = dev_get_drvdata(dev);
-
-	ret = sscanf(buf, "0x%x", &state);
-	if (ret == 0)
-		ret = -EINVAL;
-	else
-		ret = extcon_set_state(edev, state);
-
-	if (ret < 0)
-		return ret;
-
-	return count;
-}
-static DEVICE_ATTR_RW(state);
+static DEVICE_ATTR_RO(state);
 
 static ssize_t name_show(struct device *dev, struct device_attribute *attr,
 		char *buf)
-- 
1.9.1

[PATCH 4/4] extcon: Block the bit masking operation for cable state except for extcon core

[Chanwoo Choi]

This patch restrict the usage of extcon_update_state() in the extcon
core because the extcon_update_state() use the bit masking to change
the state of external connector. When this function is used in device drivers,
it may occur the probelm with the handling mistake of bit masking.

Also, this patch removes the extcon_get/set_state() functions because these
functions use the bit masking which is reluctant way. Instead, extcon
provides the extcon_set/get_cable_state_() functions.

Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/extcon/extcon.c | 20 +-------------------
 include/linux/extcon.h  | 30 ------------------------------
 2 files changed, 1 insertion(+), 49 deletions(-)

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index ad5e4546f82c..5b61000cde26 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -224,7 +224,7 @@ static ssize_t cable_state_show(struct device *dev,
  * Note that the notifier provides which bits are changed in the state
  * variable with the val parameter (second) to the callback.
  */
-int extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state)
+static int extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state)
 {
 	char name_buf[120];
 	char state_buf[120];
@@ -300,24 +300,6 @@ int extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state)
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(extcon_update_state);
-
-/**
- * extcon_set_state() - Set the cable attach states of the extcon device.
- * @edev:	the extcon device
- * @state:	new cable attach status for @edev
- *
- * Note that notifier provides which bits are changed in the state
- * variable with the val parameter (second) to the callback.
- */
-int extcon_set_state(struct extcon_dev *edev, u32 state)
-{
-	if (!edev)
-		return -EINVAL;
-
-	return extcon_update_state(edev, 0xffffffff, state);
-}
-EXPORT_SYMBOL_GPL(extcon_set_state);
 
 /**
  * extcon_get_cable_state_() - Get the status of a specific cable.
diff --git a/include/linux/extcon.h b/include/linux/extcon.h
index 61004413dc64..667b1d35af12 100644
--- a/include/linux/extcon.h
+++ b/include/linux/extcon.h
@@ -150,20 +150,6 @@ extern struct extcon_dev *devm_extcon_dev_allocate(struct device *dev,
 extern void devm_extcon_dev_free(struct device *dev, struct extcon_dev *edev);
 
 /*
- * get/set/update_state access the 32b encoded state value, which represents
- * states of all possible cables of the multistate port. For example, if one
- * calls extcon_set_state(edev, 0x7), it may mean that all the three cables
- * are attached to the port.
- */
-static inline u32 extcon_get_state(struct extcon_dev *edev)
-{
-	return edev->state;
-}
-
-extern int extcon_set_state(struct extcon_dev *edev, u32 state);
-extern int extcon_update_state(struct extcon_dev *edev, u32 mask, u32 state);
-
-/*
  * get/set_cable_state access each bit of the 32b encoded state value.
  * They are used to access the status of each cable based on the cable id.
  */
@@ -232,22 +218,6 @@ static inline struct extcon_dev *devm_extcon_dev_allocate(struct device *dev,
 
 static inline void devm_extcon_dev_free(struct extcon_dev *edev) { }
 
-static inline u32 extcon_get_state(struct extcon_dev *edev)
-{
-	return 0;
-}
-
-static inline int extcon_set_state(struct extcon_dev *edev, u32 state)
-{
-	return 0;
-}
-
-static inline int extcon_update_state(struct extcon_dev *edev, u32 mask,
-				       u32 state)
-{
-	return 0;
-}
-
 static inline int extcon_get_cable_state_(struct extcon_dev *edev,
 					  unsigned int id)
 {
-- 
1.9.1

Re: [PATCH 1/4] extcon: arizona: Remove the usage of extcon_update_state()

[Charles Keepax]

On Wed, Jul 20, 2016 at 04:38:44PM +0900, Chanwoo Choi wrote:
> This patch remvoes the usage of extcon_update_state() because
> the extcon_update_state() use directly the bit masking calculation
> to change the state of external connector without the unique id of
> external connector. It makes the code diffcult to read it.
> So, this patch uses the extcon_set_cable_state_() instead.
> 
> Cc: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> Cc: patches@opensource.wolfsonmicro.com
> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---

Acked-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

Thanks,
Charles