Re: [PATCH v2] tty: serial: qcom_geni: don't ida_free() the console port line

[Neil Armstrong]

On 9/4/26 11:24, Konrad Dybcio wrote:
> On 9/4/26 10:50 AM, Neil Armstrong wrote:
> > The console port (qcom_geni_console_port) is a static instance whose
> > uport.line is hardcoded to 0 and is never allocated from port_ida.
> > Only the non-console path in get_port_from_line() calls ida_alloc_range().
> >
> > Fix the qcom_geni_serial_remove() and the matching probe() error path
> > so unbinding the console device doesn't hit:
> >
> >    WARNING: ida_free called for id=0 which is not allocated
> >    <snip>
> >    qcom_geni_serial_remove+0x58/0x80
> >
> > Fixes: a53be6945f51 ("serial: qcom-geni: Remove alias dependency from qcom serial driver")
> > Signed-off-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
> > ---
>
> The robot continues not to find all the issues during the first round..
>
> tldr: uart_console() is only a valid check if a port is an ACTIVE
> console whereas we skip ida_alloc for ANY console ports, so in .remove()
> the check should also be for device_data->console

Right, good catch, would be nice if the robot did find issues directly related to the patch instead of random unrelated issues.

Thanks,
Neil>
>   Medium: The remove-path condition remains incorrect for an inactive GENI console port.
> Patch line 326 changes the free guard to !uart_console(uport). uart_console() is true only when uport->cons->index == uport->line (include/linux/serial_core.h:24-25), rather than when the port is the statically allocated console device. If qcom,geni-debug-uart probes but is not selected as the active ttyMSM system console, cons_ops.index remains -1 (drivers/tty/serial/qcom_geni_serial.c:1746-1757), so uart_console(uport) is false. Removal then still calls ida_free(&port_ida, 0) even though get_port_from_line(..., true, ...) never allocated an IDA entry (drivers/tty/serial/qcom_geni_serial.c:279-310).
> Use the same ownership predicate as the probe error path:
> if (!port->dev_data->console)
>      ida_free(&port_ida, uport->line);
> This makes freeing depend on whether the line was allocated, not on current console activation.
>
> Konrad