linux-kernel - Re: [PATCH] soc: qcom: pmic_glink_altmode: Print error when retimer setup fails

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 16 Oct 2023 11:38:16 -0700
From:   Bjorn Andersson <andersson@...nel.org>
To:     Luca Weiss <luca.weiss@...rphone.com>
Cc:     Andy Gross <agross@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        ~postmarketos/upstreaming@...ts.sr.ht, phone-devel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] soc: qcom: pmic_glink_altmode: Print error when retimer
 setup fails

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