From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/xe/i2c: use platform_device_add_software_node() Date: Sat, 16 May 2026 13:28:30 +1000 Message-ID: In-Reply-To: <20260512-swnode-remove-on-dev-unreg-v5-4-0035eff63812@oss.qualcomm.com> References: <20260512-swnode-remove-on-dev-unreg-v5-0-0035eff63812@oss.qualcomm.com> <20260512-swnode-remove-on-dev-unreg-v5-4-0035eff63812@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 The static software node definition is clean: ```c static const struct software_node xe_i2c_adapter_swnode = { .properties = xe_i2c_adapter_properties, }; ``` And the call to `platform_device_add_software_node()` is correct. **Bug 1: Uninitialized variable `fwnode` assigned to `i2c->adapter_node`.** The patch removes the assignment `fwnode = fwnode_create_software_node(...)` but leaves both the declaration `struct fwnode_handle *fwnode;` and the line: ```c i2c->adapter_node = fwnode; ``` After the patch, `fwnode` is declared but never assigned, so `i2c->adapter_node` receives an uninitialized pointer. The compiler should warn about this. **Bug 2: `xe_i2c_unregister_adapter()` is not updated.** The current unregister function: ```c static void xe_i2c_unregister_adapter(struct xe_i2c *i2c) { platform_device_unregister(i2c->pdev); fwnode_remove_software_node(i2c->adapter_node); } ``` After patch 5 adds `device_remove_software_node()` to the release callback, the swnode is freed during `platform_device_unregister()`. The subsequent `fwnode_remove_software_node(i2c->adapter_node)` would be a use-after-free (compounded by the uninitialized pointer from bug 1). The fix should: 1. Remove the `struct fwnode_handle *fwnode;` declaration (now unused). 2. Remove the `i2c->adapter_node = fwnode;` line. 3. Update `xe_i2c_unregister_adapter()` to remove the `fwnode_remove_software_node()` call, since the release callback now handles cleanup. If `i2c->adapter_node` is used elsewhere, it should be set to `software_node_fwnode(&xe_i2c_adapter_swnode)` instead. --- Generated by Claude Code Patch Reviewer