From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: accel/qda: Add RPMsg transport for Qualcomm DSP accelerator Date: Tue, 24 Feb 2026 18:20:11 +1000 Message-ID: In-Reply-To: <20260224-qda-firstpost-v1-3-fe46a9c1a046@oss.qualcomm.com> References: <20260224-qda-firstpost-v1-0-fe46a9c1a046@oss.qualcomm.com> <20260224-qda-firstpost-v1-3-fe46a9c1a046@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 Adds `qda_dev` structure, custom logging macros, and RPMsg client driver. - **Custom logging macros** (lines 1413-1437 of mbox): Both Bjorn and Dmitr= y flagged these. The `qda_err`/`qda_info`/`qda_dbg` macros wrap `dev_err`/`= pr_err` with a fallback path and `__func__` injection. This is unnecessary = =E2=80=94 use `drm_err()` / `drm_dbg()` or plain `dev_err()`. The DRM devic= e should be the center of lifecycle management. ```c +#define qda_err(qdev, fmt, ...) do { \ + struct device *__dev =3D qda_get_log_device(qdev); \ + if (__dev) \ + dev_err(__dev, "[%s] " fmt, __func__, ##__VA_ARGS__); \ + else \ + pr_err(DRIVER_NAME ": [%s] " fmt, __func__, ##__VA_ARGS__); \ +} while (0) ``` - **`atomic_t removing` flag**: Dmitry asked why this is needed when `dev->= unplugged` and `drm_dev_enter()`/`drm_dev_exit()` exist. This is reinventin= g DRM's own unplug infrastructure. - The `qda_rpmsg_init()` function does nothing except `dev_set_drvdata()` = =E2=80=94 inline it or remove the abstraction. - `qda_init_device()` and `qda_deinit_device()` are over-abstracted wrapper= s around `mutex_init`/`mutex_destroy`. --- Generated by Claude Code Patch Reviewer