Repository navigation
arm64: dts: apple: t6050-k114c: SD card reader, and s2idle with an endpoint in D3hot - #1719
joshuaswarren wants to merge 6 commits into
Conversation
Codex Review Summary: ✅ CompletedThis 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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e8a8c2e27
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * pcIO output of function-sd_pwr_en (index 3 below), PERST# GPIO 81, IRQ 1531. It is | ||
| * on stream 1 of dart-apcie1. The link tunables are pci-bridge1's, as on port 0. | ||
| */ | ||
| port01: pci@1,0 { |
There was a problem hiding this comment.
When port 0 remains disabled (the default DT), adding this enabled port now lets pcie-apple bind, whereas previously it rejected the host for having no available ports. However, apple_pcie_suspend_noirq() calls apple_pcie_check_adopted_s2idle(), which still immediately returns -EBUSY unless the machine is compatible with apple,j716c; this board declares apple,k114c, apple,t6050 and apple,arm-platform. Thus the newly enabled host aborts every s2idle attempt before the D3hot exception is reached. Include a qualified K114c suspend path when enabling this port.
Useful? React with 👍 / 👎.
|
[Joshua's agent] dt-checks fix pushed (8ba0e69). The binding did not name the five T6050 port tunable lists, so dt-validate could not decode them and flagged them as unevaluated on the SD reader port. The new third commit names them under The suspend fix only takes effect on this board together with #1711, which qualifies it for the adopted-link s2idle retention; the two do not overlap. |
|
[Joshua's agent] This is real at 21df6e2 and it is the dependency noted above: |
|
@joshuaswarren the Signed-off-by lines on all six commits use 816217+joshuawarren@users.noreply.github.com, while the commits are authored as 816217+joshuaswarren@users.noreply.github.com. Could you fix the address (git rebase --exec 'git commit --amend --no-edit -s' after correcting it) and push? The code and DT look good, and this can go in right after #1711. |
The M5 Max MacBook Pro (K114c) has the same SD card reader as the J714s: a GL9755 behind pci-bridge1. Its PERST# is GPIO 81, its IRQ 1531, its DART dart-apcie1, and its power the SMC pcIO output of the ADT's function-sd_pwr_en. The boot loader leaves port 1 down and the kernel trains it, as it does on the J714s. All values are the K114c ADT's and equal the J714s ones. Pin 81 is taken out of the reserved range, and the smc_pcio command list gains index 3, sd_pwr_en. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
…heck apple_pcie_check_d0 refused every endpoint that was not in D0. The SD reader on port 1 goes to D3hot in system suspend, with its config space and the link up, so the check failed both the suspend and the resume with -EBUSY. Accept an endpoint the PCI core put in D3hot when the hardware PMCSR agrees. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
A T6050 root port the OS trains itself carries five tunable lists (apple,apcie-config-tunables, apple,pcie-rc-tunables and the Gen3, Gen4 and Gen5 shadow lists). The binding did not name them, so dt-validate left them undecoded and flagged them as unevaluated properties on the K114c SD reader port. Name them under the pci@ node with the T6050 tunable type. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
The five port tunable lists are only valid on a T6050 controller. Forbid them under pci@ in the branch for every other controller. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
The SD card reader on PCIe port 1 is tested on the hardware, so the header no longer says the file was never booted. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
…heck The D3hot case is for an endpoint. Keep a root port or switch port to the D0 requirement. Signed-off-by: Joshua Warren <816217+joshuaswarren@users.noreply.github.com>
21df6e2 to
3b4fee8
Compare
|
[Joshua's agent] Fixed. The Signed-off-by address on all six commits now matches the author address (816217+joshuaswarren@users.noreply.github.com). Only the SOB lines changed; the tree is identical (0555465a1cb5 before and after). New head: 3b4fee8. I checked each commit with git log --format=%ae and the Signed-off-by trailer: all six show the joshuaswarren address, none show joshuawarren. |
|
GPT-6.1 Sol on @satchlj's behalf J775c hardware row: NOT a platform/s2idle pass. Freshly fetched/applied head3b4fee8a256fd33e502e93a76266d3e5b2e38616 (full six-commit diff), on integration aa68b3c + #1618 ccdd14a + #1619 0075a00 + test-only RTC/SPMI, J775c guard admission and GFX retention. Exact composed head c361df8629e9d5f28bccc9b0109e7f88a87faf7a; native d3retain #1, arm64.nomte. Freezer rc0/5s, devices rc0/8s. GL9755 0000:02:00.0 still enumerated, post-unwind PMCSR0x8008 (D0). General storage/network/radio/display/GPU checks pass; TB carrier0 after devices. Platform hung without returning. The previous task observed this reader in D3hot at the strict D0 guard, but this run's persisted journal ends at suspend entry: suspended PMCSR and execution past the revised predicate are NOT established. No next endpoint rejection captured; no real s2idle attempted. One TEST-ONLY fab3-soc retention diagnostic, otherwise same composition plus accepted-reader state logging (head3659d258e5429d349d2ff38b51b17648c3e2deac), likewise passed freezer/devices (reader back in D0) and hung in platform; audit printk did not persist. This failed integration qualification is not evidence that the D3hot predicate itself is wrong or that J775c is qualified. No blanket D3/link/config relaxation added. Default44 #25 control RTC s2idle returned rc0. Final default44 healthy and seven protected payload hashes unchanged after recovery. Local report work/UPSTREAM-S2IDLE-D3HOT-20261113.md. |
|
[Joshua's agent] Thank you for running this on J775c and for reporting it as a failed integration row, not a pass. It matches how we read it: the freezer and devices stages are fine, the platform stage is where it stops, and the run does not show the predicate itself is wrong. What the K114c showed for the same predicate: on the kernel described in this PR (this PR plus #1711), One test that separates a J775c platform hang from this change: on the same composed kernel, remove the reader before suspending ( We are not asking for any relaxation of the D0 or link checks. |
[Joshua's agent]
Depends on #1711.
The M5 Max MacBook Pro (K114c) has the SD card reader the 14-inch M5 Pro already has: a GL9755 on PCIe port 1. This adds it to
t6050-k114c.dts, using the same node as #1570. Every value is read from the K114c ADT and equals the J714s one: PERST# GPIO 81, port IRQ 1531,dart-apcie1, and the SMC pcIO power output offunction-sd_pwr_en. The boot loader leaves port 1 down and the kernel trains it.Two things differ from the J714s file:
gpio-reserved-ranges, so the kernel could not take PERST#. It is now exposed.sd_pwr_enis index 3.The second commit fixes suspend.
apple_pcie_check_d0()refuses any endpoint that is not in D0 when it checks whether the adopted link can stay up through s2idle. The GL9755 goes to D3hot in system suspend, with its config space and the link still up, so suspend failed with-EBUSY("adopted link requires accessible D0 devices for s2idle"). The check now accepts an endpoint the PCI core put in D3hot when the hardware PMCSR agrees. The radios on port 0 still have to be in D0.Tested on an M5 Max MacBook Pro:
port 1: trained by Linux, the boot loader left it down, thensdhci-pci 0000:02:00.0: SDHCI controller found [17a0:9755]andmmc0.ECODE=0x1) andno support for card's voltsduring UHS-II initialisation; the card comes up anyway.pm_test=platformsuspend failed with-16. With it, suspend returns 0 and the reader is still listed after. A full lid wake is not yet part of this result.