From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755060AbbKRB07 (ORCPT ); Tue, 17 Nov 2015 20:26:59 -0500 Received: from mailout3.w1.samsung.com ([210.118.77.13]:41320 "EHLO mailout3.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752856AbbKRB05 (ORCPT ); Tue, 17 Nov 2015 20:26:57 -0500 X-AuditID: cbfec7f5-f794b6d000001495-1a-564bd3dd0953 Subject: Re: [PATCH 1/3] ASoC: samsung: pass DMA channels as pointers To: Arnd Bergmann References: <5466463.9HBcuLFGQf@wuerfel> <564A7FF4.7030603@samsung.com> <25607973.ZAWqqjj8F4@wuerfel> 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 From: Krzysztof Kozlowski Message-id: <564BD3D9.7000701@samsung.com> Date: Wed, 18 Nov 2015 10:26:49 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-version: 1.0 In-reply-to: <25607973.ZAWqqjj8F4@wuerfel> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFmplkeLIzCtJLcpLzFFi42I5/e/4Fd27l73DDG5dVbK4cvEQk8XfScfY LaY+fMJm8f/Ra1aL1y8MLfofv2a2+Halg8li0+NrrBaXd81hs5hxfh+TxcUVX5gsXvbtZ3Hg 8djwuYnN4/evSYweO2fdZfdYvOclk8emVZ1sHpuX1Hv0bVnF6LH92jxmj8+b5AI4o7hsUlJz MstSi/TtErgy2k8sYCy4LF8xaYJ3A+NyiS5GTg4JAROJvR/2s0DYYhIX7q1n62Lk4hASWMoo 8XDmBXYI5wujROvbdlaQKmEBV4kv2w4yg9giAooSU188A7OFBNIlJq//ywjSwCxwh0lixZzH bCAJNgFjic3Ll4DZvAJaEm2nVrCD2CwCqhIH920DWy0qECExcUIDK0SNoMSPyffA4pxA9Vff bALq5QAaqidx/6IWSJhZQF5i85q3zBMYBWYh6ZiFUDULSdUCRuZVjKKppckFxUnpuUZ6xYm5 xaV56XrJ+bmbGCGR83UH49JjVocYBTgYlXh4Exd7hwmxJpYVV+YeYpTgYFYS4b16GCjEm5JY WZValB9fVJqTWnyIUZqDRUmcd+au9yFAzyeWpGanphakFsFkmTg4pRoYj0vdFlpfzG6Q7nxu k8n/awHBi9xW6+xcLK2xsu94UrhNc1f3Xm3r3v2fLmpvOBMhGFl8OK+q1NBPQv3ipzcvONu3 XFh1rPT49mVfOrPjHyWutjCwYbwQK1fPc1MwPeTijTKfklf/0wT+vb7C3R68oWDv5gtzpNsm p3ivONMksuCnRuIVgdR1SizFGYmGWsxFxYkAPVyKkpgCAAA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 17.11.2015 19:24, Arnd Bergmann wrote: > 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? No, I think it is okay. I'll review the v2 you sent. Best regards, Krzysztof