linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V2 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal
       [not found] <20170318155632.18099-1-zajec5@gmail.com>
@ 2017-03-23 22:30 ` Rafał Miłecki
  2017-03-23 22:30   ` [PATCH V2 2/2] thermal: broadcom: add Northstar thermal driver Rafał Miłecki
                     ` (2 more replies)
  0 siblings, 3 replies; 31+ messages in thread
From: Rafał Miłecki @ 2017-03-23 22:30 UTC (permalink / raw)
  To: linux-arm-kernel

From: Rafa? Mi?ecki <rafal@milecki.pl>

This commit documents binding for thermal used in Northstar family SoCs.

Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
---
 .../devicetree/bindings/thermal/brcm,ns-thermal         | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/brcm,ns-thermal

diff --git a/Documentation/devicetree/bindings/thermal/brcm,ns-thermal b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
new file mode 100644
index 000000000000..9a22dd76ec01
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
@@ -0,0 +1,17 @@
+* Broadcom Northstar Thermal
+
+This binding describes thermal sensor that is part of Northstar's DMU (Device
+Management Unit).
+
+Required properties:
+- compatible : Must be "brcm,ns-thermal"
+- reg : iomem address range of PVTMON registers
+- #thermal-sensor-cells : Should be <0>
+
+Example:
+
+thermal {
+	compatible = "brcm,ns-thermal";
+	reg = <0x1800c2c0 0x10>;
+	#thermal-sensor-cells = <0>;
+};
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH V2 2/2] thermal: broadcom: add Northstar thermal driver
  2017-03-23 22:30 ` [PATCH V2 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal Rafał Miłecki
@ 2017-03-23 22:30   ` Rafał Miłecki
  2017-03-24 14:35     ` Jon Mason
  2017-03-31  3:15     ` Eduardo Valentin
  2017-03-31  3:17   ` [PATCH V2 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal Eduardo Valentin
  2017-03-31  7:31   ` [PATCH V3 " Rafał Miłecki
  2 siblings, 2 replies; 31+ messages in thread
From: Rafał Miłecki @ 2017-03-23 22:30 UTC (permalink / raw)
  To: linux-arm-kernel

From: Rafa? Mi?ecki <rafal@milecki.pl>

Northstar is a SoC family commonly used in home routers. This commit
adds a driver for checking CPU temperature. As Northstar Plus seems to
also have this IP block this new symbol gets ARCH_BCM_IPROC dependency.

Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
V2: Make it iProc specific as NSP can also use this driver
    Select proper symbols in config ARCH_BCM_IPROC
    Define PVTMON register bits
    Update code selecting temperature monitor mode
    Thank you Jon!
---
 arch/arm/mach-bcm/Kconfig             |  2 +
 drivers/thermal/Kconfig               |  5 ++
 drivers/thermal/Makefile              |  1 +
 drivers/thermal/broadcom/Kconfig      |  7 +++
 drivers/thermal/broadcom/Makefile     |  1 +
 drivers/thermal/broadcom/ns-thermal.c | 98 +++++++++++++++++++++++++++++++++++
 6 files changed, 114 insertions(+)
 create mode 100644 drivers/thermal/broadcom/Kconfig
 create mode 100644 drivers/thermal/broadcom/Makefile
 create mode 100644 drivers/thermal/broadcom/ns-thermal.c

diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
index a0e66d8200c5..da2bfeb8e390 100644
--- a/arch/arm/mach-bcm/Kconfig
+++ b/arch/arm/mach-bcm/Kconfig
@@ -19,6 +19,8 @@ config ARCH_BCM_IPROC
 	select GPIOLIB
 	select ARM_AMBA
 	select PINCTRL
+	select THERMAL
+	select THERMAL_OF
 	help
 	  This enables support for systems based on Broadcom IPROC architected SoCs.
 	  The IPROC complex contains one or more ARM CPUs along with common
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 776b34396144..008e173ec825 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -392,6 +392,11 @@ config MTK_THERMAL
 	  Enable this option if you want to have support for thermal management
 	  controller present in Mediatek SoCs
 
+menu "Broadcom thermal drivers"
+depends on ARCH_BCM || COMPILE_TEST
+source "drivers/thermal/broadcom/Kconfig"
+endmenu
+
 menu "Texas Instruments thermal drivers"
 depends on ARCH_HAS_BANDGAP || COMPILE_TEST
 depends on HAS_IOMEM
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 7adae2029355..549d81b6363c 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -27,6 +27,7 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL)	+= clock_cooling.o
 thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o
 
 # platform thermal drivers
+obj-y				+= broadcom/
 obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM)	+= qcom-spmi-temp-alarm.o
 obj-$(CONFIG_SPEAR_THERMAL)	+= spear_thermal.o
 obj-$(CONFIG_ROCKCHIP_THERMAL)	+= rockchip_thermal.o
diff --git a/drivers/thermal/broadcom/Kconfig b/drivers/thermal/broadcom/Kconfig
new file mode 100644
index 000000000000..39b92a1f3584
--- /dev/null
+++ b/drivers/thermal/broadcom/Kconfig
@@ -0,0 +1,7 @@
+config BCM_NS_THERMAL
+	tristate "Northstar thermal driver"
+	depends on ARCH_BCM_IPROC || COMPILE_TEST
+	help
+	  Northstar's DMU (Device Management Unit) block contains a thermal
+	  sensor that allows checking CPU temperature. This driver provides
+	  support for it.
diff --git a/drivers/thermal/broadcom/Makefile b/drivers/thermal/broadcom/Makefile
new file mode 100644
index 000000000000..059df9a0ed69
--- /dev/null
+++ b/drivers/thermal/broadcom/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_BCM_NS_THERMAL)		+= ns-thermal.o
diff --git a/drivers/thermal/broadcom/ns-thermal.c b/drivers/thermal/broadcom/ns-thermal.c
new file mode 100644
index 000000000000..acf3a4de62a4
--- /dev/null
+++ b/drivers/thermal/broadcom/ns-thermal.c
@@ -0,0 +1,98 @@
+/*
+ * Copyright (C) 2017 Rafa? Mi?ecki <rafal@milecki.pl>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/thermal.h>
+
+#define PVTMON_CONTROL0					0x00
+#define PVTMON_CONTROL0_SEL_MASK			0x0000000e
+#define PVTMON_CONTROL0_SEL_TEMP_MONITOR		0x00000000
+#define PVTMON_CONTROL0_SEL_TEST_MODE			0x0000000e
+#define PVTMON_STATUS					0x08
+
+struct ns_thermal {
+	void __iomem *pvtmon;
+};
+
+static int ns_thermal_get_temp(void *data, int *temp)
+{
+	struct ns_thermal *ns_thermal = data;
+	u32 val;
+
+	val = readl(ns_thermal->pvtmon + PVTMON_CONTROL0);
+	if ((val & PVTMON_CONTROL0_SEL_MASK) != PVTMON_CONTROL0_SEL_TEMP_MONITOR) {
+		val &= ~PVTMON_CONTROL0_SEL_MASK;
+		val |= PVTMON_CONTROL0_SEL_TEMP_MONITOR;
+		writel(val, ns_thermal->pvtmon + PVTMON_CONTROL0);
+	}
+
+	val = readl(ns_thermal->pvtmon + PVTMON_STATUS);
+	*temp = (418 - (val * 5556 / 10000)) * 1000;
+
+	return 0;
+}
+
+const struct thermal_zone_of_device_ops ns_thermal_ops = {
+	.get_temp = ns_thermal_get_temp,
+};
+
+static int ns_thermal_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct ns_thermal *ns_thermal;
+	struct thermal_zone_device *tzd;
+
+	ns_thermal = devm_kzalloc(dev, sizeof(*ns_thermal), GFP_KERNEL);
+	if (!ns_thermal)
+		return -ENOMEM;
+
+	ns_thermal->pvtmon = of_iomap(dev_of_node(dev), 0);
+	if (WARN_ON(!ns_thermal->pvtmon))
+		return -ENOENT;
+
+	tzd = devm_thermal_zone_of_sensor_register(dev, 0, ns_thermal,
+						   &ns_thermal_ops);
+	if (IS_ERR(tzd)) {
+		iounmap(ns_thermal->pvtmon);
+		return PTR_ERR(tzd);
+	}
+
+	platform_set_drvdata(pdev, ns_thermal);
+
+	return 0;
+}
+
+static int ns_thermal_remove(struct platform_device *pdev)
+{
+	struct ns_thermal *ns_thermal = platform_get_drvdata(pdev);
+
+	iounmap(ns_thermal->pvtmon);
+
+	return 0;
+}
+
+static const struct of_device_id ns_thermal_of_match[] = {
+	{ .compatible = "brcm,ns-thermal", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ns_thermal_of_match);
+
+static struct platform_driver ns_thermal_driver = {
+	.probe		= ns_thermal_probe,
+	.remove		= ns_thermal_remove,
+	.driver = {
+		.name = "ns-thermal",
+		.of_match_table = ns_thermal_of_match,
+	},
+};
+module_platform_driver(ns_thermal_driver);
+
+MODULE_DESCRIPTION("Northstar thermal driver");
+MODULE_LICENSE("GPL v2");
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH V2 2/2] thermal: broadcom: add Northstar thermal driver
  2017-03-23 22:30   ` [PATCH V2 2/2] thermal: broadcom: add Northstar thermal driver Rafał Miłecki
@ 2017-03-24 14:35     ` Jon Mason
  2017-03-31  7:03       ` Rafał Miłecki
  2017-03-31  3:15     ` Eduardo Valentin
  1 sibling, 1 reply; 31+ messages in thread
From: Jon Mason @ 2017-03-24 14:35 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Mar 23, 2017 at 11:30:45PM +0100, Rafa? Mi?ecki wrote:
> From: Rafa? Mi?ecki <rafal@milecki.pl>
> 
> Northstar is a SoC family commonly used in home routers. This commit
> adds a driver for checking CPU temperature. As Northstar Plus seems to
> also have this IP block this new symbol gets ARCH_BCM_IPROC dependency.
> 
> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> ---
> V2: Make it iProc specific as NSP can also use this driver
>     Select proper symbols in config ARCH_BCM_IPROC
>     Define PVTMON register bits
>     Update code selecting temperature monitor mode
>     Thank you Jon!
> ---
>  arch/arm/mach-bcm/Kconfig             |  2 +
>  drivers/thermal/Kconfig               |  5 ++
>  drivers/thermal/Makefile              |  1 +
>  drivers/thermal/broadcom/Kconfig      |  7 +++
>  drivers/thermal/broadcom/Makefile     |  1 +
>  drivers/thermal/broadcom/ns-thermal.c | 98 +++++++++++++++++++++++++++++++++++
>  6 files changed, 114 insertions(+)
>  create mode 100644 drivers/thermal/broadcom/Kconfig
>  create mode 100644 drivers/thermal/broadcom/Makefile
>  create mode 100644 drivers/thermal/broadcom/ns-thermal.c
> 
> diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> index a0e66d8200c5..da2bfeb8e390 100644
> --- a/arch/arm/mach-bcm/Kconfig
> +++ b/arch/arm/mach-bcm/Kconfig
> @@ -19,6 +19,8 @@ config ARCH_BCM_IPROC
>  	select GPIOLIB
>  	select ARM_AMBA
>  	select PINCTRL
> +	select THERMAL
> +	select THERMAL_OF
>  	help
>  	  This enables support for systems based on Broadcom IPROC architected SoCs.
>  	  The IPROC complex contains one or more ARM CPUs along with common
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 776b34396144..008e173ec825 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -392,6 +392,11 @@ config MTK_THERMAL
>  	  Enable this option if you want to have support for thermal management
>  	  controller present in Mediatek SoCs
>  
> +menu "Broadcom thermal drivers"
> +depends on ARCH_BCM || COMPILE_TEST
> +source "drivers/thermal/broadcom/Kconfig"
> +endmenu
> +
>  menu "Texas Instruments thermal drivers"
>  depends on ARCH_HAS_BANDGAP || COMPILE_TEST
>  depends on HAS_IOMEM
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 7adae2029355..549d81b6363c 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -27,6 +27,7 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL)	+= clock_cooling.o
>  thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o
>  
>  # platform thermal drivers
> +obj-y				+= broadcom/
>  obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM)	+= qcom-spmi-temp-alarm.o
>  obj-$(CONFIG_SPEAR_THERMAL)	+= spear_thermal.o
>  obj-$(CONFIG_ROCKCHIP_THERMAL)	+= rockchip_thermal.o
> diff --git a/drivers/thermal/broadcom/Kconfig b/drivers/thermal/broadcom/Kconfig
> new file mode 100644
> index 000000000000..39b92a1f3584
> --- /dev/null
> +++ b/drivers/thermal/broadcom/Kconfig
> @@ -0,0 +1,7 @@
> +config BCM_NS_THERMAL
> +	tristate "Northstar thermal driver"
> +	depends on ARCH_BCM_IPROC || COMPILE_TEST
> +	help
> +	  Northstar's DMU (Device Management Unit) block contains a thermal
> +	  sensor that allows checking CPU temperature. This driver provides
> +	  support for it.
> diff --git a/drivers/thermal/broadcom/Makefile b/drivers/thermal/broadcom/Makefile
> new file mode 100644
> index 000000000000..059df9a0ed69
> --- /dev/null
> +++ b/drivers/thermal/broadcom/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_BCM_NS_THERMAL)		+= ns-thermal.o
> diff --git a/drivers/thermal/broadcom/ns-thermal.c b/drivers/thermal/broadcom/ns-thermal.c
> new file mode 100644
> index 000000000000..acf3a4de62a4
> --- /dev/null
> +++ b/drivers/thermal/broadcom/ns-thermal.c
> @@ -0,0 +1,98 @@
> +/*
> + * Copyright (C) 2017 Rafa? Mi?ecki <rafal@milecki.pl>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/thermal.h>
> +
> +#define PVTMON_CONTROL0					0x00
> +#define PVTMON_CONTROL0_SEL_MASK			0x0000000e
> +#define PVTMON_CONTROL0_SEL_TEMP_MONITOR		0x00000000
> +#define PVTMON_CONTROL0_SEL_TEST_MODE			0x0000000e
> +#define PVTMON_STATUS					0x08
> +
> +struct ns_thermal {
> +	void __iomem *pvtmon;
> +};
> +
> +static int ns_thermal_get_temp(void *data, int *temp)
> +{
> +	struct ns_thermal *ns_thermal = data;
> +	u32 val;
> +
> +	val = readl(ns_thermal->pvtmon + PVTMON_CONTROL0);
> +	if ((val & PVTMON_CONTROL0_SEL_MASK) != PVTMON_CONTROL0_SEL_TEMP_MONITOR) {
> +		val &= ~PVTMON_CONTROL0_SEL_MASK;
> +		val |= PVTMON_CONTROL0_SEL_TEMP_MONITOR;

I think this is slightly confusing here.  If this was off, ORing in 0
will not enable it.  I think we need to flip the #define to make it
PVTMON_CONTROL0_SEL_TEMP_MONITOR_DISABLE (or a less crappy name).
then use this line here to toggle it.  Something like

                val &= ~PVTMON_CONTROL0_SEL_TEMP_MONITOR_DISABLE;

Thanks,
Jon

> +		writel(val, ns_thermal->pvtmon + PVTMON_CONTROL0);
> +	}
> +
> +	val = readl(ns_thermal->pvtmon + PVTMON_STATUS);
> +	*temp = (418 - (val * 5556 / 10000)) * 1000;
> +
> +	return 0;
> +}
> +
> +const struct thermal_zone_of_device_ops ns_thermal_ops = {
> +	.get_temp = ns_thermal_get_temp,
> +};
> +
> +static int ns_thermal_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct ns_thermal *ns_thermal;
> +	struct thermal_zone_device *tzd;
> +
> +	ns_thermal = devm_kzalloc(dev, sizeof(*ns_thermal), GFP_KERNEL);
> +	if (!ns_thermal)
> +		return -ENOMEM;
> +
> +	ns_thermal->pvtmon = of_iomap(dev_of_node(dev), 0);
> +	if (WARN_ON(!ns_thermal->pvtmon))
> +		return -ENOENT;
> +
> +	tzd = devm_thermal_zone_of_sensor_register(dev, 0, ns_thermal,
> +						   &ns_thermal_ops);
> +	if (IS_ERR(tzd)) {
> +		iounmap(ns_thermal->pvtmon);
> +		return PTR_ERR(tzd);
> +	}
> +
> +	platform_set_drvdata(pdev, ns_thermal);
> +
> +	return 0;
> +}
> +
> +static int ns_thermal_remove(struct platform_device *pdev)
> +{
> +	struct ns_thermal *ns_thermal = platform_get_drvdata(pdev);
> +
> +	iounmap(ns_thermal->pvtmon);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ns_thermal_of_match[] = {
> +	{ .compatible = "brcm,ns-thermal", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, ns_thermal_of_match);
> +
> +static struct platform_driver ns_thermal_driver = {
> +	.probe		= ns_thermal_probe,
> +	.remove		= ns_thermal_remove,
> +	.driver = {
> +		.name = "ns-thermal",
> +		.of_match_table = ns_thermal_of_match,
> +	},
> +};
> +module_platform_driver(ns_thermal_driver);
> +
> +MODULE_DESCRIPTION("Northstar thermal driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.11.0
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V2 2/2] thermal: broadcom: add Northstar thermal driver
  2017-03-23 22:30   ` [PATCH V2 2/2] thermal: broadcom: add Northstar thermal driver Rafał Miłecki
  2017-03-24 14:35     ` Jon Mason
@ 2017-03-31  3:15     ` Eduardo Valentin
  2017-03-31  7:08       ` Rafał Miłecki
  1 sibling, 1 reply; 31+ messages in thread
From: Eduardo Valentin @ 2017-03-31  3:15 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On Thu, Mar 23, 2017 at 11:30:45PM +0100, Rafa? Mi?ecki wrote:
> From: Rafa? Mi?ecki <rafal@milecki.pl>
> 
> Northstar is a SoC family commonly used in home routers. This commit
> adds a driver for checking CPU temperature. As Northstar Plus seems to
> also have this IP block this new symbol gets ARCH_BCM_IPROC dependency.
> 

Can you please educate me on how different this driver is from 
https://patchwork.kernel.org/patch/9619579/

?

> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> ---
> V2: Make it iProc specific as NSP can also use this driver
>     Select proper symbols in config ARCH_BCM_IPROC
>     Define PVTMON register bits
>     Update code selecting temperature monitor mode
>     Thank you Jon!
> ---
>  arch/arm/mach-bcm/Kconfig             |  2 +
>  drivers/thermal/Kconfig               |  5 ++
>  drivers/thermal/Makefile              |  1 +
>  drivers/thermal/broadcom/Kconfig      |  7 +++
>  drivers/thermal/broadcom/Makefile     |  1 +
>  drivers/thermal/broadcom/ns-thermal.c | 98 +++++++++++++++++++++++++++++++++++

Ok. This driver attempts to create a common place for broadcom thermal
drivers, which is good, but..

>  6 files changed, 114 insertions(+)
>  create mode 100644 drivers/thermal/broadcom/Kconfig
>  create mode 100644 drivers/thermal/broadcom/Makefile
>  create mode 100644 drivers/thermal/broadcom/ns-thermal.c
> 
> diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
> index a0e66d8200c5..da2bfeb8e390 100644
> --- a/arch/arm/mach-bcm/Kconfig
> +++ b/arch/arm/mach-bcm/Kconfig
> @@ -19,6 +19,8 @@ config ARCH_BCM_IPROC
>  	select GPIOLIB
>  	select ARM_AMBA
>  	select PINCTRL
> +	select THERMAL
> +	select THERMAL_OF
>  	help
>  	  This enables support for systems based on Broadcom IPROC architected SoCs.
>  	  The IPROC complex contains one or more ARM CPUs along with common

To avoid conflicts I would suggest getting this patch split into driver
and arch code changes.

> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 776b34396144..008e173ec825 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -392,6 +392,11 @@ config MTK_THERMAL
>  	  Enable this option if you want to have support for thermal management
>  	  controller present in Mediatek SoCs
>  
> +menu "Broadcom thermal drivers"
> +depends on ARCH_BCM || COMPILE_TEST

Does the above dependency also work for other BCM chips?

> +source "drivers/thermal/broadcom/Kconfig"
> +endmenu
> +
>  menu "Texas Instruments thermal drivers"
>  depends on ARCH_HAS_BANDGAP || COMPILE_TEST
>  depends on HAS_IOMEM
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 7adae2029355..549d81b6363c 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -27,6 +27,7 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL)	+= clock_cooling.o
>  thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o
>  
>  # platform thermal drivers
> +obj-y				+= broadcom/
>  obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM)	+= qcom-spmi-temp-alarm.o
>  obj-$(CONFIG_SPEAR_THERMAL)	+= spear_thermal.o
>  obj-$(CONFIG_ROCKCHIP_THERMAL)	+= rockchip_thermal.o
> diff --git a/drivers/thermal/broadcom/Kconfig b/drivers/thermal/broadcom/Kconfig
> new file mode 100644
> index 000000000000..39b92a1f3584
> --- /dev/null
> +++ b/drivers/thermal/broadcom/Kconfig
> @@ -0,0 +1,7 @@
> +config BCM_NS_THERMAL
> +	tristate "Northstar thermal driver"
> +	depends on ARCH_BCM_IPROC || COMPILE_TEST
> +	help
> +	  Northstar's DMU (Device Management Unit) block contains a thermal
> +	  sensor that allows checking CPU temperature. This driver provides
> +	  support for it.

It would be good if you could include a list of socs that have DMUs.

> diff --git a/drivers/thermal/broadcom/Makefile b/drivers/thermal/broadcom/Makefile
> new file mode 100644
> index 000000000000..059df9a0ed69
> --- /dev/null
> +++ b/drivers/thermal/broadcom/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_BCM_NS_THERMAL)		+= ns-thermal.o
> diff --git a/drivers/thermal/broadcom/ns-thermal.c b/drivers/thermal/broadcom/ns-thermal.c
> new file mode 100644
> index 000000000000..acf3a4de62a4
> --- /dev/null
> +++ b/drivers/thermal/broadcom/ns-thermal.c
> @@ -0,0 +1,98 @@
> +/*
> + * Copyright (C) 2017 Rafa? Mi?ecki <rafal@milecki.pl>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/thermal.h>
> +
> +#define PVTMON_CONTROL0					0x00
> +#define PVTMON_CONTROL0_SEL_MASK			0x0000000e
> +#define PVTMON_CONTROL0_SEL_TEMP_MONITOR		0x00000000
> +#define PVTMON_CONTROL0_SEL_TEST_MODE			0x0000000e
> +#define PVTMON_STATUS					0x08
> +
> +struct ns_thermal {
> +	void __iomem *pvtmon;
> +};
> +
> +static int ns_thermal_get_temp(void *data, int *temp)
> +{
> +	struct ns_thermal *ns_thermal = data;
> +	u32 val;
> +
> +	val = readl(ns_thermal->pvtmon + PVTMON_CONTROL0);
> +	if ((val & PVTMON_CONTROL0_SEL_MASK) != PVTMON_CONTROL0_SEL_TEMP_MONITOR) {
> +		val &= ~PVTMON_CONTROL0_SEL_MASK;
> +		val |= PVTMON_CONTROL0_SEL_TEMP_MONITOR;
> +		writel(val, ns_thermal->pvtmon + PVTMON_CONTROL0);
> +	}
> +
> +	val = readl(ns_thermal->pvtmon + PVTMON_STATUS);
> +	*temp = (418 - (val * 5556 / 10000)) * 1000;
> +

Could you move the above equation to DT? The other BCM driver I posted
the link above just went through the same process.

> +	return 0;
> +}
> +
> +const struct thermal_zone_of_device_ops ns_thermal_ops = {
> +	.get_temp = ns_thermal_get_temp,
> +};
> +
> +static int ns_thermal_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct ns_thermal *ns_thermal;
> +	struct thermal_zone_device *tzd;
> +
> +	ns_thermal = devm_kzalloc(dev, sizeof(*ns_thermal), GFP_KERNEL);
> +	if (!ns_thermal)
> +		return -ENOMEM;
> +
> +	ns_thermal->pvtmon = of_iomap(dev_of_node(dev), 0);
> +	if (WARN_ON(!ns_thermal->pvtmon))
> +		return -ENOENT;
> +
> +	tzd = devm_thermal_zone_of_sensor_register(dev, 0, ns_thermal,
> +						   &ns_thermal_ops);
> +	if (IS_ERR(tzd)) {
> +		iounmap(ns_thermal->pvtmon);
> +		return PTR_ERR(tzd);
> +	}
> +
> +	platform_set_drvdata(pdev, ns_thermal);
> +
> +	return 0;
> +}
> +
> +static int ns_thermal_remove(struct platform_device *pdev)
> +{
> +	struct ns_thermal *ns_thermal = platform_get_drvdata(pdev);
> +
> +	iounmap(ns_thermal->pvtmon);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ns_thermal_of_match[] = {
> +	{ .compatible = "brcm,ns-thermal", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, ns_thermal_of_match);
> +
> +static struct platform_driver ns_thermal_driver = {
> +	.probe		= ns_thermal_probe,
> +	.remove		= ns_thermal_remove,
> +	.driver = {
> +		.name = "ns-thermal",
> +		.of_match_table = ns_thermal_of_match,
> +	},
> +};
> +module_platform_driver(ns_thermal_driver);
> +
> +MODULE_DESCRIPTION("Northstar thermal driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.11.0
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V2 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal
  2017-03-23 22:30 ` [PATCH V2 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal Rafał Miłecki
  2017-03-23 22:30   ` [PATCH V2 2/2] thermal: broadcom: add Northstar thermal driver Rafał Miłecki
@ 2017-03-31  3:17   ` Eduardo Valentin
  2017-03-31  7:31   ` [PATCH V3 " Rafał Miłecki
  2 siblings, 0 replies; 31+ messages in thread
From: Eduardo Valentin @ 2017-03-31  3:17 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Rafal,

On Thu, Mar 23, 2017 at 11:30:44PM +0100, Rafa? Mi?ecki wrote:
> From: Rafa? Mi?ecki <rafal@milecki.pl>
> 
> This commit documents binding for thermal used in Northstar family SoCs.
> 
> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
> ---
>  .../devicetree/bindings/thermal/brcm,ns-thermal         | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/brcm,ns-thermal
> 
> diff --git a/Documentation/devicetree/bindings/thermal/brcm,ns-thermal b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
> new file mode 100644
> index 000000000000..9a22dd76ec01
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
> @@ -0,0 +1,17 @@
> +* Broadcom Northstar Thermal
> +
> +This binding describes thermal sensor that is part of Northstar's DMU (Device
> +Management Unit).
> +
> +Required properties:
> +- compatible : Must be "brcm,ns-thermal"
> +- reg : iomem address range of PVTMON registers
> +- #thermal-sensor-cells : Should be <0>
> +
> +Example:
> +
> +thermal {
> +	compatible = "brcm,ns-thermal";
> +	reg = <0x1800c2c0 0x10>;
> +	#thermal-sensor-cells = <0>;
> +};


Could you please also include an example of the thermal zone one should
include in the board/cpu DTS while using this sensor? 

Besides, based on what I saw you in your driver, you could also describe
your offset and slope in DT, and read them using thermal helpers.

BR,
> -- 
> 2.11.0
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V2 2/2] thermal: broadcom: add Northstar thermal driver
  2017-03-24 14:35     ` Jon Mason
@ 2017-03-31  7:03       ` Rafał Miłecki
  2017-03-31 14:23         ` Jon Mason
  0 siblings, 1 reply; 31+ messages in thread
From: Rafał Miłecki @ 2017-03-31  7:03 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/24/2017 03:35 PM, Jon Mason wrote:
> On Thu, Mar 23, 2017 at 11:30:45PM +0100, Rafa? Mi?ecki wrote:
>> diff --git a/drivers/thermal/broadcom/ns-thermal.c b/drivers/thermal/broadcom/ns-thermal.c
>> new file mode 100644
>> index 000000000000..acf3a4de62a4
>> --- /dev/null
>> +++ b/drivers/thermal/broadcom/ns-thermal.c
>> @@ -0,0 +1,98 @@
>> +/*
>> + * Copyright (C) 2017 Rafa? Mi?ecki <rafal@milecki.pl>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/of_address.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/thermal.h>
>> +
>> +#define PVTMON_CONTROL0					0x00
>> +#define PVTMON_CONTROL0_SEL_MASK			0x0000000e
>> +#define PVTMON_CONTROL0_SEL_TEMP_MONITOR		0x00000000
>> +#define PVTMON_CONTROL0_SEL_TEST_MODE			0x0000000e
>> +#define PVTMON_STATUS					0x08
>> +
>> +struct ns_thermal {
>> +	void __iomem *pvtmon;
>> +};
>> +
>> +static int ns_thermal_get_temp(void *data, int *temp)
>> +{
>> +	struct ns_thermal *ns_thermal = data;
>> +	u32 val;
>> +
>> +	val = readl(ns_thermal->pvtmon + PVTMON_CONTROL0);
>> +	if ((val & PVTMON_CONTROL0_SEL_MASK) != PVTMON_CONTROL0_SEL_TEMP_MONITOR) {
>> +		val &= ~PVTMON_CONTROL0_SEL_MASK;
>> +		val |= PVTMON_CONTROL0_SEL_TEMP_MONITOR;
>
> I think this is slightly confusing here.  If this was off, ORing in 0
> will not enable it.  I think we need to flip the #define to make it
> PVTMON_CONTROL0_SEL_TEMP_MONITOR_DISABLE (or a less crappy name).
> then use this line here to toggle it.  Something like
>
>                 val &= ~PVTMON_CONTROL0_SEL_TEMP_MONITOR_DISABLE;

I don't understand this, can you help me further?

OR-ing with 0 is only a cosmetic thing obviously - just to make it clear which
value we expect to be set in bits 1:3. The important part is:
val &= ~PVTMON_CONTROL0_SEL_MASK;

AFAIU selecting any mode other than TEMP_MONITOR will disable monitor access.
AFAIU even switchint to TEST_MODE will prevent access to temp, so I don't see
how we could use PVTMON_CONTROL0_SEL_TEMP_MONITOR_DISABLE. It could be any
value other than 0.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V2 2/2] thermal: broadcom: add Northstar thermal driver
  2017-03-31  3:15     ` Eduardo Valentin
@ 2017-03-31  7:08       ` Rafał Miłecki
  0 siblings, 0 replies; 31+ messages in thread
From: Rafał Miłecki @ 2017-03-31  7:08 UTC (permalink / raw)
  To: linux-arm-kernel

Thanks for your review!

On 03/31/2017 05:15 AM, Eduardo Valentin wrote:
> On Thu, Mar 23, 2017 at 11:30:45PM +0100, Rafa? Mi?ecki wrote:
>> From: Rafa? Mi?ecki <rafal@milecki.pl>
>>
>> Northstar is a SoC family commonly used in home routers. This commit
>> adds a driver for checking CPU temperature. As Northstar Plus seems to
>> also have this IP block this new symbol gets ARCH_BCM_IPROC dependency.
>>
>
> Can you please educate me on how different this driver is from
> https://patchwork.kernel.org/patch/9619579/
>
> ?

This is a different hardware block. Northstar and BCM283x are totally different
SoCs. Northstar is mostly used in home routers, BCM283x mostly in Rasperry Pi.


>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>> index 776b34396144..008e173ec825 100644
>> --- a/drivers/thermal/Kconfig
>> +++ b/drivers/thermal/Kconfig
>> @@ -392,6 +392,11 @@ config MTK_THERMAL
>>  	  Enable this option if you want to have support for thermal management
>>  	  controller present in Mediatek SoCs
>>
>> +menu "Broadcom thermal drivers"
>> +depends on ARCH_BCM || COMPILE_TEST
>
> Does the above dependency also work for other BCM chips?

I believe so. If there ever appears a driver in drivers/thermal/broadcom/ that
can't be freely compiled with COMPILE_TEST, it can just have a proper
dependency.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V3 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal
  2017-03-23 22:30 ` [PATCH V2 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal Rafał Miłecki
  2017-03-23 22:30   ` [PATCH V2 2/2] thermal: broadcom: add Northstar thermal driver Rafał Miłecki
  2017-03-31  3:17   ` [PATCH V2 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal Eduardo Valentin
@ 2017-03-31  7:31   ` Rafał Miłecki
  2017-03-31  7:31     ` [PATCH V3 2/2] thermal: broadcom: add Northstar thermal driver Rafał Miłecki
  2 siblings, 1 reply; 31+ messages in thread
From: Rafał Miłecki @ 2017-03-31  7:31 UTC (permalink / raw)
  To: linux-arm-kernel

From: Rafa? Mi?ecki <rafal@milecki.pl>

This commit documents binding for thermal used in Northstar family SoCs.

Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
---
V3: Add thermal-zones to the example
    Rob: Because of this update, I didn't include Acked-by I got for V2
---
 .../devicetree/bindings/thermal/brcm,ns-thermal    | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/brcm,ns-thermal

diff --git a/Documentation/devicetree/bindings/thermal/brcm,ns-thermal b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
new file mode 100644
index 000000000000..c561c7349f17
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
@@ -0,0 +1,26 @@
+* Broadcom Northstar Thermal
+
+This binding describes thermal sensor that is part of Northstar's DMU (Device
+Management Unit).
+
+Required properties:
+- compatible : Must be "brcm,ns-thermal"
+- reg : iomem address range of PVTMON registers
+- #thermal-sensor-cells : Should be <0>
+
+Example:
+
+thermal: thermal at 1800c2c0 {
+	compatible = "brcm,ns-thermal";
+	reg = <0x1800c2c0 0x10>;
+	#thermal-sensor-cells = <0>;
+};
+
+thermal-zones {
+	cpu_thermal: cpu-thermal {
+		polling-delay-passive = <0>;
+		polling-delay = <1000>;
+		coefficients = <(-556) 418000>;
+		thermal-sensors = <&thermal>;
+	};
+};
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH V3 2/2] thermal: broadcom: add Northstar thermal driver
  2017-03-31  7:31   ` [PATCH V3 " Rafał Miłecki
@ 2017-03-31  7:31     ` Rafał Miłecki
  2017-03-31 20:11       ` [PATCH V4 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal Rafał Miłecki
  0 siblings, 1 reply; 31+ messages in thread
From: Rafał Miłecki @ 2017-03-31  7:31 UTC (permalink / raw)
  To: linux-arm-kernel

From: Rafa? Mi?ecki <rafal@milecki.pl>

Northstar is a SoC family commonly used in home routers. This commit
adds a driver for checking CPU temperature. As Northstar Plus seems to
also have this IP block this new symbol gets ARCH_BCM_IPROC dependency.

Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
V2: Make it iProc specific as NSP can also use this driver
    Select proper symbols in config ARCH_BCM_IPROC
    Define PVTMON register bits
    Update code selecting temperature monitor mode
    Thank you Jon!
V3: More details in help message for BCM_NS_THERMAL
    Use slope & offset
    Drop arch code change (I'll be submitted using a proper tree)
    Thank you Eduardo!
---
 drivers/thermal/Kconfig               |   5 ++
 drivers/thermal/Makefile              |   1 +
 drivers/thermal/broadcom/Kconfig      |   8 +++
 drivers/thermal/broadcom/Makefile     |   1 +
 drivers/thermal/broadcom/ns-thermal.c | 101 ++++++++++++++++++++++++++++++++++
 5 files changed, 116 insertions(+)
 create mode 100644 drivers/thermal/broadcom/Kconfig
 create mode 100644 drivers/thermal/broadcom/Makefile
 create mode 100644 drivers/thermal/broadcom/ns-thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 776b34396144..008e173ec825 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -392,6 +392,11 @@ config MTK_THERMAL
 	  Enable this option if you want to have support for thermal management
 	  controller present in Mediatek SoCs
 
+menu "Broadcom thermal drivers"
+depends on ARCH_BCM || COMPILE_TEST
+source "drivers/thermal/broadcom/Kconfig"
+endmenu
+
 menu "Texas Instruments thermal drivers"
 depends on ARCH_HAS_BANDGAP || COMPILE_TEST
 depends on HAS_IOMEM
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 7adae2029355..549d81b6363c 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -27,6 +27,7 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL)	+= clock_cooling.o
 thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o
 
 # platform thermal drivers
+obj-y				+= broadcom/
 obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM)	+= qcom-spmi-temp-alarm.o
 obj-$(CONFIG_SPEAR_THERMAL)	+= spear_thermal.o
 obj-$(CONFIG_ROCKCHIP_THERMAL)	+= rockchip_thermal.o
diff --git a/drivers/thermal/broadcom/Kconfig b/drivers/thermal/broadcom/Kconfig
new file mode 100644
index 000000000000..f0dea8a8e002
--- /dev/null
+++ b/drivers/thermal/broadcom/Kconfig
@@ -0,0 +1,8 @@
+config BCM_NS_THERMAL
+	tristate "Northstar thermal driver"
+	depends on ARCH_BCM_IPROC || COMPILE_TEST
+	help
+	  Northstar is a family of SoCs that includes e.g. BCM4708, BCM47081,
+	  BCM4709 and BCM47094. It contains DMU (Device Management Unit) block
+	  with a thermal sensor that allows checking CPU temperature. This
+	  driver provides support for it.
diff --git a/drivers/thermal/broadcom/Makefile b/drivers/thermal/broadcom/Makefile
new file mode 100644
index 000000000000..059df9a0ed69
--- /dev/null
+++ b/drivers/thermal/broadcom/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_BCM_NS_THERMAL)		+= ns-thermal.o
diff --git a/drivers/thermal/broadcom/ns-thermal.c b/drivers/thermal/broadcom/ns-thermal.c
new file mode 100644
index 000000000000..245fe8c6012b
--- /dev/null
+++ b/drivers/thermal/broadcom/ns-thermal.c
@@ -0,0 +1,101 @@
+/*
+ * Copyright (C) 2017 Rafa? Mi?ecki <rafal@milecki.pl>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/thermal.h>
+
+#define PVTMON_CONTROL0					0x00
+#define PVTMON_CONTROL0_SEL_MASK			0x0000000e
+#define PVTMON_CONTROL0_SEL_TEMP_MONITOR		0x00000000
+#define PVTMON_CONTROL0_SEL_TEST_MODE			0x0000000e
+#define PVTMON_STATUS					0x08
+
+struct ns_thermal {
+	struct thermal_zone_device *tz;
+	void __iomem *pvtmon;
+};
+
+static int ns_thermal_get_temp(void *data, int *temp)
+{
+	struct ns_thermal *ns_thermal = data;
+	int offset = thermal_zone_get_offset(ns_thermal->tz);
+	int slope = thermal_zone_get_slope(ns_thermal->tz);
+	u32 val;
+
+	val = readl(ns_thermal->pvtmon + PVTMON_CONTROL0);
+	if ((val & PVTMON_CONTROL0_SEL_MASK) != PVTMON_CONTROL0_SEL_TEMP_MONITOR) {
+		val &= ~PVTMON_CONTROL0_SEL_MASK;
+		val |= PVTMON_CONTROL0_SEL_TEMP_MONITOR;
+		writel(val, ns_thermal->pvtmon + PVTMON_CONTROL0);
+	}
+
+	val = readl(ns_thermal->pvtmon + PVTMON_STATUS);
+	*temp = slope * val + offset;
+
+	return 0;
+}
+
+const struct thermal_zone_of_device_ops ns_thermal_ops = {
+	.get_temp = ns_thermal_get_temp,
+};
+
+static int ns_thermal_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct ns_thermal *ns_thermal;
+
+	ns_thermal = devm_kzalloc(dev, sizeof(*ns_thermal), GFP_KERNEL);
+	if (!ns_thermal)
+		return -ENOMEM;
+
+	ns_thermal->pvtmon = of_iomap(dev_of_node(dev), 0);
+	if (WARN_ON(!ns_thermal->pvtmon))
+		return -ENOENT;
+
+	ns_thermal->tz = devm_thermal_zone_of_sensor_register(dev, 0,
+							      ns_thermal,
+							      &ns_thermal_ops);
+	if (IS_ERR(ns_thermal->tz)) {
+		iounmap(ns_thermal->pvtmon);
+		return PTR_ERR(ns_thermal->tz);
+	}
+
+	platform_set_drvdata(pdev, ns_thermal);
+
+	return 0;
+}
+
+static int ns_thermal_remove(struct platform_device *pdev)
+{
+	struct ns_thermal *ns_thermal = platform_get_drvdata(pdev);
+
+	iounmap(ns_thermal->pvtmon);
+
+	return 0;
+}
+
+static const struct of_device_id ns_thermal_of_match[] = {
+	{ .compatible = "brcm,ns-thermal", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ns_thermal_of_match);
+
+static struct platform_driver ns_thermal_driver = {
+	.probe		= ns_thermal_probe,
+	.remove		= ns_thermal_remove,
+	.driver = {
+		.name = "ns-thermal",
+		.of_match_table = ns_thermal_of_match,
+	},
+};
+module_platform_driver(ns_thermal_driver);
+
+MODULE_DESCRIPTION("Northstar thermal driver");
+MODULE_LICENSE("GPL v2");
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH V2 2/2] thermal: broadcom: add Northstar thermal driver
  2017-03-31  7:03       ` Rafał Miłecki
@ 2017-03-31 14:23         ` Jon Mason
  2017-03-31 14:49           ` Rafał Miłecki
  0 siblings, 1 reply; 31+ messages in thread
From: Jon Mason @ 2017-03-31 14:23 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Mar 31, 2017 at 3:03 AM, Rafa? Mi?ecki <zajec5@gmail.com> wrote:
> On 03/24/2017 03:35 PM, Jon Mason wrote:
>>
>> On Thu, Mar 23, 2017 at 11:30:45PM +0100, Rafa? Mi?ecki wrote:
>>>
>>> diff --git a/drivers/thermal/broadcom/ns-thermal.c
>>> b/drivers/thermal/broadcom/ns-thermal.c
>>> new file mode 100644
>>> index 000000000000..acf3a4de62a4
>>> --- /dev/null
>>> +++ b/drivers/thermal/broadcom/ns-thermal.c
>>> @@ -0,0 +1,98 @@
>>> +/*
>>> + * Copyright (C) 2017 Rafa? Mi?ecki <rafal@milecki.pl>
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/of_address.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/thermal.h>
>>> +
>>> +#define PVTMON_CONTROL0                                        0x00
>>> +#define PVTMON_CONTROL0_SEL_MASK                       0x0000000e
>>> +#define PVTMON_CONTROL0_SEL_TEMP_MONITOR               0x00000000
>>> +#define PVTMON_CONTROL0_SEL_TEST_MODE                  0x0000000e
>>> +#define PVTMON_STATUS                                  0x08
>>> +
>>> +struct ns_thermal {
>>> +       void __iomem *pvtmon;
>>> +};
>>> +
>>> +static int ns_thermal_get_temp(void *data, int *temp)
>>> +{
>>> +       struct ns_thermal *ns_thermal = data;
>>> +       u32 val;
>>> +
>>> +       val = readl(ns_thermal->pvtmon + PVTMON_CONTROL0);
>>> +       if ((val & PVTMON_CONTROL0_SEL_MASK) !=
>>> PVTMON_CONTROL0_SEL_TEMP_MONITOR) {
>>> +               val &= ~PVTMON_CONTROL0_SEL_MASK;
>>> +               val |= PVTMON_CONTROL0_SEL_TEMP_MONITOR;
>>
>>
>> I think this is slightly confusing here.  If this was off, ORing in 0
>> will not enable it.  I think we need to flip the #define to make it
>> PVTMON_CONTROL0_SEL_TEMP_MONITOR_DISABLE (or a less crappy name).
>> then use this line here to toggle it.  Something like
>>
>>                 val &= ~PVTMON_CONTROL0_SEL_TEMP_MONITOR_DISABLE;
>
>
> I don't understand this, can you help me further?
>
> OR-ing with 0 is only a cosmetic thing obviously - just to make it clear
> which
> value we expect to be set in bits 1:3. The important part is:
> val &= ~PVTMON_CONTROL0_SEL_MASK;

You are using a side effect of the masking to clear/enable the block.
I'm simply saying that we should be explicit about enabling it.  My
concern is that using the side effect hides what is being done and
could result in a bug somewhere down the line.  I think this is
improbable based on the code, but wanted to err on the side of
caution.

>
> AFAIU selecting any mode other than TEMP_MONITOR will disable monitor
> access.
> AFAIU even switchint to TEST_MODE will prevent access to temp, so I don't
> see
> how we could use PVTMON_CONTROL0_SEL_TEMP_MONITOR_DISABLE. It could be any
> value other than 0.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V2 2/2] thermal: broadcom: add Northstar thermal driver
  2017-03-31 14:23         ` Jon Mason
@ 2017-03-31 14:49           ` Rafał Miłecki
  2017-03-31 17:29             ` Jon Mason
  0 siblings, 1 reply; 31+ messages in thread
From: Rafał Miłecki @ 2017-03-31 14:49 UTC (permalink / raw)
  To: linux-arm-kernel

On 03/31/2017 04:23 PM, Jon Mason wrote:
> On Fri, Mar 31, 2017 at 3:03 AM, Rafa? Mi?ecki <zajec5@gmail.com> wrote:
>> On 03/24/2017 03:35 PM, Jon Mason wrote:
>>>
>>> On Thu, Mar 23, 2017 at 11:30:45PM +0100, Rafa? Mi?ecki wrote:
>>>>
>>>> diff --git a/drivers/thermal/broadcom/ns-thermal.c
>>>> b/drivers/thermal/broadcom/ns-thermal.c
>>>> new file mode 100644
>>>> index 000000000000..acf3a4de62a4
>>>> --- /dev/null
>>>> +++ b/drivers/thermal/broadcom/ns-thermal.c
>>>> @@ -0,0 +1,98 @@
>>>> +/*
>>>> + * Copyright (C) 2017 Rafa? Mi?ecki <rafal@milecki.pl>
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or modify
>>>> + * it under the terms of the GNU General Public License version 2 as
>>>> + * published by the Free Software Foundation.
>>>> + */
>>>> +
>>>> +#include <linux/module.h>
>>>> +#include <linux/of_address.h>
>>>> +#include <linux/platform_device.h>
>>>> +#include <linux/thermal.h>
>>>> +
>>>> +#define PVTMON_CONTROL0                                        0x00
>>>> +#define PVTMON_CONTROL0_SEL_MASK                       0x0000000e
>>>> +#define PVTMON_CONTROL0_SEL_TEMP_MONITOR               0x00000000
>>>> +#define PVTMON_CONTROL0_SEL_TEST_MODE                  0x0000000e
>>>> +#define PVTMON_STATUS                                  0x08
>>>> +
>>>> +struct ns_thermal {
>>>> +       void __iomem *pvtmon;
>>>> +};
>>>> +
>>>> +static int ns_thermal_get_temp(void *data, int *temp)
>>>> +{
>>>> +       struct ns_thermal *ns_thermal = data;
>>>> +       u32 val;
>>>> +
>>>> +       val = readl(ns_thermal->pvtmon + PVTMON_CONTROL0);
>>>> +       if ((val & PVTMON_CONTROL0_SEL_MASK) !=
>>>> PVTMON_CONTROL0_SEL_TEMP_MONITOR) {
>>>> +               val &= ~PVTMON_CONTROL0_SEL_MASK;
>>>> +               val |= PVTMON_CONTROL0_SEL_TEMP_MONITOR;
>>>
>>>
>>> I think this is slightly confusing here.  If this was off, ORing in 0
>>> will not enable it.  I think we need to flip the #define to make it
>>> PVTMON_CONTROL0_SEL_TEMP_MONITOR_DISABLE (or a less crappy name).
>>> then use this line here to toggle it.  Something like
>>>
>>>                 val &= ~PVTMON_CONTROL0_SEL_TEMP_MONITOR_DISABLE;
>>
>>
>> I don't understand this, can you help me further?
>>
>> OR-ing with 0 is only a cosmetic thing obviously - just to make it clear
>> which
>> value we expect to be set in bits 1:3. The important part is:
>> val &= ~PVTMON_CONTROL0_SEL_MASK;
>
> You are using a side effect of the masking to clear/enable the block.
> I'm simply saying that we should be explicit about enabling it.  My
> concern is that using the side effect hides what is being done and
> could result in a bug somewhere down the line.  I think this is
> improbable based on the code, but wanted to err on the side of
> caution.

Well, I'm clearing current mode selection and "selecting" the mode I want. By
OR-ing PVTMON_CONTROL0_SEL_TEMP_MONITOR I'm clearly indicating I want temp
monitor more.

How else I could make it more obvious? Should I add some comment maybe?

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V2 2/2] thermal: broadcom: add Northstar thermal driver
  2017-03-31 14:49           ` Rafał Miłecki
@ 2017-03-31 17:29             ` Jon Mason
  0 siblings, 0 replies; 31+ messages in thread
From: Jon Mason @ 2017-03-31 17:29 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Mar 31, 2017 at 10:49 AM, Rafa? Mi?ecki <rafal@milecki.pl> wrote:
> On 03/31/2017 04:23 PM, Jon Mason wrote:
>>
>> On Fri, Mar 31, 2017 at 3:03 AM, Rafa? Mi?ecki <zajec5@gmail.com> wrote:
>>>
>>> On 03/24/2017 03:35 PM, Jon Mason wrote:
>>>>
>>>>
>>>> On Thu, Mar 23, 2017 at 11:30:45PM +0100, Rafa? Mi?ecki wrote:
>>>>>
>>>>>
>>>>> diff --git a/drivers/thermal/broadcom/ns-thermal.c
>>>>> b/drivers/thermal/broadcom/ns-thermal.c
>>>>> new file mode 100644
>>>>> index 000000000000..acf3a4de62a4
>>>>> --- /dev/null
>>>>> +++ b/drivers/thermal/broadcom/ns-thermal.c
>>>>> @@ -0,0 +1,98 @@
>>>>> +/*
>>>>> + * Copyright (C) 2017 Rafa? Mi?ecki <rafal@milecki.pl>
>>>>> + *
>>>>> + * This program is free software; you can redistribute it and/or
>>>>> modify
>>>>> + * it under the terms of the GNU General Public License version 2 as
>>>>> + * published by the Free Software Foundation.
>>>>> + */
>>>>> +
>>>>> +#include <linux/module.h>
>>>>> +#include <linux/of_address.h>
>>>>> +#include <linux/platform_device.h>
>>>>> +#include <linux/thermal.h>
>>>>> +
>>>>> +#define PVTMON_CONTROL0                                        0x00
>>>>> +#define PVTMON_CONTROL0_SEL_MASK                       0x0000000e
>>>>> +#define PVTMON_CONTROL0_SEL_TEMP_MONITOR               0x00000000
>>>>> +#define PVTMON_CONTROL0_SEL_TEST_MODE                  0x0000000e
>>>>> +#define PVTMON_STATUS                                  0x08
>>>>> +
>>>>> +struct ns_thermal {
>>>>> +       void __iomem *pvtmon;
>>>>> +};
>>>>> +
>>>>> +static int ns_thermal_get_temp(void *data, int *temp)
>>>>> +{
>>>>> +       struct ns_thermal *ns_thermal = data;
>>>>> +       u32 val;
>>>>> +
>>>>> +       val = readl(ns_thermal->pvtmon + PVTMON_CONTROL0);
>>>>> +       if ((val & PVTMON_CONTROL0_SEL_MASK) !=
>>>>> PVTMON_CONTROL0_SEL_TEMP_MONITOR) {
>>>>> +               val &= ~PVTMON_CONTROL0_SEL_MASK;
>>>>> +               val |= PVTMON_CONTROL0_SEL_TEMP_MONITOR;
>>>>
>>>>
>>>>
>>>> I think this is slightly confusing here.  If this was off, ORing in 0
>>>> will not enable it.  I think we need to flip the #define to make it
>>>> PVTMON_CONTROL0_SEL_TEMP_MONITOR_DISABLE (or a less crappy name).
>>>> then use this line here to toggle it.  Something like
>>>>
>>>>                 val &= ~PVTMON_CONTROL0_SEL_TEMP_MONITOR_DISABLE;
>>>
>>>
>>>
>>> I don't understand this, can you help me further?
>>>
>>> OR-ing with 0 is only a cosmetic thing obviously - just to make it clear
>>> which
>>> value we expect to be set in bits 1:3. The important part is:
>>> val &= ~PVTMON_CONTROL0_SEL_MASK;
>>
>>
>> You are using a side effect of the masking to clear/enable the block.
>> I'm simply saying that we should be explicit about enabling it.  My
>> concern is that using the side effect hides what is being done and
>> could result in a bug somewhere down the line.  I think this is
>> improbable based on the code, but wanted to err on the side of
>> caution.
>
>
> Well, I'm clearing current mode selection and "selecting" the mode I want.
> By
> OR-ing PVTMON_CONTROL0_SEL_TEMP_MONITOR I'm clearly indicating I want temp
> monitor more.
>
> How else I could make it more obvious? Should I add some comment maybe?

I think it would be acceptable to remove the ORing of the 0, and
simply put a comment there that the masking of it is clearing the 0th
bit, thus enabling the IP block.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V4 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal
  2017-03-31  7:31     ` [PATCH V3 2/2] thermal: broadcom: add Northstar thermal driver Rafał Miłecki
@ 2017-03-31 20:11       ` Rafał Miłecki
  2017-03-31 20:11         ` [PATCH V4 2/2] thermal: broadcom: add Northstar thermal driver Rafał Miłecki
                           ` (2 more replies)
  0 siblings, 3 replies; 31+ messages in thread
From: Rafał Miłecki @ 2017-03-31 20:11 UTC (permalink / raw)
  To: linux-arm-kernel

From: Rafa? Mi?ecki <rafal@milecki.pl>

This commit documents binding for thermal used in Northstar family SoCs.

Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
---
V3: Add thermal-zones to the example
    Rob: Because of this update, I didn't include Acked-by I got for V2
---
 .../devicetree/bindings/thermal/brcm,ns-thermal    | 26 ++++++++++++++++++++++
 1 file changed, 26 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/brcm,ns-thermal

diff --git a/Documentation/devicetree/bindings/thermal/brcm,ns-thermal b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
new file mode 100644
index 000000000000..c561c7349f17
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
@@ -0,0 +1,26 @@
+* Broadcom Northstar Thermal
+
+This binding describes thermal sensor that is part of Northstar's DMU (Device
+Management Unit).
+
+Required properties:
+- compatible : Must be "brcm,ns-thermal"
+- reg : iomem address range of PVTMON registers
+- #thermal-sensor-cells : Should be <0>
+
+Example:
+
+thermal: thermal at 1800c2c0 {
+	compatible = "brcm,ns-thermal";
+	reg = <0x1800c2c0 0x10>;
+	#thermal-sensor-cells = <0>;
+};
+
+thermal-zones {
+	cpu_thermal: cpu-thermal {
+		polling-delay-passive = <0>;
+		polling-delay = <1000>;
+		coefficients = <(-556) 418000>;
+		thermal-sensors = <&thermal>;
+	};
+};
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH V4 2/2] thermal: broadcom: add Northstar thermal driver
  2017-03-31 20:11       ` [PATCH V4 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal Rafał Miłecki
@ 2017-03-31 20:11         ` Rafał Miłecki
  2017-04-01 19:54           ` Eduardo Valentin
  2017-04-01 19:51         ` [PATCH V4 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal Eduardo Valentin
  2017-04-03 15:48         ` [PATCH V5 " Rafał Miłecki
  2 siblings, 1 reply; 31+ messages in thread
From: Rafał Miłecki @ 2017-03-31 20:11 UTC (permalink / raw)
  To: linux-arm-kernel

From: Rafa? Mi?ecki <rafal@milecki.pl>

Northstar is a SoC family commonly used in home routers. This commit
adds a driver for checking CPU temperature. As Northstar Plus seems to
also have this IP block this new symbol gets ARCH_BCM_IPROC dependency.

Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
V2: Make it iProc specific as NSP can also use this driver
    Select proper symbols in config ARCH_BCM_IPROC
    Define PVTMON register bits
    Update code selecting temperature monitor mode
    Thank you Jon!
V3: More details in help message for BCM_NS_THERMAL
    Use slope & offset
    Drop arch code change (I'll be submitted using a proper tree)
    Thank you Eduardo!
V4: Comment operations on PVTMON_CONTROL0 register
---
 drivers/thermal/Kconfig               |   5 ++
 drivers/thermal/Makefile              |   1 +
 drivers/thermal/broadcom/Kconfig      |   8 +++
 drivers/thermal/broadcom/Makefile     |   1 +
 drivers/thermal/broadcom/ns-thermal.c | 105 ++++++++++++++++++++++++++++++++++
 5 files changed, 120 insertions(+)
 create mode 100644 drivers/thermal/broadcom/Kconfig
 create mode 100644 drivers/thermal/broadcom/Makefile
 create mode 100644 drivers/thermal/broadcom/ns-thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 776b34396144..008e173ec825 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -392,6 +392,11 @@ config MTK_THERMAL
 	  Enable this option if you want to have support for thermal management
 	  controller present in Mediatek SoCs
 
+menu "Broadcom thermal drivers"
+depends on ARCH_BCM || COMPILE_TEST
+source "drivers/thermal/broadcom/Kconfig"
+endmenu
+
 menu "Texas Instruments thermal drivers"
 depends on ARCH_HAS_BANDGAP || COMPILE_TEST
 depends on HAS_IOMEM
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 7adae2029355..549d81b6363c 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -27,6 +27,7 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL)	+= clock_cooling.o
 thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o
 
 # platform thermal drivers
+obj-y				+= broadcom/
 obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM)	+= qcom-spmi-temp-alarm.o
 obj-$(CONFIG_SPEAR_THERMAL)	+= spear_thermal.o
 obj-$(CONFIG_ROCKCHIP_THERMAL)	+= rockchip_thermal.o
diff --git a/drivers/thermal/broadcom/Kconfig b/drivers/thermal/broadcom/Kconfig
new file mode 100644
index 000000000000..f0dea8a8e002
--- /dev/null
+++ b/drivers/thermal/broadcom/Kconfig
@@ -0,0 +1,8 @@
+config BCM_NS_THERMAL
+	tristate "Northstar thermal driver"
+	depends on ARCH_BCM_IPROC || COMPILE_TEST
+	help
+	  Northstar is a family of SoCs that includes e.g. BCM4708, BCM47081,
+	  BCM4709 and BCM47094. It contains DMU (Device Management Unit) block
+	  with a thermal sensor that allows checking CPU temperature. This
+	  driver provides support for it.
diff --git a/drivers/thermal/broadcom/Makefile b/drivers/thermal/broadcom/Makefile
new file mode 100644
index 000000000000..059df9a0ed69
--- /dev/null
+++ b/drivers/thermal/broadcom/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_BCM_NS_THERMAL)		+= ns-thermal.o
diff --git a/drivers/thermal/broadcom/ns-thermal.c b/drivers/thermal/broadcom/ns-thermal.c
new file mode 100644
index 000000000000..eab96b3572b9
--- /dev/null
+++ b/drivers/thermal/broadcom/ns-thermal.c
@@ -0,0 +1,105 @@
+/*
+ * Copyright (C) 2017 Rafa? Mi?ecki <rafal@milecki.pl>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/thermal.h>
+
+#define PVTMON_CONTROL0					0x00
+#define PVTMON_CONTROL0_SEL_MASK			0x0000000e
+#define PVTMON_CONTROL0_SEL_TEMP_MONITOR		0x00000000
+#define PVTMON_CONTROL0_SEL_TEST_MODE			0x0000000e
+#define PVTMON_STATUS					0x08
+
+struct ns_thermal {
+	struct thermal_zone_device *tz;
+	void __iomem *pvtmon;
+};
+
+static int ns_thermal_get_temp(void *data, int *temp)
+{
+	struct ns_thermal *ns_thermal = data;
+	int offset = thermal_zone_get_offset(ns_thermal->tz);
+	int slope = thermal_zone_get_slope(ns_thermal->tz);
+	u32 val;
+
+	val = readl(ns_thermal->pvtmon + PVTMON_CONTROL0);
+	if ((val & PVTMON_CONTROL0_SEL_MASK) != PVTMON_CONTROL0_SEL_TEMP_MONITOR) {
+		/* Clear current mode selection */
+		val &= ~PVTMON_CONTROL0_SEL_MASK;
+
+		/* Set temp monitor mode (it's the default actually) */
+		val |= PVTMON_CONTROL0_SEL_TEMP_MONITOR;
+
+		writel(val, ns_thermal->pvtmon + PVTMON_CONTROL0);
+	}
+
+	val = readl(ns_thermal->pvtmon + PVTMON_STATUS);
+	*temp = slope * val + offset;
+
+	return 0;
+}
+
+const struct thermal_zone_of_device_ops ns_thermal_ops = {
+	.get_temp = ns_thermal_get_temp,
+};
+
+static int ns_thermal_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct ns_thermal *ns_thermal;
+
+	ns_thermal = devm_kzalloc(dev, sizeof(*ns_thermal), GFP_KERNEL);
+	if (!ns_thermal)
+		return -ENOMEM;
+
+	ns_thermal->pvtmon = of_iomap(dev_of_node(dev), 0);
+	if (WARN_ON(!ns_thermal->pvtmon))
+		return -ENOENT;
+
+	ns_thermal->tz = devm_thermal_zone_of_sensor_register(dev, 0,
+							      ns_thermal,
+							      &ns_thermal_ops);
+	if (IS_ERR(ns_thermal->tz)) {
+		iounmap(ns_thermal->pvtmon);
+		return PTR_ERR(ns_thermal->tz);
+	}
+
+	platform_set_drvdata(pdev, ns_thermal);
+
+	return 0;
+}
+
+static int ns_thermal_remove(struct platform_device *pdev)
+{
+	struct ns_thermal *ns_thermal = platform_get_drvdata(pdev);
+
+	iounmap(ns_thermal->pvtmon);
+
+	return 0;
+}
+
+static const struct of_device_id ns_thermal_of_match[] = {
+	{ .compatible = "brcm,ns-thermal", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ns_thermal_of_match);
+
+static struct platform_driver ns_thermal_driver = {
+	.probe		= ns_thermal_probe,
+	.remove		= ns_thermal_remove,
+	.driver = {
+		.name = "ns-thermal",
+		.of_match_table = ns_thermal_of_match,
+	},
+};
+module_platform_driver(ns_thermal_driver);
+
+MODULE_DESCRIPTION("Northstar thermal driver");
+MODULE_LICENSE("GPL v2");
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH V4 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal
  2017-03-31 20:11       ` [PATCH V4 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal Rafał Miłecki
  2017-03-31 20:11         ` [PATCH V4 2/2] thermal: broadcom: add Northstar thermal driver Rafał Miłecki
@ 2017-04-01 19:51         ` Eduardo Valentin
  2017-04-01 21:50           ` Rafał Miłecki
  2017-04-03 15:48         ` [PATCH V5 " Rafał Miłecki
  2 siblings, 1 reply; 31+ messages in thread
From: Eduardo Valentin @ 2017-04-01 19:51 UTC (permalink / raw)
  To: linux-arm-kernel

Rafal,

On Fri, Mar 31, 2017 at 10:11:23PM +0200, Rafa? Mi?ecki wrote:
> From: Rafa? Mi?ecki <rafal@milecki.pl>
> 
> This commit documents binding for thermal used in Northstar family SoCs.
> 
> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
> ---
> V3: Add thermal-zones to the example
>     Rob: Because of this update, I didn't include Acked-by I got for V2
> ---
>  .../devicetree/bindings/thermal/brcm,ns-thermal    | 26 ++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/brcm,ns-thermal
> 
> diff --git a/Documentation/devicetree/bindings/thermal/brcm,ns-thermal b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
> new file mode 100644
> index 000000000000..c561c7349f17
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
> @@ -0,0 +1,26 @@
> +* Broadcom Northstar Thermal
> +
> +This binding describes thermal sensor that is part of Northstar's DMU (Device
> +Management Unit).
> +
> +Required properties:
> +- compatible : Must be "brcm,ns-thermal"
> +- reg : iomem address range of PVTMON registers
> +- #thermal-sensor-cells : Should be <0>
> +
> +Example:
> +
> +thermal: thermal at 1800c2c0 {
> +	compatible = "brcm,ns-thermal";
> +	reg = <0x1800c2c0 0x10>;
> +	#thermal-sensor-cells = <0>;
> +};
> +
> +thermal-zones {
> +	cpu_thermal: cpu-thermal {
> +		polling-delay-passive = <0>;
> +		polling-delay = <1000>;
> +		coefficients = <(-556) 418000>;
> +		thermal-sensors = <&thermal>;

You need to define trips and cooling devices here. Otherwise, makes
little sense to have this device in thermal subsystem. Here is an
example of minimal set:
https://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git/commit/?h=linus&id=1e2ac9821de6a85d3e8358f238436708d1d46869

The above has no passive action. It is just gonna shutdown the system if
temperature crosses a threshold. 

But, a typical cooling device would be CPU frequency throttling. Do you have
that up and running in your routers?

> +	};
> +};
> -- 
> 2.11.0
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V4 2/2] thermal: broadcom: add Northstar thermal driver
  2017-03-31 20:11         ` [PATCH V4 2/2] thermal: broadcom: add Northstar thermal driver Rafał Miłecki
@ 2017-04-01 19:54           ` Eduardo Valentin
  2017-04-01 20:20             ` Florian Fainelli
  2017-04-01 21:41             ` Rafał Miłecki
  0 siblings, 2 replies; 31+ messages in thread
From: Eduardo Valentin @ 2017-04-01 19:54 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Mar 31, 2017 at 10:11:24PM +0200, Rafa? Mi?ecki wrote:
> From: Rafa? Mi?ecki <rafal@milecki.pl>
> 
> Northstar is a SoC family commonly used in home routers. This commit
> adds a driver for checking CPU temperature. As Northstar Plus seems to
> also have this IP block this new symbol gets ARCH_BCM_IPROC dependency.
> 
> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> ---

This driver looks fine from what concerns the of thermal usage.
I had only one request on the DT bindings example. I believe better to
get the example fixed so bad DTs does not get copied.


> V2: Make it iProc specific as NSP can also use this driver
>     Select proper symbols in config ARCH_BCM_IPROC
>     Define PVTMON register bits
>     Update code selecting temperature monitor mode
>     Thank you Jon!
> V3: More details in help message for BCM_NS_THERMAL
>     Use slope & offset
>     Drop arch code change (I'll be submitted using a proper tree)
>     Thank you Eduardo!
> V4: Comment operations on PVTMON_CONTROL0 register
> ---
>  drivers/thermal/Kconfig               |   5 ++
>  drivers/thermal/Makefile              |   1 +
>  drivers/thermal/broadcom/Kconfig      |   8 +++
>  drivers/thermal/broadcom/Makefile     |   1 +
>  drivers/thermal/broadcom/ns-thermal.c | 105 ++++++++++++++++++++++++++++++++++

Also, I have just merged a BRCM driver. Does it make sense to move it
here too?

It does not need to be a blocking request for this driver though.

>  5 files changed, 120 insertions(+)
>  create mode 100644 drivers/thermal/broadcom/Kconfig
>  create mode 100644 drivers/thermal/broadcom/Makefile
>  create mode 100644 drivers/thermal/broadcom/ns-thermal.c
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 776b34396144..008e173ec825 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -392,6 +392,11 @@ config MTK_THERMAL
>  	  Enable this option if you want to have support for thermal management
>  	  controller present in Mediatek SoCs
>  
> +menu "Broadcom thermal drivers"
> +depends on ARCH_BCM || COMPILE_TEST
> +source "drivers/thermal/broadcom/Kconfig"
> +endmenu
> +
>  menu "Texas Instruments thermal drivers"
>  depends on ARCH_HAS_BANDGAP || COMPILE_TEST
>  depends on HAS_IOMEM
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 7adae2029355..549d81b6363c 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -27,6 +27,7 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL)	+= clock_cooling.o
>  thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o
>  
>  # platform thermal drivers
> +obj-y				+= broadcom/
>  obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM)	+= qcom-spmi-temp-alarm.o
>  obj-$(CONFIG_SPEAR_THERMAL)	+= spear_thermal.o
>  obj-$(CONFIG_ROCKCHIP_THERMAL)	+= rockchip_thermal.o
> diff --git a/drivers/thermal/broadcom/Kconfig b/drivers/thermal/broadcom/Kconfig
> new file mode 100644
> index 000000000000..f0dea8a8e002
> --- /dev/null
> +++ b/drivers/thermal/broadcom/Kconfig
> @@ -0,0 +1,8 @@
> +config BCM_NS_THERMAL
> +	tristate "Northstar thermal driver"
> +	depends on ARCH_BCM_IPROC || COMPILE_TEST
> +	help
> +	  Northstar is a family of SoCs that includes e.g. BCM4708, BCM47081,
> +	  BCM4709 and BCM47094. It contains DMU (Device Management Unit) block
> +	  with a thermal sensor that allows checking CPU temperature. This
> +	  driver provides support for it.
> diff --git a/drivers/thermal/broadcom/Makefile b/drivers/thermal/broadcom/Makefile
> new file mode 100644
> index 000000000000..059df9a0ed69
> --- /dev/null
> +++ b/drivers/thermal/broadcom/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_BCM_NS_THERMAL)		+= ns-thermal.o
> diff --git a/drivers/thermal/broadcom/ns-thermal.c b/drivers/thermal/broadcom/ns-thermal.c
> new file mode 100644
> index 000000000000..eab96b3572b9
> --- /dev/null
> +++ b/drivers/thermal/broadcom/ns-thermal.c
> @@ -0,0 +1,105 @@
> +/*
> + * Copyright (C) 2017 Rafa? Mi?ecki <rafal@milecki.pl>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/thermal.h>
> +
> +#define PVTMON_CONTROL0					0x00
> +#define PVTMON_CONTROL0_SEL_MASK			0x0000000e
> +#define PVTMON_CONTROL0_SEL_TEMP_MONITOR		0x00000000
> +#define PVTMON_CONTROL0_SEL_TEST_MODE			0x0000000e
> +#define PVTMON_STATUS					0x08
> +
> +struct ns_thermal {
> +	struct thermal_zone_device *tz;
> +	void __iomem *pvtmon;
> +};
> +
> +static int ns_thermal_get_temp(void *data, int *temp)
> +{
> +	struct ns_thermal *ns_thermal = data;
> +	int offset = thermal_zone_get_offset(ns_thermal->tz);
> +	int slope = thermal_zone_get_slope(ns_thermal->tz);
> +	u32 val;
> +
> +	val = readl(ns_thermal->pvtmon + PVTMON_CONTROL0);
> +	if ((val & PVTMON_CONTROL0_SEL_MASK) != PVTMON_CONTROL0_SEL_TEMP_MONITOR) {
> +		/* Clear current mode selection */
> +		val &= ~PVTMON_CONTROL0_SEL_MASK;
> +
> +		/* Set temp monitor mode (it's the default actually) */
> +		val |= PVTMON_CONTROL0_SEL_TEMP_MONITOR;
> +
> +		writel(val, ns_thermal->pvtmon + PVTMON_CONTROL0);
> +	}
> +
> +	val = readl(ns_thermal->pvtmon + PVTMON_STATUS);
> +	*temp = slope * val + offset;
> +
> +	return 0;
> +}
> +
> +const struct thermal_zone_of_device_ops ns_thermal_ops = {
> +	.get_temp = ns_thermal_get_temp,
> +};
> +
> +static int ns_thermal_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct ns_thermal *ns_thermal;
> +
> +	ns_thermal = devm_kzalloc(dev, sizeof(*ns_thermal), GFP_KERNEL);
> +	if (!ns_thermal)
> +		return -ENOMEM;
> +
> +	ns_thermal->pvtmon = of_iomap(dev_of_node(dev), 0);
> +	if (WARN_ON(!ns_thermal->pvtmon))
> +		return -ENOENT;
> +
> +	ns_thermal->tz = devm_thermal_zone_of_sensor_register(dev, 0,
> +							      ns_thermal,
> +							      &ns_thermal_ops);
> +	if (IS_ERR(ns_thermal->tz)) {
> +		iounmap(ns_thermal->pvtmon);
> +		return PTR_ERR(ns_thermal->tz);
> +	}
> +
> +	platform_set_drvdata(pdev, ns_thermal);
> +
> +	return 0;
> +}
> +
> +static int ns_thermal_remove(struct platform_device *pdev)
> +{
> +	struct ns_thermal *ns_thermal = platform_get_drvdata(pdev);
> +
> +	iounmap(ns_thermal->pvtmon);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ns_thermal_of_match[] = {
> +	{ .compatible = "brcm,ns-thermal", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, ns_thermal_of_match);
> +
> +static struct platform_driver ns_thermal_driver = {
> +	.probe		= ns_thermal_probe,
> +	.remove		= ns_thermal_remove,
> +	.driver = {
> +		.name = "ns-thermal",
> +		.of_match_table = ns_thermal_of_match,
> +	},
> +};
> +module_platform_driver(ns_thermal_driver);
> +
> +MODULE_DESCRIPTION("Northstar thermal driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.11.0
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V4 2/2] thermal: broadcom: add Northstar thermal driver
  2017-04-01 19:54           ` Eduardo Valentin
@ 2017-04-01 20:20             ` Florian Fainelli
  2017-04-01 21:41             ` Rafał Miłecki
  1 sibling, 0 replies; 31+ messages in thread
From: Florian Fainelli @ 2017-04-01 20:20 UTC (permalink / raw)
  To: linux-arm-kernel

On April 1, 2017 12:54:45 PM PDT, Eduardo Valentin <edubezval@gmail.com> wrote:
>Also, I have just merged a BRCM driver. Does it make sense to move it
>here too?

I think it does make sense to have all Broadcom SoCs thermal drivers under a broadcom/ directory. As a matter of fact, I will be submitting another one (for brcmstb SoCs) shortly.
Hi Eduardo,
-- 
Florian

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V4 2/2] thermal: broadcom: add Northstar thermal driver
  2017-04-01 19:54           ` Eduardo Valentin
  2017-04-01 20:20             ` Florian Fainelli
@ 2017-04-01 21:41             ` Rafał Miłecki
  1 sibling, 0 replies; 31+ messages in thread
From: Rafał Miłecki @ 2017-04-01 21:41 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/01/2017 09:54 PM, Eduardo Valentin wrote:
> On Fri, Mar 31, 2017 at 10:11:24PM +0200, Rafa? Mi?ecki wrote:
>> From: Rafa? Mi?ecki <rafal@milecki.pl>
>>
>> Northstar is a SoC family commonly used in home routers. This commit
>> adds a driver for checking CPU temperature. As Northstar Plus seems to
>> also have this IP block this new symbol gets ARCH_BCM_IPROC dependency.
>>
>> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
>> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
>> ---
>
> This driver looks fine from what concerns the of thermal usage.
> I had only one request on the DT bindings example. I believe better to
> get the example fixed so bad DTs does not get copied.

Thanks! I'll comment on this in a reply to patch 1/2.


>> V2: Make it iProc specific as NSP can also use this driver
>>     Select proper symbols in config ARCH_BCM_IPROC
>>     Define PVTMON register bits
>>     Update code selecting temperature monitor mode
>>     Thank you Jon!
>> V3: More details in help message for BCM_NS_THERMAL
>>     Use slope & offset
>>     Drop arch code change (I'll be submitted using a proper tree)
>>     Thank you Eduardo!
>> V4: Comment operations on PVTMON_CONTROL0 register
>> ---
>>  drivers/thermal/Kconfig               |   5 ++
>>  drivers/thermal/Makefile              |   1 +
>>  drivers/thermal/broadcom/Kconfig      |   8 +++
>>  drivers/thermal/broadcom/Makefile     |   1 +
>>  drivers/thermal/broadcom/ns-thermal.c | 105 ++++++++++++++++++++++++++++++++++
>
> Also, I have just merged a BRCM driver. Does it make sense to move it
> here too?
>
> It does not need to be a blocking request for this driver though.

Yes, I plan to send a patch for that after getting this one accepted.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V4 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal
  2017-04-01 19:51         ` [PATCH V4 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal Eduardo Valentin
@ 2017-04-01 21:50           ` Rafał Miłecki
  2017-04-02  4:13             ` Eduardo Valentin
  2017-04-03  3:07             ` Jon Mason
  0 siblings, 2 replies; 31+ messages in thread
From: Rafał Miłecki @ 2017-04-01 21:50 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/01/2017 09:51 PM, Eduardo Valentin wrote:
> On Fri, Mar 31, 2017 at 10:11:23PM +0200, Rafa? Mi?ecki wrote:
>> From: Rafa? Mi?ecki <rafal@milecki.pl>
>>
>> This commit documents binding for thermal used in Northstar family SoCs.
>>
>> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
>> ---
>> V3: Add thermal-zones to the example
>>     Rob: Because of this update, I didn't include Acked-by I got for V2
>> ---
>>  .../devicetree/bindings/thermal/brcm,ns-thermal    | 26 ++++++++++++++++++++++
>>  1 file changed, 26 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/thermal/brcm,ns-thermal
>>
>> diff --git a/Documentation/devicetree/bindings/thermal/brcm,ns-thermal b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
>> new file mode 100644
>> index 000000000000..c561c7349f17
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
>> @@ -0,0 +1,26 @@
>> +* Broadcom Northstar Thermal
>> +
>> +This binding describes thermal sensor that is part of Northstar's DMU (Device
>> +Management Unit).
>> +
>> +Required properties:
>> +- compatible : Must be "brcm,ns-thermal"
>> +- reg : iomem address range of PVTMON registers
>> +- #thermal-sensor-cells : Should be <0>
>> +
>> +Example:
>> +
>> +thermal: thermal at 1800c2c0 {
>> +	compatible = "brcm,ns-thermal";
>> +	reg = <0x1800c2c0 0x10>;
>> +	#thermal-sensor-cells = <0>;
>> +};
>> +
>> +thermal-zones {
>> +	cpu_thermal: cpu-thermal {
>> +		polling-delay-passive = <0>;
>> +		polling-delay = <1000>;
>> +		coefficients = <(-556) 418000>;
>> +		thermal-sensors = <&thermal>;
>
> You need to define trips and cooling devices here. Otherwise, makes
> little sense to have this device in thermal subsystem. Here is an
> example of minimal set:
> https://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git/commit/?h=linus&id=1e2ac9821de6a85d3e8358f238436708d1d46869
>
> The above has no passive action. It is just gonna shutdown the system if
> temperature crosses a threshold.
>
> But, a typical cooling device would be CPU frequency throttling. Do you have
> that up and running in your routers?

I don't have CPU freq throttling, so shutdown will be the only solution for
critical temp right now.

I know I should have at least a trip for critical temperature, but the problem
is I don't know what value to use. There isn't any info about this in public
datasheets. Broadcom's SDK doesn't mention it. Vendors share only the max
environment temp, not the max CPU temp.

So for now I only meant to provide user space access to reading current CPU
temperature. I could do some stress tests and ask other users to do it as well.

Or maybe I could just put in Documentation some round value that makes more or
less sense and then work on a proper content of real DTS files?

Unless we can get some hint from Broadcom people. Jon? Florian? Anyone?

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V4 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal
  2017-04-01 21:50           ` Rafał Miłecki
@ 2017-04-02  4:13             ` Eduardo Valentin
  2017-04-03  3:07             ` Jon Mason
  1 sibling, 0 replies; 31+ messages in thread
From: Eduardo Valentin @ 2017-04-02  4:13 UTC (permalink / raw)
  To: linux-arm-kernel

Hey,

On Sat, Apr 01, 2017 at 11:50:26PM +0200, Rafa? Mi?ecki wrote:
> On 04/01/2017 09:51 PM, Eduardo Valentin wrote:
> >On Fri, Mar 31, 2017 at 10:11:23PM +0200, Rafa? Mi?ecki wrote:
> >>From: Rafa? Mi?ecki <rafal@milecki.pl>
> >>
> >>This commit documents binding for thermal used in Northstar family SoCs.
> >>
> >>Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
> >>---
> >>V3: Add thermal-zones to the example
> >>    Rob: Because of this update, I didn't include Acked-by I got for V2
> >>---
> >> .../devicetree/bindings/thermal/brcm,ns-thermal    | 26 ++++++++++++++++++++++
> >> 1 file changed, 26 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/thermal/brcm,ns-thermal
> >>
> >>diff --git a/Documentation/devicetree/bindings/thermal/brcm,ns-thermal b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
> >>new file mode 100644
> >>index 000000000000..c561c7349f17
> >>--- /dev/null
> >>+++ b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
> >>@@ -0,0 +1,26 @@
> >>+* Broadcom Northstar Thermal
> >>+
> >>+This binding describes thermal sensor that is part of Northstar's DMU (Device
> >>+Management Unit).
> >>+
> >>+Required properties:
> >>+- compatible : Must be "brcm,ns-thermal"
> >>+- reg : iomem address range of PVTMON registers
> >>+- #thermal-sensor-cells : Should be <0>
> >>+
> >>+Example:
> >>+
> >>+thermal: thermal at 1800c2c0 {
> >>+	compatible = "brcm,ns-thermal";
> >>+	reg = <0x1800c2c0 0x10>;
> >>+	#thermal-sensor-cells = <0>;
> >>+};
> >>+
> >>+thermal-zones {
> >>+	cpu_thermal: cpu-thermal {
> >>+		polling-delay-passive = <0>;
> >>+		polling-delay = <1000>;
> >>+		coefficients = <(-556) 418000>;
> >>+		thermal-sensors = <&thermal>;
> >
> >You need to define trips and cooling devices here. Otherwise, makes
> >little sense to have this device in thermal subsystem. Here is an
> >example of minimal set:
> >https://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git/commit/?h=linus&id=1e2ac9821de6a85d3e8358f238436708d1d46869
> >
> >The above has no passive action. It is just gonna shutdown the system if
> >temperature crosses a threshold.
> >
> >But, a typical cooling device would be CPU frequency throttling. Do you have
> >that up and running in your routers?
> 
> I don't have CPU freq throttling, so shutdown will be the only solution for
> critical temp right now.

OK. Is there any plans to get cpu cooling / DVFS working on your boards?

> 
> I know I should have at least a trip for critical temperature, but the problem
> is I don't know what value to use. There isn't any info about this in public

And that is enough justification to block this to go upstream. We need
the correct shutdown threshold value to put as example. If we don't,
users of this drivers will be pretty much guessing.

> datasheets. Broadcom's SDK doesn't mention it. Vendors share only the max
> environment temp, not the max CPU temp.

Silicon goes bad typically at 125C. But that also typically requires a
good extrapolation of hotspot(s). Some drivers prefer to play safer and
use lower numbers (100C).

> 
> So for now I only meant to provide user space access to reading current CPU
> temperature. I could do some stress tests and ask other users to do it as well.
> 

I see..


> Or maybe I could just put in Documentation some round value that makes more or
> less sense and then work on a proper content of real DTS files?
> 

The problem with the above is that I see very commonly examples being
copied into DTS. So, better to have a good binding
example/documentation. That is what they are for anyway.

> Unless we can get some hint from Broadcom people. Jon? Florian? Anyone?
> 

Yeah, that would be probably best.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V4 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal
  2017-04-01 21:50           ` Rafał Miłecki
  2017-04-02  4:13             ` Eduardo Valentin
@ 2017-04-03  3:07             ` Jon Mason
  2017-04-03 14:54               ` Jon Mason
  1 sibling, 1 reply; 31+ messages in thread
From: Jon Mason @ 2017-04-03  3:07 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Apr 1, 2017 at 5:50 PM, Rafa? Mi?ecki <zajec5@gmail.com> wrote:
> On 04/01/2017 09:51 PM, Eduardo Valentin wrote:
>>
>> On Fri, Mar 31, 2017 at 10:11:23PM +0200, Rafa? Mi?ecki wrote:
>>>
>>> From: Rafa? Mi?ecki <rafal@milecki.pl>
>>>
>>> This commit documents binding for thermal used in Northstar family SoCs.
>>>
>>> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
>>> ---
>>> V3: Add thermal-zones to the example
>>>     Rob: Because of this update, I didn't include Acked-by I got for V2
>>> ---
>>>  .../devicetree/bindings/thermal/brcm,ns-thermal    | 26
>>> ++++++++++++++++++++++
>>>  1 file changed, 26 insertions(+)
>>>  create mode 100644
>>> Documentation/devicetree/bindings/thermal/brcm,ns-thermal
>>>
>>> diff --git a/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
>>> b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
>>> new file mode 100644
>>> index 000000000000..c561c7349f17
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
>>> @@ -0,0 +1,26 @@
>>> +* Broadcom Northstar Thermal
>>> +
>>> +This binding describes thermal sensor that is part of Northstar's DMU
>>> (Device
>>> +Management Unit).
>>> +
>>> +Required properties:
>>> +- compatible : Must be "brcm,ns-thermal"
>>> +- reg : iomem address range of PVTMON registers
>>> +- #thermal-sensor-cells : Should be <0>
>>> +
>>> +Example:
>>> +
>>> +thermal: thermal at 1800c2c0 {
>>> +       compatible = "brcm,ns-thermal";
>>> +       reg = <0x1800c2c0 0x10>;
>>> +       #thermal-sensor-cells = <0>;
>>> +};
>>> +
>>> +thermal-zones {
>>> +       cpu_thermal: cpu-thermal {
>>> +               polling-delay-passive = <0>;
>>> +               polling-delay = <1000>;
>>> +               coefficients = <(-556) 418000>;
>>> +               thermal-sensors = <&thermal>;
>>
>>
>> You need to define trips and cooling devices here. Otherwise, makes
>> little sense to have this device in thermal subsystem. Here is an
>> example of minimal set:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git/commit/?h=linus&id=1e2ac9821de6a85d3e8358f238436708d1d46869
>>
>> The above has no passive action. It is just gonna shutdown the system if
>> temperature crosses a threshold.
>>
>> But, a typical cooling device would be CPU frequency throttling. Do you
>> have
>> that up and running in your routers?
>
>
> I don't have CPU freq throttling, so shutdown will be the only solution for
> critical temp right now.
>
> I know I should have at least a trip for critical temperature, but the
> problem
> is I don't know what value to use. There isn't any info about this in public
> datasheets. Broadcom's SDK doesn't mention it. Vendors share only the max
> environment temp, not the max CPU temp.
>
> So for now I only meant to provide user space access to reading current CPU
> temperature. I could do some stress tests and ask other users to do it as
> well.
>
> Or maybe I could just put in Documentation some round value that makes more
> or
> less sense and then work on a proper content of real DTS files?
>
> Unless we can get some hint from Broadcom people. Jon? Florian? Anyone?

I'll poke around and see if I can find a datasheet for NS/NSP.  Worst
case, I can ask one of the HW engineers for NSP, and we can use the
same value for NS.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V4 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal
  2017-04-03  3:07             ` Jon Mason
@ 2017-04-03 14:54               ` Jon Mason
  2017-04-03 14:57                 ` Rafał Miłecki
  0 siblings, 1 reply; 31+ messages in thread
From: Jon Mason @ 2017-04-03 14:54 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun, Apr 2, 2017 at 11:07 PM, Jon Mason <jon.mason@broadcom.com> wrote:
> On Sat, Apr 1, 2017 at 5:50 PM, Rafa? Mi?ecki <zajec5@gmail.com> wrote:
>> On 04/01/2017 09:51 PM, Eduardo Valentin wrote:
>>>
>>> On Fri, Mar 31, 2017 at 10:11:23PM +0200, Rafa? Mi?ecki wrote:
>>>>
>>>> From: Rafa? Mi?ecki <rafal@milecki.pl>
>>>>
>>>> This commit documents binding for thermal used in Northstar family SoCs.
>>>>
>>>> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
>>>> ---
>>>> V3: Add thermal-zones to the example
>>>>     Rob: Because of this update, I didn't include Acked-by I got for V2
>>>> ---
>>>>  .../devicetree/bindings/thermal/brcm,ns-thermal    | 26
>>>> ++++++++++++++++++++++
>>>>  1 file changed, 26 insertions(+)
>>>>  create mode 100644
>>>> Documentation/devicetree/bindings/thermal/brcm,ns-thermal
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
>>>> b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
>>>> new file mode 100644
>>>> index 000000000000..c561c7349f17
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
>>>> @@ -0,0 +1,26 @@
>>>> +* Broadcom Northstar Thermal
>>>> +
>>>> +This binding describes thermal sensor that is part of Northstar's DMU
>>>> (Device
>>>> +Management Unit).
>>>> +
>>>> +Required properties:
>>>> +- compatible : Must be "brcm,ns-thermal"
>>>> +- reg : iomem address range of PVTMON registers
>>>> +- #thermal-sensor-cells : Should be <0>
>>>> +
>>>> +Example:
>>>> +
>>>> +thermal: thermal at 1800c2c0 {
>>>> +       compatible = "brcm,ns-thermal";
>>>> +       reg = <0x1800c2c0 0x10>;
>>>> +       #thermal-sensor-cells = <0>;
>>>> +};
>>>> +
>>>> +thermal-zones {
>>>> +       cpu_thermal: cpu-thermal {
>>>> +               polling-delay-passive = <0>;
>>>> +               polling-delay = <1000>;
>>>> +               coefficients = <(-556) 418000>;
>>>> +               thermal-sensors = <&thermal>;
>>>
>>>
>>> You need to define trips and cooling devices here. Otherwise, makes
>>> little sense to have this device in thermal subsystem. Here is an
>>> example of minimal set:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git/commit/?h=linus&id=1e2ac9821de6a85d3e8358f238436708d1d46869
>>>
>>> The above has no passive action. It is just gonna shutdown the system if
>>> temperature crosses a threshold.
>>>
>>> But, a typical cooling device would be CPU frequency throttling. Do you
>>> have
>>> that up and running in your routers?
>>
>>
>> I don't have CPU freq throttling, so shutdown will be the only solution for
>> critical temp right now.
>>
>> I know I should have at least a trip for critical temperature, but the
>> problem
>> is I don't know what value to use. There isn't any info about this in public
>> datasheets. Broadcom's SDK doesn't mention it. Vendors share only the max
>> environment temp, not the max CPU temp.
>>
>> So for now I only meant to provide user space access to reading current CPU
>> temperature. I could do some stress tests and ask other users to do it as
>> well.
>>
>> Or maybe I could just put in Documentation some round value that makes more
>> or
>> less sense and then work on a proper content of real DTS files?
>>
>> Unless we can get some hint from Broadcom people. Jon? Florian? Anyone?
>
> I'll poke around and see if I can find a datasheet for NS/NSP.  Worst
> case, I can ask one of the HW engineers for NSP, and we can use the
> same value for NS.

In the NS documentation, under "Absolute Maximum Ratings":

The "Maximum Junction Temperature" is 125 C
The "Commercial Ambient Temperature (Operating)" range is 0 to 75 C
The "Industrial  Ambient Temperature (Operating)" range is -40 to 85 C
The "Storage Temperature" range is -40 to 125 C

In the NSP documentation, under "Absolute Maximum Ratings":

The "Maximum Junction Temperature" is 110 C
The "Commercial Ambient Temperature (Operating)" range is 0 to 75 C
The "Industrial  Ambient Temperature (Operating)" range is -40 to 85 C
The "Storage Temperature" range is -40 to 125 C

I believe the first one is the number you are looking for.

Thanks,
Jon

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V4 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal
  2017-04-03 14:54               ` Jon Mason
@ 2017-04-03 14:57                 ` Rafał Miłecki
  0 siblings, 0 replies; 31+ messages in thread
From: Rafał Miłecki @ 2017-04-03 14:57 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/03/2017 04:54 PM, Jon Mason wrote:
> On Sun, Apr 2, 2017 at 11:07 PM, Jon Mason <jon.mason@broadcom.com> wrote:
>> On Sat, Apr 1, 2017 at 5:50 PM, Rafa? Mi?ecki <zajec5@gmail.com> wrote:
>>> On 04/01/2017 09:51 PM, Eduardo Valentin wrote:
>>>>
>>>> On Fri, Mar 31, 2017 at 10:11:23PM +0200, Rafa? Mi?ecki wrote:
>>>>>
>>>>> From: Rafa? Mi?ecki <rafal@milecki.pl>
>>>>>
>>>>> This commit documents binding for thermal used in Northstar family SoCs.
>>>>>
>>>>> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
>>>>> ---
>>>>> V3: Add thermal-zones to the example
>>>>>     Rob: Because of this update, I didn't include Acked-by I got for V2
>>>>> ---
>>>>>  .../devicetree/bindings/thermal/brcm,ns-thermal    | 26
>>>>> ++++++++++++++++++++++
>>>>>  1 file changed, 26 insertions(+)
>>>>>  create mode 100644
>>>>> Documentation/devicetree/bindings/thermal/brcm,ns-thermal
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
>>>>> b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
>>>>> new file mode 100644
>>>>> index 000000000000..c561c7349f17
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
>>>>> @@ -0,0 +1,26 @@
>>>>> +* Broadcom Northstar Thermal
>>>>> +
>>>>> +This binding describes thermal sensor that is part of Northstar's DMU
>>>>> (Device
>>>>> +Management Unit).
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible : Must be "brcm,ns-thermal"
>>>>> +- reg : iomem address range of PVTMON registers
>>>>> +- #thermal-sensor-cells : Should be <0>
>>>>> +
>>>>> +Example:
>>>>> +
>>>>> +thermal: thermal at 1800c2c0 {
>>>>> +       compatible = "brcm,ns-thermal";
>>>>> +       reg = <0x1800c2c0 0x10>;
>>>>> +       #thermal-sensor-cells = <0>;
>>>>> +};
>>>>> +
>>>>> +thermal-zones {
>>>>> +       cpu_thermal: cpu-thermal {
>>>>> +               polling-delay-passive = <0>;
>>>>> +               polling-delay = <1000>;
>>>>> +               coefficients = <(-556) 418000>;
>>>>> +               thermal-sensors = <&thermal>;
>>>>
>>>>
>>>> You need to define trips and cooling devices here. Otherwise, makes
>>>> little sense to have this device in thermal subsystem. Here is an
>>>> example of minimal set:
>>>>
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git/commit/?h=linus&id=1e2ac9821de6a85d3e8358f238436708d1d46869
>>>>
>>>> The above has no passive action. It is just gonna shutdown the system if
>>>> temperature crosses a threshold.
>>>>
>>>> But, a typical cooling device would be CPU frequency throttling. Do you
>>>> have
>>>> that up and running in your routers?
>>>
>>>
>>> I don't have CPU freq throttling, so shutdown will be the only solution for
>>> critical temp right now.
>>>
>>> I know I should have at least a trip for critical temperature, but the
>>> problem
>>> is I don't know what value to use. There isn't any info about this in public
>>> datasheets. Broadcom's SDK doesn't mention it. Vendors share only the max
>>> environment temp, not the max CPU temp.
>>>
>>> So for now I only meant to provide user space access to reading current CPU
>>> temperature. I could do some stress tests and ask other users to do it as
>>> well.
>>>
>>> Or maybe I could just put in Documentation some round value that makes more
>>> or
>>> less sense and then work on a proper content of real DTS files?
>>>
>>> Unless we can get some hint from Broadcom people. Jon? Florian? Anyone?
>>
>> I'll poke around and see if I can find a datasheet for NS/NSP.  Worst
>> case, I can ask one of the HW engineers for NSP, and we can use the
>> same value for NS.
>
> In the NS documentation, under "Absolute Maximum Ratings":
>
> The "Maximum Junction Temperature" is 125 C
> The "Commercial Ambient Temperature (Operating)" range is 0 to 75 C
> The "Industrial  Ambient Temperature (Operating)" range is -40 to 85 C
> The "Storage Temperature" range is -40 to 125 C
>
> In the NSP documentation, under "Absolute Maximum Ratings":
>
> The "Maximum Junction Temperature" is 110 C
> The "Commercial Ambient Temperature (Operating)" range is 0 to 75 C
> The "Industrial  Ambient Temperature (Operating)" range is -40 to 85 C
> The "Storage Temperature" range is -40 to 125 C
>
> I believe the first one is the number you are looking for.

Thanks a lot for this valuable info! I'll send next version today.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V5 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal
  2017-03-31 20:11       ` [PATCH V4 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal Rafał Miłecki
  2017-03-31 20:11         ` [PATCH V4 2/2] thermal: broadcom: add Northstar thermal driver Rafał Miłecki
  2017-04-01 19:51         ` [PATCH V4 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal Eduardo Valentin
@ 2017-04-03 15:48         ` Rafał Miłecki
  2017-04-03 15:48           ` [PATCH V5 2/2] thermal: broadcom: add Northstar thermal driver Rafał Miłecki
  2017-04-10 15:00           ` [PATCH V5 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal Rob Herring
  2 siblings, 2 replies; 31+ messages in thread
From: Rafał Miłecki @ 2017-04-03 15:48 UTC (permalink / raw)
  To: linux-arm-kernel

From: Rafa? Mi?ecki <rafal@milecki.pl>

This commit documents binding for thermal used in Northstar family SoCs.

There isn't any known Northstar device with active cooling system so DT
example has empty cooling-maps node. There is also no support for CPU
frequency throttling so I put only a critical trip in the example.

Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
---
V3: Add thermal-zones to the example
V5: Extend example by including trips and cooling-maps. Update commit message
---
 .../devicetree/bindings/thermal/brcm,ns-thermal    | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/brcm,ns-thermal

diff --git a/Documentation/devicetree/bindings/thermal/brcm,ns-thermal b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
new file mode 100644
index 000000000000..68e047170039
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/brcm,ns-thermal
@@ -0,0 +1,37 @@
+* Broadcom Northstar Thermal
+
+This binding describes thermal sensor that is part of Northstar's DMU (Device
+Management Unit).
+
+Required properties:
+- compatible : Must be "brcm,ns-thermal"
+- reg : iomem address range of PVTMON registers
+- #thermal-sensor-cells : Should be <0>
+
+Example:
+
+thermal: thermal at 1800c2c0 {
+	compatible = "brcm,ns-thermal";
+	reg = <0x1800c2c0 0x10>;
+	#thermal-sensor-cells = <0>;
+};
+
+thermal-zones {
+	cpu_thermal: cpu-thermal {
+		polling-delay-passive = <0>;
+		polling-delay = <1000>;
+		coefficients = <(-556) 418000>;
+		thermal-sensors = <&thermal>;
+
+		trips {
+			cpu-crit {
+				temperature	= <125000>;
+				hysteresis	= <0>;
+				type		= "critical";
+			};
+		};
+
+		cooling-maps {
+		};
+	};
+};
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH V5 2/2] thermal: broadcom: add Northstar thermal driver
  2017-04-03 15:48         ` [PATCH V5 " Rafał Miłecki
@ 2017-04-03 15:48           ` Rafał Miłecki
  2017-04-07  4:42             ` Eduardo Valentin
  2017-04-10 15:00           ` [PATCH V5 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal Rob Herring
  1 sibling, 1 reply; 31+ messages in thread
From: Rafał Miłecki @ 2017-04-03 15:48 UTC (permalink / raw)
  To: linux-arm-kernel

From: Rafa? Mi?ecki <rafal@milecki.pl>

Northstar is a SoC family commonly used in home routers. This commit
adds a driver for checking CPU temperature. As Northstar Plus seems to
also have this IP block this new symbol gets ARCH_BCM_IPROC dependency.

Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
Signed-off-by: Jon Mason <jon.mason@broadcom.com>
---
V2: Make it iProc specific as NSP can also use this driver
    Select proper symbols in config ARCH_BCM_IPROC
    Define PVTMON register bits
    Update code selecting temperature monitor mode
    Thank you Jon!
V3: More details in help message for BCM_NS_THERMAL
    Use slope & offset
    Drop arch code change (I'll be submitted using a proper tree)
    Thank you Eduardo!
V4: Comment operations on PVTMON_CONTROL0 register
---
 drivers/thermal/Kconfig               |   5 ++
 drivers/thermal/Makefile              |   1 +
 drivers/thermal/broadcom/Kconfig      |   8 +++
 drivers/thermal/broadcom/Makefile     |   1 +
 drivers/thermal/broadcom/ns-thermal.c | 105 ++++++++++++++++++++++++++++++++++
 5 files changed, 120 insertions(+)
 create mode 100644 drivers/thermal/broadcom/Kconfig
 create mode 100644 drivers/thermal/broadcom/Makefile
 create mode 100644 drivers/thermal/broadcom/ns-thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 3bd24063375e..ac7301703d03 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -392,6 +392,11 @@ config MTK_THERMAL
 	  Enable this option if you want to have support for thermal management
 	  controller present in Mediatek SoCs
 
+menu "Broadcom thermal drivers"
+depends on ARCH_BCM || COMPILE_TEST
+source "drivers/thermal/broadcom/Kconfig"
+endmenu
+
 menu "Texas Instruments thermal drivers"
 depends on ARCH_HAS_BANDGAP || COMPILE_TEST
 depends on HAS_IOMEM
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index f23cde05dac6..6b7706b9f27c 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -27,6 +27,7 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL)	+= clock_cooling.o
 thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o
 
 # platform thermal drivers
+obj-y				+= broadcom/
 obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM)	+= qcom-spmi-temp-alarm.o
 obj-$(CONFIG_SPEAR_THERMAL)	+= spear_thermal.o
 obj-$(CONFIG_ROCKCHIP_THERMAL)	+= rockchip_thermal.o
diff --git a/drivers/thermal/broadcom/Kconfig b/drivers/thermal/broadcom/Kconfig
new file mode 100644
index 000000000000..f0dea8a8e002
--- /dev/null
+++ b/drivers/thermal/broadcom/Kconfig
@@ -0,0 +1,8 @@
+config BCM_NS_THERMAL
+	tristate "Northstar thermal driver"
+	depends on ARCH_BCM_IPROC || COMPILE_TEST
+	help
+	  Northstar is a family of SoCs that includes e.g. BCM4708, BCM47081,
+	  BCM4709 and BCM47094. It contains DMU (Device Management Unit) block
+	  with a thermal sensor that allows checking CPU temperature. This
+	  driver provides support for it.
diff --git a/drivers/thermal/broadcom/Makefile b/drivers/thermal/broadcom/Makefile
new file mode 100644
index 000000000000..059df9a0ed69
--- /dev/null
+++ b/drivers/thermal/broadcom/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_BCM_NS_THERMAL)		+= ns-thermal.o
diff --git a/drivers/thermal/broadcom/ns-thermal.c b/drivers/thermal/broadcom/ns-thermal.c
new file mode 100644
index 000000000000..eab96b3572b9
--- /dev/null
+++ b/drivers/thermal/broadcom/ns-thermal.c
@@ -0,0 +1,105 @@
+/*
+ * Copyright (C) 2017 Rafa? Mi?ecki <rafal@milecki.pl>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/thermal.h>
+
+#define PVTMON_CONTROL0					0x00
+#define PVTMON_CONTROL0_SEL_MASK			0x0000000e
+#define PVTMON_CONTROL0_SEL_TEMP_MONITOR		0x00000000
+#define PVTMON_CONTROL0_SEL_TEST_MODE			0x0000000e
+#define PVTMON_STATUS					0x08
+
+struct ns_thermal {
+	struct thermal_zone_device *tz;
+	void __iomem *pvtmon;
+};
+
+static int ns_thermal_get_temp(void *data, int *temp)
+{
+	struct ns_thermal *ns_thermal = data;
+	int offset = thermal_zone_get_offset(ns_thermal->tz);
+	int slope = thermal_zone_get_slope(ns_thermal->tz);
+	u32 val;
+
+	val = readl(ns_thermal->pvtmon + PVTMON_CONTROL0);
+	if ((val & PVTMON_CONTROL0_SEL_MASK) != PVTMON_CONTROL0_SEL_TEMP_MONITOR) {
+		/* Clear current mode selection */
+		val &= ~PVTMON_CONTROL0_SEL_MASK;
+
+		/* Set temp monitor mode (it's the default actually) */
+		val |= PVTMON_CONTROL0_SEL_TEMP_MONITOR;
+
+		writel(val, ns_thermal->pvtmon + PVTMON_CONTROL0);
+	}
+
+	val = readl(ns_thermal->pvtmon + PVTMON_STATUS);
+	*temp = slope * val + offset;
+
+	return 0;
+}
+
+const struct thermal_zone_of_device_ops ns_thermal_ops = {
+	.get_temp = ns_thermal_get_temp,
+};
+
+static int ns_thermal_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct ns_thermal *ns_thermal;
+
+	ns_thermal = devm_kzalloc(dev, sizeof(*ns_thermal), GFP_KERNEL);
+	if (!ns_thermal)
+		return -ENOMEM;
+
+	ns_thermal->pvtmon = of_iomap(dev_of_node(dev), 0);
+	if (WARN_ON(!ns_thermal->pvtmon))
+		return -ENOENT;
+
+	ns_thermal->tz = devm_thermal_zone_of_sensor_register(dev, 0,
+							      ns_thermal,
+							      &ns_thermal_ops);
+	if (IS_ERR(ns_thermal->tz)) {
+		iounmap(ns_thermal->pvtmon);
+		return PTR_ERR(ns_thermal->tz);
+	}
+
+	platform_set_drvdata(pdev, ns_thermal);
+
+	return 0;
+}
+
+static int ns_thermal_remove(struct platform_device *pdev)
+{
+	struct ns_thermal *ns_thermal = platform_get_drvdata(pdev);
+
+	iounmap(ns_thermal->pvtmon);
+
+	return 0;
+}
+
+static const struct of_device_id ns_thermal_of_match[] = {
+	{ .compatible = "brcm,ns-thermal", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ns_thermal_of_match);
+
+static struct platform_driver ns_thermal_driver = {
+	.probe		= ns_thermal_probe,
+	.remove		= ns_thermal_remove,
+	.driver = {
+		.name = "ns-thermal",
+		.of_match_table = ns_thermal_of_match,
+	},
+};
+module_platform_driver(ns_thermal_driver);
+
+MODULE_DESCRIPTION("Northstar thermal driver");
+MODULE_LICENSE("GPL v2");
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH V5 2/2] thermal: broadcom: add Northstar thermal driver
  2017-04-03 15:48           ` [PATCH V5 2/2] thermal: broadcom: add Northstar thermal driver Rafał Miłecki
@ 2017-04-07  4:42             ` Eduardo Valentin
  2017-04-14 12:16               ` Rafał Miłecki
  0 siblings, 1 reply; 31+ messages in thread
From: Eduardo Valentin @ 2017-04-07  4:42 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Apr 03, 2017 at 05:48:29PM +0200, Rafa? Mi?ecki wrote:
> From: Rafa? Mi?ecki <rafal@milecki.pl>
> 
> Northstar is a SoC family commonly used in home routers. This commit
> adds a driver for checking CPU temperature. As Northstar Plus seems to
> also have this IP block this new symbol gets ARCH_BCM_IPROC dependency.
> 
> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
> Signed-off-by: Jon Mason <jon.mason@broadcom.com>

If no objection, I am applying this series.


> ---
> V2: Make it iProc specific as NSP can also use this driver
>     Select proper symbols in config ARCH_BCM_IPROC
>     Define PVTMON register bits
>     Update code selecting temperature monitor mode
>     Thank you Jon!
> V3: More details in help message for BCM_NS_THERMAL
>     Use slope & offset
>     Drop arch code change (I'll be submitted using a proper tree)
>     Thank you Eduardo!
> V4: Comment operations on PVTMON_CONTROL0 register
> ---
>  drivers/thermal/Kconfig               |   5 ++
>  drivers/thermal/Makefile              |   1 +
>  drivers/thermal/broadcom/Kconfig      |   8 +++
>  drivers/thermal/broadcom/Makefile     |   1 +
>  drivers/thermal/broadcom/ns-thermal.c | 105 ++++++++++++++++++++++++++++++++++
>  5 files changed, 120 insertions(+)
>  create mode 100644 drivers/thermal/broadcom/Kconfig
>  create mode 100644 drivers/thermal/broadcom/Makefile
>  create mode 100644 drivers/thermal/broadcom/ns-thermal.c
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 3bd24063375e..ac7301703d03 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -392,6 +392,11 @@ config MTK_THERMAL
>  	  Enable this option if you want to have support for thermal management
>  	  controller present in Mediatek SoCs
>  
> +menu "Broadcom thermal drivers"
> +depends on ARCH_BCM || COMPILE_TEST
> +source "drivers/thermal/broadcom/Kconfig"
> +endmenu
> +
>  menu "Texas Instruments thermal drivers"
>  depends on ARCH_HAS_BANDGAP || COMPILE_TEST
>  depends on HAS_IOMEM
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index f23cde05dac6..6b7706b9f27c 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -27,6 +27,7 @@ thermal_sys-$(CONFIG_CLOCK_THERMAL)	+= clock_cooling.o
>  thermal_sys-$(CONFIG_DEVFREQ_THERMAL) += devfreq_cooling.o
>  
>  # platform thermal drivers
> +obj-y				+= broadcom/
>  obj-$(CONFIG_QCOM_SPMI_TEMP_ALARM)	+= qcom-spmi-temp-alarm.o
>  obj-$(CONFIG_SPEAR_THERMAL)	+= spear_thermal.o
>  obj-$(CONFIG_ROCKCHIP_THERMAL)	+= rockchip_thermal.o
> diff --git a/drivers/thermal/broadcom/Kconfig b/drivers/thermal/broadcom/Kconfig
> new file mode 100644
> index 000000000000..f0dea8a8e002
> --- /dev/null
> +++ b/drivers/thermal/broadcom/Kconfig
> @@ -0,0 +1,8 @@
> +config BCM_NS_THERMAL
> +	tristate "Northstar thermal driver"
> +	depends on ARCH_BCM_IPROC || COMPILE_TEST
> +	help
> +	  Northstar is a family of SoCs that includes e.g. BCM4708, BCM47081,
> +	  BCM4709 and BCM47094. It contains DMU (Device Management Unit) block
> +	  with a thermal sensor that allows checking CPU temperature. This
> +	  driver provides support for it.
> diff --git a/drivers/thermal/broadcom/Makefile b/drivers/thermal/broadcom/Makefile
> new file mode 100644
> index 000000000000..059df9a0ed69
> --- /dev/null
> +++ b/drivers/thermal/broadcom/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_BCM_NS_THERMAL)		+= ns-thermal.o
> diff --git a/drivers/thermal/broadcom/ns-thermal.c b/drivers/thermal/broadcom/ns-thermal.c
> new file mode 100644
> index 000000000000..eab96b3572b9
> --- /dev/null
> +++ b/drivers/thermal/broadcom/ns-thermal.c
> @@ -0,0 +1,105 @@
> +/*
> + * Copyright (C) 2017 Rafa? Mi?ecki <rafal@milecki.pl>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/thermal.h>
> +
> +#define PVTMON_CONTROL0					0x00
> +#define PVTMON_CONTROL0_SEL_MASK			0x0000000e
> +#define PVTMON_CONTROL0_SEL_TEMP_MONITOR		0x00000000
> +#define PVTMON_CONTROL0_SEL_TEST_MODE			0x0000000e
> +#define PVTMON_STATUS					0x08
> +
> +struct ns_thermal {
> +	struct thermal_zone_device *tz;
> +	void __iomem *pvtmon;
> +};
> +
> +static int ns_thermal_get_temp(void *data, int *temp)
> +{
> +	struct ns_thermal *ns_thermal = data;
> +	int offset = thermal_zone_get_offset(ns_thermal->tz);
> +	int slope = thermal_zone_get_slope(ns_thermal->tz);
> +	u32 val;
> +
> +	val = readl(ns_thermal->pvtmon + PVTMON_CONTROL0);
> +	if ((val & PVTMON_CONTROL0_SEL_MASK) != PVTMON_CONTROL0_SEL_TEMP_MONITOR) {
> +		/* Clear current mode selection */
> +		val &= ~PVTMON_CONTROL0_SEL_MASK;
> +
> +		/* Set temp monitor mode (it's the default actually) */
> +		val |= PVTMON_CONTROL0_SEL_TEMP_MONITOR;
> +
> +		writel(val, ns_thermal->pvtmon + PVTMON_CONTROL0);
> +	}
> +
> +	val = readl(ns_thermal->pvtmon + PVTMON_STATUS);
> +	*temp = slope * val + offset;
> +
> +	return 0;
> +}
> +
> +const struct thermal_zone_of_device_ops ns_thermal_ops = {

minor correction here:

-const struct thermal_zone_of_device_ops ns_thermal_ops = {
+static const struct thermal_zone_of_device_ops ns_thermal_ops = {

but I am applying this already in my tree.

> +	.get_temp = ns_thermal_get_temp,
> +};
> +
> +static int ns_thermal_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct ns_thermal *ns_thermal;
> +
> +	ns_thermal = devm_kzalloc(dev, sizeof(*ns_thermal), GFP_KERNEL);
> +	if (!ns_thermal)
> +		return -ENOMEM;
> +
> +	ns_thermal->pvtmon = of_iomap(dev_of_node(dev), 0);
> +	if (WARN_ON(!ns_thermal->pvtmon))
> +		return -ENOENT;
> +
> +	ns_thermal->tz = devm_thermal_zone_of_sensor_register(dev, 0,
> +							      ns_thermal,
> +							      &ns_thermal_ops);
> +	if (IS_ERR(ns_thermal->tz)) {
> +		iounmap(ns_thermal->pvtmon);
> +		return PTR_ERR(ns_thermal->tz);
> +	}
> +
> +	platform_set_drvdata(pdev, ns_thermal);
> +
> +	return 0;
> +}
> +
> +static int ns_thermal_remove(struct platform_device *pdev)
> +{
> +	struct ns_thermal *ns_thermal = platform_get_drvdata(pdev);
> +
> +	iounmap(ns_thermal->pvtmon);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id ns_thermal_of_match[] = {
> +	{ .compatible = "brcm,ns-thermal", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, ns_thermal_of_match);
> +
> +static struct platform_driver ns_thermal_driver = {
> +	.probe		= ns_thermal_probe,
> +	.remove		= ns_thermal_remove,
> +	.driver = {
> +		.name = "ns-thermal",
> +		.of_match_table = ns_thermal_of_match,
> +	},
> +};
> +module_platform_driver(ns_thermal_driver);
> +
> +MODULE_DESCRIPTION("Northstar thermal driver");
> +MODULE_LICENSE("GPL v2");
> -- 
> 2.11.0
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170406/53f45255/attachment.sig>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V5 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal
  2017-04-03 15:48         ` [PATCH V5 " Rafał Miłecki
  2017-04-03 15:48           ` [PATCH V5 2/2] thermal: broadcom: add Northstar thermal driver Rafał Miłecki
@ 2017-04-10 15:00           ` Rob Herring
  1 sibling, 0 replies; 31+ messages in thread
From: Rob Herring @ 2017-04-10 15:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Apr 03, 2017 at 05:48:28PM +0200, Rafa? Mi?ecki wrote:
> From: Rafa? Mi?ecki <rafal@milecki.pl>
> 
> This commit documents binding for thermal used in Northstar family SoCs.
> 
> There isn't any known Northstar device with active cooling system so DT
> example has empty cooling-maps node. There is also no support for CPU
> frequency throttling so I put only a critical trip in the example.
> 
> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
> ---
> V3: Add thermal-zones to the example
> V5: Extend example by including trips and cooling-maps. Update commit message
> ---
>  .../devicetree/bindings/thermal/brcm,ns-thermal    | 37 ++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/thermal/brcm,ns-thermal

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V5 2/2] thermal: broadcom: add Northstar thermal driver
  2017-04-07  4:42             ` Eduardo Valentin
@ 2017-04-14 12:16               ` Rafał Miłecki
  2017-04-14 15:16                 ` Eduardo Valentin
  2017-04-17 20:09                 ` Stefan Wahren
  0 siblings, 2 replies; 31+ messages in thread
From: Rafał Miłecki @ 2017-04-14 12:16 UTC (permalink / raw)
  To: linux-arm-kernel

On 04/07/2017 06:42 AM, Eduardo Valentin wrote:
> On Mon, Apr 03, 2017 at 05:48:29PM +0200, Rafa? Mi?ecki wrote:
>> From: Rafa? Mi?ecki <rafal@milecki.pl>
>>
>> Northstar is a SoC family commonly used in home routers. This commit
>> adds a driver for checking CPU temperature. As Northstar Plus seems to
>> also have this IP block this new symbol gets ARCH_BCM_IPROC dependency.
>>
>> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
>> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
>
> If no objection, I am applying this series.

Cool, hopefully there aren't any more objections :) Once applied should I
expect this in
https://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git/log/?h=next
?

That would allow me to move bcm2835_thermal.c to the broadcom subdir.


>> +const struct thermal_zone_of_device_ops ns_thermal_ops = {
>
> minor correction here:
>
> -const struct thermal_zone_of_device_ops ns_thermal_ops = {
> +static const struct thermal_zone_of_device_ops ns_thermal_ops = {
>
> but I am applying this already in my tree.
>
>> +	.get_temp = ns_thermal_get_temp,
>> +};

Thank you!

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V5 2/2] thermal: broadcom: add Northstar thermal driver
  2017-04-14 12:16               ` Rafał Miłecki
@ 2017-04-14 15:16                 ` Eduardo Valentin
  2017-04-14 15:19                   ` Rafał Miłecki
  2017-04-17 20:09                 ` Stefan Wahren
  1 sibling, 1 reply; 31+ messages in thread
From: Eduardo Valentin @ 2017-04-14 15:16 UTC (permalink / raw)
  To: linux-arm-kernel

Hello Rafal,

On Fri, Apr 14, 2017 at 02:16:36PM +0200, Rafa? Mi?ecki wrote:
> On 04/07/2017 06:42 AM, Eduardo Valentin wrote:
> >On Mon, Apr 03, 2017 at 05:48:29PM +0200, Rafa? Mi?ecki wrote:
> >>From: Rafa? Mi?ecki <rafal@milecki.pl>
> >>
> >>Northstar is a SoC family commonly used in home routers. This commit
> >>adds a driver for checking CPU temperature. As Northstar Plus seems to
> >>also have this IP block this new symbol gets ARCH_BCM_IPROC dependency.
> >>
> >>Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
> >>Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> >
> >If no objection, I am applying this series.
> 
> Cool, hopefully there aren't any more objections :) Once applied should I
> expect this in
> https://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git/log/?h=next
> ?

They were already applied, but in my #linus branch. Now they are also in
my #next branch. The #linus branch goes to the coming merge window. The
#next branch goes to linux-next testing. Your change is now on
linux-next test, kernelci testing, and for the coming merge windown.

> 
> That would allow me to move bcm2835_thermal.c to the broadcom subdir.
> 
> 

Please send the patch.

> >>+const struct thermal_zone_of_device_ops ns_thermal_ops = {
> >
> >minor correction here:
> >
> >-const struct thermal_zone_of_device_ops ns_thermal_ops = {
> >+static const struct thermal_zone_of_device_ops ns_thermal_ops = {
> >
> >but I am applying this already in my tree.
> >
> >>+	.get_temp = ns_thermal_get_temp,
> >>+};
> 
> Thank you!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170414/d7a4f133/attachment.sig>

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V5 2/2] thermal: broadcom: add Northstar thermal driver
  2017-04-14 15:16                 ` Eduardo Valentin
@ 2017-04-14 15:19                   ` Rafał Miłecki
  0 siblings, 0 replies; 31+ messages in thread
From: Rafał Miłecki @ 2017-04-14 15:19 UTC (permalink / raw)
  To: linux-arm-kernel

On 14 April 2017 at 17:16, Eduardo Valentin <edubezval@gmail.com> wrote:
> Hello Rafal,
>
> On Fri, Apr 14, 2017 at 02:16:36PM +0200, Rafa? Mi?ecki wrote:
>> On 04/07/2017 06:42 AM, Eduardo Valentin wrote:
>> >On Mon, Apr 03, 2017 at 05:48:29PM +0200, Rafa? Mi?ecki wrote:
>> >>From: Rafa? Mi?ecki <rafal@milecki.pl>
>> >>
>> >>Northstar is a SoC family commonly used in home routers. This commit
>> >>adds a driver for checking CPU temperature. As Northstar Plus seems to
>> >>also have this IP block this new symbol gets ARCH_BCM_IPROC dependency.
>> >>
>> >>Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
>> >>Signed-off-by: Jon Mason <jon.mason@broadcom.com>
>> >
>> >If no objection, I am applying this series.
>>
>> Cool, hopefully there aren't any more objections :) Once applied should I
>> expect this in
>> https://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git/log/?h=next
>> ?
>
> They were already applied, but in my #linus branch. Now they are also in
> my #next branch. The #linus branch goes to the coming merge window. The
> #next branch goes to linux-next testing. Your change is now on
> linux-next test, kernelci testing, and for the coming merge windown.

I got it, thank you!

-- 
Rafa?

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH V5 2/2] thermal: broadcom: add Northstar thermal driver
  2017-04-14 12:16               ` Rafał Miłecki
  2017-04-14 15:16                 ` Eduardo Valentin
@ 2017-04-17 20:09                 ` Stefan Wahren
  1 sibling, 0 replies; 31+ messages in thread
From: Stefan Wahren @ 2017-04-17 20:09 UTC (permalink / raw)
  To: linux-arm-kernel

> Rafa? Mi?ecki <rafal@milecki.pl> hat am 14. April 2017 um 14:16 geschrieben:
> 
> 
> On 04/07/2017 06:42 AM, Eduardo Valentin wrote:
> > On Mon, Apr 03, 2017 at 05:48:29PM +0200, Rafa? Mi?ecki wrote:
> >> From: Rafa? Mi?ecki <rafal@milecki.pl>
> >>
> >> Northstar is a SoC family commonly used in home routers. This commit
> >> adds a driver for checking CPU temperature. As Northstar Plus seems to
> >> also have this IP block this new symbol gets ARCH_BCM_IPROC dependency.
> >>
> >> Signed-off-by: Rafa? Mi?ecki <rafal@milecki.pl>
> >> Signed-off-by: Jon Mason <jon.mason@broadcom.com>
> >
> > If no objection, I am applying this series.
> 
> Cool, hopefully there aren't any more objections :) Once applied should I
> expect this in
> https://git.kernel.org/pub/scm/linux/kernel/git/evalenti/linux-soc-thermal.git/log/?h=next
> ?
> 
> That would allow me to move bcm2835_thermal.c to the broadcom subdir.

Thanks for doing this. Btw Northstar thermal driver misses a MODULE_AUTHOR.

Regards
Stefan

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2017-04-17 20:09 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20170318155632.18099-1-zajec5@gmail.com>
2017-03-23 22:30 ` [PATCH V2 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal Rafał Miłecki
2017-03-23 22:30   ` [PATCH V2 2/2] thermal: broadcom: add Northstar thermal driver Rafał Miłecki
2017-03-24 14:35     ` Jon Mason
2017-03-31  7:03       ` Rafał Miłecki
2017-03-31 14:23         ` Jon Mason
2017-03-31 14:49           ` Rafał Miłecki
2017-03-31 17:29             ` Jon Mason
2017-03-31  3:15     ` Eduardo Valentin
2017-03-31  7:08       ` Rafał Miłecki
2017-03-31  3:17   ` [PATCH V2 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal Eduardo Valentin
2017-03-31  7:31   ` [PATCH V3 " Rafał Miłecki
2017-03-31  7:31     ` [PATCH V3 2/2] thermal: broadcom: add Northstar thermal driver Rafał Miłecki
2017-03-31 20:11       ` [PATCH V4 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal Rafał Miłecki
2017-03-31 20:11         ` [PATCH V4 2/2] thermal: broadcom: add Northstar thermal driver Rafał Miłecki
2017-04-01 19:54           ` Eduardo Valentin
2017-04-01 20:20             ` Florian Fainelli
2017-04-01 21:41             ` Rafał Miłecki
2017-04-01 19:51         ` [PATCH V4 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal Eduardo Valentin
2017-04-01 21:50           ` Rafał Miłecki
2017-04-02  4:13             ` Eduardo Valentin
2017-04-03  3:07             ` Jon Mason
2017-04-03 14:54               ` Jon Mason
2017-04-03 14:57                 ` Rafał Miłecki
2017-04-03 15:48         ` [PATCH V5 " Rafał Miłecki
2017-04-03 15:48           ` [PATCH V5 2/2] thermal: broadcom: add Northstar thermal driver Rafał Miłecki
2017-04-07  4:42             ` Eduardo Valentin
2017-04-14 12:16               ` Rafał Miłecki
2017-04-14 15:16                 ` Eduardo Valentin
2017-04-14 15:19                   ` Rafał Miłecki
2017-04-17 20:09                 ` Stefan Wahren
2017-04-10 15:00           ` [PATCH V5 1/2] dt-bindings: thermal: add support for Broadcom's Northstar thermal Rob Herring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).