Jiaxun Yang <jiaxun.y...@flygoat.com> writes: > +static void htvec_mask_irq(struct irq_data *d) > +{ > + struct htvec *priv = irq_data_get_irq_chip_data(d); > + void __iomem *addr = priv->base + HTVEC_EN_OFF; > + unsigned long flags; > + u32 reg; > + > + raw_spin_lock_irqsave(&priv->htvec_lock, flags);
No need for irqsave() these functions are called with interrupts disabled. > + addr += VEC_REG_IDX(d->hwirq) * 4; > + reg = readl(addr); > + reg &= ~BIT(VEC_REG_BIT(d->hwirq)); > + writel(reg, addr); > + raw_spin_unlock_irqrestore(&priv->htvec_lock, flags); > +} > +static int htvec_domain_alloc(struct irq_domain *domain, unsigned int virq, > + unsigned int nr_irqs, void *arg) > +{ > + struct htvec *priv = domain->host_data; > + unsigned long hwirq; > + unsigned int type, i; > + > + irq_domain_translate_onecell(domain, arg, &hwirq, &type); > + > + for (i = 0; i < nr_irqs; i++) > + irq_domain_set_info(domain, virq + i, hwirq + i, > &htvec_irq_chip, > + priv, handle_edge_irq, NULL, NULL); This wants curly brackets and the second line of arguments wants to be aligned with the first argument: for (i = 0; i < nr_irqs; i++) { irq_domain_set_info(domain, virq + i, hwirq + i, &htvec_irq_chip, priv, handle_edge_irq, NULL, NULL); } See https://lore.kernel.org/lkml/alpine.DEB.2.20.1701171956290.3645@nanos/ The alignment of arguments wants to be fixed all over the place. > +static int htvec_of_init(struct device_node *node, > + struct device_node *parent) > +{ > + struct htvec *priv; > + int err, parent_irq[4], num_parents = 0, i; Please order the variable declaration in reverse fir tree length order: int err, parent_irq[4], num_parents = 0, i; struct htvec *priv; That's way better readable than the above. All over the place please. > + priv->htvec_domain = irq_domain_create_linear(of_node_to_fwnode(node), > + VEC_COUNT, > + &htvec_domain_ops, > + priv); > + if (!priv->htvec_domain) { > + pr_err("Failed to create IRQ domain\n"); > + err = -ENOMEM; > + goto iounmap_base; > + } > + > + htvec_reset(priv); > + > + for (i = 0; i < num_parents; i++) > + irq_set_chained_handler_and_data(parent_irq[i], > + htvec_irq_dispatch, priv); See above. Thanks, tglx