dmaengine: kill off dma_slave_config->slave_id

From: Arnd Bergmann <[email protected]>

This field is never set, and serves no purpose, so remove it.

sound/soc/tegra/tegra20_spdif.c +0 -0

  	pm_runtime_enable(&pdev->dev);
 

Quoted 3 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

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.

Quoted 4 lines · Lars-Peter Clausen

Quoted 3 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

This field is never set, and serves no purpose, so remove it.

Quoted 8 lines

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.

Quoted 19 lines

Quoted 4 lines · Lars-Peter Clausen

Quoted 3 lines · Arnd Bergmann

From: Arnd Bergmann <[email protected]>

This field is never set, and serves no purpose, so remove it.

Quoted 8 lines

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.

Quoted 5 lines

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.

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.

From: Arnd Bergmann <[email protected]>

Setting slave_id makes no sense with DT based probing, and should eventually get removed entirely. Address this driver by no longer setting the field here.

From: Arnd Bergmann <[email protected]>

The field is not interpreted by the DMA engine driver, as all the data is passed from devicetree instead. Remove the assignment so the field can eventually be deleted.

Signed-off-by Arnd Bergmann <[email protected]>

---

From: Arnd Bergmann <[email protected]>

The field is not interpreted by the DMA engine driver, as all the data is passed from devicetree instead. Remove the assignment so the field can eventually be deleted.

Signed-off-by Arnd Bergmann <[email protected]>

---
 drivers/mmc/host/bcm2835.c | 2 --
 1 file changed, 2 deletions(-)

drivers/mmc/host/bcm2835.c +0 -3

index 8c2361e66277..463b707d9e99 100644
@@ -1293,14 +1293,12 @@ static int bcm2835_add_host(struct bcm2835_host *host)
 
                host->dma_cfg_tx.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
                host->dma_cfg_tx.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
-               host->dma_cfg_tx.slave_id = 13;         /* DREQ channel */
                host->dma_cfg_tx.direction = DMA_MEM_TO_DEV;
                host->dma_cfg_tx.src_addr = 0;
                host->dma_cfg_tx.dst_addr = host->phys_addr + SDDATA;
 
                host->dma_cfg_rx.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
                host->dma_cfg_rx.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
-               host->dma_cfg_rx.slave_id = 13;         /* DREQ channel */
                host->dma_cfg_rx.direction = DMA_DEV_TO_MEM;
                host->dma_cfg_rx.src_addr = host->phys_addr + SDDATA;
                host->dma_cfg_rx.dst_addr = 0;
--

2.30.2

From: Arnd Bergmann <[email protected]>

The slave device is picked through either devicetree or a filter function, and any remaining out-of-tree drivers would have warned about this usage since 2015.

Stop interpreting the field finally so it can be removed from the interface.

Signed-off-by Arnd Bergmann <[email protected]>

---
 drivers/dma/sh/shdma-base.c | 8 --------
 1 file changed, 8 deletions(-)

drivers/dma/sh/shdma-base.c +0 -8

index 7f72b3f4cd1a..41c6bc650fa3 100644
@@ -786,14 +786,6 @@ static int shdma_config(struct dma_chan *chan,
 	if (!config)
 		return -EINVAL;
 
-	/*
-	 * overriding the slave_id through dma_slave_config is deprecated,
-	 * but possibly some out-of-tree drivers still do it.
-	 */
-	if (WARN_ON_ONCE(config->slave_id &&
-			 config->slave_id != schan->real_slave_id))
-		schan->real_slave_id = config->slave_id;
-
 	/*
 	 * We could lock this, but you shouldn't be configuring the
 	 * channel, while using it...

From: Arnd Bergmann <[email protected]>

It appears that the code that reads the slave_id from the channel config was copied incorrectly from other drivers. Nothing ever sets this field on platforms that use this driver, so remove the reference.

Signed-off-by Arnd Bergmann <[email protected]>

If you fix the issue, kindly add following tag as appropriate

Reported-by kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

Quoted 1 line

Quoted 1 line

drivers/dma/qcom/qcom_adm.c:712:18: warning: no previous prototype for function 'adm_dma_xlate' [-Wmissing-prototypes]

struct dma_chan *adm_dma_xlate(struct of_phandle_args *dma_spec,

                    ^

drivers/dma/qcom/qcom_adm.c:712:1: note: declare 'static' if the function is not intended to be used outside of this translation unit struct dma_chan *adm_dma_xlate(struct of_phandle_args *dma_spec,
^
static
1 warning generated.

Quoted 6 lines · kernel test robot

If you fix the issue, kindly add following tag as appropriate

Reported-by kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

Quoted 8 lines

Quoted 1 line

Quoted 1 line

drivers/dma/qcom/qcom_adm.c:712:18: warning: no previous prototype for function 'adm_dma_xlate' [-Wmissing-prototypes]

struct dma_chan *adm_dma_xlate(struct of_phandle_args *dma_spec,

                    ^

drivers/dma/qcom/qcom_adm.c:712:1: note: declare 'static' if the function is not intended to be used outside of this translation unit struct dma_chan *adm_dma_xlate(struct of_phandle_args *dma_spec,
^
static
1 warning generated.

I noticed this mistake slipped into v2 as well, the function needs to be marked 'static'.

Vinod, let me know how you want me to address this. Should I just fold the fix (see below)
and the final Acks into the patch and send an updated pull request, or do a complete v3
submission?

Arnd

8<---

drivers/dma/qcom/qcom_adm.c +2 -2

index bb338b303af6..65697bee4db0 100644
@@ -709,8 +709,8 @@ static void adm_channel_init(struct adm_device
 *adev, struct adm_chan *achan,
  *
  * Returns pointer to appropriate dma channel on success or NULL on error.
  */
-struct dma_chan *adm_dma_xlate(struct of_phandle_args *dma_spec,
-                              struct of_dma *ofdma)
+static struct dma_chan *adm_dma_xlate(struct of_phandle_args *dma_spec,
+                                     struct of_dma *ofdma)
 {
        struct dma_device *dev = ofdma->of_dma_data;

        struct dma_chan *chan, *candidate = NULL;

From: Arnd Bergmann <[email protected]>

The display driver wants to pass a custom flag to the DMA engine driver, which it started doing by using the slave_id field that was traditionally used for a different purpose.

As there is no longer a correct use for the slave_id field, it should really be removed, and the remaining users changed over to something different.

The new mechanism for passing nonstandard settings is using the .peripheral_config field, so use that to pass a newly defined structure here, making it clear that this will not work in portable drivers.

Signed-off-by Arnd Bergmann <[email protected]>

---
 drivers/dma/xilinx/xilinx_dpdma.c  | 12 ++++++++----
 drivers/gpu/drm/xlnx/zynqmp_disp.c |  9 +++++++--
 include/linux/dma/xilinx_dpdma.h   | 11 +++++++++++
 3 files changed, 26 insertions(+), 6 deletions(-)
 create mode 100644 include/linux/dma/xilinx_dpdma.h

drivers/dma/xilinx/xilinx_dpdma.c +2 -0

index ce5c66e6897d..e2c1ef7a659c 100644
@@ -12,6 +12,7 @@
 #include <linux/clk.h>
 #include <linux/debugfs.h>
 #include <linux/delay.h>
+#include <linux/dma/xilinx_dpdma.h>
 #include <linux/dmaengine.h>
 #include <linux/dmapool.h>
 #include <linux/interrupt.h>

@@ -1273,6 +1274,7 @@ static int xilinx_dpdma_config(struct dma_chan *dchan,
 			       struct dma_slave_config *config)
 {
 	struct xilinx_dpdma_chan *chan = to_xilinx_chan(dchan);
+	struct xilinx_dpdma_peripheral_config *pconfig;
 	unsigned long flags;
 
 	/*

drivers/dma/xilinx/xilinx_dpdma.c +1 -0

+		chan->video_group = pconfig->video_group;
 
 	spin_unlock_irqrestore(&chan->lock, flags);
 

drivers/gpu/drm/xlnx/zynqmp_disp.c +7 -2

index ff2b308d8651..11c409cbc88e 100644
@@ -24,6 +24,7 @@
 
 #include <linux/clk.h>
 #include <linux/delay.h>
+#include <linux/dma/xilinx_dpdma.h>
 #include <linux/dma-mapping.h>
 #include <linux/dmaengine.h>
 #include <linux/module.h>

@@ -1058,14 +1059,18 @@ static void zynqmp_disp_layer_set_format(struct zynqmp_disp_layer *layer,
 	zynqmp_disp_avbuf_set_format(layer->disp, layer, layer->disp_fmt);
 
 	/*
-	 * Set slave_id for each DMA channel to indicate they're part of a
+	 * Set pconfig for each DMA channel to indicate they're part of a
 	 * video group.
 	 */
 	for (i = 0; i < info->num_planes; i++) {
 		struct zynqmp_disp_layer_dma *dma = &layer->dmas[i];
+		struct xilinx_dpdma_peripheral_config pconfig = {
+			.video_group = true,
+		};
 		struct dma_slave_config config = {
 			.direction = DMA_MEM_TO_DEV,
-			.slave_id = 1,
+			.peripheral_config = &pconfig,
+			.peripheral_size = sizeof(pconfig),
 		};
 
 		dmaengine_slave_config(dma->chan, &config);

include/linux/dma/xilinx_dpdma.h new file +11 -0

new file mode 100644
index 000000000000..83a1377f03f8
@@ -0,0 +1,11 @@
+// SPDX-License-Identifier: GPL-2.0
+#ifndef __LINUX_DMA_XILINX_DPDMA_H
+#define __LINUX_DMA_XILINX_DPDMA_H
+
+#include <linux/types.h>
+
+struct xilinx_dpdma_peripheral_config {
+	bool video_group;
+};
+
+#endif /* __LINUX_DMA_XILINX_DPDMA_H */

drivers/dma/xilinx/xilinx_dpdma.c static int xilinx_dpdma_config(struct dma_chan *dchan, · line 1290

Quoted 7 lines · Arnd Bergmann

drivers/dma/xilinx/xilinx_dpdma.c +1 -2

@@ -1285,11 +1287,13 @@ static int xilinx_dpdma_config(struct dma_chan *dchan,
      spin_lock_irqsave(&chan->lock, flags);
 
      /*
-      * Abuse the slave_id to indicate that the channel is part of a video
-      * group.
+      * Abuse the peripheral_config to indicate that the channel is part

Is it still an abuse, or is this now the right way to pass custom data to the DMA engine driver ?

drivers/dma/xilinx/xilinx_dpdma.c static int xilinx_dpdma_config(struct dma_chan *dchan, · line 1293

Quoted 5 lines

drivers/dma/xilinx/xilinx_dpdma.c +2 -2

+      * of a video group.
       */
-     if (chan->id <= ZYNQMP_DPDMA_VIDEO2)
-             chan->video_group = config->slave_id != 0;
+     pconfig = config->peripheral_config;

This could be moved to the variable declaration above, up to you.

drivers/dma/xilinx/xilinx_dpdma.c static int xilinx_dpdma_config(struct dma_chan *dchan, · line 1295

Quoted 2 lines

drivers/dma/xilinx/xilinx_dpdma.c +2 -0

+     if (chan->id <= ZYNQMP_DPDMA_VIDEO2 &&
+         config->peripheral_size == sizeof(*pconfig))

Silently ignoring a size mismatch isn't nice. Could we validate the size at the beginning of the function and return an error ?

With these issues addressed,

Reviewed-by Laurent Pinchart <[email protected]>

Quoted 11 lines · Laurent Pinchart

drivers/dma/xilinx/xilinx_dpdma.c static int xilinx_dpdma_config(struct dma_chan *dchan, · line 1290

Quoted 7 lines · Arnd Bergmann

drivers/dma/xilinx/xilinx_dpdma.c +1 -2

@@ -1285,11 +1287,13 @@ static int xilinx_dpdma_config(struct dma_chan *dchan,
      spin_lock_irqsave(&chan->lock, flags);
 
      /*
-      * Abuse the slave_id to indicate that the channel is part of a video
-      * group.
+      * Abuse the peripheral_config to indicate that the channel is part

Is it still an abuse, or is this now the right way to pass custom data to the DMA engine driver ?

It doesn't make the driver any more portable, but it's now being more explicit about it. As far as I can tell, this is the best way to pass data that cannot be expressed through the regular interfaces in DT and the dmaengine API.

Ideally there would be a generic way to pass this flag, but I couldn't figure out what this is actually doing, or whether there is a better way. Maybe Vinod has an idea.

I'll change s/Abuse/Use/ for the moment until I get a definite answer.

Quoted 7 lines

drivers/dma/xilinx/xilinx_dpdma.c static int xilinx_dpdma_config(struct dma_chan *dchan, · line 1293

Quoted 5 lines

drivers/dma/xilinx/xilinx_dpdma.c +2 -2

+      * of a video group.
       */
-     if (chan->id <= ZYNQMP_DPDMA_VIDEO2)
-             chan->video_group = config->slave_id != 0;
+     pconfig = config->peripheral_config;

This could be moved to the variable declaration above, up to you.

I considered that but since it doesn't fit in a normal 80-column line, it seemed best to do it here.

Quoted 5 lines

drivers/dma/xilinx/xilinx_dpdma.c static int xilinx_dpdma_config(struct dma_chan *dchan, · line 1295

Quoted 2 lines

drivers/dma/xilinx/xilinx_dpdma.c +2 -0

+     if (chan->id <= ZYNQMP_DPDMA_VIDEO2 &&
+         config->peripheral_size == sizeof(*pconfig))

Silently ignoring a size mismatch isn't nice. Could we validate the size at the beginning of the function and return an error ?

Yes, good idea. Since this would mean a bug in another driver, I'll add a WARN_ON() as well to make it clear which driver caused it. This is what I have now, let me know if you have any further suggestions:

        /*
         * Use the peripheral_config to indicate that the channel is part
         * of a video group. This requires matching use of the custom
         * structure in each driver.
         */
        pconfig = config->peripheral_config;
        if (WARN_ON(config->peripheral_size != 0 &&
                    config->peripheral_size != sizeof(*pconfig)))
                return -EINVAL;

        spin_lock_irqsave(&chan->lock, flags);
        if (chan->id <= ZYNQMP_DPDMA_VIDEO2 &&
            config->peripheral_size == sizeof(*pconfig))

                chan->video_group = pconfig->video_group;
        spin_unlock_irqrestore(&chan->lock, flags);

        return 0;

Quoted 3 lines

With these issues addressed,

Reviewed-by Laurent Pinchart <[email protected]>

Quoted 18 lines · Arnd Bergmann

Quoted 11 lines · Laurent Pinchart

drivers/dma/xilinx/xilinx_dpdma.c static int xilinx_dpdma_config(struct dma_chan *dchan, · line 1290

Quoted 7 lines · Arnd Bergmann

drivers/dma/xilinx/xilinx_dpdma.c +1 -2

@@ -1285,11 +1287,13 @@ static int xilinx_dpdma_config(struct dma_chan *dchan,
      spin_lock_irqsave(&chan->lock, flags);
 
      /*
-      * Abuse the slave_id to indicate that the channel is part of a video
-      * group.
+      * Abuse the peripheral_config to indicate that the channel is part

Is it still an abuse, or is this now the right way to pass custom data to the DMA engine driver ?

It doesn't make the driver any more portable, but it's now being more explicit about it. As far as I can tell, this is the best way to pass data that cannot be expressed through the regular interfaces in DT and the dmaengine API.

Quoted 3 lines

Ideally there would be a generic way to pass this flag, but I couldn't figure out what this is actually doing, or whether there is a better way. Maybe Vinod has an idea.

I don't think we need a generic API in this case. The DMA engine is specific to the display device, I don't foresee a need to mix-n-match.

Quoted 9 lines

        /*
         * Use the peripheral_config to indicate that the channel is part
         * of a video group. This requires matching use of the custom
         * structure in each driver.
         */
        pconfig = config->peripheral_config;
        if (WARN_ON(config->peripheral_size != 0 &&
                    config->peripheral_size != sizeof(*pconfig)))
                return -EINVAL;

How about

        if (WARN_ON(config->peripheral_config &&
                    config->peripheral_size != sizeof(*pconfig)))

Quoted 1 line

Quoted 3 lines

        spin_lock_irqsave(&chan->lock, flags);
        if (chan->id <= ZYNQMP_DPDMA_VIDEO2 &&
            config->peripheral_size == sizeof(*pconfig))

And here you can test pconfig != NULL.

Quoted 23 lines · Laurent Pinchart

Quoted 21 lines · Arnd Bergmann

Quoted 11 lines · Laurent Pinchart

drivers/dma/xilinx/xilinx_dpdma.c static int xilinx_dpdma_config(struct dma_chan *dchan, · line 1290

Quoted 7 lines · Arnd Bergmann

drivers/dma/xilinx/xilinx_dpdma.c +1 -2

@@ -1285,11 +1287,13 @@ static int xilinx_dpdma_config(struct dma_chan *dchan,
      spin_lock_irqsave(&chan->lock, flags);
 
      /*
-      * Abuse the slave_id to indicate that the channel is part of a video
-      * group.
+      * Abuse the peripheral_config to indicate that the channel is part

Is it still an abuse, or is this now the right way to pass custom data to the DMA engine driver ?

It doesn't make the driver any more portable, but it's now being more explicit about it. As far as I can tell, this is the best way to pass data that cannot be expressed through the regular interfaces in DT and the dmaengine API.

Ideally there would be a generic way to pass this flag, but I couldn't figure out what this is actually doing, or whether there is a better way. Maybe Vinod has an idea.

Quoted 2 lines

I don't think we need a generic API in this case. The DMA engine is specific to the display device, I don't foresee a need to mix-n-match.

Right. I wonder if there is even a point in using the dmaengine API in that case, I think for other single-purpose drivers we tend to just integrate the functionality in the client driver. No point changing this now of course, but it does feel odd.

From my earlier reading of the driver, my impression was that this is just a memory-to-memory device, so it could be used that way as well, but does need a flag when working on the video memory. I couldn't quite make sense of that though.

Quoted 16 lines

Quoted 9 lines

        /*
         * Use the peripheral_config to indicate that the channel is part
         * of a video group. This requires matching use of the custom
         * structure in each driver.
         */
        pconfig = config->peripheral_config;
        if (WARN_ON(config->peripheral_size != 0 &&
                    config->peripheral_size != sizeof(*pconfig)))
                return -EINVAL;

How about

        if (WARN_ON(config->peripheral_config &&
                    config->peripheral_size != sizeof(*pconfig)))

Quoted 1 line

Quoted 5 lines

Quoted 3 lines

        spin_lock_irqsave(&chan->lock, flags);
        if (chan->id <= ZYNQMP_DPDMA_VIDEO2 &&
            config->peripheral_size == sizeof(*pconfig))

And here you can test pconfig != NULL.

Good idea. Changed now, using 'if (pconfig)' without the '!= NULL' in both expressions.

Quoted 6 lines · Arnd Bergmann

On Mon, Nov 15, 2021 at 12:49 PM Laurent Pinchart wrote:

Right. I wonder if there is even a point in using the dmaengine API in that case, I think for other single-purpose drivers we tend to just integrate the functionality in the client driver. No point changing this now of course, but it does feel odd.

I agree, and that's what I would have done as well, if it wasn't for the fact that the DMA engine also supports a second client for audio. This isn't supported in upstream yet. We could still have created an ad-hoc solution, possibly based on the components framework, but the DMA engine subsystem wasn't a bad fit.

Quoted 11 lines · Laurent Pinchart

drivers/dma/xilinx/xilinx_dpdma.c static int xilinx_dpdma_config(struct dma_chan *dchan, · line 1290

Quoted 7 lines · Arnd Bergmann

drivers/dma/xilinx/xilinx_dpdma.c +1 -2

@@ -1285,11 +1287,13 @@ static int xilinx_dpdma_config(struct dma_chan *dchan,
      spin_lock_irqsave(&chan->lock, flags);
 
      /*
-      * Abuse the slave_id to indicate that the channel is part of a video
-      * group.
+      * Abuse the peripheral_config to indicate that the channel is part

Is it still an abuse, or is this now the right way to pass custom data to the DMA engine driver ?

It doesn't make the driver any more portable, but it's now being more explicit about it. As far as I can tell, this is the best way to pass data that cannot be expressed through the regular interfaces in DT and the dmaengine API.

Ideally there would be a generic way to pass this flag, but I couldn't figure out what this is actually doing, or whether there is a better way. Maybe Vinod has an idea.

I'll change s/Abuse/Use/ for the moment until I get a definite answer.

From: Arnd Bergmann <[email protected]>

Picking the connection between a DMA controller and its attached device is done through devicetree using the "dmas" property, which is implemented by the gdma driver, but it also allows overriding the "req" configuration with the slave_id field, as it was done in some linux-2.6 era drivers.

There is no driver in the tree that sets these values, so stop interpreting them before anything accidentally starts relying on it. Rename the field in the channel from "slave_id" to "req" to better match the purpose and the naming in the hardware.

If any driver actually starts using this DMA engine, it may be necessary to implement a .xlate callback that sets this field at probe time.

Signed-off-by Arnd Bergmann <[email protected]>

---
 drivers/staging/ralink-gdma/ralink-gdma.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

drivers/staging/ralink-gdma/ralink-gdma.c +5 -8

index b5229bc6eae5..f00240e62e1b 100644
@@ -106,7 +106,7 @@ struct gdma_dma_desc {
 struct gdma_dmaengine_chan {
        struct virt_dma_chan vchan;
        unsigned int id;
-       unsigned int slave_id;
+       unsigned int req;
 
        dma_addr_t fifo_addr;
        enum gdma_dma_transfer_size burst_size;

@@ -194,7 +194,6 @@ static int gdma_dma_config(struct dma_chan *c,
                        dev_err(dma_dev->ddev.dev, "only support 4 byte buswidth\n");
                        return -EINVAL;
                }
-               chan->slave_id = config->slave_id;
                chan->fifo_addr = config->dst_addr;
                chan->burst_size = gdma_dma_maxburst(config->dst_maxburst);
                break;

@@ -203,7 +202,6 @@ static int gdma_dma_config(struct dma_chan *c,
                        dev_err(dma_dev->ddev.dev, "only support 4 byte buswidth\n");
                        return -EINVAL;
                }
-               chan->slave_id = config->slave_id;
                chan->fifo_addr = config->src_addr;
                chan->burst_size = gdma_dma_maxburst(config->src_maxburst);
                break;

@@ -288,12 +286,12 @@ static int rt305x_gdma_start_transfer(struct gdma_dmaengine_chan *chan)
                dst_addr = chan->fifo_addr;
                ctrl0 = GDMA_REG_CTRL0_DST_ADDR_FIXED |
                        (8 << GDMA_RT305X_CTRL0_SRC_REQ_SHIFT) |
-                       (chan->slave_id << GDMA_RT305X_CTRL0_DST_REQ_SHIFT);
+                       (chan->req << GDMA_RT305X_CTRL0_DST_REQ_SHIFT);
        } else if (chan->desc->direction == DMA_DEV_TO_MEM) {
                src_addr = chan->fifo_addr;
                dst_addr = sg->dst_addr;
                ctrl0 = GDMA_REG_CTRL0_SRC_ADDR_FIXED |
-                       (chan->slave_id << GDMA_RT305X_CTRL0_SRC_REQ_SHIFT) |
+                       (chan->req << GDMA_RT305X_CTRL0_SRC_REQ_SHIFT) |
                        (8 << GDMA_RT305X_CTRL0_DST_REQ_SHIFT);
        } else if (chan->desc->direction == DMA_MEM_TO_MEM) {
                /*

@@ -365,12 +363,12 @@ static int rt3883_gdma_start_transfer(struct gdma_dmaengine_chan *chan)
                dst_addr = chan->fifo_addr;
                ctrl0 = GDMA_REG_CTRL0_DST_ADDR_FIXED;
                ctrl1 = (32 << GDMA_REG_CTRL1_SRC_REQ_SHIFT) |
-                       (chan->slave_id << GDMA_REG_CTRL1_DST_REQ_SHIFT);
+                       (chan->req << GDMA_REG_CTRL1_DST_REQ_SHIFT);
        } else if (chan->desc->direction == DMA_DEV_TO_MEM) {
                src_addr = chan->fifo_addr;
                dst_addr = sg->dst_addr;
                ctrl0 = GDMA_REG_CTRL0_SRC_ADDR_FIXED;
-               ctrl1 = (chan->slave_id << GDMA_REG_CTRL1_SRC_REQ_SHIFT) |
+               ctrl1 = (chan->req << GDMA_REG_CTRL1_SRC_REQ_SHIFT) |
                        (32 << GDMA_REG_CTRL1_DST_REQ_SHIFT) |
                        GDMA_REG_CTRL1_COHERENT;
        } else if (chan->desc->direction == DMA_MEM_TO_MEM) {
--

2.30.2

Quoted 3 lines · Arnd Bergmann

---
 drivers/staging/ralink-gdma/ralink-gdma.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

This driver has been already deleted from the staging tree. See [0].

From: Arnd Bergmann <[email protected]>

All references to the slave_id field have been removed, so remove the field as well to prevent new references from creeping in again.

Signed-off-by Arnd Bergmann <[email protected]>

---
 include/linux/dmaengine.h | 4 ----
 1 file changed, 4 deletions(-)

include/linux/dmaengine.h +0 -4

index 9000f3ffce8b..0349b35235e6 100644
@@ -418,9 +418,6 @@ enum dma_slave_buswidth {
  * @device_fc: Flow Controller Settings. Only valid for slave channels. Fill
  * with 'true' if peripheral should be flow controller. Direction will be
  * selected at Runtime.
- * @slave_id: Slave requester id. Only valid for slave channels. The dma
- * slave peripheral will have unique id as dma requester which need to be
- * pass as slave config.
  * @peripheral_config: peripheral configuration for programming peripheral
  * for dmaengine transfer
  * @peripheral_size: peripheral configuration buffer size

@@ -448,7 +445,6 @@ struct dma_slave_config {
 	u32 src_port_window_size;
 	u32 dst_port_window_size;
 	bool device_fc;
-	unsigned int slave_id;
 	void *peripheral_config;
 	size_t peripheral_size;
 };