Claude review: arm64: dts: imx952: Describe Mali G310 GPU

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

Patch Review

**Missing `coregroup` (APB) clock**

The imx95 GPU node specifies two clocks:
```
clocks = <&scmi_clk IMX95_CLK_GPU_CGC>, <&scmi_clk IMX95_CLK_GPUAPB>;
clock-names = "core", "coregroup";
```

But this patch only specifies one:
```c
+			clocks = <&scmi_clk IMX952_CLK_GPU>;
+			clock-names = "core";
```

The imx952-clock.h header *does* define `IMX952_CLK_GPUAPB` (value 83), so the clock exists in the SoC. The commit message says "A hardware GPU auto clock-gating mechanism has been introduced, enabling GPUMIX to automatically manage the GPU clock" — this presumably explains why the APB clock is omitted (it's auto-managed now). However, a couple of concerns:

1. It would be helpful to clarify in the commit message *which* clock is auto-managed — the APB clock specifically? Saying "the GPU clock" is ambiguous.
2. The `arm,mali-valhall-csf` binding allows 1–3 clocks, so dropping one is valid from a binding perspective. But the Panthor driver may behave differently depending on whether the "coregroup" clock is provided. If the hardware truly gates it automatically, this should be fine, but it deserves explicit mention.

Also note that `imx95` uses `IMX95_CLK_GPU_CGC` for the "core" clock, while this patch uses `IMX952_CLK_GPU`. This is presumably correct for the different SCMI clock topology on i.MX952, but worth double-checking.

**Missing `#cooling-cells`**

The imx95 GPU node has:
```
#cooling-cells = <2>;
```

This patch omits it. If the i.MX952 doesn't connect the GPU to a thermal zone as a cooling device, this is fine — but it should be noted in the commit message or cover letter. If thermal management is planned for a follow-up, adding `#cooling-cells = <2>` now would avoid a future patch that only adds this one line.

**`opp-hz-real` property**

The OPP table entries include:
```
+		opp-500000000 {
+			opp-hz = /bits/ 64 <500000000>;
+			opp-hz-real = /bits/ 64 <500000000>;
+			opp-microvolt = <920000>;
+		};
```

`opp-hz-real` is not a standard property in the OPP v2 binding (`Documentation/devicetree/bindings/opp/opp-v2-base.yaml`). However, this is consistent with the existing `imx95.dtsi` which uses the same property, and the OPP table is actually identical to the one in imx95. So this is not a new issue introduced by this patch, but it's worth noting for the record.

**Positive observations**:
- The interrupt ordering (job, mmu, gpu) correctly matches the `arm,mali-valhall-csf.yaml` binding, which was a fix from v2→v3.
- Using `"nxp,imx95-mali"` rather than `"nxp,imx952-mali"` as the compatible string is the right call since it's the same GPU IP — no need for a new compatible and binding update.
- The register address (`0x4d900000`) and size (`0x480000`) match the imx95 GPU, which is consistent with "same GPU IP."
- Node placement at the end of the `soc` bus maintains address ordering (0x4d9... > 0x4c2...).
- The OPP table is correctly placed outside the `soc` node, consistent with imx95.dtsi.

**Summary of requested changes**:
1. Please clarify in the commit message which specific clock is auto-managed by hardware (presumably the APB / `coregroup` clock), and why only one clock is needed on imx952 vs. two on imx95.
2. Consider adding `#cooling-cells = <2>` for consistency with imx95, or explain in the commit message why it's intentionally omitted.

---
Generated by Claude Code Patch Reviewer