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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 23/02/2026 16:12, Bjorn Andersson wrote:
> 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@oss.qualcomm.com> - PCIe
>>>     Monish Chunara <monish.chunara@oss.qualcomm.com> - EEPROM.
>>>
>>> Signed-off-by: Umang Chheda <umang.chheda@oss.qualcomm.com>
>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>>> ---
>>>  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.

Pointless meaning it has no point, because the "vreg" is just redundant.
It gives no new information.


> 
> 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.

The regulator-name is to match schematics. Node name should follow DT
spec expectations to show the purpose of the node.

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

I don't see what you are proving here. The "vreg" middle name addon is
not differentiating multiple 3.3V regulators. It changes nothing in the
problem of this duplication.

> 
>>> +		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.

Maybe, I guess, but I am pretty certain none of DT maintainers ever
asked for such nodes.

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

In general, non-controller fixed regulators should not be there at all,
except when they serve certain purpose, like fulfill the binding
requirement. It's their only point.

And a chain of:

A -> B -> C -> device

is completely redundant if all A+B+C are non-controlled.




Best regards,
Krzysztof