dmaengine: add k3dma
Subsystems (2)
v2 cover dmaengine: add k3dma
change since v1:
Follow Arnd suggestion:
using dma_get_slave_channel interface
Correct dma-channels and dma-requests for phy and virt channel
Follow Vinod suggestion:
add name space
use SIMPLE_DEV_PM_OPS
Besides:
add pause/resume
Using kzalloc instead of dma pool
Zhangfei Gao (2):
dmaengine: add interface of dma_get_slave_channel
dmaengine: Add hisilicon k3 DMA engine driver
Documentation/devicetree/bindings/dma/k3dma.txt | 46 ++ drivers/dma/Kconfig | 9 + drivers/dma/Makefile | 1 + drivers/dma/dmaengine.c | 26 + drivers/dma/k3dma.c | 853 +++++++++++++++++++++++ include/linux/dmaengine.h | 1 + 6 files changed, 936 insertions(+) create mode 100644 Documentation/devicetree/bindings/dma/k3dma.txt create mode 100644 drivers/dma/k3dma.c
Signature · 1 line
1.7.9.5
PATCH v2 1/2 dmaengine: add interface of dma_get_slave_channel
Suggested by Arnd, add dma_get_slave_channel interface
Dma host driver could get specific channel specificied by request line, rather than filter.
host example:
static struct dma_chan *xx_of_dma_simple_xlate(struct of_phandle_args *dma_spec,
struct of_dma *ofdma)
{
struct xx_dma_dev *d = ofdma->of_dma_data;
unsigned int request = dma_spec->args[0];
if (request > d->dma_requests)
return NULL;
return dma_get_slave_channel(&(d->chans[request].vc.chan));
}probe:of_dma_controller_register((&op->dev)->of_node, xx_of_dma_simple_xlate, d);
--- drivers/dma/dmaengine.c | 26 ++++++++++++++++++++++++++ include/linux/dmaengine.h | 1 + 2 files changed, 27 insertions(+)
drivers/dma/dmaengine.c +26 -0
@@ -504,6 +504,32 @@ static struct dma_chan *private_candidate(const dma_cap_mask_t *mask, }/** +* dma_request_channel - try to get specific channel exclusively +* @chan: target channel +*/ +struct dma_chan *dma_get_slave_channel (struct dma_chan *chan) +{ +int err = -EBUSY ; + +/* lock against __dma_request_channel */ +mutex_lock (&dma_list_mutex); + +if (chan->client_count ==0 ) + err =dma_chan_get (chan); +else + chan =NULL ; + +mutex_unlock (&dma_list_mutex); + +if (err) +pr_debug ("%s: failed to get %s: (%d)\n" , + __func__,dma_chan_name (chan), err); + +return chan; +} +EXPORT_SYMBOL_GPL (dma_get_slave_channel); + +/** * dma_request_channel - try to allocate an exclusive channel * @mask: capabilities that the channel must satisfy * @fn: optional callback to disposition available channels
include/linux/dmaengine.h +1 -0
@@ -1000,6 +1000,7 @@ int dma_async_device_register(struct dma_device *device);void dma_async_device_unregister (struct dma_device *device);void dma_run_dependencies (struct dma_async_tx_descriptor *tx);struct dma_chan *dma_find_channel (enum dma_transaction_type tx_type); +struct dma_chan *dma_get_slave_channel (struct dma_chan *chan);struct dma_chan *net_dma_find_channel (void );#define dma_request_channel (mask, x, y) __dma_request_channel(&(mask), x, y)#define dma_request_slave_channel_compat (mask, x, y, dev, name) \
Signature · 1 line
1.7.9.5
PATCH v2 1/2 dmaengine: add interface of dma_get_slave_channel
Quoted 16 lines · Zhangfei Gao
Suggested by Arnd, add
dma_get_slave_channelinterface
Dma host driver could get specific channel specificied by request line, rather than filter.host example:
static structdma_chan*xx_of_dma_simple_xlate(structof_phandle_args*dma_spec,
structof_dma*ofdma){ struct xx_dma_dev *d = ofdma->of_dma_data; unsigned int request = dma_spec->args[0]; if (request > d->dma_requests) return NULL; return dma_get_slave_channel(&(d->chans[request].vc.chan)); }
Quoted 4 lines
probe:
of_dma_controller_register((&op->dev)->of_node, xx_of_dma_simple_xlate, d);Signed-off-by Zhangfei Gao <[email protected]>
Vinod Koul
AppliedPATCH v2 1/2 dmaengine: add interface of dma_get_slave_channel
In reply to Zhangfei GaoQuoted 16 lines · Zhangfei Gao
Suggested by Arnd, add
dma_get_slave_channelinterface
Dma host driver could get specific channel specificied by request line, rather than filter.host example:
static structdma_chan*xx_of_dma_simple_xlate(structof_phandle_args*dma_spec,
structof_dma*ofdma){ struct xx_dma_dev *d = ofdma->of_dma_data; unsigned int request = dma_spec->args[0]; if (request > d->dma_requests) return NULL; return dma_get_slave_channel(&(d->chans[request].vc.chan)); }
Quoted 4 lines
probe:
of_dma_controller_register((&op->dev)->of_node, xx_of_dma_simple_xlate, d);Signed-off-by Zhangfei Gao <[email protected]>
Applied, thanks
~Vinod
PATCH v2 2/2 dmaengine: Add hisilicon k3 DMA engine driver
In reply to Zhangfei GaoAdd dmaengine driver for hisilicon k3 platform based on virt_dma
--- Documentation/devicetree/bindings/dma/k3dma.txt | 46 ++ drivers/dma/Kconfig | 9 + drivers/dma/Makefile | 1 + drivers/dma/k3dma.c | 849 +++++++++++++++++++++++ 4 files changed, 905 insertions(+) create mode 100644 Documentation/devicetree/bindings/dma/k3dma.txt create mode 100644 drivers/dma/k3dma.c
Documentation/devicetree/bindings/dma/k3dma.txt new file +46 -0
@@ -0,0 +1,46 @@ +* Hisilicon K3 DMA controller + +See dma.txt first + +Required properties: +- compatible: Should be "hisilicon,k3-dma-1.0" +- reg: Should contain DMA registers location and length. +- interrupts: Should contain one interrupt shared by all channel +- #dma-cells: see dma.txt, should be 1, para number +- dma-channels: physical channels supported +- dma-requests: virtual channels supported, each virtual channel + have specific request line +- clocks: clock required + +Example: + +Controller: + dma0: dma at fcd02000 { + compatible = "hisilicon,k3-dma-1.0"; + reg = <0xfcd02000 0x1000>; + #dma-cells = <1>; + dma-channels = <16>; + dma-requests = <27>; + interrupts = <0 12 4>; + clocks = <&pclk>; + status = "disable"; + }; + +Client: +Use specific request line passing from dmax +For example, i2c0 read channel request line is 18, while write channel use 19 + + i2c0: i2c at fcb08000 { + compatible = "snps,designware-i2c"; + dmas = <&dma0 18 /* read channel */ + &dma0 19>; /* write channel */ + dma-names = "rx", "tx"; + }; + + i2c1: i2c at fcb09000 { + compatible = "snps,designware-i2c"; + dmas = <&dma0 20 /* read channel */ + &dma0 21>; /* write channel */ + dma-names = "rx", "tx"; + }; +
drivers/dma/Kconfig +9 -0
@@ -312,6 +312,15 @@ config MMP_PDMA help Support the MMP PDMA engine for PXA and MMP platfrom. +config K3_DMA + tristate "Hisilicon K3 DMA support" + depends on ARCH_HI3xxx + select DMA_ENGINE + select DMA_VIRTUAL_CHANNELS + help + Support the DMA engine for Hisilicon K3 platform + devices. + config DMA_ENGINE bool
drivers/dma/Makefile +1 -0
@@ -38,3 +38,4 @@ obj-$(CONFIG_DMA_SA11X0) += sa11x0-dma.o obj-$(CONFIG_MMP_TDMA) += mmp_tdma.o obj-$(CONFIG_DMA_OMAP) += omap-dma.o obj-$(CONFIG_MMP_PDMA) += mmp_pdma.o +obj-$(CONFIG_K3_DMA) += k3dma.o
drivers/dma/k3dma.c new file +849 -0
849 lines not on this page · raw message
Signature · 1 line
1.7.9.5
PATCH v2 2/2 dmaengine: Add hisilicon k3 DMA engine driver
Quoted 28 lines · Zhangfei Gao
Add dmaengine driver for hisilicon k3 platform based on
virt_dmaSigned-off-by Zhangfei Gao <[email protected]>Tested-by Kai Yang <[email protected]>Acked-by Arnd Bergmann <[email protected]>---
drivers/dma/k3dma.c new file +21
-0+#define DRIVER_NAME "k3-dma" +#define DMA_ALIGN 3 +#define DMA_MAX_SIZE 0x1ffc + +#define INT_STAT 0x00 +#define INT_TC1 0x04 +#define INT_ERR1 0x0c +#define INT_ERR2 0x10 +#define INT_TC1_MASK 0x18 +#define INT_ERR1_MASK 0x20 +#define INT_ERR2_MASK 0x24 +#define INT_TC1_RAW 0x600 +#define INT_ERR1_RAW 0x608 +#define INT_ERR2_RAW 0x610 +#define CH_PRI 0x688 +#define CH_STAT 0x690 +#define CX_CUR_CNT 0x704 +#define CX_LLI 0x800 +#define CX_CNT 0x810 +#define CX_SRC 0x814 +#define CX_DST 0x818
drivers/dma/k3dma.c line 56
Quoted 10 lines
drivers/dma/k3dma.c new file +10
-0+#define CX_CONFIG 0x81c +#define AXI_CONFIG 0x820 +#define DEF_AXI_CONFIG 0x201201 + +#define CX_LLI_CHAIN_EN 0x2 +#define CCFG_EN 0x1 +#define CCFG_MEM2PER (0x1 << 2) +#define CCFG_PER2MEM (0x2 << 2) +#define CCFG_SRCINCR (0x1 << 31) +#define CCFG_DSTINCR (0x1 << 30)
I see these are not namespace aptly and can collide..
Quoted 33 lines
drivers/dma/k3dma.c new file +33
-0+static int k3_dma_control (struct dma_chan *chan,enum dma_ctrl_cmd cmd, +unsigned long arg) +{ +struct k3_dma_chan *c =to_k3_chan (chan); +struct k3_dma_dev *d =to_k3_dma (chan->device); +struct dma_slave_config *cfg = (void *)arg; +struct k3_dma_phy *p =NULL ; +unsigned long flags; +u32 maxburst =0 , val =0 ; +enum dma_slave_buswidth width =DMA_SLAVE_BUSWIDTH_UNDEFINED ; +LIST_HEAD (head); + +if (c) + p = c->phy; + +switch (cmd) { +case DMA_SLAVE_CONFIG : +if (cfg ==NULL ) +return -EINVAL ; + c->dir = cfg->direction; +if (c->dir ==DMA_DEV_TO_MEM ) { + c->ccfg =CCFG_DSTINCR ; + c->dev_addr = cfg->src_addr; + maxburst = cfg->src_maxburst; + width = cfg->src_addr_width; + }else if (c->dir ==DMA_MEM_TO_DEV ) { + c->ccfg =CCFG_SRCINCR ; + c->dev_addr = cfg->dst_addr; + maxburst = cfg->dst_maxburst; + width = cfg->dst_addr_width; + } +switch (width) { +case DMA_SLAVE_BUSWIDTH_1_BYTE :
drivers/dma/k3dma.c line 568
Quoted 10 lines
drivers/dma/k3dma.c new file +10
-0+ val =0 ; +break ; +case DMA_SLAVE_BUSWIDTH_2_BYTES : + val =1 ; +break ; +case DMA_SLAVE_BUSWIDTH_4_BYTES : + val =2 ; +break ; +case DMA_SLAVE_BUSWIDTH_8_BYTES : + val =3 ;
DMA_SLAVE_BUSWIDTHS are 1, 2, 4, 8...
so if you can do val = ffs(width) as well?
drivers/dma/k3dma.c line 618
Quoted 11 lines
drivers/dma/k3dma.c new file +11
-0+case DMA_PAUSE : +dev_dbg (d->slave.dev,"vchan %p: pause\n" , &c->vc); +if (c->status ==DMA_IN_PROGRESS ) { + c->status =DMA_PAUSED ; +if (p) { +k3_dma_pause_dma (p, false); + }else { +spin_lock (&d->lock); +list_del_init (&c->node); +spin_unlock (&d->lock); + }
why do we need the else part here?
Quoted 5 lines
drivers/dma/k3dma.c new file +5
-0+ } +break ; + +case DMA_RESUME : +dev_dbg (d->slave.dev,"vchan %p: resume\n" , &c->vc);
drivers/dma/k3dma.c line 633
Quoted 10 lines
drivers/dma/k3dma.c new file +10
-0+spin_lock_irqsave (&c->vc.lock, flags); +if (c->status ==DMA_PAUSED ) { + c->status =DMA_IN_PROGRESS ; +if (p) { +k3_dma_pause_dma (p, true); + }else if (!list_empty (&c->vc.desc_issued)) { +spin_lock (&d->lock); +list_add_tail (&c->node, &d->chan_pending); +spin_unlock (&d->lock); + }
ditto?
Quoted 35 lines
drivers/dma/k3dma.c new file +34
-0+ } +spin_unlock_irqrestore (&c->vc.lock, flags); +break ; +default : +return -ENXIO ; + } +return 0 ; +} + + +#ifdef CONFIG_PM_SLEEP +static int k3_dma_pltfm_suspend (struct device *dev) +{ +struct k3_dma_dev *d =dev_get_drvdata (dev); +u32 stat =0 ; + + stat =k3_dma_get_chan_stat (d); +if (stat) { +dev_warn (d->slave.dev, +"chan %d is running fail to suspend\n" , stat); +return -1 ; + } +k3_dma_enable_dma (d, false); +clk_disable_unprepare (d->clk); +return 0 ; +} + +static int k3_dma_pltfm_resume (struct device *dev) +{ +struct k3_dma_dev *d =dev_get_drvdata (dev); +int ret =0 ; + + ret =clk_prepare_enable (d->clk); +if (ret <0 ) {
drivers/dma/k3dma.c line 831
Quoted 10 lines
drivers/dma/k3dma.c new file +10
-0+dev_err (d->slave.dev,"clk_prepare_enable failed: %d\n" , ret); + return -EINVAL; + } +k3_dma_enable_dma (d, true); +return 0 ; +} +#else +#define k3_dma_pltfm_suspend NULL +#define k3_dma_pltfm_resume NULL +#endif /* CONFIG_PM_SLEEP */
you dont need #ifdef here, then whats the use of SIMPLE_DEV_PM_OPS below?
drivers/dma/k3dma.c line 833
Quoted 2 lines
drivers/dma/k3dma.c new file +2
-0+ +SIMPLE_DEV_PM_OPS (k3_dma_pltfm_pmops, k3_dma_pltfm_suspend, k3_dma_pltfm_resume);
pltfm... can we skip this or use platform, plat...
Quoted 6 lines
drivers/dma/k3dma.c new file +6
-0+ +static struct platform_driver k3_pdma_driver = { + .driver = { + .name =DRIVER_NAME , + .owner =THIS_MODULE , + .pm = &k3_dma_pltfm_pmops,
drivers/dma/k3dma.c line 849
Quoted 10 lines
drivers/dma/k3dma.c new file +10
-0+ .of_match_table = k3_pdma_dt_ids, + }, + .probe = k3_dma_probe, + .remove = k3_dma_remove, +}; + +module_platform_driver (k3_pdma_driver); + +MODULE_DESCRIPTION ("Hisilicon k3 DMA Driver" ); +MODULE_LICENSE ("GPL v2" );
MODULE_ALIAS?
~Vinod
PATCH v2 2/2 dmaengine: Add hisilicon k3 DMA engine driver
Quoted 32 lines · Vinod Koul
Quoted 31 lines · Zhangfei Gao
Add dmaengine driver for hisilicon k3 platform based on
virt_dmaSigned-off-by Zhangfei Gao <[email protected]>Tested-by Kai Yang <[email protected]>Acked-by Arnd Bergmann <[email protected]>---
drivers/dma/k3dma.c new file +24
-0+#define DRIVER_NAME "k3-dma" +#define DMA_ALIGN 3 +#define DMA_MAX_SIZE 0x1ffc + +#define INT_STAT 0x00 +#define INT_TC1 0x04 +#define INT_ERR1 0x0c +#define INT_ERR2 0x10 +#define INT_TC1_MASK 0x18 +#define INT_ERR1_MASK 0x20 +#define INT_ERR2_MASK 0x24 +#define INT_TC1_RAW 0x600 +#define INT_ERR1_RAW 0x608 +#define INT_ERR2_RAW 0x610 +#define CH_PRI 0x688 +#define CH_STAT 0x690 +#define CX_CUR_CNT 0x704 +#define CX_LLI 0x800 +#define CX_CNT 0x810 +#define CX_SRC 0x814 +#define CX_DST 0x818 +#define CX_CONFIG 0x81c +#define AXI_CONFIG 0x820 +#define DEF_AXI_CONFIG 0x201201
Quoted 8 lines
drivers/dma/k3dma.c line 56
Quoted 7 lines
drivers/dma/k3dma.c new file +7
-0+ +#define CX_LLI_CHAIN_EN 0x2 +#define CCFG_EN 0x1 +#define CCFG_MEM2PER (0x1 << 2) +#define CCFG_PER2MEM (0x2 << 2) +#define CCFG_SRCINCR (0x1 << 31) +#define CCFG_DSTINCR (0x1 << 30)I see these are not namespace aptly and can collide..
OK,
How about change name to
#define CX_CFG 0x81c
#define CX_CFG_EN 0x1
#define CX_CFG_MEM2PER (0x1 << 2)
~
Quoted 12 lines
drivers/dma/k3dma.c line 568
Quoted 12 lines
drivers/dma/k3dma.c new file +12
-0+switch (width) { +case DMA_SLAVE_BUSWIDTH_1_BYTE : + val =0 ; +break ; +case DMA_SLAVE_BUSWIDTH_2_BYTES : + val =1 ; +break ; +case DMA_SLAVE_BUSWIDTH_4_BYTES : + val =2 ; +break ; +case DMA_SLAVE_BUSWIDTH_8_BYTES : + val =3 ;
Quoted 2 lines
DMA_SLAVE_BUSWIDTHSare 1, 2, 4, 8...
so if you can do val = ffs(width) as well?
Good idea, will use __ffs(width) here.
Quoted 13 lines
drivers/dma/k3dma.c line 618
Quoted 13 lines
drivers/dma/k3dma.c new file +11
-0+case DMA_PAUSE : +dev_dbg (d->slave.dev,"vchan %p: pause\n" , &c->vc); +if (c->status ==DMA_IN_PROGRESS ) { + c->status =DMA_PAUSED ; +if (p) { +k3_dma_pause_dma (p, false); + }else { +spin_lock (&d->lock); +list_del_init (&c->node); +spin_unlock (&d->lock); + }
Quoted 1 line
why do we need the else part here?
Since asynchronous mode is supported.
Desc is submitted to list but may not get physical channel to run.
Quoted 29 lines
Quoted 29 lines
drivers/dma/k3dma.c new file +28
-0+ +#ifdef CONFIG_PM_SLEEP +static int k3_dma_pltfm_suspend (struct device *dev) +{ +struct k3_dma_dev *d =dev_get_drvdata (dev); +u32 stat =0 ; + + stat =k3_dma_get_chan_stat (d); +if (stat) { +dev_warn (d->slave.dev, +"chan %d is running fail to suspend\n" , stat); +return -1 ; + } +k3_dma_enable_dma (d, false); +clk_disable_unprepare (d->clk); +return 0 ; +} + +static int k3_dma_pltfm_resume (struct device *dev) +{ +struct k3_dma_dev *d =dev_get_drvdata (dev); +int ret =0 ; + + ret =clk_prepare_enable (d->clk); +if (ret <0 ) { +dev_err (d->slave.dev,"clk_prepare_enable failed: %d\n" , ret); +return -EINVAL ; + }
Quoted 8 lines
drivers/dma/k3dma.c line 831
Quoted 7 lines
drivers/dma/k3dma.c new file +7
-0+k3_dma_enable_dma (d, true); + return 0; +} +#else +#define k3_dma_pltfm_suspend NULL +#define k3_dma_pltfm_resume NULL +#endif /* CONFIG_PM_SLEEP */ you dont need #ifdef here, then whats the use of
SIMPLE_DEV_PM_OPSbelow?
Yes.
Quoted 3 lines
drivers/dma/k3dma.c line 833
Quoted 2 lines
drivers/dma/k3dma.c new file +2
-0+ +SIMPLE_DEV_PM_OPS (k3_dma_pltfm_pmops, k3_dma_pltfm_suspend, k3_dma_pltfm_resume);pltfm... can we skip this or use platform, plat...
Got it.
Quoted 9 lines
Quoted 9 lines
drivers/dma/k3dma.c new file +9
-0+ +static struct platform_driver k3_pdma_driver = { + .driver = { + .name =DRIVER_NAME , + .owner =THIS_MODULE , + .pm = &k3_dma_pltfm_pmops, + .of_match_table = k3_pdma_dt_ids, + }, + .probe = k3_dma_probe,
Quoted 8 lines
drivers/dma/k3dma.c line 849
Quoted 7 lines
drivers/dma/k3dma.c new file +7
-0+ .remove = k3_dma_remove, +}; + +module_platform_driver (k3_pdma_driver); + +MODULE_DESCRIPTION ("Hisilicon k3 DMA Driver" ); +MODULE_LICENSE ("GPL v2" );
MODULE_ALIAS?
OK.
Quoted 7 lines
~Vinod
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
PATCH v2 2/2 dmaengine: Add hisilicon k3 DMA engine driver
Quoted 41 lines · zhangfei gao
Quoted 32 lines · Vinod Koul
Quoted 31 lines · Zhangfei Gao
Add dmaengine driver for hisilicon k3 platform based on
virt_dmaSigned-off-by Zhangfei Gao <[email protected]>Tested-by Kai Yang <[email protected]>Acked-by Arnd Bergmann <[email protected]>---
drivers/dma/k3dma.c new file +24
-0+#define DRIVER_NAME "k3-dma" +#define DMA_ALIGN 3 +#define DMA_MAX_SIZE 0x1ffc + +#define INT_STAT 0x00 +#define INT_TC1 0x04 +#define INT_ERR1 0x0c +#define INT_ERR2 0x10 +#define INT_TC1_MASK 0x18 +#define INT_ERR1_MASK 0x20 +#define INT_ERR2_MASK 0x24 +#define INT_TC1_RAW 0x600 +#define INT_ERR1_RAW 0x608 +#define INT_ERR2_RAW 0x610 +#define CH_PRI 0x688 +#define CH_STAT 0x690 +#define CX_CUR_CNT 0x704 +#define CX_LLI 0x800 +#define CX_CNT 0x810 +#define CX_SRC 0x814 +#define CX_DST 0x818 +#define CX_CONFIG 0x81c +#define AXI_CONFIG 0x820 +#define DEF_AXI_CONFIG 0x201201Quoted 8 lines
drivers/dma/k3dma.c line 56
Quoted 7 lines
drivers/dma/k3dma.c new file +7
-0+ +#define CX_LLI_CHAIN_EN 0x2 +#define CCFG_EN 0x1 +#define CCFG_MEM2PER (0x1 << 2) +#define CCFG_PER2MEM (0x2 << 2) +#define CCFG_SRCINCR (0x1 << 31) +#define CCFG_DSTINCR (0x1 << 30)I see these are not namespace aptly and can collide..
Quoted 3 lines
OK,
How about change name to
#defineCX_CFG0x81c
since you are calling your driver K3_DMA it would be apt to namespace all of the above with K3_INT_STAT to K3_CFG_EN and so on...
Quoted 35 lines
#define
CX_CFG_EN0x1
#defineCX_CFG_MEM2PER(0x1<< 2)
~Quoted 12 lines
drivers/dma/k3dma.c line 568
Quoted 12 lines
drivers/dma/k3dma.c new file +12
-0+switch (width) { +case DMA_SLAVE_BUSWIDTH_1_BYTE : + val =0 ; +break ; +case DMA_SLAVE_BUSWIDTH_2_BYTES : + val =1 ; +break ; +case DMA_SLAVE_BUSWIDTH_4_BYTES : + val =2 ; +break ; +case DMA_SLAVE_BUSWIDTH_8_BYTES : + val =3 ;Quoted 2 lines
DMA_SLAVE_BUSWIDTHSare 1, 2, 4, 8...
so if you can do val = ffs(width) as well?Good idea, will use
__ffs(width)here.Quoted 13 lines
Quoted 13 lines
drivers/dma/k3dma.c new file +11
-0+case DMA_PAUSE : +dev_dbg (d->slave.dev,"vchan %p: pause\n" , &c->vc); +if (c->status ==DMA_IN_PROGRESS ) { + c->status =DMA_PAUSED ; +if (p) { +k3_dma_pause_dma (p, false); + }else { +spin_lock (&d->lock); +list_del_init (&c->node); +spin_unlock (&d->lock); + }
Quoted 3 lines
Quoted 1 line
why do we need the else part here?
Since asynchronous mode is supported.
Desc is submitted to list but may not get physical channel to run.
But when you pause you pause the running channel. You dont pause a descriptor. So whatever you are trying to imply doesnt make sense to me.
~Vinod
PATCH v2 2/2 dmaengine: Add hisilicon k3 DMA engine driver
Quoted 42 lines · Vinod Koul
Quoted 41 lines · zhangfei gao
Quoted 40 lines · Vinod Koul
Quoted 28 lines · Zhangfei Gao
Add dmaengine driver for hisilicon k3 platform based on
virt_dmaSigned-off-by Zhangfei Gao <[email protected]>Tested-by Kai Yang <[email protected]>Acked-by Arnd Bergmann <[email protected]>---
drivers/dma/k3dma.c new file +21
-0+#define DRIVER_NAME "k3-dma" +#define DMA_ALIGN 3 +#define DMA_MAX_SIZE 0x1ffc + +#define INT_STAT 0x00 +#define INT_TC1 0x04 +#define INT_ERR1 0x0c +#define INT_ERR2 0x10 +#define INT_TC1_MASK 0x18 +#define INT_ERR1_MASK 0x20 +#define INT_ERR2_MASK 0x24 +#define INT_TC1_RAW 0x600 +#define INT_ERR1_RAW 0x608 +#define INT_ERR2_RAW 0x610 +#define CH_PRI 0x688 +#define CH_STAT 0x690 +#define CX_CUR_CNT 0x704 +#define CX_LLI 0x800 +#define CX_CNT 0x810 +#define CX_SRC 0x814 +#define CX_DST 0x818drivers/dma/k3dma.c line 56
Quoted 10 lines
drivers/dma/k3dma.c new file +10
-0+#define CX_CONFIG 0x81c +#define AXI_CONFIG 0x820 +#define DEF_AXI_CONFIG 0x201201 + +#define CX_LLI_CHAIN_EN 0x2 +#define CCFG_EN 0x1 +#define CCFG_MEM2PER (0x1 << 2) +#define CCFG_PER2MEM (0x2 << 2) +#define CCFG_SRCINCR (0x1 << 31) +#define CCFG_DSTINCR (0x1 << 30)I see these are not namespace aptly and can collide..
Quoted 5 lines
Quoted 3 lines
OK,
How about change name to
#defineCX_CFG0x81csince you are calling your driver
K3_DMAit would be apt to namespace all of the above withK3_INT_STATtoK3_CFG_ENand so on...
Sorry, does define of register need add name space? Is it looks too long?
There are many example keep define simple.drivers/dma/sa11x0-dma.c
#define DCSR_RUN (1 << 0) #define DCSR_IE (1 << 1) #define DCSR_ERROR (1 << 2)
drivers/dma/mxs-dma.c
#define CCW_CHAIN (1 << 2) #define CCW_IRQ (1 << 3) #define CCW_DEC_SEM (1 << 6)
Quoted 18 lines
Quoted 18 lines
#define
CX_CFG_EN0x1
#defineCX_CFG_MEM2PER(0x1<< 2)
~Quoted 12 lines
Quoted 12 lines
drivers/dma/k3dma.c new file +11
-0+case DMA_PAUSE : +dev_dbg (d->slave.dev,"vchan %p: pause\n" , &c->vc); +if (c->status ==DMA_IN_PROGRESS ) { + c->status =DMA_PAUSED ; +if (p) { +k3_dma_pause_dma (p, false); + }else { +spin_lock (&d->lock); +list_del_init (&c->node); +spin_unlock (&d->lock); + }
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
why do we need the else part here?
Since asynchronous mode is supported.
Desc is submitted to list but may not get physical channel to run.But when you pause you pause the running channel. You dont pause a descriptor. So whatever you are trying to imply doesnt make sense to me.
Here delete node from chan_pending, which will be quired from tasklet.
The physical channel is free matched.dma_issue_pending will put node to d->k3_dma_issue_pending if no phy allocated. Tasklet do two jobs
1, check running channel for new request form desc_issued.
2, check any new chan_pending and alloc phy if available.
If no phy, the node will be put in chan_pending.
If pause does not remove from chan_pending, it may be got from tasklet to start a new transaction.
So it's safe to remove from chan_pending when pause, and add back when resume.
PATCH v2 2/2 dmaengine: Add hisilicon k3 DMA engine driver
Quoted 1 line · zhangfei gao
There are many example keep define simple.
and there are others examples which have done the otherway. I would prefer that way please!
Quoted 14 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
why do we need the else part here?
Since asynchronous mode is supported.
Desc is submitted to list but may not get physical channel to run.But when you pause you pause the running channel. You dont pause a descriptor. So whatever you are trying to imply doesnt make sense to me.
Here delete node from
chan_pending, which will be quired from tasklet.The physical channel is free matched.
dma_issue_pendingwill put node tod->k3_dma_issue_pendingif no phy allocated. Tasklet do two jobs
1, check running channel for new request formdesc_issued.
2, check any newchan_pendingand alloc phy if available.
Quoted 4 lines
If no phy, the node will be put in
chan_pending.
If pause does not remove fromchan_pending, it may be got from tasklet to start a new transaction.
So it's safe to remove fromchan_pendingwhen pause, and add back when resume.
But when you add, where do you start from, from the start of the descriptor or the previosu position.
The point is pause-resume you dont need to do all that. Channel is paused so just pause it by stopping exuection, not more. Then you resume by asking controller to start from where it left off.
~Vinod
PATCH v2 2/2 dmaengine: Add hisilicon k3 DMA engine driver
Quoted 5 lines · Vinod Koul
Quoted 1 line · zhangfei gao
There are many example keep define simple.
and there are others examples which have done the otherway. I would prefer that way please!
Personally, I prefer simple is better, it is only used in the file. But is it fine to change.
Quoted 19 lines
Quoted 15 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
why do we need the else part here?
Since asynchronous mode is supported.
Desc is submitted to list but may not get physical channel to run.But when you pause you pause the running channel. You dont pause a descriptor. So whatever you are trying to imply doesnt make sense to me.
Here delete node from
chan_pending, which will be quired from tasklet.The physical channel is free matched.
dma_issue_pendingwill put node tod->k3_dma_issue_pendingif no phy allocated. Tasklet do two jobs
1, check running channel for new request formdesc_issued.
2, check any newchan_pendingand alloc phy if available.Quoted 4 lines
If no phy, the node will be put in
chan_pending.
If pause does not remove fromchan_pending, it may be got from tasklet to start a new transaction.
So it's safe to remove fromchan_pendingwhen pause, and add back when resume.
Quoted 6 lines
But when you add, where do you start from, from the start of the descriptor or the previosu position.
The point is pause-resume you dont need to do all that. Channel is paused so just pause it by stopping exuection, not more. Then you resume by asking controller to start from where it left off.
Since it is async mode, it does not know the physical channel is really started or not.
When desc is submitted, it can be
a). get phy and run, pause can stop and resume where it stops. b). Dot not start at all since no phy available (16 phys vs 27 request line), if pause do nothing it will stay in chan_pending, if tasklet happens again, it will be fetched and started, while upper layer thought it is already paused.
Do we need consider case b?
PATCH v2 2/2 dmaengine: Add hisilicon k3 DMA engine driver
Quoted 28 lines · zhangfei
Quoted 18 lines
Quoted 14 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
why do we need the else part here?
Since asynchronous mode is supported.
Desc is submitted to list but may not get physical channel to run.But when you pause you pause the running channel. You dont pause a descriptor. So whatever you are trying to imply doesnt make sense to me.
Here delete node from
chan_pending, which will be quired from tasklet.The physical channel is free matched.
dma_issue_pendingwill put node tod->k3_dma_issue_pendingif no phy allocated. Tasklet do two jobs
1, check running channel for new request formdesc_issued.
2, check any newchan_pendingand alloc phy if available.Quoted 4 lines
If no phy, the node will be put in
chan_pending.
If pause does not remove fromchan_pending, it may be got from tasklet to start a new transaction.
So it's safe to remove fromchan_pendingwhen pause, and add back when resume.Quoted 6 lines
But when you add, where do you start from, from the start of the descriptor or the previosu position.
The point is pause-resume you dont need to do all that. Channel is paused so just pause it by stopping exuection, not more. Then you resume by asking controller to start from where it left off.
Since it is async mode, it does not know the physical channel is really started or not.
Quoted 8 lines
When desc is submitted, it can be
a). get phy and run, pause can stop and resume where it stops. b). Dot not start at all since no phy available (16 phys vs 27 request line), if pause do nothing it will stay inchan_pending, if tasklet happens again, it will be fetched and started, while upper layer thought it is already paused.Do we need consider case b?
and how do you handle when the desc is running on a phy.
~Vinod
PATCH v2 2/2 dmaengine: Add hisilicon k3 DMA engine driver
Quoted 36 lines · Vinod Koul
Quoted 35 lines · zhangfei
Quoted 18 lines
Quoted 14 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
why do we need the else part here?
Since asynchronous mode is supported.
Desc is submitted to list but may not get physical channel to run.But when you pause you pause the running channel. You dont pause a descriptor. So whatever you are trying to imply doesnt make sense to me.
Here delete node from
chan_pending, which will be quired from tasklet.The physical channel is free matched.
dma_issue_pendingwill put node tod->k3_dma_issue_pendingif no phy allocated. Tasklet do two jobs
1, check running channel for new request formdesc_issued.
2, check any newchan_pendingand alloc phy if available.Quoted 4 lines
If no phy, the node will be put in
chan_pending.
If pause does not remove fromchan_pending, it may be got from tasklet to start a new transaction.
So it's safe to remove fromchan_pendingwhen pause, and add back when resume.Quoted 6 lines
But when you add, where do you start from, from the start of the descriptor or the previosu position.
The point is pause-resume you dont need to do all that. Channel is paused so just pause it by stopping exuection, not more. Then you resume by asking controller to start from where it left off.
Since it is async mode, it does not know the physical channel is really started or not.
When desc is submitted, it can be
a). get phy and run, pause can stop and resume where it stops. b). Dot not start at all since no phy available (16 phys vs 27 request line), if pause do nothing it will stay inchan_pending, if tasklet happens again, it will be fetched and started, while upper layer thought it is already paused.
Quoted 2 lines
Quoted 1 line
Do we need consider case b?
and how do you handle when the desc is running on a phy.
Currently k3dma is used with uart.
drivers/tty/serial/amba-pl011.cpl011_dma_rx_irq: dmaengine_pause -> device_tx_status
First pause channel, then get residue.k3_dma_tx_status:
a) if desc in issue queue, then it still not started, return total size.
b) otherwise get the residue via counting descriptor.
pl011 get the residue and recaculate the pending.
Have pass the stress test with amba-pl011.
Thanks
PATCH v2 2/2 dmaengine: Add hisilicon k3 DMA engine driver
Quoted 11 lines · zhangfei gao
Quoted 1 line
and how do you handle when the desc is running on a phy.
Currently k3dma is used with uart.
drivers/tty/serial/amba-pl011.cpl011_dma_rx_irq:dmaengine_pause->device_tx_status
First pause channel, then get residue.k3_dma_tx_status:
a) if desc in issue queue, then it still not started, return total size.
b) otherwise get the residue via counting descriptor.
pl011 get the residue and recaculate the pending.
Okay thanks, While looks like for uart this is usage (note this driver doesnt not call resume. So for that you dont need resume to be implemented.
Also, the driver will call the residue and then terminate always. So again I dont see a point of yours with what your are doing with descriptors.
And since you support PAUSE/RESUME, if someone tries to use this with sound, this will break as sound will call pause and then resume. No resubmitting or terminating.
Quoted 1 line
Have pass the stress test with amba-pl011.
See the example of amba-pl08x.c, I think Linux W had this in mind when he added this and other exapmple of PAUSE and RESUME implemented in drivers
Also, again you PAUSE a channel not a trancation, so it doesnt not really matter where the descriptor is. In phy or somehwere else! We shouldnt care
~Vinod
PATCH v2 2/2 dmaengine: Add hisilicon k3 DMA engine driver
Quoted 15 lines · Vinod Koul
Quoted 11 lines · zhangfei gao
Quoted 1 line
and how do you handle when the desc is running on a phy.
Currently k3dma is used with uart.
drivers/tty/serial/amba-pl011.cpl011_dma_rx_irq:dmaengine_pause->device_tx_status
First pause channel, then get residue.k3_dma_tx_status:
a) if desc in issue queue, then it still not started, return total size.
b) otherwise get the residue via counting descriptor.
pl011 get the residue and recaculate the pending.Okay thanks, While looks like for uart this is usage (note this driver doesnt not call resume. So for that you dont need resume to be implemented.
Quoted 6 lines
Also, the driver will call the residue and then terminate always. So again I dont see a point of yours with what your are doing with descriptors.
And since you support PAUSE/RESUME, if someone tries to use this with sound, this will break as sound will call pause and then resume. No resubmitting or terminating.
In k3v2 platform, k3dma is shared dma, used by peripherals, like uart, ssp. While audio has special dma, can not used by others, even not use dmaengine, which expose to system.
PAUSE & RESUME is added ONLY because we found amba-pl011 need when enable dma.
Quoted 7 lines
Quoted 2 lines
Have pass the stress test with amba-pl011.
See the example of
amba-pl08x.c, I think Linux W had this in mind when he added this and other exapmple of PAUSE and RESUME implemented in driversAlso, again you PAUSE a channel not a trancation, so it doesnt not really matter where the descriptor is. In phy or somehwere else! We shouldnt care
Excuse me for my bad English.
I am really lost and don't know what to do now. Are you concern:
1. Why need else in case DMA_PAUSE, no need?
2. device_tx_status is not called in audio and directly dmaengine_pause -> dmaengine_resume?
3. ?
Besides, where I should take example of amba-pl08x.c, pl08x_control, case DMA_PAUSE & DMA_RESUME or pl08x_dma_tx_status?
Some difference I see is k3dma.c only has one tasklet for all channels, The tasklet will check pending channel and alloc physical channel. While the issue_pending does not really alloc physical channel to support more request then physical channel.
So when suspend we have to consider remove the channel from chan_pending.
When trying use virt-dma, have study drivers/dma/sa11x0-dma.c as good reference.
Sorry for my bad understanding.
PATCH v2 2/2 dmaengine: Add hisilicon k3 DMA engine driver
Quoted 17 lines · zhangfei gao
Excuse me for my bad English.
I am really lost and don't know what to do now. Are you concern:
1. Why need else in caseDMA_PAUSE, no need?
2.device_tx_statusis not called in audio and directlydmaengine_pause->dmaengine_resume?
3. ?Besides, where I should take example of
amba-pl08x.c,pl08x_control, caseDMA_PAUSE&DMA_RESUMEorpl08x_dma_tx_status?Some difference I see is
k3dma.conly has one tasklet for all channels, The tasklet will check pending channel and alloc physical channel. While theissue_pendingdoes not really alloc physical channel to support more request then physical channel.
So when suspend we have to consider remove the channel fromchan_pending.
Quoted 3 lines
When trying use virt-dma, have study
drivers/dma/sa11x0-dma.cas good reference.Sorry for my bad understanding.
No issues...
I think i am okay with else part looking at the driver again.
Can you resend by rebasing on my next and fxing any other issues
~Vinod
PATCH v2 2/2 dmaengine: Add hisilicon k3 DMA engine driver
In reply to Vinod KoulQuoted 22 lines · Vinod Koul
Quoted 11 lines · zhangfei gao
Quoted 1 line
and how do you handle when the desc is running on a phy.
Currently k3dma is used with uart.
drivers/tty/serial/amba-pl011.cpl011_dma_rx_irq:dmaengine_pause->device_tx_status
First pause channel, then get residue.k3_dma_tx_status:
a) if desc in issue queue, then it still not started, return total size.
b) otherwise get the residue via counting descriptor.
pl011 get the residue and recaculate the pending.Okay thanks, While looks like for uart this is usage (note this driver doesnt not call resume. So for that you dont need resume to be implemented.
Also, the driver will call the residue and then terminate always. So again I dont see a point of yours with what your are doing with descriptors.
And since you support PAUSE/RESUME, if someone tries to use this with sound, this will break as sound will call pause and then resume. No resubmitting or terminating.
Quoted 8 lines
Quoted 1 line
Have pass the stress test with amba-pl011.
See the example of
amba-pl08x.c, I think Linux W had this in mind when he added this and other exapmple of PAUSE and RESUME implemented in driversAlso, again you PAUSE a channel not a trancation, so it doesnt not really matter where the descriptor is. In phy or somehwere else! We shouldnt care
~Vinod
Dear Vinod
I think your concern is device_tx_status does not return right pos.
prep_slave_sg -> vchan_tx_prep -> vd->tx.tx_submit = vchan_tx_submit; vchan_tx_submit -> cookie = dma_cookie_assign(tx)
So one cookie is assigned for one tx_submit
sound/soc/soc-dmaengine-pcm.c:dmaengine_pcm_prepare_and_submit -> dmaengine_prep_dma_cyclic -> prtd->cookie = dmaengine_submit
The cookie is get for one pare prepare and submit.
k3_dma_tx_status:
If cookie is in the issue_queue, means sg list not send out, return total size of the prepare, including all descriptor.
If the sg list already run, the count descriptor and return the left value. Since only one cookie match all descriptor, or descriptor is organized inside, transparent to upper layer, return total left bytes.
Is this fine?
Thanks