[PATCH] staging: fbtft: fbtft-bus: replace function-defining macro with concrete functions

[PATCH] staging: fbtft: fbtft-bus: replace function-defining macro with concrete functions

[Alexandru Hossu]

The define_fbtft_write_reg macro defines full function bodies including
a goto statement and a trailing semicolon on EXPORT_SYMBOL(), which
violates kernel coding style (checkpatch reports 2 ERRORs, 2 WARNINGs,
and 5 CHECKs).

Replace it with three concrete C functions that are semantically
identical to the macro expansions:
  - fbtft_write_reg8_bus8   (u8 buffer, u8 data)
  - fbtft_write_reg16_bus8  (__be16 buffer, u16 data, cpu_to_be16)
  - fbtft_write_reg16_bus16 (u16 buffer, u16 data)

The function declarations in fbtft.h are already present and unchanged.

Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
---
 drivers/staging/fbtft/fbtft-bus.c | 191 +++++++++++++++++++++---------
 1 file changed, 137 insertions(+), 54 deletions(-)

diff --git a/drivers/staging/fbtft/fbtft-bus.c b/drivers/staging/fbtft/fbtft-bus.c
index 30e436ff1..acd035203 100644
--- a/drivers/staging/fbtft/fbtft-bus.c
+++ b/drivers/staging/fbtft/fbtft-bus.c
@@ -11,60 +11,143 @@
  *
  *****************************************************************************/
 
-#define define_fbtft_write_reg(func, buffer_type, data_type, modifier)        \
-void func(struct fbtft_par *par, int len, ...)                                \
-{                                                                             \
-	va_list args;                                                         \
-	int i, ret;                                                           \
-	int offset = 0;                                                       \
-	buffer_type *buf = (buffer_type *)par->buf;                           \
-									      \
-	if (unlikely(par->debug & DEBUG_WRITE_REGISTER)) {                    \
-		va_start(args, len);                                          \
-		for (i = 0; i < len; i++) {                                   \
-			buf[i] = modifier((data_type)va_arg(args,             \
-							    unsigned int));   \
-		}                                                             \
-		va_end(args);                                                 \
-		fbtft_par_dbg_hex(DEBUG_WRITE_REGISTER, par,                  \
-				  par->info->device, buffer_type, buf, len,   \
-				  "%s: ", __func__);                          \
-	}                                                                     \
-									      \
-	va_start(args, len);                                                  \
-									      \
-	if (par->startbyte) {                                                 \
-		*(u8 *)par->buf = par->startbyte;                             \
-		buf = (buffer_type *)(par->buf + 1);                          \
-		offset = 1;                                                   \
-	}                                                                     \
-									      \
-	*buf = modifier((data_type)va_arg(args, unsigned int));               \
-	ret = fbtft_write_buf_dc(par, par->buf, sizeof(data_type) + offset,   \
-				 0);                                          \
-	if (ret < 0)							      \
-		goto out;						      \
-	len--;                                                                \
-									      \
-	if (par->startbyte)                                                   \
-		*(u8 *)par->buf = par->startbyte | 0x2;                       \
-									      \
-	if (len) {                                                            \
-		i = len;                                                      \
-		while (i--)						      \
-			*buf++ = modifier((data_type)va_arg(args,             \
-							    unsigned int));   \
-		fbtft_write_buf_dc(par, par->buf,			      \
-				   len * (sizeof(data_type) + offset), 1);    \
-	}                                                                     \
-out:									      \
-	va_end(args);                                                         \
-}                                                                             \
-EXPORT_SYMBOL(func);
-
-define_fbtft_write_reg(fbtft_write_reg8_bus8, u8, u8, )
-define_fbtft_write_reg(fbtft_write_reg16_bus8, __be16, u16, cpu_to_be16)
-define_fbtft_write_reg(fbtft_write_reg16_bus16, u16, u16, )
+void fbtft_write_reg8_bus8(struct fbtft_par *par, int len, ...)
+{
+	va_list args;
+	int i, ret;
+	int offset = 0;
+	u8 *buf = (u8 *)par->buf;
+
+	if (unlikely(par->debug & DEBUG_WRITE_REGISTER)) {
+		va_start(args, len);
+		for (i = 0; i < len; i++)
+			buf[i] = (u8)va_arg(args, unsigned int);
+		va_end(args);
+		fbtft_par_dbg_hex(DEBUG_WRITE_REGISTER, par,
+				  par->info->device, u8, buf, len,
+				  "%s: ", __func__);
+	}
+
+	va_start(args, len);
+
+	if (par->startbyte) {
+		*(u8 *)par->buf = par->startbyte;
+		buf = (u8 *)(par->buf + 1);
+		offset = 1;
+	}
+
+	*buf = (u8)va_arg(args, unsigned int);
+	ret = fbtft_write_buf_dc(par, par->buf, sizeof(u8) + offset, 0);
+	if (ret < 0)
+		goto out;
+	len--;
+
+	if (par->startbyte)
+		*(u8 *)par->buf = par->startbyte | 0x2;
+
+	if (len) {
+		i = len;
+		while (i--)
+			*buf++ = (u8)va_arg(args, unsigned int);
+		fbtft_write_buf_dc(par, par->buf,
+				   len * (sizeof(u8) + offset), 1);
+	}
+out:
+	va_end(args);
+}
+EXPORT_SYMBOL(fbtft_write_reg8_bus8);
+
+void fbtft_write_reg16_bus8(struct fbtft_par *par, int len, ...)
+{
+	va_list args;
+	int i, ret;
+	int offset = 0;
+	__be16 *buf = (__be16 *)par->buf;
+
+	if (unlikely(par->debug & DEBUG_WRITE_REGISTER)) {
+		va_start(args, len);
+		for (i = 0; i < len; i++)
+			buf[i] = cpu_to_be16((u16)va_arg(args, unsigned int));
+		va_end(args);
+		fbtft_par_dbg_hex(DEBUG_WRITE_REGISTER, par,
+				  par->info->device, __be16, buf, len,
+				  "%s: ", __func__);
+	}
+
+	va_start(args, len);
+
+	if (par->startbyte) {
+		*(u8 *)par->buf = par->startbyte;
+		buf = (__be16 *)(par->buf + 1);
+		offset = 1;
+	}
+
+	*buf = cpu_to_be16((u16)va_arg(args, unsigned int));
+	ret = fbtft_write_buf_dc(par, par->buf, sizeof(u16) + offset, 0);
+	if (ret < 0)
+		goto out;
+	len--;
+
+	if (par->startbyte)
+		*(u8 *)par->buf = par->startbyte | 0x2;
+
+	if (len) {
+		i = len;
+		while (i--)
+			*buf++ = cpu_to_be16((u16)va_arg(args, unsigned int));
+		fbtft_write_buf_dc(par, par->buf,
+				   len * (sizeof(u16) + offset), 1);
+	}
+out:
+	va_end(args);
+}
+EXPORT_SYMBOL(fbtft_write_reg16_bus8);
+
+void fbtft_write_reg16_bus16(struct fbtft_par *par, int len, ...)
+{
+	va_list args;
+	int i, ret;
+	int offset = 0;
+	u16 *buf = (u16 *)par->buf;
+
+	if (unlikely(par->debug & DEBUG_WRITE_REGISTER)) {
+		va_start(args, len);
+		for (i = 0; i < len; i++)
+			buf[i] = (u16)va_arg(args, unsigned int);
+		va_end(args);
+		fbtft_par_dbg_hex(DEBUG_WRITE_REGISTER, par,
+				  par->info->device, u16, buf, len,
+				  "%s: ", __func__);
+	}
+
+	va_start(args, len);
+
+	if (par->startbyte) {
+		*(u8 *)par->buf = par->startbyte;
+		buf = (u16 *)(par->buf + 1);
+		offset = 1;
+	}
+
+	*buf = (u16)va_arg(args, unsigned int);
+	ret = fbtft_write_buf_dc(par, par->buf, sizeof(u16) + offset, 0);
+	if (ret < 0)
+		goto out;
+	len--;
+
+	if (par->startbyte)
+		*(u8 *)par->buf = par->startbyte | 0x2;
+
+	if (len) {
+		i = len;
+		while (i--)
+			*buf++ = (u16)va_arg(args, unsigned int);
+		fbtft_write_buf_dc(par, par->buf,
+				   len * (sizeof(u16) + offset), 1);
+	}
+out:
+	va_end(args);
+}
+EXPORT_SYMBOL(fbtft_write_reg16_bus16);
 
 void fbtft_write_reg8_bus9(struct fbtft_par *par, int len, ...)
 {
-- 
2.53.0

Re: [PATCH] staging: fbtft: fbtft-bus: replace function-defining macro with concrete functions

[Andy Shevchenko]

On Fri, Apr 24, 2026 at 11:28:18AM +0200, Alexandru Hossu wrote:
> The define_fbtft_write_reg macro defines full function bodies including
> a goto statement and a trailing semicolon on EXPORT_SYMBOL(), which
> violates kernel coding style (checkpatch reports 2 ERRORs, 2 WARNINGs,
> and 5 CHECKs).

OK.

> Replace it with three concrete C functions that are semantically
> identical to the macro expansions:
>   - fbtft_write_reg8_bus8   (u8 buffer, u8 data)
>   - fbtft_write_reg16_bus8  (__be16 buffer, u16 data, cpu_to_be16)
>   - fbtft_write_reg16_bus16 (u16 buffer, u16 data)
> 
> The function declarations in fbtft.h are already present and unchanged.

I'm not sure this patch improves the code. What I see it's harder to follow.
NAK.

You can consider different approach(es), using _Generic() or so, but I forecast
that none of them will be better than the current code.

You also can address just a small chunk of that, exempli gratia, moving out
EXPORT_*() along with some wrappers leaving the main body of the macro
untouched. This might be a compromise. Dunno. If Greg has no objections, you
can try it out.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] staging: fbtft: fbtft-bus: replace function-defining macro with concrete functions

[Alexandru Hossu]

On Fri, Apr 24, 2026 at 12:38:45PM +0300, Andy Shevchenko wrote:
> I'm not sure this patch improves the code. What I see it's harder to follow.
> NAK.

Fair point. Three near-identical functions hide the pattern the macro
makes explicit. I'll drop this approach.

If there's appetite for a minimal fix, I can send a v2 that moves
EXPORT_SYMBOL() outside the macro body only, leaving the function
definition untouched. Otherwise I'll leave it as-is and wait for
Greg's take.

Thanks for the quick review.

Alexandru

Re: [PATCH] staging: fbtft: fbtft-bus: replace function-defining macro with concrete functions

[Andy Shevchenko]

On Fri, Apr 24, 2026 at 09:40:48AM -0000, Alexandru Hossu wrote:
> On Fri, Apr 24, 2026 at 12:38:45PM +0300, Andy Shevchenko wrote:
> > I'm not sure this patch improves the code. What I see it's harder to follow.
> > NAK.
> 
> Fair point. Three near-identical functions hide the pattern the macro
> makes explicit. I'll drop this approach.
> 
> If there's appetite for a minimal fix, I can send a v2 that moves
> EXPORT_SYMBOL() outside the macro body only, leaving the function
> definition untouched. Otherwise I'll leave it as-is and wait for
> Greg's take.

You can try that, as I said it would be a compromise because at least I agree
with the awkwardness of having EXPORT_*() be hidden by the macro.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] staging: fbtft: fbtft-bus: replace function-defining macro with concrete functions

[Greg KH]

On Fri, Apr 24, 2026 at 09:40:48AM -0000, Alexandru Hossu wrote:
> On Fri, Apr 24, 2026 at 12:38:45PM +0300, Andy Shevchenko wrote:
> > I'm not sure this patch improves the code. What I see it's harder to follow.
> > NAK.
> 
> Fair point. Three near-identical functions hide the pattern the macro
> makes explicit. I'll drop this approach.
> 
> If there's appetite for a minimal fix, I can send a v2 that moves
> EXPORT_SYMBOL() outside the macro body only, leaving the function
> definition untouched. Otherwise I'll leave it as-is and wait for
> Greg's take.

Please leave as-is, there's nothing wrong with the existing code here.

thanks,

greg k-h