Claude review: drm/amdgpu: deduplicate register access and helper routines

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

Patch Review

**`amdgpu_common.c` — `common_sw_init()`:**

```c
int common_sw_init(struct amdgpu_ip_block *ip_block)
{
	struct amdgpu_device *adev = ip_block->adev;

	if (amdgpu_sriov_vf(adev))
		xgpu_nv_mailbox_add_irq_id(adev);

	return 0;
}
```

This is **not** a universal replacement:
- `soc15_common_sw_init()` calls `xgpu_ai_mailbox_add_irq_id()` (different function) and also has `adev->df.funcs->sw_init()` logic. The patch doesn't replace `soc15_common_sw_init()` in the diff, but the function is misleadingly named as if it were universal.
- `soc_v1_0_common_sw_init()` is just `return 0;` — it should **not** gain an `xgpu_nv_mailbox_add_irq_id()` call.

**`soc_v1_0.c` change:**
```c
 static int soc_v1_0_common_sw_init(struct amdgpu_ip_block *ip_block)
 {
-	return 0;
+	return common_sw_init(ip_block);
 }
```
This introduces a **behavioral change** on soc_v1_0 hardware — the function now calls `xgpu_nv_mailbox_add_irq_id()` in SR-IOV VF mode, which it never did before.

**`read_indexed_register()` and `program_aspm()` deduplication**: These two are genuinely identical across all call sites and the mechanical replacement is correct. However, the function names need an `amdgpu_` prefix to avoid namespace pollution.

**`amdgpu_common.h`:**
```c
#ifndef __AMDGPU_COMMON_H__
#define __AMDGPU_COMMON_H__

uint32_t read_indexed_register(struct amdgpu_device *adev,
			       u32 se_num, u32 sh_num, u32 reg_offset);
```
Missing forward declarations for `struct amdgpu_device` and `struct amdgpu_ip_block`. Add them or `#include "amdgpu.h"`.

**Recommendation**: Drop the `common_sw_init()` consolidation entirely (it's not actually a common function). Keep the `read_indexed_register` and `program_aspm` deduplication but add proper `amdgpu_` prefixes. Consider placing them in existing files rather than creating a new catch-all `amdgpu_common.c`.

---
Generated by Claude Code Patch Reviewer