All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Revert "leds: class: Store the color index in struct led_classdev"
@ 2023-11-21 14:26 Takashi Iwai
  2023-11-21 15:10 ` Lee Jones
  0 siblings, 1 reply; 4+ messages in thread
From: Takashi Iwai @ 2023-11-21 14:26 UTC (permalink / raw)
  To: Pavel Machek, Lee Jones
  Cc: Jean-Jacques Hiblot, linux-kernel, linux-leds,
	Johannes Penßel, Jeremy Soller, Hans de Goede,
	Bagas Sanjaya

This reverts commit c7d80059b086c4986cd994a1973ec7a5d75f8eea.

The commit caused regressions to a few drivers (such as Logitech
keyboard or System76 ACPI) that already had "color" leds sysfs entry;
now they conflict with the commonly created sysfs of led class.

For addressing the regression, we revert the commit, so that the
conflicting "color" sysfs entry is removed.

Note that the revert also removes the newly introduced
led_classdev.color field, too.  Since there is no user for that field
yet, it should be OK to revert the whole.  If this field is supposed
to be used in future, only the necessary part can be re-applied
later.

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>
---
 Documentation/ABI/testing/sysfs-class-led |  9 ---------
 drivers/leds/led-class.c                  | 21 ---------------------
 include/linux/leds.h                      |  1 -
 3 files changed, 31 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..4b3f1d49c625 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,
 };
 
@@ -505,10 +491,6 @@ int led_classdev_register_ext(struct device *parent,
 			fwnode_property_read_u32(init_data->fwnode,
 				"max-brightness",
 				&led_cdev->max_brightness);
-
-			if (fwnode_property_present(init_data->fwnode, "color"))
-				fwnode_property_read_u32(init_data->fwnode, "color",
-							 &led_cdev->color);
 		}
 	} else {
 		proposed_name = led_cdev->name;
@@ -518,9 +500,6 @@ int led_classdev_register_ext(struct device *parent,
 	if (ret < 0)
 		return ret;
 
-	if (led_cdev->color >= LED_COLOR_ID_MAX)
-		dev_warn(parent, "LED %s color identifier out of range\n", final_name);
-
 	mutex_init(&led_cdev->led_access);
 	mutex_lock(&led_cdev->led_access);
 	led_cdev->dev = device_create_with_groups(&leds_class, parent, 0,
diff --git a/include/linux/leds.h b/include/linux/leds.h
index aa16dc2a8230..8740b4e47f88 100644
--- a/include/linux/leds.h
+++ b/include/linux/leds.h
@@ -100,7 +100,6 @@ struct led_classdev {
 	const char		*name;
 	unsigned int brightness;
 	unsigned int max_brightness;
-	unsigned int color;
 	int			 flags;
 
 	/* Lower 16 bits reflect status */
-- 
2.35.3


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

* Re: [PATCH] Revert "leds: class: Store the color index in struct led_classdev"
  2023-11-21 14:26 [PATCH] Revert "leds: class: Store the color index in struct led_classdev" Takashi Iwai
@ 2023-11-21 15:10 ` Lee Jones
  2023-11-21 16:13   ` Takashi Iwai
  0 siblings, 1 reply; 4+ messages in thread
From: Lee Jones @ 2023-11-21 15:10 UTC (permalink / raw)
  To: Pavel Machek, Takashi Iwai
  Cc: Jean-Jacques Hiblot, linux-kernel, linux-leds,
	Johannes Penßel, Jeremy Soller, Hans de Goede,
	Bagas Sanjaya

On Tue, 21 Nov 2023 15:26:29 +0100, Takashi Iwai wrote:
> This reverts commit c7d80059b086c4986cd994a1973ec7a5d75f8eea.
> 
> The commit caused regressions to a few drivers (such as Logitech
> keyboard or System76 ACPI) that already had "color" leds sysfs entry;
> now they conflict with the commonly created sysfs of led class.
> 
> For addressing the regression, we revert the commit, so that the
> conflicting "color" sysfs entry is removed.
> 
> [...]

Applied, thanks!

[1/1] Revert "leds: class: Store the color index in struct led_classdev"
      commit: 17e1ad377bb1d39260e0594ee284d8e03f64fdfb

--
Lee Jones [李琼斯]


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

* Re: [PATCH] Revert "leds: class: Store the color index in struct led_classdev"
  2023-11-21 15:10 ` Lee Jones
@ 2023-11-21 16:13   ` Takashi Iwai
  2023-11-22 11:44     ` Lee Jones
  0 siblings, 1 reply; 4+ messages in thread
From: Takashi Iwai @ 2023-11-21 16:13 UTC (permalink / raw)
  To: Lee Jones
  Cc: Pavel Machek, Takashi Iwai, Jean-Jacques Hiblot, linux-kernel,
	linux-leds, Johannes Penßel, Jeremy Soller, Hans de Goede,
	Bagas Sanjaya

On Tue, 21 Nov 2023 16:10:12 +0100,
Lee Jones wrote:
> 
> On Tue, 21 Nov 2023 15:26:29 +0100, Takashi Iwai wrote:
> > This reverts commit c7d80059b086c4986cd994a1973ec7a5d75f8eea.
> > 
> > The commit caused regressions to a few drivers (such as Logitech
> > keyboard or System76 ACPI) that already had "color" leds sysfs entry;
> > now they conflict with the commonly created sysfs of led class.
> > 
> > For addressing the regression, we revert the commit, so that the
> > conflicting "color" sysfs entry is removed.
> > 
> > [...]
> 
> Applied, thanks!
> 
> [1/1] Revert "leds: class: Store the color index in struct led_classdev"
>       commit: 17e1ad377bb1d39260e0594ee284d8e03f64fdfb

It turned out that drivers/leds/rgb/leds-group-multicolor.c
initializes the cdev->color, and this revert breaks it.

So please scratch my previous revert patch.
I'm going to submit a partial revert instead.


thanks,

Takashi

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

* Re: [PATCH] Revert "leds: class: Store the color index in struct led_classdev"
  2023-11-21 16:13   ` Takashi Iwai
@ 2023-11-22 11:44     ` Lee Jones
  0 siblings, 0 replies; 4+ messages in thread
From: Lee Jones @ 2023-11-22 11:44 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, Takashi Iwai wrote:

> On Tue, 21 Nov 2023 16:10:12 +0100,
> Lee Jones wrote:
> > 
> > On Tue, 21 Nov 2023 15:26:29 +0100, Takashi Iwai wrote:
> > > This reverts commit c7d80059b086c4986cd994a1973ec7a5d75f8eea.
> > > 
> > > The commit caused regressions to a few drivers (such as Logitech
> > > keyboard or System76 ACPI) that already had "color" leds sysfs entry;
> > > now they conflict with the commonly created sysfs of led class.
> > > 
> > > For addressing the regression, we revert the commit, so that the
> > > conflicting "color" sysfs entry is removed.
> > > 
> > > [...]
> > 
> > Applied, thanks!
> > 
> > [1/1] Revert "leds: class: Store the color index in struct led_classdev"
> >       commit: 17e1ad377bb1d39260e0594ee284d8e03f64fdfb
> 
> It turned out that drivers/leds/rgb/leds-group-multicolor.c
> initializes the cdev->color, and this revert breaks it.

Indeed:

linux/leds-fixes/drivers/leds/rgb/leds-group-multicolor.c: In function ‘leds_gmc_probe’:
linux/leds-fixes/drivers/leds/rgb/leds-group-multicolor.c:109:49: error: ‘struct led_classdev’ has no member named ‘color’
linux/leds-fixes/drivers/leds/rgb/leds-group-multicolor.c:120:13: error: ‘struct led_classdev’ has no member named ‘color’

> So please scratch my previous revert patch.
> I'm going to submit a partial revert instead.

Sure.

-- 
Lee Jones [李琼斯]

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

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

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-21 14:26 [PATCH] Revert "leds: class: Store the color index in struct led_classdev" Takashi Iwai
2023-11-21 15:10 ` Lee Jones
2023-11-21 16:13   ` Takashi Iwai
2023-11-22 11:44     ` Lee Jones

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.