[PATCH v3 1/7] extcon: arizona: Add defines for microphone detection levels

[PATCH v3 1/7] extcon: arizona: Add defines for microphone detection levels

[Charles Keepax]

Improve readability by creating a define for each microphone detection
level.

Acked-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/extcon/extcon-arizona.c       |   19 ++++++++++++++-----
 include/linux/mfd/arizona/registers.h |    9 +++++++++
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index ec9a14e..d611420 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -44,6 +44,15 @@
 #define HPDET_DEBOUNCE 500
 #define DEFAULT_MICD_TIMEOUT 2000
 
+#define MICD_LVL_1_TO_7 (ARIZONA_MICD_LVL_1 | ARIZONA_MICD_LVL_2 | \
+			 ARIZONA_MICD_LVL_3 | ARIZONA_MICD_LVL_4 | \
+			 ARIZONA_MICD_LVL_5 | ARIZONA_MICD_LVL_6 | \
+			 ARIZONA_MICD_LVL_7)
+
+#define MICD_LVL_0_TO_7 (ARIZONA_MICD_LVL_0 | MICD_LVL_1_TO_7)
+
+#define MICD_LVL_0_TO_8 (MICD_LVL_0_TO_7 | ARIZONA_MICD_LVL_8)
+
 struct arizona_extcon_info {
 	struct device *dev;
 	struct arizona *arizona;
@@ -767,7 +776,7 @@ static void arizona_micd_detect(struct work_struct *work)
 
 	mutex_lock(&info->lock);
 
-	for (i = 0; i < 10 && !(val & 0x7fc); i++) {
+	for (i = 0; i < 10 && !(val & MICD_LVL_0_TO_8); i++) {
 		ret = regmap_read(arizona->regmap, ARIZONA_MIC_DETECT_3, &val);
 		if (ret != 0) {
 			dev_err(arizona->dev, "Failed to read MICDET: %d\n", ret);
@@ -784,7 +793,7 @@ static void arizona_micd_detect(struct work_struct *work)
 		}
 	}
 
-	if (i == 10 && !(val & 0x7fc)) {
+	if (i == 10 && !(val & MICD_LVL_0_TO_8)) {
 		dev_err(arizona->dev, "Failed to get valid MICDET value\n");
 		mutex_unlock(&info->lock);
 		return;
@@ -798,7 +807,7 @@ static void arizona_micd_detect(struct work_struct *work)
 	}
 
 	/* If we got a high impedence we should have a headset, report it. */
-	if (info->detecting && (val & 0x400)) {
+	if (info->detecting && (val & ARIZONA_MICD_LVL_8)) {
 		arizona_identify_headphone(info);
 
 		ret = extcon_update_state(&info->edev,
@@ -827,7 +836,7 @@ static void arizona_micd_detect(struct work_struct *work)
 	 * plain headphones.  If both polarities report a low
 	 * impedence then give up and report headphones.
 	 */
-	if (info->detecting && (val & 0x3f8)) {
+	if (info->detecting && (val & MICD_LVL_1_TO_7)) {
 		if (info->jack_flips >= info->micd_num_modes * 10) {
 			dev_dbg(arizona->dev, "Detected HP/line\n");
 			arizona_identify_headphone(info);
@@ -851,7 +860,7 @@ static void arizona_micd_detect(struct work_struct *work)
 	 * If we're still detecting and we detect a short then we've
 	 * got a headphone.  Otherwise it's a button press.
 	 */
-	if (val & 0x3fc) {
+	if (val & MICD_LVL_0_TO_7) {
 		if (info->mic) {
 			dev_dbg(arizona->dev, "Mic button detected\n");
 
diff --git a/include/linux/mfd/arizona/registers.h b/include/linux/mfd/arizona/registers.h
index 4706d3d..10d9e70 100644
--- a/include/linux/mfd/arizona/registers.h
+++ b/include/linux/mfd/arizona/registers.h
@@ -2196,6 +2196,15 @@
 /*
  * R677 (0x2A5) - Mic Detect 3
  */
+#define ARIZONA_MICD_LVL_0                       0x0004  /* MICD_LVL - [2] */
+#define ARIZONA_MICD_LVL_1                       0x0008  /* MICD_LVL - [3] */
+#define ARIZONA_MICD_LVL_2                       0x0010  /* MICD_LVL - [4] */
+#define ARIZONA_MICD_LVL_3                       0x0020  /* MICD_LVL - [5] */
+#define ARIZONA_MICD_LVL_4                       0x0040  /* MICD_LVL - [6] */
+#define ARIZONA_MICD_LVL_5                       0x0080  /* MICD_LVL - [7] */
+#define ARIZONA_MICD_LVL_6                       0x0100  /* MICD_LVL - [8] */
+#define ARIZONA_MICD_LVL_7                       0x0200  /* MICD_LVL - [9] */
+#define ARIZONA_MICD_LVL_8                       0x0400  /* MICD_LVL - [10] */
 #define ARIZONA_MICD_LVL_MASK                    0x07FC  /* MICD_LVL - [10:2] */
 #define ARIZONA_MICD_LVL_SHIFT                        2  /* MICD_LVL - [10:2] */
 #define ARIZONA_MICD_LVL_WIDTH                        9  /* MICD_LVL - [10:2] */
-- 
1.7.2.5

[PATCH v3 2/7] extcon: arizona: Fix reset of HPDET after race with removal

[Charles Keepax]

We need to make sure we reset back to our starting state, especially
making sure that we have disabled poll in the register cache.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/extcon/extcon-arizona.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index d611420..61f9d6e 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -605,9 +605,15 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
 		dev_err(arizona->dev, "Failed to report HP/line: %d\n",
 			ret);
 
+done:
+	/* Reset back to starting range */
+	regmap_update_bits(arizona->regmap,
+			   ARIZONA_HEADPHONE_DETECT_1,
+			   ARIZONA_HP_IMPEDANCE_RANGE_MASK | ARIZONA_HP_POLL,
+			   0);
+
 	arizona_extcon_do_magic(info, 0);
 
-done:
 	if (id_gpio)
 		gpio_set_value_cansleep(id_gpio, 0);
 
-- 
1.7.2.5

[PATCH v3 3/7] extcon: arizona: Fix race with microphone detection and removal

[Charles Keepax]

The microphone detection code is run as delayed work to provide
additional debounce, it is possible that the jack could have been
removed by the time we process the microphone detection. Turn this
case into a no op.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/extcon/extcon-arizona.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index 61f9d6e..bc60e8c 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -782,6 +782,19 @@ static void arizona_micd_detect(struct work_struct *work)
 
 	mutex_lock(&info->lock);
 
+	/* If the cable was removed while measuring ignore the result */
+	ret = extcon_get_cable_state_(&info->edev, ARIZONA_CABLE_MECHANICAL);
+	if (ret < 0) {
+		dev_err(arizona->dev, "Failed to check cable state: %d\n",
+				ret);
+		mutex_unlock(&info->lock);
+		return;
+	} else if (!ret) {
+		dev_dbg(arizona->dev, "Ignoring MICDET for removed cable\n");
+		mutex_unlock(&info->lock);
+		return;
+	}
+
 	for (i = 0; i < 10 && !(val & MICD_LVL_0_TO_8); i++) {
 		ret = regmap_read(arizona->regmap, ARIZONA_MIC_DETECT_3, &val);
 		if (ret != 0) {
-- 
1.7.2.5

[PATCH v3 4/7] extcon: arizona: No need to switch back down HPDET ranges

[Charles Keepax]

No point in revisiting ranges the detection will be no more accurate
the second time simply report that the resistance is right on the
range boundry.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/extcon/extcon-arizona.c |   21 +++++++--------------
 1 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index bc60e8c..1a48b10 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -441,20 +441,7 @@ static int arizona_hpdet_read(struct arizona_extcon_info *info)
 		range = (range & ARIZONA_HP_IMPEDANCE_RANGE_MASK)
 			   >> ARIZONA_HP_IMPEDANCE_RANGE_SHIFT;
 
-		/* Skip up or down a range? */
-		if (range && (val < arizona_hpdet_c_ranges[range].min)) {
-			range--;
-			dev_dbg(arizona->dev, "Moving to HPDET range %d-%d\n",
-				arizona_hpdet_c_ranges[range].min,
-				arizona_hpdet_c_ranges[range].max);
-			regmap_update_bits(arizona->regmap,
-					   ARIZONA_HEADPHONE_DETECT_1,
-					   ARIZONA_HP_IMPEDANCE_RANGE_MASK,
-					   range <<
-					   ARIZONA_HP_IMPEDANCE_RANGE_SHIFT);
-			return -EAGAIN;
-		}
-
+		/* Skip up a range, or report? */
 		if (range < ARRAY_SIZE(arizona_hpdet_c_ranges) - 1 &&
 		    (val >= arizona_hpdet_c_ranges[range].max)) {
 			range++;
@@ -468,6 +455,12 @@ static int arizona_hpdet_read(struct arizona_extcon_info *info)
 					   ARIZONA_HP_IMPEDANCE_RANGE_SHIFT);
 			return -EAGAIN;
 		}
+
+		if (range && (val < arizona_hpdet_c_ranges[range].min)) {
+			dev_dbg(arizona->dev, "Reporting range boundary %d\n",
+				arizona_hpdet_c_ranges[range].min);
+			val = arizona_hpdet_c_ranges[range].min;
+		}
 	}
 
 	dev_dbg(arizona->dev, "HP impedance %d ohms\n", val);
-- 
1.7.2.5

[PATCH v3 5/7] extcon: arizona: Add support for headphone detection on wm5110 rev D

[Charles Keepax]

wm5110 rev D is the first chip to use headphone detection IP 2, specify
such and make a small correction as the impedance value is actually read
in 0.5 ohm increments now.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/extcon/extcon-arizona.c |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index 1a48b10..f4fe2d1 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -435,6 +435,8 @@ static int arizona_hpdet_read(struct arizona_extcon_info *info)
 		}
 
 		val &= ARIZONA_HP_LVL_B_MASK;
+		/* Convert to ohms, the value is in 0.5 ohm increments */
+		val /= 2;
 
 		regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_1,
 			    &range);
@@ -1146,6 +1148,16 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 			break;
 		}
 		break;
+	case WM5110:
+		switch (arizona->rev) {
+		case 0 ... 2:
+			break;
+		default:
+			info->micd_clamp = true;
+			info->hpdet_ip = 2;
+			break;
+		}
+		break;
 	default:
 		break;
 	}
-- 
1.7.2.5

[PATCH v3 6/7] extcon: arizona: Factor out different headphone detection IPs

[Charles Keepax]

This patch factors out each headphone detection IP in a seperate
function this makes the code a little more readable and prevents us
getting to excessive levels of indentation.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/extcon/extcon-arizona.c |  215 +++++++++++++++++++++++---------------
 1 files changed, 130 insertions(+), 85 deletions(-)

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index f4fe2d1..d71a738 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -354,7 +354,31 @@ static struct {
 	{ 1000, 10000 },
 };
 
-static int arizona_hpdet_read(struct arizona_extcon_info *info)
+static int arizona_hpdet_read_ip0(struct arizona_extcon_info *info)
+{
+	struct arizona *arizona = info->arizona;
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_2, &val);
+	if (ret != 0) {
+		dev_err(arizona->dev, "Failed to read HPDET status: %d\n",
+			ret);
+		return ret;
+	}
+
+
+	if (!(val & ARIZONA_HP_DONE)) {
+		dev_err(arizona->dev, "HPDET did not complete: %x\n", val);
+		return -EAGAIN;
+	}
+
+	val &= ARIZONA_HP_LVL_MASK;
+
+	return val;
+}
+
+static int arizona_hpdet_read_ip1(struct arizona_extcon_info *info)
 {
 	struct arizona *arizona = info->arizona;
 	unsigned int val, range;
@@ -367,106 +391,127 @@ static int arizona_hpdet_read(struct arizona_extcon_info *info)
 		return ret;
 	}
 
-	switch (info->hpdet_ip) {
-	case 0:
-		if (!(val & ARIZONA_HP_DONE)) {
-			dev_err(arizona->dev, "HPDET did not complete: %x\n",
-				val);
-			return -EAGAIN;
-		}
+	if (!(val & ARIZONA_HP_DONE_B)) {
+		dev_err(arizona->dev, "HPDET did not complete: %x\n", val);
+		return -EAGAIN;
+	}
 
-		val &= ARIZONA_HP_LVL_MASK;
-		break;
+	ret = regmap_read(arizona->regmap, ARIZONA_HP_DACVAL, &val);
+	if (ret != 0) {
+		dev_err(arizona->dev, "Failed to read HP value: %d\n", ret);
+		return -EAGAIN;
+	}
 
-	case 1:
-		if (!(val & ARIZONA_HP_DONE_B)) {
-			dev_err(arizona->dev, "HPDET did not complete: %x\n",
-				val);
-			return -EAGAIN;
-		}
+	regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_1, &range);
+	range = (range & ARIZONA_HP_IMPEDANCE_RANGE_MASK)
+		   >> ARIZONA_HP_IMPEDANCE_RANGE_SHIFT;
 
-		ret = regmap_read(arizona->regmap, ARIZONA_HP_DACVAL, &val);
-		if (ret != 0) {
-			dev_err(arizona->dev, "Failed to read HP value: %d\n",
-				ret);
-			return -EAGAIN;
-		}
+	if (range < ARRAY_SIZE(arizona_hpdet_b_ranges) - 1 &&
+	    (val < 100 || val >= 0x3fb)) {
+		range++;
+		dev_dbg(arizona->dev, "Moving to HPDET range %d\n", range);
+		regmap_update_bits(arizona->regmap,
+				   ARIZONA_HEADPHONE_DETECT_1,
+				   ARIZONA_HP_IMPEDANCE_RANGE_MASK,
+				   range <<
+				   ARIZONA_HP_IMPEDANCE_RANGE_SHIFT);
+		return -EAGAIN;
+	}
 
-		regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_1,
-			    &range);
-		range = (range & ARIZONA_HP_IMPEDANCE_RANGE_MASK)
-			   >> ARIZONA_HP_IMPEDANCE_RANGE_SHIFT;
+	/* If we go out of range report top of range */
+	if (val < 100 || val >= 0x3fb) {
+		dev_dbg(arizona->dev, "Measurement out of range\n");
+		return ARIZONA_HPDET_MAX;
+	}
 
-		if (range < ARRAY_SIZE(arizona_hpdet_b_ranges) - 1 &&
-		    (val < 100 || val >= 0x3fb)) {
-			range++;
-			dev_dbg(arizona->dev, "Moving to HPDET range %d\n",
-				range);
-			regmap_update_bits(arizona->regmap,
-					   ARIZONA_HEADPHONE_DETECT_1,
-					   ARIZONA_HP_IMPEDANCE_RANGE_MASK,
-					   range <<
-					   ARIZONA_HP_IMPEDANCE_RANGE_SHIFT);
-			return -EAGAIN;
-		}
+	dev_dbg(arizona->dev, "HPDET read %d in range %d\n", val, range);
 
-		/* If we go out of range report top of range */
-		if (val < 100 || val >= 0x3fb) {
-			dev_dbg(arizona->dev, "Measurement out of range\n");
-			return ARIZONA_HPDET_MAX;
-		}
+	val = arizona_hpdet_b_ranges[range].factor_b
+		/ ((val * 100) - arizona_hpdet_b_ranges[range].factor_a);
+
+	return val;
+}
+
+static int arizona_hpdet_read_ip2(struct arizona_extcon_info *info)
+{
+	struct arizona *arizona = info->arizona;
+	unsigned int val, range;
+	int ret;
+
+	ret = regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_2, &val);
+	if (ret != 0) {
+		dev_err(arizona->dev, "Failed to read HPDET status: %d\n",
+			ret);
+		return ret;
+	}
+
+	if (!(val & ARIZONA_HP_DONE_B)) {
+		dev_err(arizona->dev, "HPDET did not complete: %x\n", val);
+		return -EAGAIN;
+	}
+
+	val &= ARIZONA_HP_LVL_B_MASK;
+	/* Convert to ohms, the value is in 0.5 ohm increments */
+	val /= 2;
+
+	regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_1, &range);
+	range = (range & ARIZONA_HP_IMPEDANCE_RANGE_MASK)
+		   >> ARIZONA_HP_IMPEDANCE_RANGE_SHIFT;
+
+	/* Skip up a range, or report? */
+	if (range < ARRAY_SIZE(arizona_hpdet_c_ranges) - 1 &&
+	    (val >= arizona_hpdet_c_ranges[range].max)) {
+		range++;
+		dev_dbg(arizona->dev, "Moving to HPDET range %d-%d\n",
+			arizona_hpdet_c_ranges[range].min,
+			arizona_hpdet_c_ranges[range].max);
+		regmap_update_bits(arizona->regmap,
+				   ARIZONA_HEADPHONE_DETECT_1,
+				   ARIZONA_HP_IMPEDANCE_RANGE_MASK,
+				   range <<
+				   ARIZONA_HP_IMPEDANCE_RANGE_SHIFT);
+		return -EAGAIN;
+	}
+
+	if (range && (val < arizona_hpdet_c_ranges[range].min)) {
+		dev_dbg(arizona->dev, "Reporting range boundary %d\n",
+			arizona_hpdet_c_ranges[range].min);
+		val = arizona_hpdet_c_ranges[range].min;
+	}
+
+	return val;
+}
 
-		dev_dbg(arizona->dev, "HPDET read %d in range %d\n",
-			val, range);
+static int arizona_hpdet_read(struct arizona_extcon_info *info)
+{
+	struct arizona *arizona = info->arizona;
+	int ret = 0;
 
-		val = arizona_hpdet_b_ranges[range].factor_b
-			/ ((val * 100) -
-			   arizona_hpdet_b_ranges[range].factor_a);
+	switch (info->hpdet_ip) {
+	case 0:
+		ret = arizona_hpdet_read_ip0(info);
+		if (ret < 0)
+			return ret;
+		break;
+
+	case 1:
+		ret = arizona_hpdet_read_ip1(info);
+		if (ret < 0)
+			return ret;
 		break;
 
 	default:
 		dev_warn(arizona->dev, "Unknown HPDET IP revision %d\n",
 			 info->hpdet_ip);
 	case 2:
-		if (!(val & ARIZONA_HP_DONE_B)) {
-			dev_err(arizona->dev, "HPDET did not complete: %x\n",
-				val);
-			return -EAGAIN;
-		}
-
-		val &= ARIZONA_HP_LVL_B_MASK;
-		/* Convert to ohms, the value is in 0.5 ohm increments */
-		val /= 2;
-
-		regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_1,
-			    &range);
-		range = (range & ARIZONA_HP_IMPEDANCE_RANGE_MASK)
-			   >> ARIZONA_HP_IMPEDANCE_RANGE_SHIFT;
-
-		/* Skip up a range, or report? */
-		if (range < ARRAY_SIZE(arizona_hpdet_c_ranges) - 1 &&
-		    (val >= arizona_hpdet_c_ranges[range].max)) {
-			range++;
-			dev_dbg(arizona->dev, "Moving to HPDET range %d-%d\n",
-				arizona_hpdet_c_ranges[range].min,
-				arizona_hpdet_c_ranges[range].max);
-			regmap_update_bits(arizona->regmap,
-					   ARIZONA_HEADPHONE_DETECT_1,
-					   ARIZONA_HP_IMPEDANCE_RANGE_MASK,
-					   range <<
-					   ARIZONA_HP_IMPEDANCE_RANGE_SHIFT);
-			return -EAGAIN;
-		}
-
-		if (range && (val < arizona_hpdet_c_ranges[range].min)) {
-			dev_dbg(arizona->dev, "Reporting range boundary %d\n",
-				arizona_hpdet_c_ranges[range].min);
-			val = arizona_hpdet_c_ranges[range].min;
-		}
+		ret = arizona_hpdet_read_ip2(info);
+		if (ret < 0)
+			return ret;
+		break;
 	}
 
-	dev_dbg(arizona->dev, "HP impedance %d ohms\n", val);
-	return val;
+	dev_dbg(arizona->dev, "HP impedance %d ohms\n", ret);
+	return ret;
 }
 
 static int arizona_hpdet_do_id(struct arizona_extcon_info *info, int *reading,
-- 
1.7.2.5

[PATCH v3 7/7] extcon: arizona: Improve headphone detection error handling

[Charles Keepax]

In several places the return value of regmap operations is not checked
whilst handling headphone detection. This patch adds checks for these
return values. Additionally, failing to read/write a register is a
serious enough error we should abort the headphone detection process so
correct a couple of cases where this was not the case.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/extcon/extcon-arizona.c |   82 ++++++++++++++++++++++++++++----------
 1 files changed, 60 insertions(+), 22 deletions(-)

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index d71a738..321dcc9 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -399,22 +399,32 @@ static int arizona_hpdet_read_ip1(struct arizona_extcon_info *info)
 	ret = regmap_read(arizona->regmap, ARIZONA_HP_DACVAL, &val);
 	if (ret != 0) {
 		dev_err(arizona->dev, "Failed to read HP value: %d\n", ret);
-		return -EAGAIN;
+		return ret;
 	}
 
-	regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_1, &range);
+	ret = regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_1, &range);
 	range = (range & ARIZONA_HP_IMPEDANCE_RANGE_MASK)
 		   >> ARIZONA_HP_IMPEDANCE_RANGE_SHIFT;
+	if (ret != 0) {
+		dev_err(arizona->dev, "Failed to read HP range: %d\n", ret);
+		return ret;
+	}
 
 	if (range < ARRAY_SIZE(arizona_hpdet_b_ranges) - 1 &&
 	    (val < 100 || val >= 0x3fb)) {
 		range++;
 		dev_dbg(arizona->dev, "Moving to HPDET range %d\n", range);
-		regmap_update_bits(arizona->regmap,
-				   ARIZONA_HEADPHONE_DETECT_1,
-				   ARIZONA_HP_IMPEDANCE_RANGE_MASK,
-				   range <<
-				   ARIZONA_HP_IMPEDANCE_RANGE_SHIFT);
+		ret = regmap_update_bits(arizona->regmap,
+					 ARIZONA_HEADPHONE_DETECT_1,
+					 ARIZONA_HP_IMPEDANCE_RANGE_MASK,
+					 range <<
+					 ARIZONA_HP_IMPEDANCE_RANGE_SHIFT);
+		if (ret != 0) {
+			dev_err(arizona->dev,
+				"Failed to move range (%d): %d\n",
+				range, ret);
+			return ret;
+		}
 		return -EAGAIN;
 	}
 
@@ -454,7 +464,11 @@ static int arizona_hpdet_read_ip2(struct arizona_extcon_info *info)
 	/* Convert to ohms, the value is in 0.5 ohm increments */
 	val /= 2;
 
-	regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_1, &range);
+	ret = regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_1, &range);
+	if (ret != 0) {
+		dev_err(arizona->dev, "Failed to read HP range: %d\n", ret);
+		return ret;
+	}
 	range = (range & ARIZONA_HP_IMPEDANCE_RANGE_MASK)
 		   >> ARIZONA_HP_IMPEDANCE_RANGE_SHIFT;
 
@@ -465,11 +479,17 @@ static int arizona_hpdet_read_ip2(struct arizona_extcon_info *info)
 		dev_dbg(arizona->dev, "Moving to HPDET range %d-%d\n",
 			arizona_hpdet_c_ranges[range].min,
 			arizona_hpdet_c_ranges[range].max);
-		regmap_update_bits(arizona->regmap,
+		ret = regmap_update_bits(arizona->regmap,
 				   ARIZONA_HEADPHONE_DETECT_1,
 				   ARIZONA_HP_IMPEDANCE_RANGE_MASK,
 				   range <<
 				   ARIZONA_HP_IMPEDANCE_RANGE_SHIFT);
+		if (ret != 0) {
+			dev_err(arizona->dev,
+				"Failed to move range (%d): %d\n",
+				range, ret);
+			return ret;
+		}
 		return -EAGAIN;
 	}
 
@@ -519,6 +539,7 @@ static int arizona_hpdet_do_id(struct arizona_extcon_info *info, int *reading,
 {
 	struct arizona *arizona = info->arizona;
 	int id_gpio = arizona->pdata.hpdet_id_gpio;
+	int ret;
 
 	/*
 	 * If we're using HPDET for accessory identification we need
@@ -531,18 +552,30 @@ static int arizona_hpdet_do_id(struct arizona_extcon_info *info, int *reading,
 		if (id_gpio && info->num_hpdet_res == 1) {
 			dev_dbg(arizona->dev, "Measuring mic\n");
 
-			regmap_update_bits(arizona->regmap,
-					   ARIZONA_ACCESSORY_DETECT_MODE_1,
-					   ARIZONA_ACCDET_MODE_MASK |
-					   ARIZONA_ACCDET_SRC,
-					   ARIZONA_ACCDET_MODE_HPR |
-					   info->micd_modes[0].src);
+			ret = regmap_update_bits(arizona->regmap,
+					ARIZONA_ACCESSORY_DETECT_MODE_1,
+					ARIZONA_ACCDET_MODE_MASK |
+					ARIZONA_ACCDET_SRC,
+					ARIZONA_ACCDET_MODE_HPR |
+					info->micd_modes[0].src);
+			if (ret != 0) {
+				dev_err(arizona->dev,
+					"Failed to change HPDET source: %d\n",
+					ret);
+				return ret;
+			}
 
 			gpio_set_value_cansleep(id_gpio, 1);
 
-			regmap_update_bits(arizona->regmap,
-					   ARIZONA_HEADPHONE_DETECT_1,
-					   ARIZONA_HP_POLL, ARIZONA_HP_POLL);
+			ret = regmap_update_bits(arizona->regmap,
+					ARIZONA_HEADPHONE_DETECT_1,
+					ARIZONA_HP_POLL, ARIZONA_HP_POLL);
+			if (ret != 0) {
+				dev_err(arizona->dev,
+					"Failed to re-enable HPDET: %d\n",
+					ret);
+				return ret;
+			}
 			return -EAGAIN;
 		}
 
@@ -575,10 +608,15 @@ static int arizona_hpdet_do_id(struct arizona_extcon_info *info, int *reading,
 		}
 
 		/* Make sure everything is reset back to the real polarity */
-		regmap_update_bits(arizona->regmap,
-				   ARIZONA_ACCESSORY_DETECT_MODE_1,
-				   ARIZONA_ACCDET_SRC,
-				   info->micd_modes[0].src);
+		ret = regmap_update_bits(arizona->regmap,
+					 ARIZONA_ACCESSORY_DETECT_MODE_1,
+					 ARIZONA_ACCDET_SRC,
+					 info->micd_modes[0].src);
+		if (ret != 0) {
+			dev_err(arizona->dev, "Failed to reset polarity: %d\n",
+				ret);
+			return ret;
+		}
 	}
 
 	return 0;
-- 
1.7.2.5

Re: [PATCH v3 7/7] extcon: arizona: Improve headphone detection error handling

[Chanwoo Choi]

Hi Charles,

On 11/15/2013 01:18 AM, Charles Keepax wrote:
> In several places the return value of regmap operations is not checked
> whilst handling headphone detection. This patch adds checks for these
> return values. Additionally, failing to read/write a register is a
> serious enough error we should abort the headphone detection process so
> correct a couple of cases where this was not the case.
> 
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/extcon/extcon-arizona.c |   82 ++++++++++++++++++++++++++++----------
>  1 files changed, 60 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
> index d71a738..321dcc9 100644
> --- a/drivers/extcon/extcon-arizona.c
> +++ b/drivers/extcon/extcon-arizona.c
> @@ -399,22 +399,32 @@ static int arizona_hpdet_read_ip1(struct arizona_extcon_info *info)
>  	ret = regmap_read(arizona->regmap, ARIZONA_HP_DACVAL, &val);
>  	if (ret != 0) {

I prefer following checking statement
	if (ret < 0) instead of if (ret != 0) 

>  		dev_err(arizona->dev, "Failed to read HP value: %d\n", ret);
> -		return -EAGAIN;
> +		return ret;
>  	}
>  
> -	regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_1, &range);
> +	ret = regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_1, &range);
>  	range = (range & ARIZONA_HP_IMPEDANCE_RANGE_MASK)
>  		   >> ARIZONA_HP_IMPEDANCE_RANGE_SHIFT;

I prefer to move 'range operation' statement after checking ret value.

> +	if (ret != 0) {

ditto.

> +		dev_err(arizona->dev, "Failed to read HP range: %d\n", ret);
> +		return ret;
> +	}
>  
>  	if (range < ARRAY_SIZE(arizona_hpdet_b_ranges) - 1 &&
>  	    (val < 100 || val >= 0x3fb)) {
>  		range++;
>  		dev_dbg(arizona->dev, "Moving to HPDET range %d\n", range);
> -		regmap_update_bits(arizona->regmap,
> -				   ARIZONA_HEADPHONE_DETECT_1,
> -				   ARIZONA_HP_IMPEDANCE_RANGE_MASK,
> -				   range <<
> -				   ARIZONA_HP_IMPEDANCE_RANGE_SHIFT);
> +		ret = regmap_update_bits(arizona->regmap,
> +					 ARIZONA_HEADPHONE_DETECT_1,
> +					 ARIZONA_HP_IMPEDANCE_RANGE_MASK,
> +					 range <<
> +					 ARIZONA_HP_IMPEDANCE_RANGE_SHIFT);
> +		if (ret != 0) {

ditto.

> +			dev_err(arizona->dev,
> +				"Failed to move range (%d): %d\n",
> +				range, ret);
> +			return ret;
> +		}
>  		return -EAGAIN;


If below statement is true, arizona_hpdet_read_ip1 always return error?
This if statement return either 'ret' or '-EAGAIN'. 

  	if (range < ARRAY_SIZE(arizona_hpdet_b_ranges) - 1 &&
  	    (val < 100 || val >= 0x3fb)) {


>  	}
>  
> @@ -454,7 +464,11 @@ static int arizona_hpdet_read_ip2(struct arizona_extcon_info *info)
>  	/* Convert to ohms, the value is in 0.5 ohm increments */
>  	val /= 2;
>  
> -	regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_1, &range);
> +	ret = regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_1, &range);
> +	if (ret != 0) {

ditto.

> +		dev_err(arizona->dev, "Failed to read HP range: %d\n", ret);
> +		return ret;
> +	}
>  	range = (range & ARIZONA_HP_IMPEDANCE_RANGE_MASK)
>  		   >> ARIZONA_HP_IMPEDANCE_RANGE_SHIFT;
>  
> @@ -465,11 +479,17 @@ static int arizona_hpdet_read_ip2(struct arizona_extcon_info *info)
>  		dev_dbg(arizona->dev, "Moving to HPDET range %d-%d\n",
>  			arizona_hpdet_c_ranges[range].min,
>  			arizona_hpdet_c_ranges[range].max);
> -		regmap_update_bits(arizona->regmap,
> +		ret = regmap_update_bits(arizona->regmap,
>  				   ARIZONA_HEADPHONE_DETECT_1,
>  				   ARIZONA_HP_IMPEDANCE_RANGE_MASK,
>  				   range <<
>  				   ARIZONA_HP_IMPEDANCE_RANGE_SHIFT);
> +		if (ret != 0) {

ditto.

> +			dev_err(arizona->dev,
> +				"Failed to move range (%d): %d\n",
> +				range, ret);
> +			return ret;
> +		}
>  		return -EAGAIN;

ditto.

>  	}
>  
> @@ -519,6 +539,7 @@ static int arizona_hpdet_do_id(struct arizona_extcon_info *info, int *reading,
>  {
>  	struct arizona *arizona = info->arizona;
>  	int id_gpio = arizona->pdata.hpdet_id_gpio;
> +	int ret;
>  
>  	/*
>  	 * If we're using HPDET for accessory identification we need
> @@ -531,18 +552,30 @@ static int arizona_hpdet_do_id(struct arizona_extcon_info *info, int *reading,
>  		if (id_gpio && info->num_hpdet_res == 1) {
>  			dev_dbg(arizona->dev, "Measuring mic\n");
>  
> -			regmap_update_bits(arizona->regmap,
> -					   ARIZONA_ACCESSORY_DETECT_MODE_1,
> -					   ARIZONA_ACCDET_MODE_MASK |
> -					   ARIZONA_ACCDET_SRC,
> -					   ARIZONA_ACCDET_MODE_HPR |
> -					   info->micd_modes[0].src);
> +			ret = regmap_update_bits(arizona->regmap,
> +					ARIZONA_ACCESSORY_DETECT_MODE_1,
> +					ARIZONA_ACCDET_MODE_MASK |
> +					ARIZONA_ACCDET_SRC,
> +					ARIZONA_ACCDET_MODE_HPR |
> +					info->micd_modes[0].src);
> +			if (ret != 0) {
> +				dev_err(arizona->dev,
> +					"Failed to change HPDET source: %d\n",
> +					ret);
> +				return ret;
> +			}
>  
>  			gpio_set_value_cansleep(id_gpio, 1);
>  
> -			regmap_update_bits(arizona->regmap,
> -					   ARIZONA_HEADPHONE_DETECT_1,
> -					   ARIZONA_HP_POLL, ARIZONA_HP_POLL);
> +			ret = regmap_update_bits(arizona->regmap,
> +					ARIZONA_HEADPHONE_DETECT_1,
> +					ARIZONA_HP_POLL, ARIZONA_HP_POLL);
> +			if (ret != 0) {
> +				dev_err(arizona->dev,
> +					"Failed to re-enable HPDET: %d\n",
> +					ret);
> +				return ret;
> +			}
>  			return -EAGAIN;

Always return error?

>  		}
>  
> @@ -575,10 +608,15 @@ static int arizona_hpdet_do_id(struct arizona_extcon_info *info, int *reading,
>  		}
>  
>  		/* Make sure everything is reset back to the real polarity */
> -		regmap_update_bits(arizona->regmap,
> -				   ARIZONA_ACCESSORY_DETECT_MODE_1,
> -				   ARIZONA_ACCDET_SRC,
> -				   info->micd_modes[0].src);
> +		ret = regmap_update_bits(arizona->regmap,
> +					 ARIZONA_ACCESSORY_DETECT_MODE_1,
> +					 ARIZONA_ACCDET_SRC,
> +					 info->micd_modes[0].src);
> +		if (ret != 0) {

ditto.

> +			dev_err(arizona->dev, "Failed to reset polarity: %d\n",
> +				ret);
> +			return ret;
> +		}
>  	}
>  
>  	return 0;
> 

Thanks,
Chanwoo Choi

Re: [PATCH v3 6/7] extcon: arizona: Factor out different headphone detection IPs

[Chanwoo Choi]

Hi Charles,

On 11/15/2013 01:18 AM, Charles Keepax wrote:
> This patch factors out each headphone detection IP in a seperate
> function this makes the code a little more readable and prevents us
> getting to excessive levels of indentation.
> 
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/extcon/extcon-arizona.c |  215 +++++++++++++++++++++++---------------
>  1 files changed, 130 insertions(+), 85 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
> index f4fe2d1..d71a738 100644
> --- a/drivers/extcon/extcon-arizona.c
> +++ b/drivers/extcon/extcon-arizona.c
> @@ -354,7 +354,31 @@ static struct {
>  	{ 1000, 10000 },
>  };
>  
> -static int arizona_hpdet_read(struct arizona_extcon_info *info)
> +static int arizona_hpdet_read_ip0(struct arizona_extcon_info *info)
> +{
> +	struct arizona *arizona = info->arizona;
> +	unsigned int val;
> +	int ret;
> +
> +	ret = regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_2, &val);
> +	if (ret != 0) {

I prefer following checking statement
	if (ret < 0) instead of if (ret != 0) 

> +		dev_err(arizona->dev, "Failed to read HPDET status: %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +
> +	if (!(val & ARIZONA_HP_DONE)) {
> +		dev_err(arizona->dev, "HPDET did not complete: %x\n", val);
> +		return -EAGAIN;
> +	}
> +
> +	val &= ARIZONA_HP_LVL_MASK;
> +
> +	return val;
> +}
> +
> +static int arizona_hpdet_read_ip1(struct arizona_extcon_info *info)
>  {
>  	struct arizona *arizona = info->arizona;
>  	unsigned int val, range;
> @@ -367,106 +391,127 @@ static int arizona_hpdet_read(struct arizona_extcon_info *info)
>  		return ret;
>  	}
>  
> -	switch (info->hpdet_ip) {
> -	case 0:
> -		if (!(val & ARIZONA_HP_DONE)) {
> -			dev_err(arizona->dev, "HPDET did not complete: %x\n",
> -				val);
> -			return -EAGAIN;
> -		}
> +	if (!(val & ARIZONA_HP_DONE_B)) {
> +		dev_err(arizona->dev, "HPDET did not complete: %x\n", val);
> +		return -EAGAIN;
> +	}
>  
> -		val &= ARIZONA_HP_LVL_MASK;
> -		break;
> +	ret = regmap_read(arizona->regmap, ARIZONA_HP_DACVAL, &val);
> +	if (ret != 0) {

ditto.

> +		dev_err(arizona->dev, "Failed to read HP value: %d\n", ret);
> +		return -EAGAIN;
> +	}
>  
> -	case 1:
> -		if (!(val & ARIZONA_HP_DONE_B)) {
> -			dev_err(arizona->dev, "HPDET did not complete: %x\n",
> -				val);
> -			return -EAGAIN;
> -		}
> +	regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_1, &range);
> +	range = (range & ARIZONA_HP_IMPEDANCE_RANGE_MASK)
> +		   >> ARIZONA_HP_IMPEDANCE_RANGE_SHIFT;
>  
> -		ret = regmap_read(arizona->regmap, ARIZONA_HP_DACVAL, &val);
> -		if (ret != 0) {
> -			dev_err(arizona->dev, "Failed to read HP value: %d\n",
> -				ret);
> -			return -EAGAIN;
> -		}
> +	if (range < ARRAY_SIZE(arizona_hpdet_b_ranges) - 1 &&
> +	    (val < 100 || val >= 0x3fb)) {

If you possible, I'd like you to define '100' and '0x3fb' as defined constant with 'define' keyword
to improve readability.

> +		range++;
> +		dev_dbg(arizona->dev, "Moving to HPDET range %d\n", range);
> +		regmap_update_bits(arizona->regmap,
> +				   ARIZONA_HEADPHONE_DETECT_1,
> +				   ARIZONA_HP_IMPEDANCE_RANGE_MASK,
> +				   range <<
> +				   ARIZONA_HP_IMPEDANCE_RANGE_SHIFT);
> +		return -EAGAIN;
> +	}
>  
> -		regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_1,
> -			    &range);
> -		range = (range & ARIZONA_HP_IMPEDANCE_RANGE_MASK)
> -			   >> ARIZONA_HP_IMPEDANCE_RANGE_SHIFT;
> +	/* If we go out of range report top of range */
> +	if (val < 100 || val >= 0x3fb) {

ditto.

> +		dev_dbg(arizona->dev, "Measurement out of range\n");
> +		return ARIZONA_HPDET_MAX;
> +	}
>  
> -		if (range < ARRAY_SIZE(arizona_hpdet_b_ranges) - 1 &&
> -		    (val < 100 || val >= 0x3fb)) {
> -			range++;
> -			dev_dbg(arizona->dev, "Moving to HPDET range %d\n",
> -				range);
> -			regmap_update_bits(arizona->regmap,
> -					   ARIZONA_HEADPHONE_DETECT_1,
> -					   ARIZONA_HP_IMPEDANCE_RANGE_MASK,
> -					   range <<
> -					   ARIZONA_HP_IMPEDANCE_RANGE_SHIFT);
> -			return -EAGAIN;
> -		}
> +	dev_dbg(arizona->dev, "HPDET read %d in range %d\n", val, range);
>  
> -		/* If we go out of range report top of range */
> -		if (val < 100 || val >= 0x3fb) {
> -			dev_dbg(arizona->dev, "Measurement out of range\n");
> -			return ARIZONA_HPDET_MAX;
> -		}
> +	val = arizona_hpdet_b_ranges[range].factor_b
> +		/ ((val * 100) - arizona_hpdet_b_ranges[range].factor_a);

ditto.

What is meaning of 100? I prefer to define constant variable.

> +
> +	return val;
> +}
> +
> +static int arizona_hpdet_read_ip2(struct arizona_extcon_info *info)
> +{
> +	struct arizona *arizona = info->arizona;
> +	unsigned int val, range;
> +	int ret;
> +
> +	ret = regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_2, &val);
> +	if (ret != 0) {

ditto.

> +		dev_err(arizona->dev, "Failed to read HPDET status: %d\n",
> +			ret);
> +		return ret;
> +	}
> +
> +	if (!(val & ARIZONA_HP_DONE_B)) {
> +		dev_err(arizona->dev, "HPDET did not complete: %x\n", val);
> +		return -EAGAIN;
> +	}
> +
> +	val &= ARIZONA_HP_LVL_B_MASK;
> +	/* Convert to ohms, the value is in 0.5 ohm increments */
> +	val /= 2;
> +
> +	regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_1, &range);
> +	range = (range & ARIZONA_HP_IMPEDANCE_RANGE_MASK)
> +		   >> ARIZONA_HP_IMPEDANCE_RANGE_SHIFT;
> +
> +	/* Skip up a range, or report? */
> +	if (range < ARRAY_SIZE(arizona_hpdet_c_ranges) - 1 &&
> +	    (val >= arizona_hpdet_c_ranges[range].max)) {
> +		range++;
> +		dev_dbg(arizona->dev, "Moving to HPDET range %d-%d\n",
> +			arizona_hpdet_c_ranges[range].min,
> +			arizona_hpdet_c_ranges[range].max);
> +		regmap_update_bits(arizona->regmap,
> +				   ARIZONA_HEADPHONE_DETECT_1,
> +				   ARIZONA_HP_IMPEDANCE_RANGE_MASK,
> +				   range <<
> +				   ARIZONA_HP_IMPEDANCE_RANGE_SHIFT);
> +		return -EAGAIN;
> +	}
> +
> +	if (range && (val < arizona_hpdet_c_ranges[range].min)) {
> +		dev_dbg(arizona->dev, "Reporting range boundary %d\n",
> +			arizona_hpdet_c_ranges[range].min);
> +		val = arizona_hpdet_c_ranges[range].min;
> +	}
> +
> +	return val;
> +}
>  
> -		dev_dbg(arizona->dev, "HPDET read %d in range %d\n",
> -			val, range);
> +static int arizona_hpdet_read(struct arizona_extcon_info *info)
> +{
> +	struct arizona *arizona = info->arizona;
> +	int ret = 0;
>  
> -		val = arizona_hpdet_b_ranges[range].factor_b
> -			/ ((val * 100) -
> -			   arizona_hpdet_b_ranges[range].factor_a);
> +	switch (info->hpdet_ip) {
> +	case 0:
> +		ret = arizona_hpdet_read_ip0(info);
> +		if (ret < 0)
> +			return ret;
> +		break;
> +
> +	case 1:
> +		ret = arizona_hpdet_read_ip1(info);
> +		if (ret < 0)
> +			return ret;
>  		break;
>  
>  	default:
>  		dev_warn(arizona->dev, "Unknown HPDET IP revision %d\n",
>  			 info->hpdet_ip);
>  	case 2:
> -		if (!(val & ARIZONA_HP_DONE_B)) {
> -			dev_err(arizona->dev, "HPDET did not complete: %x\n",
> -				val);
> -			return -EAGAIN;
> -		}
> -
> -		val &= ARIZONA_HP_LVL_B_MASK;
> -		/* Convert to ohms, the value is in 0.5 ohm increments */
> -		val /= 2;
> -
> -		regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_1,
> -			    &range);
> -		range = (range & ARIZONA_HP_IMPEDANCE_RANGE_MASK)
> -			   >> ARIZONA_HP_IMPEDANCE_RANGE_SHIFT;
> -
> -		/* Skip up a range, or report? */
> -		if (range < ARRAY_SIZE(arizona_hpdet_c_ranges) - 1 &&
> -		    (val >= arizona_hpdet_c_ranges[range].max)) {
> -			range++;
> -			dev_dbg(arizona->dev, "Moving to HPDET range %d-%d\n",
> -				arizona_hpdet_c_ranges[range].min,
> -				arizona_hpdet_c_ranges[range].max);
> -			regmap_update_bits(arizona->regmap,
> -					   ARIZONA_HEADPHONE_DETECT_1,
> -					   ARIZONA_HP_IMPEDANCE_RANGE_MASK,
> -					   range <<
> -					   ARIZONA_HP_IMPEDANCE_RANGE_SHIFT);
> -			return -EAGAIN;
> -		}
> -
> -		if (range && (val < arizona_hpdet_c_ranges[range].min)) {
> -			dev_dbg(arizona->dev, "Reporting range boundary %d\n",
> -				arizona_hpdet_c_ranges[range].min);
> -			val = arizona_hpdet_c_ranges[range].min;
> -		}
> +		ret = arizona_hpdet_read_ip2(info);
> +		if (ret < 0)
> +			return ret;
> +		break;
>  	}
>  
> -	dev_dbg(arizona->dev, "HP impedance %d ohms\n", val);
> -	return val;
> +	dev_dbg(arizona->dev, "HP impedance %d ohms\n", ret);

Need blank line.

> +	return ret;
>  }
>  
>  static int arizona_hpdet_do_id(struct arizona_extcon_info *info, int *reading,
> 

Thanks,
Chanwoo Choi

Re: [PATCH v3 1/7] extcon: arizona: Add defines for microphone detection levels

[Chanwoo Choi]

Hi Charles,

On 11/15/2013 01:18 AM, Charles Keepax wrote:
> Improve readability by creating a define for each microphone detection
> level.
> 
> Acked-by: Lee Jones <lee.jones@linaro.org>
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/extcon/extcon-arizona.c       |   19 ++++++++++++++-----
>  include/linux/mfd/arizona/registers.h |    9 +++++++++
>  2 files changed, 23 insertions(+), 5 deletions(-)
> 

Applied patch[1-5] on extcon-next branch.
I reply patch[6,7] with minor comment.

And, I'd like you to send cover-letter with patchset if you send multiple patches.

Thanks
Chanwoo Choi