Re: [PATCH v6 1/4] HID: playstation: add DualSense lightbar support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Feb 15, 2021 at 7:55 AM Marek Behun <marek.behun@xxxxxx> wrote:
>
> On Mon, 15 Feb 2021 07:36:58 -0800
> Roderick Colenbrander <roderick@xxxxxxxxxx> wrote:
>
> > Hi Marek,
> >
> > On Mon, Feb 15, 2021 at 5:31 AM Marek Behun <marek.behun@xxxxxx> wrote:
> > >
> > > On Sun, 14 Feb 2021 16:45:46 -0800
> > > Roderick Colenbrander <roderick@xxxxxxxxxx> wrote:
> > >
> > > > +     led_cdev->name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "playstation::%pMR::rgb",
> > > > +                     ps_dev->mac_address);
> > > ...
> > > > +     ret = devm_led_classdev_multicolor_register(&hdev->dev, lightbar_mc_dev);
> > >
> > > The LED subsystem has a predefined schema by which LED names should
> > > look like:
> > >   devicename:color:function
> > > (Not all fields are required, but the order must be preserved. The ':'
> > >  character should be used only as separator of these fields, so not MAC
> > >  addresses in these names, it will confuse userspace parsers.)
> > > See Documentation/leds/leds-class.rst
> > >
> > > The devicename part should not be "playstation". It should be something
> > > otherwise recognizable from userspace. For example an mmc indicator has
> > > devicename "mmc0", keyboard capslock LED can have devicename "input0"...
> > >
> > > In your case the name should be something like:
> > >   input3:rgb:indicator
> >
> > Naming is a little bit tricky. The LEDs as well as other sysfs nodes
> > are added to the 'parent' HID device, not the input devices. In case
> > of DualSense it is actually implemented as a composite device with
> > mulitple input devices (gamepad, touchpad and motion sensors) per HID
> > device. The device name of HID devices seems to be something like:
> > <bus>:<vendor_id>:<product_id>:<some other id> e.g. for DualSense USB
> > 0003:054C:0CE6.0029 or Bluetooth 0005:054C:0CE6.002B
> >
> > This is I guess why many HID devices in general pick their own names
> > (and not all have need to have input devices I guess). Though Benjamin
> > and Jiri know better.
> >
> > I'm not sure what naming could make sense here. The previous Sony
> > driver for PlayStation devices used: HID_name "::red" for e.g. red LED
> > on DualShock 4.
>
> We have to find a reasonable devicename here. If each joystick registers
> multiple input devices, it cannot be "input%d". I suppose there isn't
> an API for grouping mulitple input devices toghether into inputgroups.
> Maybe it could be in the format "joystick%d".

Yeah, there is no inputgroups mechanism.  It could use some type of
joystick name if that's what desired. However, there is no common ID
code. Individual drivers are sometimes calculating their own IDs
(hid-nintendo, hid-sony, hid-playstation and xpad I think). At least
for hid-sony/hid-playstation the use case for tracking IDs is for a
part to prevent duplicate devices as you can connect your device using
both bluetooth and USB. So would be "ps-joystick0"

At the HID layer there does seem to be a unique ID, but it is only
exposed in the name string: This is how the name is constructed:
     dev_set_name(&hdev->dev, "%04X:%04X:%04X.%04X", hdev->bus,
             hdev->vendor, hdev->product, atomic_inc_return(&id));

This ID is HID specific, but not all input devices use HID.

I'm not entirely sure what makes sense...


> But I don't think it can be "playstation". Nor can MAC addresses be
> there if they contain ':'s.
>
> Marek



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux