From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: fastrpc: Reduce log level for DSP info and reserved memory messages Date: Sat, 16 May 2026 11:18:47 +1000 Message-ID: In-Reply-To: <20260514062825.50172-1-jianping.li@oss.qualcomm.com> References: <20260514062825.50172-1-jianping.li@oss.qualcomm.com> <20260514062825.50172-1-jianping.li@oss.qualcomm.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Hunk 1 (line ~1804): DSP information error =E2=86=92 dev_dbg** ```c } else if (err) { dev_dbg(&cctx->rpdev->dev, "Error: dsp information is incorrect err: %d= \n", err); ``` This path is reached from `fastrpc_get_dsp_info()` (line 1836), which is an= ioctl handler =E2=80=94 so this is user-triggerable. That actually support= s the downgrade: a `dev_err` on a user-triggerable path can be used to floo= d the kernel log. However, this is also a genuine error return path (the fu= nction returns `err` to the caller), not just a "try again later" situation. - **Nit**: The message string still says `"Error: ..."` which is slightly o= dd for a `dev_dbg` message. If the intent is that this is an expected/trans= ient condition, consider rewording to something like `"failed to get dsp in= fo, err: %d\n"`. But this is cosmetic. - **Potential concern**: The commit message says this happens "temporarily = during boot" =E2=80=94 if it only happens during boot and self-resolves, `d= ev_dbg` is fine. But if the DSP never comes up and this error persists, hav= ing it at `dev_dbg` means an operator will see no indication of the problem= unless dynamic debug is enabled. A possible alternative would be `dev_info= _ratelimited()` to reduce flood while keeping visibility, but the author kn= ows their platform better than I do and `dev_dbg` is a defensible choice gi= ven the ioctl-triggerable nature. **Hunk 2 (line ~2363): Reserved DMA memory =E2=86=92 dev_dbg** ```c if (of_reserved_mem_device_init_by_idx(rdev, rdev->of_node, 0)) dev_dbg(rdev, "no reserved DMA memory for FASTRPC\n"); ``` This is in the probe path (`fastrpc_rpmsg_probe`). The original `dev_info` = is reasonable to downgrade =E2=80=94 the commit message correctly notes tha= t lack of reserved DMA memory is a valid configuration, not an error. The p= robe continues regardless of this result, so the message is purely informat= ional. `dev_dbg` is appropriate here. **Verdict**: The patch is correct and reasonable. The only suggestion is co= smetic (dropping the "Error:" prefix from the `dev_dbg` string in hunk 1). = No objections to merging. --- Generated by Claude Code Patch Reviewer