Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <cover.1572526427.git.amit.kucheria@linaro.org> <18717de35f31098d3ebc12564c2767b6d54d37d8.1572526427.git.amit.kucheria@linaro.org>
In-Reply-To: <18717de35f31098d3ebc12564c2767b6d54d37d8.1572526427.git.amit.kucheria@linaro.org>
From:   Doug Anderson <dianders@chromium.org>
Date:   Fri, 1 Nov 2019 15:53:26 -0700
Message-ID: <CAD=FV=X3o3JKye8pZZckEtk=9XNoajf-Kj8XGUemut7NS2bZjw@mail.gmail.com>
Subject: Re: [PATCH v7 03/15] drivers: thermal: tsens: Add __func__ identifier
 to debug statements
To:     Amit Kucheria <amit.kucheria@linaro.org>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        linux-arm-msm <linux-arm-msm@vger.kernel.org>,
        Bjorn Andersson <bjorn.andersson@linaro.org>,
        Eduardo Valentin <edubezval@gmail.com>,
        Andy Gross <agross@kernel.org>, masneyb@onstation.org,
        Stephen Boyd <swboyd@chromium.org>, julia.lawall@lip6.fr,
        Amit Kucheria <amit.kucheria@verdurent.com>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Rob Herring <robh+dt@kernel.org>,
        Zhang Rui <rui.zhang@intel.com>,
        Linux PM <linux-pm@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi,

On Thu, Oct 31, 2019 at 11:38 AM Amit Kucheria <amit.kucheria@linaro.org> wrote:
>
> Printing the function name when enabling debugging makes logs easier to
> read.
>
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
>  drivers/thermal/qcom/tsens-common.c | 8 ++++----
>  drivers/thermal/qcom/tsens.c        | 6 +++---
>  2 files changed, 7 insertions(+), 7 deletions(-)

Obviously my opinion doesn't trump maintainers ones, but I have always
heard that this is the wrong thing to do for "dev_xxx" debug
statements.  Specifically:

* For "dev_xxx" statements, the device name is already printed which
is pretty good for getting you to the right driver.

* Once you're in the right driver, error messages should be unique
enough to find the right function.

If having __func__ in all messages was beneficial then the "dev_xxx"
macros would include it by default.  They don't and such things just
uglify the logs and chew up log space.  I suppose you could try
including a CONFIG option to add it to all "dev_xxx" functions and see
if it would be accepted?  Then you can turn it on locally.

Using __func__ in cases where you don't happen to have a "struct
device" (and can't get one easily) makes some sense, but otherwise I
believe it should be kept out.

-Doug