Re: [PATCH v3 01/10] dt-bindings: input: samsung,s3c6410-keypad: introduce compact binding

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly introduced compact DT binding is completely unsupported by the driver, leading to guaranteed probe failure.
--

--- Patch [1]: [PATCH v3 01/10] dt-bindings: input: samsung,s3c6410-keypad: introduce compact binding ---
commit c3ae19e70e1090e5ebc5290a0659633378fd46d7
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

dt-bindings: input: samsung,s3c6410-keypad: introduce compact binding

This commit updates the samsung,s3c6410-keypad devicetree binding to support
the standard matrix keymap binding. It makes the legacy
samsung,keypad-num-columns and samsung,keypad-num-rows properties mutually
exclusive with the standard linux,keymap property.

> diff --git a/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml b/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml
> --- a/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml
> +++ b/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml
[ ... ]
> @@ -81,12 +85,33 @@ patternProperties:
>        - keypad,row
>        - linux,code
>  
> +dependencies:
> +  linux,keymap: [ "keypad,num-columns", "keypad,num-rows" ]
> +
>  required:
>    - compatible
>    - reg
>    - interrupts
> -  - samsung,keypad-num-columns
> -  - samsung,keypad-num-rows
> +
> +allOf:
> +  - $ref: input.yaml#
> +  - $ref: matrix-keymap.yaml#
> +  - if:
> +      required:
> +        - linux,keymap
> +    then:
> +      properties:
> +        samsung,keypad-num-columns: false
> +        samsung,keypad-num-rows: false

[Severity: Medium]
Will device trees using this new compact binding fail to probe?

The new binding explicitly forbids the samsung,keypad-num-columns and 
samsung,keypad-num-rows properties when linux,keymap is used. However, the
current driver code in drivers/input/keyboard/samsung-keypad.c unconditionally
requires them:

drivers/input/keyboard/samsung-keypad.c:samsung_keypad_parse_dt() {
    ...
    of_property_read_u32(np, "samsung,keypad-num-rows", &num_rows);
    of_property_read_u32(np, "samsung,keypad-num-columns", &num_cols);
    if (!num_rows || !num_cols) {
        dev_err(dev, "number of keypad rows/columns not specified\n");
        return ERR_PTR(-EINVAL);
    }
    ...
}

This means a device tree written with the new compact binding format will
cause the keypad hardware probe to fail immediately with -EINVAL.

Additionally, if that check were bypassed, does the driver's current keymap
initialization conflict with the standard matrix keypad parsing? 

drivers/input/keyboard/samsung-keypad.c:samsung_keypad_probe() {
    ...
    error = matrix_keypad_build_keymap(keymap_data, NULL,
                                       pdata->rows, pdata->cols,
                                       keypad->keycodes, input_dev);
    ...
}

Because samsung_keypad_parse_dt() currently unconditionally allocates an
empty keymap_data structure, passing a non-null pointer here prevents
matrix_keypad_build_keymap() from falling back to parsing the linux,keymap
property directly from the device tree.

Are there corresponding driver updates planned to support these new
properties alongside this binding change?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711-samsung-kp-v3-0-b2fcaba77aff@gmail.com?part=1