[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <54F2B09C-13F1-41E8-AFA2-6AEF4492F659@collabora.com>
Date: Fri, 25 Jul 2025 13:49:00 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: Alexandre Courbot <acourbot@...dia.com>
Cc: Danilo Krummrich <dakr@...nel.org>,
David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
Beata Michalska <beata.michalska@....com>,
nouveau@...ts.freedesktop.org,
dri-devel@...ts.freedesktop.org,
rust-for-linux@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 04/19] gpu: nova-core: register: improve documentation
for basic registers
> On 18 Jul 2025, at 04:26, Alexandre Courbot <acourbot@...dia.com> wrote:
>
> Reword parts of the documentation that were a bit heavy to read, and
> harmonize/fix the examples.
>
> The relative registers section is about to be redesigned and its
> documentation rewritten, so do not touch this part.
>
> Signed-off-by: Alexandre Courbot <acourbot@...dia.com>
> ---
> drivers/gpu/nova-core/regs/macros.rs | 24 +++++++++++++-----------
> 1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/regs/macros.rs b/drivers/gpu/nova-core/regs/macros.rs
> index d015a9f8a0b01afe1ff5093991845864aa81665e..dac02f8055e76da68e9a82133fa09a1e794252bc 100644
> --- a/drivers/gpu/nova-core/regs/macros.rs
> +++ b/drivers/gpu/nova-core/regs/macros.rs
> @@ -33,25 +33,25 @@
> /// let boot0 = BOOT_0::read(&bar);
> /// pr_info!("chip revision: {}.{}", boot0.major_revision(), boot0.minor_revision());
> ///
> -/// // `Chipset::try_from` will be called with the value of the field and returns an error if the
> -/// // value is invalid.
> +/// // `Chipset::try_from` is called with the value of the `chipset` field and returns an
> +/// // error if it is invalid.
> /// let chipset = boot0.chipset()?;
> ///
> /// // Update some fields and write the value back.
> /// boot0.set_major_revision(3).set_minor_revision(10).write(&bar);
> ///
> -/// // Or just read and update the register in a single step:
> +/// // Or, just read and update the register in a single step:
> /// BOOT_0::alter(&bar, |r| r.set_major_revision(3).set_minor_revision(10));
> /// ```
> ///
> -/// Fields can be defined as follows:
> +/// Fields are defined as follows:
> ///
> -/// - `as <type>` simply returns the field value casted as the requested integer type, typically
> -/// `u32`, `u16`, `u8` or `bool`. Note that `bool` fields must have a range of 1 bit.
> +/// - `as <type>` simply returns the field value casted to <type>, typically `u32`, `u16`, `u8` or
> +/// `bool`. Note that `bool` fields must have a range of 1 bit.
> /// - `as <type> => <into_type>` calls `<into_type>`'s `From::<<type>>` implementation and returns
> /// the result.
> /// - `as <type> ?=> <try_into_type>` calls `<try_into_type>`'s `TryFrom::<<type>>` implementation
> -/// and returns the result. This is useful on fields for which not all values are value.
> +/// and returns the result. This is useful with fields for which not all values are valid.
> ///
> /// The documentation strings are optional. If present, they will be added to the type's
> /// definition, or the field getter and setter methods they are attached to.
> @@ -76,15 +76,17 @@
> /// for cases where a register's interpretation depends on the context:
> ///
> /// ```no_run
> -/// register!(SCRATCH_0 @ 0x0000100, "Scratch register 0" {
> +/// register!(SCRATCH @ 0x00000200, "Scratch register" {
> /// 31:0 value as u32, "Raw value";
> +/// });
> ///
> -/// register!(SCRATCH_0_BOOT_STATUS => SCRATCH_0, "Boot status of the firmware" {
> +/// register!(SCRATCH_BOOT_STATUS => SCRATCH, "Boot status of the firmware" {
> /// 0:0 completed as bool, "Whether the firmware has completed booting";
> +/// });
> /// ```
> ///
> -/// In this example, `SCRATCH_0_BOOT_STATUS` uses the same I/O address as `SCRATCH_0`, while also
> -/// providing its own `completed` method.
> +/// In this example, `SCRATCH_0_BOOT_STATUS` uses the same I/O address as `SCRATCH`, while also
> +/// providing its own `completed` field.
> macro_rules! register {
> // Creates a register at a fixed offset of the MMIO space.
> (
>
> --
> 2.50.1
>
Reviewed-by: Daniel Almeida <daniel.almeida@...labora.com>
Powered by blists - more mailing lists