Hi Marek,

On Mon, Feb 15, 2021 at 5:31 AM Marek Behun [off-list ref] wrote:

On Sun, 14 Feb 2021 16:45:46 -0800
Roderick Colenbrander [off-list ref] wrote:

quoted

+     led_cdev->name = devm_kasprintf(&hdev->dev, GFP_KERNEL, "playstation::%pMR::rgb",
+                     ps_dev->mac_address);

...

quoted

+     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.

Different existing functions are defined in
include/dt-bindings/leds/common.h.

BTW there are extended versions of LED registering functions, suffixed
by "_ext". These accept a struct led_init_data. If a fwnode of the LED
is passed to the registering function via this struct, the LED core
will compose a name for the LED itself. But since your LEDs don't have
device-tree nodes because they are on USB/BlueTooth joysticks, you
either have to compose the name itself like your code is doing now, or
you can propose a patch to the LED core, so that LED core will be able
to compose the LED name even without a device-tree node.

JFI, the function part is (in the future) supposed to somehow define LED
trigger which the system will assign to the LED on probe, but this is
not implemented yet. Currently when the LED has a devicetree node,
the trigger is assigned from the `linux,default-trigger` property, but
the idea is to infer it from the `function` property.

Thanks for the info. Might be handy in the future.

What is this RGB LED supposed to do on the joystick? Is it just for
nice colors? Or should it blink somehow? Can the hardware in the
joystick blink the LED itself? Or maybe fade between colors?

I mentioned a bit in the other email. These LEDs are under direct
control from PlayStation games. Some may change the color on a per
video frame basis. Use cases include health (green) and when a
character loses health becomes more red/orange and can start flashing.
I have seen games use this for police car and then mixing between blue
and red. Others use it with a static player ID color. The console side
API is literally raw RGB values. There is no hardware blink support.
The previous controllers had it though.

There is for example the pattern LED trigger which changes the LED
brightness by a defined pattern. I am planning to add multicolor
support to this trigger, because our RGB LED controller can offload
such thing to hardware.

Marek

Thanks,
Roderick