Messages in this thread | | | Date | Thu, 30 Apr 2020 19:38:55 +0300 | From | Andy Shevchenko <> | Subject | Re: [PATCH v3 4/4] gpio: xilinx: Utilize for_each_set_clump macro |
| |
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).
> Also, don't you think that with BITS_PER_TYPE(), in case later the type > of 'state' changes, it will be reflected in this code without any code > change?
If it changes the bits per type will be least issues there. The rationale behind is to have code readable. In proposed change it is not.
> Let me know if I have misunderstood something. > > > > > > + DECLARE_BITMAP(old, TOTAL_BITS); > > > + DECLARE_BITMAP(new, TOTAL_BITS); > > > + DECLARE_BITMAP(changed, TOTAL_BITS);
-- With Best Regards, Andy Shevchenko
| |