* [U-Boot] [PATCH 0/3] thermal: Add multiple instance support
@ 2019-03-11 6:12 Keerthy
2019-03-11 6:12 ` [U-Boot] [PATCH 1/3] " Keerthy
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Keerthy @ 2019-03-11 6:12 UTC (permalink / raw)
To: u-boot
This series adds support for multiple sensor tempeatures
read & also a cmd option to read out temperatures
from u-boot prompt.
Keerthy (3):
thermal: Add multiple instance support
cmd: thermal: Add command line interface to read out temperatures
configs: dra7xx_evm_defconfig: Enable TI_DRA7_THERMAL & CMD_THERMAL
arch/arm/mach-imx/cpu.c | 2 +-
cmd/Kconfig | 5 +++
cmd/Makefile | 1 +
cmd/thermal.c | 59 ++++++++++++++++++++++++++++++++
configs/dra7xx_evm_defconfig | 3 ++
drivers/mmc/omap_hsmmc.c | 2 +-
drivers/thermal/imx_thermal.c | 2 +-
drivers/thermal/thermal-uclass.c | 4 +--
drivers/thermal/ti-bandgap.c | 13 +++++--
include/thermal.h | 6 ++--
10 files changed, 87 insertions(+), 10 deletions(-)
create mode 100644 cmd/thermal.c
--
2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 1/3] thermal: Add multiple instance support
2019-03-11 6:12 [U-Boot] [PATCH 0/3] thermal: Add multiple instance support Keerthy
@ 2019-03-11 6:12 ` Keerthy
2019-03-19 1:23 ` Simon Glass
2019-03-11 6:12 ` [U-Boot] [PATCH 2/3] cmd: thermal: Add command line interface to read out temperatures Keerthy
2019-03-11 6:12 ` [U-Boot] [PATCH 3/3] configs: dra7xx_evm_defconfig: Enable TI_DRA7_THERMAL & CMD_THERMAL Keerthy
2 siblings, 1 reply; 8+ messages in thread
From: Keerthy @ 2019-03-11 6:12 UTC (permalink / raw)
To: u-boot
Currently single instance temperature read out is supported.
Enhance the same to support multiple instances.
Signed-off-by: Keerthy <j-keerthy@ti.com>
---
arch/arm/mach-imx/cpu.c | 2 +-
drivers/mmc/omap_hsmmc.c | 2 +-
drivers/thermal/imx_thermal.c | 2 +-
drivers/thermal/thermal-uclass.c | 4 ++--
drivers/thermal/ti-bandgap.c | 13 ++++++++++---
include/thermal.h | 6 ++++--
6 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/arch/arm/mach-imx/cpu.c b/arch/arm/mach-imx/cpu.c
index 6b83f92662..27d12b410e 100644
--- a/arch/arm/mach-imx/cpu.c
+++ b/arch/arm/mach-imx/cpu.c
@@ -231,7 +231,7 @@ int print_cpuinfo(void)
printf("(%dC to %dC)", minc, maxc);
ret = uclass_get_device(UCLASS_THERMAL, 0, &thermal_dev);
if (!ret) {
- ret = thermal_get_temp(thermal_dev, &cpu_tmp);
+ ret = thermal_get_temp(thermal_dev, 0, &cpu_tmp);
if (!ret)
printf(" at %dC\n", cpu_tmp);
diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
index 826a39fad7..1872273dd9 100644
--- a/drivers/mmc/omap_hsmmc.c
+++ b/drivers/mmc/omap_hsmmc.c
@@ -642,7 +642,7 @@ static int omap_hsmmc_execute_tuning(struct udevice *dev, uint opcode)
printf("Couldn't get thermal device for tuning\n");
return ret;
}
- ret = thermal_get_temp(thermal_dev, &temperature);
+ ret = thermal_get_temp(thermal_dev, 0, &temperature);
if (ret) {
printf("Couldn't get temperature for tuning\n");
return ret;
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index e50b85bd59..271a61356c 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -207,7 +207,7 @@ static int read_cpu_temperature(struct udevice *dev)
}
#endif
-int imx_thermal_get_temp(struct udevice *dev, int *temp)
+int imx_thermal_get_temp(struct udevice *dev, int instance, int *temp)
{
struct thermal_data *priv = dev_get_priv(dev);
int cpu_tmp = 0;
diff --git a/drivers/thermal/thermal-uclass.c b/drivers/thermal/thermal-uclass.c
index a4ea1e2914..b4a31280cb 100644
--- a/drivers/thermal/thermal-uclass.c
+++ b/drivers/thermal/thermal-uclass.c
@@ -13,14 +13,14 @@
#include <linux/list.h>
-int thermal_get_temp(struct udevice *dev, int *temp)
+int thermal_get_temp(struct udevice *dev, u8 instance, int *temp)
{
const struct dm_thermal_ops *ops = device_get_ops(dev);
if (!ops->get_temp)
return -ENOSYS;
- return ops->get_temp(dev, temp);
+ return ops->get_temp(dev, instance, temp);
}
UCLASS_DRIVER(thermal) = {
diff --git a/drivers/thermal/ti-bandgap.c b/drivers/thermal/ti-bandgap.c
index b490391e96..6d3606b0fc 100644
--- a/drivers/thermal/ti-bandgap.c
+++ b/drivers/thermal/ti-bandgap.c
@@ -27,6 +27,7 @@
struct ti_bandgap {
ulong base;
int temperature; /* in mili degree celsius */
+ int sens_cnt;
};
/*
@@ -158,11 +159,16 @@ static int dra752_adc_to_temp[] = {
123800, 124200, 124600, 124900, 125000, 125000,
};
-static int ti_bandgap_get_temp(struct udevice *dev, int *temp)
+static int ti_bandgap_get_temp(struct udevice *dev, u8 instance, int *temp)
{
struct ti_bandgap *bgp = dev_get_priv(dev);
- bgp->temperature = 0x3ff & readl(bgp->base + CTRL_CORE_TEMP_SENSOR_MPU);
+ if (instance >= bgp->sens_cnt) {
+ printf("Only %d insatnces are supported\n", bgp->sens_cnt);
+ return -EINVAL;
+ }
+
+ bgp->temperature = 0x3ff & readl(bgp->base + instance * 4);
*temp = dra752_adc_to_temp[bgp->temperature - DRA752_ADC_START_VALUE];
return 0;
@@ -177,13 +183,14 @@ static int ti_bandgap_probe(struct udevice *dev)
struct ti_bandgap *bgp = dev_get_priv(dev);
bgp->base = devfdt_get_addr_index(dev, 1);
+ bgp->sens_cnt = dev_get_driver_data(dev);
return 0;
}
static const struct udevice_id of_ti_bandgap_match[] = {
{
- .compatible = "ti,dra752-bandgap",
+ .compatible = "ti,dra752-bandgap", .data = 3,
},
{},
};
diff --git a/include/thermal.h b/include/thermal.h
index 11d75256e0..82d80f3ded 100644
--- a/include/thermal.h
+++ b/include/thermal.h
@@ -9,7 +9,7 @@
#include <dm.h>
-int thermal_get_temp(struct udevice *dev, int *temp);
+int thermal_get_temp(struct udevice *dev, u8 instance, int *temp);
/**
* struct dm_thermal_ops - Driver model Thermal operations
@@ -25,9 +25,11 @@ struct dm_thermal_ops {
* It will enable and initialize any Thermal hardware as necessary.
*
* @dev: The Thermal device
+ * @instance: The instance of sensor in case multiple sensors are
+ * present if single sensor then this should be 0
* @temp: pointer that returns the measured temperature
*/
- int (*get_temp)(struct udevice *dev, int *temp);
+ int (*get_temp)(struct udevice *dev, u8 instance, int *temp);
};
#endif /* _THERMAL_H_ */
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 2/3] cmd: thermal: Add command line interface to read out temperatures
2019-03-11 6:12 [U-Boot] [PATCH 0/3] thermal: Add multiple instance support Keerthy
2019-03-11 6:12 ` [U-Boot] [PATCH 1/3] " Keerthy
@ 2019-03-11 6:12 ` Keerthy
2019-03-22 7:53 ` Simon Glass
2019-03-11 6:12 ` [U-Boot] [PATCH 3/3] configs: dra7xx_evm_defconfig: Enable TI_DRA7_THERMAL & CMD_THERMAL Keerthy
2 siblings, 1 reply; 8+ messages in thread
From: Keerthy @ 2019-03-11 6:12 UTC (permalink / raw)
To: u-boot
Add command line interface to read out temperatures from SoC.
Takes two arguments. One is 'temperature' and the other is
instance number. In case instance number is not provided by
default 0th instance temperature is read out.
Signed-off-by: Keerthy <j-keerthy@ti.com>
---
cmd/Kconfig | 5 +++++
cmd/Makefile | 1 +
cmd/thermal.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 65 insertions(+)
create mode 100644 cmd/thermal.c
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 4bcc5c4557..d252e93d64 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1044,6 +1044,11 @@ config CMD_SPI
help
SPI utility command.
+config CMD_THERMAL
+ bool "thermal"
+ help
+ THERMAL support.
+
config CMD_TSI148
bool "tsi148 - Command to access tsi148 device"
help
diff --git a/cmd/Makefile b/cmd/Makefile
index acb85f49fb..2b66f9e36a 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -128,6 +128,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o
obj-$(CONFIG_CMD_STRINGS) += strings.o
obj-$(CONFIG_CMD_SMC) += smccc.o
obj-$(CONFIG_CMD_TERMINAL) += terminal.o
+obj-$(CONFIG_CMD_THERMAL) += thermal.o
obj-$(CONFIG_CMD_TIME) += time.o
obj-$(CONFIG_CMD_TRACE) += trace.o
obj-$(CONFIG_HUSH_PARSER) += test.o
diff --git a/cmd/thermal.c b/cmd/thermal.c
new file mode 100644
index 0000000000..a31f992425
--- /dev/null
+++ b/cmd/thermal.c
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Thermal CMD
+ *
+ * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
+ */
+#include <common.h>
+#include <errno.h>
+#include <dm.h>
+#include <dm/uclass-internal.h>
+#include <thermal.h>
+
+#define MAX_TEMP 150
+
+static int do_read_temp(cmd_tbl_t *cmdtp, int flag, int argc,
+ char * const argv[])
+{
+ struct udevice *tdev;
+ int cpu_temp, ret = 0;
+ u8 instance;
+ const char *units;
+
+ if (argc > 2) {
+ printf("Max Number of arguments is 2\n");
+ return -EINVAL;
+ }
+
+ /* In case instance is not given default 0th instnace is reported */
+ if (argc == 2)
+ instance = (u8)simple_strtoul(argv[1], NULL, 10);
+ else
+ instance = 0;
+
+ ret = uclass_get_device(UCLASS_THERMAL, 0, &tdev);
+ if (!ret) {
+ ret = thermal_get_temp(tdev, instance, &cpu_temp);
+ if (!ret) {
+ if (abs(cpu_temp) < MAX_TEMP)
+ units = "C";
+ else
+ units = "mC";
+
+ printf("Instance %d Temperature@%d%s\n", instance,
+ cpu_temp, units);
+ } else {
+ debug(" - invalid sensor data\n");
+ }
+ } else {
+ printf(" - invalid sensor device\n");
+ }
+
+ return ret;
+}
+
+U_BOOT_CMD(
+ temperature, 2, 0, do_read_temp,
+ "Reads temperature of a given sensor",
+ " [sensor number] - number of the temperature sensor"
+);
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 3/3] configs: dra7xx_evm_defconfig: Enable TI_DRA7_THERMAL & CMD_THERMAL
2019-03-11 6:12 [U-Boot] [PATCH 0/3] thermal: Add multiple instance support Keerthy
2019-03-11 6:12 ` [U-Boot] [PATCH 1/3] " Keerthy
2019-03-11 6:12 ` [U-Boot] [PATCH 2/3] cmd: thermal: Add command line interface to read out temperatures Keerthy
@ 2019-03-11 6:12 ` Keerthy
2 siblings, 0 replies; 8+ messages in thread
From: Keerthy @ 2019-03-11 6:12 UTC (permalink / raw)
To: u-boot
Enable TI_DRA7_THERMAL & CMD_THERMAL config options to
read out temperatures of different instances in u-boot.
Signed-off-by: Keerthy <j-keerthy@ti.com>
---
configs/dra7xx_evm_defconfig | 3 +++
1 file changed, 3 insertions(+)
diff --git a/configs/dra7xx_evm_defconfig b/configs/dra7xx_evm_defconfig
index ef061501ef..036a164058 100644
--- a/configs/dra7xx_evm_defconfig
+++ b/configs/dra7xx_evm_defconfig
@@ -101,3 +101,6 @@ CONFIG_USB_GADGET=y
CONFIG_USB_GADGET_MANUFACTURER="Texas Instruments"
CONFIG_USB_GADGET_VENDOR_NUM=0x0451
CONFIG_USB_GADGET_PRODUCT_NUM=0xd022
+CONFIG_DM_THERMAL=y
+CONFIG_TI_DRA7_THERMAL=y
+CONFIG_CMD_THERMAL=y
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 1/3] thermal: Add multiple instance support
2019-03-11 6:12 ` [U-Boot] [PATCH 1/3] " Keerthy
@ 2019-03-19 1:23 ` Simon Glass
2019-03-19 14:23 ` keerthy
0 siblings, 1 reply; 8+ messages in thread
From: Simon Glass @ 2019-03-19 1:23 UTC (permalink / raw)
To: u-boot
Hi Keerthy,
On Mon, 11 Mar 2019 at 14:13, Keerthy <j-keerthy@ti.com> wrote:
>
> Currently single instance temperature read out is supported.
> Enhance the same to support multiple instances.
>
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
> arch/arm/mach-imx/cpu.c | 2 +-
> drivers/mmc/omap_hsmmc.c | 2 +-
> drivers/thermal/imx_thermal.c | 2 +-
> drivers/thermal/thermal-uclass.c | 4 ++--
> drivers/thermal/ti-bandgap.c | 13 ++++++++++---
> include/thermal.h | 6 ++++--
> 6 files changed, 19 insertions(+), 10 deletions(-)
Please can you add a simple test for the thermal uclass in test/dm?
>
> diff --git a/arch/arm/mach-imx/cpu.c b/arch/arm/mach-imx/cpu.c
> index 6b83f92662..27d12b410e 100644
> --- a/arch/arm/mach-imx/cpu.c
> +++ b/arch/arm/mach-imx/cpu.c
> @@ -231,7 +231,7 @@ int print_cpuinfo(void)
> printf("(%dC to %dC)", minc, maxc);
> ret = uclass_get_device(UCLASS_THERMAL, 0, &thermal_dev);
> if (!ret) {
> - ret = thermal_get_temp(thermal_dev, &cpu_tmp);
> + ret = thermal_get_temp(thermal_dev, 0, &cpu_tmp);
>
> if (!ret)
> printf(" at %dC\n", cpu_tmp);
> diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
> index 826a39fad7..1872273dd9 100644
> --- a/drivers/mmc/omap_hsmmc.c
> +++ b/drivers/mmc/omap_hsmmc.c
> @@ -642,7 +642,7 @@ static int omap_hsmmc_execute_tuning(struct udevice *dev, uint opcode)
> printf("Couldn't get thermal device for tuning\n");
> return ret;
> }
> - ret = thermal_get_temp(thermal_dev, &temperature);
> + ret = thermal_get_temp(thermal_dev, 0, &temperature);
> if (ret) {
> printf("Couldn't get temperature for tuning\n");
> return ret;
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index e50b85bd59..271a61356c 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -207,7 +207,7 @@ static int read_cpu_temperature(struct udevice *dev)
> }
> #endif
>
> -int imx_thermal_get_temp(struct udevice *dev, int *temp)
> +int imx_thermal_get_temp(struct udevice *dev, int instance, int *temp)
> {
> struct thermal_data *priv = dev_get_priv(dev);
> int cpu_tmp = 0;
> diff --git a/drivers/thermal/thermal-uclass.c b/drivers/thermal/thermal-uclass.c
> index a4ea1e2914..b4a31280cb 100644
> --- a/drivers/thermal/thermal-uclass.c
> +++ b/drivers/thermal/thermal-uclass.c
> @@ -13,14 +13,14 @@
> #include <linux/list.h>
>
>
> -int thermal_get_temp(struct udevice *dev, int *temp)
> +int thermal_get_temp(struct udevice *dev, u8 instance, int *temp)
> {
> const struct dm_thermal_ops *ops = device_get_ops(dev);
>
> if (!ops->get_temp)
> return -ENOSYS;
>
> - return ops->get_temp(dev, temp);
> + return ops->get_temp(dev, instance, temp);
> }
>
> UCLASS_DRIVER(thermal) = {
> diff --git a/drivers/thermal/ti-bandgap.c b/drivers/thermal/ti-bandgap.c
> index b490391e96..6d3606b0fc 100644
> --- a/drivers/thermal/ti-bandgap.c
> +++ b/drivers/thermal/ti-bandgap.c
> @@ -27,6 +27,7 @@
> struct ti_bandgap {
> ulong base;
> int temperature; /* in mili degree celsius */
> + int sens_cnt;
> };
>
> /*
> @@ -158,11 +159,16 @@ static int dra752_adc_to_temp[] = {
> 123800, 124200, 124600, 124900, 125000, 125000,
> };
>
> -static int ti_bandgap_get_temp(struct udevice *dev, int *temp)
> +static int ti_bandgap_get_temp(struct udevice *dev, u8 instance, int *temp)
> {
> struct ti_bandgap *bgp = dev_get_priv(dev);
>
> - bgp->temperature = 0x3ff & readl(bgp->base + CTRL_CORE_TEMP_SENSOR_MPU);
> + if (instance >= bgp->sens_cnt) {
> + printf("Only %d insatnces are supported\n", bgp->sens_cnt);
> + return -EINVAL;
> + }
> +
> + bgp->temperature = 0x3ff & readl(bgp->base + instance * 4);
> *temp = dra752_adc_to_temp[bgp->temperature - DRA752_ADC_START_VALUE];
>
> return 0;
> @@ -177,13 +183,14 @@ static int ti_bandgap_probe(struct udevice *dev)
> struct ti_bandgap *bgp = dev_get_priv(dev);
>
> bgp->base = devfdt_get_addr_index(dev, 1);
> + bgp->sens_cnt = dev_get_driver_data(dev);
>
> return 0;
> }
>
> static const struct udevice_id of_ti_bandgap_match[] = {
> {
> - .compatible = "ti,dra752-bandgap",
> + .compatible = "ti,dra752-bandgap", .data = 3,
> },
> {},
> };
> diff --git a/include/thermal.h b/include/thermal.h
> index 11d75256e0..82d80f3ded 100644
> --- a/include/thermal.h
> +++ b/include/thermal.h
> @@ -9,7 +9,7 @@
>
> #include <dm.h>
>
> -int thermal_get_temp(struct udevice *dev, int *temp);
> +int thermal_get_temp(struct udevice *dev, u8 instance, int *temp);
Please add function comment.
>
> /**
> * struct dm_thermal_ops - Driver model Thermal operations
> @@ -25,9 +25,11 @@ struct dm_thermal_ops {
> * It will enable and initialize any Thermal hardware as necessary.
> *
> * @dev: The Thermal device
> + * @instance: The instance of sensor in case multiple sensors are
> + * present if single sensor then this should be 0
> * @temp: pointer that returns the measured temperature
> */
> - int (*get_temp)(struct udevice *dev, int *temp);
> + int (*get_temp)(struct udevice *dev, u8 instance, int *temp);
I don't like the name 'instance' very much. With pwm we use 'channel'.
Would that be OK?
> };
>
> #endif /* _THERMAL_H_ */
> --
> 2.17.1
>
Regards,
Simon
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 1/3] thermal: Add multiple instance support
2019-03-19 1:23 ` Simon Glass
@ 2019-03-19 14:23 ` keerthy
2019-03-21 5:22 ` Simon Glass
0 siblings, 1 reply; 8+ messages in thread
From: keerthy @ 2019-03-19 14:23 UTC (permalink / raw)
To: u-boot
On 3/19/2019 6:53 AM, Simon Glass wrote:
> Hi Keerthy,
>
> On Mon, 11 Mar 2019 at 14:13, Keerthy <j-keerthy@ti.com> wrote:
>>
>> Currently single instance temperature read out is supported.
>> Enhance the same to support multiple instances.
>>
>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>> ---
>> arch/arm/mach-imx/cpu.c | 2 +-
>> drivers/mmc/omap_hsmmc.c | 2 +-
>> drivers/thermal/imx_thermal.c | 2 +-
>> drivers/thermal/thermal-uclass.c | 4 ++--
>> drivers/thermal/ti-bandgap.c | 13 ++++++++++---
>> include/thermal.h | 6 ++++--
>> 6 files changed, 19 insertions(+), 10 deletions(-)
>
> Please can you add a simple test for the thermal uclass in test/dm?
Okay. I will try to add that.
>
>>
>> diff --git a/arch/arm/mach-imx/cpu.c b/arch/arm/mach-imx/cpu.c
>> index 6b83f92662..27d12b410e 100644
>> --- a/arch/arm/mach-imx/cpu.c
>> +++ b/arch/arm/mach-imx/cpu.c
>> @@ -231,7 +231,7 @@ int print_cpuinfo(void)
>> printf("(%dC to %dC)", minc, maxc);
>> ret = uclass_get_device(UCLASS_THERMAL, 0, &thermal_dev);
>> if (!ret) {
>> - ret = thermal_get_temp(thermal_dev, &cpu_tmp);
>> + ret = thermal_get_temp(thermal_dev, 0, &cpu_tmp);
>>
>> if (!ret)
>> printf(" at %dC\n", cpu_tmp);
>> diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
>> index 826a39fad7..1872273dd9 100644
>> --- a/drivers/mmc/omap_hsmmc.c
>> +++ b/drivers/mmc/omap_hsmmc.c
>> @@ -642,7 +642,7 @@ static int omap_hsmmc_execute_tuning(struct udevice *dev, uint opcode)
>> printf("Couldn't get thermal device for tuning\n");
>> return ret;
>> }
>> - ret = thermal_get_temp(thermal_dev, &temperature);
>> + ret = thermal_get_temp(thermal_dev, 0, &temperature);
>> if (ret) {
>> printf("Couldn't get temperature for tuning\n");
>> return ret;
>> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
>> index e50b85bd59..271a61356c 100644
>> --- a/drivers/thermal/imx_thermal.c
>> +++ b/drivers/thermal/imx_thermal.c
>> @@ -207,7 +207,7 @@ static int read_cpu_temperature(struct udevice *dev)
>> }
>> #endif
>>
>> -int imx_thermal_get_temp(struct udevice *dev, int *temp)
>> +int imx_thermal_get_temp(struct udevice *dev, int instance, int *temp)
>> {
>> struct thermal_data *priv = dev_get_priv(dev);
>> int cpu_tmp = 0;
>> diff --git a/drivers/thermal/thermal-uclass.c b/drivers/thermal/thermal-uclass.c
>> index a4ea1e2914..b4a31280cb 100644
>> --- a/drivers/thermal/thermal-uclass.c
>> +++ b/drivers/thermal/thermal-uclass.c
>> @@ -13,14 +13,14 @@
>> #include <linux/list.h>
>>
>>
>> -int thermal_get_temp(struct udevice *dev, int *temp)
>> +int thermal_get_temp(struct udevice *dev, u8 instance, int *temp)
>> {
>> const struct dm_thermal_ops *ops = device_get_ops(dev);
>>
>> if (!ops->get_temp)
>> return -ENOSYS;
>>
>> - return ops->get_temp(dev, temp);
>> + return ops->get_temp(dev, instance, temp);
>> }
>>
>> UCLASS_DRIVER(thermal) = {
>> diff --git a/drivers/thermal/ti-bandgap.c b/drivers/thermal/ti-bandgap.c
>> index b490391e96..6d3606b0fc 100644
>> --- a/drivers/thermal/ti-bandgap.c
>> +++ b/drivers/thermal/ti-bandgap.c
>> @@ -27,6 +27,7 @@
>> struct ti_bandgap {
>> ulong base;
>> int temperature; /* in mili degree celsius */
>> + int sens_cnt;
>> };
>>
>> /*
>> @@ -158,11 +159,16 @@ static int dra752_adc_to_temp[] = {
>> 123800, 124200, 124600, 124900, 125000, 125000,
>> };
>>
>> -static int ti_bandgap_get_temp(struct udevice *dev, int *temp)
>> +static int ti_bandgap_get_temp(struct udevice *dev, u8 instance, int *temp)
>> {
>> struct ti_bandgap *bgp = dev_get_priv(dev);
>>
>> - bgp->temperature = 0x3ff & readl(bgp->base + CTRL_CORE_TEMP_SENSOR_MPU);
>> + if (instance >= bgp->sens_cnt) {
>> + printf("Only %d insatnces are supported\n", bgp->sens_cnt);
>> + return -EINVAL;
>> + }
>> +
>> + bgp->temperature = 0x3ff & readl(bgp->base + instance * 4);
>> *temp = dra752_adc_to_temp[bgp->temperature - DRA752_ADC_START_VALUE];
>>
>> return 0;
>> @@ -177,13 +183,14 @@ static int ti_bandgap_probe(struct udevice *dev)
>> struct ti_bandgap *bgp = dev_get_priv(dev);
>>
>> bgp->base = devfdt_get_addr_index(dev, 1);
>> + bgp->sens_cnt = dev_get_driver_data(dev);
>>
>> return 0;
>> }
>>
>> static const struct udevice_id of_ti_bandgap_match[] = {
>> {
>> - .compatible = "ti,dra752-bandgap",
>> + .compatible = "ti,dra752-bandgap", .data = 3,
>> },
>> {},
>> };
>> diff --git a/include/thermal.h b/include/thermal.h
>> index 11d75256e0..82d80f3ded 100644
>> --- a/include/thermal.h
>> +++ b/include/thermal.h
>> @@ -9,7 +9,7 @@
>>
>> #include <dm.h>
>>
>> -int thermal_get_temp(struct udevice *dev, int *temp);
>> +int thermal_get_temp(struct udevice *dev, u8 instance, int *temp);
>
> Please add function comment.
Okay.
>
>>
>> /**
>> * struct dm_thermal_ops - Driver model Thermal operations
>> @@ -25,9 +25,11 @@ struct dm_thermal_ops {
>> * It will enable and initialize any Thermal hardware as necessary.
>> *
>> * @dev: The Thermal device
>> + * @instance: The instance of sensor in case multiple sensors are
>> + * present if single sensor then this should be 0
>> * @temp: pointer that returns the measured temperature
>> */
>> - int (*get_temp)(struct udevice *dev, int *temp);
>> + int (*get_temp)(struct udevice *dev, u8 instance, int *temp);
>
> I don't like the name 'instance' very much. With pwm we use 'channel'.
> Would that be OK?
How about 'sensors'? Basically multiple sensors give out temperatures
of different thermal zones.
>
>> };
>>
>> #endif /* _THERMAL_H_ */
>> --
>> 2.17.1
>>
>
> Regards,
> Simon
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 1/3] thermal: Add multiple instance support
2019-03-19 14:23 ` keerthy
@ 2019-03-21 5:22 ` Simon Glass
0 siblings, 0 replies; 8+ messages in thread
From: Simon Glass @ 2019-03-21 5:22 UTC (permalink / raw)
To: u-boot
Hi Keerthy,
On Tue, 19 Mar 2019 at 22:23, keerthy <j-keerthy@ti.com> wrote:
>
>
>
> On 3/19/2019 6:53 AM, Simon Glass wrote:
> > Hi Keerthy,
> >
> > On Mon, 11 Mar 2019 at 14:13, Keerthy <j-keerthy@ti.com> wrote:
> >>
> >> Currently single instance temperature read out is supported.
> >> Enhance the same to support multiple instances.
> >>
> >> Signed-off-by: Keerthy <j-keerthy@ti.com>
> >> ---
> >> arch/arm/mach-imx/cpu.c | 2 +-
> >> drivers/mmc/omap_hsmmc.c | 2 +-
> >> drivers/thermal/imx_thermal.c | 2 +-
> >> drivers/thermal/thermal-uclass.c | 4 ++--
> >> drivers/thermal/ti-bandgap.c | 13 ++++++++++---
> >> include/thermal.h | 6 ++++--
> >> 6 files changed, 19 insertions(+), 10 deletions(-)
> >
> > Please can you add a simple test for the thermal uclass in test/dm?
>
> Okay. I will try to add that.
>
> >
> >>
> >> diff --git a/arch/arm/mach-imx/cpu.c b/arch/arm/mach-imx/cpu.c
> >> index 6b83f92662..27d12b410e 100644
> >> --- a/arch/arm/mach-imx/cpu.c
> >> +++ b/arch/arm/mach-imx/cpu.c
> >> @@ -231,7 +231,7 @@ int print_cpuinfo(void)
> >> printf("(%dC to %dC)", minc, maxc);
> >> ret = uclass_get_device(UCLASS_THERMAL, 0, &thermal_dev);
> >> if (!ret) {
> >> - ret = thermal_get_temp(thermal_dev, &cpu_tmp);
> >> + ret = thermal_get_temp(thermal_dev, 0, &cpu_tmp);
> >>
> >> if (!ret)
> >> printf(" at %dC\n", cpu_tmp);
> >> diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
> >> index 826a39fad7..1872273dd9 100644
> >> --- a/drivers/mmc/omap_hsmmc.c
> >> +++ b/drivers/mmc/omap_hsmmc.c
> >> @@ -642,7 +642,7 @@ static int omap_hsmmc_execute_tuning(struct udevice *dev, uint opcode)
> >> printf("Couldn't get thermal device for tuning\n");
> >> return ret;
> >> }
> >> - ret = thermal_get_temp(thermal_dev, &temperature);
> >> + ret = thermal_get_temp(thermal_dev, 0, &temperature);
> >> if (ret) {
> >> printf("Couldn't get temperature for tuning\n");
> >> return ret;
> >> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> >> index e50b85bd59..271a61356c 100644
> >> --- a/drivers/thermal/imx_thermal.c
> >> +++ b/drivers/thermal/imx_thermal.c
> >> @@ -207,7 +207,7 @@ static int read_cpu_temperature(struct udevice *dev)
> >> }
> >> #endif
> >>
> >> -int imx_thermal_get_temp(struct udevice *dev, int *temp)
> >> +int imx_thermal_get_temp(struct udevice *dev, int instance, int *temp)
> >> {
> >> struct thermal_data *priv = dev_get_priv(dev);
> >> int cpu_tmp = 0;
> >> diff --git a/drivers/thermal/thermal-uclass.c b/drivers/thermal/thermal-uclass.c
> >> index a4ea1e2914..b4a31280cb 100644
> >> --- a/drivers/thermal/thermal-uclass.c
> >> +++ b/drivers/thermal/thermal-uclass.c
> >> @@ -13,14 +13,14 @@
> >> #include <linux/list.h>
> >>
> >>
> >> -int thermal_get_temp(struct udevice *dev, int *temp)
> >> +int thermal_get_temp(struct udevice *dev, u8 instance, int *temp)
> >> {
> >> const struct dm_thermal_ops *ops = device_get_ops(dev);
> >>
> >> if (!ops->get_temp)
> >> return -ENOSYS;
> >>
> >> - return ops->get_temp(dev, temp);
> >> + return ops->get_temp(dev, instance, temp);
> >> }
> >>
> >> UCLASS_DRIVER(thermal) = {
> >> diff --git a/drivers/thermal/ti-bandgap.c b/drivers/thermal/ti-bandgap.c
> >> index b490391e96..6d3606b0fc 100644
> >> --- a/drivers/thermal/ti-bandgap.c
> >> +++ b/drivers/thermal/ti-bandgap.c
> >> @@ -27,6 +27,7 @@
> >> struct ti_bandgap {
> >> ulong base;
> >> int temperature; /* in mili degree celsius */
> >> + int sens_cnt;
> >> };
> >>
> >> /*
> >> @@ -158,11 +159,16 @@ static int dra752_adc_to_temp[] = {
> >> 123800, 124200, 124600, 124900, 125000, 125000,
> >> };
> >>
> >> -static int ti_bandgap_get_temp(struct udevice *dev, int *temp)
> >> +static int ti_bandgap_get_temp(struct udevice *dev, u8 instance, int *temp)
> >> {
> >> struct ti_bandgap *bgp = dev_get_priv(dev);
> >>
> >> - bgp->temperature = 0x3ff & readl(bgp->base + CTRL_CORE_TEMP_SENSOR_MPU);
> >> + if (instance >= bgp->sens_cnt) {
> >> + printf("Only %d insatnces are supported\n", bgp->sens_cnt);
> >> + return -EINVAL;
> >> + }
> >> +
> >> + bgp->temperature = 0x3ff & readl(bgp->base + instance * 4);
> >> *temp = dra752_adc_to_temp[bgp->temperature - DRA752_ADC_START_VALUE];
> >>
> >> return 0;
> >> @@ -177,13 +183,14 @@ static int ti_bandgap_probe(struct udevice *dev)
> >> struct ti_bandgap *bgp = dev_get_priv(dev);
> >>
> >> bgp->base = devfdt_get_addr_index(dev, 1);
> >> + bgp->sens_cnt = dev_get_driver_data(dev);
> >>
> >> return 0;
> >> }
> >>
> >> static const struct udevice_id of_ti_bandgap_match[] = {
> >> {
> >> - .compatible = "ti,dra752-bandgap",
> >> + .compatible = "ti,dra752-bandgap", .data = 3,
> >> },
> >> {},
> >> };
> >> diff --git a/include/thermal.h b/include/thermal.h
> >> index 11d75256e0..82d80f3ded 100644
> >> --- a/include/thermal.h
> >> +++ b/include/thermal.h
> >> @@ -9,7 +9,7 @@
> >>
> >> #include <dm.h>
> >>
> >> -int thermal_get_temp(struct udevice *dev, int *temp);
> >> +int thermal_get_temp(struct udevice *dev, u8 instance, int *temp);
> >
> > Please add function comment.
>
> Okay.
>
> >
> >>
> >> /**
> >> * struct dm_thermal_ops - Driver model Thermal operations
> >> @@ -25,9 +25,11 @@ struct dm_thermal_ops {
> >> * It will enable and initialize any Thermal hardware as necessary.
> >> *
> >> * @dev: The Thermal device
> >> + * @instance: The instance of sensor in case multiple sensors are
> >> + * present if single sensor then this should be 0
> >> * @temp: pointer that returns the measured temperature
> >> */
> >> - int (*get_temp)(struct udevice *dev, int *temp);
> >> + int (*get_temp)(struct udevice *dev, u8 instance, int *temp);
> >
> > I don't like the name 'instance' very much. With pwm we use 'channel'.
> > Would that be OK?
>
> How about 'sensors'? Basically multiple sensors give out temperatures
> of different thermal zones.
Hmm I still prefer channel to fit in with PWM. But if you really want
to use 'sensor' (singular, not plural) then OK.
Regards,
Simon
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH 2/3] cmd: thermal: Add command line interface to read out temperatures
2019-03-11 6:12 ` [U-Boot] [PATCH 2/3] cmd: thermal: Add command line interface to read out temperatures Keerthy
@ 2019-03-22 7:53 ` Simon Glass
0 siblings, 0 replies; 8+ messages in thread
From: Simon Glass @ 2019-03-22 7:53 UTC (permalink / raw)
To: u-boot
Hi Keerthy,
On Mon, 11 Mar 2019 at 14:13, Keerthy <j-keerthy@ti.com> wrote:
>
> Add command line interface to read out temperatures from SoC.
> Takes two arguments. One is 'temperature' and the other is
> instance number. In case instance number is not provided by
> default 0th instance temperature is read out.
>
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
> cmd/Kconfig | 5 +++++
> cmd/Makefile | 1 +
> cmd/thermal.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 65 insertions(+)
> create mode 100644 cmd/thermal.c
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 4bcc5c4557..d252e93d64 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1044,6 +1044,11 @@ config CMD_SPI
> help
> SPI utility command.
>
> +config CMD_THERMAL
> + bool "thermal"
> + help
> + THERMAL support.
Thermal support
> +
> config CMD_TSI148
> bool "tsi148 - Command to access tsi148 device"
> help
> diff --git a/cmd/Makefile b/cmd/Makefile
> index acb85f49fb..2b66f9e36a 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -128,6 +128,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o
> obj-$(CONFIG_CMD_STRINGS) += strings.o
> obj-$(CONFIG_CMD_SMC) += smccc.o
> obj-$(CONFIG_CMD_TERMINAL) += terminal.o
> +obj-$(CONFIG_CMD_THERMAL) += thermal.o
> obj-$(CONFIG_CMD_TIME) += time.o
> obj-$(CONFIG_CMD_TRACE) += trace.o
> obj-$(CONFIG_HUSH_PARSER) += test.o
> diff --git a/cmd/thermal.c b/cmd/thermal.c
> new file mode 100644
> index 0000000000..a31f992425
> --- /dev/null
> +++ b/cmd/thermal.c
> @@ -0,0 +1,59 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Thermal CMD
> + *
> + * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com/
> + */
> +#include <common.h>
> +#include <errno.h>
> +#include <dm.h>
> +#include <dm/uclass-internal.h>
Should go at the end
> +#include <thermal.h>
> +
> +#define MAX_TEMP 150
Is this in celsius?
> +
> +static int do_read_temp(cmd_tbl_t *cmdtp, int flag, int argc,
> + char * const argv[])
> +{
> + struct udevice *tdev;
> + int cpu_temp, ret = 0;
> + u8 instance;
uint
Please use channel instead
> + const char *units;
> +
> + if (argc > 2) {
> + printf("Max Number of arguments is 2\n");
> + return -EINVAL;
return CMD_RET_USAGE
If you return -ve values it can do strange things like cause the
calling shell to exit.
> + }
> +
> + /* In case instance is not given default 0th instnace is reported */
instance
> + if (argc == 2)
> + instance = (u8)simple_strtoul(argv[1], NULL, 10);
Drop u8 cast
> + else
> + instance = 0;
> +
> + ret = uclass_get_device(UCLASS_THERMAL, 0, &tdev);
uclass_first_device_err()?
That is more robust than requiring it be seq #0.
> + if (!ret) {
> + ret = thermal_get_temp(tdev, instance, &cpu_temp);
> + if (!ret) {
> + if (abs(cpu_temp) < MAX_TEMP)
> + units = "C";
> + else
> + units = "mC";
The interface should be in mC, then. We can't have the range of the
value determine the units.
> +
> + printf("Instance %d Temperature at %d%s\n", instance,
> + cpu_temp, units);
> + } else {
> + debug(" - invalid sensor data\n");
Maybe -ENOENT means invalid channel
-ENODATA means the channel exists but temperature could not be read?
> + }
> + } else {
> + printf(" - invalid sensor device\n");
> + }
> +
if (ret)
return CMD_RET_FAILURE;
return 0
> + return ret;
Don't return -ve values from commands.
> +}
> +
> +U_BOOT_CMD(
> + temperature, 2, 0, do_read_temp,
> + "Reads temperature of a given sensor",
> + " [sensor number] - number of the temperature sensor"
> +);
> --
> 2.17.1
>
Could we have a pytest for this command for sandbox?
Regards,
Simon
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-03-22 7:53 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-11 6:12 [U-Boot] [PATCH 0/3] thermal: Add multiple instance support Keerthy
2019-03-11 6:12 ` [U-Boot] [PATCH 1/3] " Keerthy
2019-03-19 1:23 ` Simon Glass
2019-03-19 14:23 ` keerthy
2019-03-21 5:22 ` Simon Glass
2019-03-11 6:12 ` [U-Boot] [PATCH 2/3] cmd: thermal: Add command line interface to read out temperatures Keerthy
2019-03-22 7:53 ` Simon Glass
2019-03-11 6:12 ` [U-Boot] [PATCH 3/3] configs: dra7xx_evm_defconfig: Enable TI_DRA7_THERMAL & CMD_THERMAL Keerthy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.