Hi Lee, On 19-Jun-25 5:17 PM, Lee Jones wrote: > On Thu, 19 Jun 2025, Hans de Goede wrote: > >> Hi Lee, >> >> On 19-Jun-25 11:47 AM, Lee Jones wrote: >>> On Tue, 17 Jun 2025, Armin Wolf wrote: >>> >>>> Am 16.06.25 um 14:46 schrieb Werner Sembach: >>>> >>>>> Hi, small additon >>>>> >>>>> Am 15.06.25 um 19:59 schrieb Armin Wolf: >>>>>> + functionality. >>>>>> + >>>>>> +What: /sys/bus/wmi/devices/ABBC0F6F-8EA1-11D1-00A0-C90629100000[-X]/rainbow_animation >>>>>> +Date: Juni 2025 >>>>>> +KernelVersion: 6.17 >>>>>> +Contact: Armin Wolf <W_Armin@xxxxxx> >>>>>> +Description: >>>>>> + Forces the integrated lightbar to display a rainbow >>>>>> animation when the machine >>>>>> + is not suspended. Writing "enable"/"disable" into this file >>>>>> enables/disables >>>>>> + this functionality. >>>>>> + >>>>>> + Reading this file returns the current status of the rainbow >>>>>> animation functionality. >>>>>> + >>>>>> +What: /sys/bus/wmi/devices/ABBC0F6F-8EA1-11D1-00A0-C90629100000[-X]/breathing_in_suspend >>>>>> +Date: Juni 2025 >>>>>> +KernelVersion: 6.17 >>>>>> +Contact: Armin Wolf <W_Armin@xxxxxx> >>>>>> +Description: >>>>>> + Causes the integrated lightbar to display a breathing >>>>>> animation when the machine >>>>>> + has been suspended and is running on AC power. Writing >>>>>> "enable"/"disable" into >>>>>> + this file enables/disables this functionality. >>>>>> + >>>>>> + Reading this file returns the current status of the >>>>>> breathing animation >>>>>> + functionality. >>>>> >>>>> maybe this would be better under the /sys/class/leds/*/ tree if possible >>>> >>>> I CCed the LED mailing list so that they can give us advice on which location is the preferred one for new drivers. >>> >>> No need to involve the LED subsystem for a hardware function controlled >>> by a single register value just because the interface involves an LED. >> >> Lee, the question here is where put the sysfs attribute to put the lightbar >> in breathing mode e.g. which of these 2 should be used? : >> >> 1. /sys/bus/wmi/devices/ABBC0F6F-8EA1-11D1-00A0-C90629100000[-X]/breathing_in_suspend >> 2. /sys/class/leds/uniwill-lightbar/breathing_in_suspend >> >> I think this is a fair question and since 2. involves the LED class userspace >> API I also think that asking for the LED maintainers input is reasonable. >> >> FWIW I'm not sure myself. 2. is the more logical place / path. But 2. adds >> a custom sysfs attr the LED class device. Whereas 1. adds a custom sysfs attr >> in a place where these are more or less expected. > > Right. It was a reasonable question. Did I imply otherwise? Sorry, my bad, I interpreted your "No need to involve the LED subsystem for a hardware function ..." remark as meaning that you did not understand why you were Cc-ed. I now realize that you meant that you believe the control for this does not need to be under /sys/class/leds/ > If it wasn't clear, my vote (this is not a dictatorship) is for 1. Ok, 1. works for me and that is what the patch is already doing, so lets keep it as as. Regards, Hans
