| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <7ajr2aukrd7bdnns34ur7d37xk4aibaqsjyuoc334uiclay3yt@qiym6ju2kn3c>
Date: Thu, 13 Mar 2025 17:58:19 +0100
From: Benjamin Tissoires <bentiss@...nel.org>
To: Rahul Rameshbabu <sergeantsagara@...tonmail.com>
Cc: linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
linux-input@...r.kernel.org, dri-devel@...ts.freedesktop.org, Jiri Kosina <jikos@...nel.org>,
Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>,
Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>, Benno Lossin <benno.lossin@...ton.me>,
Andreas Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>,
Trevor Gross <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>, Julius Zint <julius@...t.sh>
Subject: Re: [PATCH RFC 2/3] rust: hid: USB Monitor Control Class driver
On Mar 13 2025, Rahul Rameshbabu wrote:
> This code will eventually contain the logic needed to drive the backlight
> of displays that implement the USB Monitor Control Class specification.
> Examples include the Apple Studio Display and Apple Pro Display XDR
> monitors. USB Monitor Control Class encompasses more than just backlight
> control, so the driver could be further extended as monitors support more
> functionality in the specification.
>
> This code is a skeleton currently, where the focus right now is on the core
> Rust API. The driver skeleton was written before approaching the Rust API
> and C binding work. This was done to provide a guide for what the Rust API
> should look like and avoid any rough C binding work from being exposed to
> Rust HID device drivers.
skeletons are good for documentation, but not really for code review as
they can not compile.
You should make this patch part of a documentation in
Documentation/hid/, and squash it with the next one (having a minimal
full driver instead of skeleton+fill in the voids).
Cheers,
Benjamin
>
> To go forward with this driver for the purpose of external monitor
> backlight control, a new DRM backlight API that is scoped per connector
> will be required. I am currently in the process of developing this new API.
> I document the details in my related blog posts. The issue with the current
> backlight API is it was designed on the assumption that only internal
> panels have controllable backlights. Using this assumption combined with
> another that there can only ever be a single internal panel, having more
> than one device register with the backlight interface would confuse
> userspace applications.
>
> Julius Zint originally tried to implement such a driver a bit more than a
> year ago with a C driver but was blocked by the limitations of the
> backlight API. I asked him for permission to continue the work in Rust
> while accrediting him for the HID report parsing logic for the backlight
> support in the USB Monitor Control Class specification.
>
> Cc: Julius Zint <julius@...t.sh>
> Link: https://lore.kernel.org/lkml/20230820094118.20521-1-julius@zint.sh/
> Link: https://binary-eater.github.io/posts/linux_usb_monitor_control/
> Link: https://www.usb.org/sites/default/files/usbmon11.pdf
> Signed-off-by: Rahul Rameshbabu <sergeantsagara@...tonmail.com>
> ---
> drivers/hid/Kconfig | 8 +++++++
> drivers/hid/Makefile | 1 +
> drivers/hid/hid_monitor_control.rs | 37 ++++++++++++++++++++++++++++++
> 3 files changed, 46 insertions(+)
> create mode 100644 drivers/hid/hid_monitor_control.rs
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index e085964c7ffc..92be13acb956 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -722,6 +722,14 @@ config RUST_HID_ABSTRACTIONS
> Adds support needed for HID drivers written in Rust. It provides a
> wrapper around the C hid core.
>
> +config HID_MONITOR_CONTROL
> + tristate "USB Monitor Control Class support"
> + depends on USB_HID
> + depends on RUST_HID_ABSTRACTIONS
> + help
> + Say Y here if you want to enable control over a monitor that uses USB
> + Monitor Control Class.
> +
> config HID_REDRAGON
> tristate "Redragon keyboards"
> default !EXPERT
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index 482b096eea28..bf8b096bcf23 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -86,6 +86,7 @@ obj-$(CONFIG_HID_MCP2221) += hid-mcp2221.o
> obj-$(CONFIG_HID_MAYFLASH) += hid-mf.o
> obj-$(CONFIG_HID_MEGAWORLD_FF) += hid-megaworld.o
> obj-$(CONFIG_HID_MICROSOFT) += hid-microsoft.o
> +obj-$(CONFIG_HID_MONITOR_CONTROL) += hid_monitor_control.o
> obj-$(CONFIG_HID_MONTEREY) += hid-monterey.o
> obj-$(CONFIG_HID_MULTITOUCH) += hid-multitouch.o
> obj-$(CONFIG_HID_NINTENDO) += hid-nintendo.o
> diff --git a/drivers/hid/hid_monitor_control.rs b/drivers/hid/hid_monitor_control.rs
> new file mode 100644
> index 000000000000..18afd69a56d5
> --- /dev/null
> +++ b/drivers/hid/hid_monitor_control.rs
> @@ -0,0 +1,37 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +// Copyright (C) 2025 Rahul Rameshbabu <sergeantsagara@...tonmail.com>
> +
> +use kernel::prelude::*;
> +use kernel::hid::{
> + self,
> + Driver,
> +};
> +
> +struct HidMonitorControl;
> +
> +#[vtable]
> +impl Driver for HidMonitorControl {
> + fn probe(dev: &mut hid::Device, id: &hid::DeviceId) -> Result<()> {
> + /* TODO implement */
> + Ok(())
> + }
> +
> + fn remove(dev: &mut hid::Device) {
> + /* TODO implement */
> + }
> +}
> +
> +kernel::module_hid_driver! {
> + driver: HidMonitorControl,
> + id_table: [
> + kernel::usb_device! {
> + vendor: /* TODO fill in */,
> + product: /* TODO fill in */,
> + },
> + ],
> + name: "monitor_control",
> + author: "Rahul Rameshbabu <sergeantsagara@...tonmail.com>",
> + description: "Driver for the USB Monitor Control Class",
> + license: "GPL",
> +}
> --
> 2.47.2
>
>
Powered by blists - more mailing lists