Hi,

On Mon, 2020-07-27 at 16:48 +0100, Lee Jones wrote:

On Thu, 23 Jul 2020, Hsin-Hsiung Wang wrote:

quoted

This patch refines the interrupt related code to support new chips.

Refines in what way?

What makes this better?

Thanks for the comment. I will add more information into comment message
based on my below explanation.

quoted

Signed-off-by: Hsin-Hsiung Wang <redacted>
---
 drivers/mfd/mt6358-irq.c        | 65 ++++++++++++++++++++++++-----------------
 include/linux/mfd/mt6358/core.h |  8 ++---
 2 files changed, 41 insertions(+), 32 deletions(-)

diff --git a/drivers/mfd/mt6358-irq.c b/drivers/mfd/mt6358-irq.c
index db734f2..4b094e5 100644
--- a/drivers/mfd/mt6358-irq.c
+++ b/drivers/mfd/mt6358-irq.c

@@ -13,7 +13,9 @@
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
 
-static struct irq_top_t mt6358_ints[] = {
+#define MTK_PMIC_REG_WIDTH 16
+
+static const struct irq_top_t mt6358_ints[] = {
 	MT6358_TOP_GEN(BUCK),
 	MT6358_TOP_GEN(LDO),
 	MT6358_TOP_GEN(PSC),

@@ -24,6 +26,13 @@ static struct irq_top_t mt6358_ints[] = {
 	MT6358_TOP_GEN(MISC),
 };
 
+static struct pmic_irq_data mt6358_irqd = {
+	.num_top = ARRAY_SIZE(mt6358_ints),
+	.num_pmic_irqs = MT6358_IRQ_NR,
+	.top_int_status_reg = MT6358_TOP_INT_STATUS0,
+	.pmic_ints = mt6358_ints,
+};

Dynamically assigned driver data is usually preferred.

Why have you gone static?

Do you consider the memory allocation?
Below modification is to assign necessary data dynamically and the code
will become longer with more chips if we assign every member of the
structure.

@@ -180,17 +190,18 @@  int mt6358_irq_init(struct mt6397_chip *chip)
               int i, j, ret;
               struct pmic_irq_data *irqd;

-              irqd = devm_kzalloc(chip->dev, sizeof(*irqd),

GFP_KERNEL);
-              if (!irqd)
-                              return -ENOMEM;
+             switch (chip->chip_id) {
+             case MT6358_CHIP_ID:
+                             chip->irq_data = &mt6358_irqd;
+                             break;

-              chip->irq_data = irqd;
+             default:
+                             dev_err(chip->dev, "unsupported chip: 0x%x
\n", chip->chip_id);
+                             return -ENODEV;
+             }

                mutex_init(&chip->irqlock);
-              irqd->top_int_status_reg = MT6358_TOP_INT_STATUS0;
-              irqd->num_pmic_irqs = MT6358_IRQ_NR;
-              irqd->num_top = ARRAY_SIZE(mt6358_ints);
-
+             irqd = chip->irq_data;
               irqd->enable_hwirq = devm_kcalloc(chip->dev,

irqd->num_pmic_irqs,

sizeof(*irqd->enable_hwirq),

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel