Thread (31 messages) 31 messages, 3 authors, 2021-02-16

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

From: Marek Behun <hidden>
Date: 2021-02-16 17:57:05
Also in: linux-input

On Tue, 16 Feb 2021 18:29:46 +0100
Benjamin Tissoires [off-list ref] wrote:
quoted
So all HIDs can be uniqely determined via this atomic_inc_return(&id),
but it is only stored in string form as part of device name.  
Yes and no. This atomic_inc is only used to allow a sysfs tree,
because you can have several HID devices below the same USB, I2C or
UHID physical device. From the userspace, no-one cares about that ID,
because all HID devices are exported as input, IIO or hidraw nodes.

So using this "id" would not allow for a direct mapping HID device ->
sysfs entry because users will still have to walk through the tree to
find out which is which.
So you are saying that the fact that userspace cannot take the number
from "hidN" string and simply do a lookup /sys/bus/hid/devices/hidN is
the problem here.

This is not a problem in my opinion, because userspace can simply
access the parent HID device via /sys/class/leds/hidN:color:func/parent.

In fact we did something similar for LEDs connected to ethernet PHYs.
To summarize:
  - ethernet PHYs are identified by long, sometimes crazy strings like
      d0032004.mdio-mii:01
    or even
      /soc/internal-regs@d0000000/mdio@32004/switch0@10/mdio:08
  - for the purposes of having a sane devicename part in LED names, I
    sent this patch
    https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2301470.html
    which adds a simple incrementing integer ID to each PHY device.
    (The code is not in upstream yet because there is other work needed
     and because I decided that some functionality has to be available
     via a different mechanism, but this part is complete and reviewed.)
An actual one-to-one mapping would using 'hidrawX' because there is a
one-to-one mapping between /dev/hidrawX for HID devices. However, this
means that we consider the bus to be hidraw which is plain wrong too.

The unique ID of HID devices (in /sys/bus/hid/devices) is in the form
`BUS:VID:PID.XXXX`. I understand the need to not have colons, so could
we standardize LEDs on the HID subsystem to be named
`hid-bus_vid_pid_xxxx:color:fun(-n)?`? That would allow a mapping
between the LED and the sysfs, and would also allow users to quickly
filter out the playstation ones.
As I wrote in other e-mail some minutes ago, this just means that we
need to wait for other people's opinions. Please do not send this
pull-request with the LED patches until this is resolved.

Marek
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help