On Fri, Sep 14, 2018 at 7:51 PM Daniel Lezcano [off-list ref]
wrote:

On 09/08/2018 14:54, Srinath Mannam wrote:

quoted

From: Pramod Kumar <redacted>

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

quoted

Signed-off-by: Pramod Kumar <redacted>
Signed-off-by: Srinath Mannam <redacted>
Reviewed-by: Ray Jui <redacted>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Reviewed-by: Vikram Prakash <redacted>
---
 drivers/thermal/Kconfig               |   3 +-
 drivers/thermal/broadcom/Kconfig      |   9 ++
 drivers/thermal/broadcom/Makefile     |   1 +
 drivers/thermal/broadcom/sr-thermal.c | 216

++++++++++++++++++++++++++++++++++

quoted

 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

|| \

quoted

+             COMPILE_TEST
 source "drivers/thermal/broadcom/Kconfig"
 endmenu

diff --git a/drivers/thermal/broadcom/Kconfig

b/drivers/thermal/broadcom/Kconfig

quoted

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

quoted

        Management Unit) block with a thermal sensor that allows

checking CPU

quoted

        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

quoted

+       iHost, CRMU and NITRO has thermal sensor that allows checking its
+       temperature.

diff --git a/drivers/thermal/broadcom/Makefile

b/drivers/thermal/broadcom/Makefile

quoted

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

quoted

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?

quoted

+#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[] = {
        ...
};

quoted

+     "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?

quoted

+     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");

quoted

+             return -EINVAL;
+     }
+     return 0;
+}
+
+static int sr_get_trip_temp(struct thermal_zone_device *tz, int trip,

int *temp)

quoted

+{
+     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");

quoted

+             return -EINVAL;
+     }
+     return 0;
+}
+
+static int sr_set_trip_temp(struct thermal_zone_device *tz, int trip,

int temp)

quoted

+{
+     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?

quoted

+             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 ?

quoted

+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,

quoted

+                                      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);

quoted

+     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++;

quoted

+             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);

quoted

+     }
+
+     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);

quoted

+
+     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 [off-list ref]");
+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