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 arnd@arndb.de
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_SPDIFWe 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