[alsa-devel] [PATCH 0/5] ASoC: tlv320dac33: Adding new FIFO mode
Hello,
the following series adds new FIFO operating mode to the tlv320dac33 codec driver. To be able to add new FIFO mode the code has to be restructured in order to keep it readable after the new mode is added. Since both of the support FIFO mode in the driver needs the codec to be the master, an additional check is added to avoid misconfiguration.
--- Peter Ujfalusi (5): ASoC: tlv320dac33: Change nsample switch to FIFO mode enum ASoC: tlv320dac33: Introduce prefill and playback state handlers ASoC: tlv320dac33: Clean up the hardware configuration code ASoC: tlv320dac33: Add new FIFO mode: mode 7 ASoC: tlv320dac33: Safety check for codec slave mode
sound/soc/codecs/tlv320dac33.c | 190 ++++++++++++++++++++++++++++++++-------- 1 files changed, 153 insertions(+), 37 deletions(-)
In order to have support for more FIFO modes supported by tlv320dac33, the switch for enabling/disabling the FIFO use has to be replaced with an enum.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/tlv320dac33.c | 49 ++++++++++++++++++++++++++-------------- 1 files changed, 32 insertions(+), 17 deletions(-)
diff --git a/sound/soc/codecs/tlv320dac33.c b/sound/soc/codecs/tlv320dac33.c index 5037454..b67961d 100644 --- a/sound/soc/codecs/tlv320dac33.c +++ b/sound/soc/codecs/tlv320dac33.c @@ -59,6 +59,12 @@ enum dac33_state { DAC33_FLUSH, };
+enum dac33_fifo_modes { + DAC33_FIFO_BYPASS = 0, + DAC33_FIFO_MODE1, + DAC33_FIFO_LAST_MODE, +}; + #define DAC33_NUM_SUPPLIES 3 static const char *dac33_supply_names[DAC33_NUM_SUPPLIES] = { "AVDD", @@ -82,7 +88,7 @@ struct tlv320dac33_priv { * this */ unsigned int nsample_max; /* nsample should not be higher than * this */ - unsigned int nsample_switch; /* Use FIFO or bypass FIFO switch */ + enum dac33_fifo_modes fifo_mode;/* FIFO mode selection */ unsigned int nsample; /* burst read amount from host */
enum dac33_state state; @@ -381,39 +387,48 @@ static int dac33_set_nsample(struct snd_kcontrol *kcontrol, return ret; }
-static int dac33_get_nsample_switch(struct snd_kcontrol *kcontrol, +static int dac33_get_fifo_mode(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol); struct tlv320dac33_priv *dac33 = codec->private_data;
- ucontrol->value.integer.value[0] = dac33->nsample_switch; + ucontrol->value.integer.value[0] = dac33->fifo_mode;
return 0; }
-static int dac33_set_nsample_switch(struct snd_kcontrol *kcontrol, +static int dac33_set_fifo_mode(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol); struct tlv320dac33_priv *dac33 = codec->private_data; int ret = 0;
- if (dac33->nsample_switch == ucontrol->value.integer.value[0]) + if (dac33->fifo_mode == ucontrol->value.integer.value[0]) return 0; /* Do not allow changes while stream is running*/ if (codec->active) return -EPERM;
if (ucontrol->value.integer.value[0] < 0 || - ucontrol->value.integer.value[0] > 1) + ucontrol->value.integer.value[0] >= DAC33_FIFO_LAST_MODE) ret = -EINVAL; else - dac33->nsample_switch = ucontrol->value.integer.value[0]; + dac33->fifo_mode = ucontrol->value.integer.value[0];
return ret; }
+/* Codec operation modes */ +static const char *dac33_fifo_mode_texts[] = { + "Bypass", "Mode 1" +}; + +static const struct soc_enum dac33_fifo_mode_enum = + SOC_ENUM_SINGLE_EXT(ARRAY_SIZE(dac33_fifo_mode_texts), + dac33_fifo_mode_texts); + /* * DACL/R digital volume control: * from 0 dB to -63.5 in 0.5 dB steps @@ -436,8 +451,8 @@ static const struct snd_kcontrol_new dac33_snd_controls[] = { static const struct snd_kcontrol_new dac33_nsample_snd_controls[] = { SOC_SINGLE_EXT("nSample", 0, 0, 5900, 0, dac33_get_nsample, dac33_set_nsample), - SOC_SINGLE_EXT("nSample Switch", 0, 0, 1, 0, - dac33_get_nsample_switch, dac33_set_nsample_switch), + SOC_ENUM_EXT("FIFO Mode", dac33_fifo_mode_enum, + dac33_get_fifo_mode, dac33_set_fifo_mode), };
/* Analog bypass */ @@ -586,7 +601,7 @@ static void dac33_shutdown(struct snd_pcm_substream *substream, unsigned int pwr_ctrl;
/* Stop pending workqueue */ - if (dac33->nsample_switch) + if (dac33->fifo_mode) cancel_work_sync(&dac33->work);
mutex_lock(&dac33->mutex); @@ -714,7 +729,7 @@ static int dac33_prepare_chip(struct snd_pcm_substream *substream)
dac33_oscwait(codec);
- if (dac33->nsample_switch) { + if (dac33->fifo_mode) { /* 50-51 : ASRC Control registers */ dac33_write(codec, DAC33_ASRC_CTRL_A, (1 << 4)); /* div=2 */ dac33_write(codec, DAC33_ASRC_CTRL_B, 1); /* ??? */ @@ -734,7 +749,7 @@ static int dac33_prepare_chip(struct snd_pcm_substream *substream) dac33_write(codec, DAC33_ASRC_CTRL_B, 0); /* ??? */ }
- if (dac33->nsample_switch) + if (dac33->fifo_mode) fifoctrl_a &= ~DAC33_FBYPAS; else fifoctrl_a |= DAC33_FBYPAS; @@ -742,13 +757,13 @@ static int dac33_prepare_chip(struct snd_pcm_substream *substream)
dac33_write(codec, DAC33_SER_AUDIOIF_CTRL_A, aictrl_a); reg_tmp = dac33_read_reg_cache(codec, DAC33_SER_AUDIOIF_CTRL_B); - if (dac33->nsample_switch) + if (dac33->fifo_mode) reg_tmp &= ~DAC33_BCLKON; else reg_tmp |= DAC33_BCLKON; dac33_write(codec, DAC33_SER_AUDIOIF_CTRL_B, reg_tmp);
- if (dac33->nsample_switch) { + if (dac33->fifo_mode) { /* 20: BCLK divide ratio */ dac33_write(codec, DAC33_SER_AUDIOIF_CTRL_C, 3);
@@ -828,7 +843,7 @@ static int dac33_pcm_trigger(struct snd_pcm_substream *substream, int cmd, case SNDRV_PCM_TRIGGER_START: case SNDRV_PCM_TRIGGER_RESUME: case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: - if (dac33->nsample_switch) { + if (dac33->fifo_mode) { dac33->state = DAC33_PREFILL; queue_work(dac33->dac33_wq, &dac33->work); } @@ -836,7 +851,7 @@ static int dac33_pcm_trigger(struct snd_pcm_substream *substream, int cmd, case SNDRV_PCM_TRIGGER_STOP: case SNDRV_PCM_TRIGGER_SUSPEND: case SNDRV_PCM_TRIGGER_PAUSE_PUSH: - if (dac33->nsample_switch) { + if (dac33->fifo_mode) { dac33->state = DAC33_FLUSH; queue_work(dac33->dac33_wq, &dac33->work); } @@ -1125,7 +1140,7 @@ static int dac33_i2c_probe(struct i2c_client *client, dac33->irq = client->irq; dac33->nsample = NSAMPLE_MAX; /* Disable FIFO use by default */ - dac33->nsample_switch = 0; + dac33->fifo_mode = DAC33_FIFO_BYPASS;
tlv320dac33_codec = codec;
Ensure that the code is going to be readable, when new FIFO modes are introduced later. Move the prefill and playback state handling to inlined functions.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/tlv320dac33.c | 46 ++++++++++++++++++++++++++++++++++----- 1 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/sound/soc/codecs/tlv320dac33.c b/sound/soc/codecs/tlv320dac33.c index b67961d..f7c7bbc 100644 --- a/sound/soc/codecs/tlv320dac33.c +++ b/sound/soc/codecs/tlv320dac33.c @@ -543,6 +543,44 @@ static int dac33_set_bias_level(struct snd_soc_codec *codec, return 0; }
+static inline void dac33_prefill_handler(struct tlv320dac33_priv *dac33) +{ + struct snd_soc_codec *codec; + + codec = &dac33->codec; + + switch (dac33->fifo_mode) { + case DAC33_FIFO_MODE1: + dac33_write16(codec, DAC33_NSAMPLE_MSB, + DAC33_THRREG(dac33->nsample)); + dac33_write16(codec, DAC33_PREFILL_MSB, + DAC33_THRREG(dac33->alarm_threshold)); + break; + default: + dev_warn(codec->dev, "Unhandled FIFO mode: %d\n", + dac33->fifo_mode); + break; + } +} + +static inline void dac33_playback_handler(struct tlv320dac33_priv *dac33) +{ + struct snd_soc_codec *codec; + + codec = &dac33->codec; + + switch (dac33->fifo_mode) { + case DAC33_FIFO_MODE1: + dac33_write16(codec, DAC33_NSAMPLE_MSB, + DAC33_THRREG(dac33->nsample)); + break; + default: + dev_warn(codec->dev, "Unhandled FIFO mode: %d\n", + dac33->fifo_mode); + break; + } +} + static void dac33_work(struct work_struct *work) { struct snd_soc_codec *codec; @@ -556,14 +594,10 @@ static void dac33_work(struct work_struct *work) switch (dac33->state) { case DAC33_PREFILL: dac33->state = DAC33_PLAYBACK; - dac33_write16(codec, DAC33_NSAMPLE_MSB, - DAC33_THRREG(dac33->nsample)); - dac33_write16(codec, DAC33_PREFILL_MSB, - DAC33_THRREG(dac33->alarm_threshold)); + dac33_prefill_handler(dac33); break; case DAC33_PLAYBACK: - dac33_write16(codec, DAC33_NSAMPLE_MSB, - DAC33_THRREG(dac33->nsample)); + dac33_playback_handler(dac33); break; case DAC33_IDLE: break;
Use switch instead of if statements to configure FIFO bypass and mode1. With this change adding new FIFO mode is going to be easier, and cleaner.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/tlv320dac33.c | 61 +++++++++++++++++++++++++++++---------- 1 files changed, 45 insertions(+), 16 deletions(-)
diff --git a/sound/soc/codecs/tlv320dac33.c b/sound/soc/codecs/tlv320dac33.c index f7c7bbc..c684aa2 100644 --- a/sound/soc/codecs/tlv320dac33.c +++ b/sound/soc/codecs/tlv320dac33.c @@ -707,7 +707,7 @@ static int dac33_prepare_chip(struct snd_pcm_substream *substream) struct snd_soc_codec *codec = socdev->card->codec; struct tlv320dac33_priv *dac33 = codec->private_data; unsigned int oscset, ratioset, pwr_ctrl, reg_tmp; - u8 aictrl_a, fifoctrl_a; + u8 aictrl_a, aictrl_b, fifoctrl_a;
switch (substream->runtime->rate) { case 44100: @@ -764,6 +764,7 @@ static int dac33_prepare_chip(struct snd_pcm_substream *substream) dac33_oscwait(codec);
if (dac33->fifo_mode) { + /* Generic for all FIFO modes */ /* 50-51 : ASRC Control registers */ dac33_write(codec, DAC33_ASRC_CTRL_A, (1 << 4)); /* div=2 */ dac33_write(codec, DAC33_ASRC_CTRL_B, 1); /* ??? */ @@ -773,38 +774,66 @@ static int dac33_prepare_chip(struct snd_pcm_substream *substream)
/* Set interrupts to high active */ dac33_write(codec, DAC33_INTP_CTRL_A, DAC33_INTPM_AHIGH); - - dac33_write(codec, DAC33_FIFO_IRQ_MODE_B, - DAC33_ATM(DAC33_FIFO_IRQ_MODE_LEVEL)); - dac33_write(codec, DAC33_FIFO_IRQ_MASK, DAC33_MAT); } else { + /* FIFO bypass mode */ /* 50-51 : ASRC Control registers */ dac33_write(codec, DAC33_ASRC_CTRL_A, DAC33_SRCBYP); dac33_write(codec, DAC33_ASRC_CTRL_B, 0); /* ??? */ }
- if (dac33->fifo_mode) + /* Interrupt behaviour configuration */ + switch (dac33->fifo_mode) { + case DAC33_FIFO_MODE1: + dac33_write(codec, DAC33_FIFO_IRQ_MODE_B, + DAC33_ATM(DAC33_FIFO_IRQ_MODE_LEVEL)); + dac33_write(codec, DAC33_FIFO_IRQ_MASK, DAC33_MAT); + break; + default: + /* in FIFO bypass mode, the interrupts are not used */ + break; + } + + aictrl_b = dac33_read_reg_cache(codec, DAC33_SER_AUDIOIF_CTRL_B); + + switch (dac33->fifo_mode) { + case DAC33_FIFO_MODE1: + /* + * For mode1: + * Disable the FIFO bypass (Enable the use of FIFO) + * Select nSample mode + * BCLK is only running when data is needed by DAC33 + */ fifoctrl_a &= ~DAC33_FBYPAS; - else + fifoctrl_a &= ~DAC33_FAUTO; + aictrl_b &= ~DAC33_BCLKON; + break; + default: + /* + * For FIFO bypass mode: + * Enable the FIFO bypass (Disable the FIFO use) + * Set the BCLK as continous + */ fifoctrl_a |= DAC33_FBYPAS; - dac33_write(codec, DAC33_FIFO_CTRL_A, fifoctrl_a); + aictrl_b |= DAC33_BCLKON; + break; + }
+ dac33_write(codec, DAC33_FIFO_CTRL_A, fifoctrl_a); dac33_write(codec, DAC33_SER_AUDIOIF_CTRL_A, aictrl_a); - reg_tmp = dac33_read_reg_cache(codec, DAC33_SER_AUDIOIF_CTRL_B); - if (dac33->fifo_mode) - reg_tmp &= ~DAC33_BCLKON; - else - reg_tmp |= DAC33_BCLKON; - dac33_write(codec, DAC33_SER_AUDIOIF_CTRL_B, reg_tmp); + dac33_write(codec, DAC33_SER_AUDIOIF_CTRL_B, aictrl_b);
- if (dac33->fifo_mode) { + switch (dac33->fifo_mode) { + case DAC33_FIFO_MODE1: /* 20: BCLK divide ratio */ dac33_write(codec, DAC33_SER_AUDIOIF_CTRL_C, 3);
dac33_write16(codec, DAC33_ATHR_MSB, DAC33_THRREG(dac33->alarm_threshold)); - } else { + break; + default: + /* BYPASS mode */ dac33_write(codec, DAC33_SER_AUDIOIF_CTRL_C, 32); + break; }
mutex_unlock(&dac33->mutex);
Mode 7 of tlv320dac33 operates in the following way: The codec is in master mode. Host configures upper and lower thresholds in tlv320dac33 During playback the codec will clock in the data until the upper threshold is reached in FIFO. At this point the codec stops the colocks on the serial bus. When the FIFO fill is reaching the lower threshold limit the codec will enable the clocks on the serial bus, and clocks in data till the upper threshold is reached.
In this mode, we can also request interrupts for threshold events (upper, lower and alarm), which could be used for power management.
At this point the interrupts are not enabled for this mode, but it can be taken into use in the future, when the surrounding code makes it possible to use it.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/tlv320dac33.c | 35 ++++++++++++++++++++++++++++++++++- 1 files changed, 34 insertions(+), 1 deletions(-)
diff --git a/sound/soc/codecs/tlv320dac33.c b/sound/soc/codecs/tlv320dac33.c index c684aa2..bc35f3f 100644 --- a/sound/soc/codecs/tlv320dac33.c +++ b/sound/soc/codecs/tlv320dac33.c @@ -62,6 +62,7 @@ enum dac33_state { enum dac33_fifo_modes { DAC33_FIFO_BYPASS = 0, DAC33_FIFO_MODE1, + DAC33_FIFO_MODE7, DAC33_FIFO_LAST_MODE, };
@@ -422,7 +423,7 @@ static int dac33_set_fifo_mode(struct snd_kcontrol *kcontrol,
/* Codec operation modes */ static const char *dac33_fifo_mode_texts[] = { - "Bypass", "Mode 1" + "Bypass", "Mode 1", "Mode 7" };
static const struct soc_enum dac33_fifo_mode_enum = @@ -556,6 +557,10 @@ static inline void dac33_prefill_handler(struct tlv320dac33_priv *dac33) dac33_write16(codec, DAC33_PREFILL_MSB, DAC33_THRREG(dac33->alarm_threshold)); break; + case DAC33_FIFO_MODE7: + dac33_write16(codec, DAC33_PREFILL_MSB, + DAC33_THRREG(20)); + break; default: dev_warn(codec->dev, "Unhandled FIFO mode: %d\n", dac33->fifo_mode); @@ -574,6 +579,9 @@ static inline void dac33_playback_handler(struct tlv320dac33_priv *dac33) dac33_write16(codec, DAC33_NSAMPLE_MSB, DAC33_THRREG(dac33->nsample)); break; + case DAC33_FIFO_MODE7: + /* At the moment we are not using interrupts in mode7 */ + break; default: dev_warn(codec->dev, "Unhandled FIFO mode: %d\n", dac33->fifo_mode); @@ -788,6 +796,10 @@ static int dac33_prepare_chip(struct snd_pcm_substream *substream) DAC33_ATM(DAC33_FIFO_IRQ_MODE_LEVEL)); dac33_write(codec, DAC33_FIFO_IRQ_MASK, DAC33_MAT); break; + case DAC33_FIFO_MODE7: + /* Disable all interrupts */ + dac33_write(codec, DAC33_FIFO_IRQ_MASK, 0); + break; default: /* in FIFO bypass mode, the interrupts are not used */ break; @@ -807,6 +819,17 @@ static int dac33_prepare_chip(struct snd_pcm_substream *substream) fifoctrl_a &= ~DAC33_FAUTO; aictrl_b &= ~DAC33_BCLKON; break; + case DAC33_FIFO_MODE7: + /* + * For mode1: + * Disable the FIFO bypass (Enable the use of FIFO) + * Select Threshold mode + * BCLK is only running when data is needed by DAC33 + */ + fifoctrl_a &= ~DAC33_FBYPAS; + fifoctrl_a |= DAC33_FAUTO; + aictrl_b &= ~DAC33_BCLKON; + break; default: /* * For FIFO bypass mode: @@ -830,6 +853,16 @@ static int dac33_prepare_chip(struct snd_pcm_substream *substream) dac33_write16(codec, DAC33_ATHR_MSB, DAC33_THRREG(dac33->alarm_threshold)); break; + case DAC33_FIFO_MODE7: + /* + * Configure the threshold levels, and leave 10 sample space + * at the bottom, and also at the top of the FIFO + */ + dac33_write16(codec, DAC33_UTHR_MSB, + DAC33_THRREG(DAC33_BUFFER_SIZE_SAMPLES - 10)); + dac33_write16(codec, DAC33_LTHR_MSB, + DAC33_THRREG(10)); + break; default: /* BYPASS mode */ dac33_write(codec, DAC33_SER_AUDIOIF_CTRL_C, 32);
The currently available FIFO modes (mode1 and mode7) require master mode from the codec. Do not allow the slave configuration when the FIFO is in use.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com --- sound/soc/codecs/tlv320dac33.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/sound/soc/codecs/tlv320dac33.c b/sound/soc/codecs/tlv320dac33.c index bc35f3f..3ef3255 100644 --- a/sound/soc/codecs/tlv320dac33.c +++ b/sound/soc/codecs/tlv320dac33.c @@ -993,6 +993,7 @@ static int dac33_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt) { struct snd_soc_codec *codec = codec_dai->codec; + struct tlv320dac33_priv *dac33 = codec->private_data; u8 aictrl_a, aictrl_b;
aictrl_a = dac33_read_reg_cache(codec, DAC33_SER_AUDIOIF_CTRL_A); @@ -1005,7 +1006,11 @@ static int dac33_set_dai_fmt(struct snd_soc_dai *codec_dai, break; case SND_SOC_DAIFMT_CBS_CFS: /* Codec Slave */ - aictrl_a &= ~(DAC33_MSBCLK | DAC33_MSWCLK); + if (dac33->fifo_mode) { + dev_err(codec->dev, "FIFO mode requires master mode\n"); + return -EINVAL; + } else + aictrl_a &= ~(DAC33_MSBCLK | DAC33_MSWCLK); break; default: return -EINVAL;
On Thu, Dec 31, 2009 at 10:30:23AM +0200, Peter Ujfalusi wrote:
The currently available FIFO modes (mode1 and mode7) require master mode from the codec. Do not allow the slave configuration when the FIFO is in use.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com
Ideally the mode configuration control ought to have a corresponding limit in it too, so that these modes can't be enabled when the device is configured as a slave and is active.
Hello,
On Thursday 31 December 2009 13:52:19 ext Mark Brown wrote:
On Thu, Dec 31, 2009 at 10:30:23AM +0200, Peter Ujfalusi wrote:
The currently available FIFO modes (mode1 and mode7) require master mode from the codec. Do not allow the slave configuration when the FIFO is in use.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@nokia.com
Ideally the mode configuration control ought to have a corresponding limit in it too, so that these modes can't be enabled when the device is configured as a slave and is active.
Well, the FIFO mode can not be changed runtime (while the playback is running) for various reasons. The control is protected against changes while the playback is running. Since at the moment the driver only supports master mode FIFO scenarios (expect for the FIFO bypass mode, which should work in slave mode as well) I think this check is sufficient to ensure that the codec/cpu dai is configured correctly. As far as I know the set_dai_fmt is the place where the machine driver configures the codec to master/slave, so this must be the correct place for a safety check.
On Thu, Dec 31, 2009 at 10:30:18AM +0200, Peter Ujfalusi wrote:
the following series adds new FIFO operating mode to the tlv320dac33 codec driver. To be able to add new FIFO mode the code has to be restructured in order to keep it readable after the new mode is added. Since both of the support FIFO mode in the driver needs the codec to be the master, an additional check is added to avoid misconfiguration.
Applied all, thanks.
participants (2)
-
Mark Brown -
Peter Ujfalusi