May 2015 - Alsa-devel - mailman.alsa-project.org

Re: [alsa-devel] [PATCH 1/2] ASoC: zx: Add ZTE zx296702 pcm support
by Mark Brown 01 May '15

01 May '15

On Thu, Apr 30, 2015 at 12:07:29PM +0800, Jun Nie wrote: > +static const struct snd_pcm_hardware snd_zx_hardware = { > + .info = SNDRV_PCM_INFO_MMAP | > + SNDRV_PCM_INFO_MMAP_VALID | > + SNDRV_PCM_INFO_INTERLEAVED | > + SNDRV_PCM_INFO_PAUSE | > + SNDRV_PCM_INFO_RESUME | > + SNDRV_PCM_INFO_NO_PERIOD_WAKEUP, > + .period_bytes_min = 32, > + .period_bytes_max = 16 * 1024, > + .periods_min = 2, > + .periods_max = 32, > + .buffer_bytes_max = 60 * 1024, > +}; Can the generic DMA code not work this out by querying the dmaengine driver? > +int zx_pcm_platform_register(struct device *dev) > +{ > + return devm_snd_dmaengine_pcm_register(dev, &zx_dmaengine_pcm_config, > + SND_DMAENGINE_PCM_FLAG_CUSTOM_CHANNEL_NAME | > + SND_DMAENGINE_PCM_FLAG_NO_RESIDUE); _NO_RESIDUE has been removed in current code, the core figures it out automatically. Like I said looking at the driver code I'm not clear why this is specifying _CUSTOM_CHANNEL_NAME either, it looked like the driver just used the one standard channel name.

1 0

Re: [alsa-devel] [PATCH 2/2] ASoC: zx: Add zx296702 SPDIF support
by Mark Brown 01 May '15

01 May '15

On Thu, Apr 30, 2015 at 12:07:30PM +0800, Jun Nie wrote: > Add zx296702 SPDIF support This is adding a new device with DT bindings but there is no bindings document. All new DT bindings must be documented. > --- /dev/null > +++ b/include/sound/zx_hdmi_audio.h > @@ -0,0 +1,7 @@ > +#ifndef __ZX_HDMI_AUDIO_H__ > +#define __ZX_HDMI_AUDIO_H__ > + > +int zx_hdmi_audio_cfg(int audio_codec, int audio_way, > + u32 sample_rate, u32 sample_len); > +void zx_hdmi_audio_en(int on); > +#endif /* __ZX_HDMI_AUDIO_H__ */ So this isn't a S/PDIF controller but rather a HDMI controller and from the looks of it one with yet another custom interface between the video part and the audio part. As I keep saying I'd really like to see more code sharing between the various controllers doing this. Please take a look at some of the other controllers and see what can be shared, and also at this: http://thread.gmane.org/gmane.comp.video.dri.devel/126588 series from Russell King. > +config ZX296702_SPDIF > + bool "ZX296702 spdif" > + select ZX_PCM_DMA > + help > + Say Y or M if you want to add support for codecs attached to the > + zx296702 spdif interface The indentation above looks inconsistent - probably a mix of tabs and spaces. > +static struct snd_dmaengine_dai_dma_data zx_spdif_pcm_stereo_out = { > + .addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES, > + .chan_name = "tx", > + .maxburst = 8, > +}; The DMA driver said it needed custom channel names but this looks like the standard "tx" for transmit? > +static int zx_spdif_dai_probe(struct snd_soc_dai *dai) > +{ > + struct zx_spdif_info *zx_spdif = dev_get_drvdata(dai->dev); > + > + snd_soc_dai_set_drvdata(dai, zx_spdif); > + zx_spdif_pcm_stereo_out.addr = zx_spdif->mapbase + ZX_DATA; > + snd_soc_dai_init_dma_data(dai, &zx_spdif_pcm_stereo_out, NULL); > + return 0; > +} This appears to be modifying a global variable rather than some driver data, please don't do that - store anything discovered per device at runtime in the driver datta. > +static void zx_spdif_chanstats(void __iomem *base, unsigned int rate) > +{ > + u32 cstas1; > + > + switch (rate) { > + case 22050: > + default: > + return; > + } This should handle and report errrors, not ignore them. > + return -EINVAL; > + } > + if (2 == params_channels(params)) > + val |= ZX_CTRL_DOUBLE_TRACK; > + else > + val |= ZX_CTRL_LEFT_TRACK; > + writel_relaxed(val, zx_spdif->reg_base + ZX_CTRL); Missing blank line between this and the above block and please reverse the arguments for the == - it's the more normal kernel coding style. > +#if defined(CONFIG_ZX_HDMI_SND_SPDIF) > + sample_len = params_physical_width(params); > + zx_hdmi_audio_cfg(1, 0, params_rate(params), sample_len); > +#endif But this is the S/PDIF dirver... in any case, this looks like something that depends on the hardware rather than something that should be a compile time option. We should be able to boot the same kernel on many systems. > + val = readl_relaxed(base + ZX_CTRL); > + val &= ~(ZX_CTRL_ENB_MASK | ZX_CTRL_TX_MASK); > + val |= on ? (ZX_CTRL_TX_OPEN | ZX_CTRL_ENB) > + : (ZX_CTRL_TX_CLOSE | ZX_CTRL_DNB); > + writel_relaxed(val, base + ZX_CTRL); Please don't use the ternery operator like this, it does nothing for legibility. > + if (clk_prepare_enable(zx_spdif->dai_clk)) > + return -EIO; Don't discard return values, pass them back. You should probably also be managing this clock at runtime to save power. > + &zx_spdif_dai, 1); > + if (ret) { > + dev_err(&pdev->dev, "Register DAI failed\n"); Print error codes as well.

1 0

[alsa-devel] [PATCH v2 1/5] ASoC: Correct typo in SOC_VALUE_ENUM_SINGLE macro
by Charles Keepax 01 May '15

01 May '15

xnitmes is clearly intended to be xnitems, but all other macros just refer to this as xitems, so change it to that. Signed-off-by: Charles Keepax <ckeepax(a)opensource.wolfsonmicro.com> --- Changes since v1: - Use first entry of values array as off value for the mux Thanks, Charles include/sound/soc.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/sound/soc.h b/include/sound/soc.h index a73d855..45cfd69 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -190,8 +190,8 @@ #define SOC_VALUE_ENUM_DOUBLE(xreg, xshift_l, xshift_r, xmask, xitems, xtexts, xvalues) \ { .reg = xreg, .shift_l = xshift_l, .shift_r = xshift_r, \ .mask = xmask, .items = xitems, .texts = xtexts, .values = xvalues} -#define SOC_VALUE_ENUM_SINGLE(xreg, xshift, xmask, xnitmes, xtexts, xvalues) \ - SOC_VALUE_ENUM_DOUBLE(xreg, xshift, xshift, xmask, xnitmes, xtexts, xvalues) +#define SOC_VALUE_ENUM_SINGLE(xreg, xshift, xmask, xitems, xtexts, xvalues) \ + SOC_VALUE_ENUM_DOUBLE(xreg, xshift, xshift, xmask, xitems, xtexts, xvalues) #define SOC_ENUM_SINGLE_VIRT(xitems, xtexts) \ SOC_ENUM_SINGLE(SND_SOC_NOPM, 0, xitems, xtexts) #define SOC_ENUM(xname, xenum) \ -- 1.7.2.5

2 7

[alsa-devel] [PATCH] ALSA: hda - Add headphone quirk for Lifebook E752
by Takashi Iwai 01 May '15

01 May '15

Fujitsu Lifebook E752 laptop needs a similar quirk done for Lifebook T731. Otherwise the headphone is always muted. Reported-and-tested-by: Christian Weber <we_chris(a)hotmail.com> Cc: <stable(a)vger.kernel.org> Signed-off-by: Takashi Iwai <tiwai(a)suse.de> --- sound/pci/hda/patch_realtek.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index e2afd53cc14c..eb2fc79cbd32 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -5119,6 +5119,7 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = { SND_PCI_QUIRK(0x104d, 0x9099, "Sony VAIO S13", ALC275_FIXUP_SONY_DISABLE_AAMIX), SND_PCI_QUIRK(0x10cf, 0x1475, "Lifebook", ALC269_FIXUP_LIFEBOOK), SND_PCI_QUIRK(0x10cf, 0x15dc, "Lifebook T731", ALC269_FIXUP_LIFEBOOK_HP_PIN), + SND_PCI_QUIRK(0x10cf, 0x1757, "Lifebook E752", ALC269_FIXUP_LIFEBOOK_HP_PIN), SND_PCI_QUIRK(0x10cf, 0x1845, "Lifebook U904", ALC269_FIXUP_LIFEBOOK_EXTMIC), SND_PCI_QUIRK(0x144d, 0xc109, "Samsung Ativ book 9 (NP900X3G)", ALC269_FIXUP_INV_DMIC), SND_PCI_QUIRK(0x1458, 0xfa53, "Gigabyte BXBT-2807", ALC283_FIXUP_BXBT2807_MIC), -- 2.3.6

1 0