[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20201106121715.GA49612@sirena.org.uk>
Date: Fri, 6 Nov 2020 12:17:15 +0000
From: Mark Brown <broonie@...nel.org>
To: Bartosz Golaszewski <brgl@...ev.pl>
Cc: Linus Walleij <linus.walleij@...aro.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Jan Kiszka <jan.kiszka@...mens.com>,
David Laight <David.Laight@...lab.com>,
"open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Bartosz Golaszewski <bgolaszewski@...libre.com>
Subject: Re: [RFT PATCH v2 7/8] gpio: exar: switch to using regmap
On Fri, Nov 06, 2020 at 12:13:55PM +0100, Bartosz Golaszewski wrote:
> On Thu, Nov 5, 2020 at 6:41 PM Mark Brown <broonie@...nel.org> wrote:
> > AFAICT (and indeed now I dig around assign_bit() only works on a single
> > bit and does both shifts which makes the correspondance with that
> > interface super unclear, we're not mirroring that interface here). If
> > you're trying to clone the bitops function it should probably be an
> > actual clone of the bitops function not something different, that would
> > be clearer and it'd be easier to understand why someone would want the
> > API in the first place. But perhaps I'm missing something here?
> It's true that bitops set/clear/assign bit macros work on single bits
> and take their offsets as arguments. However all regmap helpers
> operate on masks. Two release cycles back we added two helpers
> regmap_set_bits() and regmap_clear_bits() which are just wrappers
> around regmap_update_bits(). The naming was inspired by bitops
> (because how would one name these operations differently anyway?) but
> it was supposed to be able to clear/set multiple bits at once - at
> least this was my use-case in mtk-star-emac driver I was writing at
> the time and for which I wrote these helpers.
Which is fine and not at all unclear since there's no separate value
argument, the value comes along with the name.
> Now the regmap_assign_bits() helper is just an extension to these two
> which allows users to use one line instead of four. I'm not trying to
> clone bitops - it's just that I don't have a better idea for the
> naming.
I really don't see the benefit to the helper, it makes sense in the
context of bitops where the operation does all the shifting and it's
only a single bit but for regmap where it's dealing with bitmasks as
well and the naming doesn't make it crystal clear I can only see this
being confusing to people. Had the set and clear helpers for regmap
been done as single bits it'd be a lot easier but that's not the case
and it'd also be odd to have just this one helper that took a shift
rather than a bitmask.
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists