[PATCH v2 0/5] Thermal reset support in PMC

[PATCH v2 0/5] Thermal reset support in PMC

[Mikko Perttunen]

Hi,

this series adds support for hardware-triggered thermal reset to the PMC
driver. Namely, it adds device tree properties for specifying the I2C
command to be sent when thermtrip is triggered. It is to be noted
that thermtrip won't be ever triggered without a soctherm driver to
calibrate the sensors.

Git repo at
  git://github.com/cyndis/linux.git pmc-thermtrip-v2

Testing script at https://gist.github.com/cyndis/66126c9c176b5f94a76f.
If your fan stops it is working.

Mikko Perttunen (5):
  of: Add descriptions of thermtrip properties to Tegra PMC bindings
  of: Add nvidia,controller-id property to Tegra I2C bindings
  ARM: tegra124: Add I2C controller ids to device tree
  ARM: tegra: Add PMC thermtrip programming to Jetson TK1 device tree
  ARM: tegra: Add thermal reset (thermtrip) support to PMC

 .../bindings/arm/tegra/nvidia,tegra20-pmc.txt      |  21 ++
 .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt |   5 +
 arch/arm/boot/dts/tegra124-jetson-tk1.dts          |   6 +
 arch/arm/boot/dts/tegra124.dtsi                    |   6 +
 drivers/soc/tegra/pmc.c                            | 379 ++++++++++++++-------
 5 files changed, 285 insertions(+), 132 deletions(-)

-- 
1.8.1.5

[PATCH v2 0/5] Thermal reset support in PMC

[Mikko Perttunen]

Hi,

this series adds support for hardware-triggered thermal reset to the PMC
driver. Namely, it adds device tree properties for specifying the I2C
command to be sent when thermtrip is triggered. It is to be noted
that thermtrip won't be ever triggered without a soctherm driver to
calibrate the sensors.

Git repo at
  git://github.com/cyndis/linux.git pmc-thermtrip-v2

Testing script at https://gist.github.com/cyndis/66126c9c176b5f94a76f.
If your fan stops it is working.

Mikko Perttunen (5):
  of: Add descriptions of thermtrip properties to Tegra PMC bindings
  of: Add nvidia,controller-id property to Tegra I2C bindings
  ARM: tegra124: Add I2C controller ids to device tree
  ARM: tegra: Add PMC thermtrip programming to Jetson TK1 device tree
  ARM: tegra: Add thermal reset (thermtrip) support to PMC

 .../bindings/arm/tegra/nvidia,tegra20-pmc.txt      |  21 ++
 .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt |   5 +
 arch/arm/boot/dts/tegra124-jetson-tk1.dts          |   6 +
 arch/arm/boot/dts/tegra124.dtsi                    |   6 +
 drivers/soc/tegra/pmc.c                            | 379 ++++++++++++++-------
 5 files changed, 285 insertions(+), 132 deletions(-)

-- 
1.8.1.5

[PATCH v2 0/5] Thermal reset support in PMC

[Mikko Perttunen]

Hi,

this series adds support for hardware-triggered thermal reset to the PMC
driver. Namely, it adds device tree properties for specifying the I2C
command to be sent when thermtrip is triggered. It is to be noted
that thermtrip won't be ever triggered without a soctherm driver to
calibrate the sensors.

Git repo at
  git://github.com/cyndis/linux.git pmc-thermtrip-v2

Testing script at https://gist.github.com/cyndis/66126c9c176b5f94a76f.
If your fan stops it is working.

Mikko Perttunen (5):
  of: Add descriptions of thermtrip properties to Tegra PMC bindings
  of: Add nvidia,controller-id property to Tegra I2C bindings
  ARM: tegra124: Add I2C controller ids to device tree
  ARM: tegra: Add PMC thermtrip programming to Jetson TK1 device tree
  ARM: tegra: Add thermal reset (thermtrip) support to PMC

 .../bindings/arm/tegra/nvidia,tegra20-pmc.txt      |  21 ++
 .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt |   5 +
 arch/arm/boot/dts/tegra124-jetson-tk1.dts          |   6 +
 arch/arm/boot/dts/tegra124.dtsi                    |   6 +
 drivers/soc/tegra/pmc.c                            | 379 ++++++++++++++-------
 5 files changed, 285 insertions(+), 132 deletions(-)

-- 
1.8.1.5

[PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Mikko Perttunen]

Hardware-triggered thermal reset requires configuring the I2C
reset procedure. This configuration is read from the device tree,
so document the relevant properties in the binding documentation.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 .../bindings/arm/tegra/nvidia,tegra20-pmc.txt       | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
index 68ac65f..ae6aef8 100644
--- a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
+++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
@@ -47,6 +47,19 @@ Required properties when nvidia,suspend-mode=<0>:
   sleep mode, the warm boot code will restore some PLLs, clocks and then
   bring up CPU0 for resuming the system.
 
+Hardware-triggered thermal reset:
+On Tegra30, Tegra114 and Tegra124, if the 'i2c-thermtrip' subnode exists,
+hardware-triggered thermal reset will be enabled.
+
+Required properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):
+- nvidia,pmu : Phandle to power management unit / PMIC handling poweroff
+- nvidia,reg-addr : I2C register address to write poweroff command to
+- nvidia,reg-data : Poweroff command to write to PMU
+
+Optional properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):
+- nvidia,pinmux-id : Pinmux used by the hardware when issuing poweroff command.
+                     Defaults to 0.
+
 Example:
 
 / SoC dts including file
@@ -69,6 +82,14 @@ pmc@7000f400 {
 / Tegra board dts file
 {
 	...
+	pmc@7000f400 {
+		i2c-thermtrip {
+			nvidia,pmu = <&pmic>;
+			nvidia,reg-addr = <0x36>;
+			nvidia,reg-data = <0x2>;
+		};
+	};
+	...
 	clocks {
 		compatible = "simple-bus";
 		#address-cells = <1>;
-- 
1.8.1.5

[PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Mikko Perttunen]

Hardware-triggered thermal reset requires configuring the I2C
reset procedure. This configuration is read from the device tree,
so document the relevant properties in the binding documentation.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 .../bindings/arm/tegra/nvidia,tegra20-pmc.txt       | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
index 68ac65f..ae6aef8 100644
--- a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
+++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
@@ -47,6 +47,19 @@ Required properties when nvidia,suspend-mode=<0>:
   sleep mode, the warm boot code will restore some PLLs, clocks and then
   bring up CPU0 for resuming the system.
 
+Hardware-triggered thermal reset:
+On Tegra30, Tegra114 and Tegra124, if the 'i2c-thermtrip' subnode exists,
+hardware-triggered thermal reset will be enabled.
+
+Required properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):
+- nvidia,pmu : Phandle to power management unit / PMIC handling poweroff
+- nvidia,reg-addr : I2C register address to write poweroff command to
+- nvidia,reg-data : Poweroff command to write to PMU
+
+Optional properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):
+- nvidia,pinmux-id : Pinmux used by the hardware when issuing poweroff command.
+                     Defaults to 0.
+
 Example:
 
 / SoC dts including file
@@ -69,6 +82,14 @@ pmc@7000f400 {
 / Tegra board dts file
 {
 	...
+	pmc@7000f400 {
+		i2c-thermtrip {
+			nvidia,pmu = <&pmic>;
+			nvidia,reg-addr = <0x36>;
+			nvidia,reg-data = <0x2>;
+		};
+	};
+	...
 	clocks {
 		compatible = "simple-bus";
 		#address-cells = <1>;
-- 
1.8.1.5

[PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Mikko Perttunen]

Hardware-triggered thermal reset requires configuring the I2C
reset procedure. This configuration is read from the device tree,
so document the relevant properties in the binding documentation.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 .../bindings/arm/tegra/nvidia,tegra20-pmc.txt       | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
index 68ac65f..ae6aef8 100644
--- a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
+++ b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
@@ -47,6 +47,19 @@ Required properties when nvidia,suspend-mode=<0>:
   sleep mode, the warm boot code will restore some PLLs, clocks and then
   bring up CPU0 for resuming the system.
 
+Hardware-triggered thermal reset:
+On Tegra30, Tegra114 and Tegra124, if the 'i2c-thermtrip' subnode exists,
+hardware-triggered thermal reset will be enabled.
+
+Required properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):
+- nvidia,pmu : Phandle to power management unit / PMIC handling poweroff
+- nvidia,reg-addr : I2C register address to write poweroff command to
+- nvidia,reg-data : Poweroff command to write to PMU
+
+Optional properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):
+- nvidia,pinmux-id : Pinmux used by the hardware when issuing poweroff command.
+                     Defaults to 0.
+
 Example:
 
 / SoC dts including file
@@ -69,6 +82,14 @@ pmc at 7000f400 {
 / Tegra board dts file
 {
 	...
+	pmc at 7000f400 {
+		i2c-thermtrip {
+			nvidia,pmu = <&pmic>;
+			nvidia,reg-addr = <0x36>;
+			nvidia,reg-data = <0x2>;
+		};
+	};
+	...
 	clocks {
 		compatible = "simple-bus";
 		#address-cells = <1>;
-- 
1.8.1.5

[PATCH v2 2/5] of: Add nvidia,controller-id property to Tegra I2C bindings

[Mikko Perttunen]

Sometimes, hardware blocks want to issue requests to devices
connected to I2C buses by itself. In such case, the bus the
target device resides on must be configured into a register.
For this purpose, each I2C controller has a defined ID known
by the hardware. Add a property for these IDs to the device tree
bindings, so that drivers can know what ID to write to a hardware
register when configuring a block that sends I2C messages autonomously.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt
index 87507e9..e9e5994 100644
--- a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt
@@ -57,6 +57,10 @@ Required properties:
   - rx
   - tx
 
+Optional properties:
+- nvidia,controller-id: ID of controller when referred to in
+                        hardware registers.
+
 Example:
 
 	i2c@7000c000 {
@@ -71,5 +75,6 @@ Example:
 		reset-names = "i2c";
 		dmas = <&apbdma 16>, <&apbdma 16>;
 		dma-names = "rx", "tx";
+		nvidia,controller-id = <0>;
 		status = "disabled";
 	};
-- 
1.8.1.5

[PATCH v2 2/5] of: Add nvidia,controller-id property to Tegra I2C bindings

[Mikko Perttunen]

Sometimes, hardware blocks want to issue requests to devices
connected to I2C buses by itself. In such case, the bus the
target device resides on must be configured into a register.
For this purpose, each I2C controller has a defined ID known
by the hardware. Add a property for these IDs to the device tree
bindings, so that drivers can know what ID to write to a hardware
register when configuring a block that sends I2C messages autonomously.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt
index 87507e9..e9e5994 100644
--- a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt
@@ -57,6 +57,10 @@ Required properties:
   - rx
   - tx
 
+Optional properties:
+- nvidia,controller-id: ID of controller when referred to in
+                        hardware registers.
+
 Example:
 
 	i2c@7000c000 {
@@ -71,5 +75,6 @@ Example:
 		reset-names = "i2c";
 		dmas = <&apbdma 16>, <&apbdma 16>;
 		dma-names = "rx", "tx";
+		nvidia,controller-id = <0>;
 		status = "disabled";
 	};
-- 
1.8.1.5

[PATCH v2 2/5] of: Add nvidia, controller-id property to Tegra I2C bindings

[Mikko Perttunen]

Sometimes, hardware blocks want to issue requests to devices
connected to I2C buses by itself. In such case, the bus the
target device resides on must be configured into a register.
For this purpose, each I2C controller has a defined ID known
by the hardware. Add a property for these IDs to the device tree
bindings, so that drivers can know what ID to write to a hardware
register when configuring a block that sends I2C messages autonomously.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt
index 87507e9..e9e5994 100644
--- a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt
@@ -57,6 +57,10 @@ Required properties:
   - rx
   - tx
 
+Optional properties:
+- nvidia,controller-id: ID of controller when referred to in
+                        hardware registers.
+
 Example:
 
 	i2c at 7000c000 {
@@ -71,5 +75,6 @@ Example:
 		reset-names = "i2c";
 		dmas = <&apbdma 16>, <&apbdma 16>;
 		dma-names = "rx", "tx";
+		nvidia,controller-id = <0>;
 		status = "disabled";
 	};
-- 
1.8.1.5

[PATCH v2 3/5] ARM: tegra124: Add I2C controller ids to device tree

[Mikko Perttunen]

I2C controller ids are required when programming hardware blocks
that send messages to devices connected to an I2C bus, such as
when the PMC sends a poweroff message to the PMIC. Add ids
to all I2C controllers in Tegra124.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 arch/arm/boot/dts/tegra124.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
index 03916ef..a101960 100644
--- a/arch/arm/boot/dts/tegra124.dtsi
+++ b/arch/arm/boot/dts/tegra124.dtsi
@@ -289,6 +289,7 @@
 		reset-names = "i2c";
 		dmas = <&apbdma 21>, <&apbdma 21>;
 		dma-names = "rx", "tx";
+		nvidia,controller-id = <0>;
 		status = "disabled";
 	};
 
@@ -304,6 +305,7 @@
 		reset-names = "i2c";
 		dmas = <&apbdma 22>, <&apbdma 22>;
 		dma-names = "rx", "tx";
+		nvidia,controller-id = <1>;
 		status = "disabled";
 	};
 
@@ -319,6 +321,7 @@
 		reset-names = "i2c";
 		dmas = <&apbdma 23>, <&apbdma 23>;
 		dma-names = "rx", "tx";
+		nvidia,controller-id = <2>;
 		status = "disabled";
 	};
 
@@ -334,6 +337,7 @@
 		reset-names = "i2c";
 		dmas = <&apbdma 26>, <&apbdma 26>;
 		dma-names = "rx", "tx";
+		nvidia,controller-id = <3>;
 		status = "disabled";
 	};
 
@@ -349,6 +353,7 @@
 		reset-names = "i2c";
 		dmas = <&apbdma 24>, <&apbdma 24>;
 		dma-names = "rx", "tx";
+		nvidia,controller-id = <4>;
 		status = "disabled";
 	};
 
@@ -364,6 +369,7 @@
 		reset-names = "i2c";
 		dmas = <&apbdma 30>, <&apbdma 30>;
 		dma-names = "rx", "tx";
+		nvidia,controller-id = <5>;
 		status = "disabled";
 	};
 
-- 
1.8.1.5

[PATCH v2 3/5] ARM: tegra124: Add I2C controller ids to device tree

[Mikko Perttunen]

I2C controller ids are required when programming hardware blocks
that send messages to devices connected to an I2C bus, such as
when the PMC sends a poweroff message to the PMIC. Add ids
to all I2C controllers in Tegra124.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 arch/arm/boot/dts/tegra124.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
index 03916ef..a101960 100644
--- a/arch/arm/boot/dts/tegra124.dtsi
+++ b/arch/arm/boot/dts/tegra124.dtsi
@@ -289,6 +289,7 @@
 		reset-names = "i2c";
 		dmas = <&apbdma 21>, <&apbdma 21>;
 		dma-names = "rx", "tx";
+		nvidia,controller-id = <0>;
 		status = "disabled";
 	};
 
@@ -304,6 +305,7 @@
 		reset-names = "i2c";
 		dmas = <&apbdma 22>, <&apbdma 22>;
 		dma-names = "rx", "tx";
+		nvidia,controller-id = <1>;
 		status = "disabled";
 	};
 
@@ -319,6 +321,7 @@
 		reset-names = "i2c";
 		dmas = <&apbdma 23>, <&apbdma 23>;
 		dma-names = "rx", "tx";
+		nvidia,controller-id = <2>;
 		status = "disabled";
 	};
 
@@ -334,6 +337,7 @@
 		reset-names = "i2c";
 		dmas = <&apbdma 26>, <&apbdma 26>;
 		dma-names = "rx", "tx";
+		nvidia,controller-id = <3>;
 		status = "disabled";
 	};
 
@@ -349,6 +353,7 @@
 		reset-names = "i2c";
 		dmas = <&apbdma 24>, <&apbdma 24>;
 		dma-names = "rx", "tx";
+		nvidia,controller-id = <4>;
 		status = "disabled";
 	};
 
@@ -364,6 +369,7 @@
 		reset-names = "i2c";
 		dmas = <&apbdma 30>, <&apbdma 30>;
 		dma-names = "rx", "tx";
+		nvidia,controller-id = <5>;
 		status = "disabled";
 	};
 
-- 
1.8.1.5

[PATCH v2 3/5] ARM: tegra124: Add I2C controller ids to device tree

[Mikko Perttunen]

I2C controller ids are required when programming hardware blocks
that send messages to devices connected to an I2C bus, such as
when the PMC sends a poweroff message to the PMIC. Add ids
to all I2C controllers in Tegra124.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 arch/arm/boot/dts/tegra124.dtsi | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
index 03916ef..a101960 100644
--- a/arch/arm/boot/dts/tegra124.dtsi
+++ b/arch/arm/boot/dts/tegra124.dtsi
@@ -289,6 +289,7 @@
 		reset-names = "i2c";
 		dmas = <&apbdma 21>, <&apbdma 21>;
 		dma-names = "rx", "tx";
+		nvidia,controller-id = <0>;
 		status = "disabled";
 	};
 
@@ -304,6 +305,7 @@
 		reset-names = "i2c";
 		dmas = <&apbdma 22>, <&apbdma 22>;
 		dma-names = "rx", "tx";
+		nvidia,controller-id = <1>;
 		status = "disabled";
 	};
 
@@ -319,6 +321,7 @@
 		reset-names = "i2c";
 		dmas = <&apbdma 23>, <&apbdma 23>;
 		dma-names = "rx", "tx";
+		nvidia,controller-id = <2>;
 		status = "disabled";
 	};
 
@@ -334,6 +337,7 @@
 		reset-names = "i2c";
 		dmas = <&apbdma 26>, <&apbdma 26>;
 		dma-names = "rx", "tx";
+		nvidia,controller-id = <3>;
 		status = "disabled";
 	};
 
@@ -349,6 +353,7 @@
 		reset-names = "i2c";
 		dmas = <&apbdma 24>, <&apbdma 24>;
 		dma-names = "rx", "tx";
+		nvidia,controller-id = <4>;
 		status = "disabled";
 	};
 
@@ -364,6 +369,7 @@
 		reset-names = "i2c";
 		dmas = <&apbdma 30>, <&apbdma 30>;
 		dma-names = "rx", "tx";
+		nvidia,controller-id = <5>;
 		status = "disabled";
 	};
 
-- 
1.8.1.5

[PATCH v2 4/5] ARM: tegra: Add PMC thermtrip programming to Jetson TK1 device tree

[Mikko Perttunen]

This adds the required information to reset the board during an overheating
situation to the Jetson TK1 device tree. The thermal reset is handled by the
PMC by sending an I2C message to the PMIC. The entries specify the I2C
message to be sent.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 arch/arm/boot/dts/tegra124-jetson-tk1.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/tegra124-jetson-tk1.dts b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
index 624b0fb..9ff69ad 100644
--- a/arch/arm/boot/dts/tegra124-jetson-tk1.dts
+++ b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
@@ -1617,6 +1617,12 @@
 		nvidia,core-pwr-off-time = <61036>;
 		nvidia,core-power-req-active-high;
 		nvidia,sys-clock-req-active-high;
+
+		i2c-thermtrip {
+			nvidia,pmu = <&pmic>;
+			nvidia,reg-addr = <0x36>;
+			nvidia,reg-data = <0x2>;
+		};
 	};
 
 	padctl@0,7009f000 {
-- 
1.8.1.5

[PATCH v2 4/5] ARM: tegra: Add PMC thermtrip programming to Jetson TK1 device tree

[Mikko Perttunen]

This adds the required information to reset the board during an overheating
situation to the Jetson TK1 device tree. The thermal reset is handled by the
PMC by sending an I2C message to the PMIC. The entries specify the I2C
message to be sent.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 arch/arm/boot/dts/tegra124-jetson-tk1.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/tegra124-jetson-tk1.dts b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
index 624b0fb..9ff69ad 100644
--- a/arch/arm/boot/dts/tegra124-jetson-tk1.dts
+++ b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
@@ -1617,6 +1617,12 @@
 		nvidia,core-pwr-off-time = <61036>;
 		nvidia,core-power-req-active-high;
 		nvidia,sys-clock-req-active-high;
+
+		i2c-thermtrip {
+			nvidia,pmu = <&pmic>;
+			nvidia,reg-addr = <0x36>;
+			nvidia,reg-data = <0x2>;
+		};
 	};
 
 	padctl@0,7009f000 {
-- 
1.8.1.5

[PATCH v2 4/5] ARM: tegra: Add PMC thermtrip programming to Jetson TK1 device tree

[Mikko Perttunen]

This adds the required information to reset the board during an overheating
situation to the Jetson TK1 device tree. The thermal reset is handled by the
PMC by sending an I2C message to the PMIC. The entries specify the I2C
message to be sent.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 arch/arm/boot/dts/tegra124-jetson-tk1.dts | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/boot/dts/tegra124-jetson-tk1.dts b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
index 624b0fb..9ff69ad 100644
--- a/arch/arm/boot/dts/tegra124-jetson-tk1.dts
+++ b/arch/arm/boot/dts/tegra124-jetson-tk1.dts
@@ -1617,6 +1617,12 @@
 		nvidia,core-pwr-off-time = <61036>;
 		nvidia,core-power-req-active-high;
 		nvidia,sys-clock-req-active-high;
+
+		i2c-thermtrip {
+			nvidia,pmu = <&pmic>;
+			nvidia,reg-addr = <0x36>;
+			nvidia,reg-data = <0x2>;
+		};
 	};
 
 	padctl at 0,7009f000 {
-- 
1.8.1.5

[PATCH v2 5/5] ARM: tegra: Add thermal reset (thermtrip) support to PMC

[Mikko Perttunen]

This adds a device tree controlled option to enable PMC-based
thermal reset in overheating situations. Thermtrip is supported on
Tegra30, Tegra114 and Tegra124. The thermal reset only works when
the thermal sensors are calibrated, so a soctherm driver is also
required.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/soc/tegra/pmc.c | 379 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 247 insertions(+), 132 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index a2c0ceb..a5a35c4 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -83,11 +83,28 @@
 
 #define GPU_RG_CNTRL			0x2d4
 
+#define PMC_SENSOR_CTRL			0x1b0
+#define PMC_SENSOR_CTRL_SCRATCH_WRITE	(1 << 2)
+#define PMC_SENSOR_CTRL_ENABLE_RST	(1 << 1)
+
+#define PMC_SCRATCH54			0x258
+#define PMC_SCRATCH54_DATA_SHIFT	8
+#define PMC_SCRATCH54_ADDR_SHIFT	0
+
+#define PMC_SCRATCH55			0x25c
+#define PMC_SCRATCH55_RESET_TEGRA	(1 << 31)
+#define PMC_SCRATCH55_CNTRL_ID_SHIFT	27
+#define PMC_SCRATCH55_PINMUX_SHIFT	24
+#define PMC_SCRATCH55_16BITOP		(1 << 15)
+#define PMC_SCRATCH55_CHECKSUM_SHIFT	16
+#define PMC_SCRATCH55_I2CSLV1_SHIFT	0
+
 struct tegra_pmc_soc {
 	unsigned int num_powergates;
 	const char *const *powergates;
 	unsigned int num_cpu_powergates;
 	const u8 *cpu_powergates;
+	bool has_tsense_reset;
 };
 
 /**
@@ -606,6 +623,234 @@ void tegra_pmc_enter_suspend_mode(enum tegra_suspend_mode mode)
 }
 #endif
 
+static const char * const tegra20_powergates[] = {
+	[TEGRA_POWERGATE_CPU] = "cpu",
+	[TEGRA_POWERGATE_3D] = "3d",
+	[TEGRA_POWERGATE_VENC] = "venc",
+	[TEGRA_POWERGATE_VDEC] = "vdec",
+	[TEGRA_POWERGATE_PCIE] = "pcie",
+	[TEGRA_POWERGATE_L2] = "l2",
+	[TEGRA_POWERGATE_MPE] = "mpe",
+};
+
+static const struct tegra_pmc_soc tegra20_pmc_soc = {
+	.num_powergates = ARRAY_SIZE(tegra20_powergates),
+	.powergates = tegra20_powergates,
+	.num_cpu_powergates = 0,
+	.cpu_powergates = NULL,
+};
+
+static const char * const tegra30_powergates[] = {
+	[TEGRA_POWERGATE_CPU] = "cpu0",
+	[TEGRA_POWERGATE_3D] = "3d0",
+	[TEGRA_POWERGATE_VENC] = "venc",
+	[TEGRA_POWERGATE_VDEC] = "vdec",
+	[TEGRA_POWERGATE_PCIE] = "pcie",
+	[TEGRA_POWERGATE_L2] = "l2",
+	[TEGRA_POWERGATE_MPE] = "mpe",
+	[TEGRA_POWERGATE_HEG] = "heg",
+	[TEGRA_POWERGATE_SATA] = "sata",
+	[TEGRA_POWERGATE_CPU1] = "cpu1",
+	[TEGRA_POWERGATE_CPU2] = "cpu2",
+	[TEGRA_POWERGATE_CPU3] = "cpu3",
+	[TEGRA_POWERGATE_CELP] = "celp",
+	[TEGRA_POWERGATE_3D1] = "3d1",
+};
+
+static const u8 tegra30_cpu_powergates[] = {
+	TEGRA_POWERGATE_CPU,
+	TEGRA_POWERGATE_CPU1,
+	TEGRA_POWERGATE_CPU2,
+	TEGRA_POWERGATE_CPU3,
+};
+
+static const struct tegra_pmc_soc tegra30_pmc_soc = {
+	.num_powergates = ARRAY_SIZE(tegra30_powergates),
+	.powergates = tegra30_powergates,
+	.num_cpu_powergates = ARRAY_SIZE(tegra30_cpu_powergates),
+	.cpu_powergates = tegra30_cpu_powergates,
+	.has_tsense_reset = true,
+};
+
+static const char * const tegra114_powergates[] = {
+	[TEGRA_POWERGATE_CPU] = "crail",
+	[TEGRA_POWERGATE_3D] = "3d",
+	[TEGRA_POWERGATE_VENC] = "venc",
+	[TEGRA_POWERGATE_VDEC] = "vdec",
+	[TEGRA_POWERGATE_MPE] = "mpe",
+	[TEGRA_POWERGATE_HEG] = "heg",
+	[TEGRA_POWERGATE_CPU1] = "cpu1",
+	[TEGRA_POWERGATE_CPU2] = "cpu2",
+	[TEGRA_POWERGATE_CPU3] = "cpu3",
+	[TEGRA_POWERGATE_CELP] = "celp",
+	[TEGRA_POWERGATE_CPU0] = "cpu0",
+	[TEGRA_POWERGATE_C0NC] = "c0nc",
+	[TEGRA_POWERGATE_C1NC] = "c1nc",
+	[TEGRA_POWERGATE_DIS] = "dis",
+	[TEGRA_POWERGATE_DISB] = "disb",
+	[TEGRA_POWERGATE_XUSBA] = "xusba",
+	[TEGRA_POWERGATE_XUSBB] = "xusbb",
+	[TEGRA_POWERGATE_XUSBC] = "xusbc",
+};
+
+static const u8 tegra114_cpu_powergates[] = {
+	TEGRA_POWERGATE_CPU0,
+	TEGRA_POWERGATE_CPU1,
+	TEGRA_POWERGATE_CPU2,
+	TEGRA_POWERGATE_CPU3,
+};
+
+static const struct tegra_pmc_soc tegra114_pmc_soc = {
+	.num_powergates = ARRAY_SIZE(tegra114_powergates),
+	.powergates = tegra114_powergates,
+	.num_cpu_powergates = ARRAY_SIZE(tegra114_cpu_powergates),
+	.cpu_powergates = tegra114_cpu_powergates,
+	.has_tsense_reset = true,
+};
+
+static const char * const tegra124_powergates[] = {
+	[TEGRA_POWERGATE_CPU] = "crail",
+	[TEGRA_POWERGATE_3D] = "3d",
+	[TEGRA_POWERGATE_VENC] = "venc",
+	[TEGRA_POWERGATE_PCIE] = "pcie",
+	[TEGRA_POWERGATE_VDEC] = "vdec",
+	[TEGRA_POWERGATE_L2] = "l2",
+	[TEGRA_POWERGATE_MPE] = "mpe",
+	[TEGRA_POWERGATE_HEG] = "heg",
+	[TEGRA_POWERGATE_SATA] = "sata",
+	[TEGRA_POWERGATE_CPU1] = "cpu1",
+	[TEGRA_POWERGATE_CPU2] = "cpu2",
+	[TEGRA_POWERGATE_CPU3] = "cpu3",
+	[TEGRA_POWERGATE_CELP] = "celp",
+	[TEGRA_POWERGATE_CPU0] = "cpu0",
+	[TEGRA_POWERGATE_C0NC] = "c0nc",
+	[TEGRA_POWERGATE_C1NC] = "c1nc",
+	[TEGRA_POWERGATE_SOR] = "sor",
+	[TEGRA_POWERGATE_DIS] = "dis",
+	[TEGRA_POWERGATE_DISB] = "disb",
+	[TEGRA_POWERGATE_XUSBA] = "xusba",
+	[TEGRA_POWERGATE_XUSBB] = "xusbb",
+	[TEGRA_POWERGATE_XUSBC] = "xusbc",
+	[TEGRA_POWERGATE_VIC] = "vic",
+	[TEGRA_POWERGATE_IRAM] = "iram",
+};
+
+static const u8 tegra124_cpu_powergates[] = {
+	TEGRA_POWERGATE_CPU0,
+	TEGRA_POWERGATE_CPU1,
+	TEGRA_POWERGATE_CPU2,
+	TEGRA_POWERGATE_CPU3,
+};
+
+static const struct tegra_pmc_soc tegra124_pmc_soc = {
+	.num_powergates = ARRAY_SIZE(tegra124_powergates),
+	.powergates = tegra124_powergates,
+	.num_cpu_powergates = ARRAY_SIZE(tegra124_cpu_powergates),
+	.cpu_powergates = tegra124_cpu_powergates,
+	.has_tsense_reset = true,
+};
+
+static const struct of_device_id tegra_pmc_match[] = {
+	{ .compatible = "nvidia,tegra124-pmc", .data = &tegra124_pmc_soc },
+	{ .compatible = "nvidia,tegra114-pmc", .data = &tegra114_pmc_soc },
+	{ .compatible = "nvidia,tegra30-pmc", .data = &tegra30_pmc_soc },
+	{ .compatible = "nvidia,tegra20-pmc", .data = &tegra20_pmc_soc },
+	{ }
+};
+
+void tegra_pmc_init_tsense_reset(struct device *dev)
+{
+	u32 pmu_i2c_addr, i2c_ctrl_id, reg_addr, reg_data, pinmux;
+	u32 value, checksum;
+	struct device_node *np = dev->of_node;
+	struct device_node *i2c_np, *tt_np;
+	const struct of_device_id *match = of_match_node(tegra_pmc_match, np);
+	const struct tegra_pmc_soc *data = match->data;
+
+	if (!data->has_tsense_reset)
+		return;
+
+	tt_np = of_find_node_by_name(np, "i2c-thermtrip");
+	if (!tt_np) {
+		dev_warn(dev, "no i2c-thermtrip node found, disabling emergency thermal reset\n");
+		return;
+	}
+
+	i2c_np = of_parse_phandle(tt_np, "nvidia,pmu", 0);
+	if (!i2c_np) {
+		dev_err(dev, "PMU reference missing, disabling emergency thermal reset\n");
+		goto put_tt;
+	}
+
+	if (of_property_read_u32(i2c_np, "reg", &pmu_i2c_addr)) {
+		dev_err(dev, "PMU address missing, disabling emergency thermal reset\n");
+		goto put_i2c;
+	}
+
+	if (of_property_read_u32(i2c_np->parent, "nvidia,controller-id", &i2c_ctrl_id)) {
+		dev_err(dev, "PMU controller id missing, disabling emergency thermal reset\n");
+		goto put_i2c;
+	}
+
+	of_node_put(i2c_np);
+
+	if (of_property_read_u32(tt_np, "nvidia,reg-addr", &reg_addr)) {
+		dev_err(dev, "nvidia,reg-addr missing, disabling emergency thermal reset\n");
+		goto put_tt;
+	}
+
+	if (of_property_read_u32(tt_np, "nvidia,reg-data", &reg_data)) {
+		dev_err(dev, "nvidia,reg-data missing, disabling emergency thermal reset\n");
+		goto put_tt;
+	}
+
+	if (of_property_read_u32(tt_np, "nvidia,pinmux-id", &pinmux))
+		pinmux = 0;
+
+	of_node_put(tt_np);
+
+	value = tegra_pmc_readl(PMC_SENSOR_CTRL);
+	value |= PMC_SENSOR_CTRL_SCRATCH_WRITE;
+	tegra_pmc_writel(value, PMC_SENSOR_CTRL);
+
+	value = (reg_data << PMC_SCRATCH54_DATA_SHIFT) |
+	      (reg_addr << PMC_SCRATCH54_ADDR_SHIFT);
+	tegra_pmc_writel(value, PMC_SCRATCH54);
+
+	value = 0;
+	value |= PMC_SCRATCH55_RESET_TEGRA;
+	value |= i2c_ctrl_id << PMC_SCRATCH55_CNTRL_ID_SHIFT;
+	value |= pinmux << PMC_SCRATCH55_PINMUX_SHIFT;
+	value |= pmu_i2c_addr << PMC_SCRATCH55_I2CSLV1_SHIFT;
+
+	/* Calculate checksum of SCRATCH54, SCRATCH55 fields.
+	 * Bits 23:16 will contain the checksum and are currently zero,
+	 * so they are not added.
+	 */
+	checksum = reg_addr + reg_data + (value & 0xff) + ((value >> 8) & 0xff) +
+		((value >> 24) & 0xff);
+	checksum &= 0xff;
+	checksum = 0x100 - checksum;
+
+	value |= checksum << PMC_SCRATCH55_CHECKSUM_SHIFT;
+
+	tegra_pmc_writel(value, PMC_SCRATCH55);
+
+	value = tegra_pmc_readl(PMC_SENSOR_CTRL);
+	value |= PMC_SENSOR_CTRL_ENABLE_RST;
+	tegra_pmc_writel(value, PMC_SENSOR_CTRL);
+
+	dev_info(dev, "PMC emergency thermal reset enabled\n");
+
+	return;
+
+put_i2c:
+	of_node_put(i2c_np);
+
+put_tt:
+	of_node_put(tt_np);
+}
+
 static int tegra_pmc_parse_dt(struct tegra_pmc *pmc, struct device_node *np)
 {
 	u32 value, values[2];
@@ -730,6 +975,8 @@ static int tegra_pmc_probe(struct platform_device *pdev)
 
 	tegra_pmc_init(pmc);
 
+	tegra_pmc_init_tsense_reset(&pdev->dev);
+
 	if (IS_ENABLED(CONFIG_DEBUG_FS)) {
 		err = tegra_powergate_debugfs_init();
 		if (err < 0)
@@ -757,138 +1004,6 @@ static int tegra_pmc_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(tegra_pmc_pm_ops, tegra_pmc_suspend, tegra_pmc_resume);
 
-static const char * const tegra20_powergates[] = {
-	[TEGRA_POWERGATE_CPU] = "cpu",
-	[TEGRA_POWERGATE_3D] = "3d",
-	[TEGRA_POWERGATE_VENC] = "venc",
-	[TEGRA_POWERGATE_VDEC] = "vdec",
-	[TEGRA_POWERGATE_PCIE] = "pcie",
-	[TEGRA_POWERGATE_L2] = "l2",
-	[TEGRA_POWERGATE_MPE] = "mpe",
-};
-
-static const struct tegra_pmc_soc tegra20_pmc_soc = {
-	.num_powergates = ARRAY_SIZE(tegra20_powergates),
-	.powergates = tegra20_powergates,
-	.num_cpu_powergates = 0,
-	.cpu_powergates = NULL,
-};
-
-static const char * const tegra30_powergates[] = {
-	[TEGRA_POWERGATE_CPU] = "cpu0",
-	[TEGRA_POWERGATE_3D] = "3d0",
-	[TEGRA_POWERGATE_VENC] = "venc",
-	[TEGRA_POWERGATE_VDEC] = "vdec",
-	[TEGRA_POWERGATE_PCIE] = "pcie",
-	[TEGRA_POWERGATE_L2] = "l2",
-	[TEGRA_POWERGATE_MPE] = "mpe",
-	[TEGRA_POWERGATE_HEG] = "heg",
-	[TEGRA_POWERGATE_SATA] = "sata",
-	[TEGRA_POWERGATE_CPU1] = "cpu1",
-	[TEGRA_POWERGATE_CPU2] = "cpu2",
-	[TEGRA_POWERGATE_CPU3] = "cpu3",
-	[TEGRA_POWERGATE_CELP] = "celp",
-	[TEGRA_POWERGATE_3D1] = "3d1",
-};
-
-static const u8 tegra30_cpu_powergates[] = {
-	TEGRA_POWERGATE_CPU,
-	TEGRA_POWERGATE_CPU1,
-	TEGRA_POWERGATE_CPU2,
-	TEGRA_POWERGATE_CPU3,
-};
-
-static const struct tegra_pmc_soc tegra30_pmc_soc = {
-	.num_powergates = ARRAY_SIZE(tegra30_powergates),
-	.powergates = tegra30_powergates,
-	.num_cpu_powergates = ARRAY_SIZE(tegra30_cpu_powergates),
-	.cpu_powergates = tegra30_cpu_powergates,
-};
-
-static const char * const tegra114_powergates[] = {
-	[TEGRA_POWERGATE_CPU] = "crail",
-	[TEGRA_POWERGATE_3D] = "3d",
-	[TEGRA_POWERGATE_VENC] = "venc",
-	[TEGRA_POWERGATE_VDEC] = "vdec",
-	[TEGRA_POWERGATE_MPE] = "mpe",
-	[TEGRA_POWERGATE_HEG] = "heg",
-	[TEGRA_POWERGATE_CPU1] = "cpu1",
-	[TEGRA_POWERGATE_CPU2] = "cpu2",
-	[TEGRA_POWERGATE_CPU3] = "cpu3",
-	[TEGRA_POWERGATE_CELP] = "celp",
-	[TEGRA_POWERGATE_CPU0] = "cpu0",
-	[TEGRA_POWERGATE_C0NC] = "c0nc",
-	[TEGRA_POWERGATE_C1NC] = "c1nc",
-	[TEGRA_POWERGATE_DIS] = "dis",
-	[TEGRA_POWERGATE_DISB] = "disb",
-	[TEGRA_POWERGATE_XUSBA] = "xusba",
-	[TEGRA_POWERGATE_XUSBB] = "xusbb",
-	[TEGRA_POWERGATE_XUSBC] = "xusbc",
-};
-
-static const u8 tegra114_cpu_powergates[] = {
-	TEGRA_POWERGATE_CPU0,
-	TEGRA_POWERGATE_CPU1,
-	TEGRA_POWERGATE_CPU2,
-	TEGRA_POWERGATE_CPU3,
-};
-
-static const struct tegra_pmc_soc tegra114_pmc_soc = {
-	.num_powergates = ARRAY_SIZE(tegra114_powergates),
-	.powergates = tegra114_powergates,
-	.num_cpu_powergates = ARRAY_SIZE(tegra114_cpu_powergates),
-	.cpu_powergates = tegra114_cpu_powergates,
-};
-
-static const char * const tegra124_powergates[] = {
-	[TEGRA_POWERGATE_CPU] = "crail",
-	[TEGRA_POWERGATE_3D] = "3d",
-	[TEGRA_POWERGATE_VENC] = "venc",
-	[TEGRA_POWERGATE_PCIE] = "pcie",
-	[TEGRA_POWERGATE_VDEC] = "vdec",
-	[TEGRA_POWERGATE_L2] = "l2",
-	[TEGRA_POWERGATE_MPE] = "mpe",
-	[TEGRA_POWERGATE_HEG] = "heg",
-	[TEGRA_POWERGATE_SATA] = "sata",
-	[TEGRA_POWERGATE_CPU1] = "cpu1",
-	[TEGRA_POWERGATE_CPU2] = "cpu2",
-	[TEGRA_POWERGATE_CPU3] = "cpu3",
-	[TEGRA_POWERGATE_CELP] = "celp",
-	[TEGRA_POWERGATE_CPU0] = "cpu0",
-	[TEGRA_POWERGATE_C0NC] = "c0nc",
-	[TEGRA_POWERGATE_C1NC] = "c1nc",
-	[TEGRA_POWERGATE_SOR] = "sor",
-	[TEGRA_POWERGATE_DIS] = "dis",
-	[TEGRA_POWERGATE_DISB] = "disb",
-	[TEGRA_POWERGATE_XUSBA] = "xusba",
-	[TEGRA_POWERGATE_XUSBB] = "xusbb",
-	[TEGRA_POWERGATE_XUSBC] = "xusbc",
-	[TEGRA_POWERGATE_VIC] = "vic",
-	[TEGRA_POWERGATE_IRAM] = "iram",
-};
-
-static const u8 tegra124_cpu_powergates[] = {
-	TEGRA_POWERGATE_CPU0,
-	TEGRA_POWERGATE_CPU1,
-	TEGRA_POWERGATE_CPU2,
-	TEGRA_POWERGATE_CPU3,
-};
-
-static const struct tegra_pmc_soc tegra124_pmc_soc = {
-	.num_powergates = ARRAY_SIZE(tegra124_powergates),
-	.powergates = tegra124_powergates,
-	.num_cpu_powergates = ARRAY_SIZE(tegra124_cpu_powergates),
-	.cpu_powergates = tegra124_cpu_powergates,
-};
-
-static const struct of_device_id tegra_pmc_match[] = {
-	{ .compatible = "nvidia,tegra124-pmc", .data = &tegra124_pmc_soc },
-	{ .compatible = "nvidia,tegra114-pmc", .data = &tegra114_pmc_soc },
-	{ .compatible = "nvidia,tegra30-pmc", .data = &tegra30_pmc_soc },
-	{ .compatible = "nvidia,tegra20-pmc", .data = &tegra20_pmc_soc },
-	{ }
-};
-
 static struct platform_driver tegra_pmc_driver = {
 	.driver = {
 		.name = "tegra-pmc",
-- 
1.8.1.5

[PATCH v2 5/5] ARM: tegra: Add thermal reset (thermtrip) support to PMC

[Mikko Perttunen]

This adds a device tree controlled option to enable PMC-based
thermal reset in overheating situations. Thermtrip is supported on
Tegra30, Tegra114 and Tegra124. The thermal reset only works when
the thermal sensors are calibrated, so a soctherm driver is also
required.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/soc/tegra/pmc.c | 379 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 247 insertions(+), 132 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index a2c0ceb..a5a35c4 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -83,11 +83,28 @@
 
 #define GPU_RG_CNTRL			0x2d4
 
+#define PMC_SENSOR_CTRL			0x1b0
+#define PMC_SENSOR_CTRL_SCRATCH_WRITE	(1 << 2)
+#define PMC_SENSOR_CTRL_ENABLE_RST	(1 << 1)
+
+#define PMC_SCRATCH54			0x258
+#define PMC_SCRATCH54_DATA_SHIFT	8
+#define PMC_SCRATCH54_ADDR_SHIFT	0
+
+#define PMC_SCRATCH55			0x25c
+#define PMC_SCRATCH55_RESET_TEGRA	(1 << 31)
+#define PMC_SCRATCH55_CNTRL_ID_SHIFT	27
+#define PMC_SCRATCH55_PINMUX_SHIFT	24
+#define PMC_SCRATCH55_16BITOP		(1 << 15)
+#define PMC_SCRATCH55_CHECKSUM_SHIFT	16
+#define PMC_SCRATCH55_I2CSLV1_SHIFT	0
+
 struct tegra_pmc_soc {
 	unsigned int num_powergates;
 	const char *const *powergates;
 	unsigned int num_cpu_powergates;
 	const u8 *cpu_powergates;
+	bool has_tsense_reset;
 };
 
 /**
@@ -606,6 +623,234 @@ void tegra_pmc_enter_suspend_mode(enum tegra_suspend_mode mode)
 }
 #endif
 
+static const char * const tegra20_powergates[] = {
+	[TEGRA_POWERGATE_CPU] = "cpu",
+	[TEGRA_POWERGATE_3D] = "3d",
+	[TEGRA_POWERGATE_VENC] = "venc",
+	[TEGRA_POWERGATE_VDEC] = "vdec",
+	[TEGRA_POWERGATE_PCIE] = "pcie",
+	[TEGRA_POWERGATE_L2] = "l2",
+	[TEGRA_POWERGATE_MPE] = "mpe",
+};
+
+static const struct tegra_pmc_soc tegra20_pmc_soc = {
+	.num_powergates = ARRAY_SIZE(tegra20_powergates),
+	.powergates = tegra20_powergates,
+	.num_cpu_powergates = 0,
+	.cpu_powergates = NULL,
+};
+
+static const char * const tegra30_powergates[] = {
+	[TEGRA_POWERGATE_CPU] = "cpu0",
+	[TEGRA_POWERGATE_3D] = "3d0",
+	[TEGRA_POWERGATE_VENC] = "venc",
+	[TEGRA_POWERGATE_VDEC] = "vdec",
+	[TEGRA_POWERGATE_PCIE] = "pcie",
+	[TEGRA_POWERGATE_L2] = "l2",
+	[TEGRA_POWERGATE_MPE] = "mpe",
+	[TEGRA_POWERGATE_HEG] = "heg",
+	[TEGRA_POWERGATE_SATA] = "sata",
+	[TEGRA_POWERGATE_CPU1] = "cpu1",
+	[TEGRA_POWERGATE_CPU2] = "cpu2",
+	[TEGRA_POWERGATE_CPU3] = "cpu3",
+	[TEGRA_POWERGATE_CELP] = "celp",
+	[TEGRA_POWERGATE_3D1] = "3d1",
+};
+
+static const u8 tegra30_cpu_powergates[] = {
+	TEGRA_POWERGATE_CPU,
+	TEGRA_POWERGATE_CPU1,
+	TEGRA_POWERGATE_CPU2,
+	TEGRA_POWERGATE_CPU3,
+};
+
+static const struct tegra_pmc_soc tegra30_pmc_soc = {
+	.num_powergates = ARRAY_SIZE(tegra30_powergates),
+	.powergates = tegra30_powergates,
+	.num_cpu_powergates = ARRAY_SIZE(tegra30_cpu_powergates),
+	.cpu_powergates = tegra30_cpu_powergates,
+	.has_tsense_reset = true,
+};
+
+static const char * const tegra114_powergates[] = {
+	[TEGRA_POWERGATE_CPU] = "crail",
+	[TEGRA_POWERGATE_3D] = "3d",
+	[TEGRA_POWERGATE_VENC] = "venc",
+	[TEGRA_POWERGATE_VDEC] = "vdec",
+	[TEGRA_POWERGATE_MPE] = "mpe",
+	[TEGRA_POWERGATE_HEG] = "heg",
+	[TEGRA_POWERGATE_CPU1] = "cpu1",
+	[TEGRA_POWERGATE_CPU2] = "cpu2",
+	[TEGRA_POWERGATE_CPU3] = "cpu3",
+	[TEGRA_POWERGATE_CELP] = "celp",
+	[TEGRA_POWERGATE_CPU0] = "cpu0",
+	[TEGRA_POWERGATE_C0NC] = "c0nc",
+	[TEGRA_POWERGATE_C1NC] = "c1nc",
+	[TEGRA_POWERGATE_DIS] = "dis",
+	[TEGRA_POWERGATE_DISB] = "disb",
+	[TEGRA_POWERGATE_XUSBA] = "xusba",
+	[TEGRA_POWERGATE_XUSBB] = "xusbb",
+	[TEGRA_POWERGATE_XUSBC] = "xusbc",
+};
+
+static const u8 tegra114_cpu_powergates[] = {
+	TEGRA_POWERGATE_CPU0,
+	TEGRA_POWERGATE_CPU1,
+	TEGRA_POWERGATE_CPU2,
+	TEGRA_POWERGATE_CPU3,
+};
+
+static const struct tegra_pmc_soc tegra114_pmc_soc = {
+	.num_powergates = ARRAY_SIZE(tegra114_powergates),
+	.powergates = tegra114_powergates,
+	.num_cpu_powergates = ARRAY_SIZE(tegra114_cpu_powergates),
+	.cpu_powergates = tegra114_cpu_powergates,
+	.has_tsense_reset = true,
+};
+
+static const char * const tegra124_powergates[] = {
+	[TEGRA_POWERGATE_CPU] = "crail",
+	[TEGRA_POWERGATE_3D] = "3d",
+	[TEGRA_POWERGATE_VENC] = "venc",
+	[TEGRA_POWERGATE_PCIE] = "pcie",
+	[TEGRA_POWERGATE_VDEC] = "vdec",
+	[TEGRA_POWERGATE_L2] = "l2",
+	[TEGRA_POWERGATE_MPE] = "mpe",
+	[TEGRA_POWERGATE_HEG] = "heg",
+	[TEGRA_POWERGATE_SATA] = "sata",
+	[TEGRA_POWERGATE_CPU1] = "cpu1",
+	[TEGRA_POWERGATE_CPU2] = "cpu2",
+	[TEGRA_POWERGATE_CPU3] = "cpu3",
+	[TEGRA_POWERGATE_CELP] = "celp",
+	[TEGRA_POWERGATE_CPU0] = "cpu0",
+	[TEGRA_POWERGATE_C0NC] = "c0nc",
+	[TEGRA_POWERGATE_C1NC] = "c1nc",
+	[TEGRA_POWERGATE_SOR] = "sor",
+	[TEGRA_POWERGATE_DIS] = "dis",
+	[TEGRA_POWERGATE_DISB] = "disb",
+	[TEGRA_POWERGATE_XUSBA] = "xusba",
+	[TEGRA_POWERGATE_XUSBB] = "xusbb",
+	[TEGRA_POWERGATE_XUSBC] = "xusbc",
+	[TEGRA_POWERGATE_VIC] = "vic",
+	[TEGRA_POWERGATE_IRAM] = "iram",
+};
+
+static const u8 tegra124_cpu_powergates[] = {
+	TEGRA_POWERGATE_CPU0,
+	TEGRA_POWERGATE_CPU1,
+	TEGRA_POWERGATE_CPU2,
+	TEGRA_POWERGATE_CPU3,
+};
+
+static const struct tegra_pmc_soc tegra124_pmc_soc = {
+	.num_powergates = ARRAY_SIZE(tegra124_powergates),
+	.powergates = tegra124_powergates,
+	.num_cpu_powergates = ARRAY_SIZE(tegra124_cpu_powergates),
+	.cpu_powergates = tegra124_cpu_powergates,
+	.has_tsense_reset = true,
+};
+
+static const struct of_device_id tegra_pmc_match[] = {
+	{ .compatible = "nvidia,tegra124-pmc", .data = &tegra124_pmc_soc },
+	{ .compatible = "nvidia,tegra114-pmc", .data = &tegra114_pmc_soc },
+	{ .compatible = "nvidia,tegra30-pmc", .data = &tegra30_pmc_soc },
+	{ .compatible = "nvidia,tegra20-pmc", .data = &tegra20_pmc_soc },
+	{ }
+};
+
+void tegra_pmc_init_tsense_reset(struct device *dev)
+{
+	u32 pmu_i2c_addr, i2c_ctrl_id, reg_addr, reg_data, pinmux;
+	u32 value, checksum;
+	struct device_node *np = dev->of_node;
+	struct device_node *i2c_np, *tt_np;
+	const struct of_device_id *match = of_match_node(tegra_pmc_match, np);
+	const struct tegra_pmc_soc *data = match->data;
+
+	if (!data->has_tsense_reset)
+		return;
+
+	tt_np = of_find_node_by_name(np, "i2c-thermtrip");
+	if (!tt_np) {
+		dev_warn(dev, "no i2c-thermtrip node found, disabling emergency thermal reset\n");
+		return;
+	}
+
+	i2c_np = of_parse_phandle(tt_np, "nvidia,pmu", 0);
+	if (!i2c_np) {
+		dev_err(dev, "PMU reference missing, disabling emergency thermal reset\n");
+		goto put_tt;
+	}
+
+	if (of_property_read_u32(i2c_np, "reg", &pmu_i2c_addr)) {
+		dev_err(dev, "PMU address missing, disabling emergency thermal reset\n");
+		goto put_i2c;
+	}
+
+	if (of_property_read_u32(i2c_np->parent, "nvidia,controller-id", &i2c_ctrl_id)) {
+		dev_err(dev, "PMU controller id missing, disabling emergency thermal reset\n");
+		goto put_i2c;
+	}
+
+	of_node_put(i2c_np);
+
+	if (of_property_read_u32(tt_np, "nvidia,reg-addr", &reg_addr)) {
+		dev_err(dev, "nvidia,reg-addr missing, disabling emergency thermal reset\n");
+		goto put_tt;
+	}
+
+	if (of_property_read_u32(tt_np, "nvidia,reg-data", &reg_data)) {
+		dev_err(dev, "nvidia,reg-data missing, disabling emergency thermal reset\n");
+		goto put_tt;
+	}
+
+	if (of_property_read_u32(tt_np, "nvidia,pinmux-id", &pinmux))
+		pinmux = 0;
+
+	of_node_put(tt_np);
+
+	value = tegra_pmc_readl(PMC_SENSOR_CTRL);
+	value |= PMC_SENSOR_CTRL_SCRATCH_WRITE;
+	tegra_pmc_writel(value, PMC_SENSOR_CTRL);
+
+	value = (reg_data << PMC_SCRATCH54_DATA_SHIFT) |
+	      (reg_addr << PMC_SCRATCH54_ADDR_SHIFT);
+	tegra_pmc_writel(value, PMC_SCRATCH54);
+
+	value = 0;
+	value |= PMC_SCRATCH55_RESET_TEGRA;
+	value |= i2c_ctrl_id << PMC_SCRATCH55_CNTRL_ID_SHIFT;
+	value |= pinmux << PMC_SCRATCH55_PINMUX_SHIFT;
+	value |= pmu_i2c_addr << PMC_SCRATCH55_I2CSLV1_SHIFT;
+
+	/* Calculate checksum of SCRATCH54, SCRATCH55 fields.
+	 * Bits 23:16 will contain the checksum and are currently zero,
+	 * so they are not added.
+	 */
+	checksum = reg_addr + reg_data + (value & 0xff) + ((value >> 8) & 0xff) +
+		((value >> 24) & 0xff);
+	checksum &= 0xff;
+	checksum = 0x100 - checksum;
+
+	value |= checksum << PMC_SCRATCH55_CHECKSUM_SHIFT;
+
+	tegra_pmc_writel(value, PMC_SCRATCH55);
+
+	value = tegra_pmc_readl(PMC_SENSOR_CTRL);
+	value |= PMC_SENSOR_CTRL_ENABLE_RST;
+	tegra_pmc_writel(value, PMC_SENSOR_CTRL);
+
+	dev_info(dev, "PMC emergency thermal reset enabled\n");
+
+	return;
+
+put_i2c:
+	of_node_put(i2c_np);
+
+put_tt:
+	of_node_put(tt_np);
+}
+
 static int tegra_pmc_parse_dt(struct tegra_pmc *pmc, struct device_node *np)
 {
 	u32 value, values[2];
@@ -730,6 +975,8 @@ static int tegra_pmc_probe(struct platform_device *pdev)
 
 	tegra_pmc_init(pmc);
 
+	tegra_pmc_init_tsense_reset(&pdev->dev);
+
 	if (IS_ENABLED(CONFIG_DEBUG_FS)) {
 		err = tegra_powergate_debugfs_init();
 		if (err < 0)
@@ -757,138 +1004,6 @@ static int tegra_pmc_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(tegra_pmc_pm_ops, tegra_pmc_suspend, tegra_pmc_resume);
 
-static const char * const tegra20_powergates[] = {
-	[TEGRA_POWERGATE_CPU] = "cpu",
-	[TEGRA_POWERGATE_3D] = "3d",
-	[TEGRA_POWERGATE_VENC] = "venc",
-	[TEGRA_POWERGATE_VDEC] = "vdec",
-	[TEGRA_POWERGATE_PCIE] = "pcie",
-	[TEGRA_POWERGATE_L2] = "l2",
-	[TEGRA_POWERGATE_MPE] = "mpe",
-};
-
-static const struct tegra_pmc_soc tegra20_pmc_soc = {
-	.num_powergates = ARRAY_SIZE(tegra20_powergates),
-	.powergates = tegra20_powergates,
-	.num_cpu_powergates = 0,
-	.cpu_powergates = NULL,
-};
-
-static const char * const tegra30_powergates[] = {
-	[TEGRA_POWERGATE_CPU] = "cpu0",
-	[TEGRA_POWERGATE_3D] = "3d0",
-	[TEGRA_POWERGATE_VENC] = "venc",
-	[TEGRA_POWERGATE_VDEC] = "vdec",
-	[TEGRA_POWERGATE_PCIE] = "pcie",
-	[TEGRA_POWERGATE_L2] = "l2",
-	[TEGRA_POWERGATE_MPE] = "mpe",
-	[TEGRA_POWERGATE_HEG] = "heg",
-	[TEGRA_POWERGATE_SATA] = "sata",
-	[TEGRA_POWERGATE_CPU1] = "cpu1",
-	[TEGRA_POWERGATE_CPU2] = "cpu2",
-	[TEGRA_POWERGATE_CPU3] = "cpu3",
-	[TEGRA_POWERGATE_CELP] = "celp",
-	[TEGRA_POWERGATE_3D1] = "3d1",
-};
-
-static const u8 tegra30_cpu_powergates[] = {
-	TEGRA_POWERGATE_CPU,
-	TEGRA_POWERGATE_CPU1,
-	TEGRA_POWERGATE_CPU2,
-	TEGRA_POWERGATE_CPU3,
-};
-
-static const struct tegra_pmc_soc tegra30_pmc_soc = {
-	.num_powergates = ARRAY_SIZE(tegra30_powergates),
-	.powergates = tegra30_powergates,
-	.num_cpu_powergates = ARRAY_SIZE(tegra30_cpu_powergates),
-	.cpu_powergates = tegra30_cpu_powergates,
-};
-
-static const char * const tegra114_powergates[] = {
-	[TEGRA_POWERGATE_CPU] = "crail",
-	[TEGRA_POWERGATE_3D] = "3d",
-	[TEGRA_POWERGATE_VENC] = "venc",
-	[TEGRA_POWERGATE_VDEC] = "vdec",
-	[TEGRA_POWERGATE_MPE] = "mpe",
-	[TEGRA_POWERGATE_HEG] = "heg",
-	[TEGRA_POWERGATE_CPU1] = "cpu1",
-	[TEGRA_POWERGATE_CPU2] = "cpu2",
-	[TEGRA_POWERGATE_CPU3] = "cpu3",
-	[TEGRA_POWERGATE_CELP] = "celp",
-	[TEGRA_POWERGATE_CPU0] = "cpu0",
-	[TEGRA_POWERGATE_C0NC] = "c0nc",
-	[TEGRA_POWERGATE_C1NC] = "c1nc",
-	[TEGRA_POWERGATE_DIS] = "dis",
-	[TEGRA_POWERGATE_DISB] = "disb",
-	[TEGRA_POWERGATE_XUSBA] = "xusba",
-	[TEGRA_POWERGATE_XUSBB] = "xusbb",
-	[TEGRA_POWERGATE_XUSBC] = "xusbc",
-};
-
-static const u8 tegra114_cpu_powergates[] = {
-	TEGRA_POWERGATE_CPU0,
-	TEGRA_POWERGATE_CPU1,
-	TEGRA_POWERGATE_CPU2,
-	TEGRA_POWERGATE_CPU3,
-};
-
-static const struct tegra_pmc_soc tegra114_pmc_soc = {
-	.num_powergates = ARRAY_SIZE(tegra114_powergates),
-	.powergates = tegra114_powergates,
-	.num_cpu_powergates = ARRAY_SIZE(tegra114_cpu_powergates),
-	.cpu_powergates = tegra114_cpu_powergates,
-};
-
-static const char * const tegra124_powergates[] = {
-	[TEGRA_POWERGATE_CPU] = "crail",
-	[TEGRA_POWERGATE_3D] = "3d",
-	[TEGRA_POWERGATE_VENC] = "venc",
-	[TEGRA_POWERGATE_PCIE] = "pcie",
-	[TEGRA_POWERGATE_VDEC] = "vdec",
-	[TEGRA_POWERGATE_L2] = "l2",
-	[TEGRA_POWERGATE_MPE] = "mpe",
-	[TEGRA_POWERGATE_HEG] = "heg",
-	[TEGRA_POWERGATE_SATA] = "sata",
-	[TEGRA_POWERGATE_CPU1] = "cpu1",
-	[TEGRA_POWERGATE_CPU2] = "cpu2",
-	[TEGRA_POWERGATE_CPU3] = "cpu3",
-	[TEGRA_POWERGATE_CELP] = "celp",
-	[TEGRA_POWERGATE_CPU0] = "cpu0",
-	[TEGRA_POWERGATE_C0NC] = "c0nc",
-	[TEGRA_POWERGATE_C1NC] = "c1nc",
-	[TEGRA_POWERGATE_SOR] = "sor",
-	[TEGRA_POWERGATE_DIS] = "dis",
-	[TEGRA_POWERGATE_DISB] = "disb",
-	[TEGRA_POWERGATE_XUSBA] = "xusba",
-	[TEGRA_POWERGATE_XUSBB] = "xusbb",
-	[TEGRA_POWERGATE_XUSBC] = "xusbc",
-	[TEGRA_POWERGATE_VIC] = "vic",
-	[TEGRA_POWERGATE_IRAM] = "iram",
-};
-
-static const u8 tegra124_cpu_powergates[] = {
-	TEGRA_POWERGATE_CPU0,
-	TEGRA_POWERGATE_CPU1,
-	TEGRA_POWERGATE_CPU2,
-	TEGRA_POWERGATE_CPU3,
-};
-
-static const struct tegra_pmc_soc tegra124_pmc_soc = {
-	.num_powergates = ARRAY_SIZE(tegra124_powergates),
-	.powergates = tegra124_powergates,
-	.num_cpu_powergates = ARRAY_SIZE(tegra124_cpu_powergates),
-	.cpu_powergates = tegra124_cpu_powergates,
-};
-
-static const struct of_device_id tegra_pmc_match[] = {
-	{ .compatible = "nvidia,tegra124-pmc", .data = &tegra124_pmc_soc },
-	{ .compatible = "nvidia,tegra114-pmc", .data = &tegra114_pmc_soc },
-	{ .compatible = "nvidia,tegra30-pmc", .data = &tegra30_pmc_soc },
-	{ .compatible = "nvidia,tegra20-pmc", .data = &tegra20_pmc_soc },
-	{ }
-};
-
 static struct platform_driver tegra_pmc_driver = {
 	.driver = {
 		.name = "tegra-pmc",
-- 
1.8.1.5

[PATCH v2 5/5] ARM: tegra: Add thermal reset (thermtrip) support to PMC

[Mikko Perttunen]

This adds a device tree controlled option to enable PMC-based
thermal reset in overheating situations. Thermtrip is supported on
Tegra30, Tegra114 and Tegra124. The thermal reset only works when
the thermal sensors are calibrated, so a soctherm driver is also
required.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/soc/tegra/pmc.c | 379 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 247 insertions(+), 132 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index a2c0ceb..a5a35c4 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -83,11 +83,28 @@
 
 #define GPU_RG_CNTRL			0x2d4
 
+#define PMC_SENSOR_CTRL			0x1b0
+#define PMC_SENSOR_CTRL_SCRATCH_WRITE	(1 << 2)
+#define PMC_SENSOR_CTRL_ENABLE_RST	(1 << 1)
+
+#define PMC_SCRATCH54			0x258
+#define PMC_SCRATCH54_DATA_SHIFT	8
+#define PMC_SCRATCH54_ADDR_SHIFT	0
+
+#define PMC_SCRATCH55			0x25c
+#define PMC_SCRATCH55_RESET_TEGRA	(1 << 31)
+#define PMC_SCRATCH55_CNTRL_ID_SHIFT	27
+#define PMC_SCRATCH55_PINMUX_SHIFT	24
+#define PMC_SCRATCH55_16BITOP		(1 << 15)
+#define PMC_SCRATCH55_CHECKSUM_SHIFT	16
+#define PMC_SCRATCH55_I2CSLV1_SHIFT	0
+
 struct tegra_pmc_soc {
 	unsigned int num_powergates;
 	const char *const *powergates;
 	unsigned int num_cpu_powergates;
 	const u8 *cpu_powergates;
+	bool has_tsense_reset;
 };
 
 /**
@@ -606,6 +623,234 @@ void tegra_pmc_enter_suspend_mode(enum tegra_suspend_mode mode)
 }
 #endif
 
+static const char * const tegra20_powergates[] = {
+	[TEGRA_POWERGATE_CPU] = "cpu",
+	[TEGRA_POWERGATE_3D] = "3d",
+	[TEGRA_POWERGATE_VENC] = "venc",
+	[TEGRA_POWERGATE_VDEC] = "vdec",
+	[TEGRA_POWERGATE_PCIE] = "pcie",
+	[TEGRA_POWERGATE_L2] = "l2",
+	[TEGRA_POWERGATE_MPE] = "mpe",
+};
+
+static const struct tegra_pmc_soc tegra20_pmc_soc = {
+	.num_powergates = ARRAY_SIZE(tegra20_powergates),
+	.powergates = tegra20_powergates,
+	.num_cpu_powergates = 0,
+	.cpu_powergates = NULL,
+};
+
+static const char * const tegra30_powergates[] = {
+	[TEGRA_POWERGATE_CPU] = "cpu0",
+	[TEGRA_POWERGATE_3D] = "3d0",
+	[TEGRA_POWERGATE_VENC] = "venc",
+	[TEGRA_POWERGATE_VDEC] = "vdec",
+	[TEGRA_POWERGATE_PCIE] = "pcie",
+	[TEGRA_POWERGATE_L2] = "l2",
+	[TEGRA_POWERGATE_MPE] = "mpe",
+	[TEGRA_POWERGATE_HEG] = "heg",
+	[TEGRA_POWERGATE_SATA] = "sata",
+	[TEGRA_POWERGATE_CPU1] = "cpu1",
+	[TEGRA_POWERGATE_CPU2] = "cpu2",
+	[TEGRA_POWERGATE_CPU3] = "cpu3",
+	[TEGRA_POWERGATE_CELP] = "celp",
+	[TEGRA_POWERGATE_3D1] = "3d1",
+};
+
+static const u8 tegra30_cpu_powergates[] = {
+	TEGRA_POWERGATE_CPU,
+	TEGRA_POWERGATE_CPU1,
+	TEGRA_POWERGATE_CPU2,
+	TEGRA_POWERGATE_CPU3,
+};
+
+static const struct tegra_pmc_soc tegra30_pmc_soc = {
+	.num_powergates = ARRAY_SIZE(tegra30_powergates),
+	.powergates = tegra30_powergates,
+	.num_cpu_powergates = ARRAY_SIZE(tegra30_cpu_powergates),
+	.cpu_powergates = tegra30_cpu_powergates,
+	.has_tsense_reset = true,
+};
+
+static const char * const tegra114_powergates[] = {
+	[TEGRA_POWERGATE_CPU] = "crail",
+	[TEGRA_POWERGATE_3D] = "3d",
+	[TEGRA_POWERGATE_VENC] = "venc",
+	[TEGRA_POWERGATE_VDEC] = "vdec",
+	[TEGRA_POWERGATE_MPE] = "mpe",
+	[TEGRA_POWERGATE_HEG] = "heg",
+	[TEGRA_POWERGATE_CPU1] = "cpu1",
+	[TEGRA_POWERGATE_CPU2] = "cpu2",
+	[TEGRA_POWERGATE_CPU3] = "cpu3",
+	[TEGRA_POWERGATE_CELP] = "celp",
+	[TEGRA_POWERGATE_CPU0] = "cpu0",
+	[TEGRA_POWERGATE_C0NC] = "c0nc",
+	[TEGRA_POWERGATE_C1NC] = "c1nc",
+	[TEGRA_POWERGATE_DIS] = "dis",
+	[TEGRA_POWERGATE_DISB] = "disb",
+	[TEGRA_POWERGATE_XUSBA] = "xusba",
+	[TEGRA_POWERGATE_XUSBB] = "xusbb",
+	[TEGRA_POWERGATE_XUSBC] = "xusbc",
+};
+
+static const u8 tegra114_cpu_powergates[] = {
+	TEGRA_POWERGATE_CPU0,
+	TEGRA_POWERGATE_CPU1,
+	TEGRA_POWERGATE_CPU2,
+	TEGRA_POWERGATE_CPU3,
+};
+
+static const struct tegra_pmc_soc tegra114_pmc_soc = {
+	.num_powergates = ARRAY_SIZE(tegra114_powergates),
+	.powergates = tegra114_powergates,
+	.num_cpu_powergates = ARRAY_SIZE(tegra114_cpu_powergates),
+	.cpu_powergates = tegra114_cpu_powergates,
+	.has_tsense_reset = true,
+};
+
+static const char * const tegra124_powergates[] = {
+	[TEGRA_POWERGATE_CPU] = "crail",
+	[TEGRA_POWERGATE_3D] = "3d",
+	[TEGRA_POWERGATE_VENC] = "venc",
+	[TEGRA_POWERGATE_PCIE] = "pcie",
+	[TEGRA_POWERGATE_VDEC] = "vdec",
+	[TEGRA_POWERGATE_L2] = "l2",
+	[TEGRA_POWERGATE_MPE] = "mpe",
+	[TEGRA_POWERGATE_HEG] = "heg",
+	[TEGRA_POWERGATE_SATA] = "sata",
+	[TEGRA_POWERGATE_CPU1] = "cpu1",
+	[TEGRA_POWERGATE_CPU2] = "cpu2",
+	[TEGRA_POWERGATE_CPU3] = "cpu3",
+	[TEGRA_POWERGATE_CELP] = "celp",
+	[TEGRA_POWERGATE_CPU0] = "cpu0",
+	[TEGRA_POWERGATE_C0NC] = "c0nc",
+	[TEGRA_POWERGATE_C1NC] = "c1nc",
+	[TEGRA_POWERGATE_SOR] = "sor",
+	[TEGRA_POWERGATE_DIS] = "dis",
+	[TEGRA_POWERGATE_DISB] = "disb",
+	[TEGRA_POWERGATE_XUSBA] = "xusba",
+	[TEGRA_POWERGATE_XUSBB] = "xusbb",
+	[TEGRA_POWERGATE_XUSBC] = "xusbc",
+	[TEGRA_POWERGATE_VIC] = "vic",
+	[TEGRA_POWERGATE_IRAM] = "iram",
+};
+
+static const u8 tegra124_cpu_powergates[] = {
+	TEGRA_POWERGATE_CPU0,
+	TEGRA_POWERGATE_CPU1,
+	TEGRA_POWERGATE_CPU2,
+	TEGRA_POWERGATE_CPU3,
+};
+
+static const struct tegra_pmc_soc tegra124_pmc_soc = {
+	.num_powergates = ARRAY_SIZE(tegra124_powergates),
+	.powergates = tegra124_powergates,
+	.num_cpu_powergates = ARRAY_SIZE(tegra124_cpu_powergates),
+	.cpu_powergates = tegra124_cpu_powergates,
+	.has_tsense_reset = true,
+};
+
+static const struct of_device_id tegra_pmc_match[] = {
+	{ .compatible = "nvidia,tegra124-pmc", .data = &tegra124_pmc_soc },
+	{ .compatible = "nvidia,tegra114-pmc", .data = &tegra114_pmc_soc },
+	{ .compatible = "nvidia,tegra30-pmc", .data = &tegra30_pmc_soc },
+	{ .compatible = "nvidia,tegra20-pmc", .data = &tegra20_pmc_soc },
+	{ }
+};
+
+void tegra_pmc_init_tsense_reset(struct device *dev)
+{
+	u32 pmu_i2c_addr, i2c_ctrl_id, reg_addr, reg_data, pinmux;
+	u32 value, checksum;
+	struct device_node *np = dev->of_node;
+	struct device_node *i2c_np, *tt_np;
+	const struct of_device_id *match = of_match_node(tegra_pmc_match, np);
+	const struct tegra_pmc_soc *data = match->data;
+
+	if (!data->has_tsense_reset)
+		return;
+
+	tt_np = of_find_node_by_name(np, "i2c-thermtrip");
+	if (!tt_np) {
+		dev_warn(dev, "no i2c-thermtrip node found, disabling emergency thermal reset\n");
+		return;
+	}
+
+	i2c_np = of_parse_phandle(tt_np, "nvidia,pmu", 0);
+	if (!i2c_np) {
+		dev_err(dev, "PMU reference missing, disabling emergency thermal reset\n");
+		goto put_tt;
+	}
+
+	if (of_property_read_u32(i2c_np, "reg", &pmu_i2c_addr)) {
+		dev_err(dev, "PMU address missing, disabling emergency thermal reset\n");
+		goto put_i2c;
+	}
+
+	if (of_property_read_u32(i2c_np->parent, "nvidia,controller-id", &i2c_ctrl_id)) {
+		dev_err(dev, "PMU controller id missing, disabling emergency thermal reset\n");
+		goto put_i2c;
+	}
+
+	of_node_put(i2c_np);
+
+	if (of_property_read_u32(tt_np, "nvidia,reg-addr", &reg_addr)) {
+		dev_err(dev, "nvidia,reg-addr missing, disabling emergency thermal reset\n");
+		goto put_tt;
+	}
+
+	if (of_property_read_u32(tt_np, "nvidia,reg-data", &reg_data)) {
+		dev_err(dev, "nvidia,reg-data missing, disabling emergency thermal reset\n");
+		goto put_tt;
+	}
+
+	if (of_property_read_u32(tt_np, "nvidia,pinmux-id", &pinmux))
+		pinmux = 0;
+
+	of_node_put(tt_np);
+
+	value = tegra_pmc_readl(PMC_SENSOR_CTRL);
+	value |= PMC_SENSOR_CTRL_SCRATCH_WRITE;
+	tegra_pmc_writel(value, PMC_SENSOR_CTRL);
+
+	value = (reg_data << PMC_SCRATCH54_DATA_SHIFT) |
+	      (reg_addr << PMC_SCRATCH54_ADDR_SHIFT);
+	tegra_pmc_writel(value, PMC_SCRATCH54);
+
+	value = 0;
+	value |= PMC_SCRATCH55_RESET_TEGRA;
+	value |= i2c_ctrl_id << PMC_SCRATCH55_CNTRL_ID_SHIFT;
+	value |= pinmux << PMC_SCRATCH55_PINMUX_SHIFT;
+	value |= pmu_i2c_addr << PMC_SCRATCH55_I2CSLV1_SHIFT;
+
+	/* Calculate checksum of SCRATCH54, SCRATCH55 fields.
+	 * Bits 23:16 will contain the checksum and are currently zero,
+	 * so they are not added.
+	 */
+	checksum = reg_addr + reg_data + (value & 0xff) + ((value >> 8) & 0xff) +
+		((value >> 24) & 0xff);
+	checksum &= 0xff;
+	checksum = 0x100 - checksum;
+
+	value |= checksum << PMC_SCRATCH55_CHECKSUM_SHIFT;
+
+	tegra_pmc_writel(value, PMC_SCRATCH55);
+
+	value = tegra_pmc_readl(PMC_SENSOR_CTRL);
+	value |= PMC_SENSOR_CTRL_ENABLE_RST;
+	tegra_pmc_writel(value, PMC_SENSOR_CTRL);
+
+	dev_info(dev, "PMC emergency thermal reset enabled\n");
+
+	return;
+
+put_i2c:
+	of_node_put(i2c_np);
+
+put_tt:
+	of_node_put(tt_np);
+}
+
 static int tegra_pmc_parse_dt(struct tegra_pmc *pmc, struct device_node *np)
 {
 	u32 value, values[2];
@@ -730,6 +975,8 @@ static int tegra_pmc_probe(struct platform_device *pdev)
 
 	tegra_pmc_init(pmc);
 
+	tegra_pmc_init_tsense_reset(&pdev->dev);
+
 	if (IS_ENABLED(CONFIG_DEBUG_FS)) {
 		err = tegra_powergate_debugfs_init();
 		if (err < 0)
@@ -757,138 +1004,6 @@ static int tegra_pmc_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(tegra_pmc_pm_ops, tegra_pmc_suspend, tegra_pmc_resume);
 
-static const char * const tegra20_powergates[] = {
-	[TEGRA_POWERGATE_CPU] = "cpu",
-	[TEGRA_POWERGATE_3D] = "3d",
-	[TEGRA_POWERGATE_VENC] = "venc",
-	[TEGRA_POWERGATE_VDEC] = "vdec",
-	[TEGRA_POWERGATE_PCIE] = "pcie",
-	[TEGRA_POWERGATE_L2] = "l2",
-	[TEGRA_POWERGATE_MPE] = "mpe",
-};
-
-static const struct tegra_pmc_soc tegra20_pmc_soc = {
-	.num_powergates = ARRAY_SIZE(tegra20_powergates),
-	.powergates = tegra20_powergates,
-	.num_cpu_powergates = 0,
-	.cpu_powergates = NULL,
-};
-
-static const char * const tegra30_powergates[] = {
-	[TEGRA_POWERGATE_CPU] = "cpu0",
-	[TEGRA_POWERGATE_3D] = "3d0",
-	[TEGRA_POWERGATE_VENC] = "venc",
-	[TEGRA_POWERGATE_VDEC] = "vdec",
-	[TEGRA_POWERGATE_PCIE] = "pcie",
-	[TEGRA_POWERGATE_L2] = "l2",
-	[TEGRA_POWERGATE_MPE] = "mpe",
-	[TEGRA_POWERGATE_HEG] = "heg",
-	[TEGRA_POWERGATE_SATA] = "sata",
-	[TEGRA_POWERGATE_CPU1] = "cpu1",
-	[TEGRA_POWERGATE_CPU2] = "cpu2",
-	[TEGRA_POWERGATE_CPU3] = "cpu3",
-	[TEGRA_POWERGATE_CELP] = "celp",
-	[TEGRA_POWERGATE_3D1] = "3d1",
-};
-
-static const u8 tegra30_cpu_powergates[] = {
-	TEGRA_POWERGATE_CPU,
-	TEGRA_POWERGATE_CPU1,
-	TEGRA_POWERGATE_CPU2,
-	TEGRA_POWERGATE_CPU3,
-};
-
-static const struct tegra_pmc_soc tegra30_pmc_soc = {
-	.num_powergates = ARRAY_SIZE(tegra30_powergates),
-	.powergates = tegra30_powergates,
-	.num_cpu_powergates = ARRAY_SIZE(tegra30_cpu_powergates),
-	.cpu_powergates = tegra30_cpu_powergates,
-};
-
-static const char * const tegra114_powergates[] = {
-	[TEGRA_POWERGATE_CPU] = "crail",
-	[TEGRA_POWERGATE_3D] = "3d",
-	[TEGRA_POWERGATE_VENC] = "venc",
-	[TEGRA_POWERGATE_VDEC] = "vdec",
-	[TEGRA_POWERGATE_MPE] = "mpe",
-	[TEGRA_POWERGATE_HEG] = "heg",
-	[TEGRA_POWERGATE_CPU1] = "cpu1",
-	[TEGRA_POWERGATE_CPU2] = "cpu2",
-	[TEGRA_POWERGATE_CPU3] = "cpu3",
-	[TEGRA_POWERGATE_CELP] = "celp",
-	[TEGRA_POWERGATE_CPU0] = "cpu0",
-	[TEGRA_POWERGATE_C0NC] = "c0nc",
-	[TEGRA_POWERGATE_C1NC] = "c1nc",
-	[TEGRA_POWERGATE_DIS] = "dis",
-	[TEGRA_POWERGATE_DISB] = "disb",
-	[TEGRA_POWERGATE_XUSBA] = "xusba",
-	[TEGRA_POWERGATE_XUSBB] = "xusbb",
-	[TEGRA_POWERGATE_XUSBC] = "xusbc",
-};
-
-static const u8 tegra114_cpu_powergates[] = {
-	TEGRA_POWERGATE_CPU0,
-	TEGRA_POWERGATE_CPU1,
-	TEGRA_POWERGATE_CPU2,
-	TEGRA_POWERGATE_CPU3,
-};
-
-static const struct tegra_pmc_soc tegra114_pmc_soc = {
-	.num_powergates = ARRAY_SIZE(tegra114_powergates),
-	.powergates = tegra114_powergates,
-	.num_cpu_powergates = ARRAY_SIZE(tegra114_cpu_powergates),
-	.cpu_powergates = tegra114_cpu_powergates,
-};
-
-static const char * const tegra124_powergates[] = {
-	[TEGRA_POWERGATE_CPU] = "crail",
-	[TEGRA_POWERGATE_3D] = "3d",
-	[TEGRA_POWERGATE_VENC] = "venc",
-	[TEGRA_POWERGATE_PCIE] = "pcie",
-	[TEGRA_POWERGATE_VDEC] = "vdec",
-	[TEGRA_POWERGATE_L2] = "l2",
-	[TEGRA_POWERGATE_MPE] = "mpe",
-	[TEGRA_POWERGATE_HEG] = "heg",
-	[TEGRA_POWERGATE_SATA] = "sata",
-	[TEGRA_POWERGATE_CPU1] = "cpu1",
-	[TEGRA_POWERGATE_CPU2] = "cpu2",
-	[TEGRA_POWERGATE_CPU3] = "cpu3",
-	[TEGRA_POWERGATE_CELP] = "celp",
-	[TEGRA_POWERGATE_CPU0] = "cpu0",
-	[TEGRA_POWERGATE_C0NC] = "c0nc",
-	[TEGRA_POWERGATE_C1NC] = "c1nc",
-	[TEGRA_POWERGATE_SOR] = "sor",
-	[TEGRA_POWERGATE_DIS] = "dis",
-	[TEGRA_POWERGATE_DISB] = "disb",
-	[TEGRA_POWERGATE_XUSBA] = "xusba",
-	[TEGRA_POWERGATE_XUSBB] = "xusbb",
-	[TEGRA_POWERGATE_XUSBC] = "xusbc",
-	[TEGRA_POWERGATE_VIC] = "vic",
-	[TEGRA_POWERGATE_IRAM] = "iram",
-};
-
-static const u8 tegra124_cpu_powergates[] = {
-	TEGRA_POWERGATE_CPU0,
-	TEGRA_POWERGATE_CPU1,
-	TEGRA_POWERGATE_CPU2,
-	TEGRA_POWERGATE_CPU3,
-};
-
-static const struct tegra_pmc_soc tegra124_pmc_soc = {
-	.num_powergates = ARRAY_SIZE(tegra124_powergates),
-	.powergates = tegra124_powergates,
-	.num_cpu_powergates = ARRAY_SIZE(tegra124_cpu_powergates),
-	.cpu_powergates = tegra124_cpu_powergates,
-};
-
-static const struct of_device_id tegra_pmc_match[] = {
-	{ .compatible = "nvidia,tegra124-pmc", .data = &tegra124_pmc_soc },
-	{ .compatible = "nvidia,tegra114-pmc", .data = &tegra114_pmc_soc },
-	{ .compatible = "nvidia,tegra30-pmc", .data = &tegra30_pmc_soc },
-	{ .compatible = "nvidia,tegra20-pmc", .data = &tegra20_pmc_soc },
-	{ }
-};
-
 static struct platform_driver tegra_pmc_driver = {
 	.driver = {
 		.name = "tegra-pmc",
-- 
1.8.1.5

Re: [PATCH v2 0/5] Thermal reset support in PMC

[Wei Ni]

Looks good to me, and I test it with soc-therm driver, the system can be
shutdown when overheat.

Thanks.
Wei.

On 08/13/2014 08:41 PM, Mikko Perttunen wrote:
> Hi,
> 
> this series adds support for hardware-triggered thermal reset to the PMC
> driver. Namely, it adds device tree properties for specifying the I2C
> command to be sent when thermtrip is triggered. It is to be noted
> that thermtrip won't be ever triggered without a soctherm driver to
> calibrate the sensors.
> 
> Git repo at
>   git://github.com/cyndis/linux.git pmc-thermtrip-v2
> 
> Testing script at https://gist.github.com/cyndis/66126c9c176b5f94a76f.
> If your fan stops it is working.
> 
> Mikko Perttunen (5):
>   of: Add descriptions of thermtrip properties to Tegra PMC bindings
>   of: Add nvidia,controller-id property to Tegra I2C bindings
>   ARM: tegra124: Add I2C controller ids to device tree
>   ARM: tegra: Add PMC thermtrip programming to Jetson TK1 device tree
>   ARM: tegra: Add thermal reset (thermtrip) support to PMC
> 
>  .../bindings/arm/tegra/nvidia,tegra20-pmc.txt      |  21 ++
>  .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt |   5 +
>  arch/arm/boot/dts/tegra124-jetson-tk1.dts          |   6 +
>  arch/arm/boot/dts/tegra124.dtsi                    |   6 +
>  drivers/soc/tegra/pmc.c                            | 379 ++++++++++++++-------
>  5 files changed, 285 insertions(+), 132 deletions(-)
> 

Re: [PATCH v2 0/5] Thermal reset support in PMC

[Wei Ni]

Looks good to me, and I test it with soc-therm driver, the system can be
shutdown when overheat.

Thanks.
Wei.

On 08/13/2014 08:41 PM, Mikko Perttunen wrote:
> Hi,
> 
> this series adds support for hardware-triggered thermal reset to the PMC
> driver. Namely, it adds device tree properties for specifying the I2C
> command to be sent when thermtrip is triggered. It is to be noted
> that thermtrip won't be ever triggered without a soctherm driver to
> calibrate the sensors.
> 
> Git repo at
>   git://github.com/cyndis/linux.git pmc-thermtrip-v2
> 
> Testing script at https://gist.github.com/cyndis/66126c9c176b5f94a76f.
> If your fan stops it is working.
> 
> Mikko Perttunen (5):
>   of: Add descriptions of thermtrip properties to Tegra PMC bindings
>   of: Add nvidia,controller-id property to Tegra I2C bindings
>   ARM: tegra124: Add I2C controller ids to device tree
>   ARM: tegra: Add PMC thermtrip programming to Jetson TK1 device tree
>   ARM: tegra: Add thermal reset (thermtrip) support to PMC
> 
>  .../bindings/arm/tegra/nvidia,tegra20-pmc.txt      |  21 ++
>  .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt |   5 +
>  arch/arm/boot/dts/tegra124-jetson-tk1.dts          |   6 +
>  arch/arm/boot/dts/tegra124.dtsi                    |   6 +
>  drivers/soc/tegra/pmc.c                            | 379 ++++++++++++++-------
>  5 files changed, 285 insertions(+), 132 deletions(-)
> 

[PATCH v2 0/5] Thermal reset support in PMC

[Wei Ni]

Looks good to me, and I test it with soc-therm driver, the system can be
shutdown when overheat.

Thanks.
Wei.

On 08/13/2014 08:41 PM, Mikko Perttunen wrote:
> Hi,
> 
> this series adds support for hardware-triggered thermal reset to the PMC
> driver. Namely, it adds device tree properties for specifying the I2C
> command to be sent when thermtrip is triggered. It is to be noted
> that thermtrip won't be ever triggered without a soctherm driver to
> calibrate the sensors.
> 
> Git repo at
>   git://github.com/cyndis/linux.git pmc-thermtrip-v2
> 
> Testing script at https://gist.github.com/cyndis/66126c9c176b5f94a76f.
> If your fan stops it is working.
> 
> Mikko Perttunen (5):
>   of: Add descriptions of thermtrip properties to Tegra PMC bindings
>   of: Add nvidia,controller-id property to Tegra I2C bindings
>   ARM: tegra124: Add I2C controller ids to device tree
>   ARM: tegra: Add PMC thermtrip programming to Jetson TK1 device tree
>   ARM: tegra: Add thermal reset (thermtrip) support to PMC
> 
>  .../bindings/arm/tegra/nvidia,tegra20-pmc.txt      |  21 ++
>  .../devicetree/bindings/i2c/nvidia,tegra20-i2c.txt |   5 +
>  arch/arm/boot/dts/tegra124-jetson-tk1.dts          |   6 +
>  arch/arm/boot/dts/tegra124.dtsi                    |   6 +
>  drivers/soc/tegra/pmc.c                            | 379 ++++++++++++++-------
>  5 files changed, 285 insertions(+), 132 deletions(-)
> 

Re: [PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Stephen Warren]

On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
> Hardware-triggered thermal reset requires configuring the I2C
> reset procedure. This configuration is read from the device tree,
> so document the relevant properties in the binding documentation.

> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt

> +Hardware-triggered thermal reset:
> +On Tegra30, Tegra114 and Tegra124, if the 'i2c-thermtrip' subnode exists,
> +hardware-triggered thermal reset will be enabled.

"will be enabled" sounds like SW behaviour, whereas DT is suppose to 
describe HW, and leave SW to define its own behaviour. I would suggest:

Optional sub-nodes:
i2c-thermtrip: Describes how to power off the system in the event of a
   thermal emergency.

> +Required properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):

Simpler might be:

Required properties for i2c-thermtrip node:

> +- nvidia,pmu : Phandle to power management unit / PMIC handling poweroff
> +- nvidia,reg-addr : I2C register address to write poweroff command to
> +- nvidia,reg-data : Poweroff command to write to PMU

Why are both the PMU/PMIC phandle and the register address/data 
required? I thought the purpose of having the phandle was to allow the 
register address and data to be queried from the PMU/PMIC driver.

To me, it seems much simpler to get rid of the phandle and just 
hard-code the I2C bus number, address, and data into this node, rather 
than having to go query it from the PMU/PMIC driver, then find the I2C 
controller, then query it for its ID (and hope that all HW modules that 
talk to I2C controllers directly use the same numbering scheme...)

Re: [PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Stephen Warren]

On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
> Hardware-triggered thermal reset requires configuring the I2C
> reset procedure. This configuration is read from the device tree,
> so document the relevant properties in the binding documentation.

> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt

> +Hardware-triggered thermal reset:
> +On Tegra30, Tegra114 and Tegra124, if the 'i2c-thermtrip' subnode exists,
> +hardware-triggered thermal reset will be enabled.

"will be enabled" sounds like SW behaviour, whereas DT is suppose to 
describe HW, and leave SW to define its own behaviour. I would suggest:

Optional sub-nodes:
i2c-thermtrip: Describes how to power off the system in the event of a
   thermal emergency.

> +Required properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):

Simpler might be:

Required properties for i2c-thermtrip node:

> +- nvidia,pmu : Phandle to power management unit / PMIC handling poweroff
> +- nvidia,reg-addr : I2C register address to write poweroff command to
> +- nvidia,reg-data : Poweroff command to write to PMU

Why are both the PMU/PMIC phandle and the register address/data 
required? I thought the purpose of having the phandle was to allow the 
register address and data to be queried from the PMU/PMIC driver.

To me, it seems much simpler to get rid of the phandle and just 
hard-code the I2C bus number, address, and data into this node, rather 
than having to go query it from the PMU/PMIC driver, then find the I2C 
controller, then query it for its ID (and hope that all HW modules that 
talk to I2C controllers directly use the same numbering scheme...)

[PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Stephen Warren]

On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
> Hardware-triggered thermal reset requires configuring the I2C
> reset procedure. This configuration is read from the device tree,
> so document the relevant properties in the binding documentation.

> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt

> +Hardware-triggered thermal reset:
> +On Tegra30, Tegra114 and Tegra124, if the 'i2c-thermtrip' subnode exists,
> +hardware-triggered thermal reset will be enabled.

"will be enabled" sounds like SW behaviour, whereas DT is suppose to 
describe HW, and leave SW to define its own behaviour. I would suggest:

Optional sub-nodes:
i2c-thermtrip: Describes how to power off the system in the event of a
   thermal emergency.

> +Required properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):

Simpler might be:

Required properties for i2c-thermtrip node:

> +- nvidia,pmu : Phandle to power management unit / PMIC handling poweroff
> +- nvidia,reg-addr : I2C register address to write poweroff command to
> +- nvidia,reg-data : Poweroff command to write to PMU

Why are both the PMU/PMIC phandle and the register address/data 
required? I thought the purpose of having the phandle was to allow the 
register address and data to be queried from the PMU/PMIC driver.

To me, it seems much simpler to get rid of the phandle and just 
hard-code the I2C bus number, address, and data into this node, rather 
than having to go query it from the PMU/PMIC driver, then find the I2C 
controller, then query it for its ID (and hope that all HW modules that 
talk to I2C controllers directly use the same numbering scheme...)

Re: [PATCH v2 2/5] of: Add nvidia,controller-id property to Tegra I2C bindings

[Stephen Warren]

On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
> Sometimes, hardware blocks want to issue requests to devices
> connected to I2C buses by itself. In such case, the bus the
> target device resides on must be configured into a register.
> For this purpose, each I2C controller has a defined ID known
> by the hardware. Add a property for these IDs to the device tree
> bindings, so that drivers can know what ID to write to a hardware
> register when configuring a block that sends I2C messages autonomously.

> diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt

> +Optional properties:
> +- nvidia,controller-id: ID of controller when referred to in
> +                        hardware registers.

I'd prefer to put this information into the thermal trip node, since 
this represents what ID the PMC uses to communicate with the I2C 
controller, and there's no absolute guarantee that multiple clients that 
communicate directly with an I2C controller would use the same numbering 
scheme.

If that doesn't work, can be at least name this nvidia,pmc-controller-id 
or nvidia,id-in-pmc so that if there are different numbering schemes, 
there's a clear path to represent this in different properties without 
conflicting names?

Re: [PATCH v2 2/5] of: Add nvidia,controller-id property to Tegra I2C bindings

[Stephen Warren]

On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
> Sometimes, hardware blocks want to issue requests to devices
> connected to I2C buses by itself. In such case, the bus the
> target device resides on must be configured into a register.
> For this purpose, each I2C controller has a defined ID known
> by the hardware. Add a property for these IDs to the device tree
> bindings, so that drivers can know what ID to write to a hardware
> register when configuring a block that sends I2C messages autonomously.

> diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt

> +Optional properties:
> +- nvidia,controller-id: ID of controller when referred to in
> +                        hardware registers.

I'd prefer to put this information into the thermal trip node, since 
this represents what ID the PMC uses to communicate with the I2C 
controller, and there's no absolute guarantee that multiple clients that 
communicate directly with an I2C controller would use the same numbering 
scheme.

If that doesn't work, can be at least name this nvidia,pmc-controller-id 
or nvidia,id-in-pmc so that if there are different numbering schemes, 
there's a clear path to represent this in different properties without 
conflicting names?

[PATCH v2 2/5] of: Add nvidia,controller-id property to Tegra I2C bindings

[Stephen Warren]

On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
> Sometimes, hardware blocks want to issue requests to devices
> connected to I2C buses by itself. In such case, the bus the
> target device resides on must be configured into a register.
> For this purpose, each I2C controller has a defined ID known
> by the hardware. Add a property for these IDs to the device tree
> bindings, so that drivers can know what ID to write to a hardware
> register when configuring a block that sends I2C messages autonomously.

> diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt

> +Optional properties:
> +- nvidia,controller-id: ID of controller when referred to in
> +                        hardware registers.

I'd prefer to put this information into the thermal trip node, since 
this represents what ID the PMC uses to communicate with the I2C 
controller, and there's no absolute guarantee that multiple clients that 
communicate directly with an I2C controller would use the same numbering 
scheme.

If that doesn't work, can be at least name this nvidia,pmc-controller-id 
or nvidia,id-in-pmc so that if there are different numbering schemes, 
there's a clear path to represent this in different properties without 
conflicting names?

Re: [PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Stephen Warren]

On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
> Hardware-triggered thermal reset requires configuring the I2C
> reset procedure. This configuration is read from the device tree,
> so document the relevant properties in the binding documentation.

> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt

> +Optional properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):
> +- nvidia,pinmux-id : Pinmux used by the hardware when issuing poweroff command.
> +                     Defaults to 0.

We should either enumerate the legal values and what they mean, or 
include a reference to the TRM section which explains the legal values.

Re: [PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Stephen Warren]

On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
> Hardware-triggered thermal reset requires configuring the I2C
> reset procedure. This configuration is read from the device tree,
> so document the relevant properties in the binding documentation.

> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt

> +Optional properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):
> +- nvidia,pinmux-id : Pinmux used by the hardware when issuing poweroff command.
> +                     Defaults to 0.

We should either enumerate the legal values and what they mean, or 
include a reference to the TRM section which explains the legal values.

[PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Stephen Warren]

On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
> Hardware-triggered thermal reset requires configuring the I2C
> reset procedure. This configuration is read from the device tree,
> so document the relevant properties in the binding documentation.

> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt

> +Optional properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):
> +- nvidia,pinmux-id : Pinmux used by the hardware when issuing poweroff command.
> +                     Defaults to 0.

We should either enumerate the legal values and what they mean, or 
include a reference to the TRM section which explains the legal values.

Re: [PATCH v2 5/5] ARM: tegra: Add thermal reset (thermtrip) support to PMC

[Stephen Warren]

On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
> This adds a device tree controlled option to enable PMC-based
> thermal reset in overheating situations. Thermtrip is supported on
> Tegra30, Tegra114 and Tegra124. The thermal reset only works when
> the thermal sensors are calibrated, so a soctherm driver is also
> required.

If calibration is required, presumably the soctherm must initialize 
before this thermtrip code can initialize, or this thermtrip logic might 
be triggered by uncalibrated sensors?

If so, then there needs to be some explicit mechanism to force the two 
drivers into probing in the right order.

Re: [PATCH v2 5/5] ARM: tegra: Add thermal reset (thermtrip) support to PMC

[Stephen Warren]

On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
> This adds a device tree controlled option to enable PMC-based
> thermal reset in overheating situations. Thermtrip is supported on
> Tegra30, Tegra114 and Tegra124. The thermal reset only works when
> the thermal sensors are calibrated, so a soctherm driver is also
> required.

If calibration is required, presumably the soctherm must initialize 
before this thermtrip code can initialize, or this thermtrip logic might 
be triggered by uncalibrated sensors?

If so, then there needs to be some explicit mechanism to force the two 
drivers into probing in the right order.

[PATCH v2 5/5] ARM: tegra: Add thermal reset (thermtrip) support to PMC

[Stephen Warren]

On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
> This adds a device tree controlled option to enable PMC-based
> thermal reset in overheating situations. Thermtrip is supported on
> Tegra30, Tegra114 and Tegra124. The thermal reset only works when
> the thermal sensors are calibrated, so a soctherm driver is also
> required.

If calibration is required, presumably the soctherm must initialize 
before this thermtrip code can initialize, or this thermtrip logic might 
be triggered by uncalibrated sensors?

If so, then there needs to be some explicit mechanism to force the two 
drivers into probing in the right order.

Re: [PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2337 bytes --]

On Wed, Aug 20, 2014 at 02:16:49PM -0600, Stephen Warren wrote:
> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
> >Hardware-triggered thermal reset requires configuring the I2C
> >reset procedure. This configuration is read from the device tree,
> >so document the relevant properties in the binding documentation.
> 
> >diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
> 
> >+Hardware-triggered thermal reset:
> >+On Tegra30, Tegra114 and Tegra124, if the 'i2c-thermtrip' subnode exists,
> >+hardware-triggered thermal reset will be enabled.
> 
> "will be enabled" sounds like SW behaviour, whereas DT is suppose to
> describe HW, and leave SW to define its own behaviour. I would suggest:
> 
> Optional sub-nodes:
> i2c-thermtrip: Describes how to power off the system in the event of a
>   thermal emergency.
> 
> >+Required properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):
> 
> Simpler might be:
> 
> Required properties for i2c-thermtrip node:
> 
> >+- nvidia,pmu : Phandle to power management unit / PMIC handling poweroff
> >+- nvidia,reg-addr : I2C register address to write poweroff command to
> >+- nvidia,reg-data : Poweroff command to write to PMU
> 
> Why are both the PMU/PMIC phandle and the register address/data required? I
> thought the purpose of having the phandle was to allow the register address
> and data to be queried from the PMU/PMIC driver.
> 
> To me, it seems much simpler to get rid of the phandle and just hard-code
> the I2C bus number, address, and data into this node, rather than having to
> go query it from the PMU/PMIC driver, then find the I2C controller, then
> query it for its ID (and hope that all HW modules that talk to I2C
> controllers directly use the same numbering scheme...)

I originally requested this to be changed. It seems wrong to duplicate
information about the PMIC in both the PMIC device tree node and the
i2c-thermtrip node if we can get the same information from the driver
directly (via the phandle). It certainly requires a little more code,
but at the advantage of not having to figure out the I2C controller
hardware number and I2C slave addresses when writing the i2c-thermtrip
node.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 2337 bytes --]

On Wed, Aug 20, 2014 at 02:16:49PM -0600, Stephen Warren wrote:
> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
> >Hardware-triggered thermal reset requires configuring the I2C
> >reset procedure. This configuration is read from the device tree,
> >so document the relevant properties in the binding documentation.
> 
> >diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
> 
> >+Hardware-triggered thermal reset:
> >+On Tegra30, Tegra114 and Tegra124, if the 'i2c-thermtrip' subnode exists,
> >+hardware-triggered thermal reset will be enabled.
> 
> "will be enabled" sounds like SW behaviour, whereas DT is suppose to
> describe HW, and leave SW to define its own behaviour. I would suggest:
> 
> Optional sub-nodes:
> i2c-thermtrip: Describes how to power off the system in the event of a
>   thermal emergency.
> 
> >+Required properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):
> 
> Simpler might be:
> 
> Required properties for i2c-thermtrip node:
> 
> >+- nvidia,pmu : Phandle to power management unit / PMIC handling poweroff
> >+- nvidia,reg-addr : I2C register address to write poweroff command to
> >+- nvidia,reg-data : Poweroff command to write to PMU
> 
> Why are both the PMU/PMIC phandle and the register address/data required? I
> thought the purpose of having the phandle was to allow the register address
> and data to be queried from the PMU/PMIC driver.
> 
> To me, it seems much simpler to get rid of the phandle and just hard-code
> the I2C bus number, address, and data into this node, rather than having to
> go query it from the PMU/PMIC driver, then find the I2C controller, then
> query it for its ID (and hope that all HW modules that talk to I2C
> controllers directly use the same numbering scheme...)

I originally requested this to be changed. It seems wrong to duplicate
information about the PMIC in both the PMIC device tree node and the
i2c-thermtrip node if we can get the same information from the driver
directly (via the phandle). It certainly requires a little more code,
but at the advantage of not having to figure out the I2C controller
hardware number and I2C slave addresses when writing the i2c-thermtrip
node.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Thierry Reding]

On Wed, Aug 20, 2014 at 02:16:49PM -0600, Stephen Warren wrote:
> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
> >Hardware-triggered thermal reset requires configuring the I2C
> >reset procedure. This configuration is read from the device tree,
> >so document the relevant properties in the binding documentation.
> 
> >diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
> 
> >+Hardware-triggered thermal reset:
> >+On Tegra30, Tegra114 and Tegra124, if the 'i2c-thermtrip' subnode exists,
> >+hardware-triggered thermal reset will be enabled.
> 
> "will be enabled" sounds like SW behaviour, whereas DT is suppose to
> describe HW, and leave SW to define its own behaviour. I would suggest:
> 
> Optional sub-nodes:
> i2c-thermtrip: Describes how to power off the system in the event of a
>   thermal emergency.
> 
> >+Required properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):
> 
> Simpler might be:
> 
> Required properties for i2c-thermtrip node:
> 
> >+- nvidia,pmu : Phandle to power management unit / PMIC handling poweroff
> >+- nvidia,reg-addr : I2C register address to write poweroff command to
> >+- nvidia,reg-data : Poweroff command to write to PMU
> 
> Why are both the PMU/PMIC phandle and the register address/data required? I
> thought the purpose of having the phandle was to allow the register address
> and data to be queried from the PMU/PMIC driver.
> 
> To me, it seems much simpler to get rid of the phandle and just hard-code
> the I2C bus number, address, and data into this node, rather than having to
> go query it from the PMU/PMIC driver, then find the I2C controller, then
> query it for its ID (and hope that all HW modules that talk to I2C
> controllers directly use the same numbering scheme...)

I originally requested this to be changed. It seems wrong to duplicate
information about the PMIC in both the PMIC device tree node and the
i2c-thermtrip node if we can get the same information from the driver
directly (via the phandle). It certainly requires a little more code,
but at the advantage of not having to figure out the I2C controller
hardware number and I2C slave addresses when writing the i2c-thermtrip
node.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140821/173ffb99/attachment.sig>

Re: [PATCH v2 2/5] of: Add nvidia,controller-id property to Tegra I2C bindings

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1973 bytes --]

On Wed, Aug 20, 2014 at 02:19:35PM -0600, Stephen Warren wrote:
> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
> >Sometimes, hardware blocks want to issue requests to devices
> >connected to I2C buses by itself. In such case, the bus the
> >target device resides on must be configured into a register.
> >For this purpose, each I2C controller has a defined ID known
> >by the hardware. Add a property for these IDs to the device tree
> >bindings, so that drivers can know what ID to write to a hardware
> >register when configuring a block that sends I2C messages autonomously.
> 
> >diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt
> 
> >+Optional properties:
> >+- nvidia,controller-id: ID of controller when referred to in
> >+                        hardware registers.
> 
> I'd prefer to put this information into the thermal trip node, since this
> represents what ID the PMC uses to communicate with the I2C controller, and
> there's no absolute guarantee that multiple clients that communicate
> directly with an I2C controller would use the same numbering scheme.
> 
> If that doesn't work, can be at least name this nvidia,pmc-controller-id or
> nvidia,id-in-pmc so that if there are different numbering schemes, there's a
> clear path to represent this in different properties without conflicting
> names?

This is the ID of the controller used internally by the documentation.
And as far as I can tell every aspect of the documentation refers to the
controllers by the same ID (clocks, interrupts, ...). The PMC uses this
same numbering scheme. That makes the ID about as canonical as it gets,
so the extra prefix isn't warranted in my opinion.

I'd argue that if ever there was a case where something was referring to
the controller using a different ID then that should be considered the
oddball and get special treatment.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 2/5] of: Add nvidia,controller-id property to Tegra I2C bindings

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1973 bytes --]

On Wed, Aug 20, 2014 at 02:19:35PM -0600, Stephen Warren wrote:
> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
> >Sometimes, hardware blocks want to issue requests to devices
> >connected to I2C buses by itself. In such case, the bus the
> >target device resides on must be configured into a register.
> >For this purpose, each I2C controller has a defined ID known
> >by the hardware. Add a property for these IDs to the device tree
> >bindings, so that drivers can know what ID to write to a hardware
> >register when configuring a block that sends I2C messages autonomously.
> 
> >diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt
> 
> >+Optional properties:
> >+- nvidia,controller-id: ID of controller when referred to in
> >+                        hardware registers.
> 
> I'd prefer to put this information into the thermal trip node, since this
> represents what ID the PMC uses to communicate with the I2C controller, and
> there's no absolute guarantee that multiple clients that communicate
> directly with an I2C controller would use the same numbering scheme.
> 
> If that doesn't work, can be at least name this nvidia,pmc-controller-id or
> nvidia,id-in-pmc so that if there are different numbering schemes, there's a
> clear path to represent this in different properties without conflicting
> names?

This is the ID of the controller used internally by the documentation.
And as far as I can tell every aspect of the documentation refers to the
controllers by the same ID (clocks, interrupts, ...). The PMC uses this
same numbering scheme. That makes the ID about as canonical as it gets,
so the extra prefix isn't warranted in my opinion.

I'd argue that if ever there was a case where something was referring to
the controller using a different ID then that should be considered the
oddball and get special treatment.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v2 2/5] of: Add nvidia,controller-id property to Tegra I2C bindings

[Thierry Reding]

On Wed, Aug 20, 2014 at 02:19:35PM -0600, Stephen Warren wrote:
> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
> >Sometimes, hardware blocks want to issue requests to devices
> >connected to I2C buses by itself. In such case, the bus the
> >target device resides on must be configured into a register.
> >For this purpose, each I2C controller has a defined ID known
> >by the hardware. Add a property for these IDs to the device tree
> >bindings, so that drivers can know what ID to write to a hardware
> >register when configuring a block that sends I2C messages autonomously.
> 
> >diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt
> 
> >+Optional properties:
> >+- nvidia,controller-id: ID of controller when referred to in
> >+                        hardware registers.
> 
> I'd prefer to put this information into the thermal trip node, since this
> represents what ID the PMC uses to communicate with the I2C controller, and
> there's no absolute guarantee that multiple clients that communicate
> directly with an I2C controller would use the same numbering scheme.
> 
> If that doesn't work, can be at least name this nvidia,pmc-controller-id or
> nvidia,id-in-pmc so that if there are different numbering schemes, there's a
> clear path to represent this in different properties without conflicting
> names?

This is the ID of the controller used internally by the documentation.
And as far as I can tell every aspect of the documentation refers to the
controllers by the same ID (clocks, interrupts, ...). The PMC uses this
same numbering scheme. That makes the ID about as canonical as it gets,
so the extra prefix isn't warranted in my opinion.

I'd argue that if ever there was a case where something was referring to
the controller using a different ID then that should be considered the
oddball and get special treatment.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140821/71c5f115/attachment.sig>

Re: [PATCH v2 5/5] ARM: tegra: Add thermal reset (thermtrip) support to PMC

[Mikko Perttunen]

On 20/08/14 23:25, Stephen Warren wrote:
> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>> This adds a device tree controlled option to enable PMC-based
>> thermal reset in overheating situations. Thermtrip is supported on
>> Tegra30, Tegra114 and Tegra124. The thermal reset only works when
>> the thermal sensors are calibrated, so a soctherm driver is also
>> required.
>
> If calibration is required, presumably the soctherm must initialize
> before this thermtrip code can initialize, or this thermtrip logic might
> be triggered by uncalibrated sensors?

SOCTHERM requires that each sensor be explicitly enabled before it gives 
readings. If a sensor is not enabled, the temperature given by the 
register (and used to trigger thermtrip) will be zero. So in order for a 
thermtrip shutdown to be caused before soctherm is initialized, the 
thermtrip temperature would have to be programmed to below zero (the 
default value is 105C), in which case an immediate shutdown would 
probably be in order anyway (unless the user uses LN2 or something to 
cool the soc below zero).

>
> If so, then there needs to be some explicit mechanism to force the two
> drivers into probing in the right order.

Because of the above, I think it isn't necessary to probe these in order.

Cheers,
Mikko

Re: [PATCH v2 5/5] ARM: tegra: Add thermal reset (thermtrip) support to PMC

[Mikko Perttunen]

On 20/08/14 23:25, Stephen Warren wrote:
> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>> This adds a device tree controlled option to enable PMC-based
>> thermal reset in overheating situations. Thermtrip is supported on
>> Tegra30, Tegra114 and Tegra124. The thermal reset only works when
>> the thermal sensors are calibrated, so a soctherm driver is also
>> required.
>
> If calibration is required, presumably the soctherm must initialize
> before this thermtrip code can initialize, or this thermtrip logic might
> be triggered by uncalibrated sensors?

SOCTHERM requires that each sensor be explicitly enabled before it gives 
readings. If a sensor is not enabled, the temperature given by the 
register (and used to trigger thermtrip) will be zero. So in order for a 
thermtrip shutdown to be caused before soctherm is initialized, the 
thermtrip temperature would have to be programmed to below zero (the 
default value is 105C), in which case an immediate shutdown would 
probably be in order anyway (unless the user uses LN2 or something to 
cool the soc below zero).

>
> If so, then there needs to be some explicit mechanism to force the two
> drivers into probing in the right order.

Because of the above, I think it isn't necessary to probe these in order.

Cheers,
Mikko

[PATCH v2 5/5] ARM: tegra: Add thermal reset (thermtrip) support to PMC

[Mikko Perttunen]

On 20/08/14 23:25, Stephen Warren wrote:
> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>> This adds a device tree controlled option to enable PMC-based
>> thermal reset in overheating situations. Thermtrip is supported on
>> Tegra30, Tegra114 and Tegra124. The thermal reset only works when
>> the thermal sensors are calibrated, so a soctherm driver is also
>> required.
>
> If calibration is required, presumably the soctherm must initialize
> before this thermtrip code can initialize, or this thermtrip logic might
> be triggered by uncalibrated sensors?

SOCTHERM requires that each sensor be explicitly enabled before it gives 
readings. If a sensor is not enabled, the temperature given by the 
register (and used to trigger thermtrip) will be zero. So in order for a 
thermtrip shutdown to be caused before soctherm is initialized, the 
thermtrip temperature would have to be programmed to below zero (the 
default value is 105C), in which case an immediate shutdown would 
probably be in order anyway (unless the user uses LN2 or something to 
cool the soc below zero).

>
> If so, then there needs to be some explicit mechanism to force the two
> drivers into probing in the right order.

Because of the above, I think it isn't necessary to probe these in order.

Cheers,
Mikko

Re: [PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Stephen Warren]

On 08/21/2014 12:58 AM, Thierry Reding wrote:
> On Wed, Aug 20, 2014 at 02:16:49PM -0600, Stephen Warren wrote:
>> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>>> Hardware-triggered thermal reset requires configuring the I2C
>>> reset procedure. This configuration is read from the device tree,
>>> so document the relevant properties in the binding documentation.
>>
>>> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>
>>> +Hardware-triggered thermal reset:
>>> +On Tegra30, Tegra114 and Tegra124, if the 'i2c-thermtrip' subnode exists,
>>> +hardware-triggered thermal reset will be enabled.
>>
>> "will be enabled" sounds like SW behaviour, whereas DT is suppose to
>> describe HW, and leave SW to define its own behaviour. I would suggest:
>>
>> Optional sub-nodes:
>> i2c-thermtrip: Describes how to power off the system in the event of a
>>    thermal emergency.
>>
>>> +Required properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):
>>
>> Simpler might be:
>>
>> Required properties for i2c-thermtrip node:
>>
>>> +- nvidia,pmu : Phandle to power management unit / PMIC handling poweroff
>>> +- nvidia,reg-addr : I2C register address to write poweroff command to
>>> +- nvidia,reg-data : Poweroff command to write to PMU
>>
>> Why are both the PMU/PMIC phandle and the register address/data required? I
>> thought the purpose of having the phandle was to allow the register address
>> and data to be queried from the PMU/PMIC driver.
>>
>> To me, it seems much simpler to get rid of the phandle and just hard-code
>> the I2C bus number, address, and data into this node, rather than having to
>> go query it from the PMU/PMIC driver, then find the I2C controller, then
>> query it for its ID (and hope that all HW modules that talk to I2C
>> controllers directly use the same numbering scheme...)
>
> I originally requested this to be changed. It seems wrong to duplicate
> information about the PMIC in both the PMIC device tree node and the
> i2c-thermtrip node if we can get the same information from the driver
> directly (via the phandle). It certainly requires a little more code,
> but at the advantage of not having to figure out the I2C controller
> hardware number and I2C slave addresses when writing the i2c-thermtrip
> node.

I cant see that argument, but surely the PMIC driver can also supply the 
"reg-addr" and "reg-data" values too, if it's already being queried for 
the I2C device address and bus number? The binding above appears to 
duplicate part of the information, while requiring querying of the other 
part.

Re: [PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Stephen Warren]

On 08/21/2014 12:58 AM, Thierry Reding wrote:
> On Wed, Aug 20, 2014 at 02:16:49PM -0600, Stephen Warren wrote:
>> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>>> Hardware-triggered thermal reset requires configuring the I2C
>>> reset procedure. This configuration is read from the device tree,
>>> so document the relevant properties in the binding documentation.
>>
>>> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>
>>> +Hardware-triggered thermal reset:
>>> +On Tegra30, Tegra114 and Tegra124, if the 'i2c-thermtrip' subnode exists,
>>> +hardware-triggered thermal reset will be enabled.
>>
>> "will be enabled" sounds like SW behaviour, whereas DT is suppose to
>> describe HW, and leave SW to define its own behaviour. I would suggest:
>>
>> Optional sub-nodes:
>> i2c-thermtrip: Describes how to power off the system in the event of a
>>    thermal emergency.
>>
>>> +Required properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):
>>
>> Simpler might be:
>>
>> Required properties for i2c-thermtrip node:
>>
>>> +- nvidia,pmu : Phandle to power management unit / PMIC handling poweroff
>>> +- nvidia,reg-addr : I2C register address to write poweroff command to
>>> +- nvidia,reg-data : Poweroff command to write to PMU
>>
>> Why are both the PMU/PMIC phandle and the register address/data required? I
>> thought the purpose of having the phandle was to allow the register address
>> and data to be queried from the PMU/PMIC driver.
>>
>> To me, it seems much simpler to get rid of the phandle and just hard-code
>> the I2C bus number, address, and data into this node, rather than having to
>> go query it from the PMU/PMIC driver, then find the I2C controller, then
>> query it for its ID (and hope that all HW modules that talk to I2C
>> controllers directly use the same numbering scheme...)
>
> I originally requested this to be changed. It seems wrong to duplicate
> information about the PMIC in both the PMIC device tree node and the
> i2c-thermtrip node if we can get the same information from the driver
> directly (via the phandle). It certainly requires a little more code,
> but at the advantage of not having to figure out the I2C controller
> hardware number and I2C slave addresses when writing the i2c-thermtrip
> node.

I cant see that argument, but surely the PMIC driver can also supply the 
"reg-addr" and "reg-data" values too, if it's already being queried for 
the I2C device address and bus number? The binding above appears to 
duplicate part of the information, while requiring querying of the other 
part.

[PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Stephen Warren]

On 08/21/2014 12:58 AM, Thierry Reding wrote:
> On Wed, Aug 20, 2014 at 02:16:49PM -0600, Stephen Warren wrote:
>> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>>> Hardware-triggered thermal reset requires configuring the I2C
>>> reset procedure. This configuration is read from the device tree,
>>> so document the relevant properties in the binding documentation.
>>
>>> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>
>>> +Hardware-triggered thermal reset:
>>> +On Tegra30, Tegra114 and Tegra124, if the 'i2c-thermtrip' subnode exists,
>>> +hardware-triggered thermal reset will be enabled.
>>
>> "will be enabled" sounds like SW behaviour, whereas DT is suppose to
>> describe HW, and leave SW to define its own behaviour. I would suggest:
>>
>> Optional sub-nodes:
>> i2c-thermtrip: Describes how to power off the system in the event of a
>>    thermal emergency.
>>
>>> +Required properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):
>>
>> Simpler might be:
>>
>> Required properties for i2c-thermtrip node:
>>
>>> +- nvidia,pmu : Phandle to power management unit / PMIC handling poweroff
>>> +- nvidia,reg-addr : I2C register address to write poweroff command to
>>> +- nvidia,reg-data : Poweroff command to write to PMU
>>
>> Why are both the PMU/PMIC phandle and the register address/data required? I
>> thought the purpose of having the phandle was to allow the register address
>> and data to be queried from the PMU/PMIC driver.
>>
>> To me, it seems much simpler to get rid of the phandle and just hard-code
>> the I2C bus number, address, and data into this node, rather than having to
>> go query it from the PMU/PMIC driver, then find the I2C controller, then
>> query it for its ID (and hope that all HW modules that talk to I2C
>> controllers directly use the same numbering scheme...)
>
> I originally requested this to be changed. It seems wrong to duplicate
> information about the PMIC in both the PMIC device tree node and the
> i2c-thermtrip node if we can get the same information from the driver
> directly (via the phandle). It certainly requires a little more code,
> but at the advantage of not having to figure out the I2C controller
> hardware number and I2C slave addresses when writing the i2c-thermtrip
> node.

I cant see that argument, but surely the PMIC driver can also supply the 
"reg-addr" and "reg-data" values too, if it's already being queried for 
the I2C device address and bus number? The binding above appears to 
duplicate part of the information, while requiring querying of the other 
part.

Re: [PATCH v2 5/5] ARM: tegra: Add thermal reset (thermtrip) support to PMC

[Stephen Warren]

On 08/21/2014 07:11 AM, Mikko Perttunen wrote:
> On 20/08/14 23:25, Stephen Warren wrote:
>> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>>> This adds a device tree controlled option to enable PMC-based
>>> thermal reset in overheating situations. Thermtrip is supported on
>>> Tegra30, Tegra114 and Tegra124. The thermal reset only works when
>>> the thermal sensors are calibrated, so a soctherm driver is also
>>> required.
>>
>> If calibration is required, presumably the soctherm must initialize
>> before this thermtrip code can initialize, or this thermtrip logic might
>> be triggered by uncalibrated sensors?
>
> SOCTHERM requires that each sensor be explicitly enabled before it gives
> readings. If a sensor is not enabled, the temperature given by the
> register (and used to trigger thermtrip) will be zero. So in order for a
> thermtrip shutdown to be caused before soctherm is initialized, the
> thermtrip temperature would have to be programmed to below zero (the
> default value is 105C), in which case an immediate shutdown would
> probably be in order anyway (unless the user uses LN2 or something to
> cool the soc below zero).
>
>>
>> If so, then there needs to be some explicit mechanism to force the two
>> drivers into probing in the right order.
>
> Because of the above, I think it isn't necessary to probe these in order.

OK, that makes sense. Briefly mentioning this in the commit description 
could be useful.

Re: [PATCH v2 5/5] ARM: tegra: Add thermal reset (thermtrip) support to PMC

[Stephen Warren]

On 08/21/2014 07:11 AM, Mikko Perttunen wrote:
> On 20/08/14 23:25, Stephen Warren wrote:
>> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>>> This adds a device tree controlled option to enable PMC-based
>>> thermal reset in overheating situations. Thermtrip is supported on
>>> Tegra30, Tegra114 and Tegra124. The thermal reset only works when
>>> the thermal sensors are calibrated, so a soctherm driver is also
>>> required.
>>
>> If calibration is required, presumably the soctherm must initialize
>> before this thermtrip code can initialize, or this thermtrip logic might
>> be triggered by uncalibrated sensors?
>
> SOCTHERM requires that each sensor be explicitly enabled before it gives
> readings. If a sensor is not enabled, the temperature given by the
> register (and used to trigger thermtrip) will be zero. So in order for a
> thermtrip shutdown to be caused before soctherm is initialized, the
> thermtrip temperature would have to be programmed to below zero (the
> default value is 105C), in which case an immediate shutdown would
> probably be in order anyway (unless the user uses LN2 or something to
> cool the soc below zero).
>
>>
>> If so, then there needs to be some explicit mechanism to force the two
>> drivers into probing in the right order.
>
> Because of the above, I think it isn't necessary to probe these in order.

OK, that makes sense. Briefly mentioning this in the commit description 
could be useful.

[PATCH v2 5/5] ARM: tegra: Add thermal reset (thermtrip) support to PMC

[Stephen Warren]

On 08/21/2014 07:11 AM, Mikko Perttunen wrote:
> On 20/08/14 23:25, Stephen Warren wrote:
>> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>>> This adds a device tree controlled option to enable PMC-based
>>> thermal reset in overheating situations. Thermtrip is supported on
>>> Tegra30, Tegra114 and Tegra124. The thermal reset only works when
>>> the thermal sensors are calibrated, so a soctherm driver is also
>>> required.
>>
>> If calibration is required, presumably the soctherm must initialize
>> before this thermtrip code can initialize, or this thermtrip logic might
>> be triggered by uncalibrated sensors?
>
> SOCTHERM requires that each sensor be explicitly enabled before it gives
> readings. If a sensor is not enabled, the temperature given by the
> register (and used to trigger thermtrip) will be zero. So in order for a
> thermtrip shutdown to be caused before soctherm is initialized, the
> thermtrip temperature would have to be programmed to below zero (the
> default value is 105C), in which case an immediate shutdown would
> probably be in order anyway (unless the user uses LN2 or something to
> cool the soc below zero).
>
>>
>> If so, then there needs to be some explicit mechanism to force the two
>> drivers into probing in the right order.
>
> Because of the above, I think it isn't necessary to probe these in order.

OK, that makes sense. Briefly mentioning this in the commit description 
could be useful.

Re: [PATCH v2 2/5] of: Add nvidia,controller-id property to Tegra I2C bindings

[Stephen Warren]

On 08/21/2014 01:05 AM, Thierry Reding wrote:
> On Wed, Aug 20, 2014 at 02:19:35PM -0600, Stephen Warren wrote:
>> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>>> Sometimes, hardware blocks want to issue requests to devices
>>> connected to I2C buses by itself. In such case, the bus the
>>> target device resides on must be configured into a register.
>>> For this purpose, each I2C controller has a defined ID known
>>> by the hardware. Add a property for these IDs to the device tree
>>> bindings, so that drivers can know what ID to write to a hardware
>>> register when configuring a block that sends I2C messages autonomously.
>>
>>> diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt
>>
>>> +Optional properties:
>>> +- nvidia,controller-id: ID of controller when referred to in
>>> +                        hardware registers.
>>
>> I'd prefer to put this information into the thermal trip node, since this
>> represents what ID the PMC uses to communicate with the I2C controller, and
>> there's no absolute guarantee that multiple clients that communicate
>> directly with an I2C controller would use the same numbering scheme.
>>
>> If that doesn't work, can be at least name this nvidia,pmc-controller-id or
>> nvidia,id-in-pmc so that if there are different numbering schemes, there's a
>> clear path to represent this in different properties without conflicting
>> names?
>
> This is the ID of the controller used internally by the documentation.
> And as far as I can tell every aspect of the documentation refers to the
> controllers by the same ID (clocks, interrupts, ...). The PMC uses this
> same numbering scheme. That makes the ID about as canonical as it gets,
> so the extra prefix isn't warranted in my opinion.
>
> I'd argue that if ever there was a case where something was referring to
> the controller using a different ID then that should be considered the
> oddball and get special treatment.

This is certainly true in practice in current SoCs, but in general, 
there's no reason why every HW module that talks to I2C HW modules needs 
to be hooked up to all of them in the same order, especially if SoCs 
start getting put together by mixing/matching components from all kinds 
of vendors etc.

Still, since they are today, I suppose that binding is fine; we can 
always modify the binding for any new SoC (and associated compatible 
value) in the future if the current rule ever gets broken. The code will 
be a little annoying though.

[PATCH v2 2/5] of: Add nvidia,controller-id property to Tegra I2C bindings

[Stephen Warren]

On 08/21/2014 01:05 AM, Thierry Reding wrote:
> On Wed, Aug 20, 2014 at 02:19:35PM -0600, Stephen Warren wrote:
>> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>>> Sometimes, hardware blocks want to issue requests to devices
>>> connected to I2C buses by itself. In such case, the bus the
>>> target device resides on must be configured into a register.
>>> For this purpose, each I2C controller has a defined ID known
>>> by the hardware. Add a property for these IDs to the device tree
>>> bindings, so that drivers can know what ID to write to a hardware
>>> register when configuring a block that sends I2C messages autonomously.
>>
>>> diff --git a/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt b/Documentation/devicetree/bindings/i2c/nvidia,tegra20-i2c.txt
>>
>>> +Optional properties:
>>> +- nvidia,controller-id: ID of controller when referred to in
>>> +                        hardware registers.
>>
>> I'd prefer to put this information into the thermal trip node, since this
>> represents what ID the PMC uses to communicate with the I2C controller, and
>> there's no absolute guarantee that multiple clients that communicate
>> directly with an I2C controller would use the same numbering scheme.
>>
>> If that doesn't work, can be at least name this nvidia,pmc-controller-id or
>> nvidia,id-in-pmc so that if there are different numbering schemes, there's a
>> clear path to represent this in different properties without conflicting
>> names?
>
> This is the ID of the controller used internally by the documentation.
> And as far as I can tell every aspect of the documentation refers to the
> controllers by the same ID (clocks, interrupts, ...). The PMC uses this
> same numbering scheme. That makes the ID about as canonical as it gets,
> so the extra prefix isn't warranted in my opinion.
>
> I'd argue that if ever there was a case where something was referring to
> the controller using a different ID then that should be considered the
> oddball and get special treatment.

This is certainly true in practice in current SoCs, but in general, 
there's no reason why every HW module that talks to I2C HW modules needs 
to be hooked up to all of them in the same order, especially if SoCs 
start getting put together by mixing/matching components from all kinds 
of vendors etc.

Still, since they are today, I suppose that binding is fine; we can 
always modify the binding for any new SoC (and associated compatible 
value) in the future if the current rule ever gets broken. The code will 
be a little annoying though.

Re: [PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3268 bytes --]

On Thu, Aug 21, 2014 at 09:38:29AM -0600, Stephen Warren wrote:
> On 08/21/2014 12:58 AM, Thierry Reding wrote:
> >On Wed, Aug 20, 2014 at 02:16:49PM -0600, Stephen Warren wrote:
> >>On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
> >>>Hardware-triggered thermal reset requires configuring the I2C
> >>>reset procedure. This configuration is read from the device tree,
> >>>so document the relevant properties in the binding documentation.
> >>
> >>>diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
> >>
> >>>+Hardware-triggered thermal reset:
> >>>+On Tegra30, Tegra114 and Tegra124, if the 'i2c-thermtrip' subnode exists,
> >>>+hardware-triggered thermal reset will be enabled.
> >>
> >>"will be enabled" sounds like SW behaviour, whereas DT is suppose to
> >>describe HW, and leave SW to define its own behaviour. I would suggest:
> >>
> >>Optional sub-nodes:
> >>i2c-thermtrip: Describes how to power off the system in the event of a
> >>   thermal emergency.
> >>
> >>>+Required properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):
> >>
> >>Simpler might be:
> >>
> >>Required properties for i2c-thermtrip node:
> >>
> >>>+- nvidia,pmu : Phandle to power management unit / PMIC handling poweroff
> >>>+- nvidia,reg-addr : I2C register address to write poweroff command to
> >>>+- nvidia,reg-data : Poweroff command to write to PMU
> >>
> >>Why are both the PMU/PMIC phandle and the register address/data required? I
> >>thought the purpose of having the phandle was to allow the register address
> >>and data to be queried from the PMU/PMIC driver.
> >>
> >>To me, it seems much simpler to get rid of the phandle and just hard-code
> >>the I2C bus number, address, and data into this node, rather than having to
> >>go query it from the PMU/PMIC driver, then find the I2C controller, then
> >>query it for its ID (and hope that all HW modules that talk to I2C
> >>controllers directly use the same numbering scheme...)
> >
> >I originally requested this to be changed. It seems wrong to duplicate
> >information about the PMIC in both the PMIC device tree node and the
> >i2c-thermtrip node if we can get the same information from the driver
> >directly (via the phandle). It certainly requires a little more code,
> >but at the advantage of not having to figure out the I2C controller
> >hardware number and I2C slave addresses when writing the i2c-thermtrip
> >node.
> 
> I cant see that argument, but surely the PMIC driver can also supply the
> "reg-addr" and "reg-data" values too, if it's already being queried for the
> I2C device address and bus number? The binding above appears to duplicate
> part of the information, while requiring querying of the other part.

I suppose that could be done. It would take a new function to do that,
though, since I'm not aware of a generic mechanism to query this kind of
information from a PMIC (there's no generic PMIC API, either, so perhaps
it would be a good time to create one?). The I2C controller and I2C
slave are generic I2C properties, whereas the register and data to power
off the device are very device specific.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 3268 bytes --]

On Thu, Aug 21, 2014 at 09:38:29AM -0600, Stephen Warren wrote:
> On 08/21/2014 12:58 AM, Thierry Reding wrote:
> >On Wed, Aug 20, 2014 at 02:16:49PM -0600, Stephen Warren wrote:
> >>On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
> >>>Hardware-triggered thermal reset requires configuring the I2C
> >>>reset procedure. This configuration is read from the device tree,
> >>>so document the relevant properties in the binding documentation.
> >>
> >>>diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
> >>
> >>>+Hardware-triggered thermal reset:
> >>>+On Tegra30, Tegra114 and Tegra124, if the 'i2c-thermtrip' subnode exists,
> >>>+hardware-triggered thermal reset will be enabled.
> >>
> >>"will be enabled" sounds like SW behaviour, whereas DT is suppose to
> >>describe HW, and leave SW to define its own behaviour. I would suggest:
> >>
> >>Optional sub-nodes:
> >>i2c-thermtrip: Describes how to power off the system in the event of a
> >>   thermal emergency.
> >>
> >>>+Required properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):
> >>
> >>Simpler might be:
> >>
> >>Required properties for i2c-thermtrip node:
> >>
> >>>+- nvidia,pmu : Phandle to power management unit / PMIC handling poweroff
> >>>+- nvidia,reg-addr : I2C register address to write poweroff command to
> >>>+- nvidia,reg-data : Poweroff command to write to PMU
> >>
> >>Why are both the PMU/PMIC phandle and the register address/data required? I
> >>thought the purpose of having the phandle was to allow the register address
> >>and data to be queried from the PMU/PMIC driver.
> >>
> >>To me, it seems much simpler to get rid of the phandle and just hard-code
> >>the I2C bus number, address, and data into this node, rather than having to
> >>go query it from the PMU/PMIC driver, then find the I2C controller, then
> >>query it for its ID (and hope that all HW modules that talk to I2C
> >>controllers directly use the same numbering scheme...)
> >
> >I originally requested this to be changed. It seems wrong to duplicate
> >information about the PMIC in both the PMIC device tree node and the
> >i2c-thermtrip node if we can get the same information from the driver
> >directly (via the phandle). It certainly requires a little more code,
> >but at the advantage of not having to figure out the I2C controller
> >hardware number and I2C slave addresses when writing the i2c-thermtrip
> >node.
> 
> I cant see that argument, but surely the PMIC driver can also supply the
> "reg-addr" and "reg-data" values too, if it's already being queried for the
> I2C device address and bus number? The binding above appears to duplicate
> part of the information, while requiring querying of the other part.

I suppose that could be done. It would take a new function to do that,
though, since I'm not aware of a generic mechanism to query this kind of
information from a PMIC (there's no generic PMIC API, either, so perhaps
it would be a good time to create one?). The I2C controller and I2C
slave are generic I2C properties, whereas the register and data to power
off the device are very device specific.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

[PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Thierry Reding]

On Thu, Aug 21, 2014 at 09:38:29AM -0600, Stephen Warren wrote:
> On 08/21/2014 12:58 AM, Thierry Reding wrote:
> >On Wed, Aug 20, 2014 at 02:16:49PM -0600, Stephen Warren wrote:
> >>On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
> >>>Hardware-triggered thermal reset requires configuring the I2C
> >>>reset procedure. This configuration is read from the device tree,
> >>>so document the relevant properties in the binding documentation.
> >>
> >>>diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
> >>
> >>>+Hardware-triggered thermal reset:
> >>>+On Tegra30, Tegra114 and Tegra124, if the 'i2c-thermtrip' subnode exists,
> >>>+hardware-triggered thermal reset will be enabled.
> >>
> >>"will be enabled" sounds like SW behaviour, whereas DT is suppose to
> >>describe HW, and leave SW to define its own behaviour. I would suggest:
> >>
> >>Optional sub-nodes:
> >>i2c-thermtrip: Describes how to power off the system in the event of a
> >>   thermal emergency.
> >>
> >>>+Required properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):
> >>
> >>Simpler might be:
> >>
> >>Required properties for i2c-thermtrip node:
> >>
> >>>+- nvidia,pmu : Phandle to power management unit / PMIC handling poweroff
> >>>+- nvidia,reg-addr : I2C register address to write poweroff command to
> >>>+- nvidia,reg-data : Poweroff command to write to PMU
> >>
> >>Why are both the PMU/PMIC phandle and the register address/data required? I
> >>thought the purpose of having the phandle was to allow the register address
> >>and data to be queried from the PMU/PMIC driver.
> >>
> >>To me, it seems much simpler to get rid of the phandle and just hard-code
> >>the I2C bus number, address, and data into this node, rather than having to
> >>go query it from the PMU/PMIC driver, then find the I2C controller, then
> >>query it for its ID (and hope that all HW modules that talk to I2C
> >>controllers directly use the same numbering scheme...)
> >
> >I originally requested this to be changed. It seems wrong to duplicate
> >information about the PMIC in both the PMIC device tree node and the
> >i2c-thermtrip node if we can get the same information from the driver
> >directly (via the phandle). It certainly requires a little more code,
> >but at the advantage of not having to figure out the I2C controller
> >hardware number and I2C slave addresses when writing the i2c-thermtrip
> >node.
> 
> I cant see that argument, but surely the PMIC driver can also supply the
> "reg-addr" and "reg-data" values too, if it's already being queried for the
> I2C device address and bus number? The binding above appears to duplicate
> part of the information, while requiring querying of the other part.

I suppose that could be done. It would take a new function to do that,
though, since I'm not aware of a generic mechanism to query this kind of
information from a PMIC (there's no generic PMIC API, either, so perhaps
it would be a good time to create one?). The I2C controller and I2C
slave are generic I2C properties, whereas the register and data to power
off the device are very device specific.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140821/d90b31f9/attachment.sig>

Re: [PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Stephen Warren]

On 08/21/2014 10:53 AM, Thierry Reding wrote:
> On Thu, Aug 21, 2014 at 09:38:29AM -0600, Stephen Warren wrote:
>> On 08/21/2014 12:58 AM, Thierry Reding wrote:
>>> On Wed, Aug 20, 2014 at 02:16:49PM -0600, Stephen Warren wrote:
>>>> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>>>>> Hardware-triggered thermal reset requires configuring the I2C
>>>>> reset procedure. This configuration is read from the device tree,
>>>>> so document the relevant properties in the binding documentation.
>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt

>>>>> +- nvidia,pmu : Phandle to power management unit / PMIC handling poweroff
>>>>> +- nvidia,reg-addr : I2C register address to write poweroff command to
>>>>> +- nvidia,reg-data : Poweroff command to write to PMU
>>>>
>>>> Why are both the PMU/PMIC phandle and the register address/data required? I
>>>> thought the purpose of having the phandle was to allow the register address
>>>> and data to be queried from the PMU/PMIC driver.
>>>>
>>>> To me, it seems much simpler to get rid of the phandle and just hard-code
>>>> the I2C bus number, address, and data into this node, rather than having to
>>>> go query it from the PMU/PMIC driver, then find the I2C controller, then
>>>> query it for its ID (and hope that all HW modules that talk to I2C
>>>> controllers directly use the same numbering scheme...)
>>>
>>> I originally requested this to be changed. It seems wrong to duplicate
>>> information about the PMIC in both the PMIC device tree node and the
>>> i2c-thermtrip node if we can get the same information from the driver
>>> directly (via the phandle). It certainly requires a little more code,
>>> but at the advantage of not having to figure out the I2C controller
>>> hardware number and I2C slave addresses when writing the i2c-thermtrip
>>> node.
>>
>> I cant see that argument, but surely the PMIC driver can also supply the

Oops, that was meant to be can not cant.

>> "reg-addr" and "reg-data" values too, if it's already being queried for the
>> I2C device address and bus number? The binding above appears to duplicate
>> part of the information, while requiring querying of the other part.
>
> I suppose that could be done. It would take a new function to do that,
> though, since I'm not aware of a generic mechanism to query this kind of
> information from a PMIC (there's no generic PMIC API, either, so perhaps
> it would be a good time to create one?). The I2C controller and I2C
> slave are generic I2C properties, whereas the register and data to power
> off the device are very device specific.

I don't think it's possible to generically query an I2C device for its 
address from the struct i2c_device object; the code still needs to call 
a function in the PMIC driver to obtain this.

That's because some I2C devices respond to multiple I2C addresses, and 
there's no guarantee that the one I2C address in the DT (and hence the 
struct i2c_device) is the address upon which the regulator function exists.

grep for i2c_new_dummy in drivers/mfd and you'll find quite a few 
examples. The Atmel MXT touchpad/screen is another example.

Re: [PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Stephen Warren]

On 08/21/2014 10:53 AM, Thierry Reding wrote:
> On Thu, Aug 21, 2014 at 09:38:29AM -0600, Stephen Warren wrote:
>> On 08/21/2014 12:58 AM, Thierry Reding wrote:
>>> On Wed, Aug 20, 2014 at 02:16:49PM -0600, Stephen Warren wrote:
>>>> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>>>>> Hardware-triggered thermal reset requires configuring the I2C
>>>>> reset procedure. This configuration is read from the device tree,
>>>>> so document the relevant properties in the binding documentation.
>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt

>>>>> +- nvidia,pmu : Phandle to power management unit / PMIC handling poweroff
>>>>> +- nvidia,reg-addr : I2C register address to write poweroff command to
>>>>> +- nvidia,reg-data : Poweroff command to write to PMU
>>>>
>>>> Why are both the PMU/PMIC phandle and the register address/data required? I
>>>> thought the purpose of having the phandle was to allow the register address
>>>> and data to be queried from the PMU/PMIC driver.
>>>>
>>>> To me, it seems much simpler to get rid of the phandle and just hard-code
>>>> the I2C bus number, address, and data into this node, rather than having to
>>>> go query it from the PMU/PMIC driver, then find the I2C controller, then
>>>> query it for its ID (and hope that all HW modules that talk to I2C
>>>> controllers directly use the same numbering scheme...)
>>>
>>> I originally requested this to be changed. It seems wrong to duplicate
>>> information about the PMIC in both the PMIC device tree node and the
>>> i2c-thermtrip node if we can get the same information from the driver
>>> directly (via the phandle). It certainly requires a little more code,
>>> but at the advantage of not having to figure out the I2C controller
>>> hardware number and I2C slave addresses when writing the i2c-thermtrip
>>> node.
>>
>> I cant see that argument, but surely the PMIC driver can also supply the

Oops, that was meant to be can not cant.

>> "reg-addr" and "reg-data" values too, if it's already being queried for the
>> I2C device address and bus number? The binding above appears to duplicate
>> part of the information, while requiring querying of the other part.
>
> I suppose that could be done. It would take a new function to do that,
> though, since I'm not aware of a generic mechanism to query this kind of
> information from a PMIC (there's no generic PMIC API, either, so perhaps
> it would be a good time to create one?). The I2C controller and I2C
> slave are generic I2C properties, whereas the register and data to power
> off the device are very device specific.

I don't think it's possible to generically query an I2C device for its 
address from the struct i2c_device object; the code still needs to call 
a function in the PMIC driver to obtain this.

That's because some I2C devices respond to multiple I2C addresses, and 
there's no guarantee that the one I2C address in the DT (and hence the 
struct i2c_device) is the address upon which the regulator function exists.

grep for i2c_new_dummy in drivers/mfd and you'll find quite a few 
examples. The Atmel MXT touchpad/screen is another example.

[PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Stephen Warren]

On 08/21/2014 10:53 AM, Thierry Reding wrote:
> On Thu, Aug 21, 2014 at 09:38:29AM -0600, Stephen Warren wrote:
>> On 08/21/2014 12:58 AM, Thierry Reding wrote:
>>> On Wed, Aug 20, 2014 at 02:16:49PM -0600, Stephen Warren wrote:
>>>> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>>>>> Hardware-triggered thermal reset requires configuring the I2C
>>>>> reset procedure. This configuration is read from the device tree,
>>>>> so document the relevant properties in the binding documentation.
>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt

>>>>> +- nvidia,pmu : Phandle to power management unit / PMIC handling poweroff
>>>>> +- nvidia,reg-addr : I2C register address to write poweroff command to
>>>>> +- nvidia,reg-data : Poweroff command to write to PMU
>>>>
>>>> Why are both the PMU/PMIC phandle and the register address/data required? I
>>>> thought the purpose of having the phandle was to allow the register address
>>>> and data to be queried from the PMU/PMIC driver.
>>>>
>>>> To me, it seems much simpler to get rid of the phandle and just hard-code
>>>> the I2C bus number, address, and data into this node, rather than having to
>>>> go query it from the PMU/PMIC driver, then find the I2C controller, then
>>>> query it for its ID (and hope that all HW modules that talk to I2C
>>>> controllers directly use the same numbering scheme...)
>>>
>>> I originally requested this to be changed. It seems wrong to duplicate
>>> information about the PMIC in both the PMIC device tree node and the
>>> i2c-thermtrip node if we can get the same information from the driver
>>> directly (via the phandle). It certainly requires a little more code,
>>> but at the advantage of not having to figure out the I2C controller
>>> hardware number and I2C slave addresses when writing the i2c-thermtrip
>>> node.
>>
>> I cant see that argument, but surely the PMIC driver can also supply the

Oops, that was meant to be can not cant.

>> "reg-addr" and "reg-data" values too, if it's already being queried for the
>> I2C device address and bus number? The binding above appears to duplicate
>> part of the information, while requiring querying of the other part.
>
> I suppose that could be done. It would take a new function to do that,
> though, since I'm not aware of a generic mechanism to query this kind of
> information from a PMIC (there's no generic PMIC API, either, so perhaps
> it would be a good time to create one?). The I2C controller and I2C
> slave are generic I2C properties, whereas the register and data to power
> off the device are very device specific.

I don't think it's possible to generically query an I2C device for its 
address from the struct i2c_device object; the code still needs to call 
a function in the PMIC driver to obtain this.

That's because some I2C devices respond to multiple I2C addresses, and 
there's no guarantee that the one I2C address in the DT (and hence the 
struct i2c_device) is the address upon which the regulator function exists.

grep for i2c_new_dummy in drivers/mfd and you'll find quite a few 
examples. The Atmel MXT touchpad/screen is another example.

Re: [PATCH v2 5/5] ARM: tegra: Add thermal reset (thermtrip) support to PMC

[Mikko Perttunen]

On 21/08/14 18:40, Stephen Warren wrote:
> On 08/21/2014 07:11 AM, Mikko Perttunen wrote:
>> On 20/08/14 23:25, Stephen Warren wrote:
>>> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>>>> This adds a device tree controlled option to enable PMC-based
>>>> thermal reset in overheating situations. Thermtrip is supported on
>>>> Tegra30, Tegra114 and Tegra124. The thermal reset only works when
>>>> the thermal sensors are calibrated, so a soctherm driver is also
>>>> required.
>>>
>>> If calibration is required, presumably the soctherm must initialize
>>> before this thermtrip code can initialize, or this thermtrip logic might
>>> be triggered by uncalibrated sensors?
>>
>> SOCTHERM requires that each sensor be explicitly enabled before it gives
>> readings. If a sensor is not enabled, the temperature given by the
>> register (and used to trigger thermtrip) will be zero. So in order for a
>> thermtrip shutdown to be caused before soctherm is initialized, the
>> thermtrip temperature would have to be programmed to below zero (the
>> default value is 105C), in which case an immediate shutdown would
>> probably be in order anyway (unless the user uses LN2 or something to
>> cool the soc below zero).
>>
>>>
>>> If so, then there needs to be some explicit mechanism to force the two
>>> drivers into probing in the right order.
>>
>> Because of the above, I think it isn't necessary to probe these in order.
>
> OK, that makes sense. Briefly mentioning this in the commit description
> could be useful.
>

Sure, I'll add a mention to the next version.

Mikko

Re: [PATCH v2 5/5] ARM: tegra: Add thermal reset (thermtrip) support to PMC

[Mikko Perttunen]

On 21/08/14 18:40, Stephen Warren wrote:
> On 08/21/2014 07:11 AM, Mikko Perttunen wrote:
>> On 20/08/14 23:25, Stephen Warren wrote:
>>> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>>>> This adds a device tree controlled option to enable PMC-based
>>>> thermal reset in overheating situations. Thermtrip is supported on
>>>> Tegra30, Tegra114 and Tegra124. The thermal reset only works when
>>>> the thermal sensors are calibrated, so a soctherm driver is also
>>>> required.
>>>
>>> If calibration is required, presumably the soctherm must initialize
>>> before this thermtrip code can initialize, or this thermtrip logic might
>>> be triggered by uncalibrated sensors?
>>
>> SOCTHERM requires that each sensor be explicitly enabled before it gives
>> readings. If a sensor is not enabled, the temperature given by the
>> register (and used to trigger thermtrip) will be zero. So in order for a
>> thermtrip shutdown to be caused before soctherm is initialized, the
>> thermtrip temperature would have to be programmed to below zero (the
>> default value is 105C), in which case an immediate shutdown would
>> probably be in order anyway (unless the user uses LN2 or something to
>> cool the soc below zero).
>>
>>>
>>> If so, then there needs to be some explicit mechanism to force the two
>>> drivers into probing in the right order.
>>
>> Because of the above, I think it isn't necessary to probe these in order.
>
> OK, that makes sense. Briefly mentioning this in the commit description
> could be useful.
>

Sure, I'll add a mention to the next version.

Mikko

[PATCH v2 5/5] ARM: tegra: Add thermal reset (thermtrip) support to PMC

[Mikko Perttunen]

On 21/08/14 18:40, Stephen Warren wrote:
> On 08/21/2014 07:11 AM, Mikko Perttunen wrote:
>> On 20/08/14 23:25, Stephen Warren wrote:
>>> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>>>> This adds a device tree controlled option to enable PMC-based
>>>> thermal reset in overheating situations. Thermtrip is supported on
>>>> Tegra30, Tegra114 and Tegra124. The thermal reset only works when
>>>> the thermal sensors are calibrated, so a soctherm driver is also
>>>> required.
>>>
>>> If calibration is required, presumably the soctherm must initialize
>>> before this thermtrip code can initialize, or this thermtrip logic might
>>> be triggered by uncalibrated sensors?
>>
>> SOCTHERM requires that each sensor be explicitly enabled before it gives
>> readings. If a sensor is not enabled, the temperature given by the
>> register (and used to trigger thermtrip) will be zero. So in order for a
>> thermtrip shutdown to be caused before soctherm is initialized, the
>> thermtrip temperature would have to be programmed to below zero (the
>> default value is 105C), in which case an immediate shutdown would
>> probably be in order anyway (unless the user uses LN2 or something to
>> cool the soc below zero).
>>
>>>
>>> If so, then there needs to be some explicit mechanism to force the two
>>> drivers into probing in the right order.
>>
>> Because of the above, I think it isn't necessary to probe these in order.
>
> OK, that makes sense. Briefly mentioning this in the commit description
> could be useful.
>

Sure, I'll add a mention to the next version.

Mikko

Re: [PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Mikko Perttunen]

Hi again!

I have fixed all other issues mentioned apart from this one. I can see 
three ways ahead:

1) Keep things as is. There is a slight possibility that in the future 
we will have a hardware configuration with multiple bus addresses and 
this will cause annoyance.
2) Keep things otherwise as is, but read the bus address from the 
thermtrip node. While at it, just have a reference to the the I2C bus 
rather than the PMIC in the bindings.
3) Create a new poweroff driver class with two operations: poweroff and 
get_i2c_command. The AS3722 poweroff "driver" is already separated from 
the MFD and would be relatively straightforward to port to a new driver 
subsystem. However, other PMICs we have, such as Palmas, don't do this 
and would require larger refactoring. TBH, this smells of 
overengineering to me.
4) Other ideas?

What is your opinion?

Cheers,
Mikko

On 08/21/2014 08:54 PM, Stephen Warren wrote:
> On 08/21/2014 10:53 AM, Thierry Reding wrote:
>> On Thu, Aug 21, 2014 at 09:38:29AM -0600, Stephen Warren wrote:
>>> On 08/21/2014 12:58 AM, Thierry Reding wrote:
>>>> On Wed, Aug 20, 2014 at 02:16:49PM -0600, Stephen Warren wrote:
>>>>> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>>>>>> Hardware-triggered thermal reset requires configuring the I2C
>>>>>> reset procedure. This configuration is read from the device tree,
>>>>>> so document the relevant properties in the binding documentation.
>>>>>
>>>>>> diff --git
>>>>>> a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>>>>> b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>
>>>>>> +- nvidia,pmu : Phandle to power management unit / PMIC handling
>>>>>> poweroff
>>>>>> +- nvidia,reg-addr : I2C register address to write poweroff
>>>>>> command to
>>>>>> +- nvidia,reg-data : Poweroff command to write to PMU
>>>>>
>>>>> Why are both the PMU/PMIC phandle and the register address/data
>>>>> required? I
>>>>> thought the purpose of having the phandle was to allow the register
>>>>> address
>>>>> and data to be queried from the PMU/PMIC driver.
>>>>>
>>>>> To me, it seems much simpler to get rid of the phandle and just
>>>>> hard-code
>>>>> the I2C bus number, address, and data into this node, rather than
>>>>> having to
>>>>> go query it from the PMU/PMIC driver, then find the I2C controller,
>>>>> then
>>>>> query it for its ID (and hope that all HW modules that talk to I2C
>>>>> controllers directly use the same numbering scheme...)
>>>>
>>>> I originally requested this to be changed. It seems wrong to duplicate
>>>> information about the PMIC in both the PMIC device tree node and the
>>>> i2c-thermtrip node if we can get the same information from the driver
>>>> directly (via the phandle). It certainly requires a little more code,
>>>> but at the advantage of not having to figure out the I2C controller
>>>> hardware number and I2C slave addresses when writing the i2c-thermtrip
>>>> node.
>>>
>>> I cant see that argument, but surely the PMIC driver can also supply the
>
> Oops, that was meant to be can not cant.
>
>>> "reg-addr" and "reg-data" values too, if it's already being queried
>>> for the
>>> I2C device address and bus number? The binding above appears to
>>> duplicate
>>> part of the information, while requiring querying of the other part.
>>
>> I suppose that could be done. It would take a new function to do that,
>> though, since I'm not aware of a generic mechanism to query this kind of
>> information from a PMIC (there's no generic PMIC API, either, so perhaps
>> it would be a good time to create one?). The I2C controller and I2C
>> slave are generic I2C properties, whereas the register and data to power
>> off the device are very device specific.
>
> I don't think it's possible to generically query an I2C device for its
> address from the struct i2c_device object; the code still needs to call
> a function in the PMIC driver to obtain this.
>
> That's because some I2C devices respond to multiple I2C addresses, and
> there's no guarantee that the one I2C address in the DT (and hence the
> struct i2c_device) is the address upon which the regulator function exists.
>
> grep for i2c_new_dummy in drivers/mfd and you'll find quite a few
> examples. The Atmel MXT touchpad/screen is another example.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Mikko Perttunen]

Hi again!

I have fixed all other issues mentioned apart from this one. I can see 
three ways ahead:

1) Keep things as is. There is a slight possibility that in the future 
we will have a hardware configuration with multiple bus addresses and 
this will cause annoyance.
2) Keep things otherwise as is, but read the bus address from the 
thermtrip node. While at it, just have a reference to the the I2C bus 
rather than the PMIC in the bindings.
3) Create a new poweroff driver class with two operations: poweroff and 
get_i2c_command. The AS3722 poweroff "driver" is already separated from 
the MFD and would be relatively straightforward to port to a new driver 
subsystem. However, other PMICs we have, such as Palmas, don't do this 
and would require larger refactoring. TBH, this smells of 
overengineering to me.
4) Other ideas?

What is your opinion?

Cheers,
Mikko

On 08/21/2014 08:54 PM, Stephen Warren wrote:
> On 08/21/2014 10:53 AM, Thierry Reding wrote:
>> On Thu, Aug 21, 2014 at 09:38:29AM -0600, Stephen Warren wrote:
>>> On 08/21/2014 12:58 AM, Thierry Reding wrote:
>>>> On Wed, Aug 20, 2014 at 02:16:49PM -0600, Stephen Warren wrote:
>>>>> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>>>>>> Hardware-triggered thermal reset requires configuring the I2C
>>>>>> reset procedure. This configuration is read from the device tree,
>>>>>> so document the relevant properties in the binding documentation.
>>>>>
>>>>>> diff --git
>>>>>> a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>>>>> b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>
>>>>>> +- nvidia,pmu : Phandle to power management unit / PMIC handling
>>>>>> poweroff
>>>>>> +- nvidia,reg-addr : I2C register address to write poweroff
>>>>>> command to
>>>>>> +- nvidia,reg-data : Poweroff command to write to PMU
>>>>>
>>>>> Why are both the PMU/PMIC phandle and the register address/data
>>>>> required? I
>>>>> thought the purpose of having the phandle was to allow the register
>>>>> address
>>>>> and data to be queried from the PMU/PMIC driver.
>>>>>
>>>>> To me, it seems much simpler to get rid of the phandle and just
>>>>> hard-code
>>>>> the I2C bus number, address, and data into this node, rather than
>>>>> having to
>>>>> go query it from the PMU/PMIC driver, then find the I2C controller,
>>>>> then
>>>>> query it for its ID (and hope that all HW modules that talk to I2C
>>>>> controllers directly use the same numbering scheme...)
>>>>
>>>> I originally requested this to be changed. It seems wrong to duplicate
>>>> information about the PMIC in both the PMIC device tree node and the
>>>> i2c-thermtrip node if we can get the same information from the driver
>>>> directly (via the phandle). It certainly requires a little more code,
>>>> but at the advantage of not having to figure out the I2C controller
>>>> hardware number and I2C slave addresses when writing the i2c-thermtrip
>>>> node.
>>>
>>> I cant see that argument, but surely the PMIC driver can also supply the
>
> Oops, that was meant to be can not cant.
>
>>> "reg-addr" and "reg-data" values too, if it's already being queried
>>> for the
>>> I2C device address and bus number? The binding above appears to
>>> duplicate
>>> part of the information, while requiring querying of the other part.
>>
>> I suppose that could be done. It would take a new function to do that,
>> though, since I'm not aware of a generic mechanism to query this kind of
>> information from a PMIC (there's no generic PMIC API, either, so perhaps
>> it would be a good time to create one?). The I2C controller and I2C
>> slave are generic I2C properties, whereas the register and data to power
>> off the device are very device specific.
>
> I don't think it's possible to generically query an I2C device for its
> address from the struct i2c_device object; the code still needs to call
> a function in the PMIC driver to obtain this.
>
> That's because some I2C devices respond to multiple I2C addresses, and
> there's no guarantee that the one I2C address in the DT (and hence the
> struct i2c_device) is the address upon which the regulator function exists.
>
> grep for i2c_new_dummy in drivers/mfd and you'll find quite a few
> examples. The Atmel MXT touchpad/screen is another example.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Mikko Perttunen]

Hi again!

I have fixed all other issues mentioned apart from this one. I can see 
three ways ahead:

1) Keep things as is. There is a slight possibility that in the future 
we will have a hardware configuration with multiple bus addresses and 
this will cause annoyance.
2) Keep things otherwise as is, but read the bus address from the 
thermtrip node. While at it, just have a reference to the the I2C bus 
rather than the PMIC in the bindings.
3) Create a new poweroff driver class with two operations: poweroff and 
get_i2c_command. The AS3722 poweroff "driver" is already separated from 
the MFD and would be relatively straightforward to port to a new driver 
subsystem. However, other PMICs we have, such as Palmas, don't do this 
and would require larger refactoring. TBH, this smells of 
overengineering to me.
4) Other ideas?

What is your opinion?

Cheers,
Mikko

On 08/21/2014 08:54 PM, Stephen Warren wrote:
> On 08/21/2014 10:53 AM, Thierry Reding wrote:
>> On Thu, Aug 21, 2014 at 09:38:29AM -0600, Stephen Warren wrote:
>>> On 08/21/2014 12:58 AM, Thierry Reding wrote:
>>>> On Wed, Aug 20, 2014 at 02:16:49PM -0600, Stephen Warren wrote:
>>>>> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>>>>>> Hardware-triggered thermal reset requires configuring the I2C
>>>>>> reset procedure. This configuration is read from the device tree,
>>>>>> so document the relevant properties in the binding documentation.
>>>>>
>>>>>> diff --git
>>>>>> a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>>>>> b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>
>>>>>> +- nvidia,pmu : Phandle to power management unit / PMIC handling
>>>>>> poweroff
>>>>>> +- nvidia,reg-addr : I2C register address to write poweroff
>>>>>> command to
>>>>>> +- nvidia,reg-data : Poweroff command to write to PMU
>>>>>
>>>>> Why are both the PMU/PMIC phandle and the register address/data
>>>>> required? I
>>>>> thought the purpose of having the phandle was to allow the register
>>>>> address
>>>>> and data to be queried from the PMU/PMIC driver.
>>>>>
>>>>> To me, it seems much simpler to get rid of the phandle and just
>>>>> hard-code
>>>>> the I2C bus number, address, and data into this node, rather than
>>>>> having to
>>>>> go query it from the PMU/PMIC driver, then find the I2C controller,
>>>>> then
>>>>> query it for its ID (and hope that all HW modules that talk to I2C
>>>>> controllers directly use the same numbering scheme...)
>>>>
>>>> I originally requested this to be changed. It seems wrong to duplicate
>>>> information about the PMIC in both the PMIC device tree node and the
>>>> i2c-thermtrip node if we can get the same information from the driver
>>>> directly (via the phandle). It certainly requires a little more code,
>>>> but at the advantage of not having to figure out the I2C controller
>>>> hardware number and I2C slave addresses when writing the i2c-thermtrip
>>>> node.
>>>
>>> I cant see that argument, but surely the PMIC driver can also supply the
>
> Oops, that was meant to be can not cant.
>
>>> "reg-addr" and "reg-data" values too, if it's already being queried
>>> for the
>>> I2C device address and bus number? The binding above appears to
>>> duplicate
>>> part of the information, while requiring querying of the other part.
>>
>> I suppose that could be done. It would take a new function to do that,
>> though, since I'm not aware of a generic mechanism to query this kind of
>> information from a PMIC (there's no generic PMIC API, either, so perhaps
>> it would be a good time to create one?). The I2C controller and I2C
>> slave are generic I2C properties, whereas the register and data to power
>> off the device are very device specific.
>
> I don't think it's possible to generically query an I2C device for its
> address from the struct i2c_device object; the code still needs to call
> a function in the PMIC driver to obtain this.
>
> That's because some I2C devices respond to multiple I2C addresses, and
> there's no guarantee that the one I2C address in the DT (and hence the
> struct i2c_device) is the address upon which the regulator function exists.
>
> grep for i2c_new_dummy in drivers/mfd and you'll find quite a few
> examples. The Atmel MXT touchpad/screen is another example.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Stephen Warren]

On 09/05/2014 03:50 AM, Mikko Perttunen wrote:
> Hi again!
>
> I have fixed all other issues mentioned apart from this one. I can see
> three ways ahead:
>
> 1) Keep things as is. There is a slight possibility that in the future
> we will have a hardware configuration with multiple bus addresses and
> this will cause annoyance.
> 2) Keep things otherwise as is, but read the bus address from the
> thermtrip node. While at it, just have a reference to the the I2C bus
> rather than the PMIC in the bindings.
> 3) Create a new poweroff driver class with two operations: poweroff and
> get_i2c_command. The AS3722 poweroff "driver" is already separated from
> the MFD and would be relatively straightforward to port to a new driver
> subsystem. However, other PMICs we have, such as Palmas, don't do this
> and would require larger refactoring. TBH, this smells of
> overengineering to me.
> 4) Other ideas?

I think the two possible solutions are:

1) Put the following in the thermal node:

* phandle to I2C bus
* I2C address to write to (together with 7- or 10-bit indicator, if the 
HW supports that)
* Device register address to write to (together with byte count 
indicator if the HW supports that)
* Device register value (together with byte count indicator if the HW 
supports that)

This seems simplest; it's a very direct representation of the HW, and 
requires not extra infra-structure to be written or maintained. The only 
issue is that it encodes register values in the DT which has previously 
been frowned upon, and the values duplicate a tiny tiny part of the PMIC 
driver. Personally, I don't think that's a big deal though.

or:

2) Put the following in the thermal node:

* phandle to I2C device of PMIC

... and have the thermal node's driver call function(s) in the PMIC 
driver to retrieve all the information I mentioned above.

Any other option has too many holes or special cases that aren't covered.

>
> What is your opinion?
>
> Cheers,
> Mikko
>
> On 08/21/2014 08:54 PM, Stephen Warren wrote:
>> On 08/21/2014 10:53 AM, Thierry Reding wrote:
>>> On Thu, Aug 21, 2014 at 09:38:29AM -0600, Stephen Warren wrote:
>>>> On 08/21/2014 12:58 AM, Thierry Reding wrote:
>>>>> On Wed, Aug 20, 2014 at 02:16:49PM -0600, Stephen Warren wrote:
>>>>>> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>>>>>>> Hardware-triggered thermal reset requires configuring the I2C
>>>>>>> reset procedure. This configuration is read from the device tree,
>>>>>>> so document the relevant properties in the binding documentation.
>>>>>>
>>>>>>> diff --git
>>>>>>> a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>>>>>> b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>
>>>>>>> +- nvidia,pmu : Phandle to power management unit / PMIC handling
>>>>>>> poweroff
>>>>>>> +- nvidia,reg-addr : I2C register address to write poweroff
>>>>>>> command to
>>>>>>> +- nvidia,reg-data : Poweroff command to write to PMU
>>>>>>
>>>>>> Why are both the PMU/PMIC phandle and the register address/data
>>>>>> required? I
>>>>>> thought the purpose of having the phandle was to allow the register
>>>>>> address
>>>>>> and data to be queried from the PMU/PMIC driver.
>>>>>>
>>>>>> To me, it seems much simpler to get rid of the phandle and just
>>>>>> hard-code
>>>>>> the I2C bus number, address, and data into this node, rather than
>>>>>> having to
>>>>>> go query it from the PMU/PMIC driver, then find the I2C controller,
>>>>>> then
>>>>>> query it for its ID (and hope that all HW modules that talk to I2C
>>>>>> controllers directly use the same numbering scheme...)
>>>>>
>>>>> I originally requested this to be changed. It seems wrong to duplicate
>>>>> information about the PMIC in both the PMIC device tree node and the
>>>>> i2c-thermtrip node if we can get the same information from the driver
>>>>> directly (via the phandle). It certainly requires a little more code,
>>>>> but at the advantage of not having to figure out the I2C controller
>>>>> hardware number and I2C slave addresses when writing the i2c-thermtrip
>>>>> node.
>>>>
>>>> I cant see that argument, but surely the PMIC driver can also supply
>>>> the
>>
>> Oops, that was meant to be can not cant.
>>
>>>> "reg-addr" and "reg-data" values too, if it's already being queried
>>>> for the
>>>> I2C device address and bus number? The binding above appears to
>>>> duplicate
>>>> part of the information, while requiring querying of the other part.
>>>
>>> I suppose that could be done. It would take a new function to do that,
>>> though, since I'm not aware of a generic mechanism to query this kind of
>>> information from a PMIC (there's no generic PMIC API, either, so perhaps
>>> it would be a good time to create one?). The I2C controller and I2C
>>> slave are generic I2C properties, whereas the register and data to power
>>> off the device are very device specific.
>>
>> I don't think it's possible to generically query an I2C device for its
>> address from the struct i2c_device object; the code still needs to call
>> a function in the PMIC driver to obtain this.
>>
>> That's because some I2C devices respond to multiple I2C addresses, and
>> there's no guarantee that the one I2C address in the DT (and hence the
>> struct i2c_device) is the address upon which the regulator function
>> exists.
>>
>> grep for i2c_new_dummy in drivers/mfd and you'll find quite a few
>> examples. The Atmel MXT touchpad/screen is another example.
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Stephen Warren]

On 09/05/2014 03:50 AM, Mikko Perttunen wrote:
> Hi again!
>
> I have fixed all other issues mentioned apart from this one. I can see
> three ways ahead:
>
> 1) Keep things as is. There is a slight possibility that in the future
> we will have a hardware configuration with multiple bus addresses and
> this will cause annoyance.
> 2) Keep things otherwise as is, but read the bus address from the
> thermtrip node. While at it, just have a reference to the the I2C bus
> rather than the PMIC in the bindings.
> 3) Create a new poweroff driver class with two operations: poweroff and
> get_i2c_command. The AS3722 poweroff "driver" is already separated from
> the MFD and would be relatively straightforward to port to a new driver
> subsystem. However, other PMICs we have, such as Palmas, don't do this
> and would require larger refactoring. TBH, this smells of
> overengineering to me.
> 4) Other ideas?

I think the two possible solutions are:

1) Put the following in the thermal node:

* phandle to I2C bus
* I2C address to write to (together with 7- or 10-bit indicator, if the 
HW supports that)
* Device register address to write to (together with byte count 
indicator if the HW supports that)
* Device register value (together with byte count indicator if the HW 
supports that)

This seems simplest; it's a very direct representation of the HW, and 
requires not extra infra-structure to be written or maintained. The only 
issue is that it encodes register values in the DT which has previously 
been frowned upon, and the values duplicate a tiny tiny part of the PMIC 
driver. Personally, I don't think that's a big deal though.

or:

2) Put the following in the thermal node:

* phandle to I2C device of PMIC

... and have the thermal node's driver call function(s) in the PMIC 
driver to retrieve all the information I mentioned above.

Any other option has too many holes or special cases that aren't covered.

>
> What is your opinion?
>
> Cheers,
> Mikko
>
> On 08/21/2014 08:54 PM, Stephen Warren wrote:
>> On 08/21/2014 10:53 AM, Thierry Reding wrote:
>>> On Thu, Aug 21, 2014 at 09:38:29AM -0600, Stephen Warren wrote:
>>>> On 08/21/2014 12:58 AM, Thierry Reding wrote:
>>>>> On Wed, Aug 20, 2014 at 02:16:49PM -0600, Stephen Warren wrote:
>>>>>> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>>>>>>> Hardware-triggered thermal reset requires configuring the I2C
>>>>>>> reset procedure. This configuration is read from the device tree,
>>>>>>> so document the relevant properties in the binding documentation.
>>>>>>
>>>>>>> diff --git
>>>>>>> a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>>>>>> b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>
>>>>>>> +- nvidia,pmu : Phandle to power management unit / PMIC handling
>>>>>>> poweroff
>>>>>>> +- nvidia,reg-addr : I2C register address to write poweroff
>>>>>>> command to
>>>>>>> +- nvidia,reg-data : Poweroff command to write to PMU
>>>>>>
>>>>>> Why are both the PMU/PMIC phandle and the register address/data
>>>>>> required? I
>>>>>> thought the purpose of having the phandle was to allow the register
>>>>>> address
>>>>>> and data to be queried from the PMU/PMIC driver.
>>>>>>
>>>>>> To me, it seems much simpler to get rid of the phandle and just
>>>>>> hard-code
>>>>>> the I2C bus number, address, and data into this node, rather than
>>>>>> having to
>>>>>> go query it from the PMU/PMIC driver, then find the I2C controller,
>>>>>> then
>>>>>> query it for its ID (and hope that all HW modules that talk to I2C
>>>>>> controllers directly use the same numbering scheme...)
>>>>>
>>>>> I originally requested this to be changed. It seems wrong to duplicate
>>>>> information about the PMIC in both the PMIC device tree node and the
>>>>> i2c-thermtrip node if we can get the same information from the driver
>>>>> directly (via the phandle). It certainly requires a little more code,
>>>>> but at the advantage of not having to figure out the I2C controller
>>>>> hardware number and I2C slave addresses when writing the i2c-thermtrip
>>>>> node.
>>>>
>>>> I cant see that argument, but surely the PMIC driver can also supply
>>>> the
>>
>> Oops, that was meant to be can not cant.
>>
>>>> "reg-addr" and "reg-data" values too, if it's already being queried
>>>> for the
>>>> I2C device address and bus number? The binding above appears to
>>>> duplicate
>>>> part of the information, while requiring querying of the other part.
>>>
>>> I suppose that could be done. It would take a new function to do that,
>>> though, since I'm not aware of a generic mechanism to query this kind of
>>> information from a PMIC (there's no generic PMIC API, either, so perhaps
>>> it would be a good time to create one?). The I2C controller and I2C
>>> slave are generic I2C properties, whereas the register and data to power
>>> off the device are very device specific.
>>
>> I don't think it's possible to generically query an I2C device for its
>> address from the struct i2c_device object; the code still needs to call
>> a function in the PMIC driver to obtain this.
>>
>> That's because some I2C devices respond to multiple I2C addresses, and
>> there's no guarantee that the one I2C address in the DT (and hence the
>> struct i2c_device) is the address upon which the regulator function
>> exists.
>>
>> grep for i2c_new_dummy in drivers/mfd and you'll find quite a few
>> examples. The Atmel MXT touchpad/screen is another example.
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

[Stephen Warren]

On 09/05/2014 03:50 AM, Mikko Perttunen wrote:
> Hi again!
>
> I have fixed all other issues mentioned apart from this one. I can see
> three ways ahead:
>
> 1) Keep things as is. There is a slight possibility that in the future
> we will have a hardware configuration with multiple bus addresses and
> this will cause annoyance.
> 2) Keep things otherwise as is, but read the bus address from the
> thermtrip node. While at it, just have a reference to the the I2C bus
> rather than the PMIC in the bindings.
> 3) Create a new poweroff driver class with two operations: poweroff and
> get_i2c_command. The AS3722 poweroff "driver" is already separated from
> the MFD and would be relatively straightforward to port to a new driver
> subsystem. However, other PMICs we have, such as Palmas, don't do this
> and would require larger refactoring. TBH, this smells of
> overengineering to me.
> 4) Other ideas?

I think the two possible solutions are:

1) Put the following in the thermal node:

* phandle to I2C bus
* I2C address to write to (together with 7- or 10-bit indicator, if the 
HW supports that)
* Device register address to write to (together with byte count 
indicator if the HW supports that)
* Device register value (together with byte count indicator if the HW 
supports that)

This seems simplest; it's a very direct representation of the HW, and 
requires not extra infra-structure to be written or maintained. The only 
issue is that it encodes register values in the DT which has previously 
been frowned upon, and the values duplicate a tiny tiny part of the PMIC 
driver. Personally, I don't think that's a big deal though.

or:

2) Put the following in the thermal node:

* phandle to I2C device of PMIC

... and have the thermal node's driver call function(s) in the PMIC 
driver to retrieve all the information I mentioned above.

Any other option has too many holes or special cases that aren't covered.

>
> What is your opinion?
>
> Cheers,
> Mikko
>
> On 08/21/2014 08:54 PM, Stephen Warren wrote:
>> On 08/21/2014 10:53 AM, Thierry Reding wrote:
>>> On Thu, Aug 21, 2014 at 09:38:29AM -0600, Stephen Warren wrote:
>>>> On 08/21/2014 12:58 AM, Thierry Reding wrote:
>>>>> On Wed, Aug 20, 2014 at 02:16:49PM -0600, Stephen Warren wrote:
>>>>>> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>>>>>>> Hardware-triggered thermal reset requires configuring the I2C
>>>>>>> reset procedure. This configuration is read from the device tree,
>>>>>>> so document the relevant properties in the binding documentation.
>>>>>>
>>>>>>> diff --git
>>>>>>> a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>>>>>> b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>
>>>>>>> +- nvidia,pmu : Phandle to power management unit / PMIC handling
>>>>>>> poweroff
>>>>>>> +- nvidia,reg-addr : I2C register address to write poweroff
>>>>>>> command to
>>>>>>> +- nvidia,reg-data : Poweroff command to write to PMU
>>>>>>
>>>>>> Why are both the PMU/PMIC phandle and the register address/data
>>>>>> required? I
>>>>>> thought the purpose of having the phandle was to allow the register
>>>>>> address
>>>>>> and data to be queried from the PMU/PMIC driver.
>>>>>>
>>>>>> To me, it seems much simpler to get rid of the phandle and just
>>>>>> hard-code
>>>>>> the I2C bus number, address, and data into this node, rather than
>>>>>> having to
>>>>>> go query it from the PMU/PMIC driver, then find the I2C controller,
>>>>>> then
>>>>>> query it for its ID (and hope that all HW modules that talk to I2C
>>>>>> controllers directly use the same numbering scheme...)
>>>>>
>>>>> I originally requested this to be changed. It seems wrong to duplicate
>>>>> information about the PMIC in both the PMIC device tree node and the
>>>>> i2c-thermtrip node if we can get the same information from the driver
>>>>> directly (via the phandle). It certainly requires a little more code,
>>>>> but at the advantage of not having to figure out the I2C controller
>>>>> hardware number and I2C slave addresses when writing the i2c-thermtrip
>>>>> node.
>>>>
>>>> I cant see that argument, but surely the PMIC driver can also supply
>>>> the
>>
>> Oops, that was meant to be can not cant.
>>
>>>> "reg-addr" and "reg-data" values too, if it's already being queried
>>>> for the
>>>> I2C device address and bus number? The binding above appears to
>>>> duplicate
>>>> part of the information, while requiring querying of the other part.
>>>
>>> I suppose that could be done. It would take a new function to do that,
>>> though, since I'm not aware of a generic mechanism to query this kind of
>>> information from a PMIC (there's no generic PMIC API, either, so perhaps
>>> it would be a good time to create one?). The I2C controller and I2C
>>> slave are generic I2C properties, whereas the register and data to power
>>> off the device are very device specific.
>>
>> I don't think it's possible to generically query an I2C device for its
>> address from the struct i2c_device object; the code still needs to call
>> a function in the PMIC driver to obtain this.
>>
>> That's because some I2C devices respond to multiple I2C addresses, and
>> there's no guarantee that the one I2C address in the DT (and hence the
>> struct i2c_device) is the address upon which the regulator function
>> exists.
>>
>> grep for i2c_new_dummy in drivers/mfd and you'll find quite a few
>> examples. The Atmel MXT touchpad/screen is another example.
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>