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
