Re: [PATCH v2 00/18] Rework HIDP Module

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Marcel

On Tue, Apr 2, 2013 at 8:05 PM, Marcel Holtmann <marcel@xxxxxxxxxxxx> wrote:
> Hi David,
>
>>>> This is my second attempt to get these patches merged. I added 4 more fixes at
>>>> the end and removed the patches that were already merged by Gustavo.
>>>>
>>>> This fixes a lot of issues with the HIDP layer. It is currently nearly
>>>> impossible to use HIDP devices. Every attempt (except the first) to connect a
>>>> device results in an oops or panic for me.
>>>>
>>>> I haven't found a way to fix this easily, so I decided to rewrite the session
>>>> handling of HIDP.
>>>
>>> I am generally fine with these patches after a quick read-through. However I am not 100% sure that hci_conn_user is the right approach.
>>>
>>> So my question is if we really want to base this of the ACL link itself and not base this around L2CAP directly. Every L2CAP connection implies a hci_conn in the end. Would it be hard to have something like l2cap_user instead of hci_conn_user?
>>
>> The thing is that HID uses two L2CAP channels. We would probably use
>> the CTRL channel as parent device, but this makes the synchronization
>> with the INTR channel quite hard to implement.
>> Also note that l2cap channels do not have "struct device" embedded,
>> which would disallow showing the HID device in the right location in
>> sysfs.
>> If you have some other idea, I can try to implement this. But I
>> couldn't come up with anything better than hid_conn_user.
>
> there is a difference between l2cap_conn and l2cap_chan. The L2CAP layer has a generic representation of underlying ACL link. I was talking about that one. From that we can get to the hci_conn device object.
>
> Maybe this is more cosmetic, but only going through l2cap_conn and have that bridge into hci_conn seems a bit cleaner to me. Especially since you need a bunch of exports from L2CAP layer anyway.

Sorry, I thought of l2cap_chan.. Sure, l2cap_conn actually makes
sense. I already use a helper to get the hci_conn out of l2cap_sock so
I might just as easily try to get l2cap_conn instead.

But this is indeed only a cosmetic change, because l2cap_conn->hconn
is always valid so I will just move it over to l2cap_conn_user.
I will resend in a few days, so if there are something else, I can fix it, too.

Btw., I tested this with 5 simultaneously connected devices, I tested
device reconnects, I tested system suspend, ... and everything works
fine here.

Thanks
David
--
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




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux