| Message ID | 20230329000833.2507594-1-dmitry.baryshkov@linaro.org |
|---|---|
| Headers | show |
| Series | arm64: dts: qcom: sc8280xp: remove duplication in PMIC declarations | expand |
On 29.03.2023 02:08, Dmitry Baryshkov wrote: > pmk8350 can take different addresses on SPMI bus. Rather than having a > default SID, follow the pm8350's example and make the sid explicit when > specifying ADC channels. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad > .../bindings/iio/adc/qcom,spmi-vadc.yaml | 2 +- > .../bindings/thermal/qcom-spmi-adc-tm5.yaml | 4 +- > arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 2 +- > arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi | 2 +- > .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 4 +- > .../boot/dts/qcom/sm7225-fairphone-fp4.dts | 2 +- > .../dt-bindings/iio/qcom,spmi-adc7-pmk8350.h | 52 +++++++++---------- > 7 files changed, 32 insertions(+), 36 deletions(-) > > diff --git a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml > index bd6e0d6f6e0c..df317901e7d0 100644 > --- a/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml > +++ b/Documentation/devicetree/bindings/iio/adc/qcom,spmi-vadc.yaml > @@ -293,7 +293,7 @@ examples: > > /* Other properties are omitted */ > xo-therm@44 { > - reg = <PMK8350_ADC7_AMUX_THM1_100K_PU>; > + reg = <PMK8350_ADC7_AMUX_THM1_100K_PU(0)>; > qcom,ratiometric; > qcom,hw-settle-time = <200>; > }; > diff --git a/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml b/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml > index 52ec18cf1eda..ff07d27775dc 100644 > --- a/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml > +++ b/Documentation/devicetree/bindings/thermal/qcom-spmi-adc-tm5.yaml > @@ -218,7 +218,7 @@ examples: > > /* Other properties are omitted */ > xo-therm@44 { > - reg = <PMK8350_ADC7_AMUX_THM1_100K_PU>; > + reg = <PMK8350_ADC7_AMUX_THM1_100K_PU(0)>; > qcom,ratiometric; > qcom,hw-settle-time = <200>; > }; > @@ -240,7 +240,7 @@ examples: > > pmk8350-xo-therm@0 { > reg = <0>; > - io-channels = <&pmk8350_vadc PMK8350_ADC7_AMUX_THM1_100K_PU>; > + io-channels = <&pmk8350_vadc PMK8350_ADC7_AMUX_THM1_100K_PU(0)>; > qcom,decimation = <340>; > qcom,ratiometric; > qcom,hw-settle-time-us = <200>; > diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi > index 5dc9bee28e7f..14c9bdaa46ed 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dtsi > @@ -433,7 +433,7 @@ &pcie1_phy { > > &pmk8350_vadc { > pmk8350-die-temp@3 { > - reg = <PMK8350_ADC7_DIE_TEMP>; > + reg = <PMK8350_ADC7_DIE_TEMP(0)>; > label = "pmk8350_die_temp"; > qcom,pre-scaling = <1 1>; > }; > diff --git a/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi b/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi > index cb0cc2ba2fa3..e3919e074ebd 100644 > --- a/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi > +++ b/arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi > @@ -389,7 +389,7 @@ &pm8350c_pwm { > > &pmk8350_vadc { > pmk8350-die-temp@3 { > - reg = <PMK8350_ADC7_DIE_TEMP>; > + reg = <PMK8350_ADC7_DIE_TEMP(0)>; > label = "pmk8350_die_temp"; > qcom,pre-scaling = <1 1>; > }; > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts > index 46c7fdafb840..590400985055 100644 > --- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts > @@ -824,13 +824,13 @@ &pmk8280_vadc { > status = "okay"; > > pmic-die-temp@3 { > - reg = <PMK8350_ADC7_DIE_TEMP>; > + reg = <PMK8350_ADC7_DIE_TEMP(0)>; > qcom,pre-scaling = <1 1>; > label = "pmk8350_die_temp"; > }; > > xo-therm@44 { > - reg = <PMK8350_ADC7_AMUX_THM1_100K_PU>; > + reg = <PMK8350_ADC7_AMUX_THM1_100K_PU(0)>; > qcom,hw-settle-time = <200>; > qcom,ratiometric; > label = "pmk8350_xo_therm"; > diff --git a/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts b/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts > index 7ae6aba5d2ec..af6cf4fbddc7 100644 > --- a/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts > +++ b/arch/arm64/boot/dts/qcom/sm7225-fairphone-fp4.dts > @@ -516,7 +516,7 @@ &pmk8350_rtc { > > &pmk8350_vadc { > adc-chan@644 { > - reg = <PMK8350_ADC7_AMUX_THM1_100K_PU>; > + reg = <PMK8350_ADC7_AMUX_THM1_100K_PU(0)>; > qcom,ratiometric; > qcom,hw-settle-time = <200>; > qcom,pre-scaling = <1 1>; > diff --git a/include/dt-bindings/iio/qcom,spmi-adc7-pmk8350.h b/include/dt-bindings/iio/qcom,spmi-adc7-pmk8350.h > index 6c296870e95b..ca85a2d69453 100644 > --- a/include/dt-bindings/iio/qcom,spmi-adc7-pmk8350.h > +++ b/include/dt-bindings/iio/qcom,spmi-adc7-pmk8350.h > @@ -6,41 +6,37 @@ > #ifndef _DT_BINDINGS_QCOM_SPMI_VADC_PMK8350_H > #define _DT_BINDINGS_QCOM_SPMI_VADC_PMK8350_H > > -#ifndef PMK8350_SID > -#define PMK8350_SID 0 > -#endif > - > /* ADC channels for PMK8350_ADC for PMIC7 */ > -#define PMK8350_ADC7_REF_GND (PMK8350_SID << 8 | 0x0) > -#define PMK8350_ADC7_1P25VREF (PMK8350_SID << 8 | 0x01) > -#define PMK8350_ADC7_VREF_VADC (PMK8350_SID << 8 | 0x02) > -#define PMK8350_ADC7_DIE_TEMP (PMK8350_SID << 8 | 0x03) > +#define PMK8350_ADC7_REF_GND(sid) ((sid) << 8 | 0x0) > +#define PMK8350_ADC7_1P25VREF(sid) ((sid) << 8 | 0x01) > +#define PMK8350_ADC7_VREF_VADC(sid) ((sid) << 8 | 0x02) > +#define PMK8350_ADC7_DIE_TEMP(sid) ((sid) << 8 | 0x03) > > -#define PMK8350_ADC7_AMUX_THM1 (PMK8350_SID << 8 | 0x04) > -#define PMK8350_ADC7_AMUX_THM2 (PMK8350_SID << 8 | 0x05) > -#define PMK8350_ADC7_AMUX_THM3 (PMK8350_SID << 8 | 0x06) > -#define PMK8350_ADC7_AMUX_THM4 (PMK8350_SID << 8 | 0x07) > -#define PMK8350_ADC7_AMUX_THM5 (PMK8350_SID << 8 | 0x08) > +#define PMK8350_ADC7_AMUX_THM1(sid) ((sid) << 8 | 0x04) > +#define PMK8350_ADC7_AMUX_THM2(sid) ((sid) << 8 | 0x05) > +#define PMK8350_ADC7_AMUX_THM3(sid) ((sid) << 8 | 0x06) > +#define PMK8350_ADC7_AMUX_THM4(sid) ((sid) << 8 | 0x07) > +#define PMK8350_ADC7_AMUX_THM5(sid) ((sid) << 8 | 0x08) > > /* 30k pull-up1 */ > -#define PMK8350_ADC7_AMUX_THM1_30K_PU (PMK8350_SID << 8 | 0x24) > -#define PMK8350_ADC7_AMUX_THM2_30K_PU (PMK8350_SID << 8 | 0x25) > -#define PMK8350_ADC7_AMUX_THM3_30K_PU (PMK8350_SID << 8 | 0x26) > -#define PMK8350_ADC7_AMUX_THM4_30K_PU (PMK8350_SID << 8 | 0x27) > -#define PMK8350_ADC7_AMUX_THM5_30K_PU (PMK8350_SID << 8 | 0x28) > +#define PMK8350_ADC7_AMUX_THM1_30K_PU(sid) ((sid) << 8 | 0x24) > +#define PMK8350_ADC7_AMUX_THM2_30K_PU(sid) ((sid) << 8 | 0x25) > +#define PMK8350_ADC7_AMUX_THM3_30K_PU(sid) ((sid) << 8 | 0x26) > +#define PMK8350_ADC7_AMUX_THM4_30K_PU(sid) ((sid) << 8 | 0x27) > +#define PMK8350_ADC7_AMUX_THM5_30K_PU(sid) ((sid) << 8 | 0x28) > > /* 100k pull-up2 */ > -#define PMK8350_ADC7_AMUX_THM1_100K_PU (PMK8350_SID << 8 | 0x44) > -#define PMK8350_ADC7_AMUX_THM2_100K_PU (PMK8350_SID << 8 | 0x45) > -#define PMK8350_ADC7_AMUX_THM3_100K_PU (PMK8350_SID << 8 | 0x46) > -#define PMK8350_ADC7_AMUX_THM4_100K_PU (PMK8350_SID << 8 | 0x47) > -#define PMK8350_ADC7_AMUX_THM5_100K_PU (PMK8350_SID << 8 | 0x48) > +#define PMK8350_ADC7_AMUX_THM1_100K_PU(sid) ((sid) << 8 | 0x44) > +#define PMK8350_ADC7_AMUX_THM2_100K_PU(sid) ((sid) << 8 | 0x45) > +#define PMK8350_ADC7_AMUX_THM3_100K_PU(sid) ((sid) << 8 | 0x46) > +#define PMK8350_ADC7_AMUX_THM4_100K_PU(sid) ((sid) << 8 | 0x47) > +#define PMK8350_ADC7_AMUX_THM5_100K_PU(sid) ((sid) << 8 | 0x48) > > /* 400k pull-up3 */ > -#define PMK8350_ADC7_AMUX_THM1_400K_PU (PMK8350_SID << 8 | 0x64) > -#define PMK8350_ADC7_AMUX_THM2_400K_PU (PMK8350_SID << 8 | 0x65) > -#define PMK8350_ADC7_AMUX_THM3_400K_PU (PMK8350_SID << 8 | 0x66) > -#define PMK8350_ADC7_AMUX_THM4_400K_PU (PMK8350_SID << 8 | 0x67) > -#define PMK8350_ADC7_AMUX_THM5_400K_PU (PMK8350_SID << 8 | 0x68) > +#define PMK8350_ADC7_AMUX_THM1_400K_PU(sid) ((sid) << 8 | 0x64) > +#define PMK8350_ADC7_AMUX_THM2_400K_PU(sid) ((sid) << 8 | 0x65) > +#define PMK8350_ADC7_AMUX_THM3_400K_PU(sid) ((sid) << 8 | 0x66) > +#define PMK8350_ADC7_AMUX_THM4_400K_PU(sid) ((sid) << 8 | 0x67) > +#define PMK8350_ADC7_AMUX_THM5_400K_PU(sid) ((sid) << 8 | 0x68) > > #endif /* _DT_BINDINGS_QCOM_SPMI_VADC_PMK8350_H */
On 29.03.2023 02:08, Dmitry Baryshkov wrote: > Employ existing PMK8350_SID and switch sc8280xp-pmics to use > pmk8350.dtsi to reduce duplication and possible discrepancies. > > For example, this changes sc8280xp platforms to use qcom,pmk8350-pon > for the pon device compatibility rather than the incorrect > qcom,pm8998-pon. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- Bit of a loaded patch.. This could probably go like: 1. outer join 8280 on pmk8350 (for feature parity) 2. rename all labels in 8280 to 8350 (for 3.) 3. switch over to the actual 8350 file, remove 8280 (with Fixes:) [...] > +#define PMK8350_SID 0 > +#include "pmk8350.dtsi" > +#undef PMK8350_SID Not sure if this undef is necessary With or without all that though, the goal lgtm.. Konrad > > +&spmi_bus { > pmc8280_1: pmic@1 { > compatible = "qcom,pm8350", "qcom,spmi-pmic"; > reg = <0x1 SPMI_USID>;
On Wed, 29 Mar 2023 at 03:35, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > > > On 29.03.2023 02:29, Konrad Dybcio wrote: > > > > > > On 29.03.2023 02:27, Konrad Dybcio wrote: > >> > >> > >> On 29.03.2023 02:08, Dmitry Baryshkov wrote: > >>> Employ existing PMK8350_SID and switch sc8280xp-pmics to use > >>> pmk8350.dtsi to reduce duplication and possible discrepancies. > >>> > >>> For example, this changes sc8280xp platforms to use qcom,pmk8350-pon > >>> for the pon device compatibility rather than the incorrect > >>> qcom,pm8998-pon. > >>> > >>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > >>> --- > >> Bit of a loaded patch.. > >> > >> This could probably go like: > >> > >> 1. outer join 8280 on pmk8350 (for feature parity) > >> 2. rename all labels in 8280 to 8350 (for 3.) > >> 3. switch over to the actual 8350 file, remove 8280 (with Fixes:) > >> > >> [...] > >> > >>> +#define PMK8350_SID 0 > >>> +#include "pmk8350.dtsi" > >>> +#undef PMK8350_SID > >> Not sure if this undef is necessary > > It looks like it would be for multiple instances though > > > > Konrad > Also, it'd be a good idea to use interrupt-parent, as: > > 1) it would be a regression for OpenBSD and friends to remove it Ack, I forgot about interrupts/interrupts-extended. I'll fix that for v2. > 2) the interrupts=<> is dangerously long with SPMI I don't think we can do anything about this. > > Konrad > >> > >> With or without all that though, the goal lgtm.. > >> > >> Konrad > >> > >>> > >>> +&spmi_bus { > >>> pmc8280_1: pmic@1 { > >>> compatible = "qcom,pm8350", "qcom,spmi-pmic"; > >>> reg = <0x1 SPMI_USID>;
On 29/03/2023 02:08, Dmitry Baryshkov wrote: > pmk8350 can take different addresses on SPMI bus. Rather than having a > default SID, follow the pm8350's example and make the sid explicit when > specifying ADC channels. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
The sc8280xp platform uses its own copy of PMIC declarations. This can easily end up with the issues that are fixed in the main PMIC include file, but are not fixed for sc8280xp (and vice versa). For example commit c0ee8e0ba5cc ("arm64: dts: qcom: pmk8350: Use the correct PON compatible") changed pmk8350 to use "qcom,pmk8350-pon" compat for the PON device, while sc8280xp-pmic.dtsi still has the incorrect "qcom,pm8998-pon". Another example is pm8280_2_temp_alarm device, which uses interrupts tied to SID 2, while having SID 3. This can be easily left unnoticed. Employ a small amount of C preprocessor magic to make sc8280xp-pmics.dtsi use standard PMIC include files. Dmitry Baryshkov (6): dt-bindings: iio: qcom,spmi-adc7-pmk8350.h: include sid into defines arm64: dts: qcom: pmk8350: rename pon label arm64: dts: qcom: use main pmk8350.dtsi for sc8280xp platform arm64: dts: qcom: pm8350: include SID into labels arm64: dts: qcom: sc8280xp*: use pm8350.dtsi arm64: dts: qcom: sc8280xp*: use pm8350c.dtsi and pmr735a.dtsi .../bindings/iio/adc/qcom,spmi-vadc.yaml | 2 +- .../bindings/thermal/qcom-spmi-adc-tm5.yaml | 4 +- arch/arm64/boot/dts/qcom/pm8350.dtsi | 31 ++- arch/arm64/boot/dts/qcom/pmk8350.dtsi | 13 +- arch/arm64/boot/dts/qcom/sc7280-idp.dtsi | 2 +- arch/arm64/boot/dts/qcom/sc7280-qcard.dtsi | 2 +- arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 24 +- .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 52 ++--- arch/arm64/boot/dts/qcom/sc8280xp-pmics.dtsi | 210 ++---------------- .../boot/dts/qcom/sm7225-fairphone-fp4.dts | 2 +- arch/arm64/boot/dts/qcom/sm8350-mtp.dts | 8 +- .../dts/qcom/sm8350-sony-xperia-sagami.dtsi | 12 +- .../dts/qcom/sm8450-sony-xperia-nagara.dtsi | 8 +- .../dt-bindings/iio/qcom,spmi-adc7-pmk8350.h | 52 ++--- 14 files changed, 128 insertions(+), 294 deletions(-)