DaVinci: Update for edma_alloc/free_cont_slots API

Commit Message

[e-mail] Sept. 11, 2009, 7:56 p.m. UTC

From: Sandeep Paulraj <[e-mail]>

The patch does the following

1) The edma_alloc_cont_slots API is used for obtaining a set of
contiguous slots. When we use the "_ANY" option with this
API, by definition of this option it is suppossed to start
looking for a set of contiguous slots starting from slot 64 for
DaVinci SOC's and 32 for Primus. This has been explained in
the API description in the driver itself. So when we use the
"_ANY" option with this API, the slot number passed as
an argument should be a "don't care".
This patch takes care of this condition mentioned above.
When checking to see if the starting slot is a valid number,
it checks to make sure that the "_ANY" option is not used.

2) In the reserve contiguous params function, break statement
was incorrectly implemented

3) The reserve_contiguous_params function is a remnant of past TI
LSP releases. It should be named reserve_contiguous_slots. The API
description and the function is also being appropriately changed so 
that we use the word 'slot" instead of "param".

4) In the edma_free_cont_slots API, the variable slot was being modified
and then used in the for loop, so we end up in a never ending loop.

Signed-off-by: Sandeep Paulraj <[e-mail]>
---
 arch/arm/mach-davinci/dma.c |   57 ++++++++++++++++++++++--------------------
 1 files changed, 30 insertions(+), 27 deletions(-)

Comments

Kevin Hilman Sept. 16, 2009, 3:48 p.m. UTC | #1

[e-mail] writes:

> From: Sandeep Paulraj <[e-mail]>
>
> The patch does the following
>
> 1) The edma_alloc_cont_slots API is used for obtaining a set of
> contiguous slots. When we use the "_ANY" option with this
> API, by definition of this option it is suppossed to start
> looking for a set of contiguous slots starting from slot 64 for
> DaVinci SOC's and 32 for Primus. This has been explained in
> the API description in the driver itself. So when we use the
> "_ANY" option with this API, the slot number passed as
> an argument should be a "don't care".
> This patch takes care of this condition mentioned above.
> When checking to see if the starting slot is a valid number,
> it checks to make sure that the "_ANY" option is not used.
>
> 2) In the reserve contiguous params function, break statement
> was incorrectly implemented
>
> 3) The reserve_contiguous_params function is a remnant of past TI
> LSP releases. It should be named reserve_contiguous_slots. The API
> description and the function is also being appropriately changed so 
> that we use the word 'slot" instead of "param".
>
> 4) In the edma_free_cont_slots API, the variable slot was being modified
> and then used in the for loop, so we end up in a never ending loop.
>
> Signed-off-by: Sandeep Paulraj <[e-mail]>

This patch should be broken up along the lines of your 1-4 list above.  This
helps reviewers be sure which parts are part of which change.

Also, tqhe bugfixes ones can be queued for the 2.6.31-rc cycle and the others
will be queued for 2.6.32.

Thanks,

Kevin

Patch

diff --git a/arch/arm/mach-davinci/dma.c b/arch/arm/mach-davinci/dma.c
index f2e57d2..bfce4b5 100644
--- a/arch/arm/mach-davinci/dma.c
+++ b/arch/arm/mach-davinci/dma.c
@@ -509,23 +509,24 @@  static irqreturn_t dma_tc1err_handler(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-static int reserve_contiguous_params(int ctlr, unsigned int id,
-				     unsigned int num_params,
-				     unsigned int start_param)
+static int reserve_contiguous_slots(int ctlr, unsigned int id,
+				     unsigned int num_slots,
+				     unsigned int start_slot)
 {
 	int i, j;
-	unsigned int count = num_params;
+	unsigned int count = num_slots;
 
-	for (i = start_param; i < edma_info[ctlr]->num_slots; ++i) {
+	for (i = start_slot; i < edma_info[ctlr]->num_slots; ++i) {
 		j = EDMA_CHAN_SLOT(i);
-		if (!test_and_set_bit(j, edma_info[ctlr]->edma_inuse))
+		if (!test_and_set_bit(j, edma_info[ctlr]->edma_inuse)) {
 			count--;
 			if (count == 0)
 				break;
+		}
 		else if (id == EDMA_CONT_PARAMS_FIXED_EXACT)
 			break;
 		else
-			count = num_params;
+			count = num_slots;
 	}
 
 	/*
@@ -535,17 +536,17 @@  static int reserve_contiguous_params(int ctlr, unsigned int id,
 	 * requested as we may reach the total number of parameter RAMs
 	 */
 	if (count) {
-		for (j = i - num_params + count + 1; j <= i ; ++j)
+		for (j = i - num_slots + count + 1; j <= i ; ++j)
 			clear_bit(j, edma_info[ctlr]->edma_inuse);
 
 		return -EBUSY;
 	}
 
-	for (j = i - num_params + 1; j <= i; ++j)
+	for (j = i - num_slots + 1; j <= i; ++j)
 		memcpy_toio(edmacc_regs_base[ctlr] + PARM_OFFSET(j),
 			&dummy_paramset, PARM_SIZE);
 
-	return EDMA_CTLR_CHAN(ctlr, i - num_params + 1);
+	return EDMA_CTLR_CHAN(ctlr, i - num_slots + 1);
 }
 
 /*-----------------------------------------------------------------------*/
@@ -743,26 +744,27 @@  EXPORT_SYMBOL(edma_free_slot);
 /**
  * edma_alloc_cont_slots- alloc contiguous parameter RAM slots
  * The API will return the starting point of a set of
- * contiguous PARAM's that have been requested
+ * contiguous Paramter RAM slots that have been requested
  *
  * @id: can only be EDMA_CONT_PARAMS_ANY or EDMA_CONT_PARAMS_FIXED_EXACT
  * or EDMA_CONT_PARAMS_FIXED_NOT_EXACT
- * @count: number of contiguous Paramter RAM's
- * @param  - the start value of Parameter RAM that should be passed if id
+ * @count: number of contiguous Paramter RAM slots
+ * @slot:  the start value of Parameter RAM slot that should be passed if id
  * is EDMA_CONT_PARAMS_FIXED_EXACT or EDMA_CONT_PARAMS_FIXED_NOT_EXACT
  *
  * If id is EDMA_CONT_PARAMS_ANY then the API starts looking for a set of
- * contiguous Parameter RAMs from parameter RAM 64 in the case of DaVinci SOCs
- * and 32 in the case of Primus
+ * contiguous Parameter RAM slots from parameter RAM slot 64 in the case of
+ * DaVinci SOCs and 32 in the case of Primus
  *
  * If id is EDMA_CONT_PARAMS_FIXED_EXACT then the API starts looking for a
- * set of contiguous parameter RAMs from the "param" that is passed as an
+ * set of contiguous parameter RAM slots from the "slot" that is passed as an
  * argument to the API.
  *
  * If id is EDMA_CONT_PARAMS_FIXED_NOT_EXACT then the API initially tries
- * starts looking for a set of contiguous parameter RAMs from the "param"
+ * starts looking for a set of contiguous parameter RAM slots from the "slot"
  * that is passed as an argument to the API. On failure the API will try to
- * find a set of contiguous Parameter RAMs in the remaining Parameter RAMs
+ * find a set of contiguous Parameter RAM slots in the remaining Parameter RAM
+ * slots
  */
 int edma_alloc_cont_slots(unsigned ctlr, unsigned int id, int slot, int count)
 {
@@ -771,12 +773,13 @@  int edma_alloc_cont_slots(unsigned ctlr, unsigned int id, int slot, int count)
 	 * the number of channels and lesser than the total number
 	 * of slots
 	 */
-	if (slot < edma_info[ctlr]->num_channels ||
-		slot >= edma_info[ctlr]->num_slots)
+	if ((id != EDMA_CONT_PARAMS_ANY) &&
+		(slot < edma_info[ctlr]->num_channels ||
+		slot >= edma_info[ctlr]->num_slots))
 		return -EINVAL;
 
 	/*
-	 * The number of parameter RAMs requested cannot be less than 1
+	 * The number of parameter RAM slots requested cannot be less than 1
 	 * and cannot be more than the number of slots minus the number of
 	 * channels
 	 */
@@ -786,11 +789,11 @@  int edma_alloc_cont_slots(unsigned ctlr, unsigned int id, int slot, int count)
 
 	switch (id) {
 	case EDMA_CONT_PARAMS_ANY:
-		return reserve_contiguous_params(ctlr, id, count,
+		return reserve_contiguous_slots(ctlr, id, count,
 						 edma_info[ctlr]->num_channels);
 	case EDMA_CONT_PARAMS_FIXED_EXACT:
 	case EDMA_CONT_PARAMS_FIXED_NOT_EXACT:
-		return reserve_contiguous_params(ctlr, id, count, slot);
+		return reserve_contiguous_slots(ctlr, id, count, slot);
 	default:
 		return -EINVAL;
 	}
@@ -813,7 +816,7 @@  EXPORT_SYMBOL(edma_alloc_cont_slots);
  */
 int edma_free_cont_slots(unsigned slot, int count)
 {
-	unsigned ctlr;
+	unsigned ctlr, slot_to_free;
 	int i;
 
 	ctlr = EDMA_CTLR(slot);
@@ -826,11 +829,11 @@  int edma_free_cont_slots(unsigned slot, int count)
 
 	for (i = slot; i < slot + count; ++i) {
 		ctlr = EDMA_CTLR(i);
-		slot = EDMA_CHAN_SLOT(i);
+		slot_to_free = EDMA_CHAN_SLOT(i);
 
-		memcpy_toio(edmacc_regs_base[ctlr] + PARM_OFFSET(slot),
+		memcpy_toio(edmacc_regs_base[ctlr] + PARM_OFFSET(slot_to_free),
 			&dummy_paramset, PARM_SIZE);
-		clear_bit(slot, edma_info[ctlr]->edma_inuse);
+		clear_bit(slot_to_free, edma_info[ctlr]->edma_inuse);
 	}
 
 	return 0;