Hi Andrei,
On Fri, Apr 13, 2012 at 3:39 PM, Andrei Emeltchenko
<andrei.emeltchenko.news@xxxxxxxxx> wrote:
> Hi Ulisses,
>
> On Fri, Apr 13, 2012 at 7:25 PM, Ulisses Furquim <ulisses@xxxxxxxxxxxxxx> wrote:
>
>> On Fri, Apr 13, 2012 at 8:21 AM, Andrei Emeltchenko
>> <Andrei.Emeltchenko.news@xxxxxxxxx> wrote:
>>>
>>> From: Andrei Emeltchenko <andrei.emeltchenko@xxxxxxxxx>
>>>
>>> When adding HCI devices hci_register_dev may assigns the same name
>>> hciX for subsequently added HCI devices. Use bitmask to find free
>>> device ID the same way as it is done in other subsystems.
>>>
>>> ...
>>> [ 6958.381886] sysfs: cannot create duplicate filename
>>> '/devices/virtual/bluetooth/hci1
>> Well, I don't like this bitmask approach, really. We used to have this
>> list ordered and the code was doing that before we added the
>> restriction of index 0 for AMP devices, I think.
>
> I think it just was not tested enough.
Probably.
>> Using a bitmask looks
>> overkill to me when we can probably modify the loop to find the first
>
> This approach is used in many places when choosing device id, specifically
> in network device code, so I just took it as I do not like to reinvent
> the wheel.
I know and I agree not reinventing the wheel but AFAIK they use
bitmask when they don't have the list and thus mark everything and
then extract the information they need afterwards. Our case is very
simple, we do have the list already and the code doesn't run often so
it looks overkill to me the bitmask approach.
>> available ID. Marcel even suggested an approach with a pseudo code
>> that seems to be the best way to go. Something along the lines of:
>>
>> list_for_each(p, &hci_dev_list) {
>> if (list_entry(p, struct hci_dev, list)->id > id)
>> /* just replacing != with > */
>> break;
>> head = p; id++;
>> }
>>
>> We assume id starts with the number we'll try to add the new device
>> and keep iterating until we find the proper place. The only difference
>> is we start with 0 for BR/EDR and 1 for AMP devices. Then every
>> hdev->id in the _ordered_ list <= to the id we want we increment id
>> and move head. In the end we'll have id as the first available one and
>> head is where you need to add hdev (before head) to the list to keep
>> it ordered. Right? Does that work for you? Am I missing anything here?
>
> Please better send a patch ;) Either way is OK for me.
Did it work for you? Sorry, couldn't test it here and that's why I
just sent comments and asked if I wasn't missing anything. :-) But I
can try to send a patch later today, just in case.
Best regards,
--
Ulisses Furquim
ProFUSION embedded systems
http://profusion.mobi
Mobile: +55 19 9250 0942
Skype: ulissesffs
--
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
