Re: [PATCH 1/3] serial: qcom-geni: Propagate errors from optional IRQ lookup

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The error cleanup path in `qcom_geni_serial_probe()` leaks the IDA allocated for the UART line index, which is severely exacerbated by the newly added IRQ probe deferral path.
--

--- Patch [1]: [PATCH 1/3] serial: qcom-geni: Propagate errors from optional IRQ lookup ---
commit 9ec5f0b3d9989a6071c2d4680336709af497a5bf
Author: bui duc phuc <[email protected]>

serial: qcom-geni: Propagate errors from optional IRQ lookup

This commit updates qcom_geni_serial_probe() to propagate errors, such as
-EPROBE_DEFER, from platform_get_irq_optional() for the wakeup IRQ.
Previously, errors other than -ENXIO were ignored and probe continued.

> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 7ead87b4eb65b..240a6d67d8475 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -1883,8 +1883,13 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>  	uport->irq = irq;
>  	uport->has_sysrq = IS_ENABLED(CONFIG_SERIAL_QCOM_GENI_CONSOLE);
>  
> -	if (!data->console)
> +	if (!data->console) {
>  		port->wakeup_irq = platform_get_irq_optional(pdev, 1);
> +		if (port->wakeup_irq < 0 && port->wakeup_irq != -ENXIO) {
> +			ret = port->wakeup_irq;
> +			goto error;

[Severity: High]
This is a pre-existing issue, but does this goto error path leak the port IDA?

In get_port_from_line(), an IDA is allocated for the UART line index with a
very restricted range:

drivers/tty/serial/qcom_geni_serial.c:get_port_from_line() {
    ...
		if (line < 0)
			line = ida_alloc_range(&port_ida, max_alias_num + 1,
					       nr_ports - 1, GFP_KERNEL);
		else
			line = ida_alloc_range(&port_ida, line,
					       nr_ports - 1, GFP_KERNEL);
    ...
}

However, when jumping to the error label in qcom_geni_serial_probe(), the
cleanup only detaches PM domains and misses calling ida_free():

drivers/tty/serial/qcom_geni_serial.c:qcom_geni_serial_probe() {
    ...
error:
	dev_pm_domain_detach_list(port->pd_list);
	return ret;
}

Since this patch adds a check that propagates -EPROBE_DEFER when the optional
wakeup IRQ is not yet ready, this error path will be frequently hit during
normal boot.

Because the max IDA limit is very small, just a few probe deferrals will
exhaust the available IDs, causing get_port_from_line() to return -ENXIO and
permanently preventing the UART driver from probing on subsequent attempts.

Should ida_free(&port_ida, uport->line) be called before jumping to the error
label, or added to the error label itself, to prevent exhausting the ID space?

> +		}
> +	}
>  
>  	if (of_property_read_bool(pdev->dev.of_node, "rx-tx-swap"))
>  		port->rx_tx_swap = true;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1