Hi.

2011/9/27 Ping Cheng <[email protected]>:
> On Tue, Sep 27, 2011 at 12:09 PM, Eduard Hasenleithner
> <[email protected]> wrote:
>> So let me summarize: The only important point of the function selector
>> (led) is to provide another axis to the X11 client application.
>> The X11 client is free to decide on what to do with the additional axis.
>> In general, It is used for switching between many sets of expresskey
>> allocations.
>
> Yes, and no. Yes for the led to provide new app functions. No if you
> think we are going to report the LED through XInput valuators. Due to
> backward compatibility, we will not report LED as a new axis. It will
> only be reported as a Wacom property through xsetwacom.

Ah, ok, so we have no XInput valuator then.

>> Please find attached how I see it what happens when the toggle button
>> is pressed.
>
> I agree with you except one step: we do not ignore the round button
> event. It will still be reported to the userland. There is no need to
> prevent userland to assign a new function for it.

I guessed that it is the X11 input driver (xf86-input-wacom) which
increases the LED number when the toggle button is pressed. This
cannot be influenced by the X11 app, right?

>> In that case, I have another question out of curiosity:
>> What prevents this new axis to be synthesized solely in the X11 input
>> driver (without kernel support)?
>
> Do you mean we send the led switch command from the X driver directly
> to the device without going through the kernel driver? I am afraid we
> can not. How do we talk to the device without creating a sysfsctl? We
> need a new access point/thread to communite with the device.

No, I meant synthesizing the valuator event. Reading the patch you
attached, the axis is synthesized in the kernel. And I'm asking, why
not synthesize it in the xf86-input-wacom driver? I still have the
feeling, I have a misconception on how this status led reporting is
planned work in the end.

>> The current conclusion for me is that I don't implement the led
>> selection, but only the luminance settings.
>
> I guess so. Attached is a rough kernel patch. I'll update the
> xf86-input-wacom after your X patch is merged.
>
> I'll submit the attached patch if I get a reviewed-by or tested-by
> from you. Thank you.

Sorry, I wanted to test the patch, but I couldn't because the
indentation is garbled. Please see the inline comments. In principle,
I'm ok with the patch. I just want to test it.

> ----------------
>
> diff --git a/drivers/input/tablet/wacom.h b/drivers/input/tablet/wacom.h
> index 00332d66..3cb80cc 100644
> --- a/drivers/input/tablet/wacom.h
> +++ b/drivers/input/tablet/wacom.h

> @@ -93,7 +93,7 @@
>  /*
>  * Version Information
>  */
> -#define DRIVER_VERSION "v1.52"
> +#define DRIVER_VERSION "v1.53"
>  #define DRIVER_AUTHOR "Vojtech Pavlik <[email protected]>"
>  #define DRIVER_DESC "USB Wacom tablet driver"
>  #define DRIVER_LICENSE "GPL"
> @@ -115,7 +115,8 @@ struct wacom {
>        bool open;
>        char phys[32];
>        struct wacom_led {
> -               u8 select; /* status led selector (0..3, -1=none) */
> +               /* 0xf0 for LEDs on the left; 0x0f on the right */
> +               u8 select; /* status led selector (1..4, 0=none) */

Dmitry explicitly wanted me to have all values as separate properties.
I guess he will want these two also separate. For me, the separation
is not important.

Nevertheless, the ABI documentation will also need to be updated.

>                u8 llv;    /* status led brightness no button */
>                u8 hlv;    /* status led brightness button pressed */
>                u8 img_lum;   /* OLED matrix display brightness */
> diff --git a/drivers/input/tablet/wacom_sys.c 
> b/drivers/input/tablet/wacom_sys.c
> index 0eccf57..488f2d9 100644
> --- a/drivers/input/tablet/wacom_sys.c
> +++ b/drivers/input/tablet/wacom_sys.c

> @@ -566,9 +567,17 @@ static ssize_t wacom_led_select_store(struct device *dev,
>
>        mutex_lock(&wacom->lock);
>
> -       wacom->led.select = id;
> +       right_id = id & 0xf;
> +       right_id = right_id > 0 ? right_id | 4 : 0;
> +       left_id = id & 0xf0;
> +       left_id = left_id > 0 ? left_id | 0x40 : 0;
> +       wacom->led.select = left_id | right_id;

For the Intuos4 at least, the counting starts from 0 (zero). This
patch makes it starting from 1, but no translation here. So the LEDs
would be lit in this order: 2, 3, 4, 1. I guess, you should "add" -1
to the right_id and left_id values.

> +
>        err = wacom_led_control(wacom);
>
> +       if (err > 0)
> +               input_report_abs(input, ABS_LED_STATUS, wacom->led.select);
> +
>        mutex_unlock(&wacom->lock);
>
>        return err < 0 ? err : count;

------------------------------------------------------------------------------
All the data continuously generated in your IT infrastructure contains a
definitive record of customers, application performance, security
threats, fraudulent activity and more. Splunk takes this data and makes
sense of it. Business sense. IT sense. Common sense.
http://p.sf.net/sfu/splunk-d2dcopy1
_______________________________________________
Linuxwacom-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxwacom-devel

Reply via email to