On 2017년 01월 25일 18:34, Charles Keepax wrote:
> As the HPDET can't be aborted mid way through we should not allow any new
> insertion to be processed until the previous HPDET has finished. It is very
> unlikely but with low enough debounce settings you could start a new HPDET
> before the old one has completed, which results in an erroneous reading.
> 
> Signed-off-by: Charles Keepax <ckee...@opensource.wolfsonmicro.com>
> ---
> 
> Changes since v1:
>  - Added defines for the count and delay
>  - Added a comment to explain why we call arizona_hpdet_wait
> 
> Thanks,
> Charles
> 
>  drivers/extcon/extcon-arizona.c | 45 
> +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
> index ed78b7c..df98d84 100644
> --- a/drivers/extcon/extcon-arizona.c
> +++ b/drivers/extcon/extcon-arizona.c
> @@ -1049,6 +1049,42 @@ static void arizona_hpdet_work(struct work_struct 
> *work)
>       mutex_unlock(&info->lock);
>  }
>  
> +#define ARIZONA_HPDET_WAIT_COUNT 15
> +#define ARIZONA_HPDET_WAIT_DELAY_MS 20
> +
> +static int arizona_hpdet_wait(struct arizona_extcon_info *info)
> +{
> +     struct arizona *arizona = info->arizona;
> +     unsigned int val;
> +     int i, ret;
> +
> +     for (i = 0; i < ARIZONA_HPDET_WAIT_COUNT; i++) {
> +             ret = regmap_read(arizona->regmap, ARIZONA_HEADPHONE_DETECT_2,
> +                               &val);
> +             if (ret) {
> +                     dev_err(arizona->dev,
> +                             "Failed to read HPDET state: %d\n", ret);
> +                     return ret;
> +             }
> +
> +             switch (info->hpdet_ip_version) {
> +             case 0:
> +                     if (val & ARIZONA_HP_DONE)
> +                             return 0;
> +                     break;
> +             default:
> +                     if (val & ARIZONA_HP_DONE_B)
> +                             return 0;
> +                     break;
> +             }
> +
> +             msleep(ARIZONA_HPDET_WAIT_DELAY_MS);
> +     }
> +
> +     dev_err(arizona->dev, "HPDET did not appear to complete\n");
> +     return -ETIMEDOUT;
> +}
> +
>  static irqreturn_t arizona_jackdet(int irq, void *data)
>  {
>       struct arizona_extcon_info *info = data;
> @@ -1155,6 +1191,15 @@ static irqreturn_t arizona_jackdet(int irq, void *data)
>                                       "Removal report failed: %d\n", ret);
>               }
>  
> +             /*
> +              * If the jack was removed during a headphone detection we
> +              * need to wait for the headphone detection to finish, as
> +              * it can not be aborted. We don't want to be able to start
> +              * a new headphone detection from a fresh insert until this
> +              * one is finished.
> +              */
> +             arizona_hpdet_wait(info);

If there is no necessary to handle the error return value,
I recommend that you better to use the dev_warn() instead of dev_err().

        dev_warn(arizona->dev, "HPDET did not appear to complete\n");


How about changing the debug level with dev_warn()?

> +
>               regmap_update_bits(arizona->regmap,
>                                  ARIZONA_JACK_DETECT_DEBOUNCE,
>                                  ARIZONA_MICD_CLAMP_DB | ARIZONA_JD1_DB,
> 


-- 
Best Regards,
Chanwoo Choi
Samsung Electronics

Reply via email to