Re: [PATCH] extcon: arizona: Implement button detection support

[Chanwoo Choi <cw00.choi@samsung.com>]

Hi Mark,

On 07/21/2012 01:07 AM, Mark Brown wrote:

> As well as identifying accessories the accessory detection hardware on
> Arizona class devices can also detect a number of buttons which we should
> report via the input API.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---
>  drivers/extcon/extcon-arizona.c |   72 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 65 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
> index 427a289..fa2114f 100644
> --- a/drivers/extcon/extcon-arizona.c
> +++ b/drivers/extcon/extcon-arizona.c
> @@ -21,6 +21,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/err.h>
>  #include <linux/gpio.h>
> +#include <linux/input.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/regulator/consumer.h>
> @@ -30,11 +31,14 @@
>  #include <linux/mfd/arizona/pdata.h>
>  #include <linux/mfd/arizona/registers.h>
>  
> +#define ARIZONA_NUM_BUTTONS 6
> +
>  struct arizona_extcon_info {
>  	struct device *dev;
>  	struct arizona *arizona;
>  	struct mutex lock;
>  	struct regulator *micvdd;
> +	struct input_dev *input;
>  
>  	int micd_mode;
>  	const struct arizona_micd_config *micd_modes;
> @@ -54,6 +58,18 @@ static const struct arizona_micd_config micd_default_modes[] = {
>  	{ 0,                  2 << ARIZONA_MICD_BIAS_SRC_SHIFT, 1 },
>  };
>  
> +static struct {
> +	u16 status;
> +	int report;
> +} arizona_lvl_to_key[ARIZONA_NUM_BUTTONS] = {
> +	{  0x1, BTN_0 },
> +	{  0x2, BTN_1 },
> +	{  0x4, BTN_2 },
> +	{  0x8, BTN_3 },
> +	{ 0x10, BTN_4 },
> +	{ 0x20, BTN_5 },
> +};
> +
>  #define ARIZONA_CABLE_MECHANICAL 0
>  #define ARIZONA_CABLE_MICROPHONE 1
>  #define ARIZONA_CABLE_HEADPHONE  2
> @@ -133,6 +149,7 @@ static void arizona_stop_mic(struct arizona_extcon_info *info)
>  
>  	if (change) {
>  		regulator_disable(info->micvdd);
> +		pm_runtime_mark_last_busy(info->dev);
>  		pm_runtime_put_autosuspend(info->dev);
>  	}
>  }
> @@ -141,8 +158,8 @@ static irqreturn_t arizona_micdet(int irq, void *data)
>  {
>  	struct arizona_extcon_info *info = data;
>  	struct arizona *arizona = info->arizona;
> -	unsigned int val;
> -	int ret;
> +	unsigned int val, lvl;
> +	int ret, i;
>  
>  	mutex_lock(&info->lock);
>  
> @@ -219,13 +236,22 @@ static irqreturn_t arizona_micdet(int irq, void *data)
>  
>  	/*
>  	 * If we're still detecting and we detect a short then we've
> -	 * got a headphone.  Otherwise it's a button press, the
> -	 * button reporting is stubbed out for now.
> +	 * got a headphone.  Otherwise it's a button press.
>  	 */
>  	if (val & 0x3fc) {
>  		if (info->mic) {
>  			dev_dbg(arizona->dev, "Mic button detected\n");
>  
> +			lvl = val & ARIZONA_MICD_LVL_MASK;
> +			lvl >>= ARIZONA_MICD_LVL_SHIFT;
> +
> +			for (i = 0; i < ARIZONA_NUM_BUTTONS; i++)
> +				if (lvl & arizona_lvl_to_key[i].status)
> +					input_report_key(info->input,
> +							 arizona_lvl_to_key[i].report,
> +							 1);
> +			input_sync(info->input);
> +
>  		} else if (info->detecting) {
>  			dev_dbg(arizona->dev, "Headphone detected\n");
>  			info->detecting = false;
> @@ -244,6 +270,10 @@ static irqreturn_t arizona_micdet(int irq, void *data)
>  		}
>  	} else {
>  		dev_dbg(arizona->dev, "Mic button released\n");
> +		for (i = 0; i < ARIZONA_NUM_BUTTONS; i++)
> +			input_report_key(info->input,
> +					 arizona_lvl_to_key[i].report, 0);

> +		input_sync(info->input);

>  	}


Why do you should report released event to all of buttons? I think that
you should only
report released event to previous pressed button. If user press two
button on the headset
at the same time and then user release only one button with pressed
another button, extcon-arizona driver have to report released event to
previous pressed button except for still pressed another button.

Thank you,
Chanwoo Choi