linux-leds.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] leds: class: Don't expose color sysfs entry
@ 2023-11-21 16:23 Takashi Iwai
  2023-11-21 17:34 ` Hans de Goede
  2023-11-22 11:46 ` Lee Jones
  0 siblings, 2 replies; 3+ messages in thread
From: Takashi Iwai @ 2023-11-21 16:23 UTC (permalink / raw)
  To: Lee Jones
  Cc: Pavel Machek, Jean-Jacques Hiblot, linux-kernel, linux-leds,
	Johannes Penßel, Jeremy Soller, Hans de Goede,
	Bagas Sanjaya

The commit c7d80059b086 ("leds: class: Store the color index in struct
led_classdev") introduced a new sysfs entry "color" that is commonly
created for the led classdev.  Unfortunately, this conflicts with the
"color" sysfs entry of already existing drivers such as Logitech HID
or System76 ACPI drivers.  The driver probe fails due to the conflict,
hence it leads to a severe regression with the missing keyboard, for
example.

This patch reverts partially the change in the commit above for
removing the led class color sysfs entries again for addressing the
regressions.  The newly introduced led_classdev.color field is kept as
it's already used by other driver.

Fixes: c7d80059b086 ("leds: class: Store the color index in struct led_classdev")
Reported-by: Johannes Penßel <johannes.penssel@gmail.com>
Closes: https://lore.kernel.org/r/b5646db3-acff-45aa-baef-df3f660486fb@gmail.com
Link: https://bugzilla.kernel.org/show_bug.cgi?id=218045
Link: https://bugzilla.kernel.org/show_bug.cgi?id=218155
Link: https://bugzilla.suse.com/show_bug.cgi?id=1217172
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---

This is a sort of v2 patch, as it turned out that the full revert
leads to a build error.

 Documentation/ABI/testing/sysfs-class-led |  9 ---------
 drivers/leds/led-class.c                  | 14 --------------
 2 files changed, 23 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-led b/Documentation/ABI/testing/sysfs-class-led
index b2ff0012c0f2..2e24ac3bd7ef 100644
--- a/Documentation/ABI/testing/sysfs-class-led
+++ b/Documentation/ABI/testing/sysfs-class-led
@@ -59,15 +59,6 @@ Description:
 		brightness. Reading this file when no hw brightness change
 		event has happened will return an ENODATA error.
 
-What:		/sys/class/leds/<led>/color
-Date:		June 2023
-KernelVersion:	6.5
-Description:
-		Color of the LED.
-
-		This is a read-only file. Reading this file returns the color
-		of the LED as a string (e.g: "red", "green", "multicolor").
-
 What:		/sys/class/leds/<led>/trigger
 Date:		March 2006
 KernelVersion:	2.6.17
diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 974b84f6bd6a..ba1be15cfd8e 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -75,19 +75,6 @@ static ssize_t max_brightness_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(max_brightness);
 
-static ssize_t color_show(struct device *dev,
-		struct device_attribute *attr, char *buf)
-{
-	const char *color_text = "invalid";
-	struct led_classdev *led_cdev = dev_get_drvdata(dev);
-
-	if (led_cdev->color < LED_COLOR_ID_MAX)
-		color_text = led_colors[led_cdev->color];
-
-	return sysfs_emit(buf, "%s\n", color_text);
-}
-static DEVICE_ATTR_RO(color);
-
 #ifdef CONFIG_LEDS_TRIGGERS
 static BIN_ATTR(trigger, 0644, led_trigger_read, led_trigger_write, 0);
 static struct bin_attribute *led_trigger_bin_attrs[] = {
@@ -102,7 +89,6 @@ static const struct attribute_group led_trigger_group = {
 static struct attribute *led_class_attrs[] = {
 	&dev_attr_brightness.attr,
 	&dev_attr_max_brightness.attr,
-	&dev_attr_color.attr,
 	NULL,
 };
 
-- 
2.35.3


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] leds: class: Don't expose color sysfs entry
  2023-11-21 16:23 [PATCH] leds: class: Don't expose color sysfs entry Takashi Iwai
@ 2023-11-21 17:34 ` Hans de Goede
  2023-11-22 11:46 ` Lee Jones
  1 sibling, 0 replies; 3+ messages in thread
From: Hans de Goede @ 2023-11-21 17:34 UTC (permalink / raw)
  To: Takashi Iwai, Lee Jones
  Cc: Pavel Machek, Jean-Jacques Hiblot, linux-kernel, linux-leds,
	Johannes Penßel, Jeremy Soller, Bagas Sanjaya

Hi,

On 11/21/23 17:23, Takashi Iwai wrote:
> The commit c7d80059b086 ("leds: class: Store the color index in struct
> led_classdev") introduced a new sysfs entry "color" that is commonly
> created for the led classdev.  Unfortunately, this conflicts with the
> "color" sysfs entry of already existing drivers such as Logitech HID
> or System76 ACPI drivers.  The driver probe fails due to the conflict,
> hence it leads to a severe regression with the missing keyboard, for
> example.
> 
> This patch reverts partially the change in the commit above for
> removing the led class color sysfs entries again for addressing the
> regressions.  The newly introduced led_classdev.color field is kept as
> it's already used by other driver.
> 
> Fixes: c7d80059b086 ("leds: class: Store the color index in struct led_classdev")
> Reported-by: Johannes Penßel <johannes.penssel@gmail.com>
> Closes: https://lore.kernel.org/r/b5646db3-acff-45aa-baef-df3f660486fb@gmail.com
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=218045
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=218155
> Link: https://bugzilla.suse.com/show_bug.cgi?id=1217172
> Signed-off-by: Takashi Iwai <tiwai@suse.de>

Thank you for taking care of this, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans

> ---
> 
> This is a sort of v2 patch, as it turned out that the full revert
> leads to a build error.
> 
>  Documentation/ABI/testing/sysfs-class-led |  9 ---------
>  drivers/leds/led-class.c                  | 14 --------------
>  2 files changed, 23 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-led b/Documentation/ABI/testing/sysfs-class-led
> index b2ff0012c0f2..2e24ac3bd7ef 100644
> --- a/Documentation/ABI/testing/sysfs-class-led
> +++ b/Documentation/ABI/testing/sysfs-class-led
> @@ -59,15 +59,6 @@ Description:
>  		brightness. Reading this file when no hw brightness change
>  		event has happened will return an ENODATA error.
>  
> -What:		/sys/class/leds/<led>/color
> -Date:		June 2023
> -KernelVersion:	6.5
> -Description:
> -		Color of the LED.
> -
> -		This is a read-only file. Reading this file returns the color
> -		of the LED as a string (e.g: "red", "green", "multicolor").
> -
>  What:		/sys/class/leds/<led>/trigger
>  Date:		March 2006
>  KernelVersion:	2.6.17
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index 974b84f6bd6a..ba1be15cfd8e 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -75,19 +75,6 @@ static ssize_t max_brightness_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(max_brightness);
>  
> -static ssize_t color_show(struct device *dev,
> -		struct device_attribute *attr, char *buf)
> -{
> -	const char *color_text = "invalid";
> -	struct led_classdev *led_cdev = dev_get_drvdata(dev);
> -
> -	if (led_cdev->color < LED_COLOR_ID_MAX)
> -		color_text = led_colors[led_cdev->color];
> -
> -	return sysfs_emit(buf, "%s\n", color_text);
> -}
> -static DEVICE_ATTR_RO(color);
> -
>  #ifdef CONFIG_LEDS_TRIGGERS
>  static BIN_ATTR(trigger, 0644, led_trigger_read, led_trigger_write, 0);
>  static struct bin_attribute *led_trigger_bin_attrs[] = {
> @@ -102,7 +89,6 @@ static const struct attribute_group led_trigger_group = {
>  static struct attribute *led_class_attrs[] = {
>  	&dev_attr_brightness.attr,
>  	&dev_attr_max_brightness.attr,
> -	&dev_attr_color.attr,
>  	NULL,
>  };
>  


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] leds: class: Don't expose color sysfs entry
  2023-11-21 16:23 [PATCH] leds: class: Don't expose color sysfs entry Takashi Iwai
  2023-11-21 17:34 ` Hans de Goede
@ 2023-11-22 11:46 ` Lee Jones
  1 sibling, 0 replies; 3+ messages in thread
From: Lee Jones @ 2023-11-22 11:46 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Pavel Machek, Jean-Jacques Hiblot, linux-kernel, linux-leds,
	Johannes Penßel, Jeremy Soller, Hans de Goede,
	Bagas Sanjaya

On Tue, 21 Nov 2023 17:23:59 +0100, Takashi Iwai wrote:
> The commit c7d80059b086 ("leds: class: Store the color index in struct
> led_classdev") introduced a new sysfs entry "color" that is commonly
> created for the led classdev.  Unfortunately, this conflicts with the
> "color" sysfs entry of already existing drivers such as Logitech HID
> or System76 ACPI drivers.  The driver probe fails due to the conflict,
> hence it leads to a severe regression with the missing keyboard, for
> example.
> 
> [...]

Applied, thanks!

[1/1] leds: class: Don't expose color sysfs entry
      commit: 8f2244c9af245ff72185c0473827125ee6b2d1a5

--
Lee Jones [李琼斯]


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-11-22 11:46 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-21 16:23 [PATCH] leds: class: Don't expose color sysfs entry Takashi Iwai
2023-11-21 17:34 ` Hans de Goede
2023-11-22 11:46 ` Lee Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).