Hi Dmitry,
On 26/09/17 17:06, Dmitry Osipenko wrote:
> Hi Jon,
>
> On 26.09.2017 17:45, Jon Hunter wrote:
>> Hi Dmitry,
>>
>> On 26/09/17 00:22, Dmitry Osipenko wrote:
>>> AHB DMA controller presents on Tegra20/30 SoC's, it supports transfers
>>> memory <-> AHB bus peripherals as well as mem-to-mem transfers. Driver
>>> doesn't yet implement transfers larger than 64K and scatter-gather
>>> transfers that have NENT > 1, HW doesn't have native support for these
>>> cases.
>>>
>>> Signed-off-by: Dmitry Osipenko <dig...@gmail.com>
>>> ---
>>> drivers/dma/Kconfig | 9 +
>>> drivers/dma/Makefile | 1 +
>>> drivers/dma/tegra20-ahb-dma.c | 679
>>> ++++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 689 insertions(+)
>>> create mode 100644 drivers/dma/tegra20-ahb-dma.c
>>
>> ...
>>
>>> diff --git a/drivers/dma/tegra20-ahb-dma.c b/drivers/dma/tegra20-ahb-dma.c
>>> new file mode 100644
>>> index 000000000000..8316d64e35e1
>>> --- /dev/null
>>> +++ b/drivers/dma/tegra20-ahb-dma.c
>>> @@ -0,0 +1,679 @@
>>> +/*
>>> + * Copyright 2017 Dmitry Osipenko <dig...@gmail.com>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify it
>>> + * under the terms and conditions of the GNU General Public License,
>>> + * version 2, as published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope it will be useful, but WITHOUT
>>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
>>> for
>>> + * more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
>>> + */
>>> +
>>> +#include <linux/clk.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/io.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/of_dma.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/reset.h>
>>> +#include <linux/slab.h>
>>> +#include <linux/spinlock.h>
>>> +
>>> +#include "dmaengine.h"
>>> +
>>> +#define TEGRA_AHBDMA_CMD 0x0
>>> +#define TEGRA_AHBDMA_CMD_ENABLE BIT(31)
>>> +
>>> +#define TEGRA_AHBDMA_IRQ_ENB_MASK 0x20
>>> +#define TEGRA_AHBDMA_IRQ_ENB_CH(ch) BIT(ch)
>>> +
>>> +#define TEGRA_AHBDMA_CHANNEL_BASE(ch) (0x1000 + (ch) * 0x20)
>>> +
>>> +#define TEGRA_AHBDMA_CHANNEL_CSR 0x0
>>> +#define TEGRA_AHBDMA_CHANNEL_ADDR_WRAP BIT(18)
>>> +#define TEGRA_AHBDMA_CHANNEL_FLOW BIT(24)
>>> +#define TEGRA_AHBDMA_CHANNEL_ONCE BIT(26)
>>> +#define TEGRA_AHBDMA_CHANNEL_DIR_TO_XMB BIT(27)
>>> +#define TEGRA_AHBDMA_CHANNEL_IE_EOC BIT(30)
>>> +#define TEGRA_AHBDMA_CHANNEL_ENABLE BIT(31)
>>> +#define TEGRA_AHBDMA_CHANNEL_REQ_SEL_SHIFT 16
>>> +#define TEGRA_AHBDMA_CHANNEL_WCOUNT_MASK 0xFFFC
>>> +
>>> +#define TEGRA_AHBDMA_CHANNEL_STA 0x4
>>> +#define TEGRA_AHBDMA_CHANNEL_IS_EOC BIT(30)
>>> +
>>> +#define TEGRA_AHBDMA_CHANNEL_AHB_PTR 0x10
>>> +
>>> +#define TEGRA_AHBDMA_CHANNEL_AHB_SEQ 0x14
>>> +#define TEGRA_AHBDMA_CHANNEL_INTR_ENB BIT(31)
>>> +#define TEGRA_AHBDMA_CHANNEL_AHB_BURST_SHIFT 24
>>> +#define TEGRA_AHBDMA_CHANNEL_AHB_BURST_1 2
>>> +#define TEGRA_AHBDMA_CHANNEL_AHB_BURST_4 3
>>> +#define TEGRA_AHBDMA_CHANNEL_AHB_BURST_8 4
>>> +
>>> +#define TEGRA_AHBDMA_CHANNEL_XMB_PTR 0x18
>>> +
>>> +#define TEGRA_AHBDMA_BUS_WIDTH
>>> BIT(DMA_SLAVE_BUSWIDTH_4_BYTES)
>>> +
>>> +#define TEGRA_AHBDMA_DIRECTIONS BIT(DMA_DEV_TO_MEM) | \
>>> + BIT(DMA_MEM_TO_DEV)
>>> +
>>> +struct tegra_ahbdma_tx_desc {
>>> + struct dma_async_tx_descriptor desc;
>>> + struct tasklet_struct tasklet;
>>> + struct list_head node;
>>
>> Any reason why we cannot use the virt-dma framework for this driver? I
>> would hope it would simplify the driver a bit.
>>
>
> IIUC virt-dma is supposed to provide virtually unlimited number of channels.
> I've looked at it and decided that it would just add unnecessary functionality
> and, as a result, complexity. As I wrote in the cover-letter, it is supposed
> that this driver would have only one consumer - the host1x. It shouldn't be
> difficult to implement virt-dma later, if desired. But again it is very
> unlikely that it would be needed.
I think that the biggest benefit is that is simplifies the linked list
management. See the tegra210-adma driver.
>>> + enum dma_transfer_direction dir;
>>> + dma_addr_t mem_paddr;
>>> + unsigned long flags;
>>> + size_t size;
>>> + bool in_fly;
>>> + bool cyclic;
>>> +};
>>> +
>>> +struct tegra_ahbdma_chan {
>>> + struct dma_chan dma_chan;
>>> + struct list_head active_list;
>>> + struct list_head pending_list;
>>> + struct completion idling;
>>> + void __iomem *regs;
>>> + spinlock_t lock;
>>> + unsigned int id;
>>> +};
>>> +
>>> +struct tegra_ahbdma {
>>> + struct tegra_ahbdma_chan channels[4];
>>> + struct dma_device dma_dev;
>>> + struct reset_control *rst;
>>> + struct clk *clk;
>>> + void __iomem *regs;
>>> +};
>>> +
>>> +static inline struct tegra_ahbdma *to_ahbdma(struct dma_device *dev)
>>> +{
>>> + return container_of(dev, struct tegra_ahbdma, dma_dev);
>>> +}
>>> +
>>> +static inline struct tegra_ahbdma_chan *to_ahbdma_chan(struct dma_chan
>>> *chan)
>>> +{
>>> + return container_of(chan, struct tegra_ahbdma_chan, dma_chan);
>>> +}
>>> +
>>> +static inline struct tegra_ahbdma_tx_desc *to_ahbdma_tx_desc(
>>> + struct dma_async_tx_descriptor *tx)
>>> +{
>>> + return container_of(tx, struct tegra_ahbdma_tx_desc, desc);
>>> +}
>>> +
>>> +static void tegra_ahbdma_submit_tx(struct tegra_ahbdma_chan *chan,
>>> + struct tegra_ahbdma_tx_desc *tx)
>>> +{
>>> + u32 csr;
>>> +
>>> + writel_relaxed(tx->mem_paddr,
>>> + chan->regs + TEGRA_AHBDMA_CHANNEL_XMB_PTR);
>>> +
>>> + csr = readl_relaxed(chan->regs + TEGRA_AHBDMA_CHANNEL_CSR);
>>> +
>>> + csr &= ~TEGRA_AHBDMA_CHANNEL_WCOUNT_MASK;
>>> + csr &= ~TEGRA_AHBDMA_CHANNEL_DIR_TO_XMB;
>>> + csr |= TEGRA_AHBDMA_CHANNEL_ENABLE;
>>> + csr |= TEGRA_AHBDMA_CHANNEL_IE_EOC;
>>> + csr |= tx->size - sizeof(u32);
>>> +
>>> + if (tx->dir == DMA_DEV_TO_MEM)
>>> + csr |= TEGRA_AHBDMA_CHANNEL_DIR_TO_XMB;
>>> +
>>> + if (!tx->cyclic)
>>> + csr |= TEGRA_AHBDMA_CHANNEL_ONCE;
>>> +
>>> + writel_relaxed(csr, chan->regs + TEGRA_AHBDMA_CHANNEL_CSR);
>>> +
>>> + tx->in_fly = true;
>>> +}
>>> +
>>> +static void tegra_ahbdma_tasklet(unsigned long data)
>>> +{
>>> + struct tegra_ahbdma_tx_desc *tx = (struct tegra_ahbdma_tx_desc *)data;
>>> + struct dma_async_tx_descriptor *desc = &tx->desc;
>>> +
>>> + dmaengine_desc_get_callback_invoke(desc, NULL);
>>> +
>>> + if (!tx->cyclic && !dmaengine_desc_test_reuse(desc))
>>> + kfree(tx);
>>> +}
>>> +
>>> +static bool tegra_ahbdma_tx_completed(struct tegra_ahbdma_chan *chan,
>>> + struct tegra_ahbdma_tx_desc *tx)
>>> +{
>>> + struct dma_async_tx_descriptor *desc = &tx->desc;
>>> + bool reuse = dmaengine_desc_test_reuse(desc);
>>> + bool interrupt = tx->flags & DMA_PREP_INTERRUPT;
>>> + bool completed = !tx->cyclic;
>>> +
>>> + if (completed)
>>> + dma_cookie_complete(desc);
>>> +
>>> + if (interrupt)
>>> + tasklet_schedule(&tx->tasklet);
>>> +
>>> + if (completed) {
>>> + list_del(&tx->node);
>>> +
>>> + if (reuse)
>>> + tx->in_fly = false;
>>> +
>>> + if (!interrupt && !reuse)
>>> + kfree(tx);
>>> + }
>>> +
>>> + return completed;
>>> +}
>>> +
>>> +static bool tegra_ahbdma_next_tx_issued(struct tegra_ahbdma_chan *chan)
>>> +{
>>> + struct tegra_ahbdma_tx_desc *tx;
>>> +
>>> + tx = list_first_entry_or_null(&chan->active_list,
>>> + struct tegra_ahbdma_tx_desc,
>>> + node);
>>> + if (tx)
>>> + tegra_ahbdma_submit_tx(chan, tx);
>>> +
>>> + return !!tx;
>>> +}
>>> +
>>> +static void tegra_ahbdma_handle_channel(struct tegra_ahbdma_chan *chan)
>>> +{
>>> + struct tegra_ahbdma_tx_desc *tx;
>>> + unsigned long flags;
>>> + u32 status;
>>> +
>>> + status = readl_relaxed(chan->regs + TEGRA_AHBDMA_CHANNEL_STA);
>>> + if (!(status & TEGRA_AHBDMA_CHANNEL_IS_EOC))
>>> + return;
>>> +
>>> + writel_relaxed(TEGRA_AHBDMA_CHANNEL_IS_EOC,
>>> + chan->regs + TEGRA_AHBDMA_CHANNEL_STA);
>>> +
>>> + spin_lock_irqsave(&chan->lock, flags);
>>> +
>>> + if (!completion_done(&chan->idling)) {
>>> + tx = list_first_entry(&chan->active_list,
>>> + struct tegra_ahbdma_tx_desc,
>>> + node);
>>> +
>>> + if (tegra_ahbdma_tx_completed(chan, tx) &&
>>> + !tegra_ahbdma_next_tx_issued(chan))
>>> + complete_all(&chan->idling);
>>> + }
>>> +
>>> + spin_unlock_irqrestore(&chan->lock, flags);
>>> +}
>>> +
>>> +static irqreturn_t tegra_ahbdma_isr(int irq, void *dev_id)
>>> +{
>>> + struct tegra_ahbdma *tdma = dev_id;
>>> + unsigned int i;
>>> +
>>> + for (i = 0; i < ARRAY_SIZE(tdma->channels); i++)
>>> + tegra_ahbdma_handle_channel(&tdma->channels[i]);
>>> +
>>> + return IRQ_HANDLED;
>>> +}
>>> +
>>> +static dma_cookie_t tegra_ahbdma_tx_submit(struct dma_async_tx_descriptor
>>> *desc)
>>> +{
>>> + struct tegra_ahbdma_tx_desc *tx = to_ahbdma_tx_desc(desc);
>>> + struct tegra_ahbdma_chan *chan = to_ahbdma_chan(desc->chan);
>>> + dma_cookie_t cookie;
>>> +
>>> + cookie = dma_cookie_assign(desc);
>>> +
>>> + spin_lock_irq(&chan->lock);
>>> + list_add_tail(&tx->node, &chan->pending_list);
>>> + spin_unlock_irq(&chan->lock);
>>> +
>>> + return cookie;
>>> +}
>>> +
>>> +static int tegra_ahbdma_tx_desc_free(struct dma_async_tx_descriptor *desc)
>>> +{
>>> + kfree(to_ahbdma_tx_desc(desc));
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static struct dma_async_tx_descriptor *tegra_ahbdma_prep_slave_sg(
>>> + struct dma_chan *chan,
>>> + struct scatterlist *sgl,
>>> + unsigned int sg_len,
>>> + enum dma_transfer_direction dir,
>>> + unsigned long flags,
>>> + void *context)
>>> +{
>>> + struct tegra_ahbdma_tx_desc *tx;
>>> +
>>> + /* unimplemented */
>>> + if (sg_len != 1 || sg_dma_len(sgl) > SZ_64K)
>>> + return NULL;
>>> +
>>> + tx = kzalloc(sizeof(*tx), GFP_KERNEL);
>>> + if (!tx)
>>> + return NULL;
>>> +
>>> + dma_async_tx_descriptor_init(&tx->desc, chan);
>>> +
>>> + tx->desc.tx_submit = tegra_ahbdma_tx_submit;
>>> + tx->desc.desc_free = tegra_ahbdma_tx_desc_free;
>>> + tx->mem_paddr = sg_dma_address(sgl);
>>> + tx->size = sg_dma_len(sgl);
>>> + tx->flags = flags;
>>> + tx->dir = dir;
>>> +
>>> + tasklet_init(&tx->tasklet, tegra_ahbdma_tasklet, (unsigned long)tx);
>>> +
>>> + return &tx->desc;
>>> +}
>>> +
>>> +static struct dma_async_tx_descriptor *tegra_ahbdma_prep_dma_cyclic(
>>> + struct dma_chan *chan,
>>> + dma_addr_t buf_addr,
>>> + size_t buf_len,
>>> + size_t period_len,
>>> + enum dma_transfer_direction dir,
>>> + unsigned long flags)
>>> +{
>>> + struct tegra_ahbdma_tx_desc *tx;
>>> +
>>> + /* unimplemented */
>>> + if (buf_len != period_len || buf_len > SZ_64K)
>>> + return NULL;
>>> +
>>> + tx = kzalloc(sizeof(*tx), GFP_KERNEL);
>>> + if (!tx)
>>> + return NULL;
>>> +
>>> + dma_async_tx_descriptor_init(&tx->desc, chan);
>>> +
>>> + tx->desc.tx_submit = tegra_ahbdma_tx_submit;
>>> + tx->mem_paddr = buf_addr;
>>> + tx->size = buf_len;
>>> + tx->flags = flags;
>>> + tx->cyclic = true;
>>> + tx->dir = dir;
>>> +
>>> + tasklet_init(&tx->tasklet, tegra_ahbdma_tasklet, (unsigned long)tx);
>>> +
>>> + return &tx->desc;
>>> +}
>>> +
>>> +static void tegra_ahbdma_issue_pending(struct dma_chan *chan)
>>> +{
>>> + struct tegra_ahbdma_chan *ahbdma_chan = to_ahbdma_chan(chan);
>>> + struct tegra_ahbdma_tx_desc *tx;
>>> + struct list_head *entry, *tmp;
>>> + unsigned long flags;
>>> +
>>> + spin_lock_irqsave(&ahbdma_chan->lock, flags);
>>> +
>>> + list_for_each_safe(entry, tmp, &ahbdma_chan->pending_list)
>>> + list_move_tail(entry, &ahbdma_chan->active_list);
>>> +
>>> + if (completion_done(&ahbdma_chan->idling)) {
>>> + tx = list_first_entry_or_null(&ahbdma_chan->active_list,
>>> + struct tegra_ahbdma_tx_desc,
>>> + node);
>>> + if (tx) {
>>> + tegra_ahbdma_submit_tx(ahbdma_chan, tx);
>>> + reinit_completion(&ahbdma_chan->idling);
>>> + }
>>> + }
>>> +
>>> + spin_unlock_irqrestore(&ahbdma_chan->lock, flags);
>>> +}
>>> +
>>> +static enum dma_status tegra_ahbdma_tx_status(struct dma_chan *chan,
>>> + dma_cookie_t cookie,
>>> + struct dma_tx_state *state)
>>> +{
>>> + struct tegra_ahbdma_chan *ahbdma_chan = to_ahbdma_chan(chan);
>>> + struct tegra_ahbdma_tx_desc *tx;
>>> + enum dma_status cookie_status;
>>> + unsigned long flags;
>>> + size_t residual;
>>> + u32 status;
>>> +
>>> + spin_lock_irqsave(&ahbdma_chan->lock, flags);
>>> +
>>> + cookie_status = dma_cookie_status(chan, cookie, state);
>>> + if (cookie_status != DMA_COMPLETE) {
>>> + list_for_each_entry(tx, &ahbdma_chan->active_list, node) {
>>> + if (tx->desc.cookie == cookie)
>>> + goto found;
>>> + }
>>> + }
>>> +
>>> + goto unlock;
>>> +
>>> +found:
>>> + if (tx->in_fly) {
>>> + status = readl_relaxed(
>>> + ahbdma_chan->regs + TEGRA_AHBDMA_CHANNEL_STA);
>>> + status &= TEGRA_AHBDMA_CHANNEL_WCOUNT_MASK;
>>> +
>>> + residual = status;
>>> + } else
>>> + residual = tx->size;
>>> +
>>> + dma_set_residue(state, residual);
>>> +
>>> +unlock:
>>> + spin_unlock_irqrestore(&ahbdma_chan->lock, flags);
>>> +
>>> + return cookie_status;
>>> +}
>>> +
>>> +static int tegra_ahbdma_terminate_all(struct dma_chan *chan)
>>> +{
>>> + struct tegra_ahbdma_chan *ahbdma_chan = to_ahbdma_chan(chan);
>>> + struct tegra_ahbdma_tx_desc *tx;
>>> + struct list_head *entry, *tmp;
>>> + u32 csr;
>>> +
>>> + spin_lock_irq(&ahbdma_chan->lock);
>>> +
>>> + csr = readl_relaxed(ahbdma_chan->regs + TEGRA_AHBDMA_CHANNEL_CSR);
>>> + csr &= ~TEGRA_AHBDMA_CHANNEL_ENABLE;
>>> +
>>> + writel_relaxed(csr, ahbdma_chan->regs + TEGRA_AHBDMA_CHANNEL_CSR);
>>> +
>>> + list_for_each_safe(entry, tmp, &ahbdma_chan->active_list) {
>>> + tx = list_entry(entry, struct tegra_ahbdma_tx_desc, node);
>>> + list_del(entry);
>>> + kfree(tx);
>>> + }
>>> +
>>> + list_for_each_safe(entry, tmp, &ahbdma_chan->pending_list) {
>>> + tx = list_entry(entry, struct tegra_ahbdma_tx_desc, node);
>>> + list_del(entry);
>>> + kfree(tx);
>>> + }
>>> +
>>> + complete_all(&ahbdma_chan->idling);
>>> +
>>> + spin_unlock_irq(&ahbdma_chan->lock);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int tegra_ahbdma_config(struct dma_chan *chan,
>>> + struct dma_slave_config *sconfig)
>>> +{
>>> + struct tegra_ahbdma_chan *ahbdma_chan = to_ahbdma_chan(chan);
>>> + enum dma_transfer_direction dir = sconfig->direction;
>>> + u32 burst, ahb_seq, ahb_addr;
>>> +
>>> + if (sconfig->src_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES ||
>>> + sconfig->dst_addr_width != DMA_SLAVE_BUSWIDTH_4_BYTES)
>>> + return -EINVAL;
>>> +
>>> + if (dir == DMA_DEV_TO_MEM) {
>>> + burst = sconfig->src_maxburst;
>>> + ahb_addr = sconfig->src_addr;
>>> + } else {
>>> + burst = sconfig->dst_maxburst;
>>> + ahb_addr = sconfig->dst_addr;
>>> + }
>>> +
>>> + switch (burst) {
>>> + case 1: burst = TEGRA_AHBDMA_CHANNEL_AHB_BURST_1; break;
>>> + case 4: burst = TEGRA_AHBDMA_CHANNEL_AHB_BURST_4; break;
>>> + case 8: burst = TEGRA_AHBDMA_CHANNEL_AHB_BURST_8; break;
>>> + default:
>>> + return -EINVAL;
>>> + }
>>> +
>>> + ahb_seq = burst << TEGRA_AHBDMA_CHANNEL_AHB_BURST_SHIFT;
>>> + ahb_seq |= TEGRA_AHBDMA_CHANNEL_ADDR_WRAP;
>>> + ahb_seq |= TEGRA_AHBDMA_CHANNEL_INTR_ENB;
>>> +
>>> + writel_relaxed(ahb_seq,
>>> + ahbdma_chan->regs + TEGRA_AHBDMA_CHANNEL_AHB_SEQ);
>>> +
>>> + writel_relaxed(ahb_addr,
>>> + ahbdma_chan->regs + TEGRA_AHBDMA_CHANNEL_AHB_PTR);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void tegra_ahbdma_synchronize(struct dma_chan *chan)
>>> +{
>>> + wait_for_completion(&to_ahbdma_chan(chan)->idling);
>>> +}
>>> +
>>> +static void tegra_ahbdma_free_chan_resources(struct dma_chan *chan)
>>> +{
>>> + struct tegra_ahbdma_chan *ahbdma_chan = to_ahbdma_chan(chan);
>>> + struct tegra_ahbdma_tx_desc *tx;
>>> + struct list_head *entry, *tmp;
>>> +
>>> + list_for_each_safe(entry, tmp, &ahbdma_chan->pending_list) {
>>> + tx = list_entry(entry, struct tegra_ahbdma_tx_desc, node);
>>> + list_del(entry);
>>> + kfree(tx);
>>> + }
>>> +}
>>> +
>>> +static void tegra_ahbdma_init_channel(struct tegra_ahbdma *tdma,
>>> + unsigned int chan_id)
>>> +{
>>> + struct tegra_ahbdma_chan *ahbdma_chan = &tdma->channels[chan_id];
>>> + struct dma_chan *dma_chan = &ahbdma_chan->dma_chan;
>>> + struct dma_device *dma_dev = &tdma->dma_dev;
>>> +
>>> + INIT_LIST_HEAD(&ahbdma_chan->active_list);
>>> + INIT_LIST_HEAD(&ahbdma_chan->pending_list);
>>> + init_completion(&ahbdma_chan->idling);
>>> + spin_lock_init(&ahbdma_chan->lock);
>>> + complete(&ahbdma_chan->idling);
>>> +
>>> + ahbdma_chan->regs = tdma->regs + TEGRA_AHBDMA_CHANNEL_BASE(chan_id);
>>> + ahbdma_chan->id = chan_id;
>>> +
>>> + dma_cookie_init(dma_chan);
>>> + dma_chan->device = dma_dev;
>>> +
>>> + list_add_tail(&dma_chan->device_node, &dma_dev->channels);
>>> +}
>>> +
>>> +static struct dma_chan *tegra_ahbdma_of_xlate(struct of_phandle_args
>>> *dma_spec,
>>> + struct of_dma *ofdma)
>>> +{
>>> + struct tegra_ahbdma *tdma = ofdma->of_dma_data;
>>> + struct dma_chan *chan;
>>> + u32 csr;
>>> +
>>> + chan = dma_get_any_slave_channel(&tdma->dma_dev);
>>> + if (!chan)
>>> + return NULL;
>>> +
>>> + /* enable channels flow control */
>>> + if (dma_spec->args_count == 1) {
>>
>> The DT doc says #dma-cells should be '1' and so if not equal 1, is this
>> not an error?
>>
>
> I wanted to differentiate slave/master modes here. But if we'd want to add
> TRIG_SEL as another cell, then it probably would worth to implement a custom
> DMA
> configure options, like documentation suggests - to wrap generic
> dma_slave_config into the custom one. On the other hand that probably would
> add
> an unused functionality to the driver.
>
>>> + csr = TEGRA_AHBDMA_CHANNEL_FLOW;
>>> + csr |= dma_spec->args[0] << TEGRA_AHBDMA_CHANNEL_REQ_SEL_SHIFT;
>>
>> What about the TRIG_REQ field?
>>
>
> Not implemented, there is no test case for it yet.
>
>>> +
>>> + writel_relaxed(csr,
>>> + to_ahbdma_chan(chan)->regs + TEGRA_AHBDMA_CHANNEL_CSR);
>>> + }
>>> +
>>> + return chan;
>>> +}
>>> +
>>> +static int tegra_ahbdma_init_hw(struct tegra_ahbdma *tdma, struct device
>>> *dev)
>>> +{
>>> + int err;
>>> +
>>> + err = reset_control_assert(tdma->rst);
>>> + if (err) {
>>> + dev_err(dev, "Failed to assert reset: %d\n", err);
>>> + return err;
>>> + }
>>> +
>>> + err = clk_prepare_enable(tdma->clk);
>>> + if (err) {
>>> + dev_err(dev, "Failed to enable clock: %d\n", err);
>>> + return err;
>>> + }
>>> +
>>> + usleep_range(1000, 2000);
>>> +
>>> + err = reset_control_deassert(tdma->rst);
>>> + if (err) {
>>> + dev_err(dev, "Failed to deassert reset: %d\n", err);
>>> + return err;
>>> + }
>>> +
>>> + writel_relaxed(TEGRA_AHBDMA_CMD_ENABLE, tdma->regs + TEGRA_AHBDMA_CMD);
>>> +
>>> + writel_relaxed(TEGRA_AHBDMA_IRQ_ENB_CH(0) |
>>> + TEGRA_AHBDMA_IRQ_ENB_CH(1) |
>>> + TEGRA_AHBDMA_IRQ_ENB_CH(2) |
>>> + TEGRA_AHBDMA_IRQ_ENB_CH(3),
>>> + tdma->regs + TEGRA_AHBDMA_IRQ_ENB_MASK);
>>> +
>>> + return 0;
>>> +}
>>
>> Personally I would use the pm_runtime callbacks for this sort of thing
>> and ...
>>
>
> I decided that it probaby would be better to implement PM later if needed. I'm
> not sure whether DMA controller consumes any substantial amounts of power
> while
> idling. If it's not, why bother? Unnecessary power managment would just cause
> CPU to waste its cycles (and power) doing PM.
Yes it probably does not but it is easy to do and so even though there
are probably a ton of other clocks left running, I still think it is
good practice.
Cheers
Jon
--
nvpublic
