* [PATCH v2 02/11] Input: samsung-keypad - handle compact binding
From: Dmitry Torokhov @ 2026-07-09 4:53 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Peter Griffin,
Alim Akhtar, Russell King, Mark Brown, Linus Walleij,
Charles Keepax, Sam Protsenko, Arnd Bergmann, Bartosz Golaszewski
Cc: Krzysztof Kozlowski, linux-input, devicetree, linux-kernel,
linux-arm-kernel, linux-samsung-soc, patches
In-Reply-To: <20260708-samsung-kp-v2-0-3c6ed4c9b3b6@gmail.com>
Add support for standard matrix keymap binding (in addition to the
existing verbose binding with a sub-node for each key). This will
allow easier conversions from platform data to device properties when
using static device properties.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/keyboard/samsung-keypad.c | 130 ++++++++++++++++----------------
1 file changed, 66 insertions(+), 64 deletions(-)
diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
index 17127269e3f0..b9d4ea5f202a 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -231,83 +231,83 @@ static void samsung_keypad_close(struct input_dev *input_dev)
samsung_keypad_stop(keypad);
}
-#ifdef CONFIG_OF
-static struct samsung_keypad_platdata *
-samsung_keypad_parse_dt(struct device *dev)
+static const struct matrix_keymap_data *
+samsung_parse_verbose_keymap(struct device *dev)
{
- struct samsung_keypad_platdata *pdata;
struct matrix_keymap_data *keymap_data;
- uint32_t *keymap, num_rows = 0, num_cols = 0;
- struct device_node *np = dev->of_node, *key_np;
+ struct fwnode_handle *child;
+ u32 *keymap;
unsigned int key_count;
- if (!np) {
- dev_err(dev, "missing device tree data\n");
- return ERR_PTR(-EINVAL);
+ keymap_data = devm_kzalloc(dev, sizeof(*keymap_data), GFP_KERNEL);
+ if (!keymap_data)
+ return ERR_PTR(-ENOMEM);
+
+ key_count = device_get_child_node_count(dev);
+ keymap = devm_kcalloc(dev, key_count, sizeof(*keymap), GFP_KERNEL);
+ if (!keymap)
+ return ERR_PTR(-ENOMEM);
+
+ keymap_data->keymap_size = key_count;
+ keymap_data->keymap = keymap;
+
+ device_for_each_child_node(dev, child) {
+ u32 row, col, key_code;
+
+ fwnode_property_read_u32(child, "keypad,row", &row);
+ fwnode_property_read_u32(child, "keypad,column", &col);
+ fwnode_property_read_u32(child, "linux,code", &key_code);
+
+ *keymap++ = KEY(row, col, key_code);
}
+ return keymap_data;
+}
+
+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);
+
pdata->rows = num_rows;
pdata->cols = num_cols;
- keymap_data = devm_kzalloc(dev, sizeof(*keymap_data), GFP_KERNEL);
- if (!keymap_data) {
- dev_err(dev, "could not allocate memory for keymap data\n");
- return ERR_PTR(-ENOMEM);
- }
- pdata->keymap_data = keymap_data;
+ if (!device_property_present(dev, "linux,keymap")) {
+ keymap_data = samsung_parse_verbose_keymap(dev);
+ if (IS_ERR(keymap_data))
+ return ERR_CAST(keymap_data);
- key_count = of_get_child_count(np);
- keymap_data->keymap_size = key_count;
- keymap = devm_kcalloc(dev, key_count, sizeof(uint32_t), GFP_KERNEL);
- if (!keymap) {
- dev_err(dev, "could not allocate memory for keymap\n");
- return ERR_PTR(-ENOMEM);
+ pdata->keymap_data = keymap_data;
}
- keymap_data->keymap = keymap;
- for_each_child_of_node(np, key_np) {
- u32 row, col, key_code;
- of_property_read_u32(key_np, "keypad,row", &row);
- of_property_read_u32(key_np, "keypad,column", &col);
- of_property_read_u32(key_np, "linux,code", &key_code);
- *keymap++ = KEY(row, col, key_code);
- }
- pdata->no_autorepeat = of_property_read_bool(np, "linux,input-no-autorepeat");
+ pdata->no_autorepeat =
+ device_property_read_bool(dev, "linux,input-no-autorepeat");
- pdata->wakeup = of_property_read_bool(np, "wakeup-source") ||
+ pdata->wakeup = device_property_read_bool(dev, "wakeup-source") ||
/* legacy name */
- of_property_read_bool(np, "linux,input-wakeup");
-
+ device_property_read_bool(dev, "linux,input-wakeup");
return pdata;
}
-#else
-static struct samsung_keypad_platdata *
-samsung_keypad_parse_dt(struct device *dev)
-{
- dev_err(dev, "no platform data defined\n");
-
- return ERR_PTR(-EINVAL);
-}
-#endif
static int samsung_keypad_probe(struct platform_device *pdev)
{
const struct samsung_keypad_platdata *pdata;
- const struct matrix_keymap_data *keymap_data;
const struct platform_device_id *id;
struct samsung_keypad *keypad;
struct resource *res;
@@ -316,18 +316,17 @@ static int samsung_keypad_probe(struct platform_device *pdev)
int error;
pdata = dev_get_platdata(&pdev->dev);
- if (!pdata) {
- pdata = samsung_keypad_parse_dt(&pdev->dev);
+ if (pdata) {
+ if (!pdata->keymap_data) {
+ dev_err(&pdev->dev, "no keymap data defined\n");
+ return -EINVAL;
+ }
+ } else {
+ pdata = samsung_keypad_parse_properties(&pdev->dev);
if (IS_ERR(pdata))
return PTR_ERR(pdata);
}
- keymap_data = pdata->keymap_data;
- if (!keymap_data) {
- dev_err(&pdev->dev, "no keymap data defined\n");
- return -EINVAL;
- }
-
if (!pdata->rows || pdata->rows > SAMSUNG_MAX_ROWS)
return -EINVAL;
@@ -391,7 +390,7 @@ static int samsung_keypad_probe(struct platform_device *pdev)
input_dev->open = samsung_keypad_open;
input_dev->close = samsung_keypad_close;
- error = matrix_keypad_build_keymap(keymap_data, NULL,
+ error = matrix_keypad_build_keymap(pdata->keymap_data, NULL,
pdata->rows, pdata->cols,
keypad->keycodes, input_dev);
if (error) {
@@ -430,11 +429,14 @@ static int samsung_keypad_probe(struct platform_device *pdev)
if (error)
return error;
- if (pdev->dev.of_node) {
- devm_kfree(&pdev->dev, (void *)pdata->keymap_data->keymap);
- devm_kfree(&pdev->dev, (void *)pdata->keymap_data);
+ if (!dev_get_platdata(&pdev->dev)) {
+ if (pdata->keymap_data) {
+ devm_kfree(&pdev->dev, (void *)pdata->keymap_data->keymap);
+ devm_kfree(&pdev->dev, (void *)pdata->keymap_data);
+ }
devm_kfree(&pdev->dev, (void *)pdata);
}
+
return 0;
}
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related
* [PATCH v2 03/11] ARM: s3c: register and attach software nodes for Samsung gpio_chips
From: Dmitry Torokhov @ 2026-07-09 4:53 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Peter Griffin,
Alim Akhtar, Russell King, Mark Brown, Linus Walleij,
Charles Keepax, Sam Protsenko, Arnd Bergmann, Bartosz Golaszewski
Cc: Krzysztof Kozlowski, linux-input, devicetree, linux-kernel,
linux-arm-kernel, linux-samsung-soc, patches
In-Reply-To: <20260708-samsung-kp-v2-0-3c6ed4c9b3b6@gmail.com>
Define and register software nodes for the Samsung GPIO chips on legacy
(non-DT) platforms.
Attach the matching software node to each gpio_chip's fwnode during
registration, using the bank label to calculate the index.
This provides the infrastructure for converting board files and drivers
to use software nodes/properties instead of legacy platform data or GPIO
lookup tables.
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
arch/arm/mach-s3c/gpio-core.h | 3 ++
arch/arm/mach-s3c/gpio-samsung-s3c64xx.h | 5 +++
arch/arm/mach-s3c/gpio-samsung.c | 72 ++++++++++++++++++++++++--------
3 files changed, 63 insertions(+), 17 deletions(-)
diff --git a/arch/arm/mach-s3c/gpio-core.h b/arch/arm/mach-s3c/gpio-core.h
index 6801c85fb9da..464b0d02f102 100644
--- a/arch/arm/mach-s3c/gpio-core.h
+++ b/arch/arm/mach-s3c/gpio-core.h
@@ -14,6 +14,8 @@
#include "gpio-samsung.h"
#include <linux/gpio/driver.h>
+struct software_node;
+
#define GPIOCON_OFF (0x00)
#define GPIODAT_OFF (0x04)
@@ -66,6 +68,7 @@ struct samsung_gpio_cfg;
*/
struct samsung_gpio_chip {
struct gpio_chip chip;
+ const struct software_node *swnode;
struct samsung_gpio_cfg *config;
struct samsung_gpio_pm *pm;
void __iomem *base;
diff --git a/arch/arm/mach-s3c/gpio-samsung-s3c64xx.h b/arch/arm/mach-s3c/gpio-samsung-s3c64xx.h
index 8ed144a0d474..fc21d3f50ce9 100644
--- a/arch/arm/mach-s3c/gpio-samsung-s3c64xx.h
+++ b/arch/arm/mach-s3c/gpio-samsung-s3c64xx.h
@@ -13,6 +13,8 @@
#ifdef CONFIG_GPIO_SAMSUNG
+#include <linux/property.h>
+
/* GPIO bank sizes */
#define S3C64XX_GPIO_A_NR (8)
#define S3C64XX_GPIO_B_NR (7)
@@ -89,6 +91,9 @@ enum s3c_gpio_number {
/* define the number of gpios we need to the one after the GPQ() range */
#define GPIO_BOARD_START (S3C64XX_GPQ(S3C64XX_GPIO_Q_NR) + 1)
+extern const struct software_node samsung_gpiochip_nodes[];
+#define SAMSUNG_GPIO_NODE(node) (&samsung_gpiochip_nodes[(node) - 'A'])
+
#endif /* GPIO_SAMSUNG */
#endif /* GPIO_SAMSUNG_S3C64XX_H */
diff --git a/arch/arm/mach-s3c/gpio-samsung.c b/arch/arm/mach-s3c/gpio-samsung.c
index 81e198e5a6d3..b35cf62a9157 100644
--- a/arch/arm/mach-s3c/gpio-samsung.c
+++ b/arch/arm/mach-s3c/gpio-samsung.c
@@ -37,6 +37,28 @@
#include "gpio-cfg-helpers.h"
#include "pm.h"
+const struct software_node samsung_gpiochip_nodes[] = {
+ SOFTWARE_NODE("GPA", NULL, NULL),
+ SOFTWARE_NODE("GPB", NULL, NULL),
+ SOFTWARE_NODE("GPC", NULL, NULL),
+ SOFTWARE_NODE("GPD", NULL, NULL),
+ SOFTWARE_NODE("GPE", NULL, NULL),
+ SOFTWARE_NODE("GPF", NULL, NULL),
+ SOFTWARE_NODE("GPG", NULL, NULL),
+ SOFTWARE_NODE("GPH", NULL, NULL),
+ SOFTWARE_NODE("GPI", NULL, NULL),
+ SOFTWARE_NODE("GPJ", NULL, NULL),
+ SOFTWARE_NODE("GPK", NULL, NULL),
+ SOFTWARE_NODE("GPL", NULL, NULL),
+ SOFTWARE_NODE("GPM", NULL, NULL),
+ SOFTWARE_NODE("GPN", NULL, NULL),
+ SOFTWARE_NODE("GPO", NULL, NULL),
+ SOFTWARE_NODE("GPP", NULL, NULL),
+ SOFTWARE_NODE("GPQ", NULL, NULL),
+};
+
+#define NUM_SAMSUNG_GPIOCHIPS ARRAY_SIZE(samsung_gpiochip_nodes)
+
static int samsung_gpio_setpull_updown(struct samsung_gpio_chip *chip,
unsigned int off, samsung_gpio_pull_t pull)
{
@@ -491,6 +513,16 @@ static __init void s3c_gpiolib_track(struct samsung_gpio_chip *chip)
}
#endif /* CONFIG_S3C_GPIO_TRACK */
+static void __init samsung_setup_gpiochip_nodes(void)
+{
+ const struct software_node *group[NUM_SAMSUNG_GPIOCHIPS + 1] = { 0 };
+
+ for (unsigned int i = 0; i < NUM_SAMSUNG_GPIOCHIPS; i++)
+ group[i] = &samsung_gpiochip_nodes[i];
+
+ software_node_register_node_group(group);
+}
+
/*
* samsung_gpiolib_add() - add the Samsung gpio_chip.
* @chip: The chip to register
@@ -506,12 +538,16 @@ static void __init samsung_gpiolib_add(struct samsung_gpio_chip *chip)
struct gpio_chip *gc = &chip->chip;
int ret;
+ gc->label = chip->swnode->name;
+
BUG_ON(!chip->base);
BUG_ON(!gc->label);
BUG_ON(!gc->ngpio);
spin_lock_init(&chip->lock);
+ gc->fwnode = software_node_fwnode(chip->swnode);
+
if (!gc->direction_input)
gc->direction_input = samsung_gpiolib_2bit_input;
if (!gc->direction_output)
@@ -659,49 +695,49 @@ static struct samsung_gpio_chip s3c64xx_gpios_4bit[] = {
.chip = {
.base = S3C64XX_GPA(0),
.ngpio = S3C64XX_GPIO_A_NR,
- .label = "GPA",
},
+ .swnode = SAMSUNG_GPIO_NODE('A'),
}, {
.chip = {
.base = S3C64XX_GPB(0),
.ngpio = S3C64XX_GPIO_B_NR,
- .label = "GPB",
},
+ .swnode = SAMSUNG_GPIO_NODE('B'),
}, {
.chip = {
.base = S3C64XX_GPC(0),
.ngpio = S3C64XX_GPIO_C_NR,
- .label = "GPC",
},
+ .swnode = SAMSUNG_GPIO_NODE('C'),
}, {
.chip = {
.base = S3C64XX_GPD(0),
.ngpio = S3C64XX_GPIO_D_NR,
- .label = "GPD",
},
+ .swnode = SAMSUNG_GPIO_NODE('D'),
}, {
.config = &samsung_gpio_cfgs[0],
.chip = {
.base = S3C64XX_GPE(0),
.ngpio = S3C64XX_GPIO_E_NR,
- .label = "GPE",
},
+ .swnode = SAMSUNG_GPIO_NODE('E'),
}, {
.base = S3C64XX_GPG_BASE,
.chip = {
.base = S3C64XX_GPG(0),
.ngpio = S3C64XX_GPIO_G_NR,
- .label = "GPG",
},
+ .swnode = SAMSUNG_GPIO_NODE('G'),
}, {
.base = S3C64XX_GPM_BASE,
.config = &samsung_gpio_cfgs[1],
.chip = {
.base = S3C64XX_GPM(0),
.ngpio = S3C64XX_GPIO_M_NR,
- .label = "GPM",
.to_irq = s3c64xx_gpiolib_mbank_to_irq,
},
+ .swnode = SAMSUNG_GPIO_NODE('M'),
},
};
@@ -711,25 +747,25 @@ static struct samsung_gpio_chip s3c64xx_gpios_4bit2[] = {
.chip = {
.base = S3C64XX_GPH(0),
.ngpio = S3C64XX_GPIO_H_NR,
- .label = "GPH",
},
+ .swnode = SAMSUNG_GPIO_NODE('H'),
}, {
.base = S3C64XX_GPK_BASE + 0x4,
.config = &samsung_gpio_cfgs[0],
.chip = {
.base = S3C64XX_GPK(0),
.ngpio = S3C64XX_GPIO_K_NR,
- .label = "GPK",
},
+ .swnode = SAMSUNG_GPIO_NODE('K'),
}, {
.base = S3C64XX_GPL_BASE + 0x4,
.config = &samsung_gpio_cfgs[1],
.chip = {
.base = S3C64XX_GPL(0),
.ngpio = S3C64XX_GPIO_L_NR,
- .label = "GPL",
.to_irq = s3c64xx_gpiolib_lbank_to_irq,
},
+ .swnode = SAMSUNG_GPIO_NODE('L'),
},
};
@@ -740,43 +776,43 @@ static struct samsung_gpio_chip s3c64xx_gpios_2bit[] = {
.chip = {
.base = S3C64XX_GPF(0),
.ngpio = S3C64XX_GPIO_F_NR,
- .label = "GPF",
},
+ .swnode = SAMSUNG_GPIO_NODE('F'),
}, {
.config = &samsung_gpio_cfgs[7],
.chip = {
.base = S3C64XX_GPI(0),
.ngpio = S3C64XX_GPIO_I_NR,
- .label = "GPI",
},
+ .swnode = SAMSUNG_GPIO_NODE('I'),
}, {
.config = &samsung_gpio_cfgs[7],
.chip = {
.base = S3C64XX_GPJ(0),
.ngpio = S3C64XX_GPIO_J_NR,
- .label = "GPJ",
},
+ .swnode = SAMSUNG_GPIO_NODE('J'),
}, {
.config = &samsung_gpio_cfgs[6],
.chip = {
.base = S3C64XX_GPO(0),
.ngpio = S3C64XX_GPIO_O_NR,
- .label = "GPO",
},
+ .swnode = SAMSUNG_GPIO_NODE('O'),
}, {
.config = &samsung_gpio_cfgs[6],
.chip = {
.base = S3C64XX_GPP(0),
.ngpio = S3C64XX_GPIO_P_NR,
- .label = "GPP",
},
+ .swnode = SAMSUNG_GPIO_NODE('P'),
}, {
.config = &samsung_gpio_cfgs[6],
.chip = {
.base = S3C64XX_GPQ(0),
.ngpio = S3C64XX_GPIO_Q_NR,
- .label = "GPQ",
},
+ .swnode = SAMSUNG_GPIO_NODE('Q'),
}, {
.base = S3C64XX_GPN_BASE,
.irq_base = IRQ_EINT(0),
@@ -784,9 +820,9 @@ static struct samsung_gpio_chip s3c64xx_gpios_2bit[] = {
.chip = {
.base = S3C64XX_GPN(0),
.ngpio = S3C64XX_GPIO_N_NR,
- .label = "GPN",
.to_irq = samsung_gpiolib_to_irq,
},
+ .swnode = SAMSUNG_GPIO_NODE('N'),
},
};
@@ -803,6 +839,8 @@ static __init int samsung_gpiolib_init(void)
return 0;
if (soc_is_s3c64xx()) {
+ samsung_setup_gpiochip_nodes();
+
samsung_gpiolib_set_cfg(samsung_gpio_cfgs,
ARRAY_SIZE(samsung_gpio_cfgs));
samsung_gpiolib_add_2bit_chips(s3c64xx_gpios_2bit,
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related
* [PATCH v2 04/11] ARM: s3c: crag6410: switch keypad device to software properties
From: Dmitry Torokhov @ 2026-07-09 4:53 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Peter Griffin,
Alim Akhtar, Russell King, Mark Brown, Linus Walleij,
Charles Keepax, Sam Protsenko, Arnd Bergmann, Bartosz Golaszewski
Cc: Krzysztof Kozlowski, linux-input, devicetree, linux-kernel,
linux-arm-kernel, linux-samsung-soc, patches, Krzysztof Kozlowski
In-Reply-To: <20260708-samsung-kp-v2-0-3c6ed4c9b3b6@gmail.com>
Switch the keypad device to use software properties to describe the
keypad. This will allow dropping support for platform data from the
samsung-keypad driver.
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
arch/arm/mach-s3c/Kconfig | 5 ---
arch/arm/mach-s3c/Kconfig.s3c64xx | 1 -
arch/arm/mach-s3c/Makefile.s3c64xx | 1 -
arch/arm/mach-s3c/devs.c | 27 ---------------
arch/arm/mach-s3c/devs.h | 1 -
arch/arm/mach-s3c/keypad.h | 27 ---------------
arch/arm/mach-s3c/mach-crag6410.c | 56 ++++++++++++++++++++++++--------
arch/arm/mach-s3c/setup-keypad-s3c64xx.c | 20 ------------
8 files changed, 42 insertions(+), 96 deletions(-)
diff --git a/arch/arm/mach-s3c/Kconfig b/arch/arm/mach-s3c/Kconfig
index b3656109f1f7..2e77bb4f3a8c 100644
--- a/arch/arm/mach-s3c/Kconfig
+++ b/arch/arm/mach-s3c/Kconfig
@@ -111,11 +111,6 @@ config S3C64XX_DEV_SPI0
Compile in platform device definitions for S3C64XX's type
SPI controller 0
-config SAMSUNG_DEV_KEYPAD
- bool
- help
- Compile in platform device definitions for keypad
-
config SAMSUNG_DEV_PWM
bool
help
diff --git a/arch/arm/mach-s3c/Kconfig.s3c64xx b/arch/arm/mach-s3c/Kconfig.s3c64xx
index 3f97fba8e4f5..2f5c2e75a1b7 100644
--- a/arch/arm/mach-s3c/Kconfig.s3c64xx
+++ b/arch/arm/mach-s3c/Kconfig.s3c64xx
@@ -117,7 +117,6 @@ config MACH_WLF_CRAGG_6410
select S3C_DEV_I2C1
select S3C_DEV_USB_HOST
select S3C_DEV_USB_HSOTG
- select SAMSUNG_DEV_KEYPAD
select SAMSUNG_DEV_PWM
help
Machine support for the Wolfson Cragganmore S3C6410 variant.
diff --git a/arch/arm/mach-s3c/Makefile.s3c64xx b/arch/arm/mach-s3c/Makefile.s3c64xx
index 61287ad2ea42..1686b1f344f8 100644
--- a/arch/arm/mach-s3c/Makefile.s3c64xx
+++ b/arch/arm/mach-s3c/Makefile.s3c64xx
@@ -32,7 +32,6 @@ obj-y += dev-audio-s3c64xx.o
obj-$(CONFIG_S3C64XX_SETUP_FB_24BPP) += setup-fb-24bpp-s3c64xx.o
obj-$(CONFIG_S3C64XX_SETUP_I2C0) += setup-i2c0-s3c64xx.o
obj-$(CONFIG_S3C64XX_SETUP_I2C1) += setup-i2c1-s3c64xx.o
-obj-$(CONFIG_S3C64XX_SETUP_KEYPAD) += setup-keypad-s3c64xx.o
obj-$(CONFIG_S3C64XX_SETUP_SDHCI_GPIO) += setup-sdhci-gpio-s3c64xx.o
obj-$(CONFIG_S3C64XX_SETUP_SPI) += setup-spi-s3c64xx.o
obj-$(CONFIG_S3C64XX_SETUP_USB_PHY) += setup-usb-phy-s3c64xx.o
diff --git a/arch/arm/mach-s3c/devs.c b/arch/arm/mach-s3c/devs.c
index bab2abd8a34a..0237307b9238 100644
--- a/arch/arm/mach-s3c/devs.c
+++ b/arch/arm/mach-s3c/devs.c
@@ -39,7 +39,6 @@
#include "devs.h"
#include "fb.h"
#include <linux/platform_data/i2c-s3c2410.h>
-#include "keypad.h"
#include "pwm-core.h"
#include "sdhci.h"
#include "usb-phy.h"
@@ -265,32 +264,6 @@ void __init s3c_i2c1_set_platdata(struct s3c2410_platform_i2c *pd)
}
#endif /* CONFIG_S3C_DEV_I2C1 */
-/* KEYPAD */
-
-#ifdef CONFIG_SAMSUNG_DEV_KEYPAD
-static struct resource samsung_keypad_resources[] = {
- [0] = DEFINE_RES_MEM(SAMSUNG_PA_KEYPAD, SZ_32),
- [1] = DEFINE_RES_IRQ(IRQ_KEYPAD),
-};
-
-struct platform_device samsung_device_keypad = {
- .name = "samsung-keypad",
- .id = -1,
- .num_resources = ARRAY_SIZE(samsung_keypad_resources),
- .resource = samsung_keypad_resources,
-};
-
-void __init samsung_keypad_set_platdata(struct samsung_keypad_platdata *pd)
-{
- struct samsung_keypad_platdata *npd;
-
- npd = s3c_set_platdata(pd, sizeof(*npd), &samsung_device_keypad);
-
- if (!npd->cfg_gpio)
- npd->cfg_gpio = samsung_keypad_cfg_gpio;
-}
-#endif /* CONFIG_SAMSUNG_DEV_KEYPAD */
-
/* PWM Timer */
#ifdef CONFIG_SAMSUNG_DEV_PWM
diff --git a/arch/arm/mach-s3c/devs.h b/arch/arm/mach-s3c/devs.h
index 21c00786c264..2737990063b1 100644
--- a/arch/arm/mach-s3c/devs.h
+++ b/arch/arm/mach-s3c/devs.h
@@ -39,7 +39,6 @@ extern struct platform_device s3c_device_i2c1;
extern struct platform_device s3c_device_ohci;
extern struct platform_device s3c_device_usb_hsotg;
-extern struct platform_device samsung_device_keypad;
extern struct platform_device samsung_device_pwm;
/**
diff --git a/arch/arm/mach-s3c/keypad.h b/arch/arm/mach-s3c/keypad.h
deleted file mode 100644
index 9754b9a29945..000000000000
--- a/arch/arm/mach-s3c/keypad.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
-/*
- * Samsung Platform - Keypad platform data definitions
- *
- * Copyright (C) 2010 Samsung Electronics Co.Ltd
- * Author: Joonyoung Shim <jy0922.shim@samsung.com>
- */
-
-#ifndef __PLAT_SAMSUNG_KEYPAD_H
-#define __PLAT_SAMSUNG_KEYPAD_H
-
-#include <linux/input/samsung-keypad.h>
-
-/**
- * samsung_keypad_set_platdata - Set platform data for Samsung Keypad device.
- * @pd: Platform data to register to device.
- *
- * Register the given platform data for use with Samsung Keypad device.
- * The call will copy the platform data, so the board definitions can
- * make the structure itself __initdata.
- */
-extern void samsung_keypad_set_platdata(struct samsung_keypad_platdata *pd);
-
-/* defined by architecture to configure gpio. */
-extern void samsung_keypad_cfg_gpio(unsigned int rows, unsigned int cols);
-
-#endif /* __PLAT_SAMSUNG_KEYPAD_H */
diff --git a/arch/arm/mach-s3c/mach-crag6410.c b/arch/arm/mach-s3c/mach-crag6410.c
index c4040aad1ed3..6354fc2a49b8 100644
--- a/arch/arm/mach-s3c/mach-crag6410.c
+++ b/arch/arm/mach-s3c/mach-crag6410.c
@@ -15,6 +15,7 @@
#include <linux/io.h>
#include <linux/init.h>
#include <linux/input-event-codes.h>
+#include <linux/input/matrix_keypad.h>
#include <linux/gpio.h>
#include <linux/gpio/machine.h>
#include <linux/leds.h>
@@ -22,6 +23,7 @@
#include <linux/mmc/host.h>
#include <linux/regulator/machine.h>
#include <linux/regulator/fixed.h>
+#include <linux/property.h>
#include <linux/pwm.h>
#include <linux/pwm_backlight.h>
#include <linux/dm9000.h>
@@ -53,7 +55,6 @@
#include "gpio-cfg.h"
#include <linux/platform_data/spi-s3c64xx.h>
-#include "keypad.h"
#include "devs.h"
#include "cpu.h"
#include <linux/platform_data/i2c-s3c2410.h>
@@ -176,7 +177,7 @@ static struct s3c_fb_platdata crag6410_lcd_pdata = {
/* 2x6 keypad */
-static uint32_t crag6410_keymap[] = {
+static const uint32_t crag6410_keymap[] __initconst = {
/* KEY(row, col, keycode) */
KEY(0, 0, KEY_VOLUMEUP),
KEY(0, 1, KEY_HOME),
@@ -192,17 +193,41 @@ static uint32_t crag6410_keymap[] = {
KEY(1, 5, KEY_CAMERA),
};
-static struct matrix_keymap_data crag6410_keymap_data = {
- .keymap = crag6410_keymap,
- .keymap_size = ARRAY_SIZE(crag6410_keymap),
+static const struct property_entry crag6410_keypad_props[] __initconst = {
+ PROPERTY_ENTRY_U32("keypad,num-columns", 6),
+ PROPERTY_ENTRY_U32("keypad,num-rows", 2),
+ PROPERTY_ENTRY_U32_ARRAY("linux,keymap", crag6410_keymap),
+ { }
+};
+
+static const struct resource crag6410_keypad_resources[] __initconst = {
+ [0] = DEFINE_RES_MEM(SAMSUNG_PA_KEYPAD, SZ_32),
+ [1] = DEFINE_RES_IRQ(IRQ_KEYPAD),
};
-static struct samsung_keypad_platdata crag6410_keypad_data = {
- .keymap_data = &crag6410_keymap_data,
- .rows = 2,
- .cols = 6,
+static const struct platform_device_info crag6410_keypad_info __initconst = {
+ .name = "samsung-keypad",
+ .id = PLATFORM_DEVID_NONE,
+ .res = crag6410_keypad_resources,
+ .num_res = ARRAY_SIZE(crag6410_keypad_resources),
+ .properties = crag6410_keypad_props,
};
+static void __init crag6410_setup_keypad(void)
+{
+ struct platform_device *pd;
+
+ /* Set all the necessary GPK pins to special-function 3: KP_ROW[x] */
+ s3c_gpio_cfgrange_nopull(S3C64XX_GPK(8), 2, S3C_GPIO_SFN(3));
+
+ /* Set all the necessary GPL pins to special-function 3: KP_COL[x] */
+ s3c_gpio_cfgrange_nopull(S3C64XX_GPL(0), 6, S3C_GPIO_SFN(3));
+
+ pd = platform_device_register_full(&crag6410_keypad_info);
+ if (IS_ERR(pd))
+ pr_err("failed to instantiate keypad device");
+}
+
static struct gpio_keys_button crag6410_gpio_keys[] = {
[0] = {
.code = KEY_SUSPEND,
@@ -361,7 +386,7 @@ static struct platform_device wallvdd_device = {
},
};
-static struct platform_device *crag6410_devices[] __initdata = {
+static struct platform_device *crag6410_devs0[] __initdata = {
&s3c_device_hsmmc0,
&s3c_device_hsmmc2,
&s3c_device_i2c0,
@@ -372,8 +397,10 @@ static struct platform_device *crag6410_devices[] __initdata = {
&samsung_device_pwm,
&s3c64xx_device_iis0,
&s3c64xx_device_iis1,
- &samsung_device_keypad,
&crag6410_gpio_keydev,
+};
+
+static struct platform_device *crag6410_devs1[] __initdata = {
&crag6410_dm9k_device,
&s3c64xx_device_spi0,
&crag6410_lcd_powerdev,
@@ -873,16 +900,17 @@ static void __init crag6410_machine_init(void)
gpiod_add_lookup_table(&crag_wm1250_ev1_gpiod_table);
i2c_register_board_info(1, i2c_devs1, ARRAY_SIZE(i2c_devs1));
- samsung_keypad_set_platdata(&crag6410_keypad_data);
-
gpiod_add_lookup_table(&crag_spi0_gpiod_table);
s3c64xx_spi0_set_platdata(0, 2);
pwm_add_table(crag6410_pwm_lookup, ARRAY_SIZE(crag6410_pwm_lookup));
- platform_add_devices(crag6410_devices, ARRAY_SIZE(crag6410_devices));
+ platform_add_devices(crag6410_devs0, ARRAY_SIZE(crag6410_devs0));
platform_device_register_full(&crag6410_mmgpio_devinfo);
gpiod_add_lookup_table(&crag_leds_table);
+ crag6410_setup_keypad();
+
+ platform_add_devices(crag6410_devs1, ARRAY_SIZE(crag6410_devs1));
gpio_led_register_device(-1, &gpio_leds_pdata);
regulator_has_full_constraints();
diff --git a/arch/arm/mach-s3c/setup-keypad-s3c64xx.c b/arch/arm/mach-s3c/setup-keypad-s3c64xx.c
deleted file mode 100644
index 8463ad37c6ab..000000000000
--- a/arch/arm/mach-s3c/setup-keypad-s3c64xx.c
+++ /dev/null
@@ -1,20 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-//
-// Copyright (c) 2010 Samsung Electronics Co., Ltd.
-// http://www.samsung.com/
-//
-// GPIO configuration for S3C64XX KeyPad device
-
-#include <linux/gpio.h>
-#include "gpio-cfg.h"
-#include "keypad.h"
-#include "gpio-samsung.h"
-
-void samsung_keypad_cfg_gpio(unsigned int rows, unsigned int cols)
-{
- /* Set all the necessary GPK pins to special-function 3: KP_ROW[x] */
- s3c_gpio_cfgrange_nopull(S3C64XX_GPK(8), rows, S3C_GPIO_SFN(3));
-
- /* Set all the necessary GPL pins to special-function 3: KP_COL[x] */
- s3c_gpio_cfgrange_nopull(S3C64XX_GPL(0), cols, S3C_GPIO_SFN(3));
-}
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related
* [PATCH v2 05/11] Input: samsung-keypad - remove support for platform data
From: Dmitry Torokhov @ 2026-07-09 4:53 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Peter Griffin,
Alim Akhtar, Russell King, Mark Brown, Linus Walleij,
Charles Keepax, Sam Protsenko, Arnd Bergmann, Bartosz Golaszewski
Cc: Krzysztof Kozlowski, linux-input, devicetree, linux-kernel,
linux-arm-kernel, linux-samsung-soc, patches
In-Reply-To: <20260708-samsung-kp-v2-0-3c6ed4c9b3b6@gmail.com>
Because there are no more users of samsung_keypad_platdata left in
the kernel remove support for it from the driver.
The driver supports generic device properties so all configuration
should be done using them instead of a custom platform data.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/keyboard/samsung-keypad.c | 184 ++++++++++++--------------------
include/linux/input/samsung-keypad.h | 39 -------
2 files changed, 68 insertions(+), 155 deletions(-)
diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
index b9d4ea5f202a..6f1d766a4134 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -12,6 +12,7 @@
#include <linux/delay.h>
#include <linux/err.h>
#include <linux/input.h>
+#include <linux/input/matrix_keypad.h>
#include <linux/interrupt.h>
#include <linux/io.h>
#include <linux/module.h>
@@ -21,7 +22,9 @@
#include <linux/slab.h>
#include <linux/of.h>
#include <linux/sched.h>
-#include <linux/input/samsung-keypad.h>
+
+#define SAMSUNG_MAX_ROWS 8
+#define SAMSUNG_MAX_COLS 8
#define SAMSUNG_KEYIFCON 0x00
#define SAMSUNG_KEYIFSTSCLR 0x04
@@ -231,144 +234,77 @@ static void samsung_keypad_close(struct input_dev *input_dev)
samsung_keypad_stop(keypad);
}
-static const struct matrix_keymap_data *
-samsung_parse_verbose_keymap(struct device *dev)
+static int samsung_keypad_parse_keymap(struct samsung_keypad *keypad)
{
- struct matrix_keymap_data *keymap_data;
+ struct matrix_keymap_data keymap_data = { 0 };
+ struct device *dev = &keypad->pdev->dev;
struct fwnode_handle *child;
u32 *keymap;
unsigned int key_count;
-
- keymap_data = devm_kzalloc(dev, sizeof(*keymap_data), GFP_KERNEL);
- if (!keymap_data)
- return ERR_PTR(-ENOMEM);
+ int retval;
key_count = device_get_child_node_count(dev);
- keymap = devm_kcalloc(dev, key_count, sizeof(*keymap), GFP_KERNEL);
- if (!keymap)
- return ERR_PTR(-ENOMEM);
+ if (key_count) {
+ keymap = kcalloc(key_count, sizeof(*keymap), GFP_KERNEL);
+ if (!keymap)
+ return -ENOMEM;
- keymap_data->keymap_size = key_count;
- keymap_data->keymap = keymap;
+ keymap_data.keymap = keymap;
+ keymap_data.keymap_size = key_count;
- device_for_each_child_node(dev, child) {
- u32 row, col, key_code;
+ device_for_each_child_node(dev, child) {
+ u32 row, col, key_code;
- fwnode_property_read_u32(child, "keypad,row", &row);
- fwnode_property_read_u32(child, "keypad,column", &col);
- fwnode_property_read_u32(child, "linux,code", &key_code);
+ fwnode_property_read_u32(child, "keypad,row", &row);
+ fwnode_property_read_u32(child, "keypad,column", &col);
+ fwnode_property_read_u32(child, "linux,code", &key_code);
- *keymap++ = KEY(row, col, key_code);
+ *keymap++ = KEY(row, col, key_code);
+ }
}
- return keymap_data;
-}
-
-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)
- return ERR_PTR(-ENOMEM);
-
- 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);
-
- pdata->rows = num_rows;
- pdata->cols = num_cols;
-
- if (!device_property_present(dev, "linux,keymap")) {
- keymap_data = samsung_parse_verbose_keymap(dev);
- if (IS_ERR(keymap_data))
- return ERR_CAST(keymap_data);
-
- pdata->keymap_data = keymap_data;
- }
-
-
- pdata->no_autorepeat =
- device_property_read_bool(dev, "linux,input-no-autorepeat");
-
- pdata->wakeup = device_property_read_bool(dev, "wakeup-source") ||
- /* legacy name */
- device_property_read_bool(dev, "linux,input-wakeup");
-
- return pdata;
+ retval = matrix_keypad_build_keymap(key_count ? &keymap_data : NULL,
+ NULL, keypad->rows, keypad->cols,
+ keypad->keycodes,
+ keypad->input_dev);
+ kfree(keymap_data.keymap);
+ return retval;
}
static int samsung_keypad_probe(struct platform_device *pdev)
{
- const struct samsung_keypad_platdata *pdata;
const struct platform_device_id *id;
+ struct device *dev = &pdev->dev;
struct samsung_keypad *keypad;
struct resource *res;
struct input_dev *input_dev;
unsigned int row_shift;
+ u32 num_rows = 0, num_cols = 0;
+ bool wakeup;
int error;
+ device_property_read_u32(dev, "samsung,keypad-num-rows", &num_rows);
+ device_property_read_u32(dev, "samsung,keypad-num-columns", &num_cols);
- pdata = dev_get_platdata(&pdev->dev);
- if (pdata) {
- if (!pdata->keymap_data) {
- dev_err(&pdev->dev, "no keymap data defined\n");
- return -EINVAL;
- }
- } else {
- pdata = samsung_keypad_parse_properties(&pdev->dev);
- if (IS_ERR(pdata))
- return PTR_ERR(pdata);
- }
-
- if (!pdata->rows || pdata->rows > SAMSUNG_MAX_ROWS)
- return -EINVAL;
+ error = matrix_keypad_parse_properties(dev, &num_rows, &num_cols);
+ if (error)
+ return error;
- if (!pdata->cols || pdata->cols > SAMSUNG_MAX_COLS)
+ if (num_rows > SAMSUNG_MAX_ROWS || num_cols > SAMSUNG_MAX_COLS)
return -EINVAL;
- /* initialize the gpio */
- if (pdata->cfg_gpio)
- pdata->cfg_gpio(pdata->rows, pdata->cols);
-
- row_shift = get_count_order(pdata->cols);
+ row_shift = get_count_order(num_cols);
keypad = devm_kzalloc(&pdev->dev,
struct_size(keypad, keycodes,
- pdata->rows << row_shift),
+ num_rows << row_shift),
GFP_KERNEL);
if (!keypad)
return -ENOMEM;
- input_dev = devm_input_allocate_device(&pdev->dev);
- if (!input_dev)
- return -ENOMEM;
-
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res)
- return -ENODEV;
-
- keypad->base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
- if (!keypad->base)
- return -EBUSY;
-
- keypad->clk = devm_clk_get_prepared(&pdev->dev, "keypad");
- if (IS_ERR(keypad->clk)) {
- dev_err(&pdev->dev, "failed to get keypad clk\n");
- return PTR_ERR(keypad->clk);
- }
-
- keypad->input_dev = input_dev;
keypad->pdev = pdev;
keypad->row_shift = row_shift;
- keypad->rows = pdata->rows;
- keypad->cols = pdata->cols;
+ keypad->rows = num_rows;
+ keypad->cols = num_cols;
keypad->stopped = true;
init_waitqueue_head(&keypad->wait);
@@ -384,26 +320,45 @@ static int samsung_keypad_probe(struct platform_device *pdev)
return -EINVAL;
}
+ input_dev = devm_input_allocate_device(&pdev->dev);
+ if (!input_dev)
+ return -ENOMEM;
+
+ keypad->input_dev = input_dev;
+
input_dev->name = pdev->name;
input_dev->id.bustype = BUS_HOST;
input_dev->open = samsung_keypad_open;
input_dev->close = samsung_keypad_close;
- error = matrix_keypad_build_keymap(pdata->keymap_data, NULL,
- pdata->rows, pdata->cols,
- keypad->keycodes, input_dev);
+ error = samsung_keypad_parse_keymap(keypad);
if (error) {
dev_err(&pdev->dev, "failed to build keymap\n");
return error;
}
input_set_capability(input_dev, EV_MSC, MSC_SCAN);
- if (!pdata->no_autorepeat)
+
+ if (!device_property_read_bool(&pdev->dev, "linux,input-no-autorepeat"))
__set_bit(EV_REP, input_dev->evbit);
input_set_drvdata(input_dev, keypad);
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -ENODEV;
+
+ keypad->base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+ if (!keypad->base)
+ return -EBUSY;
+
+ keypad->clk = devm_clk_get_prepared(&pdev->dev, "keypad");
+ if (IS_ERR(keypad->clk)) {
+ dev_err(&pdev->dev, "failed to get keypad clk\n");
+ return PTR_ERR(keypad->clk);
+ }
+
keypad->irq = platform_get_irq(pdev, 0);
if (keypad->irq < 0) {
error = keypad->irq;
@@ -418,7 +373,11 @@ static int samsung_keypad_probe(struct platform_device *pdev)
return error;
}
- device_init_wakeup(&pdev->dev, pdata->wakeup);
+ wakeup = device_property_read_bool(dev, "wakeup-source") ||
+ /* legacy name */
+ device_property_read_bool(dev, "linux,input-wakeup");
+ device_init_wakeup(&pdev->dev, wakeup);
+
platform_set_drvdata(pdev, keypad);
error = devm_pm_runtime_enable(&pdev->dev);
@@ -429,13 +388,6 @@ static int samsung_keypad_probe(struct platform_device *pdev)
if (error)
return error;
- if (!dev_get_platdata(&pdev->dev)) {
- if (pdata->keymap_data) {
- devm_kfree(&pdev->dev, (void *)pdata->keymap_data->keymap);
- devm_kfree(&pdev->dev, (void *)pdata->keymap_data);
- }
- devm_kfree(&pdev->dev, (void *)pdata);
- }
return 0;
}
diff --git a/include/linux/input/samsung-keypad.h b/include/linux/input/samsung-keypad.h
deleted file mode 100644
index ab6b97114c08..000000000000
--- a/include/linux/input/samsung-keypad.h
+++ /dev/null
@@ -1,39 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * Samsung Keypad platform data definitions
- *
- * Copyright (C) 2010 Samsung Electronics Co.Ltd
- * Author: Joonyoung Shim <jy0922.shim@samsung.com>
- */
-
-#ifndef __SAMSUNG_KEYPAD_H
-#define __SAMSUNG_KEYPAD_H
-
-#include <linux/input/matrix_keypad.h>
-
-#define SAMSUNG_MAX_ROWS 8
-#define SAMSUNG_MAX_COLS 8
-
-/**
- * struct samsung_keypad_platdata - Platform device data for Samsung Keypad.
- * @keymap_data: pointer to &matrix_keymap_data.
- * @rows: number of keypad row supported.
- * @cols: number of keypad col supported.
- * @no_autorepeat: disable key autorepeat.
- * @wakeup: controls whether the device should be set up as wakeup source.
- * @cfg_gpio: configure the GPIO.
- *
- * Initialisation data specific to either the machine or the platform
- * for the device driver to use or call-back when configuring gpio.
- */
-struct samsung_keypad_platdata {
- const struct matrix_keymap_data *keymap_data;
- unsigned int rows;
- unsigned int cols;
- bool no_autorepeat;
- bool wakeup;
-
- void (*cfg_gpio)(unsigned int rows, unsigned int cols);
-};
-
-#endif /* __SAMSUNG_KEYPAD_H */
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related
* [PATCH v2 06/11] ARM: s3c: crag6410: use software nodes/properties to set up GPIO keys
From: Dmitry Torokhov @ 2026-07-09 4:53 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Peter Griffin,
Alim Akhtar, Russell King, Mark Brown, Linus Walleij,
Charles Keepax, Sam Protsenko, Arnd Bergmann, Bartosz Golaszewski
Cc: Krzysztof Kozlowski, linux-input, devicetree, linux-kernel,
linux-arm-kernel, linux-samsung-soc, patches, Krzysztof Kozlowski
In-Reply-To: <20260708-samsung-kp-v2-0-3c6ed4c9b3b6@gmail.com>
Switch the gpio-keys device to use software inodes/properties to
describe the buttons and switches. This will allow dropping support
for platform data from the gpio-keys driver in the future.
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
arch/arm/mach-s3c/mach-crag6410.c | 79 ++++++++++++++++++++++++++++-----------
1 file changed, 58 insertions(+), 21 deletions(-)
diff --git a/arch/arm/mach-s3c/mach-crag6410.c b/arch/arm/mach-s3c/mach-crag6410.c
index 6354fc2a49b8..9682ef67b866 100644
--- a/arch/arm/mach-s3c/mach-crag6410.c
+++ b/arch/arm/mach-s3c/mach-crag6410.c
@@ -18,6 +18,7 @@
#include <linux/input/matrix_keypad.h>
#include <linux/gpio.h>
#include <linux/gpio/machine.h>
+#include <linux/gpio/property.h>
#include <linux/leds.h>
#include <linux/delay.h>
#include <linux/mmc/host.h>
@@ -228,32 +229,68 @@ static void __init crag6410_setup_keypad(void)
pr_err("failed to instantiate keypad device");
}
-static struct gpio_keys_button crag6410_gpio_keys[] = {
- [0] = {
- .code = KEY_SUSPEND,
- .gpio = S3C64XX_GPL(10), /* EINT 18 */
- .type = EV_KEY,
- .wakeup = 1,
- .active_low = 1,
- },
- [1] = {
- .code = SW_FRONT_PROXIMITY,
- .gpio = S3C64XX_GPN(11), /* EINT 11 */
- .type = EV_SW,
- },
+static const struct software_node crag6410_gpio_keys_node = {
+ .name = "crag6410-gpio-keys",
};
-static struct gpio_keys_platform_data crag6410_gpio_keydata = {
- .buttons = crag6410_gpio_keys,
- .nbuttons = ARRAY_SIZE(crag6410_gpio_keys),
+static const struct property_entry crag6410_suspend_key_props[] = {
+ PROPERTY_ENTRY_U32("linux,code", KEY_SUSPEND),
+ PROPERTY_ENTRY_GPIO("gpios",
+ SAMSUNG_GPIO_NODE('L'), 10, /* EINT 18 */
+ GPIO_ACTIVE_LOW),
+ PROPERTY_ENTRY_BOOL("wakeup-source"),
+ { }
};
-static struct platform_device crag6410_gpio_keydev = {
- .name = "gpio-keys",
- .id = 0,
- .dev.platform_data = &crag6410_gpio_keydata,
+static const struct software_node crag6410_suspend_key_node = {
+ .parent = &crag6410_gpio_keys_node,
+ .properties = crag6410_suspend_key_props,
+};
+
+static const struct property_entry crag6410_prox_sw_props[] = {
+ PROPERTY_ENTRY_U32("linux,input-type", EV_SW),
+ PROPERTY_ENTRY_U32("linux,code", SW_FRONT_PROXIMITY),
+ PROPERTY_ENTRY_GPIO("gpios",
+ SAMSUNG_GPIO_NODE('N'), 11, /* EINT 11 */
+ GPIO_ACTIVE_HIGH),
+ { }
+};
+
+static const struct software_node crag6410_prox_sw_node = {
+ .parent = &crag6410_gpio_keys_node,
+ .properties = crag6410_prox_sw_props,
};
+static const struct software_node *crag6410_gpio_keys_swnodes[] = {
+ &crag6410_gpio_keys_node,
+ &crag6410_suspend_key_node,
+ &crag6410_prox_sw_node,
+ NULL
+};
+
+static void __init crag6410_setup_gpio_keys(void)
+{
+ struct platform_device_info keys_info = {
+ .name = "gpio-keys",
+ .id = 0,
+ };
+ struct platform_device *pd;
+ int err;
+
+ err = software_node_register_node_group(crag6410_gpio_keys_swnodes);
+ if (err) {
+ pr_err("failed to register gpio-keys software nodes: %d\n", err);
+ return;
+ }
+
+ keys_info.fwnode = software_node_fwnode(&crag6410_gpio_keys_node);
+
+ pd = platform_device_register_full(&keys_info);
+ err = PTR_ERR_OR_ZERO(pd);
+ if (err)
+ pr_err("failed to create gpio-keys device: %d\n", err);
+}
+
static struct resource crag6410_dm9k_resource[] = {
[0] = DEFINE_RES_MEM(S3C64XX_PA_XM0CSN5, 2),
[1] = DEFINE_RES_MEM(S3C64XX_PA_XM0CSN5 + (1 << 8), 2),
@@ -397,7 +434,6 @@ static struct platform_device *crag6410_devs0[] __initdata = {
&samsung_device_pwm,
&s3c64xx_device_iis0,
&s3c64xx_device_iis1,
- &crag6410_gpio_keydev,
};
static struct platform_device *crag6410_devs1[] __initdata = {
@@ -909,6 +945,7 @@ static void __init crag6410_machine_init(void)
gpiod_add_lookup_table(&crag_leds_table);
crag6410_setup_keypad();
+ crag6410_setup_gpio_keys();
platform_add_devices(crag6410_devs1, ARRAY_SIZE(crag6410_devs1));
gpio_led_register_device(-1, &gpio_leds_pdata);
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related
* [PATCH v2 07/11] regulator: wm831x: support software node in platform data
From: Dmitry Torokhov @ 2026-07-09 4:53 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Peter Griffin,
Alim Akhtar, Russell King, Mark Brown, Linus Walleij,
Charles Keepax, Sam Protsenko, Arnd Bergmann, Bartosz Golaszewski
Cc: Krzysztof Kozlowski, linux-input, devicetree, linux-kernel,
linux-arm-kernel, linux-samsung-soc, patches
In-Reply-To: <20260708-samsung-kp-v2-0-3c6ed4c9b3b6@gmail.com>
Allow passing a software node via platform data to the wm831x buckv
regulators. This is useful for non-DT/non-ACPI platforms that want to
associate device properties (like DVS GPIOs) with the regulator devices
using software nodes.
If the software node is present, the driver will also attempt to read
DVS configuration ("wlf,dvs-init-state" and "wlf,dvs-control-src") from
it, falling back to legacy platform data fields if properties are missing.
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/regulator/wm831x-dcdc.c | 24 +++++++++++++++++++++---
include/linux/mfd/wm831x/pdata.h | 2 ++
2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/drivers/regulator/wm831x-dcdc.c b/drivers/regulator/wm831x-dcdc.c
index 834d7c181971..ce43c51c0170 100644
--- a/drivers/regulator/wm831x-dcdc.c
+++ b/drivers/regulator/wm831x-dcdc.c
@@ -16,6 +16,7 @@
#include <linux/regulator/driver.h>
#include <linux/regulator/machine.h>
#include <linux/gpio/consumer.h>
+#include <linux/property.h>
#include <linux/slab.h>
#include <linux/mfd/wm831x/core.h>
@@ -332,14 +333,26 @@ static void wm831x_buckv_dvs_init(struct platform_device *pdev,
struct wm831x *wm831x = dcdc->wm831x;
int ret;
u16 ctrl;
+ int dvs_control_src;
+ u32 val;
if (!pdata)
return;
+ if (pdata->swnode) {
+ struct fwnode_handle *fwnode = software_node_fwnode(pdata->swnode);
+
+ if (fwnode)
+ device_set_node(&pdev->dev, fwnode);
+ }
+
/* gpiolib won't let us read the GPIO status so pick the higher
* of the two existing voltages so we take it as platform data.
*/
- dcdc->dvs_gpio_state = pdata->dvs_init_state;
+ if (device_property_read_u32(&pdev->dev, "wlf,dvs-init-state", &val) == 0)
+ dcdc->dvs_gpio_state = val;
+ else
+ dcdc->dvs_gpio_state = pdata->dvs_init_state;
dcdc->dvs_gpiod = devm_gpiod_get(&pdev->dev, "dvs",
dcdc->dvs_gpio_state ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW);
@@ -349,7 +362,12 @@ static void wm831x_buckv_dvs_init(struct platform_device *pdev,
return;
}
- switch (pdata->dvs_control_src) {
+ if (device_property_read_u32(&pdev->dev, "wlf,dvs-control-src", &val) == 0)
+ dvs_control_src = val;
+ else
+ dvs_control_src = pdata->dvs_control_src;
+
+ switch (dvs_control_src) {
case 1:
ctrl = 2 << WM831X_DC1_DVS_SRC_SHIFT;
break;
@@ -358,7 +376,7 @@ static void wm831x_buckv_dvs_init(struct platform_device *pdev,
break;
default:
dev_err(wm831x->dev, "Invalid DVS control source %d for %s\n",
- pdata->dvs_control_src, dcdc->name);
+ dvs_control_src, dcdc->name);
return;
}
diff --git a/include/linux/mfd/wm831x/pdata.h b/include/linux/mfd/wm831x/pdata.h
index 75aa94dadf1c..d73a04c82ca1 100644
--- a/include/linux/mfd/wm831x/pdata.h
+++ b/include/linux/mfd/wm831x/pdata.h
@@ -12,6 +12,7 @@
struct wm831x;
struct regulator_init_data;
+struct software_node;
struct wm831x_backlight_pdata {
int isink; /** ISINK to use, 1 or 2 */
@@ -50,6 +51,7 @@ struct wm831x_buckv_pdata {
int dvs_control_src; /** Hardware DVS source to use (1 or 2) */
int dvs_init_state; /** DVS state to expect on startup */
int dvs_state_gpio; /** CPU GPIO to use for monitoring status */
+ const struct software_node *swnode; /** Software node for properties */
};
/* Sources for status LED configuration. Values are register values
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related
* [PATCH v2 08/11] ARM: s3c: crag6410: convert PMIC to software properties
From: Dmitry Torokhov @ 2026-07-09 4:53 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Peter Griffin,
Alim Akhtar, Russell King, Mark Brown, Linus Walleij,
Charles Keepax, Sam Protsenko, Arnd Bergmann, Bartosz Golaszewski
Cc: Krzysztof Kozlowski, linux-input, devicetree, linux-kernel,
linux-arm-kernel, linux-samsung-soc, patches
In-Reply-To: <20260708-samsung-kp-v2-0-3c6ed4c9b3b6@gmail.com>
Convert the PMIC DVS GPIO configuration from a legacy lookup table to
software properties. We use the new 'swnode' field in 'struct
wm831x_buckv_pdata' to pass the software node to the regulator driver.
The DVS control source is also passed via software node properties.
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
arch/arm/mach-s3c/mach-crag6410.c | 33 ++++++++++++++-------------------
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/arch/arm/mach-s3c/mach-crag6410.c b/arch/arm/mach-s3c/mach-crag6410.c
index 9682ef67b866..4c9b8c1fb08e 100644
--- a/arch/arm/mach-s3c/mach-crag6410.c
+++ b/arch/arm/mach-s3c/mach-crag6410.c
@@ -456,9 +456,21 @@ static struct pca953x_platform_data crag6410_pca_data = {
.irq_base = -1,
};
+static const struct property_entry crag_dcdc1_properties[] = {
+ PROPERTY_ENTRY_GPIO("dvs-gpios",
+ SAMSUNG_GPIO_NODE('K'), 0, GPIO_ACTIVE_HIGH),
+ PROPERTY_ENTRY_U32("wlf,dvs-control-src", 1),
+ { }
+};
+
+static const struct software_node crag_dcdc1_swnode = {
+ .name = "wm831x-buckv-dcdc1",
+ .properties = crag_dcdc1_properties,
+};
+
/* VDDARM is controlled by DVS1 connected to GPK(0) */
static struct wm831x_buckv_pdata vddarm_pdata = {
- .dvs_control_src = 1,
+ .swnode = &crag_dcdc1_swnode,
};
static struct regulator_consumer_supply vddarm_consumers[] = {
@@ -656,23 +668,6 @@ static struct wm831x_pdata crag_pmic_pdata = {
.touch = &touch_pdata,
};
-/*
- * VDDARM is eventually ending up as a regulator hanging on the MFD cell device
- * "wm831x-buckv.1" spawn from drivers/mfd/wm831x-core.c.
- *
- * From the note on the platform data we can see that this is clearly DVS1
- * and assigned as dcdc1 resource to the MFD core which sets .id of the cell
- * spawning the DVS1 platform device to 1, then the cell platform device
- * name is calculated from 10*instance + id resulting in the device name
- * "wm831x-buckv.11"
- */
-static struct gpiod_lookup_table crag_pmic_gpiod_table = {
- .dev_id = "wm831x-buckv.11",
- .table = {
- GPIO_LOOKUP("GPIOK", 0, "dvs", GPIO_ACTIVE_HIGH),
- { },
- },
-};
static struct i2c_board_info i2c_devs0[] = {
{ I2C_BOARD_INFO("24c08", 0x50), },
@@ -931,7 +926,7 @@ static void __init crag6410_machine_init(void)
s3c_fb_set_platdata(&crag6410_lcd_pdata);
dwc2_hsotg_set_platdata(&crag6410_hsotg_pdata);
- gpiod_add_lookup_table(&crag_pmic_gpiod_table);
+ software_node_register(&crag_dcdc1_swnode);
i2c_register_board_info(0, i2c_devs0, ARRAY_SIZE(i2c_devs0));
gpiod_add_lookup_table(&crag_wm1250_ev1_gpiod_table);
i2c_register_board_info(1, i2c_devs1, ARRAY_SIZE(i2c_devs1));
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related
* [PATCH v2 09/11] regulator: wm831x: remove legacy DVS platform data
From: Dmitry Torokhov @ 2026-07-09 4:53 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Peter Griffin,
Alim Akhtar, Russell King, Mark Brown, Linus Walleij,
Charles Keepax, Sam Protsenko, Arnd Bergmann, Bartosz Golaszewski
Cc: Krzysztof Kozlowski, linux-input, devicetree, linux-kernel,
linux-arm-kernel, linux-samsung-soc, patches
In-Reply-To: <20260708-samsung-kp-v2-0-3c6ed4c9b3b6@gmail.com>
Now that the only board file in mainline using wm831x buckv DVS has been
converted to use software properties, we can remove the unused legacy
DVS fields from 'struct wm831x_buckv_pdata'. Also remove the fallback
logic from the regulator driver, making DVS support purely
property-based when software nodes are used.
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/regulator/wm831x-dcdc.c | 22 ++++++++++------------
include/linux/mfd/wm831x/pdata.h | 3 ---
2 files changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/regulator/wm831x-dcdc.c b/drivers/regulator/wm831x-dcdc.c
index ce43c51c0170..05f49b2f5bad 100644
--- a/drivers/regulator/wm831x-dcdc.c
+++ b/drivers/regulator/wm831x-dcdc.c
@@ -336,15 +336,10 @@ static void wm831x_buckv_dvs_init(struct platform_device *pdev,
int dvs_control_src;
u32 val;
- if (!pdata)
+ if (!pdata || !pdata->swnode)
return;
- if (pdata->swnode) {
- struct fwnode_handle *fwnode = software_node_fwnode(pdata->swnode);
-
- if (fwnode)
- device_set_node(&pdev->dev, fwnode);
- }
+ device_set_node(&pdev->dev, software_node_fwnode(pdata->swnode));
/* gpiolib won't let us read the GPIO status so pick the higher
* of the two existing voltages so we take it as platform data.
@@ -352,7 +347,7 @@ static void wm831x_buckv_dvs_init(struct platform_device *pdev,
if (device_property_read_u32(&pdev->dev, "wlf,dvs-init-state", &val) == 0)
dcdc->dvs_gpio_state = val;
else
- dcdc->dvs_gpio_state = pdata->dvs_init_state;
+ dcdc->dvs_gpio_state = 0;
dcdc->dvs_gpiod = devm_gpiod_get(&pdev->dev, "dvs",
dcdc->dvs_gpio_state ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW);
@@ -362,10 +357,13 @@ static void wm831x_buckv_dvs_init(struct platform_device *pdev,
return;
}
- if (device_property_read_u32(&pdev->dev, "wlf,dvs-control-src", &val) == 0)
- dvs_control_src = val;
- else
- dvs_control_src = pdata->dvs_control_src;
+ ret = device_property_read_u32(&pdev->dev, "wlf,dvs-control-src", &val);
+ if (ret) {
+ dev_err(wm831x->dev, "Failed to read DVS control source for %s: %d\n",
+ dcdc->name, ret);
+ return;
+ }
+ dvs_control_src = val;
switch (dvs_control_src) {
case 1:
diff --git a/include/linux/mfd/wm831x/pdata.h b/include/linux/mfd/wm831x/pdata.h
index d73a04c82ca1..c48333552f5a 100644
--- a/include/linux/mfd/wm831x/pdata.h
+++ b/include/linux/mfd/wm831x/pdata.h
@@ -48,9 +48,6 @@ struct wm831x_battery_pdata {
* I2C or SPI buses.
*/
struct wm831x_buckv_pdata {
- int dvs_control_src; /** Hardware DVS source to use (1 or 2) */
- int dvs_init_state; /** DVS state to expect on startup */
- int dvs_state_gpio; /** CPU GPIO to use for monitoring status */
const struct software_node *swnode; /** Software node for properties */
};
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related
* [PATCH v2 10/11] ARM: s3c: crag6410: convert remaining GPIO lookup tables to property entries
From: Dmitry Torokhov @ 2026-07-09 4:53 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Peter Griffin,
Alim Akhtar, Russell King, Mark Brown, Linus Walleij,
Charles Keepax, Sam Protsenko, Arnd Bergmann, Bartosz Golaszewski
Cc: Krzysztof Kozlowski, linux-input, devicetree, linux-kernel,
linux-arm-kernel, linux-samsung-soc, patches
In-Reply-To: <20260708-samsung-kp-v2-0-3c6ed4c9b3b6@gmail.com>
Static property entries support defining GPIOs and are more similar to
device tree properties and are not prone to losing link between device
and a lookup table because of changes in device name. Convert the board
to use them for WM1250 and SPI0 devices.
This also fixes issue with recent conversion to GPIO descriptors
where GPIO lookup tables were specifying incorrect GPIO chip name
("GPIO<N>" vs "GP<N>").
Fixes: 10a366f36e2a ("ASoC: wm1250-ev1: Convert to GPIO descriptors")
Fixes: a45cf3cc72dd ("spi: s3c64xx: Convert to use GPIO descriptors")
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
arch/arm/mach-s3c/devs.c | 35 -----------------
arch/arm/mach-s3c/devs.h | 1 -
arch/arm/mach-s3c/mach-crag6410.c | 83 ++++++++++++++++++++++++++++-----------
3 files changed, 59 insertions(+), 60 deletions(-)
diff --git a/arch/arm/mach-s3c/devs.c b/arch/arm/mach-s3c/devs.c
index 0237307b9238..d1125316043b 100644
--- a/arch/arm/mach-s3c/devs.c
+++ b/arch/arm/mach-s3c/devs.c
@@ -335,38 +335,3 @@ void __init dwc2_hsotg_set_platdata(struct dwc2_hsotg_plat *pd)
npd->phy_exit = s3c_usb_phy_exit;
}
#endif /* CONFIG_S3C_DEV_USB_HSOTG */
-
-#ifdef CONFIG_S3C64XX_DEV_SPI0
-static struct resource s3c64xx_spi0_resource[] = {
- [0] = DEFINE_RES_MEM(S3C_PA_SPI0, SZ_256),
- [1] = DEFINE_RES_IRQ(IRQ_SPI0),
-};
-
-struct platform_device s3c64xx_device_spi0 = {
- .name = "s3c6410-spi",
- .id = 0,
- .num_resources = ARRAY_SIZE(s3c64xx_spi0_resource),
- .resource = s3c64xx_spi0_resource,
- .dev = {
- .dma_mask = &samsung_device_dma_mask,
- .coherent_dma_mask = DMA_BIT_MASK(32),
- },
-};
-
-void __init s3c64xx_spi0_set_platdata(int src_clk_nr, int num_cs)
-{
- struct s3c64xx_spi_info pd;
-
- /* Reject invalid configuration */
- if (!num_cs || src_clk_nr < 0) {
- pr_err("%s: Invalid SPI configuration\n", __func__);
- return;
- }
-
- pd.num_cs = num_cs;
- pd.src_clk_nr = src_clk_nr;
- pd.cfg_gpio = s3c64xx_spi0_cfg_gpio;
-
- s3c_set_platdata(&pd, sizeof(pd), &s3c64xx_device_spi0);
-}
-#endif /* CONFIG_S3C64XX_DEV_SPI0 */
diff --git a/arch/arm/mach-s3c/devs.h b/arch/arm/mach-s3c/devs.h
index 2737990063b1..90a86ade3570 100644
--- a/arch/arm/mach-s3c/devs.h
+++ b/arch/arm/mach-s3c/devs.h
@@ -27,7 +27,6 @@ extern struct platform_device *s3c24xx_uart_src[];
extern struct platform_device s3c64xx_device_iis0;
extern struct platform_device s3c64xx_device_iis1;
-extern struct platform_device s3c64xx_device_spi0;
extern struct platform_device s3c_device_fb;
extern struct platform_device s3c_device_hsmmc0;
diff --git a/arch/arm/mach-s3c/mach-crag6410.c b/arch/arm/mach-s3c/mach-crag6410.c
index 4c9b8c1fb08e..03a97a20e38c 100644
--- a/arch/arm/mach-s3c/mach-crag6410.c
+++ b/arch/arm/mach-s3c/mach-crag6410.c
@@ -438,7 +438,6 @@ static struct platform_device *crag6410_devs0[] __initdata = {
static struct platform_device *crag6410_devs1[] __initdata = {
&crag6410_dm9k_device,
- &s3c64xx_device_spi0,
&crag6410_lcd_powerdev,
&crag6410_backlight_device,
&speyside_device,
@@ -771,17 +770,22 @@ static struct wm831x_pdata glenfarclas_pmic_pdata = {
.disable_touch = true,
};
-static struct gpiod_lookup_table crag_wm1250_ev1_gpiod_table = {
- /* The WM1250-EV1 is device 0027 on I2C bus 1 */
- .dev_id = "1-0027",
- .table = {
- GPIO_LOOKUP("GPION", 12, "clk-ena", GPIO_ACTIVE_HIGH),
- GPIO_LOOKUP("GPIOL", 12, "clk-sel0", GPIO_ACTIVE_HIGH),
- GPIO_LOOKUP("GPIOL", 13, "clk-sel1", GPIO_ACTIVE_HIGH),
- GPIO_LOOKUP("GPIOL", 14, "osr", GPIO_ACTIVE_HIGH),
- GPIO_LOOKUP("GPIOL", 8, "master", GPIO_ACTIVE_HIGH),
- { },
- },
+static const struct property_entry crag_wm1250_ev1_properties[] = {
+ PROPERTY_ENTRY_GPIO("clk-ena-gpios",
+ SAMSUNG_GPIO_NODE('N'), 12, GPIO_ACTIVE_HIGH),
+ PROPERTY_ENTRY_GPIO("clk-sel0-gpios",
+ SAMSUNG_GPIO_NODE('L'), 12, GPIO_ACTIVE_HIGH),
+ PROPERTY_ENTRY_GPIO("clk-sel1-gpios",
+ SAMSUNG_GPIO_NODE('L'), 13, GPIO_ACTIVE_HIGH),
+ PROPERTY_ENTRY_GPIO("osr-gpios",
+ SAMSUNG_GPIO_NODE('L'), 14, GPIO_ACTIVE_HIGH),
+ PROPERTY_ENTRY_GPIO("master-gpios",
+ SAMSUNG_GPIO_NODE('L'), 8, GPIO_ACTIVE_HIGH),
+ { }
+};
+
+static const struct software_node crag_wm1250_ev1_swnode = {
+ .properties = crag_wm1250_ev1_properties,
};
static struct i2c_board_info i2c_devs1[] = {
@@ -794,7 +798,8 @@ static struct i2c_board_info i2c_devs1[] = {
{ I2C_BOARD_INFO("wlf-gf-module", 0x24) },
{ I2C_BOARD_INFO("wlf-gf-module", 0x25) },
{ I2C_BOARD_INFO("wlf-gf-module", 0x26) },
- { I2C_BOARD_INFO("wm1250-ev1", 0x27), },
+ { I2C_BOARD_INFO("wm1250-ev1", 0x27),
+ .swnode = &crag_wm1250_ev1_swnode, },
};
static struct s3c2410_platform_i2c i2c1_pdata = {
@@ -891,15 +896,48 @@ static const struct gpio_led_platform_data gpio_leds_pdata = {
static struct dwc2_hsotg_plat crag6410_hsotg_pdata;
-static struct gpiod_lookup_table crag_spi0_gpiod_table = {
- .dev_id = "s3c6410-spi.0",
- .table = {
- GPIO_LOOKUP_IDX("GPIOC", 3, "cs", 0, GPIO_ACTIVE_LOW),
- GPIO_LOOKUP_IDX("GPION", 5, "cs", 1, GPIO_ACTIVE_LOW),
- { },
- },
+static const struct software_node_ref_args crag6410_spi0_gpio_refs[] = {
+ SOFTWARE_NODE_REFERENCE(SAMSUNG_GPIO_NODE('C'), 3, GPIO_ACTIVE_LOW),
+ SOFTWARE_NODE_REFERENCE(SAMSUNG_GPIO_NODE('N'), 5, GPIO_ACTIVE_LOW),
+};
+
+static const struct property_entry crag6410_spi0_properties[] __initconst = {
+ PROPERTY_ENTRY_REF_ARRAY("cs-gpios", crag6410_spi0_gpio_refs),
+ { }
+};
+
+static const struct resource crag6410_spi0_resource[] __initconst = {
+ [0] = DEFINE_RES_MEM(S3C_PA_SPI0, SZ_256),
+ [1] = DEFINE_RES_IRQ(IRQ_SPI0),
};
+static const struct s3c64xx_spi_info crag6410_spi0_platform_data __initconst = {
+ .num_cs = 2,
+ .cfg_gpio = s3c64xx_spi0_cfg_gpio,
+};
+
+static const struct platform_device_info crag6410_spi0_info __initconst = {
+ .name = "s3c6410-spi",
+ .id = 0,
+ .res = crag6410_spi0_resource,
+ .num_res = ARRAY_SIZE(crag6410_spi0_resource),
+ .data = &crag6410_spi0_platform_data,
+ .size_data = sizeof(crag6410_spi0_platform_data),
+ .dma_mask = DMA_BIT_MASK(32),
+ .properties = crag6410_spi0_properties,
+};
+
+static void __init crag6410_setup_spi0(void)
+{
+ struct platform_device *pd;
+ int err;
+
+ pd = platform_device_register_full(&crag6410_spi0_info);
+ err = PTR_ERR_OR_ZERO(pd);
+ if (err)
+ pr_err("failed to create spi0 device: %d\n", err);
+}
+
static void __init crag6410_machine_init(void)
{
/* Open drain IRQs need pullups */
@@ -928,12 +966,8 @@ static void __init crag6410_machine_init(void)
software_node_register(&crag_dcdc1_swnode);
i2c_register_board_info(0, i2c_devs0, ARRAY_SIZE(i2c_devs0));
- gpiod_add_lookup_table(&crag_wm1250_ev1_gpiod_table);
i2c_register_board_info(1, i2c_devs1, ARRAY_SIZE(i2c_devs1));
- gpiod_add_lookup_table(&crag_spi0_gpiod_table);
- s3c64xx_spi0_set_platdata(0, 2);
-
pwm_add_table(crag6410_pwm_lookup, ARRAY_SIZE(crag6410_pwm_lookup));
platform_add_devices(crag6410_devs0, ARRAY_SIZE(crag6410_devs0));
platform_device_register_full(&crag6410_mmgpio_devinfo);
@@ -941,6 +975,7 @@ static void __init crag6410_machine_init(void)
gpiod_add_lookup_table(&crag_leds_table);
crag6410_setup_keypad();
crag6410_setup_gpio_keys();
+ crag6410_setup_spi0();
platform_add_devices(crag6410_devs1, ARRAY_SIZE(crag6410_devs1));
gpio_led_register_device(-1, &gpio_leds_pdata);
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related
* [PATCH v2 11/11] ARM: s3c: crag6410: convert basic-mmio-gpio and LEDs to software properties
From: Dmitry Torokhov @ 2026-07-09 4:53 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Peter Griffin,
Alim Akhtar, Russell King, Mark Brown, Linus Walleij,
Charles Keepax, Sam Protsenko, Arnd Bergmann, Bartosz Golaszewski
Cc: Krzysztof Kozlowski, linux-input, devicetree, linux-kernel,
linux-arm-kernel, linux-samsung-soc, patches
In-Reply-To: <20260708-samsung-kp-v2-0-3c6ed4c9b3b6@gmail.com>
Convert the basic-mmio-gpio device and the GPIO-controlled LEDs on the
Cragganmore 6410 board to use software nodes/properties. This allows
removing the crag_leds_table GPIO lookup table.
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
arch/arm/mach-s3c/mach-crag6410.c | 117 +++++++++++++++++++++-----------------
1 file changed, 64 insertions(+), 53 deletions(-)
diff --git a/arch/arm/mach-s3c/mach-crag6410.c b/arch/arm/mach-s3c/mach-crag6410.c
index 03a97a20e38c..b48980920aee 100644
--- a/arch/arm/mach-s3c/mach-crag6410.c
+++ b/arch/arm/mach-s3c/mach-crag6410.c
@@ -319,12 +319,17 @@ static const struct property_entry crag6410_mmgpio_props[] = {
{ }
};
+static const struct software_node crag6410_mmgpio_node = {
+ .name = "basic-mmio-gpio",
+ .properties = crag6410_mmgpio_props,
+};
+
static struct platform_device_info crag6410_mmgpio_devinfo = {
.name = "basic-mmio-gpio",
.id = -1,
.res = crag6410_mmgpio_resource,
.num_res = ARRAY_SIZE(crag6410_mmgpio_resource),
- .properties = crag6410_mmgpio_props,
+ .swnode = &crag6410_mmgpio_node,
};
static struct platform_device speyside_device = {
@@ -839,61 +844,68 @@ static struct s3c_sdhci_platdata crag6410_hsmmc0_pdata = {
.host_caps = MMC_CAP_POWER_OFF_CARD,
};
-static const struct gpio_led gpio_leds[] = {
- {
- .name = "d13:green:",
- .default_state = LEDS_GPIO_DEFSTATE_ON,
- },
- {
- .name = "d14:green:",
- .default_state = LEDS_GPIO_DEFSTATE_ON,
- },
- {
- .name = "d15:green:",
- .default_state = LEDS_GPIO_DEFSTATE_ON,
- },
- {
- .name = "d16:green:",
- .default_state = LEDS_GPIO_DEFSTATE_ON,
- },
- {
- .name = "d17:green:",
- .default_state = LEDS_GPIO_DEFSTATE_ON,
- },
- {
- .name = "d18:green:",
- .default_state = LEDS_GPIO_DEFSTATE_ON,
- },
- {
- .name = "d19:green:",
- .default_state = LEDS_GPIO_DEFSTATE_ON,
- },
- {
- .name = "d20:green:",
- .default_state = LEDS_GPIO_DEFSTATE_ON,
- },
+static const struct software_node crag6410_leds_node = {
+ .name = "leds-gpio",
};
-static struct gpiod_lookup_table crag_leds_table = {
- .dev_id = "leds-gpio",
- .table = {
- GPIO_LOOKUP_IDX("basic-mmio-gpio", 0, "cs", 0, GPIO_ACTIVE_LOW),
- GPIO_LOOKUP_IDX("basic-mmio-gpio", 1, "cs", 1, GPIO_ACTIVE_LOW),
- GPIO_LOOKUP_IDX("basic-mmio-gpio", 2, "cs", 2, GPIO_ACTIVE_LOW),
- GPIO_LOOKUP_IDX("basic-mmio-gpio", 3, "cs", 3, GPIO_ACTIVE_LOW),
- GPIO_LOOKUP_IDX("basic-mmio-gpio", 4, "cs", 4, GPIO_ACTIVE_LOW),
- GPIO_LOOKUP_IDX("basic-mmio-gpio", 5, "cs", 5, GPIO_ACTIVE_LOW),
- GPIO_LOOKUP_IDX("basic-mmio-gpio", 6, "cs", 6, GPIO_ACTIVE_LOW),
- GPIO_LOOKUP_IDX("basic-mmio-gpio", 7, "cs", 7, GPIO_ACTIVE_LOW),
- { },
- },
-};
+#define CRAG6410_LED_NODE(_idx, _name) \
+static const struct property_entry crag6410_led##_idx##_props[] = { \
+ PROPERTY_ENTRY_STRING("label", _name), \
+ PROPERTY_ENTRY_GPIO("gpios", &crag6410_mmgpio_node, \
+ _idx, GPIO_ACTIVE_LOW), \
+ PROPERTY_ENTRY_STRING("default-state", "on"), \
+ { } \
+}; \
+static const struct software_node crag6410_led##_idx##_node = { \
+ .parent = &crag6410_leds_node, \
+ .properties = crag6410_led##_idx##_props, \
+}
-static const struct gpio_led_platform_data gpio_leds_pdata = {
- .leds = gpio_leds,
- .num_leds = ARRAY_SIZE(gpio_leds),
+CRAG6410_LED_NODE(0, "d13:green:");
+CRAG6410_LED_NODE(1, "d14:green:");
+CRAG6410_LED_NODE(2, "d15:green:");
+CRAG6410_LED_NODE(3, "d16:green:");
+CRAG6410_LED_NODE(4, "d17:green:");
+CRAG6410_LED_NODE(5, "d18:green:");
+CRAG6410_LED_NODE(6, "d19:green:");
+CRAG6410_LED_NODE(7, "d20:green:");
+
+static const struct software_node *crag6410_leds_swnodes[] = {
+ &crag6410_leds_node,
+ &crag6410_led0_node,
+ &crag6410_led1_node,
+ &crag6410_led2_node,
+ &crag6410_led3_node,
+ &crag6410_led4_node,
+ &crag6410_led5_node,
+ &crag6410_led6_node,
+ &crag6410_led7_node,
+ NULL
};
+static void __init crag6410_setup_leds(void)
+{
+ struct platform_device_info leds_info = {
+ .name = "leds-gpio",
+ .id = PLATFORM_DEVID_NONE,
+ };
+ struct platform_device *pd;
+ int err;
+
+ err = software_node_register_node_group(crag6410_leds_swnodes);
+ if (err) {
+ pr_err("failed to register leds software nodes: %d\n", err);
+ return;
+ }
+
+ leds_info.fwnode = software_node_fwnode(&crag6410_leds_node);
+
+ pd = platform_device_register_full(&leds_info);
+ err = PTR_ERR_OR_ZERO(pd);
+ if (err)
+ pr_err("failed to create leds-gpio device: %d\n", err);
+}
+
static struct dwc2_hsotg_plat crag6410_hsotg_pdata;
static const struct software_node_ref_args crag6410_spi0_gpio_refs[] = {
@@ -972,13 +984,12 @@ static void __init crag6410_machine_init(void)
platform_add_devices(crag6410_devs0, ARRAY_SIZE(crag6410_devs0));
platform_device_register_full(&crag6410_mmgpio_devinfo);
- gpiod_add_lookup_table(&crag_leds_table);
crag6410_setup_keypad();
crag6410_setup_gpio_keys();
crag6410_setup_spi0();
platform_add_devices(crag6410_devs1, ARRAY_SIZE(crag6410_devs1));
- gpio_led_register_device(-1, &gpio_leds_pdata);
+ crag6410_setup_leds();
regulator_has_full_constraints();
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related
* Re: [PATCH v2 07/11] regulator: wm831x: support software node in platform data
From: sashiko-bot @ 2026-07-09 5:03 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: dmitry.torokhov, devicetree, conor+dt, robh, linux-input
In-Reply-To: <20260708-samsung-kp-v2-7-3c6ed4c9b3b6@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Using `device_set_node()` on an already-registered platform device destructively overwrites its primary firmware node and introduces dangling pointer/use-after-free risks.
--
--- Patch [7]: [PATCH v2 07/11] regulator: wm831x: support software node in platform data ---
commit 41eed35e80fd0a1bdad7b80c52433a74629a0ec0
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
regulator: wm831x: support software node in platform data
This commit allows passing a software node via platform data to the
wm831x buckv regulators. It attempts to read DVS configuration from the
software node if it is present, falling back to legacy platform data.
> diff --git a/drivers/regulator/wm831x-dcdc.c b/drivers/regulator/wm831x-dcdc.c
> --- a/drivers/regulator/wm831x-dcdc.c
> +++ b/drivers/regulator/wm831x-dcdc.c
[ ... ]
> @@ -332,14 +333,26 @@ static void wm831x_buckv_dvs_init(struct platform_device *pdev,
> struct wm831x *wm831x = dcdc->wm831x;
> int ret;
> u16 ctrl;
> + int dvs_control_src;
> + u32 val;
>
> if (!pdata)
> return;
>
> + if (pdata->swnode) {
> + struct fwnode_handle *fwnode = software_node_fwnode(pdata->swnode);
> +
> + if (fwnode)
> + device_set_node(&pdev->dev, fwnode);
[Severity: High]
Does overwriting the primary firmware node here with device_set_node()
safely manage the device lifecycle?
Calling device_set_node() on an already-registered platform device destructively
overwrites its primary fwnode, clearing any Device Tree or ACPI bindings
established by the MFD core. It also bypasses the correct API for secondary
nodes (device_add_software_node()).
If a later step in wm831x_buckv_probe() fails and the driver unwinds, the
modified pdev->dev.fwnode is never restored. This leaves a dangling pointer
to the unrefcounted software node.
Should this code use device_add_software_node() instead?
> + }
> +
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-samsung-kp-v2-0-3c6ed4c9b3b6@gmail.com?part=7
^ permalink raw reply
* Re: [PATCH v2 05/11] Input: samsung-keypad - remove support for platform data
From: sashiko-bot @ 2026-07-09 5:04 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, conor+dt, dmitry.torokhov, devicetree, robh
In-Reply-To: <20260708-samsung-kp-v2-5-3c6ed4c9b3b6@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Legacy device tree properties for row and column counts are immediately overwritten and discarded by matrix_keypad_parse_properties(), breaking backward compatibility.
Pre-existing issues:
- [Critical] Stack variables `row`, `col`, and `key_code` are used uninitialized if property reads fail, potentially leading to out-of-bounds keymap generation and memory corruption.
- [High] The IRQ handler is requested before hardware clocks are enabled, leading to an external abort (system crash) if an interrupt fires early.
--
--- Patch [5]: [PATCH v2 05/11] Input: samsung-keypad - remove support for platform data ---
commit 6b84c0425d2f995a2e30b98f55d0febfc3a74cb7
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Input: samsung-keypad - remove support for platform data
Because there are no more users of samsung_keypad_platdata left in
the kernel remove support for it from the driver. The driver supports
generic device properties so all configuration should be done using
them instead of a custom platform data.
> diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
> index b9d4ea5f202ae..6f1d766a4134d 100644
> --- a/drivers/input/keyboard/samsung-keypad.c
> +++ b/drivers/input/keyboard/samsung-keypad.c
[ ... ]
> @@ -231,144 +234,77 @@ static void samsung_keypad_close(struct input_dev *input_dev)
> samsung_keypad_stop(keypad);
> }
>
> -static const struct matrix_keymap_data *
> -samsung_parse_verbose_keymap(struct device *dev)
> +static int samsung_keypad_parse_keymap(struct samsung_keypad *keypad)
> {
[ ... ]
> + device_for_each_child_node(dev, child) {
> + u32 row, col, key_code;
>
> - fwnode_property_read_u32(child, "keypad,row", &row);
> - fwnode_property_read_u32(child, "keypad,column", &col);
> - fwnode_property_read_u32(child, "linux,code", &key_code);
> + fwnode_property_read_u32(child, "keypad,row", &row);
> + fwnode_property_read_u32(child, "keypad,column", &col);
> + fwnode_property_read_u32(child, "linux,code", &key_code);
>
> - *keymap++ = KEY(row, col, key_code);
> + *keymap++ = KEY(row, col, key_code);
[Severity: Critical]
This is a pre-existing issue, but what happens if the property reads fail?
If the device tree child node is missing properties (like linux,code),
fwnode_property_read_u32() will return an error and leave row, col, and
key_code uninitialized.
Could this stack garbage be packed into the keymap array and subsequently
cause an out-of-bounds bitwise write in matrix_keypad_build_keymap() when it
calls __set_bit(code, input_dev->keybit)?
> + }
> }
>
[ ... ]
> static int samsung_keypad_probe(struct platform_device *pdev)
> {
[ ... ]
> + 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);
> -
> - pdata->rows = num_rows;
> - pdata->cols = num_cols;
> -
> - if (!device_property_present(dev, "linux,keymap")) {
> - keymap_data = samsung_parse_verbose_keymap(dev);
> - if (IS_ERR(keymap_data))
> - return ERR_CAST(keymap_data);
> -
> - pdata->keymap_data = keymap_data;
> - }
> -
> -
> - pdata->no_autorepeat =
> - device_property_read_bool(dev, "linux,input-no-autorepeat");
> -
> - pdata->wakeup = device_property_read_bool(dev, "wakeup-source") ||
> - /* legacy name */
> - device_property_read_bool(dev, "linux,input-wakeup");
> -
> - return pdata;
> + error = matrix_keypad_parse_properties(dev, &num_rows, &num_cols);
> + if (error)
> + return error;
[Severity: High]
Does matrix_keypad_parse_properties() overwrite the legacy properties we just
read?
Looking at matrix_keypad_parse_properties(), it unconditionally resets the
pointers passed to it:
matrix_keypad_parse_properties() {
*rows = *cols = 0;
device_property_read_u32(dev, "keypad,num-rows", rows);
device_property_read_u32(dev, "keypad,num-columns", cols);
}
If a device tree uses the legacy bindings (samsung,keypad-num-rows and
samsung,keypad-num-columns) without the modern prefixes, it seems the values
read into num_rows and num_cols will be discarded here.
Since matrix_keypad_parse_properties() will return -EINVAL if the generic
properties are missing, will this cause probe to fail on existing platforms
using the older bindings?
[ ... ]
> @@ -418,7 +373,11 @@ static int samsung_keypad_probe(struct platform_device *pdev)
> return error;
> }
[Severity: High]
This is a pre-existing issue, but is the interrupt handler requested too early?
In samsung_keypad_probe(), devm_request_threaded_irq() is called just above
this block before the hardware clocks are fully enabled. During probe,
keypad->stopped is true.
If an interrupt fires immediately (for instance, if the bootloader left the
IP block active), samsung_keypad_irq() will run. While the handler calls
pm_runtime_get_sync(), the runtime resume callback skips enabling the clock
because keypad->stopped is true.
Could this result in the handler attempting an unclocked read:
samsung_keypad_irq() {
...
readl(keypad->base + SAMSUNG_KEYIFSTSCLR);
...
}
and causing a synchronous external abort?
> - device_init_wakeup(&pdev->dev, pdata->wakeup);
> + wakeup = device_property_read_bool(dev, "wakeup-source") ||
> + /* legacy name */
> + device_property_read_bool(dev, "linux,input-wakeup");
> + device_init_wakeup(&pdev->dev, wakeup);
> +
> platform_set_drvdata(pdev, keypad);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-samsung-kp-v2-0-3c6ed4c9b3b6@gmail.com?part=5
^ permalink raw reply
* Re: [PATCH v2 01/11] dt-bindings: input: samsung,s3c6410-keypad: introduce compact binding
From: sashiko-bot @ 2026-07-09 5:05 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, robh, conor+dt, devicetree, dmitry.torokhov
In-Reply-To: <20260708-samsung-kp-v2-1-3c6ed4c9b3b6@gmail.com>
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 <dmitry.torokhov@gmail.com>
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.
> 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
> @@ -37,6 +37,10 @@ properties:
>
> wakeup-source: true
>
> + keypad,num-columns: true
> + keypad,num-rows: true
> + linux,keymap: true
> +
[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:
[ ... ]
> @@ -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: 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?
> + patternProperties:
> + '^key-[0-9a-z]+$': false
> + else:
> + properties:
> + keypad,num-columns: false
> + keypad,num-rows: false
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-samsung-kp-v2-0-3c6ed4c9b3b6@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH v2 02/11] Input: samsung-keypad - handle compact binding
From: sashiko-bot @ 2026-07-09 5:06 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, conor+dt, robh, devicetree, dmitry.torokhov
In-Reply-To: <20260708-samsung-kp-v2-2-3c6ed4c9b3b6@gmail.com>
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
^ permalink raw reply
* Re: [PATCH v2 04/11] ARM: s3c: crag6410: switch keypad device to software properties
From: sashiko-bot @ 2026-07-09 5:09 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: dmitry.torokhov, linux-input, robh, conor+dt, devicetree
In-Reply-To: <20260708-samsung-kp-v2-4-3c6ed4c9b3b6@gmail.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [High] Legacy device tree parsing fails because matrix_keypad_parse_properties() overwrites legacy values.
- [Low] pr_err() message lacks trailing newline.
- [Low] S3C64XX_SETUP_KEYPAD Kconfig symbol is dead code but remains selected.
Pre-existing issues:
- [High] Uninitialized variables in samsung_parse_verbose_keymap() generate random keycodes and leak kernel memory.
- [High] Asynchronous data abort occurs due to IRQ registration before PM runtime enablement.
--
--- Patch [4]: [PATCH v2 04/11] ARM: s3c: crag6410: switch keypad device to software properties ---
commit acad872c671c37651ef2ad547b4a44b17438b49e
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Subject: ARM: s3c: crag6410: switch keypad device to software properties
This commit switches the keypad device to use software properties. This change
allows dropping support for platform data from the samsung-keypad driver. The
platform data and related setup code are removed from the machine
initialization.
> diff --git a/arch/arm/mach-s3c/Makefile.s3c64xx b/arch/arm/mach-s3c/Makefile.s3c64xx
> --- a/arch/arm/mach-s3c/Makefile.s3c64xx
> +++ b/arch/arm/mach-s3c/Makefile.s3c64xx
> @@ -32,7 +32,6 @@ obj-y += dev-audio-s3c64xx.o
> obj-$(CONFIG_S3C64XX_SETUP_FB_24BPP) += setup-fb-24bpp-s3c64xx.o
> obj-$(CONFIG_S3C64XX_SETUP_I2C0) += setup-i2c0-s3c64xx.o
> obj-$(CONFIG_S3C64XX_SETUP_I2C1) += setup-i2c1-s3c64xx.o
> -obj-$(CONFIG_S3C64XX_SETUP_KEYPAD) += setup-keypad-s3c64xx.o
[Severity: Low]
Is the S3C64XX_SETUP_KEYPAD Kconfig symbol dead code now?
Since the file is removed from the Makefile, the config symbol doesn't seem
to compile any code anymore, but it is still selected by MACH_WLF_CRAGG_6410
in Kconfig.s3c64xx.
[ ... ]
> diff --git a/arch/arm/mach-s3c/mach-crag6410.c b/arch/arm/mach-s3c/mach-crag6410.c
> --- a/arch/arm/mach-s3c/mach-crag6410.c
> +++ b/arch/arm/mach-s3c/mach-crag6410.c
[ ... ]
> @@ -192,17 +193,41 @@ static uint32_t crag6410_keymap[] = {
[ ... ]
> +static void __init crag6410_setup_keypad(void)
> +{
> + struct platform_device *pd;
> +
> + /* Set all the necessary GPK pins to special-function 3: KP_ROW[x] */
> + s3c_gpio_cfgrange_nopull(S3C64XX_GPK(8), 2, S3C_GPIO_SFN(3));
> +
> + /* Set all the necessary GPL pins to special-function 3: KP_COL[x] */
> + s3c_gpio_cfgrange_nopull(S3C64XX_GPL(0), 6, S3C_GPIO_SFN(3));
> +
> + pd = platform_device_register_full(&crag6410_keypad_info);
> + if (IS_ERR(pd))
> + pr_err("failed to instantiate keypad device");
[Severity: Low]
Would it be better to add a trailing newline to this pr_err() message?
It looks like the kernel log output might become garbled if the registration
fails.
> +}
[Severity: High]
Could this sequence cause a regression where legacy device trees fail to probe?
When reading properties:
drivers/input/keyboard/samsung-keypad.c:samsung_keypad_parse_properties() {
...
error = matrix_keypad_parse_properties(dev, &num_rows, &num_cols);
...
}
The matrix_keypad_parse_properties() function unconditionally zeroes the
variables at entry, so if modern properties are absent, the legacy
samsung,keypad-num-rows properties are overwritten and the function returns
-EINVAL.
[Severity: High]
This is a pre-existing issue, but might samsung_parse_verbose_keymap() leak
uninitialized stack memory to userspace?
drivers/input/keyboard/samsung-keypad.c:samsung_parse_verbose_keymap() {
...
u32 row, col, key_code;
fwnode_property_read_u32(child, "keypad,row", &row);
...
*keymap++ = KEY(row, col, key_code);
...
}
If a child node is missing the keypad,row, keypad,column, or linux,code
properties, fwnode_property_read_u32() leaves row, col, and key_code
uninitialized. These stack values are used in the KEY() macro and stored in
the keymap, where userspace can read them via the EVIOCGKEYCODE ioctl.
[Severity: High]
This is a pre-existing issue, but is it possible for an asynchronous data abort
to occur if an IRQ is pending before PM runtime is enabled?
In probe, the IRQ is requested before the device is powered on:
drivers/input/keyboard/samsung-keypad.c:samsung_keypad_probe() {
...
error = devm_request_threaded_irq(&pdev->dev, keypad->irq, NULL,
samsung_keypad_irq, IRQF_ONESHOT,
dev_name(&pdev->dev), keypad);
...
}
If an IRQ is pending from the bootloader, the handler runs immediately:
drivers/input/keyboard/samsung-keypad.c:samsung_keypad_irq() {
...
pm_runtime_get_sync(&keypad->pdev->dev);
do {
readl(keypad->base + SAMSUNG_KEYIFSTSCLR);
...
}
Since the device is unclocked and pm_runtime_get_sync() fails, readl()
accesses unclocked registers and triggers a data abort.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-samsung-kp-v2-0-3c6ed4c9b3b6@gmail.com?part=4
^ permalink raw reply
* Re: [PATCH v2 09/11] regulator: wm831x: remove legacy DVS platform data
From: sashiko-bot @ 2026-07-09 5:10 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: conor+dt, dmitry.torokhov, linux-input, devicetree, robh
In-Reply-To: <20260708-samsung-kp-v2-9-3c6ed4c9b3b6@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing error cleanup on DVS initialization failure paths leaves dangling GPIO pointers or unconfigured hardware state, causing silent voltage scaling failures.
--
--- Patch [9]: [PATCH v2 09/11] regulator: wm831x: remove legacy DVS platform data ---
commit 314c31c6b7ec71e1f10af8307e95e6718e0fea28
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
regulator: wm831x: remove legacy DVS platform data
Now that the only board file in mainline using wm831x buckv DVS has been
converted to use software properties, we can remove the unused legacy
DVS fields from 'struct wm831x_buckv_pdata'. Also remove the fallback
logic from the regulator driver, making DVS support purely
property-based when software nodes are used.
> diff --git a/drivers/regulator/wm831x-dcdc.c b/drivers/regulator/wm831x-dcdc.c
> index ce43c51c01704..05f49b2f5bade 100644
> --- a/drivers/regulator/wm831x-dcdc.c
> +++ b/drivers/regulator/wm831x-dcdc.c
[ ... ]
> @@ -352,7 +347,7 @@ static void wm831x_buckv_dvs_init(struct platform_device *pdev,
> if (device_property_read_u32(&pdev->dev, "wlf,dvs-init-state", &val) == 0)
> dcdc->dvs_gpio_state = val;
> else
> - dcdc->dvs_gpio_state = pdata->dvs_init_state;
> + dcdc->dvs_gpio_state = 0;
>
> dcdc->dvs_gpiod = devm_gpiod_get(&pdev->dev, "dvs",
> dcdc->dvs_gpio_state ? GPIOD_OUT_HIGH : GPIOD_OUT_LOW);
> @@ -362,10 +357,13 @@ static void wm831x_buckv_dvs_init(struct platform_device *pdev,
> return;
> }
>
> - if (device_property_read_u32(&pdev->dev, "wlf,dvs-control-src", &val) == 0)
> - dvs_control_src = val;
> - else
> - dvs_control_src = pdata->dvs_control_src;
> + ret = device_property_read_u32(&pdev->dev, "wlf,dvs-control-src", &val);
> + if (ret) {
> + dev_err(wm831x->dev, "Failed to read DVS control source for %s: %d\n",
> + dcdc->name, ret);
> + return;
> + }
[Severity: High]
This is a pre-existing issue, but the new error return path here caught my
eye while reviewing.
If initialization fails here (or slightly above when devm_gpiod_get()
returns an error pointer), we return early but don't clear dcdc->dvs_gpiod.
If we fail to read "wlf,dvs-control-src", dcdc->dvs_gpiod is already
successfully populated, but the hardware DVS source is left unconfigured.
If devm_gpiod_get() failed earlier, dcdc->dvs_gpiod holds an error pointer.
In both scenarios, could this cause silent voltage scaling failures later on?
Looking at wm831x_buckv_set_voltage_sel(), there is this check:
wm831x_buckv_set_voltage_sel() {
...
if (dcdc->dvs_gpiod && dcdc->dvs_vsel == vsel)
return wm831x_buckv_set_dvs(rdev, 1);
...
}
Since dcdc->dvs_gpiod is either an error pointer or a valid GPIO (but without
configured hardware), this condition still evaluates to true. Does this skip
updating the hardware on_reg, causing the voltage change to silently fail
and leaving the system at a lower voltage?
Should dcdc->dvs_gpiod be explicitly set to NULL on these error paths so the
driver correctly falls back to standard voltage scaling?
> + dvs_control_src = val;
>
> switch (dvs_control_src) {
> case 1:
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-samsung-kp-v2-0-3c6ed4c9b3b6@gmail.com?part=9
^ permalink raw reply
* Re: (subset) [PATCH 0/37] PCI/MSI: Enforce explicit IRQ vector management by removing devres auto-free
From: Manivannan Sadhasivam @ 2026-07-09 5:40 UTC (permalink / raw)
To: Bjorn Helgaas, Vaibhaav Ram T . L, Kumaravel Thiagarajan, Even Xu,
Xinpeng Sun, Srinivas Pandruvada, Jiri Kosina, Alexandre Belloni,
Zhou Wang, Longfang Liu, Vinod Koul, Lee Jones, Jijie Shao,
Jian Shen, Sunil Goutham, Andrew Lunn, Heiner Kallweit,
David S . Miller, Jeff Hugo, Oded Gabbay, Maciej Falkowski,
Karol Wachowski, Min Ma, Lizhi Hou, Andreas Noever,
Mika Westerberg, Will Deacon, Xinliang Liu, Tian Tao,
Davidlohr Bueso, Srujana Challa, Bharat Bhushan, Antoine Tenart,
Herbert Xu, Raag Jadav, Hans de Goede, Greg Kroah-Hartman,
Jiri Slaby, Andy Shevchenko, Mika Westerberg, Andi Shyti,
Robert Richter, Mark Brown, Nirmal Patel, Kurt Schwemmer,
Logan Gunthorpe, Linus Walleij, Bartosz Golaszewski, Sakari Ailus,
Bingbu Cao, Tomasz Jeznach, Jonathan Cameron, Ulf Hansson,
Shawn Lin
Cc: Arnd Bergmann, Benjamin Tissoires, linux-input, linux-i3c,
dmaengine, Philipp Stanner, netdev, nic_swsd, linux-arm-msm,
dri-devel, linux-usb, iommu, linux-riscv, David Airlie,
Simona Vetter, linux-cxl, linux-crypto, platform-driver-x86,
linux-serial, mhi, Andy Shevchenko, Jan Dabros, linux-i2c,
Daniel Mack, Haojian Zhuang, linux-spi, Jonathan Derrick,
linux-pci, linux-gpio, Mauro Carvalho Chehab, linux-media,
linux-mmc
In-Reply-To: <1771860581-82092-1-git-send-email-shawn.lin@rock-chips.com>
On Mon, 23 Feb 2026 23:29:39 +0800, Shawn Lin wrote:
> This patch series addresses a long-standing design issue in the PCI/MSI
> subsystem where the implicit, automatic management of IRQ vectors by
> the devres framework conflicts with explicit driver cleanup, creating
> ambiguity and potential resource management bugs.
>
> ==== The Problem: Implicit vs. Explicit Management ====
> Historically, `pcim_enable_device()` not only manages standard PCI resources
> (BARs) via devres but also implicitly triggers automatic IRQ vector management
> by setting a flag that registers `pcim_msi_release()` as a cleanup action.
>
> [...]
Applied, thanks!
[13/37] bus: mhi: host: Replace pci_alloc_irq_vectors() with pcim_alloc_irq_vectors()
commit: 256995e80fcd39cab94eee8135dd90f6da6ac744
Best regards,
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: (subset) [PATCH 0/37] PCI/MSI: Enforce explicit IRQ vector management by removing devres auto-free
From: Manivannan Sadhasivam @ 2026-07-09 5:43 UTC (permalink / raw)
To: Bjorn Helgaas, Vaibhaav Ram T . L, Kumaravel Thiagarajan, Even Xu,
Xinpeng Sun, Srinivas Pandruvada, Jiri Kosina, Alexandre Belloni,
Zhou Wang, Longfang Liu, Vinod Koul, Lee Jones, Jijie Shao,
Jian Shen, Sunil Goutham, Andrew Lunn, Heiner Kallweit,
David S . Miller, Jeff Hugo, Oded Gabbay, Maciej Falkowski,
Karol Wachowski, Min Ma, Lizhi Hou, Andreas Noever,
Mika Westerberg, Will Deacon, Xinliang Liu, Tian Tao,
Davidlohr Bueso, Srujana Challa, Bharat Bhushan, Antoine Tenart,
Herbert Xu, Raag Jadav, Hans de Goede, Greg Kroah-Hartman,
Jiri Slaby, Andy Shevchenko, Mika Westerberg, Andi Shyti,
Robert Richter, Mark Brown, Nirmal Patel, Kurt Schwemmer,
Logan Gunthorpe, Linus Walleij, Bartosz Golaszewski, Sakari Ailus,
Bingbu Cao, Tomasz Jeznach, Jonathan Cameron, Ulf Hansson,
Shawn Lin
Cc: Arnd Bergmann, Benjamin Tissoires, linux-input, linux-i3c,
dmaengine, Philipp Stanner, netdev, nic_swsd, linux-arm-msm,
dri-devel, linux-usb, iommu, linux-riscv, David Airlie,
Simona Vetter, linux-cxl, linux-crypto, platform-driver-x86,
linux-serial, mhi, Andy Shevchenko, Jan Dabros, linux-i2c,
Daniel Mack, Haojian Zhuang, linux-spi, Jonathan Derrick,
linux-pci, linux-gpio, Mauro Carvalho Chehab, linux-media,
linux-mmc
In-Reply-To: <178357565325.731734.7291964273199898845.b4-ty@b4>
On Thu, Jul 09, 2026 at 07:40:53AM +0200, Manivannan Sadhasivam wrote:
>
> On Mon, 23 Feb 2026 23:29:39 +0800, Shawn Lin wrote:
> > This patch series addresses a long-standing design issue in the PCI/MSI
> > subsystem where the implicit, automatic management of IRQ vectors by
> > the devres framework conflicts with explicit driver cleanup, creating
> > ambiguity and potential resource management bugs.
> >
> > ==== The Problem: Implicit vs. Explicit Management ====
> > Historically, `pcim_enable_device()` not only manages standard PCI resources
> > (BARs) via devres but also implicitly triggers automatic IRQ vector management
> > by setting a flag that registers `pcim_msi_release()` as a cleanup action.
> >
> > [...]
>
> Applied, thanks!
>
> [13/37] bus: mhi: host: Replace pci_alloc_irq_vectors() with pcim_alloc_irq_vectors()
> commit: 256995e80fcd39cab94eee8135dd90f6da6ac744
>
Sorry, this is a mistake. This patch is not applied.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* [PATCH] Input: cm109 - fix URB double-submit due to missing open check
From: Xue Lei @ 2026-07-09 7:31 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: aeh, linux-input, linux-kernel, Xue.Lei
Add a dev->open check at the beginning of cm109_input_ev() to reject
events when the device is not open. There is no point in handling
input events for a device that has not been fully initialized.
The cm109 driver's event callback cm109_input_ev() can be invoked via
kd_mksound() -> input_inject_event() as soon as the input handle is
registered in kbd_connect(), which happens during
input_register_device() before cm109_input_open() has completed.
Without checking dev->open, the callback calls
cm109_toggle_buzzer_async() which submits urb_ctl while
cm109_input_open() is also about to submit the same URB, causing a
'list_add double add' BUG in usb_hcd_link_urb_to_ep().
Two crash scenarios have been observed:
Scenario 1: Same-CPU preemption (CONFIG_PREEMPT=y)
During input_register_device(), kbd_connect() calls
input_open_device() which invokes cm109_input_open(). Between
setting ctl_urb_pending=1 and submitting the URB, the kworker
thread is preempted by a process writing to a tty (triggering
kd_mksound). The preempting process calls input_inject_event()
-> cm109_input_ev() -> cm109_toggle_buzzer_async(), which sees
ctl_urb_pending=0 (not yet set) and submits urb_ctl. When the
kworker resumes, it also submits the same urb_ctl.
CPU 0 (kworker) CPU 0 (preempting)
--------------- ------------------
input_open():
// about to set pending=1
<preempted>
input_ev():
toggle_buzzer_async():
ctl_urb_pending == 0
submit_urb(urb_ctl) [1st]
<resumes>
ctl_urb_pending = 1
submit_urb(urb_ctl) [2nd] --> CRASH
Scenario 2: Cross-CPU concurrency
kbd_connect() has made the device handle visible to kd_mksound
on CPU 1, but cm109_input_open() on CPU 0 has not yet finished.
CPU 1 calls cm109_input_ev() first, sees dev->open=0 and
ctl_urb_pending=0 (device was just kzalloc'd), and submits
urb_ctl. Shortly after, CPU 0's cm109_input_open() also submits
the same urb_ctl that is already on the endpoint list.
CPU 0 (kworker) CPU 1 (repro)
--------------- ---------------
input_register_device()
kbd_connect()
input_register_handle()
kd_mksound() sees handle
input_open_device() input_ev():
cm109_input_open(): toggle_buzzer_async():
// not started yet ctl_urb_pending == 0
open == 0
submit_urb [1st]
ctl_urb_pending = 1
submit_urb [2nd] --> CRASH
In both cases the root cause is that cm109_input_ev() does not
verify the device is open before triggering URB submission. Adding
the open check at function entry prevents the event callback from
racing with cm109_input_open() on URB submission.
Reported-by: syzbot+e69c25cf38a53d0cf64c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e69c25cf38a53d0cf64c
Fixes: c04148f915e5 ("Input: add driver for USB VoIP phones with CM109 chipset")
Signed-off-by: Xue Lei <Xue.Lei@windriver.com>
---
drivers/input/misc/cm109.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/input/misc/cm109.c b/drivers/input/misc/cm109.c
index 353d3c1d347d..558cb73d5c6d 100644
--- a/drivers/input/misc/cm109.c
+++ b/drivers/input/misc/cm109.c
@@ -601,6 +601,9 @@ static int cm109_input_ev(struct input_dev *idev, unsigned int type,
{
struct cm109_dev *dev = input_get_drvdata(idev);
+ if (!dev->open)
+ return -ENODEV;
+
dev_dbg(&dev->intf->dev,
"input_ev: type=%u code=%u value=%d\n", type, code, value);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] Input: cm109 - fix URB double-submit due to missing open check
From: sashiko-bot @ 2026-07-09 7:51 UTC (permalink / raw)
To: Xue Lei; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260709073133.373657-1-Xue.Lei@windriver.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] TOCTOU Race leading to Use-After-Free during device disconnect
- [High] State Machine Stall due to unhandled URB submission failure
- [High] DMA API Violation from passing coherent buffer to streaming DMA mapping
--
--- Patch [1]: [PATCH] Input: cm109 - fix URB double-submit due to missing open check ---
commit 8242b0347566c678c3b0f2e9b93934d136b010bb
Author: Xue Lei <Xue.Lei@windriver.com>
Input: cm109 - fix URB double-submit due to missing open check
This commit adds a dev->open check in cm109_input_ev() to reject input
events when the device is not fully initialized, preventing a double-submit
of urb_ctl.
Closes: https://syzkaller.appspot.com/bug?extid=e69c25cf38a53d0cf64c
> diff --git a/drivers/input/misc/cm109.c b/drivers/input/misc/cm109.c
> index 353d3c1d347d5..558cb73d5c6df 100644
> --- a/drivers/input/misc/cm109.c
> +++ b/drivers/input/misc/cm109.c
> @@ -601,6 +601,9 @@ static int cm109_input_ev(struct input_dev *idev, unsigned int type,
> {
> struct cm109_dev *dev = input_get_drvdata(idev);
>
> + if (!dev->open)
> + return -ENODEV;
> +
[Severity: High]
This is a pre-existing issue, but does this lockless check introduce a TOCTOU
race during device disconnect?
Looking at cm109_input_close(), dev->open is cleared after traffic is
stopped:
cm109_input_close()
cm109_stop_traffic(dev);
dev->open = 0;
If a concurrent event injection happens simultaneously, couldn't
cm109_input_ev() see dev->open == 1 just before cm109_input_close() sets
it to 0?
Since cm109_usb_cleanup() frees the dev structure without explicitly calling
usb_kill_urb(), could this allow an asynchronously submitted URB to complete
and access the freed dev structure?
> dev_dbg(&dev->intf->dev,
> "input_ev: type=%u code=%u value=%d\n", type, code, value);
>
[Severity: High]
This is a pre-existing issue, but can the URB submission loop stall
permanently on failure?
Looking at cm109_submit_buzz_toggle(), if usb_submit_urb() fails due to
memory pressure or bus errors, it logs the error but does not clear
dev->ctl_urb_pending:
cm109_submit_buzz_toggle()
error = usb_submit_urb(dev->urb_ctl, GFP_ATOMIC);
if (error)
dev_err(&dev->intf->dev, ...);
If ctl_urb_pending remains 1, wouldn't subsequent calls to
cm109_toggle_buzzer_async() always take this path:
cm109_toggle_buzzer_async()
if (dev->ctl_urb_pending) {
dev->buzzer_pending = 1;
return;
}
Since the URB was never actually submitted, its completion callback won't
run to submit urb_irq, meaning the device would permanently stop receiving
input events and buzzer updates until re-plugged?
[Severity: High]
This is a pre-existing issue, but does the synchronous buzzer toggle violate
DMA mapping rules?
In cm109_usb_probe(), dev->ctl_data is allocated using usb_alloc_coherent().
Later, cm109_toggle_buzzer_sync() passes this coherent buffer directly to
usb_control_msg():
cm109_toggle_buzzer_sync()
error = usb_control_msg(dev->udev,
usb_sndctrlpipe(dev->udev, 0),
...
dev->ctl_data,
USB_PKT_LEN, USB_CTRL_SET_TIMEOUT);
Since usb_control_msg() uses streaming DMA mapping via dma_map_single()
without the URB_NO_TRANSFER_DMA_MAP flag, could passing a buffer allocated
with a coherent API trigger mapping errors on architectures with
CONFIG_DMA_API_DEBUG enabled?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709073133.373657-1-Xue.Lei@windriver.com?part=1
^ permalink raw reply
* Re: [PATCH v2 00/11] Remove support for platform data from samsung keypad
From: Bartosz Golaszewski @ 2026-07-09 8:10 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Krzysztof Kozlowski, linux-input, devicetree, linux-kernel,
linux-arm-kernel, linux-samsung-soc, patches, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Peter Griffin, Alim Akhtar,
Russell King, Mark Brown, Linus Walleij, Charles Keepax,
Sam Protsenko, Arnd Bergmann, Bartosz Golaszewski
In-Reply-To: <20260708-samsung-kp-v2-0-3c6ed4c9b3b6@gmail.com>
On Thu, 9 Jul 2026 06:52:58 +0200, Dmitry Torokhov
<dmitry.torokhov@gmail.com> said:
> Hi,
>
> This is a reworked and extended version of the series previously posted
> to convert the Samsung keypad driver and Cragganmore 6410 board to
> generic device properties. The first 8 patches of the original series
> (general cleanups) have been merged into mainline, and this version
> focuses on the remaining keypad rework and extends the board conversion.
>
> Specifically, we rework the Samsung keypad driver to stop using platform
> data and instead rely on generic device properties, and convert the
> Cragganmore board to use software nodes for all its peripherals, removing
> legacy GPIO lookup tables.
>
> We start by introducing a compact matrix keypad binding and implementing
> it in the driver. To support referencing Samsung GPIO chips in board
> properties, we add infrastructure to register and attach software nodes
> to Samsung gpio_chips. We then switch the Cragganmore keypad to use
> software properties and drop platform data support from the driver.
>
> To convert the PMIC DVS regulator on Cragganmore, we add software node
> support to the wm831x regulator driver, allowing DVS configuration via
> device properties. Once the board is converted, we clean up the driver
> by removing legacy DVS platform data fields.
>
> Finally, we convert the remaining peripherals on Cragganmore (GPIO keys,
> PMIC, WM1250, SPI0, basic-mmio-gpio, and LEDs) to software properties.
> This allows us to eliminate all legacy GPIO lookup tables, which also
> fixes incorrect GPIO chip names ("GPIO<N>" vs "GP<N>") from previous
> descriptor conversions.
>
> Mark, it would be great if you could give this a spin on Cragganmore.
>
> Thanks!
>
I admit I haven't looked at the series in great detail but it looks sane and
you've been doing these conversions before so I trust your judgment.
Acked-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Thanks,
Bartosz
^ permalink raw reply
* Re: [PATCH v2 0/2] firmware: arm_scmi: Ensure automatic module loading
From: Sudeep Holla @ 2026-07-09 10:10 UTC (permalink / raw)
To: Hans de Goede
Cc: Bjorn Andersson, Sudeep Holla, Cristian Marussi,
Nathan Chancellor, Nicolas Schier, Michael Turquette, arm-scmi,
linux-arm-kernel, linux-kernel, linux-kbuild, Stephen Boyd,
Brian Masney, Rafael J. Wysocki, Viresh Kumar, Frank Li,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Guenter Roeck, Jyoti Bhayana, Jonathan Cameron, David Lechner,
Nuno Sá, Andy Shevchenko, Dmitry Torokhov, Ulf Hansson,
Liam Girdwood, Mark Brown, Philipp Zabel, Alexandre Belloni,
linux-clk, linux-pm, imx, linux-hwmon, linux-iio, linux-input,
linux-rtc
In-Reply-To: <8c2a4ae3-95cc-489a-a7a4-90a3ee2597e9@oss.qualcomm.com>
On Thu, Jun 18, 2026 at 10:31:12PM +0200, Hans de Goede wrote:
> Hi,
>
> On 18-Jun-26 17:56, Bjorn Andersson wrote:
> > SCMI drivers such as the Arm SCMI CPUfreq driver are allowed to built as
> > modules, but they are then not automatically loaded. Rework the SCMI
> > device table alias support to make modpost consume the information from
> > MODULE_DEVICE_TABLE(scmi, ...) and allow drivers to be loaded based on
> > this information, if known. Also add a protocol-based alias to also
> > trigger driver loading when only the SCMI protocol id is known.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>
>
> So I just gave this a test spin and unfortunately it does not work.
>
> The problem with Fedora's kernel-config / setup is that the
> request_module() from patch 2/2 runs from the initramfs, but
> the scmi_cpufreq module is only available in the rootfs.
>
> It does work if I explictly add the scmi_cpufreq module to
> the initramfs, then it does get autoloaded.
>
> We really need some place to put a uevent sysfs attr which then
> gets replayed when udev is restarted from the rootfs and then
> re-reads all the uevent files as part of its coldplug
> enumeration.
>
I don't have much knowledge on uevent to provide any suggestions/help.
But isn't this a generic requirement ? I mean you could have modules
install on the rootfs and not all of them are packed in initramfs ?
Just wondering if that works for other modules, we can examine how
do they work and what are we missing ?
--
Regards,
Sudeep
^ permalink raw reply
* Re: [RFC] Input: matrix_keypad: fix interrupt regression introduced by commit 01c84b03d80a
From: Paul Cercueil @ 2026-07-09 10:25 UTC (permalink / raw)
To: Siarhei Volkau, Alexander Kochetkov
Cc: linux-input, Dmitry Torokhov, linux-kernel
In-Reply-To: <CAKNVLfZSjWgiT7cwy4kf3QLq49yzU6PPZQBiRpmU-fARA3iqFg@mail.gmail.com>
Hi,
Siarhei, do you remember the context of that patch?
The driver will read the pin level to emulate pins configured in "both
edges" IRQ mode, and switch to "rising edge" if low, and "falling edge"
if high. That was definitely working on the SoC I tested it with
(probably JZ4770). I would be surprised if it did not work on the
JZ4755.
You mention in that commit message that you would get the pin level
before the interrupt happened, I wonder if you were just experiencing
bouncing?
Anyway, I agree that this commit can be reverted.
Le mercredi 08 juillet 2026 à 18:59 +0300, Siarhei Volkau a écrit :
> Regarding JZ4755, I agree that the [1] patch can be reverted.
> No device requiring this workaround has landed in the mainline.
> qi,lb60 (Ben Nanonote) seems unaffected as it was there before
> the patch was proposed.
>
> However, if there are genuinely two hardware operating modes:
> - "GPIO input mode" while scanning
> - "GPIO interrupt input mode" while idle
> then the keypad driver should be aware of this distinction.
>
> I'd like to propose pinctrl state transitions as the mechanism for
> switching
> these modes, wherever it makes sense (Allwinner et al).
I agree.
>
> CC Paul as the maintainer of Ingenic pinctrl driver.
>
> BR,
> Siarhei
Cheers,
-Paul
>
>
> ср, 8 июл. 2026 г. в 16:30, Alexander Kochetkov
> <al.kochet@gmail.com>:
> >
> > Hi everyone,
> >
> > I found a commit [1] that breaks the matrix_keypad behavior on the
> > Allwinner A64.
> >
> > I have a PINE A64-LTS board with a connected keypad polled by
> > matrix_keypad.
> > This commit caused interrupts to stop arriving at the matrix_keypad
> > driver entirely.
> >
> > An explicit call to gpiod_direction_input() disables interrupt
> > reception on all chips
> > where the GPIO input mode and GPIO interrupt input mode are
> > configured via
> > the pinmux register. Interrupt reception is guaranteed to break on
> > Allwinner
> > (A64, H3, H6, etc.), Broadcom (BCM2835/2711), and some Rockchip
> > SoCs. It
> > does not break on NXP i.MX (i.MX6, i.MX8), STMicroelectronics
> > (STM32MP1),
> > TI Sitara (AM335x), or Intel/AMD.
> >
> > Furthermore, the assumption that enable_row_irqs() restores the
> > interrupt mode is
> > also specific to the Ingenic pinctrl. In the vast majority of
> > drivers, enable_row_irqs() is
> > supposed to simply set the interrupt enable mask without changing
> > the pinmux.
> >
> > Commit [1] was introduced to work around a hardware limitation in
> > Ingenic's JZ4755.
> > This specific behavior is unique to Ingenic and a small number of
> > specialized chips.
> > The majority of SoCs (around 90%) allow reading a GPIO input that
> > is currently configured
> > as an interrupt source.
> >
> > In my opinion, the correct approach would be to revert this commit
> > and fix the behavior of
> > ingenic_gpio_get_value() inside pinctrl-ingenic.c instead. However,
> > I do not own an Ingenic
> > board, so I won't be able to test such a patch.
> >
> > Alternatively, I could introduce a DTS property like read-gpio-
> > quirk in matrix_keypad to restore
> > the original driver behavior by default, and add this property to
> > qi_lb60.dts so as not to break
> > the Ingenic platform.
> >
> > What do you think? What is the best way to proceed here?
> >
> > Best regards,
> > Alexander Kochetkov
> >
> > [1] commit 01c84b03d80aab9f04c4e3e1f9085f4202ff7c29 ("Input:
> > matrix_keypad - force switch rows to input mode")
^ permalink raw reply
* [RFC PATCH v2] HID: BPF: add keyboard behavioral anomaly detection
From: Krish Gulati @ 2026-07-09 10:49 UTC (permalink / raw)
To: bentiss, jikos; +Cc: linux-input, linux-kernel, Krish Gulati
In-Reply-To: <20260619073607.393248-1-krishgulati7@gmail.com>
This patch implements a HID-BPF struct_ops program that detects automated
HID injection attacks. It does this by measuring post-enumeration delay
and tracking inter-keystroke timing using Welford's online variance algorithm.
State is stored in a hash map keyed by the HID ID.
Detection results are currently surfaced via bpf_printk(). A
BPF_MAP_TYPE_RINGBUF interface with configurable userspace daemon
is planned; deferred pending validation of detection heuristics.
Signal design is grounded in: Neuner et al., "USBlock: Blocking
USB-based Keylogger Attacks", DBSec 2018.
Link: https://lore.kernel.org/linux-input/adSxXidgeWF0-Ewn@beelink/
Changes since v1:
- Added bpf_spin_lock/unlock around the Welford compound update in
kbd_hook() to close a data race on concurrent execution (was
lockless read-modify-write).
- Switched dev_details from LRU_HASH to plain HASH so eviction
under map pressure fails the insert instead of silently
displacing an existing tracked device.
- Store report_id in dev_info and gate kbd_hook() on it, so
composite devices no longer misattribute other report types'
bytes as keystrokes.
- Zero dev_info with __builtin_memset before populating in probe()
to satisfy the verifier's stack-init tracking (was relying on a
partial designated initializer).
- Replaced active-byte-count heuristic with bytewise diff/mask
(standard_boot_keypress/nkro_keypress) so consecutive keystrokes
without an intervening empty report are still detected.
- Verified bucket-size rounding (8/16/32/64) does not drop events
or read out-of-bounds, tested via hid-replay with synthetic
report sizes (9, 14 bytes, etc.).
Known limitations, not addressed in this version:
- No cleanup on device disconnect: hid_bpf_ops has no disconnect
hook, so dev_details entries persist until the map fills.
Deferred pending maintainer guidance on the right approach.
- find_keyboard_field() returns on the first Keyboard-typed
(GenericDesktop/Keyboard) Application Collection found; a device
with multiple such collections within a single report descriptor
(as opposed to multiple HID interfaces, which are already handled
correctly and verified on real composite hardware) will only have
the first one tracked. Untested; the only hardware available for
testing exhibits the multiple-interface pattern rather than the
single-descriptor multi-collection pattern, so this path has not
been exercised.
Signed-off-by: Krish Gulati <krishgulati7@gmail.com>
Suggested-by: Sashiko-bot <sashiko-bot@kernel.org>
---
src/bpf/testing/0010-Generic__keyboard.bpf.c | 311 +++++++++++++------
src/bpf/testing/meson.build | 1 +
2 files changed, 217 insertions(+), 95 deletions(-)
diff --git a/src/bpf/testing/0010-Generic__keyboard.bpf.c b/src/bpf/testing/0010-Generic__keyboard.bpf.c
index 9114587..c3f2364 100644
--- a/src/bpf/testing/0010-Generic__keyboard.bpf.c
+++ b/src/bpf/testing/0010-Generic__keyboard.bpf.c
@@ -24,11 +24,11 @@
#define HID_GUARD_PED_SUSPICIOUS_THRESH (50 * HID_GUARD_NSEC_PER_MSEC)
#define HID_GUARD_PED_WARNING_THRESH (300 * HID_GUARD_NSEC_PER_MSEC)
-/*not derived from real typing data yet*/
+/*not derived from real typing raw_buffer yet*/
#define HID_GUARD_MIN_SAMPLES 5
/*
- * Variance is "too metronomic
+ * Variance is "too metronomic"
* to be a human," expressed in ms^2 so we never need sqrt().
*/
#define HID_GUARD_VARIANCE_THRESH_MS2 (40ULL * 40ULL)
@@ -57,9 +57,17 @@ enum hid_guard_ped_flag {
};
struct dev_info {
- __u64 connection_time;
+ struct bpf_spin_lock lock;
+ __u8 report_id;
+ __u8 field_type;
+ __u8 prev_report[64];
+ __u16 bits_start;
+ __u16 bits_end;
+ __u16 usage_id;
__u32 report_size;
- __u64 prev_report[32];
+ __u32 keyboard_offset;
+ __u64 connection_time;
+ __u64 raw_buffer_length;
/*welford's variables*/
__u64 prev_keydown_ts;
__u64 count;
@@ -71,22 +79,8 @@ struct dev_info {
*/
};
-/*
- * BPF_MAP_TYPE_LRU_HASH:
- * Using an LRU map automatically prevents exhaustion by silently evicting
- * the oldest idle devices. It requires no syntax changes to the rest of the
- * code (lookup/update helpers work identically), but introduces behavioral
- * trade-offs:
- *
- * 1. Eviction wipes Welford variance history. An attacker
- * could theoretically flood the map to flush their device and reset their
- * score.
- * 2. PED Blindspot: Eviction deletes the 'connection_time' set during probe().
- * If an evicted device wakes up, it will bypass post-enumeration delay
- * checks.
- */
struct {
- __uint(type, BPF_MAP_TYPE_LRU_HASH);
+ __uint(type, BPF_MAP_TYPE_HASH);
__type(key, __u32);
__type(value, struct dev_info);
__uint(max_entries, 128);
@@ -138,11 +132,29 @@ static __always_inline void welford(struct dev_info *dev_state,
delta2 = x - dev_state->mean;
dev_state->M2 += (__u64)(delta * delta2);
+}
- bpf_printk("W[Count:%llu] Int:%llu ms, scaled_x:%lld\n",
- dev_state->count, interval_ms, x);
- bpf_printk(" -> delta1:%lld, mean:%lld\n", delta, dev_state->mean);
- bpf_printk(" -> delta2:%lld, M2:%llu\n", delta2, dev_state->M2);
+static __always_inline bool standard_boot_keypress(__u8 *current_report,
+ __u8 *prev_report)
+{
+ for (int i = 2; i < 8; i++) {
+ if (current_report[i] != prev_report[i])
+ return 1;
+ }
+ return 0;
+}
+
+static __always_inline bool
+nkro_keypress(__u8 *current_report, __u8 *prev_report, __u64 buffer_length)
+{
+ for (int i = 0; i < 64; i++) {
+ if (i >= buffer_length)
+ break;
+
+ if (current_report[i] & ~prev_report[i])
+ return 1;
+ }
+ return 0;
}
HID_BPF_CONFIG(HID_DEVICE(BUS_USB, HID_GROUP_ANY, HID_VID_ANY, HID_PID_ANY),
@@ -150,7 +162,7 @@ HID_BPF_CONFIG(HID_DEVICE(BUS_USB, HID_GROUP_ANY, HID_VID_ANY, HID_PID_ANY),
HID_PID_ANY));
SEC(HID_BPF_DEVICE_EVENT)
-int BPF_PROG(kdb_hook, struct hid_bpf_ctx *hctx)
+int BPF_PROG(kbd_hook, struct hid_bpf_ctx *hctx)
{
__u32 hid_id = hctx->hid->id;
@@ -161,124 +173,233 @@ int BPF_PROG(kdb_hook, struct hid_bpf_ctx *hctx)
if (!info)
return 0;
- __u32 size = info->report_size;
- __u32 fetch_size;
- __u8 *data;
+ __u8 *raw_buffer;
- if (size <= 8)
- fetch_size = 8;
- else if (size <= 16)
- fetch_size = 16;
- else
- fetch_size = 32;
+ __u8 *report_id_data = hid_bpf_get_data(hctx, 0, 1);
- data = hid_bpf_get_data(hctx, 0, fetch_size);
+ if (!report_id_data)
+ return 0;
- if (!data)
+ if (info->report_id != 0 && report_id_data[0] != info->report_id)
return 0;
- int now_active_ks = 0, was_active_ks = 0;
-#pragma unroll
- for (int i = 0; i < 32; i++) {
- if (i >= fetch_size)
- break;
- now_active_ks += ((__u8)data[i] + 255) >> 8;
- was_active_ks += ((__u8)info->prev_report[i] + 255) >> 8;
- info->prev_report[i] = data[i];
+ __u32 offset = info->keyboard_offset + (info->report_id != 0 ? 1 : 0);
+
+ __u64 buffer_length = info->raw_buffer_length;
+
+ if (buffer_length <= 8)
+ raw_buffer = hid_bpf_get_data(hctx, offset, 8);
+ else if (buffer_length <= 16)
+ raw_buffer = hid_bpf_get_data(hctx, offset, 16);
+ else if (buffer_length <= 32)
+ raw_buffer = hid_bpf_get_data(hctx, offset, 32);
+ else
+ raw_buffer = hid_bpf_get_data(hctx, offset, 64);
+
+ if (!raw_buffer)
+ return 0;
+
+ bool new_key_pressed = false;
+
+ if (info->report_id == 0) {
+ new_key_pressed =
+ standard_boot_keypress(raw_buffer, info->prev_report);
+ } else {
+ new_key_pressed = nkro_keypress(raw_buffer, info->prev_report,
+ buffer_length);
}
- __u64 current_ms = bpf_ktime_get_ns() / HID_GUARD_NSEC_PER_MSEC;
+ __u64 now = bpf_ktime_get_ns();
- if (now_active_ks > was_active_ks) {
+ __u64 interval_ms = 0;
+ __u64 variance_m2 = 0;
+ bool is_idle_gap = false;
+ bool is_suspicious = false;
+ enum hid_guard_ped_flag ped_flag = HID_GUARD_PED_NO_ENTRY;
+ bool run_ped = false;
+
+ bpf_spin_lock(&info->lock);
+
+ if (new_key_pressed) {
if (info->prev_keydown_ts != 0) {
- __u64 interval_ms = current_ms - info->prev_keydown_ts;
+ interval_ms = (now - info->prev_keydown_ts) /
+ HID_GUARD_NSEC_PER_MSEC;
- if (interval_ms < HID_GUARD_IDLE_GAP_THRESH_MS) {
+ if (interval_ms < HID_GUARD_IDLE_GAP_THRESH_MS)
welford(info, interval_ms);
- } else {
- bpf_printk(
- "hid %d: idle gap %llu ms excluded from sample\n",
- hid_id, interval_ms);
- }
+ else
+ is_idle_gap = true;
}
+ /*
+ * Variance check runs after welford() so the current
+ * sample is already folded in before we decide.
+ * Guard on MIN_SAMPLES: Welford's unbiased estimator
+ * (M2 / (count - 1)) is undefined for count < 2, and
+ * unreliable until a few samples have accumulated.
+ */
if (info->count >= HID_GUARD_MIN_SAMPLES) {
- __u64 variance_m2 =
+ variance_m2 =
info->M2 /
((__u64)HID_GUARD_WELFORD_SCALE *
HID_GUARD_WELFORD_SCALE * (info->count - 1));
- /*
- * the initial interval x was multiplied by HID_GUARD_WELFORD_SCALE, both
- * delta and delta2 are also scaled by that factor,
- * thus scale^2 in the denominator
- */
- if (variance_m2 < HID_GUARD_VARIANCE_THRESH_MS2) {
- bpf_printk(
- "hid %d: Suspeciously regular typing, variance=%llu ms^2\n",
- hid_id, variance_m2);
- }
+ if (variance_m2 < HID_GUARD_VARIANCE_THRESH_MS2)
+ is_suspicious = true;
}
- info->prev_keydown_ts = current_ms;
+ info->prev_keydown_ts = now;
}
- if (info->connection_time != 0) {
- enum hid_guard_ped_flag ped_flag =
- post_enumeration_delay(info, bpf_ktime_get_ns());
-
- bpf_printk("PED flag for hid %d: %d\n", hid_id, ped_flag);
+ for (int i = 0; i < 64; i++) {
+ if (i >= buffer_length)
+ break;
+ info->prev_report[i] = raw_buffer[i];
+ }
- /*
- * Prevent re-evaluation on subsequent packets for this device
- */
+ /*
+ * PED: fires exactly once per device lifetime. connection_time
+ * is set at probe() time; we clear it here so subsequent events
+ * skip this branch entirely.
+ */
+ if (info->connection_time != 0) {
+ ped_flag = post_enumeration_delay(info, now);
info->connection_time = 0;
+ run_ped = true;
}
+
+ bpf_spin_unlock(&info->lock);
+
+ if (new_key_pressed) {
+ if (is_idle_gap)
+ bpf_printk(
+ "hid %d: idle gap %llu ms excluded from sample\n",
+ hid_id, interval_ms);
+ if (is_suspicious)
+ bpf_printk(
+ "hid %d: suspiciously regular typing, variance=%llu ms^2\n",
+ hid_id, variance_m2);
+ }
+
+ if (run_ped)
+ bpf_printk("hid %d: PED flag=%d\n", hid_id, ped_flag);
+
return 0;
}
HID_BPF_OPS(hook_keyboard) = {
- .hid_device_event = (void *)kdb_hook,
+ .hid_device_event = (void *)kbd_hook,
};
struct hid_rdesc_descriptor HID_REPORT_DESCRIPTOR;
-SEC("syscall")
-int probe(struct hid_bpf_probe_args *ctx)
+static __always_inline bool find_keyboard_field(__u16 *bits_start,
+ __u16 *bits_end,
+ __u8 *report_id,
+ __u32 *size_in_bytes)
{
struct hid_rdesc_report *input;
struct hid_rdesc_field *field;
struct hid_rdesc_collection *col;
hid_bpf_for_each_input_report(&HID_REPORT_DESCRIPTOR, input) {
- __u32 size_in_bytes = (input->size_in_bits + 7) / 8;
-
- bpf_printk("Report size: %d\n", size_in_bytes);
- if (input->report_id != 0)
- size_in_bytes += 1;
-
- bpf_printk("Report size after report_id: %d\n", size_in_bytes);
-
hid_bpf_for_each_field(input, field) {
hid_bpf_for_each_collection(field, col) {
if (col->usage_page ==
HidUsagePage_GenericDesktop &&
col->usage_id == HidUsage_GD_Keyboard) {
- __u32 key = ctx->hid;
- struct dev_info info = {
- .connection_time =
- bpf_ktime_get_ns(),
- .count = 0,
- .report_size = size_in_bytes
- };
- bpf_map_update_elem(&dev_details, &key,
- &info, BPF_ANY);
- ctx->retval = 0;
- return 0;
+ *size_in_bytes =
+ input->size_in_bits / 8;
+
+ if (input->report_id != 0)
+ *size_in_bytes += 1;
+
+ *bits_start = field->bits_start;
+ *bits_end = field->bits_end;
+ *report_id = input->report_id;
+ return true;
}
}
}
}
- ctx->retval = -EINVAL;
+ return false;
+}
+
+SEC("syscall")
+int probe(struct hid_bpf_probe_args *ctx)
+{
+ __u8 report_id;
+ __u16 bits_start, bits_end;
+ __u32 size_in_bytes;
+ __u32 hid_id = ctx->hid;
+
+ struct dev_info *existing_info =
+ bpf_map_lookup_elem(&dev_details, &hid_id);
+
+ struct dev_info info;
+
+ __builtin_memset(&info, 0, sizeof(info));
+
+ if (!find_keyboard_field(&bits_start, &bits_end, &report_id,
+ &size_in_bytes)) {
+ ctx->retval = -EINVAL;
+ return 0;
+ }
+
+ if (existing_info != NULL) {
+ __u64 now = bpf_ktime_get_ns();
+
+ bpf_spin_lock(&existing_info->lock);
+
+ existing_info->connection_time = now;
+
+ existing_info->bits_start = bits_start;
+ existing_info->bits_end = bits_end;
+ existing_info->keyboard_offset = bits_start / 8;
+ existing_info->report_size = size_in_bytes;
+
+ if (existing_info->keyboard_offset >
+ existing_info->report_size) {
+ bpf_spin_unlock(&existing_info->lock);
+ ctx->retval = -EINVAL;
+ return 0;
+ }
+
+ existing_info->report_id = report_id;
+ existing_info->raw_buffer_length =
+ existing_info->report_size -
+ (existing_info->keyboard_offset -
+ (report_id != 0 ? 1 : 0));
+
+ existing_info->count = 0;
+ existing_info->mean = 0;
+ existing_info->M2 = 0;
+ existing_info->prev_keydown_ts = 0;
+
+ __builtin_memset(existing_info->prev_report, 0,
+ sizeof(existing_info->prev_report));
+
+ bpf_spin_unlock(&existing_info->lock);
+ ctx->retval = 0;
+ return 0;
+ }
+
+ info.connection_time = bpf_ktime_get_ns();
+ info.bits_start = bits_start;
+ info.bits_end = bits_end;
+ info.keyboard_offset = bits_start / 8;
+ info.report_size = size_in_bytes;
+
+ if (info.keyboard_offset > info.report_size) {
+ ctx->retval = -EINVAL;
+ return 0;
+ }
+
+ info.report_id = report_id;
+ info.raw_buffer_length = info.report_size - (info.keyboard_offset -
+ (report_id != 0 ? 1 : 0));
+
+ bpf_map_update_elem(&dev_details, &hid_id, &info, BPF_NOEXIST);
+ ctx->retval = 0;
return 0;
}
diff --git a/src/bpf/testing/meson.build b/src/bpf/testing/meson.build
index 9d1b5d3..2586aea 100644
--- a/src/bpf/testing/meson.build
+++ b/src/bpf/testing/meson.build
@@ -11,6 +11,7 @@ tracing_sources = [
# 'sources' are BPF programs only compatible with
# struct_ops (kernel v6.11+)
sources = [
+ '0010-Generic__keyboard.bpf.c',
]
foreach bpf: tracing_sources
--
2.55.0
^ permalink raw reply related
* Re: [PATCH v9 0/9] Add support for MT6392 PMIC
From: AngeloGioacchino Del Regno @ 2026-07-09 12:00 UTC (permalink / raw)
To: Luca Leonardo Scorcia, linux-mediatek
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Sen Chu, Sean Wang, Macpaul Lin, Lee Jones, Matthias Brugger,
Liam Girdwood, Mark Brown, Linus Walleij, Louis-Alexis Eyraud,
Val Packett, Julien Massot, Fabien Parent, Akari Tsuyukusa,
Chen Zhong, linux-input, devicetree, linux-kernel, linux-pm,
linux-arm-kernel, linux-gpio
In-Reply-To: <20260621081634.467858-1-l.scorcia@gmail.com>
On 6/21/26 10:13, Luca Leonardo Scorcia wrote:
> The MediaTek MT6392 PMIC is usually found on devices powered by
> the MT8516/MT8167 SoC and is yet another MT6323/MT6397 variant.
>
> This series is mostly based around patches submitted a couple
> years ago by Fabien Parent and not merged and from Val Packett's
> submission from Jan 2025 that included extra cleanups, fixes, and a
> new dtsi file similar to ones that exist for other PMICs. Some
> comments weren't addressed and the series was ultimately not merged.
>
> These patches enable four functions: keys, regulator, pinctrl and RTC.
> Mono speaker amp will follow later as I need to work further on the
> audio codec.
>
> I added a handful of device tree improvements to fix some dtbs_check
> errors, added support for the pinctrl device and addressed the comments
> from last year's reviews.
>
> Please note that patch 0006 and 0008 depend on patch 0005 as they need the
> registers.h file, but belong to different driver areas. I'm not sure if
> I'm supposed to squash them even if they belong to different driver
> areas of if it's fine like this. Any advice is welcome.
>
> Patch 0009 also depends on patch 0003 because of mt6392-regulator.h.
>
> The series has been tested on Xiaomi Mi Smart Clock X04G and on the
> Lenovo Smart Clock 2 CD-24502F.
>
While series is
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Thanks!
Angelo
> Changes in v9:
> - Correct binding for vrtc as it does not support mode setting.
>
> From sashiko:
> - Added missing include in MFD documentation example.
> - Fixed constraints for regulator-initial-mode in regulator binding.
> - Fixed wrong register write while setting LDO standby mode.
> - Added missing pmic interrupt definition in the pumpkin-common include.
>
> Changes in v8 [9]:
> From reviewers:
> - Added example code to the MFD device binding, removed it from the
> regulators docs.
> - Added minItems/maxItems constraints on the regulator mode definitions,
> improved the mode constants.
> - Fixed formatting issues in the regulator binding.
> - Import the mt6392.dtsi file in pumpkin-common.dtsi, as it was originally
> meant in [8].
>
> From sashiko:
> - Added more explicit constraints on the regulator modes definitions.
> - Use the appropriate modeget register for LDO regulators, Buck registers
> don't have the corresponding register according to the data sheet.
> - Added the missing of_map_mode function.
> - Removed some debugging code that had no use and masked error codes.
>
> Changes in v7 [7]:
> - Removed patch 0008 dependency on patch 0003.
> - Reintroduced the regulator driver. In earlier revisions of this series,
> it was proposed to remove the dedicated compatible for the regulator
> device [3]. The driver does not use actually it, but it is not possible
> at this time to remove it from the bindings since it's a required
> property.
>
> Making the regulator-required property conditional was NACKed in [5],
> with the suggestion to create a separate binding altogether for devices
> that do not require the compatible property. I tried implementing this,
> but since the parent device needs to be declared as compatible with
> mt6323, it leads to a warning in dt_binding_check since mt6323 would
> be declared as a compatible in both mt6392 and mt6397.
>
> In the end the only regulator driver from the mt6397 documentation that
> still declares an of_match is mt6397-regulator and it does not seem
> to be necessary, so it should be possible to remove it and make the
> regulator compatible optional for all regulators, but that change would
> probably deserve its own separate patch series.
>
> Changes in v6 [6]:
> - Dropped the regulators driver for the moment
> - Explained the FCHR key name origin in the commit message
> - Introduced the MFD_CELL_* macro in the sub-devices definitions.
> A separate, independent commit introduced MFD_CELL_* to all the
> subdevices in the mt6397-core.c file for consistency
> - Replaced of_device_get_match_data with device_get_match_data
> - Removed the mfd_match_data enum in favor of the preexisting
> chip_id enum
> - Adjusted the error message if the device is unsupported
>
> Changes in v5 [5]:
> - Double checked regulator driver with data sheet and Android sources.
> The data sheet I have misses a lot of register descriptions, but
> Android sources have been helpful to fill the gaps
> - Reintroduced the required attribute for the regulator compatible
> in the bindings
> - Fixed the missing reference to the MT6392 schema
> - Fixed casts/unused vars reported by kernel test robot
> - Removed Reviewed-by tags from the regulator patches as they have been
> modified in this version
>
> Changes in v4 [4]:
> - Dropped usage of the regulator compatible
> - Fixed commit messages text to properly reference the target subsystem
> - Added supply rails to the regulator
> - Reworked the regulator schema and PMIC dtsi. Now all supplies are
> documented and the schema no longer includes voltage information
> - Removed redundant ldo- / buck- prefixes
> - Renamed the pinfunc header to mediatek,mt6392-pinfunc.h
> - Modified the MFD driver to use a simple identifier in the of_match
> data properties
>
> Changes in v3 [3]:
> - Added pinctrl device
> - Changed mt6397-rtc fallback to mt6323-rtc
> - Added schema for regulators
> - Fixed checkpatch issues
>
> Changes in v2 [2]:
> - Replaced explicit compatibles with fallbacks
>
> Initial version: [1]
>
> [1] https://lore.kernel.org/linux-mediatek/cover.1771865014.git.l.scorcia@gmail.com/
> [2] https://lore.kernel.org/linux-mediatek/20260306120521.163654-1-l.scorcia@gmail.com/
> [3] https://lore.kernel.org/linux-mediatek/20260317184507.523060-1-l.scorcia@gmail.com/
> [4] https://lore.kernel.org/linux-mediatek/20260330083429.359819-1-l.scorcia@gmail.com/
> [5] https://lore.kernel.org/linux-mediatek/20260420213529.1645560-1-l.scorcia@gmail.com/
> [6] https://lore.kernel.org/linux-mediatek/20260612200717.361018-1-l.scorcia@gmail.com/
> [7] https://lore.kernel.org/linux-mediatek/20260615071836.362883-1-l.scorcia@gmail.com/
> [8] https://lore.kernel.org/linux-mediatek/20190323211612.860-25-fparent@baylibre.com/
> [9] https://lore.kernel.org/linux-mediatek/20260620200032.334192-1-l.scorcia@gmail.com/
>
> Fabien Parent (3):
> dt-bindings: input: mtk-pmic-keys: Add MT6392 PMIC keys
> mfd: mt6397: Add support for MT6392 PMIC
> regulator: Add MediaTek MT6392 regulator
>
> Luca Leonardo Scorcia (4):
> dt-bindings: mfd: mt6397: Add MT6392 PMIC
> regulator: dt-bindings: Add MediaTek MT6392 PMIC
> mfd: mt6397: Use MFD_CELL_* to describe sub-devices
> pinctrl: mediatek: mt6397: Add MediaTek MT6392
>
> Val Packett (2):
> input: keyboard: mtk-pmic-keys: Add MT6392 support
> arm64: dts: mediatek: Add MediaTek MT6392 PMIC dtsi
>
> .../bindings/input/mediatek,pmic-keys.yaml | 1 +
> .../bindings/mfd/mediatek,mt6397.yaml | 75 ++
> .../regulator/mediatek,mt6392-regulator.yaml | 112 +++
> arch/arm64/boot/dts/mediatek/mt6392.dtsi | 145 ++++
> .../boot/dts/mediatek/pumpkin-common.dtsi | 7 +
> drivers/input/keyboard/mtk-pmic-keys.c | 17 +
> drivers/mfd/mt6397-core.c | 295 ++++---
> drivers/mfd/mt6397-irq.c | 8 +
> drivers/pinctrl/mediatek/pinctrl-mt6397.c | 37 +-
> drivers/pinctrl/mediatek/pinctrl-mtk-mt6392.h | 64 ++
> drivers/regulator/Kconfig | 9 +
> drivers/regulator/Makefile | 1 +
> drivers/regulator/mt6392-regulator.c | 764 ++++++++++++++++++
> .../regulator/mediatek,mt6392-regulator.h | 23 +
> include/linux/mfd/mt6392/core.h | 43 +
> include/linux/mfd/mt6392/registers.h | 488 +++++++++++
> include/linux/mfd/mt6397/core.h | 1 +
> include/linux/regulator/mt6392-regulator.h | 42 +
> 18 files changed, 1970 insertions(+), 162 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/regulator/mediatek,mt6392-regulator.yaml
> create mode 100644 arch/arm64/boot/dts/mediatek/mt6392.dtsi
> create mode 100644 drivers/pinctrl/mediatek/pinctrl-mtk-mt6392.h
> create mode 100644 drivers/regulator/mt6392-regulator.c
> create mode 100644 include/dt-bindings/regulator/mediatek,mt6392-regulator.h
> create mode 100644 include/linux/mfd/mt6392/core.h
> create mode 100644 include/linux/mfd/mt6392/registers.h
> create mode 100644 include/linux/regulator/mt6392-regulator.h
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox