| Message ID | 20221021181906.16647-2-ansuelsmth@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [RESEND,1/2] thermal: qcom: tsens: init debugfs only with successful probe | expand |
On Fri, Oct 21, 2022 at 08:33:41PM +0200, Daniel Lezcano wrote: > On 21/10/2022 20:19, Christian Marangi wrote: > > Simplify debugfs init function. > > - 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: Christian Marangi <ansuelsmth@gmail.com> > > Reviewed-by: Thara Gopinath <thara.gopinath@linaro.org> > > --- > > drivers/thermal/qcom/tsens.c | 14 +++++--------- > > 1 file changed, 5 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c > > index cc2965b8d409..ff82626cecef 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,17 @@ 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); > > + priv->debug = debugfs_lookup(dev_name(&pdev->dev), priv->debug_root); > > Why the directory creation is replaced by the lookup ? > Hi, thanks for the review! I have to be honest and do not create some fake excuse for this. This patch is a bit old and was pending from a long time so out of despair i just tried to RESEND this hoping someone would pick it up. And it seems it have worked... Sooo sorry for making you asking this. On rechecking the change here, the entire debug_init logic seems wrong... I don't know if it's possible but what if in the system there are multiple version of tsens istance? Looks wrong to overwrite the version with the last one... I think the original idea of this was to create a directory for each istance and put in there version and sensors debugfs. I will propose this in the next version if it's ok for you and you agree with this logic. Also I think I will split this in 2 different patch one for the version fixup and one for this new logic. Waiting for your feedback and again sorry for the noise. > > 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
diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c index cc2965b8d409..ff82626cecef 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,17 @@ 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); + priv->debug = debugfs_lookup(dev_name(&pdev->dev), priv->debug_root); debugfs_create_file("sensors", 0444, priv->debug, pdev, &dbg_sensors_fops); } #else