Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 4 Oct 2021 22:12:04 -0500
From:   Bjorn Andersson <bjorn.andersson@linaro.org>
To:     Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc:     Matthias Kaehlcke <mka@chromium.org>,
        Andy Gross <agross@kernel.org>,
        Zhang Rui <rui.zhang@intel.com>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Amit Kucheria <amitk@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Thara Gopinath <thara.gopinath@linaro.org>,
        "open list:DRM DRIVER FOR MSM ADRENO GPU" 
        <linux-arm-msm@vger.kernel.org>,
        Linux PM <linux-pm@vger.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 4/4] arm64: dts: qcom: sdm845: mtp: Add vadc channels
 and thermal zones
Message-ID: <YVvChGwyiBF+TjlB@yoga>
References: <20210923212311.2877048-1-bjorn.andersson@linaro.org>
 <20210923212311.2877048-5-bjorn.andersson@linaro.org>
 <YVSzJZ8G43CLml3L@google.com>
 <YVtg3lcR1HMqVdAJ@ripper>
 <CAA8EJprYij6pWD1A17yr1+5-n5fKPW=YDA_-2+f8h6JnEh4myw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAA8EJprYij6pWD1A17yr1+5-n5fKPW=YDA_-2+f8h6JnEh4myw@mail.gmail.com>
Precedence: bulk

On Mon 04 Oct 15:56 CDT 2021, Dmitry Baryshkov wrote:

> On Mon, 4 Oct 2021 at 23:13, Bjorn Andersson <bjorn.andersson@linaro.org> 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@linaro.org>
> > > > ---
> > > >
> > > > 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