| 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: <200908071130.54337.peter.ujfalusi@nokia.com>
Date: Fri, 7 Aug 2009 11:30:54 +0300
From: Peter Ujfalusi <peter.ujfalusi@...ia.com>
To: alsa-devel@...a-project.org
Cc: ext Jarkko Nikula <jhnikula@...il.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Janusz Krzysztofik <jkrzyszt@....icnet.pl>,
Tony Lindgren <tony@...mide.com>,
Mark Brown <broonie@...nsource.wolfsonmicro.com>,
"linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>
Subject: Re: [alsa-devel] [PATCH][2.6.31-rc5] ARM: OMAP: McBSP: Fix ASoC on OMAP1510 by fixing API of omap_mcbsp_start/stop
On Friday 07 August 2009 09:59:47 ext Jarkko Nikula wrote:
> Simultaneous audio playback and capture on OMAP1510 can cause that second
> stream is stalled if there is enough delay between startup of the audio
> streams.
>
> Current implementation of the omap_mcbsp_start is starting both transmitter
> and receiver at the same time and it is called only for firstly started
> audio stream from the OMAP McBSP based ASoC DAI driver.
>
> Since DMA request lines on OMAP1510 are edge sensitive, the DMA request is
> missed if there is no DMA transfer set up at that time when the first word
> after McBSP startup is transmitted. The problem hasn't noted before since
> later OMAPs are using level sensitive DMA request lines.
>
> Fix the problem by changing API of omap_mcbsp_start and omap_mcbsp_stop by
> allowing to start and stop individually McBSP transmitter and receiver
> logics. Then call those functions individually for both audio playback
> and capture streams. This ensures that DMA transfer is setup before
> transmitter or receiver is started.
Looks nice, but I would have done it a bit differently...
>
> Thanks to Janusz Krzysztofik <jkrzyszt@....icnet.pl> for detailed problem
> analysis and Peter Ujfalusi <peter.ujfalusi@...ia.com> for info about DMA
> request line behavior differences between the OMAP generations.
>
> Reported-and-tested-by: Janusz Krzysztofik <jkrzyszt@....icnet.pl>
> Signed-off-by: Jarkko Nikula <jhnikula@...il.com>
> Cc: Mark Brown <broonie@...nsource.wolfsonmicro.com>
> Cc: Tony Lindgren <tony@...mide.com>
> ---
> arch/arm/plat-omap/include/mach/mcbsp.h | 4 +-
> arch/arm/plat-omap/mcbsp.c | 50
> +++++++++++++++++++----------- sound/soc/omap/omap-mcbsp.c |
> 10 +++---
> 3 files changed, 39 insertions(+), 25 deletions(-)
>
> diff --git a/arch/arm/plat-omap/include/mach/mcbsp.h
> b/arch/arm/plat-omap/include/mach/mcbsp.h index bb154ea..57249bb 100644
> --- a/arch/arm/plat-omap/include/mach/mcbsp.h
> +++ b/arch/arm/plat-omap/include/mach/mcbsp.h
> @@ -387,8 +387,8 @@ void omap_mcbsp_register_board_cfg(struct
> omap_mcbsp_platform_data *config, void omap_mcbsp_config(unsigned int id,
> const struct omap_mcbsp_reg_cfg * config); int omap_mcbsp_request(unsigned
> int id);
> void omap_mcbsp_free(unsigned int id);
> -void omap_mcbsp_start(unsigned int id);
> -void omap_mcbsp_stop(unsigned int id);
> +void omap_mcbsp_start(unsigned int id, int tx, int rx);
> +void omap_mcbsp_stop(unsigned int id, int tx, int rx);
void omap_mcbsp_start(unsigned int id, int dir);
void omap_mcbsp_stop(unsigned int id, int dir);
> void omap_mcbsp_xmit_word(unsigned int id, u32 word);
> u32 omap_mcbsp_recv_word(unsigned int id);
>
> diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
> index efa0e01..a3d2313 100644
> --- a/arch/arm/plat-omap/mcbsp.c
> +++ b/arch/arm/plat-omap/mcbsp.c
> @@ -328,14 +328,15 @@ void omap_mcbsp_free(unsigned int id)
> EXPORT_SYMBOL(omap_mcbsp_free);
>
> /*
> - * Here we start the McBSP, by enabling the sample
> - * generator, both transmitter and receivers,
> - * and the frame sync.
> + * Here we start the McBSP, by enabling transmitter, receiver or both.
> + * If no transmitter or receiver is active prior calling, then sample-rate
> + * generator and frame sync are started.
* dir == 0: tx
* dir == 1: rx
> */
> -void omap_mcbsp_start(unsigned int id)
> +void omap_mcbsp_start(unsigned int id, int tx, int rx)
void omap_mcbsp_start(unsigned int id, int dir)
> {
> struct omap_mcbsp *mcbsp;
> void __iomem *io_base;
> + int idle;
> u16 w;
...
> /* Enable transmitter and receiver */
> w = OMAP_MCBSP_READ(io_base, SPCR2);
> - OMAP_MCBSP_WRITE(io_base, SPCR2, w | 1);
> + OMAP_MCBSP_WRITE(io_base, SPCR2, w | (tx & 1));
>
> w = OMAP_MCBSP_READ(io_base, SPCR1);
> - OMAP_MCBSP_WRITE(io_base, SPCR1, w | 1);
> + OMAP_MCBSP_WRITE(io_base, SPCR1, w | (rx & 1));
if (dir) {
/* RX: Enable receiver */
w = OMAP_MCBSP_READ(io_base, SPCR1);
OMAP_MCBSP_WRITE(io_base, SPCR1, w | 1);
} else {
/* TX: Enable transmitter */
w = OMAP_MCBSP_READ(io_base, SPCR2);
OMAP_MCBSP_WRITE(io_base, SPCR2, w | 1);
}
> udelay(100);
...
> -void omap_mcbsp_stop(unsigned int id)
> +void omap_mcbsp_stop(unsigned int id, int tx, int rx)
void omap_mcbsp_stop(unsigned int id, int dir)
> {
> struct omap_mcbsp *mcbsp;
> void __iomem *io_base;
> + int idle;
> u16 w;
>
> if (!omap_mcbsp_check_valid_id(id)) {
> @@ -386,15 +395,20 @@ void omap_mcbsp_stop(unsigned int id)
>
> /* Reset transmitter */
> w = OMAP_MCBSP_READ(io_base, SPCR2);
> - OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~(1));
> + OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~(tx & 1));
>
> /* Reset receiver */
> w = OMAP_MCBSP_READ(io_base, SPCR1);
> - OMAP_MCBSP_WRITE(io_base, SPCR1, w & ~(1));
> + OMAP_MCBSP_WRITE(io_base, SPCR1, w & ~(rx & 1));
if (dir) {
/* RX: Reset receiver */
w = OMAP_MCBSP_READ(io_base, SPCR1);
OMAP_MCBSP_WRITE(io_base, SPCR1, w | ~(1));
} else {
/* TX: Reset transmitter */
w = OMAP_MCBSP_READ(io_base, SPCR2);
OMAP_MCBSP_WRITE(io_base, SPCR2, w | ~(1));
}
> - /* Reset the sample rate generator */
> - w = OMAP_MCBSP_READ(io_base, SPCR2);
> - OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~(1 << 6));
> + idle = !((OMAP_MCBSP_READ(io_base, SPCR2) |
> + OMAP_MCBSP_READ(io_base, SPCR1)) & 1);
> +
> + if (idle) {
> + /* Reset the sample rate generator */
> + w = OMAP_MCBSP_READ(io_base, SPCR2);
> + OMAP_MCBSP_WRITE(io_base, SPCR2, w & ~(1 << 6));
> + }
> }
> EXPORT_SYMBOL(omap_mcbsp_stop);
>
> diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c
> index a5d46a7..6a837ff 100644
> --- a/sound/soc/omap/omap-mcbsp.c
> +++ b/sound/soc/omap/omap-mcbsp.c
> @@ -183,21 +183,21 @@ static int omap_mcbsp_dai_trigger(struct
> snd_pcm_substream *substream, int cmd, struct snd_soc_pcm_runtime *rtd =
> substream->private_data;
> struct snd_soc_dai *cpu_dai = rtd->dai->cpu_dai;
> struct omap_mcbsp_data *mcbsp_data = to_mcbsp(cpu_dai->private_data);
> - int err = 0;
> + int err = 0, play = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK);
no need for this change.
>
> switch (cmd) {
> case SNDRV_PCM_TRIGGER_START:
> case SNDRV_PCM_TRIGGER_RESUME:
> case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
> - if (!mcbsp_data->active++)
> - omap_mcbsp_start(mcbsp_data->bus_id);
> + mcbsp_data->active++;
> + omap_mcbsp_start(mcbsp_data->bus_id, play, !play);
omap_mcbsp_start(mcbsp_data->bus_id, substream->stream);
> break;
>
> case SNDRV_PCM_TRIGGER_STOP:
> case SNDRV_PCM_TRIGGER_SUSPEND:
> case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
> - if (!--mcbsp_data->active)
> - omap_mcbsp_stop(mcbsp_data->bus_id);
> + omap_mcbsp_stop(mcbsp_data->bus_id, play, !play);
omap_mcbsp_stop(mcbsp_data->bus_id, substream->stream);
> + mcbsp_data->active--;
> break;
> default:
> err = -EINVAL;
--
Péter
--
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