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
From: Pramod Kumar <[email protected]>
Add binding document for supported thermal implementation
in Stingray.
Signed-off-by: Pramod Kumar <[email protected]>
Reviewed-by: Ray Jui <[email protected]>
Reviewed-by: Scott Branden <[email protected]>
Reviewed-by: Srinath Mannam <[email protected]>
---
.../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
From: Pramod Kumar <[email protected]>
This commit adds stingray thermal driver to monitor six
thermal zones temperature and trips at critical temperature.
Signed-off-by: Pramod Kumar <[email protected]>
Signed-off-by: Srinath Mannam <[email protected]>
Reviewed-by: Ray Jui <[email protected]>
Reviewed-by: Scott Branden <[email protected]>
Reviewed-by: Vikram Prakash <[email protected]>
---
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 <[email protected]>");
+MODULE_DESCRIPTION("Stingray thermal driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4
From: Pramod Kumar <[email protected]>
Add DT nodes for thermal zones memory base address
to read temperature.
Signed-off-by: Pramod Kumar <[email protected]>
Reviewed-by: Ray Jui <[email protected]>
Reviewed-by: Scott Branden <[email protected]>
Reviewed-by: Srinath Mannam <[email protected]>
---
.../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
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
On Mon, May 28, 2018 at 11:11:22AM +0530, Srinath Mannam wrote:
> From: Pramod Kumar <[email protected]>
>
> Add binding document for supported thermal implementation
> in Stingray.
>
> Signed-off-by: Pramod Kumar <[email protected]>
> Reviewed-by: Ray Jui <[email protected]>
> Reviewed-by: Scott Branden <[email protected]>
> Reviewed-by: Srinath Mannam <[email protected]>
> ---
> .../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
>
Hi Rob Herring,
Thank you for the review.
Please find my answers inline..
On Thu, May 31, 2018 at 10:18 PM, Rob Herring <[email protected]> wrote:
> On Mon, May 28, 2018 at 11:11:22AM +0530, Srinath Mannam wrote:
>> From: Pramod Kumar <[email protected]>
>>
>> Add binding document for supported thermal implementation
>> in Stingray.
>>
>> Signed-off-by: Pramod Kumar <[email protected]>
>> Reviewed-by: Ray Jui <[email protected]>
>> Reviewed-by: Scott Branden <[email protected]>
>> Reviewed-by: Srinath Mannam <[email protected]>
>> ---
>> .../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
>>
On Fri, Jun 1, 2018 at 3:51 AM, Srinath Mannam
<[email protected]> 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 <[email protected]> wrote:
>> On Mon, May 28, 2018 at 11:11:22AM +0530, Srinath Mannam wrote:
>>> From: Pramod Kumar <[email protected]>
>>>
>>> Add binding document for supported thermal implementation
>>> in Stingray.
>>>
>>> Signed-off-by: Pramod Kumar <[email protected]>
>>> Reviewed-by: Ray Jui <[email protected]>
>>> Reviewed-by: Scott Branden <[email protected]>
>>> Reviewed-by: Srinath Mannam <[email protected]>
>>> ---
>>> .../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
Hi Rob Herring,
Please find my comments in line..
On Fri, Jun 1, 2018 at 7:59 PM, Rob Herring <[email protected]> wrote:
> On Fri, Jun 1, 2018 at 3:51 AM, Srinath Mannam
> <[email protected]> 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 <[email protected]> wrote:
>>> On Mon, May 28, 2018 at 11:11:22AM +0530, Srinath Mannam wrote:
>>>> From: Pramod Kumar <[email protected]>
>>>>
>>>> Add binding document for supported thermal implementation
>>>> in Stingray.
>>>>
>>>> Signed-off-by: Pramod Kumar <[email protected]>
>>>> Reviewed-by: Ray Jui <[email protected]>
>>>> Reviewed-by: Scott Branden <[email protected]>
>>>> Reviewed-by: Srinath Mannam <[email protected]>
>>>> ---
>>>> .../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
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
<[email protected]> wrote:
> Hi Rob Herring,
>
> Please find my comments in line..
>
> On Fri, Jun 1, 2018 at 7:59 PM, Rob Herring <[email protected]> wrote:
>> On Fri, Jun 1, 2018 at 3:51 AM, Srinath Mannam
>> <[email protected]> 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 <[email protected]> wrote:
>>>> On Mon, May 28, 2018 at 11:11:22AM +0530, Srinath Mannam wrote:
>>>>> From: Pramod Kumar <[email protected]>
>>>>>
>>>>> Add binding document for supported thermal implementation
>>>>> in Stingray.
>>>>>
>>>>> Signed-off-by: Pramod Kumar <[email protected]>
>>>>> Reviewed-by: Ray Jui <[email protected]>
>>>>> Reviewed-by: Scott Branden <[email protected]>
>>>>> Reviewed-by: Srinath Mannam <[email protected]>
>>>>> ---
>>>>> .../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
On Mon, 2018-05-28 at 11:11 +0530, Srinath Mannam wrote:
> From: Pramod Kumar <[email protected]>
>
> 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?
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 <[email protected]>
> >
> > 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
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 <[email protected]> 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 <[email protected]>
> > >
> > > 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
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