From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/tyr: Use register! macro for MMU_CONTROL
Date: Sun, 12 Apr 2026 10:45:09 +1000 [thread overview]
Message-ID: <review-patch4-20260409-b4-tyr-use-register-macro-v5-v5-4-8abfff8a0204@collabora.com> (raw)
In-Reply-To: <20260409-b4-tyr-use-register-macro-v5-v5-4-8abfff8a0204@collabora.com>
Patch Review
The largest register definition patch. Defines the full MMU_CONTROL page including per-address space register arrays.
**Array register stride:**
```rust
const STRIDE: usize = 0x40;
// ...
pub(crate) TRANSTAB(u64)[MAX_AS, stride = STRIDE] @ 0x2400 {
```
The 0x40 (64 byte) stride per address space is correct per the panthor hardware layout.
**MMU_MEMATTR_STAGE1 pseudo-register:**
```rust
MMU_MEMATTR_STAGE1(u8) @ 0x0 {
0:0 alloc_w => bool;
1:1 alloc_r => bool;
3:2 alloc_sel ?=> AllocPolicySelect;
5:4 coherency ?=> Coherency;
7:6 memory_type => MemoryType;
}
```
This is defined at offset 0x0 because it's not a real MMIO register -- it's a sub-byte bitfield type used within the 64-bit `MEMATTR` register. The comment above the definition explains this clearly. The `TryFrom<Bounded<u64, 8>>` and `From<MMU_MEMATTR_STAGE1>` for `Bounded<u64, 8>` impls bridge it into the MEMATTR field types. This is a creative and clean use of the macro.
**MmuExceptionType enum:**
Comprehensive list of fault codes. The `TryFrom` match covers all defined values and returns `EINVAL` for unknown ones, which is correct since undefined exception codes from hardware should be treated as errors.
**TRANSCFG register -- bit range gap:**
```rust
pub(crate) TRANSCFG(u64)[MAX_AS, stride = STRIDE] @ 0x2430 {
3:0 mode ?=> AddressSpaceMode;
10:6 ina_bits ?=> InaBits;
18:14 outa_bits;
22:22 sl_concat_en => bool;
25:24 ptw_memattr ?=> PtwMemattr;
29:28 ptw_sh ?=> PtwShareability;
30:30 r_allocate => bool;
33:33 disable_hier_ap => bool;
34:34 disable_af_fault => bool;
35:35 wxn => bool;
36:36 xreadable => bool;
63:60 ptw_pbha;
}
```
There are gaps (bits 4-5, 11-13, 19-21, 23, 26-27, 31-32, 37-59) which are reserved/unused per the spec. The `register!` macro should handle these gaps correctly by leaving them as zero. No issues.
**`PtwShareability` -- missing value 1:**
Values are 0 (NonShareable), 2 (OuterShareable), 3 (InnerShareable). Value 1 is not defined and returns `EINVAL`. This matches the ARM shareability encoding where value 1 is reserved.
**`#[allow(clippy::enum_variant_names)]` on `PtwShareability`:**
All variants end in "Shareable" -- the clippy suppression is appropriate here since the names are descriptive.
No issues with this patch.
---
Generated by Claude Code Patch Reviewer
next prev parent reply other threads:[~2026-04-12 0:45 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-09 17:51 [PATCH v5 0/6] drm/tyr: Use register! macro Deborah Brouwer
2026-04-09 17:51 ` [PATCH v5 1/6] drm/tyr: Use register! macro for GPU_CONTROL Deborah Brouwer
2026-04-12 0:45 ` Claude review: " Claude Code Review Bot
2026-04-09 17:51 ` [PATCH v5 2/6] drm/tyr: Print GPU_ID without filtering Deborah Brouwer
2026-04-12 0:45 ` Claude review: " Claude Code Review Bot
2026-04-09 17:51 ` [PATCH v5 3/6] drm/tyr: Use register! macro for JOB_CONTROL Deborah Brouwer
2026-04-12 0:45 ` Claude review: " Claude Code Review Bot
2026-04-09 17:51 ` [PATCH v5 4/6] drm/tyr: Use register! macro for MMU_CONTROL Deborah Brouwer
2026-04-12 0:45 ` Claude Code Review Bot [this message]
2026-04-09 17:51 ` [PATCH v5 5/6] drm/tyr: Remove custom register struct Deborah Brouwer
2026-04-12 0:45 ` Claude review: " Claude Code Review Bot
2026-04-09 17:51 ` [PATCH v5 6/6] drm/tyr: Add DOORBELL_BLOCK registers Deborah Brouwer
2026-04-12 0:45 ` Claude review: " Claude Code Review Bot
2026-04-12 0:45 ` Claude review: drm/tyr: Use register! macro Claude Code Review Bot
-- strict thread matches above, loose matches on Subject: below --
2026-03-24 0:18 [PATCH v3 00/12] " Deborah Brouwer
2026-03-24 0:18 ` [PATCH v3 05/12] drm/tyr: Use register! macro for MMU_CONTROL Deborah Brouwer
2026-03-24 21:08 ` Claude review: " Claude Code Review Bot
2026-03-11 23:03 [PATCH v2 0/5] drm/tyr: Use register! macro Deborah Brouwer
2026-03-11 23:04 ` [PATCH v2 4/5] drm/tyr: Use register! macro for MMU_CONTROL Deborah Brouwer
2026-03-13 4:38 ` Claude review: " Claude Code Review Bot
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=review-patch4-20260409-b4-tyr-use-register-macro-v5-v5-4-8abfff8a0204@collabora.com \
--to=claude-review@example.com \
--cc=dri-devel-reviews@example.com \
/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