Re: [PATCH v1 3/4] irqchip: irq-mtk-cirq: Move register offsets to const array
From: AngeloGioacchino Del Regno
Date: Wed Nov 23 2022 - 04:05:16 EST
Il 22/11/22 14:03, Marc Zyngier ha scritto:
On Fri, 18 Nov 2022 10:06:38 +0000,
AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx> wrote:
In preparation to add support for new SoCs having different register
offsets, add an enumeration that documents registers and move the
register offsets definitions to a u32 array.
Of course, every usage of the definitions was changed to use the
newly introduced register offsets array.
This change brings no functional changes.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>
---
drivers/irqchip/irq-mtk-cirq.c | 62 ++++++++++++++++++++++++----------
1 file changed, 44 insertions(+), 18 deletions(-)
diff --git a/drivers/irqchip/irq-mtk-cirq.c b/drivers/irqchip/irq-mtk-cirq.c
index 9bca0918078e..affbc0f48550 100644
--- a/drivers/irqchip/irq-mtk-cirq.c
+++ b/drivers/irqchip/irq-mtk-cirq.c
@@ -15,14 +15,30 @@
#include <linux/slab.h>
#include <linux/syscore_ops.h>
-#define CIRQ_ACK 0x40
-#define CIRQ_MASK_SET 0xc0
-#define CIRQ_MASK_CLR 0x100
-#define CIRQ_SENS_SET 0x180
-#define CIRQ_SENS_CLR 0x1c0
-#define CIRQ_POL_SET 0x240
-#define CIRQ_POL_CLR 0x280
-#define CIRQ_CONTROL 0x300
+enum mtk_cirq_reg_index {
+ CIRQ_STA = 0,
Enums starting from 0 are the default.
+ CIRQ_ACK,
+ CIRQ_MASK_SET,
+ CIRQ_MASK_CLR,
+ CIRQ_SENS_SET,
+ CIRQ_SENS_CLR,
+ CIRQ_POL_SET,
+ CIRQ_POL_CLR,
+ CIRQ_CONTROL,
+ CIRQ_MAX
What's the use of this last constant?
None, was commodity for loops - will remove, thanks.
+};
+
+static const u32 mtk_cirq_regs_v1[] = {
+ [CIRQ_STA] = 0x0,
+ [CIRQ_ACK] = 0x40,
+ [CIRQ_MASK_SET] = 0xc0,
+ [CIRQ_MASK_CLR] = 0x100,
+ [CIRQ_SENS_SET] = 0x180,
+ [CIRQ_SENS_CLR] = 0x1c0,
+ [CIRQ_POL_SET] = 0x240,
+ [CIRQ_POL_CLR] = 0x280,
+ [CIRQ_CONTROL] = 0x300,
+};
#define CIRQ_EN 0x1
#define CIRQ_EDGE 0x2
@@ -32,18 +48,20 @@ struct mtk_cirq_chip_data {
void __iomem *base;
unsigned int ext_irq_start;
unsigned int ext_irq_end;
+ const u32 *regs;
This is an array of *offsets*, not registers. Please name it accordingly.
Will do
struct irq_domain *domain;
};
static struct mtk_cirq_chip_data *cirq_data;
-static void mtk_cirq_write_mask(struct irq_data *data, unsigned int offset)
+static void mtk_cirq_write_mask(struct irq_data *data, enum mtk_cirq_reg_index idx)
{
struct mtk_cirq_chip_data *chip_data = data->chip_data;
unsigned int cirq_num = data->hwirq;
u32 mask = 1 << (cirq_num % 32);
+ u32 reg = chip_data->regs[idx] + (cirq_num / 32) * 4;
Please provide an accessor that takes chip_data and an index, and
returns an address.
Ack.
- writel_relaxed(mask, chip_data->base + offset + (cirq_num / 32) * 4);
+ writel_relaxed(mask, chip_data->base + reg);
}
static void mtk_cirq_mask(struct irq_data *data)
@@ -160,7 +178,7 @@ static const struct irq_domain_ops cirq_domain_ops = {
#ifdef CONFIG_PM_SLEEP
static int mtk_cirq_suspend(void)
{
- u32 value, mask;
+ u32 value, mask, reg;
unsigned int irq, hwirq_num;
bool pending, masked;
int i, pendret, maskret;
@@ -200,31 +218,34 @@ static int mtk_cirq_suspend(void)
continue;
}
+ reg = cirq_data->regs[CIRQ_ACK] + (i / 32) * 4;
mask = 1 << (i % 32);
- writel_relaxed(mask, cirq_data->base + CIRQ_ACK + (i / 32) * 4);
+ writel_relaxed(mask, cirq_data->base + reg);
}
/* set edge_only mode, record edge-triggerd interrupts */
/* enable cirq */
- value = readl_relaxed(cirq_data->base + CIRQ_CONTROL);
+ reg = cirq_data->regs[CIRQ_CONTROL];
+ value = readl_relaxed(cirq_data->base + reg);
value |= (CIRQ_EDGE | CIRQ_EN);
- writel_relaxed(value, cirq_data->base + CIRQ_CONTROL);
+ writel_relaxed(value, cirq_data->base + reg);
return 0;
}
static void mtk_cirq_resume(void)
{
+ u32 reg = cirq_data->regs[CIRQ_CONTROL];
u32 value;
/* flush recorded interrupts, will send signals to parent controller */
- value = readl_relaxed(cirq_data->base + CIRQ_CONTROL);
- writel_relaxed(value | CIRQ_FLUSH, cirq_data->base + CIRQ_CONTROL);
+ value = readl_relaxed(cirq_data->base + reg);
+ writel_relaxed(value | CIRQ_FLUSH, cirq_data->base + reg);
/* disable cirq */
- value = readl_relaxed(cirq_data->base + CIRQ_CONTROL);
+ value = readl_relaxed(cirq_data->base + reg);
value &= ~(CIRQ_EDGE | CIRQ_EN);
- writel_relaxed(value, cirq_data->base + CIRQ_CONTROL);
+ writel_relaxed(value, cirq_data->base + reg);
}
static struct syscore_ops mtk_cirq_syscore_ops = {
@@ -240,6 +261,9 @@ static void mtk_cirq_syscore_init(void)
static inline void mtk_cirq_syscore_init(void) {}
#endif
+static const struct of_device_id mtk_cirq_of_match[] = {
+ { .compatible = "mediatek,
+
I can tell by this hunk the quality of testing this patch has
received.
Actually, the testing was performed on multiple SoCs.
As for why this is there, check below...
static int __init mtk_cirq_of_init(struct device_node *node,
struct device_node *parent)
{
@@ -274,6 +298,8 @@ static int __init mtk_cirq_of_init(struct device_node *node,
if (ret)
goto out_unmap;
+ cirq_data->regs = mtk_cirq_regs_v1;
Why isn't this obtained from the matching array?
.... I'm truly sorry for what happened, I've just checked - the right patchset
was in a different folder and I've somehow sent the wrong one.
Sorry again - will send a v2 ASAP.
Best regards,
Angelo