drivers/staging/fbtft/fbtft-bus.c | 191 +++++++++++++++++++++--------- 1 file changed, 137 insertions(+), 54 deletions(-)
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.0On 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
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
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
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
© 2016 - 2026 Red Hat, Inc.