[PATCH 1/2] extcon: arizona: Deobfuscate arizona_extcon_do_magic

[PATCH 1/2] extcon: arizona: Deobfuscate arizona_extcon_do_magic

[Charles Keepax]

arizona_extcon_do_magic does not lend a lot of clarity to the purpose
of the function, and as all the registers used are described in the
datasheet there is no need to obfuscate the code. This patch renames the
function to arizona_extcon_hp_clamp, as it controls clamping on the
headphone output.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 drivers/extcon/extcon-arizona.c  |   36 ++++++++++++++++++++----------------
 include/linux/mfd/arizona/core.h |    2 +-
 sound/soc/codecs/arizona.c       |    4 ++--
 3 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index 63f01c4..95cf7f8 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -136,18 +136,22 @@ static const char *arizona_cable[] = {
 
 static void arizona_start_hpdet_acc_id(struct arizona_extcon_info *info);
 
-static void arizona_extcon_do_magic(struct arizona_extcon_info *info,
-				    unsigned int magic)
+static void arizona_extcon_hp_clamp(struct arizona_extcon_info *info,
+				    bool clamp)
 {
 	struct arizona *arizona = info->arizona;
+	unsigned int val = 0;
 	int ret;
 
+	if (clamp)
+		val = ARIZONA_RMV_SHRT_HP1L;
+
 	mutex_lock(&arizona->dapm->card->dapm_mutex);
 
-	arizona->hpdet_magic = magic;
+	arizona->hpdet_clamp = clamp;
 
-	/* Keep the HP output stages disabled while doing the magic */
-	if (magic) {
+	/* Keep the HP output stages disabled while doing the clamp */
+	if (clamp) {
 		ret = regmap_update_bits(arizona->regmap,
 					 ARIZONA_OUTPUT_ENABLES_1,
 					 ARIZONA_OUT1L_ENA |
@@ -158,20 +162,20 @@ static void arizona_extcon_do_magic(struct arizona_extcon_info *info,
 				 ret);
 	}
 
-	ret = regmap_update_bits(arizona->regmap, 0x225, 0x4000,
-				 magic);
+	ret = regmap_update_bits(arizona->regmap, ARIZONA_HP_CTRL_1L,
+				 ARIZONA_RMV_SHRT_HP1L, val);
 	if (ret != 0)
-		dev_warn(arizona->dev, "Failed to do magic: %d\n",
+		dev_warn(arizona->dev, "Failed to do clamp: %d\n",
 				 ret);
 
-	ret = regmap_update_bits(arizona->regmap, 0x226, 0x4000,
-				 magic);
+	ret = regmap_update_bits(arizona->regmap, ARIZONA_HP_CTRL_1R,
+				 ARIZONA_RMV_SHRT_HP1R, val);
 	if (ret != 0)
-		dev_warn(arizona->dev, "Failed to do magic: %d\n",
+		dev_warn(arizona->dev, "Failed to do clamp: %d\n",
 			 ret);
 
-	/* Restore the desired state while not doing the magic */
-	if (!magic) {
+	/* Restore the desired state while not doing the clamp */
+	if (!clamp) {
 		ret = regmap_update_bits(arizona->regmap,
 					 ARIZONA_OUTPUT_ENABLES_1,
 					 ARIZONA_OUT1L_ENA |
@@ -603,7 +607,7 @@ done:
 			   ARIZONA_HP_IMPEDANCE_RANGE_MASK | ARIZONA_HP_POLL,
 			   0);
 
-	arizona_extcon_do_magic(info, 0);
+	arizona_extcon_hp_clamp(info, false);
 
 	if (id_gpio)
 		gpio_set_value_cansleep(id_gpio, 0);
@@ -648,7 +652,7 @@ static void arizona_identify_headphone(struct arizona_extcon_info *info)
 	if (info->mic)
 		arizona_stop_mic(info);
 
-	arizona_extcon_do_magic(info, 0x4000);
+	arizona_extcon_hp_clamp(info, true);
 
 	ret = regmap_update_bits(arizona->regmap,
 				 ARIZONA_ACCESSORY_DETECT_MODE_1,
@@ -699,7 +703,7 @@ static void arizona_start_hpdet_acc_id(struct arizona_extcon_info *info)
 
 	info->hpdet_active = true;
 
-	arizona_extcon_do_magic(info, 0x4000);
+	arizona_extcon_hp_clamp(info, true);
 
 	ret = regmap_update_bits(arizona->regmap,
 				 ARIZONA_ACCESSORY_DETECT_MODE_1,
diff --git a/include/linux/mfd/arizona/core.h b/include/linux/mfd/arizona/core.h
index 910e3aa..4863548 100644
--- a/include/linux/mfd/arizona/core.h
+++ b/include/linux/mfd/arizona/core.h
@@ -126,7 +126,7 @@ struct arizona {
 	struct regmap_irq_chip_data *aod_irq_chip;
 	struct regmap_irq_chip_data *irq_chip;
 
-	bool hpdet_magic;
+	bool hpdet_clamp;
 	unsigned int hp_ena;
 
 	struct mutex clk_lock;
diff --git a/sound/soc/codecs/arizona.c b/sound/soc/codecs/arizona.c
index 9550d74..a6e5c70 100644
--- a/sound/soc/codecs/arizona.c
+++ b/sound/soc/codecs/arizona.c
@@ -780,8 +780,8 @@ int arizona_hp_ev(struct snd_soc_dapm_widget *w,
 	priv->arizona->hp_ena &= ~mask;
 	priv->arizona->hp_ena |= val;
 
-	/* Force off if HPDET magic is active */
-	if (priv->arizona->hpdet_magic)
+	/* Force off if HPDET clamp is active */
+	if (priv->arizona->hpdet_clamp)
 		val = 0;
 
 	regmap_update_bits_async(arizona->regmap, ARIZONA_OUTPUT_ENABLES_1,
-- 
1.7.2.5

[PATCH 2/2] extcon: arizona: Fix headphone clamping on wm5110

[Charles Keepax]

wm5110 requires slightly different configuration of the headphone
clamps to other Arizona devices. Otherwise headphone detection accuracy
will be way off. This patch adds the needed clamping.

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

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index 95cf7f8..d9e763c 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -140,11 +140,24 @@ static void arizona_extcon_hp_clamp(struct arizona_extcon_info *info,
 				    bool clamp)
 {
 	struct arizona *arizona = info->arizona;
-	unsigned int val = 0;
+	unsigned int mask = 0, val = 0;
 	int ret;
 
-	if (clamp)
-		val = ARIZONA_RMV_SHRT_HP1L;
+	switch (arizona->type) {
+	case WM5110:
+		mask = ARIZONA_HP1L_SHRTO | ARIZONA_HP1L_FLWR |
+		       ARIZONA_HP1L_SHRTI;
+		if (clamp)
+			val = ARIZONA_HP1L_SHRTO;
+		else
+			val = ARIZONA_HP1L_FLWR | ARIZONA_HP1L_SHRTI;
+		break;
+	default:
+		mask = ARIZONA_RMV_SHRT_HP1L;
+		if (clamp)
+			val = ARIZONA_RMV_SHRT_HP1L;
+		break;
+	};
 
 	mutex_lock(&arizona->dapm->card->dapm_mutex);
 
@@ -163,13 +176,13 @@ static void arizona_extcon_hp_clamp(struct arizona_extcon_info *info,
 	}
 
 	ret = regmap_update_bits(arizona->regmap, ARIZONA_HP_CTRL_1L,
-				 ARIZONA_RMV_SHRT_HP1L, val);
+				 mask, val);
 	if (ret != 0)
 		dev_warn(arizona->dev, "Failed to do clamp: %d\n",
 				 ret);
 
 	ret = regmap_update_bits(arizona->regmap, ARIZONA_HP_CTRL_1R,
-				 ARIZONA_RMV_SHRT_HP1R, val);
+				 mask, val);
 	if (ret != 0)
 		dev_warn(arizona->dev, "Failed to do clamp: %d\n",
 			 ret);
-- 
1.7.2.5

Re: [PATCH 1/2] extcon: arizona: Deobfuscate arizona_extcon_do_magic

[Chanwoo Choi]

Hi Charles,


On Tue, Feb 17, 2015 at 12:41 AM, Charles Keepax
<ckeepax@opensource.wolfsonmicro.com> wrote:
> arizona_extcon_do_magic does not lend a lot of clarity to the purpose
> of the function, and as all the registers used are described in the
> datasheet there is no need to obfuscate the code. This patch renames the
> function to arizona_extcon_hp_clamp, as it controls clamping on the
> headphone output.
>
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/extcon/extcon-arizona.c  |   36 ++++++++++++++++++++----------------
>  include/linux/mfd/arizona/core.h |    2 +-
>  sound/soc/codecs/arizona.c       |    4 ++--
>  3 files changed, 23 insertions(+), 19 deletions(-)


Looks good to me.
But, you need more ack message by MFD maintainer.

For extcon part:
Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

Thanks,
Chanwoo Choi

Re: [PATCH 2/2] extcon: arizona: Fix headphone clamping on wm5110

[Chanwoo Choi]

Hi Charles,

On Tue, Feb 17, 2015 at 12:41 AM, Charles Keepax
<ckeepax@opensource.wolfsonmicro.com> wrote:
> wm5110 requires slightly different configuration of the headphone
> clamps to other Arizona devices. Otherwise headphone detection accuracy
> will be way off. This patch adds the needed clamping.
>
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/extcon/extcon-arizona.c |   23 ++++++++++++++++++-----
>  1 files changed, 18 insertions(+), 5 deletions(-)

Looks good to me.

I'll apply this patch on extcon-next branch
after patch1 receives the ack message by MFD maintainer.

Thanks,
Chanwoo Choi

Re: [PATCH 1/2] extcon: arizona: Deobfuscate arizona_extcon_do_magic

[Lee Jones]

On Mon, 16 Feb 2015, Charles Keepax wrote:

> arizona_extcon_do_magic does not lend a lot of clarity to the purpose
> of the function, and as all the registers used are described in the
> datasheet there is no need to obfuscate the code. This patch renames the
> function to arizona_extcon_hp_clamp, as it controls clamping on the
> headphone output.
> 
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/extcon/extcon-arizona.c  |   36 ++++++++++++++++++++----------------
>  include/linux/mfd/arizona/core.h |    2 +-

Acked-by: Lee Jones <lee.jones@linaro.org>

>  sound/soc/codecs/arizona.c       |    4 ++--
>  3 files changed, 23 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
> index 63f01c4..95cf7f8 100644
> --- a/drivers/extcon/extcon-arizona.c
> +++ b/drivers/extcon/extcon-arizona.c
> @@ -136,18 +136,22 @@ static const char *arizona_cable[] = {
>  
>  static void arizona_start_hpdet_acc_id(struct arizona_extcon_info *info);
>  
> -static void arizona_extcon_do_magic(struct arizona_extcon_info *info,
> -				    unsigned int magic)
> +static void arizona_extcon_hp_clamp(struct arizona_extcon_info *info,
> +				    bool clamp)
>  {
>  	struct arizona *arizona = info->arizona;
> +	unsigned int val = 0;
>  	int ret;
>  
> +	if (clamp)
> +		val = ARIZONA_RMV_SHRT_HP1L;
> +
>  	mutex_lock(&arizona->dapm->card->dapm_mutex);
>  
> -	arizona->hpdet_magic = magic;
> +	arizona->hpdet_clamp = clamp;
>  
> -	/* Keep the HP output stages disabled while doing the magic */
> -	if (magic) {
> +	/* Keep the HP output stages disabled while doing the clamp */
> +	if (clamp) {
>  		ret = regmap_update_bits(arizona->regmap,
>  					 ARIZONA_OUTPUT_ENABLES_1,
>  					 ARIZONA_OUT1L_ENA |
> @@ -158,20 +162,20 @@ static void arizona_extcon_do_magic(struct arizona_extcon_info *info,
>  				 ret);
>  	}
>  
> -	ret = regmap_update_bits(arizona->regmap, 0x225, 0x4000,
> -				 magic);
> +	ret = regmap_update_bits(arizona->regmap, ARIZONA_HP_CTRL_1L,
> +				 ARIZONA_RMV_SHRT_HP1L, val);
>  	if (ret != 0)
> -		dev_warn(arizona->dev, "Failed to do magic: %d\n",
> +		dev_warn(arizona->dev, "Failed to do clamp: %d\n",
>  				 ret);
>  
> -	ret = regmap_update_bits(arizona->regmap, 0x226, 0x4000,
> -				 magic);
> +	ret = regmap_update_bits(arizona->regmap, ARIZONA_HP_CTRL_1R,
> +				 ARIZONA_RMV_SHRT_HP1R, val);
>  	if (ret != 0)
> -		dev_warn(arizona->dev, "Failed to do magic: %d\n",
> +		dev_warn(arizona->dev, "Failed to do clamp: %d\n",
>  			 ret);
>  
> -	/* Restore the desired state while not doing the magic */
> -	if (!magic) {
> +	/* Restore the desired state while not doing the clamp */
> +	if (!clamp) {
>  		ret = regmap_update_bits(arizona->regmap,
>  					 ARIZONA_OUTPUT_ENABLES_1,
>  					 ARIZONA_OUT1L_ENA |
> @@ -603,7 +607,7 @@ done:
>  			   ARIZONA_HP_IMPEDANCE_RANGE_MASK | ARIZONA_HP_POLL,
>  			   0);
>  
> -	arizona_extcon_do_magic(info, 0);
> +	arizona_extcon_hp_clamp(info, false);
>  
>  	if (id_gpio)
>  		gpio_set_value_cansleep(id_gpio, 0);
> @@ -648,7 +652,7 @@ static void arizona_identify_headphone(struct arizona_extcon_info *info)
>  	if (info->mic)
>  		arizona_stop_mic(info);
>  
> -	arizona_extcon_do_magic(info, 0x4000);
> +	arizona_extcon_hp_clamp(info, true);
>  
>  	ret = regmap_update_bits(arizona->regmap,
>  				 ARIZONA_ACCESSORY_DETECT_MODE_1,
> @@ -699,7 +703,7 @@ static void arizona_start_hpdet_acc_id(struct arizona_extcon_info *info)
>  
>  	info->hpdet_active = true;
>  
> -	arizona_extcon_do_magic(info, 0x4000);
> +	arizona_extcon_hp_clamp(info, true);
>  
>  	ret = regmap_update_bits(arizona->regmap,
>  				 ARIZONA_ACCESSORY_DETECT_MODE_1,
> diff --git a/include/linux/mfd/arizona/core.h b/include/linux/mfd/arizona/core.h
> index 910e3aa..4863548 100644
> --- a/include/linux/mfd/arizona/core.h
> +++ b/include/linux/mfd/arizona/core.h
> @@ -126,7 +126,7 @@ struct arizona {
>  	struct regmap_irq_chip_data *aod_irq_chip;
>  	struct regmap_irq_chip_data *irq_chip;
>  
> -	bool hpdet_magic;
> +	bool hpdet_clamp;
>  	unsigned int hp_ena;
>  
>  	struct mutex clk_lock;
> diff --git a/sound/soc/codecs/arizona.c b/sound/soc/codecs/arizona.c
> index 9550d74..a6e5c70 100644
> --- a/sound/soc/codecs/arizona.c
> +++ b/sound/soc/codecs/arizona.c
> @@ -780,8 +780,8 @@ int arizona_hp_ev(struct snd_soc_dapm_widget *w,
>  	priv->arizona->hp_ena &= ~mask;
>  	priv->arizona->hp_ena |= val;
>  
> -	/* Force off if HPDET magic is active */
> -	if (priv->arizona->hpdet_magic)
> +	/* Force off if HPDET clamp is active */
> +	if (priv->arizona->hpdet_clamp)
>  		val = 0;
>  
>  	regmap_update_bits_async(arizona->regmap, ARIZONA_OUTPUT_ENABLES_1,

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

Re: [PATCH 1/2] extcon: arizona: Deobfuscate arizona_extcon_do_magic

[Chanwoo Choi]

Hi Charles,

On 02/17/2015 01:48 AM, Chanwoo Choi wrote:
> Hi Charles,
> 
> 
> On Tue, Feb 17, 2015 at 12:41 AM, Charles Keepax
> <ckeepax@opensource.wolfsonmicro.com> wrote:
>> arizona_extcon_do_magic does not lend a lot of clarity to the purpose
>> of the function, and as all the registers used are described in the
>> datasheet there is no need to obfuscate the code. This patch renames the
>> function to arizona_extcon_hp_clamp, as it controls clamping on the
>> headphone output.
>>
>> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
>> ---
>>  drivers/extcon/extcon-arizona.c  |   36 ++++++++++++++++++++----------------
>>  include/linux/mfd/arizona/core.h |    2 +-
>>  sound/soc/codecs/arizona.c       |    4 ++--
>>  3 files changed, 23 insertions(+), 19 deletions(-)
> 
> 
> Looks good to me.
> But, you need more ack message by MFD maintainer.
> 
> For extcon part:
> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

Applied it.

Thanks,
Chanwoo Choi

Re: [PATCH 1/2] extcon: arizona: Deobfuscate arizona_extcon_do_magic

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 483 bytes --]

On Mon, Feb 16, 2015 at 03:41:02PM +0000, Charles Keepax wrote:
> arizona_extcon_do_magic does not lend a lot of clarity to the purpose
> of the function, and as all the registers used are described in the
> datasheet there is no need to obfuscate the code. This patch renames the
> function to arizona_extcon_hp_clamp, as it controls clamping on the
> headphone output.

Probably worth noting that at the time the code was originally
implemented these registers weren't documented.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]