From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: bus: mhi: Expose DDR training data via controller sysfs Date: Wed, 11 Mar 2026 13:35:23 +1000 Message-ID: In-Reply-To: <20260310-sahara_protocol_new_v2-v3-8-994ea4b0d5ad@oss.qualcomm.com> References: <20260310-sahara_protocol_new_v2-v3-0-994ea4b0d5ad@oss.qualcomm.com> <20260310-sahara_protocol_new_v2-v3-8-994ea4b0d5ad@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 a binary sysfs attribute `ddr_training_data` on the MHI controller dev= ice. **Issues:** - **`devres_alloc` with size 1:** `devres_alloc(sahara_sysfs_devres_release= , 1, GFP_KERNEL)` allocates a 1-byte devres resource, just as a lifecycle c= ookie. While functional, `sizeof(void *)` or a named empty struct would be = more idiomatic. - **Sysfs attribute on controller device created from channel driver:** The= sysfs file is created on `mhi_dev->mhi_cntrl->mhi_dev->dev`, which is the = controller's device, not the channel device. Creating sysfs attributes on a= device owned by another subsystem is generally frowned upon =E2=80=94 it c= reates coupling between the Sahara driver and the MHI core's device lifecyc= le. - The `ddr_training_data_read` function calls `sahara_ctrl_trng_get` which = can *create* a new devres entry. A sysfs read should not allocate new devre= s resources. If no training data was captured, it should use `devres_find` = only. - Using `const struct bin_attribute` but setting `.read` instead of `.read_= new` =E2=80=94 this looks correct for the current API but worth double-chec= king against the kernel version being targeted. --- Generated by Claude Code Patch Reviewer