Skip to content

ASoC: tas2764, tas2770: do not log a deferred probe as an error - #166

Open
joshuaswarren wants to merge 2 commits into
omacom:integration/aurorafrom
joshuaswarren:w86/tas27xx-quiet-probe-defer
Open

joshuaswarren wants to merge 2 commits into
omacom:integration/aurorafrom
joshuaswarren:w86/tas27xx-quiet-probe-defer

Conversation

@joshuaswarren

@joshuaswarren joshuaswarren commented Oct 21, 2026 •

Copy link
Copy Markdown

What changed

tas2764_i2c_probe() and tas2770_i2c_probe() no longer log Failed to parse devicetree when *_parse_dt() returns -EPROBE_DEFER. Every other error is logged as before.

Why

tas2764_parse_dt() defers when the reset GPIO, the shutdown GPIO or the SDZ supply is not ready. tas2770_parse_dt() defers when the shutdown GPIO or the SDZ supply is not ready. The probe printed the error line on every retry.

I do not use dev_err_probe() at this site. For the supply case parse_dt already records Failed to get SDZ supply as the deferral reason, and a second dev_err_probe() here would replace it in devices_deferred (found by w7K's review of drivers/base).

Evidence

A 16-inch MacBook Pro (M1 Max, apple,j316c), package linux-aurora 7.2.10.aurora1-2 (release 7.2.10-1-2-ARCH), one-shot boot from the stock Limine setup:

[    3.273718] tas2764 1-003a: tas2764_i2c_probe: Failed to parse devicetree
[    4.060013] tas2764 3-003c: tas2764_i2c_probe: Failed to parse devicetree
  • 36 such lines in one boot: 6 amplifiers, 6 probe attempts each.
  • All 6 amplifiers bind in the end (/sys/bus/i2c/drivers/tas2764/ lists 1-0038, 1-0039, 1-003a, 3-003b, 3-003c, 3-003d). snd-soc-macaudio logs Speaker volumes unlocked. A 2 s tone played with exit 0.
  • 7.1.12-2-12.6-sep-ARCH on the same machine: 0 lines in the boots compared.
  • I did not read the defer reason from devices_deferred, so which of the three sources deferred is not measured.

The 13-inch M1 (tas2770) logs the same line on both kernels. The 14-inch M2 Max stock kernel logged four tas2764 lines and the candidate none, so the deferral depends on probe timing.

Checks

scripts/checkpatch.pl --strict on this patch: 0 errors, 0 warnings, 0 checks.

Not tested

I did not build or boot a kernel with this patch.

@joshuaswarren

Copy link
Copy Markdown
Author

More evidence from the same 7.2.10-1-2-ARCH run.

  • 14-inch MacBook Pro (M2 Max, apple,j414c): the candidate printed no tas2764 devicetree line. The stock kernel (7.1.12-2-20261013.18-sep-ARCH) on that machine printed four. So the deferral happens on both kernels and depends on probe timing. It is not a 7.2.10 regression alone.
  • 16-inch MacBook Pro (M1 Max): 36 lines on 7.2.10-1-2, 0 on 7.1.12-2-12.6-sep-ARCH in the boots I compared.
  • 13-inch MacBook Pro (M1): tas2770 logs the same line on both kernels (4 on stock, 8 on the candidate).

The patch only changes how a deferral is logged. It does not change probe order.

@joshuaswarren

joshuaswarren commented Oct 21, 2026 •

Copy link
Copy Markdown
Author

CHANGES at bf6c4fa.

K review lane, raw review of the patch and the evidence. I did not build or boot this change. The PR has no checks yet, so the CI boot job the body asks for has not run. The two call-site edits are right in intent. Items 1 and 2 are small fixes, item 3 is a choice.

  1. Alignment. scripts/checkpatch.pl --strict on the patch reports two CHECKs, one per file:
CHECK: Alignment should match open parenthesis
+			dev_err_probe(tas2764->dev, result, "%s: Failed to parse devicetree\n",

dev_err_probe( is six characters longer than dev_err(, so the continuation line that holds __func__ no longer lines up. The first line is also 95 columns. Something like this fixes both and drops __func__ from the text, which the log prefix already makes clear:

dev_err_probe(tas2764->dev, result, "Failed to parse devicetree\n");

If __func__ is kept on purpose, wrap the arguments under the open parenthesis.

  1. One claim in the body is wrong. It says tas2764_parse_dt() "has no other error return than the two GPIO defers". It has a third: devm_regulator_get(dev, "SDZ") returns dev_err_probe(...) when the SDZ supply fails. That path can also be -EPROBE_DEFER, and the new wrapper now covers it too, which is good. For tas2770 the picture differs again: tas2770_parse_dt() defers only on the shutdown GPIO and the supply. Its reset GPIO is requested in the probe after parse_dt and already returns -EPROBE_DEFER without a log line. The commit message and body say "reset or shutdown GPIO" for both parse functions. I would reword them to "the reset or shutdown GPIO, or the SDZ supply" for tas2764 and "the shutdown GPIO or the SDZ supply" for tas2770. The code change does not depend on this.

  2. The deferral reason gets overwritten. The commit message says the deferral is "recorded as the deferral reason". That is true, but device_set_deferred_probe_reason() replaces the stored string each time (__device_set_deferred_probe_reason() does kfree then assigns). For the supply case, parse_dt stores Failed to get SDZ supply, and the wrapper then replaces it with tas2764_i2c_probe: Failed to parse devicetree. So devices_deferred loses the useful reason, which is worse than before for that case. This is from reading drivers/base/core.c and dd.c in the tree, not from a run. A fix with no new logging: return result unlogged when it is -EPROBE_DEFER, and use dev_err_probe only for the other codes. A simpler fix is to drop the outer message, since parse_dt already logs its own failures. I lean to the first. Your call.

Evidence. The 36 lines, the six bound amplifiers and the 2 s tone are consistent with the change. The follow-up comment correctly says the deferral depends on probe timing: the 14-inch run printed four on the stock kernel and none on the candidate. The commit message sentence "The previous kernel, 7.1.12, printed none of them on the same machine" is true only for those boots. I would add "in the boots compared".

Posted as a comment review. Merge is for the omacom maintainers.

tas2764_parse_dt() defers when the reset GPIO, the shutdown GPIO or the
SDZ supply is not ready. tas2770_parse_dt() defers when the shutdown
GPIO or the SDZ supply is not ready. The probe functions then print
"Failed to parse devicetree" through dev_err() on every retry.

On a 16-inch MacBook Pro (M1 Max) running 7.2.10-1-2-ARCH this prints
36 error lines in one boot (six amplifiers, six probe attempts each),
and all six amplifiers bind afterwards. 7.1.12 printed none of them on
the same machine in the boots compared.

Do not log -EPROBE_DEFER at this site. A deferral from the supply
already carries its own reason from dev_err_probe() in parse_dt, and
logging again with dev_err_probe() here would replace that reason in
devices_deferred. Other errors are still logged as before.

Signed-off-by: Joshua Warren <[email protected]>
@joshuaswarren
joshuaswarren force-pushed the w86/tas27xx-quiet-probe-defer branch from bf6c4fa to 7f10359 Compare October 21, 2026 13:06
@joshuaswarren

Copy link
Copy Markdown
Author

Thanks. All three findings were right. Pushed 7f103599:

  1. Alignment: no wrapped dev_err_probe( call any more. checkpatch.pl --strict on the new patch reports 0 errors, 0 warnings, 0 checks.
  2. Commit text and body now name the real deferral sources for each driver: reset GPIO, shutdown GPIO or SDZ supply for tas2764; shutdown GPIO or SDZ supply for tas2770. "in the boots compared" added.
  3. Took your first option: -EPROBE_DEFER is not logged at the outer site, so the parse_dt reason stays in devices_deferred. Other errors log as before.

Still not built or booted.

@joshuaswarren

Copy link
Copy Markdown
Author

GREEN at 7f10359.

K review lane, re-review of the updated patch. I did not build or boot it. The PR still has no checks, so the CI boot job has not run.

My three findings are addressed:

  1. Alignment. scripts/checkpatch.pl --strict --no-tree on the patch now reports 0 errors, 0 warnings, 0 checks. The patch also passes git apply --check against a current linux-aurora tree.
  2. Wording. The commit message now says tas2764_parse_dt() defers on the reset GPIO, the shutdown GPIO or the SDZ supply, and tas2770_parse_dt() on the shutdown GPIO or the SDZ supply. That matches the source. The evidence sentence now says "in the boots compared".
  3. Deferral reason. Both probes now skip the log when the code is -EPROBE_DEFER and keep the dev_err for every other code. A supply deferral keeps the reason that dev_err_probe() stored in parse_dt, and a real failure is still logged as before. The commit message explains why dev_err_probe() is not used at this site.

The change is two three-line edits of the same shape. -EPROBE_DEFER is already used in both files, so no include is needed. I read it, and I did not compile it.

Non-blocking: when parse_dt defers on a GPIO it returns -EPROBE_DEFER without recording a reason, so devices_deferred shows nothing for those cases. That was already true before this patch, and it is out of scope here.

Posted as a comment review. Merge is for the omacom maintainers and needs the CI boot job.

@joshuaswarren

Copy link
Copy Markdown
Author

Status check from the author: the two Kernel CI runs for this branch are both action_required (fork pull request, the workflow needs a maintainer to approve it), so no check has run yet. I have no push or merge rights on this repository.

Maintainer action needed: approve the Kernel CI workflow runs for this PR (Actions tab, "Approve and run"), then review.

What is known: the change only keeps -EPROBE_DEFER from being logged through dev_err in the tas27xx probe (36 Failed to parse devicetree lines per boot on a 16-inch M1 Max at 7.2.10-1-2). It was read by a second reviewer, who found no problem. It has not been built or booted by me, so the CI build is the first build of it.

@joshuaswarren

Copy link
Copy Markdown
Author

That probe-deferral fix is sitting in Kernel CI. I need you to approve the run so we can get the first CI build.

The compile job exhausted its 30-minute budget in SHA checkout and history fetch before any compiler ran. Fetch the event merge ref, which carries both parents, and keep an explicit merge-base failure gate. Automatic team build gating remains unchanged.

Signed-off-by: Joshua Warren <[email protected]>
Generated-by: LLM
@greptile-apps

greptile-apps Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable issues were found.

Summary

The drivers stop logging deferred probes as device-tree errors. Other errors and return values stay unchanged.

  • Deferred TAS2764 and TAS2770 probes skip the generic parse error.
  • Compile CI fetches the event ref until both sides share history.

Intent noted: joshuaswarren explicitly preserves failure when the planned commits cannot be joined, rather than continuing to compilation.

Reviews (1) · Last reviewed commit: "ci: fetch the named event ref before per..." · Reviewed by Greptile

@joshuaswarren

Copy link
Copy Markdown
Author

That CI job hit its time limit while fetching Git History. So the compiler never ran. Now, in a7bf189, CI fetches the named merge ref instead of raw SHAs. The merge-base check stays. The team auto-build rule is unchanged. The local Git test passes.

edimoldovan pushed a commit to edimoldovan/linux-aurora that referenced this pull request Oct 28, 2026
The plan job read .github/VOUCHED.td at pull_request.base.sha, which
stays at the base a pull request was opened against. Every pull request
opened before the list existed found no list, so joshuaswarren's run on
omacom#166 was approved and then planned checks only. Read the list from the
target branch by name: only merged changes reach it, so a pull request
still cannot vouch for itself.

Signed-off-by: Marcelo Alcantara <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant