From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [PATCH 28/51] DMA-API: sound: fix dma mask handling in a lot of drivers Date: Thu, 26 Sep 2013 10:29:39 +0200 Message-ID: References: <20130919212235.GD12758@n2100.arm.linux.org.uk> <20130926075425.GX25647@n2100.arm.linux.org.uk> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: driverdev-devel-bounces@linuxdriverproject.org Sender: driverdev-devel-bounces@linuxdriverproject.org To: Russell King - ARM Linux Cc: alsa-devel@alsa-project.org, linux-doc@vger.kernel.org, linux-mmc@vger.kernel.org, linux-fbdev@vger.kernel.org, linux-nvme@lists.infradead.org, Jaroslav Kysela , Peter Ujfalusi , linux-ide@vger.kernel.org, Kukjin Kim , devel@driverdev.osuosl.org, linux-samsung-soc@vger.kernel.org, linux-scsi@vger.kernel.org, e1000-devel@lists.sourceforge.net, b43-dev@lists.infradead.org, linux-media@vger.kernel.org, devicetree@vger.kernel.org, Haojian Zhuang , Timur Tabi , Mark Brown , dri-devel@lists.freedesktop.org, Ben Dooks , linux-tegra@vger.kernel.org, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Solarflare linux maintainers , Eric Miao , Sangbeom Kim List-Id: alsa-devel@alsa-project.org At Thu, 26 Sep 2013 10:25:13 +0200, Takashi Iwai wrote: > > At Thu, 26 Sep 2013 08:54:25 +0100, > Russell King - ARM Linux wrote: > > > > On Thu, Sep 26, 2013 at 09:51:23AM +0200, Takashi Iwai wrote: > > > Hi, > > > > > > sorry for the lat response, as I've been traveling in the last weeks. > > > > > > At Thu, 19 Sep 2013 22:53:02 +0100, > > > Russell King wrote: > > > > > > > > This code sequence is unsafe in modules: > > > > > > > > static u64 mask = DMA_BIT_MASK(something); > > > > ... > > > > if (!dev->dma_mask) > > > > dev->dma_mask = &mask; > > > > > > > > as if a module is reloaded, the mask will be pointing at the original > > > > module's mask address, and this can lead to oopses. Moreover, they > > > > all follow this with: > > > > > > > > if (!dev->coherent_dma_mask) > > > > dev->coherent_dma_mask = mask; > > > > > > > > where 'mask' is the same value as the statically defined mask, and this > > > > bypasses the architecture's check on whether the DMA mask is possible. > > > > > > > > Fix these issues by using the new dma_coerce_coherent_and_mask() > > > > function. > > > > > > > > Signed-off-by: Russell King > > > > > > Applied with Mark's ack now. > > > > Which is a very stupid thing to do because you won't have > > dma_coerce_coherent_and_mask() in your tree, so all these drivers > > will fail to build for you. > > Ah, silly me, I missed the very first thing. Reverted it now... > > FWIW, below is the missing piece. Please apply it in your side if > necessary. Oh, and feel free to add my ack, if any: Acked-by: Takashi Iwai thanks, Takashi > > > Takashi > > --- > sound/soc/fsl/imx-pcm-fiq.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/sound/soc/fsl/imx-pcm-fiq.c b/sound/soc/fsl/imx-pcm-fiq.c > index 34043c5..fd5f2fb 100644 > --- a/sound/soc/fsl/imx-pcm-fiq.c > +++ b/sound/soc/fsl/imx-pcm-fiq.c > @@ -272,18 +272,16 @@ static int imx_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, int stream) > return 0; > } > > -static u64 imx_pcm_dmamask = DMA_BIT_MASK(32); > - > static int imx_pcm_new(struct snd_soc_pcm_runtime *rtd) > { > struct snd_card *card = rtd->card->snd_card; > struct snd_pcm *pcm = rtd->pcm; > - int ret = 0; > + int ret; > + > + ret = dma_coerce_mask_and_coherent(card->dev, DMA_BIT_MASK(32)); > + if (ret) > + return ret; > > - if (!card->dev->dma_mask) > - card->dev->dma_mask = &imx_pcm_dmamask; > - if (!card->dev->coherent_dma_mask) > - card->dev->coherent_dma_mask = DMA_BIT_MASK(32); > if (pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream) { > ret = imx_pcm_preallocate_dma_buffer(pcm, > SNDRV_PCM_STREAM_PLAYBACK); > -- > 1.8.4