dma: mv_xor_v2: fixes and improvements
Subsystems (2)
cover dma: mv_xor_v2: fixes and improvements
Hello,
This series brings a number of fixes (first 7 patches) and improvements (last 3 patches) to the driver used for the Marvell XORv2 engine used in Marvell Armada 7K/8K.
Thanks,
Thomas
Hanna Hawa (4):
dma: mv_xor_v2: enable XOR engine after its configuration
dma: mv_xor_v2: implement proper interrupt coalescing
dma: mv_xor_v2: remove unnecessary write to DESQ_STOP register
dma: mv_xor_v2: add support for suspend/resume
Thomas Petazzoni (6):
dma: mv_xor_v2: handle mv_xor_v2_prep_sw_desc() error properly
dma: mv_xor_v2: properly handle wrapping in the array of HW
descriptors
dma: mv_xor_v2: do not use descriptors not acked by async_tx
dma: mv_xor_v2: fix tx_submit() implementation
dma: mv_xor_v2: remove interrupt coalescing
dma: mv_xor_v2: set DMA mask to 40 bits
drivers/dma/mv_xor_v2.c | 147 ++++++++++++++++++++++++++++++------------------ 1 file changed, 91 insertions(+), 56 deletions(-)
Signature · 1 line
2.7.4
PATCH 1/10 dma: mv_xor_v2: handle mv_xor_v2_prep_sw_desc() error properly
The mv_xor_v2_prep_sw_desc() is called from a few different places in the driver, but we never take into account the fact that it might return NULL. This commit fixes that, ensuring that we don't panic if there are no more descriptors available.
--- drivers/dma/mv_xor_v2.c | 6 ++++++ 1 file changed, 6 insertions(+)
drivers/dma/mv_xor_v2.c +6 -0
@@ -389,6 +389,8 @@ mv_xor_v2_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, __func__, len, &src, &dest, flags); sw_desc =mv_xor_v2_prep_sw_desc (xor_dev); +if (!sw_desc) +return NULL ; sw_desc->async_tx.flags = flags;
@@ -443,6 +445,8 @@ mv_xor_v2_prep_dma_xor(struct dma_chan *chan, dma_addr_t dest, dma_addr_t *src, __func__, src_cnt, len, &dest, flags); sw_desc =mv_xor_v2_prep_sw_desc (xor_dev); +if (!sw_desc) +return NULL ; sw_desc->async_tx.flags = flags;
@@ -491,6 +495,8 @@ mv_xor_v2_prep_dma_interrupt(struct dma_chan *chan, unsigned long flags) container_of(chan, struct mv_xor_v2_device, dmachan); sw_desc =mv_xor_v2_prep_sw_desc (xor_dev); +if (!sw_desc) +return NULL ;/* set the HW descriptor */ hw_descriptor = &sw_desc->hw_desc;
Signature · 1 line
2.7.4
PATCH 2/10 dma: mv_xor_v2: properly handle wrapping in the array of HW descriptors
In reply to Thomas Petazzonimv_xor_v2_tasklet() is looping over completed HW descriptors. Before the loop, it initializes 'next_pending_hw_desc' to the first HW descriptor to handle, and then the loop simply increments this point, without taking care of wrapping when we reach the last HW descriptor. The 'pending_ptr' index was being wrapped back to 0 at the end, but it wasn't used in each iteration of the loop to calculate next_pending_hw_desc.
This commit fixes that, and makes next_pending_hw_desc a variable local to the loop itself.
--- drivers/dma/mv_xor_v2.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)
drivers/dma/mv_xor_v2.c +4 -10
@@ -560,7 +560,6 @@ static void mv_xor_v2_tasklet(unsigned long data) {struct mv_xor_v2_device *xor_dev = (struct mv_xor_v2_device *) data;int pending_ptr, num_of_pending, i; -struct mv_xor_v2_descriptor *next_pending_hw_desc =NULL ;struct mv_xor_v2_sw_desc *next_pending_sw_desc =NULL ;dev_dbg (xor_dev->dmadev.dev,"%s %d\n" , __func__, __LINE__);
@@ -568,17 +567,10 @@ static void mv_xor_v2_tasklet(unsigned long data)/* get the pending descriptors parameters */ num_of_pending =mv_xor_v2_get_pending_params (xor_dev, &pending_ptr); -/* next HW descriptor */ - next_pending_hw_desc = xor_dev->hw_desq_virt + pending_ptr; -/* loop over free descriptors */ for (i =0 ; i < num_of_pending; i++) { - -if (pending_ptr >MV_XOR_V2_DESC_NUM ) - pending_ptr =0 ; - -if (next_pending_sw_desc !=NULL ) - next_pending_hw_desc++; +struct mv_xor_v2_descriptor *next_pending_hw_desc = + xor_dev->hw_desq_virt + pending_ptr;/* get the SW descriptor related to the HW descriptor */ next_pending_sw_desc =
@@ -614,6 +606,8 @@ static void mv_xor_v2_tasklet(unsigned long data)/* increment the next descriptor */ pending_ptr++; +if (pending_ptr >=MV_XOR_V2_DESC_NUM ) + pending_ptr =0 ; }if (num_of_pending !=0 ) {
Signature · 1 line
2.7.4
PATCH 3/10 dma: mv_xor_v2: do not use descriptors not acked by async_tx
In reply to Thomas PetazzoniDescriptors that have not been acknowledged by the async_tx layer should not be re-used, so this commit adjusts the implementation of mv_xor_v2_prep_sw_desc() to skip descriptors for which async_tx_test_ack() is false.
--- drivers/dma/mv_xor_v2.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-)
drivers/dma/mv_xor_v2.c +22 -10
@@ -344,6 +344,7 @@ static struct mv_xor_v2_sw_desc * mv_xor_v2_prep_sw_desc(struct mv_xor_v2_device *xor_dev) {struct mv_xor_v2_sw_desc *sw_desc; +bool found = false;/* Lock the channel */ spin_lock_bh (&xor_dev->lock);
@@ -355,19 +356,23 @@ mv_xor_v2_prep_sw_desc(struct mv_xor_v2_device *xor_dev)return NULL ; } -/* get a free SW descriptor from the SW DESQ */ - sw_desc =list_first_entry (&xor_dev->free_sw_desc, - struct mv_xor_v2_sw_desc, free_list); +list_for_each_entry (sw_desc, &xor_dev->free_sw_desc, free_list) { +if (async_tx_test_ack (&sw_desc->async_tx)) { + found = true; +break ; + } + } + +if (!found) { +spin_unlock_bh (&xor_dev->lock); +return NULL ; + } +list_del (&sw_desc->free_list);/* Release the channel */ spin_unlock_bh (&xor_dev->lock); -/* set the async tx descriptor */ -dma_async_tx_descriptor_init (&sw_desc->async_tx, &xor_dev->dmachan); - sw_desc->async_tx.tx_submit = mv_xor_v2_tx_submit; -async_tx_ack (&sw_desc->async_tx); -return sw_desc; }
@@ -785,8 +790,15 @@ static int mv_xor_v2_probe(struct platform_device *pdev)/* add all SW descriptors to the free list */ for (i =0 ; i <MV_XOR_V2_DESC_NUM ; i++) { - xor_dev->sw_desq[i].idx = i; -list_add (&xor_dev->sw_desq[i].free_list, +struct mv_xor_v2_sw_desc *sw_desc = + xor_dev->sw_desq + i; + sw_desc->idx = i; +dma_async_tx_descriptor_init (&sw_desc->async_tx, + &xor_dev->dmachan); + sw_desc->async_tx.tx_submit = mv_xor_v2_tx_submit; +async_tx_ack (&sw_desc->async_tx); + +list_add (&sw_desc->free_list, &xor_dev->free_sw_desc); }
Signature · 1 line
2.7.4
PATCH 4/10 dma: mv_xor_v2: enable XOR engine after its configuration
In reply to Thomas PetazzoniFrom: Hanna Hawa <[email protected]>
The engine was enabled prior to its configuration, which isn't correct. This patch relocates the activation of the XOR engine, to be after the configuration of the XOR engine.
--- drivers/dma/mv_xor_v2.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/dma/mv_xor_v2.c +3 -3
@@ -653,9 +653,6 @@ static int mv_xor_v2_descq_init(struct mv_xor_v2_device *xor_dev)writel ((xor_dev->hw_desq &0xFFFF00000000 ) >>32 , xor_dev->dma_base +MV_XOR_V2_DMA_DESQ_BAHR_OFF ); -/* enable the DMA engine */ -writel (0 , xor_dev->dma_base +MV_XOR_V2_DMA_DESQ_STOP_OFF ); -/* * This is a temporary solution, until we activate the * SMMU. Set the attributes for reading & writing data buffers
@@ -699,6 +696,9 @@ static int mv_xor_v2_descq_init(struct mv_xor_v2_device *xor_dev) reg |=MV_XOR_V2_GLOB_PAUSE_AXI_TIME_DIS_VAL ;writel (reg, xor_dev->glob_base +MV_XOR_V2_GLOB_PAUSE ); +/* enable the DMA engine */ +writel (0 , xor_dev->dma_base +MV_XOR_V2_DMA_DESQ_STOP_OFF ); +return 0 ; }
Signature · 1 line
2.7.4
PATCH 5/10 dma: mv_xor_v2: fix tx_submit() implementation
In reply to Thomas PetazzoniThe mv_xor_v2_tx_submit() gets the next available HW descriptor by calling mv_xor_v2_get_desq_write_ptr(), which reads a HW register telling the next available HW descriptor. This was working fine when HW descriptors were issued for processing directly in tx_submit().
However, as part of the review process of the driver, a change was requested to move the actual kick-off of HW descriptors processing to ->issue_pending(). Due to this, reading the HW register to know the next available HW descriptor no longer works.
So instead of using this HW register, we implemented a software index pointing to the next available HW descriptor.
--- drivers/dma/mv_xor_v2.c | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-)
drivers/dma/mv_xor_v2.c +5 -17
@@ -161,6 +161,7 @@ struct mv_xor_v2_device {struct mv_xor_v2_sw_desc *sw_desq;int desc_size;unsigned int npendings; +unsigned int hw_queue_idx; };/**
@@ -214,18 +215,6 @@ static void mv_xor_v2_set_data_buffers(struct mv_xor_v2_device *xor_dev, }/* -* Return the next available index in the DESQ. -*/ -static int mv_xor_v2_get_desq_write_ptr (struct mv_xor_v2_device *xor_dev) -{ -/* read the index for the next available descriptor in the DESQ */ -u32 reg =readl (xor_dev->dma_base +MV_XOR_V2_DMA_DESQ_ALLOC_OFF ); - -return ((reg >>MV_XOR_V2_DMA_DESQ_ALLOC_WRPTR_SHIFT ) - &MV_XOR_V2_DMA_DESQ_ALLOC_WRPTR_MASK ); -} - -/* * notify the engine of new descriptors, and update the available index. */ static void mv_xor_v2_add_desc_to_desq(struct mv_xor_v2_device *xor_dev,
@@ -306,7 +295,6 @@ static irqreturn_t mv_xor_v2_interrupt_handler(int irq, void *data)static dma_cookie_t mv_xor_v2_tx_submit (struct dma_async_tx_descriptor *tx) { -int desq_ptr;void *dest_hw_desc;dma_cookie_t cookie; struct mv_xor_v2_sw_desc *sw_desc =
@@ -322,15 +310,15 @@ mv_xor_v2_tx_submit(struct dma_async_tx_descriptor *tx)spin_lock_bh (&xor_dev->lock); cookie =dma_cookie_assign (tx); -/* get the next available slot in the DESQ */ - desq_ptr =mv_xor_v2_get_desq_write_ptr (xor_dev); -/* copy the HW descriptor from the SW descriptor to the DESQ */ - dest_hw_desc = xor_dev->hw_desq_virt + desq_ptr; + dest_hw_desc = xor_dev->hw_desq_virt + xor_dev->hw_queue_idx;memcpy (dest_hw_desc, &sw_desc->hw_desc, xor_dev->desc_size); xor_dev->npendings++; + xor_dev->hw_queue_idx++; +if (xor_dev->hw_queue_idx >=MV_XOR_V2_DESC_NUM ) + xor_dev->hw_queue_idx =0 ;spin_unlock_bh (&xor_dev->lock);
Signature · 1 line
2.7.4
PATCH 6/10 dma: mv_xor_v2: remove interrupt coalescing
In reply to Thomas PetazzoniThe current implementation of interrupt coalescing doesn't work, because it doesn't configure the coalescing timer, which is needed to make sure we get an interrupt at some point.
As a fix for stable, we simply remove the interrupt coalescing functionality. It will be re-introduced properly in a future commit.
--- drivers/dma/mv_xor_v2.c | 25 ------------------------- 1 file changed, 25 deletions(-)
drivers/dma/mv_xor_v2.c +0 -25
@@ -246,22 +246,6 @@ static int mv_xor_v2_set_desc_size(struct mv_xor_v2_device *xor_dev)return MV_XOR_V2_EXT_DESC_SIZE ; } -/* -* Set the IMSG threshold -*/ -static inline -void mv_xor_v2_set_imsg_thrd (struct mv_xor_v2_device *xor_dev,int thrd_val) -{ -u32 reg; - - reg =readl (xor_dev->dma_base +MV_XOR_V2_DMA_IMSG_THRD_OFF ); - - reg &= (~MV_XOR_V2_DMA_IMSG_THRD_MASK <<MV_XOR_V2_DMA_IMSG_THRD_SHIFT ); - reg |= (thrd_val <<MV_XOR_V2_DMA_IMSG_THRD_SHIFT ); - -writel (reg, xor_dev->dma_base +MV_XOR_V2_DMA_IMSG_THRD_OFF ); -} -static irqreturn_t mv_xor_v2_interrupt_handler (int irq,void *data) {struct mv_xor_v2_device *xor_dev = data;
@@ -277,12 +261,6 @@ static irqreturn_t mv_xor_v2_interrupt_handler(int irq, void *data)if (!ndescs)return IRQ_NONE ; -/* -* Update IMSG threshold, to disable new IMSG interrupts until -* end of the tasklet -*/ -mv_xor_v2_set_imsg_thrd (xor_dev,MV_XOR_V2_DESC_NUM ); -/* schedule a tasklet to handle descriptors callbacks */ tasklet_schedule (&xor_dev->irq_tasklet);
@@ -607,9 +585,6 @@ static void mv_xor_v2_tasklet(unsigned long data)/* free the descriptores */ mv_xor_v2_free_desc_from_desq (xor_dev, num_of_pending); } - -/* Update IMSG threshold, to enable new IMSG interrupts */ -mv_xor_v2_set_imsg_thrd (xor_dev,0 ); }/*
Signature · 1 line
2.7.4
PATCH 7/10 dma: mv_xor_v2: set DMA mask to 40 bits
In reply to Thomas PetazzoniThe XORv2 engine on Armada 7K/8K can only access the first 40 bits of the physical address space, so the DMA mask must be set accordingly.
--- drivers/dma/mv_xor_v2.c | 4 ++++ 1 file changed, 4 insertions(+)
drivers/dma/mv_xor_v2.c +4 -0
@@ -693,6 +693,10 @@ static int mv_xor_v2_probe(struct platform_device *pdev)platform_set_drvdata (pdev, xor_dev); + ret =dma_set_mask_and_coherent (&pdev->dev,DMA_BIT_MASK (40 )); +if (ret) +return ret; + xor_dev->clk =devm_clk_get (&pdev->dev,NULL );if (IS_ERR (xor_dev->clk) &&PTR_ERR (xor_dev->clk) == -EPROBE_DEFER )return -EPROBE_DEFER ;
Signature · 1 line
2.7.4
PATCH 8/10 dma: mv_xor_v2: implement proper interrupt coalescing
In reply to Thomas PetazzoniFrom: Hanna Hawa <[email protected]>
Until now, the driver was not using interrupt coalescing: one interrupt was generated for each descriptor processed by the XOR engine. This commit changes that by using the interrupt coalescing features of the hardware, by setting both a number of descriptors processed before an interrupt is generated and a timeout before an interrupt is generated.
--- drivers/dma/mv_xor_v2.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)
drivers/dma/mv_xor_v2.c +36 -0
@@ -42,6 +42,7 @@#define MV_XOR_V2_DMA_IMSG_THRD_OFF 0x018#define MV_XOR_V2_DMA_IMSG_THRD_MASK 0x7FFF#define MV_XOR_V2_DMA_IMSG_THRD_SHIFT 0x0 +#define MV_XOR_V2_DMA_IMSG_TIMER_EN BIT(18)#define MV_XOR_V2_DMA_DESQ_AWATTR_OFF 0x01C/* Same flags as MV_XOR_V2_DMA_DESQ_ARATTR_OFF */ #define MV_XOR_V2_DMA_DESQ_ALLOC_OFF 0x04C
@@ -55,6 +56,9 @@#define MV_XOR_V2_DMA_DESQ_STOP_OFF 0x800#define MV_XOR_V2_DMA_DESQ_DEALLOC_OFF 0x804#define MV_XOR_V2_DMA_DESQ_ADD_OFF 0x808 +#define MV_XOR_V2_DMA_IMSG_TMOT 0x810 +#define MV_XOR_V2_DMA_IMSG_TIMER_THRD_MASK 0x1FFF +#define MV_XOR_V2_DMA_IMSG_TIMER_THRD_SHIFT 0/* XOR Global registers */ #define MV_XOR_V2_GLOB_BW_CTRL 0x4
@@ -90,6 +94,13 @@*/ #define MV_XOR_V2_DESC_NUM 1024 +/* +* Threshold values for descriptors and timeout, determined by +* experimentation as giving a good level of performance. +*/ +#define MV_XOR_V2_DONE_IMSG_THRD 0x14 +#define MV_XOR_V2_TIMER_THRD 0xB0 +/** * struct mv_xor_v2_descriptor - DMA HW descriptor * @desc_id: used by S/W and is not affected by H/W.
@@ -246,6 +257,29 @@ static int mv_xor_v2_set_desc_size(struct mv_xor_v2_device *xor_dev)return MV_XOR_V2_EXT_DESC_SIZE ; } +/* +* Set the IMSG threshold +*/ +static inline +void mv_xor_v2_enable_imsg_thrd (struct mv_xor_v2_device *xor_dev) +{ +u32 reg; + +/* Configure threshold of number of descriptors, and enable timer */ + reg =readl (xor_dev->dma_base +MV_XOR_V2_DMA_IMSG_THRD_OFF ); + reg &= (~MV_XOR_V2_DMA_IMSG_THRD_MASK <<MV_XOR_V2_DMA_IMSG_THRD_SHIFT ); + reg |= (MV_XOR_V2_DONE_IMSG_THRD <<MV_XOR_V2_DMA_IMSG_THRD_SHIFT ); + reg |=MV_XOR_V2_DMA_IMSG_TIMER_EN ; +writel (reg, xor_dev->dma_base +MV_XOR_V2_DMA_IMSG_THRD_OFF ); + +/* Configure Timer Threshold */ + reg =readl (xor_dev->dma_base +MV_XOR_V2_DMA_IMSG_TMOT ); + reg &= (~MV_XOR_V2_DMA_IMSG_TIMER_THRD_MASK << +MV_XOR_V2_DMA_IMSG_TIMER_THRD_SHIFT ); + reg |= (MV_XOR_V2_TIMER_THRD <<MV_XOR_V2_DMA_IMSG_TIMER_THRD_SHIFT ); +writel (reg, xor_dev->dma_base +MV_XOR_V2_DMA_IMSG_TMOT ); +} +static irqreturn_t mv_xor_v2_interrupt_handler (int irq,void *data) {struct mv_xor_v2_device *xor_dev = data;
@@ -795,6 +829,8 @@ static int mv_xor_v2_probe(struct platform_device *pdev)list_add_tail (&xor_dev->dmachan.device_node, &dma_dev->channels); +mv_xor_v2_enable_imsg_thrd (xor_dev); +mv_xor_v2_descq_init (xor_dev); ret =dma_async_device_register (dma_dev);
Signature · 1 line
2.7.4
PATCH 9/10 dma: mv_xor_v2: remove unnecessary write to DESQ_STOP register
In reply to Thomas PetazzoniFrom: Hanna Hawa <[email protected]>
Remove unnecessary write to DESQ_STOP register, this register is used to enable or disable the XOR engine, and not to issue all pending descriptors in the queue. mv_xor_v2 driver already writes to this register and enable XOR engine in the mv_xor_v2_descq_init() function, called during initialization.
--- drivers/dma/mv_xor_v2.c | 3 --- 1 file changed, 3 deletions(-)
drivers/dma/mv_xor_v2.c +0 -3
@@ -535,9 +535,6 @@ static void mv_xor_v2_issue_pending(struct dma_chan *chan)mv_xor_v2_add_desc_to_desq (xor_dev, xor_dev->npendings); xor_dev->npendings =0 ; -/* Activate the channel */ -writel (0 , xor_dev->dma_base +MV_XOR_V2_DMA_DESQ_STOP_OFF ); -spin_unlock_bh (&xor_dev->lock); }
Signature · 1 line
2.7.4
PATCH 10/10 dma: mv_xor_v2: add support for suspend/resume
In reply to Thomas PetazzoniFrom: Hanna Hawa <[email protected]>
This commit adds the support for suspend/resume in the mv_xor_v2 driver. The suspend suspend function disables the XOR engine after the DMA stack has handled all pending descriptors in the queue. The resume function re-configures the XOR engine and re-enables the engine.
--- drivers/dma/mv_xor_v2.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)
drivers/dma/mv_xor_v2.c +23 -0
@@ -696,6 +696,27 @@ static int mv_xor_v2_descq_init(struct mv_xor_v2_device *xor_dev)return 0 ; } +static int mv_xor_v2_suspend (struct platform_device *dev,pm_message_t state) +{ +struct mv_xor_v2_device *xor_dev =platform_get_drvdata (dev); + +/* Set this bit to disable to stop the XOR unit. */ +writel (0x1 , xor_dev->dma_base +MV_XOR_V2_DMA_DESQ_STOP_OFF ); + +return 0 ; +} + +static int mv_xor_v2_resume (struct platform_device *dev) +{ +struct mv_xor_v2_device *xor_dev =platform_get_drvdata (dev); + +mv_xor_v2_set_desc_size (xor_dev); +mv_xor_v2_enable_imsg_thrd (xor_dev); +mv_xor_v2_descq_init (xor_dev); + +return 0 ; +} +static int mv_xor_v2_probe (struct platform_device *pdev) {struct mv_xor_v2_device *xor_dev;
@@ -877,6 +898,8 @@ MODULE_DEVICE_TABLE(of, mv_xor_v2_dt_ids);static struct platform_driver mv_xor_v2_driver = { .probe = mv_xor_v2_probe, + .suspend = mv_xor_v2_suspend, + .resume = mv_xor_v2_resume, .remove = mv_xor_v2_remove, .driver = { .name ="mv_xor_v2" ,
Signature · 1 line
2.7.4
cover dma: mv_xor_v2: fixes and improvements
In reply to Thomas PetazzoniQuoted 5 lines · Thomas Petazzoni
Hello,
This series brings a number of fixes (first 7 patches) and improvements (last 3 patches) to the driver used for the Marvell XORv2 engine used in Marvell Armada 7K/8K.
And 7 going to stable :(
Anyway I have applied all, *after* fixing the subsystem name for all :(
Signature · 1 line
~Vinod
cover dma: mv_xor_v2: fixes and improvements
Hello,
Quoted 5 lines · Vinod Koul
Quoted 3 lines
This series brings a number of fixes (first 7 patches) and improvements (last 3 patches) to the driver used for the Marvell XORv2 engine used in Marvell Armada 7K/8K.
And 7 going to stable :(
Yes, but hopefully we should now be a good shape.
Quoted 1 line
Anyway I have applied all, *after* fixing the subsystem name for all :(
Sorry for getting the subsystem name wrong, I would have been happy to fix that myself and resend to reduce your workload. But anyway, thanks for fixing those and applying the patches!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com