[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fb8850c6c1766b4360a69419845aa8bf7a3aa7a6.camel@welchs.me.uk>
Date: Tue, 28 Jan 2020 18:47:35 +0000
From: Martyn Welch <martyn@...chs.me.uk>
To: Roderick Colenbrander <thunderbird2k@...il.com>
Cc: Jiri Kosina <jikos@...nel.org>,
Benjamin Tissoires <benjamin.tissoires@...hat.com>,
linux-input <linux-input@...r.kernel.org>,
lkml <linux-kernel@...r.kernel.org>,
Conn O'Griofa <connogriofa@...il.com>,
"Colenbrander, Roelof" <roderick.colenbrander@...y.com>
Subject: Re: [PATCH] HID: Sony: Add support for Gasia controllers
On Mon, 2020-01-27 at 14:07 -0800, Roderick Colenbrander wrote:
> 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
> Thanks,
> Roderick
>
> On Sun, Jan 26, 2020 at 12:28 PM Martyn Welch <martyn@...chs.me.uk>
> wrote:
> > 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 <connogriofa@...il.com>
> > Signed-off-by: Martyn Welch <martyn@...chs.me.uk>
> > ---
> > 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-
> > >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-
> > >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(....)
>
> > +
> > + 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
> >
Powered by blists - more mailing lists