Message-ID: <er7h34im2rk627usnvbre3clqvsx3uzev7kboy33pd7oac747c@nvtl7y2mmdde>
Date: Sun, 2 Nov 2025 10:56:32 +0100
From: Uwe Kleine-König <ukleinek@...nel.org>
To: Daniel Gomez <da.gomez@...nel.org>
Cc: Andreas Hindborg <a.hindborg@...nel.org>, 
	Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, 
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, Alice Ryhl <aliceryhl@...gle.com>, 
	Masahiro Yamada <masahiroy@...nel.org>, Nathan Chancellor <nathan@...nel.org>, 
	Luis Chamberlain <mcgrof@...nel.org>, Danilo Krummrich <dakr@...nel.org>, 
	Benno Lossin <lossin@...nel.org>, Nicolas Schier <nicolas.schier@...ux.dev>, 
	Michal Wilczynski <m.wilczynski@...sung.com>, Trevor Gross <tmgross@...ch.edu>, 
	Adam Bratschi-Kaye <ark.email@...il.com>, rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-kbuild@...r.kernel.org, Petr Pavlu <petr.pavlu@...e.com>, 
	Sami Tolvanen <samitolvanen@...gle.com>, Daniel Gomez <da.gomez@...sung.com>, 
	Simona Vetter <simona.vetter@...ll.ch>, Greg KH <gregkh@...uxfoundation.org>, 
	Fiona Behrens <me@...enk.dev>, Daniel Almeida <daniel.almeida@...labora.com>, 
	linux-modules@...r.kernel.org, Stephen Rothwell <sfr@...b.auug.org.au>, 
	linux-next@...r.kernel.org
Subject: Re: [PATCH v18 0/7] rust: extend `module!` macro with integer
 parameter support

Hello Daniel,

[Adding Stephen and linux-next to Cc]

On Sat, Nov 01, 2025 at 10:39:08PM +0100, Daniel Gomez wrote:
> On 24/09/2025 14.39, Andreas Hindborg wrote:
> > Extend the `module!` macro with support module parameters. Also add some
> > string to integer parsing functions.
> > 
> > Based on the original module parameter support by Miguel [1],
> > later extended and generalized by Adam for more types [2][3].
> > Originally tracked at [4].
> > 
> > Link: https://github.com/Rust-for-Linux/linux/pull/7 [1]
> > Link: https://github.com/Rust-for-Linux/linux/pull/82 [2]
> > Link: https://github.com/Rust-for-Linux/linux/pull/87 [3]
> > Link: https://github.com/Rust-for-Linux/linux/issues/11 [4]
> > Signed-off-by: Andreas Hindborg <a.hindborg@...nel.org>
> 
> I tested this series with rust_minimal module. They LGTM,
> 
> Tested-by: Daniel Gomez <da.gomez@...sung.com>
> 
> The patches did not apply cleanly to v6.18-rc3, at least not when using b4.
> However, when applying them to the base commit and then rebasing onto v6.18-rc3,
> I didn't see any conflicts.

I don't know how you use b4, but

	git checkout v6.18-rc3
	b4 am -3 49af6d76-bcb7-4343-8903-390040e2c49b@...nel.org
	git am -3 ./v18_20250924_a_hindborg_rust_extend_module_macro_with_integer_parameter_support.mbx

works fine on my end. Using `-3` should have the same effect as applying
the series on top of the original base and rebase it.

	git fetch https://git.kernel.org/pub/scm/linux/kernel/git/modules/linux.git rebase/20250924-module-params-v3-v18-0-bf512c35d910@...nel.org
	git range-diff FETCH_HEAD...HEAD

confirms that.
 
> I've created a temporary branch with this rebase here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/modules/linux.git/log/?h=rebase/20250924-module-params-v3-v18-0-bf512c35d910@kernel.org
> 
> Can you take a look when you can? I'll merge this shortly after checking with
> Uwe, as there are some minor conflicts with his tree.
> 
> + Uwe
> 
> These are the conflicts I see when merging the patch series from Michal [1]
> (Introduce import_ns support for Rust). I believe these are trivial things that
> we will get notified from linux-next merging. But let me know what you think as
> you have requested in that thread.
> 
> [1] Link: https://lore.kernel.org/all/20251028-pwm_fixes-v1-0-25a532d31998@samsung.com/

Yeah, I expect that Stephen will highlight the conflicts, but I prefer
to not be surprised by that and consider linux-next more a fallback
security net that I don't want to use. I like it to be the other way
round and tell Stephen about conflicts to expect :-)

> ...
> Applying: rust: macros: Add support for 'imports_ns' to module!
> Patch failed at 0008 rust: macros: Add support for 'imports_ns' to module!
> error: patch failed: rust/macros/module.rs:98
> error: rust/macros/module.rs: patch does not apply
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> hint: When you have resolved this problem, run "git am --continue".
> hint: If you prefer to skip this patch, run "git am --skip" instead.
> hint: To restore the original branch and stop patching, run "git am --abort".
> hint: Disable this message with "git config set advice.mergeConflict false"
> 
> git am --show-current-patch=diff

That command shows the patch to apply, but not the conflict, let alone
your resolution.

> ---
>  rust/macros/module.rs | 8 ++++++++
>  1 file changed, 8 insertions(+)
> ---
>  rust/macros/module.rs | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/rust/macros/module.rs b/rust/macros/module.rs
> index 5ee54a00c0b65699596e660b2d4d60e64be2a50c..408cd115487514c8be79724d901c676435696376 100644
> --- a/rust/macros/module.rs
> +++ b/rust/macros/module.rs
> @@ -98,6 +98,7 @@ struct ModuleInfo {
>      description: Option<String>,
>      alias: Option<Vec<String>>,
>      firmware: Option<Vec<String>>,
> +    imports_ns: Option<Vec<String>>,
>  }

So here the addition of `params` is missing.

> [...]

When I merge your branch mentioned above with my pwm/for-next and
resolve the merge conflicts, the resolution looks as follows. The only
non-trivial thing is that

	if let Some(imports) = info.imports_ns {

now needs a & for `info`.

Best regards
Uwe

diff --cc rust/macros/module.rs
index d62e9c1e2a89,408cd1154875..000000000000
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@@ -205,50 -98,7 +205,51 @@@ struct ModuleInfo 
      description: Option<String>,
      alias: Option<Vec<String>>,
      firmware: Option<Vec<String>>,
+     imports_ns: Option<Vec<String>>,
 +    params: Option<Vec<Parameter>>,
 +}
 +
 +#[derive(Debug)]
 +struct Parameter {
 +    name: String,
 +    ptype: String,
 +    default: String,
 +    description: String,
 +}
 +
 +fn expect_params(it: &mut token_stream::IntoIter) -> Vec<Parameter> {
 +    let params = expect_group(it);
 +    assert_eq!(params.delimiter(), Delimiter::Brace);
 +    let mut it = params.stream().into_iter();
 +    let mut parsed = Vec::new();
 +
 +    loop {
 +        let param_name = match it.next() {
 +            Some(TokenTree::Ident(ident)) => ident.to_string(),
 +            Some(_) => panic!("Expected Ident or end"),
 +            None => break,
 +        };
 +
 +        assert_eq!(expect_punct(&mut it), ':');
 +        let param_type = expect_ident(&mut it);
 +        let group = expect_group(&mut it);
 +        assert_eq!(group.delimiter(), Delimiter::Brace);
 +        assert_eq!(expect_punct(&mut it), ',');
 +
 +        let mut param_it = group.stream().into_iter();
 +        let param_default = expect_param_default(&mut param_it);
 +        let param_description = expect_string_field(&mut param_it, "description");
 +        expect_end(&mut param_it);
 +
 +        parsed.push(Parameter {
 +            name: param_name,
 +            ptype: param_type,
 +            default: param_default,
 +            description: param_description,
 +        })
 +    }
 +
 +    parsed
  }
  
  impl ModuleInfo {
@@@ -263,7 -113,7 +264,8 @@@
              "license",
              "alias",
              "firmware",
+             "imports_ns",
 +            "params",
          ];
          const REQUIRED_KEYS: &[&str] = &["type", "name", "license"];
          let mut seen_keys = Vec::new();
@@@ -289,7 -139,7 +291,8 @@@
                  "license" => info.license = expect_string_ascii(it),
                  "alias" => info.alias = Some(expect_string_array(it)),
                  "firmware" => info.firmware = Some(expect_string_array(it)),
+                 "imports_ns" => info.imports_ns = Some(expect_string_array(it)),
 +                "params" => info.params = Some(expect_params(it)),
                  _ => panic!("Unknown key \"{key}\". Valid keys are: {EXPECTED_KEYS:?}."),
              }
  
@@@ -329,25 -179,30 +332,30 @@@ pub(crate) fn module(ts: TokenStream) -
      // Rust does not allow hyphens in identifiers, use underscore instead.
      let ident = info.name.replace('-', "_");
      let mut modinfo = ModInfoBuilder::new(ident.as_ref());
 -    if let Some(authors) = info.authors {
 +    if let Some(authors) = &info.authors {
          for author in authors {
 -            modinfo.emit("author", &author);
 +            modinfo.emit("author", author);
          }
      }
 -    if let Some(description) = info.description {
 -        modinfo.emit("description", &description);
 +    if let Some(description) = &info.description {
 +        modinfo.emit("description", description);
      }
      modinfo.emit("license", &info.license);
 -    if let Some(aliases) = info.alias {
 +    if let Some(aliases) = &info.alias {
          for alias in aliases {
 -            modinfo.emit("alias", &alias);
 +            modinfo.emit("alias", alias);
          }
      }
 -    if let Some(firmware) = info.firmware {
 +    if let Some(firmware) = &info.firmware {
          for fw in firmware {
 -            modinfo.emit("firmware", &fw);
 +            modinfo.emit("firmware", fw);
          }
      }
 -    if let Some(imports) = info.imports_ns {
++    if let Some(imports) = &info.imports_ns {
+         for ns in imports {
+             modinfo.emit("import_ns", &ns);
+         }
+     }
  
      // Built-in modules also export the `file` modinfo string.
      let file =

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

Powered by blists - more mailing lists