[PATCH 0/3] Stingray thermal driver support

[PATCH 0/3] Stingray thermal driver support

[Srinath Mannam]

These patches adds the stingray thermal driver and its
corresponding DT nodes with documentation.

Pramod Kumar (3):
  dt-bindings: thermal: Add binding document for SR thermal
  arm64: dts: stingray: Add Stingray Thermal DT support.
  thermal: broadcom: Add Stingray thermal driver

 .../bindings/thermal/brcm,sr-thermal.txt           |  45 ++++++
 .../arm64/boot/dts/broadcom/stingray/stingray.dtsi |  37 +++++
 drivers/thermal/Kconfig                            |   3 +-
 drivers/thermal/broadcom/Kconfig                   |   9 ++
 drivers/thermal/broadcom/Makefile                  |   1 +
 drivers/thermal/broadcom/sr-thermal.c              | 151 +++++++++++++++++++++
 6 files changed, 245 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
 create mode 100644 drivers/thermal/broadcom/sr-thermal.c

-- 
2.7.4

[PATCH 1/3] dt-bindings: thermal: Add binding document for SR thermal

[Srinath Mannam]

From: Pramod Kumar <pramod.kumar@broadcom.com>

Add binding document for supported thermal implementation
in Stingray.

Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com>
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.com>
---
 .../bindings/thermal/brcm,sr-thermal.txt           | 45 ++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt

diff --git a/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
new file mode 100644
index 0000000..33f9e11
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
@@ -0,0 +1,45 @@
+* Broadcom Stingray Thermal
+
+This binding describes thermal sensors that is part of Stingray SoCs.
+
+Required properties:
+- compatible : Must be "brcm,sr-thermal"
+- reg : memory where tmon data will be available.
+
+Example:
+	tmons {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges;
+
+		tmon_ihost0: thermal@8f100000 {
+			compatible = "brcm,sr-thermal";
+			reg = <0x8f100000 0x4>;
+		};
+
+		tmon_ihost1: thermal@8f100004 {
+			compatible = "brcm,sr-thermal";
+			reg = <0x8f100004 0x4>;
+		};
+
+		tmon_ihost2: thermal@8f100008 {
+			compatible = "brcm,sr-thermal";
+			reg = <0x8f100008 0x4>;
+		};
+
+		tmon_ihost3: thermal@8f10000c {
+			compatible = "brcm,sr-thermal";
+			reg = <0x8f10000c 0x4>;
+		};
+
+		tmon_crmu: thermal@8f100010 {
+			compatible = "brcm,sr-thermal";
+			reg = <0x8f100010 0x4>;
+		};
+
+		tmon_nitro: thermal@8f100014 {
+			compatible = "brcm,sr-thermal";
+			reg = <0x8f100014 0x4>;
+		};
+	};
-- 
2.7.4

[PATCH 2/3] arm64: dts: stingray: Add Stingray Thermal DT support.

[Srinath Mannam]

From: Pramod Kumar <pramod.kumar@broadcom.com>

Add DT nodes for thermal zones memory base address
to read temperature.

Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com>
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.com>
---
 .../arm64/boot/dts/broadcom/stingray/stingray.dtsi | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
index 99aaff0..db1cc67 100644
--- a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
+++ b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
@@ -593,4 +593,41 @@
 			status = "disabled";
 		};
 	};
+
+	tmons {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0x0 0x0 0x8f100000 0x100>;
+
+		tmon_ihost0: thermal@0 {
+			compatible = "brcm,sr-thermal";
+			reg = <0x0 0x4>;
+		};
+
+		tmon_ihost1: thermal@4 {
+			compatible = "brcm,sr-thermal";
+			reg = <0x4 0x4>;
+		};
+
+		tmon_ihost2: thermal@8 {
+			compatible = "brcm,sr-thermal";
+			reg = <0x8 0x4>;
+		};
+
+		tmon_ihost3: thermal@c {
+			compatible = "brcm,sr-thermal";
+			reg = <0xc 0x4>;
+		};
+
+		tmon_crmu: thermal@10 {
+			compatible = "brcm,sr-thermal";
+			reg = <0x10 0x4>;
+		};
+
+		tmon_nitro: thermal@14 {
+			compatible = "brcm,sr-thermal";
+			reg = <0x14 0x4>;
+		};
+	};
 };
-- 
2.7.4

[PATCH 3/3] thermal: broadcom: Add Stingray thermal driver

[Srinath Mannam]

From: Pramod Kumar <pramod.kumar@broadcom.com>

This commit adds stingray thermal driver to monitor six
thermal zones temperature and trips at critical temperature.

Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com>
Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com>
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Vikram Prakash <vikram.prakash@broadcom.com>
---
 drivers/thermal/Kconfig               |   3 +-
 drivers/thermal/broadcom/Kconfig      |   9 ++
 drivers/thermal/broadcom/Makefile     |   1 +
 drivers/thermal/broadcom/sr-thermal.c | 151 ++++++++++++++++++++++++++++++++++
 4 files changed, 163 insertions(+), 1 deletion(-)
 create mode 100644 drivers/thermal/broadcom/sr-thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 8297988..26d39d4 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -416,7 +416,8 @@ config MTK_THERMAL
 	  controller present in Mediatek SoCs
 
 menu "Broadcom thermal drivers"
-depends on ARCH_BCM || ARCH_BRCMSTB || ARCH_BCM2835 || COMPILE_TEST
+depends on ARCH_BCM || ARCH_BRCMSTB || ARCH_BCM2835 || ARCH_BCM_IPROC || \
+		COMPILE_TEST
 source "drivers/thermal/broadcom/Kconfig"
 endmenu
 
diff --git a/drivers/thermal/broadcom/Kconfig b/drivers/thermal/broadcom/Kconfig
index c106a15..dc9a9bd 100644
--- a/drivers/thermal/broadcom/Kconfig
+++ b/drivers/thermal/broadcom/Kconfig
@@ -22,3 +22,12 @@ config BCM_NS_THERMAL
 	  BCM4708, BCM4709, BCM5301x, BCM95852X, etc). It contains DMU (Device
 	  Management Unit) block with a thermal sensor that allows checking CPU
 	  temperature.
+
+config BCM_SR_THERMAL
+	tristate "Stingray thermal driver"
+	depends on ARCH_BCM_IPROC || COMPILE_TEST
+	default ARCH_BCM_IPROC
+	help
+	  Support for the Stingray family of SoCs. Its different blocks like
+	  iHost, CRMU and NITRO has thermal sensor that allows checking its
+	  temperature.
diff --git a/drivers/thermal/broadcom/Makefile b/drivers/thermal/broadcom/Makefile
index fae10ec..79df69e 100644
--- a/drivers/thermal/broadcom/Makefile
+++ b/drivers/thermal/broadcom/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_BCM2835_THERMAL)		+= bcm2835_thermal.o
 obj-$(CONFIG_BRCMSTB_THERMAL)		+= brcmstb_thermal.o
 obj-$(CONFIG_BCM_NS_THERMAL)		+= ns-thermal.o
+obj-$(CONFIG_BCM_SR_THERMAL)		+= sr-thermal.o
diff --git a/drivers/thermal/broadcom/sr-thermal.c b/drivers/thermal/broadcom/sr-thermal.c
new file mode 100644
index 0000000..5baaa6e
--- /dev/null
+++ b/drivers/thermal/broadcom/sr-thermal.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Broadcom
+ */
+
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/thermal.h>
+
+#define TMON_CRIT_TEMP 105000 /* temp in millidegree C */
+
+struct sr_thermal {
+	struct thermal_zone_device *tz;
+	struct device *dev;
+	void __iomem *regs;
+	unsigned int crit_temp;
+};
+
+static int sr_get_temp(struct thermal_zone_device *tz, int *temp)
+{
+	struct sr_thermal *sr_thermal = tz->devdata;
+
+	*temp = readl(sr_thermal->regs);
+
+	return 0;
+}
+
+static int sr_get_trip_type(struct thermal_zone_device *tz, int trip,
+					enum thermal_trip_type *type)
+{
+	struct sr_thermal *sr_thermal = tz->devdata;
+
+	switch (trip) {
+	case 0:
+		*type = THERMAL_TRIP_CRITICAL;
+		break;
+	default:
+		dev_dbg(sr_thermal->dev,
+			"Driver does not support more than 1 trip point\n");
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int sr_get_trip_temp(struct thermal_zone_device *tz, int trip, int *temp)
+{
+	struct sr_thermal *sr_thermal = tz->devdata;
+
+	switch (trip) {
+	case 0:
+		*temp = sr_thermal->crit_temp;
+		break;
+	default:
+		dev_dbg(sr_thermal->dev,
+			"Driver does not support more than 1 trip point\n");
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int sr_set_trip_temp(struct thermal_zone_device *tz, int trip, int temp)
+{
+	struct sr_thermal *sr_thermal = tz->devdata;
+
+	switch (trip) {
+	case 0:
+		/*
+		 * Allow the user to change critical temperature
+		 * as per their requirement, could be for debug
+		 * purpose, even if it's more than the recommended
+		 * critical temperature.
+		 */
+		sr_thermal->crit_temp = temp;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static struct thermal_zone_device_ops sr_thermal_ops = {
+	.get_temp = sr_get_temp,
+	.get_trip_type = sr_get_trip_type,
+	.get_trip_temp = sr_get_trip_temp,
+	.set_trip_temp = sr_set_trip_temp,
+};
+
+static int sr_thermal_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct sr_thermal *sr_thermal;
+	struct resource *res;
+
+	sr_thermal = devm_kzalloc(dev, sizeof(*sr_thermal), GFP_KERNEL);
+	if (!sr_thermal)
+		return -ENOMEM;
+	sr_thermal->dev = dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	sr_thermal->regs = devm_memremap(&pdev->dev, res->start,
+					 resource_size(res), MEMREMAP_WB);
+	if (IS_ERR(sr_thermal->regs)) {
+		dev_err(dev, "failed to get io address\n");
+		return PTR_ERR(sr_thermal->regs);
+	}
+
+	/* initialize tmon value to 0 */
+	writel(0, sr_thermal->regs);
+	sr_thermal->crit_temp = TMON_CRIT_TEMP;
+
+	sr_thermal->tz = thermal_zone_device_register(dev_name(dev), 1, 1,
+							 sr_thermal,
+							 &sr_thermal_ops,
+							 NULL, 1000, 1000);
+	if (IS_ERR(sr_thermal->tz))
+		return PTR_ERR(sr_thermal->tz);
+
+	platform_set_drvdata(pdev, sr_thermal);
+
+	return 0;
+}
+
+static int sr_thermal_remove(struct platform_device *pdev)
+{
+	struct sr_thermal *sr_thermal = platform_get_drvdata(pdev);
+
+	thermal_zone_device_unregister(sr_thermal->tz);
+
+	return 0;
+}
+
+static const struct of_device_id sr_thermal_of_match[] = {
+	{ .compatible = "brcm,sr-thermal", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, sr_thermal_of_match);
+
+static struct platform_driver sr_thermal_driver = {
+	.probe		= sr_thermal_probe,
+	.remove		= sr_thermal_remove,
+	.driver = {
+		.name = "sr-thermal",
+		.of_match_table = sr_thermal_of_match,
+	},
+};
+module_platform_driver(sr_thermal_driver);
+
+MODULE_AUTHOR("Pramod Kumar <pramod.kumar@broadcom.com>");
+MODULE_DESCRIPTION("Stingray thermal driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

Re: [PATCH 3/3] thermal: broadcom: Add Stingray thermal driver

[kbuild test robot]

Hi Pramod,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on soc-thermal/next]
[also build test WARNING on v4.17-rc7 next-20180529]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Srinath-Mannam/Stingray-thermal-driver-support/20180529-145243
base:   https://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git next
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/thermal/broadcom/sr-thermal.c:101:26: sparse: incorrect type in assignment (different address spaces) @@    expected void [noderef] <asn:2>*regs @@    got sn:2>*regs @@
   drivers/thermal/broadcom/sr-thermal.c:101:26:    expected void [noderef] <asn:2>*regs
   drivers/thermal/broadcom/sr-thermal.c:101:26:    got void *

vim +101 drivers/thermal/broadcom/sr-thermal.c

    88	
    89	static int sr_thermal_probe(struct platform_device *pdev)
    90	{
    91		struct device *dev = &pdev->dev;
    92		struct sr_thermal *sr_thermal;
    93		struct resource *res;
    94	
    95		sr_thermal = devm_kzalloc(dev, sizeof(*sr_thermal), GFP_KERNEL);
    96		if (!sr_thermal)
    97			return -ENOMEM;
    98		sr_thermal->dev = dev;
    99	
   100		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 > 101		sr_thermal->regs = devm_memremap(&pdev->dev, res->start,
   102						 resource_size(res), MEMREMAP_WB);
   103		if (IS_ERR(sr_thermal->regs)) {
   104			dev_err(dev, "failed to get io address\n");
   105			return PTR_ERR(sr_thermal->regs);
   106		}
   107	
   108		/* initialize tmon value to 0 */
   109		writel(0, sr_thermal->regs);
   110		sr_thermal->crit_temp = TMON_CRIT_TEMP;
   111	
   112		sr_thermal->tz = thermal_zone_device_register(dev_name(dev), 1, 1,
   113								 sr_thermal,
   114								 &sr_thermal_ops,
   115								 NULL, 1000, 1000);
   116		if (IS_ERR(sr_thermal->tz))
   117			return PTR_ERR(sr_thermal->tz);
   118	
   119		platform_set_drvdata(pdev, sr_thermal);
   120	
   121		return 0;
   122	}
   123	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH 1/3] dt-bindings: thermal: Add binding document for SR thermal

[Rob Herring]

On Mon, May 28, 2018 at 11:11:22AM +0530, Srinath Mannam wrote:
> From: Pramod Kumar <pramod.kumar@broadcom.com>
> 
> Add binding document for supported thermal implementation
> in Stingray.
> 
> Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com>
> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
> Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.com>
> ---
>  .../bindings/thermal/brcm,sr-thermal.txt           | 45 ++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
> 
> diff --git a/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
> new file mode 100644
> index 0000000..33f9e11
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
> @@ -0,0 +1,45 @@
> +* Broadcom Stingray Thermal
> +
> +This binding describes thermal sensors that is part of Stingray SoCs.
> +
> +Required properties:
> +- compatible : Must be "brcm,sr-thermal"
> +- reg : memory where tmon data will be available.

What type of memory is this?

> +
> +Example:
> +	tmons {
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		tmon_ihost0: thermal@8f100000 {
> +			compatible = "brcm,sr-thermal";
> +			reg = <0x8f100000 0x4>;
> +		};

Convince me that you need a node per register. This can all be 
accomplished with a single node and either a single reg entry or a 
series of reg entries.

> +
> +		tmon_ihost1: thermal@8f100004 {
> +			compatible = "brcm,sr-thermal";
> +			reg = <0x8f100004 0x4>;
> +		};
> +
> +		tmon_ihost2: thermal@8f100008 {
> +			compatible = "brcm,sr-thermal";
> +			reg = <0x8f100008 0x4>;
> +		};
> +
> +		tmon_ihost3: thermal@8f10000c {
> +			compatible = "brcm,sr-thermal";
> +			reg = <0x8f10000c 0x4>;
> +		};
> +
> +		tmon_crmu: thermal@8f100010 {
> +			compatible = "brcm,sr-thermal";
> +			reg = <0x8f100010 0x4>;
> +		};
> +
> +		tmon_nitro: thermal@8f100014 {
> +			compatible = "brcm,sr-thermal";
> +			reg = <0x8f100014 0x4>;
> +		};
> +	};
> -- 
> 2.7.4
> 

Re: [PATCH 1/3] dt-bindings: thermal: Add binding document for SR thermal

[Srinath Mannam]

Hi Rob Herring,

Thank you for the review.
Please find my answers inline..

On Thu, May 31, 2018 at 10:18 PM, Rob Herring <robh@kernel.org> wrote:
> On Mon, May 28, 2018 at 11:11:22AM +0530, Srinath Mannam wrote:
>> From: Pramod Kumar <pramod.kumar@broadcom.com>
>>
>> Add binding document for supported thermal implementation
>> in Stingray.
>>
>> Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com>
>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>> Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.com>
>> ---
>>  .../bindings/thermal/brcm,sr-thermal.txt           | 45 ++++++++++++++++++++++
>>  1 file changed, 45 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>>
>> diff --git a/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>> new file mode 100644
>> index 0000000..33f9e11
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>> @@ -0,0 +1,45 @@
>> +* Broadcom Stingray Thermal
>> +
>> +This binding describes thermal sensors that is part of Stingray SoCs.
>> +
>> +Required properties:
>> +- compatible : Must be "brcm,sr-thermal"
>> +- reg : memory where tmon data will be available.
>
> What type of memory is this?
This is shared memory(cache-able) which is shared between two micro controllers.
One micro controller update temperature of all thermal zones in this
shared memory.
thermal driver running on another micro controller monitors
temperature data of all thermal zones.
>
>> +
>> +Example:
>> +     tmons {
>> +             compatible = "simple-bus";
>> +             #address-cells = <1>;
>> +             #size-cells = <1>;
>> +             ranges;
>> +
>> +             tmon_ihost0: thermal@8f100000 {
>> +                     compatible = "brcm,sr-thermal";
>> +                     reg = <0x8f100000 0x4>;
>> +             };
>
> Convince me that you need a node per register. This can all be
> accomplished with a single node and either a single reg entry or a
> series of reg entries.
>
In our system we have 6 different thermal zones. each node belongs to
one thermal zone.
thermal driver detects each node as separate thermal zone and monitors
separately.
register entry is only 4byte memory which contains the temperature
value of respective thermal zone.
Based on requirement we can increase or decrease the number thermal
zones need to monitor.
>> +
>> +             tmon_ihost1: thermal@8f100004 {
>> +                     compatible = "brcm,sr-thermal";
>> +                     reg = <0x8f100004 0x4>;
>> +             };
>> +
>> +             tmon_ihost2: thermal@8f100008 {
>> +                     compatible = "brcm,sr-thermal";
>> +                     reg = <0x8f100008 0x4>;
>> +             };
>> +
>> +             tmon_ihost3: thermal@8f10000c {
>> +                     compatible = "brcm,sr-thermal";
>> +                     reg = <0x8f10000c 0x4>;
>> +             };
>> +
>> +             tmon_crmu: thermal@8f100010 {
>> +                     compatible = "brcm,sr-thermal";
>> +                     reg = <0x8f100010 0x4>;
>> +             };
>> +
>> +             tmon_nitro: thermal@8f100014 {
>> +                     compatible = "brcm,sr-thermal";
>> +                     reg = <0x8f100014 0x4>;
>> +             };
>> +     };
>> --
>> 2.7.4
>>

Re: [PATCH 1/3] dt-bindings: thermal: Add binding document for SR thermal

[Rob Herring]

On Fri, Jun 1, 2018 at 3:51 AM, Srinath Mannam
<srinath.mannam@broadcom.com> wrote:
> Hi Rob Herring,
>
> Thank you for the review.
> Please find my answers inline..
>
> On Thu, May 31, 2018 at 10:18 PM, Rob Herring <robh@kernel.org> wrote:
>> On Mon, May 28, 2018 at 11:11:22AM +0530, Srinath Mannam wrote:
>>> From: Pramod Kumar <pramod.kumar@broadcom.com>
>>>
>>> Add binding document for supported thermal implementation
>>> in Stingray.
>>>
>>> Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com>
>>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>>> Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.com>
>>> ---
>>>  .../bindings/thermal/brcm,sr-thermal.txt           | 45 ++++++++++++++++++++++
>>>  1 file changed, 45 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>>> new file mode 100644
>>> index 0000000..33f9e11
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>>> @@ -0,0 +1,45 @@
>>> +* Broadcom Stingray Thermal
>>> +
>>> +This binding describes thermal sensors that is part of Stingray SoCs.
>>> +
>>> +Required properties:
>>> +- compatible : Must be "brcm,sr-thermal"
>>> +- reg : memory where tmon data will be available.
>>
>> What type of memory is this?
> This is shared memory(cache-able) which is shared between two micro controllers.

What else is in this shared memory? You should probably be
representing that as a whole somewhere in DT.

> One micro controller update temperature of all thermal zones in this
> shared memory.
> thermal driver running on another micro controller monitors
> temperature data of all thermal zones.
>>
>>> +
>>> +Example:
>>> +     tmons {
>>> +             compatible = "simple-bus";
>>> +             #address-cells = <1>;
>>> +             #size-cells = <1>;
>>> +             ranges;
>>> +
>>> +             tmon_ihost0: thermal@8f100000 {
>>> +                     compatible = "brcm,sr-thermal";
>>> +                     reg = <0x8f100000 0x4>;
>>> +             };
>>
>> Convince me that you need a node per register. This can all be
>> accomplished with a single node and either a single reg entry or a
>> series of reg entries.
>>
> In our system we have 6 different thermal zones. each node belongs to
> one thermal zone.
> thermal driver detects each node as separate thermal zone and monitors
> separately.
> register entry is only 4byte memory which contains the temperature
> value of respective thermal zone.
> Based on requirement we can increase or decrease the number thermal
> zones need to monitor.

Still, all this can be accomplished with a single node. Having a node
per register in DT doesn't scale.

Rob

Re: [PATCH 1/3] dt-bindings: thermal: Add binding document for SR thermal

[Srinath Mannam]

Hi Rob Herring,

Please find my comments in line..

On Fri, Jun 1, 2018 at 7:59 PM, Rob Herring <robh@kernel.org> wrote:
> On Fri, Jun 1, 2018 at 3:51 AM, Srinath Mannam
> <srinath.mannam@broadcom.com> wrote:
>> Hi Rob Herring,
>>
>> Thank you for the review.
>> Please find my answers inline..
>>
>> On Thu, May 31, 2018 at 10:18 PM, Rob Herring <robh@kernel.org> wrote:
>>> On Mon, May 28, 2018 at 11:11:22AM +0530, Srinath Mannam wrote:
>>>> From: Pramod Kumar <pramod.kumar@broadcom.com>
>>>>
>>>> Add binding document for supported thermal implementation
>>>> in Stingray.
>>>>
>>>> Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com>
>>>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>>>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>>>> Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.com>
>>>> ---
>>>>  .../bindings/thermal/brcm,sr-thermal.txt           | 45 ++++++++++++++++++++++
>>>>  1 file changed, 45 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>>>> new file mode 100644
>>>> index 0000000..33f9e11
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>>>> @@ -0,0 +1,45 @@
>>>> +* Broadcom Stingray Thermal
>>>> +
>>>> +This binding describes thermal sensors that is part of Stingray SoCs.
>>>> +
>>>> +Required properties:
>>>> +- compatible : Must be "brcm,sr-thermal"
>>>> +- reg : memory where tmon data will be available.
>>>
>>> What type of memory is this?
>> This is shared memory(cache-able) which is shared between two micro controllers.
>
> What else is in this shared memory? You should probably be
> representing that as a whole somewhere in DT.
Nothing else except 4 byte memory containing temperature value of one
thermal zone.
At present we configured 6 thermal zones. So six separate 4byte
memories provided in
shared memory.
>
>> One micro controller update temperature of all thermal zones in this
>> shared memory.
>> thermal driver running on another micro controller monitors
>> temperature data of all thermal zones.
>>>
>>>> +
>>>> +Example:
>>>> +     tmons {
>>>> +             compatible = "simple-bus";
>>>> +             #address-cells = <1>;
>>>> +             #size-cells = <1>;
>>>> +             ranges;
>>>> +
>>>> +             tmon_ihost0: thermal@8f100000 {
>>>> +                     compatible = "brcm,sr-thermal";
>>>> +                     reg = <0x8f100000 0x4>;
>>>> +             };
>>>
>>> Convince me that you need a node per register. This can all be
>>> accomplished with a single node and either a single reg entry or a
>>> series of reg entries.
>>>
>> In our system we have 6 different thermal zones. each node belongs to
>> one thermal zone.
>> thermal driver detects each node as separate thermal zone and monitors
>> separately.
>> register entry is only 4byte memory which contains the temperature
>> value of respective thermal zone.
>> Based on requirement we can increase or decrease the number thermal
>> zones need to monitor.
>
> Still, all this can be accomplished with a single node. Having a node
> per register in DT doesn't scale.
Our approach is like we have individual controllers have one driver
with multiple DT nodes.
In coming future we will add more thermal zones based on our
requirement. then we can just
add new DT node, instead of having driver changes.
With this approach we can configure each thermal zone at different trip values.
So we must have all thermal zones are independent.
>
> Rob

Re: [PATCH 1/3] dt-bindings: thermal: Add binding document for SR thermal

[Srinath Mannam]

Hi,

Please let me know if you have any comments to this patch.
If not can you please accept this patch.

Regards,
Srinath.

On Mon, Jun 4, 2018 at 11:36 AM, Srinath Mannam
<srinath.mannam@broadcom.com> wrote:
> Hi Rob Herring,
>
> Please find my comments in line..
>
> On Fri, Jun 1, 2018 at 7:59 PM, Rob Herring <robh@kernel.org> wrote:
>> On Fri, Jun 1, 2018 at 3:51 AM, Srinath Mannam
>> <srinath.mannam@broadcom.com> wrote:
>>> Hi Rob Herring,
>>>
>>> Thank you for the review.
>>> Please find my answers inline..
>>>
>>> On Thu, May 31, 2018 at 10:18 PM, Rob Herring <robh@kernel.org> wrote:
>>>> On Mon, May 28, 2018 at 11:11:22AM +0530, Srinath Mannam wrote:
>>>>> From: Pramod Kumar <pramod.kumar@broadcom.com>
>>>>>
>>>>> Add binding document for supported thermal implementation
>>>>> in Stingray.
>>>>>
>>>>> Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com>
>>>>> Reviewed-by: Ray Jui <ray.jui@broadcom.com>
>>>>> Reviewed-by: Scott Branden <scott.branden@broadcom.com>
>>>>> Reviewed-by: Srinath Mannam <srinath.mannam@broadcom.com>
>>>>> ---
>>>>>  .../bindings/thermal/brcm,sr-thermal.txt           | 45 ++++++++++++++++++++++
>>>>>  1 file changed, 45 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>>>>> new file mode 100644
>>>>> index 0000000..33f9e11
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
>>>>> @@ -0,0 +1,45 @@
>>>>> +* Broadcom Stingray Thermal
>>>>> +
>>>>> +This binding describes thermal sensors that is part of Stingray SoCs.
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible : Must be "brcm,sr-thermal"
>>>>> +- reg : memory where tmon data will be available.
>>>>
>>>> What type of memory is this?
>>> This is shared memory(cache-able) which is shared between two micro controllers.
>>
>> What else is in this shared memory? You should probably be
>> representing that as a whole somewhere in DT.
> Nothing else except 4 byte memory containing temperature value of one
> thermal zone.
> At present we configured 6 thermal zones. So six separate 4byte
> memories provided in
> shared memory.
>>
>>> One micro controller update temperature of all thermal zones in this
>>> shared memory.
>>> thermal driver running on another micro controller monitors
>>> temperature data of all thermal zones.
>>>>
>>>>> +
>>>>> +Example:
>>>>> +     tmons {
>>>>> +             compatible = "simple-bus";
>>>>> +             #address-cells = <1>;
>>>>> +             #size-cells = <1>;
>>>>> +             ranges;
>>>>> +
>>>>> +             tmon_ihost0: thermal@8f100000 {
>>>>> +                     compatible = "brcm,sr-thermal";
>>>>> +                     reg = <0x8f100000 0x4>;
>>>>> +             };
>>>>
>>>> Convince me that you need a node per register. This can all be
>>>> accomplished with a single node and either a single reg entry or a
>>>> series of reg entries.
>>>>
>>> In our system we have 6 different thermal zones. each node belongs to
>>> one thermal zone.
>>> thermal driver detects each node as separate thermal zone and monitors
>>> separately.
>>> register entry is only 4byte memory which contains the temperature
>>> value of respective thermal zone.
>>> Based on requirement we can increase or decrease the number thermal
>>> zones need to monitor.
>>
>> Still, all this can be accomplished with a single node. Having a node
>> per register in DT doesn't scale.
> Our approach is like we have individual controllers have one driver
> with multiple DT nodes.
> In coming future we will add more thermal zones based on our
> requirement. then we can just
> add new DT node, instead of having driver changes.
> With this approach we can configure each thermal zone at different trip values.
> So we must have all thermal zones are independent.
>>
>> Rob

Re: [PATCH 3/3] thermal: broadcom: Add Stingray thermal driver

[David Woodhouse]

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

On Mon, 2018-05-28 at 11:11 +0530, Srinath Mannam wrote:
> From: Pramod Kumar <pramod.kumar@broadcom.com>
> 
> This commit adds stingray thermal driver to monitor six
> thermal zones temperature and trips at critical temperature.

This matches an ACPI "BRCM0500" device but then calls
devm_thermal_zone_of_sensor_register(), which AFAICT is going to fail
on an ACPI system because the first thing that does is call
of_find_node_by_name(NULL, "thermal-zones") which isn't going to find a
match.

How does this work in the ACPI case?

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5174 bytes --]

Re: [PATCH 3/3] thermal: broadcom: Add Stingray thermal driver

[Sudeep Holla]

On Mon, Apr 29, 2019 at 06:07:12PM +0300, David Woodhouse wrote:
> On Mon, 2018-05-28 at 11:11 +0530, Srinath Mannam wrote:
> > From: Pramod Kumar <pramod.kumar@broadcom.com>
> >
> > This commit adds stingray thermal driver to monitor six
> > thermal zones temperature and trips at critical temperature.
>
> This matches an ACPI "BRCM0500" device but then calls
> devm_thermal_zone_of_sensor_register(), which AFAICT is going to fail
> on an ACPI system because the first thing that does is call
> of_find_node_by_name(NULL, "thermal-zones") which isn't going to find a
> match.
>

Thanks David for bringing this up. I hadn't noticed that this driver is
cheekily trying to do thermal management in ACPI using crafty
acpi_device_id match. ACPI thermal objects/methods must be used in the
firmware to do thermal management.

Pramod, can you remove the ACPI support or I can go ahead and post the
patch to do the same ?

> How does this work in the ACPI case?

It can't and shouldn't work if one can make it happen :)

--
Regards,
Sudeep

Re: [PATCH 3/3] thermal: broadcom: Add Stingray thermal driver

[Srinath Mannam]

Hi David,

Thank you for notifying..

Hi Sudeep,

I will send a patch to remove ACPI support.

Regards,
Srinath.

On Mon, Apr 29, 2019 at 8:54 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Mon, Apr 29, 2019 at 06:07:12PM +0300, David Woodhouse wrote:
> > On Mon, 2018-05-28 at 11:11 +0530, Srinath Mannam wrote:
> > > From: Pramod Kumar <pramod.kumar@broadcom.com>
> > >
> > > This commit adds stingray thermal driver to monitor six
> > > thermal zones temperature and trips at critical temperature.
> >
> > This matches an ACPI "BRCM0500" device but then calls
> > devm_thermal_zone_of_sensor_register(), which AFAICT is going to fail
> > on an ACPI system because the first thing that does is call
> > of_find_node_by_name(NULL, "thermal-zones") which isn't going to find a
> > match.
> >
>
> Thanks David for bringing this up. I hadn't noticed that this driver is
> cheekily trying to do thermal management in ACPI using crafty
> acpi_device_id match. ACPI thermal objects/methods must be used in the
> firmware to do thermal management.
>
> Pramod, can you remove the ACPI support or I can go ahead and post the
> patch to do the same ?
>
> > How does this work in the ACPI case?
>
> It can't and shouldn't work if one can make it happen :)
>
> --
> Regards,
> Sudeep

Re: [PATCH 3/3] thermal: broadcom: Add Stingray thermal driver

[Sudeep Holla]

On Mon, Apr 29, 2019 at 08:58:26PM +0530, Srinath Mannam wrote:
> Hi David,
>
> Thank you for notifying..
>
> Hi Sudeep,
>
> I will send a patch to remove ACPI support.
>

Thanks.

--
Regards,
Sudeep