From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id E947E374D2 for ; Mon, 18 Mar 2024 11:11:44 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710760306; cv=none; b=lgh7U4R6z7uLCdCBSaDYIYWa8bbzaYPAlFmHyIdRIb8sb1tFqo2pvf+nEiyccieMD1PXjkalJXWBJsAHSuAVWC8zkggyUMrNKFOZkGgwpyaRhuDeIReQ8PUBzzW9yxLWL+NsJIJKduPHkjxWAFe8+SQYH4r+lnpohwlgOKiYWqM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1710760306; c=relaxed/simple; bh=vpxP2TC2qbkVtFFP21jKsCrf2Cm91PVj9H+H8hFENmI=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=dAzdHtgxZvP8G6cX8wgmYVIz41264oka7kjOKJ+QXvPUcLcGBMWo2xMHX/f22ERVUvJRJIWxwP1HuaziNYypHmY/jdpy691nqsbDTxPUC3chED7s3c5g3DI09VCZ12CevHqq+jRGKmaJccouwkg3QztIbrWkLTTorhnjeoTcfjw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=hlmdLOVi; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="hlmdLOVi" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1710760303; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=KCRrLRgzcFtsj04jmn2kcF48w7FCWYzPNjCCNXWwhN8=; b=hlmdLOViuolPGWd/IsZNI5nzakY4Fi3Y2G+b6SaUceH75tR5wurCCV+choVAGk2kY3q4Vy PYljugOfsrMBbfrj6S6+/s6S90tOIZ4x2vWFqRtPAytyMQJ+yRQc8ldSyK6wG9BVFw+uCP GnhGj0REFXbI/bxvFiRj7zJZFj16Vqo= Received: from mail-ej1-f71.google.com (mail-ej1-f71.google.com [209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-493-yTD2o1fIMNik47o0oG_mNQ-1; Mon, 18 Mar 2024 07:11:42 -0400 X-MC-Unique: yTD2o1fIMNik47o0oG_mNQ-1 Received: by mail-ej1-f71.google.com with SMTP id a640c23a62f3a-a46ce528bf7so25377866b.0 for ; Mon, 18 Mar 2024 04:11:42 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710760301; x=1711365101; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=KCRrLRgzcFtsj04jmn2kcF48w7FCWYzPNjCCNXWwhN8=; b=Qa4epE4cJtdmusuk1uvRwdj5iNHmit8QIhq91ORQlt9ynU+HNVV6QzfF721Tbtl8Lk jO77GnScg6GaIrdkUwMmrPs9WYwoz6IgcDOV3sTFfJSD4i4JVxpmealhEbXQaxhHEUmx 2FOr0cl2QFNKJn3bDtmftSYajDOaNYYmqMPT5rMrZPmZ5CVaQGJjrsiZdq15z8Jh4xn8 62F4BUnljMpiz7muurCADo/Brc66cOuM4syltUyJZce0Jpk4ZifyZkoFsaJLodlh7mUU 1G1MmdC9DCwPVsohhM3TT91eR9NOkLp7XEIUtKadsb7Gf5cqHzVqRN9RKzguCVxRLWBN Ii8g== X-Forwarded-Encrypted: i=1; AJvYcCUkNiHEnr+SY0Zl8Xfii0Lyq9DrGZPX2jVEJGqdFOtMW3jHM3P2zx/X/RDXTFdlmavuKlivqdJcVqF2j26dch1xYSaWl3ozU/i0qA== X-Gm-Message-State: AOJu0YyCu0391tie/hba2CFqtK8qCxJAHRoT2H03ZWouE1nIEJaeJHvT SzZi2IfzWriQKxndCiIHKozXnU1v10dW+D1VdIaykybTWnRzpdVnYrpCqgPQtpTNyXwyAM9lZVc 8PyzqBIAyKehc2yHWl3k+fgA5rrC/WK06Uy5rJWjKB8FNHHZWT39RS3hp7zHwdPNNMwM= X-Received: by 2002:a17:906:ddb:b0:a46:a1f7:156d with SMTP id p27-20020a1709060ddb00b00a46a1f7156dmr4019164eji.2.1710760301084; Mon, 18 Mar 2024 04:11:41 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEknhYpk/UoBbIXiSyWkooZJA3/ywR9QXOka5TQQi9XFr2kwrDhiLCZltxYyDrNnVlq8/TlEA== X-Received: by 2002:a17:906:ddb:b0:a46:a1f7:156d with SMTP id p27-20020a1709060ddb00b00a46a1f7156dmr4019152eji.2.1710760300703; Mon, 18 Mar 2024 04:11:40 -0700 (PDT) Received: from ?IPV6:2001:1c00:c32:7800:5bfa:a036:83f0:f9ec? (2001-1c00-0c32-7800-5bfa-a036-83f0-f9ec.cable.dynamic.v6.ziggo.nl. [2001:1c00:c32:7800:5bfa:a036:83f0:f9ec]) by smtp.gmail.com with ESMTPSA id sd9-20020a170906ce2900b00a4628cacad4sm4722437ejb.195.2024.03.18.04.11.40 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 18 Mar 2024 04:11:40 -0700 (PDT) Message-ID: <281f9b71-a565-4ff3-8343-ca36d604584d@redhat.com> Date: Mon, 18 Mar 2024 12:11:39 +0100 Precedence: bulk X-Mailing-List: linux-leds@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: Future handling of complex RGB devices on Linux v3 To: Werner Sembach Cc: Lee Jones , jikos@kernel.org, linux-kernel@vger.kernel.org, Jelle van der Waa , Miguel Ojeda , "dri-devel@lists.freedesktop.org" , linux-input@vger.kernel.org, ojeda@kernel.org, linux-leds@vger.kernel.org, Pavel Machek , Gregor Riepl References: <0cdb78b1-7763-4bb6-9582-d70577781e61@tuxedocomputers.com> <7228f2c6-fbdd-4e19-b703-103b8535d77d@redhat.com> <730bead8-6e1d-4d21-90d2-4ee73155887a@tuxedocomputers.com> <952409e1-2f0e-4d7a-a7a9-3b78f2eafec7@redhat.com> <9851a06d-956e-4b57-be63-e10ff1fce8b4@tuxedocomputers.com> <1bc6d6f0-a13d-4148-80cb-9c13dec7ed32@redhat.com> <477d30ee-247e-47e6-bc74-515fd87fdc13@redhat.com> <247b5dcd-fda8-45a7-9896-eabc46568281@tuxedocomputers.com> <825129ea-d389-4c6c-8a23-39f05572e4b4@redhat.com> Content-Language: en-US, nl From: Hans de Goede In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hi Werner, Sorry for the late reply. On 2/22/24 2:14 PM, Werner Sembach wrote: > Hi, > > Thanks everyone for the exhaustive feedback. And at least this thread is a good comprehesive reference for the future ^^. > > To recap the hopefully final UAPI for complex RGB lighting devices: > > - By default there is a singular /sys/class/leds/* entry that treats the device as if it was a single zone RGB keyboard backlight with no special effects. Ack this sounds good. > > - There is an accompanying misc device with the sysfs attributes "name", "device_type",  "firmware_version_string", "serial_number" for device identification and "use_leds_uapi" that defaults to 1. You don't need a char misc device here, you can just make this sysfs attributes on the LED class device's parent device by using device_driver.dev_groups. Please don't use a char misc device just to attach sysfs attributes to it. Also I'm a bit unsure about most of these attributes, "use_leds_uapi" was discussed before and makes sense. But at least for HID devices the rest of this info is already available in sysfs attributes on the HID devices (things like vendor and product id) and since the userspace code needs per device code to drive the kbd anyways it can also have device specific code to retrieve all this info, so the other sysfs attributes just feel like duplicating information. Also there already are a ton of existing hidraw userspace rgbkbd drivers which already get this info from other places. >     - If set to 0 the /sys/class/leds/* entry disappears. The driver should keep the last state the backlight was in active if possible. > >     - If set 1 it appears again. The driver should bring it back to a static 1 zone setting while avoiding flicker if possible. Ack, if this finds it way into some documentation (which it should) please make it clear that this is about the "use_leds_uapi" sysfs attribute. > - If the device is not controllable by for example hidraw, the misc device might also implement additional ioctls or sysfs attributes to allow a more complex low level control for the keyboard backlight. This is will be a highly vendor specific UAPI. IMHO this is the only case where actually using a misc device makes sense, so that you have a chardev to do the ioctls on. misc-device-s should really only be used when you need a chardev under /dev . Since you don't need the chardev for the e.g. hidraw case you should not use a miscdev there IMHO. > >     - The actual logic interacting with this low level UAPI is implemented by a userspace driver > > Implementation wise: For the creation of the misc device with the use_leds_uapi switch a helper function/macro might be useful? Wonder if it should go into leds.h, led-class-multicolor.h, or a new header file? See above, I don't think we want the misc device for the hidraw case, at which point I think the helper becomes unnecessary since just a single sysfs write callback is necessary. Also for adding new sysfs attributes it is strongly encouraged to use device_driver.dev_groups so that the device core handled registering / unregistering the sysfs attributes which fixes a bunch of races; and using device_driver.dev_groups does not mix well with a helper as you've suggested. > > - Out of my head it would look something like this: > > led_classdev_add_optional_misc_control( >     struct led_classdev *led_cdev, >     char* name, >     char* device_type, >     char* firmware_version_string, >     char* serial_number, >     void (*deregister_led)(struct led_classdev *led_cdev), >     void (*reregister_led)(struct led_classdev *led_cdev)) > > Let me know your thoughts and hopefully I can start implementing it soon for one of our devices. I think overall the plan sounds good, with my main suggested change being to not use an unnecessary misc device for the hid-raw case. Regards, Hans