Re: [PATCH] HID: hid-sensor-hub: do not process feature reports in raw_event

["Daniel Leung" <daniel.leung@linux.intel.com>]

> On Mon, 25 Mar 2013, Srinivas Pandruvada wrote:
>
>> Daniel,
>>
>> I am looking at 3.9.rc1.
>> The only place I see the raw_event callback is called from
>> hid/hid_input_report(). hid_input_report is called with type
>> HID_INPUT_REPORT
>> in all cases, except hid_ctrl(), where it can be different depending on
>> xx.report->type. But here, the return value is not checked.
>>
>> Do you know the call chain for HID_FETURE_REPORT, where this is creating
>> problem?
>
> This was exactly what I was wondering about when I have seen the initial
> patch a few weeks ago.
>
> Daniel, could you please elaborate? Is there a out-of-tree driver for Acer
> Iconia W700?

Sorry for the late reply. I was out of town and was having difficulties
accessing the mail server.

For the call path, here is what I got (using WARN_ON()):

<4>[  150.943775] ------------[ cut here ]------------
<4>[  150.943861] WARNING: at
../../../../../../../../../../home/dleung/android/ia/private/jb-mr1-internal/kernel/intel/drivers/hid/hid-sensor-hub.c:418
sensor_hub_raw_event+0x2d0/0x3b0 [hid_sensor_hub]()
<4>[  150.944028] Hardware name: ICONIA W700
<7>[  150.944071] Modules linked in: hid_sensor_als hid_sensor_gyro_3d
hid_sensor_accel_3d uvcvideo videobuf2_vmalloc videobuf2_memops
videobuf2_core btusb bluetooth hid_multitouch ath9k ath9k_common ath9k_hw
ath snd_hda_codec_hdmi snd_hda_codec_realtek ip6table_raw snd_hda_intel
iptable_raw snd_hda_codec snd_hwdep snd_pcm snd_page_alloc snd_timer snd
soundcore hid_sensor_trigger hid_sensor_iio_common hid_sensor_hub
industrialio_triggered_buffer kfifo_buf industrialio coretemp
<7>[  150.944132] Pid: 0, comm: swapper/1 Not tainted
3.8.0-aia1-00043-gd7fd780-dirty #2
<7>[  150.944136] Call Trace:
<7>[  150.944141]  <IRQ>  [<ffffffff8105b5bf>] warn_slowpath_common+0x7f/0xc0
<7>[  150.944163]  [<ffffffff8105b61a>] warn_slowpath_null+0x1a/0x20
<7>[  150.944173]  [<ffffffffa00315e0>] sensor_hub_raw_event+0x2d0/0x3b0
[hid_sensor_hub]
<7>[  150.944184]  [<ffffffff819021e7>] ?
_raw_spin_unlock_irqrestore+0x57/0x70
<7>[  150.944195]  [<ffffffff816ccce3>] hid_input_report+0x283/0x2d0
<7>[  150.944203]  [<ffffffff816d9faf>] hid_ctrl+0x9f/0x180
<7>[  150.944212]  [<ffffffff815b2087>] usb_hcd_giveback_urb+0x77/0x110
<7>[  150.944222]  [<ffffffff8160073f>] xhci_irq+0x60f/0x1530
<7>[  150.944230]  [<ffffffff810df0fe>] ? handle_edge_irq+0x1e/0x130
<7>[  150.944239]  [<ffffffff810af04f>] ? __lock_acquire.isra.31+0x28f/0xa70
<7>[  150.944247]  [<ffffffff81601671>] xhci_msi_irq+0x11/0x20
<7>[  150.944258]  [<ffffffff810dc395>] handle_irq_event_percpu+0x55/0x210
<7>[  150.944267]  [<ffffffff810dc598>] handle_irq_event+0x48/0x70
<7>[  150.944273]  [<ffffffff810df157>] handle_edge_irq+0x77/0x130
<7>[  150.944283]  [<ffffffff81004180>] handle_irq+0x60/0x150
<7>[  150.944293]  [<ffffffff81906406>] ?
atomic_notifier_call_chain+0x16/0x20
<7>[  150.944302]  [<ffffffff8190c38a>] do_IRQ+0x5a/0xe0
<7>[  150.944309]  [<ffffffff8190266f>] common_interrupt+0x6f/0x6f
<7>[  150.944313]  <EOI>  [<ffffffff816a97f5>] ? cpuidle_wrap_enter+0x55/0xa0
<7>[  150.944327]  [<ffffffff816a97f1>] ? cpuidle_wrap_enter+0x51/0xa0
<7>[  150.944334]  [<ffffffff816a9850>] cpuidle_enter_tk+0x10/0x20
<7>[  150.944341]  [<ffffffff816a940f>] cpuidle_idle_call+0xaf/0x2b0
<7>[  150.944350]  [<ffffffff8100b3aa>] cpu_idle+0xda/0x130
<7>[  150.944360]  [<ffffffff818f081d>] start_secondary+0x266/0x268
<4>[  150.944365] ---[ end trace 32319d39ebee5616 ]---


You can see that it went through hid_ctrl().

>From what I can gather using usbmon, the sensor hub sends the feature
report correctly. However, the in-kernel struct is not updated when the
report comes in. Therefore, the report from sensor_hub_report() is always
zero-filled. When calling sensor_hub_set_feature(), those zeros are being
sent back to the device. The feature report contains fields for power
state and reporting state. If they are set to zero, the firmware does not
know what to do as zero means undefined state. Both power and reporting
states have to be set for sensor to report values. Given the zero-filled
report, and we can only deal with one field in sensor_hub_set_feature(),
setting power state to non-zero also sends zero (i.e. undefined state) to
the reporting state field, and vice versa.

The sensor hub on Acer Iconia W700 is using drivers/hid/hid-sensor-hub.c.
I also played with Sony Vaio Duo 11 a while back (before having this
patch), and was having trouble turning on gyroscope. I suspect it is
caused by the same issue. The sensor hub on Acer is from STMicro, while
the one in Vaio Duo is from Freescale.



Thanks,
Daniel