On Mon, 3 Apr 2023 at 13:44, Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> wrote:
>
>
>
> 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.
While I agree with the USID part, the PERIPH_ADDR part is not always
like that, see pm8916.dtsi / audio-codec@f000.
I'll give it a thought, but it is probably not in the forthcoming future.
--
With best wishes
Dmitry
