Skip to content

Commit

Permalink
ARM: PL08x: fix locking between prepare function and submit function
Browse files Browse the repository at this point in the history
The PL08x driver holds on to the channel lock with interrupts disabled
between the prepare and the subsequent submit API functions.  This
means that the locking state when the prepare function returns is
dependent on whether it suceeeds or not.

It did this to ensure that the physical channel wasn't released, and
as it used to add the descriptor onto the pending list at prepare time
rather than submit time.

Now that we have reorganized the code to remove those reasons, we can
now safely release the spinlock at the end of preparation and reacquire
it in our submit function.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
  • Loading branch information
Russell King - ARM Linux authored and Dan Williams committed Jan 5, 2011
1 parent 8087aac commit c370e59
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 23 deletions.
28 changes: 9 additions & 19 deletions drivers/dma/amba-pl08x.c
Original file line number Diff line number Diff line change
Expand Up @@ -981,6 +981,9 @@ static dma_cookie_t pl08x_tx_submit(struct dma_async_tx_descriptor *tx)
{
struct pl08x_dma_chan *plchan = to_pl08x_chan(tx->chan);
struct pl08x_txd *txd = to_pl08x_txd(tx);
unsigned long flags;

spin_lock_irqsave(&plchan->lock, flags);

plchan->chan.cookie += 1;
if (plchan->chan.cookie < 0)
Expand All @@ -1003,8 +1006,7 @@ static dma_cookie_t pl08x_tx_submit(struct dma_async_tx_descriptor *tx)
plchan->phychan_hold--;
}

/* This unlock follows the lock in the prep() function */
spin_unlock_irqrestore(&plchan->lock, plchan->lockflags);
spin_unlock_irqrestore(&plchan->lock, flags);

return tx->cookie;
}
Expand Down Expand Up @@ -1225,17 +1227,17 @@ static void pl08x_issue_pending(struct dma_chan *chan)
static int pl08x_prep_channel_resources(struct pl08x_dma_chan *plchan,
struct pl08x_txd *txd)
{
int num_llis;
struct pl08x_driver_data *pl08x = plchan->host;
int ret;
unsigned long flags;
int num_llis, ret;

num_llis = pl08x_fill_llis_for_desc(pl08x, txd);
if (!num_llis) {
kfree(txd);
return -EINVAL;
}

spin_lock_irqsave(&plchan->lock, plchan->lockflags);
spin_lock_irqsave(&plchan->lock, flags);

/*
* See if we already have a physical channel allocated,
Expand All @@ -1258,7 +1260,7 @@ static int pl08x_prep_channel_resources(struct pl08x_dma_chan *plchan,
if (plchan->slave) {
pl08x_free_txd_list(pl08x, plchan);
pl08x_free_txd(pl08x, txd);
spin_unlock_irqrestore(&plchan->lock, plchan->lockflags);
spin_unlock_irqrestore(&plchan->lock, flags);
return -EBUSY;
}
} else
Expand All @@ -1272,11 +1274,7 @@ static int pl08x_prep_channel_resources(struct pl08x_dma_chan *plchan,
if (plchan->state == PL08X_CHAN_IDLE)
plchan->state = PL08X_CHAN_PAUSED;

/*
* Notice that we leave plchan->lock locked on purpose:
* it will be unlocked in the subsequent tx_submit()
* call. This is a consequence of the current API.
*/
spin_unlock_irqrestore(&plchan->lock, flags);

return 0;
}
Expand Down Expand Up @@ -1355,10 +1353,6 @@ static struct dma_async_tx_descriptor *pl08x_prep_dma_memcpy(
ret = pl08x_prep_channel_resources(plchan, txd);
if (ret)
return NULL;
/*
* NB: the channel lock is held at this point so tx_submit()
* must be called in direct succession.
*/

return &txd->tx;
}
Expand Down Expand Up @@ -1444,10 +1438,6 @@ static struct dma_async_tx_descriptor *pl08x_prep_slave_sg(
ret = pl08x_prep_channel_resources(plchan, txd);
if (ret)
return NULL;
/*
* NB: the channel lock is held at this point so tx_submit()
* must be called in direct succession.
*/

return &txd->tx;
}
Expand Down
4 changes: 0 additions & 4 deletions include/linux/amba/pl08x.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,6 @@ enum pl08x_dma_chan_state {
* @lc: last completed transaction on this channel
* @pend_list: queued transactions pending on this channel
* @at: active transaction on this channel
* @lockflags: sometimes we let a lock last between two function calls,
* especially prep/submit, and then we need to store the IRQ flags
* in the channel state, here
* @lock: a lock for this channel data
* @host: a pointer to the host (internal use)
* @state: whether the channel is idle, paused, running etc
Expand All @@ -184,7 +181,6 @@ struct pl08x_dma_chan {
dma_cookie_t lc;
struct list_head pend_list;
struct pl08x_txd *at;
unsigned long lockflags;
spinlock_t lock;
struct pl08x_driver_data *host;
enum pl08x_dma_chan_state state;
Expand Down

0 comments on commit c370e59

Please sign in to comment.