thermal: qcom: tsens: init debugfs only with successful probe
Subsystems (3)
PATCH 1/2 thermal: qcom: tsens: init debugfs only with successful probe
calibrate and tsens_register can fail or PROBE_DEFER. This will cause a double or a wrong init of the debugfs information. Init debugfs only with successful probe fixing warning about directory already present.
--- drivers/thermal/qcom/tsens.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/thermal/qcom/tsens.c +6 -3
@@ -918,8 +918,6 @@ int __init init_common(struct tsens_priv *priv)if (tsens_version (priv) >=VER_0_1 )tsens_enable_irq (priv); -tsens_debug_init (op); - err_put_device:put_device (&op->dev);return ret;
@@ -1158,7 +1156,12 @@ static int tsens_probe(struct platform_device *pdev) } } -return tsens_register (priv); + ret =tsens_register (priv); + +if (!ret) +tsens_debug_init (pdev); + +return ret; }static int tsens_remove (struct platform_device *pdev)
Signature · 1 line
2.30.2
PATCH 2/2 thermal: qcom: tsens: simplify debugfs init function
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)
--- drivers/thermal/qcom/tsens.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)
drivers/thermal/qcom/tsens.c +7 -9
@@ -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); + 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
Signature · 1 line
2.30.2
Re: [thermal-next PATCH 2/2] thermal: qcom: tsens: simplify debugfs init function
Hi,
Please include a cover letter next time describing the patch series.
Quoted 36 lines · Ansuel Smith
Simplify debugfs init function.
- Drop useless variables
- Add check for existing dev directory.
- Fix wrong version indbg_version_show(with version 0.0.0, 0.1.0 was incorrectly reported)Signed-off-by Ansuel Smith <[email protected]>--- drivers/thermal/qcom/tsens.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)drivers/thermal/qcom/tsens.c +3
-5@@ -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
drivers/thermal/qcom/tsens.c DEFINE_SHOW_ATTRIBUTE(dbg_sensors); · line 721
Quoted 10 lines
drivers/thermal/qcom/tsens.c +1
-4- 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.
Quoted 7 lines
drivers/thermal/qcom/tsens.c +3
-0+ 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
Signature · 2 lines
Warm Regards Thara
Re: [thermal-next PATCH 2/2] thermal: qcom: tsens: simplify debugfs init function
Quoted 4 lines · Thara Gopinath
Hi,
Please include a cover letter next time describing the patch series.
Yes sorry, I tought that for a small series (2 patch) it wasn't needed.
Quoted 39 lines · Ansuel Smith
Quoted 38 lines
Simplify debugfs init function.
- Drop useless variables
- Add check for existing dev directory.
- Fix wrong version indbg_version_show(with version 0.0.0, 0.1.0 was incorrectly reported)Signed-off-by Ansuel Smith <[email protected]>--- drivers/thermal/qcom/tsens.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)drivers/thermal/qcom/tsens.c +4
-8@@ -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))
Quoted 9 lines
drivers/thermal/qcom/tsens.c DEFINE_SHOW_ATTRIBUTE(dbg_sensors); · line 721
Quoted 4 lines
drivers/thermal/qcom/tsens.c +0
-1debugfs_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->debughere is correct. The below if (!priv->debug) will never be true because as per your patch 1, we calltsens_debug_initonce per instance of tsens.
You are right, will send a v2 if everything else is good. What do you think?
Quoted 11 lines
Quoted 7 lines
drivers/thermal/qcom/tsens.c +3
-0+ 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 Signature · 2 lines
Warm Regards Thara
Re: [thermal-next PATCH 2/2] thermal: qcom: tsens: simplify debugfs init function
Quoted 57 lines · Ansuel Smith
Quoted 4 lines · Thara Gopinath
Hi,
Please include a cover letter next time describing the patch series.
Yes sorry, I tought that for a small series (2 patch) it wasn't needed.
Quoted 39 lines · Ansuel Smith
Quoted 38 lines
Simplify debugfs init function.
- Drop useless variables
- Add check for existing dev directory.
- Fix wrong version indbg_version_show(with version 0.0.0, 0.1.0 was incorrectly reported)Signed-off-by Ansuel Smith <[email protected]>--- drivers/thermal/qcom/tsens.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)drivers/thermal/qcom/tsens.c +4
-8@@ -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))Quoted 9 lines
drivers/thermal/qcom/tsens.c DEFINE_SHOW_ATTRIBUTE(dbg_sensors); · line 721
Quoted 4 lines
drivers/thermal/qcom/tsens.c +0
-1debugfs_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->debughere is correct. The below if (!priv->debug) will never be true because as per your patch 1, we calltsens_debug_initonce per instance of tsens.
Quoted 2 lines
You are right, will send a v2 if everything else is good. What do you think?
I have not tested this yet. The clean up itself looks okay to me.
My question is have you tried this with 8960 tsens ? That is the only version of tsens that does not use init_common and hence looks to me that a debug interface is not created. I don't think this should be a problem though. So if you can fix the above, it is a go ahead from me.
Signature · 2 lines
Warm Regards Thara
Re: [thermal-next PATCH 2/2] thermal: qcom: tsens: simplify debugfs init function
Quoted 60 lines · Thara Gopinath
Quoted 57 lines · Ansuel Smith
Quoted 4 lines · Thara Gopinath
Hi,
Please include a cover letter next time describing the patch series.
Yes sorry, I tought that for a small series (2 patch) it wasn't needed.
Quoted 48 lines · Ansuel Smith
Quoted 32 lines
Simplify debugfs init function.
- Drop useless variables
- Add check for existing dev directory.
- Fix wrong version indbg_version_show(with version 0.0.0, 0.1.0 was incorrectly reported)Signed-off-by Ansuel Smith <[email protected]>--- drivers/thermal/qcom/tsens.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)drivers/thermal/qcom/tsens.c +3
-4@@ -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)drivers/thermal/qcom/tsens.c DEFINE_SHOW_ATTRIBUTE(dbg_sensors); · line 718
Quoted 10 lines
drivers/thermal/qcom/tsens.c +1
-5priv->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->debughere is correct. The below if (!priv->debug) will never be true because as per your patch 1, we calltsens_debug_initonce per instance of tsens.
Quoted 9 lines
Quoted 2 lines
You are right, will send a v2 if everything else is good. What do you think?
I have not tested this yet. The clean up itself looks okay to me. My question is have you tried this with 8960 tsens ? That is the only version of tsens that does not use
init_commonand hence looks to me that a debug interface is not created. I don't think this should be a problem though. So if you can fix the above, it is a go ahead from me.
Recent commits should have switched 8960 to init_common. Actually I pushed this cause while testing 8960 I notice the warning about double debugfs. Anyway thx for the review. Will send v2 ASAP.
Quoted 4 lines
Signature · 3 lines
-- Warm Regards Thara
Re: [thermal-next PATCH 2/2] thermal: qcom: tsens: simplify debugfs init function
Quoted 71 lines · Ansuel Smith
Quoted 60 lines · Thara Gopinath
Quoted 57 lines · Ansuel Smith
Quoted 4 lines · Thara Gopinath
Hi,
Please include a cover letter next time describing the patch series.
Yes sorry, I tought that for a small series (2 patch) it wasn't needed.
Quoted 48 lines · Ansuel Smith
Quoted 32 lines
Simplify debugfs init function.
- Drop useless variables
- Add check for existing dev directory.
- Fix wrong version indbg_version_show(with version 0.0.0, 0.1.0 was incorrectly reported)Signed-off-by Ansuel Smith <[email protected]>--- drivers/thermal/qcom/tsens.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)drivers/thermal/qcom/tsens.c +3
-4@@ -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)drivers/thermal/qcom/tsens.c DEFINE_SHOW_ATTRIBUTE(dbg_sensors); · line 718
Quoted 10 lines
drivers/thermal/qcom/tsens.c +1
-5priv->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->debughere is correct. The below if (!priv->debug) will never be true because as per your patch 1, we calltsens_debug_initonce per instance of tsens.Quoted 9 lines
Quoted 2 lines
You are right, will send a v2 if everything else is good. What do you think?
I have not tested this yet. The clean up itself looks okay to me. My question is have you tried this with 8960 tsens ? That is the only version of tsens that does not use
init_commonand hence looks to me that a debug interface is not created. I don't think this should be a problem though. So if you can fix the above, it is a go ahead from me.
Quoted 3 lines
Recent commits should have switched 8960 to
init_common. Actually I pushed this cause while testing 8960 I notice the warning about double debugfs. Anyway thx for the review. Will send v2 ASAP.
Sounds good. Thanks.
Quoted 5 lines
Quoted 5 lines
Signature · 3 lines
-- Warm Regards Thara
Signature · 2 lines
Warm Regards Thara
Re: [thermal-next PATCH 2/2] thermal: qcom: tsens: simplify debugfs init function
In reply to Thara GopinathQuoted 3 lines · Thara Gopinath
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.
Quoted 61 lines · Ansuel Smith
Quoted 34 lines
Simplify debugfs init function.
- Drop useless variables
- Add check for existing dev directory.
- Fix wrong version indbg_version_show(with version 0.0.0, 0.1.0 was incorrectly reported)Signed-off-by Ansuel Smith <[email protected]>---
drivers/thermal/qcom/tsens.c| 16 +++++++---------1 file changed, 7 insertions(+), 9 deletions(-)drivers/thermal/qcom/tsens.c +3
-3@@ -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);drivers/thermal/qcom/tsens.c DEFINE_SHOW_ATTRIBUTE(dbg_sensors); · line 716
Quoted 10 lines
drivers/thermal/qcom/tsens.c +1
-4- 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->debughere is correct. The below if (!priv->debug) will never be true because as per your patch 1, we calltsens_debug_initonce per instance of tsens.Quoted 10 lines
drivers/thermal/qcom/tsens.c +3
-0+ 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
Signature · 5 lines
<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