> -----Original Message-----
> From: Biju Das
> Sent: 22 February 2025 08:06
> To: 'Fabrizio Castro' <fabrizio.castro.jz@xxxxxxxxxxx>; Vinod Koul <vkoul@xxxxxxxxxx>; Geert
> Uytterhoeven <geert+renesas@xxxxxxxxx>
> Cc: Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx>; Magnus Damm <magnus.damm@xxxxxxxxx>; Wolfram
> Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>; Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxx>;
> dmaengine@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-renesas-soc@xxxxxxxxxxxxxxx; Prabhakar
> Mahadev Lad <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> Subject: RE: [PATCH v4 6/7] dmaengine: sh: rz-dmac: Add RZ/V2H(P) support
>
> Hi Fabrizio Castro,
>
> > -----Original Message-----
> > From: Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx>
> > Sent: 20 February 2025 15:01
> > Subject: [PATCH v4 6/7] dmaengine: sh: rz-dmac: Add RZ/V2H(P) support
> >
> > The DMAC IP found on the Renesas RZ/V2H(P) family of SoCs is similar
> > to the version found on the Renesas RZ/G2L family of SoCs, but there are some differences:
> > * It only uses one register area
> > * It only uses one clock
> > * It only uses one reset
> > * Instead of using MID/IRD it uses REQ NO/ACK NO
> > * It is connected to the Interrupt Control Unit (ICU)
> > * On the RZ/G2L there is only 1 DMAC, on the RZ/V2H(P) there are 5
> >
> > Add specific support for the Renesas RZ/V2H(P) family of SoC by
> > tackling the aforementioned differences.
> >
> > Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx>
> > ---
> > v3->v4:
> > * Fixed an issue with mid_rid/req_no/ack_no initialization
> > v2->v3:
> > * Dropped change to Kconfig.
> > * Replaced rz_dmac_type with has_icu flag.
> > * Put req_no and ack_no in an anonymous struct, nested under an
> > anonymous union with mid_rid.
> > * Dropped data field of_rz_dmac_match[], and added logic to determine
> > value of has_icu flag from DT parsing.
> > v1->v2:
> > * Switched to new macros for minimum values.
> > ---
> > drivers/dma/sh/rz-dmac.c | 162
> > +++++++++++++++++++++++++++++++++++----
> > 1 file changed, 146 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c index
> > d7a4ce28040b..57a1fdeed734
> > 100644
> > --- a/drivers/dma/sh/rz-dmac.c
> > +++ b/drivers/dma/sh/rz-dmac.c
> > @@ -14,6 +14,7 @@
> > #include <linux/dmaengine.h>
> > #include <linux/interrupt.h>
> > #include <linux/iopoll.h>
> > +#include <linux/irqchip/irq-renesas-rzv2h.h>
> > #include <linux/list.h>
> > #include <linux/module.h>
> > #include <linux/of.h>
> > @@ -73,7 +74,6 @@ struct rz_dmac_chan {
> >
> > u32 chcfg;
> > u32 chctrl;
> > - int mid_rid;
> >
> > struct list_head ld_free;
> > struct list_head ld_queue;
> > @@ -85,20 +85,36 @@ struct rz_dmac_chan {
> > struct rz_lmdesc *tail;
> > dma_addr_t base_dma;
> > } lmdesc;
> > +
> > + union {
> > + int mid_rid;
> > + struct {
> > + u16 req_no;
> > + u8 ack_no;
> > + };
> > + };
> > };
> >
> > #define to_rz_dmac_chan(c) container_of(c, struct rz_dmac_chan, vc.chan)
> >
> > +struct rz_dmac_icu {
> > + struct platform_device *pdev;
> > + u8 dmac_index;
> > +};
> > +
> > struct rz_dmac {
> > struct dma_device engine;
> > struct device *dev;
> > struct reset_control *rstc;
> > + struct rz_dmac_icu icu;
> > void __iomem *base;
> > void __iomem *ext_base;
> >
> > unsigned int n_channels;
> > struct rz_dmac_chan *channels;
> >
> > + bool has_icu;
> > +
> > DECLARE_BITMAP(modules, 1024);
> > };
> >
> > @@ -167,6 +183,23 @@ struct rz_dmac {
> > #define RZ_DMAC_MAX_CHANNELS 16
> > #define DMAC_NR_LMDESC 64
> >
> > +/* RZ/V2H ICU related */
> > +#define RZV2H_REQ_NO_MASK GENMASK(9, 0)
> > +#define RZV2H_ACK_NO_MASK GENMASK(16, 10)
> > +#define RZV2H_HIEN_MASK BIT(17)
> > +#define RZV2H_LVL_MASK BIT(18)
> > +#define RZV2H_AM_MASK GENMASK(21, 19)
> > +#define RZV2H_TM_MASK BIT(22)
> > +#define RZV2H_EXTRACT_REQ_NO(x) FIELD_GET(RZV2H_REQ_NO_MASK, (x))
> > +#define RZV2H_EXTRACT_ACK_NO(x) FIELD_GET(RZV2H_ACK_NO_MASK, (x))
> > +#define RZVH2_EXTRACT_CHCFG(x) ((FIELD_GET(RZV2H_HIEN_MASK, (x)) << 5) | \
> > + (FIELD_GET(RZV2H_LVL_MASK, (x)) << 6) | \
> > + (FIELD_GET(RZV2H_AM_MASK, (x)) << 8) | \
> > + (FIELD_GET(RZV2H_TM_MASK, (x)) << 22))
> > +
> > +#define RZV2H_MAX_DMAC_INDEX 4
> > +#define RZV2H_ICU_PROPERTY "renesas,icu"
> > +
> > /*
> > * -----------------------------------------------------------------------------
> > * Device access
> > @@ -324,7 +357,15 @@ static void rz_dmac_prepare_desc_for_memcpy(struct rz_dmac_chan *channel)
> > lmdesc->chext = 0;
> > lmdesc->header = HEADER_LV;
> >
> > - rz_dmac_set_dmars_register(dmac, channel->index, 0);
> > + if (!dmac->has_icu) {
> > + rz_dmac_set_dmars_register(dmac, channel->index, 0);
> > + } else {
> > + rzv2h_icu_register_dma_req_ack(dmac->icu.pdev,
> > + dmac->icu.dmac_index,
> > + channel->index,
> > + RZV2H_ICU_DMAC_REQ_NO_DEFAULT,
> > + RZV2H_ICU_DMAC_ACK_NO_DEFAULT);
> > + }
> >
> > channel->chcfg = chcfg;
> > channel->chctrl = CHCTRL_STG | CHCTRL_SETEN; @@ -375,7 +416,15 @@
> > static void rz_dmac_prepare_descs_for_slave_sg(struct rz_dmac_chan
> > *channel)
> >
> > channel->lmdesc.tail = lmdesc;
> >
> > - rz_dmac_set_dmars_register(dmac, channel->index, channel->mid_rid);
> > + if (!dmac->has_icu) {
> > + rz_dmac_set_dmars_register(dmac, channel->index, channel->mid_rid);
> > + } else {
> > + rzv2h_icu_register_dma_req_ack(dmac->icu.pdev,
> > + dmac->icu.dmac_index,
> > + channel->index, channel->req_no,
> > + channel->ack_no);
> > + }
> > +
> > channel->chctrl = CHCTRL_SETEN;
> > }
> >
> > @@ -452,9 +501,15 @@ static void rz_dmac_free_chan_resources(struct dma_chan *chan)
> > list_splice_tail_init(&channel->ld_active, &channel->ld_free);
> > list_splice_tail_init(&channel->ld_queue, &channel->ld_free);
> >
> > - if (channel->mid_rid >= 0) {
> > - clear_bit(channel->mid_rid, dmac->modules);
> > - channel->mid_rid = -EINVAL;
> > + if (!dmac->has_icu) {
> > + if (channel->mid_rid >= 0) {
> > + clear_bit(channel->mid_rid, dmac->modules);
> > + channel->mid_rid = -EINVAL;
> > + }
> > + } else {
> > + clear_bit(channel->req_no, dmac->modules);
> > + channel->req_no = RZV2H_ICU_DMAC_REQ_NO_DEFAULT;
> > + channel->ack_no = RZV2H_ICU_DMAC_ACK_NO_DEFAULT;
> > }
> >
> > spin_unlock_irqrestore(&channel->vc.lock, flags); @@ -647,7 +702,15
> > @@ static void rz_dmac_device_synchronize(struct dma_chan *chan)
> > if (ret < 0)
> > dev_warn(dmac->dev, "DMA Timeout");
> >
> > - rz_dmac_set_dmars_register(dmac, channel->index, 0);
> > + if (!dmac->has_icu) {
> > + rz_dmac_set_dmars_register(dmac, channel->index, 0);
> > + } else {
> > + rzv2h_icu_register_dma_req_ack(dmac->icu.pdev,
> > + dmac->icu.dmac_index,
> > + channel->index,
> > + RZV2H_ICU_DMAC_REQ_NO_DEFAULT,
> > + RZV2H_ICU_DMAC_ACK_NO_DEFAULT);
> > + }
> > }
> >
> > /*
> > @@ -727,13 +790,30 @@ static bool rz_dmac_chan_filter(struct dma_chan *chan, void *arg)
> > struct rz_dmac *dmac = to_rz_dmac(chan->device);
> > struct of_phandle_args *dma_spec = arg;
> > u32 ch_cfg;
> > + u16 req_no;
> > +
> > + if (!dmac->has_icu) {
> > + channel->mid_rid = dma_spec->args[0] & MID_RID_MASK;
> > + ch_cfg = (dma_spec->args[0] & CHCFG_MASK) >> 10;
> > + channel->chcfg = CHCFG_FILL_TM(ch_cfg) | CHCFG_FILL_AM(ch_cfg) |
> > + CHCFG_FILL_LVL(ch_cfg) | CHCFG_FILL_HIEN(ch_cfg);
> > +
> > + return !test_and_set_bit(channel->mid_rid, dmac->modules);
> > + }
> > +
> > + req_no = RZV2H_EXTRACT_REQ_NO(dma_spec->args[0]);
> > + if (req_no >= RZV2H_ICU_DMAC_REQ_NO_MIN_FIX_OUTPUT)
> > + return false;
> > +
> > + channel->req_no = req_no;
> > +
> > + channel->ack_no = RZV2H_EXTRACT_ACK_NO(dma_spec->args[0]);
> > + if (channel->ack_no >= RZV2H_ICU_DMAC_ACK_NO_MIN_FIX_OUTPUT)
> > + channel->ack_no = RZV2H_ICU_DMAC_ACK_NO_DEFAULT;
> >
> > - channel->mid_rid = dma_spec->args[0] & MID_RID_MASK;
> > - ch_cfg = (dma_spec->args[0] & CHCFG_MASK) >> 10;
> > - channel->chcfg = CHCFG_FILL_TM(ch_cfg) | CHCFG_FILL_AM(ch_cfg) |
> > - CHCFG_FILL_LVL(ch_cfg) | CHCFG_FILL_HIEN(ch_cfg);
> > + channel->chcfg = RZVH2_EXTRACT_CHCFG(dma_spec->args[0]);
>
> Looks like a typo?? RZVH2_EXTRACT_CHCFG-> RZVH2_EXTRACT_CHCFG
Oops.
Looks like a typo?? RZVH2_*-> RZV2H_*
Cheers,
Biju
