touchscreen-compatible cause crashing when status property is missng

[barni2000]

I have found out touchscreen-compatible makes lk2nd to crash when a touchscreen node have not got
the status property.

Crashing:

        touchscreen@5d {
                compatible = "goodix,gt911";
                reg = <0x5d>;
                interrupts-extended = <&tlmm 65 IRQ_TYPE_LEVEL_LOW>;
                reset-gpios = <&tlmm 64 GPIO_ACTIVE_HIGH>;
                AVDD28-supply = <&pm8937_l10>;
                VDDIO-supply = <&pm8937_l5>;

                touchscreen-size-x = <720>;
                touchscreen-size-y = <1280>;
        };

Not Crashing:

        touchscreen@5d {
                compatible = "goodix,gt911";
                reg = <0x5d>;
                interrupts-extended = <&tlmm 65 IRQ_TYPE_LEVEL_LOW>;
                reset-gpios = <&tlmm 64 GPIO_ACTIVE_HIGH>;
                AVDD28-supply = <&pm8937_l10>;
                VDDIO-supply = <&pm8937_l5>;

                touchscreen-size-x = <720>;
                touchscreen-size-y = <1280>;

                status = "okay";
        };

[barni2000]

This is an possible fix

diff --git a/lk2nd/device/panel.c b/lk2nd/device/panel.c
index 602438824..e643939cb 100644
--- a/lk2nd/device/panel.c
+++ b/lk2nd/device/panel.c
@@ -99,7 +99,7 @@ static int lk2nd_panel_dt_update(void *dtb, const char *cmdline,
                if (ret < 0) {
                        dprintf(CRITICAL, "Failed to enable %s touchscreen: %d\n",
                                panel->ts_compatible, ret);
-                       return ret;
+                       return 0;
                }
        }

In my opinion missing status property should not block boot, missing status means status = "okay" by default.

[TravMurav]

I'd argue this is a bit problematic in case that the said dt has multiple touchscreen nodes with one being "enabled by default", then if we "enable" some other node, we suddenly have two conflicting nodes. failing to boot on missing status at least highlights that possibility.

i.e. I'd argue that if multiple touchscreen nodes exist, they all should be status=disabled, and if there is only one, there is no need in lk2nd touching it at all (so touchscreen-compatible prop can be dropped from lk2nd dt)

I somewhat fail to understand the different usecase here, what do you think?

[barni2000]

I'd argue this is a bit problematic in case that the said dt has multiple touchscreen nodes with one being "enabled by default", then if we "enable" some other node, we suddenly have two conflicting nodes. failing to boot on missing status at least highlights that possibility.

i.e. I'd argue that if multiple touchscreen nodes exist, they all should be status=disabled, and if there is only one, there is no need in lk2nd touching it at all (so touchscreen-compatible prop can be dropped from lk2nd dt)

I think a warning would be enough for this case.

I somewhat fail to understand the different usecase here, what do you think?

My issue is backward compatibility after touchscreen-compatible is introduced in lk2nd it will not work with older kernel versions where the status property is missing and it will reboot to EDL mode what can
be caused by anything.

[TravMurav]

I'm afraid noone will see this warning but I guess if everyone is careful with those, then it's fine. Feel free to make a PR with a proposed change then