Skip to contentSkip to conversation

thermal: qcom: tsens: small fixup for debugfs

PATCH Applied Christian Marangi 3 participants 11 messages Oct 22, 2022 – Dec 9, 2022

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v3 cover thermal: qcom: tsens: small fixup for debugfs

This is a small series to fixup some bug in how tsens init debufs.

The first patch just handle situation where tsens fails to register and debugfs are getting registred anyway. When tsens is tried to reprobe debugs will print a warning as the directory are already there.

The second patch is a fixup for wrong version when the ancient VER_0 is used.

The third patch is a rework of debugfs structure moving version in the tsens istance instead of ignoring any other tsens istance if it will ever be the case in the future of having multiple tsens instance with different version. It's just futureproof on it's own and also removed one additional check.

changes v3:
- remove extra space from patch 1
- split patch 2 to 2 different patch
- patch 3 rework wrong debugfs structure
changes v2:
- Changed sob name to new one.

(the resend was actually v2 but i totally forgot that I sent it as v2 with the sob name fixed... but everything should be good now...)

Christian Marangi (3):
thermal: qcom: tsens: init debugfs only with successful probe
thermal: qcom: tsens: fix wrong version id dbg_version_show
thermal: qcom: tsens: rework debugfs file structure

 drivers/thermal/qcom/tsens.c | 23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)
Signature · 1 line
2.37.2
#1 lore raw

PATCH v3 1/3 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 | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
drivers/thermal/qcom/tsens.c +5 -3
index b1b10005fb28..af688ce4d14d 100644
@@ -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;
@@ -1153,7 +1151,11 @@ 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.37.2
#2 lore raw

[thermal: thermal/next] thermal/drivers/qcom/tsens: Init debugfs only with successful probe

The following commit has been merged into the thermal/next branch of thermal:

The following commit has been merged into the thermal/next branch of thermal:

Commit-ID:     4d248fcb7c80e67dc22338ed44e00ac5d96ab0be
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git//4d248fcb7c80e67dc22338ed44e00ac5d96ab0be
Author:        Christian Marangi <[email protected]>
AuthorDate:    Sat, 22 Oct 2022 14:56:55 +02:00
Committer:     Daniel Lezcano <[email protected]>
CommitterDate: Thu, 08 Dec 2022 14:30:44 +01:00

thermal/drivers/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 | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
drivers/thermal/qcom/tsens.c +5 -3
index b7d9782..3ac7804 100644
@@ -939,8 +939,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;
@@ -1182,7 +1180,11 @@ 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)
#3 lore raw

PATCH v3 2/3 thermal: qcom: tsens: fix wrong version id dbg_version_show

In reply to Christian Marangi

For VER_0 the version was incorrectly reported as 0.1.0.

Fix that and correctly report the major version for this old tsens revision.

---
 drivers/thermal/qcom/tsens.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/thermal/qcom/tsens.c +1 -1
index af688ce4d14d..467585c45d34 100644
@@ -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;
Signature · 1 line
2.37.2
#4 lore raw

[thermal: thermal/next] thermal/drivers/qcom/tsens: Fix wrong version id dbg_version_show

The following commit has been merged into the thermal/next branch of thermal:

The following commit has been merged into the thermal/next branch of thermal:

Commit-ID:     f48301cee9719fdfa469bb619297655aa8c811f0
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git//f48301cee9719fdfa469bb619297655aa8c811f0
Author:        Christian Marangi <[email protected]>
AuthorDate:    Sat, 22 Oct 2022 14:56:56 +02:00
Committer:     Daniel Lezcano <[email protected]>
CommitterDate: Thu, 08 Dec 2022 14:30:44 +01:00

thermal/drivers/qcom/tsens: Fix wrong version id dbg_version_show

For VER_0 the version was incorrectly reported as 0.1.0.

Fix that and correctly report the major version for this old tsens revision.

---
 drivers/thermal/qcom/tsens.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/thermal/qcom/tsens.c +1 -1
index 3ac7804..5f8d8f0 100644
@@ -713,7 +713,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;
#5 lore raw

PATCH v3 3/3 thermal: qcom: tsens: rework debugfs file structure

In reply to Christian Marangi

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.

---
 drivers/thermal/qcom/tsens.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)
drivers/thermal/qcom/tsens.c +3 -10
index 467585c45d34..fc12d7c07de4 100644
@@ -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
Signature · 1 line
2.37.2
#6 lore raw

Re: [PATCH v3 3/3] thermal: qcom: tsens: rework debugfs file structure

Quoted 4 lines · Christian Marangi

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

Quoted 44 lines

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.

---
  drivers/thermal/qcom/tsens.c | 13 +++----------
  1 file changed, 3 insertions(+), 10 deletions(-)
drivers/thermal/qcom/tsens.c +3 -10
index 467585c45d34..fc12d7c07de4 100644
@@ -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
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
#7 lore raw

Re: [PATCH v3 3/3] thermal: qcom: tsens: rework debugfs file structure

Quoted 10 lines · Daniel Lezcano
Quoted 4 lines · Christian Marangi

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.

Quoted 50 lines
Quoted 42 lines

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.

---
  drivers/thermal/qcom/tsens.c | 13 +++----------
  1 file changed, 3 insertions(+), 10 deletions(-)
drivers/thermal/qcom/tsens.c +3 -10
index 467585c45d34..fc12d7c07de4 100644
@@ -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
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
Signature · 1 line
	Ansuel
#8 lore raw

Re: [PATCH v3 3/3] thermal: qcom: tsens: rework debugfs file structure

In reply to Daniel Lezcano
Quoted 10 lines · Daniel Lezcano
Quoted 4 lines · Christian Marangi

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

Quoted 50 lines
Quoted 42 lines

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.

---
  drivers/thermal/qcom/tsens.c | 13 +++----------
  1 file changed, 3 insertions(+), 10 deletions(-)
drivers/thermal/qcom/tsens.c +3 -10
index 467585c45d34..fc12d7c07de4 100644
@@ -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
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
Signature · 1 line
	Ansuel
#9 lore raw

[thermal: thermal/next] thermal/drivers/qcom/tsens: Rework debugfs file structure

In reply to Christian Marangi
The following commit has been merged into the thermal/next branch of thermal:

The following commit has been merged into the thermal/next branch of thermal:

Commit-ID:     c6a32aca175ef04adcfe6344145d1857c917dde4
Gitweb:        https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git//c6a32aca175ef04adcfe6344145d1857c917dde4
Author:        Christian Marangi <[email protected]>
AuthorDate:    Sat, 22 Oct 2022 14:56:57 +02:00
Committer:     Daniel Lezcano <[email protected]>
CommitterDate: Thu, 08 Dec 2022 14:30:44 +01:00

thermal/drivers/qcom/tsens: Rework debugfs file structure

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.

---
 drivers/thermal/qcom/tsens.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)
drivers/thermal/qcom/tsens.c +3 -10
index 5f8d8f0..b5b136f 100644
@@ -725,21 +725,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
#10 lore raw
In reply to Christian Marangi
Quoted 25 lines · Christian Marangi

This is a small series to fixup some bug in how tsens init debufs.

The first patch just handle situation where tsens fails to register and debugfs are getting registred anyway. When tsens is tried to reprobe debugs will print a warning as the directory are already there.

The second patch is a fixup for wrong version when the ancient VER_0 is used.

The third patch is a rework of debugfs structure moving version in the tsens istance instead of ignoring any other tsens istance if it will ever be the case in the future of having multiple tsens instance with different version. It's just futureproof on it's own and also removed one additional check.

changes v3:
- remove extra space from patch 1
- split patch 2 to 2 different patch
- patch 3 rework wrong debugfs structure
changes v2:
- Changed sob name to new one.

(the resend was actually v2 but i totally forgot that I sent it as v2 with the sob name fixed... but everything should be good now...)

Quoted 7 lines

Christian Marangi (3):
thermal: qcom: tsens: init debugfs only with successful probe
thermal: qcom: tsens: fix wrong version id dbg_version_show
thermal: qcom: tsens: rework debugfs file structure

  drivers/thermal/qcom/tsens.c | 23 +++++++++--------------
  1 file changed, 9 insertions(+), 14 deletions(-)

Applied thanks

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