| Message ID | cover.1550493113.git.amit.kucheria@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | qcom: dts: thermal cleanups | expand |
Hey On Mon, Feb 18, 2019 at 06:05:14PM +0530, Amit Kucheria wrote: > - Expose all temperature sensors on msm8916, msm996, msm8998, sdm845 > - split up the register address map for msm8998 > - standardize names of the various thermal-zones across boards to make it > easy for test scripts to parse > I am generally fine with the effort but please fix the following (applies for the whole series) wrt to required properties for DT thermal: a. Trip points for your zones b. Cooling Mappings for zones that have passive trips. > Amit Kucheria (12): > arm64: dts: msm8998: thermal: split address space into two > arm64: dts: msm8998: efficiency is not valid property > arm64: dts: msm8916: thermal: Add sensor for modem > arm64: dts: msm8996: thermal: Add temperature sensors near major > peripherals > arm64: dts: msm8998: thermal: Fix the cpu sensor numbers > arm64: dts: msm8998: thermal: Fix the gpu sensor number > arm64: dts: msm8998: thermal: GPU has two sensors, add the second > arm64: dts: msm8998: thermal: Add temperature sensors near major > peripherals > arm64: dts: sdm845: thermal: Add temperature sensors near major > peripherals > arm64: dts: msm8998: thermal: Make trip names consistent > arm64: dts: msm8916: thermal: Make trip names consistent > arm64: dts: msm8996: thermal: Make trip names consistent > > arch/arm64/boot/dts/qcom/msm8916.dtsi | 30 +++-- > arch/arm64/boot/dts/qcom/msm8996.dtsi | 122 ++++++++++++++++++-- > arch/arm64/boot/dts/qcom/msm8998.dtsi | 154 ++++++++++++++++++-------- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 91 +++++++++++++++ > 4 files changed, 328 insertions(+), 69 deletions(-) > > -- > 2.17.1 >
On Wed, Feb 20, 2019 at 6:56 AM Eduardo Valentin <edubezval@gmail.com> wrote: > > Hey > On Mon, Feb 18, 2019 at 06:05:14PM +0530, Amit Kucheria wrote: > > - Expose all temperature sensors on msm8916, msm996, msm8998, sdm845 > > - split up the register address map for msm8998 > > - standardize names of the various thermal-zones across boards to make it > > easy for test scripts to parse > > > > I am generally fine with the effort but please fix the following > (applies for the whole series) wrt to required properties for DT > thermal: > a. Trip points for your zones Thanks for the review. In some cases, the temperatures are just exposed so something in userspace might read it and do something with it. We don't expect kernel trips for them. Adding trip points also requires me to add cooling-maps (your point b. below). I guess I'm looking for an example of how to just expose sensor temperatures w/o any associated trips and cooling maps. > b. Cooling Mappings for zones that have passive trips. > From what I can see currently only CPUs and GPUs (among the major heat sources) can passively reduce heat by reducing frequencies. Things like cameras, display, video might have a more ON/OFF approach to throttling that might be controlled from userspace. And we don't have a way to tell in DT that these zones are managed in userspace (https://patchwork.kernel.org/patch/10259487/)
On Wed, Feb 20, 2019 at 03:09:36PM +0530, Amit Kucheria wrote: > On Wed, Feb 20, 2019 at 6:56 AM Eduardo Valentin <edubezval@gmail.com> wrote: > > > > Hey > > On Mon, Feb 18, 2019 at 06:05:14PM +0530, Amit Kucheria wrote: > > > - Expose all temperature sensors on msm8916, msm996, msm8998, sdm845 > > > - split up the register address map for msm8998 > > > - standardize names of the various thermal-zones across boards to make it > > > easy for test scripts to parse > > > > > > > I am generally fine with the effort but please fix the following > > (applies for the whole series) wrt to required properties for DT > > thermal: > > a. Trip points for your zones > > Thanks for the review. > > In some cases, the temperatures are just exposed so something in > userspace might read it and do something with it. We don't expect > kernel trips for them. Would a hwmon driver make more sense here? > > Adding trip points also requires me to add cooling-maps (your point b. below). Yes. > > I guess I'm looking for an example of how to just expose sensor > temperatures w/o any associated trips and cooling maps. > > > b. Cooling Mappings for zones that have passive trips. > > > > From what I can see currently only CPUs and GPUs (among the major heat > sources) can passively reduce heat by reducing frequencies. > > Things like cameras, display, video might have a more ON/OFF approach > to throttling that might be controlled from userspace. And we don't > have a way to tell in DT that these zones are managed in userspace You can always add a Hot trip point. To my understanding that is for notifying userspace. > (https://patchwork.kernel.org/patch/10259487/)
- Expose all temperature sensors on msm8916, msm996, msm8998, sdm845 - split up the register address map for msm8998 - standardize names of the various thermal-zones across boards to make it easy for test scripts to parse Amit Kucheria (12): arm64: dts: msm8998: thermal: split address space into two arm64: dts: msm8998: efficiency is not valid property arm64: dts: msm8916: thermal: Add sensor for modem arm64: dts: msm8996: thermal: Add temperature sensors near major peripherals arm64: dts: msm8998: thermal: Fix the cpu sensor numbers arm64: dts: msm8998: thermal: Fix the gpu sensor number arm64: dts: msm8998: thermal: GPU has two sensors, add the second arm64: dts: msm8998: thermal: Add temperature sensors near major peripherals arm64: dts: sdm845: thermal: Add temperature sensors near major peripherals arm64: dts: msm8998: thermal: Make trip names consistent arm64: dts: msm8916: thermal: Make trip names consistent arm64: dts: msm8996: thermal: Make trip names consistent arch/arm64/boot/dts/qcom/msm8916.dtsi | 30 +++-- arch/arm64/boot/dts/qcom/msm8996.dtsi | 122 ++++++++++++++++++-- arch/arm64/boot/dts/qcom/msm8998.dtsi | 154 ++++++++++++++++++-------- arch/arm64/boot/dts/qcom/sdm845.dtsi | 91 +++++++++++++++ 4 files changed, 328 insertions(+), 69 deletions(-)