Re: [PATCH v3 6/7] input: misc: Add support for MAX7360 rotary

["Mathieu Dubois-Briand" <mathieu.dubois-briand@bootlin.com>]

On Mon Jan 13, 2025 at 1:42 PM CET, Mathieu Dubois-Briand wrote:
> Add driver for Maxim Integrated MAX7360 rotary encoder controller,
> supporting a single rotary switch.
>
> Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
> ---
...
> +static irqreturn_t max7360_rotary_irq(int irq, void *data)
> +{
> +	struct max7360_rotary *max7360_rotary = data;
> +	int val;
> +	int ret;
> +
> +	ret = regmap_read(max7360_rotary->regmap, MAX7360_REG_RTR_CNT, &val);
> +	if (ret < 0) {
> +		dev_err(&max7360_rotary->input->dev,
> +			"Failed to read rotary counter\n");
> +		return IRQ_NONE;
> +	}
> +
> +	if (val == 0) {
> +		dev_dbg(&max7360_rotary->input->dev,
> +			"Got a spurious interrupt\n");
> +
> +		return IRQ_NONE;
> +	}
> +
> +	input_report_rel(max7360_rotary->input, max7360_rotary->axis,
> +			 (int8_t)val);
> +	input_sync(max7360_rotary->input);
> +

I have a question about the type of events reported here.

I used rotary_encoder.c as a reference, so I'm reporting some EV_REL
events on a given axis, such as REL_X.

On the other hand, I know there is an out-of-tree version of this
driver that is instead reporting key events, such as KEY_DOWN or KEY_UP.
I also know there are existing applications that do rely on this
behaviour.

So my question is, what is the best kind of events to report here ?
Is there any guideline that do apply here? Should I better try to mimic
the behaviour of the existing out-of-tree driver, or should I mimic the
behaviour of rotary_encoder.c, so we have a similar behaviour for all
in-kernel rotary encoder drivers?

> +	return IRQ_HANDLED;
> +}
> +

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com