[DOC] The documentation for HID Simple Driver Interface 0.5.0

[DOC] The documentation for HID Simple Driver Interface 0.5.0

[Li Yu]

==================================
HID device simple driver interface
==================================

------------------------
Note
------------------------

 If you just begin to study from writing input device driver, please see the
input-programming.txt, I am afraid this is not you want, do not confuse with the
"simple" in this name.

------------------------
Version
------------------------

 This is used for the version 0.5.0

--------------------------
Overview
--------------------------

 Under standard HID device driver development means, we need to write
one interrupt handler for each new HID device to report event to
input subsystem. However, although the most of they can not merge into
mainstream kernel tree, they have only some extended keys, e.g. many
remote controllers. I think it seem break a fly on the wheel, which
write one new interrupt handler for this reason.

 The basic idea is reuse the interrupt handler in hid-core.c. so writing
driver for new simple HID device will be more easier, quickly, and do not
need touch hid core.

 In essence, this interface just is some hooks in HID core.

 The hid-simple.h include this API. But, defore use this interface, you must
include hid.h first.

------------------------
What's you will get from this interface.
------------------------

 Use me, you can:

 1. Write the driver of USB input device without write code take care of
 details of setup and clear usage.
 2. A driver use this interface can be as a HID device input filter.

 This interface can not support the more drivers handle one device at same time
yet. I can not sure if we really need this feature.

-------------------------
Prepare
-------------------------

 Before use this interface, you must turn on these kernel configuration
items:

 CONFIG_HID_SIMPLE : HID simple driver interface

--------------------------
Register and unregister
--------------------------

1. Register a simple driver.

 int hidinput_register_simple_driver(struct hidinput_simple_driver *simple)

 Like any driver register function, it register your driver to kernel,
when the chance that match device come, your driver can probe it. At least, you
must fill the member of parameter simple : name.
 Return 0 mean register successfully. elsewise mean there have some
failures in register process.
 So far, this function only can return 0 and -EINVAL.
 When you driver matched one device, it will reregister it into input
subsystem (see the function hidinput_reconnect_core(), if you want).
 This function generally is used in HID sublayer.

2. Unregister a simple driver.

 void hidinput_unregister_simple_driver(struct hidinput_simple_driver *simple)

 Like any driver register function, it clean your driver from kernel, and
in this process, your driver will disconnect any device which it matched. However,
it do not free your memory of driver structure, that's your task.
 This may reregister the device into input subsystem (see the function
hidinput_reconnect_core(), if you want).
 This function generally is used in HID sublayer.

3. USB device driver.
 
 You should use follow functions for USB device. the usage of these functions is 
same with aboves.

int hidinput_register_simple_usb_driver(struct hidinput_simple_driver *simple)
void hidinput_unregister_simple_usb_driver(struct hidinput_simple_driver *simple)

 Both functions is used in driver. 

----------------------------------
Usage
----------------------------------

 Each simple driver have one data structure hidinput_simple_driver:

struct hidinput_simple_driver {
 /*
 * The members for implement only are ignored here,
 * please do not depend on them
 */
 /* public for HID sublayer. */
 int (*match_device)(struct hidinput_simple_driver *drv,
                               struct matched_device *dev);

 /* public for drivers */
 struct module *owner;
 char *name;
 int (*connect)(struct hid_device *, struct hid_input *);
 void (*disconnect)(struct hid_device *, struct hid_input *);
 void (*setup_usage)(struct hid_field *, struct hid_usage *);
 void (*clear_usage)(struct hid_field *, struct hid_usage *);
 int (*pre_event)(const struct hid_device *, const struct hid_field *,
				 const struct hid_usage *, const __s32);
 int (*post_event)(const struct hid_device *, const struct hid_field *,
				 const struct hid_usage *, const __s32);
 int (*open)(struct input_dev *dev);
 void (*close)(struct input_dev *dev);

 void *private;
 struct usb_device_id *id_table;
 struct usage_page_block *usage_page_table;
};

The data member description:

 struct module *owner;

 In most cases, set this member to THIS_MODULE is your want to.

 char *name;

 The name of your driver. you must fill this member before register it.

 void *private;

 You can save the data that your driver use only here. HID simple driver
core do not take care of this.

 struct usb_device_id *id_table;

 As same with other USB device driver, this is used by matching device,
if so, then call probe()/connect() methods of your driver.
 In general, you always want to fill this member with something.

 struct usage_page_block *usage_page_table;

 Totally, there are three means you can complete setup and clean HID usage work,
which use this member is one of them. Moreover, this means is the most simplest one.
By fill this member, kernel will complete setup and clean usage work automatically.
 It will spend many words to explain details, I think the best documentation
of this is the example, you can find them in usbnek4k.c and btp2118.c. I believe
you have enough intelligence to understand them, but if you had familiar
with HID first.  :)  When you read these examples, you also will find out some other
related data structures ,however, don't worry, they are too simple to not discuss
them.

--------------------------------
Generic Methods
--------------------------------

The simple driver methods description:

 Although this simple driver have not direct relation with Linux
device driver architecture, but I still make its API like it on purpose.
The simple driver have follow methods:

1. int (*connect)(struct hid_device *, struct hid_input *);
2. void (*disconnect)(struct hid_device *, struct hid_input *);

 When you simple driver is to bind with one real HID device, we will
call connect() method first. To return 0 flag that it complete its mission
successfully, so we can continue, return any other value is looked as
error, any processing do not come.
 When the device which your simple driver connect with is down, or
this simple driver is unregistered, we will call disconnect() method first.

3. void (*setup_usage)(struct hid_field *, struct hid_usage *);
4. void (*clear_usage)(struct hid_field *, struct hid_usage *);

 The setup_usage() method like hidinput_configure_usage() in
hid_input.c. You also can setup input_dev here. I think
you should be fill the pointer slot for this method, or fill usage_page_table
member of hidinput_simple_driver, elsewise the event() method do not work for
you at all. Please see example in "MS Natural Ergonomic Keyboard 4000" driver.
 The clear_usage() method is used to clear side-effect that came from
setup_usage() method when one driver disconnect one device.
 Of course, you can do same things in connect/disconnect() method, but
these method can make your life more simpler.
 At last, you can use these two methods and usage_page_table member
at same time. These two methods always will be called after kernel process
usage_page_table.

6. int (*pre_event)(const struct hid_device *, const struct hid_field*,
					const struct hid_usage *, const __s32);

 First, you can use this method send event to input subsystem,
moreover, you can use this as one usage code filter: this method is called
before all other event handling. If it return non-zero , any other event
handling do not be processed, even the hid-input itself. If this method
return zero, the normally event handling process will continue.
 See the note [1].

7. void (*post_event)(const struct hid_device *, const struct hid_field *,
					const struct hid_usage *, const __s32);

 This is called after all other event handling. So if the pre_event() method
return non-zero value, this method also do not called at all.
 See the note [1].

8. int (*open)(struct input_dev *dev);
9. void (*close)(struct input_dev *dev);

 They are same with corresponding methods of struct input_dev. I assume
you already familiar with them.

10. int (*match_device)(struct hidinput_simple_driver *drv, struct matched_device *dev);

 Return non-zero mean drv is matched for dev. An example of its implementation can be 
found in drivers/usb/input/hid-core.c

=================================================================================

NOTE:

[1] If you do not configure usage correctly, this method do not work as
 you want.

Re: [DOC] The documentation for HID Simple Driver Interface 0.5.0

[Dmitry Torokhov]

On 3/5/07, Marcel Holtmann <marcel@holtmann.org> wrote:
>
> This also means that the current keyboard and mouse
> input devices will become a HID driver.
>

Are you talking about usbmouse and usbkbd?

-- 
Dmitry

Re: [DOC] The documentation for HID Simple Driver Interface 0.5.0

[Marcel Holtmann]

Hi Li,

> ==================================
> HID device simple driver interface
> ==================================

actually, I don't think we need a simple driver interface. We need a HID
driver interface in general. For example that you can register a driver
for one or multiple report ID and then it handles input and output for
these report IDs. This also means that the current keyboard and mouse
input devices will become a HID driver.

In addition to the kernel HID driver we might also when have a new
hidraw (or modified hiddev) interface that allows to handle specific
report ID from a userspace application. Especially when it comes to LED
displays or UPS systems.

Regards

Marcel

Re: [DOC] The documentation for HID Simple Driver Interface 0.5.0

[Jiri Kosina]

On Mon, 5 Mar 2007, Marcel Holtmann wrote:

> actually, I don't think we need a simple driver interface. We need a HID 
> driver interface in general. For example that you can register a driver 
> for one or multiple report ID and then it handles input and output for 
> these report IDs. This also means that the current keyboard and mouse 
> input devices will become a HID driver.

Hi Marcel,

sorry, this is not clear to me - what keyboard and mouse devices are you 
talking here precisely? As not all keyboard and mouse devices are HID, 
this is probably not what you've meant ... ? Thanks.

> In addition to the kernel HID driver we might also when have a new 
> hidraw (or modified hiddev) interface that allows to handle specific 
> report ID from a userspace application. Especially when it comes to LED 
> displays or UPS systems.

I have already written some code of hidraw interface, but it's not yet 
finished due to too many other pending things.

Thanks,

-- 
Jiri Kosina

Re: [linux-usb-devel] [DOC] The documentation for HID Simple Driver Interface 0.5.0

[Jiri Kosina]

On Mon, 5 Mar 2007, Li Yu wrote:

>  Under standard HID device driver development means, we need to write 
> one interrupt handler for each new HID device to report event to input 
> subsystem. However, although the most of they can not merge into 
> mainstream kernel tree, they have only some extended keys, e.g. many 
> remote controllers. I think it seem break a fly on the wheel, which 
> write one new interrupt handler for this reason.

This paragraph I don't seem to understand. Either the device is claimed by 
hid-input (*), and then when the device is behaving more-or-less 
correctly, hid-input handles the usages sent in reports and maps them 
properly to input events. New mapping can be quite trivially added to 
hid-input.

If the HID device is not claimed by hid-input, then userspace can access 
the hid events through hiddev and perform any actions that are needed.

Could you please elaborate a little bit more why your approach using 
"simple HID interface" is better than for example hiddev+uinput, done 
completely in userspace?

In addition to that, as I stated in some previous e-mail, I am currently 
working on new 'hidraw' interface, which will provide more flexibility to 
userland applications willing to operate on raw HID data, than they 
currently have with hiddev.

(*) OK, I would tend to agree that the algorithm which decides whether the 
given device will be claimed by hid-input driver could be improved to be 
more HID-specification and HUT compliant. I have this in my TODO, but I 
guess it doesn't matter too much here.

Thanks,

-- 
Jiri Kosina

Re: [linux-usb-devel] [DOC] The documentation for HID Simple Driver Interface 0.5.0

[Dmitry Torokhov]

On 3/5/07, Jiri Kosina <jikos@jikos.cz> wrote:
> On Mon, 5 Mar 2007, Li Yu wrote:
>
> >  Under standard HID device driver development means, we need to write
> > one interrupt handler for each new HID device to report event to input
> > subsystem. However, although the most of they can not merge into
> > mainstream kernel tree, they have only some extended keys, e.g. many
> > remote controllers. I think it seem break a fly on the wheel, which
> > write one new interrupt handler for this reason.
>
> This paragraph I don't seem to understand. Either the device is claimed by
> hid-input (*), and then when the device is behaving more-or-less
> correctly, hid-input handles the usages sent in reports and maps them
> properly to input events. New mapping can be quite trivially added to
> hid-input.
>

We are running out if quirks and we can't continue in this fashion so
Li tried to provide mechanism to augment default HID behaviour for
sepcific devices. However I don't think we want this particular
implementation - it firrces sub-drivers to be compiled into HID driver
which may grow indefinitely.

If we define HID "bus" allowing drivers to bind on VID:PID and provide
default library module for parsing HID reports and providing access to
HID transports (USB/BT) then writing tiny drivers adjusting just a
part of hid_input_event and relying on default implemenattaion where
it makes sense will become a breeze.

-- 
Dmitry

Re: [DOC] The documentation for HID Simple Driver Interface 0.5.0

[Marcel Holtmann]

Hi Dmitry,

> > This also means that the current keyboard and mouse
> > input devices will become a HID driver.
> 
> Are you talking about usbmouse and usbkbd?

no, because if I recall correctly these are the boot mode drivers and
actually not used at all in any modern distribution.

For me the task of converting HID reports into input events shouldn't be
actually the job of the HID core layer. My understanding is that the HID
core should support multiple transport layers. This is currently
achieved through the hid_device abstraction and used by the USB and by
the Bluetooth subsystem. This is the lower interface to HID. On the
upper interface I like to see a driver like interface. So we can
register specific drivers that can handle specific use cases or vendor
specific reports. For standard keyboard and mouse reports we however
should have a standard driver that can handle most of them.

For identification we actually do have the bus type, vendor id and
product id available. We also can register drivers for specific report
ids or usages.

I do like the HID simple driver idea, but it is not a simple driver, it
should be a generic driver interface.

Regards

Marcel

Re: [linux-usb-devel] [DOC] The documentation for HID Simple Driver Interface 0.5.0

[Liyu]

Dmitry Torokhov wrote:
> If we define HID "bus" allowing drivers to bind on VID:PID and provide
> default library module for parsing HID reports and providing access to
> HID transports (USB/BT) then writing tiny drivers adjusting just a
> part of hid_input_event and relying on default implemenattaion where
> it makes sense will become a breeze.
>
>   
Well, actually, the argument have begun since the version 0.4.0 of HID
simple
interface. I clearly know its weaknesses, and I believe Dmitry said, the
HID bus/tiny
driver way is the most ultimatest way very well.

But (You may guess I will say this word :), before the HID bus or other
better
implementation come , I hope use the extended keys of my keyboard on
Linux, and I guess other people
also think same with me, so we need something here temporarily, even it
do not merge
into upstream code tree.

And, I want to know that is somebody works on HID bus, Can I join it?

Good luck.

-Li Yu

Re: [linux-usb-devel] [DOC] The documentation for HID Simple Driver Interface 0.5.0

[Robert Marquardt]

Jiri Kosina wrote:

> In addition to that, as I stated in some previous e-mail, I am currently 
> working on new 'hidraw' interface, which will provide more flexibility to 
> userland applications willing to operate on raw HID data, than they 
> currently have with hiddev.

I do not know too much about the Linux innards, but my opinion is that 
the HID API to userland should be the bare reports. Windows is handling 
it that way for a good reason. Far too many HID devices have either 
non-informative descriptors (declaring a byte array report) or declaring 
a report which does not match the data contained. Parsing a report is 
definitely a userland job.

-- 
--------------------------------------
Code Mercenaries
Hard- und Software GmbH
Karl-Marx-Str. 147a
12529 Schönefeld OT Grossziethen
Germany

Tel: x49-3379-2050920
Fax: x49-3379-2050930

HRB 16007 Potsdam
Geschäftsführer: Guido Körber, Christian Lucht

Re: [linux-usb-devel] [DOC] The documentation for HID Simple Driver Interface 0.5.0

[Jiri Kosina]

On Tue, 6 Mar 2007, Robert Marquardt wrote:

> I do not know too much about the Linux innards, but my opinion is that 
> the HID API to userland should be the bare reports. Windows is handling 
> it that way for a good reason. Far too many HID devices have either 
> non-informative descriptors (declaring a byte array report) or declaring 
> a report which does not match the data contained. Parsing a report is 
> definitely a userland job.

Which is exactly what the new hidraw interface is going to provide.

-- 
Jiri Kosina

Re: [linux-usb-devel] [DOC] The documentation for HID Simple Driver Interface 0.5.0

[Jiri Kosina]

On Tue, 6 Mar 2007, Li Yu wrote:

> > If we define HID "bus" allowing drivers to bind on VID:PID and provide 
> > default library module for parsing HID reports and providing access to 
> > HID transports (USB/BT) then writing tiny drivers adjusting just a 
> > part of hid_input_event and relying on default implemenattaion where 
> > it makes sense will become a breeze.
> But (You may guess I will say this word :), before the HID bus or other 
> better implementation come , I hope use the extended keys of my keyboard 
> on Linux, and I guess other people also think same with me, so we need 
> something here temporarily, even it do not merge into upstream code 
> tree.
> And, I want to know that is somebody works on HID bus, Can I join it?

I will probably start working on it one day. Anyway some parts of what 
Dmitry described are already in the mainline since 2.6.20 - the parser 
code (and hid_device registration and initialization in general) is 
already there. Access to transport layers (USB/BT) is partly also there 
(see hid_open(), hid_close() and hidinput_input_event() callbacks in 
struct hid_device). So really the only one pending thing which is 
non-trivial would be creating an interface for registering the device 
driver for given VID/PID on the HID bus, and solving the fallback 
mechanism, i.e. assuring that everything works for drivers which are OK 
with generic hid and hid-input behavior.

I will happily accept such patches from you, if you do them. Otherwise I 
will start to work on it in a future anyway.

Thanks,

-- 
Jiri Kosina

Re: [linux-usb-devel] [DOC] The documentation for HID Simple Driver Interface 0.5.0

[Harold Sargeant]

On 06/03/07, Li Yu <liyu@ccoss.com.cn> wrote:
> Dmitry Torokhov wrote:
> > If we define HID "bus" allowing drivers to bind on VID:PID and provide
> > default library module for parsing HID reports and providing access to
> > HID transports (USB/BT) then writing tiny drivers adjusting just a
> > part of hid_input_event and relying on default implemenattaion where
> > it makes sense will become a breeze.
> >
> >

> But (You may guess I will say this word :), before the HID bus or other
> better
> implementation come , I hope use the extended keys of my keyboard on
> Linux, and I guess other people
> also think same with me, so we need something here temporarily, even it
> do not merge
> into upstream code tree.

And until something better comes along I am happy to use what Li Yu
has produced (speaking as an ordinary user).

Thanks

--
Harold Sargeant

Re: [DOC] The documentation for HID Simple Driver Interface 0.5.0

[Dmitry Torokhov]

Hi Marcel,

On 3/5/07, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Dmitry,
>
> > > This also means that the current keyboard and mouse
> > > input devices will become a HID driver.
> >
> > Are you talking about usbmouse and usbkbd?
>
> no, because if I recall correctly these are the boot mode drivers and
> actually not used at all in any modern distribution.
>
> For me the task of converting HID reports into input events shouldn't be
> actually the job of the HID core layer. My understanding is that the HID
> core should support multiple transport layers. This is currently
> achieved through the hid_device abstraction and used by the USB and by
> the Bluetooth subsystem. This is the lower interface to HID. On the
> upper interface I like to see a driver like interface. So we can
> register specific drivers that can handle specific use cases or vendor
> specific reports. For standard keyboard and mouse reports we however
> should have a standard driver that can handle most of them.
>
> For identification we actually do have the bus type, vendor id and
> product id available. We also can register drivers for specific report
> ids or usages.
>
> I do like the HID simple driver idea, but it is not a simple driver, it
> should be a generic driver interface.
>

Ah, OK. Then we are in complete agreement ;)

-- 
Dmitry

Re: [DOC] The documentation for HID Simple Driver Interface 0.5.0

[Jiri Kosina]

On Mon, 5 Mar 2007, Marcel Holtmann wrote:

> no, because if I recall correctly these are the boot mode drivers and 
> actually not used at all in any modern distribution.

That's correct.

> For me the task of converting HID reports into input events shouldn't be 
> actually the job of the HID core layer. My understanding is that the HID 
> core should support multiple transport layers. This is currently 
> achieved through the hid_device abstraction and used by the USB and by 
> the Bluetooth subsystem. This is the lower interface to HID. On the 
> upper interface I like to see a driver like interface. So we can 
> register specific drivers that can handle specific use cases or vendor 
> specific reports. 

This I completely agree with, and have this on my TODO for quite a long 
time. If Li Yu would like to spend some time on it, it certainly would 
help. The current "simple HID interrface" is a suitable workaround for 
various buggy/broken/nonstandard hardware which is currently not handled 
by the HID kernel code properly, but it's definitely not a long-term 
solution that should go to mainline.

> For standard keyboard and mouse reports we however should have a 
> standard driver that can handle most of them.

Unfortunately there is a non-trivial bunch of hardware that pretends to be 
standard keyboard/mouse, but behaves badly. Last week I had to do 
workaround for Logitech S510, which seems to generate usages far above the 
logical maximum specified in report descriptor, to give one example.

So we will probably end up with many small driver for exotic pieces of 
hardware being registered to the hid bus. But this is definitely much 
better than the current mess of hid quirks.

On Mon, 5 Mar 2007, Dmitry Torokhov wrote:

> We are running out if quirks and we can't continue in this fashion so Li 
> tried to provide mechanism to augment default HID behaviour for sepcific 
> devices. 

That's right. We could always extend the quirks so that it's long long, or 
something like that, but this is also not sustainable in the long term.

-- 
Jiri Kosina

Re: [DOC] The documentation for HID Simple Driver Interface 0.5.0

[Marcel Holtmann]

Hi Jiri,

> > For me the task of converting HID reports into input events shouldn't be 
> > actually the job of the HID core layer. My understanding is that the HID 
> > core should support multiple transport layers. This is currently 
> > achieved through the hid_device abstraction and used by the USB and by 
> > the Bluetooth subsystem. This is the lower interface to HID. On the 
> > upper interface I like to see a driver like interface. So we can 
> > register specific drivers that can handle specific use cases or vendor 
> > specific reports. 
> 
> This I completely agree with, and have this on my TODO for quite a long 
> time. If Li Yu would like to spend some time on it, it certainly would 
> help. The current "simple HID interrface" is a suitable workaround for 
> various buggy/broken/nonstandard hardware which is currently not handled 
> by the HID kernel code properly, but it's definitely not a long-term 
> solution that should go to mainline.

my point is basically that we shouldn't waste any effort on a simple
driver while we actually need a generic driver interface. This generic
interface might be look simple and stupid in the beginning, but it can
easily grow over time. With this interface of course we need the concept
of a HID bus.

> > For standard keyboard and mouse reports we however should have a 
> > standard driver that can handle most of them.
> 
> Unfortunately there is a non-trivial bunch of hardware that pretends to be 
> standard keyboard/mouse, but behaves badly. Last week I had to do 
> workaround for Logitech S510, which seems to generate usages far above the 
> logical maximum specified in report descriptor, to give one example.
> 
> So we will probably end up with many small driver for exotic pieces of 
> hardware being registered to the hid bus. But this is definitely much 
> better than the current mess of hid quirks.

Maybe the report descriptor is broken. I have some Bluetooth HID where
they actually byte-swapped values and it totally confused the report
descriptor parser. For some devices it might be better to actually patch
the report descriptor instead of adding quirks to the HID core.

Regards

Marcel

[RFC] HID bus design overview.

[Li Yu]

--------------------------------------
    HID bus design overview.
--------------------------------------

A. Terms.

    The device of an driver: this mean the device that this driver matched.

B. Design.

    As we discussed before, The entire HID subsystem is divided into
three layers:
    
    1. HID Driver Layer (HIDDL).
        
        This layer is composite of all HID drivers, however, these
drivers are not equal each other, there are divided into three kinds:
        
        1.1 Fundamental driver.
                
        This is the most natural driver for us, they control hardware or
other something that can effect hardware directly. In most cases, each
of this kind driver is one standard and extendible implementation of HID
specification for specific HID transport layer. For example, usbhid.ko
for HID/USB or hidp.ko for Bluetooth/HID. This should alway input-able
driver, i.e, it is able to allow HIDAL register input device for its device.
                    
        1.2 Shadow driver.
        
        These driver is created for those buggy/strange/extended HID
device which fundamental driver can't handle rightly/completely. this
driver does not share HID information with related fundamental driver,
but the HID information of their devices is derived from other device of
fundamental driver. These may not be input-able driver. The shadow
driver does not share the input device with the related fundamental
driver, even shadow driver is input-able, it have new input device in
such case.
        
        1.3 HIDDEV drvier.
        
        In fact, This isn't one public kind of driver. it just is one
optionally flag parameter while register driver (also include shadow
driver). When turn on this feature, the HIDAL will create one hiddev for
each device of driver, so the every such device also have one hiddev buddy.
    
    2. HID Abstract Layer (HIDAL).        
    
        This layer maintain HID bus, HID driver, HID device, HID hiddev
such logical feature, and interact with input subsystem.

    3. HID Transport Layer (HIDTL).

        This layer may be rather thin. Up to now, we should have two
implementation of transport layer, one for USB, one for Bluetooth.

C. Progress.

Status: USB Fundamental driver works for new HID API now.
Working: Shadow driver support.
TODO:
    1. Shadow driver support.
    2. Port some drivers to new HID API, includes:
        1) All HID ff drivers.
        2) iBook/PowerBook special keys driver.
    3. Bluetooth fundamental driver.
    4. Bluetooth transport layer.
    5. Port Other drivers.
    6. HIDDEV/USB driver support.
    7. HIDDEV/Bluetooth driver support.

I think that post this text earlier is better decision, well, I guess
the new controversy about HID may begin ...
  
PS: In my words, the hiddev is same with rawdev, is it right in your words?
    I cann't acquire the support for this from my corporation, :( So, I
must complete this in my spare time, it may take longer time.

Goodluck.

-Li Yu

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Marcel Holtmann]

Hi Li,

> --------------------------------------
>     HID bus design overview.
> --------------------------------------
> 
> A. Terms.
> 
>     The device of an driver: this mean the device that this driver matched.
> 
> B. Design.
> 
>     As we discussed before, The entire HID subsystem is divided into
> three layers:
>     
>     1. HID Driver Layer (HIDDL).
>         
>         This layer is composite of all HID drivers, however, these
> drivers are not equal each other, there are divided into three kinds:
>         
>         1.1 Fundamental driver.
>                 
>         This is the most natural driver for us, they control hardware or
> other something that can effect hardware directly. In most cases, each
> of this kind driver is one standard and extendible implementation of HID
> specification for specific HID transport layer. For example, usbhid.ko
> for HID/USB or hidp.ko for Bluetooth/HID. This should alway input-able
> driver, i.e, it is able to allow HIDAL register input device for its device.
>                     
>         1.2 Shadow driver.
>         
>         These driver is created for those buggy/strange/extended HID
> device which fundamental driver can't handle rightly/completely. this
> driver does not share HID information with related fundamental driver,
> but the HID information of their devices is derived from other device of
> fundamental driver. These may not be input-able driver. The shadow
> driver does not share the input device with the related fundamental
> driver, even shadow driver is input-able, it have new input device in
> such case.
>         
>         1.3 HIDDEV drvier.
>         
>         In fact, This isn't one public kind of driver. it just is one
> optionally flag parameter while register driver (also include shadow
> driver). When turn on this feature, the HIDAL will create one hiddev for
> each device of driver, so the every such device also have one hiddev buddy.
>     
>     2. HID Abstract Layer (HIDAL).        
>     
>         This layer maintain HID bus, HID driver, HID device, HID hiddev
> such logical feature, and interact with input subsystem.
> 
>     3. HID Transport Layer (HIDTL).
> 
>         This layer may be rather thin. Up to now, we should have two
> implementation of transport layer, one for USB, one for Bluetooth.
> 
> C. Progress.
> 
> Status: USB Fundamental driver works for new HID API now.
> Working: Shadow driver support.
> TODO:
>     1. Shadow driver support.
>     2. Port some drivers to new HID API, includes:
>         1) All HID ff drivers.
>         2) iBook/PowerBook special keys driver.
>     3. Bluetooth fundamental driver.
>     4. Bluetooth transport layer.
>     5. Port Other drivers.
>     6. HIDDEV/USB driver support.
>     7. HIDDEV/Bluetooth driver support.
> 
> I think that post this text earlier is better decision, well, I guess
> the new controversy about HID may begin ...

please don't take the transport layers into account when designing the
HID bus. They have nothing to do with the upper layer. Especially the
upper layer shouldn't care at all if the actual HID reports are sent
over USB or Bluetooth.

The transport drivers (usbhid and hidp) will only communicate with the
HID core and for the bus it makes no difference. The current abstraction
for HID transports works pretty fine and there is no need to change
this.

I think that a HID bus driver should be able register to a specific HID
report for a specific device. This would allow to have a generic driver
that can handle all common keyboard and mouse functionality and then in
addition a specific driver that handles vendor specific reports. It
might also make sense to let the driver match on specific HUT codes.

Regards

Marcel

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Li Yu]

Marcel Holtmann wrote:
> please don't take the transport layers into account when designing the
> HID bus. They have nothing to do with the upper layer. Especially the
> upper layer shouldn't care at all if the actual HID reports are sent
> over USB or Bluetooth.
>
>   
I am sorry for I see this mail so later.

I also sense this. we may need not such a complete layer at all. Although the work of hiddev/rawdev support does not begin, however, as the design, especially, let hiddev/rawdev live in HIDAL, I think this may need a bit of abstract of transport layer.

> The transport drivers (usbhid and hidp) will only communicate with the
> HID core and for the bus it makes no difference. The current abstraction
> for HID transports works pretty fine and there is no need to change
> this.
>
>   
Nod. HIDAL should not care which transport layer works under it. if so, that is not HIDAL, that is HIDBL(HID Branch instruction Layer) instead of. :D

> I think that a HID bus driver should be able register to a specific HID
> report for a specific device. This would allow to have a generic driver
> that can handle all common keyboard and mouse functionality and then in
> addition a specific driver that handles vendor specific reports. It
> might also make sense to let the driver match on specific HUT codes.
>
>   
In current development implementation, the match process still can work in traditional Vendor ID/Product ID way, but it do not reject other means. If the author of driver hope to match for specific HID report, just do it. In fact, the HIDAL only know the result of match process, not how to do it.I think your "generic driver" is same with my "fundamental driver", and "specific driver" is same with "shadow driver" too.

Er, What's mean of your "HUT", HID Usage Table? if so, I think I have already explained we can do it.However, we should supply some convenient API for this work. or HIT is other mysterious thing? ;)

> Regards
>
> Marcel
>
>
>   
Progress: It seem the shadow basic support works! but It still need more tests.

Good luck.

- Li Yu

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Jiri Kosina]

On Wed, 28 Mar 2007, Li Yu wrote:

> I also sense this. we may need not such a complete layer at all. 
> Although the work of hiddev/rawdev support does not begin, however, as 
> the design, especially, let hiddev/rawdev live in HIDAL, I think this 
> may need a bit of abstract of transport layer.

JFYI the preliminary version of the hidraw interface is now in the 
hid/usbhid git tree, and has also been in a few recent -mm kernels 
already.

> In current development implementation, the match process still can work 
> in traditional Vendor ID/Product ID way, but it do not reject other 
> means. If the author of driver hope to match for specific HID report, 
> just do it. In fact, the HIDAL only know the result of match process, 
> not how to do it.

The crucial thing here is that all reports but the ones that the driver 
registered to will be processed in a standard way by the generic hid bus 
layer, and those reports that the driver registered to will be ignored by 
the layer, and passed for processing to the driver.

> Er, What's mean of your "HUT", HID Usage Table? if so, I think I have
> already explained we can do it.However, we should supply some convenient
> API for this work. or HIT is other mysterious thing? ;)

Yes, HUT is Hid Usage Table. You can obtain them from 
http://www.usb.org/developers/hidpage/#Usage_Tables

Thanks,

-- 
Jiri Kosina

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Dmitry Torokhov]

On 3/28/07, Jiri Kosina <jkosina@suse.cz> wrote:
>
> The crucial thing here is that all reports but the ones that the driver
> registered to will be processed in a standard way by the generic hid bus
> layer, and those reports that the driver registered to will be ignored by
> the layer, and passed for processing to the driver.
>

I don't think it is a good idea to register driver for specific
usages/reports. Quite often you want to adjust processing of a report
for a specific device. What if there are 2 devices that need such
quirks? How will you do hotplug and module loading? Emit new uevent
for every report? Also, what about users and Kconfig? "Driver for
usage 0x000012345. Say Y if your hardware does not wotk correctly with
defautl handler for this usage and require special processing"???

Just register based on VID/PID and provide standard
hid_default_input_event() to drivers so they would call it for reports
they don't need to do special processing on.

-- 
Dmitry

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Jiri Kosina]

On Wed, 28 Mar 2007, Dmitry Torokhov wrote:

> Just register based on VID/PID and provide standard 
> hid_default_input_event() to drivers so they would call it for reports 
> they don't need to do special processing on.

Agreed. This is actually what I meant in the original mail, but phrased it 
very badly, sorry for confusion.

Thanks,

-- 
Jiri Kosina

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Li Yu]

Jiri Kosina wrote:
> JFYI the preliminary version of the hidraw interface is now in the 
> hid/usbhid git tree, and has also been in a few recent -mm kernels 
> already.
>
>   
The shadow driver support works now.

The most largest problem is HID/Bluetooth can not work now. And, I have
no any bluetooth input device to test, So ...

I think I should port current implementation to 2.6.21-rc5-mm2, and
support hiddev, then release it.

The last word is a question, what's the future of hiddev? It will merge
into hidraw later?  I think so, but can't sure.

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Jiri Kosina]

On Thu, 29 Mar 2007, Li Yu wrote:

> The last word is a question, what's the future of hiddev? It will merge 
> into hidraw later?  I think so, but can't sure.

Hi Li,

no, it won't, it doesn't provide compatible interface for purpose.

hiddev has currently the following drawbacks:

- USB-transport specific

- uses in-kernel HID parser to parse reports and fill in usages and 
  values. This is usually not what authors of the userspace drivers for 
  HID devices want - the devices often have quirks or behave in a strange 
  way, and making workarounds is more complicated when kernel parser 
  operates on the received reports

- only a few applications use it (acupsd, nut, hid2hcianything else?). All 
  other drivers rather use libhid, which is built on top of libusb, in 
  order to be able to receive and send really raw HID reports, and parse 
  them on their own. This is however also USB-transport specific

The purpose of hidraw is to provide the applications the same 
functionality they are having when using libhid/libusb, but in a 
transport-independent way, as it is hooked to generic HID layer, which 
works as a 'proxy' for different transports that use it.

-- 
Jiri Kosina

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Li Yu]

Dmitry Torokhov wrote:
> On 3/28/07, Jiri Kosina <jkosina@suse.cz> wrote:
>>
>> The crucial thing here is that all reports but the ones that the driver
>> registered to will be processed in a standard way by the generic hid bus
>> layer, and those reports that the driver registered to will be
>> ignored by
>> the layer, and passed for processing to the driver.
>>
>
> I don't think it is a good idea to register driver for specific
> usages/reports. Quite often you want to adjust processing of a report
> for a specific device. What if there are 2 devices that need such
> quirks? How will you do hotplug and module loading? Emit new uevent
> for every report? Also, what about users and Kconfig? "Driver for
> usage 0x000012345. Say Y if your hardware does not wotk correctly with
> defautl handler for this usage and require special processing"???
>
> Just register based on VID/PID and provide standard
> hid_default_input_event() to drivers so they would call it for reports
> they don't need to do special processing on.
>

    I think the shadow driver can not share inputdev with the related
fundamental driver, so here are two input devices for one hid_device,
how we should process both? It seem we have three choices:

1. Shadow | Fundamental means

    I think this is Jiri said. Fundamental driver handle all common
input events, and Shadow driver handle anther specific input events,
this imply user space process need monitor both input devices at same
time, I do not think this is good idea.

2. Shadow & Fundamental means

    Let Fundamental driver and Shadow driver work at same time! but
shadow also handle specific input event. So, the user may get twice
input events! For example, the keyboard input in console.
Also, I do not like this.

3. Shadow ^ Fundamental means

    Let Shadow driver handle every thing, and fundamental device silent,
even unregister it. I think this is best choice between them, this means
have a bit of complex.

Welcome for your words.

PS: Have we need more than one shadow driver for same fundamental
driver? I do not think so.

Good luck.

- Li Yu

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Dmitry Torokhov]

On Thursday 29 March 2007 23:06, Li Yu wrote:
> Dmitry Torokhov wrote:
> > On 3/28/07, Jiri Kosina <jkosina@suse.cz> wrote:
> >>
> >> The crucial thing here is that all reports but the ones that the driver
> >> registered to will be processed in a standard way by the generic hid bus
> >> layer, and those reports that the driver registered to will be
> >> ignored by
> >> the layer, and passed for processing to the driver.
> >>
> >
> > I don't think it is a good idea to register driver for specific
> > usages/reports. Quite often you want to adjust processing of a report
> > for a specific device. What if there are 2 devices that need such
> > quirks? How will you do hotplug and module loading? Emit new uevent
> > for every report? Also, what about users and Kconfig? "Driver for
> > usage 0x000012345. Say Y if your hardware does not wotk correctly with
> > defautl handler for this usage and require special processing"???
> >
> > Just register based on VID/PID and provide standard
> > hid_default_input_event() to drivers so they would call it for reports
> > they don't need to do special processing on.
> >
> 
>     I think the shadow driver can not share inputdev with the related
> fundamental driver, so here are two input devices for one hid_device,
> how we should process both? It seem we have three choices:
> 
> 1. Shadow | Fundamental means
> 
>     I think this is Jiri said. Fundamental driver handle all common
> input events, and Shadow driver handle anther specific input events,
> this imply user space process need monitor both input devices at same
> time, I do not think this is good idea.
> 
> 2. Shadow & Fundamental means
> 
>     Let Fundamental driver and Shadow driver work at same time! but
> shadow also handle specific input event. So, the user may get twice
> input events! For example, the keyboard input in console.
> Also, I do not like this.
> 
> 3. Shadow ^ Fundamental means
> 
>     Let Shadow driver handle every thing, and fundamental device silent,
> even unregister it. I think this is best choice between them, this means
> have a bit of complex.
>


There should be one device and your driver should simply do:

static void my_driver_hid_event(struct hid_device *hid, struct hid_field *field,
				struct hid_usage *usage, __s32 value)
{
	if (special_processing_needed(usage)) {
		do_special_processing(...);
		input_event(field->hidinput->input, XXX, YYY, ZZZ);
		...

	} else
		hidinput_hid_event(hid, field, usage, value);
}

That is pretty much it. Your driver is not a shadow driver, it is
regular driver on HID bus that just happens to use generic hander
for standard events.

-- 
Dmitry

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Li Yu]

Dmitry Torokhov wrote:
> On Thursday 29 March 2007 23:06, Li Yu wrote:
>   
>> Dmitry Torokhov wrote:
>>     
>>> On 3/28/07, Jiri Kosina <jkosina@suse.cz> wrote:
>>>       
>>>> The crucial thing here is that all reports but the ones that the driver
>>>> registered to will be processed in a standard way by the generic hid bus
>>>> layer, and those reports that the driver registered to will be
>>>> ignored by
>>>> the layer, and passed for processing to the driver.
>>>>
>>>>         
>>> I don't think it is a good idea to register driver for specific
>>> usages/reports. Quite often you want to adjust processing of a report
>>> for a specific device. What if there are 2 devices that need such
>>> quirks? How will you do hotplug and module loading? Emit new uevent
>>> for every report? Also, what about users and Kconfig? "Driver for
>>> usage 0x000012345. Say Y if your hardware does not wotk correctly with
>>> defautl handler for this usage and require special processing"???
>>>
>>> Just register based on VID/PID and provide standard
>>> hid_default_input_event() to drivers so they would call it for reports
>>> they don't need to do special processing on.
>>>
>>>       
>>     I think the shadow driver can not share inputdev with the related
>> fundamental driver, so here are two input devices for one hid_device,
>> how we should process both? It seem we have three choices:
>>
>> 1. Shadow | Fundamental means
>>
>>     I think this is Jiri said. Fundamental driver handle all common
>> input events, and Shadow driver handle anther specific input events,
>> this imply user space process need monitor both input devices at same
>> time, I do not think this is good idea.
>>
>> 2. Shadow & Fundamental means
>>
>>     Let Fundamental driver and Shadow driver work at same time! but
>> shadow also handle specific input event. So, the user may get twice
>> input events! For example, the keyboard input in console.
>> Also, I do not like this.
>>
>> 3. Shadow ^ Fundamental means
>>
>>     Let Shadow driver handle every thing, and fundamental device silent,
>> even unregister it. I think this is best choice between them, this means
>> have a bit of complex.
>>
>>     
>
>
> There should be one device and your driver should simply do:
>
> static void my_driver_hid_event(struct hid_device *hid, struct hid_field *field,
> 				struct hid_usage *usage, __s32 value)
> {
> 	if (special_processing_needed(usage)) {
> 		do_special_processing(...);
> 		input_event(field->hidinput->input, XXX, YYY, ZZZ);
> 		...
>
> 	} else
> 		hidinput_hid_event(hid, field, usage, value);
> }
>
> That is pretty much it. Your driver is not a shadow driver, it is
> regular driver on HID bus that just happens to use generic hander
> for standard events.
>
>   

I think I can understand your words. but I need confirm:

Before specific driver register this device, the
fundamental/standard/common(select one by your mind:) driver had already
attach on it likely. At this case, we should detach this hid_device from
its working driver, let this hid_device attach with our new specific
driver. then new driver will handle all input event later, so the your
words is same with the third choice in fact, is it right? if so, the
actual behavior is same with former HID simple interface.

Good luck.

- Li Yu

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Dmitry Torokhov]

On 3/30/07, Li Yu <raise.sail@gmail.com> wrote:
> I think I can understand your words. but I need confirm:
>
> Before specific driver register this device, the
> fundamental/standard/common(select one by your mind:) driver had already
> attach on it likely. At this case, we should detach this hid_device from
> its working driver, let this hid_device attach with our new specific
> driver. then new driver will handle all input event later, so the your
> words is same with the third choice in fact, is it right? if so, the
> actual behavior is same with former HID simple interface.
>

I was thinking that as we write customized drivers we would add them
(manually or automatically) to the HID blacklist so that generic
driver would not bind to such devices. Then, as your driver loads it
would parse reports and construct input device structure in the same
fashion that it processes the reports - if it is an "interesting"
report/usage it will handle setup itself; otherwise just call generic
setup function shared with the generic HID driver. This way there is
only one input device and there is no storm of add/remove events as we
loking for the proper driver to bind to the device.

Does this make sense?

This would not quite work for (potentially out of tree) HID drivers
brought in after compiling generic HID driver but that should be a
very rate occurance and we still can manually bind such drivers via
sysfs bind/unbind and new_id manipulations.

-- 
Dmitry

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Jiri Kosina]

On Fri, 30 Mar 2007, Dmitry Torokhov wrote:

> There should be one device and your driver should simply do:
> static void my_driver_hid_event(struct hid_device *hid, struct hid_field *field,
> 				struct hid_usage *usage, __s32 value)
> {
> 	if (special_processing_needed(usage)) {
> 		do_special_processing(...);
> 		input_event(field->hidinput->input, XXX, YYY, ZZZ);
> 		...
> 
> 	} else
> 		hidinput_hid_event(hid, field, usage, value);
> }

Hi,

in fact I am not entirely sure that the specialized drivers hooked to the 
HID bus should be passed individual fields/usages by the generic HID 
driver. That would imply that generic HID layer would have to parse the 
received report using information retrieved from the report descriptor of 
the device. But this is in some way in contrary to one of the features 
this effort should be heading to, isn't it? We want to provide means how 
to bypass possible errors in HID descriptor of the device (or do any other 
possible quirky handling) - we want to be able to allow for completely 
different interpretation of fields than the generic HID parser would do, 
right?

So I guess the above should rather be

static void my_driver_hid_report(struct hid_device *hid, u8 *data, 
				 int size)
{
      if (special_processing_needed(data)) {
              do_special_processing(...);
              input_event(field->hidinput->input, XXX, YYY, ZZZ);
              ...
      } else
              hid_input_report(hid, data, size);
}


Such driver will register itself onto a HID bus. Both USB and BT 
transports could provide VID and PID which could then be easily matched 
against by the bus code to easily check whether processing by specialized 
driver is needed or handling by (existing) generic HID layer is enough.

As an added value, hooking the hidraw code to this architecture would then 
be rather a trivial task.

Thanks,

-- 
Jiri Kosina

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Li Yu]

Jiri Kosina wrote:
> On Fri, 30 Mar 2007, Dmitry Torokhov wrote:
>
>   
>> There should be one device and your driver should simply do:
>> static void my_driver_hid_event(struct hid_device *hid, struct hid_field *field,
>> 				struct hid_usage *usage, __s32 value)
>> {
>> 	if (special_processing_needed(usage)) {
>> 		do_special_processing(...);
>> 		input_event(field->hidinput->input, XXX, YYY, ZZZ);
>> 		...
>>
>> 	} else
>> 		hidinput_hid_event(hid, field, usage, value);
>> }
>>     
>
> Hi,
>
> in fact I am not entirely sure that the specialized drivers hooked to the 
> HID bus should be passed individual fields/usages by the generic HID 
> driver. That would imply that generic HID layer would have to parse the 
> received report using information retrieved from the report descriptor of 
> the device. But this is in some way in contrary to one of the features 
> this effort should be heading to, isn't it? We want to provide means how 
> to bypass possible errors in HID descriptor of the device (or do any other 
> possible quirky handling) - we want to be able to allow for completely 
> different interpretation of fields than the generic HID parser would do, 
> right?
>
> So I guess the above should rather be
>
> static void my_driver_hid_report(struct hid_device *hid, u8 *data, 
> 				 int size)
> {
>       if (special_processing_needed(data)) {
>               do_special_processing(...);
>               input_event(field->hidinput->input, XXX, YYY, ZZZ);
>               ...
>       } else
>               hid_input_report(hid, data, size);
> }
>
>
> Such driver will register itself onto a HID bus. Both USB and BT 
> transports could provide VID and PID which could then be easily matched 
> against by the bus code to easily check whether processing by specialized 
> driver is needed or handling by (existing) generic HID layer is enough.
>
> As an added value, hooking the hidraw code to this architecture would then 
> be rather a trivial task.
>
> Thanks,
>
>   
Let me explain the internal of my current HID bus implementation. I
think that selecting one user scene as example is good idea.

Well, the user A plug a USB joystick into computer. The work processing
of HID subsystem for this joystick is same with our mind: The USB/base
driver works for it. However, the world is not faultless. This joystick
is buggy, its some keys need specific handling, our good friend udev
discover that there is also have another driver can handle it, so it
insert that kernel module, then our sweet leading role specific/shadow
HID driver appear on scene. Registering shadow driver let HID core clone
a hid_device first, and start new usage configuration processing for new
cloned hid_device (if we like, even I think we can reread reports from
physical device), the shadow driver can join with HID core to
custom/hook in this recofiguration processing. If this shadow driver is
input-able, the HID core will unregister working input device come from
USB/base hid_device, and register new input device for this shadow
hid_device. So we have not two input_dev for one HID device at same time.

When user A remove this shadow driver, the USB/base driver should resume
work for this joystick, IOW, it should register back its input device again.

There have another type driver: the sticky driver, they will attach each
working device without clone or create new hid_device or input_dev. the
example is hiddev and hidraw.

Yes, this model imply there need more uevents: add, remove. Is this
model suitable? I think the essence of our discussion is how to handle
unknown abnormal device.

I had already ported this to 2.6.21-rc5-mm2, and spent almost two days
to find out one concurrency problem. I lucky known where have bug, but
not fixed it.

Good luck.

- Li Yu

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Dmitry Torokhov]

On Saturday 31 March 2007 18:49, Jiri Kosina wrote:
> 
> Hi,
> 
> in fact I am not entirely sure that the specialized drivers hooked to the 
> HID bus should be passed individual fields/usages by the generic HID 
> driver. That would imply that generic HID layer would have to parse the 
> received report using information retrieved from the report descriptor of 
> the device. But this is in some way in contrary to one of the features 
> this effort should be heading to, isn't it? We want to provide means how 
> to bypass possible errors in HID descriptor of the device (or do any other 
> possible quirky handling) - we want to be able to allow for completely 
> different interpretation of fields than the generic HID parser would do, 
> right?
> 
> So I guess the above should rather be
> 
> static void my_driver_hid_report(struct hid_device *hid, u8 *data, 
> 				 int size)
> {
>       if (special_processing_needed(data)) {
>               do_special_processing(...);
>               input_event(field->hidinput->input, XXX, YYY, ZZZ);
>               ...
>       } else
>               hid_input_report(hid, data, size);
> }
> 

Well, this of course is most flexible, however I think that for most
drivers hooking into parsed data would be much easier. That means that
we need to allow defining 2 hooks - one for raw data and another for
parsed reports and let drivers decice which one they want to use.

-- 
Dmitry

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Dmitry Torokhov]

On Sunday 01 April 2007 21:47, Li Yu wrote:
> Let me explain the internal of my current HID bus implementation. I
> think that selecting one user scene as example is good idea.
> 
> Well, the user A plug a USB joystick into computer. The work processing
> of HID subsystem for this joystick is same with our mind: The USB/base
> driver works for it. However, the world is not faultless. This joystick
> is buggy, its some keys need specific handling, our good friend udev
> discover that there is also have another driver can handle it, so it
> insert that kernel module, then our sweet leading role specific/shadow
> HID driver appear on scene. Registering shadow driver let HID core clone
> a hid_device first, and start new usage configuration processing for new
> cloned hid_device (if we like, even I think we can reread reports from
> physical device), the shadow driver can join with HID core to
> custom/hook in this recofiguration processing. If this shadow driver is
> input-able, the HID core will unregister working input device come from
> USB/base hid_device, and register new input device for this shadow
> hid_device. So we have not two input_dev for one HID device at same time.
>

No, please don't do that. As soon as there is a special driver written
for a device that device's VID/PID should be added to generic HID
blacklist. This way udev will load the proper driver right away and
there will not be any flip-flopping of input devices. 
 
> When user A remove this shadow driver, the USB/base driver should resume
> work for this joystick, IOW, it should register back its input device again.
> 

Why would we want to revert to using generic HID's implementation if we
know that it is broken for that particular device???

-- 
Dmitry

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Li Yu]

Dmitry Torokhov wrote:
>
> No, please don't do that. As soon as there is a special driver written
> for a device that device's VID/PID should be added to generic HID
> blacklist. This way udev will load the proper driver right away and
> there will not be any flip-flopping of input devices. 
>   
Hi, I do not think that using blacklist in base driver for this purpose
is good idea. If so, we need modify source when each new HID device
driver come, that's so ugly. I think the blacklist only should be used
for those really broken/buggy hardware, not for these normal hardware
with extended feature.

Er, I also want to know what are drawbacks of "flip-flopping" ?


>  
>   
>> When user A remove this shadow driver, the USB/base driver should resume
>> work for this joystick, IOW, it should register back its input device again.
>>
>>     
>
> Why would we want to revert to using generic HID's implementation if we
> know that it is broken for that particular device???
>
>   

Well, many devices just only can not play its full feature, not broken.
so base driver still can work for us.

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Greg KH]

On Mon, Apr 02, 2007 at 03:07:29PM +0800, Li Yu wrote:
> Dmitry Torokhov wrote:
> >
> > No, please don't do that. As soon as there is a special driver written
> > for a device that device's VID/PID should be added to generic HID
> > blacklist. This way udev will load the proper driver right away and
> > there will not be any flip-flopping of input devices. 
> >   
> Hi, I do not think that using blacklist in base driver for this purpose
> is good idea. If so, we need modify source when each new HID device
> driver come, that's so ugly. I think the blacklist only should be used
> for those really broken/buggy hardware, not for these normal hardware
> with extended feature.
> 
> Er, I also want to know what are drawbacks of "flip-flopping" ?

You have to create some kind of userspace program to disconnect the
device from the hid driver, and bind it to the new driver, for every new
device/driver that comes along.

Yes, you can do this, but the blacklist is simpler and easier.

thanks,

greg k-h

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Jiri Kosina]

On Mon, 2 Apr 2007, Li Yu wrote:

> Hi, I do not think that using blacklist in base driver for this purpose 
> is good idea. If so, we need modify source when each new HID device 
> driver come, that's so ugly. 

Hi Li,

well, the drivers are exceptions from the generic handling, so creating an 
exceptional rule (entry in hid_blacklist) for them is not that bad. OK, 
it's not the nicest thing on earth probably, but serves the purpose in 
current vendors-trying-to-break-hardware-in-the-most-original-way world 
quite well.

This is going to cause some headache to out-of-tree drivers. Oh well, do 
we care that much? 

In your scenario you'll still need a way how to unbind the device from the 
hid driver and bind it to the new specific device, won't you?. This could 
be done by a separate userspace program, but well ... I think just 
blacklisting the hardware is much cleaner, than needing to ship a separate 
userspace program along with the driver.

Thanks,

-- 
Jiri Kosina

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Jiri Kosina]

On Mon, 2 Apr 2007, Dmitry Torokhov wrote:

> > static void my_driver_hid_report(struct hid_device *hid, u8 *data, 
> > 				 int size)
> > {
> >       if (special_processing_needed(data)) {
> >               do_special_processing(...);
> >               input_event(field->hidinput->input, XXX, YYY, ZZZ);
> >               ...
> >       } else
> >               hid_input_report(hid, data, size);
> > }
> > 
> Well, this of course is most flexible, however I think that for most
> drivers hooking into parsed data would be much easier. That means that
> we need to allow defining 2 hooks - one for raw data and another for
> parsed reports and let drivers decice which one they want to use.

I agree. I am aware of devices for which just inspecting the parsed data 
would be OK (some keyboards with usage mappings which are not defined by 
HUT, for example), but also of devices which require special handling on 
the report level - Robert Marquardt pointed me in a private mail to a few 
devices which are broken par-excellence, and for which handling on report 
level would be convenient.

Also, handling on report level would be nice to have so that we could hook 
a hidraw driver to it.

Li, would this be OK by you?

Thanks,

-- 
Jiri Kosina

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Robert Marquardt]

Jiri Kosina wrote:

> I agree. I am aware of devices for which just inspecting the parsed data 
> would be OK (some keyboards with usage mappings which are not defined by 
> HUT, for example), but also of devices which require special handling on 
> the report level - Robert Marquardt pointed me in a private mail to a few 
> devices which are broken par-excellence, and for which handling on report 
> level would be convenient.
> 
> Also, handling on report level would be nice to have so that we could hook 
> a hidraw driver to it.

Just forgot to use "Reply All".

Here is the text (expanded):
Definitely. I know of several devices where the HID descriptor and the 
data sent completely disagree.
A Metex USB Multimeter for example tells that it has a 4 byte report 
divided into a 3 byte and a 1 byte element. The device sends two reports 
each containing two 2 byte elements with one in the second report not 
containing data.
The wierdest device is a CD archive torus which shows up as a simple 
standard mouse. Commands are sent by *reading* strings.
The Office Rocket Launcher is posing as a keyboard and reacts to the 
LEDs being set (this is of interest for the Num-Lock thread here).
I definitely hope that the latter two devices will never get official 
kernel support in any way.

Why is the report parsed on driver level at all? It should be possible 
to allow drivers to manipulate the reports, but i see no reason to split 
the report into usages that early. The Windows HID API parses the report 
happily in the HID.DLL for user programs. Drivers have the same API 
available on kernel level.
So a raw API for the drivers should be the primary API. It is usually 
easier to manipulate the report directly instead of abstract operations. 
It is also fairly safe. The device has been identified so the offending 
report layout can be presumed. Parsing for specific usages should be 
done from the driver. Preparsing the report before the data is handed to 
the driver only wastes CPU cycles.

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Marcel Holtmann]

Hi Li,

> > JFYI the preliminary version of the hidraw interface is now in the 
> > hid/usbhid git tree, and has also been in a few recent -mm kernels 
> > already.
> >
> >   
> The shadow driver support works now.
> 
> The most largest problem is HID/Bluetooth can not work now. And, I have
> no any bluetooth input device to test, So ...

if this doesn't work, then the complete design is bogus. There is no
difference between USB and Bluetooth. The HID core takes care of
abstracting it.

Regards

Marcel

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Marcel Holtmann]

Hi Dmitry,

> > The crucial thing here is that all reports but the ones that the driver
> > registered to will be processed in a standard way by the generic hid bus
> > layer, and those reports that the driver registered to will be ignored by
> > the layer, and passed for processing to the driver.
> >
> 
> I don't think it is a good idea to register driver for specific
> usages/reports. Quite often you want to adjust processing of a report
> for a specific device. What if there are 2 devices that need such
> quirks? How will you do hotplug and module loading? Emit new uevent
> for every report? Also, what about users and Kconfig? "Driver for
> usage 0x000012345. Say Y if your hardware does not wotk correctly with
> defautl handler for this usage and require special processing"???
> 
> Just register based on VID/PID and provide standard
> hid_default_input_event() to drivers so they would call it for reports
> they don't need to do special processing on.

I like this idea, but it might not solve the case where you have parts
of the driver in kernel space and other parts in user space. For example
the control of a LCD display on the keyboard. However in most cases
registering drivers for a report id should be enough.

Regards

Marcel

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Jiri Kosina]

On Mon, 2 Apr 2007, Marcel Holtmann wrote:

> I like this idea, but it might not solve the case where you have parts 
> of the driver in kernel space and other parts in user space. For example 
> the control of a LCD display on the keyboard. However in most cases 
> registering drivers for a report id should be enough.

Hi Marcel,

the specialized driver could hook on all reports of the device (as 
discussed a few mails ago in this thread) and have the possibility to do 
three different things with the obtained report:

- pass it back to generic hid driver for "standard" processing
- process the report, and issue input_event() itself
- pass it to hidraw and let userspace to consume it

This is going to work for the scenario you have described, right?

Thanks,

-- 
Jiri Kosina

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Dmitry Torokhov]

On 4/2/07, Jiri Kosina <jkosina@suse.cz> wrote:
> On Mon, 2 Apr 2007, Li Yu wrote:
>
> > Hi, I do not think that using blacklist in base driver for this purpose
> > is good idea. If so, we need modify source when each new HID device
> > driver come, that's so ugly.
>
> Hi Li,
>
> well, the drivers are exceptions from the generic handling, so creating an
> exceptional rule (entry in hid_blacklist) for them is not that bad. OK,
> it's not the nicest thing on earth probably, but serves the purpose in
> current vendors-trying-to-break-hardware-in-the-most-original-way world
> quite well.
>

Potentially we could even generate 2nd blacklist table automatically
by scanning MODULE_DEVICE_TABLEs for all drivers in HID directory and
merge it with the table we currently maintain in hid-input.c

> This is going to cause some headache to out-of-tree drivers. Oh well, do
> we care that much?

They still can be made to work using bind/unbind via sysfs, but at
this moment there are 0 drivers for HID bus and hopefully people will
be submittin them for inclusion.

-- 
Dmitry

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Marcel Holtmann]

Hi Jiri,

> > I like this idea, but it might not solve the case where you have parts 
> > of the driver in kernel space and other parts in user space. For example 
> > the control of a LCD display on the keyboard. However in most cases 
> > registering drivers for a report id should be enough.
> 
> the specialized driver could hook on all reports of the device (as 
> discussed a few mails ago in this thread) and have the possibility to do 
> three different things with the obtained report:
> 
> - pass it back to generic hid driver for "standard" processing
> - process the report, and issue input_event() itself
> - pass it to hidraw and let userspace to consume it
> 
> This is going to work for the scenario you have described, right?

it will work. However I am not sure that is the best design. We need to
make one round-trip into an extra driver. We might gonna need these kind
of driver that hook in-between and does nasty things anyway, because of
broken HID devices.

The cleanest solution without a layer violation is that you can register
a driver for a specific VID/PID and then report id (one or more). All
reports with ids that we don't have a special driver for are handled by
the default HID->input driver or handed over to hidraw if not parseable.
The reports for ids with a special driver are handed over to the driver.

And for hidraw it would be nice if we can apply filters for specific
report ids to keep the round-trips and overhead at a minimum.

Regards

Marcel

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Li Yu]

Marcel Holtmann wrote:
> The cleanest solution without a layer violation is that you can
> register a driver for a specific VID/PID and then report id (one or
> more). All
> reports with ids that we don't have a special driver for are handled by
> the default HID->input driver or handed over to hidraw if not parseable.
> The reports for ids with a special driver are handed over to the driver.
>
> And for hidraw it would be nice if we can apply filters for specific
> report ids to keep the round-trips and overhead at a minimum.
>
>   
If we don't use "flip-flopping" means, the common driver and specific
driver concepts also don't need. They are completely same driver for HID
bus, just one without some hooks, another without. The common event
processing is an API from HID core. so, here have not round-trips.

What's the position of hidraw? It only is used when all other driver is
not usable on some report? or, it should be stick every working device.

PS: In last broken "flip-flopping" resolution, the USBHID work also need
some changes ;)

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Dmitry Torokhov]

On Monday 02 April 2007 21:15, Li Yu wrote:
> 
> If we don't use "flip-flopping" means, the common driver and specific
> driver concepts also don't need. They are completely same driver for HID
> bus, just one without some hooks, another without.

Exactly. I am glad we are getting on the same page.

-- 
Dmitry

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Jiri Kosina]

On Tue, 3 Apr 2007, Li Yu wrote:

> What's the position of hidraw? It only is used when all other driver is 
> not usable on some report? or, it should be stick every working device.

Current implementation (as you can see it in -mm or in my hid.git tree) is 
creating hidraw interface for just every HID device/interface. But this 
will get changed before merge.

Passing just everything to hidraw is not a good option, as this could lead 
to confusion and duplicating of input events (i.e. in-kernel hid driver 
processes the report and generates input_event(), and also userland driver 
obtains data from hidraw and generates input event through uinput ... not 
good).

> PS: In last broken "flip-flopping" resolution, the USBHID work also need
> some changes ;)

BTW as soon as you have some presentable code, could you please send it so 
that we could see what aproach you have taken? Debating over code is 
usualy more efficient than just ranting random ideas :)

Thanks,

-- 
Jiri Kosina

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Li Yu]

Jiri Kosina wrote:
> Current implementation (as you can see it in -mm or in my hid.git tree) is 
> creating hidraw interface for just every HID device/interface. But this 
> will get changed before merge.
>
> Passing just everything to hidraw is not a good option, as this could lead 
> to confusion and duplicating of input events (i.e. in-kernel hid driver 
> processes the report and generates input_event(), and also userland driver 
> obtains data from hidraw and generates input event through uinput ... not 
> good).
>
>   
After hearing your words, I change my implementation use hidraw as an
event filter, IOW, the HID core will handle these events which hidraw
can or need not handle. The modification is easy.

> BTW as soon as you have some presentable code, could you please send it so 
> that we could see what aproach you have taken? Debating over code is 
> usualy more efficient than just ranting random ideas :)
>
> Thanks,
>
>   

Of course, It's no problem. but it must be tomorrow, two reasons:

1. The development PC is at my home, I am not SOHO.
2. After change from "flip-flopping", the patch need some cleaning.

Good luck.

- Li Yu

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Marcel Holtmann]

Hi Jiri,

> > What's the position of hidraw? It only is used when all other driver is 
> > not usable on some report? or, it should be stick every working device.
> 
> Current implementation (as you can see it in -mm or in my hid.git tree) is 
> creating hidraw interface for just every HID device/interface. But this 
> will get changed before merge.
> 
> Passing just everything to hidraw is not a good option, as this could lead 
> to confusion and duplicating of input events (i.e. in-kernel hid driver 
> processes the report and generates input_event(), and also userland driver 
> obtains data from hidraw and generates input event through uinput ... not 
> good).

at some point I thought it would be nice to have something like hiddump
(like tcpdump), but that can be easily achieved with hcidump and usbmon
on the lower level.

So if hidraw claims a report id, the kernel should no longer handle it.

Regards

Marcel

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Adam Kropelin]

Jiri Kosina wrote:
> On Tue, 3 Apr 2007, Li Yu wrote:
>
>> What's the position of hidraw? It only is used when all other driver
>> is not usable on some report? or, it should be stick every working
>> device.
>
> Current implementation (as you can see it in -mm or in my hid.git
> tree) is creating hidraw interface for just every HID
> device/interface. But this will get changed before merge.
>
> Passing just everything to hidraw is not a good option, as this could
> lead to confusion and duplicating of input events (i.e. in-kernel hid
> driver processes the report and generates input_event(), and also
> userland driver obtains data from hidraw and generates input event
> through uinput ... not good).

I apologize for picking up this thread late and asking what may be a 
question with an obvious answer...

Will hiddev still exist after hidraw and the HID bus redesign work is 
done? I have a widely-deployed userspace app that relies on hiddev, and 
I'm looking for reassurance that it will still work as it always has...

--Adam

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Jiri Kosina]

On Wed, 4 Apr 2007, Adam Kropelin wrote:

> I apologize for picking up this thread late and asking what may be a 
> question with an obvious answer... Will hiddev still exist after hidraw 
> and the HID bus redesign work is done? I have a widely-deployed 
> userspace app that relies on hiddev, and I'm looking for reassurance 
> that it will still work as it always has...

Hi Adam,

hiddev will have to stay for quite some time, exactly because of backward 
compatibility with userspace applications/drivers that use it (I am not 
aware of many of them though, but apparently there are some). I won't 
allow it to vanish, don't worry.

We just have to make sure that new users will use hidraw instead, as it 
provides more flexibility for the user, is not dependent on the underlying 
transport protocol, etc.

-- 
Jiri Kosina

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Adam Kropelin]

Jiri Kosina wrote:
> On Wed, 4 Apr 2007, Adam Kropelin wrote:
>
>> I apologize for picking up this thread late and asking what may be a
>> question with an obvious answer... Will hiddev still exist after
>> hidraw and the HID bus redesign work is done? I have a
>> widely-deployed userspace app that relies on hiddev, and I'm looking
>> for reassurance that it will still work as it always has...
>
> Hi Adam,
>
> hiddev will have to stay for quite some time, exactly because of
> backward compatibility with userspace applications/drivers that use
> it (I am not aware of many of them though, but apparently there are
> some).

Apcupsd is the one on my mind, but I believe there are others.

> I won't allow it to vanish, don't worry.

Thanks!

> We just have to make sure that new users will use hidraw instead, as
> it provides more flexibility for the user, is not dependent on the
> underlying transport protocol, etc.

On Apcupsd we've recently introduced a libusb-based driver that does all 
HID parsing in userspace. Not only does that free us from hiddev, it 
also frees us from the umpteen other proprietary HID interfaces across 
various platforms. Although the hiddev-based driver is still the default 
for Linux platforms, I plan to change that in the next major release and 
thus begin migrating folks off of hiddev.

I appreciate your pledge to keep hiddev functioning in the mean time :)

--Adam

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Li Yu]

[-- Attachment #1: Type: text/plain, Size: 1088 bytes --]

Jiri Kosina wrote:
> BTW as soon as you have some presentable code, could you please send
> it so
> that we could see what aproach you have taken? Debating over code is 
> usualy more efficient than just ranting random ideas :)
>
>   
There is a "presentable patch" in the attachment ;)

so far, the bluetooth also can not work :( TODO, TODO

TODO list include usbhid blacklist match logic should be move to HID
core too.

But It have a very very strange problem on my PC yet. I have two USB
input devices at that machine, one is a wireless mouse, another is a
joystick. When I unplug joystick, the input_dev of mouse also will be
unregistered, I found the unregister_hid_device() is called from
hid_disconnect().

I can sure the hardware have no problem, they work fine under 2.6.17.9
without any change of kernel. May, I doublt it is caused by illegal
memory access likely, or I failed to understand USB core ?! FIXING, The
fun also here.

The hiddump is good idea, I like, however, I think hidraw just is it.
the hiddump is one application of hidraw. is it right?

Good luck.

- Li Yu

[-- Attachment #2: hidbus.prototype.070404.patch.gz --]
[-- Type: application/x-gzip, Size: 21847 bytes --]

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Dmitry Torokhov]

On Wednesday 04 April 2007 21:25, Li Yu wrote:
> Jiri Kosina wrote:
> > BTW as soon as you have some presentable code, could you please send
> > it so
> > that we could see what aproach you have taken? Debating over code is 
> > usualy more efficient than just ranting random ideas :)
> >
> >   
> There is a "presentable patch" in the attachment ;)

Some random notes without reading it all carefully...

> +static int hid_bus_match(struct device *dev, struct device_driver
> *drv) +{
> +	struct hid_driver *hid_drv;
> +	struct hid_device *hid_dev;
> +
> +	hid_drv = to_hid_driver(drv);
> +	hid_dev = to_hid_device(dev);
> +
> +	if (is_hid_driver_sticky(hid_drv))
> +		/* the sticky driver match device do not pass here. */
> +		return 0;
> +	if (hid_dev->bus != hid_drv->bus)
> +		return 0;

How can this happen?

> +	if (!hid_drv->match || hid_drv->match(hid_drv, hid_dev)) {
> +		hid_dev->driver = hid_drv;

This usually done in bus->probe() function, when we know for sure that
driver binds to to the device.

> +static void hid_bus_release(struct device *dev)
> +{
> +}
> +
> +struct device hid_bus = {
> +	.bus_id   = "hidbus0",
> +	.release  = hid_bus_release
> +};
> +
> +static void hid_dev_release(struct device *dev)
> +{
> +}
> +

That will for sure raise Greg KH's blood pressure ;)

> +	for (i=0; hid_dev->attrs && hid_dev->attrs[i]; ++i) {
> +		ret = device_create_file(&hid_dev->device, hid_dev->attrs[i]);
> +		if (ret)
> +			break;
> +

That should be handled via bus's device attributes and not open coded...

> - *  Copyright (c) 2000-2005 Vojtech Pavlik <vojtech@suse.cz>
> - *  Copyright (c) 2005 Michael Haboustak <mike-@cinci.rr.com> for Concept2, Inc
> + *  Copyright (c) 2000-2005 Vojtech Pavlik <vojtech@@suse.cz>
> + *  Copyright (c) 2005 Michael Haboustak <mike-@@cinci.rr.com> for Concept2, Inc
>   *  Copyright (c) 2006 Jiri  Kosina

Any particular reason for mangling addresses?

> +	if (interrupt)
> +		local_irq_save(flags);
> +	spin_lock(&hid_lock);
> +	list_for_each_entry(driver, &hid_sticky_drivers, sticky_link) {
> +		hook = driver->hook;
> +		if (hook && hook->raw_event) {
> +			ret = hook->raw_event(hid, type, data, size, interrupt);
> +			if (!ret)
> +				break;
> +		}
> +	}
> +	spin_unlock(&hid_lock);
> +	if (interrupt)
> +		local_irq_restore(flags);
> +

This is scary. spin_lock_irqsave() and be done with it.

> +int hid_open(struct hid_device *hid)
> +{
> +	struct hid_transport *tl;
> +	int ret;
> +
> +	if (hid->driver->open)
> +		return hid->driver->open(hid);
> +	ret = 0;
> +	spin_lock(&hid_lock);
> +	tl =  hid_transports[hid->bus];
> +	if (tl->open)
> +		ret = tl->open(hid);
> +	spin_unlock(&hid_lock);
> +	return ret;
> +}

Spinlock is not the best choise here, I'd expect most ->open()
implementation wait on some IO.

-- 
Dmitry

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Li Yu]

Dmitry Torokhov wrote:
>> +static int hid_bus_match(struct device *dev, struct device_driver
>> *drv) +{
>> +	struct hid_driver *hid_drv;
>> +	struct hid_device *hid_dev;
>> +
>> +	hid_drv = to_hid_driver(drv);
>> +	hid_dev = to_hid_device(dev);
>> +
>> +	if (is_hid_driver_sticky(hid_drv))
>> +		/* the sticky driver match device do not pass here. */
>> +		return 0;
>> +	if (hid_dev->bus != hid_drv->bus)
>> +		return 0;
>>     
>
> How can this happen?
>
>   
Our HID driver just are some logical drivers, they are not attach some
physical devices directly. However, The HID core may include more than
one physical bus devices, I think HID drivers only take care of these
devices at one physical bus. So, compare here can avoid call 
hid_drv->match() across bus.
>> +	if (!hid_drv->match || hid_drv->match(hid_drv, hid_dev)) {
>> +		hid_dev->driver = hid_drv;
>>     
>
> This usually done in bus->probe() function, when we know for sure that
> driver binds to to the device.
>
>   
Yes, this may have a bit of hack, but this make hid_drv->probe() more
easier. And, as you seen, the hid_drv_probe() will check the actual
probe process is whether OK. If not so, the hid_drv_probe() will clean
this member.
>> +static void hid_bus_release(struct device *dev)
>> +{
>> +}
>> +
>> +struct device hid_bus = {
>> +	.bus_id   = "hidbus0",
>> +	.release  = hid_bus_release
>> +};
>> +
>> +static void hid_dev_release(struct device *dev)
>> +{
>> +}
>> +
>>     
>
> That will for sure raise Greg KH's blood pressure ;)
>
>   
Er, if not so, we will get some dump stack information in dmesg when
remove this module ......
>> +	for (i=0; hid_dev->attrs && hid_dev->attrs[i]; ++i) {
>> +		ret = device_create_file(&hid_dev->device, hid_dev->attrs[i]);
>> +		if (ret)
>> +			break;
>> +
>>     
>
> That should be handled via bus's device attributes and not open coded...
>
>   
I agree, this is another TODO.
>> - *  Copyright (c) 2000-2005 Vojtech Pavlik <vojtech@suse.cz>
>> - *  Copyright (c) 2005 Michael Haboustak <mike-@cinci.rr.com> for Concept2, Inc
>> + *  Copyright (c) 2000-2005 Vojtech Pavlik <vojtech@@suse.cz>
>> + *  Copyright (c) 2005 Michael Haboustak <mike-@@cinci.rr.com> for Concept2, Inc
>>   *  Copyright (c) 2006 Jiri  Kosina
>>     
>
> Any particular reason for mangling addresses?
>
>   
I also want to who changed them! ~_~

>> +	if (interrupt)
>> +		local_irq_save(flags);
>> +	spin_lock(&hid_lock);
>> +	list_for_each_entry(driver, &hid_sticky_drivers, sticky_link) {
>> +		hook = driver->hook;
>> +		if (hook && hook->raw_event) {
>> +			ret = hook->raw_event(hid, type, data, size, interrupt);
>> +			if (!ret)
>> +				break;
>> +		}
>> +	}
>> +	spin_unlock(&hid_lock);
>> +	if (interrupt)
>> +		local_irq_restore(flags);
>> +
>>     
>
> This is scary. spin_lock_irqsave() and be done with it.
>
>   
May be, I really do not want to increase interrupt disabling time in our
action.

>> +int hid_open(struct hid_device *hid)
>> +{
>> +	struct hid_transport *tl;
>> +	int ret;
>> +
>> +	if (hid->driver->open)
>> +		return hid->driver->open(hid);
>> +	ret = 0;
>> +	spin_lock(&hid_lock);
>> +	tl =  hid_transports[hid->bus];
>> +	if (tl->open)
>> +		ret = tl->open(hid);
>> +	spin_unlock(&hid_lock);
>> +	return ret;
>> +}
>>     
>
> Spinlock is not the best choise here, I'd expect most ->open()
> implementation wait on some IO.
>
>   
Yes, I agree! Also, there have another code access hid_transports[]
without spin it!

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Li Yu]

Dmitry Torokhov wrote:
>>> +int hid_open(struct hid_device *hid)
>>> +{
>>> +	struct hid_transport *tl;
>>> +	int ret;
>>> +
>>> +	if (hid->driver->open)
>>> +		return hid->driver->open(hid);
>>> +	ret = 0;
>>> +	spin_lock(&hid_lock);
>>> +	tl =  hid_transports[hid->bus];
>>> +	if (tl->open)
>>> +		ret = tl->open(hid);
>>> +	spin_unlock(&hid_lock);
>>> +	return ret;
>>> +}
>>>     
>>>       
>> Spinlock is not the best choise here, I'd expect most ->open()
>> implementation wait on some IO.
>>
>>   
>>     
> Yes, I agree! Also, there have another code access hid_transports[]
> without spin it!
>
>   
I think I found out the resolve means for this: using the refcnt of
hid_transport->module,
so we can read access hid_transports[] safely without any lock protection.

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Jiri Kosina]

On Wed, 4 Apr 2007, Adam Kropelin wrote:

> > hiddev will have to stay for quite some time, exactly because of 
> > backward compatibility with userspace applications/drivers that use it 
> > (I am not aware of many of them though, but apparently there are 
> > some).
> Apcupsd is the one on my mind, but I believe there are others.

I am aware only of apcupsd, nut and hid2hci.

> On Apcupsd we've recently introduced a libusb-based driver that does all 
> HID parsing in userspace. Not only does that free us from hiddev, it 
> also frees us from the umpteen other proprietary HID interfaces across 
> various platforms. Although the hiddev-based driver is still the default 
> for Linux platforms, I plan to change that in the next major release and 
> thus begin migrating folks off of hiddev.

Great. Do you use libusb to obtain raw hid events? Could you by any chance 
look at current implementation of hidraw (it's in -mm or I can send it to 
you as a separate patch) and check whether you have any comments on this? 
It would be good if you could use hidraw rather than reading raw usb data 
through libusb.

Thanks.

-- 
Jiri Kosina

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Adam Kropelin]

Jiri Kosina wrote:
> On Wed, 4 Apr 2007, Adam Kropelin wrote:
>> On Apcupsd we've recently introduced a libusb-based driver that does
>> all HID parsing in userspace. Not only does that free us from
>> hiddev, it also frees us from the umpteen other proprietary HID
>> interfaces across various platforms. Although the hiddev-based
>> driver is still the default for Linux platforms, I plan to change
>> that in the next major release and thus begin migrating folks off of
>> hiddev.
>
> Great. Do you use libusb to obtain raw hid events?

We do, along with libusbhid from *BSD to handle report descriptor 
parsing and report field extraction/insertion.

> Could you by any
> chance look at current implementation of hidraw (it's in -mm or I can
> send it to you as a separate patch) and check whether you have any
> comments on this?

I pulled down 2.6.21-rc5-mm4 and took a look at hidraw. I like the 
simplicity of it for sure. One feature that seems to be missing is the 
ability to force an input report to be fetched via a control transfer. 
Several APC UPSes have the unfortunate tendency to change report values 
without sending an interrupt report to notify you, so you have to poll 
them occasionally. Also, how does one fetch string descriptors via 
hidraw?

> It would be good if you could use hidraw rather
> than reading raw usb data through libusb.

I have to admit I don't see much value in switching Apcupsd to hidraw 
rather than libusb. (I see lots of value switching it away from hiddev, 
though :) We need a HID engine in userspace to handle descriptor parsing 
with any non-hiddev solution. Plus, APC UPSes tend to have buggy HID 
implementations that require a bit of care to communicate with properly. 
Having an "intelligent" kernel layer in the middle tends to just get in 
the way (see the truncated report handling patch for hid-core I just 
sent).
But honestly, the deal-breaker is that with libusb I can hit Linux, all 
the *BSDs, Darwin, Solaris, and even Windows with a single userspace 
driver. That's just too valuable to ignore.

--Adam

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Li Yu]

Dmitry Torokhov wrote:
>> +static void hid_bus_release(struct device *dev)
>> +{
>> +}
>> +
>> +struct device hid_bus = {
>> +	.bus_id   = "hidbus0",
>> +	.release  = hid_bus_release
>> +};
>> +
>> +static void hid_dev_release(struct device *dev)
>> +{
>> +}
>> +
>>     
>
> That will for sure raise Greg KH's blood pressure ;)
>   

I know your words now. The entire hid_bus device is useless. The
original code of hid bus is copied from LDD3e. It seem the API had
changed since it pressed. In fact, the new kernel only work silent
without it, or the kref_get() will warn us.

And, I fixed the double hidinput_disconnect() problem last night. It's
reason is not invalid memory access, instead of, it's normal behavior of
hidinput_disconnect(). The resolution is easy, We should move inputs
member to hid_device, not in hid_driver. so if we removed one
hid_device, it do not disconnect all devices which its driver bind, just
only itself.

Now, usbhid works fine.

Good luck.

- Li Yu

Re: [linux-usb-devel] HID bus prototype - 20070408

[Li Yu]

Hi,
   
    It seem the hid-pidff driver also should be sticky.

Good luck

- Li Yu

Re: HID bus prototype - 20070408

[Jiri Kosina]

On Sun, 8 Apr 2007, Li Yu wrote:

> 	The attachment is the latest HID bus prototype. It have such changes:
> 	0. Move the hidp directory from net/bluetooth/ to drivers/hid/.

Hi Li,

we have been discussing this with Marcel previously, and the decission was 
to let the hidp code where it is right now, due to it being very closely 
connected to the bluetooth network stack.

> 	1. HID/Bluetooth support, ONLY FOR HIGHLY EXPERIMENT. I have no 
>          any such device to test yet.

I didn't have time yet to review the patch you sent previously, but I 
don't still quite understand why does the transport layer matter here? The 
generic HID layer, as it is in kernel now, makes an abstraction in a way 
that the HID-specific drivers should not care about the underlying 
transport layer.

> 	I am sorry for it is not in patch form.

That's quite unfortunate. I'll try to review it nevertheless, but it'd be 
much more convenient if you manage to send a patch.

Thanks,

-- 
Jiri Kosina

Re: [linux-usb-devel] HID bus prototype - 20070408

[Li Yu]

Jiri Kosina wrote:
> Hi Li,
>
> we have been discussing this with Marcel previously, and the decission was 
> to let the hidp code where it is right now, due to it being very closely 
> connected to the bluetooth network stack.
>   
That's OK.
>   
>> 	1. HID/Bluetooth support, ONLY FOR HIGHLY EXPERIMENT. I have no 
>>          any such device to test yet.
>>     
>
> I didn't have time yet to review the patch you sent previously, but I 
> don't still quite understand why does the transport layer matter here? The 
> generic HID layer, as it is in kernel now, makes an abstraction in a way 
> that the HID-specific drivers should not care about the underlying 
> transport layer.
>
>   
Here is my reason for supply such hid_transport data structure:

As before, we only have one driver for each transport layer, IOW, the
common driver. In this case, we really need not such transport data
structure. However, there are many driver in one transport layer after
HID bus come, these drivers at the same transport layer implementation
would like share some something each other, these live in common driver
before. If we did not add such transport data structure, we must find
one related driver, and clone it, I think that is not good idea.

In fact, even now, the HID processing logic still do not take care of
which transport layer works under it, the cost just is increasing some
pointer reference operations.

>> 	I am sorry for it is not in patch form.
>>     
>
> That's quite unfortunate. I'll try to review it nevertheless, but it'd be 
> much more convenient if you manage to send a patch.
>
>   
OK, I will post the patch for 2.6.21-rc6-mm1 later. You are recommended
to wait to review that.

My friend yanghong buy a new bluetooth mouse, so I can test bluetooth
nowadays, Thanks him here. :D

Good luck.

- Li Yu

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Paul Walmsley]

On Tue, 3 Apr 2007, Jiri Kosina wrote:

> On Tue, 3 Apr 2007, Li Yu wrote:
>
>> May be, we need some means to change blacklist in runtime.
>
> Paul Walmsley (added to CC) sent me patches some time ago that among other
> things implemented possibility to modify the hid_blacklist[] in runtime.
> The patches had some issues which Paul said will fix - Paul, what is the
> current status please?

Hi Jiri,

I'll finish fixing those up and will send them to linux-input@ by the end 
of the week.


- Paul

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Jiri Kosina]

On Tue, 3 Apr 2007, Li Yu wrote:

> May be, we need some means to change blacklist in runtime.

Paul Walmsley (added to CC) sent me patches some time ago that among other 
things implemented possibility to modify the hid_blacklist[] in runtime. 
The patches had some issues which Paul said will fix - Paul, what is the 
current status please?

Thanks,

-- 
Jiri Kosina

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Dmitry Torokhov]

On Monday 02 April 2007 21:40, Li Yu wrote:
> May be, we need some means to change blacklist in runtime. and
> loading/unloading such driver by specific script to do it.

Please look at the new_id sysfs attribute implementation in
drivers/pci/pci-driver.c. I believe we need something similar
to dynamically adjust HID ignore blacklist.

-- 
Dmitry

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Li Yu]

Nicolas Mailhot wrote:
>> Er, I also want to know what are drawbacks of "flip-flopping" ?
>>     
>
> This will cause major havoc as soon as hot-plugging and apps listening to
> HAL events (xorg eventually) enter in play.
>
>   

~_~

It really need some extra works in user space, but I do not think this
is so critical. These HAL events should not be frequently, and happen
when system boot early very likely. In fact, these works also exist with
blacklist means, but it migrate to HID driver developer, and from
runtime move to development-time. (Of course, you can do it by sysfs,
just like vmware, I think it is so).

Although I do not agree very much, since such many guru said the
"flip-flopping" is not good idea, It is likely appropriate, I also will
change code later,  this make the implementation more easier in fact.

May be, we need some means to change blacklist in runtime. and
loading/unloading such driver by specific script to do it.

Re: [linux-usb-devel] [RFC] HID bus design overview.

[Nicolas Mailhot]

> Er, I also want to know what are drawbacks of "flip-flopping" ?

This will cause major havoc as soon as hot-plugging and apps listening to
HAL events (xorg eventually) enter in play.

-- 
Nicolas Mailhot