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

[Marek Behun <marek.behun@nic.cz>]

On Mon, 15 Feb 2021 07:36:58 -0800
Roderick Colenbrander <roderick@gaikai.com> wrote:

> Hi Marek,
> 
> On Mon, Feb 15, 2021 at 5:31 AM Marek Behun <marek.behun@nic.cz> wrote:
> >
> > On Sun, 14 Feb 2021 16:45:46 -0800
> > Roderick Colenbrander <roderick@gaikai.com> 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".

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

Marek