On Mon, 05 Aug 2019, Hsin-Hsiung Wang wrote:

[...]

+static void pmic_irq_sync_unlock(struct irq_data *data)
+{
+	unsigned int i, top_gp, en_reg, int_regs, shift;
+	struct mt6397_chip *chip = irq_data_get_irq_chip_data(data);
+	struct pmic_irq_data *irqd = chip->irq_data;
+
+	for (i = 0; i < irqd->num_pmic_irqs; i++) {
+		if (irqd->enable_hwirq[i] == irqd->cache_hwirq[i])
+			continue;
+
+		top_gp = 0;
+		while ((top_gp + 1) < ARRAY_SIZE(mt6358_ints) &&
+		       i >= mt6358_ints[top_gp + 1].hwirq_base)
+			top_gp++;

A comment here would make this easier to follow.

+		if (top_gp >= ARRAY_SIZE(mt6358_ints)) {
+			mutex_unlock(&chip->irqlock);
+			dev_err(chip->dev,
+				"Failed to get top_group: %d\n", top_gp);
+			return;
+		}
+
+		int_regs = (i - mt6358_ints[top_gp].hwirq_base) /
+			    MT6358_REG_WIDTH;
+		en_reg = mt6358_ints[top_gp].en_reg +
+			mt6358_ints[top_gp].en_reg_shift * int_regs;
+		shift = (i - mt6358_ints[top_gp].hwirq_base) % MT6358_REG_WIDTH;
+		regmap_update_bits(chip->regmap, en_reg, BIT(shift),
+				   irqd->enable_hwirq[i] << shift);
+		irqd->cache_hwirq[i] = irqd->enable_hwirq[i];
+	}
+	mutex_unlock(&chip->irqlock);
+}

[...]

+int mt6358_irq_init(struct mt6397_chip *chip)
+{
+	int i, j, ret;
+	struct pmic_irq_data *irqd;
+
+	irqd = devm_kzalloc(chip->dev, sizeof(struct pmic_irq_data *),
+			    GFP_KERNEL);
+	if (!irqd)
+		return -ENOMEM;
+
+	chip->irq_data = irqd;
+
+	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->enable_hwirq = devm_kcalloc(chip->dev,
+					  irqd->num_pmic_irqs,
+					  sizeof(bool),
+					  GFP_KERNEL);
+	if (!irqd->enable_hwirq)
+		return -ENOMEM;
+
+	irqd->cache_hwirq = devm_kcalloc(chip->dev,
+					 irqd->num_pmic_irqs,
+					 sizeof(bool),
+					 GFP_KERNEL);
+	if (!irqd->cache_hwirq)
+		return -ENOMEM;
+
+	/* Disable all interrupt for initializing */

s/interrupt/interrupts/

+	for (i = 0; i < irqd->num_top; i++) {
+		for (j = 0; j < mt6358_ints[i].num_int_regs; j++)
+			regmap_write(chip->regmap,
+				     mt6358_ints[i].en_reg +
+				     mt6358_ints[i].en_reg_shift * j, 0);
+	}
+
+	chip->irq_domain = irq_domain_add_linear(chip->dev->of_node,
+						 irqd->num_pmic_irqs,
+						 &mt6358_irq_domain_ops, chip);
+	if (!chip->irq_domain) {
+		dev_err(chip->dev, "could not create irq domain\n");

s/irq/IRQ/

+		return -ENODEV;
+	}
+
+	ret = devm_request_threaded_irq(chip->dev, chip->irq, NULL,
+					mt6358_irq_handler, IRQF_ONESHOT,
+					mt6358_irq_chip.name, chip);
+	if (ret) {
+		dev_err(chip->dev, "failed to register irq=%d; err: %d\n",
+			chip->irq, ret);
+		return ret;
+	}
+
+	enable_irq_wake(chip->irq);
+	return ret;
+}

[...]

You may with so to check for both and error out on an unsupported chip
ID.

Not -1.  Please use the defines.

[...]

+#define MT6358_TOP_GEN(sp)	\
+{	\
+	.hwirq_base = MT6358_IRQ_##sp##_BASE,	\
+	.num_int_regs =	\
+		((MT6358_IRQ_##sp##_BITS - 1) / MT6358_REG_WIDTH) + 1,	\
+	.num_int_bits = MT6358_IRQ_##sp##_BITS, \
+	.en_reg = MT6358_##sp##_TOP_INT_CON0,		\
+	.en_reg_shift = 0x6,	\
+	.sta_reg = MT6358_##sp##_TOP_INT_STATUS0,		\
+	.sta_reg_shift = 0x2,	\
+	.top_offset = MT6358_##sp##_TOP,	\
+}

Please tab out the '\'s.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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