From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: staging: fbtft: replace -1 with proper error codes
Date: Tue, 03 Mar 2026 14:07:12 +1000
Message-ID: <review-patch1-20260228202701.43337-1-officialsohamkute@gmail.com>
In-Reply-To: <20260228202701.43337-1-officialsohamkute@gmail.com>
References: <20260228202701.43337-1-officialsohamkute@gmail.com> <20260228202701.43337-1-officialsohamkute@gmail.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Patch Review

**Positive aspects:**
- The commit message is clear and enumerates each error code choice with it=
s rationale.
- The Signed-off-by is present.
- The diff is minimal and focused =E2=80=94 no unrelated changes.

**Error code review:**

1. **`fb_ra8875.c:write_spi()` =E2=80=94 `-1` =E2=86=92 `-ENODEV`**: Correc=
t. The SPI device being NULL means the device is not available.

2. **`fb_ra8875.c:init_display()` =E2=80=94 `-1` =E2=86=92 `-EINVAL`**: Cor=
rect. An unsupported display resolution is an invalid parameter/configurati=
on.

3. **`fbtft-io.c:fbtft_write_spi()` =E2=80=94 `-1` =E2=86=92 `-ENODEV`**: C=
orrect. Same SPI NULL case as above, consistent choice.

4. **`fbtft-io.c:fbtft_write_gpio16_wr_latched()` =E2=80=94 `-1` =E2=86=92 =
`-EOPNOTSUPP`**: Correct. Function is a stub, `-EOPNOTSUPP` is the right co=
de.

5. **`fbtft-bus.c:fbtft_write_vmem8_bus8()` =E2=80=94 `-1` =E2=86=92 `-EOPN=
OTSUPP`**: Correct. Same unimplemented stub pattern.

6. **`fbtft-bus.c:fbtft_write_vmem16_bus9()` =E2=80=94 `-1` =E2=86=92 `-EIN=
VAL`** for `txbuf.buf` being NULL: This is **debatable**. Looking at the co=
de, `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 i=
nvalid argument =E2=80=94 it's because the buffer was never allocated (eith=
er `txbuflen` was 0, or allocation failed and was silently handled). `-ENOM=
EM` might be slightly more accurate, but one could also argue this is an in=
ternal 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 curre=
nt subject is acceptable.

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

---
Generated by Claude Code Patch Reviewer