RE: <COMMERCIAL>: Re: Bluetooth Low Energy service crash report - when trying read a HID feature report

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

 



>Hi David, Murat,

Hi Luiz,

>
>On Mon, Jul 28, 2014 at 11:53 AM, David Herrmann <dh.herrmann@xxxxxxxxx> wrote:
>> Hi
>>
>>>> The kernel bails out if size==0, so this never happens. The 
>>>> bt_uhid_* helpers are fine, however, hog.c forward_report() is totally broken.
>>>> You use:
>>>>
>>>> bt_uhid_register(hogdev->uhid, UHID_OUTPUT, forward_report, hogdev); 
>>>> bt_uhid_register(hogdev->uhid, UHID_FEATURE, forward_report, 
>>>> hogdev);
>>>>
>>>> Therefore, you register *THE SAME* handler for UHID_OUTPUT *AND* 
>>>> UHID_FEATURE. However, in forward_report() you access ev->u.output, 
>>>> but this is only valid for UHID_OUTPUT. If you get a UHID_FEATURE 
>>>> report you must never access anything but ev->u.feature!
>>>
>>> Does the ev->u.features structure matches HID protocol, can I send as 
>>> it is or do I need to reformat it?
>>
>> The uhid.h header includes these definitions:
>>
>> struct uhid_feature_req {
>>         __u32 id;
>>         __u8 rnum;
>>         __u8 rtype;
>> } __attribute__((__packed__));
>>
>> struct uhid_feature_answer_req {
>>         __u32 id;
>>         __u16 err;
>>         __u16 size;
>>         __u8 data[UHID_DATA_MAX];
>> } __attribute__((__packed__));
>
>That I got it, what I was asking was if that is the exact same format used in HID so that I could sent as it is to the remote device. Im afraid the answer is no because id and some other fields seems to be just for context tracking as you mention bellow.
>
>>>> Furthermore, if you receive UHID_FEATURE, the kernel blocks until 
>>>> you send the answer as UHID_FEATURE_ANSWER.
>>>
>>> So if I got this right in addition to send the request we should 
>>> process the response as UHID_FEATURE_ANSWER otherwise it will block 
>>> other events?
>>
>> Kernel HID drivers can use a kernel-internal function to query an 
>> HID-feature. This call is blocking and waits for the answer from the 
>> device. uhid copied that design to avoid rewriting all drivers.
>> Therefore, IFF you get an UHID_FEATURE event, you have to send the 
>> feature request to the device, somewhere store the context, and once 
>> the device answers you have to find that context again and write the 
>> answer via UHID_FEATURE_ANSWER to the kernel. The "id" field in 
>> uhid_feature_req and uhid_feature_answer_req must match and are used 
>> for context-tracking.
>>
>> Note that this is only used for HID drivers that require synchronous 
>> feature queries. All other drivers use UHID_OUTPUT for all outoing 
>> reports. The "rtype" field specifies whether it's an INPUT, OUTPUT or 
>> FEATURE report that has to be written. It it thus totally ok to ignore 
>> UHID_FEATURE as only few drivers use it.
>
>@Murat, I guess your input is needed here, why are you using UHID_FEATURE instead of UHID_OUTPUT? Can we drop UHID_FEATURE or your driver really requires it?

The device which I tried to read feature report has configuration information (For example, software feature, firmware version and so forth). This data is not available over UHID_OUTPUT. According to the USB specification, section 6.2.2.5, this kind of data is described 'Feature Items'. 

>
>> Note that the uhid interface so far is a bit crappy. My TODO list 
>> contains "cleanup uhid API" but so far I haven't found the time for 
>> it, sorry.
>
>No problem, our HoG plugin needs a little bit of cleanup as well.

Murat


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Keep up to date with CSR on our technical blog, www.csr.com/blog, CSR people blog, www.csr.com/people, YouTube, www.youtube.com/user/CSRplc, Facebook, www.facebook.com/pages/CSR/191038434253534, or follow us on Twitter at www.twitter.com/CSR_plc.
New for 2014, you can now access the wide range of products powered by aptX at www.aptx.com.
��.n��������+%������w��{.n�����{����^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�


[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