Re: [PATCH v3 4/4] gpio: xilinx: Utilize for_each_set_clump macro

From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: William Breathitt Gray <vilhelm.gray@gmail.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Syed Nayyar Waris <syednwaris@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Walleij <linus.walleij@linaro.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	Michal Simek <michal.simek@xilinx.com>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	linux-arm Mailing List <linux-arm-kernel@lists.infradead.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 4/4] gpio: xilinx: Utilize for_each_set_clump macro
Date: Fri, 1 May 2020 16:32:32 +0300	[thread overview]
Message-ID: <CAHp75VfCC_E9uB1eq2EVK_nLe=Oo1zqO=Dmm8kX6y-9YoosDqA@mail.gmail.com> (raw)
In-Reply-To: <20200430233227.GA15963@icarus>

On Fri, May 1, 2020 at 2:38 AM William Breathitt Gray
<vilhelm.gray@gmail.com> wrote:
> On Thu, Apr 30, 2020 at 07:38:55PM +0300, Andy Shevchenko wrote:
> > On Thu, Apr 30, 2020 at 09:45:14PM +0530, Syed Nayyar Waris wrote:
> > > On Wed, Apr 29, 2020 at 01:21:14PM +0300, Andy Shevchenko wrote:
> > > > On Wed, Apr 29, 2020 at 04:39:47AM +0530, Syed Nayyar Waris wrote:
> >
> > ...
> >
> > > > > +       const unsigned long state_size = BITS_PER_TYPE(*state);
> > > >
> > > > This '*state' is unneeded complication, use BITS_PER_U32.
> > > >
> > > > > +#define TOTAL_BITS BITS_PER_TYPE(chip->gpio_state)
> > > >
> > > > This macro makes code uglier, besides the fact of absence of #undef.
> > > > And also see above.
> > >
> > > Thank you for your review comments. Just want to clarify, you want
> > >  a new macro to be created - 'BITS_PER_U32' ?
> >
> > It's already there (read bits.h).
>
> I'm having trouble finding the BITS_PER_U32 macro; are you thinking of
> BITS_PER_LONG?

Oh, my bad. I messed above with BITS_TO_U32() which is not what we want here.

> I don't think there are any cases where u32 is not 32
> bits wide, so perhaps it'll be better to just hardcode 32 directly in
> the code here to make it easier to read.

Yes, would work!

-- 
With Best Regards,
Andy Shevchenko