On Sat, Sep 14, 2019 at 4:25 AM H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> wrote: > > > > Am 13.09.2019 um 17:37 schrieb Adam Ford <aford173@xxxxxxxxx>: > > > > Because the omap34xx, omap36xx and am3517 SoC's have the same > > thermal junction limits, there is no need to duplicate the entry > > multiple times. > > > > This patch removes the thermal references from omap36xx and > > omap34xx and pushes it into the common omap3.dtsi file with > > the added benefit of enabling the thermal info on the AM3517. > > > > Signed-off-by: Adam Ford <aford173@xxxxxxxxx> Disregard this patch. I'll drop it based on Nikolaus' comments below. > > --- > > V2: Add node name for cpu and add cooling-cells entry > > > > diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi > > index 4043ecb38016..84704eb3b604 100644 > > --- a/arch/arm/boot/dts/omap3.dtsi > > +++ b/arch/arm/boot/dts/omap3.dtsi > > @@ -32,7 +32,7 @@ > > #address-cells = <1>; > > #size-cells = <0>; > > > > - cpu@0 { > > + cpu: cpu@0 { > > compatible = "arm,cortex-a8"; > > device_type = "cpu"; > > reg = <0x0>; > > @@ -41,9 +41,14 @@ > > clock-names = "cpu"; > > > > clock-latency = <300000>; /* From omap-cpufreq driver */ > > + #cooling-cells = <2>; > > }; > > }; > > Looks ok. > > > > > + thermal_zones: thermal-zones { > > + #include "omap3-cpu-thermal.dtsi" > > + }; > > + > > I have observed one compile issue: we also include this indirectly by am3517.dtsi > and the included code refers to <&bandgap 0> but there is no bandgap definition in am3517.dtsi > > Therefore I studied the am35x TRM (SPRUGR0C) and compared to the am/dm37x TRM (SPRUGN4M). > > But I can't find a bandgap temperature sensor with ADC like it is described in > "13.4.6 Band Gap Voltage and Temperature Sensor" for the am/dm37x. Only > "BANDGAP Logic" exists in both and both have the CM_FCLKEN3_CORE but with > different meaning of bit 0. I didn't read the technical details, I just read there was a bandgap logic, so I assumed it existed. > > There is also no description of an CONTROL_TEMP_SENSOR (0x48002524) register for am35x. > (note: the register is also documented for omap3530). Thanks for looking into this. > > So this might mean that the am35x does not have this feature unless TI simply > did not document it because the chip is specified for a single OPP only where it > make no sense to monitor the temperature. > > We can find out only by looking at 0x48002524 if there is an undocumented > bandgap converter. I will try to read this register when I have some time, but I have to watch Chelsea FC play in 15 minutes. ;-) > > Which means we probably can't make thermal throttling work for it. And even > if the bandgap sensor exists we are lacking an value -> celsius table. I think it's probably best to abandon this patch, per my comment based on all your comments. > > > > pmu@54000000 { > > compatible = "arm,cortex-a8-pmu"; > > reg = <0x54000000 0x800000>; > > diff --git a/arch/arm/boot/dts/omap34xx.dtsi b/arch/arm/boot/dts/omap34xx.dtsi > > index f572a477f74c..b80378d6e5c1 100644 > > --- a/arch/arm/boot/dts/omap34xx.dtsi > > +++ b/arch/arm/boot/dts/omap34xx.dtsi > > @@ -101,10 +101,6 @@ > > }; > > }; > > }; > > - > > - thermal_zones: thermal-zones { > > - #include "omap3-cpu-thermal.dtsi" > > - }; > > }; > > > > &ssi { > > diff --git a/arch/arm/boot/dts/omap36xx.dtsi b/arch/arm/boot/dts/omap36xx.dtsi > > index 6fb23ada1f64..ff2dca63a04e 100644 > > --- a/arch/arm/boot/dts/omap36xx.dtsi > > +++ b/arch/arm/boot/dts/omap36xx.dtsi > > @@ -140,10 +140,6 @@ > > }; > > }; > > }; > > - > > - thermal_zones: thermal-zones { > > - #include "omap3-cpu-thermal.dtsi" > > - }; > > }; > > So if we have to exclude the am3517 we can not apply the rearrangement part > of this patch. > > I'd suggest to move the cpu: cpu@0 and #cooling-cells into 1/2 (also to make it > compile stand-alone). And have the consolidation separately - if we can fix the > am3517 bandgap sensor issue. I'll drop this, and leave everything in the omap3-cpu-thermal file and let omap34xx and omap36xx point to them as we do now. > > > > > /* OMAP3630 needs dss_96m_fck for VENC */ > > -- > > 2.17.1 > > > > Tested-by: H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> # on GTA04A5 with dm3730cbp100 >