* Documentation: driver-api: fix dmatest.rst warning
From: Randy Dunlap @ 2019-02-11 6:26 UTC (permalink / raw)
To: LKML
Cc: Andy Shevchenko, Vinod Koul, dmaengine, linux-doc@vger.kernel.org,
Jonathan Corbet
From: Randy Dunlap <rdunlap@infradead.org>
Fix markup warning: insert a blank line before the hint.
Documentation/driver-api/dmaengine/dmatest.rst:63: WARNING: Unexpected indentation.
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: dmaengine@vger.kernel.org
---
Documentation/driver-api/dmaengine/dmatest.rst | 1 +
1 file changed, 1 insertion(+)
--- lnx-50-rc6.orig/Documentation/driver-api/dmaengine/dmatest.rst
+++ lnx-50-rc6/Documentation/driver-api/dmaengine/dmatest.rst
@@ -59,6 +59,7 @@ parameter, that specific channel is requ
is created with the existing parameters. This thread is set as pending
and will be executed once run is set to 1. Any parameters set after the thread
is created are not applied.
+
.. hint::
available channel list could be extracted by running the following command::
^ permalink raw reply
* dmaengine: fsldma: Add 64-bit I/O accessors for powerpc64
From: Peng Ma @ 2019-02-11 2:33 UTC (permalink / raw)
To: Vinod Koul
Cc: Scott Wood, Leo Li, Zhang Wei, linuxppc-dev@lists.ozlabs.org,
dmaengine@vger.kernel.org, Wen He
Hi Vinod,
Got it.
Best Regards
Peng
>-----Original Message-----
>From: Vinod Koul <vkoul@kernel.org>
>Sent: 2019年2月4日 15:28
>To: Peng Ma <peng.ma@nxp.com>
>Cc: Scott Wood <oss@buserror.net>; Leo Li <leoyang.li@nxp.com>; Zhang Wei
><zw@zh-kernel.org>; linuxppc-dev@lists.ozlabs.org;
>dmaengine@vger.kernel.org; Wen He <wen.he_1@nxp.com>
>Subject: Re: [PATCH] dmaengine: fsldma: Add 64-bit I/O accessors for
>powerpc64
>
>On 25-01-19, 05:54, Peng Ma wrote:
>> Hi Vinod,
>>
>> Sorry to replay late.
>> 1:This patch has already send to the patchwork.
>> Please see the patch link:
>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpat
>>
>chwork.kernel.org%2Fpatch%2F10741521%2F&data=02%7C01%7Cpeng.
>ma%40n
>>
>xp.com%7C5d169e9bc5684a9d14c008d68a727589%7C686ea1d3bc2b4c6fa92
>cd99c5c
>>
>301635%7C0%7C0%7C636848621546833095&sdata=q1sTaC%2F4SqEtM
>yFymzyvXe
>> qBOhCnBjyMcjPz8A9QRI4%3D&reserved=0
>> 2:I have already compile the fsl patches on arm and powerpc after patched
>https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
>work.kernel.org%2Fpatch%2F10741521%2F&data=02%7C01%7Cpeng.ma
>%40nxp.com%7C5d169e9bc5684a9d14c008d68a727589%7C686ea1d3bc2b4c
>6fa92cd99c5c301635%7C0%7C0%7C636848621546833095&sdata=q1sTa
>C%2F4SqEtMyFymzyvXeqBOhCnBjyMcjPz8A9QRI4%3D&reserved=0
>> The compile will successful, please let me know the reported regression
>results, thanks very much.
>
>And I thought there were further comments on this patch. I have applied this,
>please watch for failures reported if any..
>--
>~Vinod
^ permalink raw reply
* doc:dmaengine: clarify DMA desc. pointer after submission
From: Federico Vaga @ 2019-02-08 15:30 UTC (permalink / raw)
To: Vinod Koul, Jonathan Corbet
Cc: dmaengine, linux-doc, linux-kernel, Federico Vaga
It clarifies that the DMA description pointer returned by
`dmaengine_prep_*` function should not be used after submission.
Signed-off-by: Federico Vaga <federico.vaga@cern.ch>
---
Documentation/driver-api/dmaengine/client.rst | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/driver-api/dmaengine/client.rst b/Documentation/driver-api/dmaengine/client.rst
index fbbb2831f29f..d728e50105eb 100644
--- a/Documentation/driver-api/dmaengine/client.rst
+++ b/Documentation/driver-api/dmaengine/client.rst
@@ -168,6 +168,13 @@ The details of these operations are:
dmaengine_submit() will not start the DMA operation, it merely adds
it to the pending queue. For this, see step 5, dma_async_issue_pending.
+ .. note::
+
+ After calling ``dmaengine_submit()`` the submitted transfer descriptor
+ (``struct dma_async_tx_descriptor``) belongs to the DMA engine.
+ Consequentially, the client must consider invalid the pointer to that
+ descriptor.
+
5. Issue pending DMA requests and wait for callback notification
The transactions in the pending queue can be activated by calling the
^ permalink raw reply related
* [01/18] MIPS: lantiq: pass struct device to DMA API functions
From: Paul Burton @ 2019-02-07 23:29 UTC (permalink / raw)
To: Christoph Hellwig
Cc: John Crispin, Vinod Koul, Dmitry Tarnyagin, Nicolas Ferre,
Sudip Mukherjee, Felipe Balbi, linux-mips@vger.kernel.org,
linux-kernel@vger.kernel.org, dmaengine@vger.kernel.org,
netdev@vger.kernel.org, linux-usb@vger.kernel.org,
linux-fbdev@vger.kernel.org, alsa-devel@alsa-project.org,
iommu@lists.linux-foundation.org
Hi Christoph,
On Fri, Feb 01, 2019 at 09:47:44AM +0100, Christoph Hellwig wrote:
> The DMA API generally relies on a struct device to work properly, and
> only barely works without one for legacy reasons. Pass the easily
> available struct device from the platform_device to remedy this.
>
> Also use GFP_KERNEL instead of GFP_ATOMIC as the gfp_t for the memory
> allocation, as we aren't in interrupt context or under a lock.
>
> Note that this whole function looks somewhat bogus given that we never
> even look at the returned dma address, and the CPHYSADDR magic on
> a returned noncached mapping looks "interesting". But I'll leave
> that to people more familiar with the code to sort out.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> arch/mips/lantiq/xway/vmmc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Would you like this to go through the MIPS tree or elsewhere? If the
latter:
Acked-by: Paul Burton <paul.burton@mips.com>
Thanks,
Paul
^ permalink raw reply
* [RFC,v3,1/7] dmaengine: Add Synopsys eDMA IP core driver
From: Gustavo Pimentel @ 2019-02-06 18:06 UTC (permalink / raw)
To: Vinod Koul, Gustavo Pimentel
Cc: linux-pci@vger.kernel.org, dmaengine@vger.kernel.org,
Dan Williams, Eugeniy Paltsev, Andy Shevchenko, Russell King,
Niklas Cassel, Joao Pinto, Jose Abreu, Luis Oliveira,
Vitor Soares, Nelson Costa, Pedro Sousa
On 02/02/2019 10:07, Vinod Koul wrote:
> On 01-02-19, 11:23, Gustavo Pimentel wrote:
>> On 01/02/2019 04:14, Vinod Koul wrote:
>>> On 31-01-19, 11:33, Gustavo Pimentel wrote:
>>>> On 23/01/2019 13:08, Vinod Koul wrote:
>>>>> On 21-01-19, 15:48, Gustavo Pimentel wrote:
>>>>>> On 20/01/2019 11:44, Vinod Koul wrote:
>>>>>>> On 11-01-19, 19:33, Gustavo Pimentel wrote:
>>>
>>>>>>>> @@ -0,0 +1,1059 @@
>>>>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>>>>>> +/*
>>>>>>>> + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
>>>>>>>
>>>>>>> 2019 now
>>>>>>
>>>>>> I've changed to "Copyright (c) 2018-present Synopsys, Inc. and/or its
>>>>>> affiliates." this way it's always up to date and I also kept 2018, because it
>>>>>> was the date that I started to develop this driver, if you don't mind.
>>>>>
>>>>> yeah 18 is fine :) it need to end with current year always
>>>>
>>>> Just to be sure, are you saying that must be: "Copyright (c) 2018-2019 Synopsys,
>>>> Inc. and/or its affiliates."?
>>>
>>> Yup :)
>>>
>>>>>>>> +static struct dw_edma_chunk *dw_edma_alloc_chunk(struct dw_edma_desc *desc)
>>>>>>>> +{
>>>>>>>> + struct dw_edma_chan *chan = desc->chan;
>>>>>>>> + struct dw_edma *dw = chan->chip->dw;
>>>>>>>> + struct dw_edma_chunk *chunk;
>>>>>>>> +
>>>>>>>> + chunk = kvzalloc(sizeof(*chunk), GFP_NOWAIT);
>>>>>>>> + if (unlikely(!chunk))
>>>>>>>> + return NULL;
>>>>>>>> +
>>>>>>>> + INIT_LIST_HEAD(&chunk->list);
>>>>>>>> + chunk->chan = chan;
>>>>>>>> + chunk->cb = !(desc->chunks_alloc % 2);
>>>>>>>
>>>>>>> cb ..?
>>>>>>
>>>>>> CB = change bit, is a property of this eDMA IP. Basically it is a kind of
>>>>>> handshake which serves to validate whether the linked list has been updated or
>>>>>> not, especially useful in cases of recycled linked list elements (every linked
>>>>>> list recycle is a new chunk, this will allow to differentiate each chunk).
>>>>>
>>>>> okay please add that somewhere. Also it would help me if you explain
>>>>> what is chunk and other terminologies used in this driver
>>>>
>>>> I'm thinking to put the below description on the patch, please check if this is
>>>> sufficient explicit and clear to understand what this IP needs and does.
>>>>
>>>> In order to transfer data from point A to B as fast as possible this IP requires
>>>> a dedicated memory space where will reside a linked list of elements.
>>>
>>> rephrasing: a dedicated memory space containing linked list of elements
>>>
>>>> All elements of this linked list are continuous and each one describes a data
>>>> transfer (source and destination addresses, length and a control variable).
>>>>
>>>> For the sake of simplicity, lets assume a memory space for channel write 0 which
>>>> allows about 42 elements.
>>>>
>>>> +---------+
>>>> | Desc #0 |--+
>>>> +---------+ |
>>>> V
>>>> +----------+
>>>> | Chunk #0 |---+
>>>> | CB = 1 | | +----------+ +-----+ +-----------+ +-----+
>>>> +----------+ +-->| Burst #0 |-->| ... |-->| Burst #41 |-->| llp |
>>>> | +----------+ +-----+ +-----------+ +-----+
>>>> V
>>>> +----------+
>>>> | Chunk #1 |---+
>>>> | CB = 0 | | +-----------+ +-----+ +-----------+ +-----+
>>>> +----------+ +-->| Burst #42 |-->| ... |-->| Burst #83 |-->| llp |
>>>> | +-----------+ +-----+ +-----------+ +-----+
>>>> V
>>>> +----------+
>>>> | Chunk #2 |---+
>>>> | CB = 1 | | +-----------+ +-----+ +------------+ +-----+
>>>> +----------+ +-->| Burst #84 |-->| ... |-->| Burst #125 |-->| llp |
>>>> | +-----------+ +-----+ +------------+ +-----+
>>>> V
>>>> +----------+
>>>> | Chunk #3 |---+
>>>> | CB = 0 | | +------------+ +-----+ +------------+ +-----+
>>>> +----------+ +-->| Burst #126 |-->| ... |-->| Burst #129 |-->| llp |
>>>> +------------+ +-----+ +------------+ +-----+
>>>
>>> This is great and required to understand the driver.
>>>
>>> How does controller move from Burnst #41 of Chunk 0 to Chunk 1 ?
>>
>> I forgot to explain that...
>>
>> On every last Burst of the Chunk (Burst #41, Burst #83, Burst #125 or even Burst
>> #129) is set some flags on their control variable (RIE and LIE bits) that will
>> trigger the send of "done" interruption.
>>
>> On the interruptions callback, is decided whether to recycle the linked list
>> memory space by writing a new set of Bursts elements (if still exists Chunks to
>> transfer) or is considered completed (if there is no Chunks available to transfer)
>>
>>> Is Burst 0 to 129 a link list with Burst 0, 42, 84 and 126 having a
>>> callback (done bit set)..
>>
>> I didn't quite understand it your question.
>>
>> It comes from the prep_slave_sg n elements (130 applying the example), where
>> will be divide in several Chunks (#0, #1, #2, #3 and #4 applying the example) (a
>> linked list that will contain another linked list for the Bursts, CB, Chunk
>> size). The linked list inside of each Chunk will contain a number of Bursts
>> (limited to the memory space size), each one will possess Source Address,
>> Destination Address and size of that sub-transfer.
>
> Thanks this helps :)
>
> so in this case what does prep_slave_sg n elements corrosponds to (130
> bursts) ..? If so how do you split a transaction to chunks and bursts?
In the example case, prep_slave_sg API receives a total of 130 elements and the
dw-edma-core implementation will slice in 4 chunks (the first 3 chunks will
containing 42 bursts each and the last chunk only containing 4 bursts).
The burst maximum capacity of each chunk depends of the linked list memory space
size available or destined for, in the example case I didn't specify the
actually space amount, but the maximum number of bursts is calculated by:
maximum number of bursts = (linked list memory size / 24) - 1
the size is in bytes and 24 is the size of each burst information element.
It's also subtracted 1 element because on the end of each burst stream is needed
the llp element and for simplicity it's size is considered to be equal to the
burst element.
>
>
>>
>>>
>>> This sound **very** similar to dw dma concepts!
>>
>> I believe some parts of dw dma and dw edma behavior are similar and that makes
>> perfectly sense since both dma are done by Synopsys and may be exist a shared
>> knowledge, however they are different IPs applied to different products.
>>
>>>
>>>>
>>>> Legend:
>>>>
>>>> *Linked list*, also know as Chunk
>>>> *Linked list element*, also know as Burst
>>>> *CB*, also know as Change Bit, it's a control bit (and typically is toggled)
>>>> that allows to easily identify and differentiate between the current linked list
>>>> and the previous or the next one.
>>>> *LLP*, is a special element that indicates the end of the linked list element
>>>> stream also informs that the next CB should be toggle.
>>>>
>>>> On scatter-gather transfer mode, the client will submit a scatter-gather list of
>>>> n (on this case 130) elements, that will be divide in multiple Chunks, each
>>>> Chunk will have (on this case 42) a limited number of Bursts and after
>>>> transferring all Bursts, an interrupt will be triggered, which will allow to
>>>> recycle the all linked list dedicated memory again with the new information
>>>> relative to the next Chunk and respective Burst associated and repeat the whole
>>>> cycle again.
>>>>
>>>> On cyclic transfer mode, the client will submit a buffer pointer, length of it
>>>> and number of repetitions, in this case each burst will correspond directly to
>>>> each repetition.
>>>>
>>>> Each Burst can describes a data transfer from point A(source) to point
>>>> B(destination) with a length that can be from 1 byte up to 4 GB. Since dedicated
>>>> the memory space where the linked list will reside is limited, the whole n burst
>>>> elements will be organized in several Chunks, that will be used later to recycle
>>>> the dedicated memory space to initiate a new sequence of data transfers.
>>>>
>>>> The whole transfer is considered has completed when it was transferred all bursts.
>>>>
>>>> Currently this IP has a set well-known register map, which includes support for
>>>> legacy and unroll modes. Legacy mode is version of this register map that has
>>>
>>> whats unroll..
>>>
>>>> multiplexer register that allows to switch registers between all write and read
>>
>> The unroll is explained here, see below
>>
>>>> channels and the unroll modes repeats all write and read channels registers with
>>>> an offset between them. This register map is called v0.
>>>>
>>>> The IP team is creating a new register map more suitable to the latest PCIe
>>>> features, that very likely will change the map register, which this version will
>>>> be called v1. As soon as this new version is released by the IP team the support
>>>> for this version in be included on this driver.
>>>>
>>>> What do you think? There is any gray area that I could clarify?
>>>
>>> This sounds good. But we are also catering to a WIP IP which can change
>>> right. Doesnt sound very good idea to me :)
>>
>> This IP exists for several years like this and it works quite fine, however
>> because of new features and requests (SR-IOV, more dma channels, function
>> segregation and isolation, performance improvement) that are coming it's natural
>> to have exist improvements. The drivers should follow the evolution and be
>> sufficient robust enough to adapt to this new circumstance.
>
> Kernel driver should be modular be design, if we keep things simple then
> adding/splitting to support future revisions should be easy!
>
>>>>>>>> + dev_dbg(chan2dev(chan), "addr(physical) src=%pa, dst=%pa\n",
>>>>>>>> + &config->src_addr, &config->dst_addr);
>>>>>>>> +
>>>>>>>> + chan->src_addr = config->src_addr;
>>>>>>>> + chan->dst_addr = config->dst_addr;
>>>>>>>> +
>>>>>>>> + err = ops->device_config(dchan);
>>>>>>>
>>>>>>> what does this do?
>>>>>>
>>>>>> This is an initialization procedure to setup interrupts (data and addresses) to
>>>>>> each channel on the eDMA IP, in order to be triggered after transfer being
>>>>>> completed or aborted. Due the fact the config() can be called at anytime,
>>>>>> doesn't make sense to have this procedure here, I'll moved it to probe().
>>>>>
>>>>> Yeah I am not still convinced about having another layer! Have you
>>>>> though about using common lib for common parts ..?
>>>>
>>>> Maybe I'm explaining myself wrongly. I don't have any clue about the new
>>>> register map for the future versions. I honestly tried to implement the common
>>>> lib for the whole process that interact with dma engine controller to ease in
>>>> the future any new addition of register mapping, by having this common callbacks
>>>> that will only be responsible for interfacing the HW accordingly to register map
>>>> version. Maybe I can simplify something in the future, but I only be able to
>>>> conclude that after having some idea about the new register map.
>>>>
>>>> IMHO I think this is the easier and clean way to do it, in terms of code
>>>> maintenance and architecture, but if you have another idea that you can show me
>>>> or pointing out for a driver that implements something similar, I'm no problem
>>>> to check it out.
>>>
>>> That is what my fear was :)
>>>
>>> Lets step back and solve one problem at a time. Right now that is v0 of
>>> IP. Please write a simple driver which solve v0 without any layers
>>> involved.
>>>
>>> Once v1 is in good shape, you would know what it required and then we
>>> can split v0 driver into common lib and v0 driver and then add v1
>>> driver.
>>
>> Can I keep the code segregation as it is now? With the dw-edma-v0-core.c/h,
>> dw-edma-v0-debugfs.c/h and dw-edma-v0-regs.h
>>
>> That way I would only replace the callbacks calls to direct function calls and
>> remove the switch case callback selection base on the version.
>
> That sound better, please keep patches smallish (anything going more
> than 600 lines becomes harder to review) and logically split.
>
>>>>>>> what is the second part of the check, can you explain that, who sets
>>>>>>> chan->dir?
>>>>>>
>>>>>> The chan->dir is set on probe() during the process of configuring each channel.
>>>>>
>>>>> So you have channels that are unidirectional?
>>>>
>>>> Yes. That's one another reason IMHO to keep the dw-edma-test separate from
>>>> dma-test, since this IP is more picky and have this particularities.
>>>
>>> That is okay, that should be handled by prep calls, if you get a prep
>>> call for direction you dont support return error and move to next
>>> available one.
>>>
>>> That can be done generically in dmatest driver and to answer your
>>> question, yes I would test that :)
>>
>> Like you said, let do this in small steps. For now I would like to suggest to
>> leave out the dw-dma-test driver and just focus on the current driver, if you
>> don't mind. I never thought that his test driver would raise this kind of discuss.
>
> Well dmatest is just a testing aid and doesnt care about internal
> designs of your driver. I would say keep it as it is useful aid and will
> help other folks as well :)
>
Let's see, I honestly hope so...
Gustavo
^ permalink raw reply
* [3/3] dmaengine: at_xdmac: only monitor overflow errors for peripheral xfer
From: Ludovic Desroches @ 2019-02-05 12:35 UTC (permalink / raw)
To: Nicolas Ferre
Cc: linux-arm-kernel, dmaengine, Vinod Koul, Alexandre Belloni,
linux-kernel
On Tue, Feb 05, 2019 at 12:03:43PM +0100, Nicolas Ferre wrote:
> The overflow error flag (ROI: Request Overflow Error) is only relevant
> for the case when the channel handles a peripheral synchronized transfer.
> Not in the case of memory to memory transfer where there is no hardware
> request signal.
>
> Remove the use of this interrupt source in such a case. It's based on
> the first descriptor which holds the configuration for the whole
> linked list transfer.
>
> Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> ---
> drivers/dma/at_xdmac.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
> index ec7a29d8e448..b558a23ffbc2 100644
> --- a/drivers/dma/at_xdmac.c
> +++ b/drivers/dma/at_xdmac.c
> @@ -308,6 +308,11 @@ static inline int at_xdmac_csize(u32 maxburst)
> return csize;
> };
>
> +static inline bool at_xdmac_chan_is_peripheral_xfer(u32 cfg)
> +{
> + return cfg & AT_XDMAC_CC_TYPE_PER_TRAN;
> +}
> +
> static inline u8 at_xdmac_get_dwidth(u32 cfg)
> {
> return (cfg & AT_XDMAC_CC_DWIDTH_MASK) >> AT_XDMAC_CC_DWIDTH_OFFSET;
> @@ -389,7 +394,13 @@ static void at_xdmac_start_xfer(struct at_xdmac_chan *atchan,
> at_xdmac_chan_read(atchan, AT_XDMAC_CUBC));
>
> at_xdmac_chan_write(atchan, AT_XDMAC_CID, 0xffffffff);
> - reg = AT_XDMAC_CIE_RBEIE | AT_XDMAC_CIE_WBEIE | AT_XDMAC_CIE_ROIE;
> + reg = AT_XDMAC_CIE_RBEIE | AT_XDMAC_CIE_WBEIE;
> + /*
> + * Request Overflow Error is only for peripheral synchronized transfers
> + */
> + if (at_xdmac_chan_is_peripheral_xfer(first->lld.mbr_cfg))
> + reg |= AT_XDMAC_CIE_ROIE;
> +
> /*
> * There is no end of list when doing cyclic dma, we need to get
> * an interrupt after each periods.
> --
> 2.17.1
>
^ permalink raw reply
* [2/3] dmaengine: at_xdmac: enhance channel errors handling in tasklet
From: Ludovic Desroches @ 2019-02-05 12:33 UTC (permalink / raw)
To: Nicolas Ferre
Cc: linux-arm-kernel, dmaengine, Vinod Koul, Alexandre Belloni,
linux-kernel
On Tue, Feb 05, 2019 at 12:03:42PM +0100, Nicolas Ferre wrote:
> Complement the identification of errors with stoping the channel and
> dumping the descriptor that led to the error case.
>
> Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
> ---
> drivers/dma/at_xdmac.c | 43 ++++++++++++++++++++++++++++++++++++------
> 1 file changed, 37 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
> index 37a269420435..ec7a29d8e448 100644
> --- a/drivers/dma/at_xdmac.c
> +++ b/drivers/dma/at_xdmac.c
> @@ -1575,6 +1575,41 @@ static void at_xdmac_handle_cyclic(struct at_xdmac_chan *atchan)
> dmaengine_desc_get_callback_invoke(txd, NULL);
> }
>
> +static void at_xdmac_handle_error(struct at_xdmac_chan *atchan)
> +{
> + struct at_xdmac *atxdmac = to_at_xdmac(atchan->chan.device);
> + struct at_xdmac_desc *bad_desc;
> +
> + /*
> + * The descriptor currently at the head of the active list is
> + * broked. Since we don't have any way to report errors, we'll
> + * just have to scream loudly and try to carry on.
> + */
> + if (atchan->irq_status & AT_XDMAC_CIS_RBEIS)
> + dev_err(chan2dev(&atchan->chan), "read bus error!!!");
> + if (atchan->irq_status & AT_XDMAC_CIS_WBEIS)
> + dev_err(chan2dev(&atchan->chan), "write bus error!!!");
> + if (atchan->irq_status & AT_XDMAC_CIS_ROIS)
> + dev_err(chan2dev(&atchan->chan), "request overflow error!!!");
> +
> + spin_lock_bh(&atchan->lock);
> + /* Channel must be disabled first as it's not done automatically */
> + at_xdmac_write(atxdmac, AT_XDMAC_GD, atchan->mask);
> + while (at_xdmac_read(atxdmac, AT_XDMAC_GS) & atchan->mask)
> + cpu_relax();
> + bad_desc = list_first_entry(&atchan->xfers_list,
> + struct at_xdmac_desc,
> + xfer_node);
> + spin_unlock_bh(&atchan->lock);
> + /* Print bad descriptor's details if needed */
> + dev_dbg(chan2dev(&atchan->chan),
> + "%s: lld: mbr_sa=%pad, mbr_da=%pad, mbr_ubc=0x%08x\n",
> + __func__, &bad_desc->lld.mbr_sa, &bad_desc->lld.mbr_da,
> + bad_desc->lld.mbr_ubc);
> +
> + /* Then continue with usual descriptor management */
> +}
> +
> static void at_xdmac_tasklet(unsigned long data)
> {
> struct at_xdmac_chan *atchan = (struct at_xdmac_chan *)data;
> @@ -1594,12 +1629,8 @@ static void at_xdmac_tasklet(unsigned long data)
> || (atchan->irq_status & error_mask)) {
> struct dma_async_tx_descriptor *txd;
>
> - if (atchan->irq_status & AT_XDMAC_CIS_RBEIS)
> - dev_err(chan2dev(&atchan->chan), "read bus error!!!");
> - if (atchan->irq_status & AT_XDMAC_CIS_WBEIS)
> - dev_err(chan2dev(&atchan->chan), "write bus error!!!");
> - if (atchan->irq_status & AT_XDMAC_CIS_ROIS)
> - dev_err(chan2dev(&atchan->chan), "request overflow error!!!");
> + if (atchan->irq_status & error_mask)
> + at_xdmac_handle_error(atchan);
>
> spin_lock(&atchan->lock);
> desc = list_first_entry(&atchan->xfers_list,
> --
> 2.17.1
>
^ permalink raw reply
* [1/3] dmaengine: at_xdmac: remove BUG_ON macro in tasklet
From: Ludovic Desroches @ 2019-02-05 12:30 UTC (permalink / raw)
To: Nicolas Ferre
Cc: linux-arm-kernel, dmaengine, Vinod Koul, Alexandre Belloni,
linux-kernel
On Tue, Feb 05, 2019 at 12:03:41PM +0100, Nicolas Ferre wrote:
> Even if this case shouldn't happen when controller is properly programmed,
> it's still better to avoid dumping a kernel Oops for this.
> As the sequence may happen only for debugging purposes, log the error and
> just finish the tasklet call.
>
> Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
Thanks
> ---
> drivers/dma/at_xdmac.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
> index fe69dccfa0c0..37a269420435 100644
> --- a/drivers/dma/at_xdmac.c
> +++ b/drivers/dma/at_xdmac.c
> @@ -1606,7 +1606,11 @@ static void at_xdmac_tasklet(unsigned long data)
> struct at_xdmac_desc,
> xfer_node);
> dev_vdbg(chan2dev(&atchan->chan), "%s: desc 0x%p\n", __func__, desc);
> - BUG_ON(!desc->active_xfer);
> + if (!desc->active_xfer) {
> + dev_err(chan2dev(&atchan->chan), "Xfer not active: exiting");
> + spin_unlock_bh(&atchan->lock);
> + return;
> + }
>
> txd = &desc->tx_dma_desc;
>
> --
> 2.17.1
>
^ permalink raw reply
* [3/3] dmaengine: at_xdmac: only monitor overflow errors for peripheral xfer
From: Nicolas Ferre @ 2019-02-05 11:03 UTC (permalink / raw)
To: Ludovic Desroches, linux-arm-kernel, dmaengine, Vinod Koul
Cc: Alexandre Belloni, linux-kernel, Nicolas Ferre
The overflow error flag (ROI: Request Overflow Error) is only relevant
for the case when the channel handles a peripheral synchronized transfer.
Not in the case of memory to memory transfer where there is no hardware
request signal.
Remove the use of this interrupt source in such a case. It's based on
the first descriptor which holds the configuration for the whole
linked list transfer.
Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
drivers/dma/at_xdmac.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
index ec7a29d8e448..b558a23ffbc2 100644
--- a/drivers/dma/at_xdmac.c
+++ b/drivers/dma/at_xdmac.c
@@ -308,6 +308,11 @@ static inline int at_xdmac_csize(u32 maxburst)
return csize;
};
+static inline bool at_xdmac_chan_is_peripheral_xfer(u32 cfg)
+{
+ return cfg & AT_XDMAC_CC_TYPE_PER_TRAN;
+}
+
static inline u8 at_xdmac_get_dwidth(u32 cfg)
{
return (cfg & AT_XDMAC_CC_DWIDTH_MASK) >> AT_XDMAC_CC_DWIDTH_OFFSET;
@@ -389,7 +394,13 @@ static void at_xdmac_start_xfer(struct at_xdmac_chan *atchan,
at_xdmac_chan_read(atchan, AT_XDMAC_CUBC));
at_xdmac_chan_write(atchan, AT_XDMAC_CID, 0xffffffff);
- reg = AT_XDMAC_CIE_RBEIE | AT_XDMAC_CIE_WBEIE | AT_XDMAC_CIE_ROIE;
+ reg = AT_XDMAC_CIE_RBEIE | AT_XDMAC_CIE_WBEIE;
+ /*
+ * Request Overflow Error is only for peripheral synchronized transfers
+ */
+ if (at_xdmac_chan_is_peripheral_xfer(first->lld.mbr_cfg))
+ reg |= AT_XDMAC_CIE_ROIE;
+
/*
* There is no end of list when doing cyclic dma, we need to get
* an interrupt after each periods.
^ permalink raw reply related
* [2/3] dmaengine: at_xdmac: enhance channel errors handling in tasklet
From: Nicolas Ferre @ 2019-02-05 11:03 UTC (permalink / raw)
To: Ludovic Desroches, linux-arm-kernel, dmaengine, Vinod Koul
Cc: Alexandre Belloni, linux-kernel, Nicolas Ferre
Complement the identification of errors with stoping the channel and
dumping the descriptor that led to the error case.
Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
drivers/dma/at_xdmac.c | 43 ++++++++++++++++++++++++++++++++++++------
1 file changed, 37 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
index 37a269420435..ec7a29d8e448 100644
--- a/drivers/dma/at_xdmac.c
+++ b/drivers/dma/at_xdmac.c
@@ -1575,6 +1575,41 @@ static void at_xdmac_handle_cyclic(struct at_xdmac_chan *atchan)
dmaengine_desc_get_callback_invoke(txd, NULL);
}
+static void at_xdmac_handle_error(struct at_xdmac_chan *atchan)
+{
+ struct at_xdmac *atxdmac = to_at_xdmac(atchan->chan.device);
+ struct at_xdmac_desc *bad_desc;
+
+ /*
+ * The descriptor currently at the head of the active list is
+ * broked. Since we don't have any way to report errors, we'll
+ * just have to scream loudly and try to carry on.
+ */
+ if (atchan->irq_status & AT_XDMAC_CIS_RBEIS)
+ dev_err(chan2dev(&atchan->chan), "read bus error!!!");
+ if (atchan->irq_status & AT_XDMAC_CIS_WBEIS)
+ dev_err(chan2dev(&atchan->chan), "write bus error!!!");
+ if (atchan->irq_status & AT_XDMAC_CIS_ROIS)
+ dev_err(chan2dev(&atchan->chan), "request overflow error!!!");
+
+ spin_lock_bh(&atchan->lock);
+ /* Channel must be disabled first as it's not done automatically */
+ at_xdmac_write(atxdmac, AT_XDMAC_GD, atchan->mask);
+ while (at_xdmac_read(atxdmac, AT_XDMAC_GS) & atchan->mask)
+ cpu_relax();
+ bad_desc = list_first_entry(&atchan->xfers_list,
+ struct at_xdmac_desc,
+ xfer_node);
+ spin_unlock_bh(&atchan->lock);
+ /* Print bad descriptor's details if needed */
+ dev_dbg(chan2dev(&atchan->chan),
+ "%s: lld: mbr_sa=%pad, mbr_da=%pad, mbr_ubc=0x%08x\n",
+ __func__, &bad_desc->lld.mbr_sa, &bad_desc->lld.mbr_da,
+ bad_desc->lld.mbr_ubc);
+
+ /* Then continue with usual descriptor management */
+}
+
static void at_xdmac_tasklet(unsigned long data)
{
struct at_xdmac_chan *atchan = (struct at_xdmac_chan *)data;
@@ -1594,12 +1629,8 @@ static void at_xdmac_tasklet(unsigned long data)
|| (atchan->irq_status & error_mask)) {
struct dma_async_tx_descriptor *txd;
- if (atchan->irq_status & AT_XDMAC_CIS_RBEIS)
- dev_err(chan2dev(&atchan->chan), "read bus error!!!");
- if (atchan->irq_status & AT_XDMAC_CIS_WBEIS)
- dev_err(chan2dev(&atchan->chan), "write bus error!!!");
- if (atchan->irq_status & AT_XDMAC_CIS_ROIS)
- dev_err(chan2dev(&atchan->chan), "request overflow error!!!");
+ if (atchan->irq_status & error_mask)
+ at_xdmac_handle_error(atchan);
spin_lock(&atchan->lock);
desc = list_first_entry(&atchan->xfers_list,
^ permalink raw reply related
* [1/3] dmaengine: at_xdmac: remove BUG_ON macro in tasklet
From: Nicolas Ferre @ 2019-02-05 11:03 UTC (permalink / raw)
To: Ludovic Desroches, linux-arm-kernel, dmaengine, Vinod Koul
Cc: Alexandre Belloni, linux-kernel, Nicolas Ferre
Even if this case shouldn't happen when controller is properly programmed,
it's still better to avoid dumping a kernel Oops for this.
As the sequence may happen only for debugging purposes, log the error and
just finish the tasklet call.
Signed-off-by: Nicolas Ferre <nicolas.ferre@microchip.com>
---
drivers/dma/at_xdmac.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/at_xdmac.c b/drivers/dma/at_xdmac.c
index fe69dccfa0c0..37a269420435 100644
--- a/drivers/dma/at_xdmac.c
+++ b/drivers/dma/at_xdmac.c
@@ -1606,7 +1606,11 @@ static void at_xdmac_tasklet(unsigned long data)
struct at_xdmac_desc,
xfer_node);
dev_vdbg(chan2dev(&atchan->chan), "%s: desc 0x%p\n", __func__, desc);
- BUG_ON(!desc->active_xfer);
+ if (!desc->active_xfer) {
+ dev_err(chan2dev(&atchan->chan), "Xfer not active: exiting");
+ spin_unlock_bh(&atchan->lock);
+ return;
+ }
txd = &desc->tx_dma_desc;
^ permalink raw reply related
* [v1] dmaengine: dmatest: Abort test in case of mapping error
From: Vinod Koul @ 2019-02-04 9:04 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: dmaengine, Seraj Alijan, Dan Williams
On 30-01-19, 21:48, Andy Shevchenko wrote:
> In case of mapping error the DMA addresses are invalid and continuing
> will screw system memory or potentially something else.
>
> [ 222.480310] dmatest: dma0chan7-copy0: summary 1 tests, 3 failures 6 iops 349 KB/s (0)
> ...
> [ 240.912725] check: Corrupted low memory at 00000000c7c75ac9 (2940 phys) = 5656000000000000
> [ 240.921998] check: Corrupted low memory at 000000005715a1cd (2948 phys) = 279f2aca5595ab2b
> [ 240.931280] check: Corrupted low memory at 000000002f4024c0 (2950 phys) = 5e5624f349e793cf
> ...
>
> Abort any test if mapping failed.
Applied, thanks
^ permalink raw reply
* [5/8,v5] dma: k3dma: Add support for dma-channel-mask
From: Vinod Koul @ 2019-02-04 8:59 UTC (permalink / raw)
To: John Stultz
Cc: lkml, Li Yu, Dan Williams, Tanglei Han, Zhuangluan Su,
Ryan Grachek, Manivannan Sadhasivam, Guodong Xu, dmaengine
On 24-01-19, 12:24, John Stultz wrote:
> From: Li Yu <liyu65@hisilicon.com>
>
> Add dma-channel-mask as a property for k3dma, it defines
> available dma channels which a non-secure mode driver can use.
>
> One sample usage of this is in Hi3660 SoC. DMA channel 0 is
> reserved to lpm3, which is a coprocessor for power management. So
> as a result, any request in kernel (which runs on main processor
> and in non-secure mode) should start from at least channel 1.
>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Vinod Koul <vkoul@kernel.org>
> Cc: Tanglei Han <hantanglei@huawei.com>
> Cc: Zhuangluan Su <suzhuangluan@hisilicon.com>
> Cc: Ryan Grachek <ryan@edited.us>
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Cc: Guodong Xu <guodong.xu@linaro.org>
> Cc: dmaengine@vger.kernel.org
> Signed-off-by: Li Yu <liyu65@hisilicon.com>
> [jstultz: Reworked to use a channel mask]
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> v3: Rename to hisi-dma-avail-chan
> v4: Rename to dma-channel-mask
> v5: Use BIT(i) instead of (1<<i)
> ---
> drivers/dma/k3dma.c | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/k3dma.c b/drivers/dma/k3dma.c
> index e415c85..294ec64 100644
> --- a/drivers/dma/k3dma.c
> +++ b/drivers/dma/k3dma.c
> @@ -111,6 +111,7 @@ struct k3_dma_dev {
> struct dma_pool *pool;
> u32 dma_channels;
> u32 dma_requests;
> + u32 dma_channel_mask;
> unsigned int irq;
> };
>
> @@ -319,6 +320,9 @@ static void k3_dma_tasklet(unsigned long arg)
> /* check new channel request in d->chan_pending */
> spin_lock_irq(&d->lock);
> for (pch = 0; pch < d->dma_channels; pch++) {
> + if (!(d->dma_channel_mask & (1<<pch)))
spaces around <<
> + continue;
> +
> p = &d->phy[pch];
>
> if (p->vchan == NULL && !list_empty(&d->chan_pending)) {
> @@ -336,6 +340,9 @@ static void k3_dma_tasklet(unsigned long arg)
> spin_unlock_irq(&d->lock);
>
> for (pch = 0; pch < d->dma_channels; pch++) {
> + if (!(d->dma_channel_mask & (1<<pch)))
and here :(
> + continue;
> +
> if (pch_alloc & (1 << pch)) {
> p = &d->phy[pch];
> c = p->vchan;
> @@ -856,6 +863,13 @@ static int k3_dma_probe(struct platform_device *op)
> "dma-channels", &d->dma_channels);
> of_property_read_u32((&op->dev)->of_node,
> "dma-requests", &d->dma_requests);
> + ret = of_property_read_u32((&op->dev)->of_node,
> + "dma-channel-mask", &d->dma_channel_mask);
> + if (ret) {
> + dev_warn(&op->dev,
> + "dma-channel-mask doesn't exist, considering all as available.\n");
> + d->dma_channel_mask = (u32)~0UL;
> + }
> }
>
> if (!(soc_data->flags & K3_FLAG_NOCLK)) {
> @@ -887,8 +901,12 @@ static int k3_dma_probe(struct platform_device *op)
> return -ENOMEM;
>
> for (i = 0; i < d->dma_channels; i++) {
> - struct k3_dma_phy *p = &d->phy[i];
> + struct k3_dma_phy *p;
> +
> + if (!(d->dma_channel_mask & BIT(i)))
> + continue;
>
> + p = &d->phy[i];
> p->idx = i;
> p->base = d->base + i * 0x40;
> }
> --
> 2.7.4
^ permalink raw reply
* dmaengine: fsldma: Add 64-bit I/O accessors for powerpc64
From: Vinod Koul @ 2019-02-04 7:27 UTC (permalink / raw)
To: Peng Ma
Cc: Scott Wood, Leo Li, Zhang Wei, linuxppc-dev@lists.ozlabs.org,
dmaengine@vger.kernel.org, Wen He
On 25-01-19, 05:54, Peng Ma wrote:
> Hi Vinod,
>
> Sorry to replay late.
> 1:This patch has already send to the patchwork.
> Please see the patch link: https://patchwork.kernel.org/patch/10741521/
> 2:I have already compile the fsl patches on arm and powerpc after patched https://patchwork.kernel.org/patch/10741521/
> The compile will successful, please let me know the reported regression results, thanks very much.
And I thought there were further comments on this patch. I have applied
this, please watch for failures reported if any..
^ permalink raw reply
* [v10,1/3] dmaengine: 8250_mtk_dma: add MediaTek uart DMA support
From: Vinod Koul @ 2019-02-04 7:21 UTC (permalink / raw)
To: Long Cheng
Cc: Randy Dunlap, Rob Herring, Mark Rutland, Ryder Lee, Sean Wang,
Nicolas Boichat, Matthias Brugger, Dan Williams,
Greg Kroah-Hartman, Jiri Slaby, Sean Wang, dmaengine, devicetree,
linux-arm-kernel, linux-mediatek, linux-kernel, linux-serial,
srv_heupstream, Yingjoe Chen, YT Shen, Zhenbao Liu
On 18-01-19, 11:10, Long Cheng wrote:
> +static enum dma_status mtk_uart_apdma_tx_status(struct dma_chan *chan,
> + dma_cookie_t cookie,
> + struct dma_tx_state *txstate)
> +{
> + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> + enum dma_status ret;
> + unsigned long flags;
> +
> + if (!txstate)
> + return DMA_ERROR;
Why, it is not a mandatory arg!
> + ret = dma_cookie_status(chan, cookie, txstate);
> + spin_lock_irqsave(&c->vc.lock, flags);
> + if (ret == DMA_IN_PROGRESS) {
> + c->rx_status = mtk_uart_apdma_read(c, VFF_RPT) & VFF_RING_SIZE;
> + dma_set_residue(txstate, c->rx_status);
> + } else if (ret == DMA_COMPLETE && c->dir == DMA_DEV_TO_MEM) {
> + dma_set_residue(txstate, c->rx_status);
what is the point is setting residue to comleted txn, it is zero!
> + } else {
> + dma_set_residue(txstate, 0);
naah that doesnt sound correct!
> +static void mtk_uart_apdma_config_write(struct dma_chan *chan,
> + struct dma_slave_config *cfg,
> + enum dma_transfer_direction dir)
> +{
> + struct mtk_chan *c = to_mtk_uart_apdma_chan(chan);
> + struct mtk_uart_apdmadev *mtkd =
> + to_mtk_uart_apdma_dev(c->vc.chan.device);
> + unsigned int tmp;
> +
> + if (mtk_uart_apdma_read(c, VFF_EN) == VFF_EN_B)
> + return;
> +
> + c->dir = dir;
> +
> + if (dir == DMA_DEV_TO_MEM) {
> + tmp = cfg->src_addr_width * 1024;
why multiply by 1024?
> +static int mtk_uart_apdma_device_pause(struct dma_chan *chan)
> +{
> + /* just for check caps pass */
> + return 0;
> +}
please remove, this is not a mandatory fn
^ permalink raw reply
* dmaengine: imx-dma: fix wrong callback invoke
From: Vinod Koul @ 2019-02-04 7:06 UTC (permalink / raw)
To: Leonid Iziumtsev; +Cc: dmaengine, Dan Williams, linux-kernel, m.grzeschik
On 15-01-19, 17:15, Leonid Iziumtsev wrote:
> Once the "ld_queue" list is not empty, next descriptor will migrate
> into "ld_active" list. The "desc" variable will be overwritten
> during that transition. And later the dmaengine_desc_get_callback_invoke()
> will use it as an argument. As result we invoke wrong callback.
>
> That behaviour was in place since:
> commit fcaaba6c7136 ("dmaengine: imx-dma: fix callback path in tasklet").
> But after commit 4cd13c21b207 ("softirq: Let ksoftirqd do its job")
> things got worse, since possible delay between tasklet_schedule()
> from DMA irq handler and actual tasklet function execution got bigger.
> And that gave more time for new DMA request to be submitted and
> to be put into "ld_queue" list.
>
> It has been noticed that DMA issue is causing problems for "mxc-mmc"
> driver. While stressing the system with heavy network traffic and
> writing/reading to/from sd card simultaneously the timeout may happen:
>
> 10013000.sdhci: mxcmci_watchdog: read time out (status = 0x30004900)
>
> That often lead to file system corruption.
Applied and tagged to stable, thanks
^ permalink raw reply
* dmaengine: fsl-edma: dma map slave device address
From: Vinod Koul @ 2019-02-04 7:03 UTC (permalink / raw)
To: Laurentiu Tudor
Cc: dmaengine, linux-imx, linux-arm-kernel, linux-kernel, iommu,
robin.murphy
On 18-01-19, 12:06, Laurentiu Tudor wrote:
> This mapping needs to be created in order for slave dma transfers
> to work on systems with SMMU. The implementation mostly mimics the
> one in pl330 dma driver, authored by Robin Murphy.
Applied, thanks
^ permalink raw reply
* [02/18] dmaengine: imx-sdma: pass struct device to DMA API functions
From: Christoph Hellwig @ 2019-02-02 17:21 UTC (permalink / raw)
To: Vinod Koul
Cc: Christoph Hellwig, John Crispin, Dmitry Tarnyagin, Nicolas Ferre,
Sudip Mukherjee, Felipe Balbi, linux-mips, linux-kernel,
dmaengine, netdev, linux-usb, linux-fbdev, alsa-devel, iommu
On Sat, Feb 02, 2019 at 03:41:21PM +0530, Vinod Koul wrote:
> On 01-02-19, 09:47, Christoph Hellwig wrote:
> > The DMA API generally relies on a struct device to work properly, and
> > only barely works without one for legacy reasons. Pass the easily
> > available struct device from the platform_device to remedy this.
>
> This looks good to me but fails to apply. Can you please base it on
> dmaengine-next or linux-next please and resend
commit ceaf52265148d3a5ca24237fd1c709caa5f46184
Author: Andy Duan <fugang.duan@nxp.com>
Date: Fri Jan 11 14:29:49 2019 +0000
dmaengine: imx-sdma: pass ->dev to dma_alloc_coherent() API
in linux-next actually is equivalent to this patch, so we can drop
this one.
>
> Thanks
> --
> ~Vinod
---end quoted text---
^ permalink raw reply
* [v2] dmaengine: at_xdmac: Fix wrongfull report of a channel as in use
From: Vinod Koul @ 2019-02-02 10:25 UTC (permalink / raw)
To: Codrin.Ciubotariu
Cc: Ludovic.Desroches, dmaengine, linux-arm-kernel, linux-kernel
On 23-01-19, 16:33, Codrin.Ciubotariu@microchip.com wrote:
> From: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
>
> atchan->status variable is used to store two different information:
> - pass channel interrupts status from interrupt handler to tasklet;
> - channel information like whether it is cyclic or paused;
>
> This causes a bug when device_terminate_all() is called,
> (AT_XDMAC_CHAN_IS_CYCLIC cleared on atchan->status) and then a late End
> of Block interrupt arrives (AT_XDMAC_CIS_BIS), which sets bit 0 of
> atchan->status. Bit 0 is also used for AT_XDMAC_CHAN_IS_CYCLIC, so when
> a new descriptor for a cyclic transfer is created, the driver reports
> the channel as in use:
>
> if (test_and_set_bit(AT_XDMAC_CHAN_IS_CYCLIC, &atchan->status)) {
> dev_err(chan2dev(chan), "channel currently used\n");
> return NULL;
> }
>
> This patch fixes the bug by adding a different struct member to keep
> the interrupts status separated from the channel status bits.
Applied, thanks
^ permalink raw reply
* [02/18] dmaengine: imx-sdma: pass struct device to DMA API functions
From: Vinod Koul @ 2019-02-02 10:11 UTC (permalink / raw)
To: Christoph Hellwig
Cc: John Crispin, Dmitry Tarnyagin, Nicolas Ferre, Sudip Mukherjee,
Felipe Balbi, linux-mips, linux-kernel, dmaengine, netdev,
linux-usb, linux-fbdev, alsa-devel, iommu
On 01-02-19, 09:47, Christoph Hellwig wrote:
> The DMA API generally relies on a struct device to work properly, and
> only barely works without one for legacy reasons. Pass the easily
> available struct device from the platform_device to remedy this.
This looks good to me but fails to apply. Can you please base it on
dmaengine-next or linux-next please and resend
Thanks
^ permalink raw reply
* [RFC,v3,1/7] dmaengine: Add Synopsys eDMA IP core driver
From: Vinod Koul @ 2019-02-02 10:07 UTC (permalink / raw)
To: Gustavo Pimentel
Cc: linux-pci@vger.kernel.org, dmaengine@vger.kernel.org,
Dan Williams, Eugeniy Paltsev, Andy Shevchenko, Russell King,
Niklas Cassel, Joao Pinto, Jose Abreu, Luis Oliveira,
Vitor Soares, Nelson Costa, Pedro Sousa
On 01-02-19, 11:23, Gustavo Pimentel wrote:
> On 01/02/2019 04:14, Vinod Koul wrote:
> > On 31-01-19, 11:33, Gustavo Pimentel wrote:
> >> On 23/01/2019 13:08, Vinod Koul wrote:
> >>> On 21-01-19, 15:48, Gustavo Pimentel wrote:
> >>>> On 20/01/2019 11:44, Vinod Koul wrote:
> >>>>> On 11-01-19, 19:33, Gustavo Pimentel wrote:
> >
> >>>>>> @@ -0,0 +1,1059 @@
> >>>>>> +// SPDX-License-Identifier: GPL-2.0
> >>>>>> +/*
> >>>>>> + * Copyright (c) 2018 Synopsys, Inc. and/or its affiliates.
> >>>>>
> >>>>> 2019 now
> >>>>
> >>>> I've changed to "Copyright (c) 2018-present Synopsys, Inc. and/or its
> >>>> affiliates." this way it's always up to date and I also kept 2018, because it
> >>>> was the date that I started to develop this driver, if you don't mind.
> >>>
> >>> yeah 18 is fine :) it need to end with current year always
> >>
> >> Just to be sure, are you saying that must be: "Copyright (c) 2018-2019 Synopsys,
> >> Inc. and/or its affiliates."?
> >
> > Yup :)
> >
> >>>>>> +static struct dw_edma_chunk *dw_edma_alloc_chunk(struct dw_edma_desc *desc)
> >>>>>> +{
> >>>>>> + struct dw_edma_chan *chan = desc->chan;
> >>>>>> + struct dw_edma *dw = chan->chip->dw;
> >>>>>> + struct dw_edma_chunk *chunk;
> >>>>>> +
> >>>>>> + chunk = kvzalloc(sizeof(*chunk), GFP_NOWAIT);
> >>>>>> + if (unlikely(!chunk))
> >>>>>> + return NULL;
> >>>>>> +
> >>>>>> + INIT_LIST_HEAD(&chunk->list);
> >>>>>> + chunk->chan = chan;
> >>>>>> + chunk->cb = !(desc->chunks_alloc % 2);
> >>>>>
> >>>>> cb ..?
> >>>>
> >>>> CB = change bit, is a property of this eDMA IP. Basically it is a kind of
> >>>> handshake which serves to validate whether the linked list has been updated or
> >>>> not, especially useful in cases of recycled linked list elements (every linked
> >>>> list recycle is a new chunk, this will allow to differentiate each chunk).
> >>>
> >>> okay please add that somewhere. Also it would help me if you explain
> >>> what is chunk and other terminologies used in this driver
> >>
> >> I'm thinking to put the below description on the patch, please check if this is
> >> sufficient explicit and clear to understand what this IP needs and does.
> >>
> >> In order to transfer data from point A to B as fast as possible this IP requires
> >> a dedicated memory space where will reside a linked list of elements.
> >
> > rephrasing: a dedicated memory space containing linked list of elements
> >
> >> All elements of this linked list are continuous and each one describes a data
> >> transfer (source and destination addresses, length and a control variable).
> >>
> >> For the sake of simplicity, lets assume a memory space for channel write 0 which
> >> allows about 42 elements.
> >>
> >> +---------+
> >> | Desc #0 |--+
> >> +---------+ |
> >> V
> >> +----------+
> >> | Chunk #0 |---+
> >> | CB = 1 | | +----------+ +-----+ +-----------+ +-----+
> >> +----------+ +-->| Burst #0 |-->| ... |-->| Burst #41 |-->| llp |
> >> | +----------+ +-----+ +-----------+ +-----+
> >> V
> >> +----------+
> >> | Chunk #1 |---+
> >> | CB = 0 | | +-----------+ +-----+ +-----------+ +-----+
> >> +----------+ +-->| Burst #42 |-->| ... |-->| Burst #83 |-->| llp |
> >> | +-----------+ +-----+ +-----------+ +-----+
> >> V
> >> +----------+
> >> | Chunk #2 |---+
> >> | CB = 1 | | +-----------+ +-----+ +------------+ +-----+
> >> +----------+ +-->| Burst #84 |-->| ... |-->| Burst #125 |-->| llp |
> >> | +-----------+ +-----+ +------------+ +-----+
> >> V
> >> +----------+
> >> | Chunk #3 |---+
> >> | CB = 0 | | +------------+ +-----+ +------------+ +-----+
> >> +----------+ +-->| Burst #126 |-->| ... |-->| Burst #129 |-->| llp |
> >> +------------+ +-----+ +------------+ +-----+
> >
> > This is great and required to understand the driver.
> >
> > How does controller move from Burnst #41 of Chunk 0 to Chunk 1 ?
>
> I forgot to explain that...
>
> On every last Burst of the Chunk (Burst #41, Burst #83, Burst #125 or even Burst
> #129) is set some flags on their control variable (RIE and LIE bits) that will
> trigger the send of "done" interruption.
>
> On the interruptions callback, is decided whether to recycle the linked list
> memory space by writing a new set of Bursts elements (if still exists Chunks to
> transfer) or is considered completed (if there is no Chunks available to transfer)
>
> > Is Burst 0 to 129 a link list with Burst 0, 42, 84 and 126 having a
> > callback (done bit set)..
>
> I didn't quite understand it your question.
>
> It comes from the prep_slave_sg n elements (130 applying the example), where
> will be divide in several Chunks (#0, #1, #2, #3 and #4 applying the example) (a
> linked list that will contain another linked list for the Bursts, CB, Chunk
> size). The linked list inside of each Chunk will contain a number of Bursts
> (limited to the memory space size), each one will possess Source Address,
> Destination Address and size of that sub-transfer.
Thanks this helps :)
so in this case what does prep_slave_sg n elements corrosponds to (130
bursts) ..? If so how do you split a transaction to chunks and bursts?
>
> >
> > This sound **very** similar to dw dma concepts!
>
> I believe some parts of dw dma and dw edma behavior are similar and that makes
> perfectly sense since both dma are done by Synopsys and may be exist a shared
> knowledge, however they are different IPs applied to different products.
>
> >
> >>
> >> Legend:
> >>
> >> *Linked list*, also know as Chunk
> >> *Linked list element*, also know as Burst
> >> *CB*, also know as Change Bit, it's a control bit (and typically is toggled)
> >> that allows to easily identify and differentiate between the current linked list
> >> and the previous or the next one.
> >> *LLP*, is a special element that indicates the end of the linked list element
> >> stream also informs that the next CB should be toggle.
> >>
> >> On scatter-gather transfer mode, the client will submit a scatter-gather list of
> >> n (on this case 130) elements, that will be divide in multiple Chunks, each
> >> Chunk will have (on this case 42) a limited number of Bursts and after
> >> transferring all Bursts, an interrupt will be triggered, which will allow to
> >> recycle the all linked list dedicated memory again with the new information
> >> relative to the next Chunk and respective Burst associated and repeat the whole
> >> cycle again.
> >>
> >> On cyclic transfer mode, the client will submit a buffer pointer, length of it
> >> and number of repetitions, in this case each burst will correspond directly to
> >> each repetition.
> >>
> >> Each Burst can describes a data transfer from point A(source) to point
> >> B(destination) with a length that can be from 1 byte up to 4 GB. Since dedicated
> >> the memory space where the linked list will reside is limited, the whole n burst
> >> elements will be organized in several Chunks, that will be used later to recycle
> >> the dedicated memory space to initiate a new sequence of data transfers.
> >>
> >> The whole transfer is considered has completed when it was transferred all bursts.
> >>
> >> Currently this IP has a set well-known register map, which includes support for
> >> legacy and unroll modes. Legacy mode is version of this register map that has
> >
> > whats unroll..
> >
> >> multiplexer register that allows to switch registers between all write and read
>
> The unroll is explained here, see below
>
> >> channels and the unroll modes repeats all write and read channels registers with
> >> an offset between them. This register map is called v0.
> >>
> >> The IP team is creating a new register map more suitable to the latest PCIe
> >> features, that very likely will change the map register, which this version will
> >> be called v1. As soon as this new version is released by the IP team the support
> >> for this version in be included on this driver.
> >>
> >> What do you think? There is any gray area that I could clarify?
> >
> > This sounds good. But we are also catering to a WIP IP which can change
> > right. Doesnt sound very good idea to me :)
>
> This IP exists for several years like this and it works quite fine, however
> because of new features and requests (SR-IOV, more dma channels, function
> segregation and isolation, performance improvement) that are coming it's natural
> to have exist improvements. The drivers should follow the evolution and be
> sufficient robust enough to adapt to this new circumstance.
Kernel driver should be modular be design, if we keep things simple then
adding/splitting to support future revisions should be easy!
> >>>>>> + dev_dbg(chan2dev(chan), "addr(physical) src=%pa, dst=%pa\n",
> >>>>>> + &config->src_addr, &config->dst_addr);
> >>>>>> +
> >>>>>> + chan->src_addr = config->src_addr;
> >>>>>> + chan->dst_addr = config->dst_addr;
> >>>>>> +
> >>>>>> + err = ops->device_config(dchan);
> >>>>>
> >>>>> what does this do?
> >>>>
> >>>> This is an initialization procedure to setup interrupts (data and addresses) to
> >>>> each channel on the eDMA IP, in order to be triggered after transfer being
> >>>> completed or aborted. Due the fact the config() can be called at anytime,
> >>>> doesn't make sense to have this procedure here, I'll moved it to probe().
> >>>
> >>> Yeah I am not still convinced about having another layer! Have you
> >>> though about using common lib for common parts ..?
> >>
> >> Maybe I'm explaining myself wrongly. I don't have any clue about the new
> >> register map for the future versions. I honestly tried to implement the common
> >> lib for the whole process that interact with dma engine controller to ease in
> >> the future any new addition of register mapping, by having this common callbacks
> >> that will only be responsible for interfacing the HW accordingly to register map
> >> version. Maybe I can simplify something in the future, but I only be able to
> >> conclude that after having some idea about the new register map.
> >>
> >> IMHO I think this is the easier and clean way to do it, in terms of code
> >> maintenance and architecture, but if you have another idea that you can show me
> >> or pointing out for a driver that implements something similar, I'm no problem
> >> to check it out.
> >
> > That is what my fear was :)
> >
> > Lets step back and solve one problem at a time. Right now that is v0 of
> > IP. Please write a simple driver which solve v0 without any layers
> > involved.
> >
> > Once v1 is in good shape, you would know what it required and then we
> > can split v0 driver into common lib and v0 driver and then add v1
> > driver.
>
> Can I keep the code segregation as it is now? With the dw-edma-v0-core.c/h,
> dw-edma-v0-debugfs.c/h and dw-edma-v0-regs.h
>
> That way I would only replace the callbacks calls to direct function calls and
> remove the switch case callback selection base on the version.
That sound better, please keep patches smallish (anything going more
than 600 lines becomes harder to review) and logically split.
> >>>>> what is the second part of the check, can you explain that, who sets
> >>>>> chan->dir?
> >>>>
> >>>> The chan->dir is set on probe() during the process of configuring each channel.
> >>>
> >>> So you have channels that are unidirectional?
> >>
> >> Yes. That's one another reason IMHO to keep the dw-edma-test separate from
> >> dma-test, since this IP is more picky and have this particularities.
> >
> > That is okay, that should be handled by prep calls, if you get a prep
> > call for direction you dont support return error and move to next
> > available one.
> >
> > That can be done generically in dmatest driver and to answer your
> > question, yes I would test that :)
>
> Like you said, let do this in small steps. For now I would like to suggest to
> leave out the dw-dma-test driver and just focus on the current driver, if you
> don't mind. I never thought that his test driver would raise this kind of discuss.
Well dmatest is just a testing aid and doesnt care about internal
designs of your driver. I would say keep it as it is useful aid and will
help other folks as well :)
^ permalink raw reply
* [alsa-devel] don't pass a NULL struct device to DMA API functions
From: Takashi Iwai @ 2019-02-01 16:18 UTC (permalink / raw)
To: Christoph Hellwig
Cc: John Crispin, Vinod Koul, Dmitry Tarnyagin, Nicolas Ferre,
Sudip Mukherjee, Felipe Balbi, linux-mips, linux-kernel,
dmaengine, netdev, linux-usb, linux-fbdev, alsa-devel, iommu
On Fri, 01 Feb 2019 17:09:57 +0100,
Christoph Hellwig wrote:
>
> On Fri, Feb 01, 2019 at 02:16:08PM +0100, Takashi Iwai wrote:
> > Actually there are a bunch of ISA sound drivers that still call
> > allocators with NULL device.
> >
> > The patch below should address it, although it's only compile-tested.
>
> Oh, I missed these "indirect" calls. This looks good to me:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
OK, merged this one to for-next branch now as well.
thanks,
Takashi
^ permalink raw reply
* [17/18] ALSA: hal2: pass struct device to DMA API functions
From: Takashi Iwai @ 2019-02-01 16:17 UTC (permalink / raw)
To: Christoph Hellwig
Cc: John Crispin, Vinod Koul, Dmitry Tarnyagin, Nicolas Ferre,
Sudip Mukherjee, Felipe Balbi, linux-mips, linux-kernel,
dmaengine, netdev, linux-usb, linux-fbdev, alsa-devel, iommu
On Fri, 01 Feb 2019 17:09:06 +0100,
Christoph Hellwig wrote:
>
> On Fri, Feb 01, 2019 at 02:12:45PM +0100, Takashi Iwai wrote:
> > Shall I take this one through sound git tree or all through yours?
>
> Feel free to merge the sound bits through your tree!
Alright, merged both patches 17 and 18 now.
thanks,
Takashi
^ permalink raw reply
* [10/18] smc911x: pass struct device to DMA API functions
From: Christoph Hellwig @ 2019-02-01 16:11 UTC (permalink / raw)
To: Robin Murphy
Cc: Christoph Hellwig, John Crispin, Vinod Koul, Dmitry Tarnyagin,
Nicolas Ferre, Sudip Mukherjee, Felipe Balbi, linux-mips,
linux-kernel, dmaengine, netdev, linux-usb, linux-fbdev,
alsa-devel, iommu
On Fri, Feb 01, 2019 at 02:14:34PM +0000, Robin Murphy wrote:
> And equivalently for rxdma here. However, given that this all seems only
> relevant to antique ARCH_PXA platforms which are presumably managing to
> work as-is, it's probably not worth tinkering too much. I'd just stick a
> note in the commit message that we're still only making these
> self-consistent with the existing dma_map_single() calls rather than
> necessarily correct.
Sounds good.
^ permalink raw reply
* [03/18] net: caif: pass struct device to DMA API functions
From: Christoph Hellwig @ 2019-02-01 16:10 UTC (permalink / raw)
To: Robin Murphy
Cc: Christoph Hellwig, John Crispin, Vinod Koul, Dmitry Tarnyagin,
Nicolas Ferre, Sudip Mukherjee, Felipe Balbi, linux-mips,
linux-kernel, dmaengine, netdev, linux-usb, linux-fbdev,
alsa-devel, iommu
On Fri, Feb 01, 2019 at 01:53:09PM +0000, Robin Murphy wrote:
>> #define LOW_WATER_MARK 100
>> #define HIGH_WATER_MARK (LOW_WATER_MARK*5)
>> -#ifdef CONFIG_UML
>> +#ifdef CONFIG_HAS_DMA
>
> #ifndef, surely?
Indeed.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox