Re: [PATCH] HID: ayaneo: Add AYANEO 3 detachable controller driver

[Matías Martínez]

> They are eyeballed timings that worked during my testing. Because I
> operate in userspace I have the freedom to choose whatever timings I
> want and change them whenever I want. Carrying them to the kernel
> freezes them for the vendor device and there is a higher level of
> scrutiny required before they are merged.

To be precise about what would actually be frozen: the ABI is
"writing 'left' to eject returns once the firmware confirms the
release". The poll cadence and the timeout behind that are
implementation details, so they can be retuned in-kernel later
without breaking userspace. Your larger point stands though: they are
eyeballed numbers, they now sit under kernel scrutiny, and I am the
one signing up to maintain them.

> You also only carry part of the policy. Userspace still has to
> coordinate between the calls to the EC so that is left to userspace.
> I am pretty sure I have a lot of timing quirks there as well. So now
> you have a split policy

The split I ended up with is a bit cleaner than that: the kernel
absorbed the timed parts of the protocol (reply matching, retries,
the eject handshake polling), and what remains in userspace is
ordered rather than timed -- wait for the blocking eject write to
return, then cut controller_power. The UI I tested against has no
timing loops left in its eject path. Whether that split carries its
weight for a niche device is exactly the scope question, and I am
happy to follow the HID maintainers' call on it -- including trimming
the driver to the LED plus module identification and leaving
eject/reset to userspace over hidraw, if that is where they land.

> I did not disagree on the RGB part, that's a decent addition barring
> timing quirks being needed and it should be relatively easy to
> upstream.

Good to hear. No timing quirks on that path: the config command is a
single write with a reply echo, and solid/breathing/off all worked
first try on hardware without settle delays.

> I would advise some caution, because e.g., I noticed hid-oxp got
> upstreamed using a global drvdata table even though it is a HID
> driver and there are actually multiple OneXPlayer models that carry
> both hid devices and now they will potentially have their kernel
> memory corrupted.

Thanks for the pointer -- I went and checked. hid-ayaneo keeps all
state in a per-device struct (devm-allocated, reached through
hid_get_drvdata); the only file-scope objects are const tables, so
multiple bound instances each get their own state.

> Give it a few days before sending a V3, others should leave feedback
> as well. You sent V2 a bit too fast.

That is fair -- v3 will wait until the thread has settled and the
maintainers have had a chance to weigh in.

Thanks, this was a useful mail.

Matías