* dma: axi-dmac: Split too large segments
From: Lars-Peter Clausen @ 2019-02-18 7:34 UTC (permalink / raw)
To: Ardelean, Alexandru, lkp@intel.com
Cc: kbuild-all@01.org, dmaengine@vger.kernel.org, vkoul@kernel.org
On 2/18/19 8:28 AM, Ardelean, Alexandru wrote:
> On Sat, 2019-02-16 at 17:03 +0800, kbuild test robot wrote:
>>
>
> My bad here.
> I took this patch from our kernel tree and sent it.
> I assumed it works, since it works in our tree.
> I'll take a look and see about the order of patches, and which one(s)
> need(s) to be sent before this one
https://github.com/analogdevicesinc/linux/commit/414a555896b2abca3518c3c878f21e7b1ea95904#diff-b5b2fd5430b4f9aff1ae97bae60dd5c5
That patch was sent upstream, not sure why it was never merged.
But if necessary you can also re-write this patch to not rely on the
other one.
^ permalink raw reply
* [1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id
From: Arnd Bergmann @ 2019-02-18 10:31 UTC (permalink / raw)
To: Baolin Wang
Cc: Vinod Koul, Rob Herring, Mark Rutland, Olof Johansson, Orson Zhai,
Lyra Zhang, Dan Williams, DTML, arm-soc, Linux ARM,
Linux Kernel Mailing List, dmaengine, eric.long, Mark Brown
On Tue, Feb 12, 2019 at 9:25 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> On Fri, 1 Feb 2019 at 19:53, Baolin Wang <baolin.wang@linaro.org> wrote:
> > On Thu, 31 Jan 2019 at 00:52, Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Tue, Jan 22, 2019 at 2:21 PM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > >
> > > > Client:
> > > > DMA clients connected to the Spreadtrum DMA controller must use the format
> > > > -described in the dma.txt file, using a two-cell specifier for each channel.
> > > > -The two cells in order are:
> > > > +described in the dma.txt file, using a three-cell specifier for each channel.
> > > > +The three cells in order are:
> > > > 1. A phandle pointing to the DMA controller.
> > > > 2. The channel id.
> > > > +3. The hardware slave id which is used for clients to trigger DMA engine
> > > > +automatically.
> > >
> > > I notice that this is an incompatible binding change. Is that necessary?
> > > If the current code works, I'd suggest allowing both #dma-cells=<2>
> > > and <3>, and then implementing both in the driver.
> >
> > Yes, this is necessary.
> >
> > Yes, current code can work, but the problem is that the DMA clients
> > must add one property (something like "sprd,slave-id") to specify the
> > slave id. So considering this, we want to change the dma-cells to 2,
> > including dma channel and dma slave id, which can avoid introducing
> > some similar properties for DMA clients.
> >
> > Now there are no DMA clients in mainline for Spreadtrum platform, and
> > we want to upstream our first DMA clients: SPI controller. So no other
> > drivers need to change when we changing dma cells. Thanks.
>
> Do you have any other concerns about this patch set? If not, I think
> Vinod can apply this patch set. Thanks.
Sorry for the late reply. Yes, this makes sense since there are no
existing users then. For the DT changes going through the dmaengine
tree
Acked-by: Arnd Bergmann <arnd@arndb.de>
One more question, to make sure we don't need to edit it again:
Why do you need both a 'channel id' and a 'slave id' here? Is this
a strict hardware requirement for your DMA engine? In many
other designs, only a DMA request line number needs to
be described, and I think this would correspond to what you
call the 'hardware slave id' in your documentation.
Does each request line here correspond to a fixed channel
id as well, or can a channel be shared between multiple
slave devices?
Arnd
^ permalink raw reply
* [1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id
From: Baolin Wang @ 2019-02-18 10:52 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Vinod Koul, Rob Herring, Mark Rutland, Olof Johansson, Orson Zhai,
Lyra Zhang, Dan Williams, DTML, arm-soc, Linux ARM,
Linux Kernel Mailing List, dmaengine, eric.long, Mark Brown
Hi Arnd,
On Mon, 18 Feb 2019 at 18:31, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, Feb 12, 2019 at 9:25 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> > On Fri, 1 Feb 2019 at 19:53, Baolin Wang <baolin.wang@linaro.org> wrote:
> > > On Thu, 31 Jan 2019 at 00:52, Arnd Bergmann <arnd@arndb.de> wrote:
> > > > On Tue, Jan 22, 2019 at 2:21 PM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > >
> > > > > Client:
> > > > > DMA clients connected to the Spreadtrum DMA controller must use the format
> > > > > -described in the dma.txt file, using a two-cell specifier for each channel.
> > > > > -The two cells in order are:
> > > > > +described in the dma.txt file, using a three-cell specifier for each channel.
> > > > > +The three cells in order are:
> > > > > 1. A phandle pointing to the DMA controller.
> > > > > 2. The channel id.
> > > > > +3. The hardware slave id which is used for clients to trigger DMA engine
> > > > > +automatically.
> > > >
> > > > I notice that this is an incompatible binding change. Is that necessary?
> > > > If the current code works, I'd suggest allowing both #dma-cells=<2>
> > > > and <3>, and then implementing both in the driver.
> > >
> > > Yes, this is necessary.
> > >
> > > Yes, current code can work, but the problem is that the DMA clients
> > > must add one property (something like "sprd,slave-id") to specify the
> > > slave id. So considering this, we want to change the dma-cells to 2,
> > > including dma channel and dma slave id, which can avoid introducing
> > > some similar properties for DMA clients.
> > >
> > > Now there are no DMA clients in mainline for Spreadtrum platform, and
> > > we want to upstream our first DMA clients: SPI controller. So no other
> > > drivers need to change when we changing dma cells. Thanks.
> >
> > Do you have any other concerns about this patch set? If not, I think
> > Vinod can apply this patch set. Thanks.
>
> Sorry for the late reply. Yes, this makes sense since there are no
> existing users then. For the DT changes going through the dmaengine
> tree
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
Thanks for your reviewing.
>
> One more question, to make sure we don't need to edit it again:
> Why do you need both a 'channel id' and a 'slave id' here? Is this
> a strict hardware requirement for your DMA engine? In many
> other designs, only a DMA request line number needs to
> be described, and I think this would correspond to what you
> call the 'hardware slave id' in your documentation.
I try to explain why we need the slave id.
For our DMA engine driver, we have software request mode and hardware
request mode. For software request mode, the DMA engine driver need
trigger DMA to start transfer manually. But for hardware request mode,
we just set one unique slave id corresponding to the slave hardware to
the DMA engine, then the slave hardware can trigger DMA automatically.
And the slave id is not always same with the channel id according to
the SoC design, so we add one cell to specify the slave id.
>
> Does each request line here correspond to a fixed channel
> id as well, or can a channel be shared between multiple
Yes, each request line corresponds to a fixed channel id.
> slave devices?
>
> Arnd
^ permalink raw reply
* [1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id
From: Arnd Bergmann @ 2019-02-18 12:23 UTC (permalink / raw)
To: Baolin Wang
Cc: Vinod Koul, Rob Herring, Mark Rutland, Olof Johansson, Orson Zhai,
Lyra Zhang, Dan Williams, DTML, arm-soc, Linux ARM,
Linux Kernel Mailing List, dmaengine, eric.long, Mark Brown
On Mon, Feb 18, 2019 at 11:52 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> On Mon, 18 Feb 2019 at 18:31, Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > On Tue, Feb 12, 2019 at 9:25 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > On Fri, 1 Feb 2019 at 19:53, Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > On Thu, 31 Jan 2019 at 00:52, Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > On Tue, Jan 22, 2019 at 2:21 PM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > > >
> > > > > > Client:
> > > > > > DMA clients connected to the Spreadtrum DMA controller must use the format
> > > > > > -described in the dma.txt file, using a two-cell specifier for each channel.
> > > > > > -The two cells in order are:
> > > > > > +described in the dma.txt file, using a three-cell specifier for each channel.
> > > > > > +The three cells in order are:
> > > > > > 1. A phandle pointing to the DMA controller.
> > > > > > 2. The channel id.
> > > > > > +3. The hardware slave id which is used for clients to trigger DMA engine
> > > > > > +automatically.
> > > > >
> > > > > I notice that this is an incompatible binding change. Is that necessary?
> > > > > If the current code works, I'd suggest allowing both #dma-cells=<2>
> > > > > and <3>, and then implementing both in the driver.
> > > >
> > > > Yes, this is necessary.
> > > >
> > > > Yes, current code can work, but the problem is that the DMA clients
> > > > must add one property (something like "sprd,slave-id") to specify the
> > > > slave id. So considering this, we want to change the dma-cells to 2,
> > > > including dma channel and dma slave id, which can avoid introducing
> > > > some similar properties for DMA clients.
> > > >
> > > > Now there are no DMA clients in mainline for Spreadtrum platform, and
> > > > we want to upstream our first DMA clients: SPI controller. So no other
> > > > drivers need to change when we changing dma cells. Thanks.
> > >
> > > Do you have any other concerns about this patch set? If not, I think
> > > Vinod can apply this patch set. Thanks.
> >
> > Sorry for the late reply. Yes, this makes sense since there are no
> > existing users then. For the DT changes going through the dmaengine
> > tree
> >
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
>
> Thanks for your reviewing.
>
> >
> > One more question, to make sure we don't need to edit it again:
> > Why do you need both a 'channel id' and a 'slave id' here? Is this
> > a strict hardware requirement for your DMA engine? In many
> > other designs, only a DMA request line number needs to
> > be described, and I think this would correspond to what you
> > call the 'hardware slave id' in your documentation.
>
> I try to explain why we need the slave id.
>
> For our DMA engine driver, we have software request mode and hardware
> request mode. For software request mode, the DMA engine driver need
> trigger DMA to start transfer manually. But for hardware request mode,
> we just set one unique slave id corresponding to the slave hardware to
> the DMA engine, then the slave hardware can trigger DMA automatically.
> And the slave id is not always same with the channel id according to
> the SoC design, so we add one cell to specify the slave id.
I did understand the need for a slave-id, I was instead wondering about
the channel-id number. On many SoCs, all channels are equal, and you
just have to pick one of those with the right capabilities for a particular
slave.
Arnd
^ permalink raw reply
* [1/1] dt-bindings: dmaengine: Add MediaTek Command-Queue DMA controller bindings
From: Rob Herring @ 2019-02-18 14:47 UTC (permalink / raw)
To: shun-chih.yu
Cc: Sean Wang, Vinod Koul, Matthias Brugger, Dan Williams, dmaengine,
linux-arm-kernel, linux-mediatek, devicetree, linux-kernel,
srv_wsdupstream
On Thu, Feb 14, 2019 at 03:40:58PM +0800, shun-chih.yu@mediatek.com wrote:
> From: Shun-Chih Yu <shun-chih.yu@mediatek.com>
>
> Document the devicetree bindings for MediaTek Command-Queue DMA controller
> which could be found on MT6765 SoC or other similar Mediatek SoCs.
>
> Change-Id: I9736c8cac9be160358feeab935fabaffc5730519
Drop this.
> Signed-off-by: Shun-Chih Yu <shun-chih.yu@mediatek.com>
> ---
> .../devicetree/bindings/dma/mtk-cqdma.txt | 31 ++++++++++++++++++++
> 1 file changed, 31 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/dma/mtk-cqdma.txt
>
> diff --git a/Documentation/devicetree/bindings/dma/mtk-cqdma.txt b/Documentation/devicetree/bindings/dma/mtk-cqdma.txt
> new file mode 100644
> index 0000000..fb12927
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dma/mtk-cqdma.txt
> @@ -0,0 +1,31 @@
> +MediaTek Command-Queue DMA Controller
> +==================================
> +
> +Required properties:
> +
> +- compatible: Must be "mediatek,mt6765-cqdma" for MT6765.
> +- reg: Should contain the base address and length for each channel.
> +- interrupts: Should contain references to the interrupts for each channel.
> +- clocks: Should be the clock specifiers corresponding to the entry in
> + clock-names property.
> +- clock-names: Should contain "cqdma" entries.
> +- dma-channels: The number of DMA channels supported by the controller.
What's the range of valid values?
> +- dma-requests: The number of DMA request supported by the controller.
What's the range of valid values?
> +- #dma-cells: The length of the DMA specifier, must be <1>. This one cell
> + in dmas property of a client device represents the channel
> + number.
> +Example:
> +
> + cqdma: dma-controller@10212000 {
> + compatible = "mediatek,mt6765-cqdma";
> + reg = <0 0x10212000 0 0x1000>;
Should be 2 entries here since there are 2 channels? Or the description
above is wrong.
> + interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>,
> + <GIC_SPI 114 IRQ_TYPE_LEVEL_LOW>;
> + clocks = <&infracfg CLK_IFR_CQ_DMA>;
> + clock-names = "cqdma";
> + dma-channels = <2>;
> + dma-requests = <32>;
> + #dma-cells = <1>;
> + };
> +
> +DMA clients must use the format described in dma/dma.txt file.
> --
> 1.7.9.5
>
^ permalink raw reply
* [RFC,v5,1/6] dmaengine: Add Synopsys eDMA IP core driver
From: Gustavo Pimentel @ 2019-02-18 15:44 UTC (permalink / raw)
To: Andy Shevchenko, Gustavo Pimentel
Cc: linux-pci@vger.kernel.org, dmaengine@vger.kernel.org, Vinod Koul,
Dan Williams, Eugeniy Paltsev, Russell King, Niklas Cassel,
Joao Pinto, Jose Abreu, Luis Oliveira, Vitor Soares, Nelson Costa,
Pedro Sousa
Hi Andy,
On 11/02/2019 18:49, Andy Shevchenko wrote:
> On Mon, Feb 11, 2019 at 06:34:30PM +0100, Gustavo Pimentel wrote:
>> Add Synopsys PCIe Endpoint eDMA IP core driver to kernel.
>>
>> This IP is generally distributed with Synopsys PCIe Endpoint IP (depends
>> of the use and licensing agreement).
>>
>> This core driver, initializes and configures the eDMA IP using vma-helpers
>> functions and dma-engine subsystem.
>>
>> This driver can be compile as built-in or external module in kernel.
>>
>> To enable this driver just select DW_EDMA option in kernel configuration,
>> however it requires and selects automatically DMA_ENGINE and
>> DMA_VIRTUAL_CHANNELS option too.
>
> First comment here is that: try to shrink your code base by let's say 15%.
> I believe something here is done is suboptimal way or doesn't take into
> consideration existing facilities in the kernel.
Ok, it's quite possible. Based on this and other code revisions I've discover
some helpers that I didn't know or hasn't aware/didn't see in previous driver
implementations.
Those helpers keep the code much cleaner and readable, thanks to all for show me
them!
>
>> +static void dw_edma_free_chunk(struct dw_edma_desc *desc)
>> +{
>> + struct dw_edma_chan *chan = desc->chan;
>> + struct dw_edma_chunk *child, *_next;
>> +
>
>> + if (!desc->chunk)
>> + return;
>
> Is it necessary check? Why?
It was just a pointer validation fail safe. I can remove it.
>
>> +
>> + /* Remove all the list elements */
>> + list_for_each_entry_safe(child, _next, &desc->chunk->list, list) {
>> + dw_edma_free_burst(child);
>> + if (child->bursts_alloc)
>> + dev_dbg(chan2dev(chan), "%u bursts still allocated\n",
>> + child->bursts_alloc);
>> + list_del(&child->list);
>> + kfree(child);
>> + desc->chunks_alloc--;
>> + }
>> +
>> + /* Remove the list head */
>> + kfree(child);
>> + desc->chunk = NULL;
>> +}
>
>> +static int dw_edma_device_config(struct dma_chan *dchan,
>> + struct dma_slave_config *config)
>> +{
>> + struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
>> + unsigned long flags;
>> + int err = 0;
>> +
>> + spin_lock_irqsave(&chan->vc.lock, flags);
>> +
>
>> + if (!config) {
>> + err = -EINVAL;
>> + goto err_config;
>> + }
>
> Is it necessary check? Why?
It was just a pointer validation fail safe (old habit from user space tools,
trying to protect to all costs any misuse from the user). I can remove it, no
problem.
>
> Why this is under spin lock?
>
>> + dev_dbg(chan2dev(chan), "addr(physical) src=%pa, dst=%pa\n",
>> + &config->src_addr, &config->dst_addr);
>
> And this.
>
> What do you protect by locks? Check all your locking carefully.
In this case, I was trying to protect the config data and configure variable,
since this information is manipulated here and by dw_edma_device_transfer(),
which is called by dw_edma_device_prep_slave_sg() and/or
dw_edma_device_prep_dma_cyclic()
But you comment was about the debug print itself or about the config
data/configured variable?
>
>> +
>> + chan->src_addr = config->src_addr;
>> + chan->dst_addr = config->dst_addr;
>> +
>> + chan->configured = true;
>> + dev_dbg(chan2dev(chan), "channel configured\n");
>> +
>> +err_config:
>> + spin_unlock_irqrestore(&chan->vc.lock, flags);
>> + return err;
>> +}
>> +
>> +static int dw_edma_device_pause(struct dma_chan *dchan)
>> +{
>> + struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
>> + unsigned long flags;
>> + int err = 0;
>> +
>> + spin_lock_irqsave(&chan->vc.lock, flags);
>> +
>> + if (!chan->configured) {
>
>> + dev_err(chan2dev(chan), "(pause) channel not configured\n");
>
> Why this is under spinlock?
The goal was to protect the configured, status and request variables. Once again
your comment was about the variables itself or about the debug prints?
>
>> + err = -EPERM;
>> + goto err_pause;
>> + }
>> +
>> + if (chan->status != EDMA_ST_BUSY) {
>> + err = -EPERM;
>> + goto err_pause;
>> + }
>> +
>> + if (chan->request != EDMA_REQ_NONE) {
>> + err = -EPERM;
>> + goto err_pause;
>> + }
>> +
>> + chan->request = EDMA_REQ_PAUSE;
>
>> + dev_dbg(chan2dev(chan), "pause requested\n");
>
> Ditto.
>
> Moreover, check what functional tracer can provide you for debugging.
I saw in dw-axi-dmac driver the use of dev_dbg() instead of dev_dbg(), that is
what you recommend (besides of removing some debug prints)?
>
>> +
>> +err_pause:
>> + spin_unlock_irqrestore(&chan->vc.lock, flags);
>> + return err;
>> +}
>
>> +{
>> + struct dw_edma_chan *chan = dchan2dw_edma_chan(dchan);
>> + struct dw_edma_desc *desc;
>> + struct virt_dma_desc *vd;
>> + unsigned long flags;
>> + enum dma_status ret;
>> + u32 residue = 0;
>> +
>> + ret = dma_cookie_status(dchan, cookie, txstate);
>> + if (ret == DMA_COMPLETE)
>> + return ret;
>> +
>
>> + if (!txstate)
>> + return ret;
>
> So, if there is no txstate, you can't return PAUSED state? Why?
I misunderstood Vinod's comment "this looks better, please do keep in mind
txstate can be null, so residue can can be skipped"
The txstate variable should be verified after checking if the channel is on
PAUSE state.
>
>> +
>> + spin_lock_irqsave(&chan->vc.lock, flags);
>> +
>> + if (ret == DMA_IN_PROGRESS && chan->status == EDMA_ST_PAUSE)
>> + ret = DMA_PAUSED;
if (!txstate)
goto ret_status;
Sounds good?
>> +
>> + vd = vchan_find_desc(&chan->vc, cookie);
>> + if (!vd)
>> + goto ret_status;
>> +
>> + desc = vd2dw_edma_desc(vd);
>> + if (desc)
>> + residue = desc->alloc_sz - desc->xfer_sz;
>> +
>> +ret_status:
>> + spin_unlock_irqrestore(&chan->vc.lock, flags);
>> + dma_set_residue(txstate, residue);
>> +
>> + return ret;
>> +}
>
>> + for (i = 0; i < cnt; i++) {
>> + if (!xfer->cyclic && !sg)
>> + break;
>> +
>> + if (chunk->bursts_alloc == chan->ll_max) {
>> + chunk = dw_edma_alloc_chunk(desc);
>> + if (unlikely(!chunk))
>> + goto err_alloc;
>> + }
>> +
>> + burst = dw_edma_alloc_burst(chunk);
>> +
>> + if (unlikely(!burst))
>> + goto err_alloc;
>> +
>> + if (xfer->cyclic)
>> + burst->sz = xfer->xfer.cyclic.len;
>> + else
>> + burst->sz = sg_dma_len(sg);
>> +
>> + chunk->ll_region.sz += burst->sz;
>> + desc->alloc_sz += burst->sz;
>> +
>> + if (direction == DMA_DEV_TO_MEM) {
>> + burst->sar = src_addr;
>> + if (xfer->cyclic) {
>> + burst->dar = xfer->xfer.cyclic.paddr;
>> + } else {
>> + burst->dar = sg_dma_address(sg);
>> + src_addr += sg_dma_len(sg);
>> + }
>> + } else {
>> + burst->dar = dst_addr;
>> + if (xfer->cyclic) {
>> + burst->sar = xfer->xfer.cyclic.paddr;
>> + } else {
>> + burst->sar = sg_dma_address(sg);
>> + dst_addr += sg_dma_len(sg);
>> + }
>> + }
>> +
>> + dev_dbg(chan2dev(chan), "lli %u/%u, sar=0x%.8llx, dar=0x%.8llx, size=%u bytes\n",
>> + i + 1, cnt, burst->sar, burst->dar, burst->sz);
>> +
>> + if (!xfer->cyclic)
>> + sg = sg_next(sg);
>
> Shouldn't you rather to use for_each_sg()?
I used that helper on the prep_slave_sg(), but now that I have a common function
for both functions prep_slave_sg() and prep_dma_cyclic(). I think IMHO that
helper doesn't make much sense now, but If you have an suggestion I'll be glad
to know it.
>
>> + }
>
>
>> + spin_lock_irqsave(&chan->vc.lock, flags);
>> + vd = vchan_next_desc(&chan->vc);
>> + switch (chan->request) {
>> + case EDMA_REQ_NONE:
>
>> + if (!vd)
>> + break;
>
> Shouldn't be outside of switch?
Yes, it should.
>
>> +
>> + desc = vd2dw_edma_desc(vd);
>> + if (desc->chunks_alloc) {
>> + dev_dbg(chan2dev(chan), "sub-transfer complete\n");
>> + chan->status = EDMA_ST_BUSY;
>> + dev_dbg(chan2dev(chan), "transferred %u bytes\n",
>> + desc->xfer_sz);
>> + dw_edma_start_transfer(chan);
>> + } else {
>> + list_del(&vd->node);
>> + vchan_cookie_complete(vd);
>> + chan->status = EDMA_ST_IDLE;
>> + dev_dbg(chan2dev(chan), "transfer complete\n");
>> + }
>> + break;
>> + case EDMA_REQ_STOP:
>> + if (!vd)
>> + break;
>> +
>> + list_del(&vd->node);
>> + vchan_cookie_complete(vd);
>> + chan->request = EDMA_REQ_NONE;
>> + chan->status = EDMA_ST_IDLE;
>> + dev_dbg(chan2dev(chan), "transfer stop\n");
>> + break;
>> + case EDMA_REQ_PAUSE:
>> + chan->request = EDMA_REQ_NONE;
>> + chan->status = EDMA_ST_PAUSE;
>> + break;
>> + default:
>> + dev_err(chan2dev(chan), "invalid status state\n");
>> + break;
>> + }
>> + spin_unlock_irqrestore(&chan->vc.lock, flags);
>> +}
>
>> +static irqreturn_t dw_edma_interrupt(int irq, void *data, bool write)
>> +{
>> + struct dw_edma_irq *dw_irq = data;
>> + struct dw_edma *dw = dw_irq->dw;
>> + unsigned long total, pos, val;
>> + unsigned long off;
>> + u32 mask;
>> +
>> + if (write) {
>> + total = dw->wr_ch_cnt;
>> + off = 0;
>> + mask = dw_irq->wr_mask;
>> + } else {
>> + total = dw->rd_ch_cnt;
>> + off = dw->wr_ch_cnt;
>> + mask = dw_irq->rd_mask;
>> + }
>> +
>> + pos = 0;
>> + val = dw_edma_v0_core_status_done_int(dw, write ?
>> + EDMA_DIR_WRITE :
>> + EDMA_DIR_READ);
>> + val &= mask;
>> + while ((pos = find_next_bit(&val, total, pos)) != total) {
>
> Is this funny version of for_each_set_bit()?
I wasn't aware of that helper. I'll change the while loop.
>
>> + struct dw_edma_chan *chan = &dw->chan[pos + off];
>> +
>> + dw_edma_done_interrupt(chan);
>> + pos++;
>> + }
>> +
>> + pos = 0;
>> + val = dw_edma_v0_core_status_abort_int(dw, write ?
>> + EDMA_DIR_WRITE :
>> + EDMA_DIR_READ);
>> + val &= mask;
>
>> + while ((pos = find_next_bit(&val, total, pos)) != total) {
>
> Ditto.
I wasn't aware of that helper. I'll change the while loop.
>
>> + struct dw_edma_chan *chan = &dw->chan[pos + off];
>> +
>> + dw_edma_abort_interrupt(chan);
>> + pos++;
>> + }
>> +
>> + return IRQ_HANDLED;
>> +}
>
>> + do {
>> + ret = dw_edma_device_terminate_all(dchan);
>> + if (!ret)
>> + break;
>> +
>> + if (time_after_eq(jiffies, timeout)) {
>> + dev_err(chan2dev(chan), "free timeout\n");
>> + return;
>> + }
>> +
>> + cpu_relax();
>
>> + } while (1);
>
> So, what prevents you to do while (time_before(...)); here?
Once again, I wasn't aware of that helper. I'll change the do while loop.
>
>> +
>> + dev_dbg(dchan2dev(dchan), "channel freed\n");
>> +
>> + pm_runtime_put(chan->chip->dev);
>> +}
>
>> +static int dw_edma_channel_setup(struct dw_edma_chip *chip, bool write,
>> + u32 wr_alloc, u32 rd_alloc)
>> +{
>> + struct dw_edma_region *dt_region;
>> + struct device *dev = chip->dev;
>> + struct dw_edma *dw = chip->dw;
>> + struct dw_edma_chan *chan;
>> + size_t ll_chunk, dt_chunk;
>> + struct dw_edma_irq *irq;
>> + struct dma_device *dma;
>> + u32 i, j, cnt, ch_cnt;
>> + u32 alloc, off_alloc;
>> + int err = 0;
>> + u32 pos;
>> +
>> + ch_cnt = dw->wr_ch_cnt + dw->rd_ch_cnt;
>> + ll_chunk = dw->ll_region.sz;
>> + dt_chunk = dw->dt_region.sz;
>> +
>> + /* Calculate linked list chunk for each channel */
>> + ll_chunk /= roundup_pow_of_two(ch_cnt);
>> +
>> + /* Calculate linked list chunk for each channel */
>> + dt_chunk /= roundup_pow_of_two(ch_cnt);
>
>> + INIT_LIST_HEAD(&dma->channels);
>> +
>> + for (j = 0; (alloc || dw->nr_irqs == 1) && j < cnt; j++, i++) {
>> + chan = &dw->chan[i];
>> +
>> + dt_region = devm_kzalloc(dev, sizeof(*dt_region), GFP_KERNEL);
>> + if (!dt_region)
>> + return -ENOMEM;
>> +
>> + chan->vc.chan.private = dt_region;
>> +
>> + chan->chip = chip;
>> + chan->id = j;
>> + chan->dir = write ? EDMA_DIR_WRITE : EDMA_DIR_READ;
>> + chan->configured = false;
>> + chan->request = EDMA_REQ_NONE;
>> + chan->status = EDMA_ST_IDLE;
>> +
>> + chan->ll_off = (ll_chunk * i);
>> + chan->ll_max = (ll_chunk / EDMA_LL_SZ) - 1;
>> +
>> + chan->dt_off = (dt_chunk * i);
>> +
>> + dev_dbg(dev, "L. List:\tChannel %s[%u] off=0x%.8lx, max_cnt=%u\n",
>> + write ? "write" : "read", j,
>> + chan->ll_off, chan->ll_max);
>> +
>> + if (dw->nr_irqs == 1)
>> + pos = 0;
>> + else
>> + pos = off_alloc + (j % alloc);
>> +
>> + irq = &dw->irq[pos];
>> +
>> + if (write)
>> + irq->wr_mask |= BIT(j);
>> + else
>> + irq->rd_mask |= BIT(j);
>> +
>> + irq->dw = dw;
>> + memcpy(&chan->msi, &irq->msi, sizeof(chan->msi));
>> +
>> + dev_dbg(dev, "MSI:\t\tChannel %s[%u] addr=0x%.8x%.8x, data=0x%.8x\n",
>> + write ? "write" : "read", j,
>> + chan->msi.address_hi, chan->msi.address_lo,
>> + chan->msi.data);
>> +
>> + chan->vc.desc_free = vchan_free_desc;
>> + vchan_init(&chan->vc, dma);
>> +
>> + dt_region->paddr = dw->dt_region.paddr + chan->dt_off;
>> + dt_region->vaddr = dw->dt_region.vaddr + chan->dt_off;
>> + dt_region->sz = dt_chunk;
>> +
>> + dev_dbg(dev, "Data:\tChannel %s[%u] off=0x%.8lx\n",
>> + write ? "write" : "read", j, chan->dt_off);
>> +
>> + dw_edma_v0_core_device_config(chan);
>> + }
>> +
>> + /* Set DMA channel capabilities */
>> + dma_cap_zero(dma->cap_mask);
>> + dma_cap_set(DMA_SLAVE, dma->cap_mask);
>> + dma_cap_set(DMA_CYCLIC, dma->cap_mask);
>
> Doesn't it used for slave transfers?
> DMA_PRIVATE is good to be set for this.
Yep, makes sense.
>
>> + return err;
>> +}
>> +
>> +static inline void dw_edma_dec_irq_alloc(int *nr_irqs, u32 *alloc, u16 cnt)
>> +{
>> + if (*nr_irqs && *alloc < cnt) {
>> + (*alloc)++;
>> + (*nr_irqs)--;
>> + }
>
> I don't see any allocation here.
Bad function and variable naming... Maybe it should be dw_edma_IRQ_spreader or
dw_edma_IRQ_distributor or similar.
I tried to add some intelligence to this driver, in order to be flexible enough
to adapt it to different situations that can happen.
Because who has this IP can configure have the freedom to configure independently:
- the number of DEV_TO_MEM channels
- the number of MEM_TO_DEV channels
- the number of IRQs
this creates a world of possibilities in terms of IRQs assignment to those channels.
In a perfect world, the best case would be to have an unique IRQ assigned to
each channel, however that may not be possible.
Therefore, this algorithm tries to spread evenly the IRQs between the DEV_TO_MEM
and MEM_TO_DEV channels.
Let imagine the following case:
- 3 MEM_TO_DEV channels
- 2 DEV_TO_MEM channels
- 3 IRQs
MEM_TO_DEV channels
- #0 => IRQ #0
- #1 => IRQ #1 (shared between channels #1 and #2)
- #2 => IRQ #1 (shared between channels #1 and #2)
DEV_TO_MEM channels
- #0 => IRQ #2 (shared between channels #0 and #1)
- #1 => IRQ #2 (shared between channels #0 and #1)
or this other case:
- 5 MEM_TO_DEV channels
- 5 DEV_TO_MEM channels
- 4 IRQs
MEM_TO_DEV channels
- #0 => IRQ #0 (shared between channels #0, #1 and #2)
- #1 => IRQ #0 (shared between channels #0, #1 and #2)
- #2 => IRQ #0 (shared between channels #0, #1 and #2)
- #3 => IRQ #1 (shared between channels #3 and #4)
- #4 => IRQ #1 (shared between channels #3 and #4)
DEV_TO_MEM channels
- #0 => IRQ #2 (shared between channels #0, #1 and #2)
- #1 => IRQ #2 (shared between channels #0, #1 and #2)
- #2 => IRQ #2 (shared between channels #0, #1 and #2)
- #3 => IRQ #3 (shared between channels #3 and #4)
- #4 => IRQ #3 (shared between channels #3 and #4)
>
>> +}
>> +
>> +static inline void dw_edma_add_irq_mask(u32 *mask, u32 alloc, u16 cnt)
>> +{
>
>> + while (*mask * alloc < cnt)
>> + (*mask)++;
>
> Do you really need a loop here?
That is the algorithm that I've designed. Based on what I have explain, perhaps
there is something that you know of that already does what I pretend to do.
I've searched for algorithms and linux kernel functions it but the only thing
with similar results is linear programming, but the code implementation of it
seems to be overkill for this.
>
>> +}
>> +
>> +static int dw_edma_irq_request(struct dw_edma_chip *chip,
>> + u32 *wr_alloc, u32 *rd_alloc)
>> +{
>> + struct device *dev = chip->dev;
>> + struct dw_edma *dw = chip->dw;
>> + u32 wr_mask = 1;
>> + u32 rd_mask = 1;
>> + int i, err = 0;
>> + u32 ch_cnt;
>> +
>> + ch_cnt = dw->wr_ch_cnt + dw->rd_ch_cnt;
>> +
>> + if (dw->nr_irqs < 1) {
>> + dev_err(dev, "invalid number of irqs (%u)\n", dw->nr_irqs);
>> + return -EINVAL;
>> + }
>> +
>> + if (dw->nr_irqs == 1) {
>> + /* Common IRQ shared among all channels */
>> + err = request_irq(pci_irq_vector(to_pci_dev(dev), 0),
>> + dw_edma_interrupt_common,
>> + IRQF_SHARED, dw->name, &dw->irq[0]);
>> + if (err) {
>> + dw->nr_irqs = 0;
>> + return err;
>> + }
>> +
>> + get_cached_msi_msg(pci_irq_vector(to_pci_dev(dev), 0),
>> + &dw->irq[0].msi);
>
> Are you going to call each time to pci_irq_vector()? Btw, am I missed pci_irq_alloc()?
In this case this is called only once (dw->nr_irqs == 1).
>
>> + } else {
>
>> + /* Distribute IRQs equally among all channels */
>> + int tmp = dw->nr_irqs;
>
> Is it always achievable?
Yes, base on what I have explained before, it possible to distribute the
available IRQ among the channels which after that we have to share the already
assigned IRQ to the remaining channels in order to give the each channel a way
to generate an interrupt. Never the less this is a best effort algorithm.
>
>> +
>> + while (tmp && (*wr_alloc + *rd_alloc) < ch_cnt) {
>> + dw_edma_dec_irq_alloc(&tmp, wr_alloc, dw->wr_ch_cnt);
>> + dw_edma_dec_irq_alloc(&tmp, rd_alloc, dw->rd_ch_cnt);
>> + }
>> +
>> + dw_edma_add_irq_mask(&wr_mask, *wr_alloc, dw->wr_ch_cnt);
>> + dw_edma_add_irq_mask(&rd_mask, *rd_alloc, dw->rd_ch_cnt);
>> +
>> + for (i = 0; i < (*wr_alloc + *rd_alloc); i++) {
>> + err = request_irq(pci_irq_vector(to_pci_dev(dev), i),
>> + i < *wr_alloc ?
>> + dw_edma_interrupt_write :
>> + dw_edma_interrupt_read,
>> + IRQF_SHARED, dw->name,
>> + &dw->irq[i]);
>> + if (err) {
>> + dw->nr_irqs = i;
>> + return err;
>> + }
>> +
>> + get_cached_msi_msg(pci_irq_vector(to_pci_dev(dev), i),
>> + &dw->irq[i].msi);
>> + }
>> +
>> + dw->nr_irqs = i;
>> + }
>> +
>> + return err;
>> +}
>> +
>> +int dw_edma_probe(struct dw_edma_chip *chip)
>> +{
>> + struct device *dev = chip->dev;
>> + struct dw_edma *dw = chip->dw;
>> + u32 wr_alloc = 0;
>> + u32 rd_alloc = 0;
>> + int i, err;
>> +
>> + raw_spin_lock_init(&dw->lock);
>> +
>> + /* Find out how many write channels are supported by hardware */
>> + dw->wr_ch_cnt = dw_edma_v0_core_ch_count(dw, EDMA_DIR_WRITE);
>> + if (!dw->wr_ch_cnt) {
>> + dev_err(dev, "invalid number of write channels(0)\n");
>> + return -EINVAL;
>> + }
>> +
>> + /* Find out how many read channels are supported by hardware */
>> + dw->rd_ch_cnt = dw_edma_v0_core_ch_count(dw, EDMA_DIR_READ);
>> + if (!dw->rd_ch_cnt) {
>> + dev_err(dev, "invalid number of read channels(0)\n");
>> + return -EINVAL;
>> + }
>> +
>> + dev_dbg(dev, "Channels:\twrite=%d, read=%d\n",
>> + dw->wr_ch_cnt, dw->rd_ch_cnt);
>> +
>> + /* Allocate channels */
>> + dw->chan = devm_kcalloc(dev, dw->wr_ch_cnt + dw->rd_ch_cnt,
>> + sizeof(*dw->chan), GFP_KERNEL);
>> + if (!dw->chan)
>> + return -ENOMEM;
>> +
>> + snprintf(dw->name, sizeof(dw->name), "dw-edma-core:%d", chip->id);
>> +
>> + /* Disable eDMA, only to establish the ideal initial conditions */
>> + dw_edma_v0_core_off(dw);
>> +
>> + /* Request IRQs */
>> + err = dw_edma_irq_request(chip, &wr_alloc, &rd_alloc);
>> + if (err)
>> + return err;
>> +
>
>> + /* Setup write channels */
>> + err = dw_edma_channel_setup(chip, true, wr_alloc, rd_alloc);
>> + if (err)
>> + goto err_irq_free;
>> +
>> + /* Setup read channels */
>> + err = dw_edma_channel_setup(chip, false, wr_alloc, rd_alloc);
>> + if (err)
>> + goto err_irq_free;
>
> I think you may look into ep93xx driver to see how different type of channels
> are allocated and be set up.
I took a look at this driver, but I'm not seeing what I can copy to improve my
driver, it seems to behave differently from mine. For starters, they do not
distinguish different channels like WRITE / READ, as I have to do on mine, but I
might seen it wrongly.
>
>> +
>> + /* Power management */
>> + pm_runtime_enable(dev);
>> +
>> + /* Turn debugfs on */
>> + err = dw_edma_v0_core_debugfs_on(chip);
>> + if (err) {
>> + dev_err(dev, "unable to create debugfs structure\n");
>> + goto err_pm_disable;
>> + }
>> +
>> + return 0;
>> +
>> +err_pm_disable:
>> + pm_runtime_disable(dev);
>> +err_irq_free:
>> + for (i = (dw->nr_irqs - 1); i >= 0; i--)
>> + free_irq(pci_irq_vector(to_pci_dev(dev), i), &dw->irq[i]);
>> +
>> + dw->nr_irqs = 0;
>> +
>> + return err;
>> +}
>> +EXPORT_SYMBOL_GPL(dw_edma_probe);
>
>> +/**
>> + * struct dw_edma_chip - representation of DesignWare eDMA controller hardware
>> + * @dev: struct device of the eDMA controller
>> + * @id: instance ID
>> + * @irq: irq line
>> + * @dw: struct dw_edma that is filed by dw_edma_probe()
>> + */
>> +struct dw_edma_chip {
>> + struct device *dev;
>> + int id;
>> + int irq;
>> + struct dw_edma *dw;
>> +};
>> +
>> +/* Export to the platform drivers */
>> +#if IS_ENABLED(CONFIG_DW_EDMA)
>> +int dw_edma_probe(struct dw_edma_chip *chip);
>> +int dw_edma_remove(struct dw_edma_chip *chip);
>> +#else
>> +static inline int dw_edma_probe(struct dw_edma_chip *chip)
>> +{
>> + return -ENODEV;
>> +}
>> +
>> +static inline int dw_edma_remove(struct dw_edma_chip *chip)
>> +{
>> + return 0;
>> +}
>> +#endif /* CONFIG_DW_EDMA */
>
> Is it going to be used as a library?
My goal is to provide this code as a driver to be use by all customers that have
this IP, avoiding duplicated code and re-inventing the wheel over and over. That
way, the customers can focus their energy on writing the main driver code (which
can be USB EP, Ethernet EP, whatever) and implementing on their side the access
to this driver by looking the reference PCIe glue-logic driver that I'm
providing. I don't know if this as clarified your question or not.
>
Once again thanks for your feedback Andy!
^ permalink raw reply
* dmaengine: mv_xor: Use correct device for DMA API
From: Robin Murphy @ 2019-02-18 18:27 UTC (permalink / raw)
To: vkoul; +Cc: dmaengine, linux-arm-kernel, hch, John David Anglin
Using dma_dev->dev for mappings before it's assigned with the correct
device is unlikely to work as expected, and with future dma-direct
changes, passing a NULL device may end up crashing entirely. I don't
know enough about this hardware or the mv_xor_prep_dma_interrupt()
operation to implement the appropriate error-handling logic that would
have revealed those dma_map_single() calls failing on arm64 for as long
as the driver has been enabled there, but moving the assignment earlier
will at least make the current code operate as intended.
Fixes: 22843545b200 ("dma: mv_xor: Add support for DMA_INTERRUPT")
Reported-by: John David Anglin <dave.anglin@bell.net>
Tested-by: John David Anglin <dave.anglin@bell.net>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
drivers/dma/mv_xor.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/mv_xor.c b/drivers/dma/mv_xor.c
index 7f595355fb79..fe4a7c71fede 100644
--- a/drivers/dma/mv_xor.c
+++ b/drivers/dma/mv_xor.c
@@ -1059,6 +1059,7 @@ mv_xor_channel_add(struct mv_xor_device *xordev,
mv_chan->op_in_desc = XOR_MODE_IN_DESC;
dma_dev = &mv_chan->dmadev;
+ dma_dev->dev = &pdev->dev;
mv_chan->xordev = xordev;
/*
@@ -1091,7 +1092,6 @@ mv_xor_channel_add(struct mv_xor_device *xordev,
dma_dev->device_free_chan_resources = mv_xor_free_chan_resources;
dma_dev->device_tx_status = mv_xor_status;
dma_dev->device_issue_pending = mv_xor_issue_pending;
- dma_dev->dev = &pdev->dev;
/* set prep routines based on capability */
if (dma_has_cap(DMA_INTERRUPT, dma_dev->cap_mask))
^ permalink raw reply related
* [1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id
From: Baolin Wang @ 2019-02-19 3:14 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Vinod Koul, Rob Herring, Mark Rutland, Olof Johansson, Orson Zhai,
Lyra Zhang, Dan Williams, DTML, arm-soc, Linux ARM,
Linux Kernel Mailing List, dmaengine, eric.long, Mark Brown
On Mon, 18 Feb 2019 at 20:23, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Mon, Feb 18, 2019 at 11:52 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> > On Mon, 18 Feb 2019 at 18:31, Arnd Bergmann <arnd@arndb.de> wrote:
> > >
> > > On Tue, Feb 12, 2019 at 9:25 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > On Fri, 1 Feb 2019 at 19:53, Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > > On Thu, 31 Jan 2019 at 00:52, Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > > On Tue, Jan 22, 2019 at 2:21 PM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > > > >
> > > > > > > Client:
> > > > > > > DMA clients connected to the Spreadtrum DMA controller must use the format
> > > > > > > -described in the dma.txt file, using a two-cell specifier for each channel.
> > > > > > > -The two cells in order are:
> > > > > > > +described in the dma.txt file, using a three-cell specifier for each channel.
> > > > > > > +The three cells in order are:
> > > > > > > 1. A phandle pointing to the DMA controller.
> > > > > > > 2. The channel id.
> > > > > > > +3. The hardware slave id which is used for clients to trigger DMA engine
> > > > > > > +automatically.
> > > > > >
> > > > > > I notice that this is an incompatible binding change. Is that necessary?
> > > > > > If the current code works, I'd suggest allowing both #dma-cells=<2>
> > > > > > and <3>, and then implementing both in the driver.
> > > > >
> > > > > Yes, this is necessary.
> > > > >
> > > > > Yes, current code can work, but the problem is that the DMA clients
> > > > > must add one property (something like "sprd,slave-id") to specify the
> > > > > slave id. So considering this, we want to change the dma-cells to 2,
> > > > > including dma channel and dma slave id, which can avoid introducing
> > > > > some similar properties for DMA clients.
> > > > >
> > > > > Now there are no DMA clients in mainline for Spreadtrum platform, and
> > > > > we want to upstream our first DMA clients: SPI controller. So no other
> > > > > drivers need to change when we changing dma cells. Thanks.
> > > >
> > > > Do you have any other concerns about this patch set? If not, I think
> > > > Vinod can apply this patch set. Thanks.
> > >
> > > Sorry for the late reply. Yes, this makes sense since there are no
> > > existing users then. For the DT changes going through the dmaengine
> > > tree
> > >
> > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> >
> > Thanks for your reviewing.
> >
> > >
> > > One more question, to make sure we don't need to edit it again:
> > > Why do you need both a 'channel id' and a 'slave id' here? Is this
> > > a strict hardware requirement for your DMA engine? In many
> > > other designs, only a DMA request line number needs to
> > > be described, and I think this would correspond to what you
> > > call the 'hardware slave id' in your documentation.
> >
> > I try to explain why we need the slave id.
> >
> > For our DMA engine driver, we have software request mode and hardware
> > request mode. For software request mode, the DMA engine driver need
> > trigger DMA to start transfer manually. But for hardware request mode,
> > we just set one unique slave id corresponding to the slave hardware to
> > the DMA engine, then the slave hardware can trigger DMA automatically.
> > And the slave id is not always same with the channel id according to
> > the SoC design, so we add one cell to specify the slave id.
>
> I did understand the need for a slave-id, I was instead wondering about
> the channel-id number. On many SoCs, all channels are equal, and you
> just have to pick one of those with the right capabilities for a particular
> slave.
Yes, all channels are equal. We just set a unique slave id for the
channel for a particular slave. For example, the SPI slave can use
channel 10 for tx transfer by setting slave id 11, or it also can use
channel 9 for tx transfer by setting same slave id 11.
^ permalink raw reply
* [1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id
From: Geert Uytterhoeven @ 2019-02-19 9:30 UTC (permalink / raw)
To: Baolin Wang
Cc: Arnd Bergmann, Vinod Koul, Rob Herring, Mark Rutland,
Olof Johansson, Orson Zhai, Lyra Zhang, Dan Williams, DTML,
arm-soc, Linux ARM, Linux Kernel Mailing List, dmaengine,
eric.long, Mark Brown
Hi Baolin,
On Tue, Feb 19, 2019 at 4:15 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> On Mon, 18 Feb 2019 at 20:23, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Mon, Feb 18, 2019 at 11:52 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > On Mon, 18 Feb 2019 at 18:31, Arnd Bergmann <arnd@arndb.de> wrote:
> > > >
> > > > On Tue, Feb 12, 2019 at 9:25 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > > On Fri, 1 Feb 2019 at 19:53, Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > > > On Thu, 31 Jan 2019 at 00:52, Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > > > On Tue, Jan 22, 2019 at 2:21 PM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > > > > >
> > > > > > > > Client:
> > > > > > > > DMA clients connected to the Spreadtrum DMA controller must use the format
> > > > > > > > -described in the dma.txt file, using a two-cell specifier for each channel.
> > > > > > > > -The two cells in order are:
> > > > > > > > +described in the dma.txt file, using a three-cell specifier for each channel.
> > > > > > > > +The three cells in order are:
> > > > > > > > 1. A phandle pointing to the DMA controller.
> > > > > > > > 2. The channel id.
> > > > > > > > +3. The hardware slave id which is used for clients to trigger DMA engine
> > > > > > > > +automatically.
> > > > > > >
> > > > > > > I notice that this is an incompatible binding change. Is that necessary?
> > > > > > > If the current code works, I'd suggest allowing both #dma-cells=<2>
> > > > > > > and <3>, and then implementing both in the driver.
> > > > > >
> > > > > > Yes, this is necessary.
> > > > > >
> > > > > > Yes, current code can work, but the problem is that the DMA clients
> > > > > > must add one property (something like "sprd,slave-id") to specify the
> > > > > > slave id. So considering this, we want to change the dma-cells to 2,
> > > > > > including dma channel and dma slave id, which can avoid introducing
> > > > > > some similar properties for DMA clients.
> > > > > >
> > > > > > Now there are no DMA clients in mainline for Spreadtrum platform, and
> > > > > > we want to upstream our first DMA clients: SPI controller. So no other
> > > > > > drivers need to change when we changing dma cells. Thanks.
> > > > >
> > > > > Do you have any other concerns about this patch set? If not, I think
> > > > > Vinod can apply this patch set. Thanks.
> > > >
> > > > Sorry for the late reply. Yes, this makes sense since there are no
> > > > existing users then. For the DT changes going through the dmaengine
> > > > tree
> > > >
> > > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > >
> > > Thanks for your reviewing.
> > >
> > > >
> > > > One more question, to make sure we don't need to edit it again:
> > > > Why do you need both a 'channel id' and a 'slave id' here? Is this
> > > > a strict hardware requirement for your DMA engine? In many
> > > > other designs, only a DMA request line number needs to
> > > > be described, and I think this would correspond to what you
> > > > call the 'hardware slave id' in your documentation.
> > >
> > > I try to explain why we need the slave id.
> > >
> > > For our DMA engine driver, we have software request mode and hardware
> > > request mode. For software request mode, the DMA engine driver need
> > > trigger DMA to start transfer manually. But for hardware request mode,
> > > we just set one unique slave id corresponding to the slave hardware to
> > > the DMA engine, then the slave hardware can trigger DMA automatically.
> > > And the slave id is not always same with the channel id according to
> > > the SoC design, so we add one cell to specify the slave id.
> >
> > I did understand the need for a slave-id, I was instead wondering about
> > the channel-id number. On many SoCs, all channels are equal, and you
> > just have to pick one of those with the right capabilities for a particular
> > slave.
>
> Yes, all channels are equal. We just set a unique slave id for the
> channel for a particular slave. For example, the SPI slave can use
> channel 10 for tx transfer by setting slave id 11, or it also can use
> channel 9 for tx transfer by setting same slave id 11.
So the channel selection is software policy, not hardware description, and
thus doesn't belong in DT?
Can't the DMA engine driver allocate channels dynamically, removing the
need to specify this in DT?
Thanks!
Gr{oetje,eeting}s,
Geert
^ permalink raw reply
* [1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id
From: Baolin Wang @ 2019-02-19 9:49 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Arnd Bergmann, Vinod Koul, Rob Herring, Mark Rutland,
Olof Johansson, Orson Zhai, Lyra Zhang, Dan Williams, DTML,
arm-soc, Linux ARM, Linux Kernel Mailing List, dmaengine,
eric.long, Mark Brown
Hi Geert,
On Tue, 19 Feb 2019 at 17:30, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Baolin,
>
> On Tue, Feb 19, 2019 at 4:15 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> > On Mon, 18 Feb 2019 at 20:23, Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Mon, Feb 18, 2019 at 11:52 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > On Mon, 18 Feb 2019 at 18:31, Arnd Bergmann <arnd@arndb.de> wrote:
> > > > >
> > > > > On Tue, Feb 12, 2019 at 9:25 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > > > On Fri, 1 Feb 2019 at 19:53, Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > > > > On Thu, 31 Jan 2019 at 00:52, Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > > > > On Tue, Jan 22, 2019 at 2:21 PM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > > > > > >
> > > > > > > > > Client:
> > > > > > > > > DMA clients connected to the Spreadtrum DMA controller must use the format
> > > > > > > > > -described in the dma.txt file, using a two-cell specifier for each channel.
> > > > > > > > > -The two cells in order are:
> > > > > > > > > +described in the dma.txt file, using a three-cell specifier for each channel.
> > > > > > > > > +The three cells in order are:
> > > > > > > > > 1. A phandle pointing to the DMA controller.
> > > > > > > > > 2. The channel id.
> > > > > > > > > +3. The hardware slave id which is used for clients to trigger DMA engine
> > > > > > > > > +automatically.
> > > > > > > >
> > > > > > > > I notice that this is an incompatible binding change. Is that necessary?
> > > > > > > > If the current code works, I'd suggest allowing both #dma-cells=<2>
> > > > > > > > and <3>, and then implementing both in the driver.
> > > > > > >
> > > > > > > Yes, this is necessary.
> > > > > > >
> > > > > > > Yes, current code can work, but the problem is that the DMA clients
> > > > > > > must add one property (something like "sprd,slave-id") to specify the
> > > > > > > slave id. So considering this, we want to change the dma-cells to 2,
> > > > > > > including dma channel and dma slave id, which can avoid introducing
> > > > > > > some similar properties for DMA clients.
> > > > > > >
> > > > > > > Now there are no DMA clients in mainline for Spreadtrum platform, and
> > > > > > > we want to upstream our first DMA clients: SPI controller. So no other
> > > > > > > drivers need to change when we changing dma cells. Thanks.
> > > > > >
> > > > > > Do you have any other concerns about this patch set? If not, I think
> > > > > > Vinod can apply this patch set. Thanks.
> > > > >
> > > > > Sorry for the late reply. Yes, this makes sense since there are no
> > > > > existing users then. For the DT changes going through the dmaengine
> > > > > tree
> > > > >
> > > > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > >
> > > > Thanks for your reviewing.
> > > >
> > > > >
> > > > > One more question, to make sure we don't need to edit it again:
> > > > > Why do you need both a 'channel id' and a 'slave id' here? Is this
> > > > > a strict hardware requirement for your DMA engine? In many
> > > > > other designs, only a DMA request line number needs to
> > > > > be described, and I think this would correspond to what you
> > > > > call the 'hardware slave id' in your documentation.
> > > >
> > > > I try to explain why we need the slave id.
> > > >
> > > > For our DMA engine driver, we have software request mode and hardware
> > > > request mode. For software request mode, the DMA engine driver need
> > > > trigger DMA to start transfer manually. But for hardware request mode,
> > > > we just set one unique slave id corresponding to the slave hardware to
> > > > the DMA engine, then the slave hardware can trigger DMA automatically.
> > > > And the slave id is not always same with the channel id according to
> > > > the SoC design, so we add one cell to specify the slave id.
> > >
> > > I did understand the need for a slave-id, I was instead wondering about
> > > the channel-id number. On many SoCs, all channels are equal, and you
> > > just have to pick one of those with the right capabilities for a particular
> > > slave.
> >
> > Yes, all channels are equal. We just set a unique slave id for the
> > channel for a particular slave. For example, the SPI slave can use
> > channel 10 for tx transfer by setting slave id 11, or it also can use
> > channel 9 for tx transfer by setting same slave id 11.
>
> So the channel selection is software policy, not hardware description, and
> thus doesn't belong in DT?
>
> Can't the DMA engine driver allocate channels dynamically, removing the
> need to specify this in DT?
In theory we can do as you suggested. But we still want to
manage/assign the DMA channel resources manually for one SoC, we can
make sure some important DMA slaves (such as audio) can request a DMA
channel at runtime firstly, another benefit is that it is easy to
debug since we can easily know which channel is assigned for this
slave.
^ permalink raw reply
* dmaengine: mv_xor: Use correct device for DMA API
From: Thomas Petazzoni @ 2019-02-19 11:02 UTC (permalink / raw)
To: Robin Murphy; +Cc: vkoul, dmaengine, John David Anglin, hch, linux-arm-kernel
Hello Robin,
Thanks a lot for fixing this!
On Mon, 18 Feb 2019 18:27:06 +0000
Robin Murphy <robin.murphy@arm.com> wrote:
> Using dma_dev->dev for mappings before it's assigned with the correct
> device is unlikely to work as expected, and with future dma-direct
> changes, passing a NULL device may end up crashing entirely. I don't
> know enough about this hardware or the mv_xor_prep_dma_interrupt()
> operation to implement the appropriate error-handling logic that would
> have revealed those dma_map_single() calls failing on arm64 for as long
> as the driver has been enabled there, but moving the assignment earlier
> will at least make the current code operate as intended.
>
> Fixes: 22843545b200 ("dma: mv_xor: Add support for DMA_INTERRUPT")
> Reported-by: John David Anglin <dave.anglin@bell.net>
> Tested-by: John David Anglin <dave.anglin@bell.net>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
Acked-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Tested-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Best regards,
Thomas
^ permalink raw reply
* dmaengine: mv_xor: Use correct device for DMA API
From: Vinod Koul @ 2019-02-19 12:10 UTC (permalink / raw)
To: Robin Murphy; +Cc: dmaengine, linux-arm-kernel, hch, John David Anglin
On 18-02-19, 18:27, Robin Murphy wrote:
> Using dma_dev->dev for mappings before it's assigned with the correct
> device is unlikely to work as expected, and with future dma-direct
> changes, passing a NULL device may end up crashing entirely. I don't
> know enough about this hardware or the mv_xor_prep_dma_interrupt()
> operation to implement the appropriate error-handling logic that would
> have revealed those dma_map_single() calls failing on arm64 for as long
> as the driver has been enabled there, but moving the assignment earlier
> will at least make the current code operate as intended.
Applied, thanks
^ permalink raw reply
* [v10,1/3] dmaengine: 8250_mtk_dma: add MediaTek uart DMA support
From: Vinod Koul @ 2019-02-19 12:14 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 14-02-19, 10:44, Long Cheng wrote:
> On Mon, 2019-02-04 at 12:51 +0530, Vinod Koul wrote:
> > > +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
>
> Can't remove it. Before the mail, i had explained it. in 8250 uart
> framework, will check the function..
I think you should fix the 8250 uart to handle driver not having a
pause. Keeping a dummy pause is just wrong and sets wrong expectations
for uart..
^ permalink raw reply
* [1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id
From: Vinod Koul @ 2019-02-19 12:20 UTC (permalink / raw)
To: Baolin Wang
Cc: Geert Uytterhoeven, Arnd Bergmann, Rob Herring, Mark Rutland,
Olof Johansson, Orson Zhai, Lyra Zhang, Dan Williams, DTML,
arm-soc, Linux ARM, Linux Kernel Mailing List, dmaengine,
eric.long, Mark Brown
On 19-02-19, 17:49, Baolin Wang wrote:
> Hi Geert,
>
> On Tue, 19 Feb 2019 at 17:30, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> >
> > Hi Baolin,
> >
> > On Tue, Feb 19, 2019 at 4:15 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > On Mon, 18 Feb 2019 at 20:23, Arnd Bergmann <arnd@arndb.de> wrote:
> > > > On Mon, Feb 18, 2019 at 11:52 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > > On Mon, 18 Feb 2019 at 18:31, Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > >
> > > > > > On Tue, Feb 12, 2019 at 9:25 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > > > > On Fri, 1 Feb 2019 at 19:53, Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > > > > > On Thu, 31 Jan 2019 at 00:52, Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > > > > > On Tue, Jan 22, 2019 at 2:21 PM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > > > > > > >
> > > > > > > > > > Client:
> > > > > > > > > > DMA clients connected to the Spreadtrum DMA controller must use the format
> > > > > > > > > > -described in the dma.txt file, using a two-cell specifier for each channel.
> > > > > > > > > > -The two cells in order are:
> > > > > > > > > > +described in the dma.txt file, using a three-cell specifier for each channel.
> > > > > > > > > > +The three cells in order are:
> > > > > > > > > > 1. A phandle pointing to the DMA controller.
> > > > > > > > > > 2. The channel id.
> > > > > > > > > > +3. The hardware slave id which is used for clients to trigger DMA engine
> > > > > > > > > > +automatically.
> > > > > > > > >
> > > > > > > > > I notice that this is an incompatible binding change. Is that necessary?
> > > > > > > > > If the current code works, I'd suggest allowing both #dma-cells=<2>
> > > > > > > > > and <3>, and then implementing both in the driver.
> > > > > > > >
> > > > > > > > Yes, this is necessary.
> > > > > > > >
> > > > > > > > Yes, current code can work, but the problem is that the DMA clients
> > > > > > > > must add one property (something like "sprd,slave-id") to specify the
> > > > > > > > slave id. So considering this, we want to change the dma-cells to 2,
> > > > > > > > including dma channel and dma slave id, which can avoid introducing
> > > > > > > > some similar properties for DMA clients.
> > > > > > > >
> > > > > > > > Now there are no DMA clients in mainline for Spreadtrum platform, and
> > > > > > > > we want to upstream our first DMA clients: SPI controller. So no other
> > > > > > > > drivers need to change when we changing dma cells. Thanks.
> > > > > > >
> > > > > > > Do you have any other concerns about this patch set? If not, I think
> > > > > > > Vinod can apply this patch set. Thanks.
> > > > > >
> > > > > > Sorry for the late reply. Yes, this makes sense since there are no
> > > > > > existing users then. For the DT changes going through the dmaengine
> > > > > > tree
> > > > > >
> > > > > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > > >
> > > > > Thanks for your reviewing.
> > > > >
> > > > > >
> > > > > > One more question, to make sure we don't need to edit it again:
> > > > > > Why do you need both a 'channel id' and a 'slave id' here? Is this
> > > > > > a strict hardware requirement for your DMA engine? In many
> > > > > > other designs, only a DMA request line number needs to
> > > > > > be described, and I think this would correspond to what you
> > > > > > call the 'hardware slave id' in your documentation.
> > > > >
> > > > > I try to explain why we need the slave id.
> > > > >
> > > > > For our DMA engine driver, we have software request mode and hardware
> > > > > request mode. For software request mode, the DMA engine driver need
> > > > > trigger DMA to start transfer manually. But for hardware request mode,
> > > > > we just set one unique slave id corresponding to the slave hardware to
> > > > > the DMA engine, then the slave hardware can trigger DMA automatically.
> > > > > And the slave id is not always same with the channel id according to
> > > > > the SoC design, so we add one cell to specify the slave id.
> > > >
> > > > I did understand the need for a slave-id, I was instead wondering about
> > > > the channel-id number. On many SoCs, all channels are equal, and you
> > > > just have to pick one of those with the right capabilities for a particular
> > > > slave.
> > >
> > > Yes, all channels are equal. We just set a unique slave id for the
> > > channel for a particular slave. For example, the SPI slave can use
> > > channel 10 for tx transfer by setting slave id 11, or it also can use
> > > channel 9 for tx transfer by setting same slave id 11.
> >
> > So the channel selection is software policy, not hardware description, and
> > thus doesn't belong in DT?
> >
> > Can't the DMA engine driver allocate channels dynamically, removing the
> > need to specify this in DT?
>
> In theory we can do as you suggested. But we still want to
> manage/assign the DMA channel resources manually for one SoC, we can
> make sure some important DMA slaves (such as audio) can request a DMA
> channel at runtime firstly, another benefit is that it is easy to
> debug since we can easily know which channel is assigned for this
> slave.
Are you suggesting that you have more users than channels available?
I dont think it is hard to debug if channels are dynamic in
nature (for example we can print channel number and you know which one
are you talking about, fwiw i have worked on a such a system where we
grabbed the free channel)
^ permalink raw reply
* [1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id
From: Baolin Wang @ 2019-02-20 3:12 UTC (permalink / raw)
To: Vinod Koul
Cc: Geert Uytterhoeven, Arnd Bergmann, Rob Herring, Mark Rutland,
Olof Johansson, Orson Zhai, Lyra Zhang, Dan Williams, DTML,
arm-soc, Linux ARM, Linux Kernel Mailing List, dmaengine,
eric.long, Mark Brown
On Tue, 19 Feb 2019 at 20:20, Vinod Koul <vkoul@kernel.org> wrote:
>
> On 19-02-19, 17:49, Baolin Wang wrote:
> > Hi Geert,
> >
> > On Tue, 19 Feb 2019 at 17:30, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > >
> > > Hi Baolin,
> > >
> > > On Tue, Feb 19, 2019 at 4:15 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > On Mon, 18 Feb 2019 at 20:23, Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > On Mon, Feb 18, 2019 at 11:52 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > > > On Mon, 18 Feb 2019 at 18:31, Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > > >
> > > > > > > On Tue, Feb 12, 2019 at 9:25 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > > > > > On Fri, 1 Feb 2019 at 19:53, Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > > > > > > On Thu, 31 Jan 2019 at 00:52, Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > > > > > > On Tue, Jan 22, 2019 at 2:21 PM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > > > > > > > >
> > > > > > > > > > > Client:
> > > > > > > > > > > DMA clients connected to the Spreadtrum DMA controller must use the format
> > > > > > > > > > > -described in the dma.txt file, using a two-cell specifier for each channel.
> > > > > > > > > > > -The two cells in order are:
> > > > > > > > > > > +described in the dma.txt file, using a three-cell specifier for each channel.
> > > > > > > > > > > +The three cells in order are:
> > > > > > > > > > > 1. A phandle pointing to the DMA controller.
> > > > > > > > > > > 2. The channel id.
> > > > > > > > > > > +3. The hardware slave id which is used for clients to trigger DMA engine
> > > > > > > > > > > +automatically.
> > > > > > > > > >
> > > > > > > > > > I notice that this is an incompatible binding change. Is that necessary?
> > > > > > > > > > If the current code works, I'd suggest allowing both #dma-cells=<2>
> > > > > > > > > > and <3>, and then implementing both in the driver.
> > > > > > > > >
> > > > > > > > > Yes, this is necessary.
> > > > > > > > >
> > > > > > > > > Yes, current code can work, but the problem is that the DMA clients
> > > > > > > > > must add one property (something like "sprd,slave-id") to specify the
> > > > > > > > > slave id. So considering this, we want to change the dma-cells to 2,
> > > > > > > > > including dma channel and dma slave id, which can avoid introducing
> > > > > > > > > some similar properties for DMA clients.
> > > > > > > > >
> > > > > > > > > Now there are no DMA clients in mainline for Spreadtrum platform, and
> > > > > > > > > we want to upstream our first DMA clients: SPI controller. So no other
> > > > > > > > > drivers need to change when we changing dma cells. Thanks.
> > > > > > > >
> > > > > > > > Do you have any other concerns about this patch set? If not, I think
> > > > > > > > Vinod can apply this patch set. Thanks.
> > > > > > >
> > > > > > > Sorry for the late reply. Yes, this makes sense since there are no
> > > > > > > existing users then. For the DT changes going through the dmaengine
> > > > > > > tree
> > > > > > >
> > > > > > > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > > > > >
> > > > > > Thanks for your reviewing.
> > > > > >
> > > > > > >
> > > > > > > One more question, to make sure we don't need to edit it again:
> > > > > > > Why do you need both a 'channel id' and a 'slave id' here? Is this
> > > > > > > a strict hardware requirement for your DMA engine? In many
> > > > > > > other designs, only a DMA request line number needs to
> > > > > > > be described, and I think this would correspond to what you
> > > > > > > call the 'hardware slave id' in your documentation.
> > > > > >
> > > > > > I try to explain why we need the slave id.
> > > > > >
> > > > > > For our DMA engine driver, we have software request mode and hardware
> > > > > > request mode. For software request mode, the DMA engine driver need
> > > > > > trigger DMA to start transfer manually. But for hardware request mode,
> > > > > > we just set one unique slave id corresponding to the slave hardware to
> > > > > > the DMA engine, then the slave hardware can trigger DMA automatically.
> > > > > > And the slave id is not always same with the channel id according to
> > > > > > the SoC design, so we add one cell to specify the slave id.
> > > > >
> > > > > I did understand the need for a slave-id, I was instead wondering about
> > > > > the channel-id number. On many SoCs, all channels are equal, and you
> > > > > just have to pick one of those with the right capabilities for a particular
> > > > > slave.
> > > >
> > > > Yes, all channels are equal. We just set a unique slave id for the
> > > > channel for a particular slave. For example, the SPI slave can use
> > > > channel 10 for tx transfer by setting slave id 11, or it also can use
> > > > channel 9 for tx transfer by setting same slave id 11.
> > >
> > > So the channel selection is software policy, not hardware description, and
> > > thus doesn't belong in DT?
> > >
> > > Can't the DMA engine driver allocate channels dynamically, removing the
> > > need to specify this in DT?
> >
> > In theory we can do as you suggested. But we still want to
> > manage/assign the DMA channel resources manually for one SoC, we can
> > make sure some important DMA slaves (such as audio) can request a DMA
> > channel at runtime firstly, another benefit is that it is easy to
> > debug since we can easily know which channel is assigned for this
> > slave.
>
> Are you suggesting that you have more users than channels available?
Until now we have not met this issue, but we can not make sure if this
can happen in future. Moreover DMA channel resources are belonging to
the DMA engine's hardware resources, I think it should be described in
DT like many other drivers did.
> I dont think it is hard to debug if channels are dynamic in
> nature (for example we can print channel number and you know which one
> are you talking about, fwiw i have worked on a such a system where we
> grabbed the free channel)
>
> --
> ~Vinod
^ permalink raw reply
* [1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id
From: Arnd Bergmann @ 2019-02-20 9:07 UTC (permalink / raw)
To: Baolin Wang
Cc: Vinod Koul, Geert Uytterhoeven, Rob Herring, Mark Rutland,
Olof Johansson, Orson Zhai, Lyra Zhang, Dan Williams, DTML,
arm-soc, Linux ARM, Linux Kernel Mailing List, dmaengine,
eric.long, Mark Brown
On Wed, Feb 20, 2019 at 4:13 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> On Tue, 19 Feb 2019 at 20:20, Vinod Koul <vkoul@kernel.org> wrote:
> > On 19-02-19, 17:49, Baolin Wang wrote:
> > > On Tue, 19 Feb 2019 at 17:30, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > On Tue, Feb 19, 2019 at 4:15 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > > On Mon, 18 Feb 2019 at 20:23, Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > > On Mon, Feb 18, 2019 at 11:52 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > > > > On Mon, 18 Feb 2019 at 18:31, Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > > I did understand the need for a slave-id, I was instead wondering about
> > > > > > the channel-id number. On many SoCs, all channels are equal, and you
> > > > > > just have to pick one of those with the right capabilities for a particular
> > > > > > slave.
> > > > >
> > > > > Yes, all channels are equal. We just set a unique slave id for the
> > > > > channel for a particular slave. For example, the SPI slave can use
> > > > > channel 10 for tx transfer by setting slave id 11, or it also can use
> > > > > channel 9 for tx transfer by setting same slave id 11.
> > > >
> > > > So the channel selection is software policy, not hardware description, and
> > > > thus doesn't belong in DT?
> > > >
> > > > Can't the DMA engine driver allocate channels dynamically, removing the
> > > > need to specify this in DT?
> > >
> > > In theory we can do as you suggested. But we still want to
> > > manage/assign the DMA channel resources manually for one SoC, we can
> > > make sure some important DMA slaves (such as audio) can request a DMA
> > > channel at runtime firstly, another benefit is that it is easy to
> > > debug since we can easily know which channel is assigned for this
> > > slave.
> >
> > Are you suggesting that you have more users than channels available?
>
> Until now we have not met this issue, but we can not make sure if this
> can happen in future. Moreover DMA channel resources are belonging to
> the DMA engine's hardware resources, I think it should be described in
> DT like many other drivers did.
As far as I can tell, most platforms do not describe the assignment
of resources in DT for dma engines, the numbers in there are meant to
describe whatever is fixed, and most platforms should do it that way.
The naming is sometimes a bit confusing, as a dma request line
assignment can be called a slave-id or a channel-id depending whose
documentation you read. The drivers/dma/virt-dma.c implementation
is used in some cases to represent request numbers as virtual
channels, so a dmaengine driver can allow one dma_request_chan()
per slave, and then assign the hardware channels dynamically
while doing a transfer, rather than having a fixed channel assignment
when we first ask for a channel.
Arnd
^ permalink raw reply
* [1/3] dt-bindings: dmaengine: Add one new cell to present hardware slave id
From: Baolin Wang @ 2019-02-20 11:29 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Vinod Koul, Geert Uytterhoeven, Rob Herring, Mark Rutland,
Olof Johansson, Orson Zhai, Lyra Zhang, Dan Williams, DTML,
arm-soc, Linux ARM, Linux Kernel Mailing List, dmaengine,
eric.long, Mark Brown
On Wed, 20 Feb 2019 at 17:08, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Wed, Feb 20, 2019 at 4:13 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> > On Tue, 19 Feb 2019 at 20:20, Vinod Koul <vkoul@kernel.org> wrote:
> > > On 19-02-19, 17:49, Baolin Wang wrote:
> > > > On Tue, 19 Feb 2019 at 17:30, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > > > On Tue, Feb 19, 2019 at 4:15 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > > > On Mon, 18 Feb 2019 at 20:23, Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > > > On Mon, Feb 18, 2019 at 11:52 AM Baolin Wang <baolin.wang@linaro.org> wrote:
> > > > > > > > On Mon, 18 Feb 2019 at 18:31, Arnd Bergmann <arnd@arndb.de> wrote:
> > > > > > > I did understand the need for a slave-id, I was instead wondering about
> > > > > > > the channel-id number. On many SoCs, all channels are equal, and you
> > > > > > > just have to pick one of those with the right capabilities for a particular
> > > > > > > slave.
> > > > > >
> > > > > > Yes, all channels are equal. We just set a unique slave id for the
> > > > > > channel for a particular slave. For example, the SPI slave can use
> > > > > > channel 10 for tx transfer by setting slave id 11, or it also can use
> > > > > > channel 9 for tx transfer by setting same slave id 11.
> > > > >
> > > > > So the channel selection is software policy, not hardware description, and
> > > > > thus doesn't belong in DT?
> > > > >
> > > > > Can't the DMA engine driver allocate channels dynamically, removing the
> > > > > need to specify this in DT?
> > > >
> > > > In theory we can do as you suggested. But we still want to
> > > > manage/assign the DMA channel resources manually for one SoC, we can
> > > > make sure some important DMA slaves (such as audio) can request a DMA
> > > > channel at runtime firstly, another benefit is that it is easy to
> > > > debug since we can easily know which channel is assigned for this
> > > > slave.
> > >
> > > Are you suggesting that you have more users than channels available?
> >
> > Until now we have not met this issue, but we can not make sure if this
> > can happen in future. Moreover DMA channel resources are belonging to
> > the DMA engine's hardware resources, I think it should be described in
> > DT like many other drivers did.
>
> As far as I can tell, most platforms do not describe the assignment
> of resources in DT for dma engines, the numbers in there are meant to
> describe whatever is fixed, and most platforms should do it that way.
>
> The naming is sometimes a bit confusing, as a dma request line
> assignment can be called a slave-id or a channel-id depending whose
> documentation you read. The drivers/dma/virt-dma.c implementation
> is used in some cases to represent request numbers as virtual
> channels, so a dmaengine driver can allow one dma_request_chan()
> per slave, and then assign the hardware channels dynamically
> while doing a transfer, rather than having a fixed channel assignment
> when we first ask for a channel.
Okay, sounds reasonable to me, and I think no issues will happen if we
assign channels dynamically after some slave usages' investigation.
I will remove channel id from DT in next version. Thanks for all your help.
^ permalink raw reply
* [4/6] dma: tegra: add accurate reporting of dma state
From: Dmitry Osipenko @ 2019-02-21 0:41 UTC (permalink / raw)
To: Ben Dooks; +Cc: dan.j.williams, vkoul, ldewangan, dmaengine, linux-tegra
31.10.2018 19:03, Ben Dooks пишет:
> The tx_status callback does not report the state of the transfer
> beyond complete segments. This causes problems with users such as
> ALSA when applications want to know accurately how much data has
> been moved.
>
> This patch addes a function tegra_dma_update_residual() to query
> the hardware and modify the residual information accordinly. It
> takes into account any hardware issues when trying to read the
> state, such as delays between finishing a buffer and signalling
> the interrupt.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
Hello Ben,
Do you have any plans to submit a new version of this patch? It's really useful and fixes a real problem with the audio playback. I could help with finalizing the patch and could submit it for you if you happened to lost the interest.
^ permalink raw reply
* [v2,1/2] dt-bindings: dmaengine: sprd: Change channel id to slave id for DMA cell specifier
From: Baolin Wang @ 2019-02-21 5:34 UTC (permalink / raw)
To: vkoul, robh+dt, mark.rutland, arnd
Cc: orsonzhai, zhang.lyra, dan.j.williams, devicetree, linux-kernel,
dmaengine, eric.long, broonie, baolin.wang
For Spreadtrum DMA engine, all channels are equal, which means slave can
request any channels with setting a unique slave id to trigger this channel.
Thus we can remove the channel id from device tree to assign the channel
dynamically, moreover we should add the slave id in device tree.
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
Changes from v1:
- Remove channel id from DT.
---
Documentation/devicetree/bindings/dma/sprd-dma.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/dma/sprd-dma.txt b/Documentation/devicetree/bindings/dma/sprd-dma.txt
index 7a10fea..adccea994 100644
--- a/Documentation/devicetree/bindings/dma/sprd-dma.txt
+++ b/Documentation/devicetree/bindings/dma/sprd-dma.txt
@@ -31,7 +31,7 @@ DMA clients connected to the Spreadtrum DMA controller must use the format
described in the dma.txt file, using a two-cell specifier for each channel.
The two cells in order are:
1. A phandle pointing to the DMA controller.
-2. The channel id.
+2. The slave id.
spi0: spi@70a00000{
...
^ permalink raw reply related
* [v2,2/2] dmaengine: sprd: Change channel id to slave id for DMA cell specifier
From: Baolin Wang @ 2019-02-21 5:34 UTC (permalink / raw)
To: vkoul, robh+dt, mark.rutland, arnd
Cc: orsonzhai, zhang.lyra, dan.j.williams, devicetree, linux-kernel,
dmaengine, eric.long, broonie, baolin.wang
We will describe the slave id in DMA cell specifier instead of DMA channel
id, thus we should save the slave id from DMA engine translation function,
and remove the channel id validation.
Meanwhile we do not need set default slave id in sprd_dma_alloc_chan_resources(),
remove it.
Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
---
Changes from v1:
- Remove channel id from DT.
---
drivers/dma/sprd-dma.c | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index e2f0167..48431e2 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -580,15 +580,7 @@ static irqreturn_t dma_irq_handle(int irq, void *dev_id)
static int sprd_dma_alloc_chan_resources(struct dma_chan *chan)
{
- struct sprd_dma_chn *schan = to_sprd_dma_chan(chan);
- int ret;
-
- ret = pm_runtime_get_sync(chan->device->dev);
- if (ret < 0)
- return ret;
-
- schan->dev_id = SPRD_DMA_SOFTWARE_UID;
- return 0;
+ return pm_runtime_get_sync(chan->device->dev);
}
static void sprd_dma_free_chan_resources(struct dma_chan *chan)
@@ -1021,13 +1013,10 @@ static void sprd_dma_free_desc(struct virt_dma_desc *vd)
static bool sprd_dma_filter_fn(struct dma_chan *chan, void *param)
{
struct sprd_dma_chn *schan = to_sprd_dma_chan(chan);
- struct sprd_dma_dev *sdev = to_sprd_dma_dev(&schan->vc.chan);
- u32 req = *(u32 *)param;
+ u32 slave_id = *(u32 *)param;
- if (req < sdev->total_chns)
- return req == schan->chn_num + 1;
- else
- return false;
+ schan->dev_id = slave_id;
+ return true;
}
static int sprd_dma_probe(struct platform_device *pdev)
^ permalink raw reply related
* [4/6] dma: tegra: add accurate reporting of dma state
From: Ben Dooks @ 2019-02-21 10:06 UTC (permalink / raw)
To: Dmitry Osipenko; +Cc: dan.j.williams, vkoul, ldewangan, dmaengine, linux-tegra
On 21/02/2019 00:41, Dmitry Osipenko wrote:
> 31.10.2018 19:03, Ben Dooks пишет:
>> The tx_status callback does not report the state of the transfer
>> beyond complete segments. This causes problems with users such as
>> ALSA when applications want to know accurately how much data has
>> been moved.
>>
>> This patch addes a function tegra_dma_update_residual() to query
>> the hardware and modify the residual information accordinly. It
>> takes into account any hardware issues when trying to read the
>> state, such as delays between finishing a buffer and signalling
>> the interrupt.
>>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>
> Hello Ben,
>
> Do you have any plans to submit a new version of this patch? It's really useful and fixes a real problem with the audio playback. I could help with finalizing the patch and could submit it for you if you happened to lost the interest.
Personally I think the original version was fine. It has been tested
and returns fairly quickly (I am not a fan of just adding more delay in)
My notes say the condition doesn't last for long and the loop tends
to terminate within 2 runs.
^ permalink raw reply
* [4/6] dma: tegra: add accurate reporting of dma state
From: Dmitry Osipenko @ 2019-02-21 13:02 UTC (permalink / raw)
To: Ben Dooks; +Cc: dan.j.williams, vkoul, ldewangan, dmaengine, linux-tegra
21.02.2019 13:06, Ben Dooks пишет:
> On 21/02/2019 00:41, Dmitry Osipenko wrote:
>> 31.10.2018 19:03, Ben Dooks пишет:
>>> The tx_status callback does not report the state of the transfer
>>> beyond complete segments. This causes problems with users such as
>>> ALSA when applications want to know accurately how much data has
>>> been moved.
>>>
>>> This patch addes a function tegra_dma_update_residual() to query
>>> the hardware and modify the residual information accordinly. It
>>> takes into account any hardware issues when trying to read the
>>> state, such as delays between finishing a buffer and signalling
>>> the interrupt.
>>>
>>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>>
>> Hello Ben,
>>
>> Do you have any plans to submit a new version of this patch? It's really useful and fixes a real problem with the audio playback. I could help with finalizing the patch and could submit it for you if you happened to lost the interest.
>
> Personally I think the original version was fine. It has been tested
> and returns fairly quickly (I am not a fan of just adding more delay in)
>
> My notes say the condition doesn't last for long and the loop tends
> to terminate within 2 runs.
>
Okay, so are you going to re-send the patch? We can back to the review after, you need at least to re-send because this series has been outdated. Also please take a look and feel free to use as-is the reduced variant of yours patch that I was carrying and testing for months now [0], it works great.
[0] https://github.com/grate-driver/linux/commit/ab8a67a6f47185f265f16749b55df214aaaefad4
^ permalink raw reply
* [1/4] dmaengine: ioatdma: Add Snow Ridge ioatdma device id
From: Dave Jiang @ 2019-02-22 16:59 UTC (permalink / raw)
To: vkoul; +Cc: dmaengine
Add Snowridge Xeon-D ioatdma PCI device id. Also applies for Icelake
SP Xeon. This introduces ioatdma v3.4 platform. Also bumping driver version
to 5.0 since we are adding additional code for 3.4 support.
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
drivers/dma/ioat/dma.h | 2 +-
drivers/dma/ioat/hw.h | 2 ++
drivers/dma/ioat/init.c | 3 +++
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/ioat/dma.h b/drivers/dma/ioat/dma.h
index 1ab42ec2b7ff..aaafd0e882b5 100644
--- a/drivers/dma/ioat/dma.h
+++ b/drivers/dma/ioat/dma.h
@@ -27,7 +27,7 @@
#include "registers.h"
#include "hw.h"
-#define IOAT_DMA_VERSION "4.00"
+#define IOAT_DMA_VERSION "5.00"
#define IOAT_DMA_DCA_ANY_CPU ~0
diff --git a/drivers/dma/ioat/hw.h b/drivers/dma/ioat/hw.h
index abcc51b343ce..3e38877cae6b 100644
--- a/drivers/dma/ioat/hw.h
+++ b/drivers/dma/ioat/hw.h
@@ -66,6 +66,8 @@
#define PCI_DEVICE_ID_INTEL_IOAT_SKX 0x2021
+#define PCI_DEVICE_ID_INTEL_IOAT_ICX 0x0b00
+
#define IOAT_VER_1_2 0x12 /* Version 1.2 */
#define IOAT_VER_2_0 0x20 /* Version 2.0 */
#define IOAT_VER_3_0 0x30 /* Version 3.0 */
diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
index 2d810dfcdc48..3161dfbca505 100644
--- a/drivers/dma/ioat/init.c
+++ b/drivers/dma/ioat/init.c
@@ -119,6 +119,9 @@ static const struct pci_device_id ioat_pci_tbl[] = {
{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_IOAT_BDXDE2) },
{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_IOAT_BDXDE3) },
+ /* I/OAT v3.4 platforms */
+ { PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_IOAT_ICX) },
+
{ 0, }
};
MODULE_DEVICE_TABLE(pci, ioat_pci_tbl);
^ permalink raw reply related
* [2/4] dmaengine: ioatdma: disable DCA enabling on IOATDMA v3.4
From: Dave Jiang @ 2019-02-22 16:59 UTC (permalink / raw)
To: vkoul; +Cc: dmaengine
IOATDMA v3.4 does not support DCA. Disable
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
drivers/dma/ioat/hw.h | 1 +
drivers/dma/ioat/init.c | 2 ++
2 files changed, 3 insertions(+)
diff --git a/drivers/dma/ioat/hw.h b/drivers/dma/ioat/hw.h
index 3e38877cae6b..781c94de8e81 100644
--- a/drivers/dma/ioat/hw.h
+++ b/drivers/dma/ioat/hw.h
@@ -73,6 +73,7 @@
#define IOAT_VER_3_0 0x30 /* Version 3.0 */
#define IOAT_VER_3_2 0x32 /* Version 3.2 */
#define IOAT_VER_3_3 0x33 /* Version 3.3 */
+#define IOAT_VER_3_4 0x34 /* Version 3.4 */
int system_has_dca_enabled(struct pci_dev *pdev);
diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
index 3161dfbca505..58dd1bfd3edd 100644
--- a/drivers/dma/ioat/init.c
+++ b/drivers/dma/ioat/init.c
@@ -1353,6 +1353,8 @@ static int ioat_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
pci_set_drvdata(pdev, device);
device->version = readb(device->reg_base + IOAT_VER_OFFSET);
+ if (device->version >= IOAT_VER_3_4)
+ ioat_dca_enabled = 0;
if (device->version >= IOAT_VER_3_0) {
if (is_skx_ioat(pdev))
device->version = IOAT_VER_3_2;
^ permalink raw reply related
* [3/4] dmaengine: ioatdma: add descriptor pre-fetch support for v3.4
From: Dave Jiang @ 2019-02-22 17:00 UTC (permalink / raw)
To: vkoul; +Cc: dmaengine
Adding support for new feature on ioatdma 3.4 hardware that provides
descriptor pre-fetching in order to reduce small DMA latencies.
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
drivers/dma/ioat/dma.c | 12 ++++++++++++
drivers/dma/ioat/init.c | 8 ++++++--
drivers/dma/ioat/registers.h | 10 ++++++++++
3 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/ioat/dma.c b/drivers/dma/ioat/dma.c
index 23fb2fa04000..f373a139e0c3 100644
--- a/drivers/dma/ioat/dma.c
+++ b/drivers/dma/ioat/dma.c
@@ -372,6 +372,7 @@ struct ioat_ring_ent **
ioat_alloc_ring(struct dma_chan *c, int order, gfp_t flags)
{
struct ioatdma_chan *ioat_chan = to_ioat_chan(c);
+ struct ioatdma_device *ioat_dma = ioat_chan->ioat_dma;
struct ioat_ring_ent **ring;
int total_descs = 1 << order;
int i, chunks;
@@ -437,6 +438,17 @@ ioat_alloc_ring(struct dma_chan *c, int order, gfp_t flags)
}
ring[i]->hw->next = ring[0]->txd.phys;
+ /* setup descriptor pre-fetching for v3.4 */
+ if (ioat_dma->cap & IOAT_CAP_DPS) {
+ u16 drsctl = IOAT_CHAN_DRSZ_2MB | IOAT_CHAN_DRS_EN;
+
+ if (chunks == 1)
+ drsctl |= IOAT_CHAN_DRS_AUTOWRAP;
+
+ writew(drsctl, ioat_chan->reg_base + IOAT_CHAN_DRSCTL_OFFSET);
+
+ }
+
return ring;
}
diff --git a/drivers/dma/ioat/init.c b/drivers/dma/ioat/init.c
index 58dd1bfd3edd..020bcdecb3fb 100644
--- a/drivers/dma/ioat/init.c
+++ b/drivers/dma/ioat/init.c
@@ -138,10 +138,10 @@ static int ioat3_dma_self_test(struct ioatdma_device *ioat_dma);
static int ioat_dca_enabled = 1;
module_param(ioat_dca_enabled, int, 0644);
MODULE_PARM_DESC(ioat_dca_enabled, "control support of dca service (default: 1)");
-int ioat_pending_level = 4;
+int ioat_pending_level = 7;
module_param(ioat_pending_level, int, 0644);
MODULE_PARM_DESC(ioat_pending_level,
- "high-water mark for pushing ioat descriptors (default: 4)");
+ "high-water mark for pushing ioat descriptors (default: 7)");
static char ioat_interrupt_style[32] = "msix";
module_param_string(ioat_interrupt_style, ioat_interrupt_style,
sizeof(ioat_interrupt_style), 0644);
@@ -1188,6 +1188,10 @@ static int ioat3_dma_probe(struct ioatdma_device *ioat_dma, int dca)
if (err)
return err;
+ if (ioat_dma->cap & IOAT_CAP_DPS)
+ writeb(ioat_pending_level + 1,
+ ioat_dma->reg_base + IOAT_PREFETCH_LIMIT_OFFSET);
+
return 0;
}
diff --git a/drivers/dma/ioat/registers.h b/drivers/dma/ioat/registers.h
index 2f3bbc88ff2a..2b517d6db5fd 100644
--- a/drivers/dma/ioat/registers.h
+++ b/drivers/dma/ioat/registers.h
@@ -84,6 +84,9 @@
#define IOAT_CAP_PQ 0x00000200
#define IOAT_CAP_DWBES 0x00002000
#define IOAT_CAP_RAID16SS 0x00020000
+#define IOAT_CAP_DPS 0x00800000
+
+#define IOAT_PREFETCH_LIMIT_OFFSET 0x4C /* CHWPREFLMT */
#define IOAT_CHANNEL_MMIO_SIZE 0x80 /* Each Channel MMIO space is this size */
@@ -243,4 +246,11 @@
#define IOAT_CHANERR_MASK_OFFSET 0x2C /* 32-bit Channel Error Register */
+#define IOAT_CHAN_DRSCTL_OFFSET 0xB6
+#define IOAT_CHAN_DRSZ_4KB 0x0000
+#define IOAT_CHAN_DRSZ_8KB 0x0001
+#define IOAT_CHAN_DRSZ_2MB 0x0009
+#define IOAT_CHAN_DRS_EN 0x0100
+#define IOAT_CHAN_DRS_AUTOWRAP 0x0200
+
#endif /* _IOAT_REGISTERS_H_ */
^ permalink raw reply related
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