From: "Luke Jones" <luke@ljones.dev>
To: "Devin Bayer" <dev@doubly.so>, corentin.chary@gmail.com
Cc: "Hans de Goede" <hdegoede@redhat.com>,
platform-driver-x86@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Subject: Re: [PATCH 1/2] platform/x86: asus-wmi: support camera disable LED
Date: Fri, 21 Jun 2024 21:25:04 +1200 [thread overview]
Message-ID: <e47d7929-0977-4f2f-a894-632779ba992e@app.fastmail.com> (raw)
In-Reply-To: <a18c8b3a-90b0-47a7-aff6-a289ecddc2c0@doubly.so>
On Fri, 21 Jun 2024, at 7:50 PM, Devin Bayer wrote:
>
> Thanks for the review, Luke.
>
> On 20/06/2024 23.40, Luke Jones wrote:
> >>
> >> + if (asus_wmi_dev_is_present(asus, ASUS_WMI_DEVID_CAMERA_LED)) {
> >> + asus->camera_led.name = "platform::camera";
> >
> > What do other devices label their camera LED as? The one I could find appears to use `<vendor>::camera`. So maybe `asus::camera` would be better? This also keeps in line with `asus::kbd_backlight`.
>
> I reasoned it would be better to keep the name generic is so out of the
> box desktops could toggle the camera and the LED when KEY_CAMERA is
> pressed, just like with micmute and mute.
This might be true if one relies solely on the filesystem path, which in any case is a bad move and likely to cause the moon to drift away from earth eventually. Most Linux software will use the udev libraries available to filter devices according to any amount of criteria (and if they are not they *really should* - udev is pretty powerful and freeing.
I've tried finding prior art again and there's just not a lot to go on. ".name = "platform" shows very little except a few micmute labels. "::cam" gets one entry. So my guess is this is still a very new thing or it's not important enough to be used..
In any case looking at the rest of the possible LED entries, mostly those following keyboard, the last part of the name being sensible is what counts the most (e.g "scrolllock", "camera"). This might be setting a precedent, and if so I'd be happy with "::camera" in the LED class conveying expectations well enough.
next prev parent reply other threads:[~2024-06-21 9:25 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-20 8:22 [PATCH 0/2] platform/x86: asus-wmi: support a couple Zenbook 2023 features Devin Bayer
2024-06-20 8:22 ` [PATCH 1/2] platform/x86: asus-wmi: support camera disable LED Devin Bayer
2024-06-20 21:40 ` Luke Jones
2024-06-21 7:50 ` Devin Bayer
2024-06-21 9:25 ` Luke Jones [this message]
2024-06-20 8:22 ` [PATCH 2/2] platform/x86: asus-wmi: support newer fan_boost_mode dev_id Devin Bayer
2024-06-20 21:17 ` Luke Jones
2024-06-21 7:53 ` Devin Bayer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e47d7929-0977-4f2f-a894-632779ba992e@app.fastmail.com \
--to=luke@ljones.dev \
--cc=corentin.chary@gmail.com \
--cc=dev@doubly.so \
--cc=hdegoede@redhat.com \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=platform-driver-x86@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®