Re: [PATCH v2 4/5] dt-bindings: input/ts/zinitix: document mode 0

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Race condition during probe where the input device is registered before device properties are fully read, leading to a state mismatch and a NULL pointer dereference crash.
--

--- Patch [4]: [PATCH v2 4/5] dt-bindings: input/ts/zinitix: document mode 0 ---
commit 68cae6b2efa127f7ca443fe7f4819ab411c887fb
Author: Kaustabh Chakraborty <[email protected]>

dt-bindings: input/ts/zinitix: document mode 0

Zinitix touchscreens support three modes, numbered 0 to 2. This commit
adds mode 0 to the devicetree schema as it was previously unmentioned.

> diff --git a/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml b/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
> index f1ce837b16dfa..9b51212ea9b97 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
> @@ -76,9 +76,9 @@ properties:
>    zinitix,mode:
>      description: Mode of reporting touch points. Some modes may not work
>        with a particular ts firmware for unknown reasons. Available modes are
> -      1 and 2. Mode 2 is the default and preferred.
> +      0, 1, and 2. Mode 2 is the default and preferred.
>      $ref: /schemas/types.yaml#/definitions/uint32
> -    enum: [1, 2]
> +    enum: [0, 1, 2]

[Severity: High]
This is a pre-existing issue, but does adding mode 0 highlight a race 
condition in the driver's initialization sequence?

Looking at drivers/input/touchscreen/zinitix.c, the input device is 
registered before the properties are read:

drivers/input/touchscreen/zinitix.c:zinitix_ts_probe() {
        error = zinitix_init_input_dev(bt541);
        if (error) {
                ...
        }

        error = device_property_read_u32(&client->dev, "zinitix,mode",
                                         &bt541->zinitix_mode);
        ...
        switch (bt541->zinitix_mode) {
        case 0:
                bt541->read_point_status = zinitix_read_point_status_mode0;
                break;
        ...
        }
}

If userspace rapidly opens the device (e.g., via udevd) right after
zinitix_init_input_dev() is called, zinitix_start() will run and call 
enable_irq(). 

If an interrupt fires before probe() reaches the switch statement and assigns
the bt541->read_point_status function pointer, could this result in a NULL 
pointer dereference in the interrupt handler?

drivers/input/touchscreen/zinitix.c:zinitix_ts_irq_handler() {
        ...
        error = bt541->read_point_status(bt541, &touch_event);
        ...
}

Should the property reading and function pointer initialization be moved
before the input device is registered to prevent this?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4