Re: [PATCH 1/5]:hidbus prototype 070409:core headers

On Mon, 30 Apr 2007, Li Yu wrote:

> 	These are the splited patches of hid bus prototype 070409.  I 
> 	remember I posted them in attachment form ago ;) Now they are 
> 	splited in some separable patches. I am afraid I can not receive 
> 	in Labor Day.

Hi Li,

first, please apologize my delay in the reply, the patches are quite 
large and I have been busy with lots of different things lately.

There had been quite a long discussion some time ago, regarding the proper 
way to design hidbus. As far as I recall it, we came to the conclusion 
that the best way would be to allow defining of two different hooks - one 
for raw data and another for parsed reports and let drivers decice which 
one they want to use. I am missing the possibility to hook the parsed data 
in your implementation, or did I just overlook something?

Second, what is the exact point with hid_report_pre_event()? It is missing 
any explanatory comment (which applies to other code in your patches BTW) 
and I seem to be missing the point too.

Third, as far as my memory goes, I think we agreed that in order to design 
the layering properly and allow individual specialized drivers to work 
properly, the HID bus code should provide a way to register a driver for a 
specific VID/PID and also the report id (one or more). Then all reports 
for which there is no specialized driver are going to be handled by the 
default HID->input driver (or could be handled into hidraw if the 
in-kernel HID parser is unable to parse them). The reports for ids with a 
special driver are handed over to the driver. I unfortunately seem to be 
missing this functionality in the patch too ... ?

Last but not least, I have gone through this code several times, but I 
still don't fully understand why the 'hid_transport' abstraction is 
needed. I am still convinced that the current model works well, and there 
should be absolutely no need for hid_(un)register_transport() and the 
related things. Could some explanation please be put somewhere? (ideally 
in the patch's changelog entry).

And more generally - the patches seem a little bit bigger than they could 
be, I am not sure whether the layering and data structures you have are 
not a little bit overdesigned. But this will need some more review.

Thanks a lot,

-- 
Jiri Kosina

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel