| Message ID | 18717de35f31098d3ebc12564c2767b6d54d37d8.1572526427.git.amit.kucheria@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Eduardo Valentin |
| Headers | show |
| Series | thermal: qcom: tsens: Add interrupt support | expand |
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
diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c index c037bdf92c66..7437bfe196e5 100644 --- a/drivers/thermal/qcom/tsens-common.c +++ b/drivers/thermal/qcom/tsens-common.c @@ -42,8 +42,8 @@ void compute_intercept_slope(struct tsens_priv *priv, u32 *p1, for (i = 0; i < priv->num_sensors; i++) { dev_dbg(priv->dev, - "sensor%d - data_point1:%#x data_point2:%#x\n", - i, p1[i], p2[i]); + "%s: sensor%d - data_point1:%#x data_point2:%#x\n", + __func__, i, p1[i], p2[i]); priv->sensor[i].slope = SLOPE_DEFAULT; if (mode == TWO_PT_CALIB) { @@ -60,7 +60,7 @@ void compute_intercept_slope(struct tsens_priv *priv, u32 *p1, priv->sensor[i].offset = (p1[i] * SLOPE_FACTOR) - (CAL_DEGC_PT1 * priv->sensor[i].slope); - dev_dbg(priv->dev, "offset:%d\n", priv->sensor[i].offset); + dev_dbg(priv->dev, "%s: offset:%d\n", __func__, priv->sensor[i].offset); } } @@ -209,7 +209,7 @@ int __init init_common(struct tsens_priv *priv) if (ret) goto err_put_device; if (!enabled) { - dev_err(dev, "tsens device is not enabled\n"); + dev_err(dev, "%s: device not enabled\n", __func__); ret = -ENODEV; goto err_put_device; } diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c index 542a7f8c3d96..06c6bbd69a1a 100644 --- a/drivers/thermal/qcom/tsens.c +++ b/drivers/thermal/qcom/tsens.c @@ -127,7 +127,7 @@ static int tsens_probe(struct platform_device *pdev) of_property_read_u32(np, "#qcom,sensors", &num_sensors); if (num_sensors <= 0) { - dev_err(dev, "invalid number of sensors\n"); + dev_err(dev, "%s: invalid number of sensors\n", __func__); return -EINVAL; } @@ -156,7 +156,7 @@ static int tsens_probe(struct platform_device *pdev) ret = priv->ops->init(priv); if (ret < 0) { - dev_err(dev, "tsens init failed\n"); + dev_err(dev, "%s: init failed\n", __func__); return ret; } @@ -164,7 +164,7 @@ static int tsens_probe(struct platform_device *pdev) ret = priv->ops->calibrate(priv); if (ret < 0) { if (ret != -EPROBE_DEFER) - dev_err(dev, "tsens calibration failed\n"); + dev_err(dev, "%s: calibration failed\n", __func__); return ret; } }