Re: [PATCH v4 4/4] arm64: dts: qcom: Add IMDT QCS8550 SBC

[Konrad Dybcio]

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@xxxxxxxxxxxx/

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