Hi Marcel and Johan,
>>>>> err = hci_dev_do_open(hdev);
>>>>> if (err < 0) {
>>>>> + hci_dev_lock(hdev);
>>>>> mgmt_set_powered_failed(hdev, err);
>>>>> + hci_dev_unlock(hdev);
>>>>> return;
>>>>> }
>>>
>>>> I wonder is some of the mgmt_ function should just take the hci_dev
>>>> lock. Are there cases where we don't want them to take the look?
>>>
>>> There are many mgmt_functions called from hci_event.c which don't
>>> require lock. You prefer moving the lock inside
>>> mgmt_set_powered_failed()?
>>
>> This should be kept consistent imo. Either all mgmt_* functions called
>> from hci_core/event.c should be responsible for taking the lock
>> themselves or none should be. I think right now the general rule is that
>> hci_core/event.c take the lock first, and this is mainly because a lot
>> of code that needed calling into mgmt.c was already holding the lock.
>> I.e. moving the taking into mgmt.c would for many places have required
>> awkward looking hdev_unlock(); mgmt_foo(); hdev_lock(); constructions.
>
>we could also rename some to __mgmt_foo which means they are not taking
>the lock and others to mgmt_bar which take the lock by themselves. And then
>see if they actually have variations where we would need both.
I have raised the split patch sets and fixed coding violation as well. Renaming of
mgmt. functions can be done on top of this if required.
Thanks,
Jaganathÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ¥Šwÿº{.nÇ+‰·¥Š{±ý¹nzÚ(¶ âžØ^n‡r¡ö¦zË?ëh™¨èÚ&£ûàz¿äz¹Þ—ú+€Ê+zf£¢·hšˆ§~††Ûiÿÿï?êÿ‘êçz_è®æj:+v‰¨þ)ߣøm
