Message-ID: <4olssbvj6iap42kqycdnvpibiufemz5hhwjw65aj3qqeuzrib5@467sqzfl53vt>
Date: Mon, 22 Jul 2024 18:46:32 +0200
From: Benjamin Tissoires <bentiss@...nel.org>
To: Max Staudt <max@...as.org>
Cc: Roderick Colenbrander <thunderbird2k@...il.com>, 
	Jiri Kosina <jikos@...nel.org>, Benjamin Tissoires <benjamin.tissoires@...hat.com>, 
	Roderick Colenbrander <roderick.colenbrander@...y.com>, linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] hid-playstation: DS4: Update rumble and lightbar
 together

On Jul 21 2024, Max Staudt wrote:
> On 7/17/24 09:26, Roderick Colenbrander wrote:
> > > If not, then maybe there is protocol documentation that could help
> > > test writers in creating more precise tests?
> > 
> > Unfortunately none of the documentation for our controllers is public.
> > Just read between the lines in the code, which we cover with some clues
> > here and there :)
> 
> I've seen the clues and they are certainly an improvement of
> hid-playstation over hid-sony that I am grateful for :)
> 
> As for the documentation, I didn't mean that you should open it to the
> wide public (although that would be fabulous!). Rather, I suggested
> providing what's needed to the people writing the downstream tests that
> we've been going on about for a while now.
> 
> The utility of tests is... rather limited, if they don't verify an
> actual specification, but just a specific code's behaviour. After all,
> these tests would fail even when talking to a real PlayStation 4.
> 
> 
> > > > (The official Android tests are kind of kernel version agnostic
> > > > as they work across multiple kernel and Android versions.
> > > 
> > > Hm, sounds to me like the Android test framework is broken if it
> > > cannot be kernel-specific in such cases. What's required in order to
> > > improve this?
> > 
> > It is a bit of a long process we work on with Google. Some initial
> > fixing of some of the bugs will be for this year to make sure the 6.6
> > tests pass properly. But any work to maybe handle multiple behaviors,
> > that's quite tricky and would take quite a bit of time to be honest.
> 
> Sure, time is not the issue, as long as there is a clear light of
> correct software at the end of the tunnel :)
> 
> Are these tests open source?
> 
> Can kernel patchers run the tests easily (without installing a full
> Android SDK) and see which tests their patches would break?
> 
> 
> > Considering how widely Android and all these devices are used, I'm
> > hesitant to make changes to not cause regressions. Even just a simple
> > one can take forever to trickle down.
> 
> Apologies, I am still confused.
> 
> With v6.2, hid-playstation gained DS4 support. With v6.3, hid-sony lost
> DS4 support.
> 
> DS4 support was removed from hid-sony just one minor release after it
> was included in hid-playstation. I didn't see any hesitation there,
> despite the wire protocol seeing significant changes compared to
> hid-sony, which had been deployed for almost a decade.
> 
> Given how quickly the DS4 code was removed from hid-sony, why are fixes
> suddenly difficult, less than two years into the hid-playstation era, after
> around 8 years of DS4 support in hid-sony?

Ouch, that is worrisome.

> 
> 
> The change from hid-sony to hid-playstation caused a regression on a
> real-world device, on Android phones:
> 
> I have an 8BitDo controller that quirkily emulates a DS4, and which
> worked fine with my old Android phone, with a kernel from the hid-sony
> days. With v6.3, this controller broke, and this went into LTS v6.6.
> 
> Does this kind of regression, which has clearly spread across the Android
> ecosystem with v6.6, deserve to be fixed?

it does, yeah.

> 
> 
> As a generalised question, how about controllers that work on Android phones
> with kernel v6.1 (hid-sony), but not with v6.6 (hid-playstation), because of
> protocol changes that don't affect first-party controllers. Do we accept
> upstream regressions on actual, physical devices for the sake of passing
> downstream tests?

The upstream rule is simple: no regressions (that we know about). The
argument that downstream tests are hard to do is not correct for
upstream. As a general rule of thumb, upstream doesn't care about
downstream at all. Regressions is all we care, and a bad test from
downstream is not a correct justification to reject a fix upstream.

Please understand Roderick that I am not taking side. I perfectly
understand the downstream challenges, but we can not refuse a regression
fix because the new patch breaks a downstream test.

I followed the thread and Max seemed to be OK with waiting and I assumed
it was not critical. But if we know about a regression in a device we
supported, we should find a solution for it.

BTW, that's the reason why I finally managed to push the hid-tools tests
in the seftests dir of the kernel directly. Now each kernel has its own
set of tests, and there is no more discrepancies between tests and
regressions that are found.

Cheers,
Benjamin

> 
> 
> 
> Max

Powered by blists - more mailing lists