| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4C07DD48.2050503@metafoo.de>
Date: Thu, 03 Jun 2010 18:50:16 +0200
From: Lars-Peter Clausen <lars@...afoo.de>
To: Liam Girdwood <lrg@...mlogic.co.uk>
CC: Ralf Baechle <ralf@...ux-mips.org>, linux-mips@...ux-mips.org,
linux-kernel@...r.kernel.org,
Mark Brown <broonie@...nsource.wolfsonmicro.com>,
alsa-devel@...a-project.org
Subject: Re: [RFC][PATCH 21/26] alsa: ASoC: Add JZ4740 ASoC support
Liam Girdwood wrote:
> On Wed, 2010-06-02 at 21:12 +0200, Lars-Peter Clausen wrote:
>
>> This patch adds ASoC support for JZ4740 SoCs I2S module.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@...afoo.de>
>> Cc: Mark Brown <broonie@...nsource.wolfsonmicro.com>
>> Cc: Liam Girdwood <lrg@...mlogic.co.uk>
>> Cc: alsa-devel@...a-project.org
>> ---
>> sound/soc/Kconfig | 1 +
>> sound/soc/Makefile | 1 +
>> sound/soc/jz4740/Kconfig | 13 +
>> sound/soc/jz4740/Makefile | 9 +
>> sound/soc/jz4740/jz4740-i2s.c | 568 +++++++++++++++++++++++++++++++++++++++++
>> sound/soc/jz4740/jz4740-i2s.h | 18 ++
>> sound/soc/jz4740/jz4740-pcm.c | 350 +++++++++++++++++++++++++
>> sound/soc/jz4740/jz4740-pcm.h | 22 ++
>> 8 files changed, 982 insertions(+), 0 deletions(-)
>> create mode 100644 sound/soc/jz4740/Kconfig
>> create mode 100644 sound/soc/jz4740/Makefile
>> create mode 100644 sound/soc/jz4740/jz4740-i2s.c
>> create mode 100644 sound/soc/jz4740/jz4740-i2s.h
>> create mode 100644 sound/soc/jz4740/jz4740-pcm.c
>> create mode 100644 sound/soc/jz4740/jz4740-pcm.h
>>
>> diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig
>> index b1749bc..5a7a724 100644
>> --- a/sound/soc/Kconfig
>> +++ b/sound/soc/Kconfig
>> @@ -36,6 +36,7 @@ source "sound/soc/s3c24xx/Kconfig"
>> source "sound/soc/s6000/Kconfig"
>> source "sound/soc/sh/Kconfig"
>> source "sound/soc/txx9/Kconfig"
>> +source "sound/soc/jz4740/Kconfig"
>>
>> # Supported codecs
>> source "sound/soc/codecs/Kconfig"
>> diff --git a/sound/soc/Makefile b/sound/soc/Makefile
>> index 1470141..fdbe74d 100644
>> --- a/sound/soc/Makefile
>> +++ b/sound/soc/Makefile
>> @@ -14,3 +14,4 @@ obj-$(CONFIG_SND_SOC) += s3c24xx/
>> obj-$(CONFIG_SND_SOC) += s6000/
>> obj-$(CONFIG_SND_SOC) += sh/
>> obj-$(CONFIG_SND_SOC) += txx9/
>> +obj-$(CONFIG_SND_SOC) += jz4740/
>> diff --git a/sound/soc/jz4740/Kconfig b/sound/soc/jz4740/Kconfig
>> new file mode 100644
>> index 0000000..39df949
>> --- /dev/null
>> +++ b/sound/soc/jz4740/Kconfig
>> @@ -0,0 +1,13 @@
>> +config SND_JZ4740_SOC
>> + tristate "SoC Audio for Ingenic JZ4740 SoC"
>> + depends on SOC_JZ4740 && SND_SOC
>> + help
>> + Say Y or M if you want to add support for codecs attached to
>> + the Jz4740 AC97, I2S or SSP interface. You will also need
>>
>
> Do you have an AC97 or SSP interface ?
>
>
Whoops. Copy-paste leftover...
>> + [....]
>> +
>> +
>> +static int jz4740_i2s_trigger(struct snd_pcm_substream *substream, int cmd,
>> + struct snd_soc_dai *dai)
>> +{
>> + struct jz4740_i2s *i2s = jz4740_dai_to_i2s(dai);
>> + bool playback = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
>> +
>> + uint32_t ctrl;
>> + uint32_t mask;
>> +
>> + if (playback)
>>
>
> It's best to use (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) here.
>
>
hm, ok
>> + [...]
>> --- /dev/null
>> +++ b/sound/soc/jz4740/jz4740-pcm.c
>> @@ -0,0 +1,350 @@
>> + [...]
>> +static int jz4740_pcm_hw_params(struct snd_pcm_substream *substream,
>> + struct snd_pcm_hw_params *params)
>> +{
>> + struct snd_pcm_runtime *runtime = substream->runtime;
>> + struct jz4740_runtime_data *prtd = runtime->private_data;
>> + struct snd_soc_pcm_runtime *rtd = substream->private_data;
>> + struct jz4740_pcm_config *config;
>> +
>> + config = snd_soc_dai_get_dma_data(rtd->dai->cpu_dai, substream);
>> + if (!prtd->dma) {
>> + const char *dma_channel_name;
>> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
>> + dma_channel_name = "PCM Playback";
>> + else
>> + dma_channel_name = "PCM Capture";
>> +
>> + prtd->dma = jz4740_dma_request(substream, dma_channel_name);
>>
>
> dma_channel_name variable is not required here. Just use the const char
> * directly.
>
>
I actually had it like that before, but I think it is much more readable
in its current form. Considering that stream value will either be 0 or 1
it might work to put the channel names into a static array.
>> + [...]
>> +
>> +static snd_pcm_uframes_t jz4740_pcm_pointer(struct snd_pcm_substream *substream)
>> +{
>> + struct snd_pcm_runtime *runtime = substream->runtime;
>> + struct jz4740_runtime_data *prtd = runtime->private_data;
>> + unsigned long count, pos;
>> + snd_pcm_uframes_t offset;
>> + struct jz4740_dma_chan *dma = prtd->dma;
>> +
>> + count = jz4740_dma_get_residue(dma);
>> + if (prtd->dma_pos == prtd->dma_start)
>> + pos = prtd->dma_end - prtd->dma_start - count;
>> + else
>> + pos = prtd->dma_pos - prtd->dma_start - count;
>> +
>> + offset = bytes_to_frames(runtime, pos);
>> + if (offset >= runtime->buffer_size)
>> + offset = 0;
>> +
>>
>
> Could you comment your calculation a little more.
>
Will do.
>
> Thanks
>
> Liam
>
Thanks for the review
- Lars
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists