From: Masahiro Yamada <yamada.masahiro@socionext.com> To: William Breathitt Gray <vilhelm.gray@gmail.com> Cc: Linus Walleij <linus.walleij@linaro.org>, Andrew Morton <akpm@linux-foundation.org>, "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>, linux-arch <linux-arch@vger.kernel.org>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, Andy Shevchenko <andriy.shevchenko@linux.intel.com>, Rasmus Villemoes <linux@rasmusvillemoes.dk>, Bartosz Golaszewski <bgolaszewski@baylibre.com>, Linux ARM <linux-arm-kernel@lists.infradead.org>, Andy Shevchenko <andy.shevchenko@gmail.com>, Arnd Bergmann <arnd@arndb.de> Subject: Re: [PATCH v9 1/9] bitops: Introduce the for_each_set_clump8 macro Date: Tue, 12 Mar 2019 14:36:21 +0900 [thread overview] Message-ID: <CAK7LNARvv3Z-eJooj7Htv7xWzD_Ow1whxcPrpqn84JHyqUnEJA@mail.gmail.com> (raw) In-Reply-To: <20190308085733.GA3903@icarus> On Fri, Mar 8, 2019 at 5:57 PM William Breathitt Gray <vilhelm.gray@gmail.com> wrote: > > On Fri, Mar 08, 2019 at 09:31:00AM +0100, Linus Walleij wrote: > > On Sun, Mar 3, 2019 at 8:47 AM William Breathitt Gray > > <vilhelm.gray@gmail.com> wrote: > > > > > This macro iterates for each 8-bit group of bits (clump) with set bits, > > > within a bitmap memory region. For each iteration, "start" is set to the > > > bit offset of the found clump, while the respective clump value is > > > stored to the location pointed by "clump". Additionally, the > > > bitmap_get_value8 and bitmap_set_value8 functions are introduced to > > > respectively get and set an 8-bit value in a bitmap memory region. > > > > > > Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > > Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> > > > Cc: Arnd Bergmann <arnd@arndb.de> > > > Cc: Andrew Morton <akpm@linux-foundation.org> > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > > Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com> > > > > Andrew: would you be OK with this being merged in v5.1? > > > > If we need to move the code to drivers/gpio that's OK (though > > I think it's generally useful) but I need to know to proceed with > > the William's nice optimization of these drivers. > > > > Yours, > > Linus Walleij > > I was waiting on Andy to suggest some examples out of the GPIO realm, > but he may be under a heavy workload right so I decided to do a quick > search for one. > > In drivers/of/unittest.c, there is loop across a bitmap in the > of_unittest_destroy_tracked_overlays function: > > for (id = MAX_UNITTEST_OVERLAYS - 1; id >= 0; id--) { > if (!(overlay_id_bits[BIT_WORD(id)] & BIT_MASK(id))) > continue; > > This section of code is checking each bit individually, and skipping if > that bit is not set. This looping can be optimized by using the > for_each_set_clump8 macro Probably no. I see this comment before the loop. /* remove in reverse order */ Also, the unittest code handles per-bit whereas your helper does per-byte. > to skip clumps of nonset bits (not to mention > make the logic of the code much simpler and easier to follow by reducing > the code to a single line): > > for_each_set_clump8(id, clump, overlay_id_bits, MAX_UNITTEST_OVERLAYS-1) > > The for_each_set_clump8 macro is not specific to the GPIO subsystem; I > just happen to use it in these GPIO drivers simply because I am most > familar with this section of the kernel (it's where most of my > contributions occur afterall). > > Consider this, if I am able to find a use for this macro outside of the > GPIO subsystem within a matter minutes, then there must be some benefit > in allowing the rest of the kernel to use the for_each_set_clump8 macro. > So let's put it in bitops.h rather than restrict it to just the GPIO > subsystem. If we do not find useful cases in other subsystem, this patch set looks over-engineering to me. > William Breathitt Gray -- Best Regards Masahiro Yamada
WARNING: multiple messages have this Message-ID (diff)
From: Masahiro Yamada <yamada.masahiro@socionext.com> To: William Breathitt Gray <vilhelm.gray@gmail.com> Cc: linux-arch <linux-arch@vger.kernel.org>, Arnd Bergmann <arnd@arndb.de>, Bartosz Golaszewski <bgolaszewski@baylibre.com>, Linus Walleij <linus.walleij@linaro.org>, Rasmus Villemoes <linux@rasmusvillemoes.dk>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>, Andy Shevchenko <andy.shevchenko@gmail.com>, Andrew Morton <akpm@linux-foundation.org>, Andy Shevchenko <andriy.shevchenko@linux.intel.com>, Linux ARM <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH v9 1/9] bitops: Introduce the for_each_set_clump8 macro Date: Tue, 12 Mar 2019 14:36:21 +0900 [thread overview] Message-ID: <CAK7LNARvv3Z-eJooj7Htv7xWzD_Ow1whxcPrpqn84JHyqUnEJA@mail.gmail.com> (raw) In-Reply-To: <20190308085733.GA3903@icarus> On Fri, Mar 8, 2019 at 5:57 PM William Breathitt Gray <vilhelm.gray@gmail.com> wrote: > > On Fri, Mar 08, 2019 at 09:31:00AM +0100, Linus Walleij wrote: > > On Sun, Mar 3, 2019 at 8:47 AM William Breathitt Gray > > <vilhelm.gray@gmail.com> wrote: > > > > > This macro iterates for each 8-bit group of bits (clump) with set bits, > > > within a bitmap memory region. For each iteration, "start" is set to the > > > bit offset of the found clump, while the respective clump value is > > > stored to the location pointed by "clump". Additionally, the > > > bitmap_get_value8 and bitmap_set_value8 functions are introduced to > > > respectively get and set an 8-bit value in a bitmap memory region. > > > > > > Suggested-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > > Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> > > > Cc: Arnd Bergmann <arnd@arndb.de> > > > Cc: Andrew Morton <akpm@linux-foundation.org> > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > > Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com> > > > > Andrew: would you be OK with this being merged in v5.1? > > > > If we need to move the code to drivers/gpio that's OK (though > > I think it's generally useful) but I need to know to proceed with > > the William's nice optimization of these drivers. > > > > Yours, > > Linus Walleij > > I was waiting on Andy to suggest some examples out of the GPIO realm, > but he may be under a heavy workload right so I decided to do a quick > search for one. > > In drivers/of/unittest.c, there is loop across a bitmap in the > of_unittest_destroy_tracked_overlays function: > > for (id = MAX_UNITTEST_OVERLAYS - 1; id >= 0; id--) { > if (!(overlay_id_bits[BIT_WORD(id)] & BIT_MASK(id))) > continue; > > This section of code is checking each bit individually, and skipping if > that bit is not set. This looping can be optimized by using the > for_each_set_clump8 macro Probably no. I see this comment before the loop. /* remove in reverse order */ Also, the unittest code handles per-bit whereas your helper does per-byte. > to skip clumps of nonset bits (not to mention > make the logic of the code much simpler and easier to follow by reducing > the code to a single line): > > for_each_set_clump8(id, clump, overlay_id_bits, MAX_UNITTEST_OVERLAYS-1) > > The for_each_set_clump8 macro is not specific to the GPIO subsystem; I > just happen to use it in these GPIO drivers simply because I am most > familar with this section of the kernel (it's where most of my > contributions occur afterall). > > Consider this, if I am able to find a use for this macro outside of the > GPIO subsystem within a matter minutes, then there must be some benefit > in allowing the rest of the kernel to use the for_each_set_clump8 macro. > So let's put it in bitops.h rather than restrict it to just the GPIO > subsystem. If we do not find useful cases in other subsystem, this patch set looks over-engineering to me. > William Breathitt Gray -- Best Regards Masahiro Yamada _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-03-12 5:36 UTC|newest] Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-03-03 7:44 [PATCH v9 0/9] Introduce the for_each_set_clump8 macro William Breathitt Gray 2019-03-03 7:47 ` William Breathitt Gray 2019-03-03 7:47 ` William Breathitt Gray 2019-03-03 7:44 ` William Breathitt Gray 2019-03-03 7:48 ` [PATCH v9 1/9] bitops: " William Breathitt Gray 2019-03-03 7:48 ` William Breathitt Gray 2019-03-08 8:31 ` Linus Walleij 2019-03-08 8:31 ` Linus Walleij 2019-03-08 8:57 ` William Breathitt Gray 2019-03-08 8:57 ` William Breathitt Gray 2019-03-08 9:19 ` Andy Shevchenko 2019-03-08 9:19 ` Andy Shevchenko 2019-03-12 5:36 ` Masahiro Yamada [this message] 2019-03-12 5:36 ` Masahiro Yamada 2019-03-12 7:22 ` William Breathitt Gray 2019-03-12 7:22 ` William Breathitt Gray 2019-03-12 14:54 ` Andy Shevchenko 2019-03-12 14:54 ` Andy Shevchenko 2019-03-12 1:01 ` Andrew Morton 2019-03-12 1:01 ` Andrew Morton 2019-03-12 10:43 ` William Breathitt Gray 2019-03-12 10:43 ` William Breathitt Gray 2019-03-12 3:52 ` Masahiro Yamada 2019-03-12 3:52 ` Masahiro Yamada 2019-03-12 5:03 ` Masahiro Yamada 2019-03-12 5:03 ` Masahiro Yamada 2019-03-12 7:14 ` Andy Shevchenko 2019-03-12 7:14 ` Andy Shevchenko 2019-03-03 7:48 ` [PATCH v9 2/9] lib/test_bitmap.c: Add for_each_set_clump8 test cases William Breathitt Gray 2019-03-03 7:48 ` William Breathitt Gray 2019-03-11 7:56 ` [LKP] [lib/test_bitmap.c] ecdc93614a: kernel_selftests.lib.bitmap.sh.fail kernel test robot 2019-03-11 7:56 ` kernel test robot 2019-03-11 7:56 ` [LKP] " kernel test robot 2019-03-03 7:49 ` [PATCH v9 3/9] gpio: 104-dio-48e: Utilize for_each_set_clump8 macro William Breathitt Gray 2019-03-03 7:49 ` William Breathitt Gray 2019-03-03 7:49 ` [PATCH v9 4/9] gpio: 104-idi-48: " William Breathitt Gray 2019-03-03 7:49 ` William Breathitt Gray 2019-03-03 7:50 ` [PATCH v9 5/9] gpio: gpio-mm: " William Breathitt Gray 2019-03-03 7:50 ` William Breathitt Gray 2019-03-03 7:50 ` [PATCH v9 6/9] gpio: ws16c48: " William Breathitt Gray 2019-03-03 7:50 ` William Breathitt Gray 2019-03-03 7:51 ` [PATCH v9 7/9] gpio: pci-idio-16: " William Breathitt Gray 2019-03-03 7:51 ` William Breathitt Gray 2019-03-03 7:51 ` [PATCH v9 8/9] gpio: pcie-idio-24: " William Breathitt Gray 2019-03-03 7:51 ` William Breathitt Gray 2019-03-03 7:51 ` [PATCH v9 9/9] gpio: uniphier: " William Breathitt Gray 2019-03-03 7:51 ` William Breathitt Gray 2019-03-12 4:36 ` Masahiro Yamada 2019-03-12 4:36 ` Masahiro Yamada 2019-03-12 7:17 ` Andy Shevchenko 2019-03-12 7:17 ` Andy Shevchenko 2019-03-12 8:57 ` Masahiro Yamada 2019-03-12 8:57 ` Masahiro Yamada 2019-03-12 9:09 ` Andy Shevchenko 2019-03-12 9:09 ` Andy Shevchenko 2019-03-12 7:29 ` William Breathitt Gray 2019-03-12 7:29 ` 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=CAK7LNARvv3Z-eJooj7Htv7xWzD_Ow1whxcPrpqn84JHyqUnEJA@mail.gmail.com \ --to=yamada.masahiro@socionext.com \ --cc=akpm@linux-foundation.org \ --cc=andriy.shevchenko@linux.intel.com \ --cc=andy.shevchenko@gmail.com \ --cc=arnd@arndb.de \ --cc=bgolaszewski@baylibre.com \ --cc=linus.walleij@linaro.org \ --cc=linux-arch@vger.kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-gpio@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@rasmusvillemoes.dk \ --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: linkBe 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.