mbox series

[v3,00/16] Another round of tsens cleanups

Message ID cover.1536744310.git.amit.kucheria@linaro.org
Headers show
Series Another round of tsens cleanups | expand

Message

Amit Kucheria Sept. 12, 2018, 9:52 a.m. UTC
This is another series of tsens cleanups before we add interrupt support. This applies on top of 4.19-rc2.

Patches [1-6] can directly be applied by Eduardo.
Patches [9-16] can directly be applied by Andy.
Patches [7-8] introduce a new check and needs review and can then be
applied completely separately by Eduardo. 

There is a *dependency* however. Patch 1 should be applied to the tree
before the DT changes in patches 9 and 10.

In this series, I've collected all outstanding tsens patches by combining
the patches from the v2 posting with 3 DT patches for sdm845 thermal zones
and msm8916 camera and gpu thermal zones. Details are as follows:

- splitup 8916 and 8974 register address spaces for SROT and TM
- cleanups: move to spdx, dead code removal, removal of id field
- Add support to map the SROT address space for DTs that list it separately
- Check if TSENS IP is enabled in firmware by querying the SROT space
- Add myself as maintainer of tsens
- Add one-off patches adding sdm845 thermal zones and msm8916 gpu and
  camera thermal zones to this series.

Changes since v2:
- Split up changes to code and dt and within dt, split into arm and arm64
  patches
- Rearrange the sequence of patches so that the thermal maintainer can pick
  up the first part and the platform maintainer can pick up the latter part
  of the series
- Drop patch 09/11 because it causes some issues on systems with
  non-contiguous enabled sensors e.g. db410c
- Drop patch 10/11 because reg-name property isn't encouraged
- Fix a bug on msm8916 to check if IP is enabled in firmware. This was due
  to the register being at a different offset on msm8916. Fixed by coding
  this information as private data.

Changes since v1:
- Split up changes that split the address space and added qcom,sensors
  property into two separate patches
- Remove brackets in typo correction patch

Amit Kucheria (16):
  thermal: tsens: Prepare 8916 and 8974 tsens to use SROT and TM address
    space
  thermal: tsens: Add SPDX license identifiers
  thermal: tsens: Get rid of dead code
  thermal: tsens: Rename map field in order to add a second address map
  thermal: tsens: Add the SROT address map
  MAINTAINERS: Add entry for Qualcomm TSENS thermal drivers
  thermal: tsens: Pass register offsets as private data
  thermal: tsens: Check if the IP is correctly enabled by firmware
  arm: dts: msm8974: thermal: split address space into two
  arm64: dts: msm8916: thermal: split address space into two
  arm: dts: msm8974: thermal: Add "qcom,sensors" property
  arm64: dts: msm8916: thermal: Add "qcom,sensors" property
  dt-bindings: thermal: Fix a typo in documentation
  arm64: dts: msm8916: Add gpu thermal zone
  arm64: dts: msm8916: Add camera thermal zone
  arm64: dts: sdm845: enable tsens thermal zones

 .../devicetree/bindings/thermal/thermal.txt   |   2 +-
 MAINTAINERS                                   |   7 +
 arch/arm/boot/dts/qcom-msm8974.dtsi           |   6 +-
 arch/arm64/boot/dts/qcom/msm8916.dtsi         |  47 ++++-
 arch/arm64/boot/dts/qcom/sdm845.dtsi          | 170 ++++++++++++++++++
 drivers/thermal/qcom/tsens-8916.c             |  12 +-
 drivers/thermal/qcom/tsens-8960.c             |  41 ++---
 drivers/thermal/qcom/tsens-8974.c             |  12 +-
 drivers/thermal/qcom/tsens-common.c           |  62 ++++---
 drivers/thermal/qcom/tsens-v2.c               |   8 +-
 drivers/thermal/qcom/tsens.c                  |  19 +-
 drivers/thermal/qcom/tsens.h                  |  23 +--
 12 files changed, 309 insertions(+), 100 deletions(-)

Comments

Amit Kucheria Sept. 18, 2018, 3:01 p.m. UTC | #1
Hi Eduardo, Andy,

On Wed, Sep 12, 2018 at 2:53 AM Amit Kucheria <amit.kucheria@linaro.org> wrote:
>
> This is another series of tsens cleanups before we add interrupt support. This applies on top of 4.19-rc2.
>
> Patches [1-6] can directly be applied by Eduardo.
> Patches [9-16] can directly be applied by Andy.
> Patches [7-8] introduce a new check and needs review and can then be
> applied completely separately by Eduardo.
>
> There is a *dependency* however. Patch 1 should be applied to the tree
> before the DT changes in patches 9 and 10.

To clarify here, it might be easier if Eduardo acks patches 9 and 10
so that they can be merged through Andy's tree before the rest of the
DT patches. That would remove the need to create any immutable trees
or having to worry about the ordering of the patches.

Just a suggestion.

> In this series, I've collected all outstanding tsens patches by combining
> the patches from the v2 posting with 3 DT patches for sdm845 thermal zones
> and msm8916 camera and gpu thermal zones. Details are as follows:
>
> - splitup 8916 and 8974 register address spaces for SROT and TM
> - cleanups: move to spdx, dead code removal, removal of id field
> - Add support to map the SROT address space for DTs that list it separately
> - Check if TSENS IP is enabled in firmware by querying the SROT space
> - Add myself as maintainer of tsens
> - Add one-off patches adding sdm845 thermal zones and msm8916 gpu and
>   camera thermal zones to this series.
>
> Changes since v2:
> - Split up changes to code and dt and within dt, split into arm and arm64
>   patches
> - Rearrange the sequence of patches so that the thermal maintainer can pick
>   up the first part and the platform maintainer can pick up the latter part
>   of the series
> - Drop patch 09/11 because it causes some issues on systems with
>   non-contiguous enabled sensors e.g. db410c
> - Drop patch 10/11 because reg-name property isn't encouraged
> - Fix a bug on msm8916 to check if IP is enabled in firmware. This was due
>   to the register being at a different offset on msm8916. Fixed by coding
>   this information as private data.
>
> Changes since v1:
> - Split up changes that split the address space and added qcom,sensors
>   property into two separate patches
> - Remove brackets in typo correction patch
>
> Amit Kucheria (16):
>   thermal: tsens: Prepare 8916 and 8974 tsens to use SROT and TM address
>     space
>   thermal: tsens: Add SPDX license identifiers
>   thermal: tsens: Get rid of dead code
>   thermal: tsens: Rename map field in order to add a second address map
>   thermal: tsens: Add the SROT address map
>   MAINTAINERS: Add entry for Qualcomm TSENS thermal drivers
>   thermal: tsens: Pass register offsets as private data
>   thermal: tsens: Check if the IP is correctly enabled by firmware
>   arm: dts: msm8974: thermal: split address space into two
>   arm64: dts: msm8916: thermal: split address space into two
>   arm: dts: msm8974: thermal: Add "qcom,sensors" property
>   arm64: dts: msm8916: thermal: Add "qcom,sensors" property
>   dt-bindings: thermal: Fix a typo in documentation
>   arm64: dts: msm8916: Add gpu thermal zone
>   arm64: dts: msm8916: Add camera thermal zone
>   arm64: dts: sdm845: enable tsens thermal zones
>
>  .../devicetree/bindings/thermal/thermal.txt   |   2 +-
>  MAINTAINERS                                   |   7 +
>  arch/arm/boot/dts/qcom-msm8974.dtsi           |   6 +-
>  arch/arm64/boot/dts/qcom/msm8916.dtsi         |  47 ++++-
>  arch/arm64/boot/dts/qcom/sdm845.dtsi          | 170 ++++++++++++++++++
>  drivers/thermal/qcom/tsens-8916.c             |  12 +-
>  drivers/thermal/qcom/tsens-8960.c             |  41 ++---
>  drivers/thermal/qcom/tsens-8974.c             |  12 +-
>  drivers/thermal/qcom/tsens-common.c           |  62 ++++---
>  drivers/thermal/qcom/tsens-v2.c               |   8 +-
>  drivers/thermal/qcom/tsens.c                  |  19 +-
>  drivers/thermal/qcom/tsens.h                  |  23 +--
>  12 files changed, 309 insertions(+), 100 deletions(-)
>
> --
> 2.17.1
>
Andy Gross Sept. 20, 2018, 6:57 p.m. UTC | #2
On Wed, Sep 12, 2018 at 03:22:54PM +0530, Amit Kucheria wrote:
> We've earlier added support to split the register address space into TM
> and SROT regions. Split up the regmap address space into two for msm8974
> that has a similar register layout.
> 
> Since tsens-common.c/init_common() currently only registers one address
> space, the order is important (TM before SROT).  This is OK since the
> code doesn't really use the SROT functionality yet.
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> ---

Acked-by: Andy Gross <andy.gross@linaro.org>
Andy Gross Sept. 20, 2018, 6:57 p.m. UTC | #3
On Wed, Sep 12, 2018 at 03:22:55PM +0530, Amit Kucheria wrote:
> We've earlier added support to split the register address space into TM
> and SROT regions. Split up the regmap address space into two for msm8916
> that has a similar register layout.
> 
> Since tsens-common.c/init_common() currently only registers one address
> space, the order is important (TM before SROT).  This is OK since the
> code doesn't really use the SROT functionality yet.
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> ---

Acked-by: Andy Gross <andy.gross@linaro.org>
Andy Gross Sept. 20, 2018, 6:58 p.m. UTC | #4
On Wed, Sep 12, 2018 at 03:22:56PM +0530, Amit Kucheria wrote:
> This new property allows the number of sensors to be configured from DT
> instead of being hardcoded in platform data. Use it.
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  arch/arm/boot/dts/qcom-msm8974.dtsi | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
> index 56dbbf788d15..3c4b81c29798 100644
> --- a/arch/arm/boot/dts/qcom-msm8974.dtsi

Acked-by: Andy Gross <andy.gross@linaro.org>
Andy Gross Sept. 20, 2018, 6:58 p.m. UTC | #5
On Wed, Sep 12, 2018 at 03:22:57PM +0530, Amit Kucheria wrote:
> This new property allows the number of sensors to be configured from DT
> instead of being hardcoded in platform data. Use it.
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/msm8916.dtsi | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> index 6a277fce3333..be27d8dc9e6b 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi

Acked-by: Andy Gross <andy.gross@linaro.org>
Andy Gross Sept. 20, 2018, 6:59 p.m. UTC | #6
On Wed, Sep 12, 2018 at 03:22:59PM +0530, Amit Kucheria wrote:
> Initialise the gpu thermal zone to export temperature to userspace.
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/msm8916.dtsi | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> index be27d8dc9e6b..62f079ae9ba3 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi

Acked-by: Andy Gross <andy.gross@linaro.org>
Andy Gross Sept. 20, 2018, 7 p.m. UTC | #7
On Wed, Sep 12, 2018 at 03:23:00PM +0530, Amit Kucheria wrote:
> Initialise the camera thermal zone to export temperature to userspace.
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/msm8916.dtsi | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
> index 62f079ae9ba3..3dc8b8aa76c7 100644
> --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi

Acked-by: Andy Gross <andy.gross@linaro.org>
Andy Gross Sept. 20, 2018, 7:01 p.m. UTC | #8
On Wed, Sep 12, 2018 at 03:23:01PM +0530, Amit Kucheria wrote:
> One thermal zone per cpu is defined
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> Tested-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 170 +++++++++++++++++++++++++++
>  1 file changed, 170 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 0c9a2aa6a1b5..eb801922f6bb 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi

Acked-by: Andy Gross <andy.gross@linaro.org>
Andy Gross Sept. 20, 2018, 7:02 p.m. UTC | #9
On Wed, Sep 12, 2018 at 03:22:45PM +0530, Amit Kucheria wrote:
> This is another series of tsens cleanups before we add interrupt support. This applies on top of 4.19-rc2.
> 
> Patches [1-6] can directly be applied by Eduardo.
> Patches [9-16] can directly be applied by Andy.

Eduardo,

I ACKed the DTS patches for this.  Can you take 9-16 along with the 1-6 through
your tree?

Thanks,

Andy
Amit Kucheria Sept. 21, 2018, 5:48 p.m. UTC | #10
On Thu, Sep 20, 2018 at 12:02 PM Andy Gross <andy.gross@linaro.org> wrote:
>
> On Wed, Sep 12, 2018 at 03:22:45PM +0530, Amit Kucheria wrote:
> > This is another series of tsens cleanups before we add interrupt support. This applies on top of 4.19-rc2.
> >
> > Patches [1-6] can directly be applied by Eduardo.
> > Patches [9-16] can directly be applied by Andy.
>
> Eduardo,
>
> I ACKed the DTS patches for this.  Can you take 9-16 along with the 1-6 through
> your tree?
>

Thanks Andy.

Eduardo, I've send a fixup patch 07/16. If you like, I can resend the
entire series
or you can find the entire series hosted with all tags applied here:
https://git.linaro.org/people/amit.kucheria/kernel.git/log/?h=up/thermal/tsens-preirq-cleanup-v4

Regards,
Amit
Matthias Kaehlcke Oct. 25, 2018, 7:25 p.m. UTC | #11
Hi Amit,

vaguely related question, since you are working on SDM845 thermal
stuff: Do you have plans to add CPU cooling devices? I raised the
point during the review of the CPUfreq HW, however there was no
clear answer: https://lore.kernel.org/patchwork/patch/966833/#1164748
The driver is still under development/review, but I wonder what the
path forward is.

Cheers

Matthias

On Wed, Sep 12, 2018 at 03:23:01PM +0530, Amit Kucheria wrote:
> One thermal zone per cpu is defined
> 
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> Tested-by: Matthias Kaehlcke <mka@chromium.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 170 +++++++++++++++++++++++++++
>  1 file changed, 170 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index 0c9a2aa6a1b5..eb801922f6bb 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -1104,4 +1104,174 @@
>  			};
>  		};
>  	};
> +
> +	thermal-zones {
> +		cpu0-thermal {
> +			polling-delay-passive = <250>;
> +			polling-delay = <1000>;
> +
> +			thermal-sensors = <&tsens0 1>;
> +
> +			trips {
> +				cpu_alert0: trip0 {
> +					temperature = <75000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +
> +				cpu_crit0: trip1 {
> +					temperature = <110000>;
> +					hysteresis = <1000>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		cpu1-thermal {
> +			polling-delay-passive = <250>;
> +			polling-delay = <1000>;
> +
> +			thermal-sensors = <&tsens0 2>;
> +
> +			trips {
> +				cpu_alert1: trip0 {
> +					temperature = <75000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +
> +				cpu_crit1: trip1 {
> +					temperature = <110000>;
> +					hysteresis = <1000>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		cpu2-thermal {
> +			polling-delay-passive = <250>;
> +			polling-delay = <1000>;
> +
> +			thermal-sensors = <&tsens0 3>;
> +
> +			trips {
> +				cpu_alert2: trip0 {
> +					temperature = <75000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +
> +				cpu_crit2: trip1 {
> +					temperature = <110000>;
> +					hysteresis = <1000>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		cpu3-thermal {
> +			polling-delay-passive = <250>;
> +			polling-delay = <1000>;
> +
> +			thermal-sensors = <&tsens0 4>;
> +
> +			trips {
> +				cpu_alert3: trip0 {
> +					temperature = <75000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +
> +				cpu_crit3: trip1 {
> +					temperature = <110000>;
> +					hysteresis = <1000>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		cpu4-thermal {
> +			polling-delay-passive = <250>;
> +			polling-delay = <1000>;
> +
> +			thermal-sensors = <&tsens0 7>;
> +
> +			trips {
> +				cpu_alert4: trip0 {
> +					temperature = <75000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +
> +				cpu_crit4: trip1 {
> +					temperature = <110000>;
> +					hysteresis = <1000>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		cpu5-thermal {
> +			polling-delay-passive = <250>;
> +			polling-delay = <1000>;
> +
> +			thermal-sensors = <&tsens0 8>;
> +
> +			trips {
> +				cpu_alert5: trip0 {
> +					temperature = <75000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +
> +				cpu_crit5: trip1 {
> +					temperature = <110000>;
> +					hysteresis = <1000>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		cpu6-thermal {
> +			polling-delay-passive = <250>;
> +			polling-delay = <1000>;
> +
> +			thermal-sensors = <&tsens0 9>;
> +
> +			trips {
> +				cpu_alert6: trip0 {
> +					temperature = <75000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +
> +				cpu_crit6: trip1 {
> +					temperature = <110000>;
> +					hysteresis = <1000>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +
> +		cpu7-thermal {
> +			polling-delay-passive = <250>;
> +			polling-delay = <1000>;
> +
> +			thermal-sensors = <&tsens0 10>;
> +
> +			trips {
> +				cpu_alert7: trip0 {
> +					temperature = <75000>;
> +					hysteresis = <2000>;
> +					type = "passive";
> +				};
> +
> +				cpu_crit7: trip1 {
> +					temperature = <110000>;
> +					hysteresis = <1000>;
> +					type = "critical";
> +				};
> +			};
> +		};
> +	};
>  };
Amit Kucheria Oct. 25, 2018, 8:03 p.m. UTC | #12
On Fri, Oct 26, 2018 at 12:55 AM Matthias Kaehlcke <mka@chromium.org> wrote:
>
> Hi Amit,
>
> vaguely related question, since you are working on SDM845 thermal
> stuff: Do you have plans to add CPU cooling devices? I raised the
> point during the review of the CPUfreq HW, however there was no
> clear answer: https://lore.kernel.org/patchwork/patch/966833/#1164748
> The driver is still under development/review, but I wonder what the
> path forward is.
>

Hi Matthias,

I plan to but the OSM driver doesn't work well enough for me to send
out that patch yet. I responded with my findings on the v9 posting
where it seems to get stuck at the high OPP. It'd be nice if somebody
else could confirm (or refute) this behaviour.

Regards,
Amit
Matthias Kaehlcke Oct. 25, 2018, 8:37 p.m. UTC | #13
On Fri, Oct 26, 2018 at 01:33:40AM +0530, Amit Kucheria wrote:
> On Fri, Oct 26, 2018 at 12:55 AM Matthias Kaehlcke <mka@chromium.org> wrote:
> >
> > Hi Amit,
> >
> > vaguely related question, since you are working on SDM845 thermal
> > stuff: Do you have plans to add CPU cooling devices? I raised the
> > point during the review of the CPUfreq HW, however there was no
> > clear answer: https://lore.kernel.org/patchwork/patch/966833/#1164748
> > The driver is still under development/review, but I wonder what the
> > path forward is.
> >
> 
> Hi Matthias,
> 
> I plan to

Great!

> but the OSM driver doesn't work well enough for me to send
> out that patch yet. I responded with my findings on the v9 posting
> where it seems to get stuck at the high OPP. It'd be nice if somebody
> else could confirm (or refute) this behaviour.

Ok, we aren't using the latest version yet, I'll try to repro and
report back.

Cheers

Matthias