Claude review: drm/ttm: Support 52-bit PAs in ttm_place

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Commit message vs. code mismatch:**

The commit message says:
> Grow these to **unsigned long** to support larger physical addresses.

But the actual diff changes the type to `uint64_t`, not `unsigned long`:

```c
-	unsigned	fpfn;
-	unsigned	lpfn;
+	uint64_t	fpfn;
+	uint64_t	lpfn;
```

The code change (`uint64_t`) is actually the better choice — `unsigned long` would only be 32 bits on 32-bit architectures, which wouldn't achieve the stated goal. The commit message should be updated to say `uint64_t` to match the code.

**Title says "52-bit" but no justification:**

The subject line says "52-bit PAs" but neither the commit message nor the code explains why 52 bits specifically. With `uint64_t` page numbers and 4KB pages, the theoretical limit is 64+12 = 76-bit addressing. The title should either say "larger PAs" generically or explain why 52-bit is the relevant target (likely ARM64's 52-bit PA extension or x86's 5-level paging).

**Missing driver-side audit:**

The patch only changes the struct definition but doesn't audit the 21+ driver files that use `fpfn`/`lpfn`. Potential issues include:

1. **Truncation on assignment:** Drivers that assign `fpfn`/`lpfn` from 32-bit variables are fine (implicit widening), but any driver storing the result of these fields back into a 32-bit variable could truncate silently.

2. **Format strings:** Any `printk`/`drm_dbg` calls that print fpfn/lpfn with `%u` or `%x` will now produce warnings or incorrect output — they need `%llu` or `PRIu64`.

3. **`struct ttm_resource.start`** is `unsigned long`, which is 32-bit on 32-bit platforms. If fpfn/lpfn values >32 bits flow into `start`, there's a truncation path. On 64-bit platforms `unsigned long` is 64-bit so this isn't an issue in practice, but it's worth noting.

4. **Structure size change:** `struct ttm_place` grows from 16 bytes to 24 bytes (with padding). Since placements are typically small static/stack arrays, this shouldn't be a performance concern, but it's worth noting for any driver that allocates large arrays of `ttm_place`.

**Verdict:**

The code change itself is correct and desirable — `uint64_t` is the right type for page frame numbers that need to exceed 32-bit range. However:

- Fix the commit message to say `uint64_t` instead of `unsigned long`.
- Consider explaining *why* 52-bit in the subject, or generalize the title.
- An audit of callers for truncation and format string issues would strengthen this patch (or could be a follow-up).

---
Generated by Claude Code Patch Reviewer