On Fri, 30 Jun 2023 at 19:15, Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> wrote:
>
> On 30.06.2023 14:57, Dmitry Baryshkov wrote:
> > On Fri, 30 Jun 2023 at 14:27, Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> wrote:
> >>
> >> On 30.06.2023 12:07, Dmitry Baryshkov wrote:
> >>> On Fri, 30 Jun 2023 at 11:19, Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> 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@xxxxxxxxxx>
> >>>>> ---
> >>>> [...]
> >>>>>
> >>>>> + 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 {
> >>>
> >>>
> >>>
> >
> >
> >
--
With best wishes
Dmitry
