Re: [PATCH v3 2/4] thermal: k3: Add support for bandgap sensors

From: Randy Dunlap
Date: Wed Feb 19 2020 - 02:50:32 EST


On 2/18/20 11:43 PM, Keerthy wrote:
> The bandgap provides current and voltage reference for its internal
> circuits and other analog IP blocks. The analog-to-digital
> converter (ADC) produces an output value that is proportional
> to the silicon temperature.
>
> Currently reading temperatures and trend computing is supported
> as there are no active/passive cooling agent supported.
>
> Signed-off-by: Keerthy <j-keerthy@xxxxxx>
> ---
> drivers/thermal/Kconfig | 12 ++
> drivers/thermal/Makefile | 1 +
> drivers/thermal/k3_bandgap.c | 342 +++++++++++++++++++++++++++++++++++
> 3 files changed, 355 insertions(+)
> create mode 100644 drivers/thermal/k3_bandgap.c
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index 5a05db5438d6..fa598eddc7ac 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -251,6 +251,18 @@ config IMX_THERMAL
> cpufreq is used as the cooling device to throttle CPUs when the
> passive trip is crossed.
>
> +config K3_THERMAL
> + bool "Texas Instruments K3 thermal support"

Hi,
Why is this bool and not tristate?

> + depends on THERMAL
> + depends on ARCH_K3 || COMPILE_TEST
> + help
> + If you say yes here you get thermal support for the Texas Instruments
> + K3 SoC family. The current chip supported is:
> + - AM654
> +
> + This includes temperature reading functionality and also trend
> + computation.
> +
> config MAX77620_THERMAL
> tristate "Temperature sensor driver for Maxim MAX77620 PMIC"
> depends on MFD_MAX77620

> diff --git a/drivers/thermal/k3_bandgap.c b/drivers/thermal/k3_bandgap.c
> new file mode 100644
> index 000000000000..35d904eb9857
> --- /dev/null
> +++ b/drivers/thermal/k3_bandgap.c
> @@ -0,0 +1,342 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * TI Bandgap temperature sensor driver for K3 SoC Family
> + *
> + * Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com/
> + */

> +static int k3_bgp_read_temp(struct k3_thermal_data *devdata,
> + int *temp)
> +{
> + struct k3_bandgap *bgp;
> + unsigned int dtemp, s0, s1, s2;
> +
> + bgp = devdata->bgp;
> + /**

"/**" is used to indicate kernel-doc notation in the Linux kernel source files,
but this comment is not kernel-doc notation, so please just use "/*".

> + * Errata is applicable for am654 pg 1.0 silicon. There
> + * is a variation of the order for 8-10 degree centigrade.
> + * Work around that by getting the average of two closest
> + * readings out of three readings everytime we want to
> + * report temperatures.
> + *
> + * Errata workaround.
> + */
thanks.
--
~Randy