Skip to content
Open
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 114 additions & 0 deletions arch/arm/boot/dts/qcom/msm8226-motorola-titan.dts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,15 @@
regulator-boot-on;
};

vdd_touch_vreg: regulator-vdd-touch {
compatible = "regulator-fixed";
regulator-name = "vdd_touch";

gpio = <&tlmm 73 GPIO_ACTIVE_HIGH>;
startup-delay-us = <300>;
enable-active-high;
};

reserved-memory {
#address-cells = <1>;
#size-cells = <1>;
Expand Down Expand Up @@ -146,6 +155,40 @@
};
};

&blsp1_i2c5 {
status = "okay";

touchscreen@20 {
compatible = "syna,rmi4-i2c";
reg = <0x20>;

interrupt-parent = <&tlmm>;
interrupts = <17 IRQ_TYPE_NONE>;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use interrupts-extended = <&tlmm 17 IRQ_TYPE_*> and use proper IRQ_TYPE_* definition, IRQ_TYPE_NONE is not a valid interrupt type.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In downstream, it has the following:

			interrupt-parent = <&msmgpio>;
			interrupts = <17 0>;
			gpios = <&msmgpio 17 0>, /* IRQ */
				<&msmgpio 16 0>; /* RESET */

When I looked up the IRQ_TYPE_* definitions in include/dt-bindings/interrupt-controller/irq.h, it defines:

#define IRQ_TYPE_NONE 0

So I figured that was what I needed if downstream had 0 set as the interrupt type?

@z3ntu z3ntu Aug 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IRQ_TYPE_NONE is just not a valid interrupt type, it's one of the 4 others (edge rising, edge falling, trigger high, trigger low). That's then just not specified correctly on downstream.
Try looking into what other dts files with the same compatible use as interrupt type, or just try the 4 separately to find which ones work correctly.

Also you should get a big warning on bootup if you have that NONE specified.


vdd-supply = <&vdd_touch_vreg>;
vio-supply = <&pm8226_lvs1>;

pinctrl-0 = <&touch_pin>;
pinctrl-1 = <&tp_avdd_en_default_state>, <&tp_avdd_en_suspend_state>;
pinctrl-names = "default", "sleep";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks bogus, you can't request both tp_avdd_en_default_state and tp_avdd_en_suspend_state at the same time. Just put the tp_avdd_en_suspend_state pinctrl on the regulator-fixed where it belongs, and drop the tp_avdd_en_suspend_state.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I got confused about how to map these. I think I need to move the tp_avdd_en_default_state to pinctrl-0 here?

Then are you saying I should add pinctrl-0 = <&tp_avdd_en_suspend_state>; to vdd_touch_vreg?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I not sure exactly what I need to do here. I don't think that pinctrl-0 is a valid property in a regulator-fixed node?
Did you mean instead that the information I previously recorded in tp_avdd_en_suspend_state should be converted to use the regulator-fixed properties?

I wasn't sure how to convert the drive-strength to a regulator-fixed property, but I've added regulator-pull-down to represent bias-pull-down.

And I've deleted the tp_avdd_en_suspend_state node.

It all still works, so perhaps this is OK?
See 2b62f49.


syna,startup-delay-ms = <100>;

#address-cells = <1>;
#size-cells = <0>;

rmi4-f01@1 {
reg = <0x1>;
syna,nosleep-mode = <1>;
};

rmi4-f11@11 {
reg = <0x11>;
syna,sensor-type = <1>;
};
};
};

&pm8226_vib {
status = "okay";
};
Expand Down Expand Up @@ -180,8 +223,10 @@
};

pm8226_l3: l3 {
/* WCNSS */
regulator-min-microvolt = <750000>;
regulator-max-microvolt = <1337500>;
regulator-always-on;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

regulator-always-on is not a proper solution, pm8226_l3 is already referenced from pronto, if l8 is also needed then the bindings are incomplete. If you have schematics please double-check there.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've narrowed this down to just l8 being required.

Looking at downstream, this is the vdd_pronto_pll-supply specified in msm8226.dtsi. Does that mean it belongs in the common upstream qcom-msm8226.dtsi file rather than just in the titan dts?

	qcom,pronto@fb21b000 {
		compatible = "qcom,pil-pronto";
		reg = <0xfb21b000 0x3000>,
		      <0xfc401700 0x4>,
		      <0xfd485300 0xc>;
		reg-names = "pmu_base", "clk_base", "halt_base";
		interrupts = <0 149 1>;
		vdd_pronto_pll-supply = <&pm8226_l8>;

(Also, it's not immediately clear to me what upstream property matches this.)

};

pm8226_l4: l4 {
Expand All @@ -207,8 +252,10 @@
};

pm8226_l8: l8 {
/* WCNSS */
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
regulator-always-on;
};

pm8226_l9: l9 {
Expand Down Expand Up @@ -354,6 +401,30 @@
v3p3-supply = <&pm8226_l20>;
};

&pronto {
status = "okay";

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

status should be last property

@daviewales daviewales Aug 15, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After pinctrl-0 but before iris {? Or after the closing }; for smd-edge?
Edit: Assuming after pinctrl-0 but before iris {, because that matches qcom-msm8226.dtsi.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, properties always need to be before subnodes, otherwise it doesn't build


vddmx-supply = <&pm8226_l3>;
vddpx-supply = <&pm8226_l6>;

pinctrl-names = "default";
pinctrl-0 = <&wcnss_pin_a>;

iris {
compatible = "qcom,wcn3620";
vddxo-supply = <&pm8226_l10>;
vddrfa-supply = <&pm8226_l24>;
vddpa-supply = <&pm8226_l16>;
vdddig-supply = <&pm8226_l24>;
};

smd-edge {
wcnss {
status = "okay";
};
};
};

&tlmm {
reg_lcd_default: reg-lcd-default-state {
pins = "gpio12", "gpio13";
Expand All @@ -363,10 +434,53 @@
output-high;
};

touch_pin: touch-state {
reset-pins {
pins = "gpio16";
function = "gpio";

drive-strength = <2>;
bias-pull-up;
output-high;
Comment thread
daviewales marked this conversation as resolved.
Outdated
};

int-pins {
pins = "gpio17";
function = "gpio";

drive-strength = <2>;
bias-disable;
input;
Comment thread
daviewales marked this conversation as resolved.
Outdated
};
};

sdhc2_cd_default: sdhc2-cd-default-state {
pins = "gpio38";
function = "gpio";
drive-strength = <2>;
bias-pull-up;
};

wcnss_pin_a: wcnss-pin-active-state {
pins = "gpio40", "gpio41", "gpio42", "gpio43", "gpio44";
function = "wlan";
drive-strength = <2>;
bias-pull-up;
};

tp_avdd_en_default_state: touch-avdd-en-pin-default-state {
pins = "gpio73";
function = "gpio";
drive-strength = <2>;
bias-pull-up;
output-high;
};

tp_avdd_en_suspend_state: touch-avdd-en-pin-suspend-state {
pins = "gpio73";
function = "gpio";
drive-strength = <2>;
bias-pull-down;
input;
};
Comment thread
daviewales marked this conversation as resolved.
Outdated
};