Thread (19 messages) 19 messages, 3 authors, 1d ago

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

From: sashiko-bot@kernel.org
Date: 2026-07-12 02:22:48
Also in: linux-devicetree

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 [off-list ref]

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.
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help