Hi Fabrizio, On Thu, 20 Feb 2025 at 16:01, Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx> wrote: > On the Renesas RZ/V2H(P) family of SoCs, DMAC IPs are connected > to the Interrupt Control Unit (ICU). > For DMA transfers, a request number and an ack number must be > registered with the ICU, which means that the DMAC driver has > to be able to instruct the ICU driver with the registration of > such ids. > > Export rzv2h_icu_register_dma_req_ack() so that the DMA driver > can register both ids in one go. > > Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx> > Reviewed-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx> > --- > v3->v4: > * No change. > v2->v3: > * Replaced rzv2h_icu_register_dma_req_ack with > rzv2h_icu_register_dma_req_ack() in changelog. > * Added dummy for rzv2h_icu_register_dma_req_ack(). > * Added Rb Thomas. Thanks for the update! > --- a/drivers/irqchip/irq-renesas-rzv2h.c > +++ b/drivers/irqchip/irq-renesas-rzv2h.c > @@ -94,6 +110,45 @@ struct rzv2h_icu_priv { > raw_spinlock_t lock; > }; > > +void rzv2h_icu_register_dma_req_ack(struct platform_device *icu_dev, u8 dmac_index, u8 dmac_channel, > + u16 req_no, u8 ack_no) > +{ > + struct rzv2h_icu_priv *priv = platform_get_drvdata(icu_dev); > + u32 icu_dmackselk, dmaack, dmaack_mask; > + u32 icu_dmksely, dmareq, dmareq_mask; > + u8 k, field_no; > + u8 y, upper; > + > + if (req_no >= RZV2H_ICU_DMAC_REQ_NO_MIN_FIX_OUTPUT) > + req_no = RZV2H_ICU_DMAC_REQ_NO_DEFAULT; What is the purpose of this check? The hardware register field size is 10 bits, so I think it is better to just limit it to 0x3ff. Checking for RZV2H_ICU_DMAC_REQ_NO_MIN_FIX_OUTPUT means you will have to update this check when a new SoC supports higher values than 0x1b5. > + > + if (ack_no >= RZV2H_ICU_DMAC_ACK_NO_MIN_FIX_OUTPUT) > + ack_no = RZV2H_ICU_DMAC_ACK_NO_DEFAULT; What is the purpose of this check? There are only 23 DMACKSELk registers, so using RZV2H_ICU_DMAC_ACK_NO_DEFAULT = 0x7f will write beyond the last register below. And drivers/dma/sh/rz-dmac.c does call this function with req_no = RZV2H_ICU_DMAC_REQ_NO_DEFAULT and ack_no = RZV2H_ICU_DMAC_ACK_NO_DEFAULT... > + > + y = dmac_channel / 2; > + upper = dmac_channel % 2; > + > + dmareq = ICU_DMAC_PREP_DMAREQ(req_no, upper); > + dmareq_mask = ICU_DMAC_DMAREQ_MASK(upper); > + > + k = ack_no / 4; > + field_no = ack_no % 4; > + > + dmaack_mask = ICU_DMAC_DACK_SEL_MASK(field_no); > + dmaack = ICU_DMAC_PREP_DACK_SEL(ack_no, field_no); > + > + guard(raw_spinlock_irqsave)(&priv->lock); > + > + icu_dmksely = readl(priv->base + ICU_DMkSELy(dmac_index, y)); > + icu_dmksely = (icu_dmksely & ~dmareq_mask) | dmareq; > + writel(icu_dmksely, priv->base + ICU_DMkSELy(dmac_index, y)); > + > + icu_dmackselk = readl(priv->base + ICU_DMACKSELk(k)); > + icu_dmackselk = (icu_dmackselk & ~dmaack_mask) | dmaack; > + writel(icu_dmackselk, priv->base + ICU_DMACKSELk(k)); > +} > +EXPORT_SYMBOL_GPL(rzv2h_icu_register_dma_req_ack); > + > static inline struct rzv2h_icu_priv *irq_data_to_priv(struct irq_data *data) > { > return data->domain->host_data; Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx 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