linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Boqun Feng <boqun.feng@gmail.com>
To: Alexandre Courbot <acourbot@nvidia.com>
Cc: "Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"Benno Lossin" <lossin@kernel.org>,
	"John Hubbard" <jhubbard@nvidia.com>,
	"Ben Skeggs" <bskeggs@nvidia.com>,
	"Joel Fernandes" <joelagnelf@nvidia.com>,
	"Timur Tabi" <ttabi@nvidia.com>,
	"Alistair Popple" <apopple@nvidia.com>,
	linux-kernel@vger.kernel.org, rust-for-linux@vger.kernel.org,
	nouveau@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v5 04/23] rust: add new `num` module with `PowerOfTwo` type
Date: Sat, 14 Jun 2025 10:08:28 -0700	[thread overview]
Message-ID: <aE2sjA4DxFndTZYk@Mac.home> (raw)
In-Reply-To: <DALGWEM3TD3O.95L77CD6R62S@nvidia.com>

On Fri, Jun 13, 2025 at 11:16:10PM +0900, Alexandre Courbot wrote:
[...]
> >> +                /// Aligns `self` down to `alignment`.
> >> +                ///
> >> +                /// # Examples
> >> +                ///
> >> +                /// ```
> >> +                /// use kernel::num::PowerOfTwo;
> >> +                ///
> >> +                /// assert_eq!(PowerOfTwo::<u32>::new(0x1000).align_down(0x4fff), 0x4000);
> >> +                /// ```
> >> +                #[inline(always)]
> >> +                pub const fn align_down(self, value: $t) -> $t {
> >
> > I'm late to party, but could we instead implement:
> >
> >     pub const fn round_down<i32>(value: i32, shift: i32) -> i32 {
> >         value & !((1 << shift) - 1)
> >     }
> >
> >     pub const fn round_up<i32>(value: i32, shift: i32) -> i32 {
> >         let mask = (1 << shift) - 1;
> >         value.wrapping_add(mask) & !mask
> >     }
> >
> > ? It's much harder to pass an invalid alignment with this.
> 
> It also forces you to think in terms of shifts instead of values - i.e.
> you cannot round to `0x1000` as it commonly done in the kernel, now you

Well, for const values, you can always define:

   const ROUND_SHIFT_0X1000: i32 = 12;

because `0x1000` is just a name ;-)

or we define an Alignment in term of the shift:

    pub struct Alignment {
        shift: i8,
    }

    ipml Alignment {
        pub const new(shift: i8) -> Self {
            Self { shift }
        }
    }

then

    const ALIGN_0x1000: Alignment = Alignment::new(12);

and

    pub const fn round_down_i32(value: i32, align: Alignment) -> i32 {
        ...
    }

My point was that instead of the value itself, we can always use the
shift to represent a power of two, and that would avoid troubles when we
need to check the internal representation.

That said, after some experiments by myself, I haven't found any
significant difference between shift representations vs value
representations. So no strong reason of using a shift representation.

Regards,
Boqun

> need to do some mental gymnastics to know it is actually a shift of `12`.
> Being able to use the actual value to round to is more familiar (and
> natural) to me.

  parent reply	other threads:[~2025-06-14 17:08 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-12 14:01 [PATCH v5 00/23] nova-core: run FWSEC-FRTS to perform first stage of GSP initialization Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 01/23] rust: dma: expose the count and size of CoherentAllocation Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 02/23] rust: make ETIMEDOUT error available Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 03/23] rust: sizes: add constants up to SZ_2G Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 04/23] rust: add new `num` module with `PowerOfTwo` type Alexandre Courbot
2025-06-12 15:07   ` Boqun Feng
2025-06-12 20:00     ` John Hubbard
2025-06-12 20:05       ` Boqun Feng
2025-06-12 20:08         ` John Hubbard
2025-06-12 20:12           ` Boqun Feng
2025-06-13 14:16     ` Alexandre Courbot
2025-06-13 15:25       ` Boqun Feng
2025-06-14 17:08       ` Boqun Feng [this message]
2025-06-16  5:14         ` Alexandre Courbot
2025-06-14 17:31   ` Boqun Feng
2025-06-16  5:19     ` Alexandre Courbot
2025-06-14 19:09   ` Benno Lossin
2025-06-15 13:32   ` Miguel Ojeda
2025-06-16  5:13     ` Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 05/23] rust: num: add the `fls` operation Alexandre Courbot
2025-06-14 19:16   ` Benno Lossin
2025-06-16  6:41     ` Alexandre Courbot
2025-06-18 19:24       ` Benno Lossin
2025-06-19 13:26         ` Alexandre Courbot
2025-06-19 13:28           ` Benno Lossin
2025-06-15  9:37   ` Miguel Ojeda
2025-06-15 10:51     ` Alexandre Courbot
2025-06-15 10:58       ` Alexandre Courbot
2025-06-15 13:25         ` Miguel Ojeda
2025-06-16  6:36           ` Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 06/23] gpu: nova-core: use absolute paths in register!() macro Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 07/23] gpu: nova-core: add delimiter for helper rules " Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 08/23] gpu: nova-core: expose the offset of each register as a type constant Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 09/23] gpu: nova-core: allow register aliases Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 10/23] gpu: nova-core: increase BAR0 size to 16MB Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 11/23] gpu: nova-core: add helper function to wait on condition Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 12/23] gpu: nova-core: wait for GFW_BOOT completion Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 13/23] gpu: nova-core: add DMA object struct Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 14/23] gpu: nova-core: register sysmem flush page Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 15/23] gpu: nova-core: add falcon register definitions and base code Alexandre Courbot
2025-06-17 16:33   ` Danilo Krummrich
2025-06-18  5:26     ` Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 16/23] gpu: nova-core: firmware: add ucode descriptor used by FWSEC-FRTS Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 17/23] gpu: nova-core: vbios: Add base support for VBIOS construction and iteration Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 18/23] gpu: nova-core: vbios: Add support to look up PMU table in FWSEC Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 19/23] gpu: nova-core: vbios: Add support for FWSEC ucode extraction Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 20/23] gpu: nova-core: compute layout of the FRTS region Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 21/23] gpu: nova-core: add types for patching firmware binaries Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 22/23] gpu: nova-core: extract FWSEC from BIOS and patch it to run FWSEC-FRTS Alexandre Courbot
2025-06-12 14:01 ` [PATCH v5 23/23] gpu: nova-core: load and " Alexandre Courbot
2025-06-18 20:23   ` Danilo Krummrich
2025-06-18 20:24     ` Danilo Krummrich
2025-06-19 12:35       ` Alexandre Courbot
2025-06-19 12:43         ` Danilo Krummrich
2025-06-17 20:14 ` [PATCH v5 00/23] nova-core: run FWSEC-FRTS to perform first stage of GSP initialization Danilo Krummrich
2025-06-18  8:25   ` Alexandre Courbot
2025-06-18 20:14 ` Danilo Krummrich
2025-06-19  7:14   ` Alexandre Courbot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aE2sjA4DxFndTZYk@Mac.home \
    --to=boqun.feng@gmail.com \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=airlied@gmail.com \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=apopple@nvidia.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=bskeggs@nvidia.com \
    --cc=dakr@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gary@garyguo.net \
    --cc=jhubbard@nvidia.com \
    --cc=joelagnelf@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=nouveau@lists.freedesktop.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=simona@ffwll.ch \
    --cc=tmgross@umich.edu \
    --cc=ttabi@nvidia.com \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).