[v1,0/7] HID: playstation: DS4: LED bugfix, third-party gamepad support

  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
  

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,
  

On Tue, Jan 23, 2024 at 1:51 AM Jiri Kosina <jikos@kernel.org> wrote:
>
> 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
  

On 1/25/24 07:24, Roderick Colenbrander wrote:
> Sorry for the late reply. I had glanced over them, but didn't have an
> opportunity for a detailed review yet.

No worries, thank you for giving the patches a chance.


> 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).

I understand this very well, which is why I hope I've kept the patches small enough to be alright to follow. If you have a bad feeling about something in particular, please let me know, and maybe we can find a better solution or alleviate concerns.

On my end, I am working (currently as a hobby) on an appliance that aims to support as many controllers as possible, hence I know the choices you face in order to keep user expectations in check, and user experience consistent. See my comment on patch 7/7 - what if someone uses a third-party controller that happens to work by chance, and a kernel update breaks stuff?

So I am with you on focusing on the original devices, which have predictable behaviour and quality. That said, hopefully some or all of these patches are trivial enough to be included upstream. I feel that it's beautiful to plug random stuff into a Linux box and to find that it just works :)


> 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).

I've been wondering the same, but without a PS4 of my own, I didn't try such controllers on a real console. That would be interesting indeed!


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

I don't have all devices I've ever tested, but I do have at least one reproducer for each patch in this series.



Thanks,

Max