Linux Framebuffer Layer development
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: Joyeta Modak <joyetamdk@gmail.com>
Cc: Andy Shevchenko <andriy.shevchenko@intel.com>,
	andy@kernel.org, gregkh@linuxfoundation.org,
	dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
	linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: fbtft: use ARRAY_SIZE() in NUMARGS macro
Date: Fri, 26 Jun 2026 14:41:30 +0100	[thread overview]
Message-ID: <20260626144130.6e3c924e@pumpkin> (raw)
In-Reply-To: <CAN3JnUCem3Mw17e5D9b-aX58_ZaAvC6GZ-sBPEQfTTTP=o-WHA@mail.gmail.com>

On Fri, 26 Jun 2026 14:27:25 +0530
Joyeta Modak <joyetamdk@gmail.com> wrote:

> Thank you for the feedback and the question.
> 
> I checked every write_reg() across all fbtft drivers and found that
> the largest number of arguments is 129 in write_reg(par,
> MIPI_DCS_WRITE_LUT,...)
> As COUNT_ARGS() in args.h only supports up to 15, it is not a safe fit here.

That is also a pretty horrid way to write that message out.
The function call itself uses well over 512 bytes of stack.
Then there is all the code to push the arguments.

It really shouldn't be too hard to pass the address of a const u8[]
all the way through to the code that copies the data to the hardware.

I tried to follow the code earlier. The 'common functions' that pretty
much just call back through per-driver functions with names the 'write'
really don't make it easy.

	David

> 
> However, the kernel test robot reported a problem with my
> implementation as the __must_be_array() check in ARRAY_SIZE() requires
> the array to be a compile time constant expression and thus breaks the
> call at several places.(example par->bgr)
> 
> I tried to reproduce this locally on my system using both GCC and
> Clang with ARCH=um on x86_64 but could not reproduce the build
> failure.
> 
> Since the original sizeof() based approach had no such errors flagged,
> I am thinking of dropping the ARRAY_SIZE() approach.
> 
> Any other feedback is appreciated. Thanks again.
> 
> On Wed, Jun 24, 2026 at 5:01 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> >
> > On Wed, Jun 24, 2026 at 01:08:04PM +0530, Joyeta Modak wrote:  
> > > NUMARGS() computes the number of arguments by dividing the size of a
> > > temporary int array by sizeof(int). Using the standard ARRAY_SIZE()
> > > macro is the correct way to count array elements in the kernel, and
> > > ARRAY_SIZE() also provides a __must_be_array() compile time check. There
> > > are no functional changes.  
> >
> > ...
> >  
> > > -#define NUMARGS(...)  (sizeof((int[]){__VA_ARGS__}) / sizeof(int))
> > > +#define NUMARGS(...)  ARRAY_SIZE(((int[]){__VA_ARGS__}))
> > >
> > >  #define write_reg(par, ...)                                            \
> > >       ((par)->fbtftops.write_register(par, NUMARGS(__VA_ARGS__), __VA_ARGS__))  
> >
> > What is the maximum parameters .write_register() takes in practice in the
> > fbtft drivers? If it's less than or equal to 15, we may use args.h instead.
> >
> > --
> > With Best Regards,
> > Andy Shevchenko
> >
> >  
> 
> 


  parent reply	other threads:[~2026-06-26 13:41 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-24  7:38 [PATCH] staging: fbtft: use ARRAY_SIZE() in NUMARGS macro Joyeta Modak
2026-06-24 11:31 ` Andy Shevchenko
2026-06-26  8:57   ` Joyeta Modak
2026-06-26 12:23     ` Andy Shevchenko
2026-06-26 13:41     ` David Laight [this message]
2026-06-24 13:02 ` kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260626144130.6e3c924e@pumpkin \
    --to=david.laight.linux@gmail.com \
    --cc=andriy.shevchenko@intel.com \
    --cc=andy@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=joyetamdk@gmail.com \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox