From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753548AbbKQKZd (ORCPT ); Tue, 17 Nov 2015 05:25:33 -0500 Received: from mout.kundenserver.de ([212.227.17.13]:49525 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752690AbbKQKZa (ORCPT ); Tue, 17 Nov 2015 05:25:30 -0500 From: Arnd Bergmann To: Krzysztof Kozlowski Cc: Mark Brown , alsa-devel@alsa-project.org, Liam Girdwood , Heiko Stuebner , Kukjin Kim , Sangbeom Kim , linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Vinod Koul Subject: Re: [PATCH 1/3] ASoC: samsung: pass DMA channels as pointers Date: Tue, 17 Nov 2015 11:24:24 +0100 Message-ID: <25607973.ZAWqqjj8F4@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <564A7FF4.7030603@samsung.com> References: <5466463.9HBcuLFGQf@wuerfel> <564A7FF4.7030603@samsung.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:qMHTzeS4LBuaLTWWl3M75OsRH54vDhE0/0W9mzMr/Y5jhQlRNT0 xkwoveU5ur+QrfE7xtLlVNzuj+xDpM4MZf1ECTB+2BGJVMq+VCc+ucYNOBjHwwAVLgctY3O khC9TD7NLWKpMJlvWME9IMWqUY8a6ZTkMSmIlMYqfKeSIwj1J9qKgf3ZxI5VlX9tYqupv0O USuUI39cEiHTQDs3hsaFA== X-UI-Out-Filterresults: notjunk:1;V01:K0:egczNGOlj9o=:FKTObLRi7kWUgmx7Hxvz3w ePVGbts7rLs6n/jT9xM5emD7KhirRsyuozNVEDcvS1K5Dqs99zYPYoUMXgDVWyEZVnSuEHksF RKPmnLaiAjP5jtdpvnl0bKDXxWWeme+mNjXRC9YacYkdNuNE1fadW5yCDw9Ih30TvhyG+88o/ d7ddjAP0VDxSEf8hh5ivgA5bUjYFpr3edoI3yHImS6c48CCngLOuMz1d9nMn70Xm7KxfZAhKz Zesx6f4BeTVA6bQNC8yZDyCXIIrTQNHsxIEjgVRILKg0BBj/AevzoAHVPDKTJ2Spmncl6qjXp ZwUS9HamGiX0dEnYlEn/XGlon/OPdjA7bbK5hLtrHl1pRukJypycknILY4HGXEmxrQqXxjO0h Hq4SUyZEm/4M4PASwmS2t3VJbFm1QzZyw4wztw0xaR6WaA62ziYy1Ju6s0dTv5NT35wZuOQzY 7D59n4U/sNfPSsSUd8nN28vqx130PrmqvAHbYzrlfbYjjKH+Fz7JXfOohmbm2xgbMGW7igfrk 7CmBxKYDxawy0m4OAfgw75IG2iRy6FA70rfkSWKD26e5W11Nyp33YrAIEPjSEqepA3UD91uZ7 dDQVw6x/ByrokOmcbNcRYWYKrByq/ln8SDSeC5rjTXtyvUL+oWdnh4voBEIOWuLEz4VBT7hBB EZysNWkk7UntYHB/7nv7SRJfqjiD9TR89ljJZY7p//QYm/RM4mmp/K0nOzsDF6vTBMQGU+YfF 0JwtR9Jl1lN2EInA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tuesday 17 November 2015 10:16:36 Krzysztof Kozlowski wrote: > On 14.11.2015 02:22, Arnd Bergmann wrote: > > ARM64 allmodconfig produces a bunch of warnings when building the > > samsung ASoC code: > > > > sound/soc/samsung/dmaengine.c: In function 'samsung_asoc_init_dma_data': > > sound/soc/samsung/dmaengine.c:53:32: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] > > playback_data->filter_data = (void *)playback->channel; > > sound/soc/samsung/dmaengine.c:60:31: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] > > capture_data->filter_data = (void *)capture->channel; > > > > We could easily shut up the warning by adding an intermediate cast, > > but there is a bigger underlying problem: The use of IORESOURCE_DMA > > to pass data from platform code to device drivers is dubious to start > > with, as what we really want is a pointer that can be passed into > > a filter function. > > > > Note that on s3c64xx, the pl08x DMA data is already a pointer, but > > gets cast to resource_size_t so we can pass it as a resource, and it > > then gets converted back to a pointer. In contrast, the data we pass > > for s3c24xx is an index into a device specific table, and we artificially > > convert that into a pointer for the filter function. > > > > Signed-off-by: Arnd Bergmann > > The patch looks good. I have only two questions below. Thanks for taking a look! > > @@ -1338,11 +1325,8 @@ static int samsung_i2s_probe(struct platform_device *pdev) > > sec_dai->dma_playback.dma_addr = regs_base + I2STXDS; > > sec_dai->dma_playback.ch_name = "tx-sec"; > > > > - if (!np) { > > - res = platform_get_resource(pdev, IORESOURCE_DMA, 2); > > - if (res) > > - sec_dai->dma_playback.channel = res->start; > > - } > > + if (!np) > > + sec_dai->dma_playback.slave = i2s_pdata->dma_play_sec; > > I cannot this resource in original patch nor setting the 'dma_play_sec' > part. I guess this does not really matter as newer platforms were > converted to DT so 'np' will be set, right? Yes, that sounds correct. > > @@ -367,12 +367,6 @@ static int spdif_probe(struct platform_device *pdev) > > > > dev_dbg(&pdev->dev, "Entered %s\n", __func__); > > > > - dma_res = platform_get_resource(pdev, IORESOURCE_DMA, 0); > > - if (!dma_res) { > > - dev_err(&pdev->dev, "Unable to get dma resource.\n"); > > - return -ENXIO; > > - } > > - > > mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > if (!mem_res) { > > dev_err(&pdev->dev, "Unable to get register resource.\n"); > > @@ -432,7 +426,7 @@ static int spdif_probe(struct platform_device *pdev) > > > > spdif_stereo_out.dma_size = 2; > > spdif_stereo_out.dma_addr = mem_res->start + DATA_OUTBUF; > > - spdif_stereo_out.channel = dma_res->start; > > + spdif_stereo_out.slave = spdif_pdata->dma_playback; > > > > spdif->dma_playback = &spdif_stereo_out; > > No one sets ioresources and pdata for spdif device. No in-kernel board > files for it? Hmm, probably also a missing conversion to devicetree. The symbol used to be config SND_SOC_SAMSUNG_SMDK_SPDIF tristate "SoC S/PDIF Audio support for SMDK" - depends on SND_SOC_SAMSUNG && (MACH_SMDKC100 || MACH_SMDKC110 || MACH_SMDKV210 || MACH_SMDKV310 || MACH_SMDK4212) + depends on SND_SOC_SAMSUNG select SND_SAMSUNG_SPDIF We still support all four of the above boards, but they are all DT-only and neither spdif.c nor smdk-spdif.c ever gained DT support. Is there anything I should change in this patch for this? Arnd