In the error path of arizona_identify_headphone, neither the clamp nor
the PM runtime are cleaned up. Add calls to clean up both of these.
Signed-off-by: Charles Keepax <[email protected]>
---
drivers/extcon/extcon-arizona.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index e970134c95fab..79e9a24101823 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -724,6 +724,9 @@ static void arizona_identify_headphone(struct arizona_extcon_info *info)
return;
err:
+ arizona_extcon_hp_clamp(info, false);
+ pm_runtime_put_autosuspend(info->dev);
+
regmap_update_bits(arizona->regmap, ARIZONA_ACCESSORY_DETECT_MODE_1,
ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
--
2.11.0
It makes sense to clear the internal state of the jack detection
regardless of if the headphone detect based accessory detection or
the normal microphone detect based flow is used.
No issues are currently known because of this but the change makes
more logical sense and eases future refactoring of the code.
Signed-off-by: Charles Keepax <[email protected]>
---
drivers/extcon/extcon-arizona.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index 121c417069478..11f1d753aa102 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -1154,11 +1154,11 @@ static irqreturn_t arizona_jackdet(int irq, void *data)
dev_err(arizona->dev, "Mechanical report failed: %d\n",
ret);
- if (!arizona->pdata.hpdet_acc_id) {
- info->detecting = true;
- info->mic = false;
- info->jack_flips = 0;
+ info->detecting = true;
+ info->mic = false;
+ info->jack_flips = 0;
+ if (!arizona->pdata.hpdet_acc_id) {
arizona_start_mic(info);
} else {
queue_delayed_work(system_power_efficient_wq,
--
2.11.0
arizona_start_mic sets ACCDET_MODE as required for the microphone
detection as such it is redundant to set this outside of this function.
Signed-off-by: Charles Keepax <[email protected]>
---
drivers/extcon/extcon-arizona.c | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index 5ae111ee3d631..e7c198e798e27 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -668,11 +668,6 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
if (id_gpio)
gpio_set_value_cansleep(id_gpio, 0);
- /* Revert back to MICDET mode */
- regmap_update_bits(arizona->regmap,
- ARIZONA_ACCESSORY_DETECT_MODE_1,
- ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
-
/* If we have a mic then reenable MICDET */
if (mic || info->mic)
arizona_start_mic(info);
@@ -732,9 +727,6 @@ static void arizona_identify_headphone(struct arizona_extcon_info *info)
arizona_extcon_hp_clamp(info, false);
pm_runtime_put_autosuspend(info->dev);
- regmap_update_bits(arizona->regmap, ARIZONA_ACCESSORY_DETECT_MODE_1,
- ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
-
/* Just report headphone */
ret = extcon_set_state_sync(info->edev, EXTCON_JACK_HEADPHONE, true);
if (ret != 0)
@@ -789,9 +781,6 @@ static void arizona_start_hpdet_acc_id(struct arizona_extcon_info *info)
return;
err:
- regmap_update_bits(arizona->regmap, ARIZONA_ACCESSORY_DETECT_MODE_1,
- ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
-
/* Just report headphone */
ret = extcon_set_state_sync(info->edev, EXTCON_JACK_HEADPHONE, true);
if (ret != 0)
--
2.11.0
A WARN_ON is very strong for simply finding a button that is out of
range, downgrade this to a simple error message in the log.
Signed-off-by: Charles Keepax <[email protected]>
---
drivers/extcon/extcon-arizona.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index e7c198e798e27..3f7ced35e0b86 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -960,14 +960,13 @@ static void arizona_micd_detect(struct work_struct *work)
input_report_key(info->input,
info->micd_ranges[i].key, 0);
- WARN_ON(!lvl);
- WARN_ON(ffs(lvl) - 1 >= info->num_micd_ranges);
if (lvl && ffs(lvl) - 1 < info->num_micd_ranges) {
key = info->micd_ranges[ffs(lvl) - 1].key;
input_report_key(info->input, key, 1);
input_sync(info->input);
+ } else {
+ dev_err(arizona->dev, "Button out of range\n");
}
-
} else if (info->detecting) {
dev_dbg(arizona->dev, "Headphone detected\n");
info->detecting = false;
--
2.11.0
The microphone detection handler is very long, start breaking it up
by factoring out the actual reading of the impedance value into a
separate functions. Additionally, this also fixes a minor bug and
ensures that the microphone timeout will be rescheduled in all error
cases.
Signed-off-by: Charles Keepax <[email protected]>
---
drivers/extcon/extcon-arizona.c | 125 +++++++++++++++++++++++-----------------
1 file changed, 73 insertions(+), 52 deletions(-)
diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index a0135f44cba1e..b09a9a8ce98bc 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -804,70 +804,55 @@ static void arizona_micd_timeout_work(struct work_struct *work)
mutex_unlock(&info->lock);
}
-static void arizona_micd_detect(struct work_struct *work)
+static int arizona_micd_adc_read(struct arizona_extcon_info *info)
{
- struct arizona_extcon_info *info = container_of(work,
- struct arizona_extcon_info,
- micd_detect_work.work);
struct arizona *arizona = info->arizona;
- unsigned int val = 0, lvl;
- int ret, i, key;
-
- cancel_delayed_work_sync(&info->micd_timeout_work);
+ unsigned int val;
+ int ret;
- mutex_lock(&info->lock);
+ /* Must disable MICD before we read the ADCVAL */
+ regmap_update_bits(arizona->regmap, ARIZONA_MIC_DETECT_1,
+ ARIZONA_MICD_ENA, 0);
- /* If the cable was removed while measuring ignore the result */
- ret = extcon_get_state(info->edev, EXTCON_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;
+ ret = regmap_read(arizona->regmap, ARIZONA_MIC_DETECT_4, &val);
+ if (ret != 0) {
+ dev_err(arizona->dev,
+ "Failed to read MICDET_ADCVAL: %d\n", ret);
+ return ret;
}
- if (info->detecting && arizona->pdata.micd_software_compare) {
- /* Must disable MICD before we read the ADCVAL */
- regmap_update_bits(arizona->regmap, ARIZONA_MIC_DETECT_1,
- ARIZONA_MICD_ENA, 0);
- ret = regmap_read(arizona->regmap, ARIZONA_MIC_DETECT_4, &val);
- if (ret != 0) {
- dev_err(arizona->dev,
- "Failed to read MICDET_ADCVAL: %d\n",
- ret);
- mutex_unlock(&info->lock);
- return;
- }
+ dev_dbg(arizona->dev, "MICDET_ADCVAL: %x\n", val);
- dev_dbg(arizona->dev, "MICDET_ADCVAL: %x\n", val);
+ val &= ARIZONA_MICDET_ADCVAL_MASK;
+ if (val < ARRAY_SIZE(arizona_micd_levels))
+ val = arizona_micd_levels[val];
+ else
+ val = INT_MAX;
+
+ if (val <= QUICK_HEADPHONE_MAX_OHM)
+ val = ARIZONA_MICD_STS | ARIZONA_MICD_LVL_0;
+ else if (val <= MICROPHONE_MIN_OHM)
+ val = ARIZONA_MICD_STS | ARIZONA_MICD_LVL_1;
+ else if (val <= MICROPHONE_MAX_OHM)
+ val = ARIZONA_MICD_STS | ARIZONA_MICD_LVL_8;
+ else
+ val = ARIZONA_MICD_LVL_8;
- val &= ARIZONA_MICDET_ADCVAL_MASK;
- if (val < ARRAY_SIZE(arizona_micd_levels))
- val = arizona_micd_levels[val];
- else
- val = INT_MAX;
-
- if (val <= QUICK_HEADPHONE_MAX_OHM)
- val = ARIZONA_MICD_STS | ARIZONA_MICD_LVL_0;
- else if (val <= MICROPHONE_MIN_OHM)
- val = ARIZONA_MICD_STS | ARIZONA_MICD_LVL_1;
- else if (val <= MICROPHONE_MAX_OHM)
- val = ARIZONA_MICD_STS | ARIZONA_MICD_LVL_8;
- else
- val = ARIZONA_MICD_LVL_8;
- }
+ return val;
+}
+
+static int arizona_micd_read(struct arizona_extcon_info *info)
+{
+ struct arizona *arizona = info->arizona;
+ unsigned int val = 0;
+ int ret, 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);
- mutex_unlock(&info->lock);
- return;
+ return ret;
}
dev_dbg(arizona->dev, "MICDET: %x\n", val);
@@ -875,17 +860,53 @@ static void arizona_micd_detect(struct work_struct *work)
if (!(val & ARIZONA_MICD_VALID)) {
dev_warn(arizona->dev,
"Microphone detection state invalid\n");
- mutex_unlock(&info->lock);
- return;
+ return -EINVAL;
}
}
if (i == 10 && !(val & MICD_LVL_0_TO_8)) {
dev_err(arizona->dev, "Failed to get valid MICDET value\n");
+ return -EINVAL;
+ }
+
+ return val;
+}
+
+static void arizona_micd_detect(struct work_struct *work)
+{
+ struct arizona_extcon_info *info = container_of(work,
+ struct arizona_extcon_info,
+ micd_detect_work.work);
+ struct arizona *arizona = info->arizona;
+ unsigned int val = 0, lvl;
+ int ret, i, key;
+
+ cancel_delayed_work_sync(&info->micd_timeout_work);
+
+ mutex_lock(&info->lock);
+
+ /* If the cable was removed while measuring ignore the result */
+ ret = extcon_get_state(info->edev, EXTCON_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;
}
+ if (info->detecting && arizona->pdata.micd_software_compare)
+ ret = arizona_micd_adc_read(info);
+ else
+ ret = arizona_micd_read(info);
+ if (ret < 0)
+ goto handled;
+
+ val = ret;
+
/* Due to jack detect this should never happen */
if (!(val & ARIZONA_MICD_STS)) {
dev_warn(arizona->dev, "Detected open circuit\n");
--
2.11.0
On 12/9/19 8:09 PM, Charles Keepax wrote:
> In the error path of arizona_identify_headphone, neither the clamp nor
> the PM runtime are cleaned up. Add calls to clean up both of these.
>
> Signed-off-by: Charles Keepax <[email protected]>
> ---
> drivers/extcon/extcon-arizona.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
> index e970134c95fab..79e9a24101823 100644
> --- a/drivers/extcon/extcon-arizona.c
> +++ b/drivers/extcon/extcon-arizona.c
> @@ -724,6 +724,9 @@ static void arizona_identify_headphone(struct arizona_extcon_info *info)
> return;
>
> err:
> + arizona_extcon_hp_clamp(info, false);
> + pm_runtime_put_autosuspend(info->dev);
> +
> regmap_update_bits(arizona->regmap, ARIZONA_ACCESSORY_DETECT_MODE_1,
> ARIZONA_ACCDET_MODE_MASK, ARIZONA_ACCDET_MODE_MIC);
>
>
Applied these patches in this series. Thanks.
--
Best Regards,
Chanwoo Choi
Samsung Electronics