| Message ID | 20190806130506.8753-1-glaroque@baylibre.com |
|---|---|
| Headers | show |
| Series | Add support of New Amlogic temperature sensor for G12 SoCs | expand |
On Tue, Aug 6, 2019 at 3:06 PM Guillaume La Roque <glaroque@baylibre.com> wrote: > > Add minimal thermal zone for two temperature sensor > One is located close to the DDR and the other one is > located close to the PLLs (between the CPU and GPU) > > Signed-off-by: Guillaume La Roque <glaroque@baylibre.com> I'm not familiar with the thermal subsystem but this looks sane so: Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
On Tue, Aug 6, 2019 at 3:06 PM Guillaume La Roque <glaroque@baylibre.com> wrote: > > Add minimal thermal zone for two temperature sensor > One is located close to the DDR and the other one is > located close to the PLLs (between the CPU and GPU) > > Signed-off-by: Guillaume La Roque <glaroque@baylibre.com> I'm not familiar with the thermal subsystem but this looks sane so: Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Guillaume La Roque <glaroque@baylibre.com> writes: > This patchs series add support of New Amlogic temperature sensor and minimal > thermal zone for SEI510 and ODROID-N2 boards. > > First implementation was doing on IIO[1] but after comments i move on thermal framework. > Formulas and calibration values come from amlogic. > > Changes since v2: > - fix yaml documention > - remove unneeded status variable for temperature-sensor node > - rework driver after Martin review > - add some information in commit message > > Changes since v1: > - fix enum vs const in documentation > - fix error with thermal-sensor-cells value set to 1 instead of 0 > - add some dependencies needed to add cooling-maps > > Dependencies : > - patch 3,4 & 5: depends on Neil's patch and series : > - missing dwc2 phy-names[2] > - patchsets to add DVFS on G12a[3] which have deps on [4] and [5] > > [1] https://lore.kernel.org/linux-amlogic/20190604144714.2009-1-glaroque@baylibre.com/ > [2] https://lore.kernel.org/linux-amlogic/20190625123647.26117-1-narmstrong@baylibre.com/ > [3] https://lore.kernel.org/linux-amlogic/20190729132622.7566-1-narmstrong@baylibre.com/ > [4] https://lore.kernel.org/linux-amlogic/20190731084019.8451-5-narmstrong@baylibre.com/ > [5] https://lore.kernel.org/linux-amlogic/20190729132622.7566-3-narmstrong@baylibre.com/ Thank you for the detailed list of dependencies! Much appreciated. With all the deps, I tested this on sei510 and odroid-n2, and basic functionality seems to work. As discussed off-list: it would be nice to have an example of how cpufreq could be used as a cooling device for hot temperatures. The vendor kernel has some trip points that could be included as examples, or even included as extra patches. Also the driver patch is missing the two main thermal maintainers, so please resend at least the driver and bindings including them. Kevin
Hi Guillaume, [...] > +struct amlogic_thermal { > + struct platform_device *pdev; > + const struct amlogic_thermal_data *data; > + struct regmap *regmap; > + struct regmap *sec_ao_map; > + struct clk *clk; > + struct thermal_zone_device *tzd; > + u32 trim_info; > + void __iomem *base; nit-pick: this is only used in _probe() so you could make it a local variable there [...] > +static const struct of_device_id of_amlogic_thermal_match[] = { > + { > + .compatible = "amlogic,g12-ddr-thermal", > + .data = &amlogic_thermal_g12_ddr_param, > + }, > + { > + .compatible = "amlogic,g12-cpu-thermal", > + .data = &amlogic_thermal_g12_cpu_param, > + }, I assume you are using "g12" to indicate that it's valid for both, G12A and G12B? meson-g12-common.dtsi currently does not use any other "amlogic,g12-*" compatible string (there are some meson-axg-*, meson-gx-* and meson-g12a-* ones, but no g12-*) I would like to hear Kevin's and Neil's opinion on this one whether we should introduce that "amlogic,g12-*" prefix or stick to "amlogic,g12a-*" [...] > + ret = amlogic_thermal_enable(pdata); > + if (ret) > + clk_disable_unprepare(pdata->clk); amlogic_thermal_enable only returns an error-code if clk_prepare_enable() fails in that case the clock is neither prepared nor enabled so we must not call clk_disable_unprepare apart from that it looks good to me (as someone who doesn't know the thermal framework) Martin
On 6 Aug 2019, at 5:05 pm, Guillaume La Roque <glaroque@baylibre.com> wrote: > > This patchs series add support of New Amlogic temperature sensor and minimal > thermal zone for SEI510 and ODROID-N2 boards. > > First implementation was doing on IIO[1] but after comments i move on thermal framework. > Formulas and calibration values come from amlogic. > > Changes since v2: > - fix yaml documention > - remove unneeded status variable for temperature-sensor node > - rework driver after Martin review > - add some information in commit message > > Changes since v1: > - fix enum vs const in documentation > - fix error with thermal-sensor-cells value set to 1 instead of 0 > - add some dependencies needed to add cooling-maps > > Dependencies : > - patch 3,4 & 5: depends on Neil's patch and series : > - missing dwc2 phy-names[2] > - patchsets to add DVFS on G12a[3] which have deps on [4] and [5] > > [1] https://lore.kernel.org/linux-amlogic/20190604144714.2009-1-glaroque@baylibre.com/ > [2] https://lore.kernel.org/linux-amlogic/20190625123647.26117-1-narmstrong@baylibre.com/ > [3] https://lore.kernel.org/linux-amlogic/20190729132622.7566-1-narmstrong@baylibre.com/ > [4] https://lore.kernel.org/linux-amlogic/20190731084019.8451-5-narmstrong@baylibre.com/ > [5] https://lore.kernel.org/linux-amlogic/20190729132622.7566-3-narmstrong@baylibre.com/ > > Guillaume La Roque (6): > dt-bindings: thermal: Add DT bindings documentation for Amlogic > Thermal > thermal: amlogic: Add thermal driver to support G12 SoCs > arm64: dts: amlogic: g12: add temperature sensor > arm64: dts: meson: sei510: Add minimal thermal zone > arm64: dts: amlogic: odroid-n2: add minimal thermal zone > MAINTAINERS: add entry for Amlogic Thermal driver Tested-by: Christian Hewitt <christianshewitt@gmail.com> I’ve tested this series with Odroid N2 and Khadas VIM3, X96-Max. Patches to add support for VIM3/X96-max will be submitted once the driver is merged. VIM3:~ # dmesg | grep thermal [ 0.046375] thermal_sys: Registered thermal governor 'step_wise' VIM3:~ # cat /sys/devices/virtual/thermal/thermal_zone0/temp 51300 VIM3:~ # cat /sys/devices/virtual/thermal/thermal_zone1/temp 52800 Christian
Hi Christian, thanks for testing on this 3 boards. guillaume On 8/14/19 2:24 PM, Christian Hewitt wrote: > On 6 Aug 2019, at 5:05 pm, Guillaume La Roque <glaroque@baylibre.com> wrote: >> This patchs series add support of New Amlogic temperature sensor and minimal >> thermal zone for SEI510 and ODROID-N2 boards. >> >> First implementation was doing on IIO[1] but after comments i move on thermal framework. >> Formulas and calibration values come from amlogic. >> >> Changes since v2: >> - fix yaml documention >> - remove unneeded status variable for temperature-sensor node >> - rework driver after Martin review >> - add some information in commit message >> >> Changes since v1: >> - fix enum vs const in documentation >> - fix error with thermal-sensor-cells value set to 1 instead of 0 >> - add some dependencies needed to add cooling-maps >> >> Dependencies : >> - patch 3,4 & 5: depends on Neil's patch and series : >> - missing dwc2 phy-names[2] >> - patchsets to add DVFS on G12a[3] which have deps on [4] and [5] >> >> [1] https://lore.kernel.org/linux-amlogic/20190604144714.2009-1-glaroque@baylibre.com/ >> [2] https://lore.kernel.org/linux-amlogic/20190625123647.26117-1-narmstrong@baylibre.com/ >> [3] https://lore.kernel.org/linux-amlogic/20190729132622.7566-1-narmstrong@baylibre.com/ >> [4] https://lore.kernel.org/linux-amlogic/20190731084019.8451-5-narmstrong@baylibre.com/ >> [5] https://lore.kernel.org/linux-amlogic/20190729132622.7566-3-narmstrong@baylibre.com/ >> >> Guillaume La Roque (6): >> dt-bindings: thermal: Add DT bindings documentation for Amlogic >> Thermal >> thermal: amlogic: Add thermal driver to support G12 SoCs >> arm64: dts: amlogic: g12: add temperature sensor >> arm64: dts: meson: sei510: Add minimal thermal zone >> arm64: dts: amlogic: odroid-n2: add minimal thermal zone >> MAINTAINERS: add entry for Amlogic Thermal driver > Tested-by: Christian Hewitt <christianshewitt@gmail.com> > > I’ve tested this series with Odroid N2 and Khadas VIM3, X96-Max. Patches to add > support for VIM3/X96-max will be submitted once the driver is merged. > > VIM3:~ # dmesg | grep thermal > [ 0.046375] thermal_sys: Registered thermal governor 'step_wise' > > VIM3:~ # cat /sys/devices/virtual/thermal/thermal_zone0/temp > 51300 > > VIM3:~ # cat /sys/devices/virtual/thermal/thermal_zone1/temp > 52800 > > Christian
Kevin Hilman <khilman@baylibre.com> writes: > Guillaume La Roque <glaroque@baylibre.com> writes: > >> This patchs series add support of New Amlogic temperature sensor and minimal >> thermal zone for SEI510 and ODROID-N2 boards. >> >> First implementation was doing on IIO[1] but after comments i move on thermal framework. >> Formulas and calibration values come from amlogic. >> >> Changes since v2: >> - fix yaml documention >> - remove unneeded status variable for temperature-sensor node >> - rework driver after Martin review >> - add some information in commit message >> >> Changes since v1: >> - fix enum vs const in documentation >> - fix error with thermal-sensor-cells value set to 1 instead of 0 >> - add some dependencies needed to add cooling-maps >> >> Dependencies : >> - patch 3,4 & 5: depends on Neil's patch and series : >> - missing dwc2 phy-names[2] >> - patchsets to add DVFS on G12a[3] which have deps on [4] and [5] >> >> [1] https://lore.kernel.org/linux-amlogic/20190604144714.2009-1-glaroque@baylibre.com/ >> [2] https://lore.kernel.org/linux-amlogic/20190625123647.26117-1-narmstrong@baylibre.com/ >> [3] https://lore.kernel.org/linux-amlogic/20190729132622.7566-1-narmstrong@baylibre.com/ >> [4] https://lore.kernel.org/linux-amlogic/20190731084019.8451-5-narmstrong@baylibre.com/ >> [5] https://lore.kernel.org/linux-amlogic/20190729132622.7566-3-narmstrong@baylibre.com/ > > Thank you for the detailed list of dependencies! Much appreciated. > > With all the deps, I tested this on sei510 and odroid-n2, and basic > functionality seems to work. > > As discussed off-list: it would be nice to have an example of how > cpufreq could be used as a cooling device for hot temperatures. The > vendor kernel has some trip points that could be included as examples, > or even included as extra patches. > > Also the driver patch is missing the two main thermal maintainers, so > please resend at least the driver and bindings including them. Forgot to add... Tested-by: Kevin Hilman <khilman@baylibre.com>
This patchs series add support of New Amlogic temperature sensor and minimal thermal zone for SEI510 and ODROID-N2 boards. First implementation was doing on IIO[1] but after comments i move on thermal framework. Formulas and calibration values come from amlogic. Changes since v2: - fix yaml documention - remove unneeded status variable for temperature-sensor node - rework driver after Martin review - add some information in commit message Changes since v1: - fix enum vs const in documentation - fix error with thermal-sensor-cells value set to 1 instead of 0 - add some dependencies needed to add cooling-maps Dependencies : - patch 3,4 & 5: depends on Neil's patch and series : - missing dwc2 phy-names[2] - patchsets to add DVFS on G12a[3] which have deps on [4] and [5] [1] https://lore.kernel.org/linux-amlogic/20190604144714.2009-1-glaroque@baylibre.com/ [2] https://lore.kernel.org/linux-amlogic/20190625123647.26117-1-narmstrong@baylibre.com/ [3] https://lore.kernel.org/linux-amlogic/20190729132622.7566-1-narmstrong@baylibre.com/ [4] https://lore.kernel.org/linux-amlogic/20190731084019.8451-5-narmstrong@baylibre.com/ [5] https://lore.kernel.org/linux-amlogic/20190729132622.7566-3-narmstrong@baylibre.com/ Guillaume La Roque (6): dt-bindings: thermal: Add DT bindings documentation for Amlogic Thermal thermal: amlogic: Add thermal driver to support G12 SoCs arm64: dts: amlogic: g12: add temperature sensor arm64: dts: meson: sei510: Add minimal thermal zone arm64: dts: amlogic: odroid-n2: add minimal thermal zone MAINTAINERS: add entry for Amlogic Thermal driver .../bindings/thermal/amlogic,thermal.yaml | 54 +++ MAINTAINERS | 9 + .../boot/dts/amlogic/meson-g12-common.dtsi | 20 ++ .../boot/dts/amlogic/meson-g12a-sei510.dts | 56 +++ .../boot/dts/amlogic/meson-g12b-odroid-n2.dts | 60 ++++ drivers/thermal/Kconfig | 11 + drivers/thermal/Makefile | 1 + drivers/thermal/amlogic_thermal.c | 336 ++++++++++++++++++ 8 files changed, 547 insertions(+) create mode 100644 Documentation/devicetree/bindings/thermal/amlogic,thermal.yaml create mode 100644 drivers/thermal/amlogic_thermal.c