June 2011 - Alsa-devel - mailman.alsa-project.org

[alsa-devel] [PATCH] ALSA - HDA - Add period time constraint to hda-intel
by Raymond Yau 23 Jun '11

23 Jun '11

As there is a 10 seconds schedule_timeout in alsa-kernel The following error occur when aplay/arecord using a period time longer than 10 seconds for for those hda codec which support rate lower than 32000 when prealloc = 4096 ALSA hda_intel.c:1680: azx_pcm_prepare: bufsize=0x1aeb00, format=0x4111 ... ALSA pcm_native.c:1531: playback drain error (DMA or IRQ trouble?) aplay -v -Dhw:1 --period-time=10001000 22050.wav Playing WAVE '22050.wav' : Signed 16 bit Little Endian, Rate 22050 Hz, Stereo Hardware PCM card 1 'HDA Intel' device 0 subdevice 0 Its setup is: stream : PLAYBACK access : RW_INTERLEAVED format : S16_LE subformat : STD channels : 2 rate : 22050 exact rate : 22050 (22050/1) msbits : 16 buffer_size : 441024 period_size : 220512 period_time : 10000544 tstamp_mode : NONE period_step : 1 avail_min : 220512 period_event : 0 start_threshold : 441024 stop_threshold : 441024 silence_threshold: 0 silence_size : 0 boundary : 1806434304 appl_ptr : 0 hw_ptr : 0 ALSA hda_intel.c:1680: azx_pcm_prepare: bufsize=0x9c500, format=0x511 ALSA hda_codec.c:1295: hda_codec_setup_stream: NID=0x8, stream=0x1, channel=0, format=0x511 ALSA pcm_lib.c:1789: capture write error (DMA or IRQ trouble?) arecord -Dhw:1 -v --period-time=10005000 -r 8000 -c2 -f S16_LE /dev/null Recording WAVE '/dev/null' : Signed 16 bit Little Endian, Rate 8000 Hz, Stereo Hardware PCM card 1 'HDA Intel' device 0 subdevice 0 Its setup is: stream : CAPTURE access : RW_INTERLEAVED format : S16_LE subformat : STD channels : 2 rate : 8000 exact rate : 8000 (8000/1) msbits : 16 buffer_size : 160064 period_size : 80032 period_time : 10004000 tstamp_mode : NONE period_step : 1 avail_min : 80032 period_event : 0 start_threshold : 1 stop_threshold : 160064 silence_threshold: 0 silence_size : 0 boundary : 1311244288 appl_ptr : 0 hw_ptr : 0 arecord: pcm_read:1773: read error: Input/output error

2 4

[alsa-devel] ASoC: Sharing Codec on SSC TDM
by Fox, Jason P. 22 Jun '11

22 Jun '11

I have an application where I need an AD193X codec to be shared between 8 audio streams using one SSC channel on an AT91SAM9263. The AD1938 codec as configured in my driver has 8 channels. I need to be able to send audio to the codec from 8 different tasks simultaneously. Each task is to use only one channel. We have the 8 TDM channels working in the driver. The first task can access the codec and play audio just fine. The second task cannot open the device and gets an EBUSY return from snd_pcm_hw_open() because the card is now owned by the first task. However, the first task is only using one of the 8 channels. I want the second task to use the second channel, and so on. I can create an asound.conf that gives access to any one of the 8 channels, but only to one task at a time. I've tried using dshare and dmix, but that swamps the AT91 and there are lots of audio dropouts. There is no need to mix audio between any of the streams, i.e. they are all to be discrete audio outputs. What I am looking for is a way to structure the ASOC interface to allow use of all 8 channels simultaneously. If anyone can point me in a direction, recommend a solution, or tell me where else to look it would be VERY much appreciated. Thank you for any help, Jason Fox GAI-Tronics ******************************************************************* This email and any files transmitted with it are confidential and intended solely for the use of the individual or entity to whom they are addressed. If you have received this email in error please notify the system manager. This footnote also confirms that this email message has been swept for the presence of computer viruses. www.Hubbell.com - Hubbell Incorporated**

1 0

[alsa-devel] [PATCH] ASoC: core - Fix dai usage in pcm_new() platform handlers
by Daniel Mack 22 Jun '11

22 Jun '11

Commit 552d1ef ("ASoC: core - Optimise and refactor pcm_new() to pass only rtd") refactored the ASoC internal core to only pass a pointer to a runtime as argument to individual pcm_new() functions. However, the new code that was added to the handlers to get access to the dai now uses rtd->cpu_dai, while before this patch, rtd->codec_dai was used. This patch fixes all the pcm_new() handlers that were affected. Tested on a PXA platform. Signed-off-by: Daniel Mack <zonque(a)gmail.com> Cc: Liam Girdwood <lrg(a)ti.com> --- sound/soc/atmel/atmel-pcm.c | 2 +- sound/soc/blackfin/bf5xx-ac97-pcm.c | 2 +- sound/soc/blackfin/bf5xx-i2s-pcm.c | 2 +- sound/soc/blackfin/bf5xx-tdm-pcm.c | 2 +- sound/soc/davinci/davinci-pcm.c | 2 +- sound/soc/fsl/fsl_dma.c | 2 +- sound/soc/fsl/mpc5200_dma.c | 2 +- sound/soc/imx/imx-pcm-fiq.c | 2 +- sound/soc/imx/imx-ssi.c | 2 +- sound/soc/jz4740/jz4740-pcm.c | 2 +- sound/soc/kirkwood/kirkwood-dma.c | 2 +- sound/soc/mid-x86/sst_platform.c | 2 +- sound/soc/nuc900/nuc900-pcm.c | 2 +- sound/soc/omap/omap-pcm.c | 2 +- sound/soc/pxa/pxa2xx-pcm.c | 2 +- sound/soc/s6000/s6000-pcm.c | 2 +- sound/soc/samsung/dma.c | 2 +- sound/soc/tegra/tegra_pcm.c | 2 +- sound/soc/txx9/txx9aclc.c | 2 +- 19 files changed, 19 insertions(+), 19 deletions(-) diff --git a/sound/soc/atmel/atmel-pcm.c b/sound/soc/atmel/atmel-pcm.c index f81d4c3..d1140c1 100644 --- a/sound/soc/atmel/atmel-pcm.c +++ b/sound/soc/atmel/atmel-pcm.c @@ -367,7 +367,7 @@ static u64 atmel_pcm_dmamask = 0xffffffff; static int atmel_pcm_new(struct snd_soc_pcm_runtime *rtd) { struct snd_card *card = rtd->card->snd_card; - struct snd_soc_dai *dai = rtd->cpu_dai; + struct snd_soc_dai *dai = rtd->codec_dai; struct snd_pcm *pcm = rtd->pcm; int ret = 0; diff --git a/sound/soc/blackfin/bf5xx-ac97-pcm.c b/sound/soc/blackfin/bf5xx-ac97-pcm.c index 9e59f68..fdaed48 100644 --- a/sound/soc/blackfin/bf5xx-ac97-pcm.c +++ b/sound/soc/blackfin/bf5xx-ac97-pcm.c @@ -421,7 +421,7 @@ static u64 bf5xx_pcm_dmamask = DMA_BIT_MASK(32); int bf5xx_pcm_ac97_new(struct snd_soc_pcm_runtime *rtd) { struct snd_card *card = rtd->card->snd_card; - struct snd_soc_dai *dai = rtd->cpu_dai; + struct snd_soc_dai *dai = rtd->codec_dai; struct snd_pcm *pcm = rtd->pcm; int ret = 0; diff --git a/sound/soc/blackfin/bf5xx-i2s-pcm.c b/sound/soc/blackfin/bf5xx-i2s-pcm.c index 96d0d90..9bdf1ea 100644 --- a/sound/soc/blackfin/bf5xx-i2s-pcm.c +++ b/sound/soc/blackfin/bf5xx-i2s-pcm.c @@ -251,7 +251,7 @@ static u64 bf5xx_pcm_dmamask = DMA_BIT_MASK(32); int bf5xx_pcm_i2s_new(struct snd_soc_pcm_runtime *rtd) { struct snd_card *card = rtd->card->snd_card; - struct snd_soc_dai *dai = rtd->cpu_dai; + struct snd_soc_dai *dai = rtd->codec_dai; struct snd_pcm *pcm = rtd->pcm; int ret = 0; diff --git a/sound/soc/blackfin/bf5xx-tdm-pcm.c b/sound/soc/blackfin/bf5xx-tdm-pcm.c index c95cc03..2d7e176 100644 --- a/sound/soc/blackfin/bf5xx-tdm-pcm.c +++ b/sound/soc/blackfin/bf5xx-tdm-pcm.c @@ -286,7 +286,7 @@ static u64 bf5xx_pcm_dmamask = DMA_BIT_MASK(32); static int bf5xx_pcm_tdm_new(struct snd_soc_pcm_runtime *rtd) { struct snd_card *card = rtd->card->snd_card; - struct snd_soc_dai *dai = rtd->cpu_dai; + struct snd_soc_dai *dai = rtd->codec_dai; struct snd_pcm *pcm = rtd->pcm; int ret = 0; diff --git a/sound/soc/davinci/davinci-pcm.c b/sound/soc/davinci/davinci-pcm.c index a49e667..35b60f9 100644 --- a/sound/soc/davinci/davinci-pcm.c +++ b/sound/soc/davinci/davinci-pcm.c @@ -836,7 +836,7 @@ static u64 davinci_pcm_dmamask = 0xffffffff; static int davinci_pcm_new(struct snd_soc_pcm_runtime *rtd) { struct snd_card *card = rtd->card->snd_card; - struct snd_soc_dai *dai = rtd->cpu_dai; + struct snd_soc_dai *dai = rtd->codec_dai; struct snd_pcm *pcm = rtd->pcm; int ret; diff --git a/sound/soc/fsl/fsl_dma.c b/sound/soc/fsl/fsl_dma.c index 732208c..932da5f 100644 --- a/sound/soc/fsl/fsl_dma.c +++ b/sound/soc/fsl/fsl_dma.c @@ -297,7 +297,7 @@ static irqreturn_t fsl_dma_isr(int irq, void *dev_id) static int fsl_dma_new(struct snd_soc_pcm_runtime *rtd) { struct snd_card *card = rtd->card->snd_card; - struct snd_soc_dai *dai = rtd->cpu_dai; + struct snd_soc_dai *dai = rtd->codec_dai; struct snd_pcm *pcm = rtd->pcm; static u64 fsl_dma_dmamask = DMA_BIT_MASK(36); int ret; diff --git a/sound/soc/fsl/mpc5200_dma.c b/sound/soc/fsl/mpc5200_dma.c index 19ad0c1..729b0f8 100644 --- a/sound/soc/fsl/mpc5200_dma.c +++ b/sound/soc/fsl/mpc5200_dma.c @@ -302,7 +302,7 @@ static u64 psc_dma_dmamask = 0xffffffff; static int psc_dma_new(struct snd_soc_pcm_runtime *rtd) { struct snd_card *card = rtd->card->snd_card; - struct snd_soc_dai *dai = rtd->cpu_dai; + struct snd_soc_dai *dai = rtd->codec_dai; struct snd_pcm *pcm = rtd->pcm; struct psc_dma *psc_dma = snd_soc_dai_get_drvdata(rtd->cpu_dai); size_t size = psc_dma_hardware.buffer_bytes_max; diff --git a/sound/soc/imx/imx-pcm-fiq.c b/sound/soc/imx/imx-pcm-fiq.c index 309c59e..dc8296a 100644 --- a/sound/soc/imx/imx-pcm-fiq.c +++ b/sound/soc/imx/imx-pcm-fiq.c @@ -241,7 +241,7 @@ static int ssi_irq = 0; static int imx_pcm_fiq_new(struct snd_soc_pcm_runtime *rtd) { struct snd_card *card = rtd->card->snd_card; - struct snd_soc_dai *dai = rtd->cpu_dai; + struct snd_soc_dai *dai = rtd->codec_dai; struct snd_pcm *pcm = rtd->pcm; int ret; diff --git a/sound/soc/imx/imx-ssi.c b/sound/soc/imx/imx-ssi.c index 158a91c..5b0972b 100644 --- a/sound/soc/imx/imx-ssi.c +++ b/sound/soc/imx/imx-ssi.c @@ -391,7 +391,7 @@ static u64 imx_pcm_dmamask = DMA_BIT_MASK(32); int imx_pcm_new(struct snd_soc_pcm_runtime *rtd) { struct snd_card *card = rtd->card->snd_card; - struct snd_soc_dai *dai = rtd->cpu_dai; + struct snd_soc_dai *dai = rtd->codec_dai; struct snd_pcm *pcm = rtd->pcm; int ret = 0; diff --git a/sound/soc/jz4740/jz4740-pcm.c b/sound/soc/jz4740/jz4740-pcm.c index a7c9578..5a34dee 100644 --- a/sound/soc/jz4740/jz4740-pcm.c +++ b/sound/soc/jz4740/jz4740-pcm.c @@ -302,7 +302,7 @@ static u64 jz4740_pcm_dmamask = DMA_BIT_MASK(32); int jz4740_pcm_new(struct snd_soc_pcm_runtime *rtd) { struct snd_card *card = rtd->card->snd_card; - struct snd_soc_dai *dai = rtd->cpu_dai; + struct snd_soc_dai *dai = rtd->codec_dai; struct snd_pcm *pcm = rtd->pcm; int ret = 0; diff --git a/sound/soc/kirkwood/kirkwood-dma.c b/sound/soc/kirkwood/kirkwood-dma.c index cd33de1..75255e4 100644 --- a/sound/soc/kirkwood/kirkwood-dma.c +++ b/sound/soc/kirkwood/kirkwood-dma.c @@ -315,7 +315,7 @@ static int kirkwood_dma_preallocate_dma_buffer(struct snd_pcm *pcm, static int kirkwood_dma_new(struct snd_soc_pcm_runtime *rtd) { struct snd_card *card = rtd->card->snd_card; - struct snd_soc_dai *dai = rtd->cpu_dai; + struct snd_soc_dai *dai = rtd->codec_dai; struct snd_pcm *pcm = rtd->pcm; int ret; diff --git a/sound/soc/mid-x86/sst_platform.c b/sound/soc/mid-x86/sst_platform.c index 3e78260..92de19c 100644 --- a/sound/soc/mid-x86/sst_platform.c +++ b/sound/soc/mid-x86/sst_platform.c @@ -404,7 +404,7 @@ static void sst_pcm_free(struct snd_pcm *pcm) int sst_pcm_new(struct snd_soc_pcm_runtime *rtd) { - struct snd_soc_dai *dai = rtd->cpu_dai; + struct snd_soc_dai *dai = rtd->codec_dai; struct snd_pcm *pcm = rtd->pcm; int retval = 0; diff --git a/sound/soc/nuc900/nuc900-pcm.c b/sound/soc/nuc900/nuc900-pcm.c index d589ef1..1964801 100644 --- a/sound/soc/nuc900/nuc900-pcm.c +++ b/sound/soc/nuc900/nuc900-pcm.c @@ -318,7 +318,7 @@ static u64 nuc900_pcm_dmamask = DMA_BIT_MASK(32); static int nuc900_dma_new(struct snd_soc_pcm_runtime *rtd) { struct snd_card *card = rtd->card->snd_card; - struct snd_soc_dai *dai = rtd->cpu_dai; + struct snd_soc_dai *dai = rtd->codec_dai; struct snd_pcm *pcm = rtd->pcm; if (!card->dev->dma_mask) diff --git a/sound/soc/omap/omap-pcm.c b/sound/soc/omap/omap-pcm.c index b2f5751..0787a54 100644 --- a/sound/soc/omap/omap-pcm.c +++ b/sound/soc/omap/omap-pcm.c @@ -369,7 +369,7 @@ static void omap_pcm_free_dma_buffers(struct snd_pcm *pcm) static int omap_pcm_new(struct snd_soc_pcm_runtime *rtd) { struct snd_card *card = rtd->card->snd_card; - struct snd_soc_dai *dai = rtd->cpu_dai; + struct snd_soc_dai *dai = rtd->codec_dai; struct snd_pcm *pcm = rtd->pcm; int ret = 0; diff --git a/sound/soc/pxa/pxa2xx-pcm.c b/sound/soc/pxa/pxa2xx-pcm.c index d73d6f6..5438a67 100644 --- a/sound/soc/pxa/pxa2xx-pcm.c +++ b/sound/soc/pxa/pxa2xx-pcm.c @@ -88,7 +88,7 @@ static u64 pxa2xx_pcm_dmamask = DMA_BIT_MASK(32); static int pxa2xx_soc_pcm_new(struct snd_soc_pcm_runtime *rtd) { struct snd_card *card = rtd->card->snd_card; - struct snd_soc_dai *dai = rtd->cpu_dai; + struct snd_soc_dai *dai = rtd->codec_dai; struct snd_pcm *pcm = rtd->pcm; int ret = 0; diff --git a/sound/soc/s6000/s6000-pcm.c b/sound/soc/s6000/s6000-pcm.c index 80c85fd..925192d 100644 --- a/sound/soc/s6000/s6000-pcm.c +++ b/sound/soc/s6000/s6000-pcm.c @@ -446,7 +446,7 @@ static u64 s6000_pcm_dmamask = DMA_BIT_MASK(32); static int s6000_pcm_new(struct snd_soc_pcm_runtime *runtime) { struct snd_card *card = runtime->card->snd_card; - struct snd_soc_dai *dai = runtime->cpu_dai; + struct snd_soc_dai *dai = runtime->codec_dai; struct snd_pcm *pcm = runtime->pcm; struct s6000_pcm_dma_params *params; int res; diff --git a/sound/soc/samsung/dma.c b/sound/soc/samsung/dma.c index 9465588..a2475b2 100644 --- a/sound/soc/samsung/dma.c +++ b/sound/soc/samsung/dma.c @@ -428,7 +428,7 @@ static u64 dma_mask = DMA_BIT_MASK(32); static int dma_new(struct snd_soc_pcm_runtime *rtd) { struct snd_card *card = rtd->card->snd_card; - struct snd_soc_dai *dai = rtd->cpu_dai; + struct snd_soc_dai *dai = rtd->codec_dai; struct snd_pcm *pcm = rtd->pcm; int ret = 0; diff --git a/sound/soc/tegra/tegra_pcm.c b/sound/soc/tegra/tegra_pcm.c index ff86e5e..efd00a3 100644 --- a/sound/soc/tegra/tegra_pcm.c +++ b/sound/soc/tegra/tegra_pcm.c @@ -325,7 +325,7 @@ static u64 tegra_dma_mask = DMA_BIT_MASK(32); static int tegra_pcm_new(struct snd_soc_pcm_runtime *rtd) { struct snd_card *card = rtd->card->snd_card; - struct snd_soc_dai *dai = rtd->cpu_dai; + struct snd_soc_dai *dai = rtd->codec_dai; struct snd_pcm *pcm = rtd->pcm; int ret = 0; diff --git a/sound/soc/txx9/txx9aclc.c b/sound/soc/txx9/txx9aclc.c index 34aa972..5be6191 100644 --- a/sound/soc/txx9/txx9aclc.c +++ b/sound/soc/txx9/txx9aclc.c @@ -290,7 +290,7 @@ static void txx9aclc_pcm_free_dma_buffers(struct snd_pcm *pcm) static int txx9aclc_pcm_new(struct snd_soc_pcm_runtime *rtd) { - struct snd_soc_dai *dai = rtd->cpu_dai; + struct snd_soc_dai *dai = rtd->codec_dai; struct snd_pcm *pcm = rtd->pcm; struct platform_device *pdev = to_platform_device(dai->platform->dev); struct txx9aclc_soc_device *dev; -- 1.7.5.4

2 5

[alsa-devel] ASoC: Patches for an STA32X and WM8782
by Daniel Mack 22 Jun '11

22 Jun '11

Hi, I'm sending two patches to support the ST STA32x codec and the WM8782 ADC. The code was originally written by Johannes Stezenbach for 2.6.36. As the ASoC internals changed quite a bit due to the multi-component reconstruction, I took care for the code transition to make these drivers compatible to more recent versions. If there's anything left to care for in these drivers, please let me know. Daniel [PATCH 1/2] ALSA: ASoC: add STA32X codec driver [PATCH 2/2] ALSA: ASoC: add WM8782 ADC Codec Driver

6 37

[alsa-devel] Sharing PXA SSP ports
by Daniel Mack 22 Jun '11

22 Jun '11

The current API for requesting an SSP port on PXA platforms assumes that the caller of pxa_ssp_request() is the only user of this port, so both input and output streams must be implemented in the same driver. In a new setup I'm working on, this is not sufficient as we connect one codec to the input stream and another one to the output stream of the same CPU DAI "pxa-ssp-dai.0". I'm currently using a hack that disables the use count check in pxa_ssp_request(), but I certainly need a cleaner solution. What would be a good approach? Would we need a multiplexer for this hardware bus, or can we extend the DAI driver in any clever way to achieve that? Or is there a solution already I just don't know about? Thanks, Daniel

2 3

[alsa-devel] Query about SNDRV_PCM_FORMAT_GSM
by Patrick Lai 22 Jun '11

22 Jun '11

Hi, I see SNDRV_PCM_FORMAT_GSM is defined in asound.h. However, I do not find any reference to this macro except pcm_misc.c. I am curious how bps info(i.e 4.75 kbit/s) is passed down to the ALSA driver. Any pointers? Thanks Patrick -- Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

3 3

[alsa-devel] [PATCH] ALSA: isight: add now required waking of DMA context
by Stefan Richter 22 Jun '11

22 Jun '11

Since commit 13882a82ee16, upper-layer firewire drivers are required to flush after enqueueing a batch of isochronous buffers. Signed-off-by: Stefan Richter <stefanr(a)s5r6.in-berlin.de> --- Actually, snd-isight still worked for me without it but I guess I was just lucky that the controllers (FW643-e, JMB381, XIO2213A) never read up to the current tail of the DMA program. Dismiss this if you already have something like this in the pipeline. sound/firewire/isight.c | 1 + 1 file changed, 1 insertion(+) Index: b/sound/firewire/isight.c =================================================================== --- a/sound/firewire/isight.c +++ b/sound/firewire/isight.c @@ -213,6 +213,7 @@ static void isight_packet(struct fw_iso_ if (++index >= QUEUE_LENGTH) index = 0; isight->packet_index = index; + fw_iso_context_queue_flush(isight->context); } static int isight_connect(struct isight *isight) -- Stefan Richter -=====-==-== -=-= ===== http://arcgraph.de/sr/

2 3

[PATCH] ALSA: HDA: Pinfix quirk for HP Z200 Workstation
by David Henningsson 22 Jun '11

22 Jun '11

BIOS lists the internal speaker as an internal line-out. Change to internal speaker + model=auto for better auto-mute capabilities. BugLink: http://bugs.launchpad.net/bugs/754964 Reported-by: Marc Legris <marc.legris(a)canonical.com> Signed-off-by: David Henningsson <david.henningsson(a)canonical.com> --- sound/pci/hda/patch_realtek.c | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-) diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 4adf9e2..fe56b2b 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -12600,6 +12600,7 @@ static const struct hda_verb alc262_toshiba_rx1_unsol_verbs[] = { */ enum { PINFIX_FSC_H270, + PINFIX_HP_Z200, }; static const struct alc_fixup alc262_fixups[] = { @@ -12612,9 +12613,17 @@ static const struct alc_fixup alc262_fixups[] = { { } } }, + [PINFIX_HP_Z200] = { + .type = ALC_FIXUP_PINS, + .v.pins = (const struct alc_pincfg[]) { + { 0x16, 0x99130120 }, /* internal speaker */ + { } + } + }, }; static const struct snd_pci_quirk alc262_fixup_tbl[] = { + SND_PCI_QUIRK(0x103c, 0x170b, "HP Z200", PINFIX_HP_Z200), SND_PCI_QUIRK(0x1734, 0x1147, "FSC Celsius H270", PINFIX_FSC_H270), {} }; @@ -12731,6 +12740,8 @@ static const struct snd_pci_quirk alc262_cfg_tbl[] = { ALC262_HP_BPC), SND_PCI_QUIRK_MASK(0x103c, 0xff00, 0x1500, "HP z series", ALC262_HP_BPC), + SND_PCI_QUIRK(0x103c, 0x170b, "HP Z200", + ALC262_AUTO), SND_PCI_QUIRK_MASK(0x103c, 0xff00, 0x1700, "HP xw series", ALC262_HP_BPC), SND_PCI_QUIRK(0x103c, 0x2800, "HP D7000", ALC262_HP_BPC_D7000_WL), -- 1.7.4.1 --------------050602020501040407030505--

1 0

Re: [alsa-devel] [PATCH] ASoC: Add support for Cirrus Logic CS42L52 Lowpower Stereo Codec
by Mark Brown 22 Jun '11

22 Jun '11

On Thu, Feb 25, 2010 at 06:07:35PM -0600, Brian wrote: This is a quick first pass review only. It'd be best if you could include a Signed-off-by line in submissions so that they can be merged if they're OK, even if you expect problems you may be surprised! Overall this looks fairly good - there's a lot of comments below but the vast majority of them are stylistic issues or needing to update to current APIs rather than major structural problems with the code. The main thing I don't really follow is the register access code, there's quite a bit of stuff going on in there. > + * published by the Free Software Foundation. > + * Revision history > + * Nov 2007 Initial version. > + * Oct 2008 Updated to 2.6.26 > + * Feb 2010 Updated to latest asoc git tree Please remove the changelog, once things are merged git takes care of that. > + */ > +#define DEBUG Not by default in mainline :) > +#ifdef DEBUG > +#define SOCDBG(fmt, arg...) printk(KERN_ERR "%s: %s() " fmt, SOC_CS42L52_NAME, __FUNCTION__, ##arg) > +#else > +#define SOCDBG(fmt, arg...) > +#endif > +#define SOCINF(fmt, args...) printk(KERN_INFO "%s: " fmt, SOC_CS42L52_NAME, ##args) > +#define SOCERR(fmt, args...) printk(KERN_ERR "%s: " fmt, SOC_CS42L52_NAME, ##args) Please convert the driver to use the standard dev_ and pr_ macros for this (where possible the dev_ ones are better). > +static int soc_cs42l52_set_bias_level(struct snd_soc_codec *codec, > + enum snd_soc_bias_level level); > +static int soc_cs42l52_pcm_hw_params(struct snd_pcm_substream *substream, > + struct snd_pcm_hw_params *params, > + struct snd_soc_dai *dai); > +static int soc_cs42l52_set_sysclk(struct snd_soc_dai *codec_dai, > + int clk_id, u_int freq, int dir); > +static int soc_cs42l52_digital_mute(struct snd_soc_dai *dai, int mute); > +static int soc_cs42l52_set_fmt(struct snd_soc_dai *codec_dai, > + u_int fmt); > + > +static unsigned int soc_cs42l52_read(struct snd_soc_codec *codec, > + u_int reg); Please use 'unsigned int' rather than 'u_int' for consistency with the rest of ASoC. It's a bit surprising that you need the forward declarations at all but they do no harm. > +/** > + * snd_soc_get_volsw - single mixer get callback > + * @kcontrol: mixer control > + * @uinfo: control element information > + * > + * Callback to get the value of a single mixer control. > + * > + * Returns 0 for success. > + */ > +int snd_soc_cs42l5x_get_volsw(struct snd_kcontrol *kcontrol, > + struct snd_ctl_elem_value *ucontrol) > +{ > + struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol); > + int reg = kcontrol->private_value & 0xff; The indentation looks wrong - should be tabs for indenting the code blocks. scripts/checkpatch.pl is a pretty good check for this sort of stuff, I'll not mention other issues. > + int shift = (kcontrol->private_value >> 8) & 0x0f; > + int rshift = (kcontrol->private_value >> 12) & 0x0f; > + int max = (kcontrol->private_value >> 16) & 0xff; > + int mask = (1 << fls(max)) - 1; > + int min = (kcontrol->private_value >> 24) & 0xff; > + > + ucontrol->value.integer.value[0] = > + ((snd_soc_read(codec, reg) >> shift) - min) & mask; > + if (shift != rshift) > + ucontrol->value.integer.value[1] = > + ((snd_soc_read(codec, reg) >> rshift) - min) & mask; This and most of the other custom control stuff looks an awful lot like it should be done by adding a SND_SOC_DOUBLE_SIGNED() or something along the lines of the existing _S8() but I didn't read entirely closely. I was speaking to someone on IRC only the other day who needed some of those types for a different device. These should also be updated to use struct soc_mixer_control for the private value rather than shifting everything into the private value, it allows wider ranges and is less error prone. > +static const u8 soc_cs42l52_reg_default[] = { > + 0x00, 0xE0, 0x01, 0x07, 0x05, /*4*/ > + 0xa0, 0x00, 0x00, 0x81, /*8*/ Very nitpicky but you're using a mix of upper and lower case for the letters in the hex digits. > +static inline int soc_cs42l52_read_reg_cache(struct snd_soc_codec *codec, > + u_int reg) > +{ > + u8 *cache = codec->reg_cache; > + > + return reg > SOC_CS42L52_REG_NUM ? -EINVAL : cache[reg]; > +} You should be able to replace all your register I/O code with use of soc-cache.c with 8 bit data, 8 bit register. Except... > + if(info->flags & SOC_CS42L52_ALL_IN_ONE) > + { > + for(i = 0; i < codec->num_dai; i++) > + { > + if(info->machine_handler) > + info->machine_handler(i); ...I'm not sure what's going on here but it could really use an explanation? I'm not sure what ALL_IN_ONE is supposed to do, or what the machine_handler() function is for. > + if(info->flags & SOC_CS42L52_CHIP_SWICTH) > + { > + num = info->flags & SOC_CS42L52_CHIP_MASK; > + if(info->machine_handler) > + info->machine_handler(num); > + } This too. There's a lot of unusual code using this machine handler function. I suspect this can all use ASoC standard stuff. > +static inline int soc_cs42l52_get_revison(struct snd_soc_codec *codec) > +{ > + u8 data; > + u8 addr; > + int num, ret = 0; > + struct soc_codec_cs42l52 *info = (struct soc_codec_cs42l52*)codec->private_data; No need to cast away from void (quite a few examples of this in the code). > + return ret < 0 ? ret : data; I'm not a big fan of the ternery operator... > +static unsigned int soc_cs42l52_read(struct snd_soc_codec *codec, > + u_int reg) > +{ > + u8 data; > + u8 addr; > + int i, ret = 0; > + struct soc_codec_cs42l52 *info = (struct soc_codec_cs42l52*)codec->private_data; > +#ifndef CONFIG_CS42L52_DEBUG > + if(reg == CODEC_CS42L52_SPK_STATUS) > + { > +#endif This really shouldn't be ifdefed. If you need to handle registers that report status from the device with the soc-cache stuff use the volatile_register() callback in the CODEC, any registers that need a CODEC read will call through to the hardware rather than using the cache. > +static const char *cs42l52_adc_mux[] = {"AIN1", "AIN2", "AIN3", "AIN4", "PGA"}; > +static const char *cs42l52_mic_mux[] = {"MIC1", "MIC2"}; > +static const char *cs42l52_stereo_mux[] = {"Mono", "Stereo"}; > +static const char *cs42l52_off[] = {"On", "Off"}; > +static const char *cs42l52_hpmux[] = {"Off", "On"}; > + > +static const struct soc_enum soc_cs42l52_enum[] = { > +SOC_ENUM_DOUBLE(CODEC_CS42L52_ANALOG_HPF_CTL, 4, 6, 2, cs42l52_hpf_freeze), /*0*/ > +SOC_ENUM_SINGLE(CODEC_CS42L52_ADC_HPF_FREQ, 0, 4, cs42l52_hpf_corner_freq), Don't use an array of enums, define individual variables for them. Some drivers use the arrays for historical reasons - they're quite painful to work with if you ever want to change anything in the middle and we've found a few off by one errors in the referencing into the arrays before :/. > +static const struct snd_kcontrol_new soc_cs42l52_controls[] = { > + > +SOC_ENUM("Mic VA Capture Switch", soc_cs42l52_enum[18]), /*0*/ enums shouldn't be called Switch - Switch has a specific meaning to ALSA (see Documentation/sound/alsa/ControlNames.txt, there's quite a few other controls could do with having the components of the name reordered and various other things following that). Normally just omit the ' Switch' from the name. > +SOC_DOUBLE("Analog SR Capture Switch", CODEC_CS42L52_ANALOG_HPF_CTL, 1, 3, 1, 1), > +SOC_DOUBLE("Analog ZC Capture Switch", CODEC_CS42L52_ANALOG_HPF_CTL, 0, 2, 1, 1), This'd normally be 'Capture ZC Switch' if it's for zero cross. > +SOC_ENUM("HPF corner freq Capture Switch", soc_cs42l52_enum[1]), /*5*/ > + > +SOC_SINGLE("Ganged Ctl Capture Switch", CODEC_CS42L52_ADC_MISC_CTL, 7, 1, 1), /* should be enabled init */ What does the comment mean here? > +SOC_SINGLE("HP Analog Gain Playback Volume", CODEC_CS42L52_PB_CTL1, 5, 7, 0), Just "Headphone Volume"? > +SOC_SINGLE("Playback B=A Volume Playback Switch", CODEC_CS42L52_PB_CTL1, 4, 1, 0), /*10*/ /*should be enabled init*/ If this is the mute for the headphone volume it should have the same name but with Switch instead of Volume so applications like alsamixer can display the two together. > +static int soc_cs42l52_add_controls(struct snd_soc_codec *codec) > +{ > + int i,ret = 0; > + > + for(i = 0; i < ARRAY_SIZE(soc_cs42l52_controls); i++) > + { > + ret = snd_ctl_add(codec->card, > + snd_soc_cnew(&soc_cs42l52_controls[i], codec, NULL)); > + if(ret < 0) > + { > + SOCDBG("add cs42l52 controls failed\n"); > + break; Replace this with snd_soc_add_controls(). > + /* Sum switches */ > + SND_SOC_DAPM_PGA("AIN1A Switch", CODEC_CS42L52_ADC_PGA_A, 0, 0, NULL, 0), Without having checked the datasheet I *suspect* most of these switches should be modelled as inputs to a mixer (possibly one with no power control of its own)? The "Switch" in the name certainly looks very out of place in the name of a DAPM control. > + /* Output path */ > + SND_SOC_DAPM_MUX("Passthrough Left Playback Switch", SND_SOC_NOPM, 0, 0, &cs42l52_hpa_mux), Normally "Bypass" instead of "Passthrough". > + /* Mic Bias */ > + {"Mic Bias Capture Switch", "On", "PGA MICA"}, > + {"Mic Bias Capture Switch", "On", "PGA MICB"}, > + {"Mic-Bias", NULL, "Mic Bias Capture Switch"}, > + {"Mic-Bias", NULL, "Mic Bias Capture Switch"}, > + {"ADC Mux Left Capture Switch", "PGA", "Mic-Bias"}, > + {"ADC Mux Right Capture Switch", "PGA", "Mic-Bias"}, > + {"Passthrough Left Playback Switch", "On", "Mic-Bias"}, > + {"Passthrough Right Playback Switch", "On", "Mic-Bias"}, This looks odd - normally the microphone bias would be connected to the microphone input externally and would not be hooked up as an input wihtin the CODEC? > + /* terminator */ > + {NULL, NULL, NULL}, Not needed any more (I'm vaugely surprised this doesn't generate an error). > +#define SOC_CS42L52_RATES ( SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_11025 | \ > + SNDRV_PCM_RATE_16000 | SNDRV_PCM_RATE_22050 | \ > + SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | \ > + SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_88200 | \ > + SNDRV_PCM_RATE_96000 ) /*refer to cs42l52 datasheet page35*/ This is equivalent to SNDRV_PCM_RATE_8000_96000 > +/* #define CONFIG_MANUAL_CLK */ This might use some comments (or should possibly be deleted)? > + soc_cs42l52_set_bias_level(soc_codec, SND_SOC_BIAS_OFF); > + soc_codec->bias_level = SND_SOC_BIAS_STANDBY; > + schedule_delayed_work(&soc_codec->delayed_work, msecs_to_jiffies(1000)); > + soc_cs42l52_required_setup(soc_codec); A comment about what the delayed work is up to wouldn't hurt? > + /*set hp default volume*/ > + soc_cs42l52_write(soc_codec, CODEC_CS42L52_HPA_VOL, DEFAULT_HP_VOL); > + soc_cs42l52_write(soc_codec, CODEC_CS42L52_HPB_VOL, DEFAULT_HP_VOL); For pretty much all the configuration you're doing here just let userspace set things up, there's no telling what's an appropriate setting on any given system and presumably the chip designers made a reasonable effort to ensure that the default state of the chip is at least not harmful. > +#ifdef CONFIG_MANUAL_CLK > + soc_cs42l52_write(soc_codec, CODEC_CS42L52_CLK_CTL, > + (soc_cs42l52_read(soc_codec, CODEC_CS42L52_CLK_CTL) > + & ~CLK_CTL_AUTODECT_ENABLE)); Pass this configuration in as platform data, this will allow multiple boards to build from the same kernel. > + ret = snd_soc_init_card(soc_dev); > + if(ret) > + { > + SOCERR("add snd card failed\n"); > + goto card_err; > + } Please develop against git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound-2.6.git for-2.6.35 (or whatever the latest kernel version branch is at any given time.) You should also be calling snd_soc_register_codec() and snd_soc_register_dai() from this function. > +#if defined (CONFIG_I2C) || defined (CONFIG_I2C_MODULE) No need for the ifdefs if you only support I2C, the ifdefs are used by drivers that have multiple control interfaces (typically I2C plus SPI). > + } > + > + > + return ret; > +} > + > +static int cs42l52_i2c_remove(struct i2c_client *client) > +{ > + struct snd_soc_codec *soc_codec; > + if(client) > + { > + soc_codec = i2c_get_clientdata(client); > + if(soc_codec) > + kfree(soc_codec->reg_cache); > + } > + return 0; > +} This should be unregistering from ASoC as well, or refusing to remove if not implemented. > + case SND_SOC_DAIFMT_DSP_B: > + SOCINF("unsupported format\n"); > + ret = -EINVAL; > + goto done; > + default: > + SOCINF("invaild format\n"); > + ret = -EINVAL; > + goto done; Could just squash these together. > + case SND_SOC_DAIFMT_IB_IF: > + SOCDBG("codec dai fmt inversed sclk\n"); > + iface |= IFACE_CTL1_INV_SCLK; > + break; > + case SND_SOC_DAIFMT_IB_NF: > + iface |= IFACE_CTL1_INV_SCLK; > + break; It looks like the hardware only supports the inversion of one of the clocks so only two of the clock polarity combinations should be supported? > + info->format = iface; I'd really expect to see interaction with the hardware here? > + /* 96k */ > + {12288000, 96000, CLK_CTL_S_DS_MODE, CLK_CTL_NOT_32K, CLK_CTL_NOT_27M, CLK_CTL_RATIO_128, 0}, > + {18432000, 96000, CLK_CTL_S_DS_MODE, CLK_CTL_NOT_32K, CLK_CTL_NOT_27M, CLK_CTL_RATIO_128, 0},/*29*/ > + {12000000, 96000, CLK_CTL_S_DS_MODE, CLK_CTL_NOT_32K, CLK_CTL_NOT_27M, CLK_CTL_RATIO_125, 0}, > + {24000000, 96000, CLK_CTL_S_DS_MODE, CLK_CTL_NOT_32K, CLK_CTL_NOT_27M, CLK_CTL_RATIO_125, 1}, If you're feeling adventurous you should be able to use this data to set up constraints for the DAI in the startup() callback so only sample rates supported from the system clock can be used by userspace. > +static int soc_cs42l52_pcm_hw_params(struct snd_pcm_substream *substream, > + struct snd_pcm_hw_params *params, > + struct snd_soc_dai *dai) > +{ > + int ret = 0; > + struct snd_soc_pcm_runtime *rtd = substream->private_data; > + struct snd_soc_device *soc_dev = rtd->socdev; > + struct snd_soc_codec *soc_codec = soc_dev->card->codec; > + struct soc_codec_cs42l52 *info = (struct soc_codec_cs42l52*)soc_codec->private_data; > + > + u32 clk = 0; > + int index = soc_cs42l52_get_clk(info->sysclk, params_rate(params)); > +static int soc_cs42l52_suspend(struct platform_device *pdev, pm_message_t state) > +{ > + struct snd_soc_device *soc_dev = (struct snd_soc_device*)platform_get_drvdata(pdev); > + struct snd_soc_codec *soc_codec = soc_dev->card->codec; > + > + soc_cs42l52_write(soc_codec, CODEC_CS42L52_PWCTL1, PWCTL1_PDN_CODEC); > + soc_cs42l52_set_bias_level(soc_codec, SND_SOC_BIAS_OFF); I'd expect that set_bias_level() would do what the explict write does? > +#if defined (CONFIG_I2C) || defined (CONFIG_I2C_MODULE) > + if(soc_codec_data->i2c_addr) > + { > + ret = i2c_add_driver(&cs42l52_i2c_drv); > + > + if(ret) > + { > + SOCERR("register i2c driver failed\n"); > + goto out; > + > + } > + SOCINF("Cirrus Logic ALSA SoC codec cs42l52 driver verison 1.2 2/2010\n"); > + return ret; > + } This should be changed to use standard device model registration - see http://git.kernel.org/?p=linux/kernel/git/broonie/sound-2.6.git;a=commit;h=… for an example of doing the conversion. > +struct soc_codec_cs42l52_data{ > + u_short i2c_addr; > +}; With the device model stuff this won't be needed any more. > +/* > + *CS42L52 internal registers > + */ > +#define CODEC_CS42L52_CHIP 0x01 > +#define CHIP_ID 0xE0 > +#define CHIP_ID_MASK 0xF8 Many of the register names and other definitions for register fields and values need to be namespace to avoid collisions.

4 5

[alsa-devel] [PATCH] ASoC: codecs: Max98095: Fix logging of hardware revision.
by Taylor Hutt 22 Jun '11

22 Jun '11

The base hardware revision of the Maxim 98095 part is 0x40; the code which outputs the revision of the hardware has been updated to properly use uppercase alphabetic values for the revision numbers. Also, the use of a constant for the length 'max98095_dai' has been replaced with ARRAY_SIZE(). Signed-off-by: Taylor Hutt <thutt(a)chromium.org> --- sound/soc/codecs/max98095.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sound/soc/codecs/max98095.c b/sound/soc/codecs/max98095.c index e1d282d..9a793b3 100644 --- a/sound/soc/codecs/max98095.c +++ b/sound/soc/codecs/max98095.c @@ -2261,11 +2261,11 @@ static int max98095_probe(struct snd_soc_codec *codec) ret = snd_soc_read(codec, M98095_0FF_REV_ID); if (ret < 0) { - dev_err(codec->dev, "Failed to read device revision: %d\n", + dev_err(codec->dev, "Failure reading hardware revision: %d\n", ret); goto err_access; } - dev_info(codec->dev, "revision %c\n", ret + 'A'); + dev_info(codec->dev, "Hardware revision: %c\n", ret - 0x40 + 'A'); snd_soc_write(codec, M98095_097_PWR_SYS, M98095_PWRSV); @@ -2342,8 +2342,8 @@ static int max98095_i2c_probe(struct i2c_client *i2c, max98095->control_data = i2c; max98095->pdata = i2c->dev.platform_data; - ret = snd_soc_register_codec(&i2c->dev, - &soc_codec_dev_max98095, &max98095_dai[0], 3); + ret = snd_soc_register_codec(&i2c->dev, &soc_codec_dev_max98095, + max98095_dai, ARRAY_SIZE(max98095_dai)); if (ret < 0) kfree(max98095); return ret; -- 1.7.3.1

5 12