Re: [PATCH 5/7] arm64: dts: qcom: pm8150x: add thermal alarms and thermal zones

[Vinod Koul <vkoul@kernel.org>]

Sorry missed ccing Amit, done now.

On 04-06-20, 18:03, Dmitry Baryshkov wrote:
> On 04/06/2020 13:47, Vinod Koul wrote:
> > On 04-06-20, 03:43, Dmitry Baryshkov wrote:

> > >   		pm8150_adc: adc@3100 {
> > >   			compatible = "qcom,spmi-adc5";
> > >   			reg = <0x3100>;
> > > @@ -38,8 +47,6 @@ pm8150_adc: adc@3100 {
> > >   			#io-channel-cells = <1>;
> > >   			interrupts = <0x0 0x31 0x0 IRQ_TYPE_EDGE_RISING>;
> > > -			status = "disabled";
> > > -
> > 
> > This should not be removed, rather than this please add enabled in you
> > board dts file

...

> > > +&thermal_zones {
> > > +	pm8150_temp {
> > > +		polling-delay-passive = <0>;
> > > +		polling-delay = <0>;
> > > +
> > > +		thermal-sensors = <&pm8150_temp>;
> > > +
> > > +		trips {
> > > +			trip0 {
> > > +				temperature = <95000>;
> > > +				hysteresis = <0>;
> > > +				type = "passive";
> > > +			};
> > > +
> > > +			trip1 {
> > > +				temperature = <115000>;
> > > +				hysteresis = <0>;
> > > +				type = "passive";
> > > +			};
> > > +
> > > +			trip2 {
> > > +				temperature = <145000>;
> > > +				hysteresis = <0>;
> > > +				type = "passive";
> > > +			};
> > > +		};
> > > +
> > > +	};
> > 
> > Not sure about this, Amit..? Should this also not be in board dts?
> > 
> > Similar comments on similar ones for rest of the patch as well..
> 
> I'm not so sure. This part of the configuration seems generic to me. Unlike
> adc-tm config, which definitely goes to the board file.

I think the temperature values may be board specific, Amit can confirm
that. If that is the case then this belongs to board dts, otherwise here :)

> I can split this into a separate pm8150-temp.dtsi file. Does that sound
> better?

That might make it worse, we don't do splitting.

-- 
~Vinod