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

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

[Charles Keepax]

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

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/extcon/extcon-arizona.c       |   21 ++++++++++++++++-----
 include/linux/mfd/arizona/registers.h |    9 +++++++++
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index ec9a14e..92eb486 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -44,6 +44,17 @@
 #define HPDET_DEBOUNCE 500
 #define DEFAULT_MICD_TIMEOUT 2000
 
+enum {
+	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,
+
+	MICD_LVL_0_TO_7 = ARIZONA_MICD_LVL_0 | MICD_LVL_1_TO_7,
+
+	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 +778,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 +795,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 +809,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 +838,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 +862,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 2/2] 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 |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index 92eb486..3ce7b49 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -572,6 +572,14 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
 		goto out;
 	} else if (!ret) {
 		dev_dbg(arizona->dev, "Ignoring HPDET for removed cable\n");
+
+		/* Reset back to starting range */
+		regmap_update_bits(arizona->regmap,
+				   ARIZONA_HEADPHONE_DETECT_1,
+				   ARIZONA_HP_IMPEDANCE_RANGE_MASK |
+				   ARIZONA_HP_POLL,
+				   0);
+
 		goto done;
 	}
 
@@ -607,9 +615,9 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
 		dev_err(arizona->dev, "Failed to report HP/line: %d\n",
 			ret);
 
+done:
 	arizona_extcon_do_magic(info, 0);
 
-done:
 	if (id_gpio)
 		gpio_set_value_cansleep(id_gpio, 0);
 
-- 
1.7.2.5

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

[Lee Jones]

On Sat, 26 Oct 2013, Charles Keepax wrote:

> Improve readability by creating a define for each microphone detection
> level.
> 
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/extcon/extcon-arizona.c       |   21 ++++++++++++++++-----
>  include/linux/mfd/arizona/registers.h |    9 +++++++++
>  2 files changed, 25 insertions(+), 5 deletions(-)

I haven't checked the actual 'values' of the new defines, but I assume
you know what you're doing. I like the basic premise of the patch so
I've applied it.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

[Lee Jones]

On Sat, 26 Oct 2013, Charles Keepax wrote:

> 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 |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
> index 92eb486..3ce7b49 100644
> --- a/drivers/extcon/extcon-arizona.c
> +++ b/drivers/extcon/extcon-arizona.c
> @@ -572,6 +572,14 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>  		goto out;
>  	} else if (!ret) {
>  		dev_dbg(arizona->dev, "Ignoring HPDET for removed cable\n");
> +
> +		/* Reset back to starting range */
> +		regmap_update_bits(arizona->regmap,
> +				   ARIZONA_HEADPHONE_DETECT_1,
> +				   ARIZONA_HP_IMPEDANCE_RANGE_MASK |
> +				   ARIZONA_HP_POLL,
> +				   0);
> +
>  		goto done;
>  	}
>  
> @@ -607,9 +615,9 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>  		dev_err(arizona->dev, "Failed to report HP/line: %d\n",
>  			ret);
>  
> +done:
>  	arizona_extcon_do_magic(info, 0);
>  
> -done:
>  	if (id_gpio)
>  		gpio_set_value_cansleep(id_gpio, 0);
>  

What happens if we don't update the bits and we still do magic in the
new error path? Does that adversely affect any later 'goto done's?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

[Charles Keepax]

On Sat, Oct 26, 2013 at 09:29:07AM -0700, Lee Jones wrote:
> What happens if we don't update the bits and we still do magic in the
> new error path? Does that adversely affect any later 'goto done's?

I think best leave this patch for now I should probably rethink
it slightly, as you say I am not sure all the paths through are
correct there.

I will post a new version shortly.

Thanks,
Charles