On 2.04.2023 11:55, Krzysztof Kozlowski wrote: > On 02/04/2023 00:07, Dmitry Baryshkov wrote: >> 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 > > Preprocessor magic is disliked in DTS. We allow only simple defines, no > undefs. Sometimes some nodes or strings could be concatenated, but in > obvious way. You should not parametrize it and have different, generated > labels in DTS based on something coming external to that DTS. This again begs the question, is it time we start moving parts of the dts code to be autogenerated? Should we keep a separate file for each SID? Or should we consider the SPMI 'interrupts' implementation flawed and work towards one that does not require a SID to be specified within? Currently it's: interrupts = <USID PERIPH_ADDR>>8 IRQ_WITHIN_PERIPH IRQ_TYPE>; So the first two cells are effectively useless and can be retrieved from the parent node and the reg property. Getting rid of that would solve a decent chunk of problems that this patchset concerns. Konrad > > Best regards, > Krzysztof >