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 <digetx@xxxxxxxxx> >>> --- >>> 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 <digetx@xxxxxxxxx> >>> + * >>> + * 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html