March 2012 - Alsa-devel - mailman.alsa-project.org

Re: [alsa-devel] [PATCH 13/16] ASoC: codecs: Add AB8500 codec-driver
by Mark Brown 19 Mar '12

19 Mar '12

On Tue, Mar 13, 2012 at 04:11:40PM +0100, Ola Lilja wrote: > +++ b/sound/soc/codecs/Kconfig > @@ -12,6 +12,7 @@ config SND_SOC_ALL_CODECS > tristate "Build all ASoC CODEC drivers" > select SND_SOC_88PM860X if MFD_88PM860X > select SND_SOC_L3 > + select SND_SOC_AB8500 > select SND_SOC_AC97_CODEC if SND_SOC_AC97_BUS > select SND_SOC_AD1836 if SPI_MASTER > select SND_SOC_AD193X if SND_SOC_I2C_AND_SPI This driver really has no dependencies? That seems *extremely* surprising - have you tried building on x86? > + > +ifdef CONFIG_SND_SOC_UX500_DEBUG > +CFLAGS_ab8500_audio.o := -DDEBUG > +endif Once again if you're adding random custom stuff like this for your driver you shouldn't be. > diff --git a/sound/soc/codecs/ab8500_audio.c b/sound/soc/codecs/ab8500_audio.c Everything else uses -codec. > +/* AB8500 register cache */ > +static const u8 ab8500_reg_cache[AB8500_CACHEREGNUM]; > +static enum sid_state sid_status = SID_UNCONFIGURED; No static data or custom cache implementations. > +/* Read a register from the audio-bank of AB8500 */ > +static unsigned int ab8500_codec_read_reg(struct snd_soc_codec *codec, unsigned int reg) > +{ > + int status; > + unsigned int value = 0; > + > + if (reg < VIRT_REG_BASE) { Why are you doing this virtual register stuff? It's making the code a lot more complex and doesn't look at all driver specific. > + } else { > + dev_dbg(codec->dev, "%s: Read 0x%02x from register 0x%02x:0x%02x\n", > + __func__, value8, (u8)AB8500_AUDIO, (u8)reg); There's already *plenty* of trace for register I/O in the framework. > +static const char * const enum_ena_dis[] = {"Enabled", "Disabled"}; > +static const char * const enum_dis_ena[] = {"Disabled", "Enabled"}; Why are the controls using these enums and not Switch controls? UIs know how to display switches. > +/* Earpiece - Mute */ > +static const struct snd_kcontrol_new dapm_ear_mute[] = { > + SOC_DAPM_SINGLE("Playback Switch", REG_MUTECONF, REG_MUTECONF_MUTEAR, 1, 1), > +}; This looks like it's just a mute rather than a mixer input enable (there's only one control...) so should be a regular sound control; unless there's a very good reason mutes other than mixer inputs normally don't affect the audio path as you might get pops and you probably don't want to do things like stop clocking just because the output is silenced. Similar thing applies in several other places. > +/* Earpiece source selector */ > +static const char * const enum_ear_lineout_source[] = {"Headset Left", "IHF Left"}; > +static SOC_ENUM_SINGLE_DECL(dapm_enum_ear_lineout_source, REG_DMICFILTCONF, > + REG_DMICFILTCONF_DA3TOEAR, enum_ear_lineout_source); > +static const struct snd_kcontrol_new dapm_ear_lineout_source = > + SOC_DAPM_ENUM("Earpiece or LineOut Mono Source", dapm_enum_ear_lineout_source); This control can probably have a much better name... > +/* IHF - Enable/Disable */ > +static const struct soc_enum enum_ihf_left = SOC_ENUM_SINGLE(0, 0, 2, enum_dis_ena); 80 columns please. > +/* from -31 to 31 dB in 1 dB steps (mute instead of -32 dB) */ > +static DECLARE_TLV_DB_SCALE(adx_dig_gain_tlv, -3200, 100, 1); I'm really not a big fan of this sort of comment, adds greatly to the verbosity but just duplicates the code. > +static const char * const enum_earselcm[] = {"0.95V", "1.10V", "1.27V", "1.58V"}; > +static SOC_ENUM_SINGLE_DECL(soc_enum_earselcm, > + REG_ANACONF1, REG_ANACONF1_EARSELCM, enum_earselcm); Should this really be configured by the application layer and not platform data? > +/* Digital interface - DA from slot mapping */ > +static const char * const enum_da_from_slot_map[] = {"SLOT0", > + "SLOT1", Indentation. Also are you sure that this isn't DAPM stuff? > + SOC_ENUM("Earpiece DAC Low Power Playback Switch", > + soc_enum_eardaclowpow), These aren't switches, they're enums. Switches are strictly boolean things with values 0 and 1. > + snd_soc_update_bits(codec, > + REG_DIGIFCONF3, > + BMASK(REG_DIGIFCONF3_IF1MASTER), > + BMASK(REG_DIGIFCONF3_IF1MASTER)); You're unconditionally setting these bits? > + snd_soc_update_bits(codec, REG_DIGIFCONF4, clr_mask, set_mask); Either clr_mask and set_mask are misnamed or this doesn't do what you think it does. The bits specified by the third argument are set to the values given in the fourth argument. It looks like the code thinks that the bits in the third argument will be cleared and the bits in the fourth argument will be set. > +static int ab8500_codec_set_word_length_if1(struct snd_soc_codec *codec, unsigned int wl) > +{ Just inline stuff like this. > +static int ab8500_codec_set_bit_delay_if1(struct snd_soc_codec *codec, unsigned int delay) This too. > +/* Configures audio macrocell into the AB8500 Chip */ > +static int ab8500_codec_configure_audio_macrocell(struct snd_soc_codec *codec) > +{ > + u8 value8; > + unsigned int value; > + int status; > + > + status = ab8500_sysctrl_write(AB8500_STW4500CTRL3, > + AB8500_STW4500CTRL3_CLK32KOUT2DIS | AB8500_STW4500CTRL3_RESETAUDN, > + AB8500_STW4500CTRL3_RESETAUDN); > + if (status < 0) > + return status; You're just unconditionally setting these values - shouldn't they be platform data? > +static int ab8500_codec_add_widgets(struct snd_soc_codec *codec) > +{ > + int status; > + > + status = snd_soc_dapm_new_controls(&codec->dapm, ab8500_dapm_widgets, > + ARRAY_SIZE(ab8500_dapm_widgets)); Just point at the tables from the snd_soc_codec and remove all this code. > +static int ab8500_codec_pcm_hw_params(struct snd_pcm_substream *substream, > + struct snd_pcm_hw_params *hw_params, struct snd_soc_dai *dai) > +{ > + dev_dbg(dai->codec->dev, "%s Enter.\n", __func__); > + > + return 0; > +} Remove all empty functions. > + > +struct snd_soc_dai_driver ab8500_codec_dai[] = { > + { > + .name = "ab8500-codec-dai.0", > + .id = 0, > + .playback = { > + .stream_name = "ab8500_0p", You should hook up the links to the widgets using DAPM routes, currently you're using the implicitly created routes from the stream name which is still supported but isn't best practice. > +static int ab8500_codec_remove(struct snd_soc_codec *codec) > +{ > + dev_dbg(codec->dev, "%s Enter.\n", __func__); > + > + snd_soc_dapm_free(&codec->dapm); Why are you doing this? > + /* Create register cache */ > + ab8500_codec_driver.reg_cache_size = AB8500_LAST_REG - AB8500_FIRST_REG; > + ab8500_codec_driver.reg_cache_default = > + kzalloc(ab8500_codec_driver.reg_cache_size * sizeof(u8), GFP_KERNEL); The driver struct should be constant, the stuff you're initialising it with here is compile time constant - why are you doing this dynamically at runtime/ > + > + /* Create driver private-data struct */ > + drvdata = kzalloc(sizeof(struct ab8500_codec_drvdata), GFP_KERNEL); Anything you do alloc should be devm_kzalloc(). > + struct ab8500_codec_drvdata *drvdata = dev_get_drvdata(&pdev->dev); > + > + dev_info(&pdev->dev, "%s Enter.\n", __func__); > + > + kfree(ab8500_codec_driver.reg_cache_default); > + > + kfree(drvdata->virt_reg_cache); > + kfree(drvdata); > + > + snd_soc_unregister_codec(&pdev->dev); This would avoid issues like this where you free the data the device uses before you free the device. > +static int __devinit ab8500_codec_platform_driver_init(void) module_platform_driver. > +/* Extended interface for codec-driver */ > + > +int ab8500_audio_power_control(struct snd_soc_codec *codec, bool power_on); > +int ab8500_audio_set_word_length(struct snd_soc_dai *dai, unsigned int wl); > +int ab8500_audio_set_bit_delay(struct snd_soc_dai *dai, unsigned int delay); > +int ab8500_audio_setup_if1(struct snd_soc_codec *codec, > + unsigned int fmt, > + unsigned int wl, > + unsigned int delay); > +void ab8500_audio_anc_configure(struct snd_soc_codec *codec, > + bool apply_fir, bool apply_iir); No, you shouldn't be exporting this stuff - it all looks like basic framework stuff. > +/* Virtual register base > + * This address should be located above any physical register used by ASoC. > + * It is used to map virtual registers needed by the codec-driver. > + */ > +#define VIRT_REG_BASE 0x3000 Why are you doing this virtual register stuff? > +#define REG_DAPATHCONF_ENDACHSL 5 All these constants need to be driver local or namespaced (preferrably namespaced whatever).

4 11

[alsa-devel] [PATCH] aplay/arecord: option to treat any xrun as fatal
by Ben Gardiner 19 Mar '12

19 Mar '12

Signed-off-by: Ben Gardiner <bengardiner(a)nanometrics.ca> --- aplay/aplay.1 | 4 ++++ aplay/aplay.c | 17 +++++++++++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/aplay/aplay.1 b/aplay/aplay.1 index 0195322..bf9b53e 100644 --- a/aplay/aplay.1 +++ b/aplay/aplay.1 @@ -191,6 +191,10 @@ lists capabilities of the selected device such as supported formats, sampling rates, numbers of channels, period and buffer bytes/sizes/times. For raw device hw:X this option basically lists hardware capabilities of the soundcard. +.TP +\fI\-\-fatal\-errors\fP +Disables recovery attempts when errors (e.g. xrun) are encountered; the +aplay process instead aborts immediately. .SH SIGNALS When recording, SIGINT, SIGTERM and SIGABRT will close the output diff --git a/aplay/aplay.c b/aplay/aplay.c index 0633cbd..1ce34c4 100644 --- a/aplay/aplay.c +++ b/aplay/aplay.c @@ -115,6 +115,7 @@ static int stop_delay = 0; static int monotonic = 0; static int interactive = 0; static int can_pause = 0; +static int fatal_errors = 0; static int verbose = 0; static int vumeter = VUMETER_NONE; static int buffer_pos = 0; @@ -225,7 +226,8 @@ _("Usage: %s [OPTION]... [FILE]...\n" " for this many seconds\n" " --process-id-file write the process ID here\n" " --use-strftime apply the strftime facility to the output file name\n" -" --dump-hw-params dump hw_params of the device\n") +" --dump-hw-params dump hw_params of the device\n" +" --fatal-errors treat all errors as fatal\n") , command); printf(_("Recognized sample formats are:")); for (k = 0; k < SND_PCM_FORMAT_LAST; ++k) { @@ -419,7 +421,8 @@ enum { OPT_MAX_FILE_TIME, OPT_PROCESS_ID_FILE, OPT_USE_STRFTIME, - OPT_DUMP_HWPARAMS + OPT_DUMP_HWPARAMS, + OPT_FATAL_ERRORS, }; int main(int argc, char *argv[]) @@ -465,6 +468,7 @@ int main(int argc, char *argv[]) {"use-strftime", 0, 0, OPT_USE_STRFTIME}, {"interactive", 0, 0, 'i'}, {"dump-hw-params", 0, 0, OPT_DUMP_HWPARAMS}, + {"fatal-errors", 0, 0, OPT_FATAL_ERRORS}, {0, 0, 0, 0} }; char *pcm_name = "default"; @@ -669,6 +673,9 @@ int main(int argc, char *argv[]) case OPT_DUMP_HWPARAMS: dump_hw_params = 1; break; + case OPT_FATAL_ERRORS: + fatal_errors = 1; + break; default: fprintf(stderr, _("Try `%s --help' for more information.\n"), command); return 1; @@ -1350,6 +1357,12 @@ static void xrun(void) prg_exit(EXIT_FAILURE); } if (snd_pcm_status_get_state(status) == SND_PCM_STATE_XRUN) { + if (fatal_errors) { + error(_("fatal %s: %s"), + stream == SND_PCM_STREAM_PLAYBACK ? _("underrun") : _("overrun"), + snd_strerror(res)); + prg_exit(EXIT_FAILURE); + } if (monotonic) { #ifdef HAVE_CLOCK_GETTIME struct timespec now, diff, tstamp; -- 1.7.4.1

2 1

[alsa-devel] [PATCH] ALSA: ASoC: pxa-ssp: atomically set stream active masks
by Daniel Mack 19 Mar '12

19 Mar '12

PXA's SSP engine fails to take its current channel phase into account when enabling a stream while the engine is already running. This results in randomly swapped left/right channels on either the record or the playback side, depending on which one was enabled first. The following patch fixes this by factoring out the bit field modifications in question to a separate function that pauses the engine temporarily, modifies the bits and kicks it off again afterwards. Appearantly, a transition of SSCR0_SSE syncs both directions properly. The patch has been rolled out to quite a number of devices over the last weeks and seems to fix the issue reliably. Signed-off-by: Daniel Mack <zonque(a)gmail.com> Reported-and-tested-by: Sven Neumann <s.neumann(a)raumfeld.com> Cc: Eric Miao <eric.y.miao(a)gmail.com> Cc: Haojian Zhuang <haojian.zhuang(a)marvell.com> Cc: Liam Girdwood <lrg(a)ti.com> Cc: Mark Brown <broonie(a)opensource.wolfsonmicro.com> Cc: Johannes Stezenbach <js(a)sig21.net> Cc: stable(a)kernel.org --- sound/soc/pxa/pxa-ssp.c | 61 +++++++++++++++++++++++++++------------------- 1 files changed, 36 insertions(+), 25 deletions(-) diff --git a/sound/soc/pxa/pxa-ssp.c b/sound/soc/pxa/pxa-ssp.c index a16df0f..fd04ce1 100644 --- a/sound/soc/pxa/pxa-ssp.c +++ b/sound/soc/pxa/pxa-ssp.c @@ -668,6 +668,38 @@ static int pxa_ssp_hw_params(struct snd_pcm_substream *substream, return 0; } +static void pxa_ssp_set_running_bit(struct snd_pcm_substream *substream, + struct ssp_device *ssp, int value) +{ + uint32_t sscr0 = pxa_ssp_read_reg(ssp, SSCR0); + uint32_t sscr1 = pxa_ssp_read_reg(ssp, SSCR1); + uint32_t sspsp = pxa_ssp_read_reg(ssp, SSPSP); + uint32_t sssr = pxa_ssp_read_reg(ssp, SSSR); + + if (value && (sscr0 & SSCR0_SSE)) + pxa_ssp_write_reg(ssp, SSCR0, sscr0 & ~SSCR0_SSE); + + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { + if (value) + sscr1 |= SSCR1_TSRE; + else + sscr1 &= ~SSCR1_TSRE; + } else { + if (value) + sscr1 |= SSCR1_RSRE; + else + sscr1 &= ~SSCR1_RSRE; + } + + pxa_ssp_write_reg(ssp, SSCR1, sscr1); + + if (value) { + pxa_ssp_write_reg(ssp, SSSR, sssr); + pxa_ssp_write_reg(ssp, SSPSP, sspsp); + pxa_ssp_write_reg(ssp, SSCR0, sscr0 | SSCR0_SSE); + } +} + static int pxa_ssp_trigger(struct snd_pcm_substream *substream, int cmd, struct snd_soc_dai *cpu_dai) { @@ -681,42 +713,21 @@ static int pxa_ssp_trigger(struct snd_pcm_substream *substream, int cmd, pxa_ssp_enable(ssp); break; case SNDRV_PCM_TRIGGER_PAUSE_RELEASE: - val = pxa_ssp_read_reg(ssp, SSCR1); - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - val |= SSCR1_TSRE; - else - val |= SSCR1_RSRE; - pxa_ssp_write_reg(ssp, SSCR1, val); + pxa_ssp_set_running_bit(substream, ssp, 1); val = pxa_ssp_read_reg(ssp, SSSR); pxa_ssp_write_reg(ssp, SSSR, val); break; case SNDRV_PCM_TRIGGER_START: - val = pxa_ssp_read_reg(ssp, SSCR1); - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - val |= SSCR1_TSRE; - else - val |= SSCR1_RSRE; - pxa_ssp_write_reg(ssp, SSCR1, val); - pxa_ssp_enable(ssp); + pxa_ssp_set_running_bit(substream, ssp, 1); break; case SNDRV_PCM_TRIGGER_STOP: - val = pxa_ssp_read_reg(ssp, SSCR1); - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - val &= ~SSCR1_TSRE; - else - val &= ~SSCR1_RSRE; - pxa_ssp_write_reg(ssp, SSCR1, val); + pxa_ssp_set_running_bit(substream, ssp, 0); break; case SNDRV_PCM_TRIGGER_SUSPEND: pxa_ssp_disable(ssp); break; case SNDRV_PCM_TRIGGER_PAUSE_PUSH: - val = pxa_ssp_read_reg(ssp, SSCR1); - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - val &= ~SSCR1_TSRE; - else - val &= ~SSCR1_RSRE; - pxa_ssp_write_reg(ssp, SSCR1, val); + pxa_ssp_set_running_bit(substream, ssp, 0); break; default: -- 1.7.7.6

2 1

[alsa-devel] static /dev configuration
by Bob Tracy 19 Mar '12

19 Mar '12

Does ALSA now require dynamic devices (udev) for certain soundcards (specifically, cards handled by the snd-hda-intel driver)? My problem is that I moved a Slackware installation to new hardware, and the old devices under /dev/snd I originally created back in 2003 don't seem to work correctly with snd-hda-intel. "lspci" shows my new system having two audio devices as follows: 00:1b.0 Audio device: Intel Corporation Cougar Point High Definition Audio Controller (rev 04) 01:00.1 Audio device: ATI Technologies Inc Device aa60 (The ATI entry corresponds to a DisplayPort output on a Radeon graphics card that isn't presently of interest to me: REDWOOD firmware.) I had an opportunity to boot a modern Linux distro with udev support that accesses the Intel audio device just fine, so I took note of the device entries under /dev/snd and replicated those on the Slackware setup as follows (ignore the "root" group and 666 permissions rather than the more correct "audio" group and 660 permissions): crw-rw-rw- 1 root root 116, 9 Mar 18 00:24 controlC0 crw-rw-rw- 1 root root 116, 12 Mar 18 00:26 controlC1 crw-rw-rw- 1 root root 116, 8 Mar 18 00:27 hwC0D2 crw-rw-rw- 1 root root 116, 7 Jun 18 2003 hwC0D3 crw-rw-rw- 1 root root 116, 11 Mar 18 00:30 hwC1D0 crw-rw-rw- 1 root root 116, 6 Mar 18 00:34 pcmC0D0c crw-rw-rw- 1 root root 116, 5 Mar 18 20:44 pcmC0D0p crw-rw-rw- 1 root root 116, 4 Mar 18 00:36 pcmC0D1p crw-rw-rw- 1 root root 116, 3 Mar 18 00:39 pcmC0D2c crw-rw-rw- 1 root root 116, 2 Mar 18 00:44 pcmC0D3p crw-rw-rw- 1 root root 116, 10 Mar 18 00:42 pcmC1D3p crw-rw-rw- 1 root root 116, 1 Jun 18 2003 seq crw-rw-rw- 1 root root 116, 33 Jun 18 2003 timer Not all that surprisingly, the above entries work: "alsamixer" and "aplay" do what I expect and seem to work correctly. Note that the minor device numbers are NOT the same as what the "snddevices" script in "alsa-driver-1.0.25" creates (and that script creates pretty much exactly what I had in my static /dev tree back in 2003). According to "snddevices", the "controlCn" devices should have minor numbers 0, 32, 64, etc. The "hwC0Dn" minors are 4, 5, etc., "hwC1Dn" minors are 36, 37, etc., "hwC2Dn" minors are 68, 69, etc. Note that "hwC0D3" is at minor 7 where it should be under the old minor device numbering scheme, but "hwC0D2" is at minor 8 where 6 was expected. The various "pcm*" devices were expected to be as follows: crw-rw-rw- 1 root root 116, 24 Jun 18 2003 pcmC0D0c crw-rw-rw- 1 root root 116, 16 Mar 3 20:50 pcmC0D0p crw-rw-rw- 1 root root 116, 25 Jun 18 2003 pcmC0D1c crw-rw-rw- 1 root root 116, 17 Jun 18 2003 pcmC0D1p crw-rw-rw- 1 root root 116, 26 Jun 18 2003 pcmC0D2c crw-rw-rw- 1 root root 116, 18 Jun 18 2003 pcmC0D2p crw-rw-rw- 1 root root 116, 27 Jun 18 2003 pcmC0D3c crw-rw-rw- 1 root root 116, 19 Jun 18 2003 pcmC0D3p crw-rw-rw- 1 root root 116, 56 Jun 18 2003 pcmC1D0c crw-rw-rw- 1 root root 116, 48 Jun 18 2003 pcmC1D0p crw-rw-rw- 1 root root 116, 57 Jun 18 2003 pcmC1D1c crw-rw-rw- 1 root root 116, 49 Jun 18 2003 pcmC1D1p crw-rw-rw- 1 root root 116, 58 Jun 18 2003 pcmC1D2c crw-rw-rw- 1 root root 116, 50 Jun 18 2003 pcmC1D2p crw-rw-rw- 1 root root 116, 59 Jun 18 2003 pcmC1D3c crw-rw-rw- 1 root root 116, 51 Jun 18 2003 pcmC1D3p I'm thinking the only reason I'm getting away with using any kind of static device scheme is that my sound devices are getting detected and initialized in a consistent order each time I boot, and the /dev/snd entries I copied from the udev-enabled system just happen to agree with what "udev" would consistently create if I had "udev" installed. So, just to restate the original question for clarity, is "udev" required for certain soundcards? Thanks! --Bob

2 2

[alsa-devel] [PATCH] ASoC: Add MICBIAS routing information for WM8904
by Hong Xu 19 Mar '12

19 Mar '12

Add MICBIAS routing entries for WM8904. Tested on ATMEL's AT91SAM9N12-EK with a electret condenser microphone connected to IN1L. Signed-off-by: Hong Xu <hong.xu(a)atmel.com> --- sound/soc/codecs/wm8904.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/sound/soc/codecs/wm8904.c b/sound/soc/codecs/wm8904.c index 285ef87..2158f30 100644 --- a/sound/soc/codecs/wm8904.c +++ b/sound/soc/codecs/wm8904.c @@ -1318,6 +1318,14 @@ static const struct snd_soc_dapm_route core_intercon[] = { }; static const struct snd_soc_dapm_route adc_intercon[] = { + { "MICBIAS", NULL, "IN1L" }, + { "MICBIAS", NULL, "IN1R" }, + + { "Left Capture Mux", NULL, "MICBIAS" }, + { "Right Capture Mux", NULL, "MICBIAS" }, + { "Left Capture Inverting Mux", NULL, "MICBIAS" }, + { "Right Capture Inverting Mux", NULL, "MICBIAS" }, + { "Left Capture Mux", "IN1L", "IN1L" }, { "Left Capture Mux", "IN2L", "IN2L" }, { "Left Capture Mux", "IN3L", "IN3L" }, -- 1.7.6

3 2

Re: [alsa-devel] ASoC:Question rate constraint between the dais
by Omair Mohammed Abdullah 18 Mar '12

18 Mar '12

>> I need to make sure that all simultaneous playback and capture (across >> the all dai_links) needs to be of same frequency family. >> If the sampling frequency of a new request is different from ongoing >> playback/capture, then it needs to be rejected. > >> How do I handle it in machine driver? > > Why do you need to do this in the machine driver? Such a constraint sounds like it's coming from one of the chips in the system rather than from the machine itself so it'd seem sensible for the chip itself to impose the constraint so that other machines don't need to replicate the code. > > See wm8988 for an example of adding sample rate constraints, obviously you can change the criteria dynamically. You are right to say that it is not a machine constraint but a codec constraint. The problem is that if a DAI (say the "SN95031 Headset DAI") is opened with a 44.1 khz rate, then another DAI (say "SN95031 Voice") is opened with a 48 khz rate, then we need to reject the second open because the codec cannot support both rates at the same time. Is there any way to do this with constraint_lists? The way it is done in the wm8988 driver does not apply here because there it is a single DAI.

4 7

[alsa-devel] [PATCH 1/1] a52: set time_base to avoid crash in avcodec
by Alex Hornung 18 Mar '12

18 Mar '12

* Not setting the avctx->time_base properly results in a crash in avcodec_encode_audio() in more recent versions of it. * Fix it by setting the time_base according to the sampling rate. Signed-off-by: Alex Hornung <alex(a)alexhornung.com> --- a52/pcm_a52.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/a52/pcm_a52.c b/a52/pcm_a52.c index 00c7c59..560071d 100644 --- a/a52/pcm_a52.c +++ b/a52/pcm_a52.c @@ -436,6 +436,8 @@ static int a52_prepare(snd_pcm_ioplug_t *io) rec->avctx->bit_rate = rec->bitrate * 1000; rec->avctx->sample_rate = io->rate; rec->avctx->channels = io->channels; + rec->avctx->time_base.num = 1; + rec->avctx->time_base.den = io->rate; #if LIBAVCODEC_VERSION_MAJOR > 52 || (LIBAVCODEC_VERSION_MAJOR == 52 && LIBAVCODEC_VERSION_MINOR >= 95) rec->avctx->sample_fmt = AV_SAMPLE_FMT_S16; #else -- 1.7.9.1

1 0

[alsa-devel] [PATCH v2] ASoC: max98095: add jack detection
by Rhyland Klein 17 Mar '12

17 Mar '12

This change adds the logic to support using the jack detect mechanism built in to the codec to detect both when a jack was inserted and what type of jack is present. This change also supports the use of an external mechanism for headphone detection. If this mechanism exists, when the max98095_jack_detect function is called, the hp_jack is simply passed NULL. This change supports both simple headphones, powered headphones, microphones and headsets with both headphones and a mic. v2: - Removed useless function for irq_handler - removed enable/disable irq - Added pdata entry for jack detections slew rate - get irq directly from dev Signed-off-by: Rhyland Klein <rklein(a)nvidia.com> --- include/sound/max98095.h | 12 +++ sound/soc/codecs/max98095.c | 158 ++++++++++++++++++++++++++++++++++++++++++- sound/soc/codecs/max98095.h | 22 ++++++ 3 files changed, 191 insertions(+), 1 deletions(-) diff --git a/include/sound/max98095.h b/include/sound/max98095.h index 7513a42..65b1c90 100644 --- a/include/sound/max98095.h +++ b/include/sound/max98095.h @@ -49,6 +49,18 @@ struct max98095_pdata { */ unsigned int digmic_left_mode:1; unsigned int digmic_right_mode:1; + + /* Pin5 is the mechanical method of sensing jack insertion + * but it is something that might not be supported. + * 0 = PIN5 not supported + * 1 = PIN5 supported + */ + int jack_detect_pin5en:1; + + /* Slew amount for jack detection. Calculated as 4 * (delay + 1). + * Default delay is 24 to get a time of 100ms. + */ + unsigned int jack_detect_delay; }; #endif diff --git a/sound/soc/codecs/max98095.c b/sound/soc/codecs/max98095.c index 0bb511a..0752840 100644 --- a/sound/soc/codecs/max98095.c +++ b/sound/soc/codecs/max98095.c @@ -24,6 +24,7 @@ #include <linux/slab.h> #include <asm/div64.h> #include <sound/max98095.h> +#include <sound/jack.h> #include "max98095.h" enum max98095_type { @@ -51,6 +52,8 @@ struct max98095_priv { u8 lin_state; unsigned int mic1pre; unsigned int mic2pre; + struct snd_soc_jack *headphone_jack; + struct snd_soc_jack *mic_jack; }; static const u8 max98095_reg_def[M98095_REG_CNT] = { @@ -2173,9 +2176,125 @@ static void max98095_handle_pdata(struct snd_soc_codec *codec) max98095_handle_bq_pdata(codec); } +static irqreturn_t max98095_report_jack(int irq, void *data) +{ + struct snd_soc_codec *codec = data; + struct max98095_priv *max98095 = snd_soc_codec_get_drvdata(codec); + unsigned int value; + int hp_report = 0; + int mic_report = 0; + + /* Read the Jack Status Register */ + value = snd_soc_read(codec, M98095_007_JACK_AUTO_STS); + + /* If ddone is not set, then detection isn't finished yet */ + if ((value & M98095_DDONE) == 0) + return IRQ_NONE; + + /* if hp, check its bit, and if set, clear it */ + if ((value & M98095_HP_IN || value & M98095_LO_IN) && + max98095->headphone_jack) + hp_report |= SND_JACK_HEADPHONE; + + /* if mic, check its bit, and if set, clear it */ + if ((value & M98095_MIC_IN) && max98095->mic_jack) + mic_report |= SND_JACK_MICROPHONE; + + if (max98095->headphone_jack == max98095->mic_jack) { + snd_soc_jack_report(max98095->headphone_jack, + hp_report | mic_report, + SND_JACK_HEADSET); + } else { + if (max98095->headphone_jack) + snd_soc_jack_report(max98095->headphone_jack, + hp_report, SND_JACK_HEADPHONE); + if (max98095->mic_jack) + snd_soc_jack_report(max98095->mic_jack, + mic_report, SND_JACK_MICROPHONE); + } + + return IRQ_HANDLED; +} + +int max98095_jack_detect_enable(struct snd_soc_codec *codec) +{ + struct max98095_priv *max98095 = snd_soc_codec_get_drvdata(codec); + int ret = 0; + int detect_enable = M98095_JDEN; + unsigned int slew = M98095_DEFAULT_SLEW_DELAY; + + if (max98095->pdata->jack_detect_pin5en) + detect_enable |= M98095_PIN5EN; + + if (max98095->jack_detect_delay) + slew = max98095->jack_detect_delay; + + ret = snd_soc_write(codec, M98095_08E_JACK_DC_SLEW, slew); + if (ret < 0) { + dev_err(codec->dev, "Failed to cfg auto detect %d\n", ret); + return ret; + } + + /* configure auto detection to be enabled */ + ret = snd_soc_write(codec, M98095_089_JACK_DET_AUTO, detect_enable); + if (ret < 0) { + dev_err(codec->dev, "Failed to cfg auto detect %d\n", ret); + return ret; + } + + return ret; +} + +int max98095_jack_detect_disable(struct snd_soc_codec *codec) +{ + int ret = 0; + + /* configure auto detection to be disabled */ + ret = snd_soc_write(codec, M98095_089_JACK_DET_AUTO, 0x0); + if (ret < 0) { + dev_err(codec->dev, "Failed to cfg auto detect %d\n", ret); + return ret; + } + + return ret; +} + +int max98095_jack_detect(struct snd_soc_codec *codec, + struct snd_soc_jack *hp_jack, struct snd_soc_jack *mic_jack) +{ + struct max98095_priv *max98095 = snd_soc_codec_get_drvdata(codec); + struct i2c_client *client = to_i2c_client(codec->dev); + int ret = 0; + + max98095->headphone_jack = hp_jack; + max98095->mic_jack = mic_jack; + + /* only progress if we have at least 1 jack pointer */ + if (!hp_jack && !mic_jack) + return -EINVAL; + + max98095_jack_detect_enable(codec); + + /* enable interrupts for headphone jack detection */ + ret = snd_soc_update_bits(codec, M98095_013_JACK_INT_EN, + M98095_IDDONE, M98095_IDDONE); + if (ret < 0) { + dev_err(codec->dev, "Failed to cfg jack irqs %d\n", ret); + return ret; + } + + max98095_report_jack(client->irq, codec); + return 0; +} + #ifdef CONFIG_PM static int max98095_suspend(struct snd_soc_codec *codec) { + struct max98095_priv *max98095 = snd_soc_codec_get_drvdata(codec); + + if (max98095->headphone_jack || max98095->mic_jack) + max98095_jack_detect_disable(codec); + max98095_set_bias_level(codec, SND_SOC_BIAS_OFF); return 0; @@ -2183,8 +2302,16 @@ static int max98095_suspend(struct snd_soc_codec *codec) static int max98095_resume(struct snd_soc_codec *codec) { + struct max98095_priv *max98095 = snd_soc_codec_get_drvdata(codec); + struct i2c_client *client = to_i2c_client(codec->dev); + max98095_set_bias_level(codec, SND_SOC_BIAS_STANDBY); + if (max98095->headphone_jack || max98095->mic_jack) { + max98095_jack_detect_enable(codec); + max98095_report_jack(client->irq, codec); + } + return 0; } #else @@ -2227,6 +2354,7 @@ static int max98095_probe(struct snd_soc_codec *codec) { struct max98095_priv *max98095 = snd_soc_codec_get_drvdata(codec); struct max98095_cdata *cdata; + struct i2c_client *client; int ret = 0; ret = snd_soc_codec_set_cache_io(codec, 8, 8, SND_SOC_I2C); @@ -2238,6 +2366,8 @@ static int max98095_probe(struct snd_soc_codec *codec) /* reset the codec, the DSP core, and disable all interrupts */ max98095_reset(codec); + client = to_i2c_client(codec->dev); + /* initialize private data */ max98095->sysclk = (unsigned)-1; @@ -2266,11 +2396,23 @@ static int max98095_probe(struct snd_soc_codec *codec) max98095->mic1pre = 0; max98095->mic2pre = 0; + if (client->irq) { + /* register an audio interrupt */ + ret = request_threaded_irq(client->irq, NULL, + max98095_report_jack, + IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING, + "max98095", codec); + if (ret) { + dev_err(codec->dev, "Failed to request IRQ: %d\n", ret); + goto err_access; + } + } + ret = snd_soc_read(codec, M98095_0FF_REV_ID); if (ret < 0) { dev_err(codec->dev, "Failure reading hardware revision: %d\n", ret); - goto err_access; + goto err_irq; } dev_info(codec->dev, "Hardware revision: %c\n", ret - 0x40 + 'A'); @@ -2306,14 +2448,28 @@ static int max98095_probe(struct snd_soc_codec *codec) max98095_add_widgets(codec); + return 0; + +err_irq: + if (client->irq) + free_irq(client->irq, codec); err_access: return ret; } static int max98095_remove(struct snd_soc_codec *codec) { + struct max98095_priv *max98095 = snd_soc_codec_get_drvdata(codec); + struct i2c_client *client = to_i2c_client(codec->dev); + max98095_set_bias_level(codec, SND_SOC_BIAS_OFF); + if (max98095->headphone_jack || max98095->mic_jack) + max98095_jack_detect_disable(codec); + + if (client->irq) + free_irq(client->irq, codec); + return 0; } diff --git a/sound/soc/codecs/max98095.h b/sound/soc/codecs/max98095.h index 891584a..2ebbe4e 100644 --- a/sound/soc/codecs/max98095.h +++ b/sound/soc/codecs/max98095.h @@ -175,11 +175,23 @@ /* MAX98095 Registers Bit Fields */ +/* M98095_007_JACK_AUTO_STS */ + #define M98095_MIC_IN (1<<3) + #define M98095_LO_IN (1<<5) + #define M98095_HP_IN (1<<6) + #define M98095_DDONE (1<<7) + /* M98095_00F_HOST_CFG */ #define M98095_SEG (1<<0) #define M98095_XTEN (1<<1) #define M98095_MDLLEN (1<<2) +/* M98095_013_JACK_INT_EN */ + #define M98095_IMIC_IN (1<<3) + #define M98095_ILO_IN (1<<5) + #define M98095_IHP_IN (1<<6) + #define M98095_IDDONE (1<<7) + /* M98095_027_DAI1_CLKMODE, M98095_031_DAI2_CLKMODE, M98095_03B_DAI3_CLKMODE */ #define M98095_CLKMODE_MASK 0xFF @@ -255,6 +267,10 @@ #define M98095_EQ2EN (1<<1) #define M98095_EQ1EN (1<<0) +/* M98095_089_JACK_DET_AUTO */ + #define M98095_PIN5EN (1<<2) + #define M98095_JDEN (1<<7) + /* M98095_090_PWR_EN_IN */ #define M98095_INEN (1<<7) #define M98095_MB2EN (1<<3) @@ -296,4 +312,10 @@ #define M98095_174_DAI1_BQ_BASE 0x74 #define M98095_17E_DAI2_BQ_BASE 0x7E +/* Default Delay used in Slew Rate Calculation for Jack detection */ +#define M98095_DEFAULT_SLEW_DELAY 0x18 + +extern int max98095_jack_detect(struct snd_soc_codec *codec, + struct snd_soc_jack *hp_jack, struct snd_soc_jack *mic_jack); + #endif -- 1.7.0.4

2 1

[alsa-devel] [PATCH] ASoC: pxa2xx-i2s: Add clk_prepare/clk_unprepare calls
by Philipp Zabel 17 Mar '12

17 Mar '12

This patch adds clk_prepare/clk_unprepare calls to the pxa2xx-i2s driver by using the helper functions clk_prepare_enable and clk_disable_unprepare. Signed-off-by: Philipp Zabel <philipp.zabel(a)gmail.com> Cc: Haojian Zhuang <haojian.zhuang(a)marvell.com> Cc: Eric Miao <eric.y.miao(a)gmail.com> Cc: Liam Girdwood <lrg(a)ti.com> Cc: Mark Brown <broonie(a)opensource.wolfsonmicro.com> --- sound/soc/pxa/pxa2xx-i2s.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sound/soc/pxa/pxa2xx-i2s.c b/sound/soc/pxa/pxa2xx-i2s.c index 609abd5..453b092 100644 --- a/sound/soc/pxa/pxa2xx-i2s.c +++ b/sound/soc/pxa/pxa2xx-i2s.c @@ -165,7 +165,7 @@ static int pxa2xx_i2s_hw_params(struct snd_pcm_substream *substream, struct pxa2xx_pcm_dma_params *dma_data; BUG_ON(IS_ERR(clk_i2s)); - clk_enable(clk_i2s); + clk_prepare_enable(clk_i2s); clk_ena = 1; pxa_i2s_wait(); @@ -258,7 +258,7 @@ static void pxa2xx_i2s_shutdown(struct snd_pcm_substream *substream, SACR0 &= ~SACR0_ENB; pxa_i2s_wait(); if (clk_ena) { - clk_disable(clk_i2s); + clk_disable_unprepare(clk_i2s); clk_ena = 0; } } -- 1.7.9.1

2 1

[alsa-devel] [PATCH v3 00/11] ASoC: add imx-sgtl5000 machine driver working with fsl_ssi
by Shawn Guo 17 Mar '12

17 Mar '12

I believe I have addressed all the outstanding comments and requests reviewers put on previous versions. So, hopefully this will be the last iteration of the series. The current sound/for-next branch seems broken for me, and I have not tracking the problem down. But the series works good on SHA below. 3030763 (Merge tag 'asoc-3.4' of git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound into topic/asoc) Timur, I did not pick up you Ack-by tag, because there are some changes since v2 you will need to take another look and have the testing, I guess. Regards, Shawn Shawn Guo (11): ASoC: core: missing set_fmt should not be complaint ASoC: fsl: separate SSI and DMA Kconfig options ASoC: imx: merge sound/soc/imx into sound/soc/fsl ASoC: fsl: rename imx-pcm Kconfig options and filename ASoC: fsl: create fsl_utils to accommodate the common functions ASoC: fsl: remove helper fsl_asoc_get_codec_dev_name ASoC: fsl: check property 'compatible' for the machine name ASoC: fsl: make fsl_ssi driver compilable on ARM/IMX ASoC: fsl: remove the fatal error checking on codec-handle ASoC: fsl: let fsl_ssi work with imx pcm and machine drivers ASoC: fsl: add imx-sgtl5000 machine driver .../bindings/sound/imx-audio-sgtl5000.txt | 24 +++ arch/powerpc/configs/86xx/mpc8610_hpcd_defconfig | 1 + arch/powerpc/configs/mpc85xx_defconfig | 1 + arch/powerpc/configs/mpc85xx_smp_defconfig | 1 + sound/soc/Kconfig | 1 - sound/soc/Makefile | 1 - sound/soc/fsl/Kconfig | 124 ++++++++++++- sound/soc/fsl/Makefile | 29 +++- sound/soc/{imx => fsl}/eukrea-tlv320.c | 2 +- sound/soc/fsl/fsl_ssi.c | 146 ++++++++++++---- sound/soc/fsl/fsl_utils.c | 91 ++++++++++ sound/soc/fsl/fsl_utils.h | 26 +++ sound/soc/{imx => fsl}/imx-audmux.c | 0 sound/soc/{imx => fsl}/imx-audmux.h | 0 .../{imx/imx-pcm-dma-mx2.c => fsl/imx-pcm-dma.c} | 0 sound/soc/{imx => fsl}/imx-pcm-fiq.c | 0 sound/soc/{imx => fsl}/imx-pcm.c | 0 sound/soc/{imx => fsl}/imx-pcm.h | 0 sound/soc/fsl/imx-sgtl5000.c | 177 ++++++++++++++++++ sound/soc/{imx => fsl}/imx-ssi.c | 2 +- sound/soc/{imx => fsl}/imx-ssi.h | 0 sound/soc/fsl/mpc8610_hpcd.c | 168 ++---------------- sound/soc/{imx => fsl}/mx27vis-aic32x4.c | 0 sound/soc/fsl/p1022_ds.c | 190 ++------------------ sound/soc/{imx => fsl}/phycore-ac97.c | 0 sound/soc/{imx => fsl}/wm1133-ev1.c | 0 sound/soc/imx/Kconfig | 79 -------- sound/soc/imx/Makefile | 22 --- sound/soc/soc-core.c | 11 +- 29 files changed, 616 insertions(+), 480 deletions(-)

7 67