* [PATCH v3 0/3] Stingray thermal driver support
@ 2018-08-09 12:54 Srinath Mannam
2018-08-09 12:54 ` [PATCH v3 1/3] dt-bindings: thermal: Add binding document for SR thermal Srinath Mannam
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Srinath Mannam @ 2018-08-09 12:54 UTC (permalink / raw)
To: Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland
Cc: devicetree, linux-kernel, bcm-kernel-feedback-list, Srinath Mannam
These patches adds the stingray thermal driver and its
corresponding DT nodes with documentation.
Changes from v2:
- All stingray TMON DT nodes are combine together into single.
Temperature registers are combined into one mem resource.
brcm,tmon-mask parameter has available TMONs mask value.
- All available TMONs are initialized together in single
instance of driver probe call.
Changes from v1:
- Fixed auto build sparce warning.
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 | 23 +++
.../arm64/boot/dts/broadcom/stingray/stingray.dtsi | 13 ++
drivers/thermal/Kconfig | 3 +-
drivers/thermal/broadcom/Kconfig | 9 +
drivers/thermal/broadcom/Makefile | 1 +
drivers/thermal/broadcom/sr-thermal.c | 216 +++++++++++++++++++++
6 files changed, 264 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 1/3] dt-bindings: thermal: Add binding document for SR thermal
2018-08-09 12:54 [PATCH v3 0/3] Stingray thermal driver support Srinath Mannam
@ 2018-08-09 12:54 ` Srinath Mannam
2018-08-13 17:34 ` Rob Herring
2018-08-09 12:54 ` [PATCH v3 2/3] arm64: dts: stingray: Add Stingray Thermal DT support Srinath Mannam
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Srinath Mannam @ 2018-08-09 12:54 UTC (permalink / raw)
To: Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland
Cc: devicetree, linux-kernel, bcm-kernel-feedback-list, Pramod Kumar,
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>
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>
---
.../bindings/thermal/brcm,sr-thermal.txt | 23 ++++++++++++++++++++++
1 file changed, 23 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..fc9e6d3
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
@@ -0,0 +1,23 @@
+* 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.
+- brcm,tmon-mask: A one cell bit mask of valid TMON sources.
+ Each bit represents single TMON source.
+
+Example:
+ tmons {
+ compatible = "simple-bus";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0x0 0x0 0x8f100000 0x100>;
+
+ tmon: thermal@0 {
+ compatible = "brcm,sr-thermal";
+ reg = <0 0x40>;
+ brcm,tmon-mask = <0x3f>;
+ };
+ };
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 2/3] arm64: dts: stingray: Add Stingray Thermal DT support.
2018-08-09 12:54 [PATCH v3 0/3] Stingray thermal driver support Srinath Mannam
2018-08-09 12:54 ` [PATCH v3 1/3] dt-bindings: thermal: Add binding document for SR thermal Srinath Mannam
@ 2018-08-09 12:54 ` Srinath Mannam
2018-08-09 12:54 ` [PATCH v3 3/3] thermal: broadcom: Add Stingray thermal driver Srinath Mannam
2018-09-13 22:25 ` [PATCH v3 0/3] Stingray thermal driver support Florian Fainelli
3 siblings, 0 replies; 9+ messages in thread
From: Srinath Mannam @ 2018-08-09 12:54 UTC (permalink / raw)
To: Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland
Cc: devicetree, linux-kernel, bcm-kernel-feedback-list, Pramod Kumar,
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>
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>
---
arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
index b203152..eeccf94 100644
--- a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
+++ b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi
@@ -593,4 +593,17 @@
status = "disabled";
};
};
+
+ tmons {
+ compatible = "simple-bus";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0x0 0x0 0x8f100000 0x100>;
+
+ tmon: thermal@0 {
+ compatible = "brcm,sr-thermal";
+ reg = <0x0 0x40>;
+ brcm,tmon-mask = <0x3f>;
+ };
+ };
};
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 3/3] thermal: broadcom: Add Stingray thermal driver
2018-08-09 12:54 [PATCH v3 0/3] Stingray thermal driver support Srinath Mannam
2018-08-09 12:54 ` [PATCH v3 1/3] dt-bindings: thermal: Add binding document for SR thermal Srinath Mannam
2018-08-09 12:54 ` [PATCH v3 2/3] arm64: dts: stingray: Add Stingray Thermal DT support Srinath Mannam
@ 2018-08-09 12:54 ` Srinath Mannam
2018-09-14 14:20 ` Daniel Lezcano
2018-09-13 22:25 ` [PATCH v3 0/3] Stingray thermal driver support Florian Fainelli
3 siblings, 1 reply; 9+ messages in thread
From: Srinath Mannam @ 2018-08-09 12:54 UTC (permalink / raw)
To: Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland
Cc: devicetree, linux-kernel, bcm-kernel-feedback-list, Pramod Kumar,
Srinath Mannam
From: Pramod Kumar <pramod.kumar@broadcom.com>
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 | 216 ++++++++++++++++++++++++++++++++++
4 files changed, 228 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..909f80c
--- /dev/null
+++ b/drivers/thermal/broadcom/sr-thermal.c
@@ -0,0 +1,216 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018 Broadcom
+ */
+
+#include <linux/acpi.h>
+#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 */
+#define SR_TMON_MAX_LIST 6
+
+/*
+ * In stingray thermal IO memory,
+ * Total Number of available TMONs MASK is at offset 0
+ * temperature registers BASE is at 4 byte offset.
+ * Each TMON temperature register size is 4.
+ */
+#define SR_TMON_TEMP_BASE(id) ((id) * 0x4)
+
+static const char * const sr_tmon_names[SR_TMON_MAX_LIST] = {
+ "sr_tmon_ihost0",
+ "sr_tmon_ihost1",
+ "sr_tmon_ihost2",
+ "sr_tmon_ihost3",
+ "sr_tmon_crmu",
+ "sr_tmon_nitro",
+};
+
+struct sr_tmon {
+ struct thermal_zone_device *tz;
+ unsigned int crit_temp;
+ unsigned int tmon_id;
+ struct sr_thermal *priv;
+};
+
+struct sr_thermal {
+ struct device *dev;
+ void __iomem *regs;
+ struct sr_tmon tmon[SR_TMON_MAX_LIST];
+};
+
+static int sr_get_temp(struct thermal_zone_device *tz, int *temp)
+{
+ struct sr_tmon *tmon = tz->devdata;
+ struct sr_thermal *sr_thermal = tmon->priv;
+
+ *temp = readl(sr_thermal->regs + SR_TMON_TEMP_BASE(tmon->tmon_id));
+
+ return 0;
+}
+
+static int sr_get_trip_type(struct thermal_zone_device *tz, int trip,
+ enum thermal_trip_type *type)
+{
+ struct sr_tmon *tmon = tz->devdata;
+ struct sr_thermal *sr_thermal = tmon->priv;
+
+ 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_tmon *tmon = tz->devdata;
+ struct sr_thermal *sr_thermal = tmon->priv;
+
+ switch (trip) {
+ case 0:
+ *temp = tmon->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_tmon *tmon = 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.
+ */
+ tmon->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 sr_tmon *tmon;
+ struct resource *res;
+ uint32_t sr_tmon_list = 0;
+ unsigned int i;
+ int ret;
+
+ 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 = (void __iomem *)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);
+ }
+
+ ret = device_property_read_u32(dev, "brcm,tmon-mask", &sr_tmon_list);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < SR_TMON_MAX_LIST; i++) {
+
+ if (!(sr_tmon_list & BIT(i)))
+ continue;
+
+ /* Flush temperature registers */
+ writel(0, sr_thermal->regs + SR_TMON_TEMP_BASE(i));
+ tmon = &sr_thermal->tmon[i];
+ tmon->crit_temp = TMON_CRIT_TEMP;
+ tmon->tmon_id = i;
+ tmon->priv = sr_thermal;
+ tmon->tz = thermal_zone_device_register(sr_tmon_names[i],
+ 1, 1,
+ tmon,
+ &sr_thermal_ops,
+ NULL, 1000, 1000);
+ if (IS_ERR(tmon->tz))
+ goto err_exit;
+
+ dev_info(dev, "%s: registered\n", sr_tmon_names[i]);
+ }
+ platform_set_drvdata(pdev, sr_thermal);
+
+ return 0;
+
+err_exit:
+ while (--i >= 0) {
+ if (sr_thermal->tmon[i].tz)
+ thermal_zone_device_unregister(sr_thermal->tmon[i].tz);
+ }
+
+ return PTR_ERR(tmon->tz);
+}
+
+static int sr_thermal_remove(struct platform_device *pdev)
+{
+ struct sr_thermal *sr_thermal = platform_get_drvdata(pdev);
+ unsigned int i;
+
+ for (i = 0; i < SR_TMON_MAX_LIST; i++)
+ if (sr_thermal->tmon[i].tz)
+ thermal_zone_device_unregister(sr_thermal->tmon[i].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 const struct acpi_device_id sr_thermal_acpi_ids[] = {
+ { .id = "BRCM0500" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(acpi, sr_thermal_acpi_ids);
+
+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,
+ .acpi_match_table = ACPI_PTR(sr_thermal_acpi_ids),
+ },
+};
+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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: thermal: Add binding document for SR thermal
2018-08-09 12:54 ` [PATCH v3 1/3] dt-bindings: thermal: Add binding document for SR thermal Srinath Mannam
@ 2018-08-13 17:34 ` Rob Herring
0 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2018-08-13 17:34 UTC (permalink / raw)
To: Srinath Mannam
Cc: Zhang Rui, Eduardo Valentin, Mark Rutland, devicetree,
linux-kernel, bcm-kernel-feedback-list, Pramod Kumar
On Thu, Aug 09, 2018 at 06:24:57PM +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>
> 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>
> ---
> .../bindings/thermal/brcm,sr-thermal.txt | 23 ++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/3] Stingray thermal driver support
2018-08-09 12:54 [PATCH v3 0/3] Stingray thermal driver support Srinath Mannam
` (2 preceding siblings ...)
2018-08-09 12:54 ` [PATCH v3 3/3] thermal: broadcom: Add Stingray thermal driver Srinath Mannam
@ 2018-09-13 22:25 ` Florian Fainelli
3 siblings, 0 replies; 9+ messages in thread
From: Florian Fainelli @ 2018-09-13 22:25 UTC (permalink / raw)
To: Srinath Mannam, Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland
Cc: devicetree, linux-kernel, bcm-kernel-feedback-list
On 08/09/18 05:54, Srinath Mannam wrote:
> These patches adds the stingray thermal driver and its
> corresponding DT nodes with documentation.
>
> Changes from v2:
> - All stingray TMON DT nodes are combine together into single.
> Temperature registers are combined into one mem resource.
> brcm,tmon-mask parameter has available TMONs mask value.
> - All available TMONs are initialized together in single
> instance of driver probe call.
>
> Changes from v1:
> - Fixed auto build sparce warning.
Thermal maintainers, can you please review patch #3? The binding and DTS
changes can go through arm-soc if you would like, or you can take
patches 1 and 3 and I take patch 2, either way is fine.
Thank you
>
> 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 | 23 +++
> .../arm64/boot/dts/broadcom/stingray/stingray.dtsi | 13 ++
> drivers/thermal/Kconfig | 3 +-
> drivers/thermal/broadcom/Kconfig | 9 +
> drivers/thermal/broadcom/Makefile | 1 +
> drivers/thermal/broadcom/sr-thermal.c | 216 +++++++++++++++++++++
> 6 files changed, 264 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
> create mode 100644 drivers/thermal/broadcom/sr-thermal.c
>
--
Florian
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 3/3] thermal: broadcom: Add Stingray thermal driver
2018-08-09 12:54 ` [PATCH v3 3/3] thermal: broadcom: Add Stingray thermal driver Srinath Mannam
@ 2018-09-14 14:20 ` Daniel Lezcano
2018-09-17 3:50 ` Srinath Mannam
2018-09-17 6:53 ` Pramod Kumar
0 siblings, 2 replies; 9+ messages in thread
From: Daniel Lezcano @ 2018-09-14 14:20 UTC (permalink / raw)
To: Srinath Mannam, Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland
Cc: devicetree, linux-kernel, bcm-kernel-feedback-list, Pramod Kumar
On 09/08/2018 14:54, Srinath Mannam wrote:
> From: Pramod Kumar <pramod.kumar@broadcom.com>
>
> Adds stingray thermal driver to monitor six
> thermal zones temperature and trips at critical temperature.
Hi Pramod,
could you elaborate a bit more the description? As you are introducing a
new driver it would be nice to give the sensor details.
> 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 | 216 ++++++++++++++++++++++++++++++++++
> 4 files changed, 228 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..909f80c
> --- /dev/null
> +++ b/drivers/thermal/broadcom/sr-thermal.c
> @@ -0,0 +1,216 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 Broadcom
> + */
> +
> +#include <linux/acpi.h>
> +#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 */
I suggest to move this in the DT?
> +#define SR_TMON_MAX_LIST 6
> +
> +/*
> + * In stingray thermal IO memory,
> + * Total Number of available TMONs MASK is at offset 0
> + * temperature registers BASE is at 4 byte offset.
> + * Each TMON temperature register size is 4.
> + */
> +#define SR_TMON_TEMP_BASE(id) ((id) * 0x4)
> +
> +static const char * const sr_tmon_names[SR_TMON_MAX_LIST] = {
It will be more elegant to replace the macro SR_TMON_MAX_LIST by
ARRAY_SIZE(sr_tmon_names) and declare this array as:
static const char *const sr_tmon_name[] = {
...
};
> + "sr_tmon_ihost0",
> + "sr_tmon_ihost1",
> + "sr_tmon_ihost2",
> + "sr_tmon_ihost3",
> + "sr_tmon_crmu",
> + "sr_tmon_nitro",
> +};
> +
> +struct sr_tmon {
> + struct thermal_zone_device *tz;
> + unsigned int crit_temp;
> + unsigned int tmon_id;
> + struct sr_thermal *priv;
> +};
> +
> +struct sr_thermal {
> + struct device *dev;
This field is used for dev_dbg, may be it could be removed along with
the dev_dbg message?
> + void __iomem *regs;
> + struct sr_tmon tmon[SR_TMON_MAX_LIST];
> +};
> +
> +static int sr_get_temp(struct thermal_zone_device *tz, int *temp)
> +{
> + struct sr_tmon *tmon = tz->devdata;
> + struct sr_thermal *sr_thermal = tmon->priv;
> +
> + *temp = readl(sr_thermal->regs + SR_TMON_TEMP_BASE(tmon->tmon_id));
> +
> + return 0;
> +}
> +
> +static int sr_get_trip_type(struct thermal_zone_device *tz, int trip,
> + enum thermal_trip_type *type)
> +{
> + struct sr_tmon *tmon = tz->devdata;
> + struct sr_thermal *sr_thermal = tmon->priv;
> +
> + 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_tmon *tmon = tz->devdata;
> + struct sr_thermal *sr_thermal = tmon->priv;
> +
> + switch (trip) {
> + case 0:
> + *temp = tmon->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_tmon *tmon = 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.
> + */
Couldn't the user harm the hardware with a too high value?
Why not define this value in the DT?
> + tmon->crit_temp = temp;
> + break;
> + default:
> + return -EINVAL;
> + }
> + return 0;
> +}
Is it possible to factor out these 3 functions above and remove the
'switch' in all of them ?
> +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 sr_tmon *tmon;
> + struct resource *res;
> + uint32_t sr_tmon_list = 0;
> + unsigned int i;
> + int ret;
> +
> + 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 = (void __iomem *)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);
> + }
> +
> + ret = device_property_read_u32(dev, "brcm,tmon-mask", &sr_tmon_list);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < SR_TMON_MAX_LIST; i++) {
> +
> + if (!(sr_tmon_list & BIT(i)))
> + continue;
> +
> + /* Flush temperature registers */
> + writel(0, sr_thermal->regs + SR_TMON_TEMP_BASE(i));
> + tmon = &sr_thermal->tmon[i];
It is possible to initialize tmon to:
tmon = &sr_thermal->tmon;
and then use the pointer increment:
tmon++;
> + tmon->crit_temp = TMON_CRIT_TEMP;
> + tmon->tmon_id = i;
> + tmon->priv = sr_thermal;
> + tmon->tz = thermal_zone_device_register(sr_tmon_names[i],
> + 1, 1,
> + tmon,
> + &sr_thermal_ops,
> + NULL, 1000, 1000);
> + if (IS_ERR(tmon->tz))
> + goto err_exit;
> +
> + dev_info(dev, "%s: registered\n", sr_tmon_names[i]);
> + }
> + platform_set_drvdata(pdev, sr_thermal);
> +
> + return 0;
> +
> +err_exit:
> + while (--i >= 0) {
> + if (sr_thermal->tmon[i].tz)
> + thermal_zone_device_unregister(sr_thermal->tmon[i].tz);
> + }
> +
> + return PTR_ERR(tmon->tz);
> +}
> +
> +static int sr_thermal_remove(struct platform_device *pdev)
> +{
> + struct sr_thermal *sr_thermal = platform_get_drvdata(pdev);
> + unsigned int i;
> +
> + for (i = 0; i < SR_TMON_MAX_LIST; i++)
> + if (sr_thermal->tmon[i].tz)
> + thermal_zone_device_unregister(sr_thermal->tmon[i].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 const struct acpi_device_id sr_thermal_acpi_ids[] = {
> + { .id = "BRCM0500" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(acpi, sr_thermal_acpi_ids);
> +
> +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,
> + .acpi_match_table = ACPI_PTR(sr_thermal_acpi_ids),
> + },
> +};
> +module_platform_driver(sr_thermal_driver);
> +
> +MODULE_AUTHOR("Pramod Kumar <pramod.kumar@broadcom.com>");
> +MODULE_DESCRIPTION("Stingray thermal driver");
> +MODULE_LICENSE("GPL v2");
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/3] thermal: broadcom: Add Stingray thermal driver
2018-09-14 14:20 ` Daniel Lezcano
@ 2018-09-17 3:50 ` Srinath Mannam
2018-09-17 6:53 ` Pramod Kumar
1 sibling, 0 replies; 9+ messages in thread
From: Srinath Mannam @ 2018-09-17 3:50 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland,
devicetree, Linux Kernel Mailing List, BCM Kernel Feedback,
Pramod Kumar, Vikram Prakash
Hi Daniel,
Thank you for your review comments.
I will address all your comments in next patch set.
Regards,
Srinath.
On Fri, Sep 14, 2018 at 7:50 PM, Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
> On 09/08/2018 14:54, Srinath Mannam wrote:
>> From: Pramod Kumar <pramod.kumar@broadcom.com>
>>
>> Adds stingray thermal driver to monitor six
>> thermal zones temperature and trips at critical temperature.
>
> Hi Pramod,
>
> could you elaborate a bit more the description? As you are introducing a
> new driver it would be nice to give the sensor details.
>
>> 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 | 216 ++++++++++++++++++++++++++++++++++
>> 4 files changed, 228 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..909f80c
>> --- /dev/null
>> +++ b/drivers/thermal/broadcom/sr-thermal.c
>> @@ -0,0 +1,216 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2018 Broadcom
>> + */
>> +
>> +#include <linux/acpi.h>
>> +#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 */
>
> I suggest to move this in the DT?
>
>> +#define SR_TMON_MAX_LIST 6
>> +
>> +/*
>> + * In stingray thermal IO memory,
>> + * Total Number of available TMONs MASK is at offset 0
>> + * temperature registers BASE is at 4 byte offset.
>> + * Each TMON temperature register size is 4.
>> + */
>> +#define SR_TMON_TEMP_BASE(id) ((id) * 0x4)
>> +
>> +static const char * const sr_tmon_names[SR_TMON_MAX_LIST] = {
>
> It will be more elegant to replace the macro SR_TMON_MAX_LIST by
> ARRAY_SIZE(sr_tmon_names) and declare this array as:
>
> static const char *const sr_tmon_name[] = {
> ...
> };
>
>> + "sr_tmon_ihost0",
>> + "sr_tmon_ihost1",
>> + "sr_tmon_ihost2",
>> + "sr_tmon_ihost3",
>> + "sr_tmon_crmu",
>> + "sr_tmon_nitro",
>> +};
>> +
>> +struct sr_tmon {
>> + struct thermal_zone_device *tz;
>> + unsigned int crit_temp;
>> + unsigned int tmon_id;
>> + struct sr_thermal *priv;
>> +};
>> +
>> +struct sr_thermal {
>> + struct device *dev;
>
> This field is used for dev_dbg, may be it could be removed along with
> the dev_dbg message?
>
>> + void __iomem *regs;
>> + struct sr_tmon tmon[SR_TMON_MAX_LIST];
>> +};
>> +
>> +static int sr_get_temp(struct thermal_zone_device *tz, int *temp)
>> +{
>> + struct sr_tmon *tmon = tz->devdata;
>> + struct sr_thermal *sr_thermal = tmon->priv;
>> +
>> + *temp = readl(sr_thermal->regs + SR_TMON_TEMP_BASE(tmon->tmon_id));
>> +
>> + return 0;
>> +}
>> +
>> +static int sr_get_trip_type(struct thermal_zone_device *tz, int trip,
>> + enum thermal_trip_type *type)
>> +{
>> + struct sr_tmon *tmon = tz->devdata;
>> + struct sr_thermal *sr_thermal = tmon->priv;
>> +
>> + 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_tmon *tmon = tz->devdata;
>> + struct sr_thermal *sr_thermal = tmon->priv;
>> +
>> + switch (trip) {
>> + case 0:
>> + *temp = tmon->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_tmon *tmon = 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.
>> + */
>
> Couldn't the user harm the hardware with a too high value?
>
> Why not define this value in the DT?
>
>> + tmon->crit_temp = temp;
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> + return 0;
>> +}
>
> Is it possible to factor out these 3 functions above and remove the
> 'switch' in all of them ?
>
>> +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 sr_tmon *tmon;
>> + struct resource *res;
>> + uint32_t sr_tmon_list = 0;
>> + unsigned int i;
>> + int ret;
>> +
>> + 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 = (void __iomem *)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);
>> + }
>> +
>> + ret = device_property_read_u32(dev, "brcm,tmon-mask", &sr_tmon_list);
>> + if (ret)
>> + return ret;
>> +
>> + for (i = 0; i < SR_TMON_MAX_LIST; i++) {
>> +
>> + if (!(sr_tmon_list & BIT(i)))
>> + continue;
>> +
>> + /* Flush temperature registers */
>> + writel(0, sr_thermal->regs + SR_TMON_TEMP_BASE(i));
>> + tmon = &sr_thermal->tmon[i];
>
> It is possible to initialize tmon to:
>
> tmon = &sr_thermal->tmon;
>
> and then use the pointer increment:
> tmon++;
>
>
>> + tmon->crit_temp = TMON_CRIT_TEMP;
>> + tmon->tmon_id = i;
>> + tmon->priv = sr_thermal;
>> + tmon->tz = thermal_zone_device_register(sr_tmon_names[i],
>> + 1, 1,
>> + tmon,
>> + &sr_thermal_ops,
>> + NULL, 1000, 1000);
>> + if (IS_ERR(tmon->tz))
>> + goto err_exit;
>> +
>> + dev_info(dev, "%s: registered\n", sr_tmon_names[i]);
>> + }
>> + platform_set_drvdata(pdev, sr_thermal);
>> +
>> + return 0;
>> +
>> +err_exit:
>> + while (--i >= 0) {
>> + if (sr_thermal->tmon[i].tz)
>> + thermal_zone_device_unregister(sr_thermal->tmon[i].tz);
>> + }
>> +
>> + return PTR_ERR(tmon->tz);
>> +}
>> +
>> +static int sr_thermal_remove(struct platform_device *pdev)
>> +{
>> + struct sr_thermal *sr_thermal = platform_get_drvdata(pdev);
>> + unsigned int i;
>> +
>> + for (i = 0; i < SR_TMON_MAX_LIST; i++)
>> + if (sr_thermal->tmon[i].tz)
>> + thermal_zone_device_unregister(sr_thermal->tmon[i].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 const struct acpi_device_id sr_thermal_acpi_ids[] = {
>> + { .id = "BRCM0500" },
>> + { /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(acpi, sr_thermal_acpi_ids);
>> +
>> +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,
>> + .acpi_match_table = ACPI_PTR(sr_thermal_acpi_ids),
>> + },
>> +};
>> +module_platform_driver(sr_thermal_driver);
>> +
>> +MODULE_AUTHOR("Pramod Kumar <pramod.kumar@broadcom.com>");
>> +MODULE_DESCRIPTION("Stingray thermal driver");
>> +MODULE_LICENSE("GPL v2");
>>
>
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/3] thermal: broadcom: Add Stingray thermal driver
2018-09-14 14:20 ` Daniel Lezcano
2018-09-17 3:50 ` Srinath Mannam
@ 2018-09-17 6:53 ` Pramod Kumar
1 sibling, 0 replies; 9+ messages in thread
From: Pramod Kumar @ 2018-09-17 6:53 UTC (permalink / raw)
To: daniel.lezcano
Cc: Srinath (Srinath Babu) Mannam, rui.zhang, edubezval, Rob Herring,
Mark Rutland, devicetree, linux-kernel, BCM Kernel Feedback
[-- Attachment #1: Type: text/plain, Size: 12118 bytes --]
On Fri, Sep 14, 2018 at 7:51 PM Daniel Lezcano <daniel.lezcano@linaro.org>
wrote:
> On 09/08/2018 14:54, Srinath Mannam wrote:
> > From: Pramod Kumar <pramod.kumar@broadcom.com>
> >
> > Adds stingray thermal driver to monitor six
> > thermal zones temperature and trips at critical temperature.
>
> Hi Pramod,
>
> could you elaborate a bit more the description? As you are introducing a
> new driver it would be nice to give the sensor details.
>
Hi Denial,
Our SoC has six temperature sensor which gets configured/initialized and
controlled by m0 processor, running in secure zone. This processor sample
the sensors values and and populates in DDR memory to be read by other
agents like Linux. Since Sensors are not in direct control of Linux hence
sensor details has not been added here. Apart from this, In case Linux
fails to take action because of some unexpected reason, m0 Processor will
takes action to save the SoCs from being burned out.
Regards,
Pramod
>
> > 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 | 216
> ++++++++++++++++++++++++++++++++++
> > 4 files changed, 228 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..909f80c
> > --- /dev/null
> > +++ b/drivers/thermal/broadcom/sr-thermal.c
> > @@ -0,0 +1,216 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2018 Broadcom
> > + */
> > +
> > +#include <linux/acpi.h>
> > +#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 */
>
> I suggest to move this in the DT?
>
> > +#define SR_TMON_MAX_LIST 6
> > +
> > +/*
> > + * In stingray thermal IO memory,
> > + * Total Number of available TMONs MASK is at offset 0
> > + * temperature registers BASE is at 4 byte offset.
> > + * Each TMON temperature register size is 4.
> > + */
> > +#define SR_TMON_TEMP_BASE(id) ((id) * 0x4)
> > +
> > +static const char * const sr_tmon_names[SR_TMON_MAX_LIST] = {
>
> It will be more elegant to replace the macro SR_TMON_MAX_LIST by
> ARRAY_SIZE(sr_tmon_names) and declare this array as:
>
> static const char *const sr_tmon_name[] = {
> ...
> };
>
> > + "sr_tmon_ihost0",
> > + "sr_tmon_ihost1",
> > + "sr_tmon_ihost2",
> > + "sr_tmon_ihost3",
> > + "sr_tmon_crmu",
> > + "sr_tmon_nitro",
> > +};
> > +
> > +struct sr_tmon {
> > + struct thermal_zone_device *tz;
> > + unsigned int crit_temp;
> > + unsigned int tmon_id;
> > + struct sr_thermal *priv;
> > +};
> > +
> > +struct sr_thermal {
> > + struct device *dev;
>
> This field is used for dev_dbg, may be it could be removed along with
> the dev_dbg message?
>
> > + void __iomem *regs;
> > + struct sr_tmon tmon[SR_TMON_MAX_LIST];
> > +};
> > +
> > +static int sr_get_temp(struct thermal_zone_device *tz, int *temp)
> > +{
> > + struct sr_tmon *tmon = tz->devdata;
> > + struct sr_thermal *sr_thermal = tmon->priv;
> > +
> > + *temp = readl(sr_thermal->regs + SR_TMON_TEMP_BASE(tmon->tmon_id));
> > +
> > + return 0;
> > +}
> > +
> > +static int sr_get_trip_type(struct thermal_zone_device *tz, int trip,
> > + enum thermal_trip_type *type)
> > +{
> > + struct sr_tmon *tmon = tz->devdata;
> > + struct sr_thermal *sr_thermal = tmon->priv;
> > +
> > + 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_tmon *tmon = tz->devdata;
> > + struct sr_thermal *sr_thermal = tmon->priv;
> > +
> > + switch (trip) {
> > + case 0:
> > + *temp = tmon->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_tmon *tmon = 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.
> > + */
>
> Couldn't the user harm the hardware with a too high value?
>
> Why not define this value in the DT?
>
> > + tmon->crit_temp = temp;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > + return 0;
> > +}
>
> Is it possible to factor out these 3 functions above and remove the
> 'switch' in all of them ?
>
> > +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 sr_tmon *tmon;
> > + struct resource *res;
> > + uint32_t sr_tmon_list = 0;
> > + unsigned int i;
> > + int ret;
> > +
> > + 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 = (void __iomem *)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);
> > + }
> > +
> > + ret = device_property_read_u32(dev, "brcm,tmon-mask",
> &sr_tmon_list);
> > + if (ret)
> > + return ret;
> > +
> > + for (i = 0; i < SR_TMON_MAX_LIST; i++) {
> > +
> > + if (!(sr_tmon_list & BIT(i)))
> > + continue;
> > +
> > + /* Flush temperature registers */
> > + writel(0, sr_thermal->regs + SR_TMON_TEMP_BASE(i));
> > + tmon = &sr_thermal->tmon[i];
>
> It is possible to initialize tmon to:
>
> tmon = &sr_thermal->tmon;
>
> and then use the pointer increment:
> tmon++;
>
>
> > + tmon->crit_temp = TMON_CRIT_TEMP;
> > + tmon->tmon_id = i;
> > + tmon->priv = sr_thermal;
> > + tmon->tz = thermal_zone_device_register(sr_tmon_names[i],
> > + 1, 1,
> > + tmon,
> > + &sr_thermal_ops,
> > + NULL, 1000, 1000);
> > + if (IS_ERR(tmon->tz))
> > + goto err_exit;
> > +
> > + dev_info(dev, "%s: registered\n", sr_tmon_names[i]);
> > + }
> > + platform_set_drvdata(pdev, sr_thermal);
> > +
> > + return 0;
> > +
> > +err_exit:
> > + while (--i >= 0) {
> > + if (sr_thermal->tmon[i].tz)
> > +
> thermal_zone_device_unregister(sr_thermal->tmon[i].tz);
> > + }
> > +
> > + return PTR_ERR(tmon->tz);
> > +}
> > +
> > +static int sr_thermal_remove(struct platform_device *pdev)
> > +{
> > + struct sr_thermal *sr_thermal = platform_get_drvdata(pdev);
> > + unsigned int i;
> > +
> > + for (i = 0; i < SR_TMON_MAX_LIST; i++)
> > + if (sr_thermal->tmon[i].tz)
> > +
> thermal_zone_device_unregister(sr_thermal->tmon[i].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 const struct acpi_device_id sr_thermal_acpi_ids[] = {
> > + { .id = "BRCM0500" },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(acpi, sr_thermal_acpi_ids);
> > +
> > +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,
> > + .acpi_match_table = ACPI_PTR(sr_thermal_acpi_ids),
> > + },
> > +};
> > +module_platform_driver(sr_thermal_driver);
> > +
> > +MODULE_AUTHOR("Pramod Kumar <pramod.kumar@broadcom.com>");
> > +MODULE_DESCRIPTION("Stingray thermal driver");
> > +MODULE_LICENSE("GPL v2");
> >
>
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
>
>
[-- Attachment #2: Type: text/html, Size: 16076 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-09-17 6:53 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-09 12:54 [PATCH v3 0/3] Stingray thermal driver support Srinath Mannam
2018-08-09 12:54 ` [PATCH v3 1/3] dt-bindings: thermal: Add binding document for SR thermal Srinath Mannam
2018-08-13 17:34 ` Rob Herring
2018-08-09 12:54 ` [PATCH v3 2/3] arm64: dts: stingray: Add Stingray Thermal DT support Srinath Mannam
2018-08-09 12:54 ` [PATCH v3 3/3] thermal: broadcom: Add Stingray thermal driver Srinath Mannam
2018-09-14 14:20 ` Daniel Lezcano
2018-09-17 3:50 ` Srinath Mannam
2018-09-17 6:53 ` Pramod Kumar
2018-09-13 22:25 ` [PATCH v3 0/3] Stingray thermal driver support Florian Fainelli
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.