[v1,2/7] HID: playstation: DS4: Don't fail on MAC address request

  Some third-party controllers can't report their MAC address.

Since a unique ID is needed for ps_devices_list_add() and
ps_device_register_battery(), let's use hdev->id for this when we don't
have a MAC address.

Signed-off-by: Max Staudt <max@enpas.org>
---
 drivers/hid/hid-playstation.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
  

On Mon, Jan 15, 2024 at 6:58 AM Max Staudt <max@enpas.org> wrote:
>
> Some third-party controllers can't report their MAC address.
>
> Since a unique ID is needed for ps_devices_list_add() and
> ps_device_register_battery(), let's use hdev->id for this when we don't
> have a MAC address.
>
> Signed-off-by: Max Staudt <max@enpas.org>
> ---
>  drivers/hid/hid-playstation.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
> index 7f50e13601f0..0a3c442af305 100644
> --- a/drivers/hid/hid-playstation.c
> +++ b/drivers/hid/hid-playstation.c
> @@ -1966,7 +1966,10 @@ static int dualshock4_get_mac_address(struct dualshock4 *ds4)
>                                 DS4_FEATURE_REPORT_PAIRING_INFO_SIZE, false);
>                 if (ret) {
>                         hid_err(hdev, "Failed to retrieve DualShock4 pairing info: %d\n", ret);
> -                       goto err_free;
> +                       hid_err(hdev, "Generating fake MAC address for this device.\n");
> +                       buf[1] = (hdev->id >>  0) & 0xff;
> +                       buf[2] = (hdev->id >>  8) & 0xff;
> +                       buf[3] = (hdev->id >> 16) & 0xff;
>                 }
>
>                 memcpy(ds4->base.mac_address, &buf[1], sizeof(ds4->base.mac_address));
> @@ -1986,7 +1989,6 @@ static int dualshock4_get_mac_address(struct dualshock4 *ds4)
>                 return 0;
>         }
>
> -err_free:
>         kfree(buf);
>         return ret;
>  }
> @@ -2552,7 +2554,7 @@ static struct ps_device *dualshock4_create(struct hid_device *hdev)
>         ret = dualshock4_get_mac_address(ds4);
>         if (ret) {
>                 hid_err(hdev, "Failed to get MAC address from DualShock4\n");
> -               return ERR_PTR(ret);
> +               hid_err(hdev, "Can't detect simultaneous USB/BT connections from this device.\n");
>         }
>         snprintf(hdev->uniq, sizeof(hdev->uniq), "%pMR", ds4->base.mac_address);
>
> --
> 2.39.2
>
>

Hi Max,

For what type of devices is this not working? This one example of this
request which is very foundational for a controller working on even
the game console. Are this perhaps USB-only devices? If the case maybe
some kind of error is only needed for USB connections.

Thanks,
Roderick
  

On 1/25/24 09:39, Roderick Colenbrander wrote:
> On Mon, Jan 15, 2024 at 6:58 AM Max Staudt <max@enpas.org> wrote:
>>
>> Some third-party controllers can't report their MAC address.
>>
> For what type of devices is this not working? This one example of this
> request which is very foundational for a controller working on even
> the game console. Are this perhaps USB-only devices? If the case maybe
> some kind of error is only needed for USB connections.

IIRC I've only seen this quirk with the oddball VID/PID 7545:0104 that my patch 7/7 adds. It is indeed a USB-only controller. I have not tried this device (or any, really) with an actual console, my focus is just on Linux. Admittedly, I'd also like to know what happens on a real PS4 :)

My intention was to keep the error message and the solution as universal as possible, in case a controller comes along that has both this quirk *and* Bluetooth. It's a bit of an ugly workaround though - if you (or anyone else) have an idea for a nicer solution, I'd be really glad.


Max
  

On Sat, Jan 27, 2024 at 12:51 AM Max Staudt <max@enpas.org> wrote:
>
> On 1/25/24 09:39, Roderick Colenbrander wrote:
> > On Mon, Jan 15, 2024 at 6:58 AM Max Staudt <max@enpas.org> wrote:
> >>
> >> Some third-party controllers can't report their MAC address.
> >>
> > For what type of devices is this not working? This one example of this
> > request which is very foundational for a controller working on even
> > the game console. Are this perhaps USB-only devices? If the case maybe
> > some kind of error is only needed for USB connections.
>
> IIRC I've only seen this quirk with the oddball VID/PID 7545:0104 that my patch 7/7 adds. It is indeed a USB-only controller. I have not tried this device (or any, really) with an actual console, my focus is just on Linux. Admittedly, I'd also like to know what happens on a real PS4 :)
>
> My intention was to keep the error message and the solution as universal as possible, in case a controller comes along that has both this quirk *and* Bluetooth. It's a bit of an ugly workaround though - if you (or anyone else) have an idea for a nicer solution, I'd be really glad.
>
>
> Max
>

I remember on the console side that we support a number of controllers
including our official model and some licensed controllers. I recall
them taking some different codepaths and HID reports differently. It
has been a while, so I don't recall the details. If I remember it
could be that all of the licensed ones were USB-only (of course there
are some Bluetooth capable clones).

For this reason I think not all DS4-compatible devices have a MAC
address or handle this request. (We made some fixes in hid-playstation
relative to hid-sony, where hid-sony used another less known HID
report for the MAC address. Now we use the more commonly known one and
that helped other clone devices).

I'm not sure about the best way to handle this. I have kind of been
leaning towards doing a vid/pid like check for this case even though I
really hate it. It could be within dualshock4_get_mac_address as we do
some other special handling there too (although having the caller of
dualshock4_get_mac_address do it is an option too, but I think within
get_mac_address is slightly nicer for now).

Thanks,
Roderick
  

On 1/31/24 05:59, Roderick Colenbrander wrote:
> I remember on the console side that we support a number of controllers
> including our official model and some licensed controllers. I recall
> them taking some different codepaths and HID reports differently. It
> has been a while, so I don't recall the details. If I remember it
> could be that all of the licensed ones were USB-only (of course there
> are some Bluetooth capable clones).

Now that is an interesting tidbit... if you learn more, I'd be curious to hear about it if possible!


> I'm not sure about the best way to handle this. I have kind of been
> leaning towards doing a vid/pid like check for this case even though I
> really hate it. It could be within dualshock4_get_mac_address as we do
> some other special handling there too (although having the caller of
> dualshock4_get_mac_address do it is an option too, but I think within
> get_mac_address is slightly nicer for now).

As suggested in the 7545:0104 patch, how about dropping this patch as well, until we encounter a device we really want to add and that does not provide a MAC address?


If it's okay for you, I'd send a v2 of everything after hearing your comment on the patch to make controllers work that don't provide gyro calibration data.


Max