Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1566413 > unrolled thread

Re: [PATCH v2] extcon: arizona: Wait for any running HPDETs to complete on jack removal

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2] extcon: arizona: Wait for any running HPDETs to  complete on jack removal Chanwoo Choi <cw00.choi@samsung.com> - 2017-01-25 10:40 +0100
    Re: [PATCH v2] extcon: arizona: Wait for any running HPDETs to  complete on jack removal Charles Keepax <ckeepax@opensource.wolfsonmicro.com> - 2017-01-25 10:50 +0100

#1566413 — Re: [PATCH v2] extcon: arizona: Wait for any running HPDETs to complete on jack removal

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

[toc] | [next] | [standalone]

#1566430

On Wed, Jan 25, 2017 at 06:35:13PM +0900, Chanwoo Choi wrote:
> 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 <ckeepax@opensource.wolfsonmicro.com>
> > ---
> > +		/*
> > +		 * 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()?
> 

Yeah can do.

Thanks,
Charles

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web