| Message ID | 20221022125657.22530-4-ansuelsmth@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | thermal: qcom: tsens: small fixup for debugfs | expand |
On 22/10/2022 14:56, Christian Marangi wrote: > The current tsens debugfs structure is composed by: > - a tsens dir in debugfs with a version file > - a directory for each tsens istance with sensors file to dump all the > sensors value. s/istance/instance/ The patch looks good to me, no need to resend, I'll fix the typos > This works on the assumption that we have the same version for each > istance but this assumption seems fragile and with more than one tsens > istance results in the version file not tracking each of them. > > A better approach is to just create a subdirectory for each tsens > istance and put there version and sensors debugfs file. > > Using this new implementation results in less code since debugfs entry > are created only on successful tsens probe. > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > --- > drivers/thermal/qcom/tsens.c | 13 +++---------- > 1 file changed, 3 insertions(+), 10 deletions(-) > > diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c > index 467585c45d34..fc12d7c07de4 100644 > --- a/drivers/thermal/qcom/tsens.c > +++ b/drivers/thermal/qcom/tsens.c > @@ -704,21 +704,14 @@ 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) > - 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); > + debugfs_create_file("version", 0444, priv->debug, pdev, &dbg_version_fops); > debugfs_create_file("sensors", 0444, priv->debug, pdev, &dbg_sensors_fops); > } > #else
On Sat, Oct 22, 2022 at 03:08:46PM +0200, Daniel Lezcano wrote: > On 22/10/2022 14:56, Christian Marangi wrote: > > The current tsens debugfs structure is composed by: > > - a tsens dir in debugfs with a version file > > - a directory for each tsens istance with sensors file to dump all the > > sensors value. > > s/istance/instance/ > > The patch looks good to me, no need to resend, I'll fix the typos > Thanks for picking this, np for fixing typos. > > This works on the assumption that we have the same version for each > > istance but this assumption seems fragile and with more than one tsens > > istance results in the version file not tracking each of them. > > > > A better approach is to just create a subdirectory for each tsens > > istance and put there version and sensors debugfs file. > > > > Using this new implementation results in less code since debugfs entry > > are created only on successful tsens probe. > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > --- > > drivers/thermal/qcom/tsens.c | 13 +++---------- > > 1 file changed, 3 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c > > index 467585c45d34..fc12d7c07de4 100644 > > --- a/drivers/thermal/qcom/tsens.c > > +++ b/drivers/thermal/qcom/tsens.c > > @@ -704,21 +704,14 @@ 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) > > - 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); > > + debugfs_create_file("version", 0444, priv->debug, pdev, &dbg_version_fops); > > 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
On Sat, Oct 22, 2022 at 03:08:46PM +0200, Daniel Lezcano wrote: > On 22/10/2022 14:56, Christian Marangi wrote: > > The current tsens debugfs structure is composed by: > > - a tsens dir in debugfs with a version file > > - a directory for each tsens istance with sensors file to dump all the > > sensors value. > > s/istance/instance/ > > The patch looks good to me, no need to resend, I'll fix the typos > By checking linux-next it looks like the wrong revision was applied [0]. I think this was done by mistake while fixing the typo. Can this be fixed? The applied revision conflicts with what we agreed was a good solution. [0] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/log/drivers/thermal/qcom/tsens.c > > This works on the assumption that we have the same version for each > > istance but this assumption seems fragile and with more than one tsens > > istance results in the version file not tracking each of them. > > > > A better approach is to just create a subdirectory for each tsens > > istance and put there version and sensors debugfs file. > > > > Using this new implementation results in less code since debugfs entry > > are created only on successful tsens probe. > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > --- > > drivers/thermal/qcom/tsens.c | 13 +++---------- > > 1 file changed, 3 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c > > index 467585c45d34..fc12d7c07de4 100644 > > --- a/drivers/thermal/qcom/tsens.c > > +++ b/drivers/thermal/qcom/tsens.c > > @@ -704,21 +704,14 @@ 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) > > - 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); > > + debugfs_create_file("version", 0444, priv->debug, pdev, &dbg_version_fops); > > 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 467585c45d34..fc12d7c07de4 100644 --- a/drivers/thermal/qcom/tsens.c +++ b/drivers/thermal/qcom/tsens.c @@ -704,21 +704,14 @@ 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) - 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); + debugfs_create_file("version", 0444, priv->debug, pdev, &dbg_version_fops); debugfs_create_file("sensors", 0444, priv->debug, pdev, &dbg_sensors_fops); } #else
The current tsens debugfs structure is composed by: - a tsens dir in debugfs with a version file - a directory for each tsens istance with sensors file to dump all the sensors value. This works on the assumption that we have the same version for each istance but this assumption seems fragile and with more than one tsens istance results in the version file not tracking each of them. A better approach is to just create a subdirectory for each tsens istance and put there version and sensors debugfs file. Using this new implementation results in less code since debugfs entry are created only on successful tsens probe. Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> --- drivers/thermal/qcom/tsens.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-)