From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: firmware: qcom: Add a PAS TEE service
Date: Mon, 09 Mar 2026 08:59:08 +1000
Message-ID: <review-patch4-20260306105027.290375-5-sumit.garg@kernel.org>
In-Reply-To: <20260306105027.290375-5-sumit.garg@kernel.org>
References: <20260306105027.290375-1-sumit.garg@kernel.org> <20260306105027.290375-5-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

Patch Review

**Copy-paste error in `qcom_pas_tee_set_remote_state`.** The error message says "PAS shutdown failed" but should say "PAS set remote state failed":

```c
static int qcom_pas_tee_set_remote_state(struct device *dev, u32 state,
					 u32 pas_id)
{
	...
	if (ret < 0 || inv_arg.ret != 0) {
		dev_err(dev, "PAS shutdown failed, pas_id: %d, err: %x\n",
			pas_id, inv_arg.ret);
```

**`qcom_pas_tee_get_rsc_table` has confusing two-phase call logic.** The first `tee_client_invoke_func` call is made with `param[1].u.memref.size = input_rt_size` but no `.shm` set (it's zero-initialized). This appears to be a size query. If `param[1].u.memref.size` comes back non-zero, a second call is made with actual shared memory. But:
- If the TEE framework requires a valid shm for `TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT`, passing a NULL shm could be problematic on some implementations.
- On the second call, `input_rt` data is copied into `rt_shm_va` and sent, but the output is read back from the same buffer. This is correct for INOUT semantics but the size returned (`param[1].u.memref.size`) may differ from what was allocated.

**`DEFINE_FREE(shm_free, ...)` is defined at file scope** but only used in one function. This is fine but could be confusing if another file defines the same cleanup name.

**Error code swallowing.** Many TEE call failures return `-EINVAL` regardless of the actual error. The `inv_arg.ret` value (which contains the TEE-specific error) is logged but lost. Consider mapping TEE error codes to appropriate Linux error codes or at least returning `-EIO` for transport failures vs `-EINVAL` for parameter errors.

---
Generated by Claude Code Patch Reviewer