[PATCH v2 0/4] ROCK 4D audio enablement

[PATCH v2 0/4] ROCK 4D audio enablement

[Nicolas Frattaroli]

The ROCK 4D uses an ADC input to distinguish between a headphone (i.e.,
no mic) and a headset (i.e., with mic). After some searching, it appears
that the closest we can get to modelling this is by sending a particular
switch input event.

So this series modifies the adc-keys bindings, extends the adc-keys
driver to allow sending other input types as well, and then adds the
analog audio nodes to ROCK 4D's device tree.

It should be noted that analog capture from the TRRS jack currently
results in completely digitally silent audio for me, i.e. no data other
than 0xFF. There's a few reasons why this could happen, chief among them
that my SAI driver is broken or that the ES8328 codec driver is once
again broken. The DAPM routes when graphed out look fine though. So the
DTS part is correct, and I can fix the broken capture in a separate
follow-up patch that doesn't have to include DT people.

Another possibility is that my phone headset, despite being 4 rings and
having a little pin hole at the back of the volume doodad, does not
actually have a microphone, but in that case I'd still expect some noise
in the PCM. Maybe it's just shy.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
Changes in v2:
- Drop HDMI audio patch, as it was already merged.
- adc-keys: rename "keycode" to "code".
- adc-keys: make the keycode (now "code") local a u32 instead of an int
- adc-keys: only allow EV_KEY and EV_SW for now. Rename patch
  accordingly.
- adc-keys: Add another patch to rework probe function error logging.
- Link to v1: https://lore.kernel.org/r/20250630-rock4d-audio-v1-0-0b3c8e8fda9c@collabora.com

---
Nicolas Frattaroli (4):
      dt-bindings: input: adc-keys: allow linux,input-type property
      Input: adc-keys - support EV_SW as well, not just EV_KEY.
      Input: adc-keys - Use dev_err_probe in probe function
      arm64: dts: rockchip: add analog audio to ROCK 4D

 .../devicetree/bindings/input/adc-keys.yaml        |  3 +
 arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts    | 90 ++++++++++++++++++++++
 drivers/input/keyboard/adc-keys.c                  | 88 ++++++++++-----------
 3 files changed, 138 insertions(+), 43 deletions(-)
---
base-commit: 3e7f562e20ee87a25e104ef4fce557d39d62fa85
change-id: 20250627-rock4d-audio-cfc07f168a08

Best regards,
-- 
Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

[PATCH v2 1/4] dt-bindings: input: adc-keys: allow linux,input-type property

[Nicolas Frattaroli]

adc-keys, unlike gpio-keys, does not allow linux,input-type as a valid
property. This makes it impossible to model devices that have ADC inputs
that should generate switch events.

Add the property to the binding with the same default as gpio-keys.

Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 Documentation/devicetree/bindings/input/adc-keys.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/adc-keys.yaml b/Documentation/devicetree/bindings/input/adc-keys.yaml
index 7aa078dead37..e372ebc23d16 100644
--- a/Documentation/devicetree/bindings/input/adc-keys.yaml
+++ b/Documentation/devicetree/bindings/input/adc-keys.yaml
@@ -42,6 +42,9 @@ patternProperties:
 
       linux,code: true
 
+      linux,input-type:
+        default: 1  # EV_KEY
+
       press-threshold-microvolt:
         description:
           Voltage above or equal to which this key is considered pressed. No

-- 
2.52.0

[PATCH v2 2/4] Input: adc-keys - support EV_SW as well, not just EV_KEY.

[Nicolas Frattaroli]

Instead of doing something like what gpio-keys is doing, adc-keys
hardcodes that all keycodes must be of type EV_KEY.

This limits the usefulness of adc-keys, and overcomplicates the code
with manual bit-setting logic.

Instead, refactor the code to read the linux,input-type fwnode property,
and get rid of the custom bit setting logic, replacing it with
input_set_capability instead. input_report_key is replaced with
input_event, which allows us to explicitly pass the type.

Only EV_KEY and EV_SW is allowed at this stage.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/input/keyboard/adc-keys.c | 37 +++++++++++++++++++++++++------------
 1 file changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/input/keyboard/adc-keys.c b/drivers/input/keyboard/adc-keys.c
index f1753207429d..62376f34f7d0 100644
--- a/drivers/input/keyboard/adc-keys.c
+++ b/drivers/input/keyboard/adc-keys.c
@@ -18,13 +18,15 @@
 
 struct adc_keys_button {
 	u32 voltage;
-	u32 keycode;
+	u32 code;
+	u32 type;
 };
 
 struct adc_keys_state {
 	struct iio_channel *channel;
 	u32 num_keys;
 	u32 last_key;
+	u32 last_type;
 	u32 keyup_voltage;
 	const struct adc_keys_button *map;
 };
@@ -34,7 +36,8 @@ static void adc_keys_poll(struct input_dev *input)
 	struct adc_keys_state *st = input_get_drvdata(input);
 	int i, value, ret;
 	u32 diff, closest = 0xffffffff;
-	int keycode = 0;
+	u32 code = 0;
+	u32 type = EV_KEY;
 
 	ret = iio_read_channel_processed(st->channel, &value);
 	if (unlikely(ret < 0)) {
@@ -45,22 +48,24 @@ static void adc_keys_poll(struct input_dev *input)
 			diff = abs(st->map[i].voltage - value);
 			if (diff < closest) {
 				closest = diff;
-				keycode = st->map[i].keycode;
+				code = st->map[i].code;
+				type = st->map[i].type;
 			}
 		}
 	}
 
 	if (abs(st->keyup_voltage - value) < closest)
-		keycode = 0;
+		code = 0;
 
-	if (st->last_key && st->last_key != keycode)
-		input_report_key(input, st->last_key, 0);
+	if (st->last_key && st->last_key != code)
+		input_event(input, st->last_type, st->last_key, 0);
 
-	if (keycode)
-		input_report_key(input, keycode, 1);
+	if (code)
+		input_event(input, type, code, 1);
 
 	input_sync(input);
-	st->last_key = keycode;
+	st->last_key = code;
+	st->last_type = type;
 }
 
 static int adc_keys_load_keymap(struct device *dev, struct adc_keys_state *st)
@@ -88,11 +93,20 @@ static int adc_keys_load_keymap(struct device *dev, struct adc_keys_state *st)
 		map[i].voltage /= 1000;
 
 		if (fwnode_property_read_u32(child, "linux,code",
-					     &map[i].keycode)) {
+					     &map[i].code)) {
 			dev_err(dev, "Key with invalid or missing linux,code\n");
 			return -EINVAL;
 		}
 
+		if (fwnode_property_read_u32(child, "linux,input-type",
+					     &map[i].type))
+			map[i].type = EV_KEY;
+
+		if (map[i].type != EV_KEY && map[i].type != EV_SW)
+			return dev_err_probe(dev, -EINVAL,
+					     "Invalid linux,input-type: 0x%x\n",
+					     map[i].type);
+
 		i++;
 	}
 
@@ -156,9 +170,8 @@ static int adc_keys_probe(struct platform_device *pdev)
 	input->id.product = 0x0001;
 	input->id.version = 0x0100;
 
-	__set_bit(EV_KEY, input->evbit);
 	for (i = 0; i < st->num_keys; i++)
-		__set_bit(st->map[i].keycode, input->keybit);
+		input_set_capability(input, st->map[i].type, st->map[i].code);
 
 	if (device_property_read_bool(dev, "autorepeat"))
 		__set_bit(EV_REP, input->evbit);

-- 
2.52.0

[PATCH v2 3/4] Input: adc-keys - Use dev_err_probe in probe function

[Nicolas Frattaroli]

Rework the probe function, and functions called by the probe function,
to use dev_err_probe for error logging.

While at it, also do some minor style cleanups, like not error logging
on -ENOMEM and using ! instead of == 0.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/input/keyboard/adc-keys.c | 53 ++++++++++++++++-----------------------
 1 file changed, 21 insertions(+), 32 deletions(-)

diff --git a/drivers/input/keyboard/adc-keys.c b/drivers/input/keyboard/adc-keys.c
index 62376f34f7d0..6f2ddcecea99 100644
--- a/drivers/input/keyboard/adc-keys.c
+++ b/drivers/input/keyboard/adc-keys.c
@@ -74,10 +74,8 @@ static int adc_keys_load_keymap(struct device *dev, struct adc_keys_state *st)
 	int i;
 
 	st->num_keys = device_get_child_node_count(dev);
-	if (st->num_keys == 0) {
-		dev_err(dev, "keymap is missing\n");
-		return -EINVAL;
-	}
+	if (!st->num_keys)
+		return dev_err_probe(dev, -EINVAL, "keymap is missing\n");
 
 	map = devm_kmalloc_array(dev, st->num_keys, sizeof(*map), GFP_KERNEL);
 	if (!map)
@@ -86,17 +84,16 @@ static int adc_keys_load_keymap(struct device *dev, struct adc_keys_state *st)
 	i = 0;
 	device_for_each_child_node_scoped(dev, child) {
 		if (fwnode_property_read_u32(child, "press-threshold-microvolt",
-					     &map[i].voltage)) {
-			dev_err(dev, "Key with invalid or missing voltage\n");
-			return -EINVAL;
-		}
+					     &map[i].voltage))
+			return dev_err_probe(dev, -EINVAL,
+					     "Key with invalid or missing voltage\n");
+
 		map[i].voltage /= 1000;
 
 		if (fwnode_property_read_u32(child, "linux,code",
-					     &map[i].code)) {
-			dev_err(dev, "Key with invalid or missing linux,code\n");
-			return -EINVAL;
-		}
+					     &map[i].code))
+			return dev_err_probe(dev, -EINVAL,
+					     "Key with invalid or missing linux,code\n");
 
 		if (fwnode_property_read_u32(child, "linux,input-type",
 					     &map[i].type))
@@ -129,7 +126,8 @@ static int adc_keys_probe(struct platform_device *pdev)
 
 	st->channel = devm_iio_channel_get(dev, "buttons");
 	if (IS_ERR(st->channel))
-		return PTR_ERR(st->channel);
+		return dev_err_probe(dev, PTR_ERR(st->channel),
+				     "Could not get iio channel\n");
 
 	if (!st->channel->indio_dev)
 		return -ENXIO;
@@ -138,16 +136,13 @@ static int adc_keys_probe(struct platform_device *pdev)
 	if (error < 0)
 		return error;
 
-	if (type != IIO_VOLTAGE) {
-		dev_err(dev, "Incompatible channel type %d\n", type);
-		return -EINVAL;
-	}
+	if (type != IIO_VOLTAGE)
+		return dev_err_probe(dev, -EINVAL, "Incompatible channel type %d\n", type);
 
 	if (device_property_read_u32(dev, "keyup-threshold-microvolt",
-				     &st->keyup_voltage)) {
-		dev_err(dev, "Invalid or missing keyup voltage\n");
-		return -EINVAL;
-	}
+				     &st->keyup_voltage))
+		return dev_err_probe(dev, -EINVAL, "Invalid or missing keyup voltage\n");
+
 	st->keyup_voltage /= 1000;
 
 	error = adc_keys_load_keymap(dev, st);
@@ -155,10 +150,8 @@ static int adc_keys_probe(struct platform_device *pdev)
 		return error;
 
 	input = devm_input_allocate_device(dev);
-	if (!input) {
-		dev_err(dev, "failed to allocate input device\n");
+	if (!input)
 		return -ENOMEM;
-	}
 
 	input_set_drvdata(input, st);
 
@@ -178,19 +171,15 @@ static int adc_keys_probe(struct platform_device *pdev)
 
 
 	error = input_setup_polling(input, adc_keys_poll);
-	if (error) {
-		dev_err(dev, "Unable to set up polling: %d\n", error);
-		return error;
-	}
+	if (error)
+		return dev_err_probe(dev, error, "Unable to set up polling\n");
 
 	if (!device_property_read_u32(dev, "poll-interval", &value))
 		input_set_poll_interval(input, value);
 
 	error = input_register_device(input);
-	if (error) {
-		dev_err(dev, "Unable to register input device: %d\n", error);
-		return error;
-	}
+	if (error)
+		return dev_err_probe(dev, error, "Unable to register input device\n");
 
 	return 0;
 }

-- 
2.52.0

[PATCH v2 4/4] arm64: dts: rockchip: add analog audio to ROCK 4D

[Nicolas Frattaroli]

The RADXA ROCK 4D, like many other Rockchip-based boards, uses an ES8388
analog audio codec. On the production version of the board, the codec's
LOUT1 and ROUT1 pins are tied to the headphone jack, whereas pins LOUT2
and ROUT2 lead to a non-populated speaker amplifier that itself leads to
a non-populated speaker jack. The schematic is still haunted by the
ghosts of those symbols, but it clearly marks them as "NC".

The 3.5mm TRRS jack has its microphone ring (and ground ring) wired to
the codec's LINPUT1 and RINPUT1 pins for differential signalling.

Furthermore, it uses the SoCs ADC to detect whether the inserted cable
is of headphones (i.e., no microphone), or a headset (i.e., with
microphone). The way this is done is that the ADC input taps the output
of a 100K/100K resistor divider that divides the microphone ring pin
that's pulled up to 3.3V.

There is no ADC level difference between a completely empty jack and one
with a set of headphones (i.e., ones that don't have a microphone)
connected. Consequently headphone insertion detection isn't something
that can be done.

Add the necessary codec and audio card nodes. The non-populated parts,
i.e. LOUT2 and ROUT2, are not modeled at all, as they are not present on
the hardware.

Also, add an adc-keys node for the headset detection, which uses an
input type of EV_SW with the SW_MICROPHONE_INSERT keycode. Below the
220mV pressed voltage level of our SW_MICROPHONE_INSERT switch, we also
define a button that emits a KEY_RESERVED code, which is there to model
this part of the voltage range as not just being extra legroom for the
button above it, but actually a state that is encountered in the real
world, and should be recognised as a valid state for the ADC range to be
in so that no "closer" ADC button is chosen.

Tested-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts | 90 +++++++++++++++++++++++++
 1 file changed, 90 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
index 7023dc326d0e..d86fe2951da6 100644
--- a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
@@ -6,6 +6,7 @@
 /dts-v1/;
 
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
 #include <dt-bindings/leds/common.h>
 #include <dt-bindings/pinctrl/rockchip.h>
 #include <dt-bindings/pwm/pwm.h>
@@ -37,6 +38,31 @@ hdmi_con_in: endpoint {
 		};
 	};
 
+	es8388_sound: es8388-sound {
+		compatible = "simple-audio-card";
+		simple-audio-card,format = "i2s";
+		simple-audio-card,mclk-fs = <256>;
+		simple-audio-card,name = "On-board Analog ES8388";
+		simple-audio-card,widgets = "Microphone", "Headphone Mic",
+					    "Headphone", "Headphone";
+		simple-audio-card,routing = "Headphone", "LOUT1",
+					    "Headphone", "ROUT1",
+					    "Left PGA Mux", "Differential Mux",
+					    "Differential Mux", "LINPUT1",
+					    "Differential Mux", "RINPUT1",
+					    "LINPUT1", "Headphone Mic",
+					    "RINPUT1", "Headphone Mic";
+
+		simple-audio-card,cpu {
+			sound-dai = <&sai1>;
+		};
+
+		simple-audio-card,codec {
+			sound-dai = <&es8388>;
+			system-clock-frequency = <12288000>;
+		};
+	};
+
 	rfkill {
 		compatible = "rfkill-gpio";
 		pinctrl-names = "default";
@@ -65,6 +91,37 @@ user-led {
 		};
 	};
 
+	saradc_keys: adc-keys {
+		compatible = "adc-keys";
+		io-channels = <&saradc 3>;
+		io-channel-names = "buttons";
+		keyup-threshold-microvolt = <3000000>;
+		poll-interval = <100>;
+
+		/*
+		 * During insertion and removal of a regular set of headphones,
+		 * i.e. one without a microphone, the voltage level briefly
+		 * dips below the 220mV of the headset connection switch.
+		 * By having a button definition with a KEY_RESERVED signal
+		 * between 0 to 220, we ensure no driver implementation thinks
+		 * that the closest thing to 0V is 220mV so clearly there must
+		 * be a headset connected.
+		 */
+
+		button-headset-disconnected {
+			label = "Headset Microphone Disconnected";
+			linux,code = <KEY_RESERVED>;
+			press-threshold-microvolt = <0>;
+		};
+
+		button-headset-connected {
+			label = "Headset Microphone Connected";
+			linux,code = <SW_MICROPHONE_INSERT>;
+			linux,input-type = <EV_SW>;
+			press-threshold-microvolt = <220000>;
+		};
+	};
+
 	vcc_5v0_dcin: regulator-vcc-5v0-dcin {
 		compatible = "regulator-fixed";
 		regulator-always-on;
@@ -682,6 +739,25 @@ hym8563: rtc@51 {
 	};
 };
 
+&i2c3 {
+	status = "okay";
+
+	es8388: audio-codec@10 {
+		compatible = "everest,es8388", "everest,es8328";
+		reg = <0x10>;
+		clocks = <&cru CLK_SAI1_MCLKOUT_TO_IO>;
+		AVDD-supply = <&vcca_3v3_s0>;
+		DVDD-supply = <&vcc_3v3_s0>;
+		HPVDD-supply = <&vcca_3v3_s0>;
+		PVDD-supply = <&vcc_3v3_s0>;
+		assigned-clocks = <&cru CLK_SAI1_MCLKOUT_TO_IO>;
+		assigned-clock-rates = <12288000>;
+		#sound-dai-cells = <0>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&sai1m0_mclk>;
+	};
+};
+
 &mdio0 {
 	rgmii_phy0: ethernet-phy@1 {
 		compatible = "ethernet-phy-id001c.c916";
@@ -756,10 +832,24 @@ wifi_en_h: wifi-en-h {
 	};
 };
 
+&sai1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&sai1m0_lrck
+		     &sai1m0_sclk
+		     &sai1m0_sdi0
+		     &sai1m0_sdo0>;
+	status = "okay";
+};
+
 &sai6 {
 	status = "okay";
 };
 
+&saradc {
+	vref-supply = <&vcca1v8_pldo2_s0>;
+	status = "okay";
+};
+
 &sdmmc {
 	bus-width = <4>;
 	cap-mmc-highspeed;

-- 
2.52.0

Re: [PATCH v2 3/4] Input: adc-keys - Use dev_err_probe in probe function

[Alexandre Belloni]

On 15/12/2025 13:29:31+0100, Nicolas Frattaroli wrote:
> Rework the probe function, and functions called by the probe function,
> to use dev_err_probe for error logging.
> 
> While at it, also do some minor style cleanups, like not error logging
> on -ENOMEM and using ! instead of == 0.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
>  drivers/input/keyboard/adc-keys.c | 53 ++++++++++++++++-----------------------
>  1 file changed, 21 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/input/keyboard/adc-keys.c b/drivers/input/keyboard/adc-keys.c
> index 62376f34f7d0..6f2ddcecea99 100644
> --- a/drivers/input/keyboard/adc-keys.c
> +++ b/drivers/input/keyboard/adc-keys.c
> @@ -74,10 +74,8 @@ static int adc_keys_load_keymap(struct device *dev, struct adc_keys_state *st)
>  	int i;
>  
>  	st->num_keys = device_get_child_node_count(dev);
> -	if (st->num_keys == 0) {
> -		dev_err(dev, "keymap is missing\n");
> -		return -EINVAL;
> -	}
> +	if (!st->num_keys)
> +		return dev_err_probe(dev, -EINVAL, "keymap is missing\n");
>  
>  	map = devm_kmalloc_array(dev, st->num_keys, sizeof(*map), GFP_KERNEL);
>  	if (!map)
> @@ -86,17 +84,16 @@ static int adc_keys_load_keymap(struct device *dev, struct adc_keys_state *st)
>  	i = 0;
>  	device_for_each_child_node_scoped(dev, child) {
>  		if (fwnode_property_read_u32(child, "press-threshold-microvolt",
> -					     &map[i].voltage)) {
> -			dev_err(dev, "Key with invalid or missing voltage\n");
> -			return -EINVAL;
> -		}
> +					     &map[i].voltage))
> +			return dev_err_probe(dev, -EINVAL,
> +					     "Key with invalid or missing voltage\n");
> +
>  		map[i].voltage /= 1000;
>  
>  		if (fwnode_property_read_u32(child, "linux,code",
> -					     &map[i].code)) {
> -			dev_err(dev, "Key with invalid or missing linux,code\n");
> -			return -EINVAL;
> -		}
> +					     &map[i].code))
> +			return dev_err_probe(dev, -EINVAL,
> +					     "Key with invalid or missing linux,code\n");
>  
>  		if (fwnode_property_read_u32(child, "linux,input-type",
>  					     &map[i].type))
> @@ -129,7 +126,8 @@ static int adc_keys_probe(struct platform_device *pdev)
>  
>  	st->channel = devm_iio_channel_get(dev, "buttons");
>  	if (IS_ERR(st->channel))
> -		return PTR_ERR(st->channel);
> +		return dev_err_probe(dev, PTR_ERR(st->channel),
> +				     "Could not get iio channel\n");
>  
>  	if (!st->channel->indio_dev)
>  		return -ENXIO;
> @@ -138,16 +136,13 @@ static int adc_keys_probe(struct platform_device *pdev)
>  	if (error < 0)
>  		return error;
>  
> -	if (type != IIO_VOLTAGE) {
> -		dev_err(dev, "Incompatible channel type %d\n", type);
> -		return -EINVAL;
> -	}
> +	if (type != IIO_VOLTAGE)
> +		return dev_err_probe(dev, -EINVAL, "Incompatible channel type %d\n", type);
>  
>  	if (device_property_read_u32(dev, "keyup-threshold-microvolt",
> -				     &st->keyup_voltage)) {
> -		dev_err(dev, "Invalid or missing keyup voltage\n");
> -		return -EINVAL;
> -	}
> +				     &st->keyup_voltage))
> +		return dev_err_probe(dev, -EINVAL, "Invalid or missing keyup voltage\n");
> +
>  	st->keyup_voltage /= 1000;
>  
>  	error = adc_keys_load_keymap(dev, st);
> @@ -155,10 +150,8 @@ static int adc_keys_probe(struct platform_device *pdev)
>  		return error;
>  
>  	input = devm_input_allocate_device(dev);
> -	if (!input) {
> -		dev_err(dev, "failed to allocate input device\n");
> +	if (!input)
>  		return -ENOMEM;
> -	}
>  
>  	input_set_drvdata(input, st);
>  
> @@ -178,19 +171,15 @@ static int adc_keys_probe(struct platform_device *pdev)
>  
>  
>  	error = input_setup_polling(input, adc_keys_poll);
> -	if (error) {
> -		dev_err(dev, "Unable to set up polling: %d\n", error);
> -		return error;
> -	}
> +	if (error)
> +		return dev_err_probe(dev, error, "Unable to set up polling\n");
>  
>  	if (!device_property_read_u32(dev, "poll-interval", &value))
>  		input_set_poll_interval(input, value);
>  
>  	error = input_register_device(input);
> -	if (error) {
> -		dev_err(dev, "Unable to register input device: %d\n", error);
> -		return error;
> -	}
> +	if (error)
> +		return dev_err_probe(dev, error, "Unable to register input device\n");
>  
>  	return 0;
>  }
> 
> -- 
> 2.52.0
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 2/4] Input: adc-keys - support EV_SW as well, not just EV_KEY.

[Alexandre Belloni]

On 15/12/2025 13:29:30+0100, Nicolas Frattaroli wrote:
> Instead of doing something like what gpio-keys is doing, adc-keys
> hardcodes that all keycodes must be of type EV_KEY.
> 
> This limits the usefulness of adc-keys, and overcomplicates the code
> with manual bit-setting logic.
> 
> Instead, refactor the code to read the linux,input-type fwnode property,
> and get rid of the custom bit setting logic, replacing it with
> input_set_capability instead. input_report_key is replaced with
> input_event, which allows us to explicitly pass the type.
> 
> Only EV_KEY and EV_SW is allowed at this stage.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
>  drivers/input/keyboard/adc-keys.c | 37 +++++++++++++++++++++++++------------
>  1 file changed, 25 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/input/keyboard/adc-keys.c b/drivers/input/keyboard/adc-keys.c
> index f1753207429d..62376f34f7d0 100644
> --- a/drivers/input/keyboard/adc-keys.c
> +++ b/drivers/input/keyboard/adc-keys.c
> @@ -18,13 +18,15 @@
>  
>  struct adc_keys_button {
>  	u32 voltage;
> -	u32 keycode;
> +	u32 code;
> +	u32 type;
>  };
>  
>  struct adc_keys_state {
>  	struct iio_channel *channel;
>  	u32 num_keys;
>  	u32 last_key;
> +	u32 last_type;
>  	u32 keyup_voltage;
>  	const struct adc_keys_button *map;
>  };
> @@ -34,7 +36,8 @@ static void adc_keys_poll(struct input_dev *input)
>  	struct adc_keys_state *st = input_get_drvdata(input);
>  	int i, value, ret;
>  	u32 diff, closest = 0xffffffff;
> -	int keycode = 0;
> +	u32 code = 0;
> +	u32 type = EV_KEY;
>  
>  	ret = iio_read_channel_processed(st->channel, &value);
>  	if (unlikely(ret < 0)) {
> @@ -45,22 +48,24 @@ static void adc_keys_poll(struct input_dev *input)
>  			diff = abs(st->map[i].voltage - value);
>  			if (diff < closest) {
>  				closest = diff;
> -				keycode = st->map[i].keycode;
> +				code = st->map[i].code;
> +				type = st->map[i].type;
>  			}
>  		}
>  	}
>  
>  	if (abs(st->keyup_voltage - value) < closest)
> -		keycode = 0;
> +		code = 0;
>  
> -	if (st->last_key && st->last_key != keycode)
> -		input_report_key(input, st->last_key, 0);
> +	if (st->last_key && st->last_key != code)
> +		input_event(input, st->last_type, st->last_key, 0);
>  
> -	if (keycode)
> -		input_report_key(input, keycode, 1);
> +	if (code)
> +		input_event(input, type, code, 1);
>  
>  	input_sync(input);
> -	st->last_key = keycode;
> +	st->last_key = code;
> +	st->last_type = type;
>  }
>  
>  static int adc_keys_load_keymap(struct device *dev, struct adc_keys_state *st)
> @@ -88,11 +93,20 @@ static int adc_keys_load_keymap(struct device *dev, struct adc_keys_state *st)
>  		map[i].voltage /= 1000;
>  
>  		if (fwnode_property_read_u32(child, "linux,code",
> -					     &map[i].keycode)) {
> +					     &map[i].code)) {
>  			dev_err(dev, "Key with invalid or missing linux,code\n");
>  			return -EINVAL;
>  		}
>  
> +		if (fwnode_property_read_u32(child, "linux,input-type",
> +					     &map[i].type))
> +			map[i].type = EV_KEY;
> +
> +		if (map[i].type != EV_KEY && map[i].type != EV_SW)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "Invalid linux,input-type: 0x%x\n",
> +					     map[i].type);
> +
>  		i++;
>  	}
>  
> @@ -156,9 +170,8 @@ static int adc_keys_probe(struct platform_device *pdev)
>  	input->id.product = 0x0001;
>  	input->id.version = 0x0100;
>  
> -	__set_bit(EV_KEY, input->evbit);
>  	for (i = 0; i < st->num_keys; i++)
> -		__set_bit(st->map[i].keycode, input->keybit);
> +		input_set_capability(input, st->map[i].type, st->map[i].code);
>  
>  	if (device_property_read_bool(dev, "autorepeat"))
>  		__set_bit(EV_REP, input->evbit);
> 
> -- 
> 2.52.0
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 1/4] dt-bindings: input: adc-keys: allow linux,input-type property

[Alexandre Belloni]

On 15/12/2025 13:29:29+0100, Nicolas Frattaroli wrote:
> adc-keys, unlike gpio-keys, does not allow linux,input-type as a valid
> property. This makes it impossible to model devices that have ADC inputs
> that should generate switch events.
> 
> Add the property to the binding with the same default as gpio-keys.
> 
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
>  Documentation/devicetree/bindings/input/adc-keys.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/adc-keys.yaml b/Documentation/devicetree/bindings/input/adc-keys.yaml
> index 7aa078dead37..e372ebc23d16 100644
> --- a/Documentation/devicetree/bindings/input/adc-keys.yaml
> +++ b/Documentation/devicetree/bindings/input/adc-keys.yaml
> @@ -42,6 +42,9 @@ patternProperties:
>  
>        linux,code: true
>  
> +      linux,input-type:
> +        default: 1  # EV_KEY
> +
>        press-threshold-microvolt:
>          description:
>            Voltage above or equal to which this key is considered pressed. No
> 
> -- 
> 2.52.0
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2 1/4] dt-bindings: input: adc-keys: allow linux,input-type property

[Krzysztof Kozlowski]

On Mon, Dec 15, 2025 at 01:29:29PM +0100, Nicolas Frattaroli wrote:
> adc-keys, unlike gpio-keys, does not allow linux,input-type as a valid
> property. This makes it impossible to model devices that have ADC inputs
> that should generate switch events.

The solution is to use unevaluatedProps instead, which also allows
dropping other properties.

Best regards,
Krzysztof

Re: [PATCH v2 1/4] dt-bindings: input: adc-keys: allow linux,input-type property

[Nicolas Frattaroli]

On Wednesday, 17 December 2025 09:31:15 Central European Standard Time Krzysztof Kozlowski wrote:
> On Mon, Dec 15, 2025 at 01:29:29PM +0100, Nicolas Frattaroli wrote:
> > adc-keys, unlike gpio-keys, does not allow linux,input-type as a valid
> > property. This makes it impossible to model devices that have ADC inputs
> > that should generate switch events.
> 
> The solution is to use unevaluatedProps instead, which also allows
> dropping other properties.
> 
> Best regards,
> Krzysztof
> 
> 

Hi Krzysztof,

to understand the motivation behind this suggestion correctly:
are the "linux," vendor prefixed properties, especially with regards
to key codes, generally a bit of a thorn in the side of DT bindings
maintainers?

I'd imagine so since they technically tie the DT to a specific OS
kernel (though of course, others are free to translate those key
codes). And the whole idea of configuring which code is emitted
from something is basically abusing DT for configuring software
rather than describing hardware.

I'm mainly interested because this is a thought that has been in
the back of my mind for a while now, and I'm curious if the DT
binding maintainers happen to have arrived at the same impassé,
where linux,input-type et al abuse the DT model for something we
would tell any other vendor not to abuse it for, but no better
solution exists right now to achieve the same thing.

Kind regards,
Nicolas Frattaroli

Re: [PATCH v2 1/4] dt-bindings: input: adc-keys: allow linux,input-type property

[Rob Herring]

On Wed, Dec 17, 2025 at 01:57:46PM +0100, Nicolas Frattaroli wrote:
> On Wednesday, 17 December 2025 09:31:15 Central European Standard Time Krzysztof Kozlowski wrote:
> > On Mon, Dec 15, 2025 at 01:29:29PM +0100, Nicolas Frattaroli wrote:
> > > adc-keys, unlike gpio-keys, does not allow linux,input-type as a valid
> > > property. This makes it impossible to model devices that have ADC inputs
> > > that should generate switch events.
> > 
> > The solution is to use unevaluatedProps instead, which also allows
> > dropping other properties.
> > 
> > Best regards,
> > Krzysztof
> > 
> > 
> 
> Hi Krzysztof,
> 
> to understand the motivation behind this suggestion correctly:
> are the "linux," vendor prefixed properties, especially with regards
> to key codes, generally a bit of a thorn in the side of DT bindings
> maintainers?

Not really. Most have existed for decades. New ones get extra scrutiny 
and often end up dropping the linux prefix.

> I'd imagine so since they technically tie the DT to a specific OS
> kernel (though of course, others are free to translate those key
> codes). And the whole idea of configuring which code is emitted
> from something is basically abusing DT for configuring software
> rather than describing hardware.
> 
> I'm mainly interested because this is a thought that has been in
> the back of my mind for a while now, and I'm curious if the DT
> binding maintainers happen to have arrived at the same impassé,
> where linux,input-type et al abuse the DT model for something we
> would tell any other vendor not to abuse it for, but no better
> solution exists right now to achieve the same thing.

Not sure what the BSDs do here. It's never come up that I remember. Best 
I can tell is they just make it a userspace problem. So every possible 
keyboard needs a keymap file. Though I'm not sure how that would work 
with GPIO keys as you don't really have a scan code.

Rob