| Message ID | 1486928328-25870-2-git-send-email-stefan.wahren@i2se.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Eduardo Valentin |
| Headers | show |
On Sun, Feb 12, 2017 at 07:38:48PM +0000, Stefan Wahren wrote: > As suggested by Eduardo Valentin this adds the thermal zone for > the bcm2835 SoC with its single thermal sensor. We start with > the criticial trip point and leave the cooling devices empty > since we don't have any at the moment. > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> > --- > > Changes in V2: > - add missing thermal-sensor-cells property > - change gpu-thermal to cpu-thermal > > arch/arm/boot/dts/bcm283x.dtsi | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi > index a3106aa..4dc74f6 100644 > --- a/arch/arm/boot/dts/bcm283x.dtsi > +++ b/arch/arm/boot/dts/bcm283x.dtsi > @@ -19,6 +19,25 @@ > bootargs = "earlyprintk console=ttyAMA0"; > }; > > + thermal-zones { > + cpu_thermal: cpu-thermal { > + polling-delay-passive = <0>; > + polling-delay = <1000>; > + Check the diff I sent and also add the following for differentiating the offsets and slopes depending on which chip the zone describes: coefficients = <-538 407000>; /* for the zone on bcm2835 and bcm2836 */ and coefficients = <-538 412000>; /* for the zone on bcm2837 */ Despite the changes mentioned for the driver and DT, I am ok with the driver and the DTS descriptors. BR,
Hi Eduardo, > Eduardo Valentin <edubezval@gmail.com> hat am 19. Februar 2017 um 02:17 geschrieben: > > > On Sun, Feb 12, 2017 at 07:38:48PM +0000, Stefan Wahren wrote: > > As suggested by Eduardo Valentin this adds the thermal zone for > > the bcm2835 SoC with its single thermal sensor. We start with > > the criticial trip point and leave the cooling devices empty > > since we don't have any at the moment. > > > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> > > --- > > > > Changes in V2: > > - add missing thermal-sensor-cells property > > - change gpu-thermal to cpu-thermal > > > > arch/arm/boot/dts/bcm283x.dtsi | 20 ++++++++++++++++++++ > > 1 file changed, 20 insertions(+) > > > > diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi > > index a3106aa..4dc74f6 100644 > > --- a/arch/arm/boot/dts/bcm283x.dtsi > > +++ b/arch/arm/boot/dts/bcm283x.dtsi > > @@ -19,6 +19,25 @@ > > bootargs = "earlyprintk console=ttyAMA0"; > > }; > > > > + thermal-zones { > > + cpu_thermal: cpu-thermal { > > + polling-delay-passive = <0>; > > + polling-delay = <1000>; > > + > > Check the diff I sent and also add the following for differentiating the > offsets and slopes depending on which chip the zone describes: > coefficients = <-538 407000>; /* for the zone on bcm2835 and bcm2836 */ > > and > > coefficients = <-538 412000>; /* for the zone on bcm2837 */ > > > Despite the changes mentioned for the driver and DT, I am ok with the driver and the DTS descriptors. thanks for providing the necessary driver changes, but the coefficients above causes a DTC parse error. The Device Tree doesn't provide support for native signed integer. Looking at this old thread [1] suggests to add parentheses which fixed the parse issue. But of-thermal expected u32 for coefficients [2]. Any suggestions? [1] - http://lists.infradead.org/pipermail/linux-arm-kernel/2013-April/159681.html [2] - http://elixir.free-electrons.com/source/drivers/thermal/of-thermal.c?v=4.10-rc7#L854 > > BR,
Hey Stefan, On Sun, Feb 19, 2017 at 01:31:49PM +0100, Stefan Wahren wrote: > Hi Eduardo, > > > Eduardo Valentin <edubezval@gmail.com> hat am 19. Februar 2017 um 02:17 geschrieben: > > > > > > On Sun, Feb 12, 2017 at 07:38:48PM +0000, Stefan Wahren wrote: > > > As suggested by Eduardo Valentin this adds the thermal zone for > > > the bcm2835 SoC with its single thermal sensor. We start with > > > the criticial trip point and leave the cooling devices empty > > > since we don't have any at the moment. > > > > > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> > > > --- > > > > > > Changes in V2: > > > - add missing thermal-sensor-cells property > > > - change gpu-thermal to cpu-thermal > > > > > > arch/arm/boot/dts/bcm283x.dtsi | 20 ++++++++++++++++++++ > > > 1 file changed, 20 insertions(+) > > > > > > diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi > > > index a3106aa..4dc74f6 100644 > > > --- a/arch/arm/boot/dts/bcm283x.dtsi > > > +++ b/arch/arm/boot/dts/bcm283x.dtsi > > > @@ -19,6 +19,25 @@ > > > bootargs = "earlyprintk console=ttyAMA0"; > > > }; > > > > > > + thermal-zones { > > > + cpu_thermal: cpu-thermal { > > > + polling-delay-passive = <0>; > > > + polling-delay = <1000>; > > > + > > > > Check the diff I sent and also add the following for differentiating the > > offsets and slopes depending on which chip the zone describes: > > coefficients = <-538 407000>; /* for the zone on bcm2835 and bcm2836 */ > > > > and > > > > coefficients = <-538 412000>; /* for the zone on bcm2837 */ > > > > > > Despite the changes mentioned for the driver and DT, I am ok with the driver and the DTS descriptors. > > thanks for providing the necessary driver changes, but the coefficients above causes a DTC parse error. The Device Tree doesn't provide support for native signed integer. Looking at this old thread [1] suggests to add parentheses which fixed the parse issue. But of-thermal expected u32 for coefficients [2]. > > Any suggestions? I am OK if you provide a patch to of-thermal in your series, assuming that would fix the representation issue the data of your driver has. BR, > > [1] - http://lists.infradead.org/pipermail/linux-arm-kernel/2013-April/159681.html > [2] - http://elixir.free-electrons.com/source/drivers/thermal/of-thermal.c?v=4.10-rc7#L854 > > > > > BR,
> Eduardo Valentin <edubezval@gmail.com> hat am 19. Februar 2017 um 22:27 geschrieben: > > > Hey Stefan, > > > On Sun, Feb 19, 2017 at 01:31:49PM +0100, Stefan Wahren wrote: > > Hi Eduardo, > > > > > Eduardo Valentin <edubezval@gmail.com> hat am 19. Februar 2017 um 02:17 geschrieben: > > > > > > > > > On Sun, Feb 12, 2017 at 07:38:48PM +0000, Stefan Wahren wrote: > > > > As suggested by Eduardo Valentin this adds the thermal zone for > > > > the bcm2835 SoC with its single thermal sensor. We start with > > > > the criticial trip point and leave the cooling devices empty > > > > since we don't have any at the moment. > > > > > > > > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> > > > > --- > > > > > > > > Changes in V2: > > > > - add missing thermal-sensor-cells property > > > > - change gpu-thermal to cpu-thermal > > > > > > > > arch/arm/boot/dts/bcm283x.dtsi | 20 ++++++++++++++++++++ > > > > 1 file changed, 20 insertions(+) > > > > > > > > diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi > > > > index a3106aa..4dc74f6 100644 > > > > --- a/arch/arm/boot/dts/bcm283x.dtsi > > > > +++ b/arch/arm/boot/dts/bcm283x.dtsi > > > > @@ -19,6 +19,25 @@ > > > > bootargs = "earlyprintk console=ttyAMA0"; > > > > }; > > > > > > > > + thermal-zones { > > > > + cpu_thermal: cpu-thermal { > > > > + polling-delay-passive = <0>; > > > > + polling-delay = <1000>; > > > > + > > > > > > Check the diff I sent and also add the following for differentiating the > > > offsets and slopes depending on which chip the zone describes: > > > coefficients = <-538 407000>; /* for the zone on bcm2835 and bcm2836 */ > > > > > > and > > > > > > coefficients = <-538 412000>; /* for the zone on bcm2837 */ > > > > > > > > > Despite the changes mentioned for the driver and DT, I am ok with the driver and the DTS descriptors. > > > > thanks for providing the necessary driver changes, but the coefficients above causes a DTC parse error. The Device Tree doesn't provide support for native signed integer. Looking at this old thread [1] suggests to add parentheses which fixed the parse issue. But of-thermal expected u32 for coefficients [2]. > > > > Any suggestions? > > I am OK if you provide a patch to of-thermal in your series, assuming > that would fix the representation issue the data of your driver has. > I prepared a new patch series to fix that issue in my github repo [1]. At first we need to implement a new function of_property_read_s32_array() [2]. After that we could use this function in of-thermal in order to use signed integer for both coefficients [3]. @Eduardo: Is it okay for you? @Martin: Do you want to send the next version of the patch series or should i? [1] - https://github.com/lategoodbye/rpi-zero/commits/thermal [2] - https://github.com/lategoodbye/rpi-zero/commit/3dc43581ada74d1345f79b4c36562fdf5f7941e5 [3] - https://github.com/lategoodbye/rpi-zero/commit/c02ed37c0ebdca98aee570862af10e90b3c9a0d0 > BR, > > > > > [1] - http://lists.infradead.org/pipermail/linux-arm-kernel/2013-April/159681.html > > [2] - http://elixir.free-electrons.com/source/drivers/thermal/of-thermal.c?v=4.10-rc7#L854 > > > > > > > > BR,
Hey, On Tue, Feb 21, 2017 at 07:14:36PM +0100, Stefan Wahren wrote: > > <cut> > > I prepared a new patch series to fix that issue in my github repo [1]. At first we need to implement a new function of_property_read_s32_array() [2]. After that we could use this function in of-thermal in order to use signed integer for both coefficients [3]. > > @Eduardo: Is it okay for you? > Yes, as I mentioned before, I am OK to patch of-thermal. And the representation of offset and slope are already signed. > @Martin: Do you want to send the next version of the patch series or should i? > > [1] - https://github.com/lategoodbye/rpi-zero/commits/thermal > [2] - https://github.com/lategoodbye/rpi-zero/commit/3dc43581ada74d1345f79b4c36562fdf5f7941e5 > [3] - https://github.com/lategoodbye/rpi-zero/commit/c02ed37c0ebdca98aee570862af10e90b3c9a0d0 > Number 3 is fine with me. Please send them in a single series for proper review in linux-pm. BR,
diff --git a/arch/arm/boot/dts/bcm283x.dtsi b/arch/arm/boot/dts/bcm283x.dtsi index a3106aa..4dc74f6 100644 --- a/arch/arm/boot/dts/bcm283x.dtsi +++ b/arch/arm/boot/dts/bcm283x.dtsi @@ -19,6 +19,25 @@ bootargs = "earlyprintk console=ttyAMA0"; }; + thermal-zones { + cpu_thermal: cpu-thermal { + polling-delay-passive = <0>; + polling-delay = <1000>; + + thermal-sensors = <&thermal>; + + trips { + cpu-crit { + temperature = <80000>; + hysteresis = <0>; + type = "critical"; + }; + }; + cooling-maps { + }; + }; + }; + soc { compatible = "simple-bus"; #address-cells = <1>; @@ -394,6 +413,7 @@ compatible = "brcm,bcm2835-thermal"; reg = <0x7e212000 0x8>; clocks = <&clocks BCM2835_CLOCK_TSENS>; + #thermal-sensor-cells = <0>; status = "disabled"; };
As suggested by Eduardo Valentin this adds the thermal zone for the bcm2835 SoC with its single thermal sensor. We start with the criticial trip point and leave the cooling devices empty since we don't have any at the moment. Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com> --- Changes in V2: - add missing thermal-sensor-cells property - change gpu-thermal to cpu-thermal arch/arm/boot/dts/bcm283x.dtsi | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)