public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH]  drm/bridge: waveshare-dsi: Fix signedness bug
@ 2026-03-07  3:32 Ethan Tidmore
  2026-03-07  3:44 ` Marek Vasut
  0 siblings, 1 reply; 6+ messages in thread
From: Ethan Tidmore @ 2026-03-07  3:32 UTC (permalink / raw)
  To: Andrzej Hajda, Neil Armstrong, Robert Foss, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	dri-devel, linux-kernel
  Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
	Marek Vasut, Joseph Guo, Ethan Tidmore, kernel test robot

The function drm_of_get_data_lanes_count_ep() returns negative error
codes and dsi->lanes is an unsigned integer, so the check (dsi->lanes <
0) is always impossible.

Make the return value of drm_of_get_data_lanes_count_ep() be assigned to
ret, check for error, and then assign dsi->lanes to ret.

Detected by Smatch:
drivers/gpu/drm/bridge/waveshare-dsi.c:70 ws_bridge_attach_dsi() warn:
unsigned 'dsi->lanes' is never less than zero.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202603060341.hNj0pl9L-lkp@intel.com/
Fixes: fca11428425e9 ("drm/bridge: waveshare-dsi: Add support for 1..4 DSI data lanes")
Signed-off-by: Ethan Tidmore <ethantidmore06@gmail.com>
---
 drivers/gpu/drm/bridge/waveshare-dsi.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/waveshare-dsi.c b/drivers/gpu/drm/bridge/waveshare-dsi.c
index 0497c7ecbc7a..32d40414adb9 100644
--- a/drivers/gpu/drm/bridge/waveshare-dsi.c
+++ b/drivers/gpu/drm/bridge/waveshare-dsi.c
@@ -66,11 +66,13 @@ static int ws_bridge_attach_dsi(struct ws_bridge *ws)
 	dsi->mode_flags = MIPI_DSI_MODE_VIDEO_HSE | MIPI_DSI_MODE_VIDEO |
 			  MIPI_DSI_CLOCK_NON_CONTINUOUS;
 	dsi->format = MIPI_DSI_FMT_RGB888;
-	dsi->lanes = drm_of_get_data_lanes_count_ep(dev->of_node, 0, 0, 1, 4);
-	if (dsi->lanes < 0) {
+	ret = drm_of_get_data_lanes_count_ep(dev->of_node, 0, 0, 1, 4);
+	if (ret < 0) {
 		dev_warn(dev, "Invalid or missing DSI lane count %d, falling back to 2 lanes\n",
-			 dsi->lanes);
+			 ret);
 		dsi->lanes = 2;	/* Old DT backward compatibility */
+	} else {
+		dsi->lanes = ret;
 	}
 
 	ret = devm_mipi_dsi_attach(dev, dsi);
-- 
2.53.0


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

* Re: [PATCH] drm/bridge: waveshare-dsi: Fix signedness bug
  2026-03-07  3:32 [PATCH] drm/bridge: waveshare-dsi: Fix signedness bug Ethan Tidmore
@ 2026-03-07  3:44 ` Marek Vasut
  2026-03-07  6:42   ` Ethan Tidmore
                     ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Marek Vasut @ 2026-03-07  3:44 UTC (permalink / raw)
  To: Ethan Tidmore, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, dri-devel, linux-kernel
  Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
	Marek Vasut, Joseph Guo, kernel test robot

On 3/7/26 4:32 AM, Ethan Tidmore wrote:
> The function drm_of_get_data_lanes_count_ep() returns negative error
> codes and dsi->lanes is an unsigned integer, so the check (dsi->lanes <
> 0) is always impossible.
> 
> Make the return value of drm_of_get_data_lanes_count_ep() be assigned to
> ret, check for error, and then assign dsi->lanes to ret.
> 
> Detected by Smatch:
> drivers/gpu/drm/bridge/waveshare-dsi.c:70 ws_bridge_attach_dsi() warn:
> unsigned 'dsi->lanes' is never less than zero.
I already sent:

[PATCH] drm/bridge: waveshare-dsi: Use temporary signed variable for DSI 
lanes validation

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

* Re: [PATCH] drm/bridge: waveshare-dsi: Fix signedness bug
  2026-03-07  3:44 ` Marek Vasut
@ 2026-03-07  6:42   ` Ethan Tidmore
  2026-03-07 15:57     ` Marek Vasut
  2026-03-08 22:10   ` Claude review: " Claude Code Review Bot
  2026-03-08 22:10   ` Claude Code Review Bot
  2 siblings, 1 reply; 6+ messages in thread
From: Ethan Tidmore @ 2026-03-07  6:42 UTC (permalink / raw)
  To: Marek Vasut, Ethan Tidmore, Andrzej Hajda, Neil Armstrong,
	Robert Foss, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Simona Vetter, dri-devel, linux-kernel
  Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
	Marek Vasut, Joseph Guo, kernel test robot

On Fri Mar 6, 2026 at 9:44 PM CST, Marek Vasut wrote:
> On 3/7/26 4:32 AM, Ethan Tidmore wrote:

> I already sent:
>
> [PATCH] drm/bridge: waveshare-dsi: Use temporary signed variable for DSI 
> lanes validation

Sorry about that! I glanced at lore looking for the kernel test robot,
should have looked for patches already for this.

Thanks,

ET

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

* Re: [PATCH] drm/bridge: waveshare-dsi: Fix signedness bug
  2026-03-07  6:42   ` Ethan Tidmore
@ 2026-03-07 15:57     ` Marek Vasut
  0 siblings, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2026-03-07 15:57 UTC (permalink / raw)
  To: Ethan Tidmore, Andrzej Hajda, Neil Armstrong, Robert Foss,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
	Simona Vetter, dri-devel, linux-kernel
  Cc: Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
	Marek Vasut, Joseph Guo, kernel test robot

On 3/7/26 7:42 AM, Ethan Tidmore wrote:
> On Fri Mar 6, 2026 at 9:44 PM CST, Marek Vasut wrote:
>> On 3/7/26 4:32 AM, Ethan Tidmore wrote:
> 
>> I already sent:
>>
>> [PATCH] drm/bridge: waveshare-dsi: Use temporary signed variable for DSI
>> lanes validation
> 
> Sorry about that! I glanced at lore looking for the kernel test robot,
> should have looked for patches already for this.
Actually, looking at it one more time, the patch I sent is missing the 
update of dev_warn() , so this patch is the better fix:

Reviewed-by: Marek Vasut <marek.vasut+renesas@mailbox.org>

Thank you !

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

* Claude review: Re: [PATCH] drm/bridge: waveshare-dsi: Fix signedness bug
  2026-03-07  3:44 ` Marek Vasut
  2026-03-07  6:42   ` Ethan Tidmore
@ 2026-03-08 22:10   ` Claude Code Review Bot
  2026-03-08 22:10   ` Claude Code Review Bot
  2 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-03-08 22:10 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: Re: [PATCH] drm/bridge: waveshare-dsi: Fix signedness bug
Author: Marek Vasut <marek.vasut@mailbox.org>
Patches: 4
Reviewed: 2026-03-09T08:10:07.807754

---

This is a single-patch fix for a real signedness bug in the waveshare-dsi bridge driver. The function `drm_of_get_data_lanes_count_ep()` returns a signed `int` (negative on error), but its return value was being stored directly into `dsi->lanes`, which is `unsigned int`. This made the `if (dsi->lanes < 0)` check always false, meaning errors would silently result in a garbage lane count instead of the intended fallback to 2 lanes.

The fix is correct and straightforward. The existing `ret` variable (already declared as `int` at line 50) is reused to capture the return value, the error check is done on `ret`, and the assignment to `dsi->lanes` only happens on the success path.

**Verdict: Good patch, ready to apply.**

---
Generated by Claude Code Patch Reviewer

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

* Claude review: Re: [PATCH] drm/bridge: waveshare-dsi: Fix signedness bug
  2026-03-07  3:44 ` Marek Vasut
  2026-03-07  6:42   ` Ethan Tidmore
  2026-03-08 22:10   ` Claude review: " Claude Code Review Bot
@ 2026-03-08 22:10   ` Claude Code Review Bot
  2 siblings, 0 replies; 6+ messages in thread
From: Claude Code Review Bot @ 2026-03-08 22:10 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Correctness:** The fix is correct. By storing the return value in the already-declared `int ret` variable, the negative error check works properly. The `else` branch assigns `dsi->lanes = ret` only when `ret >= 0`, which is safe since lane counts 1–4 fit in an unsigned int.

**Commit message:** Well-written. It clearly describes the bug, the fix, includes the Smatch warning output, and has appropriate `Reported-by`, `Closes`, and `Fixes` tags.

**Minor nit:** The `dev_warn` format string uses `%d` for `ret`, which is correct (signed int). The original code used `%d` for `dsi->lanes` (unsigned), which was technically a format mismatch — so the patch also incidentally fixes that.

**Code:**
```c
-	dsi->lanes = drm_of_get_data_lanes_count_ep(dev->of_node, 0, 0, 1, 4);
-	if (dsi->lanes < 0) {
+	ret = drm_of_get_data_lanes_count_ep(dev->of_node, 0, 0, 1, 4);
+	if (ret < 0) {
 		dev_warn(dev, "Invalid or missing DSI lane count %d, falling back to 2 lanes\n",
-			 dsi->lanes);
+			 ret);
 		dsi->lanes = 2;	/* Old DT backward compatibility */
+	} else {
+		dsi->lanes = ret;
 	}
```

No issues found. The patch is minimal, targeted, and correct.

---
Generated by Claude Code Patch Reviewer

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

end of thread, other threads:[~2026-03-08 22:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-07  3:32 [PATCH] drm/bridge: waveshare-dsi: Fix signedness bug Ethan Tidmore
2026-03-07  3:44 ` Marek Vasut
2026-03-07  6:42   ` Ethan Tidmore
2026-03-07 15:57     ` Marek Vasut
2026-03-08 22:10   ` Claude review: " Claude Code Review Bot
2026-03-08 22:10   ` 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