Re: [PATCH v2 1/1] arm64: dts: qcom: monaco-evk: Add Interface Plus Mezzanine

[Bjorn Andersson]

On Mon, Feb 23, 2026 at 02:12:05PM +0100, Krzysztof Kozlowski wrote:
> On 22/02/2026 18:35, Umang Chheda wrote:
> > The Interface Plus [IFP] Mezzanine is an hardware expansion add-on
> > board designed to be stacked on top of Monaco EVK.
> > 
> > It has following peripherals :
> > 
> > - 4x Type A USB ports in host mode.
> > - TC9563 PCIe switch, which has following three downstream ports (DSP) :
> >    - 1st DSP connects M.2 E-key connector for connecting WLAN endpoints.
> >    - 2nd DSP connects M.2 B-key connector for connecting cellular
> >      modems.
> >    - 3rd DSP with support for Dual Ethernet ports.
> > - EEPROM.
> > - LVDS Display.
> > - 2*mini DP.
> > 
> > Add support for following peripherals :
> > - TC9563 PCIe Switch.
> > - EEPROM.
> > 
> > Written with inputs from :
> >     Krishna Chaitanya Chundru <krishna.chundru@xxxxxxxxxxxxxxxx> - PCIe
> >     Monish Chunara <monish.chunara@xxxxxxxxxxxxxxxx> - EEPROM.
> > 
> > Signed-off-by: Umang Chheda <umang.chheda@xxxxxxxxxxxxxxxx>
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
> > ---
> >  arch/arm64/boot/dts/qcom/Makefile             |   4 +
> >  .../dts/qcom/monaco-evk-ifp-mezzanine.dtso    | 184 ++++++++++++++++++
> >  2 files changed, 188 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/qcom/monaco-evk-ifp-mezzanine.dtso
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile
> > index f80b5d9cf1e8..9d298e7e8a90 100644
> > --- a/arch/arm64/boot/dts/qcom/Makefile
> > +++ b/arch/arm64/boot/dts/qcom/Makefile
> > @@ -45,6 +45,10 @@ lemans-evk-el2-dtbs := lemans-evk.dtb lemans-el2.dtbo
> >  dtb-$(CONFIG_ARCH_QCOM)	+= lemans-evk-el2.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= milos-fairphone-fp6.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= monaco-evk.dtb
> > +
> > +monaco-evk-ifp-mezzanine-dtbs	:= monaco-evk.dtb monaco-evk-ifp-mezzanine.dtbo
> > +
> > +dtb-$(CONFIG_ARCH_QCOM)	+= monaco-evk-ifp-mezzanine.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= msm8216-samsung-fortuna3g.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-acer-a1-724.dtb
> >  dtb-$(CONFIG_ARCH_QCOM)	+= msm8916-alcatel-idol347.dtb
> > diff --git a/arch/arm64/boot/dts/qcom/monaco-evk-ifp-mezzanine.dtso b/arch/arm64/boot/dts/qcom/monaco-evk-ifp-mezzanine.dtso
> > new file mode 100644
> > index 000000000000..f0572647200c
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/monaco-evk-ifp-mezzanine.dtso
> > @@ -0,0 +1,184 @@
> > +// SPDX-License-Identifier: BSD-3-Clause
> > +/*
> > + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> > + */
> > +
> > +/dts-v1/;
> > +/plugin/;
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +
> > +&{/} {
> > +	model = "Qualcomm Technologies, Inc. Monaco-EVK IFP Mezzanine";
> > +
> > +	vreg_0p9: regulator-vreg-0p9 {
> 
> Please use name for all fixed regulators which matches current format
> recommendation: 'regulator-[0-9]v[0-9]'
> 
> https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml
> 
> Duplicating regulator name (regulator-reg(ulator)) is pointless.
> 

"pointless" is a strong word IMO.

The recommendation that has been communicated is to based label, name
and regulator-name of the schematics, but prefix the node name with
regulator- to achieve sensible sort order.


In fact naming these regulator-0v9, regulator-1v8, and regulator-3v3
make the name useless. We further have plenty of designs where there are
multiple regulator-1v8 and regulator-3v3.

I guess the preferred name, per the binding, is to not have multiple
3.3V regulators in your design?

> > +		compatible = "regulator-fixed";
> > +		regulator-name = "VREG_0P9";
> > +
> > +		regulator-min-microvolt = <900000>;
> > +		regulator-max-microvolt = <900000>;
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +
> > +		vin-supply = <&vreg_3p3>;
> > +	};
> > +
> > +	vreg_1p8: regulator-vreg-1p8 {
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "VREG_1P8";
> > +
> > +		regulator-min-microvolt = <1800000>;
> > +		regulator-max-microvolt = <1800000>;
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +
> > +		vin-supply = <&vreg_4p2>;
> > +	};
> > +
> > +	vreg_3p3: regulator-vreg-3p3 {
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "VREG_3P3";
> > +
> > +		regulator-min-microvolt = <3300000>;
> > +		regulator-max-microvolt = <3300000>;
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +
> > +		vin-supply = <&vreg_4p2>;
> > +	};
> > +
> > +	vreg_4p2: regulator-vreg-4p2 {
> 
> Unused node (other dummies don't really count).
> 

I'm pretty sure this is a direct result of previous review feedback
requiring these to be added. I do agree that they don't add any value
in a system were we don't control the entire power grid anyways.


So I presume what you're saying is that we should at most declare one
level of non-controlled fixed regulators?

Regards,
Bjorn

> > +		compatible = "regulator-fixed";
> > +		regulator-name = "VREG_4P2";
> > +
> > +		regulator-min-microvolt = <4200000>;
> > +		regulator-max-microvolt = <4200000>;
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +
> > +		vin-supply = <&vreg_sys_pwr>;
> > +	};
> > +
> > +	vreg_sys_pwr: regulator-vreg-sys-pwr {
> 
> What is the point of this regulator? It is not used by anything (another
> dummy is not considered an user).
> 
> > +		compatible = "regulator-fixed";
> > +		regulator-name = "VREG_SYS_PWR";
> > +
> > +		regulator-min-microvolt = <24000000>;
> > +		regulator-max-microvolt = <24000000>;
> > +		regulator-always-on;
> > +		regulator-boot-on;
> > +	};
> > +};
> > +
> 
> 
> Best regards,
> Krzysztof