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

[Greg KH <gregkh@linuxfoundation.org>]

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