Re: [PATCH v2 08/16] drm/tyr: specify base type for registers

[email protected]
Newsgroups org.freedesktop.lists.dri-devel,org.kernel.vger.linux-pci
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete application of base parameter in register! macro invocations.
--

--- Patch [8]: [PATCH v2 08/16] drm/tyr: specify base type for registers ---
commit 53a6ffbf27038a15b41db145d107fac75047c4d1
Author: Gary Guo <[email protected]>

drm/tyr: specify base type for registers

This commit updates all register definitions to explicitly use the TyrRegisters
base type in the register! macro invocations.

> diff --git a/drivers/gpu/drm/tyr/regs.rs b/drivers/gpu/drm/tyr/regs.rs
> index a62724378ced2..3e6edae6e27f7 100644
> --- a/drivers/gpu/drm/tyr/regs.rs
> +++ b/drivers/gpu/drm/tyr/regs.rs

[ ... ]

> @@ -1353,6 +1384,8 @@ fn from(cmd: MmuCommand) -> Self {
>          }
>  
>          register! {
> +            base: TyrRegisters;
> +
>              /// MMU command register for each address space. Write only.
>              pub(crate) COMMAND(u32)[MAX_AS, stride = STRIDE] @ 0x2418 {
>                  7:0     command ?=> MmuCommand;

[Severity: Medium]
The commit message states that all registers are updated to use the new base
type, but it looks like a few register! invocations were missed during the
conversion.

For instance, earlier in this same file (drivers/gpu/drm/tyr/regs.rs), the
LOCKADDR register block doesn't have the new base parameter:

        register! {
            /// Lock region address for each address space.
            pub(crate) LOCKADDR(u64)[MAX_AS, stride = STRIDE] @ 0x2410 {
                /// Lock region size.
                5:0     size;

Additionally, the SectionFlags register in drivers/gpu/drm/tyr/fw.rs was also
missed by this patch:

    register! {
         #[allow(non_upper_case_globals)]
        pub(super) SectionFlags(u32) @ 0x0 {
            0:0 read => bool;

Were these intentionally left with the default base type, or were they 
overlooked during the refactoring?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.