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

[Mukesh Ojha]

On Wed, Mar 04, 2026 at 02:04:11PM -0600, Bjorn Andersson wrote:
> 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.

I double-checked and realized I was misled by downstream code that had
extra checks and returned errors without logs during debugging. Hence,
this change was made. For now, we can ignore it as we are already
covered

-Mukesh

> 
> > 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
> > 

-- 
-Mukesh Ojha