[PATCH 4/7] irqchip/renesas-rzv2h: Add rzv2h_icu_register_dma_req_ack

Fabrizio Castro posted 7 patches 1 year ago
There is a newer version of this series
[PATCH 4/7] irqchip/renesas-rzv2h: Add rzv2h_icu_register_dma_req_ack
Posted by Fabrizio Castro 1 year ago
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@renesas.com>
---
 drivers/irqchip/irq-renesas-rzv2h.c       | 61 +++++++++++++++++++++++
 include/linux/irqchip/irq-renesas-rzv2h.h | 19 +++++++
 2 files changed, 80 insertions(+)
 create mode 100644 include/linux/irqchip/irq-renesas-rzv2h.h

diff --git a/drivers/irqchip/irq-renesas-rzv2h.c b/drivers/irqchip/irq-renesas-rzv2h.c
index fe2d29e91026..0fdbf9ce89a9 100644
--- a/drivers/irqchip/irq-renesas-rzv2h.c
+++ b/drivers/irqchip/irq-renesas-rzv2h.c
@@ -15,6 +15,7 @@
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/irqchip.h>
+#include <linux/irqchip/irq-renesas-rzv2h.h>
 #include <linux/irqdomain.h>
 #include <linux/of_address.h>
 #include <linux/of_platform.h>
@@ -41,6 +42,8 @@
 #define ICU_TSCLR				0x24
 #define ICU_TITSR(k)				(0x28 + (k) * 4)
 #define ICU_TSSR(k)				(0x30 + (k) * 4)
+#define ICU_DMkSELy(k, y)			(0x420 + (k) * 0x20 + (y) * 4)
+#define ICU_DMACKSELk(k)			(0x500 + (k) * 4)
 
 /* NMI */
 #define ICU_NMI_EDGE_FALLING			0
@@ -80,6 +83,19 @@
 #define ICU_TINT_EXTRACT_GPIOINT(x)		FIELD_GET(GENMASK(31, 16), (x))
 #define ICU_PB5_TINT				0x55
 
+/* DMAC */
+#define ICU_DMAC_DkSEL_CLRON_MASK		BIT(15)
+#define ICU_DMAC_DkRQ_SEL_MASK			GENMASK(9, 0)
+#define ICU_DMAC_DMAREQ_MASK			(ICU_DMAC_DkRQ_SEL_MASK | \
+						 ICU_DMAC_DkSEL_CLRON_MASK)
+
+#define ICU_DMAC_PREP_DkSEL_CLRON(x)		FIELD_PREP(ICU_DMAC_DkSEL_CLRON_MASK, (x))
+#define ICU_DMAC_PREP_DkRQ_SEL(x)		FIELD_PREP(ICU_DMAC_DkRQ_SEL_MASK, (x))
+#define ICU_DMAC_PREP_DMAREQ(sel, clr)		(ICU_DMAC_PREP_DkRQ_SEL(sel) | \
+						ICU_DMAC_PREP_DkSEL_CLRON(clr))
+
+#define ICU_DMAC_DACK_SEL_MASK			GENMASK(6, 0)
+
 /**
  * struct rzv2h_icu_priv - Interrupt Control Unit controller private data structure.
  * @base:	Controller's base address
@@ -94,6 +110,50 @@ 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 >= 0x1b5)
+		req_no = RZV2H_ICU_DMAC_REQ_NO_DEFAULT;
+
+	if (ack_no >= 0x50)
+		ack_no = RZV2H_ICU_DMAC_ACK_NO_DEFAULT;
+
+	y = dmac_channel / 2;
+	upper = dmac_channel % 2;
+
+	dmareq = ICU_DMAC_PREP_DMAREQ(req_no, 0);
+	dmareq_mask = ICU_DMAC_DMAREQ_MASK;
+
+	if (upper) {
+		dmareq <<= 16;
+		dmareq_mask <<= 16;
+	}
+
+	k  = ack_no / 4;
+	field_no = ack_no % 4;
+
+	dmaack_mask = ICU_DMAC_DACK_SEL_MASK << (field_no * 8);
+	dmaack = ack_no << (field_no * 8);
+
+	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;
@@ -446,6 +506,7 @@ static int rzv2h_icu_init(struct device_node *node, struct device_node *parent)
 		goto put_dev;
 	}
 
+	platform_set_drvdata(pdev, rzv2h_icu_data);
 	rzv2h_icu_data->irqchip = &rzv2h_icu_chip;
 
 	rzv2h_icu_data->base = devm_of_iomap(&pdev->dev, pdev->dev.of_node, 0, NULL);
diff --git a/include/linux/irqchip/irq-renesas-rzv2h.h b/include/linux/irqchip/irq-renesas-rzv2h.h
new file mode 100644
index 000000000000..686d050a239a
--- /dev/null
+++ b/include/linux/irqchip/irq-renesas-rzv2h.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Renesas RZ/V2H(P) Interrupt Control Unit (ICU)
+ *
+ * Copyright (C) 2025 Renesas Electronics Corporation.
+ */
+
+#ifndef __LINUX_IRQ_RENESAS_RZV2H
+#define __LINUX_IRQ_RENESAS_RZV2H
+
+#include <linux/platform_device.h>
+
+#define RZV2H_ICU_DMAC_REQ_NO_DEFAULT		0x3ff
+#define RZV2H_ICU_DMAC_ACK_NO_DEFAULT		0x7f
+
+void rzv2h_icu_register_dma_req_ack(struct platform_device *icu_dev, u8 dmac_index, u8 dmac_channel,
+				    u16 req_no, u8 ack_no);
+
+#endif
-- 
2.34.1
Re: [PATCH 4/7] irqchip/renesas-rzv2h: Add rzv2h_icu_register_dma_req_ack
Posted by Thomas Gleixner 1 year ago
On Thu, Feb 06 2025 at 22:03, Fabrizio Castro wrote:

https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#function-references-in-changelogs

> On the Renesas RZ/V2H(P) family of SoCs, DMAC IPs are connected
> to the Interrupt Control Unit (ICU).
> +#define ICU_DMkSELy(k, y)			(0x420 + (k) * 0x20 + (y) * 4)
> +#define ICU_DMACKSELk(k)			(0x500 + (k) * 4)
>  
>  /* NMI */
>  #define ICU_NMI_EDGE_FALLING			0
> @@ -80,6 +83,19 @@
>  #define ICU_TINT_EXTRACT_GPIOINT(x)		FIELD_GET(GENMASK(31, 16), (x))
>  #define ICU_PB5_TINT				0x55
>  
> +/* DMAC */
> +#define ICU_DMAC_DkSEL_CLRON_MASK		BIT(15)
> +#define ICU_DMAC_DkRQ_SEL_MASK			GENMASK(9, 0)
> +#define ICU_DMAC_DMAREQ_MASK			(ICU_DMAC_DkRQ_SEL_MASK | \
> +						 ICU_DMAC_DkSEL_CLRON_MASK)
> +
> +#define ICU_DMAC_PREP_DkSEL_CLRON(x)		FIELD_PREP(ICU_DMAC_DkSEL_CLRON_MASK, (x))
> +#define ICU_DMAC_PREP_DkRQ_SEL(x)		FIELD_PREP(ICU_DMAC_DkRQ_SEL_MASK, (x))
> +#define ICU_DMAC_PREP_DMAREQ(sel, clr)		(ICU_DMAC_PREP_DkRQ_SEL(sel) | \
> +						ICU_DMAC_PREP_DkSEL_CLRON(clr))

That's a pretty convoluted way to create a mask whihc has the CLRON bit
always set to 0 according to the only usage site.

> +#define ICU_DMAC_DACK_SEL_MASK			GENMASK(6, 0)

> +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 >= 0x1b5)

In the DMA part you use proper defines for this, but here you put magic
numbers into the code. Please share the defines and use them consistently.

> +		req_no = RZV2H_ICU_DMAC_REQ_NO_DEFAULT;
> +
> +	if (ack_no >= 0x50)
> +		ack_no = RZV2H_ICU_DMAC_ACK_NO_DEFAULT;
> +
> +	y = dmac_channel / 2;
> +	upper = dmac_channel % 2;
> +
> +	dmareq = ICU_DMAC_PREP_DMAREQ(req_no, 0);
> +	dmareq_mask = ICU_DMAC_DMAREQ_MASK;
> +
> +	if (upper) {
> +		dmareq <<= 16;
> +		dmareq_mask <<= 16;
> +	}

You already have macros for this, so the obvious thing to do is to put
the shift magic into them:

/* Two 16 bit fields per register */
#define ICU_DMAC_DMAREQ_SHIFT(ch)		((ch & 0x01) * 16)

#define ICU_DMAC_PREP_DMAREQ(sel, ch)		(ICU_DMAC_PREP_DkRQ_SEL(sel)	\
                                                 << ICU_DMAC_DMAREQ_SHIFT(ch))
#define ICU_DMAC_DMAREQ_MASK(ch)		(ICU_DMAC_DkRQ_SEL_MASK		\
                                                 << ICU_DMAC_DMAREQ_SHIFT(ch))

        dmareq = ICU_DMAC_PREP_DMAREQ(req_no, ch);
        dmareq_mask = ICU_DMAC_DMAREQ_MASK(ch);

> +	k  = ack_no / 4;
> +	field_no = ack_no % 4;
> +
> +	dmaack_mask = ICU_DMAC_DACK_SEL_MASK << (field_no * 8);
> +	dmaack = ack_no << (field_no * 8);

Same here.

> +	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));

Thanks,

        tglx