Message-Id: <DCBNOTQVTDWB.EH08W10NACXL@fairphone.com>
Date: Mon, 25 Aug 2025 18:40:41 +0200
From: "Luca Weiss" <luca.weiss@...rphone.com>
To: "Dmitry Baryshkov" <dmitry.baryshkov@....qualcomm.com>
Cc: "Konrad Dybcio" <konrad.dybcio@....qualcomm.com>, "Will Deacon"
 <will@...nel.org>, "Robin Murphy" <robin.murphy@....com>, "Joerg Roedel"
 <joro@...tes.org>, "Rob Herring" <robh@...nel.org>, "Krzysztof Kozlowski"
 <krzk+dt@...nel.org>, "Conor Dooley" <conor+dt@...nel.org>, "Rafael J.
 Wysocki" <rafael@...nel.org>, "Viresh Kumar" <viresh.kumar@...aro.org>,
 "Manivannan Sadhasivam" <mani@...nel.org>, "Herbert Xu"
 <herbert@...dor.apana.org.au>, "David S. Miller" <davem@...emloft.net>,
 "Vinod Koul" <vkoul@...nel.org>, "Bjorn Andersson" <andersson@...nel.org>,
 "Konrad Dybcio" <konradybcio@...nel.org>, "Robert Marko"
 <robimarko@...il.com>, "Das Srinagesh" <quic_gurus@...cinc.com>, "Thomas
 Gleixner" <tglx@...utronix.de>, "Jassi Brar" <jassisinghbrar@...il.com>,
 "Amit Kucheria" <amitk@...nel.org>, "Thara Gopinath"
 <thara.gopinath@...il.com>, "Daniel Lezcano" <daniel.lezcano@...aro.org>,
 "Zhang Rui" <rui.zhang@...el.com>, "Lukasz Luba" <lukasz.luba@....com>,
 "Ulf Hansson" <ulf.hansson@...aro.org>,
 <~postmarketos/upstreaming@...ts.sr.ht>, <phone-devel@...r.kernel.org>,
 <linux-arm-kernel@...ts.infradead.org>, <iommu@...ts.linux.dev>,
 <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
 <linux-pm@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>,
 <linux-crypto@...r.kernel.org>, <dmaengine@...r.kernel.org>,
 <linux-mmc@...r.kernel.org>
Subject: Re: [PATCH v2 14/15] arm64: dts: qcom: Add initial Milos dtsi

On Mon Aug 25, 2025 at 6:36 PM CEST, Dmitry Baryshkov wrote:
> On Mon, Aug 25, 2025 at 05:53:53PM +0200, Luca Weiss wrote:
>> Hi Dmitry,
>> 
>> On Wed Aug 20, 2025 at 1:52 PM CEST, Dmitry Baryshkov wrote:
>> > On Wed, Aug 20, 2025 at 10:42:09AM +0200, Luca Weiss wrote:
>> >> Hi Konrad,
>> >> 
>> >> On Sat Aug 2, 2025 at 2:04 PM CEST, Konrad Dybcio wrote:
>> >> > On 7/29/25 8:49 AM, Luca Weiss wrote:
>> >> >> Hi Konrad,
>> >> >> 
>> >> >> On Thu Jul 17, 2025 at 11:46 AM CEST, Luca Weiss wrote:
>> >> >>> Hi Konrad,
>> >> >>>
>> >> >>> On Thu Jul 17, 2025 at 10:29 AM CEST, Luca Weiss wrote:
>> >> >>>> On Mon Jul 14, 2025 at 1:06 PM CEST, Konrad Dybcio wrote:
>> >> >>>>> On 7/13/25 10:05 AM, Luca Weiss wrote:
>> >> >>>>>> Add a devicetree description for the Milos SoC, which is for example
>> >> >>>>>> Snapdragon 7s Gen 3 (SM7635).
>> >> >>>>>>
>> >> >>>>>> Signed-off-by: Luca Weiss <luca.weiss@...rphone.com>
>> >> >>>>>> ---
>> >> >>>>>
>> >> >>>>> [...]
>> >> >>>>>> +
>> >> >>>>>> +		spmi_bus: spmi@...0000 {
>> >> >>>>>> +			compatible = "qcom,spmi-pmic-arb";
>> >> >>>>>
>> >> >>>>> There's two bus instances on this platform, check out the x1e binding
>> >> >>>>
>> >> >>>> Will do
>> >> >>>
>> >> >>> One problem: If we make the labels spmi_bus0 and spmi_bus1 then we can't
>> >> >>> reuse the existing PMIC dtsi files since they all reference &spmi_bus.
>> >> >>>
>> >> >>> On FP6 everything's connected to PMIC_SPMI0_*, and PMIC_SPMI1_* is not
>> >> >>> connected to anything so just adding the label spmi_bus on spmi_bus0
>> >> >>> would be fine.
>> >> >>>
>> >> >>> Can I add this to the device dts? Not going to be pretty though...
>> >> >>>
>> >> >>> diff --git a/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts b/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts
>> >> >>> index d12eaa585b31..69605c9ed344 100644
>> >> >>> --- a/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts
>> >> >>> +++ b/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts
>> >> >>> @@ -11,6 +11,9 @@
>> >> >>>  #include <dt-bindings/pinctrl/qcom,pmic-gpio.h>
>> >> >>>  #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
>> >> >>>  #include "milos.dtsi"
>> >> >>> +
>> >> >>> +spmi_bus: &spmi_bus0 {};
>> >> >>> +
>> >> >>>  #include "pm7550.dtsi"
>> >> >>>  #include "pm8550vs.dtsi"
>> >> >>>  #include "pmiv0104.dtsi" /* PMIV0108 */
>> >> >>>
>> >> >>> Or I can add a second label for the spmi_bus0 as 'spmi_bus'. Not sure
>> >> >>> other designs than SM7635 recommend using spmi_bus1 for some stuff.
>> >> >>>
>> >> >>> But I guess longer term we'd need to figure out a solution to this, how
>> >> >>> to place a PMIC on a given SPMI bus, if reference designs start to
>> >> >>> recommend putting different PMIC on the separate busses.
>> >> >> 
>> >> >> Any feedback on this regarding the spmi_bus label?
>> >> >
>> >> > I had an offline chat with Bjorn and we only came up with janky
>> >> > solutions :)
>> >> >
>> >> > What you propose works well if the PMICs are all on bus0, which is
>> >> > not the case for the newest platforms. If some instances are on bus0
>> >> > and others are on bus1, things get ugly really quick and we're going
>> >> > to drown in #ifdefs.
>> >> >
>> >> >
>> >> > An alternative that I've seen downstream is to define PMIC nodes in
>> >> > the root of a dtsi file (not in the root of DT, i.e. NOT under / { })
>> >> > and do the following:
>> >> >
>> >> > &spmi_busN {
>> >> > 	#include "pmABCDX.dtsi"
>> >> > };
>> >> >
>> >> > Which is "okay", but has the visible downside of having to define the
>> >> > temp alarm thermal zone in each board's DT separately (and doing
>> >> > mid-file includes which is.. fine I guess, but also something we avoided
>> >> > upstream for the longest time)
>> >> >
>> >> >
>> >> > Both are less than ideal when it comes to altering the SID under
>> >> > "interrupts", fixing that would help immensely. We were hoping to
>> >> > leverage something like Johan's work on drivers/mfd/qcom-pm8008.c,
>> >> > but that seems like a longer term project.
>> >> >
>> >> > Please voice your opinions
>> >> 
>> >> Since nobody else jumped in, how can we continue?
>> >> 
>> >> One janky solution in my mind is somewhat similar to the PMxxxx_SID
>> >> defines, doing something like "#define PM7550_SPMI spmi_bus0" and then
>> >> using "&PM7550_SPMI {}" in the dtsi. I didn't try it so not sure that
>> >> actually works but something like this should I imagine.
>> >> 
>> >> But fortunately my Milos device doesn't have the problem that it
>> >> actually uses both SPMI busses for different PMICs, so similar to other
>> >> SoCs that already have two SPMI busses, I could somewhat ignore the
>> >> problem and let someone else figure out how to actually place PMICs on
>> >> spmi_bus0 and spmi_bus1 if they have such a hardware.
>> >
>> > I'd say, ignore it for now.
>> 
>> You mean ignoring that there's a second SPMI bus on this SoC, and just
>> modelling one with the label "spmi_bus"? Or something else?
>> 
>> 
>> I have also actually tried out the C define solution that I was writing
>> about in my previous email and this is actually working, see diff below.
>> In my opinion it just expands on what we have with the SID defines, so
>> shouldn't be tooo unacceptable :)
>
> I think we tried previously using C preprocessor to rework SID handling
> and it wasn't accepted by DT maintainers.

I don't know anything about that, but yeah...

>
> I'd say, ignore the second bus for now, unless it gets actually used for
> major PMICs.

The only 'problem' with this is once we do figure out a solution, the
SoC bindings will change, so both dt-bindings and dtsi needs to be
updated. But that's the case also for sm8550 and friends that currently
ignore the second SPMI bus upstream.

On FP6 again, it's definitely not a problem since everything's just on
the first SPMI bus anyways.

So then I'll revert the change to compatible = "qcom,milos-spmi-pmic-arb", "qcom,x1e80100-spmi-pmic-arb";
plus associated subnodes for the next revision.

Regards
Luca

Powered by blists - more mailing lists