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: <20241023000408.GC1848992-robh@kernel.org>
Date: Tue, 22 Oct 2024 19:04:08 -0500
From: Rob Herring <robh@...nel.org>
To: Danilo Krummrich <dakr@...nel.org>
Cc: gregkh@...uxfoundation.org, rafael@...nel.org, bhelgaas@...gle.com,
	ojeda@...nel.org, alex.gaynor@...il.com, boqun.feng@...il.com,
	gary@...yguo.net, bjorn3_gh@...tonmail.com, benno.lossin@...ton.me,
	tmgross@...ch.edu, a.hindborg@...sung.com, aliceryhl@...gle.com,
	airlied@...il.com, fujita.tomonori@...il.com, lina@...hilina.net,
	pstanner@...hat.com, ajanulgu@...hat.com, lyude@...hat.com,
	daniel.almeida@...labora.com, saravanak@...gle.com,
	rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-pci@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v3 16/16] samples: rust: add Rust platform sample driver

On Tue, Oct 22, 2024 at 11:31:53PM +0200, Danilo Krummrich wrote:
> Add a sample Rust platform driver illustrating the usage of the platform
> bus abstractions.
> 
> This driver probes through either a match of device / driver name or a
> match within the OF ID table.

I know if rust compiles it works, but how does one actually use/test 
this? (I know ways, but I might be in the minority. :) )

The DT unittests already define test platform devices. I'd be happy to 
add a device node there. Then you don't have to muck with the DT on some 
device and it even works on x86 or UML.

And I've started working on DT (fwnode really) property API bindings as 
well, and this will be great to test them with.

> Signed-off-by: Danilo Krummrich <dakr@...nel.org>
> ---
>  MAINTAINERS                          |  1 +
>  samples/rust/Kconfig                 | 10 +++++
>  samples/rust/Makefile                |  1 +
>  samples/rust/rust_driver_platform.rs | 62 ++++++++++++++++++++++++++++
>  4 files changed, 74 insertions(+)
>  create mode 100644 samples/rust/rust_driver_platform.rs
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 173540375863..583b6588fd1e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6986,6 +6986,7 @@ F:	rust/kernel/device_id.rs
>  F:	rust/kernel/devres.rs
>  F:	rust/kernel/driver.rs
>  F:	rust/kernel/platform.rs
> +F:	samples/rust/rust_driver_platform.rs
>  
>  DRIVERS FOR OMAP ADAPTIVE VOLTAGE SCALING (AVS)
>  M:	Nishanth Menon <nm@...com>
> diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
> index 6d468193cdd8..70126b750426 100644
> --- a/samples/rust/Kconfig
> +++ b/samples/rust/Kconfig
> @@ -41,6 +41,16 @@ config SAMPLE_RUST_DRIVER_PCI
>  
>  	  If unsure, say N.
>  
> +config SAMPLE_RUST_DRIVER_PLATFORM
> +	tristate "Platform Driver"
> +	help
> +	  This option builds the Rust Platform driver sample.
> +
> +	  To compile this as a module, choose M here:
> +	  the module will be called rust_driver_platform.
> +
> +	  If unsure, say N.
> +
>  config SAMPLE_RUST_HOSTPROGS
>  	bool "Host programs"
>  	help
> diff --git a/samples/rust/Makefile b/samples/rust/Makefile
> index b66767f4a62a..11fcb312ed36 100644
> --- a/samples/rust/Makefile
> +++ b/samples/rust/Makefile
> @@ -3,5 +3,6 @@
>  obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
>  obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
>  obj-$(CONFIG_SAMPLE_RUST_DRIVER_PCI)		+= rust_driver_pci.o
> +obj-$(CONFIG_SAMPLE_RUST_DRIVER_PLATFORM)	+= rust_driver_platform.o
>  
>  subdir-$(CONFIG_SAMPLE_RUST_HOSTPROGS)		+= hostprogs
> diff --git a/samples/rust/rust_driver_platform.rs b/samples/rust/rust_driver_platform.rs
> new file mode 100644
> index 000000000000..55caaaa4f216
> --- /dev/null
> +++ b/samples/rust/rust_driver_platform.rs
> @@ -0,0 +1,62 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Rust Platform driver sample.
> +
> +use kernel::{c_str, of, platform, prelude::*};
> +
> +struct SampleDriver {
> +    pdev: platform::Device,
> +}
> +
> +struct Info(u32);
> +
> +kernel::of_device_table!(
> +    OF_TABLE,
> +    MODULE_OF_TABLE,
> +    <SampleDriver as platform::Driver>::IdInfo,
> +    [(
> +        of::DeviceId::new(c_str!("redhat,rust-sample-platform-driver")),

Perhaps use the same compatible as the commented example. Same comments 
on that apply to this.

> +        Info(42)

Most of the time this is a pointer to a struct. It would be better to 
show how to do that.

> +    )]
> +);
> +
> +impl platform::Driver for SampleDriver {
> +    type IdInfo = Info;
> +    const ID_TABLE: platform::IdTable<Self::IdInfo> = &OF_TABLE;

Probably want to name this OF_ID_TABLE for when ACPI_ID_TABLE is added.

> +
> +    fn probe(pdev: &mut platform::Device, info: Option<&Self::IdInfo>) -> Result<Pin<KBox<Self>>> {
> +        dev_dbg!(pdev.as_ref(), "Probe Rust Platform driver sample.\n");
> +
> +        match (Self::of_match_device(pdev), info) {

That answers my question on being exposed to drivers. This is a big no 
for me.

> +            (Some(id), Some(info)) => {
> +                dev_info!(
> +                    pdev.as_ref(),
> +                    "Probed by OF compatible match: '{}' with info: '{}'.\n",
> +                    id.compatible(),

As I mentioned, "real" drivers don't need the compatible string.

> +                    info.0
> +                );
> +            }
> +            _ => {
> +                dev_info!(pdev.as_ref(), "Probed by name.\n");
> +            }
> +        };
> +
> +        let drvdata = KBox::new(Self { pdev: pdev.clone() }, GFP_KERNEL)?;
> +
> +        Ok(drvdata.into())
> +    }
> +}
> +
> +impl Drop for SampleDriver {
> +    fn drop(&mut self) {
> +        dev_dbg!(self.pdev.as_ref(), "Remove Rust Platform driver sample.\n");
> +    }
> +}
> +
> +kernel::module_platform_driver! {
> +    type: SampleDriver,
> +    name: "rust_driver_platform",
> +    author: "Danilo Krummrich",
> +    description: "Rust Platform driver",
> +    license: "GPL v2",
> +}
> -- 
> 2.46.2
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ