Re: [PATCH v2 02/11] Input: samsung-keypad - handle compact binding

[sashiko-bot@kernel.org]

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 <dmitry.torokhov@gmail.com>

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.

> 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

[ ... ]

> +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