| Message ID | 20230401220810.3563708-22-dmitry.baryshkov@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: dts: qcom: remove duplication in PMIC declarations | expand |
On 02/04/2023 00:08, Dmitry Baryshkov wrote: > Supporting SIDs greater than 9 required additional handling in order to > properly generatae hex values. Apply this customization to pm8150.dtsi. > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > arch/arm64/boot/dts/qcom/pm8150.dtsi | 16 ++++++++-------- > arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi | 6 ++++++ > arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi | 6 ++++++ > 3 files changed, 20 insertions(+), 8 deletions(-) > > diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi > index 83a2bada48ff..f3743ef3aa13 100644 > --- a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi > +++ b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi > @@ -11,6 +11,12 @@ > > #undef NODE > > +#undef HEX > +#undef _HEX > + > +#undef PMIC_SID_HEX > +#undef PMIC_SID1_HEX > + > #undef PMIC_SID > #undef PMIC_SID1 > #undef PMIC_LABEL Same comment as for previous patches - all undefs must be gone. Maybe I should not have acked all these changes customized SID ("include sid into defines") because it looks like it opened can of worms. Best regards, Krzysztof
On 2.04.2023 11:51, Krzysztof Kozlowski wrote: > On 02/04/2023 00:08, Dmitry Baryshkov wrote: >> Supporting SIDs greater than 9 required additional handling in order to >> properly generatae hex values. Apply this customization to pm8150.dtsi. >> >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> >> --- >> arch/arm64/boot/dts/qcom/pm8150.dtsi | 16 ++++++++-------- >> arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi | 6 ++++++ >> arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi | 6 ++++++ >> 3 files changed, 20 insertions(+), 8 deletions(-) >> > >> diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi >> index 83a2bada48ff..f3743ef3aa13 100644 >> --- a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi >> +++ b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi >> @@ -11,6 +11,12 @@ >> >> #undef NODE >> >> +#undef HEX >> +#undef _HEX >> + >> +#undef PMIC_SID_HEX >> +#undef PMIC_SID1_HEX All decimal numbers can be represented as hex numbers.. Is there any point to keeping them separate? Konrad >> + >> #undef PMIC_SID >> #undef PMIC_SID1 >> #undef PMIC_LABEL > > Same comment as for previous patches - all undefs must be gone. > > Maybe I should not have acked all these changes customized SID ("include > sid into defines") because it looks like it opened can of worms. > > Best regards, > Krzysztof >
On Mon, 3 Apr 2023 at 13:35, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: > > > > On 2.04.2023 11:51, Krzysztof Kozlowski wrote: > > On 02/04/2023 00:08, Dmitry Baryshkov wrote: > >> Supporting SIDs greater than 9 required additional handling in order to > >> properly generatae hex values. Apply this customization to pm8150.dtsi. > >> > >> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > >> --- > >> arch/arm64/boot/dts/qcom/pm8150.dtsi | 16 ++++++++-------- > >> arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi | 6 ++++++ > >> arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi | 6 ++++++ > >> 3 files changed, 20 insertions(+), 8 deletions(-) > >> > > > >> diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi > >> index 83a2bada48ff..f3743ef3aa13 100644 > >> --- a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi > >> +++ b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi > >> @@ -11,6 +11,12 @@ > >> > >> #undef NODE > >> > >> +#undef HEX > >> +#undef _HEX > >> + > >> +#undef PMIC_SID_HEX > >> +#undef PMIC_SID1_HEX > All decimal numbers can be represented as hex numbers.. > Is there any point to keeping them separate? Yes, to have pmic@c rather than pmic@12 for USID = 12 = 0xc. > > Konrad > >> + > >> #undef PMIC_SID > >> #undef PMIC_SID1 > >> #undef PMIC_LABEL > > > > Same comment as for previous patches - all undefs must be gone. This means that we can not include two copies of the same PMIC (which do have on both platforms). > > > > Maybe I should not have acked all these changes customized SID ("include > > sid into defines") because it looks like it opened can of worms. I think this can of worms is still better than imperfect duplication.
On 03/04/2023 13:45, Dmitry Baryshkov wrote: >> Konrad >>>> + >>>> #undef PMIC_SID >>>> #undef PMIC_SID1 >>>> #undef PMIC_LABEL >>> >>> Same comment as for previous patches - all undefs must be gone. > > This means that we can not include two copies of the same PMIC (which > do have on both platforms). Consider spi15 and spi16: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sm8250.dtsi?h=v6.3-rc5&id=7e364e56293bb98cae1b55fd835f5991c4e96e7d#n1045 Do you see it written as #include "qcom-sm8250-spi.dtsi" with parametrizing the reg/unit address, interrupts etc? No. Neither PMIC should be. It is not a special device. Best regards, Krzysztof
On Mon, 3 Apr 2023 at 15:57, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 03/04/2023 13:45, Dmitry Baryshkov wrote: > >> Konrad > >>>> + > >>>> #undef PMIC_SID > >>>> #undef PMIC_SID1 > >>>> #undef PMIC_LABEL > >>> > >>> Same comment as for previous patches - all undefs must be gone. > > > > This means that we can not include two copies of the same PMIC (which > > do have on both platforms). > > Consider spi15 and spi16: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/qcom/sm8250.dtsi?h=v6.3-rc5&id=7e364e56293bb98cae1b55fd835f5991c4e96e7d#n1045 > > Do you see it written as #include "qcom-sm8250-spi.dtsi" with > parametrizing the reg/unit address, interrupts etc? > > No. Neither PMIC should be. It is not a special device. I think there should be balance. PMICs are complex structures. Possibly schema will help here once it is in a more enforced mode.
diff --git a/arch/arm64/boot/dts/qcom/pm8150.dtsi b/arch/arm64/boot/dts/qcom/pm8150.dtsi index 77bb325e425b..37cc99e5d1a6 100644 --- a/arch/arm64/boot/dts/qcom/pm8150.dtsi +++ b/arch/arm64/boot/dts/qcom/pm8150.dtsi @@ -58,7 +58,7 @@ trip2 { &spmi_bus { pmic@0 { compatible = "qcom,pm8150", "qcom,spmi-pmic"; - reg = <PMIC_SID SPMI_USID>; + reg = <PMIC_SID_HEX SPMI_USID>; #address-cells = <1>; #size-cells = <0>; @@ -70,7 +70,7 @@ pon: pon@800 { pon_pwrkey: pwrkey { compatible = "qcom,pm8941-pwrkey"; - interrupts = <PMIC_SID 0x8 0x0 IRQ_TYPE_EDGE_BOTH>; + interrupts = <PMIC_SID_HEX 0x8 0x0 IRQ_TYPE_EDGE_BOTH>; debounce = <15625>; bias-pull-up; linux,code = <KEY_POWER>; @@ -80,7 +80,7 @@ pon_pwrkey: pwrkey { pon_resin: resin { compatible = "qcom,pm8941-resin"; - interrupts = <PMIC_SID 0x8 0x1 IRQ_TYPE_EDGE_BOTH>; + interrupts = <PMIC_SID_HEX 0x8 0x1 IRQ_TYPE_EDGE_BOTH>; debounce = <15625>; bias-pull-up; @@ -91,7 +91,7 @@ pon_resin: resin { LABEL(temp): temp-alarm@2400 { compatible = "qcom,spmi-temp-alarm"; reg = <0x2400>; - interrupts = <PMIC_SID 0x24 0x0 IRQ_TYPE_EDGE_BOTH>; + interrupts = <PMIC_SID_HEX 0x24 0x0 IRQ_TYPE_EDGE_BOTH>; io-channels = <&LABEL(adc) ADC5_DIE_TEMP>; io-channel-names = "thermal"; #thermal-sensor-cells = <0>; @@ -103,7 +103,7 @@ LABEL(adc): adc@3100 { #address-cells = <1>; #size-cells = <0>; #io-channel-cells = <1>; - interrupts = <PMIC_SID 0x31 0x0 IRQ_TYPE_EDGE_RISING>; + interrupts = <PMIC_SID_HEX 0x31 0x0 IRQ_TYPE_EDGE_RISING>; ref-gnd@0 { reg = <ADC5_REF_GND>; @@ -127,7 +127,7 @@ die-temp@6 { LABEL(adc_tm): adc-tm@3500 { compatible = "qcom,spmi-adc-tm5"; reg = <0x3500>; - interrupts = <PMIC_SID 0x35 0x0 IRQ_TYPE_EDGE_RISING>; + interrupts = <PMIC_SID_HEX 0x35 0x0 IRQ_TYPE_EDGE_RISING>; #thermal-sensor-cells = <1>; #address-cells = <1>; #size-cells = <0>; @@ -138,7 +138,7 @@ rtc@6000 { compatible = "qcom,pm8941-rtc"; reg = <0x6000>, <0x6100>; reg-names = "rtc", "alarm"; - interrupts = <PMIC_SID 0x61 0x1 IRQ_TYPE_NONE>; + interrupts = <PMIC_SID_HEX 0x61 0x1 IRQ_TYPE_NONE>; }; LABEL(gpios): gpio@c000 { @@ -154,7 +154,7 @@ LABEL(gpios): gpio@c000 { pmic@PMIC_SID1 { compatible = "qcom,pm8150", "qcom,spmi-pmic"; - reg = <PMIC_SID1 SPMI_USID>; + reg = <PMIC_SID1_HEX SPMI_USID>; #address-cells = <1>; #size-cells = <0>; }; diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi index 83a2bada48ff..f3743ef3aa13 100644 --- a/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi +++ b/arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi @@ -11,6 +11,12 @@ #undef NODE +#undef HEX +#undef _HEX + +#undef PMIC_SID_HEX +#undef PMIC_SID1_HEX + #undef PMIC_SID #undef PMIC_SID1 #undef PMIC_LABEL diff --git a/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi b/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi index bb41c9387aba..640d1bf5ce8e 100644 --- a/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi +++ b/arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi @@ -18,3 +18,9 @@ #define __LABEL(pmic, name) pmic ## _ ## name #define NODE(name) PMIC_NODE ##-## name + +#define HEX(sid) _HEX(sid) +#define _HEX(sid) 0x## sid + +#define PMIC_SID_HEX HEX(PMIC_SID) +#define PMIC_SID1_HEX HEX(PMIC_SID1)
Supporting SIDs greater than 9 required additional handling in order to properly generatae hex values. Apply this customization to pm8150.dtsi. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- arch/arm64/boot/dts/qcom/pm8150.dtsi | 16 ++++++++-------- arch/arm64/boot/dts/qcom/pmic-dyn-footer.dtsi | 6 ++++++ arch/arm64/boot/dts/qcom/pmic-dyn-header.dtsi | 6 ++++++ 3 files changed, 20 insertions(+), 8 deletions(-)