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

Groups > linux.kernel > #1698804

Re: [PATCH v1 5/6] ASoC: codecs: msm8916-wcd-analog: add MBHC support

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

On Wed, Jul 26, 2017 at 02:35:11AM +0200, srinivas.kandagatla@linaro.org wrote:

> +	if (wcd->hphl_jack_type_normally_open)
> +		plug_type = CDC_A_HPHL_PLUG_TYPE_NO;
> +
> +	if (wcd->gnd_jack_type_normally_open)
> +		plug_type |= CDC_A_GND_PLUG_TYPE_NO;

It'd be clearer to use |= consistently, this is just confusing as it
makes it look like there's a path where things aren't initialized.

> +	btn_result = snd_soc_read(codec, CDC_A_MBHC_RESULT_1) &
> +				  CDC_A_MBHC_RESULT_1_BTN_RESULT_MASK;
> +
> +	if (!btn_result)
> +		priv->mbhc_btn0_pressed = false;
> +
> +	snd_jack_report(priv->jack->jack, 0);

This checks to see if the read worked, uses it to decide if it should
clear the button pressed flag but unconditionally reports that the
button is not pressed.  It also discards the read error if there was one
without reporting that.  This needs cleanup.

> +	btn_result = snd_soc_read(codec, CDC_A_MBHC_RESULT_1) &
> +				  CDC_A_MBHC_RESULT_1_BTN_RESULT_MASK;
> +
> +	switch (btn_result) {
> +	case 0xf:
> +		snd_jack_report(priv->jack->jack, SND_JACK_BTN_4);
> +		break;
> +	case 0x7:
> +		snd_jack_report(priv->jack->jack, SND_JACK_BTN_3);
> +		break;
> +	case 0x3:
> +		snd_jack_report(priv->jack->jack, SND_JACK_BTN_2);
> +		break;
> +	case 0x1:
> +		snd_jack_report(priv->jack->jack, SND_JACK_BTN_1);
> +		break;
> +	case 0:
> +		priv->mbhc_btn0_pressed = true;
> +		snd_jack_report(priv->jack->jack, SND_JACK_BTN_0);
> +		break;
> +	}

Here we silently ignore any value we get back from the chip that we
didn't expect and all read errors.

> +struct msm8916_wcd_mbhc_data {
> +	/* Voltage threshold when internal current source of 100uA is used */
> +	int vref_btn_cs[MBHC_MAX_BUTTONS];
> +	/* Voltage threshold when microphone bias is ON */
> +	int vref_btn_micb[MBHC_MAX_BUTTONS];
> +};

I'd expect to see some mechanism for configuring the threasholds via DT,
not all vendors use the same specs for resistive buttons.

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v1 0/6] ASoC: codecs: msm8916-wcd-analog: Add support to MBHC srinivas.kandagatla@linaro.org - 2017-07-26 02:40 +0200
  [PATCH v1 2/6] ASoC: codecs: msm8916-analog: fix DIG_CLK_CTL_RXD3_CLK_EN define srinivas.kandagatla@linaro.org - 2017-07-26 02:40 +0200
    Re: [PATCH v1 2/6] ASoC: codecs: msm8916-analog: fix  DIG_CLK_CTL_RXD3_CLK_EN define Mark Brown <broonie@kernel.org> - 2017-07-28 15:40 +0200
  [PATCH v1 4/6] ASoC: codecs: msm8916-wcd-analog: set default micbias voltage to 1.8 srinivas.kandagatla@linaro.org - 2017-07-26 02:40 +0200
    Re: [PATCH v1 4/6] ASoC: codecs: msm8916-wcd-analog: set default  micbias voltage to 1.8 Mark Brown <broonie@kernel.org> - 2017-07-28 15:50 +0200
      Re: [PATCH v1 4/6] ASoC: codecs: msm8916-wcd-analog: set default  micbias voltage to 1.8 Srinivas Kandagatla <srinivas.kandagatla@linaro.org> - 2017-07-28 16:00 +0200
        Re: [PATCH v1 4/6] ASoC: codecs: msm8916-wcd-analog: set default  micbias voltage to 1.8 Mark Brown <broonie@kernel.org> - 2017-07-28 16:10 +0200
          Re: [PATCH v1 4/6] ASoC: codecs: msm8916-wcd-analog: set default  micbias voltage to 1.8 Srinivas Kandagatla <srinivas.kandagatla@linaro.org> - 2017-07-28 16:50 +0200
            Re: [PATCH v1 4/6] ASoC: codecs: msm8916-wcd-analog: set default  micbias voltage to 1.8 Mark Brown <broonie@kernel.org> - 2017-07-31 12:10 +0200
              Re: [PATCH v1 4/6] ASoC: codecs: msm8916-wcd-analog: set default  micbias voltage to 1.8 Srinivas Kandagatla <srinivas.kandagatla@linaro.org> - 2017-07-31 12:30 +0200
  [PATCH v1 5/6] ASoC: codecs: msm8916-wcd-analog: add MBHC support srinivas.kandagatla@linaro.org - 2017-07-26 02:40 +0200
    Re: [PATCH v1 5/6] ASoC: codecs: msm8916-wcd-analog: add MBHC support Mark Brown <broonie@kernel.org> - 2017-07-28 16:00 +0200
      Re: [PATCH v1 5/6] ASoC: codecs: msm8916-wcd-analog: add MBHC support Srinivas Kandagatla <srinivas.kandagatla@linaro.org> - 2017-07-28 16:20 +0200
  [PATCH v1 1/6] ASoC: jack: fix snd_soc_codec_set_jack return error srinivas.kandagatla@linaro.org - 2017-07-26 02:40 +0200

csiph-web