| Message ID | 20240115144538.12018-3-max@enpas.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | HID: playstation: DS4: LED bugfix, third-party gamepad support | expand |
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
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);
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(-)