rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] rust: fix building firmware abstraction on 32bit arm
@ 2025-04-11  7:14 Christian Schrefl
  2025-04-11  8:37 ` Benno Lossin
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Christian Schrefl @ 2025-04-11  7:14 UTC (permalink / raw)
  To: Luis Chamberlain, Russ Weight, Danilo Krummrich, Miguel Ojeda,
	Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross
  Cc: linux-kernel, rust-for-linux, stable, Christian Schrefl

When trying to build the rust firmware abstractions on 32 bit arm the
following build error occures:

```
error[E0308]: mismatched types
  --> rust/kernel/firmware.rs:20:14
   |
20 |         Self(bindings::request_firmware)
   |         ---- ^^^^^^^^^^^^^^^^^^^^^^^^^^ expected fn pointer, found fn item
   |         |
   |         arguments to this function are incorrect
   |
   = note: expected fn pointer `unsafe extern "C" fn(_, *const i8, _) -> _`
                 found fn item `unsafe extern "C" fn(_, *const u8, _) -> _ {request_firmware}`
note: tuple struct defined here
  --> rust/kernel/firmware.rs:14:8
   |
14 | struct FwFunc(
   |        ^^^^^^

error[E0308]: mismatched types
  --> rust/kernel/firmware.rs:24:14
   |
24 |         Self(bindings::firmware_request_nowarn)
   |         ---- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected fn pointer, found fn item
   |         |
   |         arguments to this function are incorrect
   |
   = note: expected fn pointer `unsafe extern "C" fn(_, *const i8, _) -> _`
                 found fn item `unsafe extern "C" fn(_, *const u8, _) -> _ {firmware_request_nowarn}`
note: tuple struct defined here
  --> rust/kernel/firmware.rs:14:8
   |
14 | struct FwFunc(
   |        ^^^^^^

error[E0308]: mismatched types
  --> rust/kernel/firmware.rs:64:45
   |
64 |         let ret = unsafe { func.0(pfw as _, name.as_char_ptr(), dev.as_raw()) };
   |                            ------           ^^^^^^^^^^^^^^^^^^ expected `*const i8`, found `*const u8`
   |                            |
   |                            arguments to this function are incorrect
   |
   = note: expected raw pointer `*const i8`
              found raw pointer `*const u8`

error: aborting due to 3 previous errors
```

To fix this error the char pointer type in `FwFunc` is converted to
`ffi::c_char`.

Fixes: de6582833db0 ("rust: add firmware abstractions")
Cc: stable@vger.kernel.org # Backport only to 6.15 needed

Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
---
 rust/kernel/firmware.rs | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
index f04b058b09b2d2397e26344d0e055b3aa5061432..1d6284316f2a4652ef3f76272670e5e29b0ff924 100644
--- a/rust/kernel/firmware.rs
+++ b/rust/kernel/firmware.rs
@@ -5,14 +5,18 @@
 //! C header: [`include/linux/firmware.h`](srctree/include/linux/firmware.h)
 
 use crate::{bindings, device::Device, error::Error, error::Result, str::CStr};
-use core::ptr::NonNull;
+use core::{ffi, ptr::NonNull};
 
 /// # Invariants
 ///
 /// One of the following: `bindings::request_firmware`, `bindings::firmware_request_nowarn`,
 /// `bindings::firmware_request_platform`, `bindings::request_firmware_direct`.
 struct FwFunc(
-    unsafe extern "C" fn(*mut *const bindings::firmware, *const u8, *mut bindings::device) -> i32,
+    unsafe extern "C" fn(
+        *mut *const bindings::firmware,
+        *const ffi::c_char,
+        *mut bindings::device,
+    ) -> i32,
 );
 
 impl FwFunc {

---
base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
change-id: 20250408-rust_arm_fix_fw_abstaction-4c3a89d75e29

Best regards,
-- 
Christian Schrefl <chrisi.schrefl@gmail.com>


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] rust: fix building firmware abstraction on 32bit arm
  2025-04-11  7:14 [PATCH] rust: fix building firmware abstraction on 32bit arm Christian Schrefl
@ 2025-04-11  8:37 ` Benno Lossin
  2025-04-11 10:35 ` Danilo Krummrich
  2025-04-11 12:45 ` Benno Lossin
  2 siblings, 0 replies; 11+ messages in thread
From: Benno Lossin @ 2025-04-11  8:37 UTC (permalink / raw)
  To: Christian Schrefl, Luis Chamberlain, Russ Weight,
	Danilo Krummrich, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross
  Cc: linux-kernel, rust-for-linux, stable

On Fri Apr 11, 2025 at 9:14 AM CEST, Christian Schrefl wrote:
> When trying to build the rust firmware abstractions on 32 bit arm the
> following build error occures:
>
> ```

[...]

> ```
>
> To fix this error the char pointer type in `FwFunc` is converted to
> `ffi::c_char`.
>
> Fixes: de6582833db0 ("rust: add firmware abstractions")
> Cc: stable@vger.kernel.org # Backport only to 6.15 needed
>
> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>

Reviewed-by: Benno Lossin <benno.lossin@proton.me>

---
Cheers,
Benno

> ---
>  rust/kernel/firmware.rs | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] rust: fix building firmware abstraction on 32bit arm
  2025-04-11  7:14 [PATCH] rust: fix building firmware abstraction on 32bit arm Christian Schrefl
  2025-04-11  8:37 ` Benno Lossin
@ 2025-04-11 10:35 ` Danilo Krummrich
  2025-04-11 13:47   ` Christian Schrefl
  2025-04-11 12:45 ` Benno Lossin
  2 siblings, 1 reply; 11+ messages in thread
From: Danilo Krummrich @ 2025-04-11 10:35 UTC (permalink / raw)
  To: Christian Schrefl
  Cc: Luis Chamberlain, Russ Weight, Miguel Ojeda, Alex Gaynor,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, linux-kernel,
	rust-for-linux, stable

On Fri, Apr 11, 2025 at 09:14:48AM +0200, Christian Schrefl wrote:
> When trying to build the rust firmware abstractions on 32 bit arm the
> following build error occures:
> 
> ```
> error[E0308]: mismatched types
>   --> rust/kernel/firmware.rs:20:14
>    |
> 20 |         Self(bindings::request_firmware)
>    |         ---- ^^^^^^^^^^^^^^^^^^^^^^^^^^ expected fn pointer, found fn item
>    |         |
>    |         arguments to this function are incorrect
>    |
>    = note: expected fn pointer `unsafe extern "C" fn(_, *const i8, _) -> _`
>                  found fn item `unsafe extern "C" fn(_, *const u8, _) -> _ {request_firmware}`

This looks like you have local changes in your tree, running in this error. I
get the exact same errors when I apply the following diff:

diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
index f04b058b09b2..a67047e3aa6b 100644
--- a/rust/kernel/firmware.rs
+++ b/rust/kernel/firmware.rs
@@ -12,7 +12,7 @@
 /// One of the following: `bindings::request_firmware`, `bindings::firmware_request_nowarn`,
 /// `bindings::firmware_request_platform`, `bindings::request_firmware_direct`.
 struct FwFunc(
-    unsafe extern "C" fn(*mut *const bindings::firmware, *const u8, *mut bindings::device) -> i32,
+    unsafe extern "C" fn(*mut *const bindings::firmware, *const i8, *mut bindings::device) -> i32,
 );

> note: tuple struct defined here
>   --> rust/kernel/firmware.rs:14:8
>    |
> 14 | struct FwFunc(
>    |        ^^^^^^
> 
> error[E0308]: mismatched types
>   --> rust/kernel/firmware.rs:24:14
>    |
> 24 |         Self(bindings::firmware_request_nowarn)
>    |         ---- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected fn pointer, found fn item
>    |         |
>    |         arguments to this function are incorrect
>    |
>    = note: expected fn pointer `unsafe extern "C" fn(_, *const i8, _) -> _`
>                  found fn item `unsafe extern "C" fn(_, *const u8, _) -> _ {firmware_request_nowarn}`
> note: tuple struct defined here
>   --> rust/kernel/firmware.rs:14:8
>    |
> 14 | struct FwFunc(
>    |        ^^^^^^
> 
> error[E0308]: mismatched types
>   --> rust/kernel/firmware.rs:64:45
>    |
> 64 |         let ret = unsafe { func.0(pfw as _, name.as_char_ptr(), dev.as_raw()) };
>    |                            ------           ^^^^^^^^^^^^^^^^^^ expected `*const i8`, found `*const u8`
>    |                            |
>    |                            arguments to this function are incorrect
>    |
>    = note: expected raw pointer `*const i8`
>               found raw pointer `*const u8`
> 
> error: aborting due to 3 previous errors
> ```

I did a test build with multi_v7_defconfig and I can't reproduce this issue.

I think the kernel does always use -funsigned-char, as also documented in commit
1bae8729e50a ("rust: map `long` to `isize` and `char` to `u8`")?

> 
> To fix this error the char pointer type in `FwFunc` is converted to
> `ffi::c_char`.
> 
> Fixes: de6582833db0 ("rust: add firmware abstractions")
> Cc: stable@vger.kernel.org # Backport only to 6.15 needed
> 
> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
> ---
>  rust/kernel/firmware.rs | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
> index f04b058b09b2d2397e26344d0e055b3aa5061432..1d6284316f2a4652ef3f76272670e5e29b0ff924 100644
> --- a/rust/kernel/firmware.rs
> +++ b/rust/kernel/firmware.rs
> @@ -5,14 +5,18 @@
>  //! C header: [`include/linux/firmware.h`](srctree/include/linux/firmware.h)
>  
>  use crate::{bindings, device::Device, error::Error, error::Result, str::CStr};
> -use core::ptr::NonNull;
> +use core::{ffi, ptr::NonNull};

The change itself seems to be fine anyways, but I think we should use crate::ffi
instead.

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] rust: fix building firmware abstraction on 32bit arm
  2025-04-11  7:14 [PATCH] rust: fix building firmware abstraction on 32bit arm Christian Schrefl
  2025-04-11  8:37 ` Benno Lossin
  2025-04-11 10:35 ` Danilo Krummrich
@ 2025-04-11 12:45 ` Benno Lossin
  2025-04-11 14:15   ` Miguel Ojeda
  2 siblings, 1 reply; 11+ messages in thread
From: Benno Lossin @ 2025-04-11 12:45 UTC (permalink / raw)
  To: Christian Schrefl, Luis Chamberlain, Russ Weight,
	Danilo Krummrich, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross
  Cc: linux-kernel, rust-for-linux, stable

On Fri Apr 11, 2025 at 9:14 AM CEST, Christian Schrefl wrote:
> diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
> index f04b058b09b2d2397e26344d0e055b3aa5061432..1d6284316f2a4652ef3f76272670e5e29b0ff924 100644
> --- a/rust/kernel/firmware.rs
> +++ b/rust/kernel/firmware.rs
> @@ -5,14 +5,18 @@
>  //! C header: [`include/linux/firmware.h`](srctree/include/linux/firmware.h)
>  
>  use crate::{bindings, device::Device, error::Error, error::Result, str::CStr};
> -use core::ptr::NonNull;
> +use core::{ffi, ptr::NonNull};

Ah I overlooked this, you should be using `kernel::ffi` (or
`crate::ffi`) instead of `core`. (for `c_char` it doesn't matter, but we
shouldn't be using `core::ffi`, since we have our own mappings).

---
Cheers,
Benno


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] rust: fix building firmware abstraction on 32bit arm
  2025-04-11 10:35 ` Danilo Krummrich
@ 2025-04-11 13:47   ` Christian Schrefl
  2025-04-11 14:17     ` Danilo Krummrich
  0 siblings, 1 reply; 11+ messages in thread
From: Christian Schrefl @ 2025-04-11 13:47 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: Luis Chamberlain, Russ Weight, Miguel Ojeda, Alex Gaynor,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, linux-kernel,
	rust-for-linux, stable

On 11.04.25 12:35 PM, Danilo Krummrich wrote:
> On Fri, Apr 11, 2025 at 09:14:48AM +0200, Christian Schrefl wrote:
>> When trying to build the rust firmware abstractions on 32 bit arm the
>> following build error occures:
>>
>> ```
>> error[E0308]: mismatched types
>>   --> rust/kernel/firmware.rs:20:14
>>    |
>> 20 |         Self(bindings::request_firmware)
>>    |         ---- ^^^^^^^^^^^^^^^^^^^^^^^^^^ expected fn pointer, found fn item
>>    |         |
>>    |         arguments to this function are incorrect
>>    |
>>    = note: expected fn pointer `unsafe extern "C" fn(_, *const i8, _) -> _`
>>                  found fn item `unsafe extern "C" fn(_, *const u8, _) -> _ {request_firmware}`
> 
> This looks like you have local changes in your tree, running in this error. I
> get the exact same errors when I apply the following diff:
> 
> diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
> index f04b058b09b2..a67047e3aa6b 100644
> --- a/rust/kernel/firmware.rs
> +++ b/rust/kernel/firmware.rs
> @@ -12,7 +12,7 @@
>  /// One of the following: `bindings::request_firmware`, `bindings::firmware_request_nowarn`,
>  /// `bindings::firmware_request_platform`, `bindings::request_firmware_direct`.
>  struct FwFunc(
> -    unsafe extern "C" fn(*mut *const bindings::firmware, *const u8, *mut bindings::device) -> i32,
> +    unsafe extern "C" fn(*mut *const bindings::firmware, *const i8, *mut bindings::device) -> i32,
>  );
> 
>> note: tuple struct defined here
>>   --> rust/kernel/firmware.rs:14:8
>>    |
>> 14 | struct FwFunc(
>>    |        ^^^^^^
>>
>> error[E0308]: mismatched types
>>   --> rust/kernel/firmware.rs:24:14
>>    |
>> 24 |         Self(bindings::firmware_request_nowarn)
>>    |         ---- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected fn pointer, found fn item
>>    |         |
>>    |         arguments to this function are incorrect
>>    |
>>    = note: expected fn pointer `unsafe extern "C" fn(_, *const i8, _) -> _`
>>                  found fn item `unsafe extern "C" fn(_, *const u8, _) -> _ {firmware_request_nowarn}`
>> note: tuple struct defined here
>>   --> rust/kernel/firmware.rs:14:8
>>    |
>> 14 | struct FwFunc(
>>    |        ^^^^^^
>>
>> error[E0308]: mismatched types
>>   --> rust/kernel/firmware.rs:64:45
>>    |
>> 64 |         let ret = unsafe { func.0(pfw as _, name.as_char_ptr(), dev.as_raw()) };
>>    |                            ------           ^^^^^^^^^^^^^^^^^^ expected `*const i8`, found `*const u8`
>>    |                            |
>>    |                            arguments to this function are incorrect
>>    |
>>    = note: expected raw pointer `*const i8`
>>               found raw pointer `*const u8`
>>
>> error: aborting due to 3 previous errors
>> ```
> 
> I did a test build with multi_v7_defconfig and I can't reproduce this issue.
> 
Interesting, I've it seems this is only an issue on 6.13 with my arm patches applied.

It seems that it works on v6.14 and v6.15-rc1 but the error occurs on ffd294d346d1 (tag: v6.13)
with my 32-bit arm patches applied.

> I think the kernel does always use -funsigned-char, as also documented in commit
> 1bae8729e50a ("rust: map `long` to `isize` and `char` to `u8`")?
> 
>>
>> To fix this error the char pointer type in `FwFunc` is converted to
>> `ffi::c_char`.
>>
>> Fixes: de6582833db0 ("rust: add firmware abstractions")
>> Cc: stable@vger.kernel.org # Backport only to 6.15 needed
>>
>> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
>> ---
>>  rust/kernel/firmware.rs | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
>> index f04b058b09b2d2397e26344d0e055b3aa5061432..1d6284316f2a4652ef3f76272670e5e29b0ff924 100644
>> --- a/rust/kernel/firmware.rs
>> +++ b/rust/kernel/firmware.rs
>> @@ -5,14 +5,18 @@
>>  //! C header: [`include/linux/firmware.h`](srctree/include/linux/firmware.h)
>>  
>>  use crate::{bindings, device::Device, error::Error, error::Result, str::CStr};
>> -use core::ptr::NonNull;
>> +use core::{ffi, ptr::NonNull};
> 
> The change itself seems to be fine anyways, but I think we should use crate::ffi
> instead.
Right, I just did what RA recommended without thinking about it much.

I guess this patch isn't really needed. Should I still send a V2 using `crate::ffi`?

Cheers,
Christian


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] rust: fix building firmware abstraction on 32bit arm
  2025-04-11 12:45 ` Benno Lossin
@ 2025-04-11 14:15   ` Miguel Ojeda
  2025-04-11 14:18     ` Miguel Ojeda
  2025-04-12 10:01     ` Benno Lossin
  0 siblings, 2 replies; 11+ messages in thread
From: Miguel Ojeda @ 2025-04-11 14:15 UTC (permalink / raw)
  To: Benno Lossin
  Cc: Christian Schrefl, Luis Chamberlain, Russ Weight,
	Danilo Krummrich, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	linux-kernel, rust-for-linux, stable

[-- Attachment #1: Type: text/plain, Size: 2112 bytes --]

On Fri, Apr 11, 2025 at 2:46 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> Ah I overlooked this, you should be using `kernel::ffi` (or
> `crate::ffi`) instead of `core`. (for `c_char` it doesn't matter, but we
> shouldn't be using `core::ffi`, since we have our own mappings).

In 6.6, C `char` changed to unsigned, but `core::ffi::c_char` is
signed (in x86_64 at least).

We should just never use `core::ffi` (except in `rust/ffi.rs`, of
course) -- I think we should just add the C types to the prelude
(which we discussed in the past) so that it is easy to avoid the
mistake (something like the patch attached as the end result, but
tested and across a kernel cycle or two) and mention it in the Coding
Guidelines. Thoughts?

I tried to use Clippy's `disallowed-types` too:

    disallowed-types = [
        { path = "core::ffi::c_void", reason = "the `kernel::ffi`
types should be used instead" },
        { path = "core::ffi::c_char", reason = "the `kernel::ffi`
types should be used instead" },
        { path = "core::ffi::c_schar", reason = "the `kernel::ffi`
types should be used instead" },
        { path = "core::ffi::c_uchar", reason = "the `kernel::ffi`
types should be used instead" },
        { path = "core::ffi::c_short", reason = "the `kernel::ffi`
types should be used instead" },
        { path = "core::ffi::c_ushort", reason = "the `kernel::ffi`
types should be used instead" },
        { path = "core::ffi::c_int", reason = "the `kernel::ffi` types
should be used instead" },
        { path = "core::ffi::c_uint", reason = "the `kernel::ffi`
types should be used instead" },
        { path = "core::ffi::c_long", reason = "the `kernel::ffi`
types should be used instead" },
        { path = "core::ffi::c_ulong", reason = "the `kernel::ffi`
types should be used instead" },
        { path = "core::ffi::c_longlong", reason = "the `kernel::ffi`
types should be used instead" },
        { path = "core::ffi::c_ulonglong", reason = "the `kernel::ffi`
types should be used instead" },
    ]

But it goes across aliases.

Cheers,
Miguel

[-- Attachment #2: 0001-rust-add-C-types-to-the-prelude.patch --]
[-- Type: text/x-patch, Size: 37794 bytes --]

From 4083b10ff44f95fb595757f2dfeaffe08f50de9b Mon Sep 17 00:00:00 2001
From: Miguel Ojeda <ojeda@kernel.org>
Date: Fri, 11 Apr 2025 15:48:02 +0200
Subject: [RFC PATCH] rust: add C types to the prelude
---
 drivers/gpu/drm/drm_panic_qr.rs     |  2 +-
 rust/kernel/alloc/allocator.rs      |  5 ++---
 rust/kernel/alloc/allocator_test.rs |  7 ++++---
 rust/kernel/alloc/kbox.rs           | 17 +++++++++--------
 rust/kernel/block/mq/operations.rs  | 18 +++++++++---------
 rust/kernel/block/mq/raw_writer.rs  |  6 ++----
 rust/kernel/block/mq/tag_set.rs     |  4 ++--
 rust/kernel/device.rs               |  5 +++--
 rust/kernel/devres.rs               |  3 +--
 rust/kernel/dma.rs                  |  3 ++-
 rust/kernel/error.rs                | 21 +++++++++++----------
 rust/kernel/init.rs                 |  2 +-
 rust/kernel/kunit.rs                |  3 ++-
 rust/kernel/miscdevice.rs           |  1 -
 rust/kernel/net/phy.rs              | 26 ++++++++------------------
 rust/kernel/pci.rs                  |  2 +-
 rust/kernel/platform.rs             |  2 +-
 rust/kernel/prelude.rs              |  5 +++++
 rust/kernel/print.rs                |  6 +-----
 rust/kernel/seq_file.rs             |  4 ++--
 rust/kernel/str.rs                  |  6 +++---
 rust/kernel/sync/arc.rs             |  9 +++++----
 rust/kernel/sync/condvar.rs         |  2 +-
 rust/kernel/sync/lock.rs            |  7 ++-----
 rust/kernel/sync/lock/mutex.rs      |  8 +++-----
 rust/kernel/sync/lock/spinlock.rs   |  8 +++-----
 rust/kernel/task.rs                 |  2 +-
 rust/kernel/time.rs                 |  6 ++++--
 rust/kernel/types.rs                | 19 ++++++++++---------
 rust/kernel/uaccess.rs              |  3 ---
 samples/rust/rust_print_main.rs     |  2 +-
 31 files changed, 100 insertions(+), 114 deletions(-)

diff --git a/drivers/gpu/drm/drm_panic_qr.rs b/drivers/gpu/drm/drm_panic_qr.rs
index f2a99681b998..ddbec0669a1c 100644
--- a/drivers/gpu/drm/drm_panic_qr.rs
+++ b/drivers/gpu/drm/drm_panic_qr.rs
@@ -893,7 +893,7 @@ fn draw_all(&mut self, data: impl Iterator<Item = u8>) {
 /// They must remain valid for the duration of the function call.
 #[export]
 pub unsafe extern "C" fn drm_panic_qr_generate(
-    url: *const kernel::ffi::c_char,
+    url: *const c_char,
     data: *mut u8,
     data_len: usize,
     data_size: usize,
diff --git a/rust/kernel/alloc/allocator.rs b/rust/kernel/alloc/allocator.rs
index aa2dfa9dca4c..0310fd3e1b8d 100644
--- a/rust/kernel/alloc/allocator.rs
+++ b/rust/kernel/alloc/allocator.rs
@@ -16,6 +16,7 @@
 use crate::alloc::{AllocError, Allocator};
 use crate::bindings;
 use crate::pr_warn;
+use crate::prelude::*;

 /// The contiguous kernel allocator.
 ///
@@ -57,9 +58,7 @@ fn aligned_size(new_layout: Layout) -> usize {
 /// # Invariants
 ///
 /// One of the following: `krealloc`, `vrealloc`, `kvrealloc`.
-struct ReallocFunc(
-    unsafe extern "C" fn(*const crate::ffi::c_void, usize, u32) -> *mut crate::ffi::c_void,
-);
+struct ReallocFunc(unsafe extern "C" fn(*const c_void, usize, u32) -> *mut c_void);

 impl ReallocFunc {
     // INVARIANT: `krealloc` satisfies the type invariants.
diff --git a/rust/kernel/alloc/allocator_test.rs b/rust/kernel/alloc/allocator_test.rs
index c37d4c0c64e9..7057061a2a18 100644
--- a/rust/kernel/alloc/allocator_test.rs
+++ b/rust/kernel/alloc/allocator_test.rs
@@ -12,6 +12,7 @@
 use super::{flags::*, AllocError, Allocator, Flags};
 use core::alloc::Layout;
 use core::cmp;
+use core::ffi::c_void;
 use core::ptr;
 use core::ptr::NonNull;

@@ -24,10 +25,10 @@

 extern "C" {
     #[link_name = "aligned_alloc"]
-    fn libc_aligned_alloc(align: usize, size: usize) -> *mut crate::ffi::c_void;
+    fn libc_aligned_alloc(align: usize, size: usize) -> *mut c_void;

     #[link_name = "free"]
-    fn libc_free(ptr: *mut crate::ffi::c_void);
+    fn libc_free(ptr: *mut c_void);
 }

 // SAFETY:
@@ -76,7 +77,7 @@ unsafe fn realloc(
         // of writing, this is known to be the case on macOS (but not in glibc).
         //
         // Satisfy the stricter requirement to avoid spurious test failures on some platforms.
-        let min_align = core::mem::size_of::<*const crate::ffi::c_void>();
+        let min_align = core::mem::size_of::<*const c_void>();
         let layout = layout.align_to(min_align).map_err(|_| AllocError)?;
         let layout = layout.pad_to_align();

diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
index b77d32f3a58b..6e2b5a154d89 100644
--- a/rust/kernel/alloc/kbox.rs
+++ b/rust/kernel/alloc/kbox.rs
@@ -16,6 +16,7 @@
 use core::result::Result;

 use crate::init::InPlaceInit;
+use crate::prelude::*;
 use crate::types::ForeignOwnable;
 use pin_init::{InPlaceWrite, Init, PinInit, ZeroableOption};

@@ -367,23 +368,23 @@ impl<T: 'static, A> ForeignOwnable for Box<T, A>
     type Borrowed<'a> = &'a T;
     type BorrowedMut<'a> = &'a mut T;

-    fn into_foreign(self) -> *mut crate::ffi::c_void {
+    fn into_foreign(self) -> *mut c_void {
         Box::into_raw(self).cast()
     }

-    unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self {
+    unsafe fn from_foreign(ptr: *mut c_void) -> Self {
         // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous
         // call to `Self::into_foreign`.
         unsafe { Box::from_raw(ptr.cast()) }
     }

-    unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> &'a T {
+    unsafe fn borrow<'a>(ptr: *mut c_void) -> &'a T {
         // SAFETY: The safety requirements of this method ensure that the object remains alive and
         // immutable for the duration of 'a.
         unsafe { &*ptr.cast() }
     }

-    unsafe fn borrow_mut<'a>(ptr: *mut crate::ffi::c_void) -> &'a mut T {
+    unsafe fn borrow_mut<'a>(ptr: *mut c_void) -> &'a mut T {
         let ptr = ptr.cast();
         // SAFETY: The safety requirements of this method ensure that the pointer is valid and that
         // nothing else will access the value for the duration of 'a.
@@ -398,18 +399,18 @@ impl<T: 'static, A> ForeignOwnable for Pin<Box<T, A>>
     type Borrowed<'a> = Pin<&'a T>;
     type BorrowedMut<'a> = Pin<&'a mut T>;

-    fn into_foreign(self) -> *mut crate::ffi::c_void {
+    fn into_foreign(self) -> *mut c_void {
         // SAFETY: We are still treating the box as pinned.
         Box::into_raw(unsafe { Pin::into_inner_unchecked(self) }).cast()
     }

-    unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self {
+    unsafe fn from_foreign(ptr: *mut c_void) -> Self {
         // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous
         // call to `Self::into_foreign`.
         unsafe { Pin::new_unchecked(Box::from_raw(ptr.cast())) }
     }

-    unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> Pin<&'a T> {
+    unsafe fn borrow<'a>(ptr: *mut c_void) -> Pin<&'a T> {
         // SAFETY: The safety requirements for this function ensure that the object is still alive,
         // so it is safe to dereference the raw pointer.
         // The safety requirements of `from_foreign` also ensure that the object remains alive for
@@ -420,7 +421,7 @@ unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> Pin<&'a T> {
         unsafe { Pin::new_unchecked(r) }
     }

-    unsafe fn borrow_mut<'a>(ptr: *mut crate::ffi::c_void) -> Pin<&'a mut T> {
+    unsafe fn borrow_mut<'a>(ptr: *mut c_void) -> Pin<&'a mut T> {
         let ptr = ptr.cast();
         // SAFETY: The safety requirements for this function ensure that the object is still alive,
         // so it is safe to dereference the raw pointer.
diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
index 864ff379dc91..ff58707167e6 100644
--- a/rust/kernel/block/mq/operations.rs
+++ b/rust/kernel/block/mq/operations.rs
@@ -132,7 +132,7 @@ impl<T: Operations> OperationsVTable<T> {
     unsafe extern "C" fn poll_callback(
         _hctx: *mut bindings::blk_mq_hw_ctx,
         _iob: *mut bindings::io_comp_batch,
-    ) -> crate::ffi::c_int {
+    ) -> c_int {
         T::poll().into()
     }

@@ -146,9 +146,9 @@ impl<T: Operations> OperationsVTable<T> {
     /// for the same context.
     unsafe extern "C" fn init_hctx_callback(
         _hctx: *mut bindings::blk_mq_hw_ctx,
-        _tagset_data: *mut crate::ffi::c_void,
-        _hctx_idx: crate::ffi::c_uint,
-    ) -> crate::ffi::c_int {
+        _tagset_data: *mut c_void,
+        _hctx_idx: c_uint,
+    ) -> c_int {
         from_result(|| Ok(0))
     }

@@ -160,7 +160,7 @@ impl<T: Operations> OperationsVTable<T> {
     /// This function may only be called by blk-mq C infrastructure.
     unsafe extern "C" fn exit_hctx_callback(
         _hctx: *mut bindings::blk_mq_hw_ctx,
-        _hctx_idx: crate::ffi::c_uint,
+        _hctx_idx: c_uint,
     ) {
     }

@@ -177,9 +177,9 @@ impl<T: Operations> OperationsVTable<T> {
     unsafe extern "C" fn init_request_callback(
         _set: *mut bindings::blk_mq_tag_set,
         rq: *mut bindings::request,
-        _hctx_idx: crate::ffi::c_uint,
-        _numa_node: crate::ffi::c_uint,
-    ) -> crate::ffi::c_int {
+        _hctx_idx: c_uint,
+        _numa_node: c_uint,
+    ) -> c_int {
         from_result(|| {
             // SAFETY: By the safety requirements of this function, `rq` points
             // to a valid allocation.
@@ -204,7 +204,7 @@ impl<T: Operations> OperationsVTable<T> {
     unsafe extern "C" fn exit_request_callback(
         _set: *mut bindings::blk_mq_tag_set,
         rq: *mut bindings::request,
-        _hctx_idx: crate::ffi::c_uint,
+        _hctx_idx: c_uint,
     ) {
         // SAFETY: The tagset invariants guarantee that all requests are allocated with extra memory
         // for the request data.
diff --git a/rust/kernel/block/mq/raw_writer.rs b/rust/kernel/block/mq/raw_writer.rs
index 7e2159e4f6a6..439ed7c11296 100644
--- a/rust/kernel/block/mq/raw_writer.rs
+++ b/rust/kernel/block/mq/raw_writer.rs
@@ -3,7 +3,7 @@
 use core::fmt::{self, Write};

 use crate::error::Result;
-use crate::prelude::EINVAL;
+use crate::prelude::*;

 /// A mutable reference to a byte buffer where a string can be written into.
 ///
@@ -24,9 +24,7 @@ fn new(buffer: &'a mut [u8]) -> Result<RawWriter<'a>> {
         Ok(Self { buffer, pos: 0 })
     }

-    pub(crate) fn from_array<const N: usize>(
-        a: &'a mut [crate::ffi::c_char; N],
-    ) -> Result<RawWriter<'a>> {
+    pub(crate) fn from_array<const N: usize>(a: &'a mut [c_char; N]) -> Result<RawWriter<'a>> {
         Self::new(
             // SAFETY: the buffer of `a` is valid for read and write as `u8` for
             // at least `N` bytes.
diff --git a/rust/kernel/block/mq/tag_set.rs b/rust/kernel/block/mq/tag_set.rs
index bcf4214ad149..58b030621bf7 100644
--- a/rust/kernel/block/mq/tag_set.rs
+++ b/rust/kernel/block/mq/tag_set.rs
@@ -10,7 +10,7 @@
     bindings,
     block::mq::{operations::OperationsVTable, request::RequestDataWrapper, Operations},
     error,
-    prelude::try_pin_init,
+    prelude::*,
     types::Opaque,
 };
 use core::{convert::TryInto, marker::PhantomData};
@@ -52,7 +52,7 @@ pub fn new(
                     queue_depth: num_tags,
                     cmd_size,
                     flags: 0,
-                    driver_data: core::ptr::null_mut::<crate::ffi::c_void>(),
+                    driver_data: core::ptr::null_mut::<c_void>(),
                     nr_maps: num_maps,
                     ..tag_set
                 }
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 21b343a1dc4d..3ed640f8ab24 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -6,6 +6,7 @@

 use crate::{
     bindings,
+    prelude::*,
     str::CStr,
     types::{ARef, Opaque},
 };
@@ -174,10 +175,10 @@ unsafe fn printk(&self, klevel: &[u8], msg: fmt::Arguments<'_>) {
         #[cfg(CONFIG_PRINTK)]
         unsafe {
             bindings::_dev_printk(
-                klevel as *const _ as *const crate::ffi::c_char,
+                klevel as *const _ as *const c_char,
                 self.as_raw(),
                 c_str!("%pA").as_char_ptr(),
-                &msg as *const _ as *const crate::ffi::c_void,
+                &msg as *const _ as *const c_void,
             )
         };
     }
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index ddb1ce4a78d9..c0e1a0625253 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -10,7 +10,6 @@
     bindings,
     device::Device,
     error::{Error, Result},
-    ffi::c_void,
     prelude::*,
     revocable::Revocable,
     sync::Arc,
@@ -156,7 +155,7 @@ fn remove_action(this: &Arc<Self>) {
     }

     #[allow(clippy::missing_safety_doc)]
-    unsafe extern "C" fn devres_callback(ptr: *mut kernel::ffi::c_void) {
+    unsafe extern "C" fn devres_callback(ptr: *mut c_void) {
         let ptr = ptr as *mut DevresInner<T>;
         // Devres owned this memory; now that we received the callback, drop the `Arc` and hence the
         // reference.
diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index 8cdc76043ee7..1e1899893d47 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -9,6 +9,7 @@
     device::Device,
     error::code::*,
     error::Result,
+    prelude::*,
     transmute::{AsBytes, FromBytes},
     types::ARef,
 };
@@ -37,7 +38,7 @@

 impl Attrs {
     /// Get the raw representation of this attribute.
-    pub(crate) fn as_raw(self) -> crate::ffi::c_ulong {
+    pub(crate) fn as_raw(self) -> c_ulong {
         self.0 as _
     }

diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 3dee3139fcd4..efac9f970ae2 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -6,6 +6,7 @@

 use crate::{
     alloc::{layout::LayoutError, AllocError},
+    prelude::*,
     str::CStr,
 };

@@ -102,7 +103,7 @@ impl Error {
     ///
     /// It is a bug to pass an out-of-range `errno`. `EINVAL` would
     /// be returned in such a case.
-    pub fn from_errno(errno: crate::ffi::c_int) -> Error {
+    pub fn from_errno(errno: c_int) -> Error {
         if let Some(error) = Self::try_from_errno(errno) {
             error
         } else {
@@ -118,7 +119,7 @@ pub fn from_errno(errno: crate::ffi::c_int) -> Error {
     /// Creates an [`Error`] from a kernel error code.
     ///
     /// Returns [`None`] if `errno` is out-of-range.
-    const fn try_from_errno(errno: crate::ffi::c_int) -> Option<Error> {
+    const fn try_from_errno(errno: c_int) -> Option<Error> {
         if errno < -(bindings::MAX_ERRNO as i32) || errno >= 0 {
             return None;
         }
@@ -132,7 +133,7 @@ const fn try_from_errno(errno: crate::ffi::c_int) -> Option<Error> {
     /// # Safety
     ///
     /// `errno` must be within error code range (i.e. `>= -MAX_ERRNO && < 0`).
-    const unsafe fn from_errno_unchecked(errno: crate::ffi::c_int) -> Error {
+    const unsafe fn from_errno_unchecked(errno: c_int) -> Error {
         // INVARIANT: The contract ensures the type invariant
         // will hold.
         // SAFETY: The caller guarantees `errno` is non-zero.
@@ -140,7 +141,7 @@ const fn try_from_errno(errno: crate::ffi::c_int) -> Option<Error> {
     }

     /// Returns the kernel error code.
-    pub fn to_errno(self) -> crate::ffi::c_int {
+    pub fn to_errno(self) -> c_int {
         self.0.get()
     }

@@ -376,7 +377,7 @@ fn from(e: core::convert::Infallible) -> Error {

 /// Converts an integer as returned by a C kernel function to an error if it's negative, and
 /// `Ok(())` otherwise.
-pub fn to_result(err: crate::ffi::c_int) -> Result {
+pub fn to_result(err: c_int) -> Result {
     if err < 0 {
         Err(Error::from_errno(err))
     } else {
@@ -399,15 +400,15 @@ pub fn to_result(err: crate::ffi::c_int) -> Result {
 /// fn devm_platform_ioremap_resource(
 ///     pdev: &mut PlatformDevice,
 ///     index: u32,
-/// ) -> Result<*mut kernel::ffi::c_void> {
+/// ) -> Result<*mut c_void> {
 ///     // SAFETY: `pdev` points to a valid platform device. There are no safety requirements
 ///     // on `index`.
 ///     from_err_ptr(unsafe { bindings::devm_platform_ioremap_resource(pdev.to_ptr(), index) })
 /// }
 /// ```
 pub fn from_err_ptr<T>(ptr: *mut T) -> Result<*mut T> {
-    // CAST: Casting a pointer to `*const crate::ffi::c_void` is always valid.
-    let const_ptr: *const crate::ffi::c_void = ptr.cast();
+    // CAST: Casting a pointer to `*const c_void` is always valid.
+    let const_ptr: *const c_void = ptr.cast();
     // SAFETY: The FFI function does not deref the pointer.
     if unsafe { bindings::IS_ERR(const_ptr) } {
         // SAFETY: The FFI function does not deref the pointer.
@@ -423,7 +424,7 @@ pub fn from_err_ptr<T>(ptr: *mut T) -> Result<*mut T> {
         //
         // SAFETY: `IS_ERR()` ensures `err` is a
         // negative value greater-or-equal to `-bindings::MAX_ERRNO`.
-        return Err(unsafe { Error::from_errno_unchecked(err as crate::ffi::c_int) });
+        return Err(unsafe { Error::from_errno_unchecked(err as c_int) });
     }
     Ok(ptr)
 }
@@ -443,7 +444,7 @@ pub fn from_err_ptr<T>(ptr: *mut T) -> Result<*mut T> {
 /// # use kernel::bindings;
 /// unsafe extern "C" fn probe_callback(
 ///     pdev: *mut bindings::platform_device,
-/// ) -> kernel::ffi::c_int {
+/// ) -> c_int {
 ///     from_result(|| {
 ///         let ptr = devm_alloc(pdev)?;
 ///         bindings::platform_set_drvdata(pdev, ptr);
diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index 8d228c237954..e3b24e747322 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -79,7 +79,7 @@
 //! # }
 //! # // `Error::from_errno` is `pub(crate)` in the `kernel` crate, thus provide a workaround.
 //! # trait FromErrno {
-//! #     fn from_errno(errno: core::ffi::c_int) -> Error {
+//! #     fn from_errno(errno: c_int) -> Error {
 //! #         // Dummy error that can be constructed outside the `kernel` crate.
 //! #         Error::from(core::fmt::Error)
 //! #     }
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 1604fb6a5b1b..7ddf4758bf2f 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -6,7 +6,8 @@
 //!
 //! Reference: <https://docs.kernel.org/dev-tools/kunit/index.html>

-use core::{ffi::c_void, fmt};
+use core::fmt;
+use crate::prelude::*;

 /// Prints a KUnit error-level message.
 ///
diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index fa9ecc42602a..b8153de8b229 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -12,7 +12,6 @@
     bindings,
     device::Device,
     error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR},
-    ffi::{c_int, c_long, c_uint, c_ulong},
     fs::File,
     prelude::*,
     seq_file::SeqFile,
diff --git a/rust/kernel/net/phy.rs b/rust/kernel/net/phy.rs
index a59469c785e3..eba425fc5cd2 100644
--- a/rust/kernel/net/phy.rs
+++ b/rust/kernel/net/phy.rs
@@ -312,9 +312,7 @@ impl<T: Driver> Adapter<T> {
     /// # Safety
     ///
     /// `phydev` must be passed by the corresponding callback in `phy_driver`.
-    unsafe extern "C" fn soft_reset_callback(
-        phydev: *mut bindings::phy_device,
-    ) -> crate::ffi::c_int {
+    unsafe extern "C" fn soft_reset_callback(phydev: *mut bindings::phy_device) -> c_int {
         from_result(|| {
             // SAFETY: This callback is called only in contexts
             // where we hold `phy_device->lock`, so the accessors on
@@ -328,7 +326,7 @@ impl<T: Driver> Adapter<T> {
     /// # Safety
     ///
     /// `phydev` must be passed by the corresponding callback in `phy_driver`.
-    unsafe extern "C" fn probe_callback(phydev: *mut bindings::phy_device) -> crate::ffi::c_int {
+    unsafe extern "C" fn probe_callback(phydev: *mut bindings::phy_device) -> c_int {
         from_result(|| {
             // SAFETY: This callback is called only in contexts
             // where we can exclusively access `phy_device` because
@@ -343,9 +341,7 @@ impl<T: Driver> Adapter<T> {
     /// # Safety
     ///
     /// `phydev` must be passed by the corresponding callback in `phy_driver`.
-    unsafe extern "C" fn get_features_callback(
-        phydev: *mut bindings::phy_device,
-    ) -> crate::ffi::c_int {
+    unsafe extern "C" fn get_features_callback(phydev: *mut bindings::phy_device) -> c_int {
         from_result(|| {
             // SAFETY: This callback is called only in contexts
             // where we hold `phy_device->lock`, so the accessors on
@@ -359,7 +355,7 @@ impl<T: Driver> Adapter<T> {
     /// # Safety
     ///
     /// `phydev` must be passed by the corresponding callback in `phy_driver`.
-    unsafe extern "C" fn suspend_callback(phydev: *mut bindings::phy_device) -> crate::ffi::c_int {
+    unsafe extern "C" fn suspend_callback(phydev: *mut bindings::phy_device) -> c_int {
         from_result(|| {
             // SAFETY: The C core code ensures that the accessors on
             // `Device` are okay to call even though `phy_device->lock`
@@ -373,7 +369,7 @@ impl<T: Driver> Adapter<T> {
     /// # Safety
     ///
     /// `phydev` must be passed by the corresponding callback in `phy_driver`.
-    unsafe extern "C" fn resume_callback(phydev: *mut bindings::phy_device) -> crate::ffi::c_int {
+    unsafe extern "C" fn resume_callback(phydev: *mut bindings::phy_device) -> c_int {
         from_result(|| {
             // SAFETY: The C core code ensures that the accessors on
             // `Device` are okay to call even though `phy_device->lock`
@@ -387,9 +383,7 @@ impl<T: Driver> Adapter<T> {
     /// # Safety
     ///
     /// `phydev` must be passed by the corresponding callback in `phy_driver`.
-    unsafe extern "C" fn config_aneg_callback(
-        phydev: *mut bindings::phy_device,
-    ) -> crate::ffi::c_int {
+    unsafe extern "C" fn config_aneg_callback(phydev: *mut bindings::phy_device) -> c_int {
         from_result(|| {
             // SAFETY: This callback is called only in contexts
             // where we hold `phy_device->lock`, so the accessors on
@@ -403,9 +397,7 @@ impl<T: Driver> Adapter<T> {
     /// # Safety
     ///
     /// `phydev` must be passed by the corresponding callback in `phy_driver`.
-    unsafe extern "C" fn read_status_callback(
-        phydev: *mut bindings::phy_device,
-    ) -> crate::ffi::c_int {
+    unsafe extern "C" fn read_status_callback(phydev: *mut bindings::phy_device) -> c_int {
         from_result(|| {
             // SAFETY: This callback is called only in contexts
             // where we hold `phy_device->lock`, so the accessors on
@@ -419,9 +411,7 @@ impl<T: Driver> Adapter<T> {
     /// # Safety
     ///
     /// `phydev` must be passed by the corresponding callback in `phy_driver`.
-    unsafe extern "C" fn match_phy_device_callback(
-        phydev: *mut bindings::phy_device,
-    ) -> crate::ffi::c_int {
+    unsafe extern "C" fn match_phy_device_callback(phydev: *mut bindings::phy_device) -> c_int {
         // SAFETY: This callback is called only in contexts
         // where we hold `phy_device->lock`, so the accessors on
         // `Device` are okay to call.
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index c97d6d470b28..c89f05dd0aeb 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -61,7 +61,7 @@ impl<T: Driver + 'static> Adapter<T> {
     extern "C" fn probe_callback(
         pdev: *mut bindings::pci_dev,
         id: *const bindings::pci_device_id,
-    ) -> kernel::ffi::c_int {
+    ) -> c_int {
         // SAFETY: The PCI bus only ever calls the probe callback with a valid pointer to a
         // `struct pci_dev`.
         //
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 4917cb34e2fe..d1e063db5138 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -57,7 +57,7 @@ unsafe fn unregister(pdrv: &Opaque<Self::RegType>) {
 }

 impl<T: Driver + 'static> Adapter<T> {
-    extern "C" fn probe_callback(pdev: *mut bindings::platform_device) -> kernel::ffi::c_int {
+    extern "C" fn probe_callback(pdev: *mut bindings::platform_device) -> c_int {
         // SAFETY: The platform bus only ever calls the probe callback with a valid pointer to a
         // `struct platform_device`.
         //
diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
index baa774a351ce..f869b02f1f25 100644
--- a/rust/kernel/prelude.rs
+++ b/rust/kernel/prelude.rs
@@ -14,6 +14,11 @@
 #[doc(no_inline)]
 pub use core::pin::Pin;

+pub use ::ffi::{
+    c_char, c_int, c_long, c_longlong, c_schar, c_short, c_uchar, c_uint, c_ulong, c_ulonglong,
+    c_ushort, c_void,
+};
+
 pub use crate::alloc::{flags::*, Box, KBox, KVBox, KVVec, KVec, VBox, VVec, Vec};

 #[doc(no_inline)]
diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
index cf4714242e14..796e93c59b94 100644
--- a/rust/kernel/print.rs
+++ b/rust/kernel/print.rs
@@ -6,11 +6,7 @@
 //!
 //! Reference: <https://docs.kernel.org/core-api/printk-basics.html>

-use crate::{
-    ffi::{c_char, c_void},
-    prelude::*,
-    str::RawFormatter,
-};
+use crate::{prelude::*, str::RawFormatter};
 use core::fmt;

 // Called from `vsprintf` with format specifier `%pA`.
diff --git a/rust/kernel/seq_file.rs b/rust/kernel/seq_file.rs
index 7a9403eb6e5b..7aed1d6231aa 100644
--- a/rust/kernel/seq_file.rs
+++ b/rust/kernel/seq_file.rs
@@ -4,7 +4,7 @@
 //!
 //! C header: [`include/linux/seq_file.h`](srctree/include/linux/seq_file.h)

-use crate::{bindings, c_str, types::NotThreadSafe, types::Opaque};
+use crate::{bindings, c_str, prelude::*, types::NotThreadSafe, types::Opaque};

 /// A utility for generating the contents of a seq file.
 #[repr(transparent)]
@@ -37,7 +37,7 @@ pub fn call_printf(&self, args: core::fmt::Arguments<'_>) {
             bindings::seq_printf(
                 self.inner.get(),
                 c_str!("%pA").as_char_ptr(),
-                &args as *const _ as *const crate::ffi::c_void,
+                &args as *const _ as *const c_void,
             );
         }
     }
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 878111cb77bc..2c78f6be9959 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -6,7 +6,7 @@
 use core::fmt::{self, Write};
 use core::ops::{self, Deref, DerefMut, Index};

-use crate::error::{code::*, Error};
+use crate::{error::Error, prelude::*};

 /// Byte string without UTF-8 validity guarantee.
 #[repr(transparent)]
@@ -232,7 +232,7 @@ pub const fn is_empty(&self) -> bool {
     /// last at least `'a`. When `CStr` is alive, the memory pointed by `ptr`
     /// must not be mutated.
     #[inline]
-    pub unsafe fn from_char_ptr<'a>(ptr: *const crate::ffi::c_char) -> &'a Self {
+    pub unsafe fn from_char_ptr<'a>(ptr: *const c_char) -> &'a Self {
         // SAFETY: The safety precondition guarantees `ptr` is a valid pointer
         // to a `NUL`-terminated C string.
         let len = unsafe { bindings::strlen(ptr) } + 1;
@@ -295,7 +295,7 @@ pub unsafe fn from_bytes_with_nul_unchecked_mut(bytes: &mut [u8]) -> &mut CStr {

     /// Returns a C pointer to the string.
     #[inline]
-    pub const fn as_char_ptr(&self) -> *const crate::ffi::c_char {
+    pub const fn as_char_ptr(&self) -> *const c_char {
         self.0.as_ptr()
     }

diff --git a/rust/kernel/sync/arc.rs b/rust/kernel/sync/arc.rs
index 8484c814609a..7309d8723241 100644
--- a/rust/kernel/sync/arc.rs
+++ b/rust/kernel/sync/arc.rs
@@ -20,6 +20,7 @@
     alloc::{AllocError, Flags, KBox},
     bindings,
     init::InPlaceInit,
+    prelude::*,
     try_init,
     types::{ForeignOwnable, Opaque},
 };
@@ -375,11 +376,11 @@ impl<T: 'static> ForeignOwnable for Arc<T> {
     type Borrowed<'a> = ArcBorrow<'a, T>;
     type BorrowedMut<'a> = Self::Borrowed<'a>;

-    fn into_foreign(self) -> *mut crate::ffi::c_void {
+    fn into_foreign(self) -> *mut c_void {
         ManuallyDrop::new(self).ptr.as_ptr().cast()
     }

-    unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self {
+    unsafe fn from_foreign(ptr: *mut c_void) -> Self {
         // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous
         // call to `Self::into_foreign`.
         let inner = unsafe { NonNull::new_unchecked(ptr.cast::<ArcInner<T>>()) };
@@ -390,7 +391,7 @@ unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self {
         unsafe { Self::from_inner(inner) }
     }

-    unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> ArcBorrow<'a, T> {
+    unsafe fn borrow<'a>(ptr: *mut c_void) -> ArcBorrow<'a, T> {
         // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous
         // call to `Self::into_foreign`.
         let inner = unsafe { NonNull::new_unchecked(ptr.cast::<ArcInner<T>>()) };
@@ -400,7 +401,7 @@ unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> ArcBorrow<'a, T> {
         unsafe { ArcBorrow::new(inner) }
     }

-    unsafe fn borrow_mut<'a>(ptr: *mut crate::ffi::c_void) -> ArcBorrow<'a, T> {
+    unsafe fn borrow_mut<'a>(ptr: *mut c_void) -> ArcBorrow<'a, T> {
         // SAFETY: The safety requirements for `borrow_mut` are a superset of the safety
         // requirements for `borrow`.
         unsafe { Self::borrow(ptr) }
diff --git a/rust/kernel/sync/condvar.rs b/rust/kernel/sync/condvar.rs
index caebf03f553b..c59733f2db25 100644
--- a/rust/kernel/sync/condvar.rs
+++ b/rust/kernel/sync/condvar.rs
@@ -7,7 +7,7 @@

 use super::{lock::Backend, lock::Guard, LockClassKey};
 use crate::{
-    ffi::{c_int, c_long},
+    prelude::*,
     str::CStr,
     task::{
         MAX_SCHEDULE_TIMEOUT, TASK_FREEZABLE, TASK_INTERRUPTIBLE, TASK_NORMAL, TASK_UNINTERRUPTIBLE,
diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index e82fa5be289c..f94a70713494 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -7,6 +7,7 @@

 use super::LockClassKey;
 use crate::{
+    prelude::*,
     str::CStr,
     types::{NotThreadSafe, Opaque, ScopeGuard},
 };
@@ -50,11 +51,7 @@ pub unsafe trait Backend {
     ///
     /// `ptr` must be valid for write for the duration of the call, while `name` and `key` must
     /// remain valid for read indefinitely.
-    unsafe fn init(
-        ptr: *mut Self::State,
-        name: *const crate::ffi::c_char,
-        key: *mut bindings::lock_class_key,
-    );
+    unsafe fn init(ptr: *mut Self::State, name: *const c_char, key: *mut bindings::lock_class_key);

     /// Acquires the lock, making the caller its owner.
     ///
diff --git a/rust/kernel/sync/lock/mutex.rs b/rust/kernel/sync/lock/mutex.rs
index 581cee7ab842..b76fa480b9ba 100644
--- a/rust/kernel/sync/lock/mutex.rs
+++ b/rust/kernel/sync/lock/mutex.rs
@@ -4,6 +4,8 @@
 //!
 //! This module allows Rust code to use the kernel's `struct mutex`.

+use crate::prelude::*;
+
 /// Creates a [`Mutex`] initialiser with the given name and a newly-created lock class.
 ///
 /// It uses the name if one is given, otherwise it generates one based on the file name and line
@@ -102,11 +104,7 @@ unsafe impl super::Backend for MutexBackend {
     type State = bindings::mutex;
     type GuardState = ();

-    unsafe fn init(
-        ptr: *mut Self::State,
-        name: *const crate::ffi::c_char,
-        key: *mut bindings::lock_class_key,
-    ) {
+    unsafe fn init(ptr: *mut Self::State, name: *const c_char, key: *mut bindings::lock_class_key) {
         // SAFETY: The safety requirements ensure that `ptr` is valid for writes, and `name` and
         // `key` are valid for read indefinitely.
         unsafe { bindings::__mutex_init(ptr, name, key) }
diff --git a/rust/kernel/sync/lock/spinlock.rs b/rust/kernel/sync/lock/spinlock.rs
index d7be38ccbdc7..3313207160b7 100644
--- a/rust/kernel/sync/lock/spinlock.rs
+++ b/rust/kernel/sync/lock/spinlock.rs
@@ -4,6 +4,8 @@
 //!
 //! This module allows Rust code to use the kernel's `spinlock_t`.

+use crate::prelude::*;
+
 /// Creates a [`SpinLock`] initialiser with the given name and a newly-created lock class.
 ///
 /// It uses the name if one is given, otherwise it generates one based on the file name and line
@@ -101,11 +103,7 @@ unsafe impl super::Backend for SpinLockBackend {
     type State = bindings::spinlock_t;
     type GuardState = ();

-    unsafe fn init(
-        ptr: *mut Self::State,
-        name: *const crate::ffi::c_char,
-        key: *mut bindings::lock_class_key,
-    ) {
+    unsafe fn init(ptr: *mut Self::State, name: *const c_char, key: *mut bindings::lock_class_key) {
         // SAFETY: The safety requirements ensure that `ptr` is valid for writes, and `name` and
         // `key` are valid for read indefinitely.
         unsafe { bindings::__spin_lock_init(ptr, name, key) }
diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs
index 9e6f6854948d..d1d916e397c2 100644
--- a/rust/kernel/task.rs
+++ b/rust/kernel/task.rs
@@ -6,8 +6,8 @@

 use crate::{
     bindings,
-    ffi::{c_int, c_long, c_uint},
     pid_namespace::PidNamespace,
+    prelude::*,
     types::{ARef, NotThreadSafe, Opaque},
 };
 use core::{
diff --git a/rust/kernel/time.rs b/rust/kernel/time.rs
index f509cb0eb71e..ee306e151f16 100644
--- a/rust/kernel/time.rs
+++ b/rust/kernel/time.rs
@@ -8,16 +8,18 @@
 //! C header: [`include/linux/jiffies.h`](srctree/include/linux/jiffies.h).
 //! C header: [`include/linux/ktime.h`](srctree/include/linux/ktime.h).

+use crate::prelude::*;
+
 pub mod hrtimer;

 /// The number of nanoseconds per millisecond.
 pub const NSEC_PER_MSEC: i64 = bindings::NSEC_PER_MSEC as i64;

 /// The time unit of Linux kernel. One jiffy equals (1/HZ) second.
-pub type Jiffies = crate::ffi::c_ulong;
+pub type Jiffies = c_ulong;

 /// The millisecond time unit.
-pub type Msecs = crate::ffi::c_uint;
+pub type Msecs = c_uint;

 /// Converts milliseconds to jiffies.
 #[inline]
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 9d0471afc964..ecf314acfefd 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -2,6 +2,7 @@

 //! Kernel types.

+use crate::prelude::*;
 use core::{
     cell::UnsafeCell,
     marker::{PhantomData, PhantomPinned},
@@ -36,7 +37,7 @@ pub trait ForeignOwnable: Sized {
     /// [`try_from_foreign`]: Self::try_from_foreign
     /// [`borrow`]: Self::borrow
     /// [`borrow_mut`]: Self::borrow_mut
-    fn into_foreign(self) -> *mut crate::ffi::c_void;
+    fn into_foreign(self) -> *mut c_void;

     /// Converts a foreign-owned object back to a Rust-owned one.
     ///
@@ -46,7 +47,7 @@ pub trait ForeignOwnable: Sized {
     /// must not be passed to `from_foreign` more than once.
     ///
     /// [`into_foreign`]: Self::into_foreign
-    unsafe fn from_foreign(ptr: *mut crate::ffi::c_void) -> Self;
+    unsafe fn from_foreign(ptr: *mut c_void) -> Self;

     /// Tries to convert a foreign-owned object back to a Rust-owned one.
     ///
@@ -58,7 +59,7 @@ pub trait ForeignOwnable: Sized {
     /// `ptr` must either be null or satisfy the safety requirements for [`from_foreign`].
     ///
     /// [`from_foreign`]: Self::from_foreign
-    unsafe fn try_from_foreign(ptr: *mut crate::ffi::c_void) -> Option<Self> {
+    unsafe fn try_from_foreign(ptr: *mut c_void) -> Option<Self> {
         if ptr.is_null() {
             None
         } else {
@@ -81,7 +82,7 @@ unsafe fn try_from_foreign(ptr: *mut crate::ffi::c_void) -> Option<Self> {
     ///
     /// [`into_foreign`]: Self::into_foreign
     /// [`from_foreign`]: Self::from_foreign
-    unsafe fn borrow<'a>(ptr: *mut crate::ffi::c_void) -> Self::Borrowed<'a>;
+    unsafe fn borrow<'a>(ptr: *mut c_void) -> Self::Borrowed<'a>;

     /// Borrows a foreign-owned object mutably.
     ///
@@ -109,21 +110,21 @@ unsafe fn try_from_foreign(ptr: *mut crate::ffi::c_void) -> Option<Self> {
     /// [`from_foreign`]: Self::from_foreign
     /// [`borrow`]: Self::borrow
     /// [`Arc`]: crate::sync::Arc
-    unsafe fn borrow_mut<'a>(ptr: *mut crate::ffi::c_void) -> Self::BorrowedMut<'a>;
+    unsafe fn borrow_mut<'a>(ptr: *mut c_void) -> Self::BorrowedMut<'a>;
 }

 impl ForeignOwnable for () {
     type Borrowed<'a> = ();
     type BorrowedMut<'a> = ();

-    fn into_foreign(self) -> *mut crate::ffi::c_void {
+    fn into_foreign(self) -> *mut c_void {
         core::ptr::NonNull::dangling().as_ptr()
     }

-    unsafe fn from_foreign(_: *mut crate::ffi::c_void) -> Self {}
+    unsafe fn from_foreign(_: *mut c_void) -> Self {}

-    unsafe fn borrow<'a>(_: *mut crate::ffi::c_void) -> Self::Borrowed<'a> {}
-    unsafe fn borrow_mut<'a>(_: *mut crate::ffi::c_void) -> Self::BorrowedMut<'a> {}
+    unsafe fn borrow<'a>(_: *mut c_void) -> Self::Borrowed<'a> {}
+    unsafe fn borrow_mut<'a>(_: *mut c_void) -> Self::BorrowedMut<'a> {}
 }

 /// Runs a cleanup function/closure when dropped.
diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
index 80a9782b1c6e..9d10a83569c2 100644
--- a/rust/kernel/uaccess.rs
+++ b/rust/kernel/uaccess.rs
@@ -8,7 +8,6 @@
     alloc::{Allocator, Flags},
     bindings,
     error::Result,
-    ffi::c_void,
     prelude::*,
     transmute::{AsBytes, FromBytes},
 };
@@ -45,7 +44,6 @@
 /// every byte in the region.
 ///
 /// ```no_run
-/// use kernel::ffi::c_void;
 /// use kernel::error::Result;
 /// use kernel::uaccess::{UserPtr, UserSlice};
 ///
@@ -67,7 +65,6 @@
 /// Example illustrating a TOCTOU (time-of-check to time-of-use) bug.
 ///
 /// ```no_run
-/// use kernel::ffi::c_void;
 /// use kernel::error::{code::EINVAL, Result};
 /// use kernel::uaccess::{UserPtr, UserSlice};
 ///
diff --git a/samples/rust/rust_print_main.rs b/samples/rust/rust_print_main.rs
index 8ea95e8c2f36..8a0558f60f34 100644
--- a/samples/rust/rust_print_main.rs
+++ b/samples/rust/rust_print_main.rs
@@ -101,7 +101,7 @@ fn drop(&mut self) {
 }

 mod trace {
-    use kernel::ffi::c_int;
+    use kernel::prelude::*;

     kernel::declare_trace! {
         /// # Safety

base-commit: 0af2f6be1b4281385b618cb86ad946eded089ac8
--
2.49.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] rust: fix building firmware abstraction on 32bit arm
  2025-04-11 13:47   ` Christian Schrefl
@ 2025-04-11 14:17     ` Danilo Krummrich
  0 siblings, 0 replies; 11+ messages in thread
From: Danilo Krummrich @ 2025-04-11 14:17 UTC (permalink / raw)
  To: Christian Schrefl
  Cc: Luis Chamberlain, Russ Weight, Miguel Ojeda, Alex Gaynor,
	Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, linux-kernel,
	rust-for-linux, stable

On Fri, Apr 11, 2025 at 03:47:28PM +0200, Christian Schrefl wrote:
> On 11.04.25 12:35 PM, Danilo Krummrich wrote:
> > On Fri, Apr 11, 2025 at 09:14:48AM +0200, Christian Schrefl wrote:
> > I did a test build with multi_v7_defconfig and I can't reproduce this issue.
> > 
> Interesting, I've it seems this is only an issue on 6.13 with my arm patches applied.
> 
> It seems that it works on v6.14 and v6.15-rc1 but the error occurs on ffd294d346d1 (tag: v6.13)
> with my 32-bit arm patches applied.

That makes sense, commit 1bae8729e50a ("rust: map `long` to `isize` and `char`
to `u8`") changed FwFunc to take a *const u8, which previously was *const i8.

> >> diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
> >> index f04b058b09b2d2397e26344d0e055b3aa5061432..1d6284316f2a4652ef3f76272670e5e29b0ff924 100644
> >> --- a/rust/kernel/firmware.rs
> >> +++ b/rust/kernel/firmware.rs
> >> @@ -5,14 +5,18 @@
> >>  //! C header: [`include/linux/firmware.h`](srctree/include/linux/firmware.h)
> >>  
> >>  use crate::{bindings, device::Device, error::Error, error::Result, str::CStr};
> >> -use core::ptr::NonNull;
> >> +use core::{ffi, ptr::NonNull};
> > 
> > The change itself seems to be fine anyways, but I think we should use crate::ffi
> > instead.
> Right, I just did what RA recommended without thinking about it much.
> 
> I guess this patch isn't really needed. Should I still send a V2 using `crate::ffi`?

Yes, please. I think it's still an improvement.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] rust: fix building firmware abstraction on 32bit arm
  2025-04-11 14:15   ` Miguel Ojeda
@ 2025-04-11 14:18     ` Miguel Ojeda
  2025-04-12 10:01     ` Benno Lossin
  1 sibling, 0 replies; 11+ messages in thread
From: Miguel Ojeda @ 2025-04-11 14:18 UTC (permalink / raw)
  To: Benno Lossin
  Cc: Christian Schrefl, Luis Chamberlain, Russ Weight,
	Danilo Krummrich, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	linux-kernel, rust-for-linux, stable

On Fri, Apr 11, 2025 at 4:15 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> In 6.6, C `char` changed to unsigned, but `core::ffi::c_char` is
> signed (in x86_64 at least).

By 6.6, I meant since 6.6.y LTS (since that is the one I remember due
to backports), the actual change happened earlier but after 6.1.y LTS.

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] rust: fix building firmware abstraction on 32bit arm
  2025-04-11 14:15   ` Miguel Ojeda
  2025-04-11 14:18     ` Miguel Ojeda
@ 2025-04-12 10:01     ` Benno Lossin
  2025-04-14 14:05       ` Alice Ryhl
  1 sibling, 1 reply; 11+ messages in thread
From: Benno Lossin @ 2025-04-12 10:01 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Christian Schrefl, Luis Chamberlain, Russ Weight,
	Danilo Krummrich, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	linux-kernel, rust-for-linux, stable

On Fri Apr 11, 2025 at 4:15 PM CEST, Miguel Ojeda wrote:
> On Fri, Apr 11, 2025 at 2:46 PM Benno Lossin <benno.lossin@proton.me> wrote:
>>
>> Ah I overlooked this, you should be using `kernel::ffi` (or
>> `crate::ffi`) instead of `core`. (for `c_char` it doesn't matter, but we
>> shouldn't be using `core::ffi`, since we have our own mappings).
>
> In 6.6, C `char` changed to unsigned, but `core::ffi::c_char` is
> signed (in x86_64 at least).
>
> We should just never use `core::ffi` (except in `rust/ffi.rs`, of
> course) -- I think we should just add the C types to the prelude
> (which we discussed in the past) so that it is easy to avoid the
> mistake (something like the patch attached as the end result, but
> tested and across a kernel cycle or two) and mention it in the Coding
> Guidelines. Thoughts?

Yeah sounds like a good idea.

> I tried to use Clippy's `disallowed-types` too:
>
>     disallowed-types = [
>         { path = "core::ffi::c_void", reason = "the `kernel::ffi`
> types should be used instead" },
>         { path = "core::ffi::c_char", reason = "the `kernel::ffi`
> types should be used instead" },
>         { path = "core::ffi::c_schar", reason = "the `kernel::ffi`
> types should be used instead" },
>         { path = "core::ffi::c_uchar", reason = "the `kernel::ffi`
> types should be used instead" },
>         { path = "core::ffi::c_short", reason = "the `kernel::ffi`
> types should be used instead" },
>         { path = "core::ffi::c_ushort", reason = "the `kernel::ffi`
> types should be used instead" },
>         { path = "core::ffi::c_int", reason = "the `kernel::ffi` types
> should be used instead" },
>         { path = "core::ffi::c_uint", reason = "the `kernel::ffi`
> types should be used instead" },
>         { path = "core::ffi::c_long", reason = "the `kernel::ffi`
> types should be used instead" },
>         { path = "core::ffi::c_ulong", reason = "the `kernel::ffi`
> types should be used instead" },
>         { path = "core::ffi::c_longlong", reason = "the `kernel::ffi`
> types should be used instead" },
>         { path = "core::ffi::c_ulonglong", reason = "the `kernel::ffi`
> types should be used instead" },
>     ]
>
> But it goes across aliases.

We could make the types in `ffi` be transparent newtypes. But not sure
if that could interfere with kCFI or other stuff.

---
Cheers,
Benno


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] rust: fix building firmware abstraction on 32bit arm
  2025-04-12 10:01     ` Benno Lossin
@ 2025-04-14 14:05       ` Alice Ryhl
  2025-04-14 14:52         ` Benno Lossin
  0 siblings, 1 reply; 11+ messages in thread
From: Alice Ryhl @ 2025-04-14 14:05 UTC (permalink / raw)
  To: Benno Lossin
  Cc: Miguel Ojeda, Christian Schrefl, Luis Chamberlain, Russ Weight,
	Danilo Krummrich, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Trevor Gross,
	linux-kernel, rust-for-linux, stable

On Sat, Apr 12, 2025 at 10:01:22AM +0000, Benno Lossin wrote:
> On Fri Apr 11, 2025 at 4:15 PM CEST, Miguel Ojeda wrote:
> > On Fri, Apr 11, 2025 at 2:46 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >>
> >> Ah I overlooked this, you should be using `kernel::ffi` (or
> >> `crate::ffi`) instead of `core`. (for `c_char` it doesn't matter, but we
> >> shouldn't be using `core::ffi`, since we have our own mappings).
> >
> > In 6.6, C `char` changed to unsigned, but `core::ffi::c_char` is
> > signed (in x86_64 at least).
> >
> > We should just never use `core::ffi` (except in `rust/ffi.rs`, of
> > course) -- I think we should just add the C types to the prelude
> > (which we discussed in the past) so that it is easy to avoid the
> > mistake (something like the patch attached as the end result, but
> > tested and across a kernel cycle or two) and mention it in the Coding
> > Guidelines. Thoughts?
> 
> Yeah sounds like a good idea.
> 
> > I tried to use Clippy's `disallowed-types` too:
> >
> >     disallowed-types = [
> >         { path = "core::ffi::c_void", reason = "the `kernel::ffi`
> > types should be used instead" },
> >         { path = "core::ffi::c_char", reason = "the `kernel::ffi`
> > types should be used instead" },
> >         { path = "core::ffi::c_schar", reason = "the `kernel::ffi`
> > types should be used instead" },
> >         { path = "core::ffi::c_uchar", reason = "the `kernel::ffi`
> > types should be used instead" },
> >         { path = "core::ffi::c_short", reason = "the `kernel::ffi`
> > types should be used instead" },
> >         { path = "core::ffi::c_ushort", reason = "the `kernel::ffi`
> > types should be used instead" },
> >         { path = "core::ffi::c_int", reason = "the `kernel::ffi` types
> > should be used instead" },
> >         { path = "core::ffi::c_uint", reason = "the `kernel::ffi`
> > types should be used instead" },
> >         { path = "core::ffi::c_long", reason = "the `kernel::ffi`
> > types should be used instead" },
> >         { path = "core::ffi::c_ulong", reason = "the `kernel::ffi`
> > types should be used instead" },
> >         { path = "core::ffi::c_longlong", reason = "the `kernel::ffi`
> > types should be used instead" },
> >         { path = "core::ffi::c_ulonglong", reason = "the `kernel::ffi`
> > types should be used instead" },
> >     ]
> >
> > But it goes across aliases.
> 
> We could make the types in `ffi` be transparent newtypes. But not sure
> if that could interfere with kCFI or other stuff.

Transparent newtypes for all integers would be super inconvenient.

Alice

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] rust: fix building firmware abstraction on 32bit arm
  2025-04-14 14:05       ` Alice Ryhl
@ 2025-04-14 14:52         ` Benno Lossin
  0 siblings, 0 replies; 11+ messages in thread
From: Benno Lossin @ 2025-04-14 14:52 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Miguel Ojeda, Christian Schrefl, Luis Chamberlain, Russ Weight,
	Danilo Krummrich, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Andreas Hindborg, Trevor Gross,
	linux-kernel, rust-for-linux, stable

On Mon Apr 14, 2025 at 4:05 PM CEST, Alice Ryhl wrote:
> On Sat, Apr 12, 2025 at 10:01:22AM +0000, Benno Lossin wrote:
>> On Fri Apr 11, 2025 at 4:15 PM CEST, Miguel Ojeda wrote:
>> > On Fri, Apr 11, 2025 at 2:46 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> >>
>> >> Ah I overlooked this, you should be using `kernel::ffi` (or
>> >> `crate::ffi`) instead of `core`. (for `c_char` it doesn't matter, but we
>> >> shouldn't be using `core::ffi`, since we have our own mappings).
>> >
>> > In 6.6, C `char` changed to unsigned, but `core::ffi::c_char` is
>> > signed (in x86_64 at least).
>> >
>> > We should just never use `core::ffi` (except in `rust/ffi.rs`, of
>> > course) -- I think we should just add the C types to the prelude
>> > (which we discussed in the past) so that it is easy to avoid the
>> > mistake (something like the patch attached as the end result, but
>> > tested and across a kernel cycle or two) and mention it in the Coding
>> > Guidelines. Thoughts?
>> 
>> Yeah sounds like a good idea.
>> 
>> > I tried to use Clippy's `disallowed-types` too:
>> >
>> >     disallowed-types = [
>> >         { path = "core::ffi::c_void", reason = "the `kernel::ffi`
>> > types should be used instead" },
>> >         { path = "core::ffi::c_char", reason = "the `kernel::ffi`
>> > types should be used instead" },
>> >         { path = "core::ffi::c_schar", reason = "the `kernel::ffi`
>> > types should be used instead" },
>> >         { path = "core::ffi::c_uchar", reason = "the `kernel::ffi`
>> > types should be used instead" },
>> >         { path = "core::ffi::c_short", reason = "the `kernel::ffi`
>> > types should be used instead" },
>> >         { path = "core::ffi::c_ushort", reason = "the `kernel::ffi`
>> > types should be used instead" },
>> >         { path = "core::ffi::c_int", reason = "the `kernel::ffi` types
>> > should be used instead" },
>> >         { path = "core::ffi::c_uint", reason = "the `kernel::ffi`
>> > types should be used instead" },
>> >         { path = "core::ffi::c_long", reason = "the `kernel::ffi`
>> > types should be used instead" },
>> >         { path = "core::ffi::c_ulong", reason = "the `kernel::ffi`
>> > types should be used instead" },
>> >         { path = "core::ffi::c_longlong", reason = "the `kernel::ffi`
>> > types should be used instead" },
>> >         { path = "core::ffi::c_ulonglong", reason = "the `kernel::ffi`
>> > types should be used instead" },
>> >     ]
>> >
>> > But it goes across aliases.
>> 
>> We could make the types in `ffi` be transparent newtypes. But not sure
>> if that could interfere with kCFI or other stuff.
>
> Transparent newtypes for all integers would be super inconvenient.

Yeah I noticed that too when trying... We often assign integer literals
to the ffi types.

---
Cheers,
Benno


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-04-14 14:52 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-11  7:14 [PATCH] rust: fix building firmware abstraction on 32bit arm Christian Schrefl
2025-04-11  8:37 ` Benno Lossin
2025-04-11 10:35 ` Danilo Krummrich
2025-04-11 13:47   ` Christian Schrefl
2025-04-11 14:17     ` Danilo Krummrich
2025-04-11 12:45 ` Benno Lossin
2025-04-11 14:15   ` Miguel Ojeda
2025-04-11 14:18     ` Miguel Ojeda
2025-04-12 10:01     ` Benno Lossin
2025-04-14 14:05       ` Alice Ryhl
2025-04-14 14:52         ` Benno Lossin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).