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: fix coding style issue in fbtft-bus.c
Date: Mon, 13 Apr 2026 18:59:37 +1000
Message-ID: <review-patch1-20260412164953.1470-1-mzndmzn@gmail.com>
In-Reply-To: <20260412164953.1470-1-mzndmzn@gmail.com>
References: <20260412164953.1470-1-mzndmzn@gmail.com> <20260412164953.1470-1-mzndmzn@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

**Result: NAK =E2=80=94 introduces a build break**

The macro `define_fbtft_write_reg` is defined at `fbtft-bus.c:14` with **4 =
parameters**:

```c
#define define_fbtft_write_reg(func, buffer_type, data_type, modifier)
```

The `modifier` parameter is used throughout the macro body as a function-li=
ke call, e.g. at line 42:

```c
*buf =3D modifier((data_type)va_arg(args, unsigned int));
```

When the 4th argument is left **empty** (as in the original code):

```c
define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8, )
```

the preprocessor expands `modifier((data_type)va_arg(args, unsigned int))` =
to simply `((u8)va_arg(args, unsigned int))` =E2=80=94 an identity operatio=
n wrapped in parentheses. This is a deliberate and well-known C preprocesso=
r idiom for a "no-op modifier." Compare with line 66 where `cpu_to_be16` is=
 passed as the modifier to perform byte-swapping:

```c
define_fbtft_write_reg(fbtft_write_reg16_bus8, __be16, u16, cpu_to_be16)
```

The patch changes the invocations to only pass 3 arguments:

```c
-define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8, )
+define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8)
```

This will produce a compilation error like:

> `error: macro "define_fbtft_write_reg" requires 4 arguments, but only 3 g=
iven`

The trailing comma before `)` is **not** a coding style issue =E2=80=94 it =
is syntactically required to pass an empty 4th macro argument. checkpatch.p=
l's warning is a false positive in this context.

**If the author wishes to make this cleaner**, the correct approach would b=
e to refactor the macro itself, for example by introducing a separate 3-par=
ameter version without `modifier`, or by using variadic macros (`__VA_ARGS_=
_`), or by defining a no-op modifier macro like `#define IDENTITY(x) (x)` a=
nd passing that instead of an empty argument. Simply dropping the argument =
is not valid.

---
Generated by Claude Code Patch Reviewer