diff mbox series

[v2,01/11] ASoC: tegra20-spdif: stop setting slave_id

Message ID 20211122222203.4103644-2-arnd@kernel.org
State New
Headers show
Series dmaengine: kill off dma_slave_config->slave_id | expand

Commit Message

Arnd Bergmann Nov. 22, 2021, 10:21 p.m. UTC
From: Arnd Bergmann <arnd@arndb.de>

The DMA resource is never set up anywhere, and passing this as slave_id
has not been the proper procedure in a long time.

As a preparation for removing all slave_id references from the ALSA code,
remove this one.

According to Dmitry Osipenko, this driver has never been used and
the mechanism for configuring DMA would not work as it is implemented,
so this part will get rewritten when the driver gets put into use
again in the future.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 sound/soc/tegra/tegra20_spdif.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Dmitry Osipenko Nov. 24, 2021, 4:32 p.m. UTC | #1
23.11.2021 01:21, Arnd Bergmann пишет:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The DMA resource is never set up anywhere, and passing this as slave_id
> has not been the proper procedure in a long time.
> 
> As a preparation for removing all slave_id references from the ALSA code,
> remove this one.
> 
> According to Dmitry Osipenko, this driver has never been used and
> the mechanism for configuring DMA would not work as it is implemented,
> so this part will get rewritten when the driver gets put into use
> again in the future.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  sound/soc/tegra/tegra20_spdif.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/sound/soc/tegra/tegra20_spdif.c b/sound/soc/tegra/tegra20_spdif.c
> index 9fdc82d58db3..1c3385da6f82 100644
> --- a/sound/soc/tegra/tegra20_spdif.c
> +++ b/sound/soc/tegra/tegra20_spdif.c
> @@ -284,7 +284,6 @@ static int tegra20_spdif_platform_probe(struct platform_device *pdev)
>  	spdif->playback_dma_data.addr = mem->start + TEGRA20_SPDIF_DATA_OUT;
>  	spdif->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
>  	spdif->playback_dma_data.maxburst = 4;
> -	spdif->playback_dma_data.slave_id = dmareq->start;
>  
>  	pm_runtime_enable(&pdev->dev);
>  
> 

Thanks, Arnd!

The commit message is correct, however you could remove even more code
here. But there is no need to make a v3 just because this patch because
I already prepared patchset that revives this S/PDIF driver and enables
HDMI audio on Tegra20. I'll take care of cleaning up the whole code of
this driver.

diff --git a/sound/soc/tegra/tegra20_spdif.c
b/sound/soc/tegra/tegra20_spdif.c
index 7751575cd6d6..1c3385da6f82 100644
--- a/sound/soc/tegra/tegra20_spdif.c
+++ b/sound/soc/tegra/tegra20_spdif.c
@@ -251,7 +251,7 @@ static const struct regmap_config
tegra20_spdif_regmap_config = {
 static int tegra20_spdif_platform_probe(struct platform_device *pdev)
 {
 	struct tegra20_spdif *spdif;
-	struct resource *mem, *dmareq;
+	struct resource *mem;
 	void __iomem *regs;
 	int ret;

@@ -273,12 +273,6 @@ static int tegra20_spdif_platform_probe(struct
platform_device *pdev)
 	if (IS_ERR(regs))
 		return PTR_ERR(regs);

-	dmareq = platform_get_resource(pdev, IORESOURCE_DMA, 0);
-	if (!dmareq) {
-		dev_err(&pdev->dev, "No DMA resource\n");
-		return -ENODEV;
-	}
-
 	spdif->regmap = devm_regmap_init_mmio(&pdev->dev, regs,
 					    &tegra20_spdif_regmap_config);
 	if (IS_ERR(spdif->regmap)) {
@@ -290,7 +284,6 @@ static int tegra20_spdif_platform_probe(struct
platform_device *pdev)
 	spdif->playback_dma_data.addr = mem->start + TEGRA20_SPDIF_DATA_OUT;
 	spdif->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
 	spdif->playback_dma_data.maxburst = 4;
-	spdif->playback_dma_data.slave_id = dmareq->start;

 	pm_runtime_enable(&pdev->dev);
Arnd Bergmann Nov. 24, 2021, 4:47 p.m. UTC | #2
On Wed, Nov 24, 2021 at 5:32 PM Dmitry Osipenko <digetx@gmail.com> wrote:
> 23.11.2021 01:21, Arnd Bergmann пишет:
>
> The commit message is correct, however you could remove even more code
> here. But there is no need to make a v3 just because this patch because
> I already prepared patchset that revives this S/PDIF driver and enables
> HDMI audio on Tegra20. I'll take care of cleaning up the whole code of
> this driver.

Ok, perfect, thanks for taking a closer look as well.

>
> -       dmareq = platform_get_resource(pdev, IORESOURCE_DMA, 0);
> -       if (!dmareq) {
> -               dev_err(&pdev->dev, "No DMA resource\n");
> -               return -ENODEV;
> -       }
> -

Right, I think I considered doing this at some point as well, not sure
why I left it in for the version I posted. Passing the IORESOURCE_DMA
values is clearly wrong by itself and needs to be removed, though
it's not obvious what the correct way of requesting the DMA channel
is for this driver either, without a DT binding or users.

        Arnd
Dmitry Osipenko Nov. 24, 2021, 5:36 p.m. UTC | #3
24.11.2021 19:47, Arnd Bergmann пишет:
> On Wed, Nov 24, 2021 at 5:32 PM Dmitry Osipenko <digetx@gmail.com> wrote:
>> 23.11.2021 01:21, Arnd Bergmann пишет:
>>
>> The commit message is correct, however you could remove even more code
>> here. But there is no need to make a v3 just because this patch because
>> I already prepared patchset that revives this S/PDIF driver and enables
>> HDMI audio on Tegra20. I'll take care of cleaning up the whole code of
>> this driver.
> 
> Ok, perfect, thanks for taking a closer look as well.
> 
>>
>> -       dmareq = platform_get_resource(pdev, IORESOURCE_DMA, 0);
>> -       if (!dmareq) {
>> -               dev_err(&pdev->dev, "No DMA resource\n");
>> -               return -ENODEV;
>> -       }
>> -
> 
> Right, I think I considered doing this at some point as well, not sure
> why I left it in for the version I posted. Passing the IORESOURCE_DMA
> values is clearly wrong by itself and needs to be removed, though
> it's not obvious what the correct way of requesting the DMA channel
> is for this driver either, without a DT binding or users.

Yes, it's indeed not obvious.
Dmitry Osipenko Nov. 24, 2021, 5:39 p.m. UTC | #4
23.11.2021 01:21, Arnd Bergmann пишет:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The DMA resource is never set up anywhere, and passing this as slave_id
> has not been the proper procedure in a long time.
> 
> As a preparation for removing all slave_id references from the ALSA code,
> remove this one.
> 
> According to Dmitry Osipenko, this driver has never been used and
> the mechanism for configuring DMA would not work as it is implemented,
> so this part will get rewritten when the driver gets put into use
> again in the future.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  sound/soc/tegra/tegra20_spdif.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/sound/soc/tegra/tegra20_spdif.c b/sound/soc/tegra/tegra20_spdif.c
> index 9fdc82d58db3..1c3385da6f82 100644
> --- a/sound/soc/tegra/tegra20_spdif.c
> +++ b/sound/soc/tegra/tegra20_spdif.c
> @@ -284,7 +284,6 @@ static int tegra20_spdif_platform_probe(struct platform_device *pdev)
>  	spdif->playback_dma_data.addr = mem->start + TEGRA20_SPDIF_DATA_OUT;
>  	spdif->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
>  	spdif->playback_dma_data.maxburst = 4;
> -	spdif->playback_dma_data.slave_id = dmareq->start;
>  
>  	pm_runtime_enable(&pdev->dev);
>  
> 

Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
diff mbox series

Patch

diff --git a/sound/soc/tegra/tegra20_spdif.c b/sound/soc/tegra/tegra20_spdif.c
index 9fdc82d58db3..1c3385da6f82 100644
--- a/sound/soc/tegra/tegra20_spdif.c
+++ b/sound/soc/tegra/tegra20_spdif.c
@@ -284,7 +284,6 @@  static int tegra20_spdif_platform_probe(struct platform_device *pdev)
 	spdif->playback_dma_data.addr = mem->start + TEGRA20_SPDIF_DATA_OUT;
 	spdif->playback_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
 	spdif->playback_dma_data.maxburst = 4;
-	spdif->playback_dma_data.slave_id = dmareq->start;
 
 	pm_runtime_enable(&pdev->dev);