[PATCH] ASoC: nau8821: Add driver for Nuvoton codec NAU88L21

Pierre-Louis Bossart pierre-louis.bossart at linux.intel.com
Tue Aug 24 18:02:57 CEST 2021


couple of nit-picks/typos/indentations, see below.

On 8/23/21 11:16 PM, Seven Lee wrote:
> The driver is for codec NAU88L21 of Nuvoton Technology Corporation.
> The NAU88L21 is an ultra-low power high performance audio codec that
> supportsboth analog and digital audio functions.

supports both

> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index 82ee233a269d..11bcf17b5f91 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -136,6 +136,7 @@ config SND_SOC_ALL_CODECS
>  	imply SND_SOC_NAU8315
>  	imply SND_SOC_NAU8540
>  	imply SND_SOC_NAU8810
> +        imply SND_SOC_NAU8821

indent looks off?

>  	imply SND_SOC_NAU8822
>  	imply SND_SOC_NAU8824
>  	imply SND_SOC_NAU8825
> @@ -1904,6 +1905,10 @@ config SND_SOC_NAU8810
>  	tristate "Nuvoton Technology Corporation NAU88C10 CODEC"
>  	depends on I2C
>  
> +config SND_SOC_NAU8821
> +        tristate "Nuvoton Technology Corporation NAU88L21 CODEC"
> +        depends on I2C
> +

and here indent is off as well?


> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/init.h>
> +#include <linux/i2c.h>
> +#include <linux/regmap.h>
> +#include <linux/slab.h>
> +#include <linux/clk.h>
> +#include <linux/acpi.h>
> +#include <linux/math64.h>
> +#include <linux/semaphore.h>
> +#include <sound/initval.h>
> +#include <sound/tlv.h>
> +#include <sound/core.h>
> +#include <sound/pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +#include <sound/jack.h>

alphabetical order?


> +/**
> + * nau8821_sema_acquire - acquire the semaphore of nau8821
> + * @nau8821:  component to register the codec private data with
> + * @timeout: how long in jiffies to wait before failure or zero to wait
> + * until release
> + *
> + * Attempts to acquire the semaphore with number of jiffies. If no more
> + * tasks are allowed to acquire the semaphore, calling this function will
> + * put the task to sleep. If the semaphore is not released within the
> + * specified number of jiffies, this function returns.
> + * If the semaphore is not released within the specified number of jiffies,
> + * this function returns -ETIME. If the sleep is interrupted by a signal,
> + * this function will return -EINTR. It returns 0 if the semaphore was
> + * acquired successfully.
> + *
> + * Acquires the semaphore without jiffies. Try to acquire the semaphore
> + * atomically. Returns 0 if the semaphore has been acquired successfully
> + * or 1 if it cannot be acquired.
> + */
> +static int nau8821_sema_acquire(struct nau8821 *nau8821, long timeout)
> +{
> +	int ret;
> +
> +	if (!nau8821->irq)
> +		return 1;
> +
> +	if (timeout) {
> +		ret = down_timeout(&nau8821->jd_sem, timeout);
> +		if (ret < 0)
> +			dev_dbg(nau8821->dev, "Acquire semaphore timeout\n");
> +	} else {
> +		ret = down_trylock(&nau8821->jd_sem);
> +		if (ret)
> +			dev_dbg(nau8821->dev, "Acquire semaphore fail\n");
> +	}
> +
> +	return ret;
> +}
> +
> +/**
> + * nau8821_sema_release - release the semaphore of nau8821
> + * @nau8821:  component to register the codec private data with
> + *
> + * Release the semaphore which may be called from any context and
> + * even by tasks which have never called down().
> + */
> +static inline void nau8821_sema_release(struct nau8821 *nau8821)
> +{
> +	if (!nau8821->irq)
> +		return;
> +	up(&nau8821->jd_sem);
> +}

is there any specific reason why Nuvoton codec drivers use a semaphore?
isn't a mutex good enough?

> +
> +/**
> + * nau8821_sema_reset - reset the semaphore for nau8821
> + * @nau8821:  component to register the codec private data with
> + *
> + * Reset the counter of the semaphore. Call this function to restart
> + * a new round task management.
> + */
> +static inline void nau8821_sema_reset(struct nau8821 *nau8821)
> +{
> +	nau8821->jd_sem.count = 1;
> +}

> +static int nau8821_left_adc_event(struct snd_soc_dapm_widget *w,
> +	struct snd_kcontrol *kcontrol, int event)
> +{
> +	struct snd_soc_component *component =
> +		snd_soc_dapm_to_component(w->dapm);
> +	struct nau8821 *nau8821 = snd_soc_component_get_drvdata(component);
> +
> +	switch (event) {
> +	case SND_SOC_DAPM_POST_PMU:
> +		msleep(125);

use a define to keep track of sleep times in a header file?

> +		regmap_update_bits(nau8821->regmap, NAU8821_R01_ENA_CTRL,
> +			NAU8821_EN_ADCL, NAU8821_EN_ADCL);
> +		break;
> +	case SND_SOC_DAPM_POST_PMD:
> +		regmap_update_bits(nau8821->regmap,
> +			NAU8821_R01_ENA_CTRL, NAU8821_EN_ADCL, 0);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int nau8821_right_adc_event(struct snd_soc_dapm_widget *w,
> +	struct snd_kcontrol *kcontrol, int event)
> +{
> +	struct snd_soc_component *component =
> +		snd_soc_dapm_to_component(w->dapm);
> +	struct nau8821 *nau8821 = snd_soc_component_get_drvdata(component);
> +
> +	switch (event) {
> +	case SND_SOC_DAPM_POST_PMU:
> +		msleep(125);

#define?

> +		regmap_update_bits(nau8821->regmap, NAU8821_R01_ENA_CTRL,
> +			NAU8821_EN_ADCR, NAU8821_EN_ADCR);
> +		break;
> +	case SND_SOC_DAPM_POST_PMD:
> +		regmap_update_bits(nau8821->regmap,
> +			NAU8821_R01_ENA_CTRL, NAU8821_EN_ADCR, 0);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int nau8821_pump_event(struct snd_soc_dapm_widget *w,
> +	struct snd_kcontrol *kcontrol, int event)
> +{
> +	struct snd_soc_component *component =
> +		snd_soc_dapm_to_component(w->dapm);
> +	struct nau8821 *nau8821 =
> +		snd_soc_component_get_drvdata(component);
> +
> +	switch (event) {
> +	case SND_SOC_DAPM_POST_PMU:
> +		/* Prevent startup click by letting charge pump to ramp up */
> +		msleep(20);

#define?

> +		regmap_update_bits(nau8821->regmap, NAU8821_R80_CHARGE_PUMP,
> +			NAU8821_JAMNODCLOW, NAU8821_JAMNODCLOW);
> +		break;
> +	case SND_SOC_DAPM_PRE_PMD:
> +		regmap_update_bits(nau8821->regmap, NAU8821_R80_CHARGE_PUMP,
> +			NAU8821_JAMNODCLOW, 0);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int nau8821_output_dac_event(struct snd_soc_dapm_widget *w,
> +	struct snd_kcontrol *kcontrol, int event)
> +{
> +	struct snd_soc_component *component =
> +		snd_soc_dapm_to_component(w->dapm);
> +	struct nau8821 *nau8821 = snd_soc_component_get_drvdata(component);
> +
> +	switch (event) {
> +	case SND_SOC_DAPM_PRE_PMU:
> +		/* Disables the TESTDAC to let DAC signal pass through. */
> +		regmap_update_bits(nau8821->regmap, NAU8821_R66_BIAS_ADJ,
> +			NAU8821_BIAS_TESTDAC_EN, 0);
> +		break;
> +	case SND_SOC_DAPM_POST_PMD:
> +		regmap_update_bits(nau8821->regmap, NAU8821_R66_BIAS_ADJ,
> +			NAU8821_BIAS_TESTDAC_EN, NAU8821_BIAS_TESTDAC_EN);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}

> +static int nau8821_clock_check(struct nau8821 *nau8821,
> +	int stream, int rate, int osr)
> +{
> +	int osrate = 0;
> +
> +	if (stream == SNDRV_PCM_STREAM_PLAYBACK) {
> +		if (osr >= ARRAY_SIZE(osr_dac_sel))
> +			return -EINVAL;
> +		osrate = osr_dac_sel[osr].osr;
> +	} else {
> +		if (osr >= ARRAY_SIZE(osr_adc_sel))
> +			return -EINVAL;
> +		osrate = osr_adc_sel[osr].osr;
> +	}
> +
> +	if (!osrate || rate * osrate > CLK_DA_AD_MAX) {
> +		dev_err(nau8821->dev,
> +		"exceed the maximum frequency of CLK_ADC or CLK_DAC\n");

odd indentation

> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}

> +static int nau8821_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
> +{
> +	struct snd_soc_component *component = codec_dai->component;
> +	struct nau8821 *nau8821 = snd_soc_component_get_drvdata(component);
> +	unsigned int ctrl1_val = 0, ctrl2_val = 0;
> +
> +	switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
> +	case SND_SOC_DAIFMT_CBM_CFM:

use CDP_CFP

> +		ctrl2_val |= NAU8821_I2S_MS_MASTER;
> +		break;
> +	case SND_SOC_DAIFMT_CBS_CFS:

use CBC_CFC

> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> +	case SND_SOC_DAIFMT_NB_NF:
> +		break;
> +	case SND_SOC_DAIFMT_IB_NF:
> +		ctrl1_val |= NAU8821_I2S_BP_INV;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> +	case SND_SOC_DAIFMT_I2S:
> +		ctrl1_val |= NAU8821_I2S_DF_I2S;
> +		break;
> +	case SND_SOC_DAIFMT_LEFT_J:
> +		ctrl1_val |= NAU8821_I2S_DF_LEFT;
> +		break;
> +	case SND_SOC_DAIFMT_RIGHT_J:
> +		ctrl1_val |= NAU8821_I2S_DF_RIGTH;
> +		break;
> +	case SND_SOC_DAIFMT_DSP_A:
> +		ctrl1_val |= NAU8821_I2S_DF_PCM_AB;
> +		break;
> +	case SND_SOC_DAIFMT_DSP_B:
> +		ctrl1_val |= NAU8821_I2S_DF_PCM_AB;
> +		ctrl1_val |= NAU8821_I2S_PCMB_EN;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	nau8821_sema_acquire(nau8821, HZ);
> +
> +	regmap_update_bits(nau8821->regmap, NAU8821_R1C_I2S_PCM_CTRL1,
> +		NAU8821_I2S_DL_MASK | NAU8821_I2S_DF_MASK |
> +		NAU8821_I2S_BP_MASK | NAU8821_I2S_PCMB_MASK, ctrl1_val);
> +	regmap_update_bits(nau8821->regmap, NAU8821_R1D_I2S_PCM_CTRL2,
> +		NAU8821_I2S_MS_MASK, ctrl2_val);
> +
> +	nau8821_sema_release(nau8821);
> +
> +	return 0;
> +}
> +
> +static int nau8821_digital_mute(struct snd_soc_dai *dai, int mute,
> +	int direction)

indentation?

> +{
> +	struct snd_soc_component *component = dai->component;
> +	struct nau8821 *nau8821 = snd_soc_component_get_drvdata(component);
> +	unsigned int val = 0;
> +
> +	if (mute)
> +		val = NAU8821_DAC_SOFT_MUTE;
> +
> +	return regmap_update_bits(nau8821->regmap,
> +		NAU8821_R31_MUTE_CTRL, NAU8821_DAC_SOFT_MUTE, val);
> +}
> +

> +static int nau8821_set_bias_level(struct snd_soc_component *component,
> +		enum snd_soc_bias_level level)
> +{
> +	struct nau8821 *nau8821 = snd_soc_component_get_drvdata(component);
> +	struct regmap *regmap = nau8821->regmap;
> +
> +	switch (level) {
> +	case SND_SOC_BIAS_ON:
> +		break;
> +
> +	case SND_SOC_BIAS_PREPARE:
> +		break;
> +
> +	case SND_SOC_BIAS_STANDBY:
> +		/* Setup codec configuration after resume */
> +		if (snd_soc_component_get_bias_level(component) ==
> +			SND_SOC_BIAS_OFF)
> +			nau8821_resume_setup(nau8821);
> +		break;
> +
> +	case SND_SOC_BIAS_OFF:
> +		/* HPL/HPR short to ground */
> +		regmap_update_bits(regmap, NAU8821_R0D_JACK_DET_CTRL,
> +			NAU8821_SPKR_DWN1R | NAU8821_SPKR_DWN1L, 0);
> +		if (nau8821->irq) {
> +			/* Reset semaphore */
> +			nau8821_sema_reset(nau8821);
> +			/* Reset the configuration of jack type for detection.
> +			 * Detach 2kOhm Resistors from MICBIAS to MICGND1/2.
> +			 */
> +			regmap_update_bits(regmap, NAU8821_R74_MIC_BIAS,
> +				NAU8821_MICBIAS_JKR2, 0);
> +			/* Turn off all interruptions before system shutdown.
> +			 * Keep theinterruption quiet before resume
> +			 * setup completes.
> +			 */
> +			regmap_write(regmap,
> +				NAU8821_R12_INTERRUPT_DIS_CTRL, 0xffff);
> +			regmap_update_bits(regmap, NAU8821_R0F_INTERRUPT_MASK,
> +				NAU8821_IRQ_EJECT_EN | NAU8821_IRQ_INSERT_EN,
> +				NAU8821_IRQ_EJECT_EN | NAU8821_IRQ_INSERT_EN);
> +		}
> +		break;

default case?

> +	}
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused nau8821_suspend(struct snd_soc_component *component)
> +{
> +	struct nau8821 *nau8821 = snd_soc_component_get_drvdata(component);
> +
> +	if (nau8821->irq)
> +		disable_irq(nau8821->irq);
> +	snd_soc_component_force_bias_level(component, SND_SOC_BIAS_OFF);
> +	/* Power down codec power; don't suppoet button wakeup */

support?

> +	snd_soc_dapm_disable_pin(nau8821->dapm, "MICBIAS");
> +	snd_soc_dapm_sync(nau8821->dapm);
> +	regcache_cache_only(nau8821->regmap, true);
> +	regcache_mark_dirty(nau8821->regmap);
> +
> +	return 0;
> +}
> +

> +MODULE_LICENSE("GPL v2");

MODULE_LICENSE("GPL");

the license version is provided by the SPDX line



More information about the Alsa-devel mailing list