* [PATCH v3 0/3] Stingray thermal driver support
@ 2018-08-09 12:54 Srinath Mannam
2018-08-09 12:54 ` [PATCH v3 1/3] dt-bindings: thermal: Add binding document for SR thermal Srinath Mannam
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Srinath Mannam @ 2018-08-09 12:54 UTC (permalink / raw)
To: Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland
Cc: devicetree, linux-kernel, bcm-kernel-feedback-list,
Srinath Mannam
These patches adds the stingray thermal driver and its
corresponding DT nodes with documentation.
Changes from v2:
- All stingray TMON DT nodes are combine together into single.
Temperature registers are combined into one mem resource.
brcm,tmon-mask parameter has available TMONs mask value.
- All available TMONs are initialized together in single
instance of driver probe call.
Changes from v1:
- Fixed auto build sparce warning.
Pramod Kumar (3):
dt-bindings: thermal: Add binding document for SR thermal
arm64: dts: stingray: Add Stingray Thermal DT support.
thermal: broadcom: Add Stingray thermal driver
.../bindings/thermal/brcm,sr-thermal.txt | 23 +++
.../arm64/boot/dts/broadcom/stingray/stingray.dtsi | 13 ++
drivers/thermal/Kconfig | 3 +-
drivers/thermal/broadcom/Kconfig | 9 +
drivers/thermal/broadcom/Makefile | 1 +
drivers/thermal/broadcom/sr-thermal.c | 216 +++++++++++++++++++++
6 files changed, 264 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt
create mode 100644 drivers/thermal/broadcom/sr-thermal.c
--
2.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v3 1/3] dt-bindings: thermal: Add binding document for SR thermal 2018-08-09 12:54 [PATCH v3 0/3] Stingray thermal driver support Srinath Mannam @ 2018-08-09 12:54 ` Srinath Mannam 2018-08-13 17:34 ` Rob Herring 2018-08-09 12:54 ` [PATCH v3 2/3] arm64: dts: stingray: Add Stingray Thermal DT support Srinath Mannam ` (2 subsequent siblings) 3 siblings, 1 reply; 9+ messages in thread From: Srinath Mannam @ 2018-08-09 12:54 UTC (permalink / raw) To: Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland Cc: devicetree, linux-kernel, bcm-kernel-feedback-list, Pramod Kumar, Srinath Mannam From: Pramod Kumar <pramod.kumar@broadcom.com> Add binding document for supported thermal implementation in Stingray. Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com> Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com> Reviewed-by: Ray Jui <ray.jui@broadcom.com> Reviewed-by: Scott Branden <scott.branden@broadcom.com> --- .../bindings/thermal/brcm,sr-thermal.txt | 23 ++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt diff --git a/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt new file mode 100644 index 0000000..fc9e6d3 --- /dev/null +++ b/Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt @@ -0,0 +1,23 @@ +* Broadcom Stingray Thermal + +This binding describes thermal sensors that is part of Stingray SoCs. + +Required properties: +- compatible : Must be "brcm,sr-thermal" +- reg : memory where tmon data will be available. +- brcm,tmon-mask: A one cell bit mask of valid TMON sources. + Each bit represents single TMON source. + +Example: + tmons { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x0 0x8f100000 0x100>; + + tmon: thermal@0 { + compatible = "brcm,sr-thermal"; + reg = <0 0x40>; + brcm,tmon-mask = <0x3f>; + }; + }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 1/3] dt-bindings: thermal: Add binding document for SR thermal 2018-08-09 12:54 ` [PATCH v3 1/3] dt-bindings: thermal: Add binding document for SR thermal Srinath Mannam @ 2018-08-13 17:34 ` Rob Herring 0 siblings, 0 replies; 9+ messages in thread From: Rob Herring @ 2018-08-13 17:34 UTC (permalink / raw) To: Srinath Mannam Cc: Zhang Rui, Eduardo Valentin, Mark Rutland, devicetree, linux-kernel, bcm-kernel-feedback-list, Pramod Kumar On Thu, Aug 09, 2018 at 06:24:57PM +0530, Srinath Mannam wrote: > From: Pramod Kumar <pramod.kumar@broadcom.com> > > Add binding document for supported thermal implementation > in Stingray. > > Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com> > Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com> > Reviewed-by: Ray Jui <ray.jui@broadcom.com> > Reviewed-by: Scott Branden <scott.branden@broadcom.com> > --- > .../bindings/thermal/brcm,sr-thermal.txt | 23 ++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt Reviewed-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 2/3] arm64: dts: stingray: Add Stingray Thermal DT support. 2018-08-09 12:54 [PATCH v3 0/3] Stingray thermal driver support Srinath Mannam 2018-08-09 12:54 ` [PATCH v3 1/3] dt-bindings: thermal: Add binding document for SR thermal Srinath Mannam @ 2018-08-09 12:54 ` Srinath Mannam 2018-08-09 12:54 ` [PATCH v3 3/3] thermal: broadcom: Add Stingray thermal driver Srinath Mannam 2018-09-13 22:25 ` [PATCH v3 0/3] Stingray thermal driver support Florian Fainelli 3 siblings, 0 replies; 9+ messages in thread From: Srinath Mannam @ 2018-08-09 12:54 UTC (permalink / raw) To: Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland Cc: devicetree, linux-kernel, bcm-kernel-feedback-list, Pramod Kumar, Srinath Mannam From: Pramod Kumar <pramod.kumar@broadcom.com> Add DT nodes for thermal zones memory base address to read temperature. Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com> Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com> Reviewed-by: Ray Jui <ray.jui@broadcom.com> Reviewed-by: Scott Branden <scott.branden@broadcom.com> --- arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi index b203152..eeccf94 100644 --- a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi +++ b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi @@ -593,4 +593,17 @@ status = "disabled"; }; }; + + tmons { + compatible = "simple-bus"; + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x0 0x8f100000 0x100>; + + tmon: thermal@0 { + compatible = "brcm,sr-thermal"; + reg = <0x0 0x40>; + brcm,tmon-mask = <0x3f>; + }; + }; }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 3/3] thermal: broadcom: Add Stingray thermal driver 2018-08-09 12:54 [PATCH v3 0/3] Stingray thermal driver support Srinath Mannam 2018-08-09 12:54 ` [PATCH v3 1/3] dt-bindings: thermal: Add binding document for SR thermal Srinath Mannam 2018-08-09 12:54 ` [PATCH v3 2/3] arm64: dts: stingray: Add Stingray Thermal DT support Srinath Mannam @ 2018-08-09 12:54 ` Srinath Mannam 2018-09-14 14:20 ` Daniel Lezcano 2018-09-13 22:25 ` [PATCH v3 0/3] Stingray thermal driver support Florian Fainelli 3 siblings, 1 reply; 9+ messages in thread From: Srinath Mannam @ 2018-08-09 12:54 UTC (permalink / raw) To: Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland Cc: devicetree, linux-kernel, bcm-kernel-feedback-list, Pramod Kumar, Srinath Mannam From: Pramod Kumar <pramod.kumar@broadcom.com> Adds stingray thermal driver to monitor six thermal zones temperature and trips at critical temperature. Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com> Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com> Reviewed-by: Ray Jui <ray.jui@broadcom.com> Reviewed-by: Scott Branden <scott.branden@broadcom.com> Reviewed-by: Vikram Prakash <vikram.prakash@broadcom.com> --- drivers/thermal/Kconfig | 3 +- drivers/thermal/broadcom/Kconfig | 9 ++ drivers/thermal/broadcom/Makefile | 1 + drivers/thermal/broadcom/sr-thermal.c | 216 ++++++++++++++++++++++++++++++++++ 4 files changed, 228 insertions(+), 1 deletion(-) create mode 100644 drivers/thermal/broadcom/sr-thermal.c diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig index 8297988..26d39d4 100644 --- a/drivers/thermal/Kconfig +++ b/drivers/thermal/Kconfig @@ -416,7 +416,8 @@ config MTK_THERMAL controller present in Mediatek SoCs menu "Broadcom thermal drivers" -depends on ARCH_BCM || ARCH_BRCMSTB || ARCH_BCM2835 || COMPILE_TEST +depends on ARCH_BCM || ARCH_BRCMSTB || ARCH_BCM2835 || ARCH_BCM_IPROC || \ + COMPILE_TEST source "drivers/thermal/broadcom/Kconfig" endmenu diff --git a/drivers/thermal/broadcom/Kconfig b/drivers/thermal/broadcom/Kconfig index c106a15..dc9a9bd 100644 --- a/drivers/thermal/broadcom/Kconfig +++ b/drivers/thermal/broadcom/Kconfig @@ -22,3 +22,12 @@ config BCM_NS_THERMAL BCM4708, BCM4709, BCM5301x, BCM95852X, etc). It contains DMU (Device Management Unit) block with a thermal sensor that allows checking CPU temperature. + +config BCM_SR_THERMAL + tristate "Stingray thermal driver" + depends on ARCH_BCM_IPROC || COMPILE_TEST + default ARCH_BCM_IPROC + help + Support for the Stingray family of SoCs. Its different blocks like + iHost, CRMU and NITRO has thermal sensor that allows checking its + temperature. diff --git a/drivers/thermal/broadcom/Makefile b/drivers/thermal/broadcom/Makefile index fae10ec..79df69e 100644 --- a/drivers/thermal/broadcom/Makefile +++ b/drivers/thermal/broadcom/Makefile @@ -1,3 +1,4 @@ obj-$(CONFIG_BCM2835_THERMAL) += bcm2835_thermal.o obj-$(CONFIG_BRCMSTB_THERMAL) += brcmstb_thermal.o obj-$(CONFIG_BCM_NS_THERMAL) += ns-thermal.o +obj-$(CONFIG_BCM_SR_THERMAL) += sr-thermal.o diff --git a/drivers/thermal/broadcom/sr-thermal.c b/drivers/thermal/broadcom/sr-thermal.c new file mode 100644 index 0000000..909f80c --- /dev/null +++ b/drivers/thermal/broadcom/sr-thermal.c @@ -0,0 +1,216 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2018 Broadcom + */ + +#include <linux/acpi.h> +#include <linux/module.h> +#include <linux/of_address.h> +#include <linux/platform_device.h> +#include <linux/thermal.h> + +#define TMON_CRIT_TEMP 105000 /* temp in millidegree C */ +#define SR_TMON_MAX_LIST 6 + +/* + * In stingray thermal IO memory, + * Total Number of available TMONs MASK is at offset 0 + * temperature registers BASE is at 4 byte offset. + * Each TMON temperature register size is 4. + */ +#define SR_TMON_TEMP_BASE(id) ((id) * 0x4) + +static const char * const sr_tmon_names[SR_TMON_MAX_LIST] = { + "sr_tmon_ihost0", + "sr_tmon_ihost1", + "sr_tmon_ihost2", + "sr_tmon_ihost3", + "sr_tmon_crmu", + "sr_tmon_nitro", +}; + +struct sr_tmon { + struct thermal_zone_device *tz; + unsigned int crit_temp; + unsigned int tmon_id; + struct sr_thermal *priv; +}; + +struct sr_thermal { + struct device *dev; + void __iomem *regs; + struct sr_tmon tmon[SR_TMON_MAX_LIST]; +}; + +static int sr_get_temp(struct thermal_zone_device *tz, int *temp) +{ + struct sr_tmon *tmon = tz->devdata; + struct sr_thermal *sr_thermal = tmon->priv; + + *temp = readl(sr_thermal->regs + SR_TMON_TEMP_BASE(tmon->tmon_id)); + + return 0; +} + +static int sr_get_trip_type(struct thermal_zone_device *tz, int trip, + enum thermal_trip_type *type) +{ + struct sr_tmon *tmon = tz->devdata; + struct sr_thermal *sr_thermal = tmon->priv; + + switch (trip) { + case 0: + *type = THERMAL_TRIP_CRITICAL; + break; + default: + dev_dbg(sr_thermal->dev, + "Driver does not support more than 1 trip point\n"); + return -EINVAL; + } + return 0; +} + +static int sr_get_trip_temp(struct thermal_zone_device *tz, int trip, int *temp) +{ + struct sr_tmon *tmon = tz->devdata; + struct sr_thermal *sr_thermal = tmon->priv; + + switch (trip) { + case 0: + *temp = tmon->crit_temp; + break; + default: + dev_dbg(sr_thermal->dev, + "Driver does not support more than 1 trip point\n"); + return -EINVAL; + } + return 0; +} + +static int sr_set_trip_temp(struct thermal_zone_device *tz, int trip, int temp) +{ + struct sr_tmon *tmon = tz->devdata; + + switch (trip) { + case 0: + /* + * Allow the user to change critical temperature + * as per their requirement, could be for debug + * purpose, even if it's more than the recommended + * critical temperature. + */ + tmon->crit_temp = temp; + break; + default: + return -EINVAL; + } + return 0; +} + +static struct thermal_zone_device_ops sr_thermal_ops = { + .get_temp = sr_get_temp, + .get_trip_type = sr_get_trip_type, + .get_trip_temp = sr_get_trip_temp, + .set_trip_temp = sr_set_trip_temp, +}; + +static int sr_thermal_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct sr_thermal *sr_thermal; + struct sr_tmon *tmon; + struct resource *res; + uint32_t sr_tmon_list = 0; + unsigned int i; + int ret; + + sr_thermal = devm_kzalloc(dev, sizeof(*sr_thermal), GFP_KERNEL); + if (!sr_thermal) + return -ENOMEM; + + sr_thermal->dev = dev; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + sr_thermal->regs = (void __iomem *)devm_memremap(&pdev->dev, res->start, + resource_size(res), MEMREMAP_WB); + if (IS_ERR(sr_thermal->regs)) { + dev_err(dev, "failed to get io address\n"); + return PTR_ERR(sr_thermal->regs); + } + + ret = device_property_read_u32(dev, "brcm,tmon-mask", &sr_tmon_list); + if (ret) + return ret; + + for (i = 0; i < SR_TMON_MAX_LIST; i++) { + + if (!(sr_tmon_list & BIT(i))) + continue; + + /* Flush temperature registers */ + writel(0, sr_thermal->regs + SR_TMON_TEMP_BASE(i)); + tmon = &sr_thermal->tmon[i]; + tmon->crit_temp = TMON_CRIT_TEMP; + tmon->tmon_id = i; + tmon->priv = sr_thermal; + tmon->tz = thermal_zone_device_register(sr_tmon_names[i], + 1, 1, + tmon, + &sr_thermal_ops, + NULL, 1000, 1000); + if (IS_ERR(tmon->tz)) + goto err_exit; + + dev_info(dev, "%s: registered\n", sr_tmon_names[i]); + } + platform_set_drvdata(pdev, sr_thermal); + + return 0; + +err_exit: + while (--i >= 0) { + if (sr_thermal->tmon[i].tz) + thermal_zone_device_unregister(sr_thermal->tmon[i].tz); + } + + return PTR_ERR(tmon->tz); +} + +static int sr_thermal_remove(struct platform_device *pdev) +{ + struct sr_thermal *sr_thermal = platform_get_drvdata(pdev); + unsigned int i; + + for (i = 0; i < SR_TMON_MAX_LIST; i++) + if (sr_thermal->tmon[i].tz) + thermal_zone_device_unregister(sr_thermal->tmon[i].tz); + + return 0; +} + +static const struct of_device_id sr_thermal_of_match[] = { + { .compatible = "brcm,sr-thermal", }, + {}, +}; +MODULE_DEVICE_TABLE(of, sr_thermal_of_match); + +static const struct acpi_device_id sr_thermal_acpi_ids[] = { + { .id = "BRCM0500" }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(acpi, sr_thermal_acpi_ids); + +static struct platform_driver sr_thermal_driver = { + .probe = sr_thermal_probe, + .remove = sr_thermal_remove, + .driver = { + .name = "sr-thermal", + .of_match_table = sr_thermal_of_match, + .acpi_match_table = ACPI_PTR(sr_thermal_acpi_ids), + }, +}; +module_platform_driver(sr_thermal_driver); + +MODULE_AUTHOR("Pramod Kumar <pramod.kumar@broadcom.com>"); +MODULE_DESCRIPTION("Stingray thermal driver"); +MODULE_LICENSE("GPL v2"); -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 3/3] thermal: broadcom: Add Stingray thermal driver 2018-08-09 12:54 ` [PATCH v3 3/3] thermal: broadcom: Add Stingray thermal driver Srinath Mannam @ 2018-09-14 14:20 ` Daniel Lezcano 2018-09-17 3:50 ` Srinath Mannam 2018-09-17 6:53 ` Pramod Kumar 0 siblings, 2 replies; 9+ messages in thread From: Daniel Lezcano @ 2018-09-14 14:20 UTC (permalink / raw) To: Srinath Mannam, Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland Cc: devicetree, linux-kernel, bcm-kernel-feedback-list, Pramod Kumar On 09/08/2018 14:54, Srinath Mannam wrote: > From: Pramod Kumar <pramod.kumar@broadcom.com> > > Adds stingray thermal driver to monitor six > thermal zones temperature and trips at critical temperature. Hi Pramod, could you elaborate a bit more the description? As you are introducing a new driver it would be nice to give the sensor details. > Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com> > Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com> > Reviewed-by: Ray Jui <ray.jui@broadcom.com> > Reviewed-by: Scott Branden <scott.branden@broadcom.com> > Reviewed-by: Vikram Prakash <vikram.prakash@broadcom.com> > --- > drivers/thermal/Kconfig | 3 +- > drivers/thermal/broadcom/Kconfig | 9 ++ > drivers/thermal/broadcom/Makefile | 1 + > drivers/thermal/broadcom/sr-thermal.c | 216 ++++++++++++++++++++++++++++++++++ > 4 files changed, 228 insertions(+), 1 deletion(-) > create mode 100644 drivers/thermal/broadcom/sr-thermal.c > > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig > index 8297988..26d39d4 100644 > --- a/drivers/thermal/Kconfig > +++ b/drivers/thermal/Kconfig > @@ -416,7 +416,8 @@ config MTK_THERMAL > controller present in Mediatek SoCs > > menu "Broadcom thermal drivers" > -depends on ARCH_BCM || ARCH_BRCMSTB || ARCH_BCM2835 || COMPILE_TEST > +depends on ARCH_BCM || ARCH_BRCMSTB || ARCH_BCM2835 || ARCH_BCM_IPROC || \ > + COMPILE_TEST > source "drivers/thermal/broadcom/Kconfig" > endmenu > > diff --git a/drivers/thermal/broadcom/Kconfig b/drivers/thermal/broadcom/Kconfig > index c106a15..dc9a9bd 100644 > --- a/drivers/thermal/broadcom/Kconfig > +++ b/drivers/thermal/broadcom/Kconfig > @@ -22,3 +22,12 @@ config BCM_NS_THERMAL > BCM4708, BCM4709, BCM5301x, BCM95852X, etc). It contains DMU (Device > Management Unit) block with a thermal sensor that allows checking CPU > temperature. > + > +config BCM_SR_THERMAL > + tristate "Stingray thermal driver" > + depends on ARCH_BCM_IPROC || COMPILE_TEST > + default ARCH_BCM_IPROC > + help > + Support for the Stingray family of SoCs. Its different blocks like > + iHost, CRMU and NITRO has thermal sensor that allows checking its > + temperature. > diff --git a/drivers/thermal/broadcom/Makefile b/drivers/thermal/broadcom/Makefile > index fae10ec..79df69e 100644 > --- a/drivers/thermal/broadcom/Makefile > +++ b/drivers/thermal/broadcom/Makefile > @@ -1,3 +1,4 @@ > obj-$(CONFIG_BCM2835_THERMAL) += bcm2835_thermal.o > obj-$(CONFIG_BRCMSTB_THERMAL) += brcmstb_thermal.o > obj-$(CONFIG_BCM_NS_THERMAL) += ns-thermal.o > +obj-$(CONFIG_BCM_SR_THERMAL) += sr-thermal.o > diff --git a/drivers/thermal/broadcom/sr-thermal.c b/drivers/thermal/broadcom/sr-thermal.c > new file mode 100644 > index 0000000..909f80c > --- /dev/null > +++ b/drivers/thermal/broadcom/sr-thermal.c > @@ -0,0 +1,216 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2018 Broadcom > + */ > + > +#include <linux/acpi.h> > +#include <linux/module.h> > +#include <linux/of_address.h> > +#include <linux/platform_device.h> > +#include <linux/thermal.h> > + > +#define TMON_CRIT_TEMP 105000 /* temp in millidegree C */ I suggest to move this in the DT? > +#define SR_TMON_MAX_LIST 6 > + > +/* > + * In stingray thermal IO memory, > + * Total Number of available TMONs MASK is at offset 0 > + * temperature registers BASE is at 4 byte offset. > + * Each TMON temperature register size is 4. > + */ > +#define SR_TMON_TEMP_BASE(id) ((id) * 0x4) > + > +static const char * const sr_tmon_names[SR_TMON_MAX_LIST] = { It will be more elegant to replace the macro SR_TMON_MAX_LIST by ARRAY_SIZE(sr_tmon_names) and declare this array as: static const char *const sr_tmon_name[] = { ... }; > + "sr_tmon_ihost0", > + "sr_tmon_ihost1", > + "sr_tmon_ihost2", > + "sr_tmon_ihost3", > + "sr_tmon_crmu", > + "sr_tmon_nitro", > +}; > + > +struct sr_tmon { > + struct thermal_zone_device *tz; > + unsigned int crit_temp; > + unsigned int tmon_id; > + struct sr_thermal *priv; > +}; > + > +struct sr_thermal { > + struct device *dev; This field is used for dev_dbg, may be it could be removed along with the dev_dbg message? > + void __iomem *regs; > + struct sr_tmon tmon[SR_TMON_MAX_LIST]; > +}; > + > +static int sr_get_temp(struct thermal_zone_device *tz, int *temp) > +{ > + struct sr_tmon *tmon = tz->devdata; > + struct sr_thermal *sr_thermal = tmon->priv; > + > + *temp = readl(sr_thermal->regs + SR_TMON_TEMP_BASE(tmon->tmon_id)); > + > + return 0; > +} > + > +static int sr_get_trip_type(struct thermal_zone_device *tz, int trip, > + enum thermal_trip_type *type) > +{ > + struct sr_tmon *tmon = tz->devdata; > + struct sr_thermal *sr_thermal = tmon->priv; > + > + switch (trip) { > + case 0: > + *type = THERMAL_TRIP_CRITICAL; > + break; > + default: > + dev_dbg(sr_thermal->dev, > + "Driver does not support more than 1 trip point\n"); > + return -EINVAL; > + } > + return 0; > +} > + > +static int sr_get_trip_temp(struct thermal_zone_device *tz, int trip, int *temp) > +{ > + struct sr_tmon *tmon = tz->devdata; > + struct sr_thermal *sr_thermal = tmon->priv; > + > + switch (trip) { > + case 0: > + *temp = tmon->crit_temp; > + break; > + default: > + dev_dbg(sr_thermal->dev, > + "Driver does not support more than 1 trip point\n"); > + return -EINVAL; > + } > + return 0; > +} > + > +static int sr_set_trip_temp(struct thermal_zone_device *tz, int trip, int temp) > +{ > + struct sr_tmon *tmon = tz->devdata; > + > + switch (trip) { > + case 0: > + /* > + * Allow the user to change critical temperature > + * as per their requirement, could be for debug > + * purpose, even if it's more than the recommended > + * critical temperature. > + */ Couldn't the user harm the hardware with a too high value? Why not define this value in the DT? > + tmon->crit_temp = temp; > + break; > + default: > + return -EINVAL; > + } > + return 0; > +} Is it possible to factor out these 3 functions above and remove the 'switch' in all of them ? > +static struct thermal_zone_device_ops sr_thermal_ops = { > + .get_temp = sr_get_temp, > + .get_trip_type = sr_get_trip_type, > + .get_trip_temp = sr_get_trip_temp, > + .set_trip_temp = sr_set_trip_temp, > +}; > + > +static int sr_thermal_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct sr_thermal *sr_thermal; > + struct sr_tmon *tmon; > + struct resource *res; > + uint32_t sr_tmon_list = 0; > + unsigned int i; > + int ret; > + > + sr_thermal = devm_kzalloc(dev, sizeof(*sr_thermal), GFP_KERNEL); > + if (!sr_thermal) > + return -ENOMEM; > + > + sr_thermal->dev = dev; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + sr_thermal->regs = (void __iomem *)devm_memremap(&pdev->dev, res->start, > + resource_size(res), MEMREMAP_WB); > + if (IS_ERR(sr_thermal->regs)) { > + dev_err(dev, "failed to get io address\n"); > + return PTR_ERR(sr_thermal->regs); > + } > + > + ret = device_property_read_u32(dev, "brcm,tmon-mask", &sr_tmon_list); > + if (ret) > + return ret; > + > + for (i = 0; i < SR_TMON_MAX_LIST; i++) { > + > + if (!(sr_tmon_list & BIT(i))) > + continue; > + > + /* Flush temperature registers */ > + writel(0, sr_thermal->regs + SR_TMON_TEMP_BASE(i)); > + tmon = &sr_thermal->tmon[i]; It is possible to initialize tmon to: tmon = &sr_thermal->tmon; and then use the pointer increment: tmon++; > + tmon->crit_temp = TMON_CRIT_TEMP; > + tmon->tmon_id = i; > + tmon->priv = sr_thermal; > + tmon->tz = thermal_zone_device_register(sr_tmon_names[i], > + 1, 1, > + tmon, > + &sr_thermal_ops, > + NULL, 1000, 1000); > + if (IS_ERR(tmon->tz)) > + goto err_exit; > + > + dev_info(dev, "%s: registered\n", sr_tmon_names[i]); > + } > + platform_set_drvdata(pdev, sr_thermal); > + > + return 0; > + > +err_exit: > + while (--i >= 0) { > + if (sr_thermal->tmon[i].tz) > + thermal_zone_device_unregister(sr_thermal->tmon[i].tz); > + } > + > + return PTR_ERR(tmon->tz); > +} > + > +static int sr_thermal_remove(struct platform_device *pdev) > +{ > + struct sr_thermal *sr_thermal = platform_get_drvdata(pdev); > + unsigned int i; > + > + for (i = 0; i < SR_TMON_MAX_LIST; i++) > + if (sr_thermal->tmon[i].tz) > + thermal_zone_device_unregister(sr_thermal->tmon[i].tz); > + > + return 0; > +} > + > +static const struct of_device_id sr_thermal_of_match[] = { > + { .compatible = "brcm,sr-thermal", }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, sr_thermal_of_match); > + > +static const struct acpi_device_id sr_thermal_acpi_ids[] = { > + { .id = "BRCM0500" }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(acpi, sr_thermal_acpi_ids); > + > +static struct platform_driver sr_thermal_driver = { > + .probe = sr_thermal_probe, > + .remove = sr_thermal_remove, > + .driver = { > + .name = "sr-thermal", > + .of_match_table = sr_thermal_of_match, > + .acpi_match_table = ACPI_PTR(sr_thermal_acpi_ids), > + }, > +}; > +module_platform_driver(sr_thermal_driver); > + > +MODULE_AUTHOR("Pramod Kumar <pramod.kumar@broadcom.com>"); > +MODULE_DESCRIPTION("Stingray thermal driver"); > +MODULE_LICENSE("GPL v2"); > -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/3] thermal: broadcom: Add Stingray thermal driver 2018-09-14 14:20 ` Daniel Lezcano @ 2018-09-17 3:50 ` Srinath Mannam 2018-09-17 6:53 ` Pramod Kumar 1 sibling, 0 replies; 9+ messages in thread From: Srinath Mannam @ 2018-09-17 3:50 UTC (permalink / raw) To: Daniel Lezcano Cc: Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland, devicetree, Linux Kernel Mailing List, BCM Kernel Feedback, Pramod Kumar, Vikram Prakash Hi Daniel, Thank you for your review comments. I will address all your comments in next patch set. Regards, Srinath. On Fri, Sep 14, 2018 at 7:50 PM, Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > On 09/08/2018 14:54, Srinath Mannam wrote: >> From: Pramod Kumar <pramod.kumar@broadcom.com> >> >> Adds stingray thermal driver to monitor six >> thermal zones temperature and trips at critical temperature. > > Hi Pramod, > > could you elaborate a bit more the description? As you are introducing a > new driver it would be nice to give the sensor details. > >> Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com> >> Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com> >> Reviewed-by: Ray Jui <ray.jui@broadcom.com> >> Reviewed-by: Scott Branden <scott.branden@broadcom.com> >> Reviewed-by: Vikram Prakash <vikram.prakash@broadcom.com> >> --- >> drivers/thermal/Kconfig | 3 +- >> drivers/thermal/broadcom/Kconfig | 9 ++ >> drivers/thermal/broadcom/Makefile | 1 + >> drivers/thermal/broadcom/sr-thermal.c | 216 ++++++++++++++++++++++++++++++++++ >> 4 files changed, 228 insertions(+), 1 deletion(-) >> create mode 100644 drivers/thermal/broadcom/sr-thermal.c >> >> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig >> index 8297988..26d39d4 100644 >> --- a/drivers/thermal/Kconfig >> +++ b/drivers/thermal/Kconfig >> @@ -416,7 +416,8 @@ config MTK_THERMAL >> controller present in Mediatek SoCs >> >> menu "Broadcom thermal drivers" >> -depends on ARCH_BCM || ARCH_BRCMSTB || ARCH_BCM2835 || COMPILE_TEST >> +depends on ARCH_BCM || ARCH_BRCMSTB || ARCH_BCM2835 || ARCH_BCM_IPROC || \ >> + COMPILE_TEST >> source "drivers/thermal/broadcom/Kconfig" >> endmenu >> >> diff --git a/drivers/thermal/broadcom/Kconfig b/drivers/thermal/broadcom/Kconfig >> index c106a15..dc9a9bd 100644 >> --- a/drivers/thermal/broadcom/Kconfig >> +++ b/drivers/thermal/broadcom/Kconfig >> @@ -22,3 +22,12 @@ config BCM_NS_THERMAL >> BCM4708, BCM4709, BCM5301x, BCM95852X, etc). It contains DMU (Device >> Management Unit) block with a thermal sensor that allows checking CPU >> temperature. >> + >> +config BCM_SR_THERMAL >> + tristate "Stingray thermal driver" >> + depends on ARCH_BCM_IPROC || COMPILE_TEST >> + default ARCH_BCM_IPROC >> + help >> + Support for the Stingray family of SoCs. Its different blocks like >> + iHost, CRMU and NITRO has thermal sensor that allows checking its >> + temperature. >> diff --git a/drivers/thermal/broadcom/Makefile b/drivers/thermal/broadcom/Makefile >> index fae10ec..79df69e 100644 >> --- a/drivers/thermal/broadcom/Makefile >> +++ b/drivers/thermal/broadcom/Makefile >> @@ -1,3 +1,4 @@ >> obj-$(CONFIG_BCM2835_THERMAL) += bcm2835_thermal.o >> obj-$(CONFIG_BRCMSTB_THERMAL) += brcmstb_thermal.o >> obj-$(CONFIG_BCM_NS_THERMAL) += ns-thermal.o >> +obj-$(CONFIG_BCM_SR_THERMAL) += sr-thermal.o >> diff --git a/drivers/thermal/broadcom/sr-thermal.c b/drivers/thermal/broadcom/sr-thermal.c >> new file mode 100644 >> index 0000000..909f80c >> --- /dev/null >> +++ b/drivers/thermal/broadcom/sr-thermal.c >> @@ -0,0 +1,216 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (C) 2018 Broadcom >> + */ >> + >> +#include <linux/acpi.h> >> +#include <linux/module.h> >> +#include <linux/of_address.h> >> +#include <linux/platform_device.h> >> +#include <linux/thermal.h> >> + >> +#define TMON_CRIT_TEMP 105000 /* temp in millidegree C */ > > I suggest to move this in the DT? > >> +#define SR_TMON_MAX_LIST 6 >> + >> +/* >> + * In stingray thermal IO memory, >> + * Total Number of available TMONs MASK is at offset 0 >> + * temperature registers BASE is at 4 byte offset. >> + * Each TMON temperature register size is 4. >> + */ >> +#define SR_TMON_TEMP_BASE(id) ((id) * 0x4) >> + >> +static const char * const sr_tmon_names[SR_TMON_MAX_LIST] = { > > It will be more elegant to replace the macro SR_TMON_MAX_LIST by > ARRAY_SIZE(sr_tmon_names) and declare this array as: > > static const char *const sr_tmon_name[] = { > ... > }; > >> + "sr_tmon_ihost0", >> + "sr_tmon_ihost1", >> + "sr_tmon_ihost2", >> + "sr_tmon_ihost3", >> + "sr_tmon_crmu", >> + "sr_tmon_nitro", >> +}; >> + >> +struct sr_tmon { >> + struct thermal_zone_device *tz; >> + unsigned int crit_temp; >> + unsigned int tmon_id; >> + struct sr_thermal *priv; >> +}; >> + >> +struct sr_thermal { >> + struct device *dev; > > This field is used for dev_dbg, may be it could be removed along with > the dev_dbg message? > >> + void __iomem *regs; >> + struct sr_tmon tmon[SR_TMON_MAX_LIST]; >> +}; >> + >> +static int sr_get_temp(struct thermal_zone_device *tz, int *temp) >> +{ >> + struct sr_tmon *tmon = tz->devdata; >> + struct sr_thermal *sr_thermal = tmon->priv; >> + >> + *temp = readl(sr_thermal->regs + SR_TMON_TEMP_BASE(tmon->tmon_id)); >> + >> + return 0; >> +} >> + >> +static int sr_get_trip_type(struct thermal_zone_device *tz, int trip, >> + enum thermal_trip_type *type) >> +{ >> + struct sr_tmon *tmon = tz->devdata; >> + struct sr_thermal *sr_thermal = tmon->priv; >> + >> + switch (trip) { >> + case 0: >> + *type = THERMAL_TRIP_CRITICAL; >> + break; >> + default: >> + dev_dbg(sr_thermal->dev, >> + "Driver does not support more than 1 trip point\n"); >> + return -EINVAL; >> + } >> + return 0; >> +} >> + >> +static int sr_get_trip_temp(struct thermal_zone_device *tz, int trip, int *temp) >> +{ >> + struct sr_tmon *tmon = tz->devdata; >> + struct sr_thermal *sr_thermal = tmon->priv; >> + >> + switch (trip) { >> + case 0: >> + *temp = tmon->crit_temp; >> + break; >> + default: >> + dev_dbg(sr_thermal->dev, >> + "Driver does not support more than 1 trip point\n"); >> + return -EINVAL; >> + } >> + return 0; >> +} >> + >> +static int sr_set_trip_temp(struct thermal_zone_device *tz, int trip, int temp) >> +{ >> + struct sr_tmon *tmon = tz->devdata; >> + >> + switch (trip) { >> + case 0: >> + /* >> + * Allow the user to change critical temperature >> + * as per their requirement, could be for debug >> + * purpose, even if it's more than the recommended >> + * critical temperature. >> + */ > > Couldn't the user harm the hardware with a too high value? > > Why not define this value in the DT? > >> + tmon->crit_temp = temp; >> + break; >> + default: >> + return -EINVAL; >> + } >> + return 0; >> +} > > Is it possible to factor out these 3 functions above and remove the > 'switch' in all of them ? > >> +static struct thermal_zone_device_ops sr_thermal_ops = { >> + .get_temp = sr_get_temp, >> + .get_trip_type = sr_get_trip_type, >> + .get_trip_temp = sr_get_trip_temp, >> + .set_trip_temp = sr_set_trip_temp, >> +}; >> + >> +static int sr_thermal_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct sr_thermal *sr_thermal; >> + struct sr_tmon *tmon; >> + struct resource *res; >> + uint32_t sr_tmon_list = 0; >> + unsigned int i; >> + int ret; >> + >> + sr_thermal = devm_kzalloc(dev, sizeof(*sr_thermal), GFP_KERNEL); >> + if (!sr_thermal) >> + return -ENOMEM; >> + >> + sr_thermal->dev = dev; >> + >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + sr_thermal->regs = (void __iomem *)devm_memremap(&pdev->dev, res->start, >> + resource_size(res), MEMREMAP_WB); >> + if (IS_ERR(sr_thermal->regs)) { >> + dev_err(dev, "failed to get io address\n"); >> + return PTR_ERR(sr_thermal->regs); >> + } >> + >> + ret = device_property_read_u32(dev, "brcm,tmon-mask", &sr_tmon_list); >> + if (ret) >> + return ret; >> + >> + for (i = 0; i < SR_TMON_MAX_LIST; i++) { >> + >> + if (!(sr_tmon_list & BIT(i))) >> + continue; >> + >> + /* Flush temperature registers */ >> + writel(0, sr_thermal->regs + SR_TMON_TEMP_BASE(i)); >> + tmon = &sr_thermal->tmon[i]; > > It is possible to initialize tmon to: > > tmon = &sr_thermal->tmon; > > and then use the pointer increment: > tmon++; > > >> + tmon->crit_temp = TMON_CRIT_TEMP; >> + tmon->tmon_id = i; >> + tmon->priv = sr_thermal; >> + tmon->tz = thermal_zone_device_register(sr_tmon_names[i], >> + 1, 1, >> + tmon, >> + &sr_thermal_ops, >> + NULL, 1000, 1000); >> + if (IS_ERR(tmon->tz)) >> + goto err_exit; >> + >> + dev_info(dev, "%s: registered\n", sr_tmon_names[i]); >> + } >> + platform_set_drvdata(pdev, sr_thermal); >> + >> + return 0; >> + >> +err_exit: >> + while (--i >= 0) { >> + if (sr_thermal->tmon[i].tz) >> + thermal_zone_device_unregister(sr_thermal->tmon[i].tz); >> + } >> + >> + return PTR_ERR(tmon->tz); >> +} >> + >> +static int sr_thermal_remove(struct platform_device *pdev) >> +{ >> + struct sr_thermal *sr_thermal = platform_get_drvdata(pdev); >> + unsigned int i; >> + >> + for (i = 0; i < SR_TMON_MAX_LIST; i++) >> + if (sr_thermal->tmon[i].tz) >> + thermal_zone_device_unregister(sr_thermal->tmon[i].tz); >> + >> + return 0; >> +} >> + >> +static const struct of_device_id sr_thermal_of_match[] = { >> + { .compatible = "brcm,sr-thermal", }, >> + {}, >> +}; >> +MODULE_DEVICE_TABLE(of, sr_thermal_of_match); >> + >> +static const struct acpi_device_id sr_thermal_acpi_ids[] = { >> + { .id = "BRCM0500" }, >> + { /* sentinel */ } >> +}; >> +MODULE_DEVICE_TABLE(acpi, sr_thermal_acpi_ids); >> + >> +static struct platform_driver sr_thermal_driver = { >> + .probe = sr_thermal_probe, >> + .remove = sr_thermal_remove, >> + .driver = { >> + .name = "sr-thermal", >> + .of_match_table = sr_thermal_of_match, >> + .acpi_match_table = ACPI_PTR(sr_thermal_acpi_ids), >> + }, >> +}; >> +module_platform_driver(sr_thermal_driver); >> + >> +MODULE_AUTHOR("Pramod Kumar <pramod.kumar@broadcom.com>"); >> +MODULE_DESCRIPTION("Stingray thermal driver"); >> +MODULE_LICENSE("GPL v2"); >> > > > -- > <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs > > Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | > <http://twitter.com/#!/linaroorg> Twitter | > <http://www.linaro.org/linaro-blog/> Blog > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 3/3] thermal: broadcom: Add Stingray thermal driver 2018-09-14 14:20 ` Daniel Lezcano 2018-09-17 3:50 ` Srinath Mannam @ 2018-09-17 6:53 ` Pramod Kumar 1 sibling, 0 replies; 9+ messages in thread From: Pramod Kumar @ 2018-09-17 6:53 UTC (permalink / raw) To: daniel.lezcano Cc: Srinath (Srinath Babu) Mannam, rui.zhang, edubezval, Rob Herring, Mark Rutland, devicetree, linux-kernel, BCM Kernel Feedback [-- Attachment #1: Type: text/plain, Size: 12118 bytes --] On Fri, Sep 14, 2018 at 7:51 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > On 09/08/2018 14:54, Srinath Mannam wrote: > > From: Pramod Kumar <pramod.kumar@broadcom.com> > > > > Adds stingray thermal driver to monitor six > > thermal zones temperature and trips at critical temperature. > > Hi Pramod, > > could you elaborate a bit more the description? As you are introducing a > new driver it would be nice to give the sensor details. > Hi Denial, Our SoC has six temperature sensor which gets configured/initialized and controlled by m0 processor, running in secure zone. This processor sample the sensors values and and populates in DDR memory to be read by other agents like Linux. Since Sensors are not in direct control of Linux hence sensor details has not been added here. Apart from this, In case Linux fails to take action because of some unexpected reason, m0 Processor will takes action to save the SoCs from being burned out. Regards, Pramod > > > Signed-off-by: Pramod Kumar <pramod.kumar@broadcom.com> > > Signed-off-by: Srinath Mannam <srinath.mannam@broadcom.com> > > Reviewed-by: Ray Jui <ray.jui@broadcom.com> > > Reviewed-by: Scott Branden <scott.branden@broadcom.com> > > Reviewed-by: Vikram Prakash <vikram.prakash@broadcom.com> > > --- > > drivers/thermal/Kconfig | 3 +- > > drivers/thermal/broadcom/Kconfig | 9 ++ > > drivers/thermal/broadcom/Makefile | 1 + > > drivers/thermal/broadcom/sr-thermal.c | 216 > ++++++++++++++++++++++++++++++++++ > > 4 files changed, 228 insertions(+), 1 deletion(-) > > create mode 100644 drivers/thermal/broadcom/sr-thermal.c > > > > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig > > index 8297988..26d39d4 100644 > > --- a/drivers/thermal/Kconfig > > +++ b/drivers/thermal/Kconfig > > @@ -416,7 +416,8 @@ config MTK_THERMAL > > controller present in Mediatek SoCs > > > > menu "Broadcom thermal drivers" > > -depends on ARCH_BCM || ARCH_BRCMSTB || ARCH_BCM2835 || COMPILE_TEST > > +depends on ARCH_BCM || ARCH_BRCMSTB || ARCH_BCM2835 || ARCH_BCM_IPROC > || \ > > + COMPILE_TEST > > source "drivers/thermal/broadcom/Kconfig" > > endmenu > > > > diff --git a/drivers/thermal/broadcom/Kconfig > b/drivers/thermal/broadcom/Kconfig > > index c106a15..dc9a9bd 100644 > > --- a/drivers/thermal/broadcom/Kconfig > > +++ b/drivers/thermal/broadcom/Kconfig > > @@ -22,3 +22,12 @@ config BCM_NS_THERMAL > > BCM4708, BCM4709, BCM5301x, BCM95852X, etc). It contains DMU > (Device > > Management Unit) block with a thermal sensor that allows > checking CPU > > temperature. > > + > > +config BCM_SR_THERMAL > > + tristate "Stingray thermal driver" > > + depends on ARCH_BCM_IPROC || COMPILE_TEST > > + default ARCH_BCM_IPROC > > + help > > + Support for the Stingray family of SoCs. Its different blocks > like > > + iHost, CRMU and NITRO has thermal sensor that allows checking its > > + temperature. > > diff --git a/drivers/thermal/broadcom/Makefile > b/drivers/thermal/broadcom/Makefile > > index fae10ec..79df69e 100644 > > --- a/drivers/thermal/broadcom/Makefile > > +++ b/drivers/thermal/broadcom/Makefile > > @@ -1,3 +1,4 @@ > > obj-$(CONFIG_BCM2835_THERMAL) += bcm2835_thermal.o > > obj-$(CONFIG_BRCMSTB_THERMAL) += brcmstb_thermal.o > > obj-$(CONFIG_BCM_NS_THERMAL) += ns-thermal.o > > +obj-$(CONFIG_BCM_SR_THERMAL) += sr-thermal.o > > diff --git a/drivers/thermal/broadcom/sr-thermal.c > b/drivers/thermal/broadcom/sr-thermal.c > > new file mode 100644 > > index 0000000..909f80c > > --- /dev/null > > +++ b/drivers/thermal/broadcom/sr-thermal.c > > @@ -0,0 +1,216 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (C) 2018 Broadcom > > + */ > > + > > +#include <linux/acpi.h> > > +#include <linux/module.h> > > +#include <linux/of_address.h> > > +#include <linux/platform_device.h> > > +#include <linux/thermal.h> > > + > > +#define TMON_CRIT_TEMP 105000 /* temp in millidegree C */ > > I suggest to move this in the DT? > > > +#define SR_TMON_MAX_LIST 6 > > + > > +/* > > + * In stingray thermal IO memory, > > + * Total Number of available TMONs MASK is at offset 0 > > + * temperature registers BASE is at 4 byte offset. > > + * Each TMON temperature register size is 4. > > + */ > > +#define SR_TMON_TEMP_BASE(id) ((id) * 0x4) > > + > > +static const char * const sr_tmon_names[SR_TMON_MAX_LIST] = { > > It will be more elegant to replace the macro SR_TMON_MAX_LIST by > ARRAY_SIZE(sr_tmon_names) and declare this array as: > > static const char *const sr_tmon_name[] = { > ... > }; > > > + "sr_tmon_ihost0", > > + "sr_tmon_ihost1", > > + "sr_tmon_ihost2", > > + "sr_tmon_ihost3", > > + "sr_tmon_crmu", > > + "sr_tmon_nitro", > > +}; > > + > > +struct sr_tmon { > > + struct thermal_zone_device *tz; > > + unsigned int crit_temp; > > + unsigned int tmon_id; > > + struct sr_thermal *priv; > > +}; > > + > > +struct sr_thermal { > > + struct device *dev; > > This field is used for dev_dbg, may be it could be removed along with > the dev_dbg message? > > > + void __iomem *regs; > > + struct sr_tmon tmon[SR_TMON_MAX_LIST]; > > +}; > > + > > +static int sr_get_temp(struct thermal_zone_device *tz, int *temp) > > +{ > > + struct sr_tmon *tmon = tz->devdata; > > + struct sr_thermal *sr_thermal = tmon->priv; > > + > > + *temp = readl(sr_thermal->regs + SR_TMON_TEMP_BASE(tmon->tmon_id)); > > + > > + return 0; > > +} > > + > > +static int sr_get_trip_type(struct thermal_zone_device *tz, int trip, > > + enum thermal_trip_type *type) > > +{ > > + struct sr_tmon *tmon = tz->devdata; > > + struct sr_thermal *sr_thermal = tmon->priv; > > + > > + switch (trip) { > > + case 0: > > + *type = THERMAL_TRIP_CRITICAL; > > + break; > > + default: > > + dev_dbg(sr_thermal->dev, > > + "Driver does not support more than 1 trip > point\n"); > > + return -EINVAL; > > + } > > + return 0; > > +} > > + > > +static int sr_get_trip_temp(struct thermal_zone_device *tz, int trip, > int *temp) > > +{ > > + struct sr_tmon *tmon = tz->devdata; > > + struct sr_thermal *sr_thermal = tmon->priv; > > + > > + switch (trip) { > > + case 0: > > + *temp = tmon->crit_temp; > > + break; > > + default: > > + dev_dbg(sr_thermal->dev, > > + "Driver does not support more than 1 trip > point\n"); > > + return -EINVAL; > > + } > > + return 0; > > +} > > + > > +static int sr_set_trip_temp(struct thermal_zone_device *tz, int trip, > int temp) > > +{ > > + struct sr_tmon *tmon = tz->devdata; > > + > > + switch (trip) { > > + case 0: > > + /* > > + * Allow the user to change critical temperature > > + * as per their requirement, could be for debug > > + * purpose, even if it's more than the recommended > > + * critical temperature. > > + */ > > Couldn't the user harm the hardware with a too high value? > > Why not define this value in the DT? > > > + tmon->crit_temp = temp; > > + break; > > + default: > > + return -EINVAL; > > + } > > + return 0; > > +} > > Is it possible to factor out these 3 functions above and remove the > 'switch' in all of them ? > > > +static struct thermal_zone_device_ops sr_thermal_ops = { > > + .get_temp = sr_get_temp, > > + .get_trip_type = sr_get_trip_type, > > + .get_trip_temp = sr_get_trip_temp, > > + .set_trip_temp = sr_set_trip_temp, > > +}; > > + > > +static int sr_thermal_probe(struct platform_device *pdev) > > +{ > > + struct device *dev = &pdev->dev; > > + struct sr_thermal *sr_thermal; > > + struct sr_tmon *tmon; > > + struct resource *res; > > + uint32_t sr_tmon_list = 0; > > + unsigned int i; > > + int ret; > > + > > + sr_thermal = devm_kzalloc(dev, sizeof(*sr_thermal), GFP_KERNEL); > > + if (!sr_thermal) > > + return -ENOMEM; > > + > > + sr_thermal->dev = dev; > > + > > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > + sr_thermal->regs = (void __iomem *)devm_memremap(&pdev->dev, > res->start, > > + resource_size(res), MEMREMAP_WB); > > + if (IS_ERR(sr_thermal->regs)) { > > + dev_err(dev, "failed to get io address\n"); > > + return PTR_ERR(sr_thermal->regs); > > + } > > + > > + ret = device_property_read_u32(dev, "brcm,tmon-mask", > &sr_tmon_list); > > + if (ret) > > + return ret; > > + > > + for (i = 0; i < SR_TMON_MAX_LIST; i++) { > > + > > + if (!(sr_tmon_list & BIT(i))) > > + continue; > > + > > + /* Flush temperature registers */ > > + writel(0, sr_thermal->regs + SR_TMON_TEMP_BASE(i)); > > + tmon = &sr_thermal->tmon[i]; > > It is possible to initialize tmon to: > > tmon = &sr_thermal->tmon; > > and then use the pointer increment: > tmon++; > > > > + tmon->crit_temp = TMON_CRIT_TEMP; > > + tmon->tmon_id = i; > > + tmon->priv = sr_thermal; > > + tmon->tz = thermal_zone_device_register(sr_tmon_names[i], > > + 1, 1, > > + tmon, > > + &sr_thermal_ops, > > + NULL, 1000, 1000); > > + if (IS_ERR(tmon->tz)) > > + goto err_exit; > > + > > + dev_info(dev, "%s: registered\n", sr_tmon_names[i]); > > + } > > + platform_set_drvdata(pdev, sr_thermal); > > + > > + return 0; > > + > > +err_exit: > > + while (--i >= 0) { > > + if (sr_thermal->tmon[i].tz) > > + > thermal_zone_device_unregister(sr_thermal->tmon[i].tz); > > + } > > + > > + return PTR_ERR(tmon->tz); > > +} > > + > > +static int sr_thermal_remove(struct platform_device *pdev) > > +{ > > + struct sr_thermal *sr_thermal = platform_get_drvdata(pdev); > > + unsigned int i; > > + > > + for (i = 0; i < SR_TMON_MAX_LIST; i++) > > + if (sr_thermal->tmon[i].tz) > > + > thermal_zone_device_unregister(sr_thermal->tmon[i].tz); > > + > > + return 0; > > +} > > + > > +static const struct of_device_id sr_thermal_of_match[] = { > > + { .compatible = "brcm,sr-thermal", }, > > + {}, > > +}; > > +MODULE_DEVICE_TABLE(of, sr_thermal_of_match); > > + > > +static const struct acpi_device_id sr_thermal_acpi_ids[] = { > > + { .id = "BRCM0500" }, > > + { /* sentinel */ } > > +}; > > +MODULE_DEVICE_TABLE(acpi, sr_thermal_acpi_ids); > > + > > +static struct platform_driver sr_thermal_driver = { > > + .probe = sr_thermal_probe, > > + .remove = sr_thermal_remove, > > + .driver = { > > + .name = "sr-thermal", > > + .of_match_table = sr_thermal_of_match, > > + .acpi_match_table = ACPI_PTR(sr_thermal_acpi_ids), > > + }, > > +}; > > +module_platform_driver(sr_thermal_driver); > > + > > +MODULE_AUTHOR("Pramod Kumar <pramod.kumar@broadcom.com>"); > > +MODULE_DESCRIPTION("Stingray thermal driver"); > > +MODULE_LICENSE("GPL v2"); > > > > > -- > <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs > > Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | > <http://twitter.com/#!/linaroorg> Twitter | > <http://www.linaro.org/linaro-blog/> Blog > > [-- Attachment #2: Type: text/html, Size: 16076 bytes --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 0/3] Stingray thermal driver support 2018-08-09 12:54 [PATCH v3 0/3] Stingray thermal driver support Srinath Mannam ` (2 preceding siblings ...) 2018-08-09 12:54 ` [PATCH v3 3/3] thermal: broadcom: Add Stingray thermal driver Srinath Mannam @ 2018-09-13 22:25 ` Florian Fainelli 3 siblings, 0 replies; 9+ messages in thread From: Florian Fainelli @ 2018-09-13 22:25 UTC (permalink / raw) To: Srinath Mannam, Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland Cc: devicetree, linux-kernel, bcm-kernel-feedback-list On 08/09/18 05:54, Srinath Mannam wrote: > These patches adds the stingray thermal driver and its > corresponding DT nodes with documentation. > > Changes from v2: > - All stingray TMON DT nodes are combine together into single. > Temperature registers are combined into one mem resource. > brcm,tmon-mask parameter has available TMONs mask value. > - All available TMONs are initialized together in single > instance of driver probe call. > > Changes from v1: > - Fixed auto build sparce warning. Thermal maintainers, can you please review patch #3? The binding and DTS changes can go through arm-soc if you would like, or you can take patches 1 and 3 and I take patch 2, either way is fine. Thank you > > Pramod Kumar (3): > dt-bindings: thermal: Add binding document for SR thermal > arm64: dts: stingray: Add Stingray Thermal DT support. > thermal: broadcom: Add Stingray thermal driver > > .../bindings/thermal/brcm,sr-thermal.txt | 23 +++ > .../arm64/boot/dts/broadcom/stingray/stingray.dtsi | 13 ++ > drivers/thermal/Kconfig | 3 +- > drivers/thermal/broadcom/Kconfig | 9 + > drivers/thermal/broadcom/Makefile | 1 + > drivers/thermal/broadcom/sr-thermal.c | 216 +++++++++++++++++++++ > 6 files changed, 264 insertions(+), 1 deletion(-) > create mode 100644 Documentation/devicetree/bindings/thermal/brcm,sr-thermal.txt > create mode 100644 drivers/thermal/broadcom/sr-thermal.c > -- Florian ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-09-17 6:53 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-08-09 12:54 [PATCH v3 0/3] Stingray thermal driver support Srinath Mannam 2018-08-09 12:54 ` [PATCH v3 1/3] dt-bindings: thermal: Add binding document for SR thermal Srinath Mannam 2018-08-13 17:34 ` Rob Herring 2018-08-09 12:54 ` [PATCH v3 2/3] arm64: dts: stingray: Add Stingray Thermal DT support Srinath Mannam 2018-08-09 12:54 ` [PATCH v3 3/3] thermal: broadcom: Add Stingray thermal driver Srinath Mannam 2018-09-14 14:20 ` Daniel Lezcano 2018-09-17 3:50 ` Srinath Mannam 2018-09-17 6:53 ` Pramod Kumar 2018-09-13 22:25 ` [PATCH v3 0/3] Stingray thermal driver support Florian Fainelli
This is 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).