Fix dfu infinite loop - #359
Conversation
|
I thank you for the contribution. I need to test this change if it will pass our internal CI I will merge it |
| /* get i2c controller and set dfu bus clock rate */ | ||
| while (parent && i2c_parent_is_i2c_adapter(parent)) | ||
| parent = i2c_parent_is_i2c_adapter(state->client->adapter); | ||
| parent = i2c_parent_is_i2c_adapter(parent); |
There was a problem hiding this comment.
@copilot explain this change
There was a problem hiding this comment.
Sorry if my explanation wasn't clear enough. I encountered this while using a device tree with nested i2c nodes. The bug is that parent is being set to the same value (the initial value) every iteration which, by the fact of reaching that line, is not able to falsify the while condition. state->client->adapter is not updated anywhere.
The solution I found is to instead call i2c_parent_is_i2c_adapter() on parent to traverse up the tree to the root i2c adapter. There is similar logic in the kernel's own i2c-mux.c: https://github.com/torvalds/linux/blob/master/drivers/i2c/i2c-mux.c#L220
There was a problem hiding this comment.
Confirmed — parent was re-derived from state->client->adapter every iteration, so the walk never advanced and spun forever behind an I2C mux. Passing parent matches i2c-mux.c. No bug found in the change.
It will need a rebase onto dev; the surrounding DFU code has moved since February.
Auto-generated by the rs-agentic bot.
|
[rs-agentic] automated review — external contribution, bugs only. No bugs found. The fix is correct: the loop condition advanced Note this PR has been open since February and the surrounding DFU code has moved since — it will likely need a rebase onto
|
2 participants
Minor patch to fix traversing up to the parent i2c node in
ds5_dfu_device_open()andds5_dfu_device_release(). The loops were settingparentback to it's initial value, so the if the condition was true it could never become false.