On Mon 04 Oct 15:56 CDT 2021, Dmitry Baryshkov wrote:
> On Mon, 4 Oct 2021 at 23:13, Bjorn Andersson <bjorn.andersson@xxxxxxxxxx> wrote:
> >
> > On Wed 29 Sep 11:40 PDT 2021, Matthias Kaehlcke wrote:
> >
> > > On Thu, Sep 23, 2021 at 02:23:11PM -0700, Bjorn Andersson wrote:
> > > > Downstream defines four ADC channels related to thermal sensors external
> > > > to the PM8998 and two channels for internal voltage measurements.
> > > >
> > > > Add these to the upstream SDM845 MTP, describe the thermal monitor
> > > > channels and add thermal_zones for these.
> > > >
> > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
> > > > ---
> > > >
> > > > In addition to the iio channels exposed by v1, Daniel wanted thermal_zones...
> > > >
> > > > Changes since v1:
> > > > - Enable the pm8998_adc_tm and describe the ADC channels
> > > > - Add thermal-zones for the new channels
> > > >
> > > > arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 128 ++++++++++++++++++++++++
> > > > 1 file changed, 128 insertions(+)
> > > >
> > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> > [..]
> > > > +&pm8998_adc {
> > > > + adc-chan@4c {
> > > > + reg = <ADC5_XO_THERM_100K_PU>;
> > > > + label = "xo_therm";
> > > > + };
> > > > +
> > > > + adc-chan@4d {
> > > > + reg = <ADC5_AMUX_THM1_100K_PU>;
> > > > + label = "msm_therm";
> > > > + };
> > > > +
> > > > + adc-chan@4f {
> > > > + reg = <ADC5_AMUX_THM3_100K_PU>;
> > > > + label = "pa_therm1";
> > > > + };
> > > > +
> > > > + adc-chan@51 {
> > > > + reg = <ADC5_AMUX_THM5_100K_PU>;
> > > > + label = "quiet_therm";
> > > > + };
> > > > +
> > > > + adc-chan@83 {
> > > > + reg = <ADC5_VPH_PWR>;
> > > > + label = "vph_pwr";
> > > > + };
> > > > +
> > > > + adc-chan@85 {
> > > > + reg = <ADC5_VCOIN>;
> > > > + label = "vcoin";
> > > > + };
> > > > +};
> > > > +
> > > > +&pm8998_adc_tm {
> > > > + status = "okay";
> > > > +
> > > > + xo-thermistor@1 {
> > > > + reg = <1>;
> > > > + io-channels = <&pm8998_adc ADC5_XO_THERM_100K_PU>;
> > > > + qcom,ratiometric;
> > > > + qcom,hw-settle-time-us = <200>;
> > > > + };
> > > > +
> > > > + msm-thermistor@2 {
> > > > + reg = <2>;
> > > > + io-channels = <&pm8998_adc ADC5_AMUX_THM1_100K_PU>;
> > > > + qcom,ratiometric;
> > > > + qcom,hw-settle-time-us = <200>;
> > > > + };
> > > > +
> > > > + pa-thermistor@3 {
> > > > + reg = <3>;
> > > > + io-channels = <&pm8998_adc ADC5_AMUX_THM3_100K_PU>;
> > > > + qcom,ratiometric;
> > > > + qcom,hw-settle-time-us = <200>;
> > > > + };
> > > > +
> > > > + quiet-thermistor@4 {
> > > > + reg = <4>;
> > > > + io-channels = <&pm8998_adc ADC5_AMUX_THM5_100K_PU>;
> > > > + qcom,ratiometric;
> > > > + qcom,hw-settle-time-us = <200>;
> > > > + };
> > > > +};
> > > > +
> > >
> > > The example in the 'qcom,spmi-adc-tm5' binding specifies 'qcom,ratiometric'
> > > and 'qcom,hw-settle-time-us' for both the ADC and the thermal monitor, so do
> > > several board files (e.g. sm8250-mtp.dts and qrb5165-rb5.dts). This apparent
> > > redundancy bothered me earlier, it's not really clear to me whether it's
> > > needed/recommended or not. Do you happen to have any insights on this?
> >
> > Hmm, you're right and I missed this in defining my channels. I've not
> > looked at this detail, just got reasonable readings from my thermal
> > zones and was happy about that.
> >
> > Dmitry, do you have any further insights why these properties are
> > supposed to be duplicated between the adc channel and the thermal zones?
>
> Because both ADC channel and thermal zone registers should be
> programmed accordingly.
>
I presume our question is "why" to this particular part. The actual
implementation thereof seems reasonable.
Regards,
Bjorn
