[PATCH 1/4] extcon: arizona: Remove duplicate set of input parent device

[PATCH 1/4] extcon: arizona: Remove duplicate set of input parent device

[Charles Keepax]

devm_input_allocate_device already sets the parent device to be that
passed to it, we also set this manually in arizona_extcon_probe. This
patch removes the redundant set from arizona_extcon_probe.

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

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index cf90743..21ee055 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -1186,7 +1186,6 @@ static int arizona_extcon_probe(struct platform_device *pdev)
 
 	info->input->name = "Headset";
 	info->input->phys = "arizona/extcon";
-	info->input->dev.parent = &pdev->dev;
 
 	if (pdata->num_micd_configs) {
 		info->micd_modes = pdata->micd_configs;
-- 
1.7.2.5

[PATCH 2/4] extcon: arizona: Use extcon cable API

[Charles Keepax]

From: Nikesh Oswal <Nikesh.Oswal@wolfsonmicro.com>

Use extcon cable API instead of state API as it is much more
idiomatic.

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

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index 21ee055..f2c36b1 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -669,9 +669,8 @@ err:
 			   ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
 
 	/* Just report headphone */
-	ret = extcon_update_state(info->edev,
-				  1 << ARIZONA_CABLE_HEADPHONE,
-				  1 << ARIZONA_CABLE_HEADPHONE);
+	ret = extcon_set_cable_state_(info->edev,
+				      ARIZONA_CABLE_HEADPHONE, true);
 	if (ret != 0)
 		dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
 
@@ -728,9 +727,8 @@ err:
 			   ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
 
 	/* Just report headphone */
-	ret = extcon_update_state(info->edev,
-				  1 << ARIZONA_CABLE_HEADPHONE,
-				  1 << ARIZONA_CABLE_HEADPHONE);
+	ret = extcon_set_cable_state_(info->edev,
+				      ARIZONA_CABLE_HEADPHONE, true);
 	if (ret != 0)
 		dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
 
@@ -817,9 +815,8 @@ static void arizona_micd_detect(struct work_struct *work)
 	if (info->detecting && (val & ARIZONA_MICD_LVL_8)) {
 		arizona_identify_headphone(info);
 
-		ret = extcon_update_state(info->edev,
-					  1 << ARIZONA_CABLE_MICROPHONE,
-					  1 << ARIZONA_CABLE_MICROPHONE);
+		ret = extcon_set_cable_state_(info->edev,
+					      ARIZONA_CABLE_MICROPHONE, true);
 
 		if (ret != 0)
 			dev_err(arizona->dev, "Headset report failed: %d\n",
-- 
1.7.2.5

Re: [PATCH 2/4] extcon: arizona: Use extcon cable API

[Chanwoo Choi]

On 05/30/2014 12:27 AM, Charles Keepax wrote:
> From: Nikesh Oswal <Nikesh.Oswal@wolfsonmicro.com>
> 
> Use extcon cable API instead of state API as it is much more
> idiomatic.
> 
> Signed-off-by: Nikesh Oswal <Nikesh.Oswal@wolfsonmicro.com>
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/extcon/extcon-arizona.c |   15 ++++++---------
>  1 files changed, 6 insertions(+), 9 deletions(-)

Applied after modified patch title.

Thanks,
Chanwoo Choi 

[PATCH 3/4] extcon: arizona: Update manual headphone detection calculation

[Charles Keepax]

The higher levels of impedance have a higher minimum value than the
first level. As the same value was used for all levels, higher impedances
were reported with a very low level of accuracy. This patch applies the
approriate lower threshold for each level.

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

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index f2c36b1..c1c8647 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -329,12 +329,13 @@ static void arizona_stop_mic(struct arizona_extcon_info *info)
 }
 
 static struct {
+	unsigned int threshold;
 	unsigned int factor_a;
 	unsigned int factor_b;
 } arizona_hpdet_b_ranges[] = {
-	{  5528,   362464 },
-	{ 11084,  6186851 },
-	{ 11065, 65460395 },
+	{ 100,  5528,   362464 },
+	{ 169, 11084,  6186851 },
+	{ 169, 11065, 65460395 },
 };
 
 static struct {
@@ -391,7 +392,8 @@ static int arizona_hpdet_read(struct arizona_extcon_info *info)
 			   >> ARIZONA_HP_IMPEDANCE_RANGE_SHIFT;
 
 		if (range < ARRAY_SIZE(arizona_hpdet_b_ranges) - 1 &&
-		    (val < 100 || val >= 0x3fb)) {
+		    (val < arizona_hpdet_b_ranges[range].threshold ||
+		     val >= 0x3fb)) {
 			range++;
 			dev_dbg(arizona->dev, "Moving to HPDET range %d\n",
 				range);
@@ -404,7 +406,8 @@ static int arizona_hpdet_read(struct arizona_extcon_info *info)
 		}
 
 		/* If we go out of range report top of range */
-		if (val < 100 || val >= 0x3fb) {
+		if (val < arizona_hpdet_b_ranges[range].threshold ||
+		    val >= 0x3fb) {
 			dev_dbg(arizona->dev, "Measurement out of range\n");
 			return ARIZONA_HPDET_MAX;
 		}
-- 
1.7.2.5

Re: [PATCH 3/4] extcon: arizona: Update manual headphone detection calculation

[Chanwoo Choi]

Hi Charles,

On 05/30/2014 12:27 AM, Charles Keepax wrote:
> The higher levels of impedance have a higher minimum value than the
> first level. As the same value was used for all levels, higher impedances
> were reported with a very low level of accuracy. This patch applies the
> approriate lower threshold for each level.
> 
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/extcon/extcon-arizona.c |   13 ++++++++-----
>  1 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
> index f2c36b1..c1c8647 100644
> --- a/drivers/extcon/extcon-arizona.c
> +++ b/drivers/extcon/extcon-arizona.c
> @@ -329,12 +329,13 @@ static void arizona_stop_mic(struct arizona_extcon_info *info)
>  }
>  
>  static struct {
> +	unsigned int threshold;
>  	unsigned int factor_a;
>  	unsigned int factor_b;
>  } arizona_hpdet_b_ranges[] = {
> -	{  5528,   362464 },
> -	{ 11084,  6186851 },
> -	{ 11065, 65460395 },
> +	{ 100,  5528,   362464 },
> +	{ 169, 11084,  6186851 },
> +	{ 169, 11065, 65460395 },

If you possible, I want to know the 'unit' of the variable(factor_a, factor_b).

>  };
>  
>  static struct {
> @@ -391,7 +392,8 @@ static int arizona_hpdet_read(struct arizona_extcon_info *info)
>  			   >> ARIZONA_HP_IMPEDANCE_RANGE_SHIFT;
>  
>  		if (range < ARRAY_SIZE(arizona_hpdet_b_ranges) - 1 &&
> -		    (val < 100 || val >= 0x3fb)) {
> +		    (val < arizona_hpdet_b_ranges[range].threshold ||
> +		     val >= 0x3fb)) {

I don't understand the meaning of 0x3fb hex value.
I prefer to define constant variable for readability.

>  			range++;
>  			dev_dbg(arizona->dev, "Moving to HPDET range %d\n",
>  				range);
> @@ -404,7 +406,8 @@ static int arizona_hpdet_read(struct arizona_extcon_info *info)
>  		}
>  
>  		/* If we go out of range report top of range */
> -		if (val < 100 || val >= 0x3fb) {
> +		if (val < arizona_hpdet_b_ranges[range].threshold ||
> +		    val >= 0x3fb) {

ditto.

>  			dev_dbg(arizona->dev, "Measurement out of range\n");
>  			return ARIZONA_HPDET_MAX;
>  		}
> 

Thanks,
Chanwoo Choi

Re: [PATCH 3/4] extcon: arizona: Update manual headphone detection calculation

[Charles Keepax]

On Fri, May 30, 2014 at 10:27:35AM +0900, Chanwoo Choi wrote:
> Hi Charles,
> 
> On 05/30/2014 12:27 AM, Charles Keepax wrote:
> > The higher levels of impedance have a higher minimum value than the
> > first level. As the same value was used for all levels, higher impedances
> > were reported with a very low level of accuracy. This patch applies the
> > approriate lower threshold for each level.
> > 
> > Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> > ---
> >  drivers/extcon/extcon-arizona.c |   13 ++++++++-----
> >  1 files changed, 8 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
> > index f2c36b1..c1c8647 100644
> > --- a/drivers/extcon/extcon-arizona.c
> > +++ b/drivers/extcon/extcon-arizona.c
> > @@ -329,12 +329,13 @@ static void arizona_stop_mic(struct arizona_extcon_info *info)
> >  }
> >  
> >  static struct {
> > +	unsigned int threshold;
> >  	unsigned int factor_a;
> >  	unsigned int factor_b;
> >  } arizona_hpdet_b_ranges[] = {
> > -	{  5528,   362464 },
> > -	{ 11084,  6186851 },
> > -	{ 11065, 65460395 },
> > +	{ 100,  5528,   362464 },
> > +	{ 169, 11084,  6186851 },
> > +	{ 169, 11065, 65460395 },
> 
> If you possible, I want to know the 'unit' of the variable(factor_a, factor_b).

I am not sure there are really especially sensible units to give.

> 
> >  };
> >  
> >  static struct {
> > @@ -391,7 +392,8 @@ static int arizona_hpdet_read(struct arizona_extcon_info *info)
> >  			   >> ARIZONA_HP_IMPEDANCE_RANGE_SHIFT;
> >  
> >  		if (range < ARRAY_SIZE(arizona_hpdet_b_ranges) - 1 &&
> > -		    (val < 100 || val >= 0x3fb)) {
> > +		    (val < arizona_hpdet_b_ranges[range].threshold ||
> > +		     val >= 0x3fb)) {
> 
> I don't understand the meaning of 0x3fb hex value.
> I prefer to define constant variable for readability.

Thats fine I can add a define as part of this patch.

Thanks,
Charles

[PATCH 4/4] extcon: arizona: Correct typo to disable regulation for button detection

[Charles Keepax]

We can use the bypass mode on the MICVDD reg for button detection, as
the comment in the code states, however the code was mistakenly
disabling bypass. This patch corrects this and allows bypass mode during
button detection.

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

diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index c1c8647..b2770aa 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -826,7 +826,7 @@ static void arizona_micd_detect(struct work_struct *work)
 				ret);
 
 		/* Don't need to regulate for button detection */
-		ret = regulator_allow_bypass(info->micvdd, false);
+		ret = regulator_allow_bypass(info->micvdd, true);
 		if (ret != 0) {
 			dev_err(arizona->dev, "Failed to bypass MICVDD: %d\n",
 				ret);
-- 
1.7.2.5

Re: [PATCH 4/4] extcon: arizona: Correct typo to disable regulation for button detection

[Chanwoo Choi]

On 05/30/2014 12:27 AM, Charles Keepax wrote:
> We can use the bypass mode on the MICVDD reg for button detection, as
> the comment in the code states, however the code was mistakenly
> disabling bypass. This patch corrects this and allows bypass mode during
> button detection.
> 
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/extcon/extcon-arizona.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
> index c1c8647..b2770aa 100644
> --- a/drivers/extcon/extcon-arizona.c
> +++ b/drivers/extcon/extcon-arizona.c
> @@ -826,7 +826,7 @@ static void arizona_micd_detect(struct work_struct *work)
>  				ret);
>  
>  		/* Don't need to regulate for button detection */
> -		ret = regulator_allow_bypass(info->micvdd, false);
> +		ret = regulator_allow_bypass(info->micvdd, true);
>  		if (ret != 0) {
>  			dev_err(arizona->dev, "Failed to bypass MICVDD: %d\n",
>  				ret);
> 
Applied.

Thanks,
Chanwoo Choi

Re: [PATCH 1/4] extcon: arizona: Remove duplicate set of input parent device

[Chanwoo Choi]

Hi Charles,

On 05/30/2014 12:27 AM, Charles Keepax wrote:
> devm_input_allocate_device already sets the parent device to be that
> passed to it, we also set this manually in arizona_extcon_probe. This
> patch removes the redundant set from arizona_extcon_probe.
> 
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/extcon/extcon-arizona.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
> index cf90743..21ee055 100644
> --- a/drivers/extcon/extcon-arizona.c
> +++ b/drivers/extcon/extcon-arizona.c
> @@ -1186,7 +1186,6 @@ static int arizona_extcon_probe(struct platform_device *pdev)
>  
>  	info->input->name = "Headset";
>  	info->input->phys = "arizona/extcon";
> -	info->input->dev.parent = &pdev->dev;
>  
>  	if (pdata->num_micd_configs) {
>  		info->micd_modes = pdata->micd_configs;
> 

After checked this patch, I'm going to modify devm_extcon_dev_allocate()
to set parent device for extcon device as following patch.

Thanks,
Chanwoo Choi

commit b8ca2d99209e402faf12545759c8ed14fa3455a0
Author: Chanwoo Choi <cw00.choi@samsung.com>
Date:   Fri May 30 10:13:15 2014 +0900

    extcon: Set parent device of extcon device using prameter of devm_extcon_dev_allocate
    
    This patch set the parent device of extcon device using first parameter of
    devm_extco_dev_allocate() to remove duplicate code on all of extcon provider
    drivers.
    
    Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
    Reported-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
index 18d42c0..4c2f2c5 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -645,6 +645,8 @@ struct extcon_dev *devm_extcon_dev_allocate(struct device *dev,
                return edev;
        }
 
+       edev->dev.parent = dev;
+
        *ptr = edev;
        devres_add(dev, ptr);

Re: [PATCH 1/4] extcon: arizona: Remove duplicate set of input parent device

[Chanwoo Choi]

On 05/30/2014 12:27 AM, Charles Keepax wrote:
> devm_input_allocate_device already sets the parent device to be that
> passed to it, we also set this manually in arizona_extcon_probe. This
> patch removes the redundant set from arizona_extcon_probe.
> 
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
>  drivers/extcon/extcon-arizona.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
> index cf90743..21ee055 100644
> --- a/drivers/extcon/extcon-arizona.c
> +++ b/drivers/extcon/extcon-arizona.c
> @@ -1186,7 +1186,6 @@ static int arizona_extcon_probe(struct platform_device *pdev)
>  
>  	info->input->name = "Headset";
>  	info->input->phys = "arizona/extcon";
> -	info->input->dev.parent = &pdev->dev;
>  
>  	if (pdata->num_micd_configs) {
>  		info->micd_modes = pdata->micd_configs;
> 
Applied.

Thanks,
Chanwoo Choi