Hi Benjamin,
On Fri, Jun 13, 2014 at 1:29 PM, Benjamin Tissoires
<benjamin.tissoires@xxxxxxxxxx> wrote:
> This solves a bug with the wireless receiver:
Your patch does get rid of the crash. But, it does not fix it at the
root cause.
> - at plug, the wireless receiver does not know which Wacom device it is
> connected to, so it does not actually creates all the LEDs
This is the root cause - LEDs are not created for wireless devices.
Neither here, nor later when a real device is connected.
> - when the tablet connects, wacom->wacom_wac.features.type is set to the
> proper device so that wacom_wac can understand the packets
LEDs are not created for any wireless devices since we don't call
wacom_initialize_leds() when real tablets are connected.
> - when the receiver is unplugged, it detects that a LED should have been
> created (based on wacom->wacom_wac.features.type) and tries to remove
> it: crash when removing the sysfs group.
When receiver is unplugged, it remembers the last tablet that
connected to it. If that tablet supports LEDs, wacom_destroy_leds() is
called. But, no LEDs were initialized. That's why it crashes.
> Side effect, we can now safely call several times wacom_destroy_leds().
led_initialized will never be true if we keep wacom_initialize_leds()
inside probe().
To make initialize_leds() and desctroy_leds() work for wireless
devices, we need to move them to wacom_wireless_work() where we know
what type of tablet is connected/disconnected.
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx>
Thank you for your support. But, sorry
NAKed-by: Ping Cheng <pingc@xxxxxxxxx>
Ping
> ---
> drivers/input/tablet/wacom.h | 1 +
> drivers/input/tablet/wacom_sys.c | 6 ++++++
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/input/tablet/wacom.h b/drivers/input/tablet/wacom.h
> index 70b1e71..f13ad31 100644
> --- a/drivers/input/tablet/wacom.h
> +++ b/drivers/input/tablet/wacom.h
> @@ -120,6 +120,7 @@ struct wacom {
> u8 hlv; /* status led brightness button pressed (1..127) */
> u8 img_lum; /* OLED matrix display brightness */
> } led;
> + bool led_initialized;
> struct power_supply battery;
> };
>
> diff --git a/drivers/input/tablet/wacom_sys.c b/drivers/input/tablet/wacom_sys.c
> index 94096fd..7087b33 100644
> --- a/drivers/input/tablet/wacom_sys.c
> +++ b/drivers/input/tablet/wacom_sys.c
> @@ -1016,12 +1016,18 @@ static int wacom_initialize_leds(struct wacom *wacom)
> return error;
> }
> wacom_led_control(wacom);
> + wacom->led_initialized = true;
>
> return 0;
> }
>
> static void wacom_destroy_leds(struct wacom *wacom)
> {
> + if (!wacom->led_initialized)
> + return;
> +
> + wacom->led_initialized = false;
> +
> switch (wacom->wacom_wac.features.type) {
> case INTUOS4S:
> case INTUOS4:
> --
> 1.9.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
