Re: [PATCH] rust: fix building firmware abstraction on 32bit arm

[Christian Schrefl <chrisi.schrefl@gmail.com>]

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