From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: firmware: qcom: Add OP-TEE PAS service support Date: Tue, 05 May 2026 08:22:43 +1000 Message-ID: In-Reply-To: <20260504130603.1474043-1-sumit.garg@kernel.org> References: <20260504130603.1474043-1-sumit.garg@kernel.org> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Overall Series Review Subject: firmware: qcom: Add OP-TEE PAS service support Author: Sumit Garg Patches: 17 Reviewed: 2026-05-05T08:22:43.237523 --- This is a well-structured 16-patch series that introduces a generic Peripheral Authentication Service (PAS) abstraction layer (`qcom_pas`) to decouple Qualcomm co-processor firmware loading from the SCM-specific implementation. This enables alternative TrustZone backends such as OP-TEE via the TEE bus. The series follows a clean pattern: introduce the abstraction (patch 2), plug in the SCM backend (patch 3), add the TEE backend (patch 4), migrate all client drivers (patches 5-14), clean up legacy wrappers (patch 15), and add MAINTAINERS (patch 16). The architecture is sound and the migration is mechanical for the client drivers. I have a few concerns: 1. **Bug in patch 14 (ath12k):** A printf format string mismatch that will cause undefined behavior. 2. **Race condition in `qcom_pas_ops_register()`:** Two concurrent callers could both see `!qcom_pas_is_available()` and both store, with the second silently overwriting the first. 3. **TEE `get_rsc_table` first call with NULL shm:** The first `tee_client_invoke_func` call passes `MEMREF_INOUT` with a NULL shm pointer which may need comment/documentation. 4. **Transitional cast pattern in patch 3** is somewhat ugly but acceptable given it's cleaned up in patch 15. Overall: the series is in good shape for v5, but the ath12k printf bug must be fixed before merge. --- --- Generated by Claude Code Patch Reviewer