Re: [PATCH v3 6/6] arm64: qcom: monaco: Add PCIe Endpoint nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] arm64: dts: qcom: monaco: disallowed input-enable in pcie ep
  tlmm states
- [Low] arm64: dts: qcom: monaco: board-specific PCIe EP GPIOs and
  pinctrl in SoC dtsi
--

commit e2e513d816ae9e8683a2141418e5f09297f8dbd2
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

arm64: qcom: monaco: Add PCIe Endpoint nodes

This commit adds DeviceTree nodes and TLMM pin configurations for the two
PCIe Endpoint controllers on Qualcomm Monaco (QCS8300).

> diff --git a/arch/arm64/boot/dts/qcom/monaco.dtsi b/arch/arm64/boot/dts/qcom/monaco.dtsi
> index f27f0953ab9e0..4a1325aaf0726 100644
> --- a/arch/arm64/boot/dts/qcom/monaco.dtsi
> +++ b/arch/arm64/boot/dts/qcom/monaco.dtsi

[ ... ]

> @@ -2406,6 +2407,57 @@ pcieport0: pcie@0 {
>  			};
>  		};
>  
> +		pcie0_ep: pcie-ep@1c00000 {

[ ... ]

> +			phys = <&pcie0_phy>;
> +			phy-names = "pciephy";
> +			reset-gpios = <&tlmm 2 GPIO_ACTIVE_LOW>;
> +			wake-gpios = <&tlmm 0 GPIO_ACTIVE_LOW>;
> +			pinctrl-0 = <&pcie0_ep_default_state>;
> +			pinctrl-names = "default";
> +			num-lanes = <2>;
> +			linux,pci-domain = <0>;
> +
> +			status = "disabled";
> +		};

[Severity: Low]
Should reset-gpios, wake-gpios, and the GPIO pin configurations in
pcie0_ep_default_state and pcie1_ep_default_state live in board DTS files
rather than monaco.dtsi?

Only gpio1 (pcie0_clkreq) and gpio22 (pcie1_clkreq) have dedicated PCIe
pinmux functions in the QCS8300 TLMM.  The PERST# and optional WAKE# pins
(gpio0, gpio2, gpio21, and gpio23) use generic GPIO muxing that varies across
Monaco boards, and the Root Complex pcie0 and pcie1 nodes in monaco.dtsi leave
those properties to the board files.

In addition, perst-pins in pcie0_ep_default_state and pcie1_ep_default_state
sets bias-pull-up on the GPIO_ACTIVE_LOW reset inputs, which pulls an
undriven PERST# line into the deasserted state and triggers link training in
qcom_pcie_ep_perst_irq_thread() instead of keeping reset asserted with
bias-pull-down.  If any pinctrl nodes remain under tlmm in monaco.dtsi, they
should also be placed in alphabetical order after cam0-default-state.

[ ... ]

> @@ -6239,6 +6342,50 @@ tlmm: pinctrl@f100000 {
>  			#interrupt-cells = <2>;
>  			wakeup-parent = <&pdc>;
>  
> +			pcie0_ep_default_state: pcie0-ep-default-state {
> +				wake-pins {
> +					pins = "gpio0";
> +					function = "gpio";
> +					drive-strength = <2>;
> +				};
> +
> +				clkreq-pins {
> +					pins = "gpio1";
> +					function = "pcie0_clkreq";
> +					drive-strength = <2>;
> +					bias-pull-up;
> +				};
> +
> +				perst-pins {
> +					pins = "gpio2";
> +					function = "gpio";
> +					input-enable;
> +					bias-pull-up;
> +				};
> +			};
> +
> +			pcie1_ep_default_state: pcie1-ep-default-state {
> +				wake-pins {
> +					pins = "gpio21";
> +					function = "gpio";
> +					drive-strength = <2>;
> +				};
> +
> +				clkreq-pins {
> +					pins = "gpio22";
> +					function = "pcie1_clkreq";
> +					drive-strength = <2>;
> +					bias-pull-up;
> +				};
> +
> +				perst-pins {
> +					pins = "gpio23";
> +					function = "gpio";
> +					input-enable;
> +					bias-pull-up;
> +				};
> +			};

[Severity: Medium]
Does input-enable in perst-pins under pcie0_ep_default_state and
pcie1_ep_default_state break dtbs_check validation for Monaco boards?

Documentation/devicetree/bindings/pinctrl/qcom,qcs8300-tlmm.yaml validates
subnodes against qcom,tlmm-common.yaml#/$defs/qcom-tlmm-state, which sets
input-enable: false because Qualcomm TLMM pins do not have an input-buffer
enable bit.  Dropping input-enable avoids the schema validation failure.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114-monaco_ep-v3-0-7fe227cdfac0@oss.qualcomm.com?part=6