qcom: Add support for TSENS driver

drivers/clk/qcom/gcc-msm8960.c +1 -0

index eb6a4f9..2c80d03 100644
@@ -15,6 +15,7 @@
 #include <linux/bitops.h>
 #include <linux/err.h>
 #include <linux/platform_device.h>
+#include <linux/of_platform.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>

Quoted 16 lines · Rajendra Nayak

drivers/clk/qcom/gcc-msm8960.c +1 -0

index eb6a4f9..2c80d03 100644
@@ -15,6 +15,7 @@
  #include <linux/bitops.h>
  #include <linux/err.h>
  #include <linux/platform_device.h>
+#include <linux/of_platform.h>
  #include <linux/module.h>
  #include <linux/of.h>
  #include <linux/of_device.h>

@@ -3520,7 +3521,8 @@ static int gcc_msm8960_probe(struct platform_device *pdev)
  	if (IS_ERR(clk))
  		return PTR_ERR(clk);
 

drivers/clk/qcom/gcc-msm8960.c static int gcc_msm8960_probe(struct platform_device *pdev) · line 3525

Quoted 3 lines

drivers/clk/qcom/gcc-msm8960.c +2 -1

-	return qcom_cc_probe(pdev, match->data);
+	qcom_cc_probe(pdev, match->data);
+	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);

We just lost the error code from qcom_cc_probe()...

Also, I don't like having a subnode in DT. Why can't we use the same node as the GCC node and create a virtual child device here for tsens? We can assign the same of_node that this platform device has so that DT keeps working correctly.

Quoted 19 lines · Rajendra Nayak

Quoted 18 lines

drivers/clk/qcom/gcc-msm8960.c +1 -0

index eb6a4f9..2c80d03 100644
@@ -15,6 +15,7 @@
  #include <linux/bitops.h>
  #include <linux/err.h>
  #include <linux/platform_device.h>
+#include <linux/of_platform.h>
  #include <linux/module.h>
  #include <linux/of.h>
  #include <linux/of_device.h>

@@ -3520,7 +3521,8 @@ static int gcc_msm8960_probe(struct
 platform_device *pdev)
      if (IS_ERR(clk))
          return PTR_ERR(clk);
 

Quoted 7 lines

drivers/clk/qcom/gcc-msm8960.c static int gcc_msm8960_probe(struct platform_device *pdev)

Quoted 4 lines

drivers/clk/qcom/gcc-msm8960.c +2 -1

-    return qcom_cc_probe(pdev, match->data);
+    qcom_cc_probe(pdev, match->data);
+    return of_platform_populate(pdev->dev.of_node, NULL, NULL,
 &pdev->dev);

We just lost the error code from qcom_cc_probe()...

I think Rajendra picked up the wrong patch for this series, I did submit a v2 (https://patches.linaro.org/44033/) with the above fixed.

Quoted 4 lines

Also, I don't like having a subnode in DT. Why can't we use the same node as the GCC node and create a virtual child device here for tsens? We can assign the same of_node that this platform device has so that DT keeps working correctly.

Quoted 27 lines · Stephen Boyd

Quoted 19 lines · Rajendra Nayak

Quoted 18 lines

drivers/clk/qcom/gcc-msm8960.c +1 -0

index eb6a4f9..2c80d03 100644
@@ -15,6 +15,7 @@
 #include <linux/bitops.h>
 #include <linux/err.h>
 #include <linux/platform_device.h>
+#include <linux/of_platform.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>

@@ -3520,7 +3521,8 @@ static int gcc_msm8960_probe(struct
 platform_device *pdev)
     if (IS_ERR(clk))
         return PTR_ERR(clk);
 

Quoted 7 lines

drivers/clk/qcom/gcc-msm8960.c static int gcc_msm8960_probe(struct platform_device *pdev)

Quoted 4 lines

drivers/clk/qcom/gcc-msm8960.c +2 -1

-    return qcom_cc_probe(pdev, match->data);
+    qcom_cc_probe(pdev, match->data);
+    return of_platform_populate(pdev->dev.of_node, NULL, NULL,
 &pdev->dev);

We just lost the error code from qcom_cc_probe()...

Quoted 2 lines

I think Rajendra picked up the wrong patch for this series, I did submit a v2 (https://patches.linaro.org/44033/) with the above fixed.

ah, sorry about that.

Quoted 4 lines

Quoted 4 lines

Also, I don't like having a subnode in DT. Why can't we use the same node as the GCC node and create a virtual child device here for tsens? We can assign the same of_node that this platform device has so that DT keeps working correctly.

So the current driver looks up data based on compatible strings.

So you suggesting to create a virtual child device for gcc and associate the gcc DT node to it? (And have the tsens compatible mentioned as part of the gcc DT node?)

How is this any different from creating a subnode in DT anyway?

Quoted 6 lines

Quoted 4 lines

Quoted 4 lines

Also, I don't like having a subnode in DT. Why can't we use the same node as the GCC node and create a virtual child device here for tsens? We can assign the same of_node that this platform device has so that DT keeps working correctly.

So the current driver looks up data based on compatible strings.

The tsens device is always the same piece of hardware. The only

thing that's changing is the qfprom data and the number of sensors. So we should be looking at the qfprom compatible string

to figure out how to interpret the qfprom data which would include the number of sensors and how the data is encoded.

Quoted 3 lines

So you suggesting to create a virtual child device for gcc and associate the gcc DT node to it? (And have the tsens compatible mentioned as part of the gcc DT node?)

No. The driver should work just fine without having to interrogate the device's compatible string. If we still need the compatible check for some reason, then we can always match based on qcom,gcc-msm8960, qcom,gcc-apq8064, etc. But I don't see why

we need to do that when we should be looking at what type of qfprom is connected so we can correctly parse the data.

Quoted 8 lines

Quoted 6 lines

Quoted 4 lines

Quoted 4 lines

Also, I don't like having a subnode in DT. Why can't we use the same node as the GCC node and create a virtual child device here for tsens? We can assign the same of_node that this platform device has so that DT keeps working correctly.

So the current driver looks up data based on compatible strings.

The tsens device is always the same piece of hardware. The only

Well, not always. The one in 8960 does need additional initializations, requires you to save/restore context as it can be powered off not being in an always powered on domain etc.

Quoted 2 lines

thing that's changing is the qfprom data and the number of sensors. So we should be looking at the qfprom compatible string

How? Tsens uses nvmem framework apis to read the qfprom atleast in this series.

Quoted 7 lines

to figure out how to interpret the qfprom data which would include the number of sensors and how the data is encoded.

Quoted 3 lines

So you suggesting to create a virtual child device for gcc and associate the gcc DT node to it? (And have the tsens compatible mentioned as part of the gcc DT node?)

Quoted 4 lines

No. The driver should work just fine without having to interrogate the device's compatible string. If we still need the compatible check for some reason, then we can always match based on qcom,gcc-msm8960, qcom,gcc-apq8064, etc. But I don't see why

Thats not quite possible I guess. 2 drivers (gcc and tsens) matching the same compatibles? Will it not just depend on which ends up being the first match?

Stephen,

Quoted 26 lines

Quoted 8 lines

Quoted 6 lines

Quoted 4 lines

Quoted 4 lines

Also, I don't like having a subnode in DT. Why can't we use the same node as the GCC node and create a virtual child device here for tsens? We can assign the same of_node that this platform device has so that DT keeps working correctly.

So the current driver looks up data based on compatible strings.

The tsens device is always the same piece of hardware. The only

Well, not always. The one in 8960 does need additional initializations, requires you to save/restore context as it can be powered off not being in an always powered on domain etc.

Quoted 2 lines

thing that's changing is the qfprom data and the number of sensors. So we should be looking at the qfprom compatible string

How? Tsens uses nvmem framework apis to read the qfprom atleast in this series.

Quoted 7 lines

to figure out how to interpret the qfprom data which would include the number of sensors and how the data is encoded.

Quoted 3 lines

So you suggesting to create a virtual child device for gcc and associate the gcc DT node to it? (And have the tsens compatible mentioned as part of the gcc DT node?)

Quoted 8 lines

Quoted 4 lines

No. The driver should work just fine without having to interrogate the device's compatible string. If we still need the compatible check for some reason, then we can always match based on qcom,gcc-msm8960, qcom,gcc-apq8064, etc. But I don't see why

Thats not quite possible I guess. 2 drivers (gcc and tsens) matching the same compatibles? Will it not just depend on which ends up being the first match?

Any thoughts on how to move forward with this?

I tried what you were suggesting, and here's what I had to do to get things working

* Created a gcc node in DT with gcc and tsens compatibles, with gcc and tsens properties

* gcc driver probes the device/node first given gcc is registered with a core_initcall()
* creates a virtual child device attaching the same of_node (having both gcc and tsens compatibles)
* gcc ends up probing the virtual device/node _again_ (due to the gcc compatible match), fails
* At a later point, tsens driver gets registered (using module_initcall) ends up probing the virtual child node and succeeds

Quoted 9 lines

Quoted 8 lines

Quoted 4 lines

No. The driver should work just fine without having to interrogate the device's compatible string. If we still need the compatible check for some reason, then we can always match based on qcom,gcc-msm8960, qcom,gcc-apq8064, etc. But I don't see why

Thats not quite possible I guess. 2 drivers (gcc and tsens) matching the same compatibles? Will it not just depend on which ends up being the first match?

Quoted 7 lines

Any thoughts on how to move forward with this?

I tried what you were suggesting, and here's what I had to do to get things working

* Created a gcc node in DT with gcc and tsens compatibles, with gcc and tsens properties

This is not what I had in mind. This is what's should be in DT

clock-controller at f000 {

  	compatible = "qcom,gcc-msm8916";
	reg = <0xf000 ...>;
	....

};

Quoted 8 lines

* gcc driver probes the device/node first given gcc is registered with a core_initcall()
* creates a virtual child device attaching the same of_node (having both gcc and tsens compatibles)
* gcc ends up probing the virtual device/node _again_ (due to the gcc compatible match), fails
* At a later point, tsens driver gets registered (using module_initcall) ends up probing the virtual child node and succeeds

Yeah this might happen though because we've assigned the of_node pointer to the tsens device before we register it on the platform bus. The other way to pass that data down from gcc to tsens would be to not have an of_node assigned to the tsens device, and check for that case in the tsens driver. If there isn't an of_node, then we look at the parent device's of_node to figure out which gcc it is (if this even matters) and parse DT properties.

Quoted 8 lines

Yeah this might happen though because we've assigned the of_node pointer to the tsens device before we register it on the platform bus. The other way to pass that data down from gcc to tsens would be to not have an of_node assigned to the tsens device, and check for that case in the tsens driver. If there isn't an of_node, then we look at the parent device's of_node to figure out which gcc it is (if this even matters) and parse DT properties.

Parsing DT properties from parent (in the tsens driver) is fine, but the nvmem apis still expect an of_node for the tsens device and hence fail.

Associating the of_node of the parent to the tsens device while being probed ends up with the same issues of gcc ending up probing the device and failing because tsens defers probe a couple times before a successful probe.

Quoted 9 lines · Rajendra Nayak

Quoted 8 lines

Yeah this might happen though because we've assigned the of_node pointer to the tsens device before we register it on the platform bus. The other way to pass that data down from gcc to tsens would be to not have an of_node assigned to the tsens device, and check for that case in the tsens driver. If there isn't an of_node, then we look at the parent device's of_node to figure out which gcc it is (if this even matters) and parse DT properties.

Quoted 3 lines

Parsing DT properties from parent (in the tsens driver) is fine, but the nvmem apis still expect an of_node for the tsens device and hence fail.

So pass the parent device to the nvmem APIs? Or adjust the nvmem APIs to look for a parent of_node if there isn't an of_node for the device being passed? Or make the nvmem APIs work without using DT, and copy over the nvmem information from the gcc node to the virtual tsens child device?

Quoted 14 lines · Stephen Boyd

Quoted 12 lines · Rajendra Nayak

Quoted 8 lines

Yeah this might happen though because we've assigned the of_node pointer to the tsens device before we register it on the platform bus. The other way to pass that data down from gcc to tsens would be to not have an of_node assigned to the tsens device, and check for that case in the tsens driver. If there isn't an of_node, then we look at the parent device's of_node to figure out which gcc it is (if this even matters) and parse DT properties.

Parsing DT properties from parent (in the tsens driver) is fine, but the nvmem apis still expect an of_node for the tsens device and hence fail.

Quoted 5 lines

So pass the parent device to the nvmem APIs? Or adjust the nvmem APIs to look for a parent of_node if there isn't an of_node for the device being passed? Or make the nvmem APIs work without using DT, and copy over the nvmem information from the gcc node to the virtual tsens child device?

Srini, you being the nvmem maintainer, any thoughts?

Quoted 14 lines · Stephen Boyd

Quoted 12 lines · Rajendra Nayak

Quoted 8 lines

Yeah this might happen though because we've assigned the of_node pointer to the tsens device before we register it on the platform bus. The other way to pass that data down from gcc to tsens would be to not have an of_node assigned to the tsens device, and check for that case in the tsens driver. If there isn't an of_node, then we look at the parent device's of_node to figure out which gcc it is (if this even matters) and parse DT properties.

Parsing DT properties from parent (in the tsens driver) is fine, but the nvmem apis still expect an of_node for the tsens device and hence fail.

Quoted 5 lines

So pass the parent device to the nvmem APIs? Or adjust the nvmem APIs to look for a parent of_node if there isn't an of_node for the device being passed? Or make the nvmem APIs work without using DT, and copy over the nvmem information from the gcc node to the virtual tsens child device?

Srini, you being the nvmem maintainer, any thoughts?

Quoted 13 lines · Rajendra Nayak

drivers/clk/qcom/gcc-msm8960.c +1 -0

index eb6a4f9..2c80d03 100644
@@ -15,6 +15,7 @@
 #include <linux/bitops.h>
 #include <linux/err.h>
 #include <linux/platform_device.h>
+#include <linux/of_platform.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>

drivers/clk/qcom/gcc-msm8960.c static int gcc_msm8960_probe(struct platform_device *pdev) · line 3525

Quoted 6 lines

drivers/clk/qcom/gcc-msm8960.c +2 -1

@@ -3520,7 +3521,8 @@ static int gcc_msm8960_probe(struct platform_device *pdev)
 	if (IS_ERR(clk))
 		return PTR_ERR(clk);
 
-	return qcom_cc_probe(pdev, match->data);
+	qcom_cc_probe(pdev, match->data);
+	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);

We just lost the error code from qcom_cc_probe()...

Also, I don't like having a subnode in DT. Why can't we use the same node as the GCC node and create a virtual child device here for tsens? We can assign the same of_node that this platform device has so that DT keeps working correctly.

Quoted 22 lines · Stephen Boyd

Quoted 13 lines · Rajendra Nayak

drivers/clk/qcom/gcc-msm8960.c +1 -0

index eb6a4f9..2c80d03 100644
@@ -15,6 +15,7 @@
  #include <linux/bitops.h>
  #include <linux/err.h>
  #include <linux/platform_device.h>
+#include <linux/of_platform.h>
  #include <linux/module.h>
  #include <linux/of.h>
  #include <linux/of_device.h>

drivers/clk/qcom/gcc-msm8960.c static int gcc_msm8960_probe(struct platform_device *pdev) · line 3525

Quoted 6 lines

drivers/clk/qcom/gcc-msm8960.c +2 -1

@@ -3520,7 +3521,8 @@ static int gcc_msm8960_probe(struct platform_device *pdev)
  	if (IS_ERR(clk))
  		return PTR_ERR(clk);
 
-	return qcom_cc_probe(pdev, match->data);
+	qcom_cc_probe(pdev, match->data);
+	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);

We just lost the error code from qcom_cc_probe()...

Quoted 5 lines

Also, I don't like having a subnode in DT. Why can't we use the same node as the GCC node and create a virtual child device here for tsens? We can assign the same of_node that this platform device has so that DT keeps working correctly.

Can't we make the gcc driver support being a child of a simple-mfd by having it attempting to acquire the regmap of the parent and falling back to creating its own mmio regmap?

Quoted 29 lines · Bjorn Andersson

Quoted 27 lines · Stephen Boyd

Quoted 13 lines · Rajendra Nayak

drivers/clk/qcom/gcc-msm8960.c +1 -0

index eb6a4f9..2c80d03 100644
@@ -15,6 +15,7 @@
  #include <linux/bitops.h>
  #include <linux/err.h>
  #include <linux/platform_device.h>
+#include <linux/of_platform.h>
  #include <linux/module.h>
  #include <linux/of.h>
  #include <linux/of_device.h>

drivers/clk/qcom/gcc-msm8960.c static int gcc_msm8960_probe(struct platform_device *pdev) · line 3525

Quoted 6 lines

drivers/clk/qcom/gcc-msm8960.c +2 -1

@@ -3520,7 +3521,8 @@ static int gcc_msm8960_probe(struct platform_device *pdev)
  	if (IS_ERR(clk))
  		return PTR_ERR(clk);
 
-	return qcom_cc_probe(pdev, match->data);
+	qcom_cc_probe(pdev, match->data);
+	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);

We just lost the error code from qcom_cc_probe()...

Also, I don't like having a subnode in DT. Why can't we use the same node as the GCC node and create a virtual child device here for tsens? We can assign the same of_node that this platform device has so that DT keeps working correctly.

Quoted 3 lines

Can't we make the gcc driver support being a child of a simple-mfd by having it attempting to acquire the regmap of the parent and falling back to creating its own mmio regmap?

So we would need to make a new device and driver for the simple-mfd parent?

I'm confused about what you're suggesting and what benefit it has versus creating a child of the clock controller device.

Here's the patch I'm suggesting. The device name is probably wrong, but you get the idea.

Quoted 2 lines · Stephen Boyd

Here's the patch I'm suggesting. The device name is probably wrong, but you get the idea.

Looks very much like my take on it as well, I do however have concerns that suddenly the node called "clock-controller" will have to come with tsens related properties.

Are all the tsens-in-gcc blocks the same?

Or do you intend to of_match
on the gcc compatible in the tsens driver?

Changes since RFC:
* Added support for 8916 and 8084
* Based off the latest nvmem framework patches [1]
* Minor review fixes for comments mostly from Lina

** I have also added irq support using the hardware trip point support series [2]. Will post that out seperately.

This is an attempt to have a single TSENS driver for the different versions of the TSENS IP that exist, on different qcom msm/apq SoCs'
Support is added for msm8916, msm8960 and msm8974 families. Based on top of the latest nvmem framework patches [2]

A lot of the work is based of original code from Stephen Boyd and Siddartha Mohanadoss. I have also picked some of what Narendran Rajan did in his attempt to upstream the support for 8960 family. I could not keep the original authorship on any of the patches because I ended up moving the code around quite a bit in an effort to have a single driver for the various devices. I would be glad to change the authorship for any of the patches if needed.

[1] https://lkml.org/lkml/2015/6/22/609
[2] https://lkml.org/lkml/2015/5/20/472

Rajendra Nayak (8):
thermal: qcom: tsens: Add a skeletal TSENS drivers
thermal: qcom: tsens-8916: Add support for 8916 family of SoCs
thermal: qcom: tsens-8974: Add support for 8974 family of SoCs
thermal: qcom: tsens-8960: Add support for 8960 family of SoCs
arm: dts: msm8974: Add thermal zones, tsens and eeprom nodes
arm: dts: apq8064: Add thermal zones, tsens and eeprom nodes
arm: dts: apq8084: Add thermal zones, tsens and eeprom nodes
arm64: dts: msm8916: Add thermal zones, tsens and eeprom nodes

Srinivas Kandagatla (1):
clk: qcom: gcc-msm8960: add child devices support.

  .../devicetree/bindings/thermal/qcom-tsens.txt     |  37 +++
  arch/arm/boot/dts/qcom-apq8064.dtsi                | 108 ++++++++
  arch/arm/boot/dts/qcom-apq8084.dtsi                | 105 ++++++++
  arch/arm/boot/dts/qcom-msm8974.dtsi                | 105 ++++++++
  arch/arm64/boot/dts/qcom/msm8916.dtsi              |  66 +++++
  drivers/clk/qcom/gcc-msm8960.c                     |   4 +-
  drivers/thermal/Kconfig                            |   5 +
  drivers/thermal/Makefile                           |   1 +
  drivers/thermal/qcom/Kconfig                       |  10 +
  drivers/thermal/qcom/Makefile                      |   2 +
  drivers/thermal/qcom/tsens-8916.c                  | 107 ++++++++
  drivers/thermal/qcom/tsens-8960.c                  | 291 +++++++++++++++++++++
  drivers/thermal/qcom/tsens-8974.c                  | 239 +++++++++++++++++
  drivers/thermal/qcom/tsens-common.c                | 128 +++++++++
  drivers/thermal/qcom/tsens.c                       | 209 +++++++++++++++
  drivers/thermal/qcom/tsens.h                       |  69 +++++
  16 files changed, 1485 insertions(+), 1 deletion(-)

create mode 100644 Documentation/devicetree/bindings/thermal/qcom-tsens.txt create mode 100644 drivers/thermal/qcom/Kconfig
create mode 100644 drivers/thermal/qcom/Makefile
create mode 100644 drivers/thermal/qcom/tsens-8916.c
create mode 100644 drivers/thermal/qcom/tsens-8960.c
create mode 100644 drivers/thermal/qcom/tsens-8974.c
create mode 100644 drivers/thermal/qcom/tsens-common.c
create mode 100644 drivers/thermal/qcom/tsens.c
create mode 100644 drivers/thermal/qcom/tsens.h