On Tue, Mar 18, 2025 at 05:26:22PM +0100, Mathieu Dubois-Briand wrote: > GPIO controller often have support for IRQ: allow to easily allocate > both gpio-regmap and regmap-irq in one operation. ... > - if (config->irq_domain) { > - ret = gpiochip_irqchip_add_domain(chip, config->irq_domain); > + irq_domain = config->irq_domain; Better to move it into #else, so we avoid double assignment (see below). > +#ifdef CONFIG_GPIOLIB_IRQCHIP > + if (config->regmap_irq_chip) { > + struct regmap_irq_chip_data *irq_chip_data; > + > + ret = devm_regmap_add_irq_chip_fwnode(config->parent, dev_fwnode(config->parent), > + config->regmap, config->regmap_irq_irqno, > + config->regmap_irq_flags, 0, > + config->regmap_irq_chip, &irq_chip_data); > + if (ret) > + goto err_free_gpio; > + > + irq_domain = regmap_irq_get_domain(irq_chip_data); > + if (config->regmap_irq_chip_data) > + *config->regmap_irq_chip_data = irq_chip_data; Hmm... I was under impression that we don't need this to be returned. Do we have any user of it right now? If not, no need to export until it is needed. > + } } else > +#endif irq_domain = config->irq_domain; > + > + if (irq_domain) { > + ret = gpiochip_irqchip_add_domain(chip, irq_domain); > if (ret) > goto err_remove_gpiochip; > } ... > +#ifdef CONFIG_GPIOLIB_IRQCHIP > + struct regmap_irq_chip *regmap_irq_chip; > + struct regmap_irq_chip_data **regmap_irq_chip_data; But why double pointer? > + int regmap_irq_irqno; > + unsigned long regmap_irq_flags; > +#endif -- With Best Regards, Andy Shevchenko