mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] serial: qcom-geni: Fix port_ida handling for console and probe errors
@ 2026-08-31 11:12 Aniket Randive
  2026-08-31 11:38 ` Konrad Dybcio
  0 siblings, 1 reply; 5+ messages in thread
From: Aniket Randive @ 2026-08-31 11:12 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-arm-msm, linux-kernel, linux-serial, Aniket Randive

The console port uses a fixed line number and is not allocated from
port_ida. However, qcom_geni_serial_remove() unconditionally frees the
line number for all ports, including the console port.

Skip ida_free() for console ports so that only IDs allocated from
port_ida are returned.

Also release allocated IDs from the common probe error path. Currently,
IDs are freed only on the wake-IRQ failure path, causing leaks when
probe fails after a successful allocation. Move the ida_free() call to
the common error path and guard it for console ports.

Route resources_init() failures through the common error path so the
allocated line number is released correctly.

Allocate the port structure before calling ida_alloc_range() in
get_port_from_line(). This prevents IDs from being left allocated when
memory allocation fails.

Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
---
Changes in v2:
- Route port->dev_data->resources_init() failures through the common
error path so allocated line numbers are not leaked.
- Allocate the port structure before ida_alloc_range() in
get_port_from_line() to prevent ID leaks on memory-allocation
failures.
---
Link to v1: https://patch.msgid.link/20260828-master-v1-1-1f8afa5f82c2@oss.qualcomm.com
---
 drivers/tty/serial/qcom_geni_serial.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 3633723acef8..ceaf560fd8dd 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -289,6 +289,10 @@ static struct qcom_geni_serial_port *get_port_from_line(int line, bool console,
 	} else {
 		int max_alias_num = of_alias_get_highest_id("serial");
 
+		port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
+		if (!port)
+			return ERR_PTR(-ENOMEM);
+
 		if (line < 0 || line >= nr_ports)
 			line = ida_alloc_range(&port_ida, max_alias_num + 1,
 					       nr_ports - 1, GFP_KERNEL);
@@ -299,10 +303,6 @@ static struct qcom_geni_serial_port *get_port_from_line(int line, bool console,
 		if (line < 0)
 			return ERR_PTR(-ENXIO);
 
-		port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
-		if (!port)
-			return ERR_PTR(-ENOMEM);
-
 		port->uport.iotype = UPIO_MEM;
 		port->uport.ops = &qcom_geni_uart_pops;
 		port->uport.flags = UPF_BOOT_AUTOCONF;
@@ -1897,7 +1897,7 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
 
 	ret = port->dev_data->resources_init(&port->se);
 	if (ret)
-		return ret;
+		goto error;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
@@ -1979,7 +1979,6 @@ 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);
 			goto error;
 		}
 	}
@@ -2003,6 +2002,8 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
 	return 0;
 
 error:
+	if (!data->console)
+		ida_free(&port_ida, uport->line);
 	if (port->rx_dma_addr) {
 		dma_unmap_single(pdev->dev.parent, port->rx_dma_addr,
 				 DMA_RX_BUF_SIZE, DMA_FROM_DEVICE);
@@ -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 (!port->dev_data->console)
+		ida_free(&port_ida, uport->line);
 	uart_remove_one_port(drv, &port->uport);
 
 	if (port->rx_dma_addr) {

---
base-commit: a8406e6c0b793ce0788019683837c40855b55995
change-id: 20260828-master-3e806ba3492d

Best regards,
--  
Aniket Randive <aniket.randive@oss.qualcomm.com>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] serial: qcom-geni: Fix port_ida handling for console and probe errors
  2026-08-31 11:12 [PATCH v2] serial: qcom-geni: Fix port_ida handling for console and probe errors Aniket Randive
@ 2026-08-31 11:38 ` Konrad Dybcio
  2026-08-31 15:59   ` Aniket RANDIVE
  0 siblings, 1 reply; 5+ messages in thread
From: Konrad Dybcio @ 2026-08-31 11:38 UTC (permalink / raw)
  To: Aniket Randive, Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-arm-msm, linux-kernel, linux-serial

On 8/31/26 1:12 PM, Aniket Randive wrote:
> The console port uses a fixed line number and is not allocated from
> port_ida. However, qcom_geni_serial_remove() unconditionally frees the
> line number for all ports, including the console port.
> 
> Skip ida_free() for console ports so that only IDs allocated from
> port_ida are returned.
> 
> Also release allocated IDs from the common probe error path. Currently,
> IDs are freed only on the wake-IRQ failure path, causing leaks when
> probe fails after a successful allocation. Move the ida_free() call to
> the common error path and guard it for console ports.
> 
> Route resources_init() failures through the common error path so the
> allocated line number is released correctly.
> 
> Allocate the port structure before calling ida_alloc_range() in
> get_port_from_line(). This prevents IDs from being left allocated when
> memory allocation fails.
> 
> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
> ---
> Changes in v2:
> - Route port->dev_data->resources_init() failures through the common
> error path so allocated line numbers are not leaked.

I asked GPT what it thinks again and this time it found that this
will cause a double calling of dev_pm_domain_detach_list(), which
will mess up the refcounting..

I think the fix here is to drop the manual detaches since the common
helper uses a devres-managed version of attach.

Konrad

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] serial: qcom-geni: Fix port_ida handling for console and probe errors
  2026-08-31 11:38 ` Konrad Dybcio
@ 2026-08-31 15:59   ` Aniket RANDIVE
  2026-09-01  5:00     ` Praveen Talari
  2026-09-01  9:57     ` Konrad Dybcio
  0 siblings, 2 replies; 5+ messages in thread
From: Aniket RANDIVE @ 2026-08-31 15:59 UTC (permalink / raw)
  To: Konrad Dybcio, Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-arm-msm, linux-kernel, linux-serial



On 8/31/2026 5:08 PM, Konrad Dybcio wrote:
> On 8/31/26 1:12 PM, Aniket Randive wrote:
>> The console port uses a fixed line number and is not allocated from
>> port_ida. However, qcom_geni_serial_remove() unconditionally frees the
>> line number for all ports, including the console port.
>>
>> Skip ida_free() for console ports so that only IDs allocated from
>> port_ida are returned.
>>
>> Also release allocated IDs from the common probe error path. Currently,
>> IDs are freed only on the wake-IRQ failure path, causing leaks when
>> probe fails after a successful allocation. Move the ida_free() call to
>> the common error path and guard it for console ports.
>>
>> Route resources_init() failures through the common error path so the
>> allocated line number is released correctly.
>>
>> Allocate the port structure before calling ida_alloc_range() in
>> get_port_from_line(). This prevents IDs from being left allocated when
>> memory allocation fails.
>>
>> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
>> ---
>> Changes in v2:
>> - Route port->dev_data->resources_init() failures through the common
>> error path so allocated line numbers are not leaked.
> 
> I asked GPT what it thinks again and this time it found that this
> will cause a double calling of dev_pm_domain_detach_list(), which
> will mess up the refcounting..
> 
> I think the fix here is to drop the manual detaches since the common
> helper uses a devres-managed version of attach.
> 

Thanks for catching this.

Looking closer, this appears to be a separate issue from the port_ida 
fixes in this patch. The dev_pm_domain_detach_list() problem already 
exists in the current error path and is not introduced by this change.

Since this involves power-domain cleanup rather than port_ida handling, 
I'd prefer to address it in a separate follow-up patch to keep this fix 
focused and easier to review.

Is it OK to keep this patch limited to the port_ida fixes and handle the 
power-domain cleanup separately?

Thanks,
Aniket

> Konrad


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] serial: qcom-geni: Fix port_ida handling for console and probe errors
  2026-08-31 15:59   ` Aniket RANDIVE
@ 2026-09-01  5:00     ` Praveen Talari
  2026-09-01  9:57     ` Konrad Dybcio
  1 sibling, 0 replies; 5+ messages in thread
From: Praveen Talari @ 2026-09-01  5:00 UTC (permalink / raw)
  To: Aniket RANDIVE, Konrad Dybcio, Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-arm-msm, linux-kernel, linux-serial

Hi Aniket,

On 31-08-2026 21:29, Aniket RANDIVE wrote:
>
>
> On 8/31/2026 5:08 PM, Konrad Dybcio wrote:
>> On 8/31/26 1:12 PM, Aniket Randive wrote:
>>> The console port uses a fixed line number and is not allocated from
>>> port_ida. However, qcom_geni_serial_remove() unconditionally frees the
>>> line number for all ports, including the console port.
>>>
>>> Skip ida_free() for console ports so that only IDs allocated from
>>> port_ida are returned.
>>>
>>> Also release allocated IDs from the common probe error path. Currently,
>>> IDs are freed only on the wake-IRQ failure path, causing leaks when
>>> probe fails after a successful allocation. Move the ida_free() call to
>>> the common error path and guard it for console ports.
>>>
>>> Route resources_init() failures through the common error path so the
>>> allocated line number is released correctly.
>>>
>>> Allocate the port structure before calling ida_alloc_range() in
>>> get_port_from_line(). This prevents IDs from being left allocated when
>>> memory allocation fails.
>>>
>>> Signed-off-by: Aniket Randive <aniket.randive@oss.qualcomm.com>
>>> ---
>>> Changes in v2:
>>> - Route port->dev_data->resources_init() failures through the common
>>> error path so allocated line numbers are not leaked.
>>
>> I asked GPT what it thinks again and this time it found that this
>> will cause a double calling of dev_pm_domain_detach_list(), which
>> will mess up the refcounting..
>>
>> I think the fix here is to drop the manual detaches since the common
>> helper uses a devres-managed version of attach.
>>
>
> Thanks for catching this.
>
> Looking closer, this appears to be a separate issue from the port_ida 
> fixes in this patch. The dev_pm_domain_detach_list() problem already 
> exists in the current error path and is not introduced by this change.
>
> Since this involves power-domain cleanup rather than port_ida 
> handling, I'd prefer to address it in a separate follow-up patch to 
> keep this fix focused and easier to review.
>
> Is it OK to keep this patch limited to the port_ida fixes and handle 
> the power-domain cleanup separately?
That should be separate patch.

For this patch.

Reviewed-by: Praveen Talari <praveen.talari@oss.qualcomm.com>

Thanks,

Praveen Talari
>
> Thanks,
> Aniket
>
>> Konrad
>
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2] serial: qcom-geni: Fix port_ida handling for console and probe errors
  2026-08-31 15:59   ` Aniket RANDIVE
  2026-09-01  5:00     ` Praveen Talari
@ 2026-09-01  9:57     ` Konrad Dybcio
  1 sibling, 0 replies; 5+ messages in thread
From: Konrad Dybcio @ 2026-09-01  9:57 UTC (permalink / raw)
  To: Aniket RANDIVE, Greg Kroah-Hartman, Jiri Slaby
  Cc: linux-arm-msm, linux-kernel, linux-serial

On 8/31/26 5:59 PM, Aniket RANDIVE wrote:
> 
> 
> On 8/31/2026 5:08 PM, Konrad Dybcio wrote:
>> On 8/31/26 1:12 PM, Aniket Randive wrote:
>>> The console port uses a fixed line number and is not allocated from
>>> port_ida. However, qcom_geni_serial_remove() unconditionally frees the
>>> line number for all ports, including the console port.

[...]

> Thanks for catching this.
> 
> Looking closer, this appears to be a separate issue from the port_ida fixes in this patch. The dev_pm_domain_detach_list() problem already exists in the current error path and is not introduced by this change.
> 
> Since this involves power-domain cleanup rather than port_ida handling, I'd prefer to address it in a separate follow-up patch to keep this fix focused and easier to review.
> 
> Is it OK to keep this patch limited to the port_ida fixes and handle the power-domain cleanup separately?

Yes

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Konrad

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-01  9:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-31 11:12 [PATCH v2] serial: qcom-geni: Fix port_ida handling for console and probe errors Aniket Randive
2026-08-31 11:38 ` Konrad Dybcio
2026-08-31 15:59   ` Aniket RANDIVE
2026-09-01  5:00     ` Praveen Talari
2026-09-01  9:57     ` Konrad Dybcio

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®