Re: [PATCH 1/2] soc: qcom: qmi: Print error codes in failure paths

[Bjorn Andersson]

On Thu, Jan 29, 2026 at 08:53:19PM +0530, Mukesh Ojha wrote:
> Few error paths in the qmi_interface module log a failure message but
> do not include the actual error code. Include the error value in the log
> so debugging failures becomes easier.
> 

I'm definitely in favor of such improvements! But I would like to make
sure we get most signal-to-noise ratio out of it.

> Signed-off-by: Mukesh Ojha <mukesh.ojha@xxxxxxxxxxxxxxxx>
> ---
>  drivers/soc/qcom/qmi_interface.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/soc/qcom/qmi_interface.c b/drivers/soc/qcom/qmi_interface.c
> index 6500f863aae5..941612b1bd2e 100644
> --- a/drivers/soc/qcom/qmi_interface.c
> +++ b/drivers/soc/qcom/qmi_interface.c
> @@ -321,7 +321,7 @@ int qmi_txn_init(struct qmi_handle *qmi, struct qmi_txn *txn,
>  	mutex_lock(&qmi->txn_lock);
>  	ret = idr_alloc_cyclic(&qmi->txns, txn, 0, U16_MAX, GFP_KERNEL);
>  	if (ret < 0)
> -		pr_err("failed to allocate transaction id\n");
> +		pr_err("failed to allocate transaction id: %d\n", ret);

Seems this can be either -ENOMEM or -ENOSPC. In the prior case don't we
already have an error message. For the latter, is it realistic that
someone has created 16384 QMI transactions?

>  
>  	txn->id = ret;
>  	mutex_unlock(&qmi->txn_lock);
> @@ -413,7 +413,7 @@ static void qmi_invoke_handler(struct qmi_handle *qmi, struct sockaddr_qrtr *sq,
>  
>  	ret = qmi_decode_message(buf, len, handler->ei, dest);
>  	if (ret < 0)
> -		pr_err("failed to decode incoming message\n");
> +		pr_err("failed to decode incoming message: %d\n", ret);

As far as I can see, most of the qmi_decode_message() error paths has a
more useful error print already. Which errors have you seen? Can we
provide an even more useful print in those cases?

>  	else
>  		handler->fn(qmi, sq, txn, dest);
>  
> @@ -502,7 +502,7 @@ static void qmi_handle_message(struct qmi_handle *qmi,
>  		if (txn->dest && txn->ei) {
>  			ret = qmi_decode_message(buf, len, txn->ei, txn->dest);
>  			if (ret < 0)
> -				pr_err("failed to decode incoming message\n");
> +				pr_err("failed to decode incoming message: %d\n", ret);

Ditto

>  
>  			txn->result = ret;
>  			complete(&txn->completion);
> @@ -661,8 +661,8 @@ int qmi_handle_init(struct qmi_handle *qmi, size_t recv_buf_size,
>  		if (PTR_ERR(qmi->sock) == -EAFNOSUPPORT) {
>  			ret = -EPROBE_DEFER;
>  		} else {
> -			pr_err("failed to create QMI socket\n");
>  			ret = PTR_ERR(qmi->sock);
> +			pr_err("failed to create QMI socket: %d\n", ret);

pr_err("failed to create QMI socket: %pe\n", qmi->soc) seems like it
would be more helpful.

>  		}
>  		goto err_destroy_wq;
>  	}
> @@ -766,7 +766,7 @@ static ssize_t qmi_send_message(struct qmi_handle *qmi,
>  	if (qmi->sock) {
>  		ret = kernel_sendmsg(qmi->sock, &msghdr, &iv, 1, len);
>  		if (ret < 0)
> -			pr_err("failed to send QMI message\n");
> +			pr_err("failed to send QMI message: %d\n", ret);

Ditto.

Regards,
Bjorn

>  	} else {
>  		ret = -EPIPE;
>  	}
> -- 
> 2.50.1
>