lkml.org 
[lkml]   [2014]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 00/15] Input - Wacom: switch from an USB to a HID driver
From
Hi Jason,

On Thu, Jul 10, 2014 at 9:17 PM, Jason Gerecke <killertofu@gmail.com> wrote:
> On Wed, Jul 2, 2014 at 4:33 PM, Jason Gerecke <killertofu@gmail.com> wrote:
>> On Mon, Jun 30, 2014 at 2:26 PM, Benjamin Tissoires
>> <benjamin.tissoires@redhat.com> wrote:
>>> Hi guys,
>>>
>>> this patch series is a cleanup for the Wacom USB driver.
>>>
>>> I started working on this topic when I saw patches floating around which
>>> implemented a report descriptor parser within the wacom.ko module.
>>> However, we already have a nice HID subsystem which is more generic than the
>>> HID implementation we can find in this USB driver.
>>> Further details of the benefits (code reduction, regression tests) are hopefully
>>> explained in the commit messages of the corresponding patches.
>>>
>>> Also, I am working on a way to handle the new Wacom tablets in a more generic
>>> way in the hid tree, so consider this patch series as a first step in this
>>> direction.
>>>
>>> This patch series transfers the wacom.ko driver from the input tree into the hid
>>> tree. I did not made the corresponding move of the files in the series hoping
>>> that we will find a way to achieve it if this step is validated.
>>>
>>> IMO, the smoothest path would be that Jiri takes care of the wacom driver
>>> in the input tree (and that we move into into the hid subfolder). This can be
>>> achieve if the current pending wacom patches are applied in the hid tree too.
>>>
>>> Another solution could be to keep the wacom changes in the input tree and put the
>>> hid changes in the hid tree by using separate commits. Once 3.17 is out, we can
>>> then change the module into the hid subfolder.
>>>
>>> I wanted to send this patch series right now so we can figure out how we will
>>> handle the transition.
>>>
>>> I am pretty confident the patch series does not break any existing device
>>> (except for the required user space changes which can be handled correctly if
>>> we tackle them right now). The USB commands are executed in the same way,
>>> and the protocol handling is also done in the same way.
>>>
>>> Anyway, the net difference in lines of code (-307) should be enough to be of
>>> interest.
>>>
>>> Note: This patch series requires the current pending wacom patches to be applied.
>>> I set up a tree with all the patch applied if anyone wants to give a try:
>>> https://github.com/bentiss/linux/commits/hid-wacom-legacy-3.16-rc3
>>>
>>> Cheers,
>>> Benjamin
>>>
>>> Benjamin Tissoires (15):
>>> Input - wacom: include and use linux/hid.h
>>> Input - wacom: switch from an USB driver to a HID driver
>>> Input - wacom: use hid communication instead of plain usb
>>> Input - wacom: use HID core to actually fetch the report descriptor
>>> Input - wacom: compute the HID report size to get the actual packet
>>> size
>>> Input - wacom: install LED/OLED sysfs files in the HID device instead
>>> of USB
>>> Input - wacom: register the input devices on top of the HID one
>>> Input - wacom: remove usb dependency for siblings devices
>>> Input - wacom: register power device at the HID level
>>> Input - wacom: use hid_info instead of plain dev_info
>>> HID: uhid: add and set HID_TYPE_UHID for uhid devices
>>> Input - wacom: use in-kernel HID parser
>>> Input - wacom: use hidinput_calc_abs_res instead of duplicating its
>>> code
>>> Input - wacom: remove field pktlen declaration in the list of devices
>>> Input - wacom: keep wacom_ids ordered
>>>
>>> drivers/hid/hid-core.c | 15 +-
>>> drivers/hid/hid-wacom.c | 2 +-
>>> drivers/hid/uhid.c | 2 +
>>> drivers/input/tablet/wacom.h | 7 +-
>>> drivers/input/tablet/wacom_sys.c | 908 +++++++++++++--------------------------
>>> drivers/input/tablet/wacom_wac.c | 647 ++++++++++++++--------------
>>> drivers/input/tablet/wacom_wac.h | 10 +-
>>> include/linux/hid.h | 4 +-
>>> 8 files changed, 644 insertions(+), 951 deletions(-)
>>>
>>> --
>>> 2.0.0
>>>
>>
>> *cracks knuckles*
>>
>> Well, guess I better get to work poking and prodding at these and the
>> pad patches. A very quick review doesn't raise any significant flags,
>> though I do have a question or two that I might have for you in the
>> next few days (need to read through the code more carefully to be sure
>> I understand it correctly). My 24HDT is having some trouble
>> initializing with the patches applied, but I'll need some time to
>> track down the cause. Not sure how much I'll get done this week
>> (holiday weekend) but next week I should have some feedback.
>>
>> Jason
>> ---
>> Now instead of four in the eights place /
>> you’ve got three, ‘Cause you added one /
>> (That is to say, eight) to the two, /
>> But you can’t take seven from three, /
>> So you look at the sixty-fours....
>
> I've had a chance to try out a dozen tablets with these modifications
> and for the most part things seem to work fine. The 24HDT issue I
> mentioned above was due to a conflict with some other patches I was
> testing and can be ignored. Aside from the other known issues you've
> already mentioned (and my inline comments), there's not much that I
> can say (other than that I found some odd [but apparently unrelated]
> issues with wacom_w8001 and isdv4-serial-inputattach...)
>
> Reviewed-by: Jason Gerecke <killertofu@gmail.com>
> Tested-by: Jason Gerecke <killertofu@gmail.com>
>

Thanks Jason. That's really appreciated. As said I will resend a full
cleanup of these with your Rev-by and fixes, so Dmitry or Jiri can
take the various series.

Cheers,
Benjamin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2014-07-11 16:01    [W:0.131 / U:0.272 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site