Re: [PATCH v9 4/4] arm64: dts: qcom: Add Lenovo ThinkBook 16 G7 QOY device tree

On 30.07.25 10:50, Konrad Dybcio wrote:

On 7/1/25 7:02 PM, Jens Glathe via B4 Relay wrote:

quoted

  
-		pm8010-thermal {
+		pm8010_thermal: pm8010-thermal {

Let's rebase on:

https://lore.kernel.org/linux-arm-msm/20250701183625.1968246-1-alex.vinarskis@gmail.com/ (local)

Done.

quoted

+		wcd-playback-dai-link {
+			link-name = "WCD Playback";
+
+			cpu {
+				sound-dai = <&q6apmbedai RX_CODEC_DMA_RX_0>;
+			};
+
+			codec {

'co'dec < 'cp'u

Done and checked

quoted

+&i2c2 {
+	clock-frequency = <400000>;
+
+	pinctrl-0 = <&qup_i2c2_data_clk>, <&tpad_default>, <&kybd_default>;
+	pinctrl-names = "default";
+	status = "okay";
+
+	/* ELAN06FA */
+	touchpad@15 {

5 touchpad variants is a lot. Does DSDT give any clues as to whether
all of them are plausibly present on production hw?

I took sort of a survey among users, and we got mostly @2c (CIRQ1080 or 
SYNA2BA6) and one with @38  (FTCS0038). All the others are defined and 
look like wired up in the dsdt. There's also  3 display panel variants 
defined, all with the same totally odd pwm backlight mechanism according 
to the dsdt-embedded xml structures for these. Survey showed all the 
same panel as mine, except the one with the diverging touchpad. The 
sample size is ~5, so not really representative.

quoted

+&mdss_dp0_out {
+	data-lanes = <0 1 2 3>;
+	link-frequencies = /bits/ 64 <1620000000 2700000000 5400000000 8100000000>;
+};
+
+&mdss_dp1 {
+	status = "okay";
+};
+
+&mdss_dp1_out {
+	data-lanes = <0 1 2 3>;

Does defining all 4 lanes not upset the driver without additional
patches?

Yes, corrected, thank you.

quoted

+	link-frequencies = /bits/ 64 <1620000000 2700000000 5400000000 8100000000>;
+};
+
+&mdss_dp2 {
+	status = "okay";
+};
+
+&mdss_dp2_out {
+	data-lanes = <0 1 2 3>;
+	link-frequencies = /bits/ 64 <1620000000 2700000000 5400000000 8100000000>;
+};
+
+&mdss_dp2_phy {
+//	vdda-phy-supply = <&vreg_l3b>;
+// 	vdda-pll-supply = <&vreg_l6b>;

The regulators are usecase-specialized, feel free to trust what
is present on the CRD

Removed mdss_dp2 since it's not working reliably yet.

quoted

+	ports {
+		port@1 {
+			reg = <1>;
+
+			mdss_dp3_out: endpoint {
+				data-lanes = <0 1 2 3>;
+				link-frequencies = /bits/ 64 <1620000000 2700000000 5400000000 8100000000>;
+
+				remote-endpoint = <&edp_panel_in>;
+			};
+		};
+	};
+};

Please rebase on:

https://lore.kernel.org/linux-arm-msm/20250724-move-edp-endpoints-v1-3-6ca569812838@oss.qualcomm.com/ (local)

Done.

quoted

+&sdhc_2 {
+	cd-gpios = <&tlmm 71 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_l6b_1p8>;
+//	bus-width = <4>;
+//	no-sdio;
+//	no-mmc;

Please either remove the '//' or remove the lines

Done. I have an idea which driver it should be (BayHubTech driver for 
the PCI variant) since the Ideapad 5 has actually both variants - the 
Slim 5x on pcie3, and the 2in1 on sdhc_2. The Windows driver supports 
both interfaces.

quoted

+&tlmm {
+	gpio-reserved-ranges = <34 2>, /* Unused */
+			       <72 2>, /* Secure EC I2C connection (?) */
+			       <238 1>; /* UFS Reset */

Try removing the UFS reset block and see if the platform still
boots, this turned out to be a false flag on CRD

Done, still working.

I'd also suggest removing voltage suffixes from regulator names (i.e.
turn them to e.g. vreg_l6b).

I'm not really a fan of this. The X1E dtbs all have the voltage suffix, 
and it is actually quite human-readable. Which of course is not the goal 
of a dts, but well, as a human writing these I like some orientation help.

with best regards

Jens