Hi Johan, >>>>> 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. >> 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. Regards Marcel -- 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
