Message-ID: <CAO-hwJK8yGiRpTr9D86r1kB8pWdCT8A8No40t_YQdtiVm9z26Q@mail.gmail.com>
Date:   Thu, 6 Feb 2020 18:45:31 +0100
From:   Benjamin Tissoires <benjamin.tissoires@...hat.com>
To:     Roderick Colenbrander <thunderbird2k@...il.com>
Cc:     Jiri Kosina <jikos@...nel.org>, Martyn Welch <martyn@...chs.me.uk>,
        linux-input <linux-input@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        "Conn O'Griofa" <connogriofa@...il.com>,
        "Colenbrander, Roelof" <roderick.colenbrander@...y.com>
Subject: Re: [PATCH] HID: Sony: Add support for Gasia controllers

Hey,

On Thu, Feb 6, 2020 at 4:31 PM Roderick Colenbrander
<thunderbird2k@...il.com> wrote:
>
> On Thu, Feb 6, 2020 at 12:10 AM Benjamin Tissoires
> <benjamin.tissoires@...hat.com> wrote:
> >
> > Hi,
> >
> > On Mon, Feb 3, 2020 at 12:23 PM Jiri Kosina <jikos@...nel.org> wrote:
> > >
> > > On Mon, 3 Feb 2020, Benjamin Tissoires wrote:
> > >
> > > > I am definitely not in favour of that :(
> > > >
> > > > The basic problem we have here is that some vendors are overriding your
> > > > VID/PIDs, and this is nasty. And I do not see any reasons why you can't
> > > > say: "well, we broke it, sorry, but we only support *our* devices, not
> > > > third party ones".
> > >
> > > Well, it's not about "we broke it" in the first place, as far as I
> > > can tell.
> > >
> > > Roderick's concern is that 3rd party devices with overriden VID/PID
> > > malfunction for completely unrelated reason to (correctly working) changes
> > > done in favor of stock Sony devices, but it'll be Sony receiving all the
> > > reports/blame.
> >
> > After re-reading the code, I am not sure we can easily detect the
> > clones. So at some point, I think we will break them, but there is not
> > much we can do. I don't really have a solution for that :(
> >
> > >
> > > > One thing that comes to my mind (probably not the best solution), is to
> > > > taint the kernel if you are facing a non genuine product. We do that for
> > > > nvidia, and basically, we can say: "well, supporting the nvidia blob is
> > > > done on a best effort case, and see with them directly if you have an
> > > > issue". Tainting the kernel is a little bit rough, but maybe adding an
> > > > info message in the dmesg if you detect one of those can lead to a
> > > > situation were we can count on you for supporting the official products,
> > > > and you can get community support for the clones.
> > >
> > > Yeah; which I wouldn't like to do for upstream kernel, but Sony could
> > > definitely do this for the products they ship.
> > >
> > > The same way distros are tainting their kernels when unsupported modules
> > > (but otherwise perfectly fine wrt. GPL and everything else) are loaded
> > > into distro-supported kernels.
> > >
> > > > One last thing. Roderick, I am not sure if I mentioned that or not, but
> > > > I am heavily adding regression tests for HID in
> > > > https://gitlab.freedesktop.org/libevdev/hid-tools/
> > >
> > > ... and words can't express how thankful I am for that :)
> > >
> >
> > OK, I played with that idea earlier this week:
> > https://gitlab.freedesktop.org/libevdev/hid-tools/merge_requests/74
> > I only have a Sixaxis controller, and I only implemented the USB part
> > of it (AFAICT).
> > Currently this ensures the button mapping is correct, and that the
> > LEDs are working properly.
> > We are still missing a few bits and pieces, but the initialization
> > (requests made by the kernel to start the device and press on the PS
> > button) is handled properly.
> >
> > If this is something Roderick would be interested in, we can then try
> > to extend this initial work on Bluetooth controllers and the DualShock
> > ones.
>
> We can probably help out there (need to ask official permission). We
> have similar tests in Android (still adding more). Just in case you
> are not familiar this is their framework:
> https://android.googlesource.com/platform/cts/+/master/tests/tests/hardware/src/android/hardware/input/cts/tests/

thanks. That's a good pointer I wasn't aware of.

>
> It is a small Java class and then there is a json blob with the actual
> test (forgot where the json is). It defines the report descriptors
> etcetera.

Found them at https://android.googlesource.com/platform/cts/+/master/tests/tests/hardware/res/raw

Of course, I had to find advantages to my own test suite (in case you
need to explain to management):
- I am running it upstream on any patch that comes in, so less chances
to catch a failure after the fact
- I am emulating the firmware more precisely IMO (it's a python class
and you can overwrite the set_report, get_report and set_output
report)
- I am emulating both USB and Bluetooth (or whatever bus you want)
- I am testing the LED classes
- we can easily extend to test the rumbles and the battery reporting
- I am not relying on preformatted reports, meaning that it's harder
to cheat in the driver and we can extend the test cases more easily
(what if we have a left d-pad + button 7 that runs into a problem in
the driver?)

Anyway, I just merged the PS3 controller I have. I'll try to see if I
can get the DS4 working based on those json files.

Cheers,
Benjamin

>
> Thanks,
> Roderick
>
> > Adding the clones ones based on the current kernel code is something
> > doable, but I do not expect Sony to be involved in that process.
> >
> > That being said, before we merge this particular patch about Gasia
> > controllers, now we need to implement a regression test first :)
> >
> > Cheers,
> > Benjamin
> >
>

Powered by blists - more mailing lists