On 28/04/2021 18:47, Thara Gopinath wrote: > Hi, > > Please include a cover letter next time describing the patch series. Yes, a cover letter helps for the understanding of a patch series but in this case the changes are simple enough to get rid of it. > On 4/18/21 9:29 PM, Ansuel Smith wrote: >> Simplify debugfs init function. >> - Drop useless variables >> - Add check for existing dev directory. >> - Fix wrong version in dbg_version_show (with version 0.0.0, 0.1.0 was >> incorrectly reported) >> >> Signed-off-by: Ansuel Smith <ansuelsmth@xxxxxxxxx> >> --- >> drivers/thermal/qcom/tsens.c | 16 +++++++--------- >> 1 file changed, 7 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c >> index f9d50a67e..b086d1496 100644 >> --- a/drivers/thermal/qcom/tsens.c >> +++ b/drivers/thermal/qcom/tsens.c >> @@ -692,7 +692,7 @@ static int dbg_version_show(struct seq_file *s, >> void *data) >> return ret; >> seq_printf(s, "%d.%d.%d\n", maj_ver, min_ver, step_ver); >> } else { >> - seq_puts(s, "0.1.0\n"); >> + seq_printf(s, "0.%d.0\n", priv->feat->ver_major); >> } >> return 0; >> @@ -704,21 +704,19 @@ DEFINE_SHOW_ATTRIBUTE(dbg_sensors); >> static void tsens_debug_init(struct platform_device *pdev) >> { >> struct tsens_priv *priv = platform_get_drvdata(pdev); >> - struct dentry *root, *file; >> - root = debugfs_lookup("tsens", NULL); >> - if (!root) >> + priv->debug_root = debugfs_lookup("tsens", NULL); >> + if (!priv->debug_root) >> priv->debug_root = debugfs_create_dir("tsens", NULL); >> - else >> - priv->debug_root = root; >> - file = debugfs_lookup("version", priv->debug_root); >> - if (!file) >> + if (!debugfs_lookup("version", priv->debug_root)) >> debugfs_create_file("version", 0444, priv->debug_root, >> pdev, &dbg_version_fops); >> /* A directory for each instance of the TSENS IP */ >> - priv->debug = debugfs_create_dir(dev_name(&pdev->dev), >> priv->debug_root); > > Unconditionally creating priv->debug here is correct. The below if > (!priv->debug) will never be true because as per your patch 1, we call > tsens_debug_init once per instance of tsens. > >> + priv->debug = debugfs_lookup(dev_name(&pdev->dev), >> priv->debug_root); >> + if (!priv->debug) >> + priv->debug = debugfs_create_dir(dev_name(&pdev->dev), >> priv->debug_root); >> debugfs_create_file("sensors", 0444, priv->debug, pdev, >> &dbg_sensors_fops); >> } >> #else >> > -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog