linux-kernel - Re: [PATCH v1 3/4] arm64: dts: qcom: Add PCIe 3 support for HAMOA-IOT-SOM platform

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <cncyo6y47anbyi434inelfl5czvgscjezejtzii4kihffkj2hj@e5jvvk4o5l7x>
Date: Mon, 27 Oct 2025 10:57:13 -0500
From: Bjorn Andersson <andersson@...nel.org>
To: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
Cc: Ziyue Zhang <ziyue.zhang@....qualcomm.com>, konradybcio@...nel.org, 
	robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, jingoohan1@...il.com, 
	mani@...nel.org, lpieralisi@...nel.org, kwilczynski@...nel.org, 
	bhelgaas@...gle.com, johan+linaro@...nel.org, vkoul@...nel.org, kishon@...nel.org, 
	neil.armstrong@...aro.org, abel.vesa@...aro.org, kw@...ux.com, 
	linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-pci@...r.kernel.org, linux-phy@...ts.infradead.org, qiang.yu@....qualcomm.com, 
	quic_krichai@...cinc.com, quic_vbadigan@...cinc.com
Subject: Re: [PATCH v1 3/4] arm64: dts: qcom: Add PCIe 3 support for
 HAMOA-IOT-SOM platform

On Tue, Oct 14, 2025 at 11:13:42AM +0530, Krishna Chaitanya Chundru wrote:
> 
> 
> On 9/22/2025 1:25 PM, Ziyue Zhang wrote:
> > Update the HAMOA-IOT-SOM device tree to enable PCIe 3 support. Add perst
> > wake and clkreq sideband signals and required regulators in PCIe3
> > controller and PHY device tree node.

The commit message should answer the questions I pose below. This
message explains what you change, but it doesn't explain why.

Start your commit message by describing the hardware, then follow that
with the description of your change.

> > 
> > Signed-off-by: Ziyue Zhang <ziyue.zhang@....qualcomm.com
> > ---
> >   arch/arm64/boot/dts/qcom/hamoa-iot-som.dtsi | 70 +++++++++++++++++++++
> >   1 file changed, 70 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/hamoa-iot-som.dtsi b/arch/arm64/boot/dts/qcom/hamoa-iot-som.dtsi
> > index 0c8ae34c1f37..7486204a4a46 100644
> > --- a/arch/arm64/boot/dts/qcom/hamoa-iot-som.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/hamoa-iot-som.dtsi
> > @@ -390,6 +390,53 @@ &gpu_zap_shader {
> >   	firmware-name = "qcom/x1e80100/gen70500_zap.mbn";
> >   };
> > +&pm8550ve_8_gpios {
> > +	pcie_x8_12v: pcie-12v-default-state {
> > +		pins = "gpio8";
> > +		function = "normal";
> > +		output-enable;
> > +		output-high;
> > +		bias-pull-down;
> > +		power-source = <0>;
> > +	};
> > +};
> > +
> > +&pmc8380_3_gpios {
> > +	pm_sde7_aux_3p3_en: pcie-aux-3p3-default-state {
> > +		pins = "gpio8";
> > +		function = "normal";
> > +		output-enable;
> > +		output-high;
> > +		bias-pull-down;
> > +		power-source = <0>;
> > +	};
> > +
> > +	pm_sde7_main_3p3_en: pcie-main-3p3-default-state {
> > +		pins = "gpio6";
> > +		function = "normal";
> > +		output-enable;
> > +		output-high;
> > +		bias-pull-down;
> > +		power-source = <0>;
> > +	};
> > +};
> Either squash patch 3/4 with 4/4 or move these pin configuration to
> patch 4/4.
> 

Patch 3 defines properties for the SOM and patch 4 defines properties
for the EVK board, so the split sounds reasonable.

But looking at the details, why would the SOM define these states? Isn't
it the carrier board that contains the related regulators? If I use this
SOM in my design, does my board have to have a 12V supply to my x8 PCIe
that's being controlled by gpio8?


In other words, I think you're right.

Regards,
Bjorn

> - Krishna Chaitanya.
> > +
> > +&pcie3 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&pcie3_default>;
> > +	perst-gpios = <&tlmm 143 GPIO_ACTIVE_LOW>;
> > +	wake-gpios = <&tlmm 145 GPIO_ACTIVE_LOW>;
> > +
> > +	status = "okay";
> > +};
> > +
> > +&pcie3_phy {
> > +	vdda-phy-supply = <&vreg_l3c_0p8>;
> > +	vdda-pll-supply = <&vreg_l3e_1p2>;
> > +
> > +	status = "okay";
> > +};
> > +
> >   &pcie4 {
> >   	perst-gpios = <&tlmm 146 GPIO_ACTIVE_LOW>;
> >   	wake-gpios = <&tlmm 148 GPIO_ACTIVE_LOW>;
> > @@ -471,6 +518,29 @@ &tlmm {
> >   	gpio-reserved-ranges = <34 2>, /* TPM LP & INT */
> >   			       <44 4>; /* SPI (TPM) */
> > +	pcie3_default: pcie3-default-state {
> > +		clkreq-n-pins {
> > +			pins = "gpio144";
> > +			function = "pcie3_clk";
> > +			drive-strength = <2>;
> > +			bias-pull-up;
> > +		};
> > +
> > +		perst-n-pins {
> > +			pins = "gpio143";
> > +			function = "gpio";
> > +			drive-strength = <2>;
> > +			bias-pull-down;
> > +		};
> > +
> > +		wake-n-pins {
> > +			pins = "gpio145";
> > +			function = "gpio";
> > +			drive-strength = <2>;
> > +			bias-pull-up;
> > +		};
> > +	};
> > +
> >   	pcie4_default: pcie4-default-state {
> >   		clkreq-n-pins {
> >   			pins = "gpio147";