| Message ID | 20250113121620.21598-6-me@kloenk.dev |
|---|---|
| State | New |
| Headers | show |
| Series | Rust LED Driver Abstractions and Lenovo SE10 Driver with DMI and I/O Port Support | expand |
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@kloenk.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? > 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@kloenk.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. > + GFP_KERNEL, > + )?), > + ..LedConfig::new(kernel::leds::Color::Red, LedSE10) > + }, > + ), > + GFP_KERNEL, > + )?; > + > + Ok(Self { _led: led }) > + } > +} > + > +/// Valid led commands. > +#[repr(u8)] > +#[allow(missing_docs)] > +enum LedCommand { > + #[cfg(CONFIG_LEDS_TRIGGERS)] > + Trigger = 0xB2, > + Off = 0xB3, > + On = 0xB4, > + Blink = 0xB5, > +} > + > +struct LedSE10; > + > +impl LedSE10 { > + /// Base address of the command port. > + const CMD_PORT: ResourceSize = 0x6C; > + /// Length of the command port. > + const CMD_LEN: ResourceSize = 1; > + /// Blink duration the hardware supports. > + const HW_DURATION: Delta = Delta::from_millis(1000); > + > + /// Request led region. > + fn request_cmd_region(&self) -> Result<Region<'static>> { > + Region::request_muxed(Self::CMD_PORT, Self::CMD_LEN, > c_str!("leds_lenovo_se10")) > + .ok_or(EBUSY) > + } > + > + /// Send command. > + fn send_cmd(&self, cmd: LedCommand) -> Result { > + let region = self.request_cmd_region()?; > + region.outb(cmd as u8, 0); > + Ok(()) > + } > +} > + > +#[vtable] > +impl Operations for LedSE10 { > + type This = Led<LedSE10>; > + > + const MAX_BRIGHTNESS: u8 = 1; > + > + fn brightness_set(this: &mut Self::This, brightness: u8) { > + if let Err(e) = if brightness == 0 { > + this.data.send_cmd(LedCommand::Off) > + } else { > + this.data.send_cmd(LedCommand::On) > + } { > + pr_warn!("Failed to set led: {e:?}\n)") > + } > + } > + > + fn blink_set( > + this: &mut Self::This, > + delay_on: Delta, > + delay_off: Delta, > + ) -> Result<(Delta, Delta)> { > + if !(delay_on.is_zero() && delay_off.is_zero() > + || delay_on == Self::HW_DURATION && delay_off == > Self::HW_DURATION) > + { > + return Err(EINVAL); > + } > + > + this.data.send_cmd(LedCommand::Blink)?; > + Ok((Self::HW_DURATION, Self::HW_DURATION)) > + } > +} > + > +#[vtable] > +#[cfg(CONFIG_LEDS_TRIGGERS)] > +impl triggers::HardwareOperations for LedSE10 { > + fn activate(this: &mut Self::This) -> Result { > + this.data.send_cmd(LedCommand::Trigger) > + } > +} > -- > 2.47.0 I don't have the competence to review the rust code I'm afraid - so my limited feedback above is the best I can do. Not sure it's really worth a reviewed-by tag, but I did read the code and learnt a little about rust in the process (which was fun). I did test your changes on my SE10 system and it works well. Tested-by: Mark Pearson <mpearson-lenovo@squebb.ca> Thanks! Mark
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@kloenk.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 > >> 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@kloenk.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. > >> + GFP_KERNEL, >> + )?), >> + ..LedConfig::new(kernel::leds::Color::Red, LedSE10) >> + }, >> + ), >> + GFP_KERNEL, >> + )?; >> + >> + Ok(Self { _led: led }) >> + } >> +} >> + >> +/// Valid led commands. >> +#[repr(u8)] >> +#[allow(missing_docs)] >> +enum LedCommand { >> + #[cfg(CONFIG_LEDS_TRIGGERS)] >> + Trigger = 0xB2, >> + Off = 0xB3, >> + On = 0xB4, >> + Blink = 0xB5, >> +} >> + >> +struct LedSE10; >> + >> +impl LedSE10 { >> + /// Base address of the command port. >> + const CMD_PORT: ResourceSize = 0x6C; >> + /// Length of the command port. >> + const CMD_LEN: ResourceSize = 1; >> + /// Blink duration the hardware supports. >> + const HW_DURATION: Delta = Delta::from_millis(1000); >> + >> + /// Request led region. >> + fn request_cmd_region(&self) -> Result<Region<'static>> { >> + Region::request_muxed(Self::CMD_PORT, Self::CMD_LEN, >> c_str!("leds_lenovo_se10")) >> + .ok_or(EBUSY) >> + } >> + >> + /// Send command. >> + fn send_cmd(&self, cmd: LedCommand) -> Result { >> + let region = self.request_cmd_region()?; >> + region.outb(cmd as u8, 0); >> + Ok(()) >> + } >> +} >> + >> +#[vtable] >> +impl Operations for LedSE10 { >> + type This = Led<LedSE10>; >> + >> + const MAX_BRIGHTNESS: u8 = 1; >> + >> + fn brightness_set(this: &mut Self::This, brightness: u8) { >> + if let Err(e) = if brightness == 0 { >> + this.data.send_cmd(LedCommand::Off) >> + } else { >> + this.data.send_cmd(LedCommand::On) >> + } { >> + pr_warn!("Failed to set led: {e:?}\n)") >> + } >> + } >> + >> + fn blink_set( >> + this: &mut Self::This, >> + delay_on: Delta, >> + delay_off: Delta, >> + ) -> Result<(Delta, Delta)> { >> + if !(delay_on.is_zero() && delay_off.is_zero() >> + || delay_on == Self::HW_DURATION && delay_off == >> Self::HW_DURATION) >> + { >> + return Err(EINVAL); >> + } >> + >> + this.data.send_cmd(LedCommand::Blink)?; >> + Ok((Self::HW_DURATION, Self::HW_DURATION)) >> + } >> +} >> + >> +#[vtable] >> +#[cfg(CONFIG_LEDS_TRIGGERS)] >> +impl triggers::HardwareOperations for LedSE10 { >> + fn activate(this: &mut Self::This) -> Result { >> + this.data.send_cmd(LedCommand::Trigger) >> + } >> +} >> -- >> 2.47.0 > > I don't have the competence to review the rust code I'm afraid - so my limited feedback above is the best I can do. Not sure it's really worth a reviewed-by tag, but I did read the code and learnt a little about rust in the process (which was fun). > > I did test your changes on my SE10 system and it works well. > Tested-by: Mark Pearson <mpearson-lenovo@squebb.ca> Thanks a lot, Fiona > > Thanks! > Mark
On Fri, Jan 17, 2025 at 6:22 PM Mark Pearson <mpearson-lenovo@squebb.ca> wrote: > > All the other files are called leds-<name>. Should this be leds-lenovo-se10.rs? Rust requires underscores for the crate name, so we started with just using the same name for both since it is simpler and to avoid ending up with the usual mix of dashes/underscores in the future. But we can allow them if subsystems think it is important to maintain consistency within their folder etc. Cheers, Miguel
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@kloenk.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@kloenk.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
On Mon, Jan 13, 2025 at 01:16:20PM +0100, 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@kloenk.dev> > --- > drivers/leds/Kconfig | 10 +++ > drivers/leds/Makefile | 1 + > drivers/leds/leds_lenovo_se10.rs | 132 +++++++++++++++++++++++++++++++ > 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 > 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@kloenk.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")), There are currently two LED drivers utilizing the led_hw_trigger_type mechanism to make certain triggers available only for certain LEDs: - the leds-cros_ec.c driver, which registers the trigger under the name "chromeos-auto", to suggest that activating the trigger on this LED will make it blink automatically by hardware and that it is ChromeOS specific, - the leds-turris-omnia.c driver, which registers the trigger under the name "omnia-mcu", to suggest that activating the trigger will make the LED blinking be controller by the MCU on Turris Omnia. Using the name "wwan" for this trigger is too general. In the future someone may want to create a software "wwan" trigger that will be available for any LED class device, for example... Please change the name of this LED-private trigger. > + GFP_KERNEL, > + )?), > + ..LedConfig::new(kernel::leds::Color::Red, LedSE10) > + }, > + ), > + GFP_KERNEL, > + )?; > + > + Ok(Self { _led: led }) > + } > +} > + > +/// Valid led commands. > +#[repr(u8)] > +#[allow(missing_docs)] > +enum LedCommand { > + #[cfg(CONFIG_LEDS_TRIGGERS)] > + Trigger = 0xB2, > + Off = 0xB3, > + On = 0xB4, > + Blink = 0xB5, > +} > + > +struct LedSE10; > + > +impl LedSE10 { > + /// Base address of the command port. > + const CMD_PORT: ResourceSize = 0x6C; > + /// Length of the command port. > + const CMD_LEN: ResourceSize = 1; > + /// Blink duration the hardware supports. > + const HW_DURATION: Delta = Delta::from_millis(1000); > + > + /// Request led region. > + fn request_cmd_region(&self) -> Result<Region<'static>> { > + Region::request_muxed(Self::CMD_PORT, Self::CMD_LEN, c_str!("leds_lenovo_se10")) > + .ok_or(EBUSY) > + } > + > + /// Send command. > + fn send_cmd(&self, cmd: LedCommand) -> Result { > + let region = self.request_cmd_region()?; > + region.outb(cmd as u8, 0); > + Ok(()) > + } > +} > + > +#[vtable] > +impl Operations for LedSE10 { > + type This = Led<LedSE10>; > + > + const MAX_BRIGHTNESS: u8 = 1; > + > + fn brightness_set(this: &mut Self::This, brightness: u8) { > + if let Err(e) = if brightness == 0 { > + this.data.send_cmd(LedCommand::Off) > + } else { > + this.data.send_cmd(LedCommand::On) > + } { > + pr_warn!("Failed to set led: {e:?}\n)") > + } > + } > + > + fn blink_set( > + this: &mut Self::This, > + delay_on: Delta, > + delay_off: Delta, > + ) -> Result<(Delta, Delta)> { > + if !(delay_on.is_zero() && delay_off.is_zero() > + || delay_on == Self::HW_DURATION && delay_off == Self::HW_DURATION) > + { > + return Err(EINVAL); > + } > + > + this.data.send_cmd(LedCommand::Blink)?; > + Ok((Self::HW_DURATION, Self::HW_DURATION)) > + } > +} > + > +#[vtable] > +#[cfg(CONFIG_LEDS_TRIGGERS)] > +impl triggers::HardwareOperations for LedSE10 { > + fn activate(this: &mut Self::This) -> Result { > + this.data.send_cmd(LedCommand::Trigger) > + } No deactivation method for the trigger? NACK. The driver must implement the deactivation method, since LED core always allows disabling LED triggers. See led-trigger.c function led_trigger_write(): if "none" is written to the sysfs `trigger` file, the trigger is removed and the `trigger` file will afterwards report that no trigger is activated on the LED. Since you did not implement the deactivation method, this will result in the system thinking that no LED trigger is selected on the LED, but in fact your LED's blinking will still be controlled by hardware. Marek
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 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@kloenk.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")), + GFP_KERNEL, + )?), + ..LedConfig::new(kernel::leds::Color::Red, LedSE10) + }, + ), + GFP_KERNEL, + )?; + + Ok(Self { _led: led }) + } +} + +/// Valid led commands. +#[repr(u8)] +#[allow(missing_docs)] +enum LedCommand { + #[cfg(CONFIG_LEDS_TRIGGERS)] + Trigger = 0xB2, + Off = 0xB3, + On = 0xB4, + Blink = 0xB5, +} + +struct LedSE10; + +impl LedSE10 { + /// Base address of the command port. + const CMD_PORT: ResourceSize = 0x6C; + /// Length of the command port. + const CMD_LEN: ResourceSize = 1; + /// Blink duration the hardware supports. + const HW_DURATION: Delta = Delta::from_millis(1000); + + /// Request led region. + fn request_cmd_region(&self) -> Result<Region<'static>> { + Region::request_muxed(Self::CMD_PORT, Self::CMD_LEN, c_str!("leds_lenovo_se10")) + .ok_or(EBUSY) + } + + /// Send command. + fn send_cmd(&self, cmd: LedCommand) -> Result { + let region = self.request_cmd_region()?; + region.outb(cmd as u8, 0); + Ok(()) + } +} + +#[vtable] +impl Operations for LedSE10 { + type This = Led<LedSE10>; + + const MAX_BRIGHTNESS: u8 = 1; + + fn brightness_set(this: &mut Self::This, brightness: u8) { + if let Err(e) = if brightness == 0 { + this.data.send_cmd(LedCommand::Off) + } else { + this.data.send_cmd(LedCommand::On) + } { + pr_warn!("Failed to set led: {e:?}\n)") + } + } + + fn blink_set( + this: &mut Self::This, + delay_on: Delta, + delay_off: Delta, + ) -> Result<(Delta, Delta)> { + if !(delay_on.is_zero() && delay_off.is_zero() + || delay_on == Self::HW_DURATION && delay_off == Self::HW_DURATION) + { + return Err(EINVAL); + } + + this.data.send_cmd(LedCommand::Blink)?; + Ok((Self::HW_DURATION, Self::HW_DURATION)) + } +} + +#[vtable] +#[cfg(CONFIG_LEDS_TRIGGERS)] +impl triggers::HardwareOperations for LedSE10 { + fn activate(this: &mut Self::This) -> Result { + this.data.send_cmd(LedCommand::Trigger) + } +}
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@kloenk.dev> --- drivers/leds/Kconfig | 10 +++ drivers/leds/Makefile | 1 + drivers/leds/leds_lenovo_se10.rs | 132 +++++++++++++++++++++++++++++++ 3 files changed, 143 insertions(+) create mode 100644 drivers/leds/leds_lenovo_se10.rs