linux-kernel - Re: [PATCH v2 02/11] Input: add FF

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMCVhVNb5N+KR-8+pSixKhH7SAOcbO7o1ewv7NYY3JB4gNn3ZA@mail.gmail.com>
Date: Mon, 11 Aug 2025 10:26:57 -0500
From: Jonathan Denose <jdenose@...gle.com>
To: tomasz.pakula.oficjalny@...il.com
Cc: Jiri Kosina <jikos@...nel.org>, Benjamin Tissoires <bentiss@...nel.org>, 
	Dmitry Torokhov <dmitry.torokhov@...il.com>, Jonathan Corbet <corbet@....net>, 
	Henrik Rydberg <rydberg@...math.org>, linux-input@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org, 
	Angela Czubak <aczubak@...gle.com>, "Sean O'Brien" <seobrien@...gle.com>
Subject: Re: [PATCH v2 02/11] Input: add FF_HID effect type

On Mon, Aug 4, 2025 at 3:34 PM <tomasz.pakula.oficjalny@...il.com> wrote:
>
> On Mon, 2025-08-04 at 14:11 +0000, Jonathan Denose wrote:
> > From: Angela Czubak <aczubak@...gle.com>
> >
> > FF_HID effect type can be used to trigger haptic feedback with HID simple
> > haptic usages.
> >
> > Signed-off-by: Angela Czubak <aczubak@...gle.com>
> > Co-developed-by: Jonathan Denose <jdenose@...gle.com>
> > Signed-off-by: Jonathan Denose <jdenose@...gle.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).

Thanks for your review, your comments make sense to me!

I'll change ff_hid_effect to ff_haptic_effect and FF_HID to FF_HAPTIC
and upload a new version of this series.