[alsa-devel] [PATCH 1/2] ASoC: rt5645: Adds push button support for rt5650

[Bard Liao]

> -----Original Message-----
> From: Mark Brown [mailto:broonie at kernel.org]
> Sent: Tuesday, April 21, 2015 4:41 AM
> To: Bard Liao
> Cc: lgirdwood at gmail.com; alsa-devel at alsa-project.org; lars at metafoo.de;
> Flove; Oder Chiou; John Lin; koro.chen at mediatek.com;
> yang.a.fang at intel.com; zhengxing at rock-chips.com
> Subject: Re: [PATCH 1/2] ASoC: rt5645: Adds push button support for
> rt5650
> 
> On Mon, Apr 20, 2015 at 10:37:21AM +0800, Bard Liao wrote:
> 
> > --- a/sound/soc/codecs/rt5645.c
> > +++ b/sound/soc/codecs/rt5645.c
> > @@ -198,7 +198,6 @@ static const struct reg_default rt5645_reg[] = {
> >  	{ 0xda, 0x0000 },
> >  	{ 0xdb, 0x0003 },
> >  	{ 0xdc, 0x0049 },
> > -	{ 0xdd, 0x001b },
> >  	{ 0xdf, 0x0008 },
> >  	{ 0xe0, 0x4000 },
> >  	{ 0xe6, 0x8000 },
> 
> That's a bit weird, why is this in here?

The default value of register 0xdd is different between rt5645 and rt5650.
And it is for button detection configuration. That's why I put it in this
patch instead of a separate patch.


> 
> > +	if (rt5645->btn_jack && rt5645->codec_type ==
> CODEC_TYPE_RT5650) {
> 
> Can we have a switch statement for the device type so it's easier to add
> other devices if there's new derivatives please?
> 
> > +			btn_type = rt5645_button_detect(rt5645->codec);
> > +			switch (btn_type) {
> > +			case 0x8000:
> > +			case 0x4000:
> > +			case 0x2000:
> > +				report |= SND_JACK_BTN_0;
> > +				break;
> 
> Why do we mash all these values together?

rt5650 can report the action of button (one click, double click or hold).
That is basically for Windows OS. For linux, we only care about whether
the button is pressed or released. So all of the three actions are treated
as button pressed.

> 
> ------Please consider the environment before printing this e-mail.