extcon: arizona: Remove duplicate set of input parent device

PATCH Applied Charles Keepax 2 participants 10 messages May 29, 2014 – May 30, 2014 External connector subsystem (extcon)

lore

Share & tools

lore

Charles Keepax

author

2014-05-29 15:35 UTC #0 lore raw

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

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 <[email protected]>

---
 drivers/extcon/extcon-arizona.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

drivers/extcon/extcon-arizona.c +0 -1

index cf90743..21ee055 100644
@@ -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;

Signature · 1 line

1.7.2.5

Charles Keepax

author

2014-05-29 15:35 UTC #1 lore raw

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

From: Nikesh Oswal <[email protected]>

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

Signed-off-by Nikesh Oswal <[email protected]>

Signed-off-by Charles Keepax <[email protected]>

---
 drivers/extcon/extcon-arizona.c |   15 ++++++---------
 1 files changed, 6 insertions(+), 9 deletions(-)

drivers/extcon/extcon-arizona.c +6 -9

index 21ee055..f2c36b1 100644
@@ -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",

Signature · 1 line

1.7.2.5

Chanwoo Choi

maintainer

2014-05-30 01:25 UTC #2 lore raw

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

Quoted 10 lines · Charles Keepax

From: Nikesh Oswal <[email protected]>
Use extcon cable API instead of state API as it is much more idiomatic.
Signed-off-by Nikesh Oswal <[email protected]>
Signed-off-by Charles Keepax <[email protected]>
---
 drivers/extcon/extcon-arizona.c |   15 ++++++---------
 1 files changed, 6 insertions(+), 9 deletions(-)

Applied after modified patch title.

Thanks,
Chanwoo Choi

Charles Keepax

author

2014-05-29 15:36 UTC #3 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/extcon/extcon-arizona.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

drivers/extcon/extcon-arizona.c +8 -5

index f2c36b1..c1c8647 100644
@@ -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;
 		}

Signature · 1 line

1.7.2.5

Chanwoo Choi

maintainer

2014-05-30 01:27 UTC #4 lore raw

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

Hi Charles,

Quoted 18 lines · 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 <[email protected]>
---
 drivers/extcon/extcon-arizona.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)
drivers/extcon/extcon-arizona.c +0 -0
index f2c36b1..c1c8647 100644
@@ -329,12 +329,13 @@ static void arizona_stop_mic(struct arizona_extcon_info *info)
 }
 
 static struct {

drivers/extcon/extcon-arizona.c static void arizona_stop_mic(struct arizona_extcon_info *info) · line 338

Quoted 10 lines

drivers/extcon/extcon-arizona.c +4 -3

+	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).

drivers/extcon/extcon-arizona.c static int arizona_hpdet_read(struct arizona_extcon_info *info) · line 396

Quoted 10 lines

drivers/extcon/extcon-arizona.c +2 -1

 };
 
 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.

drivers/extcon/extcon-arizona.c static int arizona_hpdet_read(struct arizona_extcon_info *info) · line 410

Quoted 10 lines

drivers/extcon/extcon-arizona.c +2 -1

 			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.

drivers/extcon/extcon-arizona.c static int arizona_hpdet_read(struct arizona_extcon_info *info) · line 413

Quoted 4 lines

drivers/extcon/extcon-arizona.c +0 -0

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

Thanks,
Chanwoo Choi

Charles Keepax

author

2014-05-30 09:42 UTC #5 lore raw

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

Quoted 20 lines · Chanwoo Choi

Hi Charles,
Quoted 17 lines · 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 <[email protected]>
---
 drivers/extcon/extcon-arizona.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)
drivers/extcon/extcon-arizona.c +0 -0
index f2c36b1..c1c8647 100644
@@ -329,12 +329,13 @@ static void arizona_stop_mic(struct arizona_extcon_info *info)
 }
 

Quoted 13 lines

drivers/extcon/extcon-arizona.c static void arizona_stop_mic(struct arizona_extcon_info *info) · line 338
Quoted 11 lines
drivers/extcon/extcon-arizona.c +4 -3
 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.

Quoted 7 lines

drivers/extcon/extcon-arizona.c +0 -0

 };
 
 static struct {

@@ -391,7 +392,8 @@ static int arizona_hpdet_read(struct arizona_extcon_info *info)
 			   >> ARIZONA_HP_IMPEDANCE_RANGE_SHIFT;

Quoted 7 lines

drivers/extcon/extcon-arizona.c static int arizona_hpdet_read(struct arizona_extcon_info *info) · line 396
Quoted 4 lines
drivers/extcon/extcon-arizona.c +2 -1
 		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

Charles Keepax

author

2014-05-29 15:35 UTC #6 lore raw

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

In reply to 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 <[email protected]>

---
 drivers/extcon/extcon-arizona.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/extcon/extcon-arizona.c +1 -1

index c1c8647..b2770aa 100644
@@ -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);

Signature · 1 line

1.7.2.5

Chanwoo Choi

maintainer Applied

2014-05-30 01:31 UTC #7 lore raw

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

Quoted 16 lines · 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 <[email protected]>
---
 drivers/extcon/extcon-arizona.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/extcon/extcon-arizona.c +0 -0
index c1c8647..b2770aa 100644

drivers/extcon/extcon-arizona.c static void arizona_micd_detect(struct work_struct *work) · line 832

Quoted 8 lines

drivers/extcon/extcon-arizona.c +1 -1

@@ -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

Chanwoo Choi

maintainer

2014-05-30 01:18 UTC #8 lore raw

In reply to Charles Keepax

Hi Charles,

Quoted 15 lines · 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 <[email protected]>
---
 drivers/extcon/extcon-arizona.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
drivers/extcon/extcon-arizona.c +0 -0
index cf90743..21ee055 100644

drivers/extcon/extcon-arizona.c static int arizona_extcon_probe(struct platform_device *pdev) · line 1191

Quoted 7 lines

drivers/extcon/extcon-arizona.c +0 -1

@@ -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 <[email protected]>
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 <[email protected]>

Reported-by Charles Keepax <[email protected]>

drivers/extcon/extcon-class.c +2 -0

index 18d42c0..4c2f2c5 100644
@@ -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);

Chanwoo Choi

maintainer Applied

2014-05-30 01:25 UTC #9 lore raw

In reply to Charles Keepax

Quoted 15 lines · 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 <[email protected]>
---
 drivers/extcon/extcon-arizona.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
drivers/extcon/extcon-arizona.c +0 -0
index cf90743..21ee055 100644

drivers/extcon/extcon-arizona.c static int arizona_extcon_probe(struct platform_device *pdev) · line 1191

Quoted 7 lines

drivers/extcon/extcon-arizona.c +0 -1

@@ -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