[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <4cba946d-f0ef-4ac8-bbdf-bcf2e3d0550b@app.fastmail.com>
Date: Fri, 17 Jan 2025 13:02:53 -0500
From: "Mark Pearson" <mpearson-lenovo@...ebb.ca>
To: "Fiona Behrens" <me@...enk.dev>, "Miguel Ojeda" <ojeda@...nel.org>
Cc: "Alex Gaynor" <alex.gaynor@...il.com>, "Pavel Machek" <pavel@....cz>,
"Lee Jones" <lee@...nel.org>, "Jean Delvare" <jdelvare@...e.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>,
"Peter Koch" <pkoch@...ovo.com>, rust-for-linux@...r.kernel.org,
linux-leds@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 5/5] leds: leds_lenovo_se10: LED driver for Lenovo SE10 platform
Hi
On Fri, Jan 17, 2025, at 12:31 PM, Fiona Behrens wrote:
> Hi,
>
> On 17 Jan 2025, at 18:21, Mark Pearson wrote:
>
>> Hi Fiona,
>>
>> On Mon, Jan 13, 2025, at 7:16 AM, Fiona Behrens wrote:
>>> Add driver for the Lenovo ThinkEdge SE10 LED.
>>>
>>> This driver supports controlling the red LED located on the front panel of the
>>> Lenovo SE10 hardware. Additionally, it supports the hardware-triggered
>>> functionality of the LED, which by default is tied to the WWAN trigger.
>>>
>>> The driver is written in Rust and adds basic LED support for the SE10 platform.
>>>
>>> Signed-off-by: Fiona Behrens <me@...enk.dev>
>>> ---
>>> drivers/leds/Kconfig | 10 +++
>>> drivers/leds/Makefile | 1 +
>>> drivers/leds/leds_lenovo_se10.rs | 132 +++++++++++++++++++++++++++++++
>>
>> All the other files are called leds-<name>. Should this be leds-lenovo-se10.rs?
>
> This does not work with rust, as the rust makefile converts this
> filename to a rust crate name, and those crate name cannot have dashes
> in them.
> Not sure if we should fix this to hold to the file name conventions,
> maybe something for @Miguel to decide
Ah - thanks for the clarification (and to Miguel in the follow up)
>
>>
>>> 3 files changed, 143 insertions(+)
>>> create mode 100644 drivers/leds/leds_lenovo_se10.rs
>>>
>>> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
>>> index b784bb74a837..89d9e98189d6 100644
>>> --- a/drivers/leds/Kconfig
>>> +++ b/drivers/leds/Kconfig
>>> @@ -223,6 +223,16 @@ config LEDS_TURRIS_OMNIA
>>> side of CZ.NIC's Turris Omnia router. There are 12 RGB LEDs on the
>>> front panel.
>>>
>>> +config LEDS_LENOVO_SE10
>>> + tristate "LED support for Lenovo ThinkEdge SE10"
>>> + depends on RUST
>>> + depends on (X86 && DMI) || COMPILE_TEST
>>> + depends on HAS_IOPORT
>>> + imply LEDS_TRIGGERS
>>> + help
>>> + This option enables basic support for the LED found on the front of
>>> + Lenovo's SE10 ThinkEdge. There is one user controlable LED on the
>>> front panel.
>>> +
>>> config LEDS_LM3530
>>> tristate "LCD Backlight driver for LM3530"
>>> depends on LEDS_CLASS
>>> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
>>> index 18afbb5a23ee..2cff22cbafcf 100644
>>> --- a/drivers/leds/Makefile
>>> +++ b/drivers/leds/Makefile
>>> @@ -37,6 +37,7 @@ obj-$(CONFIG_LEDS_IP30) += leds-ip30.o
>>> obj-$(CONFIG_LEDS_IPAQ_MICRO) += leds-ipaq-micro.o
>>> obj-$(CONFIG_LEDS_IS31FL319X) += leds-is31fl319x.o
>>> obj-$(CONFIG_LEDS_IS31FL32XX) += leds-is31fl32xx.o
>>> +obj-$(CONFIG_LEDS_LENOVO_SE10) += leds_lenovo_se10.o
>>
>> Same note above on name formatting.
>>
>>> obj-$(CONFIG_LEDS_LM3530) += leds-lm3530.o
>>> obj-$(CONFIG_LEDS_LM3532) += leds-lm3532.o
>>> obj-$(CONFIG_LEDS_LM3533) += leds-lm3533.o
>>> diff --git a/drivers/leds/leds_lenovo_se10.rs
>>> b/drivers/leds/leds_lenovo_se10.rs
>>> new file mode 100644
>>> index 000000000000..d704125610a4
>>> --- /dev/null
>>> +++ b/drivers/leds/leds_lenovo_se10.rs
>>> @@ -0,0 +1,132 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +//! LED driver for Lenovo ThinkEdge SE10.
>>> +
>>> +use kernel::ioport::{Region, ResourceSize};
>>> +#[cfg(CONFIG_LEDS_TRIGGERS)]
>>> +use kernel::leds::triggers;
>>> +use kernel::leds::{Led, LedConfig, Operations};
>>> +use kernel::prelude::*;
>>> +use kernel::time::Delta;
>>> +use kernel::{c_str, dmi_device_table};
>>> +
>>> +module! {
>>> + type: SE10,
>>> + name: "leds_lenovo_se10",
>>> + author: "Fiona Behrens <me@...enk.dev>",
>>> + description: "LED driver for Lenovo ThinkEdge SE10",
>>> + license: "GPL",
>>> +}
>>> +
>>> +dmi_device_table!(5, SE10_DMI_TABLE, [
>>> + "LENOVO-SE10": [SysVendor: "LENOVO", ProductName: "12NH"],
>>> + "LENOVO-SE10": [SysVendor: "LENOVO", ProductName: "12NJ"],
>>> + "LENOVO-SE10": [SysVendor: "LENOVO", ProductName: "12NK"],
>>> + "LENOVO-SE10": [SysVendor: "LENOVO", ProductName: "12NL"],
>>> + "LENOVO-SE10": [SysVendor: "LENOVO", ProductName: "12NM"],
>>> +]);
>>> +
>>> +struct SE10 {
>>> + /// Led registration
>>> + _led: Pin<KBox<Led<LedSE10>>>,
>>> +}
>>> +
>>> +impl kernel::Module for SE10 {
>>> + fn init(_module: &'static ThisModule) -> Result<Self> {
>>> + if SE10_DMI_TABLE.check_system().is_none() {
>>> + return Err(ENODEV);
>>> + }
>>> +
>>> + let led = KBox::try_pin_init(
>>> + Led::register(
>>> + None,
>>> + LedConfig {
>>> + name: Some(c_str!("platform:red:user")),
>>> + #[cfg(CONFIG_LEDS_TRIGGERS)]
>>> + hardware_trigger: Some(kernel::sync::Arc::pin_init(
>>> + triggers::Hardware::register(c_str!("wwan")),
>>
>> I was curious as to why the "wwan" in here.
>
> This is the hardware trigger, as to the documentation I found from
> Lenovo the trigger mode gives hardware control to the wwan module if
> installed in the hardware.
>
Ah - I should probably have known that :) All good.
Mark
Powered by blists - more mailing lists