[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6a4134f1-4075-43d6-b238-56a31197f7fc@redhat.com>
Date: Tue, 21 Nov 2023 18:34:35 +0100
From: Hans de Goede <hdegoede@...hat.com>
To: Takashi Iwai <tiwai@...e.de>, Lee Jones <lee@...nel.org>
Cc: Pavel Machek <pavel@....cz>,
Jean-Jacques Hiblot <jjhiblot@...phandler.com>,
linux-kernel@...r.kernel.org, linux-leds@...r.kernel.org,
Johannes Penßel <johannes.penssel@...il.com>,
Jeremy Soller <jeremy@...tem76.com>,
Bagas Sanjaya <bagasdotme@...il.com>
Subject: Re: [PATCH] leds: class: Don't expose color sysfs entry
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@...il.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@...e.de>
Thank you for taking care of this, patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@...hat.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,
> };
>
Powered by blists - more mailing lists