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
