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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d4b835e75db528274de38893644cdf7c61115627.camel@posteo.de>
Date: Sun, 21 Dec 2025 12:39:54 +0000
From: Markus Probst <markus.probst@...teo.de>
To: Dirk Behme <dirk.behme@...il.com>, Rob Herring <robh@...nel.org>, Greg
 Kroah-Hartman <gregkh@...uxfoundation.org>, Jiri Slaby
 <jirislaby@...nel.org>, Miguel Ojeda <ojeda@...nel.org>,  Boqun Feng
 <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
 Björn Roy Baron	 <bjorn3_gh@...tonmail.com>, Benno
 Lossin <lossin@...nel.org>, Andreas Hindborg	 <a.hindborg@...nel.org>,
 Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross	 <tmgross@...ch.edu>,
 Danilo Krummrich <dakr@...nel.org>, Kari Argillander	
 <kari.argillander@...il.com>
Cc: linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org, 
	rust-for-linux@...r.kernel.org
Subject: Re: [PATCH RFC 3/4] samples: rust: add Rust serial device bus
 sample device driver

Hi Dirk,

On Sun, 2025-12-21 at 10:11 +0100, Dirk Behme wrote:
> Hi Markus,
> 
> On 20.12.25 19:44, Markus Probst wrote:
> > Add a sample Rust serial device bus device driver illustrating the usage
> > of the platform bus abstractions.
> > 
> > This drivers probes through either a match of device / driver name or a
> > match within the OF ID table.
> > ---
> >  samples/rust/Kconfig               |  10 +++
> >  samples/rust/Makefile              |   1 +
> >  samples/rust/rust_driver_serdev.rs | 175 +++++++++++++++++++++++++++++++++++++
> >  3 files changed, 186 insertions(+)
> ...
> > diff --git a/samples/rust/rust_driver_serdev.rs b/samples/rust/rust_driver_serdev.rs
> > new file mode 100644
> > index 000000000000..f23b38a26c32
> > --- /dev/null
> > +++ b/samples/rust/rust_driver_serdev.rs
> > @@ -0,0 +1,175 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +//! Rust Serial device bus device driver sample.
> > +
> > +use kernel::{
> > +    acpi,
> > +    device::{
> > +        self,
> > +        property::{
> > +            FwNodeReferenceArgs,
> > +            NArgs, //
> > +        },
> > +        Bound,
> > +        Core, //
> > +    },
> > +    of,
> > +    prelude::*,
> > +    serdev,
> > +    str::CString,
> > +    sync::aref::ARef, //
> > +};
> > +
> > +struct SampleDriver {
> > +    sdev: ARef<serdev::Device>,
> > +}
> > +
> > +struct Info(u32);
> > +
> > +kernel::of_device_table!(
> > +    OF_TABLE,
> > +    MODULE_OF_TABLE,
> > +    <SampleDriver as serdev::Driver>::IdInfo,
> > +    [(of::DeviceId::new(c"test,rust_driver_serdev"), Info(42))]
> 
> I stopped reading here regarding the new "rust_driver_serdev" but
> re-reading Rob's
> 
> https://lore.kernel.org/rust-for-linux/20241022234712.GB1848992-robh@kernel.org/
> 
> adding "test,<whatever>" should be fine as-is without any documenation.
> 
> > +);
> > +
> > +kernel::acpi_device_table!(
> > +    ACPI_TABLE,
> > +    MODULE_ACPI_TABLE,
> > +    <SampleDriver as serdev::Driver>::IdInfo,
> > +    [(acpi::DeviceId::new(c"LNUXBEEF"), Info(0))]
> > +);
> > +
> > +#[vtable]
> > +impl serdev::Driver for SampleDriver {
> > +    type IdInfo = Info;
> > +    type InitialData = ();
> > +    type LateProbeData = ();
> > +    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
> > +    const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);
> > +
> > +    fn probe(sdev: &serdev::Device, info: Option<&Self::IdInfo>) -> impl PinInit<Self, Error> {
> > +        let dev = sdev.as_ref();
> > +
> > +        dev_dbg!(dev, "Probe Rust Serial device bus device driver sample.\n");
> > +
> > +        if let Some(info) = info {
> > +            dev_info!(dev, "Probed with info: '{}'.\n", info.0);
> > +        }
> 
> Last time I had a look to the log output from rust_driver_platform.rs
> (where this is copied from?) I was slightly confused to see the
> "Probed with ..." in the log but not the "Probe Rust ...". Well, I
> hadn't DEBUG enabled. So I wonder if the combination of `dev_dbg!()`
> and `dev_info!()` this way should be improved? At least in
> rust_driver_platform.rs because we could drop that here completely? I
> mean in rust_driver_platform.rs it makes sense to demonstrate how
> `info` is supposed to work. But do we need that here?
> 
> 
> > +        if dev.fwnode().is_some_and(|node| node.is_of_node()) {
> > +            Self::properties_parse(dev)?;
> > +        }
> 
> 
> This is a left over from copy & paste? I mean having all this
> `properties_parse()` below and calling it here does not make any sense
> here? And should be dropped completely?
It was meant to show, that this is the function in which fwnode
properties should be parsed. As I did indeed base the sample on the
platform driver sample, I left it as-is. I will minimize it down to 1
property without an extra function, given that the platform driver
sample already shows how to use properties.
> 
> 
> > +
> > +        Ok(Self { sdev: sdev.into() })
> > +    }
> > +
> > +    fn configure(
> > +        sdev: &serdev::Device<Core>,
> > +        _this: Pin<&Self>,
> > +        _id_info: Option<&Self::IdInfo>,
> > +    ) -> Result {
> > +        dev_dbg!(
> > +            sdev.as_ref(),
> > +            "Configure Rust Serial device bus device driver sample.\n"
> > +        );
> > +
> > +        sdev.set_baudrate(115200);
> > +        sdev.set_flow_control(false);
> > +        sdev.set_parity(serdev::Parity::None)?;
> > +        Ok(())
> > +    }
> > +
> > +    fn late_probe(
> > +        sdev: &serdev::Device<Bound>,
> > +        _this: Pin<&Self>,
> > +        _initial_data: Self::InitialData,
> > +    ) -> impl PinInit<Self::LateProbeData, Error> {
> > +        dev_dbg!(
> > +            sdev.as_ref(),
> > +            "Late Probe Rust Serial device bus device driver sample.\n"
> > +        );
> > +        Ok(())
> > +    }
> > +
> > +    fn receive(
> > +        sdev: &serdev::Device<Bound>,
> > +        _this: Pin<&Self>,
> > +        _late_probe_this: Pin<&Self::LateProbeData>,
> > +        data: &[u8],
> > +    ) -> usize {
> > +        let _ = sdev.write_all(data, serdev::Timeout::MaxScheduleTimeout);
> 
> 
> Is it intended to have a function with the name `receive()`calling
> `write()`?
This sample driver echos any data it receives back to the serial
device.

It shows how to receive data from the serial device and how to send
data to the serial device.

It depends on the driver, if it is necessary to call write inside
receive. The serial device may want a reply or ack from the driver
after it sent a message.

> 
> > +        data.len()
> > +    }
> > +}
> > +
> > +impl SampleDriver {
> > +    fn properties_parse(dev: &device::Device) -> Result {
> 
> 
> As mentioned above I think this is a left over from copy & paste and
> should be dropped?
> 
> Cheers,
> 
> Dirk

Thanks
- Markus Probst

Download attachment "signature.asc" of type "application/pgp-signature" (871 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ