Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Legacy device tree bindings for keypad row/column count are ignored and overwritten, breaking existing platforms.
--
--- Patch [2]: [PATCH v2 02/11] Input: samsung-keypad - handle compact binding ---
commit 43d179d444ff3e714e66f7ac32f10fcc817c6f15
Author: Dmitry Torokhov [off-list ref]
Input: samsung-keypad - handle compact binding
Adds support for standard matrix keymap bindings to the Samsung keypad
driver, allowing easier conversions from platform data to static device
properties.
quoted hunk ↗ jump to hunk
diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
index 17127269e3f05..b9d4ea5f202ae 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
[ ... ]
quoted hunk ↗ jump to hunk
+static const struct samsung_keypad_platdata *
+samsung_keypad_parse_properties(struct device *dev)
+{
+ const struct matrix_keymap_data *keymap_data;
+ struct samsung_keypad_platdata *pdata;
+ u32 num_rows = 0, num_cols = 0;
+ int error;
+
pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
- if (!pdata) {
- dev_err(dev, "could not allocate memory for platform data\n");
+ if (!pdata)
return ERR_PTR(-ENOMEM);
- }
- 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);
- }
+ device_property_read_u32(dev, "samsung,keypad-num-rows", &num_rows);
+ device_property_read_u32(dev, "samsung,keypad-num-columns", &num_cols);
+
+ error = matrix_keypad_parse_properties(dev, &num_rows, &num_cols);
+ if (error)
+ return ERR_PTR(error);
[Severity: High]
Will this break probing on existing platforms that use the legacy bindings?
Looking at matrix_keypad_parse_properties() in drivers/input/matrix-keymap.c,
it unconditionally resets the row and column pointers to 0 before attempting
to read the modern properties:
int matrix_keypad_parse_properties(struct device *dev,
unsigned int *rows, unsigned int *cols)
{
*rows = *cols = 0;
device_property_read_u32(dev, "keypad,num-rows", rows);
...
If the modern "keypad,num-rows" properties are absent, it seems this will
wipe out the successfully parsed legacy "samsung,keypad-num-rows" values
and return an error, causing the probe to abort.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-samsung-kp-v2-0-3c6ed4c9b3b6@gmail.com?part=2