On Tue, Dec 05 2023 at 18:45, Yoshinori Sato wrote: > +static void endisable_irq(struct irq_data *data, int enable) bool enable? > +{ > + struct sh7751_intc_priv *priv; > + unsigned int irq; > + > + priv = irq_data_to_priv(data); > + > + irq = irqd_to_hwirq(data); > + if (!is_valid_irq(irq)) { > + /* IRQ out of range */ > + pr_warn_once("%s: IRQ %u is out of range\n", __FILE__, irq); > + return; > + } > + > + if (irq <= MAX_IRL && !priv->irlm) > + /* IRL encoded external interrupt */ > + /* disable for SR.IMASK */ > + update_sr_imask(irq - IRQ_START, enable); > + else > + /* Internal peripheral interrupt */ > + /* mask for IPR priority 0 */ > + update_ipr(priv, irq, enable); Lacks curly brackets on the if/else > +static int irq_sh7751_map(struct irq_domain *h, unsigned int virq, > + irq_hw_number_t hw_irq_num) > +{ > + irq_set_chip_and_handler(virq, &sh7751_irq_chip, handle_level_irq); > + irq_get_irq_data(virq)->chip_data = h->host_data; > + irq_modify_status(virq, IRQ_NOREQUEST, IRQ_NOPROBE); > + return 0; > +} > +static const struct irq_domain_ops irq_ops = { Newline before 'static ...' > + .map = irq_sh7751_map, > + .xlate = irq_domain_xlate_onecell, > +}; > + > +static int __init load_ipr_map(struct device_node *intc, > + struct sh7751_intc_priv *priv) > +{ > + struct property *ipr_map; > + unsigned int num_ipr, i; > + struct ipr *ipr; > + const __be32 *p; > + u32 irq; > + > + ipr_map = of_find_property(intc, "renesas,ipr-map", &num_ipr); > + if (IS_ERR(ipr_map)) > + return PTR_ERR(ipr_map); > + num_ipr /= sizeof(u32); > + /* 3words per entry. */ > + if (num_ipr % 3) Three words per ... But you can spare the comment by doing: if (num_ipr % WORDS_PER_ENTRY) > + goto error1; > + num_ipr /= 3; > +static int __init sh7751_intc_of_init(struct device_node *intc, > + struct device_node *parent) > +{ > + struct sh7751_intc_priv *priv; > + void __iomem *base, *base2; > + struct irq_domain *domain; > + u16 icr; > + int ret; > + > + base = of_iomap(intc, 0); > + base2 = of_iomap(intc, 1); > + if (!base || !base2) { > + pr_err("%pOFP: Invalid register definition\n", intc); What unmaps 'base' if 'base' is valid and base2 == NULL? > + return -EINVAL; > + } > + > + priv = kzalloc(sizeof(struct sh7751_intc_priv), GFP_KERNEL); > + if (priv == NULL) > + return -ENOMEM; Leaks base[2] maps, no? > + ret = load_ipr_map(intc, priv); > + if (ret < 0) { > + kfree(priv); > + return ret; > + } > + > + priv->base = base; > + priv->intpri00 = base2; > + > + if (of_property_read_bool(intc, "renesas,irlm")) { > + priv->irlm = true; > + icr = __raw_readw(priv->base + R_ICR); > + icr |= ICR_IRLM; > + __raw_writew(icr, priv->base + R_ICR); > + } > + > + domain = irq_domain_add_linear(intc, NR_IRQS, &irq_ops, priv); > + if (domain == NULL) { > + pr_err("%pOFP: cannot initialize irq domain\n", intc); > + kfree(priv); > + return -ENOMEM; > + } > + > + irq_set_default_host(domain); > + pr_info("%pOFP: SH7751 Interrupt controller (%s external IRQ)", > + intc, priv->irlm ? "4 lines" : "15 level"); > + return 0; > +} > + > +IRQCHIP_DECLARE(sh_7751_intc, > + "renesas,sh7751-intc", sh7751_intc_of_init); One line please. Thanks, tglx