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/neutron: Add logging support Date: Mon, 09 Mar 2026 08:49:22 +1000 Message-ID: In-Reply-To: <20260306-neutron-v2-8-3019bd8c91ef@nxp.com> References: <20260306-neutron-v2-0-3019bd8c91ef@nxp.com> <20260306-neutron-v2-8-3019bd8c91ef@nxp.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 **`fw_log_read` not thread-safe:** ```c static ssize_t fw_log_read(struct file *f, char __user *buf, size_t count, = loff_t *pos) { ... if (ndev->flags & NEUTRON_BOOTED) neutron_read_log(ndev, count); return simple_read_from_buffer(buf, count, pos, ndev->log.buf, ndev->log.buf_count); } ``` Multiple concurrent readers could race on `ndev->log.buf` / `ndev->log.buf_= count`. `neutron_read_log` unconditionally resets `buf_count =3D 0` at the = start, so two concurrent reads would produce garbled results. Needs a mutex. **`neutron_init_logging` uses `devm_kfree`/`devm_kmalloc` for resizable buf= fer:** Using devres for buffers that may be freed/reallocated is fragile. If the f= irmware changes log size across reboots, the old buffer is freed and a new = one allocated =E2=80=94 this works but is an unusual pattern for devres. A = plain `kfree`/`kmalloc` with explicit cleanup in remove would be cleaner. **`debugfs_init` called before `drm_dev_register`:** In the probe path, `ne= utron_debugfs_init` is called before the device is fully registered. The `d= ebugfs_root` should be available after `devm_drm_dev_alloc` though, so this= is likely fine. --- Generated by Claude Code Patch Reviewer