From: William Breathitt Gray <vilhelm.gray@gmail.com>
To: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: linus.walleij@linaro.org, akpm@linux-foundation.org,
linux-gpio@vger.kernel.org, linux-arch@vger.kernel.org,
linux-kernel@vger.kernel.org, andriy.shevchenko@linux.intel.com
Subject: Re: [RESEND PATCH v4 3/8] gpio: 104-dio-48e: Utilize for_each_set_clump macro
Date: Sun, 14 Oct 2018 13:19:33 +0900 [thread overview]
Message-ID: <20181014041933.GA7335@icarus> (raw)
In-Reply-To: <822be05b-092b-41c2-3c31-8981acd5cb9e@rasmusvillemoes.dk>
On Tue, Oct 02, 2018 at 09:00:45AM +0200, Rasmus Villemoes wrote:
> On 2018-10-02 03:14, William Breathitt Gray wrote:
> > /* clear bits array to a clean slate */
> > bitmap_zero(bits, chip->ngpio);
> >
> > - /* get bits are evaluated a gpio port register at a time */
> > - for (i = 0; i < ARRAY_SIZE(ports); i++) {
> > - /* gpio offset in bits array */
> > - bits_offset = i * gpio_reg_size;
> > -
> > - /* word index for bits array */
> > - word_index = BIT_WORD(bits_offset);
> > -
> > - /* gpio offset within current word of bits array */
> > - word_offset = bits_offset % BITS_PER_LONG;
> > -
> > - /* mask of get bits for current gpio within current word */
> > - word_mask = mask[word_index] & (port_mask << word_offset);
> > - if (!word_mask) {
> > - /* no get bits in this port so skip to next one */
> > - continue;
> > - }
> > -
> > - /* read bits from current gpio port */
> > + for_each_set_clump(i, word, offset, mask, ARRAY_SIZE(ports), 8) {
> > port_state = inb(dio48egpio->base + ports[i]);
> > -
> > - /* store acquired bits at respective bits array offset */
> > - bits[word_index] |= port_state << word_offset;
> > + bits[word] |= port_state << offset;
>
> Somewhat unrelated to this series, but is the existing code correct? I'd
> expect the RHS to be masked by word_mask; otherwise we might set bits in
> bits[] that were not requested? And if one does that, the !word_mask
> test is merely an optimization to avoid reading the gpios when the
> result would be ignored anyway. Perhaps no caller cares.
>
> Rasmus
I don't think the caller cares in this case. Take a look at the
gpiod_get_array_value_complex function: the desired inputs are collected
before gpio_chip_get_multiple is called and then looped through after --
unrequested bits are simply ignored.
This caller behavior also makes sense because a bit value of 0 in the
bits array does not necessarily mean the input was not requested, but
may instead mean that the value at the input is 0; therefore, the caller
must keep track of the requested inputs rather than try to deduce them
from the values in the bits array.
William Breathitt Gray
next prev parent reply other threads:[~2018-10-14 4:19 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-02 1:12 [RESEND PATCH v4 0/8] Introduce the for_each_set_clump macro William Breathitt Gray
2018-10-02 1:13 ` [RESEND PATCH v4 1/8] bitops: " William Breathitt Gray
2018-10-02 7:42 ` Rasmus Villemoes
2018-10-02 8:21 ` Andy Shevchenko
2018-10-03 11:48 ` Andy Shevchenko
2018-10-04 10:36 ` William Breathitt Gray
2018-10-04 12:10 ` Andy Shevchenko
2018-10-04 10:30 ` William Breathitt Gray
2018-10-04 10:03 ` William Breathitt Gray
2018-10-02 1:14 ` [RESEND PATCH v4 2/8] lib/test_bitmap.c: Add for_each_set_clump test cases William Breathitt Gray
2018-10-02 1:14 ` [RESEND PATCH v4 3/8] gpio: 104-dio-48e: Utilize for_each_set_clump macro William Breathitt Gray
2018-10-02 7:00 ` Rasmus Villemoes
2018-10-14 4:19 ` William Breathitt Gray [this message]
2018-10-15 11:59 ` Rasmus Villemoes
2018-10-17 1:54 ` William Breathitt Gray
2018-10-02 1:15 ` [RESEND PATCH v4 4/8] gpio: 104-idi-48: " William Breathitt Gray
2018-10-02 1:15 ` [RESEND PATCH v4 5/8] gpio: gpio-mm: " William Breathitt Gray
2018-10-02 1:15 ` [RESEND PATCH v4 6/8] gpio: ws16c48: " William Breathitt Gray
2018-10-02 1:16 ` [RESEND PATCH v4 7/8] gpio: pci-idio-16: " William Breathitt Gray
2018-10-02 1:16 ` [RESEND PATCH v4 8/8] gpio: pcie-idio-24: " 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=20181014041933.GA7335@icarus \
--to=vilhelm.gray@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=andriy.shevchenko@linux.intel.com \
--cc=linus.walleij@linaro.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@rasmusvillemoes.dk \
/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;
as well as URLs for NNTP newsgroup(s).