From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: misc: fastrpc: Fail Audio PD init when reserved memory is missing Date: Thu, 04 Jun 2026 13:11:15 +1000 Message-ID: In-Reply-To: <20260602071750.526-4-jianping.li@oss.qualcomm.com> References: <20260602071750.526-1-jianping.li@oss.qualcomm.com> <20260602071750.526-4-jianping.li@oss.qualcomm.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review **Purpose:** Add early validation that `remote_heap` exists before proceeding with static process creation. **Analysis:** This patch adds a check at the very top of `fastrpc_init_create_static_process`: ```c + if (!fl->cctx->remote_heap || + !fl->cctx->remote_heap->dma_addr || + !fl->cctx->remote_heap->size) { + err = -ENOMEM; + dev_dbg(fl->sctx->dev, "remote heap memory region is not added\n"); + return err; + } ``` **Issue 1 - Series ordering problem:** This check is added in patch 3, but at this point in the series (after patches 1-2), `remote_heap` is still allocated lazily inside `fastrpc_init_create_static_process` itself (the `if (!fl->cctx->remote_heap)` block from line 1328). So this check would cause the *first* call to `fastrpc_init_create_static_process` to always fail with `-ENOMEM`, since `remote_heap` hasn't been allocated yet. This only makes sense *after* patch 4 moves allocation to probe. The patch ordering should be swapped: patch 4 should come before patch 3, or they should be squashed. **Issue 2 - Error code:** `-ENOMEM` is not the right error code when reserved memory is missing from the DT configuration. `-ENODEV` or `-EINVAL` would be more appropriate, as this is a configuration/platform issue, not a memory allocation failure. **Issue 3:** The function also applies to SDSP, not just Audio PD. This check could break SDSP static process creation if `remote_heap` is not set up for SDSP (which it wouldn't be, since SDSP remote heap allocation works differently). Need to verify whether this function is called for SDSP contexts as well or only ADSP. **Verdict:** The ordering relative to patch 4 is problematic. As written, this would break Audio PD initialization when applied without patch 4. Additionally, the check is applied unconditionally but the cover letter only discusses Audio PD. --- --- Generated by Claude Code Patch Reviewer