Similar comment is present over routine also pl08x_choose_master_bus(). Keeping
one of them. Also rewrite that comment to convey message clearly.
Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
As mentioned in Documentation/CodingStyle,
The preferred form for passing a size of a struct is the following:
p = kmalloc(sizeof(*p), ...);
The alternative form where struct name is spelled out hurts readability and
introduces an opportunity for a bug when the pointer variable type is changed
but the corresponding sizeof that is passed to a memory allocator is not.
This patch replaces (struct xyz) with *ptr at several occurrences in driver.
Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Header files included in driver are not present in alphabetical order. Rearrange
them in alphabetical order.
Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
There were few formatting related issues in code. This patch fixes them.
Fixes include:
- Remove extra blank lines
- align code to 80 cols
- combine several lines to one line
Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
pl08x_width function does not handle rest of enums for DMA_SLAVE_BUSWIDTH_xxxx
which causes gcc to emit below warining
drivers/dma/amba-pl08x.c: In function 'pl08x_width':
drivers/dma/amba-pl08x.c:1119: warning: enumeration value
'DMA_SLAVE_BUSWIDTH_UNDEFINED' not handled in switch
drivers/dma/amba-pl08x.c:1119: warning: enumeration value
'DMA_SLAVE_BUSWIDTH_8_BYTES' not handled in switch
this patch adds a default case which returns error
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Now that we have separate cctl values for M>P and P>M transfers, we can
avoid calculating the cctl value each time we prepare a transaction.
Move the bus selection and increment setting to the slave configuration
and initialization functions.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Store the source/destination cctl values into the channel structure.
This moves us towards being able to avoid a configuration call each
time we use the channel.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Store the source/destination slave address separately into the channel
structure. This moves us towards being able to avoid a configuration
call each time we use the channel.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Clean up debugging when setting up the LLI list. This reduces the
amount of output while preserving the information, and makes it easier
to read.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Avoid re-selecting the LLI bus each time we create an LLI. Move it out
of the LLI setup loops.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
PL08X_WQ_PERIODMIN and PL08X_MAX_ALLOCS are not used, remove them.
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Make Primecell driver probe functions take a const pointer to their
ID tables. Drivers should never modify their ID tables in their
probe handler.
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
If a transfer is initiated from memory to a peripheral, then data is
fetched and the channel is marked busy. This busy status persists until
the HALT bit is set and the queued data has been transfered to the
peripheral. Waiting indefinitely after setting the HALT bit results in
system lockups. Timeout this operation, and print an error when this
happens.
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>
If we try to pause a channel when terminating a transfer, we could end
up spinning for it to become inactive indefinitely, and can result in
an uninterruptible wait requiring a reset to recover from.
Terminating a transfer is supposed to take effect immediately, but may
result in data loss.
To make this clear, rename the function to pl08x_terminate_phy_chan().
Also, make sure it is always consistently called - with the spinlock
held and IRQs disabled, and ensure that the TC and ERR interrupt status
is always cleared.
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>
Cleanup the formatting of comments, remove some which don't make sense
anymore.
Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
[fix conflict with 96a608a4]
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Prevent dma_set_runtime_config() being used to alter the configuration
supplied by the platform for memcpy channel configuration. No one
should be trying to change this configuration.
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>
There are cases in dma_set_runtime_config() where we fail to perform
the requested action - and we just issue a KERN_ERR message in that
case. We have the facility to return an error to the caller, so that
is what we should do.
When we encounter an error due to invalid parameters, we should not
modify driver state.
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>
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>
Introduce 'phychan_hold' to hold on to physical DMA channels while we're
preparing a new descriptor for it. This will be incremented when we
allocate a physical channel and set the MUX registers during the
preparation of the TXD, and will only be decremented when the TXD is
submitted.
This prevents the physical channel being given up before the new TXD
is placed on the queue.
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>
Don't place TXDs on the pending list when they're prepared - place
them on the list when they're ready to be submitted. Also, only
place memcpy requests in the wait state when they're submitted and
don't have a physical channel associated.
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>
This 'desc_list' is actually a list of pending descriptors, so name
it after its function (pending list) rather than what it contains
(descriptors).
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>
The DMA engine API requires DMA engine implementations to unmap buffers
passed into the non-slave DMA methods unless the relevant completion
flag is set. We aren't doing this, so implement this facility.
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>
Like other DMA engine drivers do, store the passed flags into the
async_tx structure, so they can be checked when the operation
completes.
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>
We only need to store the dma address.
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>
Don't alter any txd->srcbus or txd->dstbus values while building the
LLI list. This allows us to see the original dma_addr_t values passed
in via the prep_memcpy() method.
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>
The number of bytes we want to fill into any LLI is the minimum of:
- number of bytes remaining in the transfer
- number of bytes we can transfer in a single LLI
- number of bytes we can transfer without overflowing the source boundary
- number of bytes we can transfer without overflowing the destination boundary
The minimum of the first two is already calculated (target_len). We
limit the boundary calculations to this number of bytes, which will
then give us the number of bytes we can place into this LLI.
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>
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>
pl08x_pre_boundary() was unsafe with addresses towards the top of
memory space:
boundary = ((addr >> PL08X_BOUNDARY_SHIFT) + 1)
<< PL08X_BOUNDARY_SHIFT;
This can overflow a 32-bit number, producing zero. When it does:
if (boundary < addr + len)
return boundary - addr;
else
return len;
results in (boundary - addr) returning either a large positive value.
Also if addr + len overflows, this calculation also fails.
We can fix this trivially as the only thing we're actually interested
in is the value of the least significant PL08X_BOUNDARY_SHIFT bits:
boundary_len = PL08X_BOUNDARY_SIZE -
(addr & (PL08X_BOUNDARY_SIZE - 1));
gives us the number of bytes before 'addr' becomes a multiple of
PL08X_BOUNDARY_SIZE. We can then just take the min() of the two
calculated lengths.
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>
We don't need pl08x_fill_lli_for_desc() to return num_llis + 1 as
we know that's what it always does. We can just pass in num_llis
and use post-increment in the caller.
This makes the code slightly easier to read.
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>
Calling the callback handler with spinlocks in the tasklet held leads
to deadlock when dmaengine functions are called:
BUG: spinlock lockup on CPU#0, sh/417, c1870a08
Backtrace:
...
[<c017b408>] (do_raw_spin_lock+0x0/0x154) from [<c02c4b98>] (_raw_spin_lock_irqsave+0x54/0x60)
[<c02c4b44>] (_raw_spin_lock_irqsave+0x0/0x60) from [<c01f5828>] (pl08x_prep_channel_resources+0x718/0x8b4)
[<c01f5110>] (pl08x_prep_channel_resources+0x0/0x8b4) from [<c01f5bb4>] (pl08x_prep_slave_sg+0x120/0x19c)
[<c01f5a94>] (pl08x_prep_slave_sg+0x0/0x19c) from [<c01be7a0>] (pl011_dma_tx_refill+0x164/0x224)
[<c01be63c>] (pl011_dma_tx_refill+0x0/0x224) from [<c01bf1c8>] (pl011_dma_tx_callback+0x7c/0xc4)
[<c01bf14c>] (pl011_dma_tx_callback+0x0/0xc4) from [<c01f4d34>] (pl08x_tasklet+0x60/0x368)
[<c01f4cd4>] (pl08x_tasklet+0x0/0x368) from [<c004d978>] (tasklet_action+0xa0/0x100)
Dan quoted the documentation:
> 2/ Completion callback routines cannot submit new operations. This
> results in recursion in the synchronous case and spin_locks being
> acquired twice in the asynchronous case.
but then followed up to say:
> I should clarify, this is the async_memcpy() api requirement which is
> not used outside of md/raid5. DMA drivers can and do allow new
> submissions from callbacks, and the ones that do so properly move the
> callback outside of the driver lock.
So let's fix it by moving the callback out of the spinlocked region.
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>
Platforms need to be able to control which AHB master interface is used,
as each AHB master interface may be asymetric. Allow the interfaces
used for fetching LLIs, memory, and each peripheral to be configured
individually.
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>
As we initialize the default cctl value in the prep_* functions along
with the increment settings, we don't need to repeat the selection of
the AHB ports each time we create a LLI entry. Do this in the prep_*
functions once per transfer.
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>
We don't need to initialize the cctl increment and protection values
in the runtime_config method - we have all the inforamtion to setup
these values in prep_slave_sg(). Move their initialization there.
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>
Rather than modifying platform data while preparing a transfer, copy
the cctl value into the txd structure and modify the value there.
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>
There is no need to wait until we start processing a tx descriptor
before setting up the DMA request selection in the ccfg register.
We know which channel and request will be used in prep_phy_channel(),
so setup the ccfg request selection at txd creation time in
prep_phy_channel().
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>
The ccfg register is used to configure the channel parameters - the type
and direction of transfer, the flow control signal and IRQ mask enables.
The type and direction of transfer is known in the relevent prep_*
function where a txd is created. The IRQ mask enables are always set,
and the flow control signals are always set when we start processing a
txd according to phychan->signal.
If we store the ccfg value in the txd structure, we can avoid modifying
platform data - and even having it in platform data at all.
So, remove it from platform data too.
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>
As we now have all the code accessing the phychan {csrc,cdst,clli,cctl,
ccfg} members in one function, there's no point storing the data into
the struct. Get rid of the struct members. Re-order the register dump
in the dev_dbg() to reflect the order we write the registers to the DMA
device.
The txd {csrc,cdst,clli,cctl} values are duplicates of the lli[0]
values, so there's no point duplicating these either. Program the DMAC
registers directly from the lli[0] values.
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>
There is no need for pl08x_config_phychan_for_txd(), pl08x_set_cregs()
and pl08x_enable_phy_chan() to be separate - they are always called in
sequence. Combine them into one 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>
As the LLI list is an array, we can use maths to locate which LLI
index we're currently at, and then sum up the remaining LLI entries
until we reach the end of the list.
This makes the code much easier to read, and much less susceptible
to falling off the end of the array.
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>
The LLI pointer in the documentation is placed into the LLI register,
so name it LLI rather than 'next'.
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>
Use 'u32' for the LLI structure members, which are defined by hardware
to be 32-bit. dma_addr_t is much more vague about its actual size.
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>
Use size_t for variables denoting lengths throughout, and use the 'z'
qualifier for printing the value. For safety, add a BUG_ON() in
pl08x_fill_lli_for_desc() to catch the remainder potentially becoming
negative.
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>
llis_bus is the DMA address of the LLI array. Casting it to be a
pointer just to be able to use pointer arithmetic on it is not nice.
We can trivially deal with the places where we do arithmetic on it,
and it's actually cleaner this way.
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>
We only want use the address of the LLI pointer when locating the
corresponding structure in memory, so clear the master bus selection
bit.
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>
Tight loops should use cpu_relax() to allow CPUs to reduce power
consumption while waiting for events.
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>
Consolidate duplicated channel release code into release_phy_channel()
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>
Consolidate code which allocates and initializes txds.
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>
Avoid using 'void *' struct fields when the structs are not defined
in linux/amba/pl08x.h - instead, forward declare the struct names, and
use these instead. This ensures we have proper typechecking.
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>
