Input: LEDs - skip unnamed LEDs

Devices may declare more LEDs than what is known to input-leds (HID does this for some devices). Instead of showing ugly warnings on connect and, even worse, oopsing on disconnect, let's simply ignore LEDs that are not known to us.

Reported-by Vlastimil Babka <[email protected]>

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/input-leds.c |   16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

drivers/input/input-leds.c +14 -2

index 074a65e..766bf26 100644
@@ -71,6 +71,18 @@ static void input_leds_event(struct input_handle *handle, unsigned int type,
 {
 }
 
+static int input_leds_get_count(struct input_dev *dev)
+{
+	unsigned int led_code;
+	int count = 0;
+
+	for_each_set_bit(led_code, dev->ledbit, LED_CNT)
+		if (input_led_info[led_code].name)
+			count++;
+
+	return count;
+}
+
 static int input_leds_connect(struct input_handler *handler,
 			      struct input_dev *dev,
 			      const struct input_device_id *id)

@@ -81,7 +93,7 @@ static int input_leds_connect(struct input_handler *handler,
 	int led_no;
 	int error;
 
-	num_leds = bitmap_weight(dev->ledbit, LED_CNT);
+	num_leds = input_leds_get_count(dev);
 	if (!num_leds)
 		return -ENXIO;
 

@@ -112,7 +124,7 @@ static int input_leds_connect(struct input_handler *handler,
 		led->handle = &leds->handle;
 		led->code = led_code;
 
-		if (WARN_ON(!input_led_info[led_code].name))
+		if (!input_led_info[led_code].name)
 			continue;
 
 		led->cdev.name = kasprintf(GFP_KERNEL, "%s::%s",

Devices may declare more LEDs than what is known to input-leds (HID does this for some devices). Instead of showing ugly warnings on connect and, even worse, oopsing on disconnect, let's simply ignore LEDs that are not known to us.

Reported-by Vlastimil Babka <[email protected]>

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/input-leds.c |   16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

drivers/input/input-leds.c +13 -1

index 074a65e..766bf26 100644
@@ -71,6 +71,18 @@ static void input_leds_event(struct input_handle *handle, unsigned int type,
 {
 }
 
+static int input_leds_get_count(struct input_dev *dev)
+{
+	unsigned int led_code;
+	int count = 0;
+
+	for_each_set_bit(led_code, dev->ledbit, LED_CNT)
+		if (input_led_info[led_code].name)
+			count++;
+
+	return count;
+}
+
 static int input_leds_connect(struct input_handler *handler,
 			      struct input_dev *dev,
 			      const struct input_device_id *id)

@@ -81,7 +93,7 @@ static int input_leds_connect(struct input_handler *handler,
 	int led_no;
 	int error;
 
-	num_leds = bitmap_weight(dev->ledbit, LED_CNT);
+	num_leds = input_leds_get_count(dev);
 	if (!num_leds)
 		return -ENXIO;
 

Quoted 51 lines · Dmitry Torokhov

Devices may declare more LEDs than what is known to input-leds (HID does this for some devices). Instead of showing ugly warnings on connect and, even worse, oopsing on disconnect, let's simply ignore LEDs that are not known to us.

Reported-by Vlastimil Babka <[email protected]>

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/input-leds.c |   16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

drivers/input/input-leds.c +14 -2

index 074a65e..766bf26 100644
@@ -71,6 +71,18 @@ static void input_leds_event(struct input_handle *handle, unsigned int type,
 {
 }
 
+static int input_leds_get_count(struct input_dev *dev)
+{
+	unsigned int led_code;
+	int count = 0;
+
+	for_each_set_bit(led_code, dev->ledbit, LED_CNT)
+		if (input_led_info[led_code].name)
+			count++;
+
+	return count;
+}
+
 static int input_leds_connect(struct input_handler *handler,
 			      struct input_dev *dev,
 			      const struct input_device_id *id)

@@ -81,7 +93,7 @@ static int input_leds_connect(struct input_handler *handler,
 	int led_no;
 	int error;
 
-	num_leds = bitmap_weight(dev->ledbit, LED_CNT);
+	num_leds = input_leds_get_count(dev);
 	if (!num_leds)
 		return -ENXIO;
 

@@ -112,7 +124,7 @@ static int input_leds_connect(struct input_handler *handler,
 		led->handle = &leds->handle;
 		led->code = led_code;
 
-		if (WARN_ON(!input_led_info[led_code].name))
+		if (!input_led_info[led_code].name)
 			continue;
 

drivers/input/input-leds.c static int input_leds_connect(struct input_handler *handler, · line 130

Quoted 2 lines

drivers/input/input-leds.c +0 -0

 		led->cdev.name = kasprintf(GFP_KERNEL, "%s::%s",

Are you sure? AFAICT you need to fix err_unregister_leds not to unregister leds with no name...

Quoted 52 lines · Pavel Machek

Quoted 51 lines · Dmitry Torokhov

Devices may declare more LEDs than what is known to input-leds (HID does this for some devices). Instead of showing ugly warnings on connect and, even worse, oopsing on disconnect, let's simply ignore LEDs that are not known to us.

Reported-by Vlastimil Babka <[email protected]>

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/input-leds.c |   16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

drivers/input/input-leds.c +14 -2

index 074a65e..766bf26 100644
@@ -71,6 +71,18 @@ static void input_leds_event(struct input_handle *handle, unsigned int type,
 {
 }
 
+static int input_leds_get_count(struct input_dev *dev)
+{
+	unsigned int led_code;
+	int count = 0;
+
+	for_each_set_bit(led_code, dev->ledbit, LED_CNT)
+		if (input_led_info[led_code].name)
+			count++;
+
+	return count;
+}
+
 static int input_leds_connect(struct input_handler *handler,
 			      struct input_dev *dev,
 			      const struct input_device_id *id)

@@ -81,7 +93,7 @@ static int input_leds_connect(struct input_handler *handler,
 	int led_no;
 	int error;
 
-	num_leds = bitmap_weight(dev->ledbit, LED_CNT);
+	num_leds = input_leds_get_count(dev);
 	if (!num_leds)
 		return -ENXIO;
 

@@ -112,7 +124,7 @@ static int input_leds_connect(struct input_handler *handler,
 		led->handle = &leds->handle;
 		led->code = led_code;
 
-		if (WARN_ON(!input_led_info[led_code].name))
+		if (!input_led_info[led_code].name)
 			continue;
 

Quoted 5 lines

drivers/input/input-leds.c static int input_leds_connect(struct input_handler *handler, · line 130

Quoted 2 lines

drivers/input/input-leds.c +0 -0

 		led->cdev.name = kasprintf(GFP_KERNEL, "%s::%s",

Are you sure? AFAICT you need to fix err_unregister_leds not to unregister leds with no name...

Well, if we skip unnamed leds and do not include them into total count then we won't need to unregister them.

Quoted 59 lines · Dmitry Torokhov

Quoted 57 lines · Pavel Machek

Quoted 45 lines · Dmitry Torokhov

Devices may declare more LEDs than what is known to input-leds (HID does this for some devices). Instead of showing ugly warnings on connect and, even worse, oopsing on disconnect, let's simply ignore LEDs that are not known to us.

Reported-by Vlastimil Babka <[email protected]>

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/input-leds.c |   16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

drivers/input/input-leds.c +13 -1

index 074a65e..766bf26 100644
@@ -71,6 +71,18 @@ static void input_leds_event(struct input_handle *handle, unsigned int type,
 {
 }
 
+static int input_leds_get_count(struct input_dev *dev)
+{
+	unsigned int led_code;
+	int count = 0;
+
+	for_each_set_bit(led_code, dev->ledbit, LED_CNT)
+		if (input_led_info[led_code].name)
+			count++;
+
+	return count;
+}
+
 static int input_leds_connect(struct input_handler *handler,
 			      struct input_dev *dev,
 			      const struct input_device_id *id)

@@ -81,7 +93,7 @@ static int input_leds_connect(struct input_handler *handler,
 	int led_no;
 	int error;
 
-	num_leds = bitmap_weight(dev->ledbit, LED_CNT);
+	num_leds = input_leds_get_count(dev);
 	if (!num_leds)
 		return -ENXIO;
 

drivers/input/input-leds.c static int input_leds_connect(struct input_handler *handler, · line 130

Quoted 8 lines

drivers/input/input-leds.c +1 -1

@@ -112,7 +124,7 @@ static int input_leds_connect(struct input_handler *handler,
 		led->handle = &leds->handle;
 		led->code = led_code;
 
-		if (WARN_ON(!input_led_info[led_code].name))
+		if (!input_led_info[led_code].name)
 			continue;
 
 		led->cdev.name = kasprintf(GFP_KERNEL, "%s::%s",

Are you sure? AFAICT you need to fix err_unregister_leds not to unregister leds with no name...

Quoted 2 lines

Well, if we skip unnamed leds and do not include them into total count then we won't need to unregister them.

I don't get it.

If there's unnamed led at index 0, and named one at indexes 1 and 2.. and there's -ENOMEM registering 2, it will try to unregister leds 0 and 1, and crash, no?

Devices may declare more LEDs than what is known to input-leds (HID does this for some devices). Instead of showing ugly warnings on connect and, even worse, oopsing on disconnect, let's simply ignore LEDs that are not known to us.

Reported-by Vlastimil Babka <[email protected]>

Quoted 6 lines · Dmitry Torokhov

Devices may declare more LEDs than what is known to input-leds (HID does this for some devices). Instead of showing ugly warnings on connect and, even worse, oopsing on disconnect, let's simply ignore LEDs that are not known to us.

Reported-by Vlastimil Babka <[email protected]>

No more warnings and oopses, so:

Reported-and-tested-by Vlastimil Babka <[email protected]>

FTR, it now looks like this:
gusiac:/sys/bus/hid/devices/0003:046D:C50E.0004/input/input15 # ls -1 | grep input15
input15::charging
input15::mail
input15::misc

All three have max_brightness of 1, but echo 1 > brightness does nothing. Clearly the leds are indeed autonomous only and the mouse shouldn't even report them.