| Message ID | 20211115085403.360194-2-arnd@kernel.org |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | dmaengine: kill off dma_slave_config->slave_id | expand |
On 11/15/21 9:53 AM, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > This field is never set, and serves no purpose, so remove it. I agree that we should remove it. Its been legacy support code for a while, but the description that there is no user is not right. The tegra20_spdif driver obviously uses it and that user is removed in this patch. I think it makes sense to split that out into a separate patch with a description why the driver will still work even with slave_id removed. Maybe the best is to remove the whole tegra20_spdif driver. > 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; > dmareq is now unused and should be removed as well. > pm_runtime_enable(&pdev->dev); >
On Mon, Nov 15, 2021 at 11:14 AM Lars-Peter Clausen <lars@metafoo.de> wrote: > > On 11/15/21 9:53 AM, Arnd Bergmann wrote: > > From: Arnd Bergmann <arnd@arndb.de> > > > > This field is never set, and serves no purpose, so remove it. > > I agree that we should remove it. Its been legacy support code for a > while, but the description that there is no user is not right. > > The tegra20_spdif driver obviously uses it and that user is removed in > this patch. I think it makes sense to split that out into a separate > patch with a description why the driver will still work even with > slave_id removed. Maybe the best is to remove the whole tegra20_spdif > driver. Ok, I'll split out the tegra patch and try to come up with a better description for it. What I saw in that driver is it just passes down the slave_id number from a 'struct resource', but there is nothing in the kernel that sets up this resource. Do you or someone else have more information on the state of this driver? I can see that it does not contain any of_device_id based probing, so it seems that this is either dead code, the platform_device gets created by some other code that is no longer compatible with this driver. Arnd
On 11/15/21 11:42 AM, Arnd Bergmann wrote: > On Mon, Nov 15, 2021 at 11:14 AM Lars-Peter Clausen <lars@metafoo.de> wrote: >> On 11/15/21 9:53 AM, Arnd Bergmann wrote: >>> From: Arnd Bergmann <arnd@arndb.de> >>> >>> This field is never set, and serves no purpose, so remove it. >> I agree that we should remove it. Its been legacy support code for a >> while, but the description that there is no user is not right. >> >> The tegra20_spdif driver obviously uses it and that user is removed in >> this patch. I think it makes sense to split that out into a separate >> patch with a description why the driver will still work even with >> slave_id removed. Maybe the best is to remove the whole tegra20_spdif >> driver. > Ok, I'll split out the tegra patch and try to come up with a better > description for it. What I saw in that driver is it just passes down the > slave_id number from a 'struct resource', but there is nothing in > the kernel that sets up this resource. > > Do you or someone else have more information on the state of this > driver? I can see that it does not contain any of_device_id based > probing, so it seems that this is either dead code, the platform_device > gets created by some other code that is no longer compatible with > this driver. I've looked into this a while back, when I tried to remove slave_id. And as far as I can tell there were never any in-tree users of this driver, even back when we used platform board files. Maybe somebody from Nvidia knows if there are out-of-tree users. - Lars
15.11.2021 14:53, Lars-Peter Clausen пишет: > On 11/15/21 11:42 AM, Arnd Bergmann wrote: >> On Mon, Nov 15, 2021 at 11:14 AM Lars-Peter Clausen <lars@metafoo.de> >> wrote: >>> On 11/15/21 9:53 AM, Arnd Bergmann wrote: >>>> From: Arnd Bergmann <arnd@arndb.de> >>>> >>>> This field is never set, and serves no purpose, so remove it. >>> I agree that we should remove it. Its been legacy support code for a >>> while, but the description that there is no user is not right. >>> >>> The tegra20_spdif driver obviously uses it and that user is removed in >>> this patch. I think it makes sense to split that out into a separate >>> patch with a description why the driver will still work even with >>> slave_id removed. Maybe the best is to remove the whole tegra20_spdif >>> driver. >> Ok, I'll split out the tegra patch and try to come up with a better >> description for it. What I saw in that driver is it just passes down the >> slave_id number from a 'struct resource', but there is nothing in >> the kernel that sets up this resource. >> >> Do you or someone else have more information on the state of this >> driver? I can see that it does not contain any of_device_id based >> probing, so it seems that this is either dead code, the platform_device >> gets created by some other code that is no longer compatible with >> this driver. > > I've looked into this a while back, when I tried to remove slave_id. And > as far as I can tell there were never any in-tree users of this driver, > even back when we used platform board files. Maybe somebody from Nvidia > knows if there are out-of-tree users. That Tegra SPDIF driver was never used. Still there is a growing interest nowadays in making it alive by implementing HDMI audio support for Tegra20 SoC. It was on my todo list for a long time, I'll try to prioritize that work 5.17, it shouldn't take much effort. The slave_id should be removed anyways, it won't be needed.
On Mon, Nov 15, 2021 at 3:46 PM Dmitry Osipenko <digetx@gmail.com> wrote: > 15.11.2021 14:53, Lars-Peter Clausen пишет: > That Tegra SPDIF driver was never used. Still there is a growing > interest nowadays in making it alive by implementing HDMI audio support > for Tegra20 SoC. It was on my todo list for a long time, I'll try to > prioritize that work 5.17, it shouldn't take much effort. > > The slave_id should be removed anyways, it won't be needed. Ok, thanks for the background, I'll mention that in the changelog text then. Arnd
diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h index 9144bd547851..7403870c28bd 100644 --- a/include/sound/dmaengine_pcm.h +++ b/include/sound/dmaengine_pcm.h @@ -58,7 +58,6 @@ struct dma_chan *snd_dmaengine_pcm_get_chan(struct snd_pcm_substream *substream) * @maxburst: Maximum number of words(note: words, as in units of the * src_addr_width member, not bytes) that can be send to or received from the * DAI in one burst. - * @slave_id: Slave requester id for the DMA channel. * @filter_data: Custom DMA channel filter data, this will usually be used when * requesting the DMA channel. * @chan_name: Custom channel name to use when requesting DMA channel. @@ -72,7 +71,6 @@ struct snd_dmaengine_dai_dma_data { dma_addr_t addr; enum dma_slave_buswidth addr_width; u32 maxburst; - unsigned int slave_id; void *filter_data; const char *chan_name; unsigned int fifo_size; diff --git a/sound/core/pcm_dmaengine.c b/sound/core/pcm_dmaengine.c index af08bb4bf578..6762fb2083e1 100644 --- a/sound/core/pcm_dmaengine.c +++ b/sound/core/pcm_dmaengine.c @@ -91,8 +91,8 @@ EXPORT_SYMBOL_GPL(snd_hwparams_to_dma_slave_config); * @dma_data: DAI DMA data * @slave_config: DMA slave configuration * - * Initializes the {dst,src}_addr, {dst,src}_maxburst, {dst,src}_addr_width and - * slave_id fields of the DMA slave config from the same fields of the DAI DMA + * Initializes the {dst,src}_addr, {dst,src}_maxburst, {dst,src}_addr_width + * fields of the DMA slave config from the same fields of the DAI DMA * data struct. The src and dst fields will be initialized depending on the * direction of the substream. If the substream is a playback stream the dst * fields will be initialized, if it is a capture stream the src fields will be @@ -124,7 +124,6 @@ void snd_dmaengine_pcm_set_config_from_dai_data( slave_config->src_addr_width = dma_data->addr_width; } - slave_config->slave_id = dma_data->slave_id; slave_config->peripheral_config = dma_data->peripheral_config; slave_config->peripheral_size = dma_data->peripheral_size; } 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);