All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: William Breathitt Gray <vilhelm.gray@gmail.com>
Cc: Syed Nayyar Waris <syednwaris@gmail.com>,
	Arnd Bergmann <arnd@kernel.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Michal Simek <michal.simek@xilinx.com>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v12 4/4] gpio: xilinx: Utilize generic bitmap_get_value and _set_value
Date: Mon, 9 Nov 2020 19:22:20 +0200	[thread overview]
Message-ID: <20201109172220.GI4077@smile.fi.intel.com> (raw)
In-Reply-To: <20201109171140.GA14045@shinobu>

On Mon, Nov 09, 2020 at 12:11:40PM -0500, William Breathitt Gray wrote:
> On Mon, Nov 09, 2020 at 10:15:29PM +0530, Syed Nayyar Waris wrote:
> > On Mon, Nov 09, 2020 at 03:41:53PM +0100, Arnd Bergmann wrote:

...

> >  static inline void bitmap_set_value(unsigned long *map,
> > -                                    unsigned long value,
> > +                                    unsigned long value, const size_t length,
> >                                      unsigned long start, unsigned long nbits)
> >  {
> >          const size_t index = BIT_WORD(start);
> > @@ -15,6 +15,10 @@ static inline void bitmap_set_value(unsigned long *map,
> >          } else {
> >                  map[index + 0] &= ~BITMAP_FIRST_WORD_MASK(start);
> >                  map[index + 0] |= value << offset;
> > +
> > +               if (index + 1 >= length)
> > +                       __builtin_unreachable();
> > +
> >                  map[index + 1] &= ~BITMAP_LAST_WORD_MASK(start + nbits);
> >                  map[index + 1] |= value >> space;
> >          }
> 
> Hi Syed,
> 
> Let's rename 'length' to 'nbits' as Arnd suggested, and rename 'nbits'
> to value_width.

length here is in longs. I guess this is the point of entire patch.

But to me sounds like it would be better to have simply bitmap_set_value64() /
bitmap_set_value32() with proper optimization done and forget about variadic
ones for now.

-- 
With Best Regards,
Andy Shevchenko



WARNING: multiple messages have this Message-ID (diff)
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: William Breathitt Gray <vilhelm.gray@gmail.com>
Cc: Arnd Bergmann <arnd@kernel.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Michal Simek <michal.simek@xilinx.com>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Syed Nayyar Waris <syednwaris@gmail.com>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v12 4/4] gpio: xilinx: Utilize generic bitmap_get_value and _set_value
Date: Mon, 9 Nov 2020 19:22:20 +0200	[thread overview]
Message-ID: <20201109172220.GI4077@smile.fi.intel.com> (raw)
In-Reply-To: <20201109171140.GA14045@shinobu>

On Mon, Nov 09, 2020 at 12:11:40PM -0500, William Breathitt Gray wrote:
> On Mon, Nov 09, 2020 at 10:15:29PM +0530, Syed Nayyar Waris wrote:
> > On Mon, Nov 09, 2020 at 03:41:53PM +0100, Arnd Bergmann wrote:

...

> >  static inline void bitmap_set_value(unsigned long *map,
> > -                                    unsigned long value,
> > +                                    unsigned long value, const size_t length,
> >                                      unsigned long start, unsigned long nbits)
> >  {
> >          const size_t index = BIT_WORD(start);
> > @@ -15,6 +15,10 @@ static inline void bitmap_set_value(unsigned long *map,
> >          } else {
> >                  map[index + 0] &= ~BITMAP_FIRST_WORD_MASK(start);
> >                  map[index + 0] |= value << offset;
> > +
> > +               if (index + 1 >= length)
> > +                       __builtin_unreachable();
> > +
> >                  map[index + 1] &= ~BITMAP_LAST_WORD_MASK(start + nbits);
> >                  map[index + 1] |= value >> space;
> >          }
> 
> Hi Syed,
> 
> Let's rename 'length' to 'nbits' as Arnd suggested, and rename 'nbits'
> to value_width.

length here is in longs. I guess this is the point of entire patch.

But to me sounds like it would be better to have simply bitmap_set_value64() /
bitmap_set_value32() with proper optimization done and forget about variadic
ones for now.

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-11-09 17:21 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-18 21:37 [PATCH v12 0/4] Introduce the for_each_set_clump macro Syed Nayyar Waris
2020-10-18 21:37 ` Syed Nayyar Waris
2020-10-18 21:38 ` [PATCH v12 1/4] bitops: " Syed Nayyar Waris
2020-10-18 21:39 ` [PATCH v12 2/4] lib/test_bitmap.c: Add for_each_set_clump test cases Syed Nayyar Waris
2020-10-18 21:40 ` [PATCH v12 3/4] gpio: thunderx: Utilize for_each_set_clump macro Syed Nayyar Waris
2020-11-05  9:10   ` Linus Walleij
2020-10-18 21:41 ` [PATCH v12 4/4] gpio: xilinx: Utilize generic bitmap_get_value and _set_value Syed Nayyar Waris
2020-10-18 21:41   ` Syed Nayyar Waris
2020-10-29 22:44   ` Arnd Bergmann
2020-10-29 22:44     ` Arnd Bergmann
2020-11-01 15:00     ` William Breathitt Gray
2020-11-01 15:00       ` William Breathitt Gray
2020-11-01 20:08       ` Arnd Bergmann
2020-11-01 20:08         ` Arnd Bergmann
2020-11-09 12:34         ` Syed Nayyar Waris
2020-11-09 12:34           ` Syed Nayyar Waris
2020-11-09 13:13           ` Arnd Bergmann
2020-11-09 13:13             ` Arnd Bergmann
2020-11-09 13:41           ` William Breathitt Gray
2020-11-09 13:41             ` William Breathitt Gray
2020-11-09 14:38             ` William Breathitt Gray
2020-11-09 14:38               ` William Breathitt Gray
2020-11-09 14:48               ` Syed Nayyar Waris
2020-11-09 14:48                 ` Syed Nayyar Waris
2020-11-09 15:18                 ` William Breathitt Gray
2020-11-09 15:18                   ` William Breathitt Gray
2020-11-09 14:41             ` Arnd Bergmann
2020-11-09 14:41               ` Arnd Bergmann
2020-11-09 16:45               ` Syed Nayyar Waris
2020-11-09 16:45                 ` Syed Nayyar Waris
2020-11-09 17:11                 ` William Breathitt Gray
2020-11-09 17:11                   ` William Breathitt Gray
2020-11-09 17:22                   ` Andy Shevchenko [this message]
2020-11-09 17:22                     ` Andy Shevchenko
2020-11-09 17:31                     ` William Breathitt Gray
2020-11-09 17:31                       ` William Breathitt Gray
2020-11-10 10:02                       ` Michal Simek
2020-11-10 10:02                         ` Michal Simek
2020-11-10 12:35                         ` William Breathitt Gray
2020-11-10 12:35                           ` William Breathitt Gray
2020-11-10 17:22                           ` Syed Nayyar Waris
2020-11-10 17:22                             ` Syed Nayyar Waris
2020-11-10 17:43                             ` William Breathitt Gray
2020-11-10 17:43                               ` William Breathitt Gray
2020-11-10 22:00                               ` Syed Nayyar Waris
2020-11-10 22:00                                 ` Syed Nayyar Waris
2020-11-13 16:52                                 ` Syed Nayyar Waris
2020-11-13 16:52                                   ` Syed Nayyar Waris
2020-11-20 13:26                                   ` Arnd Bergmann
2020-11-20 13:26                                     ` Arnd Bergmann
2020-11-20 13:45                                     ` William Breathitt Gray
2020-11-20 13:45                                       ` William Breathitt Gray

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=20201109172220.GI4077@smile.fi.intel.com \
    --to=andriy.shevchenko@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@kernel.org \
    --cc=bgolaszewski@baylibre.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michal.simek@xilinx.com \
    --cc=syednwaris@gmail.com \
    --cc=vilhelm.gray@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.