Re: Re: [RFC PATCH] usb: typec: ucsi: retry init when the PPM answers early commands incorrectly
huangwei <[email protected]>
| Newsgroups | org.kernel.vger.linux-usb |
|---|---|
| Organization | 麒麟软件有限公司 |
| Message-ID | <[email protected]> |
Hi Jacob, Thanks - that resolves my concern. I had missed that the "possible UCSI driver bug" dev_err in ucsi_read_error() is untouched by your patch, so first-occurrence visibility is preserved and the bounded retry only delays the final verdict by ~10s. Restricting the retry to the ucsi_init_work() path is a reasonable discriminator, given the error codes really are identical between the two cases. No further comments from my side. Good luck with the RFC. Best regards, Huang Wei 于 2026-08-17 12:37, Jacob Riff 写道: On Mon, 17 Aug 2026 11:19:24 +0800, huangwei wrote: > There the commands do complete, but the EC takes longer than the 5s > hardcoded wait in ucsi_sync_control_common(). The fix for that case is > already reviewed [1] (thanks Heikki, Fedor), so both failure modes of > the "PPM not ready during boot" window would be covered: > > - slow answers -> longer completion timeout [1] > - wrong answers -> init retry (this RFC) Hi Huang Wei, Thanks for taking a look, and good to see the two failure modes of the boot window end up with complementary fixes. > One small question on the -EINVAL retry: ucsi_read_error() also > returns -EINVAL for UCSI_ERROR_INVALID_CON_NUM / UNREGONIZED_CMD / > INVALID_CMD_ARGUMENT, which are logged as "possible UCSI driver bug". > Retrying those is harmless in practice (debug-level logging, bounded > attempts), but it delays the report of a genuine driver bug from the > first occurrence to the 100th. Did you consider distinguishing the > PPM-not-ready case from the real error case, or is the simplicity of > retrying both worth that trade-off? I considered it, but by error code the two cases are identical. On this machine the not-ready PPM answers a valid standard init command with UNREGONIZED_CMD / INVALID_CMD_ARGUMENT - exactly the codes ucsi_read_error() maps to "possible UCSI driver bug". So the only discriminators left are context and persistence, which is what the patch relies on: the retry exists only in the ucsi_init_work() path (a runtime -EINVAL is unaffected), the not-ready window has cleared after a single retry in every boot observed here, and a genuine bug fails deterministically, exhausts the bounded attempts (100 x 100ms = 10s with the role switch constants) and still ends in the loud "PPM init failed, stop trying". On delaying the report: the "possible UCSI driver bug" message is dev_err in ucsi_read_error() and this patch does not touch it, so it still fires on the first occurrence. With the retries a deterministic bug would print it on every attempt before the final error, so the cost is a 10 second delay of the final verdict rather than a hidden report. That seemed a fair price for reusing the existing role switch retry machinery unchanged. Best regards, Jacob 2026-08-17 公司:麒麟软件有限公司 姓名:黄伟 (Daway.Huang) 地址:上海市徐汇区番禺路1028号数娱大厦12楼 邮编:200030 电话:021-51098866-6033 手机:13814996575 Email:[email protected]