mbox series

[v1,00/24] thermal: tsens: Refactor to use regmap_field

Message ID cover.1549525708.git.amit.kucheria@linaro.org
Headers show
Series thermal: tsens: Refactor to use regmap_field | expand

Message

Amit Kucheria Feb. 7, 2019, 10:49 a.m. UTC
Short term bandaid fixes to add support for more versions of the TSENS IP
is not proving to be very scalable. Here is a series to finally convert the
driver's core to use regmap_fields so that register addresses and bitfields
can be abstracted away from the code.

This series is posted as one to give a complete view of where I'm going
with this refactor. We can probably split it into smaller sets if required.
Currently it consists of the following parts:

- Patch 01-06: Style changes to make code easier to read (mostly naming
  related). This has no functional change in the driver, it is a series of
  search and replace operations. Ideally this will get merged first,
  otherwise everything else will become painful :-)

- Patch 07-09: Merge the 8916 and 8974 source files into a single one
  called v0_1.c denoting the version of the IP and open up more
  opportunities for reuse.

- Patch 10: This is the core patch that adds IP-specific data based on two
  data structures: tsens_features has bit flags to identify if a certain
  feature is available in a version or not and an array of reg_field
  pointers.

  regfield_ids list the important fields across IP versions and are used as
  an index.

  This patch converts over all platforms except 8960 which currently
  doesn't use DT data for tsens and has custom functions for everything. We
  will get to it eventually, but it's conversion is not a necessity for
  this series to be merged.

- Patch 11-16: Clean ups and new functionality based on regmap_field

- Patch 17-19: Refactor the get_temp routine. We expect to have a single
  get_temp routine once all the conversion is finished.

- Patch 20-23: Introduce qcs404 support using new regmap infrastructure.

Testing
-------

Since these changes affect every platform using TSENS, it should be
clarified that I have only been focussing on developing/testing this on
sdm845 and qcs404. I will test this more thoroughly on msm8916, msm8996,
msm8998 but it is still pending.

Future work
-----------

a. Get rid of get_temp_common in favour of get_temp_tsens_valid
b. irq support
c. Convert over 8960 to use regmap_field and as a result common functions

Regards,
Amit

Amit Kucheria (24):
  drivers: thermal: tsens: Document the data structures
  drivers: thermal: tsens: Rename tsens_data
  drivers: thermal: tsens: Rename tsens_device
  drivers: thermal: tsens: Rename variable tmdev
  drivers: thermal: tsens: Use consistent names for variables
  drivers: thermal: tsens: Function prototypes should have argument
    names
  drivers: thermal: tsens: Rename tsens-8916 to prepare to merge with
    tsens-8974
  drivers: thermal: tsens: Rename constants to prepare to merge with
    tsens-8974
  drivers: thermal: tsens: Merge tsens-8974 into tsens-v0_1
  drivers: thermal: tsens: Introduce reg_fields to deal with register
    description
  drivers: thermal: tsens: Save reference to the device pointer and use
    it
  drivers: thermal: tsens: Don't print error message on -EPROBE_DEFER
  drivers: thermal: tsens: Print IP version
  drivers: thermal: tsens: Add new operation to check if a sensor is
    enabled
  drivers: thermal: tsens: change data type for sensor IDs
  drivers: thermal: tsens: Introduce IP-specific max_sensor count
  drivers: thermal: tsens: simplify get_temp_tsens_v2 routine
  drivers: thermal: tsens: Move get_temp_tsens_v2 to allow sharing
  drivers: thermal: tsens: Common get_temp() learns to do ADC conversion
  dt: thermal: tsens: Add bindings for qcs404
  drivers: thermal: tsens: Add generic support for TSENS v1 IP
  arm64: dts: qcom: qcs404: Add tsens controller
  arm64: dts: qcom: qcs404: Add thermal zones for each sensor
  drivers: thermal: tsens: Move calibration constants to header file

 .../bindings/thermal/qcom-tsens.txt           |  14 +
 arch/arm64/boot/dts/qcom/qcs404.dtsi          | 263 ++++++++++++++++++
 drivers/thermal/qcom/Makefile                 |   4 +-
 drivers/thermal/qcom/tsens-8916.c             | 105 -------
 drivers/thermal/qcom/tsens-8960.c             |  84 +++---
 drivers/thermal/qcom/tsens-common.c           | 172 +++++++++---
 .../qcom/{tsens-8974.c => tsens-v0_1.c}       | 167 ++++++++++-
 drivers/thermal/qcom/tsens-v1.c               | 229 +++++++++++++++
 drivers/thermal/qcom/tsens-v2.c               | 148 ++++++----
 drivers/thermal/qcom/tsens.c                  | 100 ++++---
 drivers/thermal/qcom/tsens.h                  | 238 ++++++++++++++--
 11 files changed, 1191 insertions(+), 333 deletions(-)
 delete mode 100644 drivers/thermal/qcom/tsens-8916.c
 rename drivers/thermal/qcom/{tsens-8974.c => tsens-v0_1.c} (55%)
 create mode 100644 drivers/thermal/qcom/tsens-v1.c

-- 
2.17.1

Comments

Eduardo Valentin Feb. 20, 2019, 1:17 a.m. UTC | #1
On Thu, Feb 07, 2019 at 04:19:31PM +0530, Amit Kucheria wrote:
> On some TSENS IP, version is stored. Print that version at init.

> 

> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>

> ---

>  drivers/thermal/qcom/tsens-common.c | 23 ++++++++++++++++++++++-

>  1 file changed, 22 insertions(+), 1 deletion(-)

> 

> diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c

> index aae3d71d7eed..39cd5733fd44 100644

> --- a/drivers/thermal/qcom/tsens-common.c

> +++ b/drivers/thermal/qcom/tsens-common.c

> @@ -121,7 +121,7 @@ int __init init_common(struct tsens_priv *priv)

>  	void __iomem *tm_base, *srot_base;

>  	struct device *dev = priv->dev;

>  	struct resource *res;

> -	u32 enabled;

> +	u32 enabled, maj_ver, min_ver;

>  	int ret, i, j;

>  	struct platform_device *op = of_find_device_by_node(priv->dev->of_node);

>  

> @@ -155,6 +155,27 @@ int __init init_common(struct tsens_priv *priv)

>  	if (IS_ERR(priv->tm_map))

>  		return PTR_ERR(priv->tm_map);

>  

> +	for (i = 0; i < MAX_REGFIELDS; i++) {

> +		priv->rf[i] = NULL;

> +	}

> +

> +	/* alloc regmap_fields in srot_map */

> +	if (priv->feat->ver_info) {

> +		for (i = 0, j = VER_MAJOR; i < 2; i++, j++) {

> +			priv->rf[j] = devm_regmap_field_alloc(dev, priv->srot_map,

> +							      priv->fields[j]);

> +			if (IS_ERR(priv->rf[j]))

> +				return PTR_ERR(priv->rf[j]);

> +		}

> +		ret = regmap_field_read(priv->rf[VER_MAJOR], &maj_ver);

> +		if (ret)

> +			return ret;

> +		ret = regmap_field_read(priv->rf[VER_MINOR], &min_ver);

> +		if (ret)

> +			return ret;

> +		dev_info(dev, "version: %d.%d\n", maj_ver, min_ver);


Is this really needed? Printing stuff into kernel log during boot can
just add a lot of info that ends up just slowing down things. Besides,
if this info is important shouldnt be available somewhere else as the
kernel log is a circular buffer and eventually that line will get
wrapped up?

> +	}

> +

>  	priv->rf[TSENS_EN] = devm_regmap_field_alloc(dev, priv->srot_map,

>  						     priv->fields[TSENS_EN]);

>  	if (IS_ERR(priv->rf[TSENS_EN]))

> -- 

> 2.17.1

>
Amit Kucheria Feb. 20, 2019, 9:40 a.m. UTC | #2
On Wed, Feb 20, 2019 at 6:39 AM Eduardo Valentin <edubezval@gmail.com> wrote:
>

> On Thu, Feb 07, 2019 at 04:19:41PM +0530, Amit Kucheria wrote:

> > qcs404 has 10 sensors connected to the single TSENS IP. Define a thermal

> > zone for each of those sensors to expose the temperature of each zone.

> >

> > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>

> > ---

> >  arch/arm64/boot/dts/qcom/qcs404.dtsi | 243 +++++++++++++++++++++++++++

> >  1 file changed, 243 insertions(+)

> >

> > diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi

> > index 57d14d8f0c90..ca99c45864df 100644

> > --- a/arch/arm64/boot/dts/qcom/qcs404.dtsi

> > +++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi

> > @@ -3,6 +3,7 @@

> >

> >  #include <dt-bindings/interrupt-controller/arm-gic.h>

> >  #include <dt-bindings/clock/qcom,gcc-qcs404.h>

> > +#include <dt-bindings/thermal/thermal.h>

> >

> >  / {

> >       interrupt-parent = <&intc>;

> > @@ -30,6 +31,7 @@

> >                       reg = <0x100>;

> >                       enable-method = "psci";

> >                       next-level-cache = <&L2_0>;

> > +                     #cooling-cells= <2>;

> >               };

> >

> >               CPU1: cpu@101 {

> > @@ -38,6 +40,7 @@

> >                       reg = <0x101>;

> >                       enable-method = "psci";

> >                       next-level-cache = <&L2_0>;

> > +                     #cooling-cells= <2>;

> >               };

> >

> >               CPU2: cpu@102 {

> > @@ -46,6 +49,7 @@

> >                       reg = <0x102>;

> >                       enable-method = "psci";

> >                       next-level-cache = <&L2_0>;

> > +                     #cooling-cells= <2>;

> >               };

> >

> >               CPU3: cpu@103 {

> > @@ -54,6 +58,7 @@

> >                       reg = <0x103>;

> >                       enable-method = "psci";

> >                       next-level-cache = <&L2_0>;

> > +                     #cooling-cells= <2>;

> >               };

> >

> >               L2_0: l2-cache {

> > @@ -507,4 +512,242 @@

> >                       #interrupt-cells = <2>;

> >               };

> >       };

> > +

> > +     thermal-zones {

>

> There are several zones with passive trips and no cooling maps.


Indeed. The policy bits are still a WIP and in some cases the
throttling is handled by someone in userspace. But I'd like to expose
at least the temperatures at each of the sensors to make it easier for
others to use this information. See my reply to the cover letter.

Regards,
Amit




> > +             aoss-thermal {

> > +                     polling-delay-passive = <250>;

> > +                     polling-delay = <1000>;

> > +

> > +                     thermal-sensors = <&tsens 0>;

> > +

> > +                     trips {

> > +                             aoss_alert0: trip-point@0 {

> > +                                     temperature = <95000>;

> > +                                     hysteresis = <2000>;

> > +                                     type = "passive";

> > +                             };

> > +                             aoss_crit: aoss_crit {

> > +                                     temperature = <110000>;

> > +                                     hysteresis = <2000>;

> > +                                     type = "critical";

> > +                             };

> > +                     };

> > +             };

> > +

> > +             dsp-thermal {

> > +                     polling-delay-passive = <250>;

> > +                     polling-delay = <1000>;

> > +

> > +                     thermal-sensors = <&tsens 1>;

> > +

> > +                     trips {

> > +                             dsp_alert0: trip-point@0 {

> > +                                     temperature = <95000>;

> > +                                     hysteresis = <2000>;

> > +                                     type = "passive";

> > +                             };

> > +                             dsp_crit: dsp_crit {

> > +                                     temperature = <110000>;

> > +                                     hysteresis = <2000>;

> > +                                     type = "critical";

> > +                             };

> > +                     };

> > +             };

> > +

> > +             lpass-thermal {

> > +                     polling-delay-passive = <250>;

> > +                     polling-delay = <1000>;

> > +

> > +                     thermal-sensors = <&tsens 2>;

> > +

> > +                     trips {

> > +                             lpass_alert0: trip-point@0 {

> > +                                     temperature = <95000>;

> > +                                     hysteresis = <2000>;

> > +                                     type = "passive";

> > +                             };

> > +                             lpass_crit: lpass_crit {

> > +                                     temperature = <110000>;

> > +                                     hysteresis = <2000>;

> > +                                     type = "critical";

> > +                             };

> > +                     };

> > +             };

> > +

> > +             wlan-thermal {

> > +                     polling-delay-passive = <250>;

> > +                     polling-delay = <1000>;

> > +

> > +                     thermal-sensors = <&tsens 3>;

> > +

> > +                     trips {

> > +                             wlan_alert0: trip-point@0 {

> > +                                     temperature = <95000>;

> > +                                     hysteresis = <2000>;

> > +                                     type = "passive";

> > +                             };

> > +                             wlan_crit: wlan_crit {

> > +                                     temperature = <110000>;

> > +                                     hysteresis = <2000>;

> > +                                     type = "critical";

> > +                             };

> > +                     };

> > +             };

> > +

> > +             cluster-thermal {

> > +                     polling-delay-passive = <250>;

> > +                     polling-delay = <1000>;

> > +

> > +                     thermal-sensors = <&tsens 4>;

> > +

> > +                     trips {

> > +                             cluster_alert0: trip-point@0 {

> > +                                     temperature = <95000>;

> > +                                     hysteresis = <2000>;

> > +                                     type = "passive";

> > +                             };

> > +                             cluster_crit: cluster_crit {

> > +                                     temperature = <110000>;

> > +                                     hysteresis = <2000>;

> > +                                     type = "critical";

> > +                             };

> > +                     };

> > +             };

> > +

> > +             cpu0-thermal {

> > +                     polling-delay-passive = <250>;

> > +                     polling-delay = <1000>;

> > +

> > +                     thermal-sensors = <&tsens 5>;

> > +

> > +                     trips {

> > +                             cpu0_alert0: trip-point@0 {

> > +                                     temperature = <95000>;

> > +                                     hysteresis = <2000>;

> > +                                     type = "passive";

> > +                             };

> > +                             cpu0_crit: cpu_crit {

> > +                                     temperature = <110000>;

> > +                                     hysteresis = <2000>;

> > +                                     type = "critical";

> > +                             };

> > +                     };

> > +                     cooling-maps {

> > +                             map0 {

> > +                                     trip = <&cpu0_alert0>;

> > +                                     cooling-device = <&CPU0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,

> > +                                                    <&CPU1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,

> > +                                                    <&CPU2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,

> > +                                                    <&CPU3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;

> > +                             };

> > +                     };

> > +             };

> > +

> > +             cpu1-thermal {

> > +                     polling-delay-passive = <250>;

> > +                     polling-delay = <1000>;

> > +

> > +                     thermal-sensors = <&tsens 6>;

> > +

> > +                     trips {

> > +                             cpu1_alert0: trip-point@0 {

> > +                                     temperature = <95000>;

> > +                                     hysteresis = <2000>;

> > +                                     type = "passive";

> > +                             };

> > +                             cpu1_crit: cpu_crit {

> > +                                     temperature = <110000>;

> > +                                     hysteresis = <2000>;

> > +                                     type = "critical";

> > +                             };

> > +                     };

> > +                     cooling-maps {

> > +                             map0 {

> > +                                     trip = <&cpu1_alert0>;

> > +                                     cooling-device = <&CPU0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,

> > +                                                    <&CPU1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,

> > +                                                    <&CPU2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,

> > +                                                    <&CPU3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;

> > +                             };

> > +                     };

> > +             };

> > +

> > +             cpu2-thermal {

> > +                     polling-delay-passive = <250>;

> > +                     polling-delay = <1000>;

> > +

> > +                     thermal-sensors = <&tsens 7>;

> > +

> > +                     trips {

> > +                             cpu2_alert0: trip-point@0 {

> > +                                     temperature = <95000>;

> > +                                     hysteresis = <2000>;

> > +                                     type = "passive";

> > +                             };

> > +                             cpu2_crit: cpu_crit {

> > +                                     temperature = <110000>;

> > +                                     hysteresis = <2000>;

> > +                                     type = "critical";

> > +                             };

> > +                     };

> > +                     cooling-maps {

> > +                             map0 {

> > +                                     trip = <&cpu2_alert0>;

> > +                                     cooling-device = <&CPU0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,

> > +                                                    <&CPU1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,

> > +                                                    <&CPU2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,

> > +                                                    <&CPU3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;

> > +                             };

> > +                     };

> > +             };

> > +

> > +             cpu3-thermal {

> > +                     polling-delay-passive = <250>;

> > +                     polling-delay = <1000>;

> > +

> > +                     thermal-sensors = <&tsens 8>;

> > +

> > +                     trips {

> > +                             cpu3_alert0: trip-point@0 {

> > +                                     temperature = <95000>;

> > +                                     hysteresis = <2000>;

> > +                                     type = "passive";

> > +                             };

> > +                             cpu3_crit: cpu_crit {

> > +                                     temperature = <110000>;

> > +                                     hysteresis = <2000>;

> > +                                     type = "critical";

> > +                             };

> > +                     };

> > +                     cooling-maps {

> > +                             map0 {

> > +                                     trip = <&cpu3_alert0>;

> > +                                     cooling-device = <&CPU0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,

> > +                                                    <&CPU1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,

> > +                                                    <&CPU2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,

> > +                                                    <&CPU3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;

> > +                             };

> > +                     };

> > +             };

> > +

> > +             gpu-thermal {

> > +                     polling-delay-passive = <250>;

> > +                     polling-delay = <1000>;

> > +

> > +                     thermal-sensors = <&tsens 9>;

> > +

> > +                     trips {

> > +                             gpu_alert0: trip-point@0 {

> > +                                     temperature = <95000>;

> > +                                     hysteresis = <2000>;

> > +                                     type = "passive";

> > +                             };

> > +                             gpu_crit: gpu_crit {

> > +                                     temperature = <95000>;

> > +                                     hysteresis = <2000>;

> > +                                     type = "critical";

> > +                             };

> > +                     };

> > +             };

> > +     };

> >  };

> > --

> > 2.17.1

> >