Add thermal control driver for Sunplus SP7021 SoC
Subsystems (2)
v4 cover Add thermal control driver for Sunplus SP7021 SoC
This is a patch series for thermal driver for Sunplus SP7021 SoC.
Sunplus SP7021 is an ARM Cortex A7 (4 cores) based SoC. It integrates many peripherals (ex: UART, I2C, SPI, SDIO, eMMC, USB, SD card and etc.) into a single chip. It is designed for industrial control.
Refer to:
https://sunplus-tibbo.atlassian.net/wiki/spaces/doc/overview
https://tibbo.com/store/plus1.html
Li-hao Kuo (2):
thermal: Add thermal driver for Sunplus SP7021
dt-bindings:thermal: Add Sunplus SP7021 schema
.../bindings/thermal/sunplus_thermal.yaml | 49 +++++++ MAINTAINERS | 7 + drivers/thermal/Kconfig | 10 ++ drivers/thermal/Makefile | 1 + drivers/thermal/sunplus_thermal.c | 157 +++++++++++++++++++++ 5 files changed, 224 insertions(+) create mode 100644 Documentation/devicetree/bindings/thermal/sunplus_thermal.yaml create mode 100644 drivers/thermal/sunplus_thermal.c
Signature · 1 line
2.7.4
PATCH v4 1/2 thermal: Add thermal driver for Sunplus SP7021
Add thermal driver for Sunplus SP7021.
---
Changes in v4:
- Modify yaml file remove reg name and change nvmem name
MAINTAINERS | 6 ++ drivers/thermal/Kconfig | 10 +++ drivers/thermal/Makefile | 1 + drivers/thermal/sunplus_thermal.c | 157 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 174 insertions(+) create mode 100644 drivers/thermal/sunplus_thermal.c
MAINTAINERS +6 -0
@@ -18423,6 +18423,12 @@ L: [email protected] S: Maintained F: drivers/net/ethernet/dlink/sundance.c +SUNPLUS THERMAL DRIVER +M: Li-hao Kuo <[email protected]> +L: [email protected] +S: Maintained +F: drivers/thermal/sunplus_thermal.c + SUPERH M: Yoshinori Sato <[email protected]> M: Rich Felker <[email protected]>
drivers/thermal/Kconfig +10 -0
@@ -502,4 +502,14 @@ config KHADAS_MCU_FAN_THERMAL If you say yes here you get support for the FAN controlled by the Microcontroller found on the Khadas VIM boards. +config SUNPLUS_THERMAL + tristate "Sunplus thermal drivers" + depends on SOC_SP7021 + help + This the Sunplus SP7021 thermal driver, which supports the primitive + temperature sensor embedded in Sunplus SP7021 SoC. + + If you have a Sunplus SP7021 platform say Y here and enable this option + to have support for thermal management + endif
drivers/thermal/Makefile +1 -0
@@ -61,3 +61,4 @@ obj-$(CONFIG_UNIPHIER_THERMAL) += uniphier_thermal.o obj-$(CONFIG_AMLOGIC_THERMAL) += amlogic_thermal.o obj-$(CONFIG_SPRD_THERMAL) += sprd_thermal.o obj-$(CONFIG_KHADAS_MCU_FAN_THERMAL) += khadas_mcu_fan.o +obj-$(CONFIG_SUNPLUS_THERMAL) += sunplus_thermal.o
drivers/thermal/sunplus_thermal.c new file +157 -0
157 lines not on this page · raw message
Signature · 1 line
2.7.4
Re: [PATCH v4 1/2] thermal: Add thermal driver for Sunplus SP7021
Quoted 1 line · Li-hao Kuo
Add thermal driver for Sunplus SP7021.
Please give a detailed description of the sensor so we can refer to the changelog to understand the code below.
Quoted 73 lines
Signed-off-by Li-hao Kuo <[email protected]>---
Changes in v4:
- Modify yaml file remove reg name and change nvmem nameMAINTAINERS | 6 ++ drivers/thermal/Kconfig | 10 +++ drivers/thermal/Makefile | 1 + drivers/thermal/sunplus_thermal.c | 157 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 174 insertions(+)create mode 100644
drivers/thermal/sunplus_thermal.cMAINTAINERS +6
-0@@ -18423,6 +18423,12 @@ L: [email protected] S: Maintained F: drivers/net/ethernet/dlink/sundance.c +SUNPLUS THERMAL DRIVER +M: Li-hao Kuo <[email protected]> +L: [email protected] +S: Maintained +F: drivers/thermal/sunplus_thermal.c + SUPERH M: Yoshinori Sato <[email protected]> M: Rich Felker <[email protected]>drivers/thermal/Kconfig +10
-0@@ -502,4 +502,14 @@ config KHADAS_MCU_FAN_THERMAL If you say yes here you get support for the FAN controlled by the Microcontroller found on the Khadas VIM boards. +config SUNPLUS_THERMAL + tristate "Sunplus thermal drivers" + depends on SOC_SP7021 + help + This the Sunplus SP7021 thermal driver, which supports the primitive + temperature sensor embedded in Sunplus SP7021 SoC. + + If you have a Sunplus SP7021 platform say Y here and enable this option + to have support for thermal management + endifdrivers/thermal/Makefile +1
-0@@ -61,3 +61,4 @@ obj-$(CONFIG_UNIPHIER_THERMAL) += uniphier_thermal.o obj-$(CONFIG_AMLOGIC_THERMAL) += amlogic_thermal.o obj-$(CONFIG_SPRD_THERMAL) += sprd_thermal.o obj-$(CONFIG_KHADAS_MCU_FAN_THERMAL) += khadas_mcu_fan.o +obj-$(CONFIG_SUNPLUS_THERMAL) += sunplus_thermal.odrivers/thermal/sunplus_thermal.c new file +9
-0@@ -0,0 +1,157 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* +* Copyright (c) Sunplus Inc. +* Author: Li-hao Kuo <[email protected]> +*/ + +#include <linux/bitfield.h> +#include <linux/clk.h> +#include <linux/cpufreq.h>
drivers/thermal/sunplus_thermal.c line 19
Quoted 10 lines
drivers/thermal/sunplus_thermal.c new file +10
-0+#include <linux/delay.h> +#include <linux/err.h> +#include <linux/interrupt.h> +#include <linux/io.h> +#include <linux/module.h> +#include <linux/nvmem-consumer.h> +#include <linux/platform_device.h> +#include <linux/reset.h> +#include <linux/rtc.h> +#include <linux/thermal.h>
Please check the headers above. Some are not necessary (eg cpufreq, delay, rtc ...) AFAICT
drivers/thermal/sunplus_thermal.c line 23
Quoted 3 lines
drivers/thermal/sunplus_thermal.c new file +3
-0+#define DISABLE_THREMAL (BIT(31) | BIT(15)) +#define ENABLE_THREMAL BIT(31) +#define SP_THREMAL_MASK GENMASK(10, 0)
THREMAL ?
s/THREMAL/THERMAL/ ?
Quoted 19 lines
drivers/thermal/sunplus_thermal.c new file +19
-0+#define TEMP_RATE 608 +#define TEMP_BASE 3500 +#define TEMP_OTP_BASE 1518 + +#define SP_THERMAL_CTL0_REG 0x0000 +#define SP_THERMAL_STS0_REG 0x0030 + +/* common data structures */ +struct sp_thermal_data { +struct thermal_zone_device *pcb_tz; +struct platform_device *pdev; +enum thermal_device_mode mode; +long sensor_temp; +void __iomem *regs; +int otp_temp0; +int otp_temp1; +u32 id; +}; +
drivers/thermal/sunplus_thermal.c line 53
Quoted 10 lines
drivers/thermal/sunplus_thermal.c new file +10
-0+static char *sp7021_otp_coef_read (struct device *dev,ssize_t *len) +{ +char *ret =NULL ; +struct nvmem_cell *c =nvmem_cell_get (dev,"calib" ); + +if (IS_ERR_OR_NULL (c)) +return NULL ; + + ret =nvmem_cell_read (c, len); +nvmem_cell_put (c);
That is wrong. Please refer to the documentation driver-api/nvmem.rst
nvmem_cell_put() must be called when the pointer returned by nvmem_cell_read() is no longer used.
Quoted 4 lines
drivers/thermal/sunplus_thermal.c new file +4
-0+return ret; +} + +static void sp7021_get_otp_temp_coef (struct sp_thermal_data *sp_data,struct device *_d)
drivers/thermal/sunplus_thermal.c line 67
Quoted 10 lines
drivers/thermal/sunplus_thermal.c new file +10
-0+{ +ssize_t otp_l =0 ; +char *otp_v; + + otp_v =sp7021_otp_coef_read (_d, &otp_l); +if (otp_l <3 ) +return ; +if (IS_ERR_OR_NULL (otp_v)) +return ; + sp_data->otp_temp0 = otp_v[0 ] | (otp_v[1 ] <<8 );
What do you expect by 8bits shifting a char here ?
drivers/thermal/sunplus_thermal.c line 66
Quoted 1 line
drivers/thermal/sunplus_thermal.c new file +1
-0+ sp_data->otp_temp0 = otp_v[0 ] | (otp_v[1 ] <<8 );
duplicate line
drivers/thermal/sunplus_thermal.c line 73
Quoted 5 lines
drivers/thermal/sunplus_thermal.c new file +5
-0+ sp_data->otp_temp0 =FIELD_GET (SP_THREMAL_MASK , sp_data->otp_temp0); + sp_data->otp_temp1 = (otp_v[1 ] >>3 ) | (otp_v[2 ] <<5 ); + sp_data->otp_temp1 =FIELD_GET (SP_THREMAL_MASK , sp_data->otp_temp1); +if (sp_data->otp_temp0 ==0 ) + sp_data->otp_temp0 =TEMP_OTP_BASE ;
Can you add a comment explaining how is stored the coef in the nvcell, so we can understand the above actions ?
drivers/thermal/sunplus_thermal.c line 84
Quoted 11 lines
drivers/thermal/sunplus_thermal.c new file +11
-0+} + +static int sp_thermal_get_sensor_temp (void *data,int *temp) +{ +struct sp_thermal_data *sp_data = data; +int t_code; + + t_code =readl (sp_data->regs +SP_THERMAL_STS0_REG ); + t_code =FIELD_GET (SP_THREMAL_MASK , t_code); + *temp = ((sp_data->otp_temp0 - t_code) *10000 /TEMP_RATE ) +TEMP_BASE ; + *temp *=10 ;
A comment would be welcome here also
Quoted 10 lines
drivers/thermal/sunplus_thermal.c new file +10
-0+return 0 ; +} + +static struct thermal_zone_of_device_ops sp_of_thermal_ops = { + .get_temp = sp_thermal_get_sensor_temp, +}; + +static int sp_thermal_register_sensor (struct platform_device *pdev,struct sp_thermal_data *data, +int index) +{
drivers/thermal/sunplus_thermal.c line 104
Quoted 10 lines
drivers/thermal/sunplus_thermal.c new file +10
-0+int ret; + + data->id = index; + data->pcb_tz =devm_thermal_zone_of_sensor_register (&pdev->dev, + data->id, data, &sp_of_thermal_ops); +if (!IS_ERR_OR_NULL (data->pcb_tz)) +return 0 ; + ret =PTR_ERR (data->pcb_tz); + data->pcb_tz =NULL ; +return ret;
Usually the logic is inverted
data->pcb_tz = devm_thermal_zone_of_sensor_register(); if (IS_ERR_OR_NULL(data->pcb_tz)) return PTR_ERR(data->pcb_tz); return 0;
No need to set pcb_tz to NULL
Quoted 5 lines
drivers/thermal/sunplus_thermal.c new file +5
-0+} + +static int sp7021_thermal_probe (struct platform_device *pdev) +{ +struct sp_thermal_data *sp_data;
drivers/thermal/sunplus_thermal.c line 119
Quoted 10 lines
drivers/thermal/sunplus_thermal.c new file +10
-0+struct resource *res; +int ret; + + sp_data =devm_kzalloc (&(pdev->dev),sizeof (*sp_data),GFP_KERNEL ); +if (!sp_data) +return -ENOMEM ; + + res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); +if (IS_ERR (res)) +return dev_err_probe (&(pdev->dev),PTR_ERR (res),"resource get fail\n" );
Use an intermediate 'dev' variable to shorten the line
drivers/thermal/sunplus_thermal.c line 123
Quoted 3 lines
drivers/thermal/sunplus_thermal.c new file +3
-0+ sp_data->regs =devm_ioremap (&(pdev->dev), res->start,resource_size (res)); +if (IS_ERR (sp_data->regs)) +return dev_err_probe (&(pdev->dev),PTR_ERR (sp_data->regs),"mas_base get fail\n" );
typo: mas_base -> ioremap
drivers/thermal/sunplus_thermal.c line 129
Quoted 5 lines
drivers/thermal/sunplus_thermal.c new file +5
-0+writel (ENABLE_THREMAL , sp_data->regs +SP_THERMAL_CTL0_REG ); + +platform_set_drvdata (pdev, sp_data); +sp7021_get_otp_temp_coef (sp_data, &pdev->dev); + ret =sp_thermal_register_sensor (pdev, sp_data,0 );
return sp_thermal_register_sensor(...);
Quoted 29 lines
drivers/thermal/sunplus_thermal.c new file +28
-0+ +return ret; +} + +static int sp7021_thermal_remove (struct platform_device *_pd) +{ +return 0 ; +} + +static const struct of_device_id of_sp7021_thermal_ids[] = { + { .compatible ="sunplus,sp7021-thermal" }, + {/* sentinel */ } +}; +MODULE_DEVICE_TABLE (of, of_sp7021_thermal_ids); + +static struct platform_driver sp7021_thermal_driver = { + .probe = sp7021_thermal_probe, + .remove = sp7021_thermal_remove, + .driver = { + .name ="sp7021-thermal" , + .of_match_table =of_match_ptr (of_sp7021_thermal_ids), + }, +}; +module_platform_driver (sp7021_thermal_driver); + +MODULE_AUTHOR ("Li-hao Kuo <[email protected]>" ); +MODULE_DESCRIPTION ("Thermal driver for SP7021 SoC" ); +MODULE_LICENSE ("GPL" );
Signature · 5 lines
<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
RE: [PATCH v4 1/2] thermal: Add thermal driver for Sunplus SP7021
Hi Mr Daniel Lezcano:
Thank you for your review
Quoted 10 lines
Please check the headers above. Some are not necessary (eg cpufreq, delay, rtc ...) AFAICT
drivers/thermal/sunplus_thermal.c line 23
Quoted 3 lines
drivers/thermal/sunplus_thermal.c new file +3
-0+#define DISABLE_THREMAL (BIT(31) | BIT(15)) +#define ENABLE_THREMAL BIT(31) +#define SP_THREMAL_MASK GENMASK(10, 0)THREMAL ?
s/THREMAL/THERMAL/ ?
Sorry, I'll correct it in next commit.
Quoted 13 lines
drivers/thermal/sunplus_thermal.c line 53
Quoted 10 lines
drivers/thermal/sunplus_thermal.c new file +10
-0+static char *sp7021_otp_coef_read (struct device *dev,ssize_t *len) +{ +char *ret =NULL ; +struct nvmem_cell *c =nvmem_cell_get (dev,"calib" ); + +if (IS_ERR_OR_NULL (c)) +return NULL ; + + ret =nvmem_cell_read (c, len); +nvmem_cell_put (c);That is wrong. Please refer to the documentation
driver-api/nvmem.rst
Quoted 3 lines
nvmem_cell_put()must be called when the pointer returned bynvmem_cell_read()is no longer used.
Sorry for that. I don't know which part is wrong. nvmem_cell_put() has been called after nvmem_cell_read() I am coding according to the example.
Quoted 12 lines
duplicate line
drivers/thermal/sunplus_thermal.c line 73
Quoted 5 lines
drivers/thermal/sunplus_thermal.c new file +5
-0+ sp_data->otp_temp0 =FIELD_GET (SP_THREMAL_MASK , sp_data->otp_temp0); + sp_data->otp_temp1 = (otp_v[1 ] >>3 ) | (otp_v[2 ] <<5 ); + sp_data->otp_temp1 =FIELD_GET (SP_THREMAL_MASK , sp_data->otp_temp1); +if (sp_data->otp_temp0 ==0 ) + sp_data->otp_temp0 =TEMP_OTP_BASE ;Can you add a comment explaining how is stored the coef in the nvcell, so we can understand the above actions ?
Is the comment below ok?
/*
*When remanufacturing, the 35 degree T_CODE will be read and stored in nvcell. *TEMP_RATE is the SP7021 temperature slope.
*T_CODE : 11 digits in total
*/
PATCH v4 2/2 dt-bindings:thermal: Add Sunplus SP7021 schema
In reply to Li-hao KuoAdd bindings for Sunplus SP7021 thermal driver
---
Changes in v4:
- Modify yaml file remove reg name and change nvmem name
.../bindings/thermal/sunplus_thermal.yaml | 49 ++++++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/thermal/sunplus_thermal.yaml
Documentation/devicetree/bindings/thermal/sunplus_thermal.yaml new file +43 -0
@@ -0,0 +1,49 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +# Copyright (C) Sunplus Co., Ltd. +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/thermal/sunplus_thermal.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Sunplus Thermal controller + +maintainers: + - Li-hao Kuo <[email protected]> + +properties: + compatible: + enum: + - sunplus,sp7021-thermal + + reg: + maxItems: 1 + + nvmem-cells: + maxItems: 1 + + nvmem-cell-names: + const: calib + +required: + - compatible + - reg + - nvmem-cells + - nvmem-cell-names + +additionalProperties: false + +examples: + - | + thermal@9c000280 { + compatible = "sunplus,sp7021-thermal"; + reg = <0x9c000280 0x80>; + nvmem-cells = <&therm_calib>; + nvmem-cell-names = "calib"; + }; +...
MAINTAINERS +1 -0
@@ -18427,6 +18427,7 @@ SUNPLUS THERMAL DRIVER M: Li-hao Kuo <[email protected]> L: [email protected] S: Maintained +F: Documentation/devicetree/bindings/thermal/sunplus_thermal.yaml F: drivers/thermal/sunplus_thermal.c SUPERH
Signature · 1 line
2.7.4
Re: [PATCH v4 2/2] dt-bindings:thermal: Add Sunplus SP7021 schema
Quoted 12 lines · Li-hao Kuo
Add bindings for Sunplus SP7021 thermal driver
Signed-off-by Li-hao Kuo <[email protected]>---
Changes in v4:
- Modify yaml file remove reg name and change nvmem name.../bindings/thermal/sunplus_thermal.yaml | 49 ++++++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 50 insertions(+) create mode 100644 Documentation/devicetree/bindings/thermal/sunplus_thermal.yaml