diff mbox series

[7/7] arm64: dts: qcom: sm8450-hdk: add ADC-TM thermal zones

Message ID 20230630061315.4027453-8-dmitry.baryshkov@linaro.org (mailing list archive)
State Superseded
Headers show
Series arm64: dts: qcom: sm8450-hdk: improve thermal monitoring | expand

Commit Message

Dmitry Baryshkov June 30, 2023, 6:13 a.m. UTC 
Add thermal zones controlled through the ADC-TM (ADC thermal monitoring)
PMIC interface. This includes several onboard sensors and the XO thermal
sensor.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8450-hdk.dts | 244 ++++++++++++++++++++++++
 1 file changed, 244 insertions(+)

Comments

Konrad Dybcio June 30, 2023, 8:19 a.m. UTC | #1 
On 30.06.2023 08:13, Dmitry Baryshkov wrote:
> Add thermal zones controlled through the ADC-TM (ADC thermal monitoring)
> PMIC interface. This includes several onboard sensors and the XO thermal
> sensor.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
[...]
>  
> +	channel@144 {
> +		reg = <PM8350_ADC7_AMUX_THM1_100K_PU(1)>;
This define should be cleaned up.. Since it takes a sid argument,
it really is ADC7_AMUX_THM1_100K_PU(sid)

Konrad

> +		qcom,hw-settle-time = <200>;
> +		qcom,ratiometric;
> +		label = "skin_msm_temp";
> +	};
> +
> +	channel@145 {
> +		reg = <PM8350_ADC7_AMUX_THM2_100K_PU(1)>;
> +		qcom,hw-settle-time = <200>;
> +		qcom,ratiometric;
> +		label = "camera_temp";
> +	};
> +
> +	channel@146 {
> +		reg = <PM8350_ADC7_AMUX_THM3_100K_PU(1)>;
> +		qcom,hw-settle-time = <200>;
> +		qcom,ratiometric;
> +		label = "therm1_temp";
> +	};
> +
> +	channel@147 {
> +		reg = <PM8350_ADC7_AMUX_THM4_100K_PU(1)>;
> +		qcom,hw-settle-time = <200>;
> +		qcom,ratiometric;
> +		label = "wide_rfc_temp";
> +	};
> +
> +	channel@148 {
> +		reg = <PM8350_ADC7_AMUX_THM5_100K_PU(1)>;
> +		qcom,hw-settle-time = <200>;
> +		qcom,ratiometric;
> +		label = "rear_tof_temp";
> +	};
> +
> +	channel@14c {
> +		reg = <PM8350_ADC7_GPIO3_100K_PU(1)>;
> +		qcom,hw-settle-time = <200>;
> +		qcom,ratiometric;
> +		label = "therm2_temp";
> +	};
> +
>  	channel@303 {
>  		reg = <PM8350B_ADC7_DIE_TEMP>;
>  		label = "pm8350b_die_temp";
>  	};
>  
> +	channel@348 {
> +		reg = <PM8350B_ADC7_AMUX_THM5_100K_PU>;
> +		qcom,hw-settle-time = <200>;
> +		qcom,ratiometric;
> +		label = "usb_conn_temp";
> +	};
> +
>  	channel@403 {
>  		reg = <PMR735A_ADC7_DIE_TEMP>;
>  		label = "pmr735a_die_temp";
>  	};
> +
> +	channel@44a {
> +		reg = <PMR735A_ADC7_GPIO1_100K_PU>;
> +		qcom,hw-settle-time = <200>;
> +		qcom,ratiometric;
> +		label = "qtm_w_temp";
> +	};
> +
> +	channel@44b {
> +		reg = <PMR735A_ADC7_GPIO2_100K_PU>;
> +		qcom,hw-settle-time = <200>;
> +		qcom,ratiometric;
> +		label = "qtm_n_temp";
> +	};
>  };
>  
>  &remoteproc_adsp {
Dmitry Baryshkov June 30, 2023, 10:07 a.m. UTC | #2 
On Fri, 30 Jun 2023 at 11:19, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> On 30.06.2023 08:13, Dmitry Baryshkov wrote:
> > Add thermal zones controlled through the ADC-TM (ADC thermal monitoring)
> > PMIC interface. This includes several onboard sensors and the XO thermal
> > sensor.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> [...]
> >
> > +     channel@144 {
> > +             reg = <PM8350_ADC7_AMUX_THM1_100K_PU(1)>;
> This define should be cleaned up.. Since it takes a sid argument,
> it really is ADC7_AMUX_THM1_100K_PU(sid)

I don't think I understood your comment. The define itself is specific
to PM8350, other PMICs might have different channel assignments.

>
> Konrad
>
> > +             qcom,hw-settle-time = <200>;
> > +             qcom,ratiometric;
> > +             label = "skin_msm_temp";
> > +     };
> > +
> > +     channel@145 {
> > +             reg = <PM8350_ADC7_AMUX_THM2_100K_PU(1)>;
> > +             qcom,hw-settle-time = <200>;
> > +             qcom,ratiometric;
> > +             label = "camera_temp";
> > +     };
> > +
> > +     channel@146 {
> > +             reg = <PM8350_ADC7_AMUX_THM3_100K_PU(1)>;
> > +             qcom,hw-settle-time = <200>;
> > +             qcom,ratiometric;
> > +             label = "therm1_temp";
> > +     };
> > +
> > +     channel@147 {
> > +             reg = <PM8350_ADC7_AMUX_THM4_100K_PU(1)>;
> > +             qcom,hw-settle-time = <200>;
> > +             qcom,ratiometric;
> > +             label = "wide_rfc_temp";
> > +     };
> > +
> > +     channel@148 {
> > +             reg = <PM8350_ADC7_AMUX_THM5_100K_PU(1)>;
> > +             qcom,hw-settle-time = <200>;
> > +             qcom,ratiometric;
> > +             label = "rear_tof_temp";
> > +     };
> > +
> > +     channel@14c {
> > +             reg = <PM8350_ADC7_GPIO3_100K_PU(1)>;
> > +             qcom,hw-settle-time = <200>;
> > +             qcom,ratiometric;
> > +             label = "therm2_temp";
> > +     };
> > +
> >       channel@303 {
> >               reg = <PM8350B_ADC7_DIE_TEMP>;
> >               label = "pm8350b_die_temp";
> >       };
> >
> > +     channel@348 {
> > +             reg = <PM8350B_ADC7_AMUX_THM5_100K_PU>;
> > +             qcom,hw-settle-time = <200>;
> > +             qcom,ratiometric;
> > +             label = "usb_conn_temp";
> > +     };
> > +
> >       channel@403 {
> >               reg = <PMR735A_ADC7_DIE_TEMP>;
> >               label = "pmr735a_die_temp";
> >       };
> > +
> > +     channel@44a {
> > +             reg = <PMR735A_ADC7_GPIO1_100K_PU>;
> > +             qcom,hw-settle-time = <200>;
> > +             qcom,ratiometric;
> > +             label = "qtm_w_temp";
> > +     };
> > +
> > +     channel@44b {
> > +             reg = <PMR735A_ADC7_GPIO2_100K_PU>;
> > +             qcom,hw-settle-time = <200>;
> > +             qcom,ratiometric;
> > +             label = "qtm_n_temp";
> > +     };
> >  };
> >
> >  &remoteproc_adsp {
Konrad Dybcio June 30, 2023, 11:27 a.m. UTC | #3 
On 30.06.2023 12:07, Dmitry Baryshkov wrote:
> On Fri, 30 Jun 2023 at 11:19, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>
>> On 30.06.2023 08:13, Dmitry Baryshkov wrote:
>>> Add thermal zones controlled through the ADC-TM (ADC thermal monitoring)
>>> PMIC interface. This includes several onboard sensors and the XO thermal
>>> sensor.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>> [...]
>>>
>>> +     channel@144 {
>>> +             reg = <PM8350_ADC7_AMUX_THM1_100K_PU(1)>;
>> This define should be cleaned up.. Since it takes a sid argument,
>> it really is ADC7_AMUX_THM1_100K_PU(sid)
> 
> I don't think I understood your comment. The define itself is specific
> to PM8350, other PMICs might have different channel assignments.

include/dt-bindings/iio/qcom,spmi-vadc.h
263:#define ADC7_AMUX_THM1_100K_PU                      0x44

Konrad
> 
>>
>> Konrad
>>
>>> +             qcom,hw-settle-time = <200>;
>>> +             qcom,ratiometric;
>>> +             label = "skin_msm_temp";
>>> +     };
>>> +
>>> +     channel@145 {
>>> +             reg = <PM8350_ADC7_AMUX_THM2_100K_PU(1)>;
>>> +             qcom,hw-settle-time = <200>;
>>> +             qcom,ratiometric;
>>> +             label = "camera_temp";
>>> +     };
>>> +
>>> +     channel@146 {
>>> +             reg = <PM8350_ADC7_AMUX_THM3_100K_PU(1)>;
>>> +             qcom,hw-settle-time = <200>;
>>> +             qcom,ratiometric;
>>> +             label = "therm1_temp";
>>> +     };
>>> +
>>> +     channel@147 {
>>> +             reg = <PM8350_ADC7_AMUX_THM4_100K_PU(1)>;
>>> +             qcom,hw-settle-time = <200>;
>>> +             qcom,ratiometric;
>>> +             label = "wide_rfc_temp";
>>> +     };
>>> +
>>> +     channel@148 {
>>> +             reg = <PM8350_ADC7_AMUX_THM5_100K_PU(1)>;
>>> +             qcom,hw-settle-time = <200>;
>>> +             qcom,ratiometric;
>>> +             label = "rear_tof_temp";
>>> +     };
>>> +
>>> +     channel@14c {
>>> +             reg = <PM8350_ADC7_GPIO3_100K_PU(1)>;
>>> +             qcom,hw-settle-time = <200>;
>>> +             qcom,ratiometric;
>>> +             label = "therm2_temp";
>>> +     };
>>> +
>>>       channel@303 {
>>>               reg = <PM8350B_ADC7_DIE_TEMP>;
>>>               label = "pm8350b_die_temp";
>>>       };
>>>
>>> +     channel@348 {
>>> +             reg = <PM8350B_ADC7_AMUX_THM5_100K_PU>;
>>> +             qcom,hw-settle-time = <200>;
>>> +             qcom,ratiometric;
>>> +             label = "usb_conn_temp";
>>> +     };
>>> +
>>>       channel@403 {
>>>               reg = <PMR735A_ADC7_DIE_TEMP>;
>>>               label = "pmr735a_die_temp";
>>>       };
>>> +
>>> +     channel@44a {
>>> +             reg = <PMR735A_ADC7_GPIO1_100K_PU>;
>>> +             qcom,hw-settle-time = <200>;
>>> +             qcom,ratiometric;
>>> +             label = "qtm_w_temp";
>>> +     };
>>> +
>>> +     channel@44b {
>>> +             reg = <PMR735A_ADC7_GPIO2_100K_PU>;
>>> +             qcom,hw-settle-time = <200>;
>>> +             qcom,ratiometric;
>>> +             label = "qtm_n_temp";
>>> +     };
>>>  };
>>>
>>>  &remoteproc_adsp {
> 
> 
>
Dmitry Baryshkov June 30, 2023, 12:57 p.m. UTC | #4 
On Fri, 30 Jun 2023 at 14:27, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> On 30.06.2023 12:07, Dmitry Baryshkov wrote:
> > On Fri, 30 Jun 2023 at 11:19, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> >>
> >> On 30.06.2023 08:13, Dmitry Baryshkov wrote:
> >>> Add thermal zones controlled through the ADC-TM (ADC thermal monitoring)
> >>> PMIC interface. This includes several onboard sensors and the XO thermal
> >>> sensor.
> >>>
> >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >>> ---
> >> [...]
> >>>
> >>> +     channel@144 {
> >>> +             reg = <PM8350_ADC7_AMUX_THM1_100K_PU(1)>;
> >> This define should be cleaned up.. Since it takes a sid argument,
> >> it really is ADC7_AMUX_THM1_100K_PU(sid)
> >
> > I don't think I understood your comment. The define itself is specific
> > to PM8350, other PMICs might have different channel assignments.
>
> include/dt-bindings/iio/qcom,spmi-vadc.h
> 263:#define ADC7_AMUX_THM1_100K_PU                      0x44

Do you want to define PM8350_ADC7_AMUX_THM1_100K_PU(sid) using
ADC7_AMUX_THM1_100K_PU ?
Or to make all users use ADC7_AMUX_THM1_100K_PU? Or add the SID
argument to ADC7_AMUX_THM1_100K_PU and switch to it?

>
> Konrad
> >
> >>
> >> Konrad
> >>
> >>> +             qcom,hw-settle-time = <200>;
> >>> +             qcom,ratiometric;
> >>> +             label = "skin_msm_temp";
> >>> +     };
> >>> +
> >>> +     channel@145 {
> >>> +             reg = <PM8350_ADC7_AMUX_THM2_100K_PU(1)>;
> >>> +             qcom,hw-settle-time = <200>;
> >>> +             qcom,ratiometric;
> >>> +             label = "camera_temp";
> >>> +     };
> >>> +
> >>> +     channel@146 {
> >>> +             reg = <PM8350_ADC7_AMUX_THM3_100K_PU(1)>;
> >>> +             qcom,hw-settle-time = <200>;
> >>> +             qcom,ratiometric;
> >>> +             label = "therm1_temp";
> >>> +     };
> >>> +
> >>> +     channel@147 {
> >>> +             reg = <PM8350_ADC7_AMUX_THM4_100K_PU(1)>;
> >>> +             qcom,hw-settle-time = <200>;
> >>> +             qcom,ratiometric;
> >>> +             label = "wide_rfc_temp";
> >>> +     };
> >>> +
> >>> +     channel@148 {
> >>> +             reg = <PM8350_ADC7_AMUX_THM5_100K_PU(1)>;
> >>> +             qcom,hw-settle-time = <200>;
> >>> +             qcom,ratiometric;
> >>> +             label = "rear_tof_temp";
> >>> +     };
> >>> +
> >>> +     channel@14c {
> >>> +             reg = <PM8350_ADC7_GPIO3_100K_PU(1)>;
> >>> +             qcom,hw-settle-time = <200>;
> >>> +             qcom,ratiometric;
> >>> +             label = "therm2_temp";
> >>> +     };
> >>> +
> >>>       channel@303 {
> >>>               reg = <PM8350B_ADC7_DIE_TEMP>;
> >>>               label = "pm8350b_die_temp";
> >>>       };
> >>>
> >>> +     channel@348 {
> >>> +             reg = <PM8350B_ADC7_AMUX_THM5_100K_PU>;
> >>> +             qcom,hw-settle-time = <200>;
> >>> +             qcom,ratiometric;
> >>> +             label = "usb_conn_temp";
> >>> +     };
> >>> +
> >>>       channel@403 {
> >>>               reg = <PMR735A_ADC7_DIE_TEMP>;
> >>>               label = "pmr735a_die_temp";
> >>>       };
> >>> +
> >>> +     channel@44a {
> >>> +             reg = <PMR735A_ADC7_GPIO1_100K_PU>;
> >>> +             qcom,hw-settle-time = <200>;
> >>> +             qcom,ratiometric;
> >>> +             label = "qtm_w_temp";
> >>> +     };
> >>> +
> >>> +     channel@44b {
> >>> +             reg = <PMR735A_ADC7_GPIO2_100K_PU>;
> >>> +             qcom,hw-settle-time = <200>;
> >>> +             qcom,ratiometric;
> >>> +             label = "qtm_n_temp";
> >>> +     };
> >>>  };
> >>>
> >>>  &remoteproc_adsp {
> >
> >
> >
Konrad Dybcio June 30, 2023, 4:15 p.m. UTC | #5 
On 30.06.2023 14:57, Dmitry Baryshkov wrote:
> On Fri, 30 Jun 2023 at 14:27, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>
>> On 30.06.2023 12:07, Dmitry Baryshkov wrote:
>>> On Fri, 30 Jun 2023 at 11:19, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>>>
>>>> On 30.06.2023 08:13, Dmitry Baryshkov wrote:
>>>>> Add thermal zones controlled through the ADC-TM (ADC thermal monitoring)
>>>>> PMIC interface. This includes several onboard sensors and the XO thermal
>>>>> sensor.
>>>>>
>>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>> ---
>>>> [...]
>>>>>
>>>>> +     channel@144 {
>>>>> +             reg = <PM8350_ADC7_AMUX_THM1_100K_PU(1)>;
>>>> This define should be cleaned up.. Since it takes a sid argument,
>>>> it really is ADC7_AMUX_THM1_100K_PU(sid)
>>>
>>> I don't think I understood your comment. The define itself is specific
>>> to PM8350, other PMICs might have different channel assignments.
>>
>> include/dt-bindings/iio/qcom,spmi-vadc.h
>> 263:#define ADC7_AMUX_THM1_100K_PU                      0x44
> 
> Do you want to define PM8350_ADC7_AMUX_THM1_100K_PU(sid) using
> ADC7_AMUX_THM1_100K_PU ?
> Or to make all users use ADC7_AMUX_THM1_100K_PU?


>Or add the SID
> argument to ADC7_AMUX_THM1_100K_PU and switch to it?
This.

Since we have a generic binding for it (not sure what sort of ABI-ish rules
apply here, probably not very many since it's just a dumb preprocessor define),
we should not redefine it for each PMIC, especially since the SIDs are variable
nowadays :/

Sorry for being too vague.

Konrad
> 
>>
>> Konrad
>>>
>>>>
>>>> Konrad
>>>>
>>>>> +             qcom,hw-settle-time = <200>;
>>>>> +             qcom,ratiometric;
>>>>> +             label = "skin_msm_temp";
>>>>> +     };
>>>>> +
>>>>> +     channel@145 {
>>>>> +             reg = <PM8350_ADC7_AMUX_THM2_100K_PU(1)>;
>>>>> +             qcom,hw-settle-time = <200>;
>>>>> +             qcom,ratiometric;
>>>>> +             label = "camera_temp";
>>>>> +     };
>>>>> +
>>>>> +     channel@146 {
>>>>> +             reg = <PM8350_ADC7_AMUX_THM3_100K_PU(1)>;
>>>>> +             qcom,hw-settle-time = <200>;
>>>>> +             qcom,ratiometric;
>>>>> +             label = "therm1_temp";
>>>>> +     };
>>>>> +
>>>>> +     channel@147 {
>>>>> +             reg = <PM8350_ADC7_AMUX_THM4_100K_PU(1)>;
>>>>> +             qcom,hw-settle-time = <200>;
>>>>> +             qcom,ratiometric;
>>>>> +             label = "wide_rfc_temp";
>>>>> +     };
>>>>> +
>>>>> +     channel@148 {
>>>>> +             reg = <PM8350_ADC7_AMUX_THM5_100K_PU(1)>;
>>>>> +             qcom,hw-settle-time = <200>;
>>>>> +             qcom,ratiometric;
>>>>> +             label = "rear_tof_temp";
>>>>> +     };
>>>>> +
>>>>> +     channel@14c {
>>>>> +             reg = <PM8350_ADC7_GPIO3_100K_PU(1)>;
>>>>> +             qcom,hw-settle-time = <200>;
>>>>> +             qcom,ratiometric;
>>>>> +             label = "therm2_temp";
>>>>> +     };
>>>>> +
>>>>>       channel@303 {
>>>>>               reg = <PM8350B_ADC7_DIE_TEMP>;
>>>>>               label = "pm8350b_die_temp";
>>>>>       };
>>>>>
>>>>> +     channel@348 {
>>>>> +             reg = <PM8350B_ADC7_AMUX_THM5_100K_PU>;
>>>>> +             qcom,hw-settle-time = <200>;
>>>>> +             qcom,ratiometric;
>>>>> +             label = "usb_conn_temp";
>>>>> +     };
>>>>> +
>>>>>       channel@403 {
>>>>>               reg = <PMR735A_ADC7_DIE_TEMP>;
>>>>>               label = "pmr735a_die_temp";
>>>>>       };
>>>>> +
>>>>> +     channel@44a {
>>>>> +             reg = <PMR735A_ADC7_GPIO1_100K_PU>;
>>>>> +             qcom,hw-settle-time = <200>;
>>>>> +             qcom,ratiometric;
>>>>> +             label = "qtm_w_temp";
>>>>> +     };
>>>>> +
>>>>> +     channel@44b {
>>>>> +             reg = <PMR735A_ADC7_GPIO2_100K_PU>;
>>>>> +             qcom,hw-settle-time = <200>;
>>>>> +             qcom,ratiometric;
>>>>> +             label = "qtm_n_temp";
>>>>> +     };
>>>>>  };
>>>>>
>>>>>  &remoteproc_adsp {
>>>
>>>
>>>
> 
> 
>
Dmitry Baryshkov June 30, 2023, 5:42 p.m. UTC | #6 
On Fri, 30 Jun 2023 at 19:15, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> On 30.06.2023 14:57, Dmitry Baryshkov wrote:
> > On Fri, 30 Jun 2023 at 14:27, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> >>
> >> On 30.06.2023 12:07, Dmitry Baryshkov wrote:
> >>> On Fri, 30 Jun 2023 at 11:19, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> >>>>
> >>>> On 30.06.2023 08:13, Dmitry Baryshkov wrote:
> >>>>> Add thermal zones controlled through the ADC-TM (ADC thermal monitoring)
> >>>>> PMIC interface. This includes several onboard sensors and the XO thermal
> >>>>> sensor.
> >>>>>
> >>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >>>>> ---
> >>>> [...]
> >>>>>
> >>>>> +     channel@144 {
> >>>>> +             reg = <PM8350_ADC7_AMUX_THM1_100K_PU(1)>;
> >>>> This define should be cleaned up.. Since it takes a sid argument,
> >>>> it really is ADC7_AMUX_THM1_100K_PU(sid)
> >>>
> >>> I don't think I understood your comment. The define itself is specific
> >>> to PM8350, other PMICs might have different channel assignments.
> >>
> >> include/dt-bindings/iio/qcom,spmi-vadc.h
> >> 263:#define ADC7_AMUX_THM1_100K_PU                      0x44
> >
> > Do you want to define PM8350_ADC7_AMUX_THM1_100K_PU(sid) using
> > ADC7_AMUX_THM1_100K_PU ?
> > Or to make all users use ADC7_AMUX_THM1_100K_PU?
>
>
> >Or add the SID
> > argument to ADC7_AMUX_THM1_100K_PU and switch to it?
> This.
>
> Since we have a generic binding for it (not sure what sort of ABI-ish rules
> apply here, probably not very many since it's just a dumb preprocessor define),
> we should not redefine it for each PMIC, especially since the SIDs are variable
> nowadays :/

I think it would be worth to have per-PMIC headers (which define which
channels are available on this PMIC), but to use values from the
qcom,spmi-vadc.h  header to define those per-PMIC values.

WDYT?

>
> Sorry for being too vague.
>
> Konrad
> >
> >>
> >> Konrad
> >>>
> >>>>
> >>>> Konrad
> >>>>
> >>>>> +             qcom,hw-settle-time = <200>;
> >>>>> +             qcom,ratiometric;
> >>>>> +             label = "skin_msm_temp";
> >>>>> +     };
> >>>>> +
> >>>>> +     channel@145 {
> >>>>> +             reg = <PM8350_ADC7_AMUX_THM2_100K_PU(1)>;
> >>>>> +             qcom,hw-settle-time = <200>;
> >>>>> +             qcom,ratiometric;
> >>>>> +             label = "camera_temp";
> >>>>> +     };
> >>>>> +
> >>>>> +     channel@146 {
> >>>>> +             reg = <PM8350_ADC7_AMUX_THM3_100K_PU(1)>;
> >>>>> +             qcom,hw-settle-time = <200>;
> >>>>> +             qcom,ratiometric;
> >>>>> +             label = "therm1_temp";
> >>>>> +     };
> >>>>> +
> >>>>> +     channel@147 {
> >>>>> +             reg = <PM8350_ADC7_AMUX_THM4_100K_PU(1)>;
> >>>>> +             qcom,hw-settle-time = <200>;
> >>>>> +             qcom,ratiometric;
> >>>>> +             label = "wide_rfc_temp";
> >>>>> +     };
> >>>>> +
> >>>>> +     channel@148 {
> >>>>> +             reg = <PM8350_ADC7_AMUX_THM5_100K_PU(1)>;
> >>>>> +             qcom,hw-settle-time = <200>;
> >>>>> +             qcom,ratiometric;
> >>>>> +             label = "rear_tof_temp";
> >>>>> +     };
> >>>>> +
> >>>>> +     channel@14c {
> >>>>> +             reg = <PM8350_ADC7_GPIO3_100K_PU(1)>;
> >>>>> +             qcom,hw-settle-time = <200>;
> >>>>> +             qcom,ratiometric;
> >>>>> +             label = "therm2_temp";
> >>>>> +     };
> >>>>> +
> >>>>>       channel@303 {
> >>>>>               reg = <PM8350B_ADC7_DIE_TEMP>;
> >>>>>               label = "pm8350b_die_temp";
> >>>>>       };
> >>>>>
> >>>>> +     channel@348 {
> >>>>> +             reg = <PM8350B_ADC7_AMUX_THM5_100K_PU>;
> >>>>> +             qcom,hw-settle-time = <200>;
> >>>>> +             qcom,ratiometric;
> >>>>> +             label = "usb_conn_temp";
> >>>>> +     };
> >>>>> +
> >>>>>       channel@403 {
> >>>>>               reg = <PMR735A_ADC7_DIE_TEMP>;
> >>>>>               label = "pmr735a_die_temp";
> >>>>>       };
> >>>>> +
> >>>>> +     channel@44a {
> >>>>> +             reg = <PMR735A_ADC7_GPIO1_100K_PU>;
> >>>>> +             qcom,hw-settle-time = <200>;
> >>>>> +             qcom,ratiometric;
> >>>>> +             label = "qtm_w_temp";
> >>>>> +     };
> >>>>> +
> >>>>> +     channel@44b {
> >>>>> +             reg = <PMR735A_ADC7_GPIO2_100K_PU>;
> >>>>> +             qcom,hw-settle-time = <200>;
> >>>>> +             qcom,ratiometric;
> >>>>> +             label = "qtm_n_temp";
> >>>>> +     };
> >>>>>  };
> >>>>>
> >>>>>  &remoteproc_adsp {
> >>>
> >>>
> >>>
> >
> >
> >
Bjorn Andersson June 30, 2023, 8:59 p.m. UTC | #7 
On Fri, Jun 30, 2023 at 08:42:23PM +0300, Dmitry Baryshkov wrote:
> On Fri, 30 Jun 2023 at 19:15, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> >
> > On 30.06.2023 14:57, Dmitry Baryshkov wrote:
> > > On Fri, 30 Jun 2023 at 14:27, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> > >>
> > >> On 30.06.2023 12:07, Dmitry Baryshkov wrote:
> > >>> On Fri, 30 Jun 2023 at 11:19, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> > >>>>
> > >>>> On 30.06.2023 08:13, Dmitry Baryshkov wrote:
> > >>>>> Add thermal zones controlled through the ADC-TM (ADC thermal monitoring)
> > >>>>> PMIC interface. This includes several onboard sensors and the XO thermal
> > >>>>> sensor.
> > >>>>>
> > >>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > >>>>> ---
> > >>>> [...]
> > >>>>>
> > >>>>> +     channel@144 {
> > >>>>> +             reg = <PM8350_ADC7_AMUX_THM1_100K_PU(1)>;
> > >>>> This define should be cleaned up.. Since it takes a sid argument,
> > >>>> it really is ADC7_AMUX_THM1_100K_PU(sid)
> > >>>
> > >>> I don't think I understood your comment. The define itself is specific
> > >>> to PM8350, other PMICs might have different channel assignments.
> > >>
> > >> include/dt-bindings/iio/qcom,spmi-vadc.h
> > >> 263:#define ADC7_AMUX_THM1_100K_PU                      0x44
> > >
> > > Do you want to define PM8350_ADC7_AMUX_THM1_100K_PU(sid) using
> > > ADC7_AMUX_THM1_100K_PU ?
> > > Or to make all users use ADC7_AMUX_THM1_100K_PU?
> >
> >
> > >Or add the SID
> > > argument to ADC7_AMUX_THM1_100K_PU and switch to it?
> > This.
> >
> > Since we have a generic binding for it (not sure what sort of ABI-ish rules
> > apply here, probably not very many since it's just a dumb preprocessor define),
> > we should not redefine it for each PMIC, especially since the SIDs are variable
> > nowadays :/
> 
> I think it would be worth to have per-PMIC headers (which define which
> channels are available on this PMIC), but to use values from the
> qcom,spmi-vadc.h  header to define those per-PMIC values.
> 
> WDYT?
> 

Note that the channel relates to this PMIC, but the sid is of the
measured PMIC. So I'm not sure which PMIC you're thinking of when you
say per-PMIC...

Regards,
Bjorn
Dmitry Baryshkov June 30, 2023, 9:04 p.m. UTC | #8 
On Fri, 30 Jun 2023 at 23:59, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
>
> On Fri, Jun 30, 2023 at 08:42:23PM +0300, Dmitry Baryshkov wrote:
> > On Fri, 30 Jun 2023 at 19:15, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> > >
> > > On 30.06.2023 14:57, Dmitry Baryshkov wrote:
> > > > On Fri, 30 Jun 2023 at 14:27, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> > > >>
> > > >> On 30.06.2023 12:07, Dmitry Baryshkov wrote:
> > > >>> On Fri, 30 Jun 2023 at 11:19, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> > > >>>>
> > > >>>> On 30.06.2023 08:13, Dmitry Baryshkov wrote:
> > > >>>>> Add thermal zones controlled through the ADC-TM (ADC thermal monitoring)
> > > >>>>> PMIC interface. This includes several onboard sensors and the XO thermal
> > > >>>>> sensor.
> > > >>>>>
> > > >>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > >>>>> ---
> > > >>>> [...]
> > > >>>>>
> > > >>>>> +     channel@144 {
> > > >>>>> +             reg = <PM8350_ADC7_AMUX_THM1_100K_PU(1)>;
> > > >>>> This define should be cleaned up.. Since it takes a sid argument,
> > > >>>> it really is ADC7_AMUX_THM1_100K_PU(sid)
> > > >>>
> > > >>> I don't think I understood your comment. The define itself is specific
> > > >>> to PM8350, other PMICs might have different channel assignments.
> > > >>
> > > >> include/dt-bindings/iio/qcom,spmi-vadc.h
> > > >> 263:#define ADC7_AMUX_THM1_100K_PU                      0x44
> > > >
> > > > Do you want to define PM8350_ADC7_AMUX_THM1_100K_PU(sid) using
> > > > ADC7_AMUX_THM1_100K_PU ?
> > > > Or to make all users use ADC7_AMUX_THM1_100K_PU?
> > >
> > >
> > > >Or add the SID
> > > > argument to ADC7_AMUX_THM1_100K_PU and switch to it?
> > > This.
> > >
> > > Since we have a generic binding for it (not sure what sort of ABI-ish rules
> > > apply here, probably not very many since it's just a dumb preprocessor define),
> > > we should not redefine it for each PMIC, especially since the SIDs are variable
> > > nowadays :/
> >
> > I think it would be worth to have per-PMIC headers (which define which
> > channels are available on this PMIC), but to use values from the
> > qcom,spmi-vadc.h  header to define those per-PMIC values.
> >
> > WDYT?
> >
>
> Note that the channel relates to this PMIC, but the sid is of the
> measured PMIC. So I'm not sure which PMIC you're thinking of when you
> say per-PMIC...

Not quite. Both sid and channel are for the peasuring PMIC (pm8350b,
pm8350, etc). On pmk8350 we further multiplex them into the 8 channels
of adc-tm.

>
> Regards,
> Bjorn
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sm8450-hdk.dts b/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
index d07e402eaba3..4dfc964a4bb1 100644
--- a/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
+++ b/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
@@ -133,6 +133,120 @@  pmic_glink_sbu: endpoint {
 		};
 	};
 
+	thermal-zones {
+		camera-thermal {
+			polling-delay-passive = <250>;
+			polling-delay = <0>;
+			thermal-sensors = <&pmk8350_adc_tm 2>;
+
+			trips {
+				active-config0 {
+					temperature = <75000>;
+					hysteresis = <4000>;
+					type = "passive";
+				};
+			};
+		};
+
+		rear-tof-thermal {
+			polling-delay-passive = <250>;
+			polling-delay = <0>;
+			thermal-sensors = <&pmk8350_adc_tm 5>;
+
+			trips {
+				active-config0 {
+					temperature = <75000>;
+					hysteresis = <4000>;
+					type = "passive";
+				};
+			};
+		};
+
+		skin-msm-thermal {
+			polling-delay-passive = <250>;
+			polling-delay = <0>;
+			thermal-sensors = <&pmk8350_adc_tm 1>;
+
+			trips {
+				active-config0 {
+					temperature = <75000>;
+					hysteresis = <4000>;
+					type = "passive";
+				};
+			};
+		};
+
+		therm1-thermal {
+			polling-delay-passive = <250>;
+			polling-delay = <0>;
+			thermal-sensors = <&pmk8350_adc_tm 3>;
+
+			trips {
+				active-config0 {
+					temperature = <75000>;
+					hysteresis = <4000>;
+					type = "passive";
+				};
+			};
+		};
+
+		therm2-thermal {
+			polling-delay-passive = <250>;
+			polling-delay = <0>;
+			thermal-sensors = <&pmk8350_adc_tm 6>;
+
+			trips {
+				active-config0 {
+					temperature = <75000>;
+					hysteresis = <4000>;
+					type = "passive";
+				};
+			};
+		};
+
+		usb-conn-thermal {
+			polling-delay-passive = <250>;
+			polling-delay = <0>;
+			thermal-sensors = <&pmk8350_adc_tm 7>;
+
+			trips {
+				active-config0 {
+					temperature = <75000>;
+					hysteresis = <4000>;
+					type = "passive";
+				};
+			};
+		};
+
+		wide-rfc-thermal {
+			polling-delay-passive = <250>;
+			polling-delay = <0>;
+			thermal-sensors = <&pmk8350_adc_tm 4>;
+
+			trips {
+				active-config0 {
+					temperature = <75000>;
+					hysteresis = <4000>;
+					type = "passive";
+				};
+			};
+		};
+
+		xo-thermal {
+			polling-delay-passive = <0>;
+			polling-delay = <0>;
+			thermal-sensors = <&pmk8350_adc_tm 0>;
+
+			trips {
+				active-config0 {
+					temperature = <50000>;
+					hysteresis = <4000>;
+					type = "passive";
+				};
+			};
+		};
+	};
+
 	vph_pwr: vph-pwr-regulator {
 		compatible = "regulator-fixed";
 		regulator-name = "vph_pwr";
@@ -607,6 +721,66 @@  &pmr735a_temp_alarm {
 	io-channel-names = "thermal";
 };
 
+&pmk8350_adc_tm {
+	status = "okay";
+
+	xo-therm@0 {
+		reg = <0>;
+		io-channels = <&pmk8350_vadc PMK8350_ADC7_AMUX_THM1_100K_PU>;
+		qcom,ratiometric;
+		qcom,hw-settle-time-us = <200>;
+	};
+
+	skin-msm-therm@1 {
+		reg = <1>;
+		io-channels = <&pmk8350_vadc PM8350_ADC7_AMUX_THM1_100K_PU(1)>;
+		qcom,ratiometric;
+		qcom,hw-settle-time-us = <200>;
+	};
+
+	camera-therm@2 {
+		reg = <2>;
+		io-channels = <&pmk8350_vadc PM8350_ADC7_AMUX_THM2_100K_PU(1)>;
+		qcom,ratiometric;
+		qcom,hw-settle-time-us = <200>;
+	};
+
+	therm1-therm@3 {
+		reg = <3>;
+		io-channels = <&pmk8350_vadc PM8350_ADC7_AMUX_THM3_100K_PU(1)>;
+		qcom,ratiometric;
+		qcom,hw-settle-time-us = <200>;
+	};
+
+	wide-rfc-therm@4 {
+		reg = <4>;
+		io-channels = <&pmk8350_vadc PM8350_ADC7_AMUX_THM4_100K_PU(1)>;
+		qcom,ratiometric;
+		qcom,hw-settle-time-us = <200>;
+	};
+
+	rear-tof-therm@5 {
+		reg = <5>;
+		io-channels = <&pmk8350_vadc PM8350_ADC7_AMUX_THM5_100K_PU(1)>;
+		qcom,ratiometric;
+		qcom,hw-settle-time-us = <200>;
+	};
+
+	therm2-therm@6 {
+		reg = <6>;
+		io-channels = <&pmk8350_vadc PM8350_ADC7_GPIO3_100K_PU(1)>;
+		qcom,ratiometric;
+		qcom,hw-settle-time-us = <200>;
+	};
+
+	usb-conn-therm@7 {
+		reg = <7>;
+		io-channels = <&pmk8350_vadc PM8350B_ADC7_AMUX_THM5_100K_PU>;
+		qcom,ratiometric;
+		qcom,hw-settle-time-us = <200>;
+	};
+};
+
 &pmk8350_vadc {
 	status = "okay";
 
@@ -615,20 +789,90 @@  channel@3 {
 		label = "pmk8350_die_temp";
 	};
 
+	channel@44 {
+		reg = <PMK8350_ADC7_AMUX_THM1_100K_PU>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "pmk8350_xo_therm";
+	};
+
 	channel@103 {
 		reg = <PM8350_ADC7_DIE_TEMP(1)>;
 		label = "pm8350_die_temp";
 	};
 
+	channel@144 {
+		reg = <PM8350_ADC7_AMUX_THM1_100K_PU(1)>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "skin_msm_temp";
+	};
+
+	channel@145 {
+		reg = <PM8350_ADC7_AMUX_THM2_100K_PU(1)>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "camera_temp";
+	};
+
+	channel@146 {
+		reg = <PM8350_ADC7_AMUX_THM3_100K_PU(1)>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "therm1_temp";
+	};
+
+	channel@147 {
+		reg = <PM8350_ADC7_AMUX_THM4_100K_PU(1)>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "wide_rfc_temp";
+	};
+
+	channel@148 {
+		reg = <PM8350_ADC7_AMUX_THM5_100K_PU(1)>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "rear_tof_temp";
+	};
+
+	channel@14c {
+		reg = <PM8350_ADC7_GPIO3_100K_PU(1)>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "therm2_temp";
+	};
+
 	channel@303 {
 		reg = <PM8350B_ADC7_DIE_TEMP>;
 		label = "pm8350b_die_temp";
 	};
 
+	channel@348 {
+		reg = <PM8350B_ADC7_AMUX_THM5_100K_PU>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "usb_conn_temp";
+	};
+
 	channel@403 {
 		reg = <PMR735A_ADC7_DIE_TEMP>;
 		label = "pmr735a_die_temp";
 	};
+
+	channel@44a {
+		reg = <PMR735A_ADC7_GPIO1_100K_PU>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "qtm_w_temp";
+	};
+
+	channel@44b {
+		reg = <PMR735A_ADC7_GPIO2_100K_PU>;
+		qcom,hw-settle-time = <200>;
+		qcom,ratiometric;
+		label = "qtm_n_temp";
+	};
 };
 
 &remoteproc_adsp {