On Tue, Jan 28, 2020 at 10:47 AM Martyn Welch [off-list ref] wrote:

On Mon, 2020-01-27 at 14:07 -0800, Roderick Colenbrander wrote:

quoted

Hi Martyn,

Thanks for sharing your patch. Though I must say with my Sony hat on,
I don't really like supporting clone devices (they hijack our device
ids.. etcetera) and we support hid-sony in an official capacity now
across various devices. Though thischange  all relates to PS3
generation, which is not that important anymore so it shouldn't
matter
that much.

Hi Roderick,

I can understand that sentiment to a degree, however the hardware
exists and there are no doubt others like me that don't even own a PS3
and would just like to be able to use the gamepads they've purchased to
play games.

Martyn

Let me explain the situation a little bit better from our angle. These
devices exist and from the Linux community perspective of course they
should see some level of support. And as I said since this is PS3
generation I don't have much of a concern.

Where it becomes tricky for any company in our situation is the
support side. We don't know these devices and don't have access to
datasheets or anything, but when such code is in our "official driver"
it means we have to involve them in our QA process and support them in
some manner (kind of legitimizing their existence as well). We now
support this driver in a large capacity (pretty much all mobile
devices will start shipping it) it puts challenges on our partners
(not a big issue since just PS3 right now).

As you can see this creates an awkward situation. I'm sure there other
such devices as well e.g. counterfeit Logitech keyboards, USB serial
adapters and other periperhals with similar challenges. In an ideal
world the support would live in another driver, but since in case of
this "fake" PS3 controller it "share" our product / vendor ids it is
tricky. At this point there is not a strong enough case yet to augment
the "hid-quirks" to do so, but perhaps if it became a serious issue
(e.g. for newer controllers) maybe we need to think of something.

Thanks,
Roderick

quoted

Thanks,
Roderick

On Sun, Jan 26, 2020 at 12:28 PM Martyn Welch [off-list ref]
wrote:

quoted

There seems to be a number of subtly different firmwares for the
Playstation controllers made by "Gasia Co.,Ltd". Whilst such
controllers
are easily detectable when attached via USB that is not always the
case
via Bluetooth. Some controllers are named "PLAYSTATION(R)3
Controller"
where as the official Sony controllers are named
"Sony PLAYSTATION(R)3 Controller", however some versions of
firmware use
the exact name used by the official controllers. The only way I've
been
able to distinguish these versions of the controller (when
connected via
Bluetooth) is that the Bluetooth Class of Device incorrectly
reports the
controller as a keyboard rather than a gamepad. I've so far failed
to work
out how to access this information from a HID driver.

The Gasia controllers need output reports to be configured in the
same way
as the Shanwan controllers. In order to ensure both types of Gasia
firmware
will work, this patch adds a quirk for those devices it can detect
and
reworks `sixaxis_send_output_report()` to attempt
`hid_hw_output_report()`
should `hid_hw_raw_request()` be known to be the wrong option (as
is the
case with the Shanwan controllers) or fails.

This has got all the controllers I have working, with the slight
anoyance that the Gasia controllers that don't currently get marked
with
a quirk require the call to `hid_hw_raw_request()` to fail before
the
controller finishes initialising (which adds a significant extra
delay
before the controller is ready).

This patch is based on the following patch by Conn O'Griofa:

https://github.com/RetroPie/RetroPie-Setup/pull/2263/commits/017f00f6e15f04b3272ff1abae8742dc4c47b608

Cc: Conn O'Griofa <redacted>
Signed-off-by: Martyn Welch <redacted>
---
 drivers/hid/hid-sony.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 4c6ed6ef31f1..d1088a85cb59 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c

@@ -56,6 +56,7 @@
 #define NSG_MR5U_REMOTE_BT        BIT(14)
 #define NSG_MR7U_REMOTE_BT        BIT(15)
 #define SHANWAN_GAMEPAD           BIT(16)
+#define GASIA_GAMEPAD             BIT(17)

 #define SIXAXIS_CONTROLLER (SIXAXIS_CONTROLLER_USB |

SIXAXIS_CONTROLLER_BT)
 #define MOTION_CONTROLLER (MOTION_CONTROLLER_USB |
MOTION_CONTROLLER_BT)

@@ -2067,6 +2068,7 @@ static void sixaxis_send_output_report(struct

sony_sc *sc)
        struct sixaxis_output_report *report =
                (struct sixaxis_output_report *)sc-

quoted

output_report_dmabuf;

        int n;
+       int ret = -1;

        /* Initialize the report with default values */
        memcpy(report, &default_report, sizeof(struct
sixaxis_output_report));

@@ -2101,14 +2103,23 @@ static void

sixaxis_send_output_report(struct sony_sc *sc)
                }
        }

-       /* SHANWAN controllers require output reports via intr
channel */
-       if (sc->quirks & SHANWAN_GAMEPAD)
-               hid_hw_output_report(sc->hdev, (u8 *)report,
-                               sizeof(struct
sixaxis_output_report));
-       else
-               hid_hw_raw_request(sc->hdev, report->report_id, (u8
*)report,
+       /*
+        * SHANWAN & GASIA controllers require output reports via
intr channel.
+        * Some of the Gasia controllers are basically
indistinguishable from
+        * the official ones and thus try hid_hw_output_report()
should
+        * hid_hw_raw_request() fail.
+        */
+       if (!(sc->quirks & (SHANWAN_GAMEPAD | GASIA_GAMEPAD)))
+               ret = hid_hw_raw_request(sc->hdev, report-

quoted

report_id,

+                               (u8 *)report,
                                sizeof(struct
sixaxis_output_report),
                                HID_OUTPUT_REPORT,
HID_REQ_SET_REPORT);
+
+       if (ret >= 0)
+               return;

I don't like this pattern so much. We only need this "ret" check for
SHANWAN and GASIA. I would just do:

if (!(sc->quirks & (SHANWAN_GAMEPAD | GASIA_GAMEPAD))) {
    int ret = hid_hw_raw_request(....);
    if (ret >= 0)
       return;
}

hid_hw_output_report(....)

quoted

+
+       hid_hw_output_report(sc->hdev, (u8 *)report,
+                       sizeof(struct sixaxis_output_report));
 }

 static void dualshock4_send_output_report(struct sony_sc *sc)

@@ -2833,6 +2844,14 @@ static int sony_probe(struct hid_device

*hdev, const struct hid_device_id *id)
        if (!strcmp(hdev->name, "SHANWAN PS3 GamePad"))
                quirks |= SHANWAN_GAMEPAD;

+       /*
+        * Some Gasia controllers are named "PLAYSTATION(R)3
Controller"
+        * where as the official Sony controllers are named
+        * "Sony PLAYSTATION(R)3 Controller".
+        */
+       if (!strcmp(hdev->name, "PLAYSTATION(R)3 Controller"))
+               quirks |= GASIA_GAMEPAD;
+
        sc = devm_kzalloc(&hdev->dev, sizeof(*sc), GFP_KERNEL);
        if (sc == NULL) {
                hid_err(hdev, "can't alloc sony descriptor\n");
--
2.20.1