Hi On Mon, Feb 3, 2014 at 6:57 PM, Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote: > On Mon, Feb 03, 2014 at 06:45:52PM +0100, David Herrmann wrote: >> Hi >> >> Adding Dmitry+Jiri, maybe they can clarify this. >> >> [snip] >> >>> >> >>> Yes >> >>> >> >>>> hidp sets it to point to the hci_conn struct from which src address for the >> >>>> Bluetooth connection can be obtained, but making assumptions about an opaque >> >>>> pointer like that seems dangerous. Is the parent pointer guaranteed to >> >>>> point to the hci_conn struct as long as the bus type is Bluetooth? >> >>> >> >>> And nope. If you use uhid, then the parent will not be a hci_conn. >> >>> >> >>> With enough guards, you should be able to use it, but it's not ideal I agree. >> >>> I really want to have David's opinion regarding the UNIQ field. IMO, >> >>> this seems to be the most transport-agnostic way of doing it. >> >> >> >> UNIQ is definitely wrong. It is used to assign a run-time *unique* >> >> value to the connection. So ideally it should be different if a device >> >> is reconnected. The PHYS field is actually used to identify a physical >> >> device. So please, if we're doing this, then we should do it via PHYS. >> >> >> >> I'm fine with hard-coding PHYS as bt-address for BT-HID, but please >> >> add a comment to hidp_setup_hid() in net/bluetooth/hidp/core.c before >> >> doing the snprintf() there. >> >> >> >> The reason why I disliked hard-coding this behavior is that if a >> >> single BT-device is connected twice to us, we usually want two >> >> different PHYS entries for both depending on which service this >> >> connection provides. However, this seems like an unlikely and >> >> overengineered problem so lets not do that. Furthermore, while BT-HID >> >> would allow such setups with some hacks, it isn't supported in a clean >> >> way. So yeah, I'm actually fine with using PHYS for that. >> >> >> >> Thanks >> >> David >> > >> > PHYS should definitely be changed if this is the case because right >> > now it is set to the MAC of the host adapter which means that it's the >> > same for every Bluetooth device and connection. I believe that the >> > hidraw documentation also specifies that for Bluetooth devices the >> > HIDIOCGRAWPHYS ioctl should return a string with the MAC address of >> > the associated device rather than that of the host adapter as the >> > current behavior does. >> >> Oh, yes, nice catch! >> Ok, maybe we need to clear up what PHYS and UNIQ do before relying on >> them. I thought, they were defined as: >> >> PHYS: A string describing the physical device. It should be the same >> if a device reconnects. It can be used by user-space to track devices >> across disconnects >> >> UNIQ: A string describing the current connection to a device. If the >> device reconnects, the UNIQ string should change. It can be used by >> user-space to track a single device-session. >> >> afaik both strings have no common format and drivers are free to >> provide any kind of information, as long as it follows the given >> rules. That's why I disliked the idea of relying on them and parsing >> them. But maybe I just have no idea what the original intention was >> behind them? > > PHYS: describes physical connection of the device in a given box, > supposed to be unique within a box. > > UNIQ: unique identifier (if exists) assigned to the device, should > ideally be unique globally and should not change when device is moved > within a box out between boxes. Think serial number in USB descriptors. Thanks, so it's basically the other way around as I thought. So I think using UNIQ is the way to go, sorry for the confusion. Thanks David -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html