Re: [PATCH 4/4] dmaengine: rcar-dmac: Add support for R-Car V3U
From: Vinod Koul <vkoul@kernel.org>
Date: 2021-01-12 17:05:26
Also in:
linux-devicetree, lkml
On 12-01-21, 16:54, Geert Uytterhoeven wrote:
Hi Vinod, On Tue, Jan 12, 2021 at 11:36 AM Vinod Koul [off-list ref] wrote:quoted
On 07-01-21, 19:15, Geert Uytterhoeven wrote:quoted
The DMACs (both SYS-DMAC and RT-DMAC) on R-Car V3U differ slightly from the DMACs on R-Car Gen2 and other R-Car Gen3 SoCs: 1. The per-channel registers are located in a second register block. Add support for mapping the second block, using the appropriate offsets and stride. 2. The common Channel Clear Register (DMACHCLR) was replaced by a per-channel register. Update rcar_dmac_chan_clear{,_all}() to handle this. As rcar_dmac_init() needs to clear the status before the individual channels are probed, channel index and base address initialization are moved forward. Inspired by a patch in the BSP by Phong Hoang [off-list ref]. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>quoted
quoted
--- a/drivers/dma/sh/rcar-dmac.c +++ b/drivers/dma/sh/rcar-dmac.c@@ -189,7 +189,7 @@ struct rcar_dmac_chan { * struct rcar_dmac - R-Car Gen2 DMA Controller * @engine: base DMA engine object * @dev: the hardware device - * @iomem: remapped I/O memory base + * @iomem: remapped I/O memory bases (second is optional) * @n_channels: number of available channels * @channels: array of DMAC channels * @channels_mask: bitfield of which DMA channels are managed by this driver@@ -198,7 +198,7 @@ struct rcar_dmac_chan { struct rcar_dmac { struct dma_device engine; struct device *dev; - void __iomem *iomem; + void __iomem *iomem[2];do you forsee many more memory regions, if not then why not add secondNo I don't. TBH, I didn't foresee this change either; you never know what the hardware people have on their mind for the next SoC ;-)quoted
region, that way changes in this patch will be lesser..?I did consider that option. However, doing so would imply that (a) the code to map the memory regions can no longer be a loop, but has to be unrolled manually, and (b) rcar_dmac_of_data.chan_reg_block can no longer be used to index iomem[], but needs a conditional expression or statement.quoted
and it would be better to refer to a region by its name rather than iomem[1]..- * @iomem: remapped I/O memory base + * @common_base: remapped common or combined I/O memory base + * @channel_base: remapped optional channel I/O memory base - void __iomem *iomem; + void __iomem *common_base; + void __iomem *channel_base; If you still think this is worthwhile, I can make these changes.
Either way suits me, TBH it is not a deal breaker, so i would leave it upto you :) -- ~Vinod