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/bridge: sii902x: inline i2c_check_functionality check Date: Mon, 18 May 2026 16:02:26 +1000 Message-ID: In-Reply-To: <20260517172136.3321-2-thorsten.blum@linux.dev> References: <20260517172136.3321-2-thorsten.blum@linux.dev> <20260517172136.3321-2-thorsten.blum@linux.dev> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Verdict: Looks good.** The patch removes the intermediate `ret` assignment: ```c - ret =3D i2c_check_functionality(client->adapter, - I2C_FUNC_SMBUS_BYTE_DATA); - if (!ret) { + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA)) { ``` This is a clean improvement: 1. `i2c_check_functionality()` returns a boolean (true if the adapter suppo= rts the requested functionality), not an error code. Storing it in `ret` an= d testing `!ret` follows an error-code pattern that doesn't match the actua= l semantics. 2. The inlined form reads more naturally as a boolean predicate: "if the ad= apter doesn't support this functionality, bail out." 3. `ret` is declared at the top of the function and is still used later for= actual error-code returns, so removing this one assignment doesn't leave a= dangling unused variable. One minor note: the commit message says "the function returns a boolean sta= tus rather than an error code," which is accurate. The old code wasn't *bug= gy* =E2=80=94 `!ret` on a boolean works the same as `!ret` on a zero/nonzer= o =E2=80=94 but the refactoring improves readability. **Reviewed-by: appropriate.** --- Generated by Claude Code Patch Reviewer