driver: input: add input driver for rts5912 #86755

JasonLin-RealTek · 2025-03-07T07:30:04Z

Add input driver for Realtek rts5912.

fabiobaltieri

Cool, just a couple of minor comments

fabiobaltieri · 2025-03-11T15:33:32Z

boards/realtek/rts5912_evb/rts5912_evb.dts

@@ -17,6 +17,7 @@
 		zephyr,sram = &sram0;
 		zephyr,console = &uart0;
 		zephyr,flash = &flash0;
+		zephyr,keyboard-scan = &kscan_input;


can you drop this? this api is being removed in this release, sorry for the noise

fabiobaltieri · 2025-03-11T15:33:41Z

boards/realtek/rts5912_evb/rts5912_evb.dts

+	kscan_input: kscan-input {
+		compatible = "zephyr,kscan-input";
+	};


Done, drop it.

fabiobaltieri · 2025-03-11T15:33:59Z

drivers/input/Kconfig.rts5912

@@ -0,0 +1,10 @@
+# Copyright (c) 2021 Realtek Corporation. All Rights Reserved.


2025, happy new year :-)

Happy new year!
it's 2025

fabiobaltieri · 2025-03-11T15:34:39Z

drivers/input/Kconfig.rts5912

+	depends on DT_HAS_REALTEK_RTS5912_KBD_ENABLED
+	select INPUT_KBD_MATRIX
+	help
+	  This option enables the REALTEK keyboard scan driver.


Realtek? RealTek? Pick one and stick to it

We picked RealTek here.

fabiobaltieri · 2025-03-11T15:35:32Z

drivers/input/input_realtek_rts5912_kbd.c

+#define LOG_LEVEL CONFIG_INPUT_LOG_LEVEL
+LOG_MODULE_REGISTER(input_realtek_rts5912_kbd);


LOG_MODULE_REGISTER(input_realtek_rts5912_kbd, CONFIG_INPUT_LOG_LEVEL); keep it compact

fabiobaltieri · 2025-03-11T15:38:58Z

drivers/input/input_realtek_rts5912_kbd.c

+
+	/* Set KSO output data */
+	key = irq_lock();
+	inst->SCANOUT = kso_val;


hey just checking, inst->SCANOUT is not going to change pins that are not mapped to the keyboad driver in pin control right? As in: if some of those pins are used as gpios, this should not be affecting them

We add the kso_ignore_mask to this function.

fabiobaltieri · 2025-03-11T15:39:33Z

drivers/input/input_realtek_rts5912_kbd.c

+	KBM_Type *const inst = config->base;
+
+	/* Bits are active-low, so toggle it (return 1 means key pressed) */
+	return ((inst->SCANIN ^ data->ksi_pin_mask) & data->ksi_pin_mask);


could you not just invert all the bits in the register all the time?

fabiobaltieri · 2025-03-11T15:45:24Z

drivers/input/input_realtek_rts5912_kbd.c

+BUILD_ASSERT(IN_RANGE(DT_INST_PROP(0, row_size), 1, 10), "invalid row-size");
+BUILD_ASSERT(IN_RANGE(DT_INST_PROP(0, col_size), 1, 20), "invalid col-size");


10x20? that's a seriously big keyboard :-)

Thanks for review!
Our maximum keyboard can support kso19 and ksi9.

keith-zephyr

@fabiobaltieri are there any tests or samples that this driver can be added to so that the module gets compiled by CI?

fabiobaltieri · 2025-03-15T07:54:15Z

@fabiobaltieri are there any tests or samples that this driver can be added to so that the module gets compiled by CI?

This should happen automatically since the device is instantiated in the board, at least in the weekly, but it also looks like the PR run here correctly built it as well, I see these in the junit file

                <testcase classname="rts5912_evb/rts5912:sample.input.input_dump" name="sample.input.input_dump" time="0.00" />
                <testcase classname="rts5912_evb/rts5912:sample.input.input_dump_shell" name="sample.input.input_dump_shell" time="0.00" />

keith-zephyr · 2025-03-17T17:42:03Z

soc/realtek/ec/rts5912/reg/reg_kbm.h

+ * @brief Keyboard Matrix Controller (KBM)
+ */
+
+typedef struct {


Don't create new typedefs. Use struct kbm_regs or something similar. Also, make the entire structure volatile instead of the individual fields.

volatile struct kbm_regs *base;

Thanks,
We change the way to define the kbm register and use volatile on the whole fields of the structure.

keith-zephyr · 2025-03-18T17:56:34Z

soc/realtek/ec/rts5912/reg/reg_kbm.h

+	uint32_t SCANOUT;
+	uint32_t SCANIN;
+	uint32_t INTEN;
+	uint32_t CTRL;


Please snake case to match Zephyr coding style. Convert these fields to lower case.
https://docs.zephyrproject.org/latest/contribute/style/code.html

Thanks,
We use the snake case to name the variable.

boards/realtek/rts5912_evb/rts5912_evb_defconfig

dts/bindings/input/realtek,rts5912-kbd.yaml

drivers/input/Kconfig.rts5912

Add input driver for Realtek rts5912. Signed-off-by: Lin Yu-Cheng <lin_yu_cheng@realtek.com>

zephyrbot added platform: Realtek EC area: Input Input Subsystem and Drivers labels Mar 7, 2025

zephyrbot requested review from fabiobaltieri and gmarull March 7, 2025 07:30

zephyrbot assigned fabiobaltieri Mar 7, 2025

JasonLin-RealTek force-pushed the add_realtek_rts5912_input_driver branch 2 times, most recently from e0e0726 to bd23873 Compare March 7, 2025 11:13

JasonLin-RealTek requested a review from keith-zephyr March 7, 2025 12:05

fabiobaltieri reviewed Mar 11, 2025

View reviewed changes

JasonLin-RealTek force-pushed the add_realtek_rts5912_input_driver branch 5 times, most recently from ff52039 to d6a13c7 Compare March 14, 2025 11:41

keith-zephyr reviewed Mar 14, 2025

View reviewed changes

keith-zephyr reviewed Mar 17, 2025

View reviewed changes

JasonLin-RealTek force-pushed the add_realtek_rts5912_input_driver branch from d6a13c7 to 85af136 Compare March 18, 2025 10:13

fabiobaltieri previously approved these changes Mar 18, 2025

View reviewed changes

keith-zephyr reviewed Mar 18, 2025

View reviewed changes

kartben requested changes Mar 19, 2025

View reviewed changes

boards/realtek/rts5912_evb/rts5912_evb_defconfig Outdated Show resolved Hide resolved

dts/bindings/input/realtek,rts5912-kbd.yaml Outdated Show resolved Hide resolved

drivers/input/Kconfig.rts5912 Outdated Show resolved Hide resolved

JasonLin-RealTek dismissed fabiobaltieri’s stale review via 8d6408c March 19, 2025 10:19

JasonLin-RealTek force-pushed the add_realtek_rts5912_input_driver branch from 85af136 to 8d6408c Compare March 19, 2025 10:19

driver: input: add input driver for rts5912

faa0352

Add input driver for Realtek rts5912. Signed-off-by: Lin Yu-Cheng <lin_yu_cheng@realtek.com>

JasonLin-RealTek force-pushed the add_realtek_rts5912_input_driver branch from 8d6408c to faa0352 Compare March 19, 2025 10:39

kartben approved these changes Mar 19, 2025

View reviewed changes

kartben requested review from fabiobaltieri and keith-zephyr March 19, 2025 10:48

fabiobaltieri approved these changes Mar 19, 2025

View reviewed changes

kartben merged commit 2d541a0 into zephyrproject-rtos:main Mar 19, 2025
20 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

driver: input: add input driver for rts5912 #86755

driver: input: add input driver for rts5912 #86755

JasonLin-RealTek commented Mar 7, 2025

fabiobaltieri left a comment

fabiobaltieri Mar 11, 2025

JasonLin-RealTek Mar 14, 2025

fabiobaltieri Mar 11, 2025

JasonLin-RealTek Mar 14, 2025

fabiobaltieri Mar 11, 2025

JasonLin-RealTek Mar 14, 2025

fabiobaltieri Mar 11, 2025

JasonLin-RealTek Mar 14, 2025

fabiobaltieri Mar 11, 2025

JasonLin-RealTek Mar 14, 2025

fabiobaltieri Mar 11, 2025

JasonLin-RealTek Mar 14, 2025

fabiobaltieri Mar 11, 2025

JasonLin-RealTek Mar 14, 2025

fabiobaltieri Mar 11, 2025

JasonLin-RealTek Mar 14, 2025

keith-zephyr left a comment

fabiobaltieri commented Mar 15, 2025

keith-zephyr Mar 17, 2025

JasonLin-RealTek Mar 18, 2025

keith-zephyr Mar 18, 2025

JasonLin-RealTek Mar 19, 2025

		@@ -0,0 +1,10 @@
		# Copyright (c) 2021 Realtek Corporation. All Rights Reserved.

		#define LOG_LEVEL CONFIG_INPUT_LOG_LEVEL
		LOG_MODULE_REGISTER(input_realtek_rts5912_kbd);

		BUILD_ASSERT(IN_RANGE(DT_INST_PROP(0, row_size), 1, 10), "invalid row-size");
		BUILD_ASSERT(IN_RANGE(DT_INST_PROP(0, col_size), 1, 20), "invalid col-size");

driver: input: add input driver for rts5912 #86755

driver: input: add input driver for rts5912 #86755

Conversation

JasonLin-RealTek commented Mar 7, 2025

fabiobaltieri left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keith-zephyr left a comment

Choose a reason for hiding this comment

fabiobaltieri commented Mar 15, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment