* [PATCH v1] rust: add 'firmware' field support to module! macro
@ 2024-04-26 9:56 FUJITA Tomonori
2024-05-01 9:06 ` Benno Lossin
0 siblings, 1 reply; 3+ messages in thread
From: FUJITA Tomonori @ 2024-04-26 9:56 UTC (permalink / raw
To: rust-for-linux; +Cc: miguel.ojeda.sandonis, benno.lossin
This adds 'firmware' field support to module! macro, corresponds to
MODULE_FIRMWARE macro. You can specify the file names of binary
firmware that the kernel module requires. The information is embedded
in the modinfo section of the kernel module. For example, a tool to
build an initramfs uses this information to put the firmware files
into the initramfs image.
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
rust/macros/lib.rs | 31 +++++++++++++++++++++++++++++++
rust/macros/module.rs | 18 ++++++++++++++++--
2 files changed, 47 insertions(+), 2 deletions(-)
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index f489f3157383..73fa768fce64 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -67,6 +67,36 @@
/// }
/// ```
///
+/// ## Firmware
+///
+/// The following example shows how to declare a kernel module that needs
+/// to load binary firmware files. You need to specify the file names of
+/// the firmware in the `firmware` field. The information is embedded
+/// in the `modinfo` section of the kernel module. For example, a tool to
+/// build an initramfs uses this information to put the firmware files into
+/// the initramfs image.
+///
+/// ```ignore
+/// use kernel::prelude::*;
+///
+/// module!{
+/// type: MyDeviceDriverModule,
+/// name: "my_device_driver_module",
+/// author: "Rust for Linux Contributors",
+/// description: "My device driver requires firmware",
+/// license: "GPL",
+/// firmware: ["my_device_firmware1.bin", "my_device_firmware2.bin"],
+/// }
+///
+/// struct MyDeviceDriverModule;
+///
+/// impl kernel::Module for MyDeviceDriverModule {
+/// fn init() -> Result<Self> {
+/// Ok(Self)
+/// }
+/// }
+/// ```
+///
/// # Supported argument types
/// - `type`: type which implements the [`Module`] trait (required).
/// - `name`: byte array of the name of the kernel module (required).
@@ -74,6 +104,7 @@
/// - `description`: byte array of the description of the kernel module.
/// - `license`: byte array of the license of the kernel module (required).
/// - `alias`: byte array of alias name of the kernel module.
+/// - `firmware`: array of the file names of firmware required by the kernel module.
#[proc_macro]
pub fn module(ts: TokenStream) -> TokenStream {
module::module(ts)
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 27979e582e4b..5fdaac17a733 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -97,14 +97,22 @@ struct ModuleInfo {
author: Option<String>,
description: Option<String>,
alias: Option<Vec<String>>,
+ firmware: Option<Vec<String>>,
}
impl ModuleInfo {
fn parse(it: &mut token_stream::IntoIter) -> Self {
let mut info = ModuleInfo::default();
- const EXPECTED_KEYS: &[&str] =
- &["type", "name", "author", "description", "license", "alias"];
+ const EXPECTED_KEYS: &[&str] = &[
+ "type",
+ "name",
+ "author",
+ "description",
+ "license",
+ "alias",
+ "firmware",
+ ];
const REQUIRED_KEYS: &[&str] = &["type", "name", "license"];
let mut seen_keys = Vec::new();
@@ -131,6 +139,7 @@ fn parse(it: &mut token_stream::IntoIter) -> Self {
"description" => info.description = Some(expect_string(it)),
"license" => info.license = expect_string_ascii(it),
"alias" => info.alias = Some(expect_string_array(it)),
+ "firmware" => info.firmware = Some(expect_string_array(it)),
_ => panic!(
"Unknown key \"{}\". Valid keys are: {:?}.",
key, EXPECTED_KEYS
@@ -186,6 +195,11 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
modinfo.emit("alias", &alias);
}
}
+ if let Some(firmware) = info.firmware {
+ for fw in firmware {
+ modinfo.emit("firmware", &fw);
+ }
+ }
// Built-in modules also export the `file` modinfo string.
let file =
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v1] rust: add 'firmware' field support to module! macro
2024-04-26 9:56 [PATCH v1] rust: add 'firmware' field support to module! macro FUJITA Tomonori
@ 2024-05-01 9:06 ` Benno Lossin
2024-05-01 12:37 ` FUJITA Tomonori
0 siblings, 1 reply; 3+ messages in thread
From: Benno Lossin @ 2024-05-01 9:06 UTC (permalink / raw
To: FUJITA Tomonori, rust-for-linux; +Cc: miguel.ojeda.sandonis
On 26.04.24 11:56, FUJITA Tomonori wrote:
> This adds 'firmware' field support to module! macro, corresponds to
> MODULE_FIRMWARE macro. You can specify the file names of binary
> firmware that the kernel module requires. The information is embedded
> in the modinfo section of the kernel module. For example, a tool to
> build an initramfs uses this information to put the firmware files
> into the initramfs image.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
> rust/macros/lib.rs | 31 +++++++++++++++++++++++++++++++
> rust/macros/module.rs | 18 ++++++++++++++++--
> 2 files changed, 47 insertions(+), 2 deletions(-)
I left one comment below, otherwise LGTM:
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index f489f3157383..73fa768fce64 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -67,6 +67,36 @@
> /// }
> /// ```
> ///
> +/// ## Firmware
> +///
> +/// The following example shows how to declare a kernel module that needs
> +/// to load binary firmware files. You need to specify the file names of
> +/// the firmware in the `firmware` field. The information is embedded
> +/// in the `modinfo` section of the kernel module. For example, a tool to
> +/// build an initramfs uses this information to put the firmware files into
> +/// the initramfs image.
> +///
> +/// ```ignore
> +/// use kernel::prelude::*;
> +///
> +/// module!{
> +/// type: MyDeviceDriverModule,
> +/// name: "my_device_driver_module",
> +/// author: "Rust for Linux Contributors",
> +/// description: "My device driver requires firmware",
> +/// license: "GPL",
> +/// firmware: ["my_device_firmware1.bin", "my_device_firmware2.bin"],
> +/// }
> +///
> +/// struct MyDeviceDriverModule;
> +///
> +/// impl kernel::Module for MyDeviceDriverModule {
> +/// fn init() -> Result<Self> {
> +/// Ok(Self)
> +/// }
> +/// }
> +/// ```
> +///
> /// # Supported argument types
> /// - `type`: type which implements the [`Module`] trait (required).
> /// - `name`: byte array of the name of the kernel module (required).
> @@ -74,6 +104,7 @@
> /// - `description`: byte array of the description of the kernel module.
> /// - `license`: byte array of the license of the kernel module (required).
> /// - `alias`: byte array of alias name of the kernel module.
> +/// - `firmware`: array of the file names of firmware required by the kernel module.
There is a patch [1] that updates this list. It will have a v2, with
this list specifying the types that can be put there due to this [2]
comment.
We should have the same kind of item for the `firmware` field here. I
would suggest:
/// - `firmware`: array of ASCII string literals of the firmware files of the kernel module.
[1]: https://lore.kernel.org/rust-for-linux/20240419215015.157258-3-aswinunni01@gmail.com/
[2]: https://lore.kernel.org/rust-for-linux/CANiq72=g4f60n8x=-fCxqQ4QzD2eOrEb=vEXgoNidY-N4omN1A@mail.gmail.com/
--
Cheers,
Benno
> #[proc_macro]
> pub fn module(ts: TokenStream) -> TokenStream {
> module::module(ts)
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v1] rust: add 'firmware' field support to module! macro
2024-05-01 9:06 ` Benno Lossin
@ 2024-05-01 12:37 ` FUJITA Tomonori
0 siblings, 0 replies; 3+ messages in thread
From: FUJITA Tomonori @ 2024-05-01 12:37 UTC (permalink / raw
To: benno.lossin; +Cc: fujita.tomonori, rust-for-linux, miguel.ojeda.sandonis
Hi,
On Wed, 01 May 2024 09:06:18 +0000
Benno Lossin <benno.lossin@proton.me> wrote:
>> /// # Supported argument types
>> /// - `type`: type which implements the [`Module`] trait (required).
>> /// - `name`: byte array of the name of the kernel module (required).
>> @@ -74,6 +104,7 @@
>> /// - `description`: byte array of the description of the kernel module.
>> /// - `license`: byte array of the license of the kernel module (required).
>> /// - `alias`: byte array of alias name of the kernel module.
>> +/// - `firmware`: array of the file names of firmware required by the kernel module.
>
> There is a patch [1] that updates this list. It will have a v2, with
> this list specifying the types that can be put there due to this [2]
> comment.
>
> We should have the same kind of item for the `firmware` field here. I
> would suggest:
>
> /// - `firmware`: array of ASCII string literals of the firmware files of the kernel module.
>
> [1]: https://lore.kernel.org/rust-for-linux/20240419215015.157258-3-aswinunni01@gmail.com/
> [2]: https://lore.kernel.org/rust-for-linux/CANiq72=g4f60n8x=-fCxqQ4QzD2eOrEb=vEXgoNidY-N4omN1A@mail.gmail.com/
Thanks a lot!
I updated the comment as you suggested. I'll send v2 with your
Reviewed-by added shortly.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-05-01 12:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-26 9:56 [PATCH v1] rust: add 'firmware' field support to module! macro FUJITA Tomonori
2024-05-01 9:06 ` Benno Lossin
2024-05-01 12:37 ` FUJITA Tomonori
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.