public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] staging: fbtft: replace -1 with proper error codes
@ 2026-02-28 20:27 Soham Kute
  2026-03-03  4:07 ` Claude review: " Claude Code Review Bot
  2026-03-03  4:07 ` Claude Code Review Bot
  0 siblings, 2 replies; 3+ messages in thread
From: Soham Kute @ 2026-02-28 20:27 UTC (permalink / raw)
  To: andy, gregkh
  Cc: dri-devel, linux-fbdev, linux-staging, linux-kernel, Soham Kute

Replace return -1 with proper kernel error codes:
- -ENODEV when SPI device is NULL
- -EINVAL when display size or buffer is invalid
- -EOPNOTSUPP for unimplemented functions

Signed-off-by: Soham Kute <officialsohamkute@gmail.com>
---
 drivers/staging/fbtft/fb_ra8875.c | 4 ++--
 drivers/staging/fbtft/fbtft-bus.c | 4 ++--
 drivers/staging/fbtft/fbtft-io.c  | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/fbtft/fb_ra8875.c b/drivers/staging/fbtft/fb_ra8875.c
index 0ab1de6647d0..c2e5c6276415 100644
--- a/drivers/staging/fbtft/fb_ra8875.c
+++ b/drivers/staging/fbtft/fb_ra8875.c
@@ -29,7 +29,7 @@ static int write_spi(struct fbtft_par *par, void *buf, size_t len)
 	if (!par->spi) {
 		dev_err(par->info->device,
 			"%s: par->spi is unexpectedly NULL\n", __func__);
-		return -1;
+		return -ENODEV;
 	}
 
 	spi_message_init(&m);
@@ -144,7 +144,7 @@ static int init_display(struct fbtft_par *par)
 		write_reg(par, 0x1F, 0x01);
 	} else {
 		dev_err(par->info->device, "display size is not supported!!");
-		return -1;
+		return -EINVAL;
 	}
 
 	/* PWM clock */
diff --git a/drivers/staging/fbtft/fbtft-bus.c b/drivers/staging/fbtft/fbtft-bus.c
index 30e436ff19e4..9a87bddd7d19 100644
--- a/drivers/staging/fbtft/fbtft-bus.c
+++ b/drivers/staging/fbtft/fbtft-bus.c
@@ -181,7 +181,7 @@ int fbtft_write_vmem16_bus9(struct fbtft_par *par, size_t offset, size_t len)
 
 	if (!par->txbuf.buf) {
 		dev_err(par->info->device, "%s: txbuf.buf is NULL\n", __func__);
-		return -1;
+		return -EINVAL;
 	}
 
 	remain = len;
@@ -217,7 +217,7 @@ EXPORT_SYMBOL(fbtft_write_vmem16_bus9);
 int fbtft_write_vmem8_bus8(struct fbtft_par *par, size_t offset, size_t len)
 {
 	dev_err(par->info->device, "%s: function not implemented\n", __func__);
-	return -1;
+	return -EOPNOTSUPP;
 }
 EXPORT_SYMBOL(fbtft_write_vmem8_bus8);
 
diff --git a/drivers/staging/fbtft/fbtft-io.c b/drivers/staging/fbtft/fbtft-io.c
index de1904a443c2..2144f5257fa9 100644
--- a/drivers/staging/fbtft/fbtft-io.c
+++ b/drivers/staging/fbtft/fbtft-io.c
@@ -19,7 +19,7 @@ int fbtft_write_spi(struct fbtft_par *par, void *buf, size_t len)
 	if (!par->spi) {
 		dev_err(par->info->device,
 			"%s: par->spi is unexpectedly NULL\n", __func__);
-		return -1;
+		return -ENODEV;
 	}
 
 	spi_message_init(&m);
@@ -231,6 +231,6 @@ EXPORT_SYMBOL(fbtft_write_gpio16_wr);
 int fbtft_write_gpio16_wr_latched(struct fbtft_par *par, void *buf, size_t len)
 {
 	dev_err(par->info->device, "%s: function not implemented\n", __func__);
-	return -1;
+	return -EOPNOTSUPP;
 }
 EXPORT_SYMBOL(fbtft_write_gpio16_wr_latched);
-- 
2.34.1


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

* Claude review: staging: fbtft: replace -1 with proper error codes
  2026-02-28 20:27 [PATCH] staging: fbtft: replace -1 with proper error codes Soham Kute
@ 2026-03-03  4:07 ` Claude Code Review Bot
  2026-03-03  4:07 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-03  4:07 UTC (permalink / raw)
  To: dri-devel-reviews

Overall Series Review

Subject: staging: fbtft: replace -1 with proper error codes
Author: Soham Kute <officialsohamkute@gmail.com>
Patches: 1
Reviewed: 2026-03-03T14:07:12.200424

---

This is a straightforward single-patch cleanup that replaces bare `return -1` with proper kernel error codes across three files in `drivers/staging/fbtft/`. The change is correct in principle — returning `-1` (which maps to `-EPERM`) from these functions is incorrect and misleading. The chosen error codes are mostly reasonable and well-matched to the failure conditions. This is typical staging cleanup work.

One minor quibble with error code selection (see below), but overall this is a clean, correct patch.

---
Generated by Claude Code Patch Reviewer

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

* Claude review: staging: fbtft: replace -1 with proper error codes
  2026-02-28 20:27 [PATCH] staging: fbtft: replace -1 with proper error codes Soham Kute
  2026-03-03  4:07 ` Claude review: " Claude Code Review Bot
@ 2026-03-03  4:07 ` Claude Code Review Bot
  1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-03  4:07 UTC (permalink / raw)
  To: dri-devel-reviews

Patch Review

**Positive aspects:**
- The commit message is clear and enumerates each error code choice with its rationale.
- The Signed-off-by is present.
- The diff is minimal and focused — no unrelated changes.

**Error code review:**

1. **`fb_ra8875.c:write_spi()` — `-1` → `-ENODEV`**: Correct. The SPI device being NULL means the device is not available.

2. **`fb_ra8875.c:init_display()` — `-1` → `-EINVAL`**: Correct. An unsupported display resolution is an invalid parameter/configuration.

3. **`fbtft-io.c:fbtft_write_spi()` — `-1` → `-ENODEV`**: Correct. Same SPI NULL case as above, consistent choice.

4. **`fbtft-io.c:fbtft_write_gpio16_wr_latched()` — `-1` → `-EOPNOTSUPP`**: Correct. Function is a stub, `-EOPNOTSUPP` is the right code.

5. **`fbtft-bus.c:fbtft_write_vmem8_bus8()` — `-1` → `-EOPNOTSUPP`**: Correct. Same unimplemented stub pattern.

6. **`fbtft-bus.c:fbtft_write_vmem16_bus9()` — `-1` → `-EINVAL`** for `txbuf.buf` being NULL: This is **debatable**. Looking at the code, `txbuf.buf` is allocated via `kzalloc()` during probe in `fbtft-core.c:670`. If it's NULL here at runtime, it's not because the caller passed an invalid argument — it's because the buffer was never allocated (either `txbuflen` was 0, or allocation failed and was silently handled). `-ENOMEM` might be slightly more accurate, but one could also argue this is an internal state error best expressed as `-EINVAL`. It's a minor point and not worth blocking over.

**Nit:** The commit subject could be slightly more specific (e.g., `staging: fbtft: replace bare -1 returns with standard error codes`), but the current subject is acceptable.

**Verdict:** The patch is clean and correct. The only point worth raising to the author is whether `-ENOMEM` would be more appropriate than `-EINVAL` for the `txbuf.buf` NULL case in `fbtft_write_vmem16_bus9()`, since the NULL condition results from a missing buffer allocation rather than an invalid argument from the caller.

---
Generated by Claude Code Patch Reviewer

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

end of thread, other threads:[~2026-03-03  4:07 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-28 20:27 [PATCH] staging: fbtft: replace -1 with proper error codes Soham Kute
2026-03-03  4:07 ` Claude review: " Claude Code Review Bot
2026-03-03  4:07 ` 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