Hi all,
This patch series adds support for the Direct Memory Access Controller
variant in the Renesas R-Car V3U (R8A779A0) SoC, to both DT bindings and
driver.
This has been tested on the Renesas Falcon board, using external SPI
loopback (spi-loopback-test) on MSIOF1 and MSIOF2.
Thanks for your comments!
Geert Uytterhoeven (4):
dt-bindings: renesas,rcar-dmac: Add r8a779a0 support
dmaengine: rcar-dmac: Add for_each_rcar_dmac_chan() helper
dmaengine: rcar-dmac: Add helpers for clearing DMA channel status
dmaengine: rcar-dmac: Add support for R-Car V3U
.../bindings/dma/renesas,rcar-dmac.yaml | 76 ++++++++-----
drivers/dma/sh/rcar-dmac.c | 100 ++++++++++++------
2 files changed, 118 insertions(+), 58 deletions(-)
--
2.25.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Extract the code to clear the status of one or all channels into their
own helpers, to prepare for the different handling of the R-Car V3U SoC.
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
drivers/dma/sh/rcar-dmac.c | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index 71cdaf446fcaeba5..990d78849a7de704 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -337,6 +337,17 @@ static void rcar_dmac_chan_write(struct rcar_dmac_chan *chan, u32 reg, u32 data)
writel(data, chan->iomem + reg);
}
+static void rcar_dmac_chan_clear(struct rcar_dmac *dmac,
+ struct rcar_dmac_chan *chan)
+{
+ rcar_dmac_write(dmac, RCAR_DMACHCLR, BIT(chan->index));
+}
+
+static void rcar_dmac_chan_clear_all(struct rcar_dmac *dmac)
+{
+ rcar_dmac_write(dmac, RCAR_DMACHCLR, dmac->channels_mask);
+}
+
/* -----------------------------------------------------------------------------
* Initialization and configuration
*/
@@ -452,7 +463,7 @@ static int rcar_dmac_init(struct rcar_dmac *dmac)
u16 dmaor;
/* Clear all channels and enable the DMAC globally. */
- rcar_dmac_write(dmac, RCAR_DMACHCLR, dmac->channels_mask);
+ rcar_dmac_chan_clear_all(dmac);
rcar_dmac_write(dmac, RCAR_DMAOR,
RCAR_DMAOR_PRI_FIXED | RCAR_DMAOR_DME);
@@ -1567,7 +1578,7 @@ static irqreturn_t rcar_dmac_isr_channel(int irq, void *dev)
* because channel is already stopped in error case.
* We need to clear register and check DE bit as recovery.
*/
- rcar_dmac_write(dmac, RCAR_DMACHCLR, 1 << chan->index);
+ rcar_dmac_chan_clear(dmac, chan);
rcar_dmac_chcr_de_barrier(chan);
reinit = true;
goto spin_lock_end;
--
2.25.1
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
<[email protected]>.
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
drivers/dma/sh/rcar-dmac.c | 68 +++++++++++++++++++++++++++-----------
1 file changed, 49 insertions(+), 19 deletions(-)
diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index 990d78849a7de704..c11e6255eba1fc6b 100644
--- 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];
unsigned int n_channels;
struct rcar_dmac_chan *channels;
@@ -216,10 +216,12 @@ struct rcar_dmac {
/*
* struct rcar_dmac_of_data - This driver's OF data
+ * @chan_reg_block: Register block index for DMAC channels
* @chan_offset_base: DMAC channels base offset
* @chan_offset_stride: DMAC channels offset stride
*/
struct rcar_dmac_of_data {
+ unsigned int chan_reg_block;
u32 chan_offset_base;
u32 chan_offset_stride;
};
@@ -235,7 +237,7 @@ struct rcar_dmac_of_data {
#define RCAR_DMAOR_PRI_ROUND_ROBIN (3 << 8)
#define RCAR_DMAOR_AE (1 << 2)
#define RCAR_DMAOR_DME (1 << 0)
-#define RCAR_DMACHCLR 0x0080
+#define RCAR_DMACHCLR 0x0080 /* Not on R-Car V3U */
#define RCAR_DMADPSEC 0x00a0
#define RCAR_DMASAR 0x0000
@@ -298,6 +300,9 @@ struct rcar_dmac_of_data {
#define RCAR_DMAFIXDAR 0x0014
#define RCAR_DMAFIXDPBASE 0x0060
+/* For R-Car V3U */
+#define RCAR_V3U_DMACHCLR 0x0100
+
/* Hardcode the MEMCPY transfer size to 4 bytes. */
#define RCAR_DMAC_MEMCPY_XFER_SIZE 4
@@ -308,17 +313,17 @@ struct rcar_dmac_of_data {
static void rcar_dmac_write(struct rcar_dmac *dmac, u32 reg, u32 data)
{
if (reg == RCAR_DMAOR)
- writew(data, dmac->iomem + reg);
+ writew(data, dmac->iomem[0] + reg);
else
- writel(data, dmac->iomem + reg);
+ writel(data, dmac->iomem[0] + reg);
}
static u32 rcar_dmac_read(struct rcar_dmac *dmac, u32 reg)
{
if (reg == RCAR_DMAOR)
- return readw(dmac->iomem + reg);
+ return readw(dmac->iomem[0] + reg);
else
- return readl(dmac->iomem + reg);
+ return readl(dmac->iomem[0] + reg);
}
static u32 rcar_dmac_chan_read(struct rcar_dmac_chan *chan, u32 reg)
@@ -340,12 +345,23 @@ static void rcar_dmac_chan_write(struct rcar_dmac_chan *chan, u32 reg, u32 data)
static void rcar_dmac_chan_clear(struct rcar_dmac *dmac,
struct rcar_dmac_chan *chan)
{
- rcar_dmac_write(dmac, RCAR_DMACHCLR, BIT(chan->index));
+ if (dmac->iomem[1])
+ rcar_dmac_chan_write(chan, RCAR_V3U_DMACHCLR, 1);
+ else
+ rcar_dmac_write(dmac, RCAR_DMACHCLR, BIT(chan->index));
}
static void rcar_dmac_chan_clear_all(struct rcar_dmac *dmac)
{
- rcar_dmac_write(dmac, RCAR_DMACHCLR, dmac->channels_mask);
+ struct rcar_dmac_chan *chan;
+ unsigned int i;
+
+ if (dmac->iomem[1]) {
+ for_each_rcar_dmac_chan(i, chan, dmac)
+ rcar_dmac_chan_write(chan, RCAR_V3U_DMACHCLR, 1);
+ } else {
+ rcar_dmac_write(dmac, RCAR_DMACHCLR, dmac->channels_mask);
+ }
}
/* -----------------------------------------------------------------------------
@@ -1745,7 +1761,6 @@ static const struct dev_pm_ops rcar_dmac_pm = {
static int rcar_dmac_chan_probe(struct rcar_dmac *dmac,
struct rcar_dmac_chan *rchan,
- const struct rcar_dmac_of_data *data,
unsigned int index)
{
struct platform_device *pdev = to_platform_device(dmac->dev);
@@ -1754,9 +1769,6 @@ static int rcar_dmac_chan_probe(struct rcar_dmac *dmac,
char *irqname;
int ret;
- rchan->index = index;
- rchan->iomem = dmac->iomem + data->chan_offset_base +
- data->chan_offset_stride * index;
rchan->mid_rid = -EINVAL;
spin_lock_init(&rchan->lock);
@@ -1881,9 +1893,17 @@ static int rcar_dmac_probe(struct platform_device *pdev)
return -ENOMEM;
/* Request resources. */
- dmac->iomem = devm_platform_ioremap_resource(pdev, 0);
- if (IS_ERR(dmac->iomem))
- return PTR_ERR(dmac->iomem);
+ for (i = 0; i <= data->chan_reg_block; i++) {
+ dmac->iomem[i] = devm_platform_ioremap_resource(pdev, i);
+ if (IS_ERR(dmac->iomem[i]))
+ return PTR_ERR(dmac->iomem[i]);
+ }
+
+ for_each_rcar_dmac_chan(i, chan, dmac) {
+ chan->index = i;
+ chan->iomem = dmac->iomem[data->chan_reg_block] +
+ data->chan_offset_base + i * data->chan_offset_stride;
+ }
/* Enable runtime PM and initialize the device. */
pm_runtime_enable(&pdev->dev);
@@ -1930,7 +1950,7 @@ static int rcar_dmac_probe(struct platform_device *pdev)
INIT_LIST_HEAD(&engine->channels);
for_each_rcar_dmac_chan(i, chan, dmac) {
- ret = rcar_dmac_chan_probe(dmac, chan, data, i);
+ ret = rcar_dmac_chan_probe(dmac, chan, i);
if (ret < 0)
goto error;
}
@@ -1978,14 +1998,24 @@ static void rcar_dmac_shutdown(struct platform_device *pdev)
}
static const struct rcar_dmac_of_data rcar_dmac_data = {
- .chan_offset_base = 0x8000,
- .chan_offset_stride = 0x80,
+ .chan_reg_block = 0,
+ .chan_offset_base = 0x8000,
+ .chan_offset_stride = 0x80,
+};
+
+static const struct rcar_dmac_of_data rcar_v3u_dmac_data = {
+ .chan_reg_block = 1,
+ .chan_offset_base = 0x0,
+ .chan_offset_stride = 0x1000,
};
static const struct of_device_id rcar_dmac_of_ids[] = {
{
.compatible = "renesas,rcar-dmac",
.data = &rcar_dmac_data,
+ }, {
+ .compatible = "renesas,dmac-r8a779a0",
+ .data = &rcar_v3u_dmac_data,
},
{ /* Sentinel */ }
};
--
2.25.1
Document the compatible value for the Direct Memory Access Controller
blocks in the Renesas R-Car V3U (R8A779A0) SoC.
The most visible difference with DMAC blocks on other R-Car SoCs is the
move of the per-channel registers to a separate register block.
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
.../bindings/dma/renesas,rcar-dmac.yaml | 76 ++++++++++++-------
1 file changed, 48 insertions(+), 28 deletions(-)
diff --git a/Documentation/devicetree/bindings/dma/renesas,rcar-dmac.yaml b/Documentation/devicetree/bindings/dma/renesas,rcar-dmac.yaml
index c07eb6f2fc8d2f12..7f2a54bc732d3a19 100644
--- a/Documentation/devicetree/bindings/dma/renesas,rcar-dmac.yaml
+++ b/Documentation/devicetree/bindings/dma/renesas,rcar-dmac.yaml
@@ -14,34 +14,37 @@ allOf:
properties:
compatible:
- items:
- - enum:
- - renesas,dmac-r8a7742 # RZ/G1H
- - renesas,dmac-r8a7743 # RZ/G1M
- - renesas,dmac-r8a7744 # RZ/G1N
- - renesas,dmac-r8a7745 # RZ/G1E
- - renesas,dmac-r8a77470 # RZ/G1C
- - renesas,dmac-r8a774a1 # RZ/G2M
- - renesas,dmac-r8a774b1 # RZ/G2N
- - renesas,dmac-r8a774c0 # RZ/G2E
- - renesas,dmac-r8a774e1 # RZ/G2H
- - renesas,dmac-r8a7790 # R-Car H2
- - renesas,dmac-r8a7791 # R-Car M2-W
- - renesas,dmac-r8a7792 # R-Car V2H
- - renesas,dmac-r8a7793 # R-Car M2-N
- - renesas,dmac-r8a7794 # R-Car E2
- - renesas,dmac-r8a7795 # R-Car H3
- - renesas,dmac-r8a7796 # R-Car M3-W
- - renesas,dmac-r8a77961 # R-Car M3-W+
- - renesas,dmac-r8a77965 # R-Car M3-N
- - renesas,dmac-r8a77970 # R-Car V3M
- - renesas,dmac-r8a77980 # R-Car V3H
- - renesas,dmac-r8a77990 # R-Car E3
- - renesas,dmac-r8a77995 # R-Car D3
- - const: renesas,rcar-dmac
-
- reg:
- maxItems: 1
+ oneOf:
+ - items:
+ - enum:
+ - renesas,dmac-r8a7742 # RZ/G1H
+ - renesas,dmac-r8a7743 # RZ/G1M
+ - renesas,dmac-r8a7744 # RZ/G1N
+ - renesas,dmac-r8a7745 # RZ/G1E
+ - renesas,dmac-r8a77470 # RZ/G1C
+ - renesas,dmac-r8a774a1 # RZ/G2M
+ - renesas,dmac-r8a774b1 # RZ/G2N
+ - renesas,dmac-r8a774c0 # RZ/G2E
+ - renesas,dmac-r8a774e1 # RZ/G2H
+ - renesas,dmac-r8a7790 # R-Car H2
+ - renesas,dmac-r8a7791 # R-Car M2-W
+ - renesas,dmac-r8a7792 # R-Car V2H
+ - renesas,dmac-r8a7793 # R-Car M2-N
+ - renesas,dmac-r8a7794 # R-Car E2
+ - renesas,dmac-r8a7795 # R-Car H3
+ - renesas,dmac-r8a7796 # R-Car M3-W
+ - renesas,dmac-r8a77961 # R-Car M3-W+
+ - renesas,dmac-r8a77965 # R-Car M3-N
+ - renesas,dmac-r8a77970 # R-Car V3M
+ - renesas,dmac-r8a77980 # R-Car V3H
+ - renesas,dmac-r8a77990 # R-Car E3
+ - renesas,dmac-r8a77995 # R-Car D3
+ - const: renesas,rcar-dmac
+
+ - items:
+ - const: renesas,dmac-r8a779a0 # R-Car V3U
+
+ reg: true
interrupts:
minItems: 9
@@ -110,6 +113,23 @@ required:
- power-domains
- resets
+if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - renesas,dmac-r8a779a0
+then:
+ properties:
+ reg:
+ items:
+ - description: Base register block
+ - description: Channel register block
+else:
+ properties:
+ reg:
+ maxItems: 1
+
additionalProperties: false
examples:
--
2.25.1
Add and helper macro for iterating over all DMAC channels, taking into
account the channel mask. Use it where appropriate, to simplify code.
Restore "reverse Christmas tree" order of local variables while adding a
new variable.
Signed-off-by: Geert Uytterhoeven <[email protected]>
---
drivers/dma/sh/rcar-dmac.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index a57705356e8bb796..71cdaf446fcaeba5 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -209,6 +209,11 @@ struct rcar_dmac {
#define to_rcar_dmac(d) container_of(d, struct rcar_dmac, engine)
+#define for_each_rcar_dmac_chan(i, chan, dmac) \
+ for (i = 0, chan = &(dmac)->channels[0]; i < (dmac)->n_channels; \
+ i++, chan++) \
+ if (!((dmac)->channels_mask & BIT(i))) continue; else
+
/*
* struct rcar_dmac_of_data - This driver's OF data
* @chan_offset_base: DMAC channels base offset
@@ -817,15 +822,11 @@ static void rcar_dmac_chan_reinit(struct rcar_dmac_chan *chan)
static void rcar_dmac_stop_all_chan(struct rcar_dmac *dmac)
{
+ struct rcar_dmac_chan *chan;
unsigned int i;
/* Stop all channels. */
- for (i = 0; i < dmac->n_channels; ++i) {
- struct rcar_dmac_chan *chan = &dmac->channels[i];
-
- if (!(dmac->channels_mask & BIT(i)))
- continue;
-
+ for_each_rcar_dmac_chan(i, chan, dmac) {
/* Stop and reinitialize the channel. */
spin_lock_irq(&chan->lock);
rcar_dmac_chan_halt(chan);
@@ -1828,9 +1829,10 @@ static int rcar_dmac_probe(struct platform_device *pdev)
DMA_SLAVE_BUSWIDTH_2_BYTES | DMA_SLAVE_BUSWIDTH_4_BYTES |
DMA_SLAVE_BUSWIDTH_8_BYTES | DMA_SLAVE_BUSWIDTH_16_BYTES |
DMA_SLAVE_BUSWIDTH_32_BYTES | DMA_SLAVE_BUSWIDTH_64_BYTES;
+ const struct rcar_dmac_of_data *data;
+ struct rcar_dmac_chan *chan;
struct dma_device *engine;
struct rcar_dmac *dmac;
- const struct rcar_dmac_of_data *data;
unsigned int i;
int ret;
@@ -1916,11 +1918,8 @@ static int rcar_dmac_probe(struct platform_device *pdev)
INIT_LIST_HEAD(&engine->channels);
- for (i = 0; i < dmac->n_channels; ++i) {
- if (!(dmac->channels_mask & BIT(i)))
- continue;
-
- ret = rcar_dmac_chan_probe(dmac, &dmac->channels[i], data, i);
+ for_each_rcar_dmac_chan(i, chan, dmac) {
+ ret = rcar_dmac_chan_probe(dmac, chan, data, i);
if (ret < 0)
goto error;
}
--
2.25.1
On 07-01-21, 19:15, Geert Uytterhoeven wrote:
> Add and helper macro for iterating over all DMAC channels, taking into
> account the channel mask. Use it where appropriate, to simplify code.
>
> Restore "reverse Christmas tree" order of local variables while adding a
> new variable.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> drivers/dma/sh/rcar-dmac.c | 23 +++++++++++------------
> 1 file changed, 11 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
> index a57705356e8bb796..71cdaf446fcaeba5 100644
> --- a/drivers/dma/sh/rcar-dmac.c
> +++ b/drivers/dma/sh/rcar-dmac.c
> @@ -209,6 +209,11 @@ struct rcar_dmac {
>
> #define to_rcar_dmac(d) container_of(d, struct rcar_dmac, engine)
>
> +#define for_each_rcar_dmac_chan(i, chan, dmac) \
> + for (i = 0, chan = &(dmac)->channels[0]; i < (dmac)->n_channels; \
> + i++, chan++) \
single line to make it more readable? we have limit of 100 now :)
> + if (!((dmac)->channels_mask & BIT(i))) continue; else
> +
> /*
> * struct rcar_dmac_of_data - This driver's OF data
> * @chan_offset_base: DMAC channels base offset
> @@ -817,15 +822,11 @@ static void rcar_dmac_chan_reinit(struct rcar_dmac_chan *chan)
>
> static void rcar_dmac_stop_all_chan(struct rcar_dmac *dmac)
> {
> + struct rcar_dmac_chan *chan;
> unsigned int i;
>
> /* Stop all channels. */
> - for (i = 0; i < dmac->n_channels; ++i) {
> - struct rcar_dmac_chan *chan = &dmac->channels[i];
> -
> - if (!(dmac->channels_mask & BIT(i)))
> - continue;
> -
> + for_each_rcar_dmac_chan(i, chan, dmac) {
> /* Stop and reinitialize the channel. */
> spin_lock_irq(&chan->lock);
> rcar_dmac_chan_halt(chan);
> @@ -1828,9 +1829,10 @@ static int rcar_dmac_probe(struct platform_device *pdev)
> DMA_SLAVE_BUSWIDTH_2_BYTES | DMA_SLAVE_BUSWIDTH_4_BYTES |
> DMA_SLAVE_BUSWIDTH_8_BYTES | DMA_SLAVE_BUSWIDTH_16_BYTES |
> DMA_SLAVE_BUSWIDTH_32_BYTES | DMA_SLAVE_BUSWIDTH_64_BYTES;
> + const struct rcar_dmac_of_data *data;
> + struct rcar_dmac_chan *chan;
> struct dma_device *engine;
> struct rcar_dmac *dmac;
> - const struct rcar_dmac_of_data *data;
> unsigned int i;
> int ret;
>
> @@ -1916,11 +1918,8 @@ static int rcar_dmac_probe(struct platform_device *pdev)
>
> INIT_LIST_HEAD(&engine->channels);
>
> - for (i = 0; i < dmac->n_channels; ++i) {
> - if (!(dmac->channels_mask & BIT(i)))
> - continue;
> -
> - ret = rcar_dmac_chan_probe(dmac, &dmac->channels[i], data, i);
> + for_each_rcar_dmac_chan(i, chan, dmac) {
> + ret = rcar_dmac_chan_probe(dmac, chan, data, i);
> if (ret < 0)
> goto error;
> }
> --
> 2.25.1
--
~Vinod
Hi Vinod,
On Tue, Jan 12, 2021 at 11:19 AM Vinod Koul <[email protected]> wrote:
> On 07-01-21, 19:15, Geert Uytterhoeven wrote:
> > Add and helper macro for iterating over all DMAC channels, taking into
> > account the channel mask. Use it where appropriate, to simplify code.
> >
> > Restore "reverse Christmas tree" order of local variables while adding a
> > new variable.
> >
> > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > --- a/drivers/dma/sh/rcar-dmac.c
> > +++ b/drivers/dma/sh/rcar-dmac.c
> > @@ -209,6 +209,11 @@ struct rcar_dmac {
> >
> > #define to_rcar_dmac(d) container_of(d, struct rcar_dmac, engine)
> >
> > +#define for_each_rcar_dmac_chan(i, chan, dmac) \
> > + for (i = 0, chan = &(dmac)->channels[0]; i < (dmac)->n_channels; \
> > + i++, chan++) \
>
> single line to make it more readable? we have limit of 100 now :)
Do we have to push the limits?
BTW, the new punched cards are 96-column wide, not 100-column ;-)
https://en.wikipedia.org/wiki/Punched_card#IBM_96-column_format
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On 07-01-21, 19:15, Geert Uytterhoeven wrote:
> 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
> <[email protected]>.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> drivers/dma/sh/rcar-dmac.c | 68 +++++++++++++++++++++++++++-----------
> 1 file changed, 49 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
> index 990d78849a7de704..c11e6255eba1fc6b 100644
> --- 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 second
region, that way changes in this patch will be lesser..?
and it would be better to refer to a region by its name rather than
iomem[1]..
--
~Vinod
On 12-01-21, 11:26, Geert Uytterhoeven wrote:
> Hi Vinod,
>
> On Tue, Jan 12, 2021 at 11:19 AM Vinod Koul <[email protected]> wrote:
> > On 07-01-21, 19:15, Geert Uytterhoeven wrote:
> > > Add and helper macro for iterating over all DMAC channels, taking into
> > > account the channel mask. Use it where appropriate, to simplify code.
> > >
> > > Restore "reverse Christmas tree" order of local variables while adding a
> > > new variable.
> > >
> > > Signed-off-by: Geert Uytterhoeven <[email protected]>
>
> > > --- a/drivers/dma/sh/rcar-dmac.c
> > > +++ b/drivers/dma/sh/rcar-dmac.c
> > > @@ -209,6 +209,11 @@ struct rcar_dmac {
> > >
> > > #define to_rcar_dmac(d) container_of(d, struct rcar_dmac, engine)
> > >
> > > +#define for_each_rcar_dmac_chan(i, chan, dmac) \
> > > + for (i = 0, chan = &(dmac)->channels[0]; i < (dmac)->n_channels; \
> > > + i++, chan++) \
> >
> > single line to make it more readable? we have limit of 100 now :)
>
> Do we have to push the limits?
In cases where it helps, I certainly recommend.. I feel in this case it
makes a better read to have it in a single line..
> BTW, the new punched cards are 96-column wide, not 100-column ;-)
> https://en.wikipedia.org/wiki/Punched_card#IBM_96-column_format
Did we err in choosing 100 :D
--
~Vinod
Hi Vinod,
On Tue, Jan 12, 2021 at 11:38 AM Vinod Koul <[email protected]> wrote:
> On 12-01-21, 11:26, Geert Uytterhoeven wrote:
> > On Tue, Jan 12, 2021 at 11:19 AM Vinod Koul <[email protected]> wrote:
> > > On 07-01-21, 19:15, Geert Uytterhoeven wrote:
> > > > Add and helper macro for iterating over all DMAC channels, taking into
> > > > account the channel mask. Use it where appropriate, to simplify code.
> > > >
> > > > Restore "reverse Christmas tree" order of local variables while adding a
> > > > new variable.
> > > >
> > > > Signed-off-by: Geert Uytterhoeven <[email protected]>
> >
> > > > --- a/drivers/dma/sh/rcar-dmac.c
> > > > +++ b/drivers/dma/sh/rcar-dmac.c
> > > > @@ -209,6 +209,11 @@ struct rcar_dmac {
> > > >
> > > > #define to_rcar_dmac(d) container_of(d, struct rcar_dmac, engine)
> > > >
> > > > +#define for_each_rcar_dmac_chan(i, chan, dmac) \
> > > > + for (i = 0, chan = &(dmac)->channels[0]; i < (dmac)->n_channels; \
> > > > + i++, chan++) \
> > >
> > > single line to make it more readable? we have limit of 100 now :)
> >
> > Do we have to push the limits?
>
> In cases where it helps, I certainly recommend.. I feel in this case it
> makes a better read to have it in a single line..
OK, will fix.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Vinod,
On Tue, Jan 12, 2021 at 11:36 AM Vinod Koul <[email protected]> wrote:
> On 07-01-21, 19:15, Geert Uytterhoeven wrote:
> > 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
> > <[email protected]>.
> >
> > Signed-off-by: Geert Uytterhoeven <[email protected]>
> > --- 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 second
No 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 ;-)
> 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.
> 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.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
CC linux-renesas-soc
On Thu, Jan 7, 2021 at 7:15 PM Geert Uytterhoeven
<[email protected]> wrote:
> This patch series adds support for the Direct Memory Access Controller
> variant in the Renesas R-Car V3U (R8A779A0) SoC, to both DT bindings and
> driver.
>
> This has been tested on the Renesas Falcon board, using external SPI
> loopback (spi-loopback-test) on MSIOF1 and MSIOF2.
>
> Thanks for your comments!
>
> Geert Uytterhoeven (4):
> dt-bindings: renesas,rcar-dmac: Add r8a779a0 support
> dmaengine: rcar-dmac: Add for_each_rcar_dmac_chan() helper
> dmaengine: rcar-dmac: Add helpers for clearing DMA channel status
> dmaengine: rcar-dmac: Add support for R-Car V3U
>
> .../bindings/dma/renesas,rcar-dmac.yaml | 76 ++++++++-----
> drivers/dma/sh/rcar-dmac.c | 100 ++++++++++++------
> 2 files changed, 118 insertions(+), 58 deletions(-)
>
> --
> 2.25.1
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
On 12-01-21, 16:54, Geert Uytterhoeven wrote:
> Hi Vinod,
>
> On Tue, Jan 12, 2021 at 11:36 AM Vinod Koul <[email protected]> wrote:
> > On 07-01-21, 19:15, Geert Uytterhoeven wrote:
> > > 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
> > > <[email protected]>.
> > >
> > > Signed-off-by: Geert Uytterhoeven <[email protected]>
>
> > > --- 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 second
>
> No 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 ;-)
>
> > 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.
>
> > 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
On Thu, 07 Jan 2021 19:15:21 +0100, Geert Uytterhoeven wrote:
> Document the compatible value for the Direct Memory Access Controller
> blocks in the Renesas R-Car V3U (R8A779A0) SoC.
>
> The most visible difference with DMAC blocks on other R-Car SoCs is the
> move of the per-channel registers to a separate register block.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> .../bindings/dma/renesas,rcar-dmac.yaml | 76 ++++++++++++-------
> 1 file changed, 48 insertions(+), 28 deletions(-)
>
Reviewed-by: Rob Herring <[email protected]>
Hi Vinod,
On Tue, Jan 12, 2021 at 6:04 PM Vinod Koul <[email protected]> wrote:
> On 12-01-21, 16:54, Geert Uytterhoeven wrote:
> > On Tue, Jan 12, 2021 at 11:36 AM Vinod Koul <[email protected]> wrote:
> > > On 07-01-21, 19:15, Geert Uytterhoeven wrote:
> > > > 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
> > > > <[email protected]>.
> > > >
> > > > Signed-off-by: Geert Uytterhoeven <[email protected]>
> >
> > > > --- 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 second
> >
> > No 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 ;-)
> >
> > > 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.
> >
> > > 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 :)
I managed to use named regions at the expense of only 6 more lines of
source code, even reducing the resulting binary size.
So stay ready for v2 ;-)
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds