From: Arnd Bergmann <[email protected]>
I recently came across some new uses of the 'slave_id' field that
I had (almost) removed a few years ago. There are no legitimate
uses of this field in the kernel, only a few stale references and
two drivers that abuse the field as a side-channel between the
dmaengine driver and its client.
Let's change the xilinx and qualcomm drivers to use the documented
side-channel (peripheral_data) instead, and remove the remnants of
it to prevent new users from coming in.
As the last patch in the series depends on all the others, it would
be nice have everything merged into the dmaengine tree for v5.17.
Arnd
Arnd Bergmann (11):
ASoC: dai_dma: remove slave_id field
spi: pic32: stop setting dma_config->slave_id
mmc: bcm2835: stop setting chan_config->slave_id
dmaengine: shdma: remove legacy slave_id parsing
dmaengine: pxa/mmp: stop referencing config->slave_id
dmaengine: sprd: stop referencing config->slave_id
dmaengine: qcom-adm: stop abusing slave_id config
dmaengine: xilinx_dpdma: stop using slave_id field
dmaengine: tegra20-apb: stop checking config->slave_id
staging: ralink-gdma: stop using slave_id config
dmaengine: remove slave_id config field
drivers/dma/mmp_pdma.c | 6 ---
drivers/dma/pxa_dma.c | 7 ---
drivers/dma/qcom/qcom_adm.c | 56 ++++++++++++++++++++---
drivers/dma/sh/shdma-base.c | 8 ----
drivers/dma/sprd-dma.c | 3 --
drivers/dma/tegra20-apb-dma.c | 6 ---
drivers/dma/xilinx/xilinx_dpdma.c | 12 +++--
drivers/gpu/drm/xlnx/zynqmp_disp.c | 9 +++-
drivers/mmc/host/bcm2835.c | 2 -
drivers/mtd/nand/raw/qcom_nandc.c | 14 +++++-
drivers/spi/spi-pic32.c | 2 -
drivers/staging/ralink-gdma/ralink-gdma.c | 12 ++---
drivers/tty/serial/msm_serial.c | 15 +++++-
include/linux/dma/qcom_adm.h | 12 +++++
include/linux/dma/xilinx_dpdma.h | 11 +++++
include/linux/dmaengine.h | 4 --
include/sound/dmaengine_pcm.h | 2 -
sound/core/pcm_dmaengine.c | 5 +-
sound/soc/tegra/tegra20_spdif.c | 1 -
19 files changed, 119 insertions(+), 68 deletions(-)
create mode 100644 include/linux/dma/qcom_adm.h
create mode 100644 include/linux/dma/xilinx_dpdma.h
--
2.30.2
Cc: Andy Gross <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Baolin Wang <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Chunyan Zhang <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Hyun Kwon <[email protected]>
Cc: Jaroslav Kysela <[email protected]>
Cc: Jon Hunter <[email protected]>
Cc: Lars-Peter Clausen <[email protected]>
Cc: Laurent Pinchart <[email protected]>
Cc: Laxman Dewangan <[email protected]>
Cc: Manivannan Sadhasivam <[email protected]>
Cc: Mark Brown <[email protected]>
Cc: Michal Simek <[email protected]>
Cc: Nicolas Saenz Julienne <[email protected]>
Cc: Orson Zhai <[email protected]>
Cc: Robert Jarzmik <[email protected]>
Cc: Scott Branden <[email protected]>
Cc: Takashi Iwai <[email protected]>
Cc: Thierry Reding <[email protected]>
Cc: Vinod Koul <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
From: Arnd Bergmann <[email protected]>
This field is never set, and serves no purpose, so remove it.
Signed-off-by: Arnd Bergmann <[email protected]>
---
include/sound/dmaengine_pcm.h | 2 --
sound/core/pcm_dmaengine.c | 5 ++---
sound/soc/tegra/tegra20_spdif.c | 1 -
3 files changed, 2 insertions(+), 6 deletions(-)
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);
--
2.30.2
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.
I could not find which DMA driver is used on PIC32, if it's
in the tree at all, but none of the obvious ones even care
about slave_id any more.
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/spi/spi-pic32.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/spi/spi-pic32.c b/drivers/spi/spi-pic32.c
index 5eb7b61bbb4d..f86433b29260 100644
--- a/drivers/spi/spi-pic32.c
+++ b/drivers/spi/spi-pic32.c
@@ -370,7 +370,6 @@ static int pic32_spi_dma_config(struct pic32_spi *pic32s, u32 dma_width)
cfg.src_addr_width = dma_width;
cfg.dst_addr_width = dma_width;
/* tx channel */
- cfg.slave_id = pic32s->tx_irq;
cfg.direction = DMA_MEM_TO_DEV;
ret = dmaengine_slave_config(master->dma_tx, &cfg);
if (ret) {
@@ -378,7 +377,6 @@ static int pic32_spi_dma_config(struct pic32_spi *pic32s, u32 dma_width)
return ret;
}
/* rx channel */
- cfg.slave_id = pic32s->rx_irq;
cfg.direction = DMA_DEV_TO_MEM;
ret = dmaengine_slave_config(master->dma_rx, &cfg);
if (ret)
--
2.30.2
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(-)
diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
index 8c2361e66277..463b707d9e99 100644
--- a/drivers/mmc/host/bcm2835.c
+++ b/drivers/mmc/host/bcm2835.c
@@ -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 last driver referencing the slave_id on Marvell PXA and MMP platforms
was the SPI driver, but this stopped doing so a long time ago, so the
TODO from the earlier patch can no be removed.
Fixes: b729bf34535e ("spi/pxa2xx: Don't use slave_id of dma_slave_config")
Fixes: 13b3006b8ebd ("dma: mmp_pdma: add filter function")
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/dma/mmp_pdma.c | 6 ------
drivers/dma/pxa_dma.c | 7 -------
2 files changed, 13 deletions(-)
diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
index a23563cd118b..5a53d7fcef01 100644
--- a/drivers/dma/mmp_pdma.c
+++ b/drivers/dma/mmp_pdma.c
@@ -727,12 +727,6 @@ static int mmp_pdma_config_write(struct dma_chan *dchan,
chan->dir = direction;
chan->dev_addr = addr;
- /* FIXME: drivers should be ported over to use the filter
- * function. Once that's done, the following two lines can
- * be removed.
- */
- if (cfg->slave_id)
- chan->drcmr = cfg->slave_id;
return 0;
}
diff --git a/drivers/dma/pxa_dma.c b/drivers/dma/pxa_dma.c
index 52d04641e361..6078cc81892e 100644
--- a/drivers/dma/pxa_dma.c
+++ b/drivers/dma/pxa_dma.c
@@ -909,13 +909,6 @@ static void pxad_get_config(struct pxad_chan *chan,
*dcmd |= PXA_DCMD_BURST16;
else if (maxburst == 32)
*dcmd |= PXA_DCMD_BURST32;
-
- /* FIXME: drivers should be ported over to use the filter
- * function. Once that's done, the following two lines can
- * be removed.
- */
- if (chan->cfg.slave_id)
- chan->drcmr = chan->cfg.slave_id;
}
static struct dma_async_tx_descriptor *
--
2.30.2
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]>
---
drivers/dma/sprd-dma.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index 4357d2395e6b..7f158ef5672d 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -795,9 +795,6 @@ static int sprd_dma_fill_desc(struct dma_chan *chan,
return dst_datawidth;
}
- if (slave_cfg->slave_id)
- schan->dev_id = slave_cfg->slave_id;
-
hw->cfg = SPRD_DMA_DONOT_WAIT_BDONE << SPRD_DMA_WAIT_BDONE_OFFSET;
/*
--
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(-)
diff --git a/drivers/dma/sh/shdma-base.c b/drivers/dma/sh/shdma-base.c
index 7f72b3f4cd1a..41c6bc650fa3 100644
--- a/drivers/dma/sh/shdma-base.c
+++ b/drivers/dma/sh/shdma-base.c
@@ -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...
--
2.30.2
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
diff --git a/drivers/dma/xilinx/xilinx_dpdma.c b/drivers/dma/xilinx/xilinx_dpdma.c
index ce5c66e6897d..e2c1ef7a659c 100644
--- a/drivers/dma/xilinx/xilinx_dpdma.c
+++ b/drivers/dma/xilinx/xilinx_dpdma.c
@@ -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;
/*
@@ -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
+ * of a video group.
*/
- if (chan->id <= ZYNQMP_DPDMA_VIDEO2)
- chan->video_group = config->slave_id != 0;
+ pconfig = config->peripheral_config;
+ if (chan->id <= ZYNQMP_DPDMA_VIDEO2 &&
+ config->peripheral_size == sizeof(*pconfig))
+ chan->video_group = pconfig->video_group;
spin_unlock_irqrestore(&chan->lock, flags);
diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
index ff2b308d8651..11c409cbc88e 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
@@ -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);
diff --git a/include/linux/dma/xilinx_dpdma.h b/include/linux/dma/xilinx_dpdma.h
new file mode 100644
index 000000000000..83a1377f03f8
--- /dev/null
+++ b/include/linux/dma/xilinx_dpdma.h
@@ -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 */
--
2.30.2
From: Arnd Bergmann <[email protected]>
Nothing sets the slave_id field any more, so stop accessing
it to allow the removal of this field.
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/dma/tegra20-apb-dma.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
index b7260749e8ee..eaafcbe4ca94 100644
--- a/drivers/dma/tegra20-apb-dma.c
+++ b/drivers/dma/tegra20-apb-dma.c
@@ -343,12 +343,6 @@ static int tegra_dma_slave_config(struct dma_chan *dc,
}
memcpy(&tdc->dma_sconfig, sconfig, sizeof(*sconfig));
- if (tdc->slave_id == TEGRA_APBDMA_SLAVE_ID_INVALID &&
- sconfig->device_fc) {
- if (sconfig->slave_id > TEGRA_APBDMA_CSR_REQ_SEL_MASK)
- return -EINVAL;
- tdc->slave_id = sconfig->slave_id;
- }
tdc->config_init = true;
return 0;
--
2.30.2
From: Arnd Bergmann <[email protected]>
The slave_id was previously used to pick one DMA slave instead of another,
but this is now done through the DMA descriptors in device tree.
For the qcom_adm driver, the configuration is documented in the DT
binding to contain a tuple of device identifier and a "crci" field,
but the implementation ends up using only a single cell for identifying
the slave, with the crci getting passed in nonstandard properties of
the device, and passed through the dma driver using the old slave_id
field. Part of the problem apparently is that the nand driver ends up
using only a single DMA request ID, but requires distinct values for
"crci" depending on the type of transfer.
Change both the dmaengine driver and the two slave drivers to allow
the documented binding to work in addition to the ad-hoc passing
of crci values. In order to no longer abuse the slave_id field, pass
the data using the "peripheral_config" mechanism instead.
Signed-off-by: Arnd Bergmann <[email protected]>
---
drivers/dma/qcom/qcom_adm.c | 56 +++++++++++++++++++++++++++----
drivers/mtd/nand/raw/qcom_nandc.c | 14 ++++++--
drivers/tty/serial/msm_serial.c | 15 +++++++--
include/linux/dma/qcom_adm.h | 12 +++++++
4 files changed, 86 insertions(+), 11 deletions(-)
create mode 100644 include/linux/dma/qcom_adm.h
diff --git a/drivers/dma/qcom/qcom_adm.c b/drivers/dma/qcom/qcom_adm.c
index ee78bed8d60d..bb338b303af6 100644
--- a/drivers/dma/qcom/qcom_adm.c
+++ b/drivers/dma/qcom/qcom_adm.c
@@ -8,6 +8,7 @@
#include <linux/device.h>
#include <linux/dmaengine.h>
#include <linux/dma-mapping.h>
+#include <linux/dma/qcom_adm.h>
#include <linux/init.h>
#include <linux/interrupt.h>
#include <linux/io.h>
@@ -140,6 +141,8 @@ struct adm_chan {
struct adm_async_desc *curr_txd;
struct dma_slave_config slave;
+ u32 crci;
+ u32 mux;
struct list_head node;
int error;
@@ -379,8 +382,8 @@ static struct dma_async_tx_descriptor *adm_prep_slave_sg(struct dma_chan *chan,
return ERR_PTR(-EINVAL);
}
- crci = achan->slave.slave_id & 0xf;
- if (!crci || achan->slave.slave_id > 0x1f) {
+ crci = achan->crci & 0xf;
+ if (!crci || achan->crci > 0x1f) {
dev_err(adev->dev, "invalid crci value\n");
return ERR_PTR(-EINVAL);
}
@@ -403,9 +406,7 @@ static struct dma_async_tx_descriptor *adm_prep_slave_sg(struct dma_chan *chan,
if (!async_desc)
return ERR_PTR(-ENOMEM);
- if (crci)
- async_desc->mux = achan->slave.slave_id & ADM_CRCI_MUX_SEL ?
- ADM_CRCI_CTL_MUX_SEL : 0;
+ async_desc->mux = achan->mux ? ADM_CRCI_CTL_MUX_SEL : 0;
async_desc->crci = crci;
async_desc->blk_size = blk_size;
async_desc->dma_len = single_count * sizeof(struct adm_desc_hw_single) +
@@ -488,10 +489,13 @@ static int adm_terminate_all(struct dma_chan *chan)
static int adm_slave_config(struct dma_chan *chan, struct dma_slave_config *cfg)
{
struct adm_chan *achan = to_adm_chan(chan);
+ struct qcom_adm_peripheral_config *config = cfg->peripheral_config;
unsigned long flag;
spin_lock_irqsave(&achan->vc.lock, flag);
memcpy(&achan->slave, cfg, sizeof(struct dma_slave_config));
+ if (cfg->peripheral_size == sizeof(config))
+ achan->crci = config->crci;
spin_unlock_irqrestore(&achan->vc.lock, flag);
return 0;
@@ -694,6 +698,45 @@ static void adm_channel_init(struct adm_device *adev, struct adm_chan *achan,
achan->vc.desc_free = adm_dma_free_desc;
}
+/**
+ * adm_dma_xlate
+ * @dma_spec: pointer to DMA specifier as found in the device tree
+ * @ofdma: pointer to DMA controller data
+ *
+ * This can use either 1-cell or 2-cell formats, the first cell
+ * identifies the slave device, while the optional second cell
+ * contains the crci value.
+ *
+ * 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)
+{
+ struct dma_device *dev = ofdma->of_dma_data;
+ struct dma_chan *chan, *candidate = NULL;
+ struct adm_chan *achan;
+
+ if (!dev || dma_spec->args_count > 2)
+ return NULL;
+
+ list_for_each_entry(chan, &dev->channels, device_node)
+ if (chan->chan_id == dma_spec->args[0]) {
+ candidate = chan;
+ break;
+ }
+
+ if (!candidate)
+ return NULL;
+
+ achan = to_adm_chan(candidate);
+ if (dma_spec->args_count == 2)
+ achan->crci = dma_spec->args[1];
+ else
+ achan->crci = 0;
+
+ return dma_get_slave_channel(candidate);
+}
+
static int adm_dma_probe(struct platform_device *pdev)
{
struct adm_device *adev;
@@ -838,8 +881,7 @@ static int adm_dma_probe(struct platform_device *pdev)
goto err_disable_clks;
}
- ret = of_dma_controller_register(pdev->dev.of_node,
- of_dma_xlate_by_chan_id,
+ ret = of_dma_controller_register(pdev->dev.of_node, adm_dma_xlate,
&adev->common);
if (ret)
goto err_unregister_dma;
diff --git a/drivers/mtd/nand/raw/qcom_nandc.c b/drivers/mtd/nand/raw/qcom_nandc.c
index 04e6f7b26706..7c6efa3b6255 100644
--- a/drivers/mtd/nand/raw/qcom_nandc.c
+++ b/drivers/mtd/nand/raw/qcom_nandc.c
@@ -6,6 +6,7 @@
#include <linux/clk.h>
#include <linux/slab.h>
#include <linux/bitops.h>
+#include <linux/dma/qcom_adm.h>
#include <linux/dma-mapping.h>
#include <linux/dmaengine.h>
#include <linux/module.h>
@@ -952,6 +953,7 @@ static int prep_adm_dma_desc(struct qcom_nand_controller *nandc, bool read,
struct dma_async_tx_descriptor *dma_desc;
struct scatterlist *sgl;
struct dma_slave_config slave_conf;
+ struct qcom_adm_peripheral_config periph_conf = {};
enum dma_transfer_direction dir_eng;
int ret;
@@ -983,11 +985,19 @@ static int prep_adm_dma_desc(struct qcom_nand_controller *nandc, bool read,
if (read) {
slave_conf.src_maxburst = 16;
slave_conf.src_addr = nandc->base_dma + reg_off;
- slave_conf.slave_id = nandc->data_crci;
+ if (nandc->data_crci) {
+ periph_conf.crci = nandc->data_crci;
+ slave_conf.peripheral_config = &periph_conf;
+ slave_conf.peripheral_size = sizeof(periph_conf);
+ }
} else {
slave_conf.dst_maxburst = 16;
slave_conf.dst_addr = nandc->base_dma + reg_off;
- slave_conf.slave_id = nandc->cmd_crci;
+ if (nandc->cmd_crci) {
+ periph_conf.crci = nandc->cmd_crci;
+ slave_conf.peripheral_config = &periph_conf;
+ slave_conf.peripheral_size = sizeof(periph_conf);
+ }
}
ret = dmaengine_slave_config(nandc->chan, &slave_conf);
diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index fcef7a961430..c6be09f44dc1 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -9,6 +9,7 @@
#include <linux/kernel.h>
#include <linux/atomic.h>
+#include <linux/dma/qcom_adm.h>
#include <linux/dma-mapping.h>
#include <linux/dmaengine.h>
#include <linux/module.h>
@@ -290,6 +291,7 @@ static void msm_request_tx_dma(struct msm_port *msm_port, resource_size_t base)
{
struct device *dev = msm_port->uart.dev;
struct dma_slave_config conf;
+ struct qcom_adm_peripheral_config periph_conf = {};
struct msm_dma *dma;
u32 crci = 0;
int ret;
@@ -308,7 +310,11 @@ static void msm_request_tx_dma(struct msm_port *msm_port, resource_size_t base)
conf.device_fc = true;
conf.dst_addr = base + UARTDM_TF;
conf.dst_maxburst = UARTDM_BURST_SIZE;
- conf.slave_id = crci;
+ if (crci) {
+ conf.peripheral_config = &periph_conf;
+ conf.peripheral_size = sizeof(periph_conf);
+ periph_conf.crci = crci;
+ }
ret = dmaengine_slave_config(dma->chan, &conf);
if (ret)
@@ -333,6 +339,7 @@ static void msm_request_rx_dma(struct msm_port *msm_port, resource_size_t base)
{
struct device *dev = msm_port->uart.dev;
struct dma_slave_config conf;
+ struct qcom_adm_peripheral_config periph_conf = {};
struct msm_dma *dma;
u32 crci = 0;
int ret;
@@ -355,7 +362,11 @@ static void msm_request_rx_dma(struct msm_port *msm_port, resource_size_t base)
conf.device_fc = true;
conf.src_addr = base + UARTDM_RF;
conf.src_maxburst = UARTDM_BURST_SIZE;
- conf.slave_id = crci;
+ if (crci) {
+ conf.peripheral_config = &periph_conf;
+ conf.peripheral_size = sizeof(periph_conf);
+ periph_conf.crci = crci;
+ }
ret = dmaengine_slave_config(dma->chan, &conf);
if (ret)
diff --git a/include/linux/dma/qcom_adm.h b/include/linux/dma/qcom_adm.h
new file mode 100644
index 000000000000..af20df674f0c
--- /dev/null
+++ b/include/linux/dma/qcom_adm.h
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#ifndef __LINUX_DMA_QCOM_ADM_H
+#define __LINUX_DMA_QCOM_ADM_H
+
+#include <linux/types.h>
+
+struct qcom_adm_peripheral_config {
+ u32 crci;
+ u32 mux;
+};
+
+#endif /* __LINUX_DMA_QCOM_ADM_H */
--
2.30.2
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(-)
diff --git a/drivers/staging/ralink-gdma/ralink-gdma.c b/drivers/staging/ralink-gdma/ralink-gdma.c
index b5229bc6eae5..f00240e62e1b 100644
--- a/drivers/staging/ralink-gdma/ralink-gdma.c
+++ b/drivers/staging/ralink-gdma/ralink-gdma.c
@@ -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
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(-)
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 9000f3ffce8b..0349b35235e6 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -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;
};
--
2.30.2
Hi Arnd,
Thank you for the patch.
On Mon, Nov 15, 2021 at 09:53:56AM +0100, Arnd Bergmann wrote:
> 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]>
Reviewed-by: Laurent Pinchart <[email protected]>
> ---
> drivers/dma/sh/shdma-base.c | 8 --------
> 1 file changed, 8 deletions(-)
>
> diff --git a/drivers/dma/sh/shdma-base.c b/drivers/dma/sh/shdma-base.c
> index 7f72b3f4cd1a..41c6bc650fa3 100644
> --- a/drivers/dma/sh/shdma-base.c
> +++ b/drivers/dma/sh/shdma-base.c
> @@ -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...
--
Regards,
Laurent Pinchart
Hi Arnd,
Thank you for the patch.
On Mon, Nov 15, 2021 at 09:54:00AM +0100, Arnd Bergmann wrote:
> 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
>
> diff --git a/drivers/dma/xilinx/xilinx_dpdma.c b/drivers/dma/xilinx/xilinx_dpdma.c
> index ce5c66e6897d..e2c1ef7a659c 100644
> --- a/drivers/dma/xilinx/xilinx_dpdma.c
> +++ b/drivers/dma/xilinx/xilinx_dpdma.c
> @@ -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;
>
> /*
> @@ -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 ?
> + * 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.
> + 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]>
> + chan->video_group = pconfig->video_group;
>
> spin_unlock_irqrestore(&chan->lock, flags);
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_disp.c b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> index ff2b308d8651..11c409cbc88e 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_disp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_disp.c
> @@ -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);
> diff --git a/include/linux/dma/xilinx_dpdma.h b/include/linux/dma/xilinx_dpdma.h
> new file mode 100644
> index 000000000000..83a1377f03f8
> --- /dev/null
> +++ b/include/linux/dma/xilinx_dpdma.h
> @@ -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 */
--
Regards,
Laurent Pinchart
Hi Arnd,
Thank you for the patch.
On Mon, Nov 15, 2021 at 09:54:03AM +0100, Arnd Bergmann wrote:
> 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.
A rationale to explain why the slave_id field shouldn't be used would be
nice.
> Signed-off-by: Arnd Bergmann <[email protected]>
Reviewed-by: Laurent Pinchart <[email protected]>
> ---
> include/linux/dmaengine.h | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 9000f3ffce8b..0349b35235e6 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -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;
> };
--
Regards,
Laurent Pinchart
On Mon, 2021-11-15 at 09:53 +0100, Arnd Bergmann wrote:
> 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]>
> ---
Reviewed-by: Nicolas Saenz Julienne <[email protected]>
Regards,
Nicolas
Hi Arnd,
On Mon, Nov 15, 2021 at 9:55 AM Arnd Bergmann <[email protected]> wrote:
>
> 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(-)
This driver has been already deleted from the staging tree. See [0].
Best regards,
Sergio Paracuellos
[0]: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/commit/?h=staging-testing&id=5bfc10690c6c590a972be014ed8595e77e1e2dea
>
> diff --git a/drivers/staging/ralink-gdma/ralink-gdma.c b/drivers/staging/ralink-gdma/ralink-gdma.c
> index b5229bc6eae5..f00240e62e1b 100644
> --- a/drivers/staging/ralink-gdma/ralink-gdma.c
> +++ b/drivers/staging/ralink-gdma/ralink-gdma.c
> @@ -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
>
>
On 11/15/21 9:53 AM, Arnd Bergmann wrote:
> 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.
> 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 10:14 AM Laurent Pinchart
<[email protected]> wrote:
> On Mon, Nov 15, 2021 at 09:54:00AM +0100, Arnd Bergmann wrote:
> > @@ -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.
> > + * 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.
> > + 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;
> With these issues addressed,
>
> Reviewed-by: Laurent Pinchart <[email protected]>
Thanks,
Arnd
On Mon, Nov 15, 2021 at 10:55 AM Sergio Paracuellos
<[email protected]> wrote:
> On Mon, Nov 15, 2021 at 9:55 AM Arnd Bergmann <[email protected]> wrote:
> > ---
> > 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].
Ok, thanks! I'll just leave out the patch from future submissions, and remove
it completely once your commit hits mainline.
Arnd
On Mon, Nov 15, 2021 at 11:14 AM Lars-Peter Clausen <[email protected]> wrote:
>
> On 11/15/21 9:53 AM, Arnd Bergmann wrote:
> > 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.
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
Hi Arnd,
On Mon, Nov 15, 2021 at 11:21:30AM +0100, Arnd Bergmann wrote:
> On Mon, Nov 15, 2021 at 10:14 AM Laurent Pinchart wrote:
> > On Mon, Nov 15, 2021 at 09:54:00AM +0100, Arnd Bergmann wrote:
> > > @@ -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 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.
> I'll change s/Abuse/Use/ for the moment until I get a definite answer.
>
> > > + * 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.
>
> > > + 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;
How about
if (WARN_ON(config->peripheral_config &&
config->peripheral_size != sizeof(*pconfig)))
>
> spin_lock_irqsave(&chan->lock, flags);
> if (chan->id <= ZYNQMP_DPDMA_VIDEO2 &&
> config->peripheral_size == sizeof(*pconfig))
And here you can test pconfig != NULL.
> chan->video_group = pconfig->video_group;
> spin_unlock_irqrestore(&chan->lock, flags);
>
> return 0;
>
> > With these issues addressed,
> >
> > Reviewed-by: Laurent Pinchart <[email protected]>
--
Regards,
Laurent Pinchart
On 11/15/21 11:42 AM, Arnd Bergmann wrote:
> On Mon, Nov 15, 2021 at 11:14 AM Lars-Peter Clausen <[email protected]> wrote:
>> On 11/15/21 9:53 AM, Arnd Bergmann wrote:
>>> 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.
> 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
On Mon, Nov 15, 2021 at 12:49 PM Laurent Pinchart
<[email protected]> wrote:
> On Mon, Nov 15, 2021 at 11:21:30AM +0100, Arnd Bergmann wrote:
> > On Mon, Nov 15, 2021 at 10:14 AM Laurent Pinchart wrote:
> > > On Mon, Nov 15, 2021 at 09:54:00AM +0100, Arnd Bergmann wrote:
> > > > @@ -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 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.
> > /*
> > * 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)))
>
> >
> > 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.
Arnd
Hi Arnd,
On Mon, Nov 15, 2021 at 01:38:07PM +0100, Arnd Bergmann wrote:
> On Mon, Nov 15, 2021 at 12:49 PM Laurent Pinchart wrote:
> > On Mon, Nov 15, 2021 at 11:21:30AM +0100, Arnd Bergmann wrote:
> > > On Mon, Nov 15, 2021 at 10:14 AM Laurent Pinchart wrote:
> > > > On Mon, Nov 15, 2021 at 09:54:00AM +0100, Arnd Bergmann wrote:
> > > > > @@ -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 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.
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.
> 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.
It's only memory-to-device (video and audio). See figures 33-1 and 33-16
in https://www.xilinx.com/support/documentation/user_guides/ug1085-zynq-ultrascale-trm.pdf
> > > /*
> > > * 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)))
> >
> > >
> > > 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.
Sounds good to me.
--
Regards,
Laurent Pinchart
On Mon, 15 Nov 2021 at 09:55, Arnd Bergmann <[email protected]> wrote:
>
> 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]>
Acked-by: Ulf Hansson <[email protected]>
Kind regards
Uffe
> ---
> drivers/mmc/host/bcm2835.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/mmc/host/bcm2835.c b/drivers/mmc/host/bcm2835.c
> index 8c2361e66277..463b707d9e99 100644
> --- a/drivers/mmc/host/bcm2835.c
> +++ b/drivers/mmc/host/bcm2835.c
> @@ -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
>
On Mon, Nov 15, 2021 at 09:53:54AM +0100, Arnd Bergmann wrote:
> 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.
Acked-by: Mark Brown <[email protected]>
On Mon, Nov 15, 2021 at 2:05 PM Laurent Pinchart
<[email protected]> wrote:
> On Mon, Nov 15, 2021 at 01:38:07PM +0100, Arnd Bergmann wrote:
> > 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.
Ah, makes sense. In this case, I guess the data could have been
part of the DMA specifier after all, in a second cell after the
channel number.
Arnd
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 <[email protected]>
>> wrote:
>>> On 11/15/21 9:53 AM, Arnd Bergmann wrote:
>>>> 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.
>> 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 <[email protected]> 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
On 15-11-21, 11:21, Arnd Bergmann wrote:
> On Mon, Nov 15, 2021 at 10:14 AM Laurent Pinchart
> <[email protected]> wrote:
> > On Mon, Nov 15, 2021 at 09:54:00AM +0100, Arnd Bergmann wrote:
> > > @@ -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.
I would feel this is still not use for the peripheral_config, but lets
keep it to get rid of slave_id.
Also, I would be better if this was moved to DT as the next cell, don't
recall why that was not done/feasible.
--
~Vinod
Hi Arnd,
On Mon, Nov 15, 2021 at 4:55 PM Arnd Bergmann <[email protected]> wrote:
>
> 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]>
Thanks. LGTM.
Reviewed-by: Baolin Wang <[email protected]>
--
Baolin Wang