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

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

Posted by Neil Armstrong 3 weeks ago

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@linaro.org>
---
 drivers/tty/serial/qcom_geni_serial.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 3633723acef8..bdbdfd7dfaac 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -1979,7 +1979,8 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
 						port->wakeup_irq);
 		if (ret) {
 			device_init_wakeup(&pdev->dev, false);
-			ida_free(&port_ida, uport->line);
+			if (!uart_console(uport))
+				ida_free(&port_ida, uport->line);
 			goto error;
 		}
 	}
@@ -2024,7 +2025,8 @@ static void qcom_geni_serial_remove(struct platform_device *pdev)
 	irq_work_sync(&port->tx_kick);
 	dev_pm_clear_wake_irq(&pdev->dev);
 	device_init_wakeup(&pdev->dev, false);
-	ida_free(&port_ida, uport->line);
+	if (!uart_console(uport))
+		ida_free(&port_ida, uport->line);
 	uart_remove_one_port(drv, &port->uport);
 
 	if (port->rx_dma_addr) {

---
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
change-id: 20260904-topic-sm8x50-upstream-tty-serial-geni-fix-ida-free-4a199c25fd84

Best regards,
--  
Neil Armstrong <neil.armstrong@linaro.org>

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

Posted by Aniket RANDIVE 2 weeks, 5 days ago

Hi Neil,

This appears to be already fixed by:

commit ("serial: qcom-geni: Fix port_ida handling for console and probe 
errors")

which also fixes additional port_ida leak paths.

On 9/4/2026 12:39 PM, 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@linaro.org>
> ---
>   drivers/tty/serial/qcom_geni_serial.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 3633723acef8..bdbdfd7dfaac 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -1979,7 +1979,8 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>   						port->wakeup_irq);
>   		if (ret) {
>   			device_init_wakeup(&pdev->dev, false);
> -			ida_free(&port_ida, uport->line);
> +			if (!uart_console(uport))
> +				ida_free(&port_ida, uport->line);
>   			goto error;
>   		}
>   	}
> @@ -2024,7 +2025,8 @@ static void qcom_geni_serial_remove(struct platform_device *pdev)
>   	irq_work_sync(&port->tx_kick);
>   	dev_pm_clear_wake_irq(&pdev->dev);
>   	device_init_wakeup(&pdev->dev, false);
> -	ida_free(&port_ida, uport->line);
> +	if (!uart_console(uport))
> +		ida_free(&port_ida, uport->line);
>   	uart_remove_one_port(drv, &port->uport);
>   
>   	if (port->rx_dma_addr) {
> 
> ---
> base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
> change-id: 20260904-topic-sm8x50-upstream-tty-serial-geni-fix-ida-free-4a199c25fd84
> 
> Best regards,
> --
> Neil Armstrong <neil.armstrong@linaro.org>
> 
>

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

Posted by Konrad Dybcio 3 weeks ago

On 9/4/26 9:09 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@linaro.org>
> ---


>  drivers/tty/serial/qcom_geni_serial.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 3633723acef8..bdbdfd7dfaac 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -1979,7 +1979,8 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>  						port->wakeup_irq);
>  		if (ret) {
>  			device_init_wakeup(&pdev->dev, false);
> -			ida_free(&port_ida, uport->line);
> +			if (!uart_console(uport))
> +				ida_free(&port_ida, uport->line);

GPT points out this will always return false before uart_add_one_port()
is called

Konrad

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

Posted by Konrad Dybcio 3 weeks ago

On 9/4/26 10:02 AM, Konrad Dybcio wrote:
> On 9/4/26 9:09 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@linaro.org>
>> ---
> 
> 
>>  drivers/tty/serial/qcom_geni_serial.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
>> index 3633723acef8..bdbdfd7dfaac 100644
>> --- a/drivers/tty/serial/qcom_geni_serial.c
>> +++ b/drivers/tty/serial/qcom_geni_serial.c
>> @@ -1979,7 +1979,8 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>>  						port->wakeup_irq);
>>  		if (ret) {
>>  			device_init_wakeup(&pdev->dev, false);
>> -			ida_free(&port_ida, uport->line);
>> +			if (!uart_console(uport))
>> +				ida_free(&port_ida, uport->line);
> 
> GPT points out this will always return false before uart_add_one_port()
> is called

You can use if (data->console) instead

Konrad

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

Posted by Neil Armstrong 3 weeks ago

On 9/4/26 10:03, Konrad Dybcio wrote:
> On 9/4/26 10:02 AM, Konrad Dybcio wrote:
>> On 9/4/26 9:09 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@linaro.org>
>>> ---
>>
>>
>>>   drivers/tty/serial/qcom_geni_serial.c | 6 ++++--
>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
>>> index 3633723acef8..bdbdfd7dfaac 100644
>>> --- a/drivers/tty/serial/qcom_geni_serial.c
>>> +++ b/drivers/tty/serial/qcom_geni_serial.c
>>> @@ -1979,7 +1979,8 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>>>   						port->wakeup_irq);
>>>   		if (ret) {
>>>   			device_init_wakeup(&pdev->dev, false);
>>> -			ida_free(&port_ida, uport->line);
>>> +			if (!uart_console(uport))
>>> +				ida_free(&port_ida, uport->line);
>>
>> GPT points out this will always return false before uart_add_one_port()
>> is called
> 
> You can use if (data->console) instead

Yep will do

Thanks

> 
> Konrad