Re: [PATCH] ASoC: codecs: add support for ES8326

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 7/15/22 00:41, Zhu Ning wrote:
> The ES8326 codec is not compatible with ES8316 and requires a dedicated driver.
> 
> Signed-off-by: David Yang <yangxiaohua@xxxxxxxxxxxxxxxx>
> Signed-off-by: Zhu Ning <zhuning@xxxxxxxxxxxxxxxx>
> ---

It's helpful for reviewers when you use the -v option in git
format-patch, and a change history below the --- mark also helps
understand what has been modified between versions. Thanks!

>  sound/soc/codecs/Kconfig  |   5 +
>  sound/soc/codecs/Makefile |   2 +
>  sound/soc/codecs/es8326.c | 794 ++++++++++++++++++++++++++++++++++++++
>  sound/soc/codecs/es8326.h | 187 +++++++++
>  4 files changed, 988 insertions(+)
>  mode change 100644 => 100755 sound/soc/codecs/Kconfig
>  mode change 100644 => 100755 sound/soc/codecs/Makefile
>  create mode 100755 sound/soc/codecs/es8326.c
>  create mode 100755 sound/soc/codecs/es8326.h

> +#define es8326_RATES SNDRV_PCM_RATE_8000_96000

this definition omits 24 kHz, you may want to add it explicitly.

> +
> +#define es8326_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\
> +	SNDRV_PCM_FMTBIT_S24_LE)
> +
> +static const struct snd_soc_dai_ops es8326_ops = {
> +	.hw_params = es8326_pcm_hw_params,
> +	.set_fmt = es8326_set_dai_fmt,
> +	.set_sysclk = es8326_set_dai_sysclk,
> +};
> +
> +static struct snd_soc_dai_driver es8326_dai = {
> +	.name = "ES8326 HiFi",
> +	.playback = {
> +		.stream_name = "Playback",
> +		.channels_min = 1,
> +		.channels_max = 2,
> +		.rates = es8326_RATES,
> +		.formats = es8326_FORMATS,
> +		},
> +	.capture = {
> +		.stream_name = "Capture",
> +		.channels_min = 1,
> +		.channels_max = 2,
> +		.rates = es8326_RATES,
> +		.formats = es8326_FORMATS,
> +		},
> +	.ops = &es8326_ops,
> +	.symmetric_rate = 1,
> +};
> +
> +static void es8326_enable_micbias(struct snd_soc_component *component)
> +{
> +	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
> +
> +	snd_soc_dapm_mutex_lock(dapm);
> +	snd_soc_dapm_force_enable_pin_unlocked(dapm, "MICBIAS1");
> +	snd_soc_dapm_force_enable_pin_unlocked(dapm, "MICBIAS2");
> +	snd_soc_dapm_sync_unlocked(dapm);
> +	snd_soc_dapm_mutex_unlock(dapm);
> +}
> +
> +static void es8326_disable_micbias(struct snd_soc_component *component)
> +{
> +	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
> +
> +	snd_soc_dapm_mutex_lock(dapm);
> +	snd_soc_dapm_disable_pin_unlocked(dapm, "MICBIAS1");
> +	snd_soc_dapm_disable_pin_unlocked(dapm, "MICBIAS2");
> +	snd_soc_dapm_sync_unlocked(dapm);
> +	snd_soc_dapm_mutex_unlock(dapm);
> +}
> +static void es8326_jack_detect_handler(struct work_struct *work)
> +{
> +	struct es8326_priv *es8326 =
> +		container_of(work, struct es8326_priv, jack_detect_work.work);
> +	struct snd_soc_component *comp = es8326->component;
> +	unsigned int iface;
> +
> +	iface = snd_soc_component_read(comp, ES8326_HP_DETECT_FB);
> +	dev_dbg(comp->dev, "gpio flag %#04x", iface);
> +	if ((iface & ES8326_HPINSERT_FLAG) == 0) {
> +		dev_dbg(comp->dev, "No headset detected");
> +		snd_soc_jack_report(es8326->jack, 0, SND_JACK_HEADSET);
> +		snd_soc_component_write(comp, ES8326_ADC1_SRC_2A, es8326->mic2_src);
> +		es8326_disable_micbias(comp);
> +	} else if ((iface & ES8326_HPINSERT_FLAG) == ES8326_HPINSERT_FLAG) {
> +		if ((iface & ES8326_HPBUTTON_FLAG) == 0x00) {
> +			dev_dbg(comp->dev, "Headset detected");
> +			snd_soc_jack_report(es8326->jack, SND_JACK_HEADSET, SND_JACK_HEADSET);
> +			snd_soc_component_write(comp, ES8326_ADC1_SRC_2A, es8326->mic1_src);
> +		} else {
> +			dev_dbg(comp->dev, "Headphone detected");
> +			snd_soc_jack_report(es8326->jack, SND_JACK_HEADPHONE, SND_JACK_HEADSET);
> +		}
> +	}
> +}
> +
> +static irqreturn_t es8326_irq(int irq, void *dev_id)
> +{
> +	struct es8326_priv *es8326 = dev_id;
> +	struct snd_soc_component *comp = es8326->component;
> +
> +	if(!es8326->jack)
> +		goto out;
> +	es8326_enable_micbias(comp);
> +
> +	queue_delayed_work(system_wq, &es8326->jack_detect_work,
> +			   msecs_to_jiffies(300));
> +
> +out:
> +	return IRQ_HANDLED;
> +}
> +
> +static int es8326_resume(struct snd_soc_component *component)
> +{
> +	struct es8326_priv *es8326 = snd_soc_component_get_drvdata(component);
> +
> +	regmap_write(es8326->regmap, ES8326_CLK_CTL_01, ES8326_CLK_ON);
> +	/* Two channel ADC */
> +	regmap_write(es8326->regmap, ES8326_PULLUP_CTL_F9, 0x02);
> +	regmap_write(es8326->regmap, ES8326_CLK_INV_02, 0x00);
> +	regmap_write(es8326->regmap, ES8326_CLK_DIV_CPC_0C, 0x1F);
> +	regmap_write(es8326->regmap, ES8326_CLK_VMIDS1_10, 0xC8);
> +	regmap_write(es8326->regmap, ES8326_CLK_VMIDS2_11, 0x88);
> +	regmap_write(es8326->regmap, ES8326_CLK_CAL_TIME_12, 0x20);
> +	regmap_write(es8326->regmap, ES8326_SYS_BIAS_1D, 0x08);
> +	regmap_write(es8326->regmap, ES8326_DAC2HPMIX_25, 0x22);
> +	regmap_write(es8326->regmap, ES8326_ADC1_SRC_2A, es8326->mic1_src);
> +	regmap_write(es8326->regmap, ES8326_ADC2_SRC_2B, es8326->mic2_src);
> +	regmap_write(es8326->regmap, ES8326_HPJACK_TIMER_56, 0x88);
> +	regmap_write(es8326->regmap, ES8326_HP_DET_57,
> +		     ES8326_HP_DET_SRC_PIN9 | es8326->jack_pol);
> +	regmap_write(es8326->regmap, ES8326_INT_SOURCE_58, 0x08);
> +	regmap_write(es8326->regmap, ES8326_INTOUT_IO_59, 0x45);
> +	regmap_write(es8326->regmap, ES8326_RESET_00, ES8326_CSM_ON);
> +	snd_soc_component_update_bits(component, ES8326_PGAGAIN_23,
> +				      ES8326_MIC_SEL_MASK, ES8326_MIC1_SEL);
> +
> +	es8326_irq(es8326->irq, es8326);
> +	return 0;
> +}
> +
> +static int es8326_suspend(struct snd_soc_component *component)
> +{
> +	struct es8326_priv *es8326 = snd_soc_component_get_drvdata(component);
> +
> +	regmap_write(es8326->regmap, ES8326_CLK_CTL_01, ES8326_CLK_OFF);


For symmetry with the resume case, aren't you missing

es8326_disable_micbias(comp);

cancel_delayed_work_sync(&es8326->jack_detect_work);

> +	return 0;
> +}
> +

> +static void es8326_enable_jack_detect(struct snd_soc_component *component,
> +				struct snd_soc_jack *jack)
> +{
> +	struct es8326_priv *es8326 = snd_soc_component_get_drvdata(component);
> +
> +	mutex_lock(&es8326->lock);
> +	if (es8326->jd_inverted)
> +		snd_soc_component_update_bits(component, ES8326_HP_DET_57,
> +					      ES8326_HP_DET_JACK_POL, ~es8326->jack_pol);
> +	es8326->jack = jack;
> +
> +	mutex_unlock(&es8326->lock);
> +	es8326_irq(es8326->irq, es8326);
> +}
> +
> +static void es8326_disable_jack_detect(struct snd_soc_component *component)
> +{
> +	struct es8326_priv *es8326 = snd_soc_component_get_drvdata(component);
> +
> +	dev_dbg(component->dev, "Enter into %s\n", __func__);
> +	if (!es8326->jack)
> +		return; /* Already disabled (or never enabled) */
> +
> +	cancel_delayed_work_sync(&es8326->jack_detect_work);
> +
> +	mutex_lock(&es8326->lock);
> +	if (es8326->jack->status & SND_JACK_MICROPHONE)
> +		snd_soc_jack_report(es8326->jack, 0, SND_JACK_BTN_0);

This one is odd, you don't handle buttons anywhere in this patch but here?

> +
> +	es8326->jack = NULL;
> +	mutex_unlock(&es8326->lock);
> +}
> +

> +static int es8326_i2c_probe(struct i2c_client *i2c,
> +			    const struct i2c_device_id *id)
> +{
> +	struct es8326_priv *es8326;
> +	int ret;
> +
> +	es8326 = devm_kzalloc(&i2c->dev, sizeof(struct es8326_priv), GFP_KERNEL);
> +	if (!es8326)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(i2c, es8326);
> +	es8326->i2c = i2c;
> +	mutex_init(&es8326->lock);
> +	es8326->regmap = devm_regmap_init_i2c(i2c, &es8326_regmap_config);
> +	if (IS_ERR(es8326->regmap)) {
> +		ret = PTR_ERR(es8326->regmap);
> +		dev_err(&i2c->dev, "Failed to init regmap: %d\n", ret);
> +		return ret;
> +	}
> +
> +	es8326->irq = i2c->irq;
> +	/* ES8316 is level-based while ES8326 is edge-based */
> +	ret = devm_request_threaded_irq(&i2c->dev, es8326->irq, NULL, es8326_irq,
> +					IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> +					"es8326", es8326);
> +	if (ret)
> +		dev_err(&i2c->dev, "Failed to reguest IRQ: %d\n", ret);
> +	INIT_DELAYED_WORK(&es8326->jack_detect_work,
> +			  es8326_jack_detect_handler);

Something's weird here, either you are missing a return ret;, a newline
or both. This doesn't seem right.

> +
> +	es8326->mclk = devm_clk_get_optional(&i2c->dev, "mclk");
> +	if (IS_ERR(es8326->mclk)) {
> +		dev_err(&i2c->dev, "unable to get mclk\n");
> +		return PTR_ERR(es8326->mclk);
> +	}
> +	if (!es8326->mclk)
> +		dev_warn(&i2c->dev, "assuming static mclk\n");
> +
> +	ret = clk_prepare_enable(es8326->mclk);
> +	if (ret) {
> +		dev_err(&i2c->dev, "unable to enable mclk\n");
> +		return ret;
> +	}
> +	return devm_snd_soc_register_component(&i2c->dev,
> +					&soc_component_dev_es8326,
> +					&es8326_dai, 1);
> +}

I am also wondering if on remove you'd need to do the same thing as on
suspend (disable the mic bias and cancel the workqueue)?



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux