Re: [PATCH v10 10/11] input: misc: Add support for MAX7360 rotary

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

On Wed Jul 2, 2025 at 11:05 PM CEST, Dmitry Torokhov wrote:
> Hi Mathieu,
>
> On Fri, May 30, 2025 at 12:00:18PM +0200, Mathieu Dubois-Briand wrote:
>> +
>> +static void max7360_rotaty_report_event(struct max7360_rotary *max7360_rotary, int steps)
>
> s/rotaty/rotary
>
>> +	if (max7360_rotary->relative_axis) {
>> +		input_report_rel(max7360_rotary->input, max7360_rotary->axis, steps);
>> +	} else {
>> +		int pos = max7360_rotary->pos;
>> +		int maxval = max7360_rotary->steps;
>> +
>> +		/*
>> +		 * Add steps to the position.
>> +		 * Make sure added steps are always in ]-maxval; maxval[
>> +		 * interval, so (pos + maxval) is always >= 0.
>> +		 * Then set back pos to the [0; maxval[ interval.
>> +		 */
>> +		pos += steps % maxval;
>> +		if (max7360_rotary->rollover)
>> +			pos = (pos + maxval) % maxval;
>> +		else
>> +			pos = clamp(pos, 0, maxval);
>
> Should it be clamp(pos, 0, maxval - 1) by chance?
>

Comparing with the behaviour of rotary_encoder.c, yes, maxval should
probably be excluded.

Good catch, thanks!

> Otherwise:
>
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> Thanks.

Thanks for the review!

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