Hi Marcel, On Sat, Sep 28, 2013, Marcel Holtmann wrote: > >>>>> If either one of the HCI_SETUP or HCI_USER_CHANNEL flags is set the > >>>>> device is not considered valid for mgmt. By having these checks inside > >>>>> the mgmt_valid_hdev function the a couple of places using it can be > >>>>> simplified. > >>>> > >>>> I looked at doing this and decided not to. Reason was that the device > >>>> gets removed from mgmt anyway. > >>> > >>> I'm not sure what significance you think "device gets removed from mgmt" > >>> has. All that is is a mgmt event saying that index has been removed. In > >>> addition to that we need to ensure that we don't send any more mgmt > >>> events for such devices and that we don't include such devices in the > >>> response to mgmt_read_index list. > >>> > >>> These are the two places of the code that my patch simplifies, one is > >>> the check for whether to send a "power on" mgmt event and the other the > >>> response handling of read_index_list. > >> > >> I have taken care of that part. It is done inside mgmt_control() and > >> in case of a controller in user channel mode it returns invalid index. > > > > That's great, but it doesn't cover the read_index_list command which we > > were talking about here since the command doesn't target any specific > > controller (as per our mgmt-api.txt it has the value <non-controller> as > > the controller index). The command handler will in this case be invoked > > that that's where it'd be convenient to rely on mgmt_valid_hdev() for > > constructing the response. > > the read index list command has a special check for user channel. As I > said, I looked into this and opted against extending valid hdev macro > since we only need it in a few specific cases. Adding it there > resulted in simpler code then some magic macro that is used in more > places. > > When I wrote the patches for user channel this felt cleaner. I had > patches for both ways, but decided to go this route. Fair enough. > >> For the events, I have no idea on how we would generate events if we > >> are not going through hci_event.c when the user channel is active. > > > > The "powered on" event in question doesn't come from hci_event.c but > > from the hci_dev_open() function, so again this is not covered by the > > existing user channel checks. > > How would you trigger a power on event. When user channel is active > everything else is blocked. So every other operation will fail. That > is taken care of. > > In addition you can only activate user channel on a controller that is > powered off. That was a conscious design decision. Keep it really > simple. User channel mode is exclusive and exclusive to one user at a > time. Good point. The only hci_dev_open() call that comes to mind is the one automatically triggered by the setup stage, but user space doesn't yet know about the controller at that point. Johan -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
