Re: RFC: Wacom Bluetooth HID driver, first pass

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

 



On Mon, 2009-03-16 at 15:00 +0100, Marcel Holtmann wrote:
> Hi Bastien,
> 
> >> This is a first pass at a driver for the Wacom Graphire Bluetooth
> >> tablet, based on work by Andrew Zabolotny[1]. It's mildly tested  
> >> (eg. it
> >> works in my very few tests).
> >>
> >> I was looking for guidance for the code itself, possibly making it
> >> easier to merge in the input/tablet/wacom* drivers into it in the
> >> future, as well as some explanations as to how I'm supposed to reset
> >> tools, etc. so the user-space drivers (the X.org linuxwacom driver)
> >> doesn't need special-casing for the device.
> >>
> >> Note that it requires a user-space activation to switch to Mode 2,  
> >> using
> >> bluetoothd (as the Bluetooth HID doesn't support  
> >> hid_output_raw_event).
> >> Patch at:
> >> http://cvs.fedoraproject.org/viewvc/rpms/bluez/devel/bluez-activate-wacom-mode2.patch?view=markup
> >
> > A couple of notes:
> > - the wheel action is reversed, it's a simple fix, done locally
> > - hidp and the Bluetooth sub-system says it can't probe the device  
> > with
> > error "-14" when hid-wacom.ko isn't already loaded
> 
> what was the reason for not forcing HID to load the generic driver?

What generic driver? If you're talking about the default Wacom driver,
it isn't an HID driver, and requires a USB device.

>  I  
> lost track on how HID is suppose to handle these. You could also  
> introduce a phony export like we do with L2CAP to ensure it is loaded  
> even if userspace isn't ready to handle this dependency.

Well, the problem is that the driver _does_ load, but I'm not certain
about the sequence of events between the Bluetooth sub-system, the
user-space enabling (see the linked patch to bluetoothd), and the HID
sub-system.

> > - I'm getting oopses in hci_conn_del() when the device goes away  
> > (eg. I
> > turn it off).
> 
> Try to run a net-next-2.6 or bluetooth-next-2.6 kernel. Kyle should  
> have merged these patches into the latest rawhide kernel.

I was hoping this would be fixed in the latest linus-2.6 tree, guess
not.

> > I'd appreciate any help making this just a tad more stable.
> >
> > Any comments about the code itself?
> 
> The main parsing function looks highly complicated and could either  
> use more comments or break into two.

The parsing is adapted from the code Andrew wrote, so I don't have any
great insights into it. Right it's in "it works well enough to do
things" mode, clean-ups and bug fixes will probably follow when my
machine stops crashing when using that driver :)

> 	if ((wdata->tool = tool)) {
> 		input_report_key(input, tool, 1);
> 		need_sync = 1;
> 	}
> 
> Don't do the assignment in the if clause test.

As I mentioned, copy/paste code this is.

>  Did you run  
> checkpatch.pl on it?

Nope, I didn't, but will do.

Cheers

--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux