Re: [PATCH v2 02/11] Input: add FF_HID effect type

[tomasz.pakula.oficjalny@gmail.com]

On Mon, 2025-08-04 at 14:11 +0000, Jonathan Denose wrote:
> From: Angela Czubak <aczubak@google.com>
> 
> FF_HID effect type can be used to trigger haptic feedback with HID simple
> haptic usages.
> 
> Signed-off-by: Angela Czubak <aczubak@google.com>
> Co-developed-by: Jonathan Denose <jdenose@google.com>
> Signed-off-by: Jonathan Denose <jdenose@google.com>
> ---
>  include/uapi/linux/input.h | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index 2557eb7b056178b2b8be98d9cea855eba1bd5aaf..3ea7c826c6fb2034e46f95cb95b84ef6f5b866df 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -428,6 +428,24 @@ struct ff_rumble_effect {
>  	__u16 weak_magnitude;
>  };
>  
> +/**
> + * struct ff_hid_effect
> + * @hid_usage: hid_usage according to Haptics page (WAVEFORM_CLICK, etc.)
> + * @vendor_id: the waveform vendor ID if hid_usage is in the vendor-defined range
> + * @vendor_waveform_page: the vendor waveform page if hid_usage is in the vendor-defined range
> + * @intensity: strength of the effect as percentage
> + * @repeat_count: number of times to retrigger effect
> + * @retrigger_period: time before effect is retriggered (in ms)
> + */
> +struct ff_hid_effect {
> +	__u16 hid_usage;
> +	__u16 vendor_id;
> +	__u8  vendor_waveform_page;
> +	__u16 intensity;
> +	__u16 repeat_count;
> +	__u16 retrigger_period;
> +};

Wouldn't it make more sense to call this new effect ff_haptic_effect?
hid_effect sound generic, too generic. One could say, all ff effect are
hid effects because most ff apis (linux' included) are based on USB PID
spec.

> +
>  /**
>   * struct ff_effect - defines force feedback effect
>   * @type: type of the effect (FF_CONSTANT, FF_PERIODIC, FF_RAMP, FF_SPRING,
> @@ -464,6 +482,7 @@ struct ff_effect {
>  		struct ff_periodic_effect periodic;
>  		struct ff_condition_effect condition[2]; /* One for each axis */
>  		struct ff_rumble_effect rumble;
> +		struct ff_hid_effect hid;
>  	} u;
>  };
>  
> @@ -471,6 +490,7 @@ struct ff_effect {
>   * Force feedback effect types
>   */
>  
> +#define FF_HID		0x4f

Again here, FF_HID sounds confusing without having the broader context.
Constant, Sine, Inertia, Spring are way more descriptive. FF_HAPTIC
would be a great name to distinguish such an effect. Or maybe FF_TACTILE
with ff_tactile_effect?

>  #define FF_RUMBLE	0x50
>  #define FF_PERIODIC	0x51
>  #define FF_CONSTANT	0x52
> @@ -480,7 +500,7 @@ struct ff_effect {
>  #define FF_INERTIA	0x56
>  #define FF_RAMP		0x57
>  
> -#define FF_EFFECT_MIN	FF_RUMBLE
> +#define FF_EFFECT_MIN	FF_HID
>  #define FF_EFFECT_MAX	FF_RAMP
>  
>  /*

Overall, I'll keep an eye on this as I'm slowly working towards a
proposal for a revamped and extended ff api on Linux that would make it
fully featured (we're lacking things like device control and querying
effects and their status, arbitrary number of axes and arbitrary axes
themselves).