[PATCH v4] tty: serial: qcom_geni_serial: Improve error handling for RS485 mode

[PATCH v4] tty: serial: qcom_geni_serial: Improve error handling for RS485 mode

[Anup Kulkarni]

Fix  error handling issues of  `uart_get_rs485_mode()` function by
reordering resources_init() to occur after uart_get_rs485_mode. Remove
multiple goto paths and use dev_err_probe to simplify error paths.

Fixes: 4fcc287f3c69 ("serial: qcom-geni: Enable support for half-duplex mode")
Cc: stable@vger.kernel.org
Signed-off-by: Anup Kulkarni <quic_anupkulk@quicinc.com>
---
v3->v4
- Added Fixes and Cc tag.

v2->v3
- Reordered the function resources_init.
- Removed goto.
- Added dev_err_probe.

v1->v2
- Updated commit message.
---
 drivers/tty/serial/qcom_geni_serial.c | 38 ++++++++++-----------------
 1 file changed, 14 insertions(+), 24 deletions(-)

diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
index 32ec632fd080..be998dd45968 100644
--- a/drivers/tty/serial/qcom_geni_serial.c
+++ b/drivers/tty/serial/qcom_geni_serial.c
@@ -1882,15 +1882,9 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
 	port->se.dev = &pdev->dev;
 	port->se.wrapper = dev_get_drvdata(pdev->dev.parent);
 
-	ret = port->dev_data->resources_init(uport);
-	if (ret)
-		return ret;
-
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		ret = -EINVAL;
-		goto error;
-	}
+	if (!res)
+		return -EINVAL;
 
 	uport->mapbase = res->start;
 
@@ -1903,25 +1897,19 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
 	if (!data->console) {
 		port->rx_buf = devm_kzalloc(uport->dev,
 					    DMA_RX_BUF_SIZE, GFP_KERNEL);
-		if (!port->rx_buf) {
-			ret = -ENOMEM;
-			goto error;
-		}
+		if (!port->rx_buf)
+			return -ENOMEM;
 	}
 
 	port->name = devm_kasprintf(uport->dev, GFP_KERNEL,
 			"qcom_geni_serial_%s%d",
 			uart_console(uport) ? "console" : "uart", uport->line);
-	if (!port->name) {
-		ret = -ENOMEM;
-		goto error;
-	}
+	if (!port->name)
+		return -ENOMEM;
 
 	irq = platform_get_irq(pdev, 0);
-	if (irq < 0) {
-		ret = irq;
-		goto error;
-	}
+	if (irq < 0)
+		return irq;
 
 	uport->irq = irq;
 	uport->has_sysrq = IS_ENABLED(CONFIG_SERIAL_QCOM_GENI_CONSOLE);
@@ -1942,12 +1930,14 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
 	irq_set_status_flags(uport->irq, IRQ_NOAUTOEN);
 	ret = devm_request_irq(uport->dev, uport->irq, qcom_geni_serial_isr,
 			IRQF_TRIGGER_HIGH, port->name, uport);
-	if (ret) {
-		dev_err(uport->dev, "Failed to get IRQ ret %d\n", ret);
-		goto error;
-	}
+	if (ret)
+		return dev_err_probe(uport->dev, ret, "Failed to get IRQ\n");
 
 	ret = uart_get_rs485_mode(uport);
+	if (ret)
+		return dev_err_probe(uport->dev, ret, "Failed to get rs485 mode\n");
+
+	ret = port->dev_data->resources_init(uport);
 	if (ret)
 		return ret;
 
-- 
2.34.1

Re: [PATCH v4] tty: serial: qcom_geni_serial: Improve error handling for RS485 mode

[Greg KH]

On Mon, Aug 18, 2025 at 04:29:18PM +0530, Anup Kulkarni wrote:
> Fix  error handling issues of  `uart_get_rs485_mode()` function by
> reordering resources_init() to occur after uart_get_rs485_mode.

What exactly is wrong with the current code?

> Remove multiple goto paths and use dev_err_probe to simplify error
> paths.

Don't mix fixes with cleanups please.  Shouldn't this be 2 patches?


> 
> Fixes: 4fcc287f3c69 ("serial: qcom-geni: Enable support for half-duplex mode")
> Cc: stable@vger.kernel.org
> Signed-off-by: Anup Kulkarni <quic_anupkulk@quicinc.com>
> ---
> v3->v4
> - Added Fixes and Cc tag.
> 
> v2->v3
> - Reordered the function resources_init.
> - Removed goto.
> - Added dev_err_probe.
> 
> v1->v2
> - Updated commit message.
> ---
>  drivers/tty/serial/qcom_geni_serial.c | 38 ++++++++++-----------------
>  1 file changed, 14 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
> index 32ec632fd080..be998dd45968 100644
> --- a/drivers/tty/serial/qcom_geni_serial.c
> +++ b/drivers/tty/serial/qcom_geni_serial.c
> @@ -1882,15 +1882,9 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>  	port->se.dev = &pdev->dev;
>  	port->se.wrapper = dev_get_drvdata(pdev->dev.parent);
>  
> -	ret = port->dev_data->resources_init(uport);
> -	if (ret)
> -		return ret;
> -
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> -	if (!res) {
> -		ret = -EINVAL;
> -		goto error;
> -	}
> +	if (!res)
> +		return -EINVAL;

But now you are not calling the stuff that was previously at error:, are
you sure that is ok?  If so, why?

>  
>  	uport->mapbase = res->start;
>  
> @@ -1903,25 +1897,19 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>  	if (!data->console) {
>  		port->rx_buf = devm_kzalloc(uport->dev,
>  					    DMA_RX_BUF_SIZE, GFP_KERNEL);
> -		if (!port->rx_buf) {
> -			ret = -ENOMEM;
> -			goto error;
> -		}
> +		if (!port->rx_buf)
> +			return -ENOMEM;

Same here.

As you are mixing different things in the same patch, it's hard to find
the original "bug" you are attempting to solve here.  I can't see it...

Please redo this as a patch series.

thanks,

greg k-h

Re: [PATCH v4] tty: serial: qcom_geni_serial: Improve error handling for RS485 mode

[Anup Kulkarni]

Hi Greg, 
This patch was supposed to be for internal peer review, and I mistakenly sent it
to stable@vger.kernel.org. Apologies from my end. 

Will not include you or other lists, for internal reviews. 
Apologies from my end again. 

On 8/19/2025 4:32 PM, Greg KH wrote:
> On Mon, Aug 18, 2025 at 04:29:18PM +0530, Anup Kulkarni wrote:
>> Fix  error handling issues of  `uart_get_rs485_mode()` function by
>> reordering resources_init() to occur after uart_get_rs485_mode.
> 
> What exactly is wrong with the current code?
> 
>> Remove multiple goto paths and use dev_err_probe to simplify error
>> paths.
> 
> Don't mix fixes with cleanups please.  Shouldn't this be 2 patches?
> 
> 
>>
>> Fixes: 4fcc287f3c69 ("serial: qcom-geni: Enable support for half-duplex mode")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Anup Kulkarni <quic_anupkulk@quicinc.com>
>> ---
>> v3->v4
>> - Added Fixes and Cc tag.
>>
>> v2->v3
>> - Reordered the function resources_init.
>> - Removed goto.
>> - Added dev_err_probe.
>>
>> v1->v2
>> - Updated commit message.
>> ---
>>  drivers/tty/serial/qcom_geni_serial.c | 38 ++++++++++-----------------
>>  1 file changed, 14 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/tty/serial/qcom_geni_serial.c b/drivers/tty/serial/qcom_geni_serial.c
>> index 32ec632fd080..be998dd45968 100644
>> --- a/drivers/tty/serial/qcom_geni_serial.c
>> +++ b/drivers/tty/serial/qcom_geni_serial.c
>> @@ -1882,15 +1882,9 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>>  	port->se.dev = &pdev->dev;
>>  	port->se.wrapper = dev_get_drvdata(pdev->dev.parent);
>>  
>> -	ret = port->dev_data->resources_init(uport);
>> -	if (ret)
>> -		return ret;
>> -
>>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> -	if (!res) {
>> -		ret = -EINVAL;
>> -		goto error;
>> -	}
>> +	if (!res)
>> +		return -EINVAL;
> 
> But now you are not calling the stuff that was previously at error:, are
> you sure that is ok?  If so, why?
> 
>>  
>>  	uport->mapbase = res->start;
>>  
>> @@ -1903,25 +1897,19 @@ static int qcom_geni_serial_probe(struct platform_device *pdev)
>>  	if (!data->console) {
>>  		port->rx_buf = devm_kzalloc(uport->dev,
>>  					    DMA_RX_BUF_SIZE, GFP_KERNEL);
>> -		if (!port->rx_buf) {
>> -			ret = -ENOMEM;
>> -			goto error;
>> -		}
>> +		if (!port->rx_buf)
>> +			return -ENOMEM;
> 
> Same here.
> 
> As you are mixing different things in the same patch, it's hard to find
> the original "bug" you are attempting to solve here.  I can't see it...
> 
> Please redo this as a patch series.
> 
> thanks,
> 
> greg k-h