OK, that was the type of info I was looking for... which were the changes
were related to features lacking in older kernels.
They all seemed reasonable to me with maybe the exception of Android
patch. For the ones that don't have a Reviewed-by, you can go ahead and
add my:
Acked-by: Chris Bagwell <ch...@cnpbagwell.com>
Chris
On Fri, Sep 27, 2013 at 1:32 PM, Jason Gerecke <killert...@gmail.com> wrote:
> On Fri, Sep 27, 2013 at 10:35 AM, Chris Bagwell <ch...@cnpbagwell.com>
> wrote:
> > Hi Jason,
> >
> > I'd be happy to review these patches if you want it but can you give a
> > little more context for the series on them?
> >
> > For example, a version of this patch seems to be in linux kernel but uses
> > usb_hub_for_each_child(). Can patch be aligned with linux?
> >
> > Are other patches in linux kernel as well? Any that are not and so
> should
> > be reviewed closer?
> >
> > Chris
>
> Sure thing - I should have sent a summary but it skipped my mind.
>
> This patchset has two halves. The first half (patches 01-05) covers
> the Cintiq Companion Hybrid, and the second half (06-10) the Intuos
> Pro. Support for both devices has been merged upstream, but this set
> contains a few other "fixup" patches that aren't upstream. The patches
> which haven't gone through upstream review are:
>
> 01/10 - Back when I wrote `wacom_get_sibling`, I was under the
> impression that the list of child devices connected to a hub
> (`dev->parent->children`) was "compact" and would have only NULL
> entries after the last child. It turns out this isn't the case and
> that if the hub isn't fully populated you can have NULL entries in the
> middle of the list. This is the case with the hub internal to the
> Cintiq Companion Hybrid: a NULL child sits between the pen and touch
> sensors, which was preventing `wacom_get_sibling` from actually
> linking the two devices. As you've noticed, upstream uses
> `usb_hub_for_each_child` now. This was introduced in 3.7 along with a
> change that removed the ability for drivers to directly access the
> `dev->parent->children` list. Forward-porting this patch is
> impossible, and backporting `usb_hub_for_each_child` is a little
> unreasonable IMO.
>
> 03/10 - This is a patch included in the kernel shipping with the
> Cintiq Companion Hybrid which works around an issue Android had
> correctly interpreting the device type. ABS_MT_TOOL_TYPE isn't
> declared upstream anyway, so I figure its safe to remove from
> input-wacom as well.
>
> 04/10 - Another Android-specific patch. ABS_MISC contains pen serial
> number information, but because it is declared with min==max==0
> Android ignores it and prevents us from passing it along to apps. To
> work around this, we instead declare that ABS_MISC can take on any
> value from INT_MIN to INT_MAX, which I think is more correct anyway.
> Do note, however, that this patch has been rejected upstream!
>
> 09/10 - While testing the Intuos Pro, I noticed some weird behavior
> under CentOS 6. The touch interface was being detected as an EMR,
> complete with stylus/eraser/pad suite of tools rather than just the
> expected finger tool. From what I could tell, the fact that we didn't
> properly declare BTN_TOOL_FINGER et.al. in
> `wacom_setup_input_capabilities` was to blame. Changing this fixed the
> detection, but touch still didn't work correctly until I noticed
> `wacom_wac_irq` was incorrectly sending touch packets to
> `wacom_intuos_irq`. I could have fixed these in patch 08/10 where the
> Intuos Pro support is added, but since the exact same problems also
> affected the Intuos5 I thought it would be clearer to implement the
> pro identically at first and then fix both of the simultaneously.
>
> 10/10 - I was having trouble with the touch interface under 2.6.38+ on
> older Fedora and Ubuntu releases and found that xf86-input-wacom was
> ignoring the touch device since the ABS_X and ABS_Y axes weren't being
> declared. They still don't seem to be explicitly declared in 3.7+ (or
> upstream) but the two axes do show up as being supported in evtest. I
> think there might be something inside the MT protocol's single-touch
> emulation functions that is doing this, but I honestly can't find
> where (which makes me a little nervous).
>
> 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....
>
> >
> >
> >
> > On Wed, Sep 25, 2013 at 6:50 PM, Jason Gerecke <killert...@gmail.com>
> wrote:
> >>
> >> Instead of bailing at the first NULL child (i.e., the first non-
> >> connected port), go through all ports and simply skip any NULL
> >> children. This condition occurs in the Cintiq Companion Hybrid
> >> when trying to locate the other "half" (i.e. pen or touch device)
> >> of the tablet.
> >>
> >> Signed-off-by: Jason Gerecke <killert...@gmail.com>
> >> ---
> >> 2.6.38/wacom_sys.c | 17 ++++++++++-------
> >> 1 file changed, 10 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/2.6.38/wacom_sys.c b/2.6.38/wacom_sys.c
> >> index 177efd8..6848717 100644
> >> --- a/2.6.38/wacom_sys.c
> >> +++ b/2.6.38/wacom_sys.c
> >> @@ -622,7 +622,7 @@ static DEFINE_MUTEX(wacom_udev_list_lock);
> >>
> >> static struct usb_device *wacom_get_sibling(struct usb_device *dev, int
> >> vendor, int product)
> >> {
> >> - struct usb_device **sibling;
> >> + int i;
> >>
> >> if (vendor == 0 && product == 0)
> >> return dev;
> >> @@ -630,14 +630,17 @@ static struct usb_device *wacom_get_sibling(struct
> >> usb_device *dev, int vendor,
> >> if (dev->parent == NULL)
> >> return NULL;
> >>
> >> - sibling = dev->parent->children;
> >> - while (sibling != NULL && *sibling != NULL) {
> >> - struct usb_device_descriptor d = (*sibling)->descriptor;
> >> + for (i = 0 ; i < dev->parent->maxchild; i++) {
> >> + struct usb_device *sibling = dev->parent->children[i];
> >> + struct usb_device_descriptor d;
> >>
> >> - if (d.idVendor == vendor && d.idProduct == product)
> >> - return *sibling;
> >> + if (sibling == NULL)
> >> + continue;
> >> +
> >> + d = sibling->descriptor;
> >>
> >> - sibling++;
> >> + if (d.idVendor == vendor && d.idProduct == product)
> >> + return sibling;
> >> }
> >>
> >> return NULL;
> >> --
> >> 1.8.4
> >>
> >>
> >>
> >>
> ------------------------------------------------------------------------------
> >> October Webinars: Code for Performance
> >> Free Intel webinars can help you accelerate application performance.
> >> Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most
> >> from
> >> the latest Intel processors and coprocessors. See abstracts and
> register >
> >>
> >>
> http://pubads.g.doubleclick.net/gampad/clk?id=60133471&iu=/4140/ostg.clktrk
> >> _______________________________________________
> >> Linuxwacom-devel mailing list
> >> Linuxwacom-devel@lists.sourceforge.net
> >> https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
> >
> >
>
------------------------------------------------------------------------------
October Webinars: Code for Performance
Free Intel webinars can help you accelerate application performance.
Explore tips for MPI, OpenMP, advanced profiling, and more. Get the most from
the latest Intel processors and coprocessors. See abstracts and register >
http://pubads.g.doubleclick.net/gampad/clk?id=60133471&iu=/4140/ostg.clktrk
_______________________________________________
Linuxwacom-devel mailing list
Linuxwacom-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel
