public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH RESEND] drm/bridge: sii902x: inline i2c_check_functionality check
@ 2026-05-17 17:21 Thorsten Blum
  2026-05-17 20:59 ` Dmitry Baryshkov
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Thorsten Blum @ 2026-05-17 17:21 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
  Cc: Thorsten Blum, dri-devel, linux-kernel

Inline the i2c_check_functionality() check, since the function returns a
boolean status rather than an error code.

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 drivers/gpu/drm/bridge/sii902x.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index ea543be4ae3e..38d176d4eef2 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -1158,9 +1158,7 @@ static int sii902x_probe(struct i2c_client *client)
 	static const char * const supplies[] = {"iovcc", "cvcc12"};
 	int ret;
 
-	ret = i2c_check_functionality(client->adapter,
-				      I2C_FUNC_SMBUS_BYTE_DATA);
-	if (!ret) {
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
 		dev_err(dev, "I2C adapter not suitable\n");
 		return -EIO;
 	}

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH RESEND] drm/bridge: sii902x: inline i2c_check_functionality check
  2026-05-17 17:21 [PATCH RESEND] drm/bridge: sii902x: inline i2c_check_functionality check Thorsten Blum
@ 2026-05-17 20:59 ` Dmitry Baryshkov
  2026-05-18  6:02 ` Claude review: " Claude Code Review Bot
  2026-05-18  6:02 ` Claude Code Review Bot
  2 siblings, 0 replies; 4+ messages in thread
From: Dmitry Baryshkov @ 2026-05-17 20:59 UTC (permalink / raw)
  To: Thorsten Blum
  Cc: Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
	Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	dri-devel, linux-kernel

On Sun, May 17, 2026 at 07:21:37PM +0200, Thorsten Blum wrote:
> Inline the i2c_check_functionality() check, since the function returns a
> boolean status rather than an error code.
> 
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>  drivers/gpu/drm/bridge/sii902x.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Claude review: drm/bridge: sii902x: inline i2c_check_functionality check
  2026-05-17 17:21 [PATCH RESEND] drm/bridge: sii902x: inline i2c_check_functionality check Thorsten Blum
  2026-05-17 20:59 ` Dmitry Baryshkov
  2026-05-18  6:02 ` Claude review: " Claude Code Review Bot
@ 2026-05-18  6:02 ` Claude Code Review Bot
  2 siblings, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-05-18  6:02 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: drm/bridge: sii902x: inline i2c_check_functionality check
Author: Thorsten Blum <thorsten.blum@linux.dev>
Patches: 2
Reviewed: 2026-05-18T16:02:26.028511

---

This is a single, trivial cleanup patch. The change is correct: `i2c_check_functionality()` returns `bool` (via `__i2c_check_functionality()` which returns `u32` used as a boolean), not an error code. Storing the boolean result in `int ret` and then testing `!ret` works but is misleading — it makes it look like an error-code check pattern. Inlining the call removes the unnecessary variable assignment and makes the control flow clearer.

No functional change. No risk.

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Claude review: drm/bridge: sii902x: inline i2c_check_functionality check
  2026-05-17 17:21 [PATCH RESEND] drm/bridge: sii902x: inline i2c_check_functionality check Thorsten Blum
  2026-05-17 20:59 ` Dmitry Baryshkov
@ 2026-05-18  6:02 ` Claude Code Review Bot
  2026-05-18  6:02 ` Claude Code Review Bot
  2 siblings, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-05-18  6:02 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Verdict: Looks good.**

The patch removes the intermediate `ret` assignment:

```c
-	ret = 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 supports the requested functionality), not an error code. Storing it in `ret` and testing `!ret` follows an error-code pattern that doesn't match the actual semantics.

2. The inlined form reads more naturally as a boolean predicate: "if the adapter 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 status rather than an error code," which is accurate. The old code wasn't *buggy* — `!ret` on a boolean works the same as `!ret` on a zero/nonzero — but the refactoring improves readability.

**Reviewed-by: appropriate.**

---
Generated by Claude Code Patch Reviewer

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-05-18  6:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-17 17:21 [PATCH RESEND] drm/bridge: sii902x: inline i2c_check_functionality check Thorsten Blum
2026-05-17 20:59 ` Dmitry Baryshkov
2026-05-18  6:02 ` Claude review: " Claude Code Review Bot
2026-05-18  6:02 ` Claude Code Review Bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox