ARM: dts: qcom: msm8226-motorola-titan: Enable WiFi and touchscreen

[daviewales]

WiFi

regulator-always-on for l3 and l8 prevents bootloops and instability.

Note: the following errors appear in dmesg, particularly when trying to use Bluetooth and WiFi at the same time. So far as I can tell, this is likely an issue with the driver, not the device tree.

wcn36xx: ERROR hal_enter_bmps response failed err=1
wcn36xx: ERROR Can not enter BMPS!
wcn36xx: ERROR SMD_EVENT (312) not supported

In particular, SMD_EVENT (312) is WCN36XX_HAL_P2P_NOA_ATTR_IND from wcn36xx/hal.h which is not explicitly handled by wcn36xx_smd_rsp_process() in wcn36xx/smd.c.

I have no idea how to fix that, and WiFi works great if I don't touch Bluetooth, so figured I should just send this.
(Bluetooth also works, but with a lot more dropouts. I haven't tried Bluetooth with WiFi disabled, but I was able to detect events from a paired PS4 DualShock controller before it dropped out.)

Thanks @erikas9987 for the hint about the regulators!

Touchscreen

Enable the Synaptics RMI4 I2C touchscreen for motorola-titan.

[daviewales]

Update: Also enabled the touchscreen!

Edit: But now it doesn't seem able to stay powered off. (Yes, I'm using the right poweroff command this time!)

Edit 2: Interestingly, it powers off fine from a full boot. It only turns back on unexpectedly if I run poweroff from the debug shell... Maybe that's fine?

Edit 3: Ignore the previous two edits... I get the same behaviour without these commits.

[z3ntu]

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

[daviewales]

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]

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.

[z3ntu]

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.

[daviewales]

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?

[daviewales]

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.

[z3ntu]

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.

[daviewales]

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.)

[z3ntu]

status should be last property

[daviewales]

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.

[z3ntu]

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

[daviewales]

I've hopefully fixed most of the synaptics issues in 2b62f49. (Kept as a separate commit for review, but will squash it with the original when you're happy with the changes.)

Will work on the pronto changes in another commit.