Received-SPF: pass (google.com: domain of linux-kernel+bounces-37735-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
MIME-Version: 1.0
References: <20240115144538.12018-1-max@enpas.org> <nycvar.YFH.7.76.2401231047140.29548@cbobk.fhfr.pm>
In-Reply-To: <nycvar.YFH.7.76.2401231047140.29548@cbobk.fhfr.pm>
From: Roderick Colenbrander <thunderbird2k@gmail.com>
Date: Wed, 24 Jan 2024 14:24:22 -0800
Message-ID: <CAEc3jaATA-Z86jh=njR6yctguQBRp1BQEFcBQhYT0rYKcA4f2w@mail.gmail.com>
Subject: Re: [PATCH v1 0/7] HID: playstation: DS4: LED bugfix, third-party
 gamepad support
To: Jiri Kosina <jikos@kernel.org>
Cc: Max Staudt <max@enpas.org>, Roderick Colenbrander <roderick.colenbrander@sony.com>, 
	Benjamin Tissoires <benjamin.tissoires@redhat.com>, linux-input@vger.kernel.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Jan 23, 2024 at 1:51=E2=80=AFAM Jiri Kosina <jikos@kernel.org> wrot=
e:
>
> On Mon, 15 Jan 2024, Max Staudt wrote:
>
> > Dear hid-playstation maintainers,
> >
> > Could you please have a look at the enclosed patches for the DualShock =
4
> > driver in hid-playstation, and upstream them if possible?
> >
> > There is one bugfix, and a few small patches to enable third-party
> > controllers. They sometimes don't implement features that they
> > semantically "don't need", but which currently trip the driver.
> >
> > For example, for the DualShock 4, we don't actually need to know the
> > firmware version in order to work with the gamepad - unlike with the
> > DualSense, which has different driver logic depending on the version.
> >
> > Finally, there are two patches to add a DS4 compatible controller with
> > an unassigned VID/PID - I'd appreciate your thoughts on that.
> >
> > If I can make it easier to upstream these patches, please let me know.
> >
> > Thanks!
> >
> > Max
> >
> > Patches in this series:
> >   [PATCH v1 1/7] HID: playstation: DS4: Fix LED blinking
> >   [PATCH v1 2/7] HID: playstation: DS4: Don't fail on MAC address
> >   [PATCH v1 3/7] HID: playstation: DS4: Don't fail on FW/HW version
> >   [PATCH v1 4/7] HID: playstation: DS4: Don't fail on calibration data
> >   [PATCH v1 5/7] HID: playstation: DS4: Parse minimal report 0x01
> >   [PATCH v1 6/7] HID: playstation: Simplify device type ID
> >   [PATCH v1 7/7] HID: playstation: DS4: Add VID/PID for SZ-MYPOWER
>
> Roderick, any word on this series, please?
>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>
>

Sorry for the late reply. I had glanced over them, but didn't have an
opportunity for a detailed review yet.

I will have some input (there was a goto I remember not being needed).
My general fear is a balance between supporting clone devices vs
reliability. This driver is heavily used in devices (phones, tablets,
TVs, cars). There have been bug reports in the past and just getting
the fixes downstream takes a lot of time (e.g. Android devices).

One of the key things I really would like to see enhanced are the unit
tests (hid-tools / kernel side now). To really make sure we emulate
behavior of these other devices well. The tricky part is that they
don't always support all the HID requests of the real device (which is
weird as the game console does use those HID reports and others and I
don't know how it would have worked there).

That's in general the key feedback about the tests. A question for
Max: do you have access to all the devices being added?

Thanks,
Roderick