[PATCH v2 0/3] HID: elan: Mute LED handling fixes and improvements

[PATCH v2 0/3] HID: elan: Mute LED handling fixes and improvements

[Hans de Goede]

Hi All,

Here is v2 of my series with mute LED handling fixes and improvements
for the hid-elan driver.

New in v2:
- Add Marek Behún's Reviewed-by to all patches,thank you for all the
  reviews Marek.
- Improve error handling in patch 1/3 to return -EIO when ret != 3 and >= 0

Regards,

Hans


Hans de Goede (3):
  HID: elan: Silence mute LED errors being logged when the device is
    unplugged
  HID: elan: Set default_trigger for the mute LED
  HID: elan: Remove elan_mute_led_get_brigtness()

 drivers/hid/hid-elan.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

-- 
2.30.1

[PATCH v2 1/3] HID: elan: Silence mute LED errors being logged when the device is unplugged

[Hans de Goede]

Being integrated into an USB keyboard-dock the mute LED can go away
at any time, leading to the following errors:

[  918.667671] elan 0003:04F3:0755.0002: Failed to set mute led brightness: -19
[  918.667737] leds elan:red:mute: Setting an LED's brightness failed (-19)

Fix this by making the following changes:

1. Don't log an error from elan_mute_led_set_brigtness() when
   ret == -ENODEV
2. Set the LED_HW_PLUGGABLE flag on the mute LED led_classdev

While at it also make sure that elan_mute_led_set_brigtness() returns
an error (-EIO) when ret != 3 but it is not an error (>= 0).

Reviewed-by: Marek Behún <kabel@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
Changes in v2:
- Return -EIO when ret != 3 and >= 0
---
 drivers/hid/hid-elan.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-elan.c b/drivers/hid/hid-elan.c
index dae193749d44..473d8528319d 100644
--- a/drivers/hid/hid-elan.c
+++ b/drivers/hid/hid-elan.c
@@ -445,8 +445,9 @@ static int elan_mute_led_set_brigtness(struct led_classdev *led_cdev,
 	kfree(dmabuf);
 
 	if (ret != ELAN_LED_REPORT_SIZE) {
-		hid_err(hdev, "Failed to set mute led brightness: %d\n", ret);
-		return ret;
+		if (ret != -ENODEV)
+			hid_err(hdev, "Failed to set mute led brightness: %d\n", ret);
+		return ret < 0 ? ret : -EIO;
 	}
 
 	drvdata->mute_led_state = led_state;
@@ -462,6 +463,7 @@ static int elan_init_mute_led(struct hid_device *hdev)
 	mute_led->brightness_get = elan_mute_led_get_brigtness;
 	mute_led->brightness_set_blocking = elan_mute_led_set_brigtness;
 	mute_led->max_brightness = LED_ON;
+	mute_led->flags = LED_HW_PLUGGABLE;
 	mute_led->dev = &hdev->dev;
 
 	return devm_led_classdev_register(&hdev->dev, mute_led);
-- 
2.30.1

[PATCH v2 2/3] HID: elan: Set default_trigger for the mute LED

[Hans de Goede]

The mute LED should be automatically turned on/off based
on the audio-card's mixer settings.

Add the standardized default-trigger name for this, so that the alsa
code can turn the LED on/off as appropriate (on supported audio cards).

Reviewed-by: Marek Behún <kabel@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/hid/hid-elan.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hid/hid-elan.c b/drivers/hid/hid-elan.c
index 473d8528319d..5173f50d474d 100644
--- a/drivers/hid/hid-elan.c
+++ b/drivers/hid/hid-elan.c
@@ -460,6 +460,7 @@ static int elan_init_mute_led(struct hid_device *hdev)
 	struct led_classdev *mute_led = &drvdata->mute_led;
 
 	mute_led->name = "elan:red:mute";
+	mute_led->default_trigger = "audio-mute";
 	mute_led->brightness_get = elan_mute_led_get_brigtness;
 	mute_led->brightness_set_blocking = elan_mute_led_set_brigtness;
 	mute_led->max_brightness = LED_ON;
-- 
2.30.1

[PATCH v2 3/3] HID: elan: Remove elan_mute_led_get_brigtness()

[Hans de Goede]

The led_classdev already contains a cached value of the last set
brightness, the brightness_get callback is only meant for LED drivers
which can read back the actual / current brightness from the hardware.

Since elan_mute_led_get_brigtness() just returns the last set value
it does not add any functionality, so we can just remove it.

Reviewed-by: Marek Behún <kabel@kernel.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/hid/hid-elan.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/hid/hid-elan.c b/drivers/hid/hid-elan.c
index 5173f50d474d..021049805bb7 100644
--- a/drivers/hid/hid-elan.c
+++ b/drivers/hid/hid-elan.c
@@ -410,15 +410,6 @@ static int elan_start_multitouch(struct hid_device *hdev)
 	return 0;
 }
 
-static enum led_brightness elan_mute_led_get_brigtness(struct led_classdev *led_cdev)
-{
-	struct device *dev = led_cdev->dev->parent;
-	struct hid_device *hdev = to_hid_device(dev);
-	struct elan_drvdata *drvdata = hid_get_drvdata(hdev);
-
-	return drvdata->mute_led_state;
-}
-
 static int elan_mute_led_set_brigtness(struct led_classdev *led_cdev,
 				       enum led_brightness value)
 {
@@ -461,7 +452,6 @@ static int elan_init_mute_led(struct hid_device *hdev)
 
 	mute_led->name = "elan:red:mute";
 	mute_led->default_trigger = "audio-mute";
-	mute_led->brightness_get = elan_mute_led_get_brigtness;
 	mute_led->brightness_set_blocking = elan_mute_led_set_brigtness;
 	mute_led->max_brightness = LED_ON;
 	mute_led->flags = LED_HW_PLUGGABLE;
-- 
2.30.1

Re: [PATCH v2 0/3] HID: elan: Mute LED handling fixes and improvements

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 397 bytes --]

Hi!

> Here is v2 of my series with mute LED handling fixes and improvements
> for the hid-elan driver.
> 
> New in v2:
> - Add Marek Behún's Reviewed-by to all patches,thank you for all the
>   reviews Marek.
> - Improve error handling in patch 1/3 to return -EIO when ret != 3 and >= 0

Series:

Acked-by: Pavel Machek <pavel@ucw.cz>

-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [PATCH v2 3/3] HID: elan: Remove elan_mute_led_get_brigtness()

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 256 bytes --]

Hi!

> @@ -461,7 +452,6 @@ static int elan_init_mute_led(struct hid_device *hdev)
>  
>  	mute_led->name = "elan:red:mute";

This probably should not have "elan" prefix.

Best regards,
							Pavel

-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]