| Message ID | 51491585-16fb-93b1-bb1a-09e89683f2f0@cogentembedded.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Simon Horman |
| Headers | show |
| Series | arm64: dts: renesas: r8a77970: add thermal support | expand |
On 10/05/2018 10:25 PM, Sergei Shtylyov wrote: > Describe THS/CIVM in the R8A77970 device trees. Damn, should be singular "tree"! :-/ > Based on the original (and large) patches by Vladimir Barinov. > > Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com> > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> [...] MBR, Sergei
On Fri, Oct 05, 2018 at 10:25:47PM +0300, Sergei Shtylyov wrote: > Describe THS/CIVM in the R8A77970 device trees. > > Based on the original (and large) patches by Vladimir Barinov. > > Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com> > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > > --- > This patch is against the 'renesas-devel-20181004-v4.19-rc6' tag of Simon > Horman's 'renesas.git' repo. > > The thermal driver/bindings patches have been posted yesterday... > > arch/arm64/boot/dts/renesas/r8a77970.dtsi | 32 ++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > Index: renesas/arch/arm64/boot/dts/renesas/r8a77970.dtsi > =================================================================== > --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77970.dtsi > +++ renesas/arch/arm64/boot/dts/renesas/r8a77970.dtsi > @@ -300,6 +300,19 @@ > #power-domain-cells = <1>; > }; > > + thermal: thermal@e6190000 { > + compatible = "renesas,thermal-r8a77970"; > + reg = <0 0xe6190000 0 0x14 What is the motivation for 0x14, to me 0x10 seems like a more natural size for the register window. Otherwise the patch looks good to me. > + 0 0xe6190100 0 0x38>; > + interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cpg CPG_MOD 522>; > + power-domains = <&sysc R8A77970_PD_ALWAYS_ON>; > + resets = <&cpg 522>; > + #thermal-sensor-cells = <0>; > + }; > + > intc_ex: interrupt-controller@e61c0000 { > compatible = "renesas,intc-ex-r8a77970", "renesas,irqc"; > #interrupt-cells = <2>; > @@ -1033,6 +1046,25 @@ > }; > }; > > + thermal-zones { > + cpu-thermal { > + polling-delay-passive = <250>; > + polling-delay = <1000>; > + thermal-sensors = <&thermal>; > + > + trips { > + cpu-crit { > + temperature = <120000>; > + hysteresis = <2000>; > + type = "critical"; > + }; > + }; > + > + cooling-maps { > + }; > + }; > + }; > + > timer { > compatible = "arm,armv8-timer"; > interrupts-extended = <&gic GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>, >
Hi Sergei, On Fri, Oct 5, 2018 at 9:26 PM Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > Describe THS/CIVM in the R8A77970 device trees. > > Based on the original (and large) patches by Vladimir Barinov. > > Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com> > Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > > --- > This patch is against the 'renesas-devel-20181004-v4.19-rc6' tag of Simon > Horman's 'renesas.git' repo. > > The thermal driver/bindings patches have been posted yesterday... > > arch/arm64/boot/dts/renesas/r8a77970.dtsi | 32 ++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > Index: renesas/arch/arm64/boot/dts/renesas/r8a77970.dtsi > =================================================================== > --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77970.dtsi > +++ renesas/arch/arm64/boot/dts/renesas/r8a77970.dtsi > @@ -300,6 +300,19 @@ > #power-domain-cells = <1>; > }; > > + thermal: thermal@e6190000 { > + compatible = "renesas,thermal-r8a77970"; > + reg = <0 0xe6190000 0 0x14 0x14 was appropriate for R-Mobile APE6... > + 0 0xe6190100 0 0x38>; What about the CIVM status register? DT describes hardware, not driver limitations. > + interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&cpg CPG_MOD 522>; > + power-domains = <&sysc R8A77970_PD_ALWAYS_ON>; > + resets = <&cpg 522>; > + #thermal-sensor-cells = <0>; > + }; > + > intc_ex: interrupt-controller@e61c0000 { > compatible = "renesas,intc-ex-r8a77970", "renesas,irqc"; > #interrupt-cells = <2>; The rest looks good to me, so with the above fixed: Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
On 10/08/2018 11:12 AM, Geert Uytterhoeven wrote: >> Describe THS/CIVM in the R8A77970 device trees. >> >> Based on the original (and large) patches by Vladimir Barinov. >> >> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com> >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >> >> --- >> This patch is against the 'renesas-devel-20181004-v4.19-rc6' tag of Simon >> Horman's 'renesas.git' repo. >> >> The thermal driver/bindings patches have been posted yesterday... >> >> arch/arm64/boot/dts/renesas/r8a77970.dtsi | 32 ++++++++++++++++++++++++++++++ >> 1 file changed, 32 insertions(+) >> >> Index: renesas/arch/arm64/boot/dts/renesas/r8a77970.dtsi >> =================================================================== >> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77970.dtsi >> +++ renesas/arch/arm64/boot/dts/renesas/r8a77970.dtsi >> @@ -300,6 +300,19 @@ >> #power-domain-cells = <1>; >> }; >> >> + thermal: thermal@e6190000 { >> + compatible = "renesas,thermal-r8a77970"; >> + reg = <0 0xe6190000 0 0x14 > > 0x14 was appropriate for R-Mobile APE6... Copy&paste is to blame here, I guess... I'll fix to 0x10. > >> + 0 0xe6190100 0 0x38>; > > What about the CIVM status register? DT describes hardware, not driver > limitations. I wasn't sure whether to put it into a separate "reg" tuple (which would confuse the driver) or not. After looking into the manual again, I'm going to extend the 2nd "reg" tuple... >> + interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>, >> + <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>; >> + clocks = <&cpg CPG_MOD 522>; >> + power-domains = <&sysc R8A77970_PD_ALWAYS_ON>; >> + resets = <&cpg 522>; >> + #thermal-sensor-cells = <0>; >> + }; >> + >> intc_ex: interrupt-controller@e61c0000 { >> compatible = "renesas,intc-ex-r8a77970", "renesas,irqc"; >> #interrupt-cells = <2>; > > The rest looks good to me, so with the above fixed: > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Thanks :-) > Gr{oetje,eeting}s, > > Geert MBR, Sergei
Hi Sergei, On Mon, Oct 8, 2018 at 6:35 PM Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > On 10/08/2018 11:12 AM, Geert Uytterhoeven wrote: > >> Describe THS/CIVM in the R8A77970 device trees. > >> Based on the original (and large) patches by Vladimir Barinov. > >> > >> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com> > >> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > >> > >> --- > >> This patch is against the 'renesas-devel-20181004-v4.19-rc6' tag of Simon > >> Horman's 'renesas.git' repo. > >> > >> The thermal driver/bindings patches have been posted yesterday... > >> > >> arch/arm64/boot/dts/renesas/r8a77970.dtsi | 32 ++++++++++++++++++++++++++++++ > >> 1 file changed, 32 insertions(+) > >> > >> Index: renesas/arch/arm64/boot/dts/renesas/r8a77970.dtsi > >> =================================================================== > >> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77970.dtsi > >> +++ renesas/arch/arm64/boot/dts/renesas/r8a77970.dtsi > >> @@ -300,6 +300,19 @@ > >> #power-domain-cells = <1>; > >> }; > >> > >> + thermal: thermal@e6190000 { > >> + compatible = "renesas,thermal-r8a77970"; > >> + reg = <0 0xe6190000 0 0x14 > > > > 0x14 was appropriate for R-Mobile APE6... > > Copy&paste is to blame here, I guess... I'll fix to 0x10. OK. > >> + 0 0xe6190100 0 0x38>; > > > > What about the CIVM status register? DT describes hardware, not driver > > limitations. > > I wasn't sure whether to put it into a separate "reg" tuple (which would confuse > the driver) or not. After looking into the manual again, I'm going to extend the > 2nd "reg" tuple... The CIVM Status Register indicates the chip internal voltage. As such it's not a per-channel property, and IMHO doesn't belong in the second tuple (e.g. R-Mobile APE6 has 3 channels). Perhaps extending the bindings to handle more reg tuples, possibly using reg-names? Gr{oetje,eeting}s, Geert
Hello! On 10/08/2018 07:40 PM, Geert Uytterhoeven wrote: >>>> Describe THS/CIVM in the R8A77970 device trees. >>>> Based on the original (and large) patches by Vladimir Barinov. >>>> >>>> Signed-off-by: Vladimir Barinov <vladimir.barinov@cogentembedded.com> >>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> >>>> >>>> --- >>>> This patch is against the 'renesas-devel-20181004-v4.19-rc6' tag of Simon >>>> Horman's 'renesas.git' repo. >>>> >>>> The thermal driver/bindings patches have been posted yesterday... >>>> >>>> arch/arm64/boot/dts/renesas/r8a77970.dtsi | 32 ++++++++++++++++++++++++++++++ >>>> 1 file changed, 32 insertions(+) >>>> >>>> Index: renesas/arch/arm64/boot/dts/renesas/r8a77970.dtsi >>>> =================================================================== >>>> --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77970.dtsi >>>> +++ renesas/arch/arm64/boot/dts/renesas/r8a77970.dtsi >>>> @@ -300,6 +300,19 @@ >>>> #power-domain-cells = <1>; >>>> }; >>>> >>>> + thermal: thermal@e6190000 { >>>> + compatible = "renesas,thermal-r8a77970"; >>>> + reg = <0 0xe6190000 0 0x14 >>> >>> 0x14 was appropriate for R-Mobile APE6... >> >> Copy&paste is to blame here, I guess... I'll fix to 0x10. > > OK. > >>>> + 0 0xe6190100 0 0x38>; >>> >>> What about the CIVM status register? DT describes hardware, not driver >>> limitations. >> >> I wasn't sure whether to put it into a separate "reg" tuple (which would confuse >> the driver) or not. After looking into the manual again, I'm going to extend the >> 2nd "reg" tuple... > > The CIVM Status Register indicates the chip internal voltage. > As such it's not a per-channel property, and IMHO doesn't belong in the second > tuple I was looking a the block diagrams (both in the chapters 10A and 10B) and I got a totally different impression... > (e.g. R-Mobile APE6 has 3 channels). The driver handles that alright. It's the adding the CIVM as a separate tuple that would break it. > Perhaps extending the bindings to handle more reg tuples, possibly using > reg-names? You mean teaching the driver about one more kind of "reg" tuples? I would like to avoid that of course -- due to the need to still handle the old DTs as well... > Gr{oetje,eeting}s, > > Geert > MBR, Sergei
Hi Sergei, On Mon, Oct 8, 2018 at 8:04 PM Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > On 10/08/2018 07:40 PM, Geert Uytterhoeven wrote: > >>>> Describe THS/CIVM in the R8A77970 device trees. > >>>> Based on the original (and large) patches by Vladimir Barinov. > >>>> + 0 0xe6190100 0 0x38>; > >>> > >>> What about the CIVM status register? DT describes hardware, not driver > >>> limitations. > >> > >> I wasn't sure whether to put it into a separate "reg" tuple (which would confuse > >> the driver) or not. After looking into the manual again, I'm going to extend the > >> 2nd "reg" tuple... > > > > The CIVM Status Register indicates the chip internal voltage. > > As such it's not a per-channel property, and IMHO doesn't belong in the second > > tuple > > I was looking a the block diagrams (both in the chapters 10A and 10B) and I got a totally different impression... I stand corrected. So extending the second reg block is fine. Gr{oetje,eeting}s, Geert
Index: renesas/arch/arm64/boot/dts/renesas/r8a77970.dtsi =================================================================== --- renesas.orig/arch/arm64/boot/dts/renesas/r8a77970.dtsi +++ renesas/arch/arm64/boot/dts/renesas/r8a77970.dtsi @@ -300,6 +300,19 @@ #power-domain-cells = <1>; }; + thermal: thermal@e6190000 { + compatible = "renesas,thermal-r8a77970"; + reg = <0 0xe6190000 0 0x14 + 0 0xe6190100 0 0x38>; + interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&cpg CPG_MOD 522>; + power-domains = <&sysc R8A77970_PD_ALWAYS_ON>; + resets = <&cpg 522>; + #thermal-sensor-cells = <0>; + }; + intc_ex: interrupt-controller@e61c0000 { compatible = "renesas,intc-ex-r8a77970", "renesas,irqc"; #interrupt-cells = <2>; @@ -1033,6 +1046,25 @@ }; }; + thermal-zones { + cpu-thermal { + polling-delay-passive = <250>; + polling-delay = <1000>; + thermal-sensors = <&thermal>; + + trips { + cpu-crit { + temperature = <120000>; + hysteresis = <2000>; + type = "critical"; + }; + }; + + cooling-maps { + }; + }; + }; + timer { compatible = "arm,armv8-timer"; interrupts-extended = <&gic GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(2) | IRQ_TYPE_LEVEL_LOW)>,