On Fri, Apr 18, 2025 at 5:37 PM Tamir Duberstein <tam...@gmail.com> wrote:
>
> -            bindings::BLK_STS_OK as _
> +            bindings::BLK_STS_OK as u8

> -        unsafe { bindings::blk_mq_end_request(request_ptr, 
> bindings::BLK_STS_OK as _) };
> +        unsafe { bindings::blk_mq_end_request(request_ptr, 
> bindings::BLK_STS_OK as u8) };

For these two: `BLK_STS_OK` was discussed in a previous version, but
why are we not using `blk_status_t` type instead?

We are even already using it in the first case, and in the second it
is the parameter's type.

> -/// # use kernel::{bindings, c_str, device::Device, devres::Devres, io::{Io, 
> IoRaw}};
> +/// # use kernel::{bindings, c_str, device::Device, devres::Devres, 
> ffi::c_void, io::{Io, IoRaw}};

For v11 this can be removed since it is now in the prelude. There may
others that can be removed too (I would not add an import just to use
it in these patches, but if the prelude is already imported, then we
should use it).

> -        result.minor = bindings::MISC_DYNAMIC_MINOR as _;
> +        result.minor = bindings::MISC_DYNAMIC_MINOR as i32;

Similarly here, shouldn't we use `c_int`?

i.e. it is the one in the C side, not the "resolved" `i32` that the
compiler suggests.

> -                    bindings::wq_misc_consts_WORK_CPU_UNBOUND as _,
> +                    bindings::wq_misc_consts_WORK_CPU_UNBOUND as i32,

Similarly, this should probably be `c_int` since that is the
parameter's type, right?

Cheers,
Miguel

Reply via email to