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@xxxxxxxxxx> 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.
Attachment:
signature.asc
Description: PGP signature
