diff mbox series

rust: prelude: add bit function

Message ID 20240130-rust-bit-v1-1-ed2ed6389a58@christina-quast.de
State New
Headers
Series rust: prelude: add bit function |

Commit Message

Christina Quast Jan. 30, 2024, 7:47 p.m. UTC 
  In order to create masks easily, the define BIT() is used in C code.
This commit adds the same functionality to the rust kernel.

To use it, include the following into your rust file:
use kernel::prelude::bit

Signed-off-by: Christina Quast <contact@christina-quast.de>
---
This patch is needed for further patches porting the rockchip phy
driver to rust.
---
 rust/kernel/prelude.rs | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)


---
base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
change-id: 20240130-rust-bit-99dd6d3a0536

Best regards,

Comments

Carlos Bilbao Jan. 30, 2024, 7:59 p.m. UTC | #1 
On 1/30/24 13:47, Christina Quast wrote:
> In order to create masks easily, the define BIT() is used in C code.
> This commit adds the same functionality to the rust kernel.
> 
> To use it, include the following into your rust file:
> use kernel::prelude::bit
> 
> Signed-off-by: Christina Quast <contact@christina-quast.de>

Reviewed-by: Carlos Bilbao <carlos.bilbao@amd.com>

> ---
> This patch is needed for further patches porting the rockchip phy
> driver to rust.
> ---
>   rust/kernel/prelude.rs | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
> index ae21600970b3..16e483de2f27 100644
> --- a/rust/kernel/prelude.rs
> +++ b/rust/kernel/prelude.rs
> @@ -38,3 +38,19 @@
>   pub use super::init::{InPlaceInit, Init, PinInit};
>   
>   pub use super::current;
> +
> +/// Returns a `u32` number that has only the `n`th bit set.
> +///
> +/// # Arguments
> +///
> +/// * `n` - A `u32` that specifies the bit position (zero-based index)
> +///
> +/// # Example
> +///
> +/// ```
> +/// let b = bit(2);
> +/// assert_eq!(b, 4);
> +#[inline]
> +pub const fn bit(n: u32) -> u32 {
> +    1 << n
> +}
> 
> ---
> base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
> change-id: 20240130-rust-bit-99dd6d3a0536
> 
> Best regards,
Wedson Almeida Filho Jan. 30, 2024, 8:04 p.m. UTC | #2 
On Tue, 30 Jan 2024 at 16:48, Christina Quast
<contact@christina-quast.de> wrote:
>
> In order to create masks easily, the define BIT() is used in C code.
> This commit adds the same functionality to the rust kernel.

The `rust` branch has a generic `bit` function that allows the
returned bit to be used with or converted to several numeric types.

https://github.com/Rust-for-Linux/linux/blob/rust/rust/kernel/types.rs#L344
Boqun Feng Jan. 30, 2024, 8:05 p.m. UTC | #3 
On Tue, Jan 30, 2024 at 05:04:15PM -0300, Wedson Almeida Filho wrote:
> On Tue, 30 Jan 2024 at 16:48, Christina Quast
> <contact@christina-quast.de> wrote:
> >
> > In order to create masks easily, the define BIT() is used in C code.
> > This commit adds the same functionality to the rust kernel.
> 
> The `rust` branch has a generic `bit` function that allows the
> returned bit to be used with or converted to several numeric types.
> 
> https://github.com/Rust-for-Linux/linux/blob/rust/rust/kernel/types.rs#L344

Plus, I think it's better to put the implementation at somewhere like:

	kernel::type::bitops

Of course, it's OK to prelude it, but we shouldn't put implementation in
prelude.rs

Regards,
Boqun
Miguel Ojeda Jan. 30, 2024, 8:10 p.m. UTC | #4 
Hi Christina,

Thanks for the patch! Please see below.

On Tue, Jan 30, 2024 at 8:48 PM Christina Quast
<contact@christina-quast.de> wrote:
>
> In order to create masks easily, the define BIT() is used in C code.
> This commit adds the same functionality to the rust kernel.
>
> To use it, include the following into your rust file:
> use kernel::prelude::bit

This is the prelude, i.e. the point is that it does not need to be `use`d.

Did you check the `rust` branch? We had something like this there.

> diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
> index ae21600970b3..16e483de2f27 100644
> --- a/rust/kernel/prelude.rs
> +++ b/rust/kernel/prelude.rs

Please note that the prelude is meant to re-export existing
functionality elsewhere in the `kernel` crate, not to define items
there.

> +/// # Arguments
> +///
> +/// * `n` - A `u32` that specifies the bit position (zero-based index)

We don't use "Arguments"-like sections. Please see other functions'
documentation to see how we usually do it.

> +/// # Example

Please use the plural.

> +/// ```
> +/// let b = bit(2);
> +/// assert_eq!(b, 4);

The example is not closed.

> +#[inline]
> +pub const fn bit(n: u32) -> u32 {
> +    1 << n
> +}

Should it be a generic?

Cheers,
Miguel
diff mbox series

Patch

diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
index ae21600970b3..16e483de2f27 100644
--- a/rust/kernel/prelude.rs
+++ b/rust/kernel/prelude.rs
@@ -38,3 +38,19 @@ 
 pub use super::init::{InPlaceInit, Init, PinInit};
 
 pub use super::current;
+
+/// Returns a `u32` number that has only the `n`th bit set.
+///
+/// # Arguments
+///
+/// * `n` - A `u32` that specifies the bit position (zero-based index)
+///
+/// # Example
+///
+/// ```
+/// let b = bit(2);
+/// assert_eq!(b, 4);
+#[inline]
+pub const fn bit(n: u32) -> u32 {
+    1 << n
+}