* [08/18] moxart_ether: pass struct device to DMA API functions
From: Christoph Hellwig @ 2019-02-01 8:47 UTC (permalink / raw)
To: John Crispin, Vinod Koul, Dmitry Tarnyagin, Nicolas Ferre,
Sudip Mukherjee, Felipe Balbi, linux-mips, linux-kernel,
dmaengine, netdev, linux-usb, linux-fbdev, alsa-devel
Cc: iommu
The DMA API generally relies on a struct device to work properly, and
only barely works without one for legacy reasons. Pass the easily
available struct device from the platform_device to remedy this.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/net/ethernet/moxa/moxart_ether.c | 11 +++++++----
drivers/net/ethernet/moxa/moxart_ether.h | 1 +
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c
index b34055ac476f..00dec0ffb11b 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.c
+++ b/drivers/net/ethernet/moxa/moxart_ether.c
@@ -81,11 +81,13 @@ static void moxart_mac_free_memory(struct net_device *ndev)
priv->rx_buf_size, DMA_FROM_DEVICE);
if (priv->tx_desc_base)
- dma_free_coherent(NULL, TX_REG_DESC_SIZE * TX_DESC_NUM,
+ dma_free_coherent(&priv->pdev->dev,
+ TX_REG_DESC_SIZE * TX_DESC_NUM,
priv->tx_desc_base, priv->tx_base);
if (priv->rx_desc_base)
- dma_free_coherent(NULL, RX_REG_DESC_SIZE * RX_DESC_NUM,
+ dma_free_coherent(&priv->pdev->dev,
+ RX_REG_DESC_SIZE * RX_DESC_NUM,
priv->rx_desc_base, priv->rx_base);
kfree(priv->tx_buf_base);
@@ -476,6 +478,7 @@ static int moxart_mac_probe(struct platform_device *pdev)
priv = netdev_priv(ndev);
priv->ndev = ndev;
+ priv->pdev = pdev;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
ndev->base_addr = res->start;
@@ -491,7 +494,7 @@ static int moxart_mac_probe(struct platform_device *pdev)
priv->tx_buf_size = TX_BUF_SIZE;
priv->rx_buf_size = RX_BUF_SIZE;
- priv->tx_desc_base = dma_alloc_coherent(NULL, TX_REG_DESC_SIZE *
+ priv->tx_desc_base = dma_alloc_coherent(&pdev->dev, TX_REG_DESC_SIZE *
TX_DESC_NUM, &priv->tx_base,
GFP_DMA | GFP_KERNEL);
if (!priv->tx_desc_base) {
@@ -499,7 +502,7 @@ static int moxart_mac_probe(struct platform_device *pdev)
goto init_fail;
}
- priv->rx_desc_base = dma_alloc_coherent(NULL, RX_REG_DESC_SIZE *
+ priv->rx_desc_base = dma_alloc_coherent(&pdev->dev, RX_REG_DESC_SIZE *
RX_DESC_NUM, &priv->rx_base,
GFP_DMA | GFP_KERNEL);
if (!priv->rx_desc_base) {
diff --git a/drivers/net/ethernet/moxa/moxart_ether.h b/drivers/net/ethernet/moxa/moxart_ether.h
index bee608b547d1..bf4c3029cd0c 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.h
+++ b/drivers/net/ethernet/moxa/moxart_ether.h
@@ -292,6 +292,7 @@
#define LINK_STATUS 0x4
struct moxart_mac_priv_t {
+ struct platform_device *pdev;
void __iomem *base;
unsigned int reg_maccr;
unsigned int reg_imr;
^ permalink raw reply related
* [07/18] pxa168_eth: pass struct device to DMA API functions
From: Christoph Hellwig @ 2019-02-01 8:47 UTC (permalink / raw)
To: John Crispin, Vinod Koul, Dmitry Tarnyagin, Nicolas Ferre,
Sudip Mukherjee, Felipe Balbi, linux-mips, linux-kernel,
dmaengine, netdev, linux-usb, linux-fbdev, alsa-devel
Cc: iommu
The DMA API generally relies on a struct device to work properly, and
only barely works without one for legacy reasons. Pass the easily
available struct device from the platform_device to remedy this.
Note that this driver seems to entirely lack dma_map_single error
handling, but that is left for another time.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/net/ethernet/marvell/pxa168_eth.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
index f8a6d6e3cb7a..35f2142aac5e 100644
--- a/drivers/net/ethernet/marvell/pxa168_eth.c
+++ b/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -201,6 +201,7 @@ struct tx_desc {
};
struct pxa168_eth_private {
+ struct platform_device *pdev;
int port_num; /* User Ethernet port number */
int phy_addr;
int phy_speed;
@@ -331,7 +332,7 @@ static void rxq_refill(struct net_device *dev)
used_rx_desc = pep->rx_used_desc_q;
p_used_rx_desc = &pep->p_rx_desc_area[used_rx_desc];
size = skb_end_pointer(skb) - skb->data;
- p_used_rx_desc->buf_ptr = dma_map_single(NULL,
+ p_used_rx_desc->buf_ptr = dma_map_single(&pep->pdev->dev,
skb->data,
size,
DMA_FROM_DEVICE);
@@ -743,7 +744,7 @@ static int txq_reclaim(struct net_device *dev, int force)
netdev_err(dev, "Error in TX\n");
dev->stats.tx_errors++;
}
- dma_unmap_single(NULL, addr, count, DMA_TO_DEVICE);
+ dma_unmap_single(&pep->pdev->dev, addr, count, DMA_TO_DEVICE);
if (skb)
dev_kfree_skb_irq(skb);
released++;
@@ -805,7 +806,7 @@ static int rxq_process(struct net_device *dev, int budget)
if (rx_next_curr_desc == rx_used_desc)
pep->rx_resource_err = 1;
pep->rx_desc_count--;
- dma_unmap_single(NULL, rx_desc->buf_ptr,
+ dma_unmap_single(&pep->pdev->dev, rx_desc->buf_ptr,
rx_desc->buf_size,
DMA_FROM_DEVICE);
received_packets++;
@@ -1274,7 +1275,8 @@ pxa168_eth_start_xmit(struct sk_buff *skb, struct net_device *dev)
length = skb->len;
pep->tx_skb[tx_index] = skb;
desc->byte_cnt = length;
- desc->buf_ptr = dma_map_single(NULL, skb->data, length, DMA_TO_DEVICE);
+ desc->buf_ptr = dma_map_single(&pep->pdev->dev, skb->data, length,
+ DMA_TO_DEVICE);
skb_tx_timestamp(skb);
@@ -1528,6 +1530,7 @@ static int pxa168_eth_probe(struct platform_device *pdev)
if (err)
goto err_free_mdio;
+ pep->pdev = pdev;
SET_NETDEV_DEV(dev, &pdev->dev);
pxa168_init_hw(pep);
err = register_netdev(dev);
^ permalink raw reply related
* [06/18] lantiq_etop: pass struct device to DMA API functions
From: Christoph Hellwig @ 2019-02-01 8:47 UTC (permalink / raw)
To: John Crispin, Vinod Koul, Dmitry Tarnyagin, Nicolas Ferre,
Sudip Mukherjee, Felipe Balbi, linux-mips, linux-kernel,
dmaengine, netdev, linux-usb, linux-fbdev, alsa-devel
Cc: iommu
The DMA API generally relies on a struct device to work properly, and
only barely works without one for legacy reasons. Pass the easily
available struct device from the platform_device to remedy this.
Note this driver seems to lack dma_unmap_* calls entirely, but fixing
that is left for another time.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/net/ethernet/lantiq_etop.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/lantiq_etop.c b/drivers/net/ethernet/lantiq_etop.c
index 32ac9045cdae..f9bb890733b5 100644
--- a/drivers/net/ethernet/lantiq_etop.c
+++ b/drivers/net/ethernet/lantiq_etop.c
@@ -112,10 +112,12 @@ struct ltq_etop_priv {
static int
ltq_etop_alloc_skb(struct ltq_etop_chan *ch)
{
+ struct ltq_etop_priv *priv = netdev_priv(ch->netdev);
+
ch->skb[ch->dma.desc] = netdev_alloc_skb(ch->netdev, MAX_DMA_DATA_LEN);
if (!ch->skb[ch->dma.desc])
return -ENOMEM;
- ch->dma.desc_base[ch->dma.desc].addr = dma_map_single(NULL,
+ ch->dma.desc_base[ch->dma.desc].addr = dma_map_single(&priv->pdev->dev,
ch->skb[ch->dma.desc]->data, MAX_DMA_DATA_LEN,
DMA_FROM_DEVICE);
ch->dma.desc_base[ch->dma.desc].addr =
@@ -487,7 +489,7 @@ ltq_etop_tx(struct sk_buff *skb, struct net_device *dev)
netif_trans_update(dev);
spin_lock_irqsave(&priv->lock, flags);
- desc->addr = ((unsigned int) dma_map_single(NULL, skb->data, len,
+ desc->addr = ((unsigned int) dma_map_single(&priv->pdev->dev, skb->data, len,
DMA_TO_DEVICE)) - byte_offset;
wmb();
desc->ctl = LTQ_DMA_OWN | LTQ_DMA_SOP | LTQ_DMA_EOP |
^ permalink raw reply related
* [05/18] macb_main: pass struct device to DMA API functions
From: Christoph Hellwig @ 2019-02-01 8:47 UTC (permalink / raw)
To: John Crispin, Vinod Koul, Dmitry Tarnyagin, Nicolas Ferre,
Sudip Mukherjee, Felipe Balbi, linux-mips, linux-kernel,
dmaengine, netdev, linux-usb, linux-fbdev, alsa-devel
Cc: iommu
The DMA API generally relies on a struct device to work properly, and
only barely works without one for legacy reasons. Pass the easily
available struct device from the platform_device to remedy this.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/net/ethernet/cadence/macb_main.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 2b2882615e8b..61a27963f1d1 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -3673,9 +3673,9 @@ static netdev_tx_t at91ether_start_xmit(struct sk_buff *skb,
/* Store packet information (to free when Tx completed) */
lp->skb = skb;
lp->skb_length = skb->len;
- lp->skb_physaddr = dma_map_single(NULL, skb->data, skb->len,
- DMA_TO_DEVICE);
- if (dma_mapping_error(NULL, lp->skb_physaddr)) {
+ lp->skb_physaddr = dma_map_single(&lp->pdev->dev, skb->data,
+ skb->len, DMA_TO_DEVICE);
+ if (dma_mapping_error(&lp->pdev->dev, lp->skb_physaddr)) {
dev_kfree_skb_any(skb);
dev->stats.tx_dropped++;
netdev_err(dev, "%s: DMA mapping error\n", __func__);
@@ -3765,7 +3765,7 @@ static irqreturn_t at91ether_interrupt(int irq, void *dev_id)
if (lp->skb) {
dev_kfree_skb_irq(lp->skb);
lp->skb = NULL;
- dma_unmap_single(NULL, lp->skb_physaddr,
+ dma_unmap_single(&lp->pdev->dev, lp->skb_physaddr,
lp->skb_length, DMA_TO_DEVICE);
dev->stats.tx_packets++;
dev->stats.tx_bytes += lp->skb_length;
^ permalink raw reply related
* [04/18] au1000_eth: pass struct device to DMA API functions
From: Christoph Hellwig @ 2019-02-01 8:47 UTC (permalink / raw)
To: John Crispin, Vinod Koul, Dmitry Tarnyagin, Nicolas Ferre,
Sudip Mukherjee, Felipe Balbi, linux-mips, linux-kernel,
dmaengine, netdev, linux-usb, linux-fbdev, alsa-devel
Cc: iommu
The DMA API generally relies on a struct device to work properly, and
only barely works without one for legacy reasons. Pass the easily
available struct device from the platform_device to remedy this.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/net/ethernet/amd/au1000_eth.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/amd/au1000_eth.c b/drivers/net/ethernet/amd/au1000_eth.c
index e833d1b3fe18..e5073aeea06a 100644
--- a/drivers/net/ethernet/amd/au1000_eth.c
+++ b/drivers/net/ethernet/amd/au1000_eth.c
@@ -1167,7 +1167,7 @@ static int au1000_probe(struct platform_device *pdev)
/* Allocate the data buffers
* Snooping works fine with eth on all au1xxx
*/
- aup->vaddr = (u32)dma_alloc_attrs(NULL, MAX_BUF_SIZE *
+ aup->vaddr = (u32)dma_alloc_attrs(&pdev->dev, MAX_BUF_SIZE *
(NUM_TX_BUFFS + NUM_RX_BUFFS),
&aup->dma_addr, 0,
DMA_ATTR_NON_CONSISTENT);
@@ -1349,7 +1349,7 @@ static int au1000_probe(struct platform_device *pdev)
err_remap2:
iounmap(aup->mac);
err_remap1:
- dma_free_attrs(NULL, MAX_BUF_SIZE * (NUM_TX_BUFFS + NUM_RX_BUFFS),
+ dma_free_attrs(&pdev->dev, MAX_BUF_SIZE * (NUM_TX_BUFFS + NUM_RX_BUFFS),
(void *)aup->vaddr, aup->dma_addr,
DMA_ATTR_NON_CONSISTENT);
err_vaddr:
@@ -1383,7 +1383,7 @@ static int au1000_remove(struct platform_device *pdev)
if (aup->tx_db_inuse[i])
au1000_ReleaseDB(aup, aup->tx_db_inuse[i]);
- dma_free_attrs(NULL, MAX_BUF_SIZE * (NUM_TX_BUFFS + NUM_RX_BUFFS),
+ dma_free_attrs(&pdev->dev, MAX_BUF_SIZE * (NUM_TX_BUFFS + NUM_RX_BUFFS),
(void *)aup->vaddr, aup->dma_addr,
DMA_ATTR_NON_CONSISTENT);
^ permalink raw reply related
* [03/18] net: caif: pass struct device to DMA API functions
From: Christoph Hellwig @ 2019-02-01 8:47 UTC (permalink / raw)
To: John Crispin, Vinod Koul, Dmitry Tarnyagin, Nicolas Ferre,
Sudip Mukherjee, Felipe Balbi, linux-mips, linux-kernel,
dmaengine, netdev, linux-usb, linux-fbdev, alsa-devel
Cc: iommu
The DMA API generally relies on a struct device to work properly, and
only barely works without one for legacy reasons. Pass the easily
available struct device from the platform_device to remedy this.
Also use the proper Kconfig symbol to check for DMA API availability.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/net/caif/caif_spi.c | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/drivers/net/caif/caif_spi.c b/drivers/net/caif/caif_spi.c
index d28a1398c091..b7f3e263b57c 100644
--- a/drivers/net/caif/caif_spi.c
+++ b/drivers/net/caif/caif_spi.c
@@ -73,35 +73,37 @@ MODULE_PARM_DESC(spi_down_tail_align, "SPI downlink tail alignment.");
#define LOW_WATER_MARK 100
#define HIGH_WATER_MARK (LOW_WATER_MARK*5)
-#ifdef CONFIG_UML
+#ifdef CONFIG_HAS_DMA
/*
* We sometimes use UML for debugging, but it cannot handle
* dma_alloc_coherent so we have to wrap it.
*/
-static inline void *dma_alloc(dma_addr_t *daddr)
+static inline void *dma_alloc(struct cfspi *cfspi, dma_addr_t *daddr)
{
return kmalloc(SPI_DMA_BUF_LEN, GFP_KERNEL);
}
-static inline void dma_free(void *cpu_addr, dma_addr_t handle)
+static inline void dma_free(struct cfspi *cfspi, void *cpu_addr,
+ dma_addr_t handle)
{
kfree(cpu_addr);
}
#else
-static inline void *dma_alloc(dma_addr_t *daddr)
+static inline void *dma_alloc(struct cfspi *cfspi, dma_addr_t *daddr)
{
- return dma_alloc_coherent(NULL, SPI_DMA_BUF_LEN, daddr,
+ return dma_alloc_coherent(&cfspi->pdev->dev, SPI_DMA_BUF_LEN, daddr,
GFP_KERNEL);
}
-static inline void dma_free(void *cpu_addr, dma_addr_t handle)
+static inline void dma_free(struct cfspi *cfspi, void *cpu_addr,
+ dma_addr_t handle)
{
- dma_free_coherent(NULL, SPI_DMA_BUF_LEN, cpu_addr, handle);
+ dma_free_coherent(&cfspi->pdev->dev, SPI_DMA_BUF_LEN, cpu_addr, handle);
}
-#endif /* CONFIG_UML */
+#endif /* CONFIG_HAS_DMA */
#ifdef CONFIG_DEBUG_FS
@@ -610,13 +612,13 @@ static int cfspi_init(struct net_device *dev)
}
/* Allocate DMA buffers. */
- cfspi->xfer.va_tx[0] = dma_alloc(&cfspi->xfer.pa_tx[0]);
+ cfspi->xfer.va_tx[0] = dma_alloc(cfspi, &cfspi->xfer.pa_tx[0]);
if (!cfspi->xfer.va_tx[0]) {
res = -ENODEV;
goto err_dma_alloc_tx_0;
}
- cfspi->xfer.va_rx = dma_alloc(&cfspi->xfer.pa_rx);
+ cfspi->xfer.va_rx = dma_alloc(cfspi, &cfspi->xfer.pa_rx);
if (!cfspi->xfer.va_rx) {
res = -ENODEV;
@@ -665,9 +667,9 @@ static int cfspi_init(struct net_device *dev)
return 0;
err_create_wq:
- dma_free(cfspi->xfer.va_rx, cfspi->xfer.pa_rx);
+ dma_free(cfspi, cfspi->xfer.va_rx, cfspi->xfer.pa_rx);
err_dma_alloc_rx:
- dma_free(cfspi->xfer.va_tx[0], cfspi->xfer.pa_tx[0]);
+ dma_free(cfspi, cfspi->xfer.va_tx[0], cfspi->xfer.pa_tx[0]);
err_dma_alloc_tx_0:
return res;
}
@@ -683,8 +685,8 @@ static void cfspi_uninit(struct net_device *dev)
cfspi->ndev = NULL;
/* Free DMA buffers. */
- dma_free(cfspi->xfer.va_rx, cfspi->xfer.pa_rx);
- dma_free(cfspi->xfer.va_tx[0], cfspi->xfer.pa_tx[0]);
+ dma_free(cfspi, cfspi->xfer.va_rx, cfspi->xfer.pa_rx);
+ dma_free(cfspi, cfspi->xfer.va_tx[0], cfspi->xfer.pa_tx[0]);
set_bit(SPI_TERMINATE, &cfspi->state);
wake_up_interruptible(&cfspi->wait);
destroy_workqueue(cfspi->wq);
^ permalink raw reply related
* [02/18] dmaengine: imx-sdma: pass struct device to DMA API functions
From: Christoph Hellwig @ 2019-02-01 8:47 UTC (permalink / raw)
To: John Crispin, Vinod Koul, Dmitry Tarnyagin, Nicolas Ferre,
Sudip Mukherjee, Felipe Balbi, linux-mips, linux-kernel,
dmaengine, netdev, linux-usb, linux-fbdev, alsa-devel
Cc: iommu
The DMA API generally relies on a struct device to work properly, and
only barely works without one for legacy reasons. Pass the easily
available struct device from the platform_device to remedy this.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/dma/imx-sdma.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 86708fb9bda1..0b6bba0b9f38 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -677,7 +677,7 @@ static int sdma_load_script(struct sdma_engine *sdma, void *buf, int size,
int ret;
unsigned long flags;
- buf_virt = dma_alloc_coherent(NULL, size, &buf_phys, GFP_KERNEL);
+ buf_virt = dma_alloc_coherent(sdma->dev, size, &buf_phys, GFP_KERNEL);
if (!buf_virt) {
return -ENOMEM;
}
@@ -696,7 +696,7 @@ static int sdma_load_script(struct sdma_engine *sdma, void *buf, int size,
spin_unlock_irqrestore(&sdma->channel_0_lock, flags);
- dma_free_coherent(NULL, size, buf_virt, buf_phys);
+ dma_free_coherent(sdma->dev, size, buf_virt, buf_phys);
return ret;
}
@@ -1182,8 +1182,8 @@ static int sdma_request_channel0(struct sdma_engine *sdma)
{
int ret = -EBUSY;
- sdma->bd0 = dma_alloc_coherent(NULL, PAGE_SIZE, &sdma->bd0_phys,
- GFP_NOWAIT);
+ sdma->bd0 = dma_alloc_coherent(sdma->dev, PAGE_SIZE, &sdma->bd0_phys,
+ GFP_NOWAIT);
if (!sdma->bd0) {
ret = -ENOMEM;
goto out;
@@ -1842,7 +1842,7 @@ static int sdma_init(struct sdma_engine *sdma)
/* Be sure SDMA has not started yet */
writel_relaxed(0, sdma->regs + SDMA_H_C0PTR);
- sdma->channel_control = dma_alloc_coherent(NULL,
+ sdma->channel_control = dma_alloc_coherent(sdma->dev,
MAX_DMA_CHANNELS * sizeof (struct sdma_channel_control) +
sizeof(struct sdma_context_data),
&ccb_phys, GFP_KERNEL);
^ permalink raw reply related
* [01/18] MIPS: lantiq: pass struct device to DMA API functions
From: Christoph Hellwig @ 2019-02-01 8:47 UTC (permalink / raw)
To: John Crispin, Vinod Koul, Dmitry Tarnyagin, Nicolas Ferre,
Sudip Mukherjee, Felipe Balbi, linux-mips, linux-kernel,
dmaengine, netdev, linux-usb, linux-fbdev, alsa-devel
Cc: iommu
The DMA API generally relies on a struct device to work properly, and
only barely works without one for legacy reasons. Pass the easily
available struct device from the platform_device to remedy this.
Also use GFP_KERNEL instead of GFP_ATOMIC as the gfp_t for the memory
allocation, as we aren't in interrupt context or under a lock.
Note that this whole function looks somewhat bogus given that we never
even look at the returned dma address, and the CPHYSADDR magic on
a returned noncached mapping looks "interesting". But I'll leave
that to people more familiar with the code to sort out.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/mips/lantiq/xway/vmmc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/mips/lantiq/xway/vmmc.c b/arch/mips/lantiq/xway/vmmc.c
index 577ec81b557d..3deab9a77718 100644
--- a/arch/mips/lantiq/xway/vmmc.c
+++ b/arch/mips/lantiq/xway/vmmc.c
@@ -31,8 +31,8 @@ static int vmmc_probe(struct platform_device *pdev)
dma_addr_t dma;
cp1_base =
- (void *) CPHYSADDR(dma_alloc_coherent(NULL, CP1_SIZE,
- &dma, GFP_ATOMIC));
+ (void *) CPHYSADDR(dma_alloc_coherent(&pdev->dev, CP1_SIZE,
+ &dma, GFP_KERNEL));
gpio_count = of_gpio_count(pdev->dev.of_node);
while (gpio_count > 0) {
^ permalink raw reply related
* [RFC,v3,1/7] dmaengine: Add Synopsys eDMA IP core driver
From: Vinod Koul @ 2019-02-01 4:14 UTC (permalink / raw)
To: Gustavo Pimentel
Cc: linux-pci@vger.kernel.org, dmaengine@vger.kernel.org,
Dan Williams, Eugeniy Paltsev, Andy Shevchenko, Russell King,
Niklas Cassel, Joao Pinto, Jose Abreu, Luis Oliveira,
Vitor Soares, Nelson Costa, Pedro Sousa
On 31-01-19, 11:33, Gustavo Pimentel wrote:
> On 23/01/2019 13:08, Vinod Koul wrote:
> > On 21-01-19, 15:48, Gustavo Pimentel wrote:
> >> On 20/01/2019 11:44, Vinod Koul wrote:
> >>> On 11-01-19, 19:33, Gustavo Pimentel wrote:
> >>>> @@ -0,0 +1,1059 @@
> >>>> +// SPDX-License-Identifier: GPL-2.0
> >>>> +/*
> >>>> + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> >>>
> >>> 2019 now
> >>
> >> I've changed to "Copyright (c) 2018-present Synopsys, Inc. and/or its
> >> affiliates." this way it's always up to date and I also kept 2018, because it
> >> was the date that I started to develop this driver, if you don't mind.
> >
> > yeah 18 is fine :) it need to end with current year always
>
> Just to be sure, are you saying that must be: "Copyright (c) 2018-2019 Synopsys,
> Inc. and/or its affiliates."?
Yup :)
> >>>> +static struct dw_edma_chunk *dw_edma_alloc_chunk(struct dw_edma_desc *desc)
> >>>> +{
> >>>> + struct dw_edma_chan *chan = desc->chan;
> >>>> + struct dw_edma *dw = chan->chip->dw;
> >>>> + struct dw_edma_chunk *chunk;
> >>>> +
> >>>> + chunk = kvzalloc(sizeof(*chunk), GFP_NOWAIT);
> >>>> + if (unlikely(!chunk))
> >>>> + return NULL;
> >>>> +
> >>>> + INIT_LIST_HEAD(&chunk->list);
> >>>> + chunk->chan = chan;
> >>>> + chunk->cb = !(desc->chunks_alloc % 2);
> >>>
> >>> cb ..?
> >>
> >> CB = change bit, is a property of this eDMA IP. Basically it is a kind of
> >> handshake which serves to validate whether the linked list has been updated or
> >> not, especially useful in cases of recycled linked list elements (every linked
> >> list recycle is a new chunk, this will allow to differentiate each chunk).
> >
> > okay please add that somewhere. Also it would help me if you explain
> > what is chunk and other terminologies used in this driver
>
> I'm thinking to put the below description on the patch, please check if this is
> sufficient explicit and clear to understand what this IP needs and does.
>
> In order to transfer data from point A to B as fast as possible this IP requires
> a dedicated memory space where will reside a linked list of elements.
rephrasing: a dedicated memory space containing linked list of elements
> All elements of this linked list are continuous and each one describes a data
> transfer (source and destination addresses, length and a control variable).
>
> For the sake of simplicity, lets assume a memory space for channel write 0 which
> allows about 42 elements.
>
> +---------+
> | Desc #0 |--+
> +---------+ |
> V
> +----------+
> | Chunk #0 |---+
> | CB = 1 | | +----------+ +-----+ +-----------+ +-----+
> +----------+ +-->| Burst #0 |-->| ... |-->| Burst #41 |-->| llp |
> | +----------+ +-----+ +-----------+ +-----+
> V
> +----------+
> | Chunk #1 |---+
> | CB = 0 | | +-----------+ +-----+ +-----------+ +-----+
> +----------+ +-->| Burst #42 |-->| ... |-->| Burst #83 |-->| llp |
> | +-----------+ +-----+ +-----------+ +-----+
> V
> +----------+
> | Chunk #2 |---+
> | CB = 1 | | +-----------+ +-----+ +------------+ +-----+
> +----------+ +-->| Burst #84 |-->| ... |-->| Burst #125 |-->| llp |
> | +-----------+ +-----+ +------------+ +-----+
> V
> +----------+
> | Chunk #3 |---+
> | CB = 0 | | +------------+ +-----+ +------------+ +-----+
> +----------+ +-->| Burst #126 |-->| ... |-->| Burst #129 |-->| llp |
> +------------+ +-----+ +------------+ +-----+
This is great and required to understand the driver.
How does controller move from Burnst #41 of Chunk 0 to Chunk 1 ?
Is Burst 0 to 129 a link list with Burst 0, 42, 84 and 126 having a
callback (done bit set)..
This sound **very** similar to dw dma concepts!
>
> Legend:
>
> *Linked list*, also know as Chunk
> *Linked list element*, also know as Burst
> *CB*, also know as Change Bit, it's a control bit (and typically is toggled)
> that allows to easily identify and differentiate between the current linked list
> and the previous or the next one.
> *LLP*, is a special element that indicates the end of the linked list element
> stream also informs that the next CB should be toggle.
>
> On scatter-gather transfer mode, the client will submit a scatter-gather list of
> n (on this case 130) elements, that will be divide in multiple Chunks, each
> Chunk will have (on this case 42) a limited number of Bursts and after
> transferring all Bursts, an interrupt will be triggered, which will allow to
> recycle the all linked list dedicated memory again with the new information
> relative to the next Chunk and respective Burst associated and repeat the whole
> cycle again.
>
> On cyclic transfer mode, the client will submit a buffer pointer, length of it
> and number of repetitions, in this case each burst will correspond directly to
> each repetition.
>
> Each Burst can describes a data transfer from point A(source) to point
> B(destination) with a length that can be from 1 byte up to 4 GB. Since dedicated
> the memory space where the linked list will reside is limited, the whole n burst
> elements will be organized in several Chunks, that will be used later to recycle
> the dedicated memory space to initiate a new sequence of data transfers.
>
> The whole transfer is considered has completed when it was transferred all bursts.
>
> Currently this IP has a set well-known register map, which includes support for
> legacy and unroll modes. Legacy mode is version of this register map that has
whats unroll..
> multiplexer register that allows to switch registers between all write and read
> channels and the unroll modes repeats all write and read channels registers with
> an offset between them. This register map is called v0.
>
> The IP team is creating a new register map more suitable to the latest PCIe
> features, that very likely will change the map register, which this version will
> be called v1. As soon as this new version is released by the IP team the support
> for this version in be included on this driver.
>
> What do you think? There is any gray area that I could clarify?
This sounds good. But we are also catering to a WIP IP which can change
right. Doesnt sound very good idea to me :)
> >>>> + dev_dbg(chan2dev(chan), "addr(physical) src=%pa, dst=%pa\n",
> >>>> + &config->src_addr, &config->dst_addr);
> >>>> +
> >>>> + chan->src_addr = config->src_addr;
> >>>> + chan->dst_addr = config->dst_addr;
> >>>> +
> >>>> + err = ops->device_config(dchan);
> >>>
> >>> what does this do?
> >>
> >> This is an initialization procedure to setup interrupts (data and addresses) to
> >> each channel on the eDMA IP, in order to be triggered after transfer being
> >> completed or aborted. Due the fact the config() can be called at anytime,
> >> doesn't make sense to have this procedure here, I'll moved it to probe().
> >
> > Yeah I am not still convinced about having another layer! Have you
> > though about using common lib for common parts ..?
>
> Maybe I'm explaining myself wrongly. I don't have any clue about the new
> register map for the future versions. I honestly tried to implement the common
> lib for the whole process that interact with dma engine controller to ease in
> the future any new addition of register mapping, by having this common callbacks
> that will only be responsible for interfacing the HW accordingly to register map
> version. Maybe I can simplify something in the future, but I only be able to
> conclude that after having some idea about the new register map.
>
> IMHO I think this is the easier and clean way to do it, in terms of code
> maintenance and architecture, but if you have another idea that you can show me
> or pointing out for a driver that implements something similar, I'm no problem
> to check it out.
That is what my fear was :)
Lets step back and solve one problem at a time. Right now that is v0 of
IP. Please write a simple driver which solve v0 without any layers
involved.
Once v1 is in good shape, you would know what it required and then we
can split v0 driver into common lib and v0 driver and then add v1
driver.
> >>> what is the second part of the check, can you explain that, who sets
> >>> chan->dir?
> >>
> >> The chan->dir is set on probe() during the process of configuring each channel.
> >
> > So you have channels that are unidirectional?
>
> Yes. That's one another reason IMHO to keep the dw-edma-test separate from
> dma-test, since this IP is more picky and have this particularities.
That is okay, that should be handled by prep calls, if you get a prep
call for direction you dont support return error and move to next
available one.
That can be done generically in dmatest driver and to answer your
question, yes I would test that :)
> >>>> +int dw_edma_probe(struct dw_edma_chip *chip)
> >>>> +{
> >>>> + struct dw_edma *dw = chip->dw;
> >>>> + struct device *dev = chip->dev;
> >>>> + const struct dw_edma_core_ops *ops;
> >>>> + size_t ll_chunk = dw->ll_region.sz;
> >>>> + size_t dt_chunk = dw->dt_region.sz;
> >>>> + u32 ch_tot;
> >>>> + int i, j, err;
> >>>> +
> >>>> + raw_spin_lock_init(&dw->lock);
> >>>> +
> >>>> + /* Callback operation selection accordingly to eDMA version */
> >>>> + switch (dw->version) {
> >>>> + default:
> >>>> + dev_err(dev, "unsupported version\n");
> >>>> + return -EPERM;
> >>>> + }
> >>>
> >>> So we have only one case which returns error, was this code tested?
> >>
> >> Yes it was, but I understand what your point of view.
> >> This was done like this, because I wanna to segment the patch series like this:
> >> 1) Adding eDMA driver core, which contains the driver skeleton and the whole
> >> logic associated.
> >> 2) and 3) Adding the callbacks for the eDMA register mapping version 0 (it will
> >> appear in the future a new version, I thought that this new version would came
> >> while I was trying to get the feedback about this patch series, therefore would
> >> have another 2 patches for the version 1 isolated and independent from the
> >> version 0).
> >> 4) and 5) Adding the PCIe glue-logic and device ID associated.
> >> 6) Adding maintainer for this driver.
> >> 7) Adding a test driver.
> >>
> >> Since this switch will only have the associated case on patch 2, that why on
> >> patch 1 doesn't appear any possibility.
> >>
> >> If you feel logic to squash patch 2 with patch 1, just say something and I'll do
> >> it for you :)
> >
> > well each patch should build and work on its own, otherwise we get
> > problems :) But since this is a new driver it is okay. Anyway this patch
> > is quite _huge_ so lets not add more to it..
> >
> > I would have moved the callback check to subsequent one..
>
> Sorry. I didn't catch this. Can you explain it?
I would have moved these lines to next patch to give them right meaning,
here it feels wrong:
switch (dw->version) {
case foo:
...
default:
...
}
^ permalink raw reply
* [RFC,v3,1/7] dmaengine: Add Synopsys eDMA IP core driver
From: Gustavo Pimentel @ 2019-01-31 11:33 UTC (permalink / raw)
To: Vinod Koul, Gustavo Pimentel
Cc: linux-pci@vger.kernel.org, dmaengine@vger.kernel.org,
Dan Williams, Eugeniy Paltsev, Andy Shevchenko, Russell King,
Niklas Cassel, Joao Pinto, Jose Abreu, Luis Oliveira,
Vitor Soares, Nelson Costa, Pedro Sousa
On 23/01/2019 13:08, Vinod Koul wrote:
> On 21-01-19, 15:48, Gustavo Pimentel wrote:
>> On 20/01/2019 11:44, Vinod Koul wrote:
>>> On 11-01-19, 19:33, Gustavo Pimentel wrote:
>>>> Add Synopsys eDMA IP core driver to kernel.
>>>>
>>>> This core driver, initializes and configures the eDMA IP using vma-helpers
>>>> functions and dma-engine subsystem.
>>>
>>> A description of eDMA IP will help review the driver
>>
>> I've the IP description on the cover-letter, but I'll bring it to this patch, if
>> it helps.
>
> yeah cover doesnt get applied, changelog are very important
> documentation for kernel
Ok. See below.
>
>>>> @@ -0,0 +1,1059 @@
>>>> +// SPDX-License-Identifier: GPL-2.0
>>>> +/*
>>>> + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
>>>
>>> 2019 now
>>
>> I've changed to "Copyright (c) 2018-present Synopsys, Inc. and/or its
>> affiliates." this way it's always up to date and I also kept 2018, because it
>> was the date that I started to develop this driver, if you don't mind.
>
> yeah 18 is fine :) it need to end with current year always
Just to be sure, are you saying that must be: "Copyright (c) 2018-2019 Synopsys,
Inc. and/or its affiliates."?
>
>>>> +static struct dw_edma_chunk *dw_edma_alloc_chunk(struct dw_edma_desc *desc)
>>>> +{
>>>> + struct dw_edma_chan *chan = desc->chan;
>>>> + struct dw_edma *dw = chan->chip->dw;
>>>> + struct dw_edma_chunk *chunk;
>>>> +
>>>> + chunk = kvzalloc(sizeof(*chunk), GFP_NOWAIT);
>>>> + if (unlikely(!chunk))
>>>> + return NULL;
>>>> +
>>>> + INIT_LIST_HEAD(&chunk->list);
>>>> + chunk->chan = chan;
>>>> + chunk->cb = !(desc->chunks_alloc % 2);
>>>
>>> cb ..?
>>
>> CB = change bit, is a property of this eDMA IP. Basically it is a kind of
>> handshake which serves to validate whether the linked list has been updated or
>> not, especially useful in cases of recycled linked list elements (every linked
>> list recycle is a new chunk, this will allow to differentiate each chunk).
>
> okay please add that somewhere. Also it would help me if you explain
> what is chunk and other terminologies used in this driver
I'm thinking to put the below description on the patch, please check if this is
sufficient explicit and clear to understand what this IP needs and does.
In order to transfer data from point A to B as fast as possible this IP requires
a dedicated memory space where will reside a linked list of elements.
All elements of this linked list are continuous and each one describes a data
transfer (source and destination addresses, length and a control variable).
For the sake of simplicity, lets assume a memory space for channel write 0 which
allows about 42 elements.
+---------+
| Desc #0 |--+
+---------+ |
V
+----------+
| Chunk #0 |---+
| CB = 1 | | +----------+ +-----+ +-----------+ +-----+
+----------+ +-->| Burst #0 |-->| ... |-->| Burst #41 |-->| llp |
| +----------+ +-----+ +-----------+ +-----+
V
+----------+
| Chunk #1 |---+
| CB = 0 | | +-----------+ +-----+ +-----------+ +-----+
+----------+ +-->| Burst #42 |-->| ... |-->| Burst #83 |-->| llp |
| +-----------+ +-----+ +-----------+ +-----+
V
+----------+
| Chunk #2 |---+
| CB = 1 | | +-----------+ +-----+ +------------+ +-----+
+----------+ +-->| Burst #84 |-->| ... |-->| Burst #125 |-->| llp |
| +-----------+ +-----+ +------------+ +-----+
V
+----------+
| Chunk #3 |---+
| CB = 0 | | +------------+ +-----+ +------------+ +-----+
+----------+ +-->| Burst #126 |-->| ... |-->| Burst #129 |-->| llp |
+------------+ +-----+ +------------+ +-----+
Legend:
*Linked list*, also know as Chunk
*Linked list element*, also know as Burst
*CB*, also know as Change Bit, it's a control bit (and typically is toggled)
that allows to easily identify and differentiate between the current linked list
and the previous or the next one.
*LLP*, is a special element that indicates the end of the linked list element
stream also informs that the next CB should be toggle.
On scatter-gather transfer mode, the client will submit a scatter-gather list of
n (on this case 130) elements, that will be divide in multiple Chunks, each
Chunk will have (on this case 42) a limited number of Bursts and after
transferring all Bursts, an interrupt will be triggered, which will allow to
recycle the all linked list dedicated memory again with the new information
relative to the next Chunk and respective Burst associated and repeat the whole
cycle again.
On cyclic transfer mode, the client will submit a buffer pointer, length of it
and number of repetitions, in this case each burst will correspond directly to
each repetition.
Each Burst can describes a data transfer from point A(source) to point
B(destination) with a length that can be from 1 byte up to 4 GB. Since dedicated
the memory space where the linked list will reside is limited, the whole n burst
elements will be organized in several Chunks, that will be used later to recycle
the dedicated memory space to initiate a new sequence of data transfers.
The whole transfer is considered has completed when it was transferred all bursts.
Currently this IP has a set well-known register map, which includes support for
legacy and unroll modes. Legacy mode is version of this register map that has
multiplexer register that allows to switch registers between all write and read
channels and the unroll modes repeats all write and read channels registers with
an offset between them. This register map is called v0.
The IP team is creating a new register map more suitable to the latest PCIe
features, that very likely will change the map register, which this version will
be called v1. As soon as this new version is released by the IP team the support
for this version in be included on this driver.
What do you think? There is any gray area that I could clarify?
>
>>>> +static int dw_edma_device_config(struct dma_chan *dchan,
>>>> + struct dma_slave_config *config)
>>>> +{
>>>> + struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
>>>> + const struct dw_edma_core_ops *ops = chan2ops(chan);
>>>> + unsigned long flags;
>>>> + int err = 0;
>>>> +
>>>> + spin_lock_irqsave(&chan->vc.lock, flags);
>>>> +
>>>> + if (!config) {
>>>> + err = -EINVAL;
>>>> + goto err_config;
>>>> + }
>>>> +
>>>> + if (chan->status != EDMA_ST_IDLE) {
>>>> + dev_err(chan2dev(chan), "channel is busy or paused\n");
>>>> + err = -EPERM;
>>>
>>> this is not correct behaviour, device_config can be called anytime and
>>> values can take affect on next transaction submitted..
>>
>> Hum, I thought we could only reconfigure after transfer being finished.
>
> Nope, anytime. They take effect for next prepare when you use it
>
>>>> + dev_dbg(chan2dev(chan), "addr(physical) src=%pa, dst=%pa\n",
>>>> + &config->src_addr, &config->dst_addr);
>>>> +
>>>> + chan->src_addr = config->src_addr;
>>>> + chan->dst_addr = config->dst_addr;
>>>> +
>>>> + err = ops->device_config(dchan);
>>>
>>> what does this do?
>>
>> This is an initialization procedure to setup interrupts (data and addresses) to
>> each channel on the eDMA IP, in order to be triggered after transfer being
>> completed or aborted. Due the fact the config() can be called at anytime,
>> doesn't make sense to have this procedure here, I'll moved it to probe().
>
> Yeah I am not still convinced about having another layer! Have you
> though about using common lib for common parts ..?
Maybe I'm explaining myself wrongly. I don't have any clue about the new
register map for the future versions. I honestly tried to implement the common
lib for the whole process that interact with dma engine controller to ease in
the future any new addition of register mapping, by having this common callbacks
that will only be responsible for interfacing the HW accordingly to register map
version. Maybe I can simplify something in the future, but I only be able to
conclude that after having some idea about the new register map.
IMHO I think this is the easier and clean way to do it, in terms of code
maintenance and architecture, but if you have another idea that you can show me
or pointing out for a driver that implements something similar, I'm no problem
to check it out.
>
>>> this looks incorrect interpretation to me. The status is to be retrieved
>>> for the given cookie passed and given that you do not even use this
>>> argument tells me that you have understood this as 'channel' status
>>> reporting, which is not correct
>>
>> Yes, you're right, my interpretation assumes this function reports
>> channel/transaction status. What would be the correct implementation?
>> Something like this?
>>
>> {
>> struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
>> const struct dw_edma_core_ops *ops = chan2ops(chan);
>> struct dw_edma_desc *desc;
>> struct virt_dma_desc *vd;
>> unsigned long flags;
>> enum dma_status ret;
>> u32 residue = 0;
>>
>> spin_lock_irqsave(&chan->vc.lock, flags);
>>
>> ret = dma_cookie_status(chan, cookie, txstate);
>> if (ret == DMA_COMPLETE)
>> goto ret_status;
>>
>> vd = vchan_next_desc(&chan->vc);
>> if (!vd)
>> goto ret_status;
>>
>> desc = vd2dw_edma_desc(vd);
>> if (!desc)
>> residue = desc->alloc_sz - desc->xfer_sz;
>>
>> if (ret == DMA_IN_PROGRESS && chan->status == EDMA_ST_PAUSE)
>> ret = DMA_PAUSED;
>
> this looks better, please do keep in mind txstate can be null, so
> residue caln can be skipped
Ok.
>
>>>> +static struct dma_async_tx_descriptor *
>>>> +dw_edma_device_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
>>>> + unsigned int sg_len,
>>>> + enum dma_transfer_direction direction,
>>>> + unsigned long flags, void *context)
>>>> +{
>>>> + struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
>>>> + struct dw_edma_desc *desc;
>>>> + struct dw_edma_chunk *chunk;
>>>> + struct dw_edma_burst *burst;
>>>> + struct scatterlist *sg;
>>>> + unsigned long sflags;
>>>> + phys_addr_t src_addr;
>>>> + phys_addr_t dst_addr;
>>>> + int i;
>>>> +
>>>> + if (sg_len < 1) {
>>>> + dev_err(chan2dev(chan), "invalid sg length %u\n", sg_len);
>>>> + return NULL;
>>>> + }
>>>> +
>>>> + if (direction == DMA_DEV_TO_MEM && chan->dir == EDMA_DIR_WRITE) {
>>>
>>> what is the second part of the check, can you explain that, who sets
>>> chan->dir?
>>
>> The chan->dir is set on probe() during the process of configuring each channel.
>
> So you have channels that are unidirectional?
Yes. That's one another reason IMHO to keep the dw-edma-test separate from
dma-test, since this IP is more picky and have this particularities.
This don't invalidate to add in the future, similar features to dma-test that
are generic enough to work with other IPs, at least this is my opinion.
>
>>>> + dev_dbg(chan2dev(chan), "prepare operation (WRITE)\n");
>>>> + } else if (direction == DMA_MEM_TO_DEV && chan->dir == EDMA_DIR_READ) {
>>>> + dev_dbg(chan2dev(chan), "prepare operation (READ)\n");
>>>> + } else {
>>>> + dev_err(chan2dev(chan), "invalid direction\n");
>>>> + return NULL;
>>>> + }
>>>> +
>>>> + if (!chan->configured) {
>>>> + dev_err(chan2dev(chan), "(prep_slave_sg) channel not configured\n");
>>>> + return NULL;
>>>> + }
>>>> +
>>>> + if (chan->status != EDMA_ST_IDLE) {
>>>> + dev_err(chan2dev(chan), "channel is busy or paused\n");
>>>> + return NULL;
>>>> + }
>>>
>>> No, wrong again. The txn must be prepared and then on submit added to a
>>> queue. You are writing a driver for dmaengine, surely you dont expect
>>> the channel to be free and then do a txn.. that would be very
>>> inefficient!
>>
>> I did not realize that the flow could be as you mentioned. The documentation I
>> read about the subsystem did not give me this idea. Thank you for clarifying me.
>
> I think we have improved that part a lot, please do feel free to point
> out inconsistency
> See DMA usage in Documentation/driver-api/dmaengine/client.rst
>
>>>> +int dw_edma_probe(struct dw_edma_chip *chip)
>>>> +{
>>>> + struct dw_edma *dw = chip->dw;
>>>> + struct device *dev = chip->dev;
>>>> + const struct dw_edma_core_ops *ops;
>>>> + size_t ll_chunk = dw->ll_region.sz;
>>>> + size_t dt_chunk = dw->dt_region.sz;
>>>> + u32 ch_tot;
>>>> + int i, j, err;
>>>> +
>>>> + raw_spin_lock_init(&dw->lock);
>>>> +
>>>> + /* Callback operation selection accordingly to eDMA version */
>>>> + switch (dw->version) {
>>>> + default:
>>>> + dev_err(dev, "unsupported version\n");
>>>> + return -EPERM;
>>>> + }
>>>
>>> So we have only one case which returns error, was this code tested?
>>
>> Yes it was, but I understand what your point of view.
>> This was done like this, because I wanna to segment the patch series like this:
>> 1) Adding eDMA driver core, which contains the driver skeleton and the whole
>> logic associated.
>> 2) and 3) Adding the callbacks for the eDMA register mapping version 0 (it will
>> appear in the future a new version, I thought that this new version would came
>> while I was trying to get the feedback about this patch series, therefore would
>> have another 2 patches for the version 1 isolated and independent from the
>> version 0).
>> 4) and 5) Adding the PCIe glue-logic and device ID associated.
>> 6) Adding maintainer for this driver.
>> 7) Adding a test driver.
>>
>> Since this switch will only have the associated case on patch 2, that why on
>> patch 1 doesn't appear any possibility.
>>
>> If you feel logic to squash patch 2 with patch 1, just say something and I'll do
>> it for you :)
>
> well each patch should build and work on its own, otherwise we get
> problems :) But since this is a new driver it is okay. Anyway this patch
> is quite _huge_ so lets not add more to it..
>
> I would have moved the callback check to subsequent one..
Sorry. I didn't catch this. Can you explain it?
>
>>>> + pm_runtime_get_sync(dev);
>>>> +
>>>> + /* Find out how many write channels are supported by hardware */
>>>> + dw->wr_ch_cnt = ops->ch_count(dw, EDMA_DIR_WRITE);
>>>> + if (!dw->wr_ch_cnt) {
>>>> + dev_err(dev, "invalid number of write channels(0)\n");
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + /* Find out how many read channels are supported by hardware */
>>>> + dw->rd_ch_cnt = ops->ch_count(dw, EDMA_DIR_READ);
>>>> + if (!dw->rd_ch_cnt) {
>>>> + dev_err(dev, "invalid number of read channels(0)\n");
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + dev_dbg(dev, "Channels:\twrite=%d, read=%d\n",
>>>> + dw->wr_ch_cnt, dw->rd_ch_cnt);
>>>> +
>>>> + ch_tot = dw->wr_ch_cnt + dw->rd_ch_cnt;
>>>> +
>>>> + /* Allocate channels */
>>>> + dw->chan = devm_kcalloc(dev, ch_tot, sizeof(*dw->chan), GFP_KERNEL);
>>>
>>> you may use struct_size() here
>>
>> Hum, this would be useful if I wanted to allocate the dw struct as well, right?
>> Since dw struct is already allocated, it looks like this can't be used, or am I
>> missing something?
>
> yeah you can allocate dw + chan one shot...
I'm allocating dw struct on the PCIe glue-logic driver in order to set it later
some data that will be useful here and here I'm only adding the channels. That's
why I can't allocate all in one shot.
>
^ permalink raw reply
* [v1] dmaengine: dmatest: Abort test in case of mapping error
From: Andy Shevchenko @ 2019-01-30 19:48 UTC (permalink / raw)
To: Vinod Koul, dmaengine, Seraj Alijan; +Cc: Andy Shevchenko, Dan Williams
In case of mapping error the DMA addresses are invalid and continuing
will screw system memory or potentially something else.
[ 222.480310] dmatest: dma0chan7-copy0: summary 1 tests, 3 failures 6 iops 349 KB/s (0)
...
[ 240.912725] check: Corrupted low memory at 00000000c7c75ac9 (2940 phys) = 5656000000000000
[ 240.921998] check: Corrupted low memory at 000000005715a1cd (2948 phys) = 279f2aca5595ab2b
[ 240.931280] check: Corrupted low memory at 000000002f4024c0 (2950 phys) = 5e5624f349e793cf
...
Abort any test if mapping failed.
Fixes: 4076e755dbec ("dmatest: convert to dmaengine_unmap_data")
Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/dma/dmatest.c | 32 ++++++++++++++------------------
1 file changed, 14 insertions(+), 18 deletions(-)
diff --git a/drivers/dma/dmatest.c b/drivers/dma/dmatest.c
index 2eea4ef72915..6511928b4cdf 100644
--- a/drivers/dma/dmatest.c
+++ b/drivers/dma/dmatest.c
@@ -711,11 +711,9 @@ static int dmatest_func(void *data)
srcs[i] = um->addr[i] + src_off;
ret = dma_mapping_error(dev->dev, um->addr[i]);
if (ret) {
- dmaengine_unmap_put(um);
result("src mapping error", total_tests,
src_off, dst_off, len, ret);
- failed_tests++;
- continue;
+ goto error_unmap_continue;
}
um->to_cnt++;
}
@@ -730,11 +728,9 @@ static int dmatest_func(void *data)
DMA_BIDIRECTIONAL);
ret = dma_mapping_error(dev->dev, dsts[i]);
if (ret) {
- dmaengine_unmap_put(um);
result("dst mapping error", total_tests,
src_off, dst_off, len, ret);
- failed_tests++;
- continue;
+ goto error_unmap_continue;
}
um->bidi_cnt++;
}
@@ -762,12 +758,10 @@ static int dmatest_func(void *data)
}
if (!tx) {
- dmaengine_unmap_put(um);
result("prep error", total_tests, src_off,
dst_off, len, ret);
msleep(100);
- failed_tests++;
- continue;
+ goto error_unmap_continue;
}
done->done = false;
@@ -776,12 +770,10 @@ static int dmatest_func(void *data)
cookie = tx->tx_submit(tx);
if (dma_submit_error(cookie)) {
- dmaengine_unmap_put(um);
result("submit error", total_tests, src_off,
dst_off, len, ret);
msleep(100);
- failed_tests++;
- continue;
+ goto error_unmap_continue;
}
dma_async_issue_pending(chan);
@@ -790,22 +782,20 @@ static int dmatest_func(void *data)
status = dma_async_is_tx_complete(chan, cookie, NULL, NULL);
- dmaengine_unmap_put(um);
-
if (!done->done) {
result("test timed out", total_tests, src_off, dst_off,
len, 0);
- failed_tests++;
- continue;
+ goto error_unmap_continue;
} else if (status != DMA_COMPLETE) {
result(status == DMA_ERROR ?
"completion error status" :
"completion busy status", total_tests, src_off,
dst_off, len, ret);
- failed_tests++;
- continue;
+ goto error_unmap_continue;
}
+ dmaengine_unmap_put(um);
+
if (params->noverify) {
verbose_result("test passed", total_tests, src_off,
dst_off, len, 0);
@@ -846,6 +836,12 @@ static int dmatest_func(void *data)
verbose_result("test passed", total_tests, src_off,
dst_off, len, 0);
}
+
+ continue;
+
+error_unmap_continue:
+ dmaengine_unmap_put(um);
+ failed_tests++;
}
ktime = ktime_sub(ktime_get(), ktime);
ktime = ktime_sub(ktime, comparetime);
^ permalink raw reply related
* [1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id
From: Arnd Bergmann @ 2019-01-30 16:51 UTC (permalink / raw)
To: Baolin Wang
Cc: Vinod Koul, Rob Herring, Mark Rutland, Olof Johansson, Orson Zhai,
Lyra Zhang, Dan Williams, DTML, arm-soc, Linux ARM,
Linux Kernel Mailing List, dmaengine, eric.long, Mark Brown
On Tue, Jan 22, 2019 at 2:21 PM Baolin Wang <baolin.wang@linaro.org> wrote:
>
> The DMA engine clients can trigger DMA engine automatically by setting
> the corresponding hardware slave id for the DMA engine. Thus add one
> cell to present the hardware slave id for DMA clients.
>
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
> ---
> Documentation/devicetree/bindings/dma/sprd-dma.txt | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/dma/sprd-dma.txt b/Documentation/devicetree/bindings/dma/sprd-dma.txt
> index 7a10fea..7812cf0 100644
> --- a/Documentation/devicetree/bindings/dma/sprd-dma.txt
> +++ b/Documentation/devicetree/bindings/dma/sprd-dma.txt
> @@ -6,8 +6,8 @@ Required properties:
> - compatible: Should be "sprd,sc9860-dma".
> - reg: Should contain DMA registers location and length.
> - interrupts: Should contain one interrupt shared by all channel.
> -- #dma-cells: must be <1>. Used to represent the number of integer
> - cells in the dmas property of client device.
> +- #dma-cells: must be <2>. Used to represent the channel id and slave id
> + of integer cells in the dmas property of client device.
> - #dma-channels : Number of DMA channels supported. Should be 32.
> - clock-names: Should contain the clock of the DMA controller.
> - clocks: Should contain a clock specifier for each entry in clock-names.
> @@ -28,14 +28,16 @@ apdma: dma-controller@20100000 {
>
> Client:
> DMA clients connected to the Spreadtrum DMA controller must use the format
> -described in the dma.txt file, using a two-cell specifier for each channel.
> -The two cells in order are:
> +described in the dma.txt file, using a three-cell specifier for each channel.
> +The three cells in order are:
> 1. A phandle pointing to the DMA controller.
> 2. The channel id.
> +3. The hardware slave id which is used for clients to trigger DMA engine
> +automatically.
I notice that this is an incompatible binding change. Is that necessary?
If the current code works, I'd suggest allowing both #dma-cells=<2>
and <3>, and then implementing both in the driver.
Arnd
^ permalink raw reply
* [v6,3/3] dmaengine: imx-sdma: fix consistent dma test failures
From: Angus Ainslie @ 2019-01-28 16:03 UTC (permalink / raw)
To: angus
Cc: angus.ainslie, Vinod Koul, dmaengine, NXP Linux Team,
Pengutronix Kernel Team, linux-arm-kernel, linux-kernel,
Lucas Stach, Daniel Baluta
Without the copy being aligned sdma1 fails ~10% of the time
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
drivers/dma/imx-sdma.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index d2fae53be689..e7d4d8390813 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -2114,6 +2114,7 @@ static int sdma_probe(struct platform_device *pdev)
sdma->dma_device.device_prep_dma_memcpy = sdma_prep_memcpy;
sdma->dma_device.device_issue_pending = sdma_issue_pending;
sdma->dma_device.dev->dma_parms = &sdma->dma_parms;
+ sdma->dma_device.copy_align = 2;
dma_set_max_seg_size(sdma->dma_device.dev, SDMA_BD_MAX_CNT);
platform_set_drvdata(pdev, sdma);
^ permalink raw reply related
* [v6,2/3] dmaengine: imx-sdma: add a test for imx8mq multi sdma devices
From: Angus Ainslie @ 2019-01-28 16:03 UTC (permalink / raw)
To: angus
Cc: angus.ainslie, Vinod Koul, dmaengine, NXP Linux Team,
Pengutronix Kernel Team, linux-arm-kernel, linux-kernel,
Lucas Stach, Daniel Baluta
On i.mx8mq, there are two sdma instances, and the common dma framework
will get a channel dynamically from any available sdma instance whether
it's the first sdma device or the second sdma device. Some IPs like
SAI only work with sdma2 not sdma1. To make sure the sdma channel is from
the correct sdma device, use the node pointer to match.
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
---
drivers/dma/imx-sdma.c | 6 ++++++
include/linux/platform_data/dma-imx.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 757fad2fbfae..d2fae53be689 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1914,11 +1914,16 @@ static int sdma_init(struct sdma_engine *sdma)
static bool sdma_filter_fn(struct dma_chan *chan, void *fn_param)
{
struct sdma_channel *sdmac = to_sdma_chan(chan);
+ struct sdma_engine *sdma = sdmac->sdma;
struct imx_dma_data *data = fn_param;
if (!imx_dma_is_general_purpose(chan))
return false;
+ /* return false if it's not the right device */
+ if (sdma->dev->of_node != data->of_node)
+ return false;
+
sdmac->data = *data;
chan->private = &sdmac->data;
@@ -1946,6 +1951,7 @@ static struct dma_chan *sdma_xlate(struct of_phandle_args *dma_spec,
* be set to sdmac->event_id1.
*/
data.dma_request2 = 0;
+ data.of_node = ofdma->of_node;
return dma_request_channel(mask, sdma_filter_fn, &data);
}
diff --git a/include/linux/platform_data/dma-imx.h b/include/linux/platform_data/dma-imx.h
index 7d964e787299..9daea8d42a10 100644
--- a/include/linux/platform_data/dma-imx.h
+++ b/include/linux/platform_data/dma-imx.h
@@ -55,6 +55,7 @@ struct imx_dma_data {
int dma_request2; /* secondary DMA request line */
enum sdma_peripheral_type peripheral_type;
int priority;
+ struct device_node *of_node;
};
static inline int imx_dma_is_ipu(struct dma_chan *chan)
^ permalink raw reply related
* [v6,1/3] dmaengine: imx-sdma: add clock ratio 1:1 check
From: Angus Ainslie @ 2019-01-28 16:03 UTC (permalink / raw)
To: angus
Cc: angus.ainslie, Vinod Koul, dmaengine, NXP Linux Team,
Pengutronix Kernel Team, linux-arm-kernel, linux-kernel,
Lucas Stach, Daniel Baluta
On i.mx8 mscale B0 chip, AHB/SDMA clock ratio 2:1 can't be supportted,
since SDMA clock ratio has to be increased to 250Mhz, AHB can't reach
to 500Mhz, so use 1:1 instead.
Based on NXP commit MLK-16841-1 by Robin Gong <yibin.gong@nxp.com>
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
---
drivers/dma/imx-sdma.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 0b3a67ff8e82..757fad2fbfae 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -440,6 +440,8 @@ struct sdma_engine {
unsigned int irq;
dma_addr_t bd0_phys;
struct sdma_buffer_descriptor *bd0;
+ /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
+ bool clk_ratio;
};
static int sdma_config_write(struct dma_chan *chan,
@@ -662,8 +664,11 @@ static int sdma_run_channel0(struct sdma_engine *sdma)
dev_err(sdma->dev, "Timeout waiting for CH0 ready\n");
/* Set bits of CONFIG register with dynamic context switching */
- if (readl(sdma->regs + SDMA_H_CONFIG) == 0)
- writel_relaxed(SDMA_H_CONFIG_CSM, sdma->regs + SDMA_H_CONFIG);
+ reg = readl(sdma->regs + SDMA_H_CONFIG);
+ if ((reg & SDMA_H_CONFIG_CSM) == 0) {
+ reg |= SDMA_H_CONFIG_CSM;
+ writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
+ }
return ret;
}
@@ -1840,6 +1845,9 @@ static int sdma_init(struct sdma_engine *sdma)
if (ret)
goto disable_clk_ipg;
+ if (clk_get_rate(sdma->clk_ahb) == clk_get_rate(sdma->clk_ipg))
+ sdma->clk_ratio = 1;
+
/* Be sure SDMA has not started yet */
writel_relaxed(0, sdma->regs + SDMA_H_C0PTR);
@@ -1880,8 +1888,10 @@ static int sdma_init(struct sdma_engine *sdma)
writel_relaxed(0x4050, sdma->regs + SDMA_CHN0ADDR);
/* Set bits of CONFIG register but with static context switching */
- /* FIXME: Check whether to set ACR bit depending on clock ratios */
- writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
+ if (sdma->clk_ratio)
+ writel_relaxed(SDMA_H_CONFIG_ACR, sdma->regs + SDMA_H_CONFIG);
+ else
+ writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
^ permalink raw reply related
* [v5,4/5] dmaengine: imx-sdma: add a test for imx8mq multi sdma devices
From: Lucas Stach @ 2019-01-28 9:33 UTC (permalink / raw)
To: Angus Ainslie (Purism)
Cc: angus.ainslie, Vinod Koul, dmaengine, NXP Linux Team,
Pengutronix Kernel Team, linux-arm-kernel, linux-kernel,
Daniel Baluta
Am Sonntag, den 27.01.2019, 23:08 -0700 schrieb Angus Ainslie (Purism):
> On i.mx8mq, there are two sdma instances, and the common dma framework
> will get a channel dynamically from any available sdma instance whether
> it's the first sdma device or the second sdma device. Some IPs like
> SAI only work with sdma2 not sdma1. To make sure the sdma channel is from
> the correct sdma device, use the node pointer to match.
>
> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> drivers/dma/imx-sdma.c | 6 ++++++
> include/linux/platform_data/dma-imx.h | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index c4db4fe6bcc9..d5f86becf59e 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -1918,11 +1918,16 @@ static int sdma_init(struct sdma_engine *sdma)
> static bool sdma_filter_fn(struct dma_chan *chan, void *fn_param)
> {
> struct sdma_channel *sdmac = to_sdma_chan(chan);
> + struct sdma_engine *sdma = sdmac->sdma;
> struct imx_dma_data *data = fn_param;
>
> if (!imx_dma_is_general_purpose(chan))
> return false;
>
> + /* return false if it's not the right device */
> + if (sdma->dev->of_node != data->of_node)
> + return false;
> +
> sdmac->data = *data;
> chan->private = &sdmac->data;
>
> @@ -1950,6 +1955,7 @@ static struct dma_chan *sdma_xlate(struct of_phandle_args *dma_spec,
> * be set to sdmac->event_id1.
> */
> data.dma_request2 = 0;
> + data.of_node = ofdma->of_node;
>
> return dma_request_channel(mask, sdma_filter_fn, &data);
> }
> diff --git a/include/linux/platform_data/dma-imx.h b/include/linux/platform_data/dma-imx.h
> index 7d964e787299..9daea8d42a10 100644
> --- a/include/linux/platform_data/dma-imx.h
> +++ b/include/linux/platform_data/dma-imx.h
> @@ -55,6 +55,7 @@ struct imx_dma_data {
> int dma_request2; /* secondary DMA request line */
> enum sdma_peripheral_type peripheral_type;
> int priority;
> + struct device_node *of_node;
> };
>
> static inline int imx_dma_is_ipu(struct dma_chan *chan)
^ permalink raw reply
* [v5,2/5] dmaengine: imx-sdma: add imx8mq sdma compatible parts
From: Lucas Stach @ 2019-01-28 9:32 UTC (permalink / raw)
To: Angus Ainslie (Purism)
Cc: angus.ainslie, Vinod Koul, dmaengine, NXP Linux Team,
Pengutronix Kernel Team, linux-arm-kernel, linux-kernel,
Daniel Baluta
Am Sonntag, den 27.01.2019, 23:08 -0700 schrieb Angus Ainslie (Purism):
> This is identical to the imx7d.
So it can be dropped and the i.MX8M DT should just specify the
"fsl,imx7d-sdma" as a fallback compatible for the SDMA codes.
If both the imx8m and imx7d compatible are present in the DT, we can
introduce a more specific matchic when we actually need it. No need to
pollute the code with this from the start.
Regards,
Lucas
> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
> ---
> drivers/dma/imx-sdma.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 757fad2fbfae..c4db4fe6bcc9 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -578,6 +578,9 @@ static const struct platform_device_id
> sdma_devtypes[] = {
> }, {
> .name = "imx7d-sdma",
> .driver_data = (unsigned long)&sdma_imx7d,
> + }, {
> + .name = "imx8mq-sdma",
> + .driver_data = (unsigned long)&sdma_imx7d,
> }, {
> /* sentinel */
> }
> @@ -592,6 +595,7 @@ static const struct of_device_id sdma_dt_ids[] =
> {
> { .compatible = "fsl,imx31-sdma", .data = &sdma_imx31, },
> { .compatible = "fsl,imx25-sdma", .data = &sdma_imx25, },
> { .compatible = "fsl,imx7d-sdma", .data = &sdma_imx7d, },
> + { .compatible = "fsl,imx8mq-sdma", .data = &sdma_imx7d, },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, sdma_dt_ids);
^ permalink raw reply
* [v5,1/5] dmaengine: imx-sdma: add clock ratio 1:1 check
From: Lucas Stach @ 2019-01-28 9:29 UTC (permalink / raw)
To: Angus Ainslie (Purism)
Cc: angus.ainslie, Vinod Koul, dmaengine, NXP Linux Team,
Pengutronix Kernel Team, linux-arm-kernel, linux-kernel,
Daniel Baluta
Am Sonntag, den 27.01.2019, 23:08 -0700 schrieb Angus Ainslie (Purism):
> On i.mx8 mscale B0 chip, AHB/SDMA clock ratio 2:1 can't be supportted,
> since SDMA clock ratio has to be increased to 250Mhz, AHB can't reach
> to 500Mhz, so use 1:1 instead.
>
> Based on NXP commit MLK-16841-1 by Robin Gong <yibin.gong@nxp.com>
>
> Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> drivers/dma/imx-sdma.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 0b3a67ff8e82..757fad2fbfae 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -440,6 +440,8 @@ struct sdma_engine {
> unsigned int irq;
> dma_addr_t bd0_phys;
> struct sdma_buffer_descriptor *bd0;
> + /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
> + bool clk_ratio;
> };
>
> static int sdma_config_write(struct dma_chan *chan,
> @@ -662,8 +664,11 @@ static int sdma_run_channel0(struct sdma_engine *sdma)
> dev_err(sdma->dev, "Timeout waiting for CH0 ready\n");
>
> /* Set bits of CONFIG register with dynamic context switching */
> - if (readl(sdma->regs + SDMA_H_CONFIG) == 0)
> - writel_relaxed(SDMA_H_CONFIG_CSM, sdma->regs + SDMA_H_CONFIG);
> + reg = readl(sdma->regs + SDMA_H_CONFIG);
> + if ((reg & SDMA_H_CONFIG_CSM) == 0) {
> + reg |= SDMA_H_CONFIG_CSM;
> + writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
> + }
>
> return ret;
> }
> @@ -1840,6 +1845,9 @@ static int sdma_init(struct sdma_engine *sdma)
> if (ret)
> goto disable_clk_ipg;
>
> + if (clk_get_rate(sdma->clk_ahb) == clk_get_rate(sdma->clk_ipg))
> + sdma->clk_ratio = 1;
> +
> /* Be sure SDMA has not started yet */
> writel_relaxed(0, sdma->regs + SDMA_H_C0PTR);
>
> @@ -1880,8 +1888,10 @@ static int sdma_init(struct sdma_engine *sdma)
> writel_relaxed(0x4050, sdma->regs + SDMA_CHN0ADDR);
>
> /* Set bits of CONFIG register but with static context switching */
> - /* FIXME: Check whether to set ACR bit depending on clock ratios */
> - writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
> + if (sdma->clk_ratio)
> + writel_relaxed(SDMA_H_CONFIG_ACR, sdma->regs + SDMA_H_CONFIG);
> + else
> + writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
>
> writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
>
^ permalink raw reply
* [v5,5/5] dmaengine: imx-sdma: fix consistent dma test failures
From: Angus Ainslie @ 2019-01-28 6:08 UTC (permalink / raw)
To: angus
Cc: angus.ainslie, Vinod Koul, dmaengine, NXP Linux Team,
Pengutronix Kernel Team, linux-arm-kernel, linux-kernel,
Lucas Stach, Daniel Baluta
Without the copy being aligned sdma1 fails ~10% of the time
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
drivers/dma/imx-sdma.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index d5f86becf59e..88910ec09568 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -2118,6 +2118,7 @@ static int sdma_probe(struct platform_device *pdev)
sdma->dma_device.device_prep_dma_memcpy = sdma_prep_memcpy;
sdma->dma_device.device_issue_pending = sdma_issue_pending;
sdma->dma_device.dev->dma_parms = &sdma->dma_parms;
+ sdma->dma_device.copy_align = 2;
dma_set_max_seg_size(sdma->dma_device.dev, SDMA_BD_MAX_CNT);
platform_set_drvdata(pdev, sdma);
^ permalink raw reply related
* [v5,4/5] dmaengine: imx-sdma: add a test for imx8mq multi sdma devices
From: Angus Ainslie @ 2019-01-28 6:08 UTC (permalink / raw)
To: angus
Cc: angus.ainslie, Vinod Koul, dmaengine, NXP Linux Team,
Pengutronix Kernel Team, linux-arm-kernel, linux-kernel,
Lucas Stach, Daniel Baluta
On i.mx8mq, there are two sdma instances, and the common dma framework
will get a channel dynamically from any available sdma instance whether
it's the first sdma device or the second sdma device. Some IPs like
SAI only work with sdma2 not sdma1. To make sure the sdma channel is from
the correct sdma device, use the node pointer to match.
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
drivers/dma/imx-sdma.c | 6 ++++++
include/linux/platform_data/dma-imx.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index c4db4fe6bcc9..d5f86becf59e 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1918,11 +1918,16 @@ static int sdma_init(struct sdma_engine *sdma)
static bool sdma_filter_fn(struct dma_chan *chan, void *fn_param)
{
struct sdma_channel *sdmac = to_sdma_chan(chan);
+ struct sdma_engine *sdma = sdmac->sdma;
struct imx_dma_data *data = fn_param;
if (!imx_dma_is_general_purpose(chan))
return false;
+ /* return false if it's not the right device */
+ if (sdma->dev->of_node != data->of_node)
+ return false;
+
sdmac->data = *data;
chan->private = &sdmac->data;
@@ -1950,6 +1955,7 @@ static struct dma_chan *sdma_xlate(struct of_phandle_args *dma_spec,
* be set to sdmac->event_id1.
*/
data.dma_request2 = 0;
+ data.of_node = ofdma->of_node;
return dma_request_channel(mask, sdma_filter_fn, &data);
}
diff --git a/include/linux/platform_data/dma-imx.h b/include/linux/platform_data/dma-imx.h
index 7d964e787299..9daea8d42a10 100644
--- a/include/linux/platform_data/dma-imx.h
+++ b/include/linux/platform_data/dma-imx.h
@@ -55,6 +55,7 @@ struct imx_dma_data {
int dma_request2; /* secondary DMA request line */
enum sdma_peripheral_type peripheral_type;
int priority;
+ struct device_node *of_node;
};
static inline int imx_dma_is_ipu(struct dma_chan *chan)
^ permalink raw reply related
* [v5,3/5] dt-bindings: dma: fsl-imx-sdma: add fsl,imx8mq to the accepted compatible node
From: Angus Ainslie @ 2019-01-28 6:08 UTC (permalink / raw)
To: angus
Cc: angus.ainslie, Vinod Koul, dmaengine, NXP Linux Team,
Pengutronix Kernel Team, linux-arm-kernel, linux-kernel,
Lucas Stach, Daniel Baluta
Add an fsl,imx8mq compatible string
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
---
Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
index 3c9a57a8443b..9d8bbac27d8b 100644
--- a/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
+++ b/Documentation/devicetree/bindings/dma/fsl-imx-sdma.txt
@@ -9,6 +9,7 @@ Required properties:
"fsl,imx53-sdma"
"fsl,imx6q-sdma"
"fsl,imx7d-sdma"
+ "fsl,imx8mq-sdma"
The -to variants should be preferred since they allow to determine the
correct ROM script addresses needed for the driver to work without additional
firmware.
^ permalink raw reply related
* [v5,2/5] dmaengine: imx-sdma: add imx8mq sdma compatible parts
From: Angus Ainslie @ 2019-01-28 6:08 UTC (permalink / raw)
To: angus
Cc: angus.ainslie, Vinod Koul, dmaengine, NXP Linux Team,
Pengutronix Kernel Team, linux-arm-kernel, linux-kernel,
Lucas Stach, Daniel Baluta
This is identical to the imx7d.
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
drivers/dma/imx-sdma.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 757fad2fbfae..c4db4fe6bcc9 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -578,6 +578,9 @@ static const struct platform_device_id sdma_devtypes[] = {
}, {
.name = "imx7d-sdma",
.driver_data = (unsigned long)&sdma_imx7d,
+ }, {
+ .name = "imx8mq-sdma",
+ .driver_data = (unsigned long)&sdma_imx7d,
}, {
/* sentinel */
}
@@ -592,6 +595,7 @@ static const struct of_device_id sdma_dt_ids[] = {
{ .compatible = "fsl,imx31-sdma", .data = &sdma_imx31, },
{ .compatible = "fsl,imx25-sdma", .data = &sdma_imx25, },
{ .compatible = "fsl,imx7d-sdma", .data = &sdma_imx7d, },
+ { .compatible = "fsl,imx8mq-sdma", .data = &sdma_imx7d, },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, sdma_dt_ids);
^ permalink raw reply related
* [v5,1/5] dmaengine: imx-sdma: add clock ratio 1:1 check
From: Angus Ainslie @ 2019-01-28 6:08 UTC (permalink / raw)
To: angus
Cc: angus.ainslie, Vinod Koul, dmaengine, NXP Linux Team,
Pengutronix Kernel Team, linux-arm-kernel, linux-kernel,
Lucas Stach, Daniel Baluta
On i.mx8 mscale B0 chip, AHB/SDMA clock ratio 2:1 can't be supportted,
since SDMA clock ratio has to be increased to 250Mhz, AHB can't reach
to 500Mhz, so use 1:1 instead.
Based on NXP commit MLK-16841-1 by Robin Gong <yibin.gong@nxp.com>
Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
---
drivers/dma/imx-sdma.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 0b3a67ff8e82..757fad2fbfae 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -440,6 +440,8 @@ struct sdma_engine {
unsigned int irq;
dma_addr_t bd0_phys;
struct sdma_buffer_descriptor *bd0;
+ /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
+ bool clk_ratio;
};
static int sdma_config_write(struct dma_chan *chan,
@@ -662,8 +664,11 @@ static int sdma_run_channel0(struct sdma_engine *sdma)
dev_err(sdma->dev, "Timeout waiting for CH0 ready\n");
/* Set bits of CONFIG register with dynamic context switching */
- if (readl(sdma->regs + SDMA_H_CONFIG) == 0)
- writel_relaxed(SDMA_H_CONFIG_CSM, sdma->regs + SDMA_H_CONFIG);
+ reg = readl(sdma->regs + SDMA_H_CONFIG);
+ if ((reg & SDMA_H_CONFIG_CSM) == 0) {
+ reg |= SDMA_H_CONFIG_CSM;
+ writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
+ }
return ret;
}
@@ -1840,6 +1845,9 @@ static int sdma_init(struct sdma_engine *sdma)
if (ret)
goto disable_clk_ipg;
+ if (clk_get_rate(sdma->clk_ahb) == clk_get_rate(sdma->clk_ipg))
+ sdma->clk_ratio = 1;
+
/* Be sure SDMA has not started yet */
writel_relaxed(0, sdma->regs + SDMA_H_C0PTR);
@@ -1880,8 +1888,10 @@ static int sdma_init(struct sdma_engine *sdma)
writel_relaxed(0x4050, sdma->regs + SDMA_CHN0ADDR);
/* Set bits of CONFIG register but with static context switching */
- /* FIXME: Check whether to set ACR bit depending on clock ratios */
- writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
+ if (sdma->clk_ratio)
+ writel_relaxed(SDMA_H_CONFIG_ACR, sdma->regs + SDMA_H_CONFIG);
+ else
+ writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
^ permalink raw reply related
* [v10,1/3] dmaengine: 8250_mtk_dma: add MediaTek uart DMA support
From: Long Cheng @ 2019-01-28 3:25 UTC (permalink / raw)
To: Vinod Koul
Cc: Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee, Sean Wang,
Nicolas Boichat, Matthias Brugger, Dan Williams,
Greg Kroah-Hartman, Jiri Slaby, Sean Wang, dmaengine, devicetree,
linux-arm-kernel, linux-mediatek, linux-kernel, linux-serial,
srv_heupstream, Yingjoe Chen, YT Shen, Zhenbao Liu
On Fri, 2019-01-18 at 11:10 +0800, Long Cheng wrote:
Hi Vinod Koul,
Just a gentle ping!
thanks.
> In DMA engine framework, add 8250 uart dma to support MediaTek uart.
> If MediaTek uart enabled(SERIAL_8250_MT6577), and want to improve
> the performance, can enable the function.
>
> Signed-off-by: Long Cheng <long.cheng@mediatek.com>
> ---
> drivers/dma/mediatek/Kconfig | 11 +
> drivers/dma/mediatek/Makefile | 1 +
> drivers/dma/mediatek/mtk-uart-apdma.c | 669 +++++++++++++++++++++++++++++++++
> 3 files changed, 681 insertions(+)
> create mode 100644 drivers/dma/mediatek/mtk-uart-apdma.c
>
> diff --git a/drivers/dma/mediatek/Kconfig b/drivers/dma/mediatek/Kconfig
> index 680fc05..ac49eb6 100644
> --- a/drivers/dma/mediatek/Kconfig
> +++ b/drivers/dma/mediatek/Kconfig
> @@ -24,3 +24,14 @@ config MTK_CQDMA
>
> This controller provides the channels which is dedicated to
> memory-to-memory transfer to offload from CPU.
> +
> +config MTK_UART_APDMA
> + tristate "MediaTek SoCs APDMA support for UART"
> + depends on OF && SERIAL_8250_MT6577
> + select DMA_ENGINE
> + select DMA_VIRTUAL_CHANNELS
> + help
> + Support for the UART DMA engine found on MediaTek MTK SoCs.
> + When SERIAL_8250_MT6577 is enabled, and if you want to use DMA,
> + you can enable the config. The DMA engine can only be used
> + with MediaTek SoCs.
> diff --git a/drivers/dma/mediatek/Makefile b/drivers/dma/mediatek/Makefile
> index 41bb381..61a6d29 100644
> --- a/drivers/dma/mediatek/Makefile
> +++ b/drivers/dma/mediatek/Makefile
> @@ -1,2 +1,3 @@
> +obj-$(CONFIG_MTK_UART_APDMA) += mtk-uart-apdma.o
> obj-$(CONFIG_MTK_HSDMA) += mtk-hsdma.o
> obj-$(CONFIG_MTK_CQDMA) += mtk-cqdma.o
> diff --git a/drivers/dma/mediatek/mtk-uart-apdma.c b/drivers/dma/mediatek/mtk-uart-apdma.c
> new file mode 100644
> index 0000000..427db69
> --- /dev/null
> +++ b/drivers/dma/mediatek/mtk-uart-apdma.c
> @@ -0,0 +1,669 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * MediaTek Uart APDMA driver.
> + *
> + * Copyright (c) 2018 MediaTek Inc.
> + * Author: Long Cheng <long.cheng@mediatek.com>
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_dma.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +
> +#include "../virt-dma.h"
> +
> +/* The default number of virtual channel */
> +#define MTK_UART_APDMA_NR_VCHANS 8
> +
> +#define VFF_EN_B BIT(0)
> +#define VFF_STOP_B BIT(0)
> +#define VFF_FLUSH_B BIT(0)
> +#define VFF_4G_SUPPORT_B BIT(0)
> +#define VFF_RX_INT_EN0_B BIT(0) /* rx valid size >= vff thre */
> +#define VFF_RX_INT_EN1_B BIT(1)
> +#define VFF_TX_INT_EN_B BIT(0) /* tx left size >= vff thre */
> +#define VFF_WARM_RST_B BIT(0)
> +#define VFF_RX_INT_CLR_B (BIT(0) | BIT(1))
> +#define VFF_TX_INT_CLR_B 0
> +#define VFF_STOP_CLR_B 0
> +#define VFF_INT_EN_CLR_B 0
> +#define VFF_4G_SUPPORT_CLR_B 0
> +
> +/* interrupt trigger level for tx */
> +#define VFF_TX_THRE(n) ((n) * 7 / 8)
> +/* interrupt trigger level for rx */
> +#define VFF_RX_THRE(n) ((n) * 3 / 4)
> +
> +#define VFF_RING_SIZE 0xffffU
> +/* invert this bit when wrap ring head again */
> +#define VFF_RING_WRAP 0x10000U
> +
> +#define VFF_INT_FLAG 0x00
> +#define VFF_INT_EN 0x04
> +#define VFF_EN 0x08
> +#define VFF_RST 0x0c
> +#define VFF_STOP 0x10
> +#define VFF_FLUSH 0x14
> +#define VFF_ADDR 0x1c
> +#define VFF_LEN 0x24
> +#define VFF_THRE 0x28
> +#define VFF_WPT 0x2c
> +#define VFF_RPT 0x30
> +/* TX: the buffer size HW can read. RX: the buffer size SW can read. */
> +#define VFF_VALID_SIZE 0x3c
> +/* TX: the buffer size SW can write. RX: the buffer size HW can write. */
> +#define VFF_LEFT_SIZE 0x40
> +#define VFF_DEBUG_STATUS 0x50
> +#define VFF_4G_SUPPORT 0x54
> +
> +struct mtk_uart_apdmadev {
> + struct dma_device ddev;
> + struct clk *clk;
> + bool support_33bits;
> + unsigned int dma_requests;
> + unsigned int *dma_irq;
> +};
> +
> +struct mtk_uart_apdma_desc {
> + struct virt_dma_desc vd;
> +
> + unsigned int avail_len;
> +};
> +
> +struct mtk_chan {
> + struct virt_dma_chan vc;
> + struct dma_slave_config cfg;
> + void __iomem *base;
> + struct mtk_uart_apdma_desc *desc;
> +
> + enum dma_transfer_direction dir;
> +
> + bool requested;
> +
> + unsigned int rx_status;
> +};
> +
> +static inline struct mtk_uart_apdmadev *
> +to_mtk_uart_apdma_dev(struct dma_device *d)
> +{
> + return container_of(d, struct mtk_uart_apdmadev, ddev);
> +}
> +
> +static inline struct mtk_chan *to_mtk_uart_apdma_chan(struct dma_chan *c)
> +{
> + return container_of(c, struct mtk_chan, vc.chan);
> +}
> +
> +static inline struct mtk_uart_apdma_desc *to_mtk_uart_apdma_desc
> + (struct dma_async_tx_descriptor *t)
> +{
> + return container_of(t, struct mtk_uart_apdma_desc, vd.tx);
> +}
> +
> +static void mtk_uart_apdma_write(struct mtk_chan *c,
> + unsigned int reg, unsigned int val)
> +{
> + writel(val, c->base + reg);
> +}
> +
> +static unsigned int mtk_uart_apdma_read(struct mtk_chan *c, unsigned int reg)
> +{
> + return readl(c->base + reg);
> +}
> +
> +static void mtk_uart_apdma_desc_free(struct virt_dma_desc *vd)
> +{
> + struct dma_chan *chan = vd->tx.chan;
> + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> +
> + kfree(c->desc);
> +}
> +
> +static void mtk_uart_apdma_start_tx(struct mtk_chan *c)
> +{
> + unsigned int len, send, left, wpt, d_wpt, tmp;
> + int ret;
> +
> + left = mtk_uart_apdma_read(c, VFF_LEFT_SIZE);
> + if (!left) {
> + mtk_uart_apdma_write(c, VFF_INT_EN, VFF_TX_INT_EN_B);
> + return;
> + }
> +
> + /* Wait 1sec for flush, can't sleep */
> + ret = readx_poll_timeout(readl, c->base + VFF_FLUSH, tmp,
> + tmp != VFF_FLUSH_B, 0, 1000000);
> + if (ret)
> + dev_warn(c->vc.chan.device->dev, "tx: fail, debug=0x%x\n",
> + mtk_uart_apdma_read(c, VFF_DEBUG_STATUS));
> +
> + send = min_t(unsigned int, left, c->desc->avail_len);
> + wpt = mtk_uart_apdma_read(c, VFF_WPT);
> + len = mtk_uart_apdma_read(c, VFF_LEN);
> +
> + d_wpt = wpt + send;
> + if ((d_wpt & VFF_RING_SIZE) >= len) {
> + d_wpt = d_wpt - len;
> + d_wpt = d_wpt ^ VFF_RING_WRAP;
> + }
> + mtk_uart_apdma_write(c, VFF_WPT, d_wpt);
> +
> + c->desc->avail_len -= send;
> +
> + mtk_uart_apdma_write(c, VFF_INT_EN, VFF_TX_INT_EN_B);
> + if (mtk_uart_apdma_read(c, VFF_FLUSH) == 0U)
> + mtk_uart_apdma_write(c, VFF_FLUSH, VFF_FLUSH_B);
> +}
> +
> +static void mtk_uart_apdma_start_rx(struct mtk_chan *c)
> +{
> + struct mtk_uart_apdma_desc *d = c->desc;
> + unsigned int len, wg, rg;
> + int cnt;
> +
> + if ((mtk_uart_apdma_read(c, VFF_VALID_SIZE) == 0U) ||
> + !d || !vchan_next_desc(&c->vc))
> + return;
> +
> + len = mtk_uart_apdma_read(c, VFF_LEN);
> + rg = mtk_uart_apdma_read(c, VFF_RPT);
> + wg = mtk_uart_apdma_read(c, VFF_WPT);
> + cnt = (wg & VFF_RING_SIZE) - (rg & VFF_RING_SIZE);
> + /*
> + * The buffer is ring buffer. If wrap bit different,
> + * represents the start of the next cycle for WPT
> + */
> + if ((rg ^ wg) & VFF_RING_WRAP)
> + cnt += len;
> +
> + c->rx_status = cnt;
> + mtk_uart_apdma_write(c, VFF_RPT, wg);
> +
> + list_del(&d->vd.node);
> + vchan_cookie_complete(&d->vd);
> +}
> +
> +static irqreturn_t mtk_uart_apdma_irq_handler(int irq, void *dev_id)
> +{
> + struct dma_chan *chan = (struct dma_chan *)dev_id;
> + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> + struct mtk_uart_apdma_desc *d;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&c->vc.lock, flags);
> + if (c->dir == DMA_DEV_TO_MEM) {
> + mtk_uart_apdma_write(c, VFF_INT_FLAG, VFF_RX_INT_CLR_B);
> + mtk_uart_apdma_start_rx(c);
> + } else if (c->dir == DMA_MEM_TO_DEV) {
> + d = c->desc;
> +
> + mtk_uart_apdma_write(c, VFF_INT_FLAG, VFF_TX_INT_CLR_B);
> +
> + if (d->avail_len != 0U) {
> + mtk_uart_apdma_start_tx(c);
> + } else {
> + list_del(&d->vd.node);
> + vchan_cookie_complete(&d->vd);
> + }
> + }
> + spin_unlock_irqrestore(&c->vc.lock, flags);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int mtk_uart_apdma_alloc_chan_resources(struct dma_chan *chan)
> +{
> + struct mtk_uart_apdmadev *mtkd = to_mtk_uart_apdma_dev(chan->device);
> + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> + unsigned int tmp;
> + int ret;
> +
> + pm_runtime_get_sync(mtkd->ddev.dev);
> +
> + mtk_uart_apdma_write(c, VFF_ADDR, 0);
> + mtk_uart_apdma_write(c, VFF_THRE, 0);
> + mtk_uart_apdma_write(c, VFF_LEN, 0);
> + mtk_uart_apdma_write(c, VFF_RST, VFF_WARM_RST_B);
> +
> + ret = readx_poll_timeout(readl, c->base + VFF_EN, tmp, !tmp, 10, 100);
> + if (ret) {
> + dev_err(chan->device->dev, "dma reset: fail, timeout\n");
> + return ret;
> + }
> +
> + if (!c->requested) {
> + c->requested = true;
> + ret = request_irq(mtkd->dma_irq[chan->chan_id],
> + mtk_uart_apdma_irq_handler, IRQF_TRIGGER_NONE,
> + KBUILD_MODNAME, chan);
> + if (ret < 0) {
> + dev_err(chan->device->dev, "Can't request dma IRQ\n");
> + return -EINVAL;
> + }
> + }
> +
> + if (mtkd->support_33bits)
> + mtk_uart_apdma_write(c, VFF_4G_SUPPORT, VFF_4G_SUPPORT_CLR_B);
> +
> + return ret;
> +}
> +
> +static void mtk_uart_apdma_free_chan_resources(struct dma_chan *chan)
> +{
> + struct mtk_uart_apdmadev *mtkd = to_mtk_uart_apdma_dev(chan->device);
> + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> +
> + if (c->requested) {
> + c->requested = false;
> + free_irq(mtkd->dma_irq[chan->chan_id], chan);
> + }
> +
> + tasklet_kill(&c->vc.task);
> +
> + vchan_free_chan_resources(&c->vc);
> +
> + pm_runtime_put_sync(mtkd->ddev.dev);
> +}
> +
> +static enum dma_status mtk_uart_apdma_tx_status(struct dma_chan *chan,
> + dma_cookie_t cookie,
> + struct dma_tx_state *txstate)
> +{
> + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> + enum dma_status ret;
> + unsigned long flags;
> +
> + if (!txstate)
> + return DMA_ERROR;
> +
> + ret = dma_cookie_status(chan, cookie, txstate);
> + spin_lock_irqsave(&c->vc.lock, flags);
> + if (ret == DMA_IN_PROGRESS) {
> + c->rx_status = mtk_uart_apdma_read(c, VFF_RPT) & VFF_RING_SIZE;
> + dma_set_residue(txstate, c->rx_status);
> + } else if (ret == DMA_COMPLETE && c->dir == DMA_DEV_TO_MEM) {
> + dma_set_residue(txstate, c->rx_status);
> + } else {
> + dma_set_residue(txstate, 0);
> + }
> + spin_unlock_irqrestore(&c->vc.lock, flags);
> +
> + return ret;
> +}
> +
> +static void mtk_uart_apdma_config_write(struct dma_chan *chan,
> + struct dma_slave_config *cfg,
> + enum dma_transfer_direction dir)
> +{
> + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> + struct mtk_uart_apdmadev *mtkd =
> + to_mtk_uart_apdma_dev(c->vc.chan.device);
> + unsigned int tmp;
> +
> + if (mtk_uart_apdma_read(c, VFF_EN) == VFF_EN_B)
> + return;
> +
> + c->dir = dir;
> +
> + if (dir == DMA_DEV_TO_MEM) {
> + tmp = cfg->src_addr_width * 1024;
> +
> + mtk_uart_apdma_write(c, VFF_ADDR, cfg->src_addr);
> + mtk_uart_apdma_write(c, VFF_LEN, tmp);
> + mtk_uart_apdma_write(c, VFF_THRE, VFF_RX_THRE(tmp));
> + mtk_uart_apdma_write(c, VFF_INT_EN,
> + VFF_RX_INT_EN0_B | VFF_RX_INT_EN1_B);
> + mtk_uart_apdma_write(c, VFF_RPT, 0);
> + mtk_uart_apdma_write(c, VFF_INT_FLAG, VFF_RX_INT_CLR_B);
> + } else if (dir == DMA_MEM_TO_DEV) {
> + tmp = cfg->dst_addr_width * 1024;
> +
> + mtk_uart_apdma_write(c, VFF_ADDR, cfg->dst_addr);
> + mtk_uart_apdma_write(c, VFF_LEN, tmp);
> + mtk_uart_apdma_write(c, VFF_THRE, VFF_TX_THRE(tmp));
> + mtk_uart_apdma_write(c, VFF_WPT, 0);
> + mtk_uart_apdma_write(c, VFF_INT_FLAG, VFF_TX_INT_CLR_B);
> + }
> +
> + mtk_uart_apdma_write(c, VFF_EN, VFF_EN_B);
> +
> + if (mtkd->support_33bits)
> + mtk_uart_apdma_write(c, VFF_4G_SUPPORT, VFF_4G_SUPPORT_B);
> +
> + if (mtk_uart_apdma_read(c, VFF_EN) != VFF_EN_B)
> + dev_err(chan->device->dev, "dir[%d] fail\n", dir);
> +}
> +
> +/*
> + * dmaengine_prep_slave_single will call the function. and sglen is 1.
> + * 8250 uart using one ring buffer, and deal with one sg.
> + */
> +static struct dma_async_tx_descriptor *mtk_uart_apdma_prep_slave_sg
> + (struct dma_chan *chan, struct scatterlist *sgl,
> + unsigned int sglen, enum dma_transfer_direction dir,
> + unsigned long tx_flags, void *context)
> +{
> + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> + struct mtk_uart_apdma_desc *d;
> +
> + if (!is_slave_direction(dir))
> + return NULL;
> +
> + mtk_uart_apdma_config_write(chan, &c->cfg, dir);
> +
> + /* Now allocate and setup the descriptor */
> + d = kzalloc(sizeof(*d), GFP_ATOMIC);
> + if (!d)
> + return NULL;
> +
> + /* sglen is 1 */
> + d->avail_len = sg_dma_len(sgl);
> +
> + return vchan_tx_prep(&c->vc, &d->vd, tx_flags);
> +}
> +
> +static void mtk_uart_apdma_issue_pending(struct dma_chan *chan)
> +{
> + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> + struct virt_dma_desc *vd;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&c->vc.lock, flags);
> + if (vchan_issue_pending(&c->vc)) {
> + vd = vchan_next_desc(&c->vc);
> + c->desc = to_mtk_uart_apdma_desc(&vd->tx);
> + }
> +
> + if (c->dir == DMA_DEV_TO_MEM)
> + mtk_uart_apdma_start_rx(c);
> + else if (c->dir == DMA_MEM_TO_DEV)
> + mtk_uart_apdma_start_tx(c);
> +
> + spin_unlock_irqrestore(&c->vc.lock, flags);
> +}
> +
> +static int mtk_uart_apdma_slave_config(struct dma_chan *chan,
> + struct dma_slave_config *config)
> +{
> + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> +
> + memcpy(&c->cfg, config, sizeof(*config));
> +
> + return 0;
> +}
> +
> +static int mtk_uart_apdma_terminate_all(struct dma_chan *chan)
> +{
> + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> + unsigned long flags;
> + unsigned int tmp;
> + int ret;
> +
> + spin_lock_irqsave(&c->vc.lock, flags);
> +
> + mtk_uart_apdma_write(c, VFF_FLUSH, VFF_FLUSH_B);
> + /* Wait 1sec for flush, can't sleep */
> + ret = readx_poll_timeout(readl, c->base + VFF_FLUSH, tmp,
> + tmp != VFF_FLUSH_B, 0, 1000000);
> + if (ret)
> + dev_err(c->vc.chan.device->dev, "flush: fail, debug=0x%x\n",
> + mtk_uart_apdma_read(c, VFF_DEBUG_STATUS));
> +
> + /* set stop as 1 -> wait until en is 0 -> set stop as 0 */
> + mtk_uart_apdma_write(c, VFF_STOP, VFF_STOP_B);
> + ret = readx_poll_timeout(readl, c->base + VFF_EN, tmp, !tmp, 10, 100);
> + if (ret)
> + dev_err(c->vc.chan.device->dev, "stop: fail, debug=0x%x\n",
> + mtk_uart_apdma_read(c, VFF_DEBUG_STATUS));
> +
> + mtk_uart_apdma_write(c, VFF_STOP, VFF_STOP_CLR_B);
> + mtk_uart_apdma_write(c, VFF_INT_EN, VFF_INT_EN_CLR_B);
> +
> + if (c->dir == DMA_DEV_TO_MEM)
> + mtk_uart_apdma_write(c, VFF_INT_FLAG, VFF_RX_INT_CLR_B);
> + else if (c->dir == DMA_MEM_TO_DEV)
> + mtk_uart_apdma_write(c, VFF_INT_FLAG, VFF_TX_INT_CLR_B);
> +
> + spin_unlock_irqrestore(&c->vc.lock, flags);
> +
> + return 0;
> +}
> +
> +static int mtk_uart_apdma_device_pause(struct dma_chan *chan)
> +{
> + /* just for check caps pass */
> + return 0;
> +}
> +
> +static void mtk_uart_apdma_free(struct mtk_uart_apdmadev *mtkd)
> +{
> + while (!list_empty(&mtkd->ddev.channels)) {
> + struct mtk_chan *c = list_first_entry(&mtkd->ddev.channels,
> + struct mtk_chan, vc.chan.device_node);
> +
> + list_del(&c->vc.chan.device_node);
> + tasklet_kill(&c->vc.task);
> + }
> +}
> +
> +static const struct of_device_id mtk_uart_apdma_match[] = {
> + { .compatible = "mediatek,mt6577-uart-dma", },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, mtk_uart_apdma_match);
> +
> +static int mtk_uart_apdma_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct mtk_uart_apdmadev *mtkd;
> + struct resource *res;
> + struct mtk_chan *c;
> + int bit_mask = 32, rc;
> + unsigned int i;
> +
> + mtkd = devm_kzalloc(&pdev->dev, sizeof(*mtkd), GFP_KERNEL);
> + if (!mtkd)
> + return -ENOMEM;
> +
> + mtkd->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(mtkd->clk)) {
> + dev_err(&pdev->dev, "No clock specified\n");
> + rc = PTR_ERR(mtkd->clk);
> + return rc;
> + }
> +
> + if (of_property_read_bool(np, "dma-33bits"))
> + mtkd->support_33bits = true;
> +
> + if (mtkd->support_33bits)
> + bit_mask = 33;
> +
> + rc = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(bit_mask));
> + if (rc)
> + return rc;
> +
> + dma_cap_set(DMA_SLAVE, mtkd->ddev.cap_mask);
> + mtkd->ddev.device_alloc_chan_resources =
> + mtk_uart_apdma_alloc_chan_resources;
> + mtkd->ddev.device_free_chan_resources =
> + mtk_uart_apdma_free_chan_resources;
> + mtkd->ddev.device_tx_status = mtk_uart_apdma_tx_status;
> + mtkd->ddev.device_issue_pending = mtk_uart_apdma_issue_pending;
> + mtkd->ddev.device_prep_slave_sg = mtk_uart_apdma_prep_slave_sg;
> + mtkd->ddev.device_config = mtk_uart_apdma_slave_config;
> + mtkd->ddev.device_pause = mtk_uart_apdma_device_pause;
> + mtkd->ddev.device_terminate_all = mtk_uart_apdma_terminate_all;
> + mtkd->ddev.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE);
> + mtkd->ddev.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE);
> + mtkd->ddev.directions = BIT(DMA_DEV_TO_MEM) | BIT(DMA_MEM_TO_DEV);
> + mtkd->ddev.residue_granularity = DMA_RESIDUE_GRANULARITY_SEGMENT;
> + mtkd->ddev.dev = &pdev->dev;
> + INIT_LIST_HEAD(&mtkd->ddev.channels);
> +
> + mtkd->dma_requests = MTK_UART_APDMA_NR_VCHANS;
> + if (of_property_read_u32(np, "dma-requests", &mtkd->dma_requests)) {
> + dev_info(&pdev->dev,
> + "Using %u as missing dma-requests property\n",
> + MTK_UART_APDMA_NR_VCHANS);
> + }
> +
> + mtkd->dma_irq = devm_kcalloc(&pdev->dev, mtkd->dma_requests,
> + sizeof(*mtkd->dma_irq), GFP_KERNEL);
> + if (!mtkd->dma_irq)
> + return -ENOMEM;
> +
> + for (i = 0; i < mtkd->dma_requests; i++) {
> + c = devm_kzalloc(mtkd->ddev.dev, sizeof(*c), GFP_KERNEL);
> + if (!c) {
> + rc = -ENODEV;
> + goto err_no_dma;
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> + if (!res) {
> + rc = -ENODEV;
> + goto err_no_dma;
> + }
> +
> + c->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(c->base)) {
> + rc = PTR_ERR(c->base);
> + goto err_no_dma;
> + }
> + c->requested = false;
> + c->vc.desc_free = mtk_uart_apdma_desc_free;
> + vchan_init(&c->vc, &mtkd->ddev);
> +
> + mtkd->dma_irq[i] = platform_get_irq(pdev, i);
> + if ((int)mtkd->dma_irq[i] < 0) {
> + dev_err(&pdev->dev, "failed to get IRQ[%d]\n", i);
> + rc = -EINVAL;
> + goto err_no_dma;
> + }
> + }
> +
> + pm_runtime_enable(&pdev->dev);
> + pm_runtime_set_active(&pdev->dev);
> +
> + rc = dma_async_device_register(&mtkd->ddev);
> + if (rc)
> + goto rpm_disable;
> +
> + platform_set_drvdata(pdev, mtkd);
> +
> + /* Device-tree DMA controller registration */
> + rc = of_dma_controller_register(np, of_dma_xlate_by_chan_id, mtkd);
> + if (rc)
> + goto dma_remove;
> +
> + return rc;
> +
> +dma_remove:
> + dma_async_device_unregister(&mtkd->ddev);
> +rpm_disable:
> + pm_runtime_disable(&pdev->dev);
> +err_no_dma:
> + mtk_uart_apdma_free(mtkd);
> + return rc;
> +}
> +
> +static int mtk_uart_apdma_remove(struct platform_device *pdev)
> +{
> + struct mtk_uart_apdmadev *mtkd = platform_get_drvdata(pdev);
> +
> + if (pdev->dev.of_node)
> + of_dma_controller_free(pdev->dev.of_node);
> +
> + pm_runtime_disable(&pdev->dev);
> + pm_runtime_put_noidle(&pdev->dev);
> +
> + dma_async_device_unregister(&mtkd->ddev);
> + mtk_uart_apdma_free(mtkd);
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int mtk_uart_apdma_suspend(struct device *dev)
> +{
> + struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
> +
> + if (!pm_runtime_suspended(dev))
> + clk_disable_unprepare(mtkd->clk);
> +
> + return 0;
> +}
> +
> +static int mtk_uart_apdma_resume(struct device *dev)
> +{
> + int ret;
> + struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
> +
> + if (!pm_runtime_suspended(dev)) {
> + ret = clk_prepare_enable(mtkd->clk);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +#endif /* CONFIG_PM_SLEEP */
> +
> +#ifdef CONFIG_PM
> +static int mtk_uart_apdma_runtime_suspend(struct device *dev)
> +{
> + struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
> +
> + clk_disable_unprepare(mtkd->clk);
> +
> + return 0;
> +}
> +
> +static int mtk_uart_apdma_runtime_resume(struct device *dev)
> +{
> + int ret;
> + struct mtk_uart_apdmadev *mtkd = dev_get_drvdata(dev);
> +
> + ret = clk_prepare_enable(mtkd->clk);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +#endif /* CONFIG_PM */
> +
> +static const struct dev_pm_ops mtk_uart_apdma_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(mtk_uart_apdma_suspend, mtk_uart_apdma_resume)
> + SET_RUNTIME_PM_OPS(mtk_uart_apdma_runtime_suspend,
> + mtk_uart_apdma_runtime_resume, NULL)
> +};
> +
> +static struct platform_driver mtk_uart_apdma_driver = {
> + .probe = mtk_uart_apdma_probe,
> + .remove = mtk_uart_apdma_remove,
> + .driver = {
> + .name = KBUILD_MODNAME,
> + .pm = &mtk_uart_apdma_pm_ops,
> + .of_match_table = of_match_ptr(mtk_uart_apdma_match),
> + },
> +};
> +
> +module_platform_driver(mtk_uart_apdma_driver);
> +
> +MODULE_DESCRIPTION("MediaTek UART APDMA Controller Driver");
> +MODULE_AUTHOR("Long Cheng <long.cheng@mediatek.com>");
> +MODULE_LICENSE("GPL v2");
> +
^ permalink raw reply
* [v4,1/5] dmaengine: imx-sdma: add clock ratio 1:1 check
From: Lucas Stach @ 2019-01-25 9:42 UTC (permalink / raw)
To: Angus Ainslie (Purism)
Cc: angus.ainslie, Vinod Koul, dmaengine, NXP Linux Team,
Pengutronix Kernel Team, linux-arm-kernel, linux-kernel,
Daniel Baluta
Am Donnerstag, den 24.01.2019, 19:55 -0700 schrieb Angus Ainslie (Purism):
> On i.mx8 mscale B0 chip, AHB/SDMA clock ratio 2:1 can't be supportted,
> since SDMA clock ratio has to be increased to 250Mhz, AHB can't reach
> to 500Mhz, so use 1:1 instead.
>
> > Based on NXP commit MLK-16841-1 by Robin Gong <yibin.gong@nxp.com>
>
> > Signed-off-by: Angus Ainslie (Purism) <angus@akkea.ca>
> ---
> drivers/dma/imx-sdma.c | 21 +++++++++++++++++----
> 1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 0b3a67ff8e82..5e5ef0b5a973 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -440,6 +440,8 @@ struct sdma_engine {
> > > unsigned int irq;
> > > dma_addr_t bd0_phys;
> > > struct sdma_buffer_descriptor *bd0;
> > + /* clock ratio for AHB:SDMA core. 1:1 is 1, 2:1 is 0*/
> > > + bool clk_ratio;
> };
>
> static int sdma_config_write(struct dma_chan *chan,
> @@ -662,8 +664,14 @@ static int sdma_run_channel0(struct sdma_engine *sdma)
> > dev_err(sdma->dev, "Timeout waiting for CH0 ready\n");
>
> > /* Set bits of CONFIG register with dynamic context switching */
> > - if (readl(sdma->regs + SDMA_H_CONFIG) == 0)
> > - writel_relaxed(SDMA_H_CONFIG_CSM, sdma->regs + SDMA_H_CONFIG);
> + if ((readl(sdma->regs + SDMA_H_CONFIG) & ~SDMA_H_CONFIG_ACR) == 0) {
The intention of this code was probably to set the CSM bit if they
weren't set before, so masking out individual bits from the register
and risking to skip this when one of the other bits was set doesn't
seem right.
I guess the whole block can be simplified to:
reg = readl(sdma->regs + SDMA_H_CONFIG);
if ((reg & SDMA_H_CONFIG_CSM) != SDMA_H_CONFIG_CSM)
reg |= SDMA_H_CONFIG_CSM;
writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
Regards,
Lucas
> + reg = SDMA_H_CONFIG_CSM;
> +
> > + if (sdma->clk_ratio)
> > + reg |= SDMA_H_CONFIG_ACR;
> +
> > + writel_relaxed(reg, sdma->regs + SDMA_H_CONFIG);
> > + }
>
> > return ret;
> }
> @@ -1840,6 +1848,9 @@ static int sdma_init(struct sdma_engine *sdma)
> > if (ret)
> > goto disable_clk_ipg;
>
> > + if (clk_get_rate(sdma->clk_ahb) == clk_get_rate(sdma->clk_ipg))
> > + sdma->clk_ratio = 1;
> +
> > /* Be sure SDMA has not started yet */
> > writel_relaxed(0, sdma->regs + SDMA_H_C0PTR);
>
> @@ -1880,8 +1891,10 @@ static int sdma_init(struct sdma_engine *sdma)
> > writel_relaxed(0x4050, sdma->regs + SDMA_CHN0ADDR);
>
> > /* Set bits of CONFIG register but with static context switching */
> > - /* FIXME: Check whether to set ACR bit depending on clock ratios */
> > - writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
> > + if (sdma->clk_ratio)
> > + writel_relaxed(SDMA_H_CONFIG_ACR, sdma->regs + SDMA_H_CONFIG);
> > + else
> > + writel_relaxed(0, sdma->regs + SDMA_H_CONFIG);
>
> > writel_relaxed(ccb_phys, sdma->regs + SDMA_H_C0PTR);
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox