Re: [PATCH v2 5/5] arm64: dts: qcom: qcs8550: add QCS8550 RB5Gen2 board support

[Konrad Dybcio]

On 4/7/26 5:46 PM, Joe Sandom via B4 Relay wrote:
> From: Joe Sandom <jsandom@xxxxxxxx>
> 
> The RB5gen2 is an embedded development platform for the
> QCS8550, based on the Snapdragon 8 Gen 2 SoC (SM8550).

[...]


> +	/* Lontium LT9611UXC fails FW upgrade and has timeouts with geni-i2c */
> +	/* Workaround is to use bit-banged I2C */

Interesting.. I was under the impression that it was only an issue on
RB1 and RB2 boards.. perhaps we're missing some magic register write..


[...]

> +		pinctrl-names = "default";
> +		pinctrl-0 = <&wlan_en>, <&bt_default>, <&sw_ctrl_default>,
> +			    <&pmk8550_sleep_clk>;

nit: let's keep the order of

property-n
property-names

file-wide

[...]

> +&sdhc_2 {
> +	cd-gpios = <&pm8550_gpios 12 GPIO_ACTIVE_LOW>;
> +
> +	pinctrl-0 = <&sdc2_default>, <&sdc2_card_det_n>;
> +	pinctrl-1 = <&sdc2_sleep>, <&sdc2_card_det_n>;
> +	pinctrl-names = "default", "sleep";
> +
> +	vmmc-supply = <&vreg_l9b_2p9>;
> +	vqmmc-supply = <&vreg_l8b_1p8>;
> +
> +	max-sd-hs-hz = <37000000>;

Are you sure you want to overwrite that? The value in the SoC DTSI is
set to half a MHz higher


> +
> +	no-sdio;
> +	no-mmc;
> +
> +	status = "okay";
> +};
> +
> +&sleep_clk {
> +	clock-frequency = <32764>;
> +};
> +
> +&spi11 {
> +	status = "okay";
> +
> +	can@0 {
> +		compatible = "microchip,mcp2518fd";
> +		reg = <0>;
> +		interrupts-extended = <&tlmm 55 IRQ_TYPE_LEVEL_LOW>;
> +		clocks = <&clk40m>;
> +		spi-max-frequency = <10000000>;
> +		vdd-supply = <&vreg_l14b_3p2>;
> +		xceiver-supply = <&vreg_l14b_3p2>;

It may be that for this chip to actually be able to communiate with devices
on the bus, you need to set the new 'microchip,xstbyen' property

see:

https://lore.kernel.org/linux-arm-msm/20260321135031.3107408-1-viken.dadhaniya@xxxxxxxxxxxxxxxx/

[...]

> +&tlmm {
> +	gpio-reserved-ranges = <32 8>;

Would you happen to know what these pins are connected to, and if
so, add a comment (like in arch/arm64/boot/dts/qcom/x1-crd.dtsi)?



> +
> +	bt_default: bt-default-state {
> +		pins = "gpio81";

It would be best to keep these entries ordered by pin idx

> +		function = "gpio";
> +		drive-strength = <16>;
> +		bias-disable;
> +	};
> +
> +	sw_ctrl_default: sw-ctrl-default-state {
> +		pins = "gpio82";
> +		function = "gpio";
> +		bias-pull-down;
> +	};
> +
> +	lt9611_irq_pin: lt9611-irq-state {
> +		pins = "gpio40";
> +		function = "gpio";
> +		bias-disable;
> +	};
> +
> +	lt9611_rst_pin: lt9611-rst-state {
> +		pins = "gpio7";
> +		function = "gpio";
> +		output-high;

You shouldn't need to assert the GPIO state in the pin entry node
- the driver should take care of that

 
> +	};
> +
> +	ntn0_en: ntn0-en-state {
> +		pins = "gpio67";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-disable;
> +	};
> +
> +	ntn1_en: ntn1-en-state {
> +		pins = "gpio42";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-disable;
> +	};
> +
> +	upd_1p05_en: upd-1p05-en-state {
> +		pins = "gpio179";
> +		function = "gpio";
> +		drive-strength = <2>;
> +		bias-pull-up;
> +	};

I don't know if pulling up an active-high pin is what you want
(there's some more occurences)

Konrad