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

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Driver fails to probe and skips keymap parsing when using the newly introduced compact device tree binding.
- [Low] Suboptimal workaround for schema validation when importing matrix-keymap.yaml properties.
--

--- Patch [1]: [PATCH v2 01/11] dt-bindings: input: samsung,s3c6410-keypad: introduce compact binding ---

commit b727e1653e0c43f9e2d755869527a44de705430f
Author: Dmitry Torokhov [off-list ref]

dt-bindings: input: samsung,s3c6410-keypad: introduce compact binding
    
The binding with a sub-node per each key is very verbose and is hard to
use with static device properties. Allow standard matrix keymap binding
in addition to the verbose one.

[Severity: Low]
Is it necessary to manually declare these properties?

Since they are already defined in matrix-keymap.yaml, using
unevaluatedProperties: false at the end of the schema instead of
additionalProperties: false should allow them to be evaluated
automatically without explicitly listing them here.

   linux,input-no-autorepeat:
     type: boolean
     description:

[ ... ]

[Severity: High]
Will this cause the samsung-keypad driver to fail to probe when using
the new compact binding?

If a device tree drops the legacy properties as mandated here,
samsung_keypad_parse_dt() in drivers/input/keyboard/samsung-keypad.c
will fail to probe the device:

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

Additionally, will the driver skip parsing the linux,keymap property
entirely?

The driver currently sizes the keymap based on the child node count, which
is zero for the compact matrix binding:

samsung_keypad_parse_dt() {
        ...
        keymap_data = devm_kzalloc(dev, sizeof(*keymap_data), GFP_KERNEL);
        ...
        key_count = of_get_child_count(np);
        keymap_data->keymap_size = key_count;
        ...
}

This causes matrix_keypad_build_keymap() to bypass parsing the linux,keymap
property, resulting in an empty keymap.

Does the driver need a corresponding update to support this binding change?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-samsung-kp-v2-0-3c6ed4c9b3b6@gmail.com?part=1