Re: [PATCH] gpio: mxc: release the parent IRQ in runtime suspend

[Andy Shevchenko <andy@xxxxxxxxxx>]

On Tue, Aug 08, 2023 at 10:33:46AM -0500, Shenwei Wang wrote:
> Release the parent interrupt request during runtime suspend, allowing
> the parent interrupt controller to enter runtime suspend if there are
> no active users.
> 
> This change may not have a visible impact if the parent controller is
> the GIC, but it can enable significant power savings for parent IRQ
> controllers like IRQSteer inside a subsystem on i.MX8 SoCs. Releasing
> the parent IRQ provides an opportunity for the subsystem to enter suspend
> states if there are no active users.

...

> +static void mxc_update_irq_chained_handler(struct mxc_gpio_port *port, bool enable)
> +{
> +	if (!port)
> +		return;

When can this be true?

> +	if (enable) {
> +		irq_set_chained_handler_and_data(port->irq,
> +						 port->mx_irq_handler, port);
> +		if (port->irq_high > 0)
> +			/* setup handler for GPIO 16 to 31 */
> +			irq_set_chained_handler_and_data(port->irq_high,
> +							 port->mx_irq_handler,
> +							 port);
> +	} else {
> +		irq_set_chained_handler_and_data(port->irq, NULL, NULL);
> +		if (port->irq_high > 0)
> +			irq_set_chained_handler_and_data(port->irq_high, NULL, NULL);
> +	}
> +}

Why not

	if (enable)
		irq_set_chained_handler_and_data(port->irq,
						 port->mx_irq_handler, port);
	else
		irq_set_chained_handler_and_data(port->irq, NULL, NULL);

	/* setup handler for GPIO 16 to 31 */
	if (port->irq_high > 0) {
		if (enable)
			irq_set_chained_handler_and_data(port->irq_high,
							 port->mx_irq_handler, port);
		else
			irq_set_chained_handler_and_data(port->irq_high, NULL, NULL);
	}

Wouldn't it be better to read and understand?

-- 
With Best Regards,
Andy Shevchenko