Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Date:   Mon, 16 Oct 2023 11:38:16 -0700
From:   Bjorn Andersson <andersson@kernel.org>
To:     Luca Weiss <luca.weiss@fairphone.com>
Cc:     Andy Gross <agross@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        ~postmarketos/upstreaming@lists.sr.ht, phone-devel@vger.kernel.org,
        linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] soc: qcom: pmic_glink_altmode: Print error when retimer
 setup fails
Message-ID: <oay4wz2klkttkxocmtavldjna5vqzo3lmee5uie7yciukvc5cs@yudo2uadv7qe>
References: <20231013-glink-altmode-ret-v1-1-77941537a35b@fairphone.com>
 <6fw7eho6rapvlghujche4k3pm5mx7a7ojx6yyyreq6dhzjfwlt@ggqoxgirpcnr>
 <CW9O1NB1NZHP.34BW2ZGP1WV8C@fairphone.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CW9O1NB1NZHP.34BW2ZGP1WV8C@fairphone.com>
Precedence: bulk

On Mon, Oct 16, 2023 at 08:56:31AM +0200, Luca Weiss wrote:
> On Mon Oct 16, 2023 at 5:18 AM CEST, Bjorn Andersson wrote:
> > On Fri, Oct 13, 2023 at 03:56:40PM +0200, Luca Weiss wrote:
> > > It can be useful to know with which return value the retimer_set call
> > > failed, so include this info in the dev_err print.
> > > 
> >
> > Is this useful during development, or during normal execution etc. How
> > about using kretprobe or similar tools?
> 
> Hi Bjorn,
> 
> IIRC I encountered this during development of my retimer driver, where
> the op in that driver failed for some reason and it was very useful to
> get the return value to debug that.
> 
> And sure, I guess kretprobe might be also useful here but I think it's
> very common to include the return value in the error message when
> something fails, no?
> 

The problem with the error message is that you often get some generic
error code, but don't know where it came from anyway. So, I typically
use function_graph and set_graph_function to capture the path through
the called function(s)...

But that said, it is fairly common to include the error value, so I am
not against it.

> > If you insist, could you please make sure that the style matches across
> > the various typec_*_set() calls in the driver?
> 
> Do you mean adding the return value to the other dev_err prints after
> typec_*_set() calls also?
> 

I mean that we should be consistent across the error prints, and either
include the error value in all or none of the typec error prints.

Regards,
Bjorn