Repository navigation
arm64: dts: apple: t6050-j714c: Run the PMP and enable the right USB-C port - #1391
Conversation
Enable the PMP on the M5 Max 14-inch the way the J714s does since #1376: the PMP with its boot arguments, mailbox, DART and power states, the PMP report, the PMC's agent 0 and the GPU's request in the report. They stay enabled together, so the GPU never waits on a disabled report. The CLPC keeps its vote; the PMC takes the higher of the two. The values come from ADTs of the same silicon, not from a J714c unit: - apple,board-id is 10, the J714c's board ID (0x0a; the N1 platform ID above has it too). - apple,dram-vendor-id is 255 and pmc/pmc-pmgr are 1/63, as a J716c reports them. They are only fallbacks: the boot loader copies /chosen and the PMGR's values over them. - t6050-max-pmp-tunables.dtsi is the 173 iop-pmp0-nub properties of a J716cAP 27.0 ADT, raw bytes, in the format of the J714s file. The J714cAP and J716cAP firmware ADTs have identical PMP nodes (props and all 29 variant sub-nodes), and the h17c-family variant tables of that firmware ADT reproduce 166 of the 173 values; the other seven (the AGX DVFS voltages, the segment ranges and names, pre-loaded and three variant picks) are runtime values iBoot fills in. Not run on a J714c.
…max-atc2.dtsi The J714cAP and J716cAP firmware ADTs carry identical nodes for the right USB-C port: the HPM on nub-spmi-a1, ATC2's PHY, USB controller and DARTs, the USB4/Thunderbolt router and the PCIe tunnel. Move the J716c's description of them into t6050-max-atc2.dtsi so the J714c can include it instead of a second copy. The host DROM (apple,thunderbolt-drom) stays in the board file: this J716c unit's DROM carries a router UUID from an earlier boot. No change to the J716c: its DTB is byte-identical before and after, and so are the other ten T6050 board DTBs.
Codex Review Summary: ❌ FailedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The account paying for this security review has reached its Codex usage limits. The payer can check the Codex usage dashboard. For personal accounts, using credits requires enabling “Use credits for security reviews” in Code review settings. If you do not manage the paying account, contact this repository's admins. |
|
GREEN at 6327f72 Offline review of all three commits vs the integration/aurora base: built all 11 T6050 board DTBs at head and at base; the ten boards other than j714c are byte-identical to base (j716c included, through the ATC2 move), and only j714c differs, as intended. The new PMP block matches the merged J714s (#1376) pattern (same enable set; board-id 10, dram-vendor-id 255, pmc 1/63). Checked the 173 tunables against the J716cAP/J714cAP v272 firmware ADT dumps: the pmp0 ADT nodes of the two boards are byte-identical (props and all 29 variant sub-nodes), 98 tunables are byte-exact vs the iop-pmp0-nub direct properties and 64 more byte-exact vs the h17c-family variant sub-node values; the 7 remaining values are exactly the ones the commit message discloses as runtime picks (agx-dvfs-state-voltages, segment-ranges, segment-names, pre-loaded, afc-turbo-enable, controller, dvfs-domain). Non-blocking notes: local dt-schema tooling could not re-run validation (KeyError: 'uint8-array' on the raw byte-array schema types), so the reported single j714c warning (PCIe port0 in apciec2, inherited from j716c) stands as requester evidence, consistent with the diff; and afc-turbo-enable=1 is authored (nub and variant tables carry 0/nothing, J714sAP also 0) — disclosed in the commit message, overwritten by iBoot at boot. opencode-go/glm-5.3-flash as routed by Main (model id unverifiable); implementer model unverified. |
Include t6050-max-atc2.dtsi on the J714c: the right USB-C port's PD controller (nub-spmi-a1, hpm2), ATC2's PHY, USB controller and DARTs, the USB4/Thunderbolt router and the PCIe tunnel. The J714cAP firmware ADT describes them with the same values as the J716cAP one. The ADT also has the PD controllers hpm0 and hpm1 on nub-spmi-a0 and an hpm5; this commit does not describe them or the ports behind them (ATC0, ATC1, ATC3). The nodes the J714c board did not reach are now in t6050-max.dtsi's J714c-and-J716c blocks (ATC2, ACIO2, APCIEC2 and nub-spmi-a1), and t6050-peripherals.dtsi stops adding its own staged nub-spmi-a1 and apciec2_dart for the J714c. apple,thunderbolt-drom is the template value of the J714cAP ADT, not a unit's UUID; the boot loader replaces it. Not run on a J714c. Checked offline: all 11 T6050 board DTBs build with dtc, and the ten other than the J714c are byte-identical to the base tree. dt-validate of the J714c DTB has one warning, the PCIe "port0" in the apciec2 node, which the J716c DTB has already; the J714c had none before.
6327f72 to
88a7b23
Compare
|
GREEN at 88a7b23 Re-review at the new head: the tree is byte-identical to the previously reviewed head (empty diff, tree dda0fcf16434), only this commit's message changed. The corrected left-port wording matches the J714cAP firmware ADT: nub-spmi-a0 carries the PD controllers hpm0 (port 1, left-back), hpm1 (port 2, left-front) and hpm5, undescribed here as the message now states. No other finding; prior offline results (11 DTB builds, ten byte-identical to base; tunables provenance vs the v272 ADT dumps) carry over unchanged. opencode-go/glm-5.3-flash as routed by Main (model id unverifiable); implementer model unverified. |
|
Note anthropic/claude-sonnet-5-5 responding on behalf of joshuawarren GREEN at 88a7b23 Cross-model review. The implementer is The reviewer ran at Reviewer evidence:
Not tested: no J714c hardware boot. |
Brings the 14-inch M5 Max (J714c) up to what the J714s has with the PMP, and gives it the right USB-C port. It builds on #945 (the J714c board file) and #1376 (the J714s PMP).
Three commits, so each can be reverted alone:
t6050-j714c.dtsgets the arm64: dts: apple: t6050-j714s: Run the PMP and report the GPU to it #1376 enable block:ps_pms,ps_pmp,ps_pms_sram,pmp0_dart,pmp0_mbox,&pmp0,pmp0_report,&pmc(static voters<0 1 3>) and the GPU'spmp0_gfxrequest. Newt6050-max-pmp-tunables.dtsicarries the 173apple,tunable-*properties. If the PMP hangs a first boot, revert this commit alone.t6050-max-atc2.dtsi. The J716c keeps its own host DROM. The J716c DTB does not change.t6050-max-atc2.dtsiand sets the ADT's template DROM. The ATC2, ACIO2, APCIEC2 andnub-spmi-a1guards int6050-max.dtsiandt6050-peripherals.dtsinow take the J714c too.Where the values come from
The J714cAP and J716cAP firmware ADTs (27.0) carry identical PMP nodes (direct properties and all 29
:sotra_*variant sub-nodes) and identical ACIO, HPM and ATC nodes. A binary parse of both shows no difference in them. The 173 tunable bytes are the live J716c ADT, in the format of the J714s file. The h17c variant merge of the firmware ADT reproduces 166 of them. The other seven are runtime values iBoot fills in (AGX DVFS voltages, segment ranges and names,pre-loaded, and three variant picks).apple,board-id = <10>is the J714c's board ID.apple,dram-vendor-id = <255>andapple,pmc = <1>,apple,pmc-pmgr = <63>are what a J716c and a J714s report. The boot loader overwrites all three, so they are fallbacks.This PR describes only the right port (
nub-spmi-a1,hpm2). The ADT also has the PD controllershpm0andhpm1onnub-spmi-a0and anhpm5; they, and the ports behind them (ATC0, ATC1, ATC3), are not described here.Testing
Offline only:
d7303c39e215), the ten DTBs other than the J714c are byte-identical (cmp). J714c is the only change.dt-validateof the J714c DTB: base 0 warnings, now 1 (pcie@1c70000000:False schema does not allow ['port0']). The J716c DTB already has that warning, from the same node, and I did not change it here. The binding forapple,tunableis ambiguous at base:make CHECK_DTBS=ystops atphy/ti,cd2e22.yamlvs the Apple bindings (uint8-array vs uint32-matrix), so I randt-validatewith a schema built withoutphy/ti,cd2e22.yaml.Not tested
dram-vendor-id255 has not been read from a J714c.