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

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

Questions regarding ACPI vs FADT dump on HP XW6200 using 2.6.16 - 2.6.20 kernels?

[Fortier,Vincent [Montreal]]

Hi all,

In reference to this Bugzilla entry:
http://bugzilla.kernel.org/show_bug.cgi?id=7110

I'm getting ACPI errors in the dmesg:
[    7.443482] ACPI Error (evgpe-0711): No handler or method for GPE[
0], disabling event [20060707]
[    7.449881] ACPI Error (evgpe-0711): No handler or method for GPE[
1], disabling event [20060707]
[    7.456145] ACPI Error (evgpe-0711): No handler or method for GPE[
2], disabling event [20060707]
[    7.462271] ACPI Error (evgpe-0711): No handler or method for GPE[
5], disabling event [20060707]
[    7.468310] ACPI Error (evgpe-0711): No handler or method for GPE[
6], disabling event [20060707]
[    7.474260] ACPI Error (evgpe-0711): No handler or method for GPE[
7], disabling event [20060707]
[    7.480124] ACPI Error (evgpe-0711): No handler or method for GPE[
9], disabling event [20060707]
[    7.485893] ACPI Error (evgpe-0711): No handler or method for GPE[
A], disabling event [20060707]
[    7.491473] ACPI Error (evgpe-0711): No handler or method for GPE[
F], disabling event [20060707]
[    7.491484] ACPI Error (evgpe-0711): No handler or method for
GPE[10], disabling event [20060707]
[    7.491489] ACPI Error (evgpe-0711): No handler or method for
GPE[11], disabling event [20060707]
[    7.491494] ACPI Error (evgpe-0711): No handler or method for
GPE[12], disabling event [20060707]
[    7.491499] ACPI Error (evgpe-0711): No handler or method for
GPE[13], disabling event [20060707]
[    7.491504] ACPI Error (evgpe-0711): No handler or method for
GPE[14], disabling event [20060707]
[    7.491509] ACPI Error (evgpe-0711): No handler or method for
GPE[15], disabling event [20060707]
[    7.491514] ACPI Error (evgpe-0711): No handler or method for
GPE[16], disabling event [20060707]
[    7.491519] ACPI Error (evgpe-0711): No handler or method for
GPE[17], disabling event [20060707]
[    7.491527] ACPI Error (evgpe-0711): No handler or method for
GPE[19], disabling event [20060707]
[    7.491532] ACPI Error (evgpe-0711): No handler or method for
GPE[1A], disabling event [20060707]
[    7.491537] ACPI Error (evgpe-0711): No handler or method for
GPE[1B], disabling event [20060707]
[    7.491542] ACPI Error (evgpe-0711): No handler or method for
GPE[1C], disabling event [20060707]
[    7.491547] ACPI Error (evgpe-0711): No handler or method for
GPE[1D], disabling event [20060707]
[    7.491552] ACPI Error (evgpe-0711): No handler or method for
GPE[1E], disabling event [20060707]
[    7.491557] ACPI Error (evgpe-0711): No handler or method for
GPE[1F], disabling event [20060707]

The ACPI: FADT dmesg entry is:
[    0.000000] ACPI: RSDP (v002 COMPAQ                                )
@ 0x000e8e10
[    0.000000] ACPI: XSDT (v001 COMPAQ CPQ0063  0x20051222  0x00000000)
@ 0xbfff50ec
[    0.000000] ACPI: FADT (v003 COMPAQ TUMWATER 0x00000001  0x00000000)
@ 0xbfff52ac
[    0.000000] ACPI: SSDT (v001 COMPAQ  PROJECT 0x00000001 MSFT
0x0100000e) @ 0xbfff657a
[    0.000000] ACPI: MADT (v001 COMPAQ TUMWATER 0x00000001  0x00000000)
@ 0xbfff53b8
[    0.000000] ACPI: ASF! (v032 COMPAQ TUMWATER 0x00000001  0x00000000)
@ 0xbfff543c
[    0.000000] ACPI: MCFG (v001 COMPAQ TUMWATER 0x00000001  0x00000000)
@ 0xbfff54a6
[    0.000000] ACPI: DSDT (v001 COMPAQ     DSDT 0x00000001 MSFT
0x0100000e) @ 0x00000000

I've been told to use this command to dump this memory segment:
dd if=/dev/mem skip=$[0xbfff52ac] bs=1c count=244 2> /dev/null >
fadt.bin
But I'm always getting an empty file?

My questions are:
1) How can I dump the FADT ?
2) Could this be related to this memory detection bug
http://bugzilla.kernel.org/show_bug.cgi?id=7111 ?

Thnx!

- vin

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.

[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.

[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.

[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.

[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] [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.

[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.

[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.

[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.

[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]

[-- 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.

[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.

[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 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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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.

[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 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 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.

[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]

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 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.

[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, 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.

[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.

[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