Devicetree
 help / color / mirror / Atom feed
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
To: William Bright <william.bright@imd-tec.com>,
	Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Konrad Dybcio <konradybcio@kernel.org>
Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH v4 4/4] arm64: dts: qcom: Add IMDT QCS8550 SBC
Date: Fri, 26 Jun 2026 17:18:20 +0200	[thread overview]
Message-ID: <8afbc9f9-15ff-4b60-9a0f-845f903d3c9a@oss.qualcomm.com> (raw)
In-Reply-To: <20260610-imdt-qcs8550-sbc-rfc-v4-4-358e71d606bc@imd-tec.com>

On 6/10/26 10:57 AM, William Bright wrote:
> The IMDT QCS8550 SBC is a two-board design from IMD Technologies Ltd
> built around the Qualcomm QCS8550 SoC. An IMDT QCS8550 SoM is soldered
> onto the IMDT QCS8550 carrier board that supplies VPH_PWR and exposes
> the off-module peripherals.

[...]

> +	/* Enables 5V_PER, 3V3_PER and 1V8_PER rails. These rails
> +	 * aren't used by anything within the device-tree but are used
> +	 * for on board logic level conversion and as rails for
> +	 * pull-ups.
> +	 */
> +	per_pwr: regulator-per-pwr {
> +		compatible = "regulator-fixed";
> +		regulator-name = "per_pwr";
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pwr_per_en_default>;

style nit:

property-n
property-names

file-wide

[...]

> +&apps_rsc {
> +	regulators-0 {

This way of overriding that is super shaky - instead, use a label,
like:

--- som.dtsi
pm8550_rpmh_regulators: regulators-0 {
...

--- sbc.dts
&pm8550_rpmh_regulators {
	foo = bar;
};

[...]

> +&gpu_zap_shader {
> +	firmware-name = "qcom/sm8550/a740_zap.mbn";
> +	/* Zap shader doesn't load so is disabled */

If your SoC is production fused, you must provide your own ZAP firmware
that's signed by you. Conversely, if you have a software stack that
does not include the Gunyah hypervisor, this is likely not necessary
> +	status = "disabled";
> +};
> +
> +&i2c_master_hub_0 {
> +	status = "okay";
> +};
> +
> +&i2c_hub_2 {
> +	clock-frequency = <400000>;
> +	status = "okay";

nit: let's keep a \n before status, everywhere

[...]

> +&pcie0 {
> +	wake-gpios = <&tlmm 96 GPIO_ACTIVE_HIGH>;
> +	perst-gpios = <&tlmm 94 GPIO_ACTIVE_LOW>;
> +
> +	/*
> +	 * pcie0 hosts the M.2 Key-E slot. Apply the SDIO
> +	 * reset de-assert here so any module's chip enable is settled
> +	 * before pcie0 trains its link.
> +	 */

We now have:

Documentation/devicetree/bindings/connector/pcie-m2-e-connector.yaml

which may come in useful here

[...]

> +&pcie1 {
> +	wake-gpios = <&tlmm 99 GPIO_ACTIVE_HIGH>;
> +	perst-gpios = <&tlmm 97 GPIO_ACTIVE_LOW>;
> +
> +	/*
> +	 * pcie_switch_sel_default and gbe_reset_default are board-init
> +	 * lines that must be stable before pcie1 trains its link: the
> +	 * PCIe switch needs its mode-select strap settled, and the
> +	 * downstream LAN743x must be out of reset to enumerate.
> +	 * Applying them via pcie1's pinctrl-0 fires them during
> +	 * qcom-pcie probe, before bus enumeration.
> +	 */
> +	pinctrl-0 = <&pcie1_default_state>,
> +		    <&pcie_switch_sel_default>,
> +		    <&gbe_reset_default>;
> +	pinctrl-names = "default";
> +
> +	status = "okay";
> +};

[...]

> +	pwr_per_en_default: pwr-per-en-default-state {
> +		pwr-per-en-pins {
> +			pins = "gpio142";
> +			function = "gpio";
> +			drive-strength = <16>;
> +			bias-disable;
> +		};
> +	};

For single-group pin state definitions, you can skip the inner
level and define the properties directly under the -state {} node


> +
> +	sd_vset_default: sd-vset-default-state {
> +		sd-vset-pins {
> +			pins = "gpio4";
> +			function = "gpio";
> +			drive-strength = <16>;
> +			bias-disable;
> +		};
> +	};
> +
> +	/*
> +	 * Drive LAN743x reset high (de-asserted) when pcie1 probes,
> +	 * so the PHY enumerates on the bus.
> +	 */
> +	gbe_reset_default: gbe-reset-default-state {
> +		pins = "gpio138";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-disable;
> +		output-high;
> +	};
> +
> +	/*
> +	 * We drive this GPIO physically high on the M2 Key-E connector
> +	 * to make sure the module is enabled. An M2 Key-E module could
> +	 * be using this pin as a chip enable.
> +	 */
> +	m2e_sdio_resetn_default: m2e-sdio-resetn-default-state {
> +		pins = "gpio41";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-disable;
> +		output-high;
> +	};
> +
> +	/* Force the on-board PCIe switch to select the GbE upstream
> +	 * port.
> +	 */
> +	pcie_switch_sel_default: pcie-switch-sel-default-state {
> +		pins = "gpio16";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-disable;
> +		output-low;
> +	};

Normally this would be handled via an actual driver - see e.g.

Documentation/devicetree/bindings/pci/toshiba,tc9563.yaml
https://lore.kernel.org/linux-arm-msm/20260605010022.968612-1-elder@riscstar.com/

Konrad

> +};
> +
> +&uart7 {
> +	status = "okay";
> +};
> +
> +&ufs_mem_hc {
> +	reset-gpios = <&tlmm 210 GPIO_ACTIVE_LOW>;
> +
> +	vcc-supply = <&vreg_l17b_2p5>;
> +	vcc-max-microamp = <1300000>;
> +	vccq-supply = <&vreg_l1g_1p2>;
> +	vccq-max-microamp = <1200000>;
> +	vdd-hba-supply = <&vreg_l3g_1p2>;
> +
> +	status = "okay";
> +};
> +
> +&ufs_mem_phy {
> +	vdda-phy-supply = <&vreg_l1d_0p88>;
> +	vdda-pll-supply = <&vreg_l3e_1p2>;
> +
> +	status = "okay";
> +};
> +
> +&usb_1 {
> +	/delete-property/ usb-role-switch;
> +	dr_mode = "peripheral";

Is it really peripheral-only?

Konrad

  parent reply	other threads:[~2026-06-26 15:18 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-10  8:57 [PATCH v4 0/4] arm64: dts: qcom: Add IMDT QCS8550 SBC William Bright
2026-06-10  8:57 ` [PATCH v4 1/4] dt-bindings: vendor-prefixes: Add IMDT William Bright
2026-06-10  8:57 ` [PATCH v4 2/4] dt-bindings: arm: qcom: Document IMDT QCS8550 SBC/SoM William Bright
2026-06-10  8:57 ` [PATCH v4 3/4] arm64: dts: qcom: Add IMDT QCS8550 SoM William Bright
2026-06-26 15:08   ` Konrad Dybcio
2026-06-10  8:57 ` [PATCH v4 4/4] arm64: dts: qcom: Add IMDT QCS8550 SBC William Bright
2026-06-10  9:05   ` sashiko-bot
2026-06-26 15:18   ` Konrad Dybcio [this message]
2026-06-29 10:25     ` William Bright
2026-06-29 14:03       ` Konrad Dybcio
2026-06-30 15:04         ` William Bright
2026-06-24 13:34 ` [PATCH v4 0/4] " William Bright

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8afbc9f9-15ff-4b60-9a0f-845f903d3c9a@oss.qualcomm.com \
    --to=konrad.dybcio@oss.qualcomm.com \
    --cc=andersson@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=konradybcio@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=william.bright@imd-tec.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox